Skip to content

fix(desktop): oauth popups open from the browser preview - #8435

Draft
walid-baharwal wants to merge 2 commits into
pingdotgg:mainfrom
walid-baharwal:fix/preview-oauth-popups
Draft

fix(desktop): oauth popups open from the browser preview#8435
walid-baharwal wants to merge 2 commits into
pingdotgg:mainfrom
walid-baharwal:fix/preview-oauth-popups

Conversation

@walid-baharwal

@walid-baharwal walid-baharwal commented Aug 27, 2026

Copy link
Copy Markdown

What Changed

window.open from a page in the integrated browser preview now opens a real window when the disposition is new-window and the URL is http or https. Everything else, including target="_blank" links, keeps loading in the preview tab as it does today.

The popup is created with contextIsolation, sandbox, and nodeIntegration: false set explicitly. A popup is not a webview attach, so the will-attach-webview hardening in DesktopWindow never sees it, and an unoverridden child would inherit the picker preload's relaxed posture. about:blank popups stay in the preview tab: Chromium skips browser-side navigation for them, copies the guest preferences, and gives no way to override.

One file plus its test: apps/desktop/src/preview/Manager.ts (+56/−2).

Why

A local app opened in the preview cannot finish an OAuth popup flow. Firebase signInWithPopup(auth, new GoogleAuthProvider()) reports auth/popup-blocked and no window appears, while the same app works in a normal Chrome or Firefox window.

The handler denied every window.open and loaded the URL in the same webContents. window.open() therefore returned null, which the Firebase SDK reads as a blocked popup, and the in-tab load also destroyed the opener that the popup has to postMessage its credential back to. The handler already received disposition and ignored it, so scripted popups and target="_blank" links took the same wrong branch.

The renderer side already sets allowpopups on the <webview> (apps/web/src/browser/HostedBrowserWebview.tsx), so no client change was needed.

Surfaces

Desktop only. The preview <webview> exists inside Electron, and remote web previews never reach setWindowOpenHandler. No contract, provider, or docs change.

UI Changes

No rendered UI, layout, or motion changed. The behavior is interaction-dependent, so a screen recording of the sign-in flow is coming before this leaves draft.

Verification

  • Decision logic covered by focused unit tests next to isPreviewRefreshShortcut, the existing pure helper in the same file.
  • vp test run apps/desktop/src/preview/Manager.test.ts has not run on my machine: the install did not finish over my connection. CI is the first full run of these tests, and I will fix whatever it reports.
  • Manual pass through a Firebase signInWithPopup app in a preview tab, plus a target="_blank" link to confirm it still loads in-tab, lands with the recording.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • Video for the interaction change (draft until then)

Fixes #6561

Written with Claude Opus 5 in Claude Code.

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1031c66e-ffbc-47b0-89a7-bd7dcf557ad5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Aug 27, 2026
Scripted `window.open` calls inside the integrated browser preview were denied
and loaded in the preview tab instead. Firebase `signInWithPopup` got a null
window handle back and reported `auth/popup-blocked`, and the in-tab load also
dropped the opener the popup needs to post the credential back to.

Popups with a `new-window` disposition and an http or https URL now get a real
window, with context isolation and the sandbox turned back on: a popup is not a
webview attach, so the `will-attach-webview` hardening never sees it and an
unoverridden child would inherit the picker preload's relaxed posture.
`about:blank` popups keep loading in the preview tab, since Chromium copies the
guest preferences for them and forbids overriding. Links with `target="_blank"`
are unchanged.

Fixes pingdotgg#6561
@walid-baharwal
walid-baharwal force-pushed the fix/preview-oauth-popups branch from e598878 to 8fa05ae Compare August 27, 2026 18:16
Comment thread apps/desktop/src/preview/Manager.ts
Comment thread apps/desktop/src/preview/Manager.ts
An allowed popup carried Electron's default window-open behavior, so a page
inside it could spawn native windows without limit. The popup now denies its
own window.open calls; no OAuth flow opens a second popup.

The popup preferences also drop nodeIntegrationInSubFrames, matching the three
keys every other hardened window in the app sets.
@walid-baharwal
walid-baharwal force-pushed the fix/preview-oauth-popups branch from d2ce056 to 7c5b69a Compare August 27, 2026 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Integrated browser preview blocks OAuth popup authentication

1 participant