From b2cecc6350d377c382e1ec32ee66ec63ad68f715 Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Sun, 6 Sep 2026 17:18:12 +1000 Subject: [PATCH] fix(desktop): add sidecar credentials from the main process (#47588) --- packages/desktop/src/main/ipc-handlers/app.ts | 5 ++-- .../main/service/background-service-state.ts | 8 ++--- .../src/main/service/background-service.ts | 13 ++++---- .../main/service/sidecar-credentials.test.ts | 24 +++++++++++++++ .../src/main/service/sidecar-credentials.ts | 30 +++++++++++++++++++ packages/desktop/src/main/windows/security.ts | 22 +++++++++++++- .../desktop/src/renderer/migration-status.tsx | 8 ++--- .../renderer/startup/initialization.test.ts | 8 ++--- .../src/renderer/startup/initialization.ts | 9 +++--- packages/desktop/src/shared/ipc-contract.ts | 2 +- packages/desktop/src/shared/ipc-rpc/app.ts | 1 - 11 files changed, 99 insertions(+), 31 deletions(-) create mode 100644 packages/desktop/src/main/service/sidecar-credentials.test.ts create mode 100644 packages/desktop/src/main/service/sidecar-credentials.ts diff --git a/packages/desktop/src/main/ipc-handlers/app.ts b/packages/desktop/src/main/ipc-handlers/app.ts index 087784bb3b0..6d69709c7b6 100644 --- a/packages/desktop/src/main/ipc-handlers/app.ts +++ b/packages/desktop/src/main/ipc-handlers/app.ts @@ -14,6 +14,7 @@ import { ApplicationLifecycle } from "../lifecycle" import { finishFirstLaunchOnboarding, isFirstLaunchOnboardingPending } from "../lifecycle/onboarding" import { BackgroundService } from "../service/background-service" import { DesktopCli } from "../service/desktop-cli" +import { SidecarCredentials } from "../service/sidecar-credentials" import { getDefaultServerUrl, setDefaultServerUrl } from "../service/server-settings" import { Updater } from "../updater" import { getLastFocusedWindow, setBackgroundColor } from "../windows" @@ -29,8 +30,8 @@ export const appHandlers = AppRpcs.toLayer( const logging = yield* DesktopLogging.Service const runFork = Effect.runForkWith(yield* Effect.context()) return AppRpcs.of({ - AppAwaitInitialization: () => background.connection, - AppReconnectService: () => background.reconnect, + AppAwaitInitialization: () => background.connection.pipe(Effect.map(SidecarCredentials.ready)), + AppReconnectService: () => background.reconnect.pipe(Effect.map(SidecarCredentials.ready)), AppConsumeInitialDeepLinks: () => Effect.sync(lifecycle.consumeInitialDeepLinks), AppGetDefaultServerUrl: () => Effect.sync(getDefaultServerUrl), AppSetDefaultServerUrl: ({ url }) => Effect.sync(() => setDefaultServerUrl(url)), diff --git a/packages/desktop/src/main/service/background-service-state.ts b/packages/desktop/src/main/service/background-service-state.ts index 892a2abb92b..5d0012c5403 100644 --- a/packages/desktop/src/main/service/background-service-state.ts +++ b/packages/desktop/src/main/service/background-service-state.ts @@ -1,14 +1,14 @@ export * as BackgroundServiceState from "./background-service-state" import { Effect, Exit, Ref } from "effect" -import type { ServerReadyData } from "../../shared/ipc-contract" +import type { SidecarCredentials } from "./sidecar-credentials" export const make = Effect.fn("BackgroundServiceState.make")(function* (options: { - readonly initial: Effect.Effect - readonly reconnect: Effect.Effect + readonly initial: Effect.Effect + readonly reconnect: Effect.Effect }) { // Every Exit is an Effect, so the latest resolution replays directly for each consumer. - const current = yield* Ref.make>(yield* options.initial.pipe(Effect.exit)) + const current = yield* Ref.make>(yield* options.initial.pipe(Effect.exit)) return { connection: Ref.get(current).pipe(Effect.flatten, Effect.orDie), reconnect: options.reconnect.pipe(Effect.tap((next) => Ref.set(current, Exit.succeed(next)))), diff --git a/packages/desktop/src/main/service/background-service.ts b/packages/desktop/src/main/service/background-service.ts index a8e49543df4..a051341917d 100644 --- a/packages/desktop/src/main/service/background-service.ts +++ b/packages/desktop/src/main/service/background-service.ts @@ -1,14 +1,14 @@ import { app } from "electron" import { Context, Effect, FileSystem, Layer, Path } from "effect" -import type { ServerReadyData } from "../../shared/ipc-contract" import { BackgroundServiceState } from "./background-service-state" import { cleanStages, DesktopCli } from "./desktop-cli" +import { SidecarCredentials } from "./sidecar-credentials" export * as BackgroundService from "./background-service" export interface Interface { - readonly connection: Effect.Effect - readonly reconnect: Effect.Effect + readonly connection: Effect.Effect + readonly reconnect: Effect.Effect } export class Service extends Context.Service()("opencode/desktop/BackgroundService") {} @@ -56,10 +56,9 @@ const connect = Effect.fn("BackgroundService.connect")(function* (mode: "initial ...endpoint(url.origin), }) if (mode === "initial" && isolated && cli.binary) yield* cleanStages(cli.binary).pipe(Effect.orDie) - return { - url: url.origin, - password: service.auth.password, - } satisfies ServerReadyData + const ready = { url: url.origin, password: service.auth.password } satisfies SidecarCredentials.Data + SidecarCredentials.set(ready) + return ready }) function endpoint(url: string | undefined) { diff --git a/packages/desktop/src/main/service/sidecar-credentials.test.ts b/packages/desktop/src/main/service/sidecar-credentials.test.ts new file mode 100644 index 00000000000..da28c6bbbc6 --- /dev/null +++ b/packages/desktop/src/main/service/sidecar-credentials.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, test } from "bun:test" +import { authorization, ready } from "./sidecar-credentials" + +const sidecar = { url: "http://127.0.0.1:4096", password: "secret" } +const expected = `Basic ${Buffer.from("opencode:secret").toString("base64")}` + +describe("sidecar authorization", () => { + test("adds the Basic credential only for the sidecar origin", () => { + expect(authorization(sidecar, "http://127.0.0.1:4096/api/session?limit=1")).toBe(expected) + expect(authorization(sidecar, "http://127.0.0.1:4097/api/session")).toBeUndefined() + expect(authorization(sidecar, "http://localhost:4096/api/session")).toBeUndefined() + expect(authorization(sidecar, "https://127.0.0.1:4096/api/session")).toBeUndefined() + }) + + test("hands the renderer the origin only", () => { + expect(ready(sidecar)).toEqual({ url: sidecar.url }) + }) + + test("adds nothing before the sidecar is known or when it has no password", () => { + expect(authorization(undefined, "http://127.0.0.1:4096/api/session")).toBeUndefined() + expect(authorization({ url: sidecar.url, password: null }, "http://127.0.0.1:4096/api/session")).toBeUndefined() + expect(authorization(sidecar, "not a url")).toBeUndefined() + }) +}) diff --git a/packages/desktop/src/main/service/sidecar-credentials.ts b/packages/desktop/src/main/service/sidecar-credentials.ts new file mode 100644 index 00000000000..d803515cadf --- /dev/null +++ b/packages/desktop/src/main/service/sidecar-credentials.ts @@ -0,0 +1,30 @@ +export * as SidecarCredentials from "./sidecar-credentials" + +import type { ServerReadyData } from "../../shared/ipc-contract" + +export type Data = ServerReadyData & { password: string | null } + +// The renderer talks to the sidecar without an Authorization header; the main process adds it from +// here so GET requests stay CORS-simple and skip the preflight round trip. Both the initial connection +// and every reconnect publish the current endpoint. +let current: Data | undefined + +export function set(data: Data) { + current = data +} + +export function get() { + return current +} + +/** What the renderer learns about the sidecar: its origin, never its credential. */ +export function ready(data: Data): ServerReadyData { + return { url: data.url } +} + +/** The Basic credential for a request to the sidecar origin, or undefined for any other URL. */ +export function authorization(sidecar: Data | undefined, url: string) { + if (!sidecar?.password || !URL.canParse(url)) return + if (new URL(url).origin !== sidecar.url) return + return `Basic ${Buffer.from(`opencode:${sidecar.password}`).toString("base64")}` +} diff --git a/packages/desktop/src/main/windows/security.ts b/packages/desktop/src/main/windows/security.ts index 97b5e1cba39..2387e997134 100644 --- a/packages/desktop/src/main/windows/security.ts +++ b/packages/desktop/src/main/windows/security.ts @@ -1,5 +1,6 @@ import type { BrowserWindow } from "electron" -import { addRendererHeaders } from "./headers" +import { SidecarCredentials } from "../service/sidecar-credentials" +import { addRendererHeaders, hasHeader, upsertHeader } from "./headers" import { isRendererUrl } from "./protocol" const rendererPermissions = new Set(["clipboard-sanitized-write", "notifications"]) @@ -31,6 +32,25 @@ export function wireNavigationPolicy(win: BrowserWindow, openExternalURL: (url: } export function wireRendererHeaders(win: BrowserWindow) { + // The renderer sends sidecar requests without credentials, so its GETs are CORS-simple and need no + // preflight. Electron applies these listeners in Chromium's extraHeaders mode, after the CORS + // 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 + // no frame, such as from a service worker, are not credentialed either; the renderer registers none. + win.webContents.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 }) + }, + ) win.webContents.session.webRequest.onHeadersReceived((details, callback) => { const responseHeaders = details.responseHeaders ?? {} addRendererHeaders(responseHeaders, { document: isRendererUrl(details.url, true) }) diff --git a/packages/desktop/src/renderer/migration-status.tsx b/packages/desktop/src/renderer/migration-status.tsx index 200383d8168..18717746baf 100644 --- a/packages/desktop/src/renderer/migration-status.tsx +++ b/packages/desktop/src/renderer/migration-status.tsx @@ -49,12 +49,8 @@ export function MigrationStatus(props: { server: ServerReadyData }) { await wait(1_000, abort.signal) if (abort.signal.aborted) return - const client = OpenCode.make({ - baseUrl: props.server.url, - headers: props.server.password - ? { Authorization: `Basic ${btoa(`opencode:${props.server.password}`)}` } - : undefined, - }) + // The main process credentials sidecar requests; see `wireRendererHeaders`. + const client = OpenCode.make({ baseUrl: props.server.url }) void (async () => { while (true) { diff --git a/packages/desktop/src/renderer/startup/initialization.test.ts b/packages/desktop/src/renderer/startup/initialization.test.ts index 8e17734177f..e7395c918a4 100644 --- a/packages/desktop/src/renderer/startup/initialization.test.ts +++ b/packages/desktop/src/renderer/startup/initialization.test.ts @@ -27,7 +27,7 @@ describe("desktop renderer initialization", () => { }) test("returns initialized sidecar data", () => { - const sidecar = { url: "http://127.0.0.1:1234", password: "secret" } + const sidecar = { url: "http://127.0.0.1:1234" } expect(initializationData(Object.assign(() => sidecar, { error: undefined }))).toBe(sidecar) }) @@ -47,7 +47,7 @@ describe("desktop renderer initialization", () => { }) test("refreshes the managed sidecar endpoint", async () => { - const sidecar = { url: "http://127.0.0.1:4321", password: "next" } + const sidecar = { url: "http://127.0.0.1:4321" } const updates: (typeof sidecar)[] = [] const resolve = createSidecarResolver({ api: { reconnectService: async () => sidecar }, @@ -60,7 +60,7 @@ describe("desktop renderer initialization", () => { }) test("keeps the current sidecar when reconnection resolves the same endpoint", async () => { - const sidecar = { url: "http://127.0.0.1:4321", password: "same" } + const sidecar = { url: "http://127.0.0.1:4321" } const updates: (typeof sidecar)[] = [] const resolve = createSidecarResolver({ api: { reconnectService: async () => ({ ...sidecar }) }, @@ -73,7 +73,7 @@ describe("desktop renderer initialization", () => { }) test("does not publish a sidecar resolved after cancellation", async () => { - const sidecar = { url: "http://127.0.0.1:4321", password: "next" } + const sidecar = { url: "http://127.0.0.1:4321" } const pending = Promise.withResolvers() const updates: (typeof sidecar)[] = [] const resolve = createSidecarResolver({ diff --git a/packages/desktop/src/renderer/startup/initialization.ts b/packages/desktop/src/renderer/startup/initialization.ts index 1450a1dd7c9..d134cc336f8 100644 --- a/packages/desktop/src/renderer/startup/initialization.ts +++ b/packages/desktop/src/renderer/startup/initialization.ts @@ -7,11 +7,10 @@ export function initializationData(state: (() => A | undefined) & { error: un return state() } +// The main process adds Authorization to sidecar requests (`wireRendererHeaders`); the renderer never +// holds the password, and its GETs carry only CORS-safelisted headers so they skip the preflight. export function sidecarHttp(data: SidecarData) { - return { - url: data.url, - password: data.password ?? undefined, - } + return { url: data.url } } export function createSidecarResolver(input: { @@ -29,7 +28,7 @@ export function createSidecarResolver(input: { } function sameSidecar(current: SidecarData | undefined, next: SidecarData) { - return current?.url === next.url && current.password === next.password + return current?.url === next.url } function markLocalServerStartup(error: unknown) { diff --git a/packages/desktop/src/shared/ipc-contract.ts b/packages/desktop/src/shared/ipc-contract.ts index 9ce69b6c18d..e0769855f18 100644 --- a/packages/desktop/src/shared/ipc-contract.ts +++ b/packages/desktop/src/shared/ipc-contract.ts @@ -1,6 +1,6 @@ +// The sidecar password never crosses into the renderer; the main process adds it to sidecar requests. export type ServerReadyData = { url: string - password: string | null } export type TitlebarTheme = { diff --git a/packages/desktop/src/shared/ipc-rpc/app.ts b/packages/desktop/src/shared/ipc-rpc/app.ts index 95dd936ca34..f964e8dc198 100644 --- a/packages/desktop/src/shared/ipc-rpc/app.ts +++ b/packages/desktop/src/shared/ipc-rpc/app.ts @@ -3,7 +3,6 @@ import { Rpc, RpcGroup } from "effect/unstable/rpc" const ServerReadyData = Schema.Struct({ url: Schema.String, - password: Schema.NullOr(Schema.String), }) export const AppAwaitInitialization = Rpc.make("AppAwaitInitialization", { success: ServerReadyData })