diff --git a/MagicMock/mock._session_db.db_path/126402682339024 b/MagicMock/mock._session_db.db_path/126402682339024 new file mode 100644 index 0000000000..2cc4d1be35 Binary files /dev/null and b/MagicMock/mock._session_db.db_path/126402682339024 differ diff --git a/MagicMock/mock._session_db.db_path/126402682339024.fts_rebuild.lock b/MagicMock/mock._session_db.db_path/126402682339024.fts_rebuild.lock new file mode 100644 index 0000000000..e69de29bb2 diff --git a/MagicMock/mock._session_db.db_path/126402682339024.quarantine.lock b/MagicMock/mock._session_db.db_path/126402682339024.quarantine.lock new file mode 100644 index 0000000000..e69de29bb2 diff --git a/MagicMock/mock._session_db.db_path/126402702293264 b/MagicMock/mock._session_db.db_path/126402702293264 new file mode 100644 index 0000000000..e93b5fe5a9 Binary files /dev/null and b/MagicMock/mock._session_db.db_path/126402702293264 differ diff --git a/MagicMock/mock._session_db.db_path/126402702293264.fts_rebuild.lock b/MagicMock/mock._session_db.db_path/126402702293264.fts_rebuild.lock new file mode 100644 index 0000000000..e69de29bb2 diff --git a/MagicMock/mock._session_db.db_path/126402702293264.quarantine.lock b/MagicMock/mock._session_db.db_path/126402702293264.quarantine.lock new file mode 100644 index 0000000000..e69de29bb2 diff --git a/agent/vault_login_classifier.py b/agent/vault_login_classifier.py index 690a8175ad..684f113b45 100644 --- a/agent/vault_login_classifier.py +++ b/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") || "", diff --git a/agent/vault_store.py b/agent/vault_store.py index f69db81ff0..5eb3d934d9 100644 --- a/agent/vault_store.py +++ b/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) diff --git a/apps/desktop/src/store/prompts.ts b/apps/desktop/src/store/prompts.ts index 120d5eed39..d170d0c35a 100644 --- a/apps/desktop/src/store/prompts.ts +++ b/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) ) diff --git a/tests/test_browser_vault.py b/tests/test_browser_vault.py index 7dfc4066db..4487162e9d 100644 --- a/tests/test_browser_vault.py +++ b/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