indifferentketchup
d0334ca544
The ACP fs bridge's worktree guard used an unbounded `startsWith(resolve( worktreePath))`, so a sibling path sharing the worktree as a string prefix (`<worktree>-evil/...`) escaped the scope. Since writeWorktreeTextFile hits disk directly (no pending_changes gate), a confused/buggy ACP agent could write outside its worktree. Now uses a separator-bounded check matching write_guard.ts (resolve() + `startsWith(root + sep)` / `=== root`) via a shared resolveInWorktree, with a regression test (../ traversal + the sibling-prefix bug). Symlink-swap hardening intentionally skipped — consistent with write_guard's no-realpath stance; the agent runs with host FS access so this is a containment guard, not a trust boundary. Flagged by the automated push security review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
51 lines
1.8 KiB
TypeScript
51 lines
1.8 KiB
TypeScript
import { describe, it, expect, afterEach } from 'vitest';
|
|
import { mkdtempSync, rmSync } from 'node:fs';
|
|
import { tmpdir } from 'node:os';
|
|
import { join } from 'node:path';
|
|
import { readWorktreeTextFile, writeWorktreeTextFile } from '../acp-client-fs.js';
|
|
|
|
const created: string[] = [];
|
|
function freshWorktree(): string {
|
|
const wt = mkdtempSync(join(tmpdir(), 'acp-wt-'));
|
|
created.push(wt);
|
|
return wt;
|
|
}
|
|
afterEach(() => {
|
|
for (const d of created.splice(0)) {
|
|
try {
|
|
rmSync(d, { recursive: true, force: true });
|
|
rmSync(`${d}-evil`, { recursive: true, force: true });
|
|
} catch {
|
|
/* ignore */
|
|
}
|
|
}
|
|
});
|
|
|
|
describe('acp-client-fs worktree scoping', () => {
|
|
it('writes then reads a file inside the worktree', async () => {
|
|
const wt = freshWorktree();
|
|
await writeWorktreeTextFile(wt, 'sub/dir/note.txt', 'hello');
|
|
expect(await readWorktreeTextFile(wt, 'sub/dir/note.txt')).toBe('hello');
|
|
});
|
|
|
|
it('rejects ../ traversal on read', async () => {
|
|
const wt = freshWorktree();
|
|
await expect(readWorktreeTextFile(wt, '../../etc/passwd')).rejects.toThrow(/escapes worktree/);
|
|
});
|
|
|
|
it('rejects ../ traversal on write', async () => {
|
|
const wt = freshWorktree();
|
|
await expect(writeWorktreeTextFile(wt, '../escape.txt', 'x')).rejects.toThrow(/escapes worktree/);
|
|
});
|
|
|
|
it('rejects a sibling-prefix path (the unbounded-startsWith bug)', async () => {
|
|
const wt = freshWorktree();
|
|
// Absolute path that shares the worktree as a STRING prefix but is a sibling
|
|
// dir: `<wt>-evil/...`. A bare `startsWith(<wt>)` wrongly admits it.
|
|
await expect(readWorktreeTextFile(wt, `${wt}-evil/secret.txt`)).rejects.toThrow(/escapes worktree/);
|
|
await expect(writeWorktreeTextFile(wt, `${wt}-evil/secret.txt`, 'x')).rejects.toThrow(
|
|
/escapes worktree/,
|
|
);
|
|
});
|
|
});
|