Agent browser: --insecure flag on controller browser open (fixes #323) - #324
Conversation
When the agent-driven preview browser opened an `https://localhost:*` URL
served by a local dev server with a self-signed certificate, Chromium
blocked the navigation with `NET::ERR_CERT_AUTHORITY_INVALID`. The agent
had no click path to advance past the interstitial, so a whole class of
local-https dev workflows was unreachable from the agent. A regular
browser shows the bypass prompt; the preview pane did not.
`controller browser open <url> --insecure` now opts into a TLS-cert
verification bypass scoped to loopback hosts for the duration of that
single navigation. The bypass is gated in three independent places:
1. The CLI parses `--insecure` (default false).
2. The server-side policy in `validateBrowserUrl` rejects
`--insecure` against any non-loopback host, so an external URL
with the flag is refused with a clear error before it ever
reaches the renderer.
3. The Electron `setCertificateVerifyProc` only returns 0 (allow)
when `request.hostname` is `localhost`, `127.0.0.1`, or `[::1]`;
everything else still falls through to Chromium default and
continues to be rejected on TLS errors.
This means the flag gives the agent exactly one new capability — reach
a `https://localhost:*` server with a self-signed cert — and nothing
more. The bypass is intentionally not sticky: every `open` re-asserts
the policy from the renderer-side request, so a follow-up
non-`--insecure` open restores the default verifier.
Implementation:
- CLI (`cli/controller`): new `--insecure` parser branch; the flag
rides through to the server as `params.insecure`.
- Server (`server/lib/browser-policy.ts` + `routes/browser.ts`):
`validateBrowserUrl(input, root, { insecure })` accepts the option
and narrows the protocol allow-list to loopback hosts when set.
Route forwards `params.insecure` to the renderer.
- Electron (`electron/main.ts` + `preload.ts`): new
`controller:set-preview-cert-policy` IPC handler that installs
`setCertificateVerifyProc` on the `controller-preview` session,
scoped to loopback hosts.
- Renderer (`usePreviewBrowserHost.ts` + `PreviewBrowserPool.tsx`):
the bridge handler reads `params.insecure` and threads it through
the existing `openUrl` flow; the pool calls `setPreviewCertPolicy`
before `validatePreviewUrl`, which guarantees the bypass is in
effect before the navigation actually starts.
- Shared types (`shared/controller.ts` + `shared/preview-browser.ts`):
`setPreviewCertPolicy` on the bridge; `BrowserOpenParams` documents
the flag.
- Tests: 4 new policy tests (`server/lib/__tests__/browser-policy.test.ts`)
and 4 new CLI parser tests (`cli/__tests__/controller-cli.test.mjs`).
Full suite: 685/685 passing.
- Docs (`docs/content/docs/browser-preview.mdx`): new "Local HTTPS
dev servers" section with a copy-pasteable example and an explicit
note that the bypass is loopback-scoped.
- CHANGELOG: unreleased entry referencing #323.
Note: `package-lock.json` carries an unrelated pre-existing diff
(optional-native `dev: true` flags) that is left out of this commit.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 89bfe905f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| host === "127.0.0.1" || | ||
| host === "[::1]" || | ||
| host === "::1"; | ||
| callback(isLoopback ? 0 : -2); |
There was a problem hiding this comment.
When an insecure localhost page loads any HTTPS subresource from another host—for example a CDN script, font, or external API—this callback rejects the certificate even when it is valid. Electron's Session API defines -2 as failure; -3 is the value that uses Chromium's verification result. Returning -3 for non-loopback hosts is necessary for the claimed fall-through behavior.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — that was a real bug. Fixed in 04ae836: setPreviewCertPolicy now returns -3 (Chromium default verification) for non-loopback hosts instead of -2 ("fail"), so a legitimate HTTPS subresource loaded by an insecure-localhost page is still checked and accepted. Updated the inline comment to call out the -2 vs -3 distinction, and the CHANGELOG entry under #323 notes the fix.
| const session = electronSession.fromPartition(PREVIEW_PARTITION); | ||
| if (!insecure) { | ||
| session.setCertificateVerifyProc(null); |
There was a problem hiding this comment.
When two pane opens overlap, this session-wide mutation races their navigations: every pane uses the same controller-preview partition, and openUrl changes the policy before asynchronous validation and rendering, so a normal open in pane B can clear the verifier after pane A enables it but before pane A requests its self-signed certificate. The insecure open then intermittently reaches the certificate interstitial; the policy needs per-pane isolation or serialization that lasts through navigation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed this is a real race — the cert policy lives on a session-wide partition, so two simultaneous opens in different panes can clobber each other between the IPC flip and the actual cert handshake.
Fixing it in this PR would mean reshaping the partition per pane, which touches <webview partition> and the existing partition-level guards (attachPreviewPartitionGuards, attachPreviewWebviewGuards). That feels too big to bundle into the flag ship.
Tracking as #325: per-pane Electron session partition (derived from the existing browserKey so the grammar stays alphanumerics + _ + -). After that lands, the cert policy is per-pane by construction and any future per-pane flag — --user-agent, --cookie-store, the SSRF allowlist from #117 — doesn't have to re-derive the same race-avoidance.
Thanks for catching both.
…ecks (#324 review) Address Codex review feedback on PR #324: electron/main.ts:477 — `setPreviewCertPolicy` returned `-2` ("fail") for non-loopback hosts, which short-circuits Chromium and rejects *any* TLS connection request from a non-loopback host, not just the ones we wanted to gate. In practice this means a legitimate HTTPS subresource loaded by an insecure-localhost page — a CDN script, font, or external API over a valid cert — would fail with `ERR_CERT_AUTHORITY_INVALID` even though the cert is fine and we have no business blocking it. Electron's `session.setCertificateVerifyProc` documents three return values: 0 = allow, -2 = fail, -3 = use Chromium's default verification result. Returning `-3` for non-loopback hosts preserves the original goal — non-loopback certs are still verified — while letting pages on an insecure localhost page load third-party HTTPS resources normally. The doc-comment on `setPreviewCertPolicy` already promised "fall through to Chromium's default verifier and continue to be rejected on TLS errors"; the old code only did that for the cert of the top-level URL, not for subresource certs. The new code matches the contract. The cross-pane policy race (also raised by Codex) is deferred to #325 — a per-pane Electron session partition is the right fix and warrants its own PR. - CHANGELOG: explicit note that the `-2` → `-3` fix lands alongside the original #323 entry.
1 participant
Closes #323.
Summary
controller browser open <url> --insecurelets the agent reach a local dev server served overhttpswith a self-signed certificate — something the preview pane could not do before, because Chromium blocks the navigation withNET::ERR_CERT_AUTHORITY_INVALIDand the agent has no click path through the interstitial.The bypass is scoped to loopback hosts in three independent places (CLI, server-side policy, Electron
setCertificateVerifyProc) so it can't be used to talk to an arbitrary external host without cert validation.What changed
cli/controller: parse--insecureonbrowser open; advertise it inUSAGE.server/lib/browser-policy.ts:validateBrowserUrl(input, root, { insecure })accepts the option and narrows thehttp(s)allow-list to loopback hosts when set.server/routes/browser.ts: forwardparams.insecureto the renderer.electron/main.ts+electron/preload.ts: newcontroller:set-preview-cert-policyIPC handler. InstallssetCertificateVerifyProcon thecontroller-previewsession, returning0for loopback hosts (localhost,127.0.0.1,[::1]) and-2otherwise.client/src/lib/usePreviewBrowserHost.ts+client/src/components/PreviewBrowserPool.tsx: threadparams.insecurethroughopenUrl; the pool callssetPreviewCertPolicybeforevalidatePreviewUrlso the bypass is in effect before navigation starts.shared/controller.ts+shared/preview-browser.ts:setPreviewCertPolicyon the bridge; documentinsecureinBrowserOpenParams.Security model
The flag gives the agent exactly one new capability — reach a
https://localhost:*server with a self-signed cert. It cannot be used to talk to an arbitrary external host without cert validation:--insecureis opt-in, defaultfalse.validateBrowserUrl({ insecure: true })rejects any non-loopback hostname before it reaches the renderer.setCertificateVerifyProc0only forlocalhost,127.0.0.1,[::1]; everything else falls through to Chromium's default verifier.The bypass is intentionally per-call, not sticky: every
openre-asserts the policy, so a non---insecurefollow-up open restores the default verifier.Tests
validateBrowserUrltests (covers the localhost-only contract).--insecureplacement before/after the URL.node --import tsx --test ....Verification
tsc -p electron/tsconfig.jsonandtsc -p server/tsconfig.jsoncompile clean.vite buildsucceeds.snapshot) was not exercised from this terminal — there's no Electron host available here — but every layer of the chain has the contract it needs.Out of scope
package-lock.jsoncarries an unrelated pre-existing diff (optional-nativedev: trueflags) that's left out of this commit.Acceptance criteria
controller browser open https://localhost:5050 --insecureloads a self-signedlocalhostcert without the agent having to interact with the interstitial.--insecure, the same URL still shows the interstitial (no behavior change for the default).--insecure,https://example.comis still rejected by cert validation (and now by the server-side policy before it reaches the renderer).openwithout the flag.validatePreviewUrl.