Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions packages/runtime/src/__tests__/builtin-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2100,6 +2100,83 @@ describe('builtin write tools path containment', () => {
expect(await readFile(join(root, 'inside.txt'), 'utf8')).toBe('hello Maka');
});

test('file tools stay usable when the session cwd is reached through a symlink', async () => {
// The session cwd itself sits under a symlink (macOS hands out `/var/...`
// tmpdirs whose realpath is `/private/var/...`, and workspace roots are often
// organised with symlinks). Containment must be decided in one path space, so
// absolute paths spelled through the link are inside — and escapes still are not.
const base = await realpath(await mkdtemp(join(tmpdir(), 'maka-symlink-cwd-')));
const workspace = join(base, 'workspace');
const outside = join(base, 'outside');
await mkdir(join(workspace, 'src'), { recursive: true });
await mkdir(outside, { recursive: true });
await writeFile(join(workspace, 'src', 'inside.txt'), 'inside token\n', 'utf8');
await writeFile(join(outside, 'secret.txt'), 'secret\n', 'utf8');
await symlink(join(outside, 'secret.txt'), join(workspace, 'escape.txt'));
await symlink(outside, join(workspace, 'outside-link'));
const cwd = join(base, 'link-to-workspace');
await symlink(workspace, cwd);

const read = tool('Read');
const write = tool('Write');
const edit = tool('Edit');
const glob = tool('Glob');
const grep = tool('Grep');

expect(await runTool(read, { path: join(cwd, 'src', 'inside.txt') }, cwd)).toMatchObject({
content: 'inside token\n',
});
await runTool(write, { path: join(cwd, 'src', 'written.txt'), content: 'written\n' }, cwd);
expect(await readFile(join(workspace, 'src', 'written.txt'), 'utf8')).toBe('written\n');
await runTool(
edit,
{ path: join(cwd, 'src', 'inside.txt'), old_string: 'token', new_string: 'edited' },
cwd,
);
expect(await readFile(join(workspace, 'src', 'inside.txt'), 'utf8')).toBe('inside edited\n');
expect(await runTool(glob, { pattern: '*.txt', cwd: join(cwd, 'src') }, cwd)).toMatchObject({
files: ['inside.txt', 'written.txt'],
});
const grepResult = await runTool(grep, { pattern: 'edited', path: join(cwd, 'src') }, cwd);
expect(JSON.stringify(grepResult).includes('inside.txt')).toBe(true);

// Resolving into the canonical space must not turn "follow a symlink out of
// the workspace" into a legal path.
await expectRejects(
runTool(read, { path: join(outside, 'secret.txt') }, cwd),
/Read path must stay inside/,
);
await expectRejects(
runTool(read, { path: join(cwd, 'escape.txt') }, cwd),
/Read path must stay inside/,
);
await expectRejects(
runTool(write, { path: join(cwd, 'escape.txt'), content: 'x' }, cwd),
/Write path must stay inside/,
);
await expectRejects(
runTool(write, { path: join(cwd, 'outside-link', 'new.txt'), content: 'x' }, cwd),
/Write path must stay inside/,
);
// A link whose target does not exist yet cannot be realpath'd, but a write
// through it still lands outside the workspace, so it must be followed by
// hand rather than treated as a plain missing leaf.
await symlink(join(outside, 'not-yet.txt'), join(workspace, 'dangling-escape.txt'));
await expectRejects(
runTool(write, { path: join(cwd, 'dangling-escape.txt'), content: 'x' }, cwd),
/Write path must stay inside/,
);
await expectRejects(access(join(outside, 'not-yet.txt')), /ENOENT|no such file/);
await expectRejects(
runTool(grep, { pattern: 'secret', path: join(cwd, 'outside-link') }, cwd),
/Grep path must stay inside/,
);
await expectRejects(
runTool(glob, { pattern: '*.txt', cwd: join(cwd, 'outside-link') }, cwd),
/Glob cwd path must stay inside/,
);
});

test('concurrent Edits to the same file serialize — no lost update', async () => {
const root = await mkdtemp(join(tmpdir(), 'maka-edit-lock-'));
const n = 20;
Expand Down Expand Up @@ -2156,6 +2233,37 @@ describe('builtin write tools path containment', () => {
expect(await readFile(join(root, 'data.txt'), 'utf8')).toBe(expected);
});

test('concurrent Edits through a symlinked cwd serialize on one key', async () => {
const base = await realpath(await mkdtemp(join(tmpdir(), 'maka-edit-symlink-')));
const workspace = join(base, 'workspace');
await mkdir(workspace, { recursive: true });
const cwd = join(base, 'link-to-workspace');
await symlink(workspace, cwd);
const n = 20;
const markers = Array.from({ length: n }, (_, i) => `marker-${String(i).padStart(2, '0')}`);
await writeFile(join(workspace, 'data.txt'), `${markers.join('\n')}\n`, 'utf8');
const edit = tool('Edit');
// The relative spelling resolves against the canonical cwd while the absolute
// one is spelled through the link. Unless the lock key canonicalises both, the
// two groups take different locks and clobber each other.
const results = await Promise.all(
markers.map((m, i) =>
runTool(
edit,
{
path: i % 2 === 0 ? 'data.txt' : join(cwd, 'data.txt'),
old_string: m,
new_string: `done-${String(i).padStart(2, '0')}`,
},
cwd,
),
),
);
expect(results.every((r) => (r as { ok: boolean }).ok === true)).toBe(true);
const expected = `${Array.from({ length: n }, (_, i) => `done-${String(i).padStart(2, '0')}`).join('\n')}\n`;
expect(await readFile(join(workspace, 'data.txt'), 'utf8')).toBe(expected);
});

test('Write then Edit on one file resolves inside the lock — the fresh file is found', async () => {
const root = await mkdtemp(join(tmpdir(), 'maka-write-edit-'));
const write = tool('Write');
Expand Down
24 changes: 24 additions & 0 deletions packages/runtime/src/__tests__/filesystem-worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,30 @@ describe('filesystem worker operations', () => {
await assert.rejects(readFile(outsidePath, 'utf8'), { code: 'ENOENT' });
});

test('denies a write through a dangling symlink the boundary does not cover', async () => {
// The worker enforces its own boundary rather than trusting the caller to
// have canonicalised the path: a link inside the root whose target does not
// exist yet cannot be realpath'd, and a write through it lands on the
// target, outside the root, while the boundary names only the link.
const root = await temporaryDirectory('maka-worker-dangling-root-');
const outside = await temporaryDirectory('maka-worker-dangling-outside-');
const link = join(root, 'dangling.txt');
const target = join(outside, 'not-yet.txt');
await symlink(target, link);

const response = await executeFilesystemWorkerRequest(
requestFor(
{ kind: 'write', cwd: root, path: link, content: 'blocked' },
{ enforcementPath: target, access: 'write', scope: 'exact', targetType: 'missing' },
link,
),
);

assert.equal(response.ok, false);
if (!response.ok) assert.equal(response.error.code, 'path_denied');
await assert.rejects(readFile(target, 'utf8'), { code: 'ENOENT' });
});

test('fails when an approved target changes type before execution', async () => {
const root = await temporaryDirectory('maka-worker-type-');
const target = join(root, 'target');
Expand Down
105 changes: 102 additions & 3 deletions packages/runtime/src/__tests__/path-containment.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import { strict as assert } from 'node:assert';
import { win32, posix } from 'node:path';
import { describe, test } from 'node:test';
import { isPathInside } from '../path-containment.js';
import { mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join, win32, posix } from 'node:path';
import { afterEach, describe, test } from 'node:test';
import { isPathInside, realpathAllowMissing } from '../path-containment.js';

const cleanup: string[] = [];

afterEach(async () => {
await Promise.all(cleanup.splice(0).map((path) => rm(path, { recursive: true, force: true })));
});

describe('isPathInside', () => {
test('rejects cross-drive Windows targets (different drive is not inside root)', () => {
Expand Down Expand Up @@ -34,3 +42,94 @@ describe('isPathInside', () => {
assert.equal(isPathInside('C:\\repo', 'C:\\repo\\..rules\\AGENTS.md', win32), true);
});
});

describe('realpathAllowMissing', () => {
test('canonicalises an existing path through a symlinked ancestor', async () => {
const { workspace, link } = await symlinkedWorkspace('maka-realpath-existing-');
await writeFile(join(workspace, 'file.txt'), 'x', 'utf8');

assert.equal(await realpathAllowMissing(join(link, 'file.txt')), join(workspace, 'file.txt'));
});

test('appends missing trailing segments to the deepest existing ancestor', async () => {
const { workspace, link } = await symlinkedWorkspace('maka-realpath-missing-');

assert.equal(
await realpathAllowMissing(join(link, 'absent', 'nested', 'file.txt')),
join(workspace, 'absent', 'nested', 'file.txt'),
);
});

test('follows a dangling symlink to its target, which is where a write would land', async () => {
// realpath fails ENOENT on a link whose target does not exist. Treating it
// as a plain missing leaf returns the link's own path, which reads as
// contained while a write through it escapes.
const { base, workspace, link } = await symlinkedWorkspace('maka-realpath-dangling-');
const outside = join(base, 'outside');
await mkdir(outside);
await symlink(join(outside, 'not-yet.txt'), join(workspace, 'dangling.txt'));

assert.equal(
await realpathAllowMissing(join(link, 'dangling.txt')),
join(outside, 'not-yet.txt'),
);
});

test('follows a chain of dangling links and keeps the segments below them', async () => {
const { base, workspace, link } = await symlinkedWorkspace('maka-realpath-chain-');
const outside = join(base, 'outside');
await mkdir(outside);
await symlink(join(outside, 'absent-dir'), join(workspace, 'hop-b'));
await symlink(join(workspace, 'hop-b'), join(workspace, 'hop-a'));

assert.equal(
await realpathAllowMissing(join(link, 'hop-a', 'file.txt')),
join(outside, 'absent-dir', 'file.txt'),
);
});

test('treats a non-directory ancestor as missing rather than throwing ENOTDIR', async () => {
const { workspace, link } = await symlinkedWorkspace('maka-realpath-notdir-');
await writeFile(join(workspace, 'file.txt'), 'x', 'utf8');

assert.equal(
await realpathAllowMissing(join(link, 'file.txt', 'child')),
join(workspace, 'file.txt', 'child'),
);
});

test('rejects a pathological dangling chain instead of walking it forever', async () => {
const { workspace } = await symlinkedWorkspace('maka-realpath-hops-');
// Every link resolves to the next and only the last one dangles. The kernel
// caps its own symlink traversal well below this length, so it answers ELOOP
// before the helper's hop cap can fire; either way the walk must terminate
// by rejecting rather than by hopping forever.
const chain = 64;
await symlink(join(workspace, 'absent'), join(workspace, `hop-${chain}`));
for (let index = chain; index > 0; index -= 1) {
await symlink(join(workspace, `hop-${index}`), join(workspace, `hop-${index - 1}`));
}

await assert.rejects(realpathAllowMissing(join(workspace, 'hop-0')), /ELOOP|too many/);
});

test('propagates a symlink cycle instead of walking forever', async () => {
const { workspace } = await symlinkedWorkspace('maka-realpath-cycle-');
await symlink(join(workspace, 'b'), join(workspace, 'a'));
await symlink(join(workspace, 'a'), join(workspace, 'b'));

await assert.rejects(realpathAllowMissing(join(workspace, 'a')), { code: 'ELOOP' });
});
});

async function symlinkedWorkspace(
prefix: string,
): Promise<{ base: string; workspace: string; link: string }> {
const base = await realpath(await mkdtemp(join(tmpdir(), prefix)));
cleanup.push(base);
const workspace = join(base, 'workspace');
const link = join(base, 'link-to-workspace');
await mkdir(workspace);
await symlink(workspace, link);
return { base, workspace, link };
}
27 changes: 11 additions & 16 deletions packages/runtime/src/filesystem-worker/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { spawn } from 'node:child_process';
import { promises as fs } from 'node:fs';
import { glob as nodeGlob } from 'node:fs/promises';
import { dirname, isAbsolute, parse, resolve } from 'node:path';
import { isPathInside } from '../path-containment.js';
import { isPathInside, realpathAllowMissing } from '../path-containment.js';
import { sandboxBoundaryExpansionAllowsPath } from '@maka/core';

import { computeEditedSource } from '../edit-replace.js';
Expand Down Expand Up @@ -316,15 +316,21 @@ async function resolveWritableAllowed(
} catch (error) {
if (nodeErrorCode(error) !== 'ENOENT') throw error;
}
const parent = await fs.realpath(dirname(candidate));
assertAllowed(root, candidate, label, 'write', permission);
if (!isPathInside(root, parent) && !exactWriteCoversParent(permission, candidate, parent)) {
// The target does not exist, but it can still be a dangling symlink, and a
// write lands on what the link names rather than on the link. Authorise the
// followed path — the same one `assertTargetUnchanged` pins the request to —
// so the worker enforces its own boundary instead of trusting the caller to
// have canonicalised the path for it.
const followed = await realpathAllowMissing(candidate);
const parent = await fs.realpath(dirname(followed));
assertAllowed(root, followed, label, 'write', permission);
if (!isPathInside(root, parent) && !exactWriteCoversParent(permission, followed, parent)) {
throw operationError(
'path_denied',
`${label} parent was not covered by the operation boundary.`,
);
}
return candidate;
return followed;
}

async function resolveExistingAllowed(
Expand Down Expand Up @@ -392,17 +398,6 @@ function assertContainedGlobPattern(pattern: string): void {
}
}

async function realpathAllowMissing(path: string): Promise<string> {
try {
return await fs.realpath(path);
} catch (error) {
if (nodeErrorCode(error) !== 'ENOENT') throw error;
const parent = dirname(path);
if (parent === path) throw error;
return resolve(await realpathAllowMissing(parent), path.slice(parent.length + 1));
}
}

async function targetTypeOf(path: string): Promise<FilesystemWorkerTarget['targetType']> {
try {
const metadata = await fs.stat(path);
Expand Down
Loading
Loading