From a91b380fc0c837d884bdd9069f87ecc572b0ec80 Mon Sep 17 00:00:00 2001 From: Neeraj Sathish Kumar Date: Tue, 30 Jun 2026 14:51:39 +0530 Subject: [PATCH] fix: restrict privileged desktop IPC --- apps/desktop/src/main/index.ts | 4 +- apps/desktop/src/main/ipc/file.ipc.ts | 73 ++++++++++------- .../src/main/security/ipc-security.test.ts | 66 +++++++++++++++ .../desktop/src/main/security/ipc-security.ts | 80 +++++++++++++++++++ .../src/main/terminal/registerTerminalIpc.ts | 29 ++++--- 5 files changed, 210 insertions(+), 42 deletions(-) create mode 100644 apps/desktop/src/main/security/ipc-security.test.ts create mode 100644 apps/desktop/src/main/security/ipc-security.ts diff --git a/apps/desktop/src/main/index.ts b/apps/desktop/src/main/index.ts index f09997f..5c8c298 100644 --- a/apps/desktop/src/main/index.ts +++ b/apps/desktop/src/main/index.ts @@ -110,12 +110,12 @@ app.whenReady().then(() => { (handler) => coreProcess?.onMessage(handler), ); setupCoreEvents(coreClient); - registerTerminalIpc(terminalManager, coreClient); + registerTerminalIpc(terminalManager, launchCwd, coreClient); // Register other IPC handlers registerWindowIpc(); registerSystemIpc(coreClient); - registerFileIpc(coreClient); + registerFileIpc(coreClient, launchCwd); registerEditorIpc(); registerGitIpc(coreClient); diff --git a/apps/desktop/src/main/ipc/file.ipc.ts b/apps/desktop/src/main/ipc/file.ipc.ts index e9570d5..04570a1 100644 --- a/apps/desktop/src/main/ipc/file.ipc.ts +++ b/apps/desktop/src/main/ipc/file.ipc.ts @@ -4,60 +4,75 @@ import * as fs from "fs"; import { CHANNELS } from "../../shared/ipc/channels"; import { startFileWatcher, stopFileWatcher } from "../system/file-watcher"; import type { CoreClient } from "../app/core-client"; +import { assertTrustedSender, resolveWorkspacePath } from "../security/ipc-security"; -export function registerFileIpc(client: CoreClient): void { - ipcMain.handle(CHANNELS.FILE_LIST, async (_event, args: { path: string }) => { - return client.fileList(args.path); +export function registerFileIpc(client: CoreClient, workspaceRoot: string): void { + const safePath = (candidate: string) => resolveWorkspacePath(workspaceRoot, candidate); + + ipcMain.handle(CHANNELS.FILE_LIST, async (event, args: { path: string }) => { + assertTrustedSender(event); + return client.fileList(safePath(args.path)); }); - ipcMain.handle(CHANNELS.FILE_READ, async (_event, args: { path: string }) => { + ipcMain.handle(CHANNELS.FILE_READ, async (event, args: { path: string }) => { + assertTrustedSender(event); // Fast path: read directly via Node fs instead of round-tripping through orphix-core - const resolved = path.resolve(args.path); + const resolved = safePath(args.path); const content = await fs.promises.readFile(resolved, "utf-8"); return { content }; }); - ipcMain.handle(CHANNELS.FILE_WRITE, async (_event, args: { path: string; content: string }) => { + ipcMain.handle(CHANNELS.FILE_WRITE, async (event, args: { path: string; content: string }) => { + assertTrustedSender(event); // Fast path: write directly via Node fs - const resolved = path.resolve(args.path); + const resolved = safePath(args.path); await fs.promises.writeFile(resolved, args.content, "utf-8"); return { success: true }; }); - ipcMain.handle(CHANNELS.FILE_CREATE, async (_event, args: { path: string; isDir: boolean }) => { - return client.fileCreate(args.path, args.isDir); + ipcMain.handle(CHANNELS.FILE_CREATE, async (event, args: { path: string; isDir: boolean }) => { + assertTrustedSender(event); + return client.fileCreate(safePath(args.path), args.isDir); }); - ipcMain.handle(CHANNELS.FILE_RENAME, async (_event, args: { oldPath: string; newPath: string }) => { - return client.fileRename(args.oldPath, args.newPath); + ipcMain.handle(CHANNELS.FILE_RENAME, async (event, args: { oldPath: string; newPath: string }) => { + assertTrustedSender(event); + return client.fileRename(safePath(args.oldPath), safePath(args.newPath)); }); - ipcMain.handle(CHANNELS.FILE_DELETE, async (_event, args: { path: string }) => { - return client.fileDelete(args.path); + ipcMain.handle(CHANNELS.FILE_DELETE, async (event, args: { path: string }) => { + assertTrustedSender(event); + return client.fileDelete(safePath(args.path)); }); - ipcMain.handle(CHANNELS.FILE_COPY, async (_event, args: { srcPath: string; destPath: string }) => { - return client.fileCopy(args.srcPath, args.destPath); + ipcMain.handle(CHANNELS.FILE_COPY, async (event, args: { srcPath: string; destPath: string }) => { + assertTrustedSender(event); + return client.fileCopy(safePath(args.srcPath), safePath(args.destPath)); }); - ipcMain.handle(CHANNELS.FILE_MOVE, async (_event, args: { srcPath: string; destPath: string }) => { - return client.fileMove(args.srcPath, args.destPath); + ipcMain.handle(CHANNELS.FILE_MOVE, async (event, args: { srcPath: string; destPath: string }) => { + assertTrustedSender(event); + return client.fileMove(safePath(args.srcPath), safePath(args.destPath)); }); - ipcMain.handle(CHANNELS.FILE_STAT, async (_event, args: { path: string }) => { - return client.fileStat(args.path); + ipcMain.handle(CHANNELS.FILE_STAT, async (event, args: { path: string }) => { + assertTrustedSender(event); + return client.fileStat(safePath(args.path)); }); - ipcMain.handle(CHANNELS.FILE_WATCH, async (_event, args: { path: string }) => { - startFileWatcher(args.path); + ipcMain.handle(CHANNELS.FILE_WATCH, async (event, args: { path: string }) => { + assertTrustedSender(event); + startFileWatcher(safePath(args.path)); }); - ipcMain.handle(CHANNELS.FILE_UNWATCH, async () => { + ipcMain.handle(CHANNELS.FILE_UNWATCH, async (event) => { + assertTrustedSender(event); stopFileWatcher(); }); - ipcMain.handle(CHANNELS.FILE_OPEN_EXTERNAL, async (_event, args: { path: string }) => { - const resolved = path.resolve(args.path); + ipcMain.handle(CHANNELS.FILE_OPEN_EXTERNAL, async (event, args: { path: string }) => { + assertTrustedSender(event); + const resolved = safePath(args.path); // Only allow opening files/dirs that exist — blocks arbitrary program execution const stat = await fs.promises.stat(resolved).catch(() => null); if (!stat || (!stat.isFile() && !stat.isDirectory())) { @@ -66,13 +81,15 @@ export function registerFileIpc(client: CoreClient): void { await shell.openPath(resolved); }); - ipcMain.handle(CHANNELS.FILE_REVEAL, async (_event, args: { path: string }) => { - const resolved = path.resolve(args.path); + ipcMain.handle(CHANNELS.FILE_REVEAL, async (event, args: { path: string }) => { + assertTrustedSender(event); + const resolved = safePath(args.path); shell.showItemInFolder(resolved); }); - ipcMain.handle(CHANNELS.FILE_OPEN_TERMINAL, async (_event, args: { path: string }) => { - const resolved = path.resolve(args.path); + ipcMain.handle(CHANNELS.FILE_OPEN_TERMINAL, async (event, args: { path: string }) => { + assertTrustedSender(event); + const resolved = safePath(args.path); const stat = await fs.promises.stat(resolved).catch(() => null); const cwd = stat?.isDirectory() ? resolved : path.dirname(resolved); // Terminal creation is handled by the terminal IPC — emit event for workspace to pick up diff --git a/apps/desktop/src/main/security/ipc-security.test.ts b/apps/desktop/src/main/security/ipc-security.test.ts new file mode 100644 index 0000000..c9311d4 --- /dev/null +++ b/apps/desktop/src/main/security/ipc-security.test.ts @@ -0,0 +1,66 @@ +import { mkdtempSync, mkdirSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, test } from "vitest"; +import { assertTrustedSender, resolveWorkspacePath } from "./ipc-security"; + +const roots: string[] = []; + +function makeRoot(): string { + const root = mkdtempSync(path.join(tmpdir(), "orphix-ipc-security-")); + roots.push(root); + return realpathSync.native(root); +} + +afterEach(() => { + for (const root of roots.splice(0)) { + rmSync(root, { recursive: true, force: true }); + } +}); + +describe("resolveWorkspacePath", () => { + test("allows files inside the workspace", () => { + const root = makeRoot(); + const file = path.join(root, "notes.md"); + writeFileSync(file, "ok"); + + expect(resolveWorkspacePath(root, file)).toBe(realpathSync.native(file)); + expect(resolveWorkspacePath(root, "notes.md")).toBe(realpathSync.native(file)); + }); + + test("blocks paths outside the workspace", () => { + const root = makeRoot(); + const outside = path.join(path.dirname(root), "outside.txt"); + + expect(() => resolveWorkspacePath(root, outside)).toThrow("outside the trusted workspace"); + }); + + test("blocks symlinks that resolve outside the workspace", () => { + const root = makeRoot(); + const outsideDir = mkdtempSync(path.join(tmpdir(), "orphix-ipc-outside-")); + roots.push(outsideDir); + const outsideFile = path.join(outsideDir, "secret.txt"); + writeFileSync(outsideFile, "secret"); + const link = path.join(root, "secret-link.txt"); + symlinkSync(outsideFile, link); + + expect(() => resolveWorkspacePath(root, link)).toThrow("outside the trusted workspace"); + }); + + test("allows creating a new child path inside an existing workspace directory", () => { + const root = makeRoot(); + mkdirSync(path.join(root, "src")); + + expect(resolveWorkspacePath(root, path.join(root, "src", "new.ts"))).toBe(path.join(root, "src", "new.ts")); + }); +}); + +describe("assertTrustedSender", () => { + test("allows local app file renderers", () => { + expect(() => assertTrustedSender({ senderFrame: { url: "file:///app/index.html" }, sender: { getURL: () => "" } } as any)).not.toThrow(); + }); + + test("blocks untrusted remote renderers", () => { + expect(() => assertTrustedSender({ senderFrame: { url: "https://evil.example/" }, sender: { getURL: () => "" } } as any)).toThrow("untrusted renderer origin"); + }); +}); diff --git a/apps/desktop/src/main/security/ipc-security.ts b/apps/desktop/src/main/security/ipc-security.ts new file mode 100644 index 0000000..4957570 --- /dev/null +++ b/apps/desktop/src/main/security/ipc-security.ts @@ -0,0 +1,80 @@ +import type { IpcMainInvokeEvent } from "electron"; +import { realpathSync } from "node:fs"; +import path from "node:path"; + +const DEV_RENDERER_ORIGINS = new Set([ + "http://localhost:5173", + "http://127.0.0.1:5173", +]); + +function getSenderUrl(event: IpcMainInvokeEvent): string { + return event.senderFrame?.url || event.sender.getURL(); +} + +export function assertTrustedSender(event: IpcMainInvokeEvent): void { + const senderUrl = getSenderUrl(event); + if (!senderUrl) { + throw new Error("Blocked IPC from an unknown renderer"); + } + + let url: URL; + try { + url = new URL(senderUrl); + } catch { + throw new Error("Blocked IPC from an invalid renderer URL"); + } + + if (url.protocol === "file:") return; + + const allowDevRenderer = + process.env.NODE_ENV === "development" || + process.env.ORPHIX_ALLOW_DEV_RENDERER === "1"; + if (allowDevRenderer && DEV_RENDERER_ORIGINS.has(url.origin)) return; + + throw new Error(`Blocked IPC from untrusted renderer origin: ${url.origin}`); +} + +function normalizeRoot(root: string): string { + const resolved = path.resolve(root); + try { + return realpathSync.native(resolved); + } catch { + return resolved; + } +} + +function isPathInside(candidate: string, root: string): boolean { + const relative = path.relative(root, candidate); + return ( + relative === "" || + (!!relative && !relative.startsWith("..") && !path.isAbsolute(relative)) + ); +} + +export function resolveWorkspacePath(workspaceRoot: string, candidatePath: string): string { + if (typeof candidatePath !== "string" || candidatePath.trim() === "") { + throw new Error("Path must be a non-empty string"); + } + + const root = normalizeRoot(workspaceRoot); + const resolved = path.resolve( + path.isAbsolute(candidatePath) ? candidatePath : path.join(root, candidatePath), + ); + + if (!isPathInside(resolved, root)) { + throw new Error("Path is outside the trusted workspace"); + } + + try { + const real = realpathSync.native(resolved); + if (!isPathInside(real, root)) { + throw new Error("Path resolves outside the trusted workspace"); + } + return real; + } catch (error) { + if (error instanceof Error && error.message.includes("outside the trusted workspace")) { + throw error; + } + return resolved; + } +} diff --git a/apps/desktop/src/main/terminal/registerTerminalIpc.ts b/apps/desktop/src/main/terminal/registerTerminalIpc.ts index 8c02a85..248f336 100644 --- a/apps/desktop/src/main/terminal/registerTerminalIpc.ts +++ b/apps/desktop/src/main/terminal/registerTerminalIpc.ts @@ -1,10 +1,10 @@ -import path from 'node:path'; import { ipcMain, BrowserWindow } from 'electron'; import { TerminalManager } from './TerminalManager'; import { TERMINAL_CHANNELS } from '../../shared/terminal/terminal-ipc'; import type { CreateTerminalRequest, WriteTerminalRequest, ResizeTerminalRequest, KillTerminalRequest, TerminalSessionSnapshot, ShellInfo } from '../../shared/terminal/types'; import type { CoreClient } from '../app/core-client'; import type { TerminalSessionInfo, ShellInfoDto } from '../../shared/types/common'; +import { assertTrustedSender, resolveWorkspacePath } from '../security/ipc-security'; const CHANNELS = TERMINAL_CHANNELS; @@ -12,7 +12,7 @@ const VALID_TERMINAL_ID = /^[a-zA-Z0-9_-]+$/; const MAX_COLS = 500; const MAX_ROWS = 200; -function validateCreateRequest(request: CreateTerminalRequest): void { +function validateCreateRequest(request: CreateTerminalRequest, workspaceRoot: string): void { if (!request?.terminalId || typeof request.terminalId !== 'string') { throw new Error("terminal:create requires terminalId (string)"); } @@ -27,8 +27,7 @@ function validateCreateRequest(request: CreateTerminalRequest): void { } if (request.cwd != null) { if (typeof request.cwd !== 'string') throw new Error("terminal:create cwd must be a string"); - // Resolve to prevent path traversal - request.cwd = path.resolve(request.cwd); + request.cwd = resolveWorkspacePath(workspaceRoot, request.cwd); } if (request.command != null && typeof request.command !== 'string') { throw new Error("terminal:create command must be a string"); @@ -71,7 +70,7 @@ function toShellInfo(shell: ShellInfoDto, index: number): ShellInfo { }; } -export function registerTerminalIpc(manager: TerminalManager, coreClient?: CoreClient): void { +export function registerTerminalIpc(manager: TerminalManager, workspaceRoot: string, coreClient?: CoreClient): void { // Forward events to renderer manager.onOutput((event) => broadcast(CHANNELS.output, event)); manager.onExit((event) => broadcast(CHANNELS.exit, event)); @@ -79,8 +78,9 @@ export function registerTerminalIpc(manager: TerminalManager, coreClient?: CoreC manager.onError((event) => broadcast(CHANNELS.error, event)); // IPC handlers - ipcMain.handle(CHANNELS.create, async (_event, request: CreateTerminalRequest) => { - validateCreateRequest(request); + ipcMain.handle(CHANNELS.create, async (event, request: CreateTerminalRequest) => { + assertTrustedSender(event); + validateCreateRequest(request, workspaceRoot); if (coreClient && !request.command) { const info = await coreClient.terminalCreate({ terminal_id: request.terminalId, @@ -95,7 +95,8 @@ export function registerTerminalIpc(manager: TerminalManager, coreClient?: CoreC return manager.createTerminal(request); }); - ipcMain.handle(CHANNELS.write, async (_event, request: WriteTerminalRequest) => { + ipcMain.handle(CHANNELS.write, async (event, request: WriteTerminalRequest) => { + assertTrustedSender(event); if (!request?.terminalId || !VALID_TERMINAL_ID.test(request.terminalId)) { throw new Error("terminal:write requires valid terminalId"); } @@ -109,7 +110,8 @@ export function registerTerminalIpc(manager: TerminalManager, coreClient?: CoreC await coreClient?.terminalWrite(request.terminalId, request.data); }); - ipcMain.handle(CHANNELS.resize, async (_event, request: ResizeTerminalRequest) => { + ipcMain.handle(CHANNELS.resize, async (event, request: ResizeTerminalRequest) => { + assertTrustedSender(event); if (!request?.terminalId || !VALID_TERMINAL_ID.test(request.terminalId)) { throw new Error("terminal:resize requires valid terminalId"); } @@ -120,7 +122,8 @@ export function registerTerminalIpc(manager: TerminalManager, coreClient?: CoreC await coreClient?.terminalResize(request.terminalId, request.cols, request.rows); }); - ipcMain.handle(CHANNELS.kill, async (_event, request: KillTerminalRequest) => { + ipcMain.handle(CHANNELS.kill, async (event, request: KillTerminalRequest) => { + assertTrustedSender(event); if (!request?.terminalId || !VALID_TERMINAL_ID.test(request.terminalId)) { throw new Error("terminal:kill requires valid terminalId"); } @@ -131,14 +134,16 @@ export function registerTerminalIpc(manager: TerminalManager, coreClient?: CoreC await coreClient?.terminalKill(request.terminalId); }); - ipcMain.handle(CHANNELS.list, async () => { + ipcMain.handle(CHANNELS.list, async (event) => { + assertTrustedSender(event); const local = manager.listTerminals(); if (!coreClient) return local; const core = await coreClient.terminalList(); return [...local, ...core.map((info) => toSnapshot(info))]; }); - ipcMain.handle(CHANNELS.listShells, async () => { + ipcMain.handle(CHANNELS.listShells, async (event) => { + assertTrustedSender(event); if (!coreClient) return manager.listShells(); try { const shells = await coreClient.terminalListShells();