fix(vault): 2FA review follow-ups — per-digit fill only for an unmistakable maxlength=1 widget; honour otpauth digits/period/algorithm

Reviewer findings on #107585:
- build_otp_fills split any >=4 code-like controls into digits. A page with
  promo/zip/referral 'code' inputs next to the real OTP box would have had a
  digit sprayed across unrelated fields. Split now requires exactly len(code)
  controls that are all maxlength=1, same form, adjacent in DOM order
  (inspection JS exports maxLength); anything else fills ONE field, the
  best-scoring one. Verified on the real Browser Use stack: 6-box widget gets
  one digit each; scattered page fills only the one-time-code input.
- normalize_otp_secret dropped digits/period/algorithm from otpauth:// URIs,
  so an 8-digit or SHA-256 authenticator would get wrong codes. Non-default
  parameters are now stored as seed|digits|period|algo and honoured (RFC 6238
  SHA-256 8-digit vector added); hotp:// is rejected explicitly.
- rebase on main (prompts.ts conflict) + prettier.

agent/vault_login_classifier.py

@@ -72,10 +72,12 @@ class LoginControl:
     label: str
     name: str
     type: str
+    max_length: Optional[int] = None
 
     @classmethod
     def from_dict(cls, raw: Dict[str, Any]) -> "LoginControl":
         form_index = raw.get("formIndex", raw.get("form_index"))
+        max_length = raw.get("maxLength", raw.get("max_length"))
         return cls(
             autocomplete=str(raw.get("autocomplete") or ""),
             form_index=int(form_index) if form_index is not None else None,
@@ -83,6 +85,7 @@ class LoginControl:
             label=str(raw.get("label") or ""),
             name=str(raw.get("name") or ""),
             type=str(raw.get("type") or ""),
+            max_length=int(max_length) if max_length is not None else None,
         )
 
 
@@ -223,12 +226,18 @@ INSPECTION_STAMP_ATTR = "data-hermes-vault-slot"
 
 
 def build_otp_fills(otp_controls: List[ClassifiedLoginControl], code: str) -> List[Dict[str, Any]]:
-    """One fill per box: a single input takes the whole code; N single-char boxes (maxlength=1 pattern,
-    detected as N>=4 same-form OTP controls) each take one digit in DOM order."""
-    boxes = sorted(otp_controls, key=lambda c: c.control.index)
-    if len(boxes) >= 4 and len(boxes) <= len(code):
+    """One fill per box. Default: the single best-scoring code field takes the whole code.
+
+    Per-digit entry only when the page unmistakably uses it: exactly len(code) OTP controls that are all
+    ``maxlength=1``, all in the same form, and adjacent in DOM order (the classic N-box widget). Anything
+    looser (several code-like inputs scattered over a page) gets ONE field, never a digit sprayed across
+    unrelated inputs."""
+    best = max(otp_controls, key=lambda c: c.score)
+    boxes = sorted((c for c in otp_controls if c.control.max_length == 1), key=lambda c: c.control.index)
+    if (len(boxes) == len(code)
+            and len({b.control.form_index for b in boxes}) == 1
+            and all(b.control.index - a.control.index == 1 for a, b in zip(boxes, boxes[1:]))):
         return [{"index": b.control.index, "token": "one-time-code", "value": ch} for b, ch in zip(boxes, code)]
-    best = max(boxes, key=lambda c: c.score)
     return [{"index": best.control.index, "token": "one-time-code", "value": code}]
 
 
@@ -256,6 +265,7 @@ _LOGIN_CONTROL_INSPECTION_JS_TEMPLATE = """(() => {
       autocomplete: element.autocomplete || "",
       formIndex: resolvedFormIndex >= 0 ? resolvedFormIndex : null,
       index,
+      maxLength: element.maxLength > 0 ? element.maxLength : null,
       label: [
         ...labels,
         element.getAttribute("aria-label") || "",

agent/vault_store.py

@@ -67,34 +67,57 @@ class VaultError(Exception):
     """Vault failure that is safe to surface (never contains secret values)."""
 
 
+_OTP_ALGOS = {"SHA1": "sha1", "SHA256": "sha256", "SHA512": "sha512"}
+
+
 def normalize_otp_secret(value: str) -> str:
-    """Accept a raw base32 seed or an ``otpauth://totp/...?secret=...`` URI; return the bare base32 seed
-    (uppercase, no spaces) or "" when empty/unusable. Only the seed is stored; issuer/digits/period use
-    RFC 6238 defaults, which every mainstream site uses."""
+    """Accept a raw base32 seed or an ``otpauth://totp/...`` URI. Returns the canonical stored form:
+    the bare uppercase base32 seed, followed by ``|digits|period|algo`` ONLY when the URI departs from
+    the RFC 6238 defaults (6 / 30 / SHA1), so a plain seed stays a plain seed. Non-default parameters
+    are honoured, not dropped: an 8-digit or 60-second authenticator would otherwise get wrong codes."""
     value = (value or "").strip()
     if not value:
         return ""
+    digits, period, algo = 6, 30, "SHA1"
     if value.lower().startswith("otpauth://"):
         from urllib.parse import parse_qs, urlparse
-        qs = parse_qs(urlparse(value).query)
-        value = (qs.get("secret") or [""])[0]
+        parsed = urlparse(value)
+        if parsed.netloc.lower() != "totp":
+            raise VaultError("only otpauth://totp links are supported (counter-based HOTP is not)")
+        qs = {k.lower(): v[0] for k, v in parse_qs(parsed.query).items()}
+        value = qs.get("secret", "")
+        try:
+            digits = int(qs.get("digits", digits))
+            period = int(qs.get("period", period))
+        except ValueError:
+            raise VaultError("otpauth:// digits/period must be integers")
+        algo = qs.get("algorithm", algo).upper().replace("-", "")
+        if digits not in (6, 7, 8) or period <= 0 or algo not in _OTP_ALGOS:
+            raise VaultError("unsupported otpauth:// parameters (digits 6-8, period > 0, SHA1/SHA256/SHA512)")
     seed = re.sub(r"[\s-]", "", value).upper().rstrip("=")
     if not seed or re.search(r"[^A-Z2-7]", seed):
         raise VaultError("authenticator key must be a base32 secret or an otpauth:// URI")
-    return seed
+    if (digits, period, algo) == (6, 30, "SHA1"):
+        return seed
+    return f"{seed}|{digits}|{period}|{algo}"
 
 
 def totp_now(seed: str, *, digits: int = 6, period: int = 30, at: Optional[float] = None) -> str:
-    """RFC 6238 TOTP (SHA-1) for a base32 seed. Stdlib only: no dependency for six digits."""
+    """RFC 6238 TOTP for a stored seed (see normalize_otp_secret for the ``seed|digits|period|algo``
+    form). Stdlib only."""
     import base64
     import hashlib
     import hmac
     import struct
     import time as _time
 
+    algo = "sha1"
+    if "|" in seed:
+        seed, d, p, a = seed.split("|", 3)
+        digits, period, algo = int(d), int(p), _OTP_ALGOS.get(a.upper(), "sha1")
     key = base64.b32decode(seed + "=" * (-len(seed) % 8), casefold=True)
     counter = int((at if at is not None else _time.time()) // period)
-    digest = hmac.new(key, struct.pack(">Q", counter), hashlib.sha1).digest()
+    digest = hmac.new(key, struct.pack(">Q", counter), getattr(hashlib, algo)).digest()
     offset = digest[-1] & 0x0F
     code = (struct.unpack(">I", digest[offset:offset + 4])[0] & 0x7FFFFFFF) % (10 ** digits)
     return str(code).zfill(digits)

apps/desktop/src/store/prompts.ts

@@ -272,7 +272,15 @@ export const sessionVaultCodeRequest = (sessionId: string | null) =>
 // suppress "thinking" indicators and the Esc-to-interrupt shortcut while you
 // decide, instead of treating the wait as an in-flight turn.
 export const $activeSessionAwaitingInput = computed(
-  [$clarifyRequest, $approvalRequest, $sudoRequest, $secretRequest, $vaultUnlockRequest, $vaultSaveLoginRequest, $vaultCodeRequest],
+  [
+    $clarifyRequest,
+    $approvalRequest,
+    $sudoRequest,
+    $secretRequest,
+    $vaultUnlockRequest,
+    $vaultSaveLoginRequest,
+    $vaultCodeRequest
+  ],
   (clarify, approval, sudo, secret, vault, save, code) =>
     Boolean(clarify || approval || sudo || secret || vault || save || code)
 )

tests/test_browser_vault.py

@@ -692,6 +692,14 @@ class TestTwoFactor:
         assert normalize_otp_secret("otpauth://totp/GitHub:tek?secret=jbsw y3dp ehpk3pxp&issuer=GitHub") == "JBSWY3DPEHPK3PXP"
         with pytest.raises(VaultError):
             normalize_otp_secret("not base32!")
+        # Non-default otpauth parameters are kept and honoured (RFC 6238 SHA-256 / 8-digit vector at T=59).
+        stored = normalize_otp_secret(f"otpauth://totp/x?secret={'GEZDGNBVGY3TQOJQ' * 2}&digits=8&period=30&algorithm=SHA256")
+        assert stored.endswith("|8|30|SHA256")
+        sha256_seed = "GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQGEZA"  # "1234567890" * 3.2 -> RFC 32-byte seed
+        assert totp_now(sha256_seed + "|8|30|SHA256", at=59) == "46119246"
+        assert totp_now("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ|6|60|SHA1", at=119) == totp_now("GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ", period=60, at=119)
+        with pytest.raises(VaultError):
+            normalize_otp_secret("otpauth://hotp/x?secret=JBSWY3DPEHPK3PXP&counter=1")
 
     def test_saved_authenticator_key_mints_codes_without_asking(self, store, monkeypatch):
         """The whole point: with a seed on the login, enter_code never prompts and the code never comes back."""
@@ -729,7 +737,8 @@ class TestTwoFactor:
         from tools import browser_vault_tool
 
         unlock_mod.set_code_prompt_callback(lambda site, hint: "246 810")
-        boxes = [{"index": i, "type": "tel", "name": f"digit{i}", "label": "", "autocomplete": "one-time-code"} for i in range(6)]
+        boxes = [{"index": i, "type": "tel", "name": f"digit{i}", "label": "", "autocomplete": "one-time-code",
+                  "formIndex": 0, "maxLength": 1} for i in range(6)]
         seen = {}
         fake_eval = lambda t, e: {"success": True, "result": json.dumps(boxes) if "querySelectorAll" in e else "https://acme.test/2fa"}
 
@@ -749,6 +758,24 @@ class TestTwoFactor:
         assert re.findall(r'"value": "(\d)"', seen["expr"]) == list("246810")
         assert declined["error_type"] == "code_declined"
 
+    def test_several_code_like_inputs_that_are_not_a_digit_widget_get_one_field(self):
+        """Reviewer case: a page with 4+ code-ish inputs (promo code, zip code, a real OTP box...) must never
+        get a digit sprayed across them. Only an unmistakable maxlength=1 same-form adjacent group splits."""
+        from agent.vault_login_classifier import ClassifiedLoginControl, LoginControl, build_otp_fills
+
+        def ctl(i, form=0, maxlen=None, score=70):
+            return ClassifiedLoginControl(LoginControl("", form, i, "", f"code{i}", "text", maxlen), score, "one-time-code")
+
+        scattered = [ctl(0), ctl(3), ctl(7), ctl(9, form=1), ctl(12, score=100)]
+        assert build_otp_fills(scattered, "246810") == [{"index": 12, "token": "one-time-code", "value": "246810"}]
+        # maxlength=1 but different forms / non-adjacent: still one field
+        assert len(build_otp_fills([ctl(i, form=i % 2, maxlen=1) for i in range(6)], "246810")) == 1
+        assert len(build_otp_fills([ctl(i * 2, maxlen=1) for i in range(6)], "246810")) == 1
+        # five boxes for a six-digit code: one field
+        assert len(build_otp_fills([ctl(i, maxlen=1) for i in range(5)], "246810")) == 1
+        # the real widget
+        assert [f["value"] for f in build_otp_fills([ctl(i + 4, maxlen=1) for i in range(6)], "246810")] == list("246810")
+
     def test_no_code_field_points_at_passkey_or_device_approval(self):
         from tools import browser_vault_tool