From 9a8460852318a7dc8278e7a5c51cb8b40fb22241 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Fri, 30 Oct 2026 17:27:00 +1000 Subject: [PATCH] fix(desktop): credential the sidecar when its service listens on a non-loopback address --- .../desktop/src/main/windows/security.test.ts | 61 ++++++++++++++++++- packages/desktop/src/main/windows/security.ts | 50 ++++++++++----- 2 files changed, 93 insertions(+), 18 deletions(-) diff --git a/packages/desktop/src/main/windows/security.test.ts b/packages/desktop/src/main/windows/security.test.ts index 4fe3c8d20542..df9ad7f60830 100644 --- a/packages/desktop/src/main/windows/security.test.ts +++ b/packages/desktop/src/main/windows/security.test.ts @@ -1,7 +1,15 @@ import { expect, test } from "bun:test" import { EventEmitter } from "node:events" -import type { BrowserWindow, Session, WebContents } from "electron" -import { allowRendererPermissions } from "./security" +import type { + BeforeSendResponse, + BrowserWindow, + OnBeforeSendHeadersListenerDetails, + Session, + WebContents, + WebRequestFilter, +} from "electron" +import { SidecarCredentials } from "../service/sidecar-credentials" +import { allowRendererPermissions, wireRendererHeaders } from "./security" type RequestHandler = NonNullable[0]> @@ -121,3 +129,52 @@ test("retains permissions across all live windows on a shared session and revoke expect(await shared.ask(1, "notifications")).toBe(false) expect(shared.check(null, "notifications", { isMainFrame: false })).toBe(false) }) + +test.each([ + ["on loopback", "http://127.0.0.1:4096", "http://127.0.0.1/*"], + ["on a Tailscale address", "http://100.87.251.43:45278", "http://100.87.251.43/*"], + ["on a LAN name", "http://devbox.local:4096", "http://devbox.local/*"], +])("credentials the renderer's requests to a sidecar %s", async (_name, url, pattern) => { + const registered: Registration = { urls: [] } + + // SAFETY: wireRendererHeaders touches only webRequest.onBeforeSendHeaders and webRequest.onHeadersReceived. + // oxlint-disable-next-line anti-slop/no-chained-type-assertions -- see SAFETY above + const session = { + webRequest: { + onBeforeSendHeaders(filter: WebRequestFilter, listener: Listener) { + registered.urls = filter.urls + registered.listener = listener + }, + onHeadersReceived() {}, + }, + } as unknown as Session + + // SAFETY: wireRendererHeaders reads only `webContents.session`. + // oxlint-disable-next-line anti-slop/no-chained-type-assertions -- see SAFETY above + wireRendererHeaders({ webContents: { session } } as unknown as BrowserWindow) + // The window opens before the service is ready, so the filter has to follow the endpoint published later. + SidecarCredentials.set({ url, password: "secret" }) + expect(registered.urls).toContain(pattern) + + const listener = registered.listener + + if (!listener) throw new Error("Missing onBeforeSendHeaders listener") + + // SAFETY: the listener reads only `url`, `frame.parent`, `frame.url` and `requestHeaders`. + // oxlint-disable-next-line anti-slop/no-chained-type-assertions -- see SAFETY above + const details = { + url: `${url}/api/event`, + frame: { parent: null, url: "oc://renderer/index.html" }, + requestHeaders: {}, + } as unknown as OnBeforeSendHeadersListenerDetails + + const headers = await new Promise((resolve) => + listener(details, (response) => resolve(response.requestHeaders)), + ) + + expect(headers).toEqual({ Authorization: `Basic ${Buffer.from("opencode:secret").toString("base64")}` }) +}) + +type Listener = (details: OnBeforeSendHeadersListenerDetails, callback: (response: BeforeSendResponse) => void) => void + +type Registration = { urls: string[]; listener?: Listener } diff --git a/packages/desktop/src/main/windows/security.ts b/packages/desktop/src/main/windows/security.ts index 328164afc513..808b2b35d980 100644 --- a/packages/desktop/src/main/windows/security.ts +++ b/packages/desktop/src/main/windows/security.ts @@ -1,4 +1,4 @@ -import type { BrowserWindow, Session } from "electron" +import type { BeforeSendResponse, BrowserWindow, OnBeforeSendHeadersListenerDetails, Session } from "electron" import { SidecarCredentials } from "../service/sidecar-credentials" import { addRendererHeaders, hasHeader, upsertHeader } from "./headers" import { isRendererUrl } from "./scheme" @@ -95,25 +95,43 @@ export function wireRendererHeaders(win: BrowserWindow) { // decision, so adding Authorization here does not reintroduce one. // // Only the renderer's own top-level frame is credentialed. Other content in this session (web views, - // embedded pages) can reach the same loopback origin and must not inherit its access. Requests with + // embedded pages) can reach the same sidecar origin and must not inherit its access. Requests with // no frame, such as from a service worker, are not credentialed either; the renderer registers none. - session.webRequest.onBeforeSendHeaders( - { urls: ["http://127.0.0.1/*", "http://localhost/*"] }, - (details, callback) => { - const frame = details.frame - const renderer = !!frame && frame.parent === null && isRendererUrl(frame.url) - const authorization = renderer && SidecarCredentials.authorization(SidecarCredentials.get(), details.url) - - if (authorization && !hasHeader(details.requestHeaders, "Authorization")) { - upsertHeader(details.requestHeaders, "Authorization", authorization) - } - - callback({ requestHeaders: details.requestHeaders }) - }, - ) + const credential = ( + details: OnBeforeSendHeadersListenerDetails, + callback: (response: BeforeSendResponse) => void, + ) => { + const frame = details.frame + const renderer = !!frame && frame.parent === null && isRendererUrl(frame.url) + const authorization = renderer && SidecarCredentials.authorization(SidecarCredentials.get(), details.url) + + if (authorization && !hasHeader(details.requestHeaders, "Authorization")) { + upsertHeader(details.requestHeaders, "Authorization", authorization) + } + + callback({ requestHeaders: details.requestHeaders }) + } + + // A service configured to listen on another address (a LAN or Tailscale IP) is still the sidecar, so the filter + // follows its host. Electron keeps one listener per event, so each registration replaces the last. + const follow = () => + session.webRequest.onBeforeSendHeaders({ urls: sidecarUrls(SidecarCredentials.get()) }, credential) + + follow() + SidecarCredentials.onChange(follow) session.webRequest.onHeadersReceived((details, callback) => { const responseHeaders = details.responseHeaders ?? {} addRendererHeaders(responseHeaders, { document: isRendererUrl(details.url, true) }) callback({ responseHeaders }) }) } + +// Match patterns ignore the port when they name none, so one pattern covers every port on the host. +function sidecarUrls(sidecar: SidecarCredentials.Data | undefined) { + const loopback = ["http://127.0.0.1/*", "http://localhost/*"] + + if (!sidecar || !URL.canParse(sidecar.url)) return loopback + const url = new URL(sidecar.url) + + return [...new Set([...loopback, `${url.protocol}//${url.hostname}/*`])] +}