fix(worker): contain workspace paths against symlinks
This commit is contained in:
@@ -0,0 +1,39 @@
|
||||
import { lstat, realpath } from 'node:fs/promises';
|
||||
import path from 'node:path';
|
||||
|
||||
const realpathOrDeepestExisting = async (target: string): Promise<string> => {
|
||||
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<string> => {
|
||||
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;
|
||||
};
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user