fix(extension): do not fill a 2FA code into a form with empty credentials

extension/entrypoints/content.ts

@@ -30,6 +30,7 @@ import {
   type ClassifiedValue,
 } from "../utils/form-fields";
 import { isOneTimeCodeText } from "../utils/otp-field";
+import { shouldAutofillOtp } from "../utils/otp-autofill";
 import { filterCredentials } from "../utils/credential-filter";
 
 interface CredMatch {
@@ -617,20 +618,39 @@ export default defineContentScript({
     }
 
     // The 2FA step usually renders on its own page (or SPA view) with no password
-    // field, so the credential autofill never runs there. Fill the code under the
-    // same conditions autofill-on-load uses: the feature is on, the page is
-    // unambiguous (exactly one matching login carries a secret), and the user
-    // hasn't already typed into the field.
+    // field, so the credential autofill never runs there. Whether this pass may
+    // fire is decided by shouldAutofillOtp; this reads the page facts it needs.
     let didAutofillOtp = false;
 
+    // A visible login form whose password box is still empty is one the
+    // credential fill is about to handle (or the user is about to type into).
+    // That fill writes the username, password and code in one pass, so the
+    // standalone pass must not race it and drop a code into an otherwise empty
+    // form.
+    function hasPendingCredentialForm(): boolean {
+      return findVisibleLoginForms().some((form) => {
+        const { passwordInput } = extractCredentialInputs(form);
+        return passwordInput !== null && !passwordInput.value;
+      });
+    }
+
     function maybeAutofillOtp() {
-      if (didAutofillOtp || !settings.autofill || !isUnlocked) return;
-      const rows = totpMatches();
-      if (rows.length !== 1) return;
+      // Once-per-page latch, checked first so the steady state after a fill
+      // costs nothing on a busy page's mutation stream.
+      if (didAutofillOtp) return;
       const anchor = findOtpAnchor(document);
-      if (!anchor || !isUntouched(anchor)) return;
+      if (!anchor) return;
+      const mayFill = shouldAutofillOtp({
+        autofillEnabled: settings.autofill,
+        isUnlocked,
+        matchCount: matches.length,
+        matchHasTotp: Boolean(matches[0]?.hasTotp),
+        hasPendingCredentialForm: hasPendingCredentialForm(),
+        hasEmptyCodeField: isUntouched(anchor),
+      });
+      if (!mayFill) return;
       didAutofillOtp = true;
-      void fillTotpInto(anchor, rows[0]!);
+      void fillTotpInto(anchor, matches[0]!);
     }
 
     function clearOtpFieldIcons() {

extension/utils/otp-autofill.test.ts

@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+import { describe, it, expect } from "vitest";
+import { shouldAutofillOtp, type OtpAutofillState } from "./otp-autofill";
+
+// The 2FA step of a login that already submitted its password: one match, it
+// carries a secret, nothing else on the page is waiting to be filled.
+const secondStepPage: OtpAutofillState = {
+  autofillEnabled: true,
+  isUnlocked: true,
+  matchCount: 1,
+  matchHasTotp: true,
+  hasPendingCredentialForm: false,
+  hasEmptyCodeField: true,
+};
+
+function withState(overrides: Partial<OtpAutofillState>): OtpAutofillState {
+  return { ...secondStepPage, ...overrides };
+}
+
+describe("shouldAutofillOtp", () => {
+  it("fills the 2FA step of a page with nothing else to fill", () => {
+    expect(shouldAutofillOtp(secondStepPage)).toBe(true);
+  });
+
+  it("waits while a login form still has an empty password", () => {
+    // The regression this policy exists for: the code landing on its own while
+    // the username and password sit empty. That fill belongs to the credential
+    // pass, which writes all three together.
+    expect(
+      shouldAutofillOtp(withState({ hasPendingCredentialForm: true })),
+    ).toBe(false);
+  });
+
+  it("fills once the credential form is no longer waiting on a password", () => {
+    // Same page a moment later: the credential fill (or the user) has filled
+    // the password, and the site has just revealed its code field.
+    expect(
+      shouldAutofillOtp(withState({ hasPendingCredentialForm: false })),
+    ).toBe(true);
+  });
+
+  it("does nothing when autofill-on-load is off", () => {
+    // The emblem picker is the only way in; a code must never appear unasked.
+    expect(shouldAutofillOtp(withState({ autofillEnabled: false }))).toBe(false);
+  });
+
+  it("does nothing while the vault is locked", () => {
+    expect(shouldAutofillOtp(withState({ isUnlocked: false }))).toBe(false);
+  });
+
+  it("does not guess between several matching logins", () => {
+    // On a 2FA step there is no username on screen to disambiguate with, so a
+    // host with more than one saved login waits for an explicit pick - the same
+    // rule the credential autofill uses.
+    expect(shouldAutofillOtp(withState({ matchCount: 2 }))).toBe(false);
+  });
+
+  it("does nothing when the page has no matching login at all", () => {
+    expect(
+      shouldAutofillOtp(withState({ matchCount: 0, matchHasTotp: false })),
+    ).toBe(false);
+  });
+
+  it("does nothing when the single match carries no TOTP secret", () => {
+    expect(shouldAutofillOtp(withState({ matchHasTotp: false }))).toBe(false);
+  });
+
+  it("never overwrites a code field the user has already typed into", () => {
+    expect(shouldAutofillOtp(withState({ hasEmptyCodeField: false }))).toBe(
+      false,
+    );
+  });
+});

extension/utils/otp-autofill.ts

@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+/**
+ * Pure, DOM-free policy for the standalone 2FA-code autofill: the pass that
+ * fills a one-time-code field on a page where there is no credential fill to
+ * ride along with (the 2FA step of a login that already submitted its
+ * password). Kept side-effect-free so it is unit-testable in node: the content
+ * script reads the DOM and hands over the resulting facts.
+ *
+ * The hard part is NOT filling too eagerly. A code dropped into a page whose
+ * username and password are still empty is worse than no fill at all: it looks
+ * broken, it burns the user's attention on a code they can't submit yet, and on
+ * a form the credential fill was about to handle it arrives out of order. So a
+ * login form still waiting on its password takes precedence - the code belongs
+ * to THAT fill, which writes username, password and code in one pass.
+ */
+
+export interface OtpAutofillState {
+  // The "fill without a click" setting. Explicit picker clicks never consult
+  // this policy; they fill the code as part of the credential fill.
+  autofillEnabled: boolean;
+  isUnlocked: boolean;
+  // How many stored logins match this page, and whether the single match
+  // carries a TOTP secret. More than one match means we cannot tell which
+  // account is signing in, so the user picks instead of us guessing.
+  matchCount: number;
+  matchHasTotp: boolean;
+  // A visible login form whose password box is still empty. Its credential
+  // fill owns the code.
+  hasPendingCredentialForm: boolean;
+  hasEmptyCodeField: boolean;
+}
+
+export function shouldAutofillOtp(state: OtpAutofillState): boolean {
+  if (!state.autofillEnabled || !state.isUnlocked) return false;
+  if (state.matchCount !== 1 || !state.matchHasTotp) return false;
+  if (state.hasPendingCredentialForm) return false;
+  return state.hasEmptyCodeField;
+}