From 15f95b1dd6ea0d6d37de1eb9327a09d3b2d5809e Mon Sep 17 00:00:00 2001
From: Zohar Zilberman <popen2@gmail.com>
Date: Fri, 21 Aug 2026 09:00:47 +0000
Subject: [PATCH 1/2] Show the credential overlay when the page asks for the
 key as it loads
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The overlay was injected on `dom-ready`, which is after the page's own scripts
have run. A sign-in page that starts listening for the security key as it boots
— Google's 2SV challenge does exactly that — has already called
`navigator.credentials.get()` by then, so wrapping `navigator.credentials`
afterwards wrapped nothing: no toast, no window title, no dock bounce, no log
line. An AWS SSO login backed by Google showed the user the same silent,
apparently-hung window as before issue #17 was fixed.

Register the overlay through the WebContents debugger instead, with
`Page.addScriptToEvaluateOnNewDocument`, so it runs at document start in the
page's own world — before any script the page ships, and again on the
cross-origin hop from AWS to the identity provider, which moves the page to a
different renderer process. Three details all fail silently and are spelled out
where they live:

- `Page.enable` has to come first. Without it the registration resolves and
  does nothing.
- There has to be a renderer to talk to. On a window that has not loaded
  anything the command never resolves — not an error, a hung promise — so
  `attachLoginIndicator()` loads about:blank first, is now async, and must be
  awaited before `loadURL`. `getNewToken()` awaits it, and treats a window
  closed while it happens as the abort it is.
- It reaches the top-level document and any frame in its process, but not a
  cross-origin iframe, which gets its own CDP target. The `dom-ready` injection
  stays as the fallback for those, and for a debugger that could not attach.

`npm run check:overlay` is the regression test: it drives the real
`attachLoginIndicator()` on a real BrowserWindow against pages that ask for a
key before and after `dom-ready`, and asserts the wait reached the main
process. No security key needed — only the start of the request matters. It
fails on the code before this commit and passes after, and CI runs it under
xvfb, which the app turns out to run under perfectly well.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014s8Y8cKzE4bpyaxPMP19wi
---
 .github/workflows/ci.yaml    |  10 ++
 AGENTS.md                    |  55 +++++++++--
 package.json                 |   1 +
 src/aws-sso.ts               |  18 +++-
 src/login-indicator.ts       | 118 ++++++++++++++++++++++-
 tools/check-login-overlay.js | 179 +++++++++++++++++++++++++++++++++++
 6 files changed, 367 insertions(+), 14 deletions(-)
 create mode 100644 tools/check-login-overlay.js

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 4a68653..109f491 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -48,6 +48,16 @@ jobs:
         run: |
           npm run lint
 
+      # The login window's credential overlay fails silently when it breaks -
+      # no error, just a sign-in page that shows nothing while it waits for a
+      # security key. This drives a real BrowserWindow through a real WebAuthn
+      # wait and asserts the overlay reported it. It needs a display (xvfb) and
+      # --no-sandbox, because the runner's Electron has no setuid sandbox
+      # helper; it needs no security key.
+      - name: 🔑 Check the login overlay
+        run: |
+          xvfb-run -a npm run check:overlay -- --no-sandbox
+
   build:
     needs: lint
     uses: ./.github/workflows/build.yaml
diff --git a/AGENTS.md b/AGENTS.md
index eb4eb78..15e3996 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -69,9 +69,12 @@ npm only (`package-lock.json`; CI runs `npm ci`). Do not add a `yarn.lock`.
 
 - `npm run build` — `tsc`, then copies the tray icons and `dashboard.html`.
 - `npm run lint` — oxlint, configured by `.oxlintrc.json`.
+- `npm run check:overlay` — drives the login window's credential overlay
+  through a real WebAuthn wait. Needs `npm run build` first, and a display:
+  `xvfb-run -a npm run check:overlay -- --no-sandbox`.
 - `npm start` / `npm run package` / `npm run make` — Electron Forge.
 
-Build and lint both run in CI. Neither proves the app launches; see
+All three run in CI. Build and lint alone do not prove the app launches; see
 "Verification limits".
 
 ## ESM
@@ -90,10 +93,30 @@ for a security key looks broken. `src/login-indicator.ts` fills that gap, wired
 up before `loadURL` and only for the in-app window; the default-browser mode
 gets the browser's own prompts.
 
-- `src/login-overlay.ts` is compiled separately, read off disk, and injected on
-  every `dom-ready` — the main frame plus sub-frames reached through
-  `frame-created`, since injecting on the `WebContents` only reaches the top
-  frame. It re-injects safely; a `window` flag makes it a no-op.
+- `src/login-overlay.ts` is compiled separately, read off disk, and injected
+  into every document the login window loads. It wraps
+  `navigator.credentials.{get,create}` and draws a toast while a request is
+  pending. It re-injects safely; a `window` flag makes it a no-op.
+- **It has to be running before the page's own scripts are.** A page that asks
+  for the key as it boots — Google's security-key challenge does — has already
+  called `navigator.credentials.get()` by `dom-ready`, and a wrapper installed
+  after the call sees nothing: no toast, no title, no log line, just the silent
+  window the overlay exists to prevent. So `attachLoginIndicator()` registers
+  it through the `WebContents` debugger with
+  `Page.addScriptToEvaluateOnNewDocument`, which runs it at document start in
+  the page's own world. Three details there all fail silently:
+    - `Page.enable` first, or the registration resolves and does nothing.
+    - There has to be a renderer to talk to. On a window that has loaded
+      nothing the command never resolves — no error, a hung promise — which is
+      why `attachLoginIndicator()` loads `about:blank` first, is `async`, and
+      must be awaited before `loadURL`.
+    - It reaches the top document — across the cross-origin hop to the identity
+      provider, which changes renderer process — and frames sharing its
+      process, but not a cross-origin `<iframe>`, which has its own CDP target.
+  Injecting on `dom-ready`, the main frame plus sub-frames reached through
+  `frame-created` (injecting on the `WebContents` only reaches the top frame),
+  stays as the fallback for those and for a debugger that would not attach.
+  Being attached is also why the login window cannot open DevTools.
 - Its compile differs deliberately: `module: ESNext` (NodeNext would append an
   export statement, a syntax error in an injected classic script), `lib: DOM`
   with no Node types, and no source map. It must stay import-free — an import
@@ -114,6 +137,17 @@ gets the browser's own prompts.
   window was alive — reaching through the destroyed `WebContents` throws, which
   Electron shows as a modal dialog on close.
 
+`npm run check:overlay` is the regression test for all of that: it drives the
+real `attachLoginIndicator()` on a real `BrowserWindow` against pages that ask
+for a key before and after `dom-ready`, and asserts the wait reached the main
+process. No key needed — only the start of the request matters, and that is
+signalled the moment the page asks. CI runs it.
+
+The overlay's drawing can also be checked on its own, since it is import-free
+browser code: `new Function("window", source)` over the built
+`dist/login-overlay.js` with a stub `window`, or a Playwright page — the only
+way to try it against a strict CSP or hostile `!important` CSS.
+
 ## `~/.aws/config` ownership
 
 `src/aws-config.ts` merges into the user's file and must never rewrite it
@@ -408,8 +442,15 @@ cannot be validated locally.
 
 Headless, you can run build, lint, isolated Node checks of individual modules,
 and — with network access to the Electron downloads — `package`/`make` **for
-linux**. You **cannot** launch the app or validate macOS signing. Say so rather
-than claiming the app works.
+linux**.
+
+You *can* also launch it, given those same downloads and `xvfb`:
+`xvfb-run -a ./node_modules/electron/dist/electron --no-sandbox .` boots the
+whole app, and `npm run check:overlay` uses that to drive a real
+`BrowserWindow`. That is how the overlay's document-start bug was found; build
+and lint could not have. What it does **not** give you is a real desktop: no
+tray interaction, no dock, no security key, no keychain, no macOS signing. Say
+so rather than claiming the app works.
 
 Windows packaging cannot be exercised either: the maker needs a Windows host,
 and even plain packaging shells out to `rcedit`, which needs Wine off Windows.
diff --git a/package.json b/package.json
index 9bc1571..bf7d508 100644
--- a/package.json
+++ b/package.json
@@ -17,6 +17,7 @@
     "build": "tsc && npm run build:icons && npm run build:html && npm run build:overlay",
     "watch": "tsc -w",
     "lint": "oxlint src",
+    "check:overlay": "electron tools/check-login-overlay.js",
     "start": "electron-forge start",
     "package": "electron-forge package",
     "make": "electron-forge make"
diff --git a/src/aws-sso.ts b/src/aws-sso.ts
index 0fa1734..66bf062 100644
--- a/src/aws-sso.ts
+++ b/src/aws-sso.ts
@@ -349,10 +349,22 @@ async function getNewToken(
                 event.preventDefault()
             );
 
-            // Before loadURL, so the very first document gets the overlay that
-            // shows when the page is waiting for a security key or passkey. The
+            // Awaited, and before loadURL, so the very first document gets
+            // the overlay that shows when the page is waiting for a security
+            // key or passkey — a sign-in page that starts listening as it boots
+            // asks for the key before any later hook could wrap the call. The
             // default-browser path needs nothing: the browser has its own UI.
-            attachLoginIndicator(window);
+            await attachLoginIndicator(window);
+
+            // Arming the overlay is asynchronous, and the user can close the
+            // window while it happens. Nothing below survives a destroyed
+            // window, and this is the same "I'm not logging in now" the close
+            // handler reports.
+            if (window.isDestroyed()) {
+                throw new LoginAbortedError("Login window closed", {
+                    cancelledByUser: true,
+                });
+            }
 
             window.on("close", () => {
                 log.warn("[getNewToken] Login window closed");
diff --git a/src/login-indicator.ts b/src/login-indicator.ts
index 8cdf5de..7eeebc2 100644
--- a/src/login-indicator.ts
+++ b/src/login-indicator.ts
@@ -40,6 +40,95 @@ interface ScriptTarget {
 
 let overlaySource: string | null | undefined;
 
+/**
+ * Run the overlay at document start, before the page's own scripts.
+ *
+ * `dom-ready` is far too late for a page that starts listening for the key as
+ * it boots — Google's security-key challenge calls
+ * `navigator.credentials.get()` from its boot script, so by the time
+ * `dom-ready` fires the request is already pending and wrapping
+ * `navigator.credentials` no longer sees it. The page then sits there silently,
+ * which is exactly the symptom the overlay exists to remove (issue #17).
+ *
+ * `Page.addScriptToEvaluateOnNewDocument` is the only hook early enough: it
+ * runs our source in the page's own world before any script the page ships,
+ * and it keeps doing so across the cross-origin hop from AWS to the identity
+ * provider — which moves the page to a different renderer process. It needs the
+ * debugger, so the login window cannot open DevTools while it is attached — no
+ * loss for a window that only ever shows a sign-in page.
+ *
+ * It covers the top-level document and any frame sharing its process. A
+ * cross-origin <iframe> gets its own process and its own CDP target, which this
+ * registration does not reach; those are left to the `dom-ready` injection
+ * below, which is in time for anything but a request the frame makes as it
+ * boots.
+ *
+ * Two things about this are easy to get wrong, and silently:
+ *
+ *  - The `Page` domain has to be enabled first. Without it the registration
+ *    still resolves, and still does nothing at all.
+ *  - There has to be a renderer to talk to. On a window that has not loaded
+ *    anything yet the command never resolves — not an error, just a promise
+ *    that hangs — which is why the caller loads about:blank first.
+ *
+ * Best effort: when it cannot be armed the `dom-ready` injection below is still
+ * there, and still covers every page that asks for the key after it has loaded.
+ */
+async function injectAtDocumentStart(
+    contents: Electron.WebContents,
+    source: string
+): Promise<void> {
+    try {
+        if (!contents.debugger.isAttached()) {
+            contents.debugger.attach("1.3");
+        }
+        await contents.debugger.sendCommand("Page.enable");
+        await contents.debugger.sendCommand(
+            "Page.addScriptToEvaluateOnNewDocument",
+            {
+                source,
+                // The window is sitting on about:blank right now, and the login
+                // page is the *next* document, so this only covers about:blank
+                // itself. Harmless, and it keeps the hook honest if that ever
+                // stops being true.
+                runImmediately: true,
+            }
+        );
+        log.debug("[loginIndicator] Overlay armed at document start");
+    } catch (err) {
+        log.warn(
+            "[loginIndicator] Could not arm the overlay at document start, " +
+                "falling back to dom-ready: %s",
+            err
+        );
+    }
+}
+
+/**
+ * Give the window a renderer for the debugger to reach, without showing the
+ * user anything it would not have shown anyway: a new BrowserWindow is blank
+ * until the login page loads either way.
+ */
+async function loadBlank(contents: Electron.WebContents): Promise<boolean> {
+    try {
+        await contents.loadURL("about:blank");
+        return !contents.isDestroyed();
+    } catch (err) {
+        log.warn("[loginIndicator] Could not load about:blank: %s", err);
+        return false;
+    }
+}
+
+function detachDebugger(contents: Electron.WebContents) {
+    try {
+        if (!contents.isDestroyed() && contents.debugger.isAttached()) {
+            contents.debugger.detach();
+        }
+    } catch (err) {
+        log.debug("[loginIndicator] Could not detach the debugger: %s", err);
+    }
+}
+
 function loadOverlaySource(): string | null {
     if (overlaySource === undefined) {
         try {
@@ -88,7 +177,7 @@ function describeAccount(
  * account picker is a session-wide event, and it is unregistered when the
  * window closes so it can never outlive the sign-in it belongs to.
  */
-export function attachLoginIndicator(window: BrowserWindow) {
+export async function attachLoginIndicator(window: BrowserWindow) {
     const contents = window.webContents;
     // Held on to now, while the window is alive: by the time `closed` fires,
     // the WebContents is gone and even reading `contents.session` off it
@@ -97,6 +186,19 @@ export function attachLoginIndicator(window: BrowserWindow) {
     const session = contents.session;
     const source = loadOverlaySource();
 
+    // First, and awaited: the overlay is only useful if it is running before
+    // the login page's own scripts are, and both steps below need to finish
+    // before the caller loads that page. Everything after this is wired up
+    // synchronously.
+    if (source && (await loadBlank(contents))) {
+        await injectAtDocumentStart(contents, source);
+    }
+
+    if (window.isDestroyed()) {
+        log.warn("[loginIndicator] Window went away while arming the overlay");
+        return;
+    }
+
     if (source) {
         const inject = (target: ScriptTarget, where: string) => {
             target.executeJavaScript(source).catch((err) => {
@@ -108,12 +210,16 @@ export function attachLoginIndicator(window: BrowserWindow) {
             });
         };
 
+        // Injecting again once the document is up covers the case where the
+        // document-start hook could not be armed. The overlay no-ops when it
+        // lands in a document twice, so the two cannot collide.
         contents.on("dom-ready", () => inject(contents, contents.getURL()));
 
         // A sign-in page may delegate WebAuthn to a cross-origin <iframe> (an
-        // identity provider embedded by the AWS page), and executeJavaScript
-        // on the WebContents only reaches the top frame — so follow sub-frames
-        // as they appear. The overlay no-ops if it lands in a frame twice.
+        // identity provider embedded by the AWS page). Those run in their own
+        // process, out of reach of both executeJavaScript on the WebContents
+        // and the document-start hook, so follow sub-frames as they appear. The
+        // overlay no-ops if it lands in a frame twice.
         contents.on("frame-created", (_event, details) => {
             const frame = details.frame;
             if (!frame || frame === contents.mainFrame) return;
@@ -242,6 +348,10 @@ export function attachLoginIndicator(window: BrowserWindow) {
 
     session.on("select-webauthn-account", selectAccount);
 
+    // `close`, not `closed`: the debugger has to be let go while the
+    // WebContents it is attached to is still there to let go of.
+    window.on("close", () => detachDebugger(contents));
+
     window.on("closed", () => {
         stopWaiting();
         session.off("select-webauthn-account", selectAccount);
diff --git a/tools/check-login-overlay.js b/tools/check-login-overlay.js
new file mode 100644
index 0000000..259801b
--- /dev/null
+++ b/tools/check-login-overlay.js
@@ -0,0 +1,179 @@
+// Self-check for the login window's credential overlay.
+//
+//     npm run build && npx electron tools/check-login-overlay.js
+//
+// Headless (CI, a container): wrap it in a display —
+//
+//     xvfb-run -a npx electron --no-sandbox tools/check-login-overlay.js
+//
+// Why this exists: every part of the overlay can look right and still show the
+// user nothing, and the failure is silent — no error, no log line, just a login
+// window that sits there while the key waits to be touched. That is how it
+// shipped broken once already: the overlay was injected on `dom-ready`, which
+// is *after* the page's own scripts run, so a sign-in page that asks for the
+// key as it boots (Google's security-key challenge does) had already called
+// `navigator.credentials.get()` by the time there was anything to wrap.
+//
+// So the check drives the real thing — the real `attachLoginIndicator()` on a
+// real BrowserWindow — against both orderings, and asserts the wait actually
+// reached the main process. It needs no security key: only the *start* of the
+// request is interesting, and that is signalled the moment the page asks.
+
+import assert from "assert";
+import http from "http";
+import { createRequire } from "module";
+import { app, BrowserWindow } from "electron";
+
+import { attachLoginIndicator } from "../dist/login-indicator.js";
+
+const require = createRequire(import.meta.url);
+const { version } = require("../package.json");
+
+/** Must match SIGNAL in src/login-overlay.ts. */
+const SIGNAL = "__frost-login-overlay__:";
+
+const CASE_TIMEOUT_MS = 15000;
+
+/**
+ * A `navigator.credentials.get()` for a USB security key, which is the request
+ * the overlay most needs to explain. Nothing here can be answered without a key
+ * plugged in, so it stays pending — exactly the state under test.
+ *
+ * `rpId` has to be a domain (an IP address is rejected outright), and WebAuthn
+ * only runs in a secure context, which is why every page below is served from
+ * `localhost`.
+ */
+const REQUEST = `navigator.credentials.get({
+    publicKey: {
+        challenge: new Uint8Array(32),
+        rpId: "localhost",
+        timeout: 300000,
+        allowCredentials: [
+            { type: "public-key", id: new Uint8Array(16), transports: ["usb"] },
+        ],
+        userVerification: "discouraged",
+    },
+}).catch(function () {})`;
+
+const PAGES = {
+    // Asks for the key while the document is still being parsed. Only an
+    // injection that runs before the page's own scripts can wrap this.
+    "/early": `<!doctype html><html><head><script>${REQUEST};</script></head>
+        <body><h1>early</h1></body></html>`,
+
+    // Asks well after load, the way a page with a "Use your security key"
+    // button does. The `dom-ready` injection is enough for this one.
+    "/late": `<!doctype html><html><body><h1>late</h1><script>
+        addEventListener("load", function () {
+            setTimeout(function () { ${REQUEST}; }, 250);
+        });
+        </script></body></html>`,
+};
+
+function serve() {
+    return new Promise((resolve, reject) => {
+        const server = http.createServer((req, res) => {
+            const page = PAGES[req.url.split("?")[0]];
+            res.writeHead(page ? 200 : 404, {
+                "Content-Type": "text/html",
+                // A cached page would test the cache, not the injection.
+                "Cache-Control": "no-store",
+            });
+            res.end(page || "<!doctype html><html><body>404</body></html>");
+        });
+        server.on("error", reject);
+        // Bound to the loopback address, reached by name: `localhost` is a
+        // secure context and a usable `rpId`, `127.0.0.1` is neither.
+        server.listen(0, "127.0.0.1", () =>
+            resolve({ server, port: server.address().port })
+        );
+    });
+}
+
+/**
+ * Load `path` in a window wired up exactly like the login window, and resolve
+ * with what the main process learned about the wait.
+ */
+async function runCase(port, path) {
+    const window = new BrowserWindow({
+        width: 550,
+        height: 700,
+        show: false,
+        webPreferences: { nodeIntegration: false },
+    });
+
+    const signals = [];
+    window.webContents.on("console-message", (details) => {
+        if (details.message.startsWith(SIGNAL)) {
+            signals.push(JSON.parse(details.message.slice(SIGNAL.length)));
+        }
+    });
+
+    try {
+        await attachLoginIndicator(window);
+        await window.webContents.loadURL(`http://localhost:${port}${path}`);
+
+        const deadline = Date.now() + CASE_TIMEOUT_MS;
+        while (!signals.some((signal) => signal.state === "start")) {
+            if (Date.now() > deadline) break;
+            await new Promise((resolve) => setTimeout(resolve, 100));
+        }
+
+        return { signals, title: window.getTitle() };
+    } finally {
+        // close() is what the login flow itself does, so this exercises the
+        // same teardown; destroy() is the belt-and-braces for a window that did
+        // not go away, so one stuck case cannot leak into the next. Either way
+        // the still-pending credential request goes with it.
+        if (!window.isDestroyed()) window.close();
+        await new Promise((resolve) => setTimeout(resolve, 200));
+        if (!window.isDestroyed()) window.destroy();
+    }
+}
+
+async function check() {
+    const { server, port } = await serve();
+
+    try {
+        for (const path of ["/early", "/late"]) {
+            const { signals, title } = await runCase(port, path);
+            const started = signals.filter((s) => s.state === "start");
+
+            assert.ok(
+                started.length > 0,
+                `${path}: the overlay never reported the wait. The page asked ` +
+                    `for a security key and the user was shown nothing.`
+            );
+            assert.strictEqual(
+                started[0].kind,
+                "security-key",
+                `${path}: wait reported as ${started[0].kind}`
+            );
+            assert.strictEqual(
+                title,
+                "Frost — Touch your security key",
+                `${path}: window title was not taken over`
+            );
+            console.log(`  ${path}: overlay reported the wait, title taken over`);
+        }
+    } finally {
+        server.close();
+    }
+}
+
+// Same as src/main.ts: Frost is a tray app and does not quit when its last
+// window closes. Without this, tearing down the first case quits the app
+// underneath the second one.
+app.on("window-all-closed", () => {});
+
+app.whenReady().then(async () => {
+    console.log(`login overlay self-check (Frost ${version})`);
+    try {
+        await check();
+        console.log("login overlay self-check OK");
+        app.exit(0);
+    } catch (err) {
+        console.error(`login overlay self-check FAILED\n${err}`);
+        app.exit(1);
+    }
+});

From 1b882111b020ffc3a4413162d29e5c1bbcefa20d Mon Sep 17 00:00:00 2001
From: Zohar Zilberman <popen2@gmail.com>
Date: Fri, 21 Aug 2026 13:39:03 +0000
Subject: [PATCH 2/2] Drop the comment on the login overlay check

The step name and the command say what it runs; the why belongs in
AGENTS.md, where it already is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014s8Y8cKzE4bpyaxPMP19wi
---
 .github/workflows/ci.yaml | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index 109f491..55713aa 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -48,12 +48,6 @@ jobs:
         run: |
           npm run lint
 
-      # The login window's credential overlay fails silently when it breaks -
-      # no error, just a sign-in page that shows nothing while it waits for a
-      # security key. This drives a real BrowserWindow through a real WebAuthn
-      # wait and asserts the overlay reported it. It needs a display (xvfb) and
-      # --no-sandbox, because the runner's Electron has no setuid sandbox
-      # helper; it needs no security key.
       - name: 🔑 Check the login overlay
         run: |
           xvfb-run -a npm run check:overlay -- --no-sandbox