From cadc2e7f743b1534f42ae43931e6a37e38d768f5 Mon Sep 17 00:00:00 2001 From: John-Ryan21337 <227466507+John-Ryan21337@users.noreply.github.com> Date: Fri, 11 Sep 2026 21:44:18 -0700 Subject: [PATCH] fix(desktop): bind IPC authority to the configured renderer --- AGENTS.md | 2 + apps/desktop/src/ipc/DesktopIpc.test.ts | 102 ++++++++++++-- apps/desktop/src/ipc/DesktopIpc.ts | 130 +++++++++++------- apps/desktop/src/window/DesktopWindow.test.ts | 38 +++++ apps/desktop/src/window/DesktopWindow.ts | 15 +- 5 files changed, 226 insertions(+), 61 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c4f7d53ac..a8fdad3b1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -69,6 +69,8 @@ If a tradeoff is required, choose correctness, durability, and debuggability ove ## Security Requirements +- Bind desktop IPC authority to both the exact registered `webContents` and its configured renderer origin (or exact production file). Reject other loopback ports, deceptive loopback hostnames, child frames, and destroyed senders. Keep the trusted renderer on this navigation scope. Sender-aware handlers receive the already-validated event; use strict payload decoding for capability-bearing APIs and reject extra fields before running the handler. + - Write all code as if it will run in a security-conscious environment where adversaries will constantly try to attack local transports, provider sessions, provider credentials, persisted command ledgers, and debug surfaces. - Local provider daemon and supervisor transports must be loopback-only or IPC by default, authenticated with high-entropy capability tokens, and must never be exposed on a non-loopback interface without an explicit authenticated design. - Secrets such as provider daemon tokens, Codex auth, Claude credentials, OpenCode server passwords, saved-environment bearer sessions, and app bootstrap credentials must be stored as private files with restrictive permissions where the platform supports them. Never persist secrets in logs, debug JSON, process argv, browser local storage, or user-visible error strings. diff --git a/apps/desktop/src/ipc/DesktopIpc.test.ts b/apps/desktop/src/ipc/DesktopIpc.test.ts index 5e867ffdd..0f8297885 100644 --- a/apps/desktop/src/ipc/DesktopIpc.test.ts +++ b/apps/desktop/src/ipc/DesktopIpc.test.ts @@ -1,11 +1,36 @@ +import * as ElectronShell from "../electron/ElectronShell.ts"; +import { COPY_TEXT_CHANNEL } from "./channels.ts"; +import { copyText } from "./methods/window.ts"; import * as Effect from "effect/Effect"; +import * as Schema from "effect/Schema"; import * as Layer from "effect/Layer"; import { describe, expect, it, vi } from "vitest"; -import * as ElectronShell from "../electron/ElectronShell.ts"; -import { COPY_TEXT_CHANNEL } from "./channels.ts"; import * as DesktopIpc from "./DesktopIpc.ts"; -import { copyText } from "./methods/window.ts"; + +describe("strict sender-aware IPC methods", () => { + it("rejects excess payload fields before calling the handler", async () => { + let calls = 0; + const sender = { id: 42 }; + const method = DesktopIpc.makeIpcMethod({ + channel: "test.strict", + payload: Schema.Struct({ tabId: Schema.String }), + result: Schema.Number, + strict: true, + handler: (_input, event) => + Effect.sync(() => { + calls++; + expect(event.sender).toBe(sender); + return calls; + }), + }); + await expect( + Effect.runPromise(method.handler({ tabId: "tab", extra: "denied" }, { sender })), + ).rejects.toThrow(); + expect(calls).toBe(0); + expect(await Effect.runPromise(method.handler({ tabId: "tab" }, { sender }))).toBe(1); + }); +}); function makeTopFrame(url: string): DesktopIpc.DesktopIpcWebFrame { const frame = { @@ -59,20 +84,26 @@ describe("DesktopIpc sender validation", () => { expect(DesktopIpc.isTrustedDesktopIpcFrameUrl("http://127.0.0.1:5733/")).toBe(true); expect(DesktopIpc.isTrustedDesktopIpcFrameUrl("http://localhost:5733/")).toBe(true); expect(DesktopIpc.isTrustedDesktopIpcFrameUrl("http://[::1]:5733/")).toBe(true); + expect(DesktopIpc.isTrustedDesktopIpcFrameUrl("http://127.attacker.example:5733/")).toBe(false); + expect(DesktopIpc.isTrustedDesktopIpcFrameUrl("http://localhost.attacker.example:5733/")).toBe( + false, + ); expect(DesktopIpc.isTrustedDesktopIpcFrameUrl("https://example.com/")).toBe(false); expect(DesktopIpc.isTrustedDesktopIpcFrameUrl("app://cafe-code/index.html")).toBe(false); }); - it("allows invoke handlers from registered top-level production and dev frames", async () => { + it("pins registered webContents to their exact production file or development origin", async () => { const ipcMain = makeIpcMainStub(); const ipc = DesktopIpc.make(ipcMain.ipcMain); - const sender = { id: 7, isDestroyed: () => false }; + const productionSender = { id: 7, isDestroyed: () => false }; + const developmentSender = { id: 8, isDestroyed: () => false }; let calls = 0; await Effect.runPromise( Effect.scoped( Effect.gen(function* () { - yield* ipc.trustWebContents(sender); + yield* ipc.trustWebContents(productionSender, "file:///Applications/CafeCode/index.html"); + yield* ipc.trustWebContents(developmentSender, "http://127.0.0.1:5733/"); yield* ipc.handle({ channel: "secure.invoke", handler: (raw) => @@ -85,7 +116,10 @@ describe("DesktopIpc sender validation", () => { yield* Effect.promise(() => Promise.resolve( ipcMain.getInvokeListener()( - { sender, senderFrame: makeTopFrame("file:///Applications/CafeCode/index.html") }, + { + sender: productionSender, + senderFrame: makeTopFrame("file:///Applications/CafeCode/index.html#/chat"), + }, "production", ), ), @@ -93,7 +127,10 @@ describe("DesktopIpc sender validation", () => { yield* Effect.promise(() => Promise.resolve( ipcMain.getInvokeListener()( - { sender, senderFrame: makeTopFrame("http://127.0.0.1:5733/") }, + { + sender: developmentSender, + senderFrame: makeTopFrame("http://127.0.0.1:5733/chat"), + }, "development", ), ), @@ -105,6 +142,39 @@ describe("DesktopIpc sender validation", () => { expect(calls).toBe(2); }); + it("passes the validated sender to sender-aware handlers", async () => { + const ipcMain = makeIpcMainStub(); + const ipc = DesktopIpc.make(ipcMain.ipcMain); + const sender = { id: 42, isDestroyed: () => false }; + let handledSenderId: number | undefined; + + await Effect.runPromise( + Effect.scoped( + Effect.gen(function* () { + yield* ipc.trustWebContents(sender, "http://127.0.0.1:5733/"); + yield* ipc.handleFromSender({ + channel: "secure.sender-aware", + handler: (_raw, event) => + Effect.sync(() => { + handledSenderId = event.sender?.id; + return "handled"; + }), + }); + yield* Effect.promise(() => + Promise.resolve( + ipcMain.getInvokeListener()( + { sender, senderFrame: makeTopFrame("http://127.0.0.1:5733/") }, + undefined, + ), + ), + ); + }), + ), + ); + + expect(handledSenderId).toBe(42); + }); + it("rejects invoke handlers from untrusted origins and unexpected frames", async () => { const ipcMain = makeIpcMainStub(); const ipc = DesktopIpc.make(ipcMain.ipcMain); @@ -114,7 +184,7 @@ describe("DesktopIpc sender validation", () => { await Effect.runPromise( Effect.scoped( Effect.gen(function* () { - yield* ipc.trustWebContents(sender); + yield* ipc.trustWebContents(sender, "http://127.0.0.1:5733/"); yield* ipc.handle({ channel: "secure.invoke", handler: () => @@ -134,6 +204,15 @@ describe("DesktopIpc sender validation", () => { await expect( listener({ sender, senderFrame: makeTopFrame("https://evil.example/") }, "payload"), ).rejects.toThrow(DesktopIpc.DesktopIpcSenderValidationError); + await expect( + listener({ sender, senderFrame: makeTopFrame("http://127.0.0.1:5734/") }, "payload"), + ).rejects.toThrow(DesktopIpc.DesktopIpcSenderValidationError); + await expect( + listener( + { sender, senderFrame: makeTopFrame("http://127.attacker.example:5733/") }, + "payload", + ), + ).rejects.toThrow(DesktopIpc.DesktopIpcSenderValidationError); await expect(listener({ sender, senderFrame: childFrame }, "payload")).rejects.toThrow( DesktopIpc.DesktopIpcSenderValidationError, ); @@ -175,7 +254,6 @@ describe("DesktopIpc sender validation", () => { expect(event.returnValue).toBeNull(); expect(calls).toBe(0); }); - it("decodes trusted clipboard writes and rejects invalid payloads", async () => { const ipcMain = makeIpcMainStub(); const ipc = DesktopIpc.make(ipcMain.ipcMain); @@ -186,7 +264,7 @@ describe("DesktopIpc sender validation", () => { await Effect.runPromise( Effect.scoped( Effect.gen(function* () { - yield* ipc.trustWebContents(sender); + yield* ipc.trustWebContents(sender, "file:///Applications/CafeCode/index.html"); yield* ipc.handle(copyText); const listener = ipcMain.getInvokeListener(); @@ -215,7 +293,7 @@ describe("DesktopIpc sender validation", () => { await Effect.runPromise( Effect.scoped( Effect.gen(function* () { - yield* ipc.trustWebContents(sender); + yield* ipc.trustWebContents(sender, "file:///Applications/CafeCode/index.html"); yield* ipc.handle(copyText); }), ).pipe(Effect.provide(makeElectronShellLayer((text) => copiedTexts.push(text)))), diff --git a/apps/desktop/src/ipc/DesktopIpc.ts b/apps/desktop/src/ipc/DesktopIpc.ts index 8894fdf48..e1ac69e6b 100644 --- a/apps/desktop/src/ipc/DesktopIpc.ts +++ b/apps/desktop/src/ipc/DesktopIpc.ts @@ -6,6 +6,7 @@ import * as Scope from "effect/Scope"; export interface DesktopIpcWebContents { readonly id?: number; isDestroyed?: () => boolean; + once?: (event: "destroyed", listener: () => void) => unknown; } export interface DesktopIpcWebFrame { @@ -40,7 +41,7 @@ export interface DesktopIpcMain { export interface DesktopIpcMethod { readonly channel: string; - readonly handler: (raw: unknown) => Effect.Effect; + readonly handler: (raw: unknown, event: DesktopIpcInvokeEvent) => Effect.Effect; } export interface DesktopSyncIpcMethod { @@ -49,10 +50,16 @@ export interface DesktopSyncIpcMethod { } export interface DesktopIpcShape { - readonly trustWebContents: (webContents: DesktopIpcWebContents) => Effect.Effect; + readonly trustWebContents: ( + webContents: DesktopIpcWebContents, + rendererUrl: string, + ) => Effect.Effect; readonly handle: ( input: DesktopIpcMethod, ) => Effect.Effect; + readonly handleFromSender: ( + input: DesktopIpcMethod, + ) => Effect.Effect; readonly handleSync: ( input: DesktopSyncIpcMethod, ) => Effect.Effect; @@ -75,34 +82,53 @@ function normalizeHostname(hostname: string): string { function isLoopbackHostname(hostname: string): boolean { const normalized = normalizeHostname(hostname); - return normalized === "localhost" || normalized === "::1" || /^127(?:\.|$)/.test(normalized); + if (normalized === "localhost" || normalized === "::1") return true; + const parts = normalized.split("."); + return ( + parts.length === 4 && + parts.every((part) => /^\d{1,3}$/.test(part) && Number(part) <= 255) && + Number(parts[0]) === 127 + ); } -export function isTrustedDesktopIpcFrameUrl(rawUrl: string): boolean { +function trustedDesktopIpcFrameScope(rawUrl: string): string | null { try { const url = new URL(rawUrl); if (url.protocol === "file:") { - return true; + url.search = ""; + url.hash = ""; + return url.href; } if (url.protocol !== "http:" && url.protocol !== "https:") { - return false; + return null; } - return isLoopbackHostname(url.hostname); + return isLoopbackHostname(url.hostname) ? url.origin : null; } catch { - return false; + return null; } } +export function isTrustedDesktopIpcFrameUrl(rawUrl: string): boolean { + return trustedDesktopIpcFrameScope(rawUrl) !== null; +} + +export function isTrustedDesktopIpcNavigation(rawUrl: string, rendererUrl: string): boolean { + const trustedScope = trustedDesktopIpcFrameScope(rendererUrl); + return trustedScope !== null && trustedDesktopIpcFrameScope(rawUrl) === trustedScope; +} + function isTopLevelFrame(frame: DesktopIpcWebFrame): boolean { return frame.top === undefined || frame.top === null || frame.top === frame; } function validateDesktopIpcSender( event: DesktopIpcInvokeEvent | DesktopIpcSyncEvent, - trustedWebContents: WeakSet, + trustedWebContents: WeakMap, ): void { const sender = event.sender; - if (typeof sender !== "object" || sender === null || !trustedWebContents.has(sender)) { + const trustedScope = + typeof sender === "object" && sender !== null ? trustedWebContents.get(sender) : undefined; + if (!sender || trustedScope === undefined) { throw new DesktopIpcSenderValidationError("Rejected IPC call from an untrusted webContents."); } @@ -115,49 +141,58 @@ function validateDesktopIpcSender( throw new DesktopIpcSenderValidationError("Rejected IPC call from an untrusted frame."); } - if (!isTrustedDesktopIpcFrameUrl(frame.url)) { + if (trustedDesktopIpcFrameScope(frame.url) !== trustedScope) { throw new DesktopIpcSenderValidationError("Rejected IPC call from an untrusted frame URL."); } } export const make = (ipcMain: DesktopIpcMain): DesktopIpcShape => { - const trustedWebContents = new WeakSet(); + const trustedWebContents = new WeakMap(); - return DesktopIpc.of({ - trustWebContents: (webContents) => + const handle = Effect.fn("desktop.ipc.registerInvoke")(function* ({ + channel, + handler, + }: DesktopIpcMethod) { + yield* Effect.annotateCurrentSpan({ channel }); + const context = yield* Effect.context(); + const runPromise = Effect.runPromiseWith(context); + + yield* Effect.acquireRelease( Effect.sync(() => { - trustedWebContents.add(webContents); + ipcMain.removeHandler(channel); + ipcMain.handle(channel, (event, raw) => { + try { + validateDesktopIpcSender(event, trustedWebContents); + } catch (error) { + return Promise.reject(error); + } + + return runPromise( + Effect.gen(function* () { + yield* Effect.annotateCurrentSpan({ channel }); + return yield* handler(raw, event); + }).pipe(Effect.annotateLogs({ channel }), Effect.withSpan("desktop.ipc.invoke")), + ); + }); }), + () => Effect.sync(() => ipcMain.removeHandler(channel)), + ); + }); - handle: Effect.fn("desktop.ipc.registerInvoke")(function* ({ - channel, - handler, - }: DesktopIpcMethod) { - yield* Effect.annotateCurrentSpan({ channel }); - const context = yield* Effect.context(); - const runPromise = Effect.runPromiseWith(context); - - yield* Effect.acquireRelease( - Effect.sync(() => { - ipcMain.removeHandler(channel); - ipcMain.handle(channel, (event, raw) => { - try { - validateDesktopIpcSender(event, trustedWebContents); - } catch (error) { - return Promise.reject(error); - } + return DesktopIpc.of({ + trustWebContents: (webContents, rendererUrl) => + Effect.sync(() => { + const trustedScope = trustedDesktopIpcFrameScope(rendererUrl); + if (trustedScope === null) { + throw new DesktopIpcSenderValidationError( + "Cannot trust a webContents for an invalid renderer URL.", + ); + } + trustedWebContents.set(webContents, trustedScope); + }), - return runPromise( - Effect.gen(function* () { - yield* Effect.annotateCurrentSpan({ channel }); - return yield* handler(raw); - }).pipe(Effect.annotateLogs({ channel }), Effect.withSpan("desktop.ipc.invoke")), - ); - }); - }), - () => Effect.sync(() => ipcMain.removeHandler(channel)), - ); - }), + handle, + handleFromSender: handle, handleSync: Effect.fn("desktop.ipc.registerSync")(function* ({ channel, @@ -224,7 +259,8 @@ export interface DesktopIpcMethodRegistration< ResultDecodingServices, ResultEncodingServices >; - readonly handler: (input: Payload) => Effect.Effect; + readonly strict?: true; + readonly handler: (input: Payload, event: DesktopIpcInvokeEvent) => Effect.Effect; } export const makeIpcMethod = < @@ -260,9 +296,9 @@ export const makeIpcMethod = < return { channel: method.channel, - handler: (raw) => - decode(raw).pipe( - Effect.flatMap(method.handler), + handler: (raw, event) => + decode(raw, method.strict ? { onExcessProperty: "error" } : undefined).pipe( + Effect.flatMap((input) => method.handler(input, event)), Effect.flatMap(encode), Effect.withSpan("desktop.ipc.method", { attributes: { channel: method.channel } }), ), diff --git a/apps/desktop/src/window/DesktopWindow.test.ts b/apps/desktop/src/window/DesktopWindow.test.ts index 981ef8637..feaebcc6d 100644 --- a/apps/desktop/src/window/DesktopWindow.test.ts +++ b/apps/desktop/src/window/DesktopWindow.test.ts @@ -73,6 +73,7 @@ function makeFakeBrowserWindow() { openDevTools: webContents.openDevTools, setPermissionCheckHandler, setPermissionRequestHandler, + onWebContents: webContents.on, }; } @@ -122,6 +123,7 @@ const electronThemeLayer = Layer.succeed(ElectronTheme.ElectronTheme, { const desktopIpcLayer = Layer.succeed(DesktopIpc.DesktopIpc, { trustWebContents: () => Effect.void, handle: () => Effect.void, + handleFromSender: () => Effect.void, handleSync: () => Effect.void, } satisfies DesktopIpc.DesktopIpcShape); @@ -174,6 +176,42 @@ function makeTestLayer(input: { } describe("DesktopWindow", () => { + it.effect("pins main-frame navigation without blocking child-frame redirects", () => + Effect.gen(function* () { + const fakeWindow = makeFakeBrowserWindow(); + const createCount = yield* Ref.make(0); + const mainWindow = yield* Ref.make>(Option.none()); + const layer = makeTestLayer({ window: fakeWindow.window, createCount, mainWindow }); + yield* Effect.gen(function* () { + const desktopWindow = yield* DesktopWindow.DesktopWindow; + yield* desktopWindow.handleBackendReady; + for (const name of ["will-navigate", "will-redirect"]) { + const handler = fakeWindow.onWebContents.mock.calls.find( + ([event]) => event === name, + )?.[1]; + assert.isFunction(handler); + const untrustedMain = { + url: "https://other.example/", + isMainFrame: true, + preventDefault: vi.fn(), + }; + handler(untrustedMain); + assert.equal(untrustedMain.preventDefault.mock.calls.length, 1); + const trustedMain = { + url: "http://127.0.0.1:5733/chat", + isMainFrame: true, + preventDefault: vi.fn(), + }; + handler(trustedMain); + assert.equal(trustedMain.preventDefault.mock.calls.length, 0); + const child = { ...untrustedMain, isMainFrame: false, preventDefault: vi.fn() }; + handler(child); + assert.equal(child.preventDefault.mock.calls.length, 0); + } + }).pipe(Effect.provide(layer)); + }), + ); + it.effect("does not open a development window until the backend is ready", () => Effect.gen(function* () { const fakeWindow = makeFakeBrowserWindow(); diff --git a/apps/desktop/src/window/DesktopWindow.ts b/apps/desktop/src/window/DesktopWindow.ts index 17677b83e..ecef945db 100644 --- a/apps/desktop/src/window/DesktopWindow.ts +++ b/apps/desktop/src/window/DesktopWindow.ts @@ -274,12 +274,23 @@ const make = Effect.gen(function* () { sandbox: true, }, }); - yield* desktopIpc.trustWebContents(window.webContents); - const rendererUrl = environment.isDevelopment ? new URL(yield* resolveDesktopDevServerUrl(environment)) : backendHttpUrl; + yield* desktopIpc.trustWebContents(window.webContents, rendererUrl.href); installTrustedAudioPermissionPolicy(window.webContents, rendererUrl); + const guardRendererNavigation = ( + event: Electron.Event, + ) => { + if ( + event.isMainFrame && + !DesktopIpc.isTrustedDesktopIpcNavigation(event.url, rendererUrl.href) + ) { + event.preventDefault(); + } + }; + window.webContents.on("will-navigate", guardRendererNavigation); + window.webContents.on("will-redirect", guardRendererNavigation); window.webContents.on("context-menu", (event, params) => { event.preventDefault();