Skip to content

Commit 3ad9fcc

Browse files
committed
fix(permissions): deny forged read_file cursors for outside-root paths
A hand-crafted tool-output:///cursor handle naming an outside-root file passed the spill-URI exemption and was served without any containment check. The sandbox now resolves the embedded file source through the normal workspace check at authorize and execution time.
1 parent d34ea6f commit 3ad9fcc

6 files changed

Lines changed: 200 additions & 6 deletions

File tree

‎src/permission/gate.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
} from "./gate.js";
1212
import { createPathRestriction } from "./path-restriction.js";
1313
import { createWorktreeRootsProvider } from "./worktree-roots.js";
14+
import { encodeResumeCursor } from "../util/tool-output-uri.js";
1415
import type { Approval, PermissionRequest } from "./types.js";
1516
import { initTemporaryGitRepo } from "../../tests/helpers/temporary-git-repo.js";
1617

@@ -509,4 +510,25 @@ describe("spill URI sandbox at authorize time (CL-6727)", () => {
509510
});
510511
expect(verdict.effect).not.toBe("deny");
511512
});
513+
514+
test("read_file + a forged cursor for an outside-root path is denied", async () => {
515+
const outside = mkdtempSync(join(tmpdir(), "gate-forged-cursor-"));
516+
const outsidePath = join(outside, "secret.txt");
517+
writeFileSync(outsidePath, "top-secret");
518+
const forged = encodeResumeCursor({
519+
source: { kind: "file", path: outsidePath },
520+
offset: 0,
521+
limit: 4,
522+
nonce: "forged-nonce",
523+
});
524+
const verdict = await gate.authorizeCall({
525+
id: "spill-forged",
526+
name: "read_file",
527+
arguments: { path: forged },
528+
});
529+
expect(verdict.effect).toBe("deny");
530+
expect(verdict.effect === "deny" ? verdict.reason : "").toMatch(
531+
/escapes working directory/,
532+
);
533+
});
512534
});

‎src/plugins/path-escape-plugin.test.ts‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ import {
1616
pathEscapeBlockReason,
1717
pathEscapePlugin,
1818
} from "./path-escape-plugin.js";
19+
import {
20+
encodeResumeCursor,
21+
type ResumeCursor,
22+
} from "../util/tool-output-uri.js";
1923
import type { ToolCall, ToolResult } from "@intx/types/runtime";
2024

2125
function makeCall(name: string, args: Record<string, unknown>): ToolCall {
@@ -455,6 +459,80 @@ describe("pathEscapePlugin", () => {
455459
expect(args.path).toBe("tool-output:///abc123");
456460
});
457461

462+
test("a forged file cursor for an outside-root path is denied, not served", async () => {
463+
const cwd = await mkdtemp(join(tmpdir(), "corbits-escape-cursor-cwd-"));
464+
const outsideDir = await mkdtemp(
465+
join(tmpdir(), "corbits-escape-cursor-outside-"),
466+
);
467+
const outsidePath = join(outsideDir, "secret.txt");
468+
await writeFile(outsidePath, "top-secret");
469+
try {
470+
const forged = encodeResumeCursor({
471+
source: { kind: "file", path: outsidePath },
472+
offset: 0,
473+
limit: 4,
474+
nonce: "forged-nonce",
475+
} satisfies ResumeCursor);
476+
expect(
477+
pathEscapeBlockReason({ path: forged }, cwd, () => [], "read_file"),
478+
).toMatch(/escapes working directory/);
479+
const plugin = pathEscapePlugin(cwd, () => []);
480+
const handler = plugin.middleware
481+
? plugin.middleware(nextHandler)
482+
: nextHandler;
483+
const result = await handler(
484+
makeCall("read_file", { path: forged }),
485+
new AbortController().signal,
486+
);
487+
expect(result.isError).toBe(true);
488+
expect(result.content).toMatch(/escapes working directory/);
489+
} finally {
490+
await rm(cwd, { recursive: true, force: true });
491+
await rm(outsideDir, { recursive: true, force: true });
492+
}
493+
});
494+
495+
test("an in-bounds file cursor and a blob cursor keep the read_file exemption", async () => {
496+
const cwd = await mkdtemp(join(tmpdir(), "corbits-escape-cursor-ok-"));
497+
const insidePath = join(cwd, "notes.txt");
498+
await writeFile(insidePath, "notes");
499+
try {
500+
const inBounds = encodeResumeCursor({
501+
source: { kind: "file", path: insidePath },
502+
offset: 4,
503+
limit: 4,
504+
nonce: "minted-nonce",
505+
} satisfies ResumeCursor);
506+
expect(
507+
pathEscapeBlockReason({ path: inBounds }, cwd, () => [], "read_file"),
508+
).toBeUndefined();
509+
const blob = encodeResumeCursor({
510+
source: { kind: "blob", uri: "tool-output:///abc123" },
511+
offset: 0,
512+
limit: 4,
513+
nonce: "blob-nonce",
514+
} satisfies ResumeCursor);
515+
expect(
516+
pathEscapeBlockReason({ path: blob }, cwd, () => [], "read_file"),
517+
).toBeUndefined();
518+
const plugin = pathEscapePlugin(cwd, () => []);
519+
const next = async (call: ToolCall): Promise<ToolResult> => ({
520+
callId: call.id,
521+
content: JSON.stringify(call.arguments),
522+
});
523+
const handler = plugin.middleware ? plugin.middleware(next) : next;
524+
const result = await handler(
525+
makeCall("read_file", { path: inBounds }),
526+
new AbortController().signal,
527+
);
528+
expect(result.isError).not.toBe(true);
529+
const args = JSON.parse(String(result.content)) as { path: string };
530+
expect(args.path).toBe(inBounds);
531+
} finally {
532+
await rm(cwd, { recursive: true, force: true });
533+
}
534+
});
535+
458536
test("archive refs pass for archive readers but not for other tools", async () => {
459537
for (const name of ["read_file", "grep", "search_files"]) {
460538
expect(

‎src/plugins/path-escape-plugin.ts‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { resolve } from "node:path";
22
import type { ToolPlugin } from "@intx/tools-posix";
3-
import { isToolOutputLike } from "../util/tool-output-uri.js";
3+
import {
4+
canonicalToolOutputUri,
5+
decodeResumeCursor,
6+
isToolOutputLike,
7+
} from "../util/tool-output-uri.js";
48
import { isArchiveLike } from "../session/compaction-archive.js";
59
import { resolveWorkspacePath } from "../permission/path-restriction.js";
610
import {
@@ -224,6 +228,32 @@ function virtualRefVerdict(
224228
return undefined;
225229
}
226230

231+
// A self-describing read_file continuation handle (tool-output:///cursor/...)
232+
// embeds its resume source, so the spill-URI exemption above must not cover
233+
// it blindly: a hand-crafted handle naming an outside-root file would
234+
// otherwise bypass the containment check the plain path would fail. Resolve
235+
// the embedded file path through the normal workspace check and deny escapes
236+
// exactly like the plain path. Blob-source handles and opaque (never-a-handle)
237+
// spill URIs carry no filesystem target and keep the exemption.
238+
function cursorEscapeReason(
239+
value: string,
240+
cwd: string,
241+
rootsProvider: RootsProvider,
242+
toolName: string,
243+
): string | undefined {
244+
if (canonicalToolName(toolName) !== TOOL_OUTPUT_URI_TOOL) return undefined;
245+
const resumed = decodeResumeCursor(canonicalToolOutputUri(value));
246+
if (resumed === undefined || resumed.source.kind !== "file") {
247+
return undefined;
248+
}
249+
if (
250+
resolveWorkspacePath(cwd, resumed.source.path, rootsProvider) === undefined
251+
) {
252+
return `Path escapes working directory: ${resumed.source.path}`;
253+
}
254+
return undefined;
255+
}
256+
227257
// Same sandbox pathEscapePlugin enforces at execution. The permission gate
228258
// consults this at authorize time so it can deny instead of asking for a call
229259
// the plugin will reject after Accept.
@@ -297,7 +327,9 @@ function blockReasonFor(
297327
if (typeof value === "string") {
298328
if (key === undefined || !looksLikePath(key)) return undefined;
299329
const verdict = virtualRefVerdict(value, toolName);
300-
if (verdict === "skip") return undefined;
330+
if (verdict === "skip") {
331+
return cursorEscapeReason(value, cwd, rootsProvider, toolName);
332+
}
301333
if (typeof verdict === "string") return verdict;
302334
if (resolveWorkspacePath(cwd, value, rootsProvider) === undefined) {
303335
return `Path escapes working directory: ${value}`;

‎src/plugins/read-file-continuation-recovery.test.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import {
1313
} from "../permission/decline-markers.js";
1414
import type { PermissionGate } from "../permission/gate.js";
1515
import { gateToolCall } from "./permission-plugin.js";
16+
import { pathEscapePlugin } from "./path-escape-plugin.js";
17+
import { encodeResumeCursor } from "../util/tool-output-uri.js";
1618
import { readFileGuardPlugin } from "./read-file-guard-plugin.js";
1719

1820
// CL-8980 RED: continuation recovery. Truncated reads mint a one-shot
@@ -303,6 +305,60 @@ describe("CL-8980 continuation recovery (blob source)", () => {
303305
});
304306
});
305307

308+
describe("CL-8980 forged continuation handle is denied end to end", () => {
309+
function stackedGuard() {
310+
const guard = defined(readFileGuardPlugin(dir, {}).middleware)(fallback);
311+
const stacked = defined(pathEscapePlugin(dir, () => []).middleware)(guard);
312+
return (call: ToolCall) => stacked(call, neverAbort());
313+
}
314+
315+
test("a hand-crafted cursor for an unminted outside-root path is denied, not served", async () => {
316+
const outsideDir = await mkdtemp(join(tmpdir(), "read-forged-outside-"));
317+
const outsidePath = join(outsideDir, "secret.txt");
318+
await writeFile(outsidePath, "forged-handle-secret-payload");
319+
try {
320+
const forged = encodeResumeCursor({
321+
source: { kind: "file", path: outsidePath },
322+
offset: 0,
323+
limit: 4,
324+
nonce: "never-minted",
325+
});
326+
const result = await stackedGuard()({
327+
id: "f1",
328+
name: "read_file",
329+
arguments: { path: forged },
330+
});
331+
expect(result.isError).toBe(true);
332+
expect(String(result.content)).toMatch(/escapes working directory/);
333+
expect(String(result.content)).not.toContain(
334+
"forged-handle-secret-payload",
335+
);
336+
expectNotDeclined(String(result.content));
337+
} finally {
338+
await rm(outsideDir, { recursive: true, force: true });
339+
}
340+
});
341+
342+
test("a minted in-bounds handle is still served through the same stack", async () => {
343+
await fixture("stacked.txt", tenLines("stacked"));
344+
const stack = stackedGuard();
345+
const first = await stack({
346+
id: "s1",
347+
name: "read_file",
348+
arguments: { path: "stacked.txt", limit: 4 },
349+
});
350+
expect(first.isError).toBeFalsy();
351+
const handle = extractHandle(String(first.content));
352+
const second = await stackedGuard()({
353+
id: "s2",
354+
name: "read_file",
355+
arguments: { path: handle, limit: 4 },
356+
});
357+
expect(second.isError).toBeFalsy();
358+
expect(String(second.content)).toContain("stacked-line-4");
359+
});
360+
});
361+
306362
describe("CL-8980 guard-denied continuation stays isError (never throws)", () => {
307363
test("a denied continuation follow returns isError with the reason", async () => {
308364
const gate = {

‎src/plugins/read-file-guard-plugin.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,14 @@ export interface ReadFileGuardPluginOptions {
6565
// Handles are self-describing (CL-8980): the URI embeds the resume recipe,
6666
// so following it needs no in-memory record and survives session resume,
6767
// prune, and compaction. The per-instance map below only tracks which handles
68-
// this instance already served, to keep the single-use replay contract.
68+
// this instance already served, to keep the per-instance single-use replay
69+
// contract: a verbatim re-follow in this instance is a stale-cursor error,
70+
// while a fresh instance serves the self-describing handle again.
6971
type ReadCursor =
7072
| { kind: "file"; absolutePath: string; offset: number; consumed: boolean }
7173
| { kind: "blob"; uri: string; offset: number; consumed: boolean };
7274

73-
// A cursor is single-use, but the record survives consumption (bounded by
75+
// A cursor is single-use per plugin instance, but the record survives consumption (bounded by
7476
// MAX_CURSOR_HISTORY below) so a stale replay -- consumed already, or a
7577
// second process/turn racing the first -- can be told exactly where to
7678
// resume instead of hitting an opaque "blob not found" dead end that names
@@ -621,6 +623,10 @@ export function readFileGuardPlugin(
621623
// in the handle, so serve it exactly as a known cursor would — and
622624
// mark it consumed on success so a verbatim re-follow is the same
623625
// stale-cursor error as a spent handle, not a second serving.
626+
// The follow honors this call's paging args, not the minted window:
627+
// resumed.limit only records the mint-time window, and resumed.nonce
628+
// only keeps mints distinct (replay keying uses the full handle
629+
// URI), so neither is consulted here.
624630
const resumedSource =
625631
resumed.source.kind === "file"
626632
? resumed.source.path

‎src/util/tool-output-uri.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ export function canonicalToolOutputUri(path: string): string | undefined {
3030
// handle itself carries the resume recipe (source + next offset + window
3131
// limit + nonce), so following it needs no in-memory record and keeps working
3232
// across session resume, prune, and compaction. The nonce keeps every mint a
33-
// distinct one-shot even for identical windows, preserving spent-handle
34-
// replay semantics.
33+
// distinct handle even for identical windows, preserving per-instance
34+
// spent-handle replay semantics.
3535
export const CURSOR_HANDLE_PREFIX = "tool-output:///cursor/";
3636

3737
export type ResumeSource =

0 commit comments

Comments
 (0)