From a116d4c5c12619dc54e2ccd8c3127560d750f931 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:43:18 +0200 Subject: [PATCH 1/3] Clear the OAuth popup result from localStorage after handing it over MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The callback page writes its result to localStorage as the reliable same-origin completion channel, and the opener removes it on pickup. When nothing is listening — an abandoned flow, a reloaded opener, any caller not using the React helper — nobody ever removes it, and the payload carries the identity label (an email) and, on failure, the error preview. It sat in the user's browser profile indefinitely. The page now clears its own entry: on success just before the existing auto-close, and on failure after a delay, since a failed flow deliberately keeps the window up. This cannot cost a listener the result — a `storage` event captures `newValue` at dispatch, so an opener that was notified already holds it, and the only other reader polls `popup.closed`, not storage. The tests RUN the generated script against stub globals rather than matching its source, because a string assertion passes just as well on a script that never executes, and what is at stake here is what the browser is left holding. --- packages/core/api/src/oauth-popup.test.ts | 74 +++++++++++++++++++++++ packages/core/api/src/oauth-popup.ts | 9 ++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/packages/core/api/src/oauth-popup.test.ts b/packages/core/api/src/oauth-popup.test.ts index 12efa6971b..77278a2fa0 100644 --- a/packages/core/api/src/oauth-popup.test.ts +++ b/packages/core/api/src/oauth-popup.test.ts @@ -149,6 +149,80 @@ describe("popupDocument", () => { expect(script).toContain("channel\\u003c/script\\u003e"); }); + // The assertions below RUN the generated script against stub globals rather + // than matching its source text. A string check would pass on a script that + // never executes — and the property at stake here is what the browser is left + // holding, which only running it can show. + const runPopupScript = (html: string) => { + const script = / `; }; From 2e12352ebefbf2af322758b073b346d60021ac59 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Wed, 12 Aug 2026 22:56:39 +0200 Subject: [PATCH 2/3] Add a changeset for the popup storage-residue fix --- .changeset/oauth-popup-localstorage-residue.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/oauth-popup-localstorage-residue.md diff --git a/.changeset/oauth-popup-localstorage-residue.md b/.changeset/oauth-popup-localstorage-residue.md new file mode 100644 index 0000000000..443f6ffa58 --- /dev/null +++ b/.changeset/oauth-popup-localstorage-residue.md @@ -0,0 +1,9 @@ +--- +"executor": patch +--- + +**The OAuth popup clears its result out of `localStorage` after handing it over** + +The popup writes its result to `localStorage` as the fallback completion channel, because `postMessage` is severed when a provider's consent page sets COOP and `BroadcastChannel` can be partitioned or raced by the auto-close. Nothing removed that entry afterwards, so the payload — which carries the identity label, an email, and on failure the error preview — stayed parked in the user's browser profile. + +The entry is now cleared once the handover has had time to land. This cannot cost a listener the result: a `storage` event captures `newValue` at dispatch, so an opener that has been notified already holds it. From 9cdc3d396fd4c58ea8cb5b38d33c1fbf183edaa3 Mon Sep 17 00:00:00 2001 From: GeiserX <9169332+GeiserX@users.noreply.github.com> Date: Sun, 16 Aug 2026 14:10:52 +0200 Subject: [PATCH 3/3] fix(api): don't strand the OAuth error payload when the user closes the failure popup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The failure page never auto-closes so the user can read the error, and its localStorage clear rode solely on a 5s timer. Closing the window earlier killed the pending timer, and with no opener listening (COOP severs window.opener; a reloaded app tab has no storage listener) nothing else removed the key — the error payload sat in the browser profile forever, exactly the residue this branch exists to eliminate. A pagehide listener now clears the entry when the document dies; the timers stay for the page merely sitting open. Clearing on pagehide cannot cost a listener the result: a storage event captures newValue at dispatch, and the opener ignores the null-newValue removal event. --- .../oauth-popup-localstorage-residue.md | 2 +- packages/core/api/src/oauth-popup.test.ts | 26 +++++++++++++++++++ packages/core/api/src/oauth-popup.ts | 6 ++++- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/.changeset/oauth-popup-localstorage-residue.md b/.changeset/oauth-popup-localstorage-residue.md index 443f6ffa58..5068a726f4 100644 --- a/.changeset/oauth-popup-localstorage-residue.md +++ b/.changeset/oauth-popup-localstorage-residue.md @@ -6,4 +6,4 @@ The popup writes its result to `localStorage` as the fallback completion channel, because `postMessage` is severed when a provider's consent page sets COOP and `BroadcastChannel` can be partitioned or raced by the auto-close. Nothing removed that entry afterwards, so the payload — which carries the identity label, an email, and on failure the error preview — stayed parked in the user's browser profile. -The entry is now cleared once the handover has had time to land. This cannot cost a listener the result: a `storage` event captures `newValue` at dispatch, so an opener that has been notified already holds it. +The entry is now cleared once the handover has had time to land, and on `pagehide` as a backstop — the failure page never auto-closes so the user can read the error, and closing it by hand would otherwise cancel the pending timer and strand the entry. This cannot cost a listener the result: a `storage` event captures `newValue` at dispatch, so an opener that has been notified already holds it. diff --git a/packages/core/api/src/oauth-popup.test.ts b/packages/core/api/src/oauth-popup.test.ts index 77278a2fa0..fe0d64d1cf 100644 --- a/packages/core/api/src/oauth-popup.test.ts +++ b/packages/core/api/src/oauth-popup.test.ts @@ -158,6 +158,7 @@ describe("popupDocument", () => { expect(script).toBeDefined(); const store = new Map(); const timers: { readonly fn: () => void; readonly ms: number }[] = []; + const pagehideListeners: (() => void)[] = []; let closed = false; const win = { opener: null, @@ -165,6 +166,9 @@ describe("popupDocument", () => { close: () => { closed = true; }, + addEventListener: (type: string, listener: () => void) => { + if (type === "pagehide") pagehideListeners.push(listener); + }, }; const fn = new Function( "window", @@ -191,6 +195,11 @@ describe("popupDocument", () => { runTimers: () => { for (const t of [...timers]) t.fn(); }, + // Simulates the document dying (user closes the window): pagehide fires, + // pending timers never do. + firePagehide: () => { + for (const listener of [...pagehideListeners]) listener(); + }, }; }; @@ -223,6 +232,23 @@ describe("popupDocument", () => { expect(run.isClosed()).toBe(false); }); + it("clears the stored result when the user closes the failure window before the timer", () => { + const html = popupDocument( + { type: OAUTH_POPUP_MESSAGE_TYPE, ok: false, sessionId: null, error: "nope" }, + "chan-3", + ); + const run = runPopupScript(html); + expect(run.store.get("chan-3")).toContain("nope"); + + // The failure page never auto-closes; the user reads the error and closes + // the window before the 5s timer fires. The document dies — pending timers + // never run — so pagehide is the only thing standing between the payload + // and living in the browser profile forever. + run.firePagehide(); + + expect(run.store.has("chan-3")).toBe(false); + }); + it("posts to window.opener AND falls back to BroadcastChannel with the given channel name", () => { const html = popupDocument(successPayload, "executor:openapi-oauth-result"); expect(html).toContain("window.opener.postMessage(p,window.location.origin)"); diff --git a/packages/core/api/src/oauth-popup.ts b/packages/core/api/src/oauth-popup.ts index 13e9213855..9bfcdfda2d 100644 --- a/packages/core/api/src/oauth-popup.ts +++ b/packages/core/api/src/oauth-popup.ts @@ -124,8 +124,12 @@ try{localStorage.setItem(${serializedChannel},JSON.stringify(p))}catch(e){} // listener the result: a 'storage' event captures newValue at dispatch, so an // opener that has been notified already holds it. Leaving it would park that // data in the user's browser profile indefinitely whenever nobody is listening, -// which is every abandoned or opener-less flow. +// which is every abandoned or opener-less flow. pagehide backs the timers up: +// the failure page never auto-closes (the user must be able to read the error), +// and closing it kills any pending timer — without pagehide the entry would +// outlive the document after all. const clear=()=>{try{localStorage.removeItem(${serializedChannel})}catch(e){}}; +window.addEventListener("pagehide",clear); if(p.ok)setTimeout(()=>{clear();window.close()},400);else setTimeout(clear,5000);})(); `;