Aug 26 - Update password detect against off field

.gitea/workflows/ci.yml

@@ -146,6 +146,11 @@ jobs:
         # credentials offered on an attacker's neighbouring subdomain.
         run: node tests/js/test_psl.js
 
+      - name: Field heuristic tests
+        # Guards login-field detection. A wrong answer here means autofill
+        # silently does nothing on real login pages.
+        run: node tests/js/test_field_heuristics.js
+
   # ── Test suite ───────────────────────────────────────────────────────────────
   # Runs against in-memory SQLite (see app/config.py TestingConfig) so no MySQL
   # service is needed on the host-mode runner. That means these tests cover

CLAUDE.md

@@ -121,7 +121,9 @@ passkeeper/
 │   ├── test_registration_privacy.py # register does not disclose account existence
 │   ├── test_emergency_visibility.py # grantor sees requests + retrievals
 │   ├── test_deploy_config.py        # nginx/gunicorn/systemd/extension packaging guards
-│   └── js/test_psl.js               # PSL same-site matching (node, run in CI)
+│   └── js/
+│       ├── test_psl.js              # PSL same-site matching (node, run in CI)
+│       └── test_field_heuristics.js # login-field detection predicates
 ├── gunicorn.conf.py         # worker class, timeouts, preload_app=False
 ├── pytest.ini
 ├── requirements-dev.txt
@@ -618,7 +620,32 @@ In both `content.js` and `popup.js`. Prevents silent match failures for bare dom
 1. **YES:** `autocomplete="username|email|tel"`
 2. **NO:** non-credential autocomplete (`name`, `organization`, `search`, etc.)
 3. **YES:** `name/id/placeholder/aria-label` matches `user|email|mail|login|phone|tel|mobile|account`
-4. **Otherwise:** not decorated
+4. **Then:** `_hasPasswordSibling()` must also pass
+5. **Otherwise:** not decorated
+
+**`autocomplete="off"` is NOT a negative signal** and must never be added back to
+`NON_CRED_AC`. Routers, banks and admin panels set it on login fields precisely
+to discourage password managers. It previously caused step 2 to reject fields as
+obvious as `<input id="login_username" placeholder="Username" autocomplete="off">`
+before step 3 ever ran (ASUS RT-AX88U admin login). Letting it fall through is
+safe — the field still needs a credential keyword AND a nearby password input.
+
+### Credential capture without a `<form>`
+
+Many login UIs never use a `<form>` — the ASUS router admin page submits with
+`<div class="button" onclick="preLogin();">Sign In</div>`, so no `submit` event
+is ever dispatched and the save-credentials banner never appeared.
+
+`watchSubmissions()` therefore registers three triggers, all routed through
+`maybeCaptureCredentials(scope)`:
+
+1. `submit` on any form (scope = the form)
+2. `click` on anything `_looksLikeSubmitControl()` accepts — `<button>`,
+   `input[type=submit|button|image]`, `[role=button]`, any element with an
+   inline `onclick`, or a button-ish class name (scope = document)
+3. `Enter` keydown inside a password or likely-username field (scope = document)
+
+`_captureCooldown` (2 s) prevents two triggers double-prompting for one login.
 
 ### MutationObserver guard
 
@@ -663,6 +690,8 @@ Audit log details **never** contain plaintext item names, shared item names, or
 - `/register` must return the SAME body and status for new and existing addresses, and hash on both paths — returning early on duplicate reinstates a timing oracle
 - Never return `str(e)` from exception handlers — log with `_log.exception(...)` and return a generic user-facing message to avoid leaking DB schema details or query fragments
 - `extension/shared/psl.js` is GENERATED — never hand-edit; run `python scripts/update_psl.py`. It must load BEFORE content.js / popup.js / background.js in every manifest
+- Never add `off` back to `NON_CRED_AC` in `content.js` — `tests/js/test_field_heuristics.js` fails the build if you do
+- Login detection must not assume a `<form>` exists; route new capture triggers through `maybeCaptureCredentials()`
 - Never reintroduce `endsWith("." + host)` host matching anywhere in the extension — `tests/test_deploy_config.py` fails the build if it reappears
 - nginx rate zones: mind `r/s` vs `r/m`. `api_limit` was `60r/m` (1 req/s for the whole API) and caused spurious 429s on normal vault use
 - `preload_app` must stay `False` in `gunicorn.conf.py` — APScheduler's thread does not survive `fork()`, so `--preload` silently disables the cleanup job

extension/content/content.js

@@ -99,8 +99,21 @@
     if (["username", "email", "tel"].includes(ac)) return true;
 
     // Definite negative signals (Chrome's autocomplete token set).
+    //
+    // "off" is deliberately NOT in this list. It says nothing about whether a
+    // field holds a credential — routers, banks and admin panels set it on
+    // login inputs precisely to discourage password managers, and every major
+    // password manager (and Chrome itself, for password fields) ignores it.
+    // Treating it as a negative signal meant a field as obvious as
+    //   <input type="text" id="login_username" placeholder="Username"
+    //          autocomplete="off">
+    // was rejected before the keyword check below ever ran.
+    //
+    // Letting "off" fall through is safe: the field still has to carry a
+    // credential keyword AND sit near a password input (_hasPasswordSibling)
+    // before it is decorated.
     const NON_CRED_AC =
-      /^(name|given-name|family-name|additional-name|honorific-prefix|honorific-suffix|organization|street-address|address-line[123]|address-level[1234]|country|country-name|postal-code|cc-|transaction-|language|bday|sex|url|photo|search|new-password|current-password|one-time-code|off)$/i;
+      /^(name|given-name|family-name|additional-name|honorific-prefix|honorific-suffix|organization|street-address|address-line[123]|address-level[1234]|country|country-name|postal-code|cc-|transaction-|language|bday|sex|url|photo|search|new-password|current-password|one-time-code)$/i;
     if (ac && NON_CRED_AC.test(ac)) return false;
 
     // Check name, id, placeholder, and aria-label for credential keywords.
@@ -1134,28 +1147,44 @@
       });
   }
 
-  // ── Form submission watch ─────────────────────────────────────────────────────
+  // ── Credential capture / save prompt ─────────────────────────────
 
-  function watchSubmissions() {
-    document.addEventListener(
-      "submit",
-      async function (e) {
-        var form = e.target;
-        var pwField = form.querySelector(
-          'input[type="password"]:not([disabled])',
+  // Guards against two triggers firing for the same login (e.g. a click handler
+  // that also submits a form). Cleared after a short window.
+  var _captureCooldown = false;
+
+  /**
+   * Collect the credentials currently entered within `scope` and offer to save
+   * them. `scope` is the <form> for a real submit, or the document for the
+   * fallback triggers below.
+   */
+  async function maybeCaptureCredentials(scope) {
+    if (_captureCooldown) return;
+
+    var root = scope || document;
+    var pwField = Array.prototype.find.call(
+      root.querySelectorAll('input[type="password"]:not([disabled])'),
+      function (el) {
+        return isVisible(el) && el.value;
+      },
     );
-        if (!pwField || !pwField.value) return;
+    if (!pwField) return;
 
     var userField =
       findUsernameField(pwField) ||
-          form.querySelector('input[type="email"]:not([disabled])') ||
-          form.querySelector('input[type="text"]:not([disabled])');
+      root.querySelector('input[type="email"]:not([disabled])') ||
+      root.querySelector('input[type="text"]:not([disabled])');
 
     var username =
       (userField && userField.value && userField.value.trim()) || "";
     var password = pwField.value;
     if (!username || !password) return;
 
+    _captureCooldown = true;
+    setTimeout(function () {
+      _captureCooldown = false;
+    }, 2000);
+
     removeDropdown();
 
     // Check blocklist before doing anything else.
@@ -1171,7 +1200,7 @@
     console.log(
       "[PassKeeper] Credential state for",
       location.hostname,
-          "\u2192",
+      "→",
       credentialState,
     );
     if (credentialState === "same") return;
@@ -1179,6 +1208,71 @@
     setTimeout(function () {
       showSaveBanner(username, password, credentialState);
     }, 500);
+  }
+
+  /**
+   * Does this element look like the control that submits a login?
+   *
+   * Needed because many login UIs never use a <form> at all — the ASUS router
+   * admin page, for example, submits with
+   *   <div class="button" onclick="preLogin();">Sign In</div>
+   * so no "submit" event is ever dispatched and the save prompt never appeared.
+   */
+  function _looksLikeSubmitControl(el) {
+    if (!el || !el.tagName) return false;
+    var tag = el.tagName.toUpperCase();
+    if (tag === "BUTTON") return true;
+    if (tag === "INPUT" && /^(submit|button|image)$/i.test(el.type)) return true;
+    if (el.getAttribute("role") === "button") return true;
+    // Non-semantic controls: an inline click handler, or a button-ish class.
+    if (el.hasAttribute("onclick")) return true;
+    var cls = (el.getAttribute("class") || "").toLowerCase();
+    return /(^|[\s_-])(btn|button|submit|login|signin|sign-in)([\s_-]|$)/.test(cls);
+  }
+
+  function watchSubmissions() {
+    // 1. Real form submits.
+    document.addEventListener(
+      "submit",
+      function (e) {
+        maybeCaptureCredentials(e.target);
+      },
+      true,
+    );
+
+    // 2. Clicks on anything that looks like a submit control. Required for the
+    //    common case of a login UI built without a <form>, where no submit
+    //    event fires and the save prompt would otherwise never appear.
+    document.addEventListener(
+      "click",
+      function (e) {
+        var node = e.target;
+        for (var i = 0; i < 5 && node && node !== document; i++) {
+          if (_looksLikeSubmitControl(node)) {
+            // Let the page's own handler run first.
+            setTimeout(function () {
+              maybeCaptureCredentials(document);
+            }, 0);
+            return;
+          }
+          node = node.parentElement;
+        }
+      },
+      true,
+    );
+
+    // 3. Enter pressed inside a credential field — the other way such forms
+    //    get submitted without a <form>.
+    document.addEventListener(
+      "keydown",
+      function (e) {
+        if (e.key !== "Enter") return;
+        var el = e.target;
+        if (!el || el.tagName !== "INPUT") return;
+        if (el.type !== "password" && !_isLikelyUsernameField(el)) return;
+        setTimeout(function () {
+          maybeCaptureCredentials(document);
+        }, 0);
       },
       true,
     );

tests/js/test_field_heuristics.js

@@ -0,0 +1,119 @@
+/**
+ * tests/js/test_field_heuristics.js — run with:
+ *   node tests/js/test_field_heuristics.js
+ *
+ * Pins the field-detection predicates in extension/content/content.js against
+ * the ASUS RT-AX88U router login page, which the extension could not detect.
+ *
+ * The username input there is about as obvious as they come:
+ *
+ *   <input type="text" id="login_username" name="login_username"
+ *          class="form_input" placeholder="Username" autocomplete="off">
+ *
+ * but "off" was in the NON_CRED_AC deny-list, so _isLikelyUsernameField()
+ * returned false at that check and never reached the name/id/placeholder
+ * keyword test. `autocomplete="off"` says nothing about whether a field holds a
+ * credential — routers and admin panels set it specifically to discourage
+ * password managers.
+ *
+ * SCOPE: this exercises the regex predicates extracted from the real source
+ * file, not the DOM traversal in _hasPasswordSibling() (which needs a browser).
+ * Those paths are covered by the manual check in the accompanying notes.
+ */
+'use strict';
+
+const fs = require('fs');
+const path = require('path');
+
+const SRC = fs.readFileSync(
+  path.join(__dirname, '..', '..', 'extension', 'content', 'content.js'),
+  'utf8',
+);
+
+let failures = 0;
+
+function check(label, got, want) {
+  if (got !== want) {
+    failures++;
+    console.log(`FAIL  ${label}\n      got=${got} want=${want}`);
+  }
+}
+
+/** Pull a named regex literal out of the real source rather than copying it. */
+function extractRegex(name) {
+  const m = SRC.match(new RegExp(`const ${name} =\\s*(/[^\\n]+/[a-z]*);`));
+  if (!m) throw new Error(`could not find regex ${name} in content.js`);
+  // eslint-disable-next-line no-eval
+  return eval(m[1]);
+}
+
+const CRED_HINTS = extractRegex('CRED_HINTS');
+const NON_CRED_AC = extractRegex('NON_CRED_AC');
+
+// ── The bug: autocomplete="off" must not be a negative signal ───────────────
+check('NON_CRED_AC rejects "off"', NON_CRED_AC.test('off'), false);
+check('NON_CRED_AC rejects "OFF" (case-insensitive)', NON_CRED_AC.test('OFF'), false);
+
+// Anti-autofill junk values sites use must also fall through to the keywords.
+for (const junk of ['nope', 'no', 'false', 'disabled', 'new-user', 'chrome-off']) {
+  check(`NON_CRED_AC lets junk value "${junk}" fall through`,
+        NON_CRED_AC.test(junk), false);
+}
+
+// ── But genuine non-credential tokens must still be rejected ───────────────
+for (const token of ['name', 'given-name', 'family-name', 'organization',
+                     'street-address', 'country', 'postal-code', 'search',
+                     'url', 'bday', 'sex', 'photo', 'language',
+                     'new-password', 'current-password', 'one-time-code']) {
+  check(`NON_CRED_AC still rejects "${token}"`, NON_CRED_AC.test(token), true);
+}
+
+// ── The ASUS field's identifying attributes must read as a credential ───────
+const ASUS_USERNAME_ATTRS = ['login_username', 'login_username', 'Username', ''].join(' ');
+check('ASUS username attrs match CRED_HINTS',
+      CRED_HINTS.test(ASUS_USERNAME_ATTRS), true);
+
+// Other real-world username fields.
+for (const attrs of ['user_name', 'j_username', 'email', 'userEmail',
+                     'account', 'Login ID', 'mobile', 'tel']) {
+  check(`CRED_HINTS matches "${attrs}"`, CRED_HINTS.test(attrs), true);
+}
+
+// Fields that must NOT be treated as credential inputs on keywords alone.
+for (const attrs of ['q', 'search-query', 'first_name', 'zipcode',
+                     'street', 'company', 'comment']) {
+  check(`CRED_HINTS does not match "${attrs}"`, CRED_HINTS.test(attrs), false);
+}
+
+// ── Submit-control detection for pages with no <form> ──────────────────────
+// The ASUS page submits via <div class="button" onclick="preLogin();">, so no
+// "submit" event ever fires and the save prompt never appeared.
+const clsMatch = SRC.match(
+  /return (\/\(\^\|\[\\s_-\]\)\(btn\|button[^\n]+\/)\.test\(cls\);/,
+);
+if (!clsMatch) {
+  failures++;
+  console.log('FAIL  could not find the submit-control class regex in content.js');
+} else {
+  // eslint-disable-next-line no-eval
+  const BTN_CLS = eval(clsMatch[1]);
+  check('recognises class="button" (ASUS)', BTN_CLS.test('button'), true);
+  for (const cls of ['btn', 'btn btn-primary', 'submit', 'login-button',
+                     'signin', 'sign-in', 'form_btn button']) {
+    check(`recognises class="${cls}"`, BTN_CLS.test(cls), true);
+  }
+  for (const cls of ['form_input', 'container', 'buttonish', 'rebutton']) {
+    check(`ignores class="${cls}"`, BTN_CLS.test(cls), false);
+  }
+}
+
+// The click and keydown fallbacks must actually be registered.
+check('click fallback registered', /addEventListener\(\s*"click"/.test(SRC), true);
+check('keydown fallback registered', /addEventListener\(\s*"keydown"/.test(SRC), true);
+check('submit listener retained', /addEventListener\(\s*"submit"/.test(SRC), true);
+
+if (failures) {
+  console.log(`\n${failures} failure(s)`);
+  process.exit(1);
+}
+console.log('OK: field heuristics assertions passed');