From 57c175aecd4a7edd37b511336b384474ffdce5b7 Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Fri, 10 Jul 2026 17:09:17 -0400 Subject: [PATCH] fix(worker): contain workspace paths against symlinks --- apps/agent-worker/src/path-containment.ts | 39 ++++++++++++++ apps/agent-worker/src/user-environment.ts | 15 ++---- apps/agent-worker/src/worker.ts | 22 ++++---- .../tests/unit/path-containment.test.ts | 54 +++++++++++++++++++ 4 files changed, 107 insertions(+), 23 deletions(-) create mode 100644 apps/agent-worker/src/path-containment.ts create mode 100644 apps/agent-worker/tests/unit/path-containment.test.ts diff --git a/apps/agent-worker/src/path-containment.ts b/apps/agent-worker/src/path-containment.ts new file mode 100644 index 0000000..f53550c --- /dev/null +++ b/apps/agent-worker/src/path-containment.ts @@ -0,0 +1,39 @@ +import { lstat, realpath } from 'node:fs/promises'; +import path from 'node:path'; + +const realpathOrDeepestExisting = async (target: string): Promise => { + let current = target; + const tail: string[] = []; + for (;;) { + try { + const real = await realpath(current); + return path.join(real, ...tail.reverse()); + } catch (error) { + if ((error as { code?: string }).code !== 'ENOENT') throw error; + const parent = path.dirname(current); + if (parent === current) throw error; + tail.push(path.basename(current)); + current = parent; + } + } +}; + +export const assertContainedRealPath = async ( + root: string, + requested: string, + opts: { forWrite?: boolean } = {}, +): Promise => { + const lexical = path.resolve(root, requested); + const realRoot = await realpath(root); + const resolved = await realpathOrDeepestExisting(lexical); + if (resolved !== realRoot && !resolved.startsWith(`${realRoot}${path.sep}`)) { + throw new Error(`Refusing to access path outside root: ${requested}`); + } + if (opts.forWrite) { + const info = await lstat(lexical).catch(() => null); + if (info?.isSymbolicLink()) { + throw new Error(`Refusing to write through a symlink: ${requested}`); + } + } + return resolved; +}; diff --git a/apps/agent-worker/src/user-environment.ts b/apps/agent-worker/src/user-environment.ts index 19f6a30..da81712 100644 --- a/apps/agent-worker/src/user-environment.ts +++ b/apps/agent-worker/src/user-environment.ts @@ -7,6 +7,7 @@ import type { Id } from '@spoon/backend/convex/_generated/dataModel.js'; import { api } from '@spoon/backend/convex/_generated/api.js'; import { env } from './env'; +import { assertContainedRealPath } from './path-containment'; import { runExecInContainer } from './runtime/docker'; const client = new ConvexHttpClient(env.convexUrl); @@ -31,16 +32,6 @@ export const fetchUserEnvironment = async ( const shellQuote = (value: string) => `'${value.replaceAll("'", "'\\''")}'`; -// Keep a written path inside the home directory. -const safeHomeJoin = (homeDir: string, relPath: string) => { - const target = path.resolve(homeDir, relPath); - const root = path.resolve(homeDir); - if (target !== root && !target.startsWith(`${root}${path.sep}`)) { - throw new Error(`Refusing to write dotfile outside home: ${relPath}`); - } - return target; -}; - /** * Materializes the persistent per-user home: a `.bash_profile` so login shells * load `~/.bashrc`; (when configured and changed) a clone of the public dotfiles @@ -111,7 +102,9 @@ export const materializeUserHome = async (args: { // Editable overlay tree (wins over the repo/setup output). for (const file of userEnv.files) { - const target = safeHomeJoin(homeDir, file.path); + const target = await assertContainedRealPath(homeDir, file.path, { + forWrite: true, + }); await mkdir(path.dirname(target), { recursive: true }); await writeFile(target, file.content); if (file.isExecutable) await chmod(target, 0o755); diff --git a/apps/agent-worker/src/worker.ts b/apps/agent-worker/src/worker.ts index 55b794a..a5f6f15 100644 --- a/apps/agent-worker/src/worker.ts +++ b/apps/agent-worker/src/worker.ts @@ -34,6 +34,7 @@ import { promptOpenCodeSession, replyOpenCodePermission, } from './opencode-session'; +import { assertContainedRealPath } from './path-containment'; import { createRedactor, truncate } from './redact'; import { listWorkspaceContainerNames, @@ -1058,15 +1059,6 @@ const resolveWorkspace = (jobId: string) => { return workspace; }; -const safeWorkspacePath = (repoDir: string, filePath: string) => { - const resolved = path.resolve(repoDir, filePath); - const root = path.resolve(repoDir); - if (resolved !== root && !resolved.startsWith(`${root}${path.sep}`)) { - throw new Error(`Refusing to access path outside repository: ${filePath}`); - } - return resolved; -}; - const fileChangedType = async (repoDir: string, filePath: string) => { const status = await run('git', ['status', '--short', '--', filePath], { cwd: repoDir, @@ -1169,7 +1161,11 @@ const recordChangedFiles = async ( const materializeEnvFile = async (workspace: ActiveWorkspace) => { const { claim, repoDir } = workspace; if (!claim.job.materializeEnvFile || !claim.job.envFilePath) return; - const envPath = safeWorkspacePath(repoDir, claim.job.envFilePath); + const envPath = await assertContainedRealPath( + repoDir, + claim.job.envFilePath, + { forWrite: true }, + ); await mkdir(path.dirname(envPath), { recursive: true }); const content = `${claim.secrets .map((secret) => `${secret.name}=${JSON.stringify(secret.value)}`) @@ -1503,7 +1499,7 @@ export const listWorkspaceTree = async (jobId: string) => { export const readWorkspaceFile = async (jobId: string, filePath: string) => { const workspace = resolveWorkspace(jobId); - const target = safeWorkspacePath(workspace.repoDir, filePath); + const target = await assertContainedRealPath(workspace.repoDir, filePath); return await readFile(target, 'utf8'); }; @@ -1513,7 +1509,9 @@ export const writeWorkspaceFile = async ( content: string, ) => { const workspace = resolveWorkspace(jobId); - const target = safeWorkspacePath(workspace.repoDir, filePath); + const target = await assertContainedRealPath(workspace.repoDir, filePath, { + forWrite: true, + }); await mkdir(path.dirname(target), { recursive: true }); await writeFile(target, content); const diff = await getWorktreeDiff(workspace.repoDir, workspace.redact); diff --git a/apps/agent-worker/tests/unit/path-containment.test.ts b/apps/agent-worker/tests/unit/path-containment.test.ts new file mode 100644 index 0000000..02112a3 --- /dev/null +++ b/apps/agent-worker/tests/unit/path-containment.test.ts @@ -0,0 +1,54 @@ +import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { afterEach, describe, expect, test } from 'vitest'; + +import { assertContainedRealPath } from '../../src/path-containment'; + +let dir: string; + +afterEach(async () => { + if (dir) await rm(dir, { recursive: true, force: true }); +}); + +describe('assertContainedRealPath', () => { + test('allows a normal file inside the root', async () => { + dir = await mkdtemp(path.join(tmpdir(), 'spoon-')); + const root = path.join(dir, 'repo'); + await mkdir(root, { recursive: true }); + await writeFile(path.join(root, 'a.txt'), 'hi'); + await expect(assertContainedRealPath(root, 'a.txt')).resolves.toBe( + path.join(root, 'a.txt'), + ); + }); + + test('rejects a lexical .. escape', async () => { + dir = await mkdtemp(path.join(tmpdir(), 'spoon-')); + const root = path.join(dir, 'repo'); + await mkdir(root, { recursive: true }); + await expect(assertContainedRealPath(root, '../secret')).rejects.toThrow(); + }); + + test('rejects reading through a symlink that escapes the root', async () => { + dir = await mkdtemp(path.join(tmpdir(), 'spoon-')); + const root = path.join(dir, 'repo'); + const outside = path.join(dir, 'outside'); + await mkdir(root, { recursive: true }); + await mkdir(outside, { recursive: true }); + await writeFile(path.join(outside, 'secret'), 'S'); + await symlink(outside, path.join(root, 'link')); + await expect( + assertContainedRealPath(root, 'link/secret'), + ).rejects.toThrow(); + }); + + test('rejects a write whose final target is a symlink', async () => { + dir = await mkdtemp(path.join(tmpdir(), 'spoon-')); + const root = path.join(dir, 'repo'); + await mkdir(root, { recursive: true }); + await symlink(path.join(dir, 'evil'), path.join(root, 'out')); + await expect( + assertContainedRealPath(root, 'out', { forWrite: true }), + ).rejects.toThrow(); + }); +});