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
5 changes: 2 additions & 3 deletions src/permission/gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -719,9 +719,8 @@ export function createPermissionGate(
if (cachedDenial !== undefined)
return { kind: "deny", reason: cachedDenial };

// Path-escape will hard-reject these at execution; deny here rather than
// showing Accept for a call that cannot succeed. Workers sandbox against
// their own cwd's roots, not the session listing.
// Hard-deny workspace escapes here, before allow and before ask.
// Workers sandbox against their own cwd's roots, not the session listing.
const escapeRoots =
effectiveCwd === resolvedCwd
? rootsProvider
Expand Down
18 changes: 14 additions & 4 deletions src/permission/permission.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ import {
import type { Approval, ApprovalOutcome, PermissionRequest } from "./types.js";
import { initTemporaryGitRepo } from "../../testkit/temporary-git-repo.js";
import { secretGuardPlugin } from "../plugins/secret-guard-plugin.js";
import { pathEscapePlugin } from "../plugins/path-escape-plugin.js";
import {
pathEscapePlugin,
pathEscapeBlockReason,
} from "../plugins/path-escape-plugin.js";

const shellCall = (command: string): ToolCall =>
toolCall("run_shell", { command });
Expand Down Expand Up @@ -2668,7 +2671,7 @@ describe("listWorktreeRoots", () => {
expect(result.isError).not.toBe(true);
});

test("pathEscapePlugin still rejects a relative path into a genuinely unrelated directory", async () => {
test("the gate denies a relative path into a genuinely unrelated directory", async () => {
const { repo } = createRepoWithWorktree();
const roots = await listWorktreeRoots(repo);
const outside = mkdtempSync(join(tmpdir(), "intercode-unrelated-plugin-"));
Expand All @@ -2681,8 +2684,15 @@ describe("listWorktreeRoots", () => {
toolCall("read_file", { path: relativeToOutside }),
new AbortController().signal,
);
expect(result.isError).toBe(true);
expect(result.content).toMatch(/escapes working directory/);
expect(result.isError).not.toBe(true);
expect(
pathEscapeBlockReason(
{ path: relativeToOutside },
repo,
() => roots,
"read_file",
),
).toMatch(/escapes working directory/);
});
});

Expand Down
105 changes: 68 additions & 37 deletions src/plugins/path-escape-plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,26 +44,31 @@ describe("pathEscapePlugin", () => {
["source", "copy", "/etc/passwd"],
["filename", "write_file", "../secret.txt"],
] as const) {
test(`blocks escape via ${key} key (${tool})`, async () => {
test(`leaves an outside ${key} unchanged; the gate denies ${tool}`, async () => {
const plugin = pathEscapePlugin("/project");
const handler = pluginHandler(plugin, okHandler);
const handler = pluginHandler(plugin, echoArgsHandler);
const result = await handler(
makeToolCall(tool, { [key]: value }),
neverAbort(),
);
expect(result.isError).toBe(true);
expect(result.isError).not.toBe(true);
const args = JSON.parse(String(result.content)) as Record<string, string>;
expect(args[key]).toBe(value);
expect(
pathEscapeBlockReason({ [key]: value }, "/project", () => [], tool),
).toMatch(/escapes working directory/);
});
}

test("the block message tells the operator the path escapes the working directory", async () => {
const plugin = pathEscapePlugin("/project");
const handler = pluginHandler(plugin, okHandler);
const result = await handler(
makeToolCall("read_file", { path: "../secret.txt" }),
neverAbort(),
);
expect(result.isError).toBe(true);
expect(result.content).toMatch(/escapes working directory/);
test("the gate denial tells the operator the path escapes the working directory", () => {
expect(
pathEscapeBlockReason(
{ path: "../secret.txt" },
"/project",
() => [],
"read_file",
),
).toMatch(/escapes working directory/);
});

test("allows cwd path itself", async () => {
Expand Down Expand Up @@ -114,8 +119,17 @@ describe("pathEscapePlugin", () => {
makeToolCall("read_file", { path: "../other-repo/README.md" }),
neverAbort(),
);
expect(blocked.isError).toBe(true);
expect(blocked.content).toMatch(/escapes working directory/);
expect(blocked.isError).not.toBe(true);
const blockedArgs = JSON.parse(String(blocked.content)) as { path: string };
expect(blockedArgs.path).toBe("../other-repo/README.md");
expect(
pathEscapeBlockReason(
{ path: "../other-repo/README.md" },
"/project",
() => [],
"read_file",
),
).toMatch(/escapes working directory/);

allow = true;
const allowed = await handler(
Expand Down Expand Up @@ -204,15 +218,20 @@ describe("pathEscapePlugin", () => {
return { next, seen: () => seen };
};

test("blocks escape in a nested object under a path-like key", async () => {
test("passes a nested escape through; the gate denies it", async () => {
const args = { options: { path: "../secret.txt" } };
const plugin = pathEscapePlugin("/project");
const handler = pluginHandler(plugin, okHandler);
const { next, seen } = captureNext();
const handler = pluginHandler(plugin, next);
const result = await handler(
makeToolCall("read_file", { options: { path: "../secret.txt" } }),
makeToolCall("read_file", args),
neverAbort(),
);
expect(result.isError).toBe(true);
expect(result.content).toMatch(/escapes working directory/);
expect(result.isError).not.toBe(true);
expect(seen()).toEqual(args);
expect(
pathEscapeBlockReason(args, "/project", () => [], "read_file"),
).toMatch(/escapes working directory/);
});

test("resolves nested in-bounds paths instead of passing them through", async () => {
Expand All @@ -229,28 +248,38 @@ describe("pathEscapePlugin", () => {
});
});

test("blocks escape via the filepath spelling", async () => {
test("passes an escape via the filepath spelling; the gate denies it", async () => {
const args = { filepath: "../secret.txt" };
const plugin = pathEscapePlugin("/project");
const handler = pluginHandler(plugin, okHandler);
const { next, seen } = captureNext();
const handler = pluginHandler(plugin, next);
const result = await handler(
makeToolCall("read_file", { filepath: "../secret.txt" }),
makeToolCall("read_file", args),
neverAbort(),
);
expect(result.isError).toBe(true);
expect(result.content).toMatch(/escapes working directory/);
expect(result.isError).not.toBe(true);
expect(seen()).toEqual(args);
expect(
pathEscapeBlockReason(args, "/project", () => [], "read_file"),
).toMatch(/escapes working directory/);
});

test("blocks escape in a string array under a path-like key", async () => {
test("passes an escape in a string array; the gate denies it", async () => {
const args = { paths: ["src/index.ts", "../secret.txt"] };
const plugin = pathEscapePlugin("/project");
const handler = pluginHandler(plugin, okHandler);
const { next, seen } = captureNext();
const handler = pluginHandler(plugin, next);
const result = await handler(
makeToolCall("read_file", {
paths: ["src/index.ts", "../secret.txt"],
}),
makeToolCall("read_file", args),
neverAbort(),
);
expect(result.isError).toBe(true);
expect(result.content).toMatch(/escapes working directory/);
expect(result.isError).not.toBe(true);
expect(seen()).toEqual({
paths: ["/project/src/index.ts", "../secret.txt"],
});
expect(
pathEscapeBlockReason(args, "/project", () => [], "read_file"),
).toMatch(/escapes working directory/);
});

test("pathEscapeBlockReason agrees with execution time on nested escapes", async () => {
Expand Down Expand Up @@ -302,14 +331,15 @@ describe("pathEscapePlugin", () => {

test("FILE_PATH and file-path match case- and separator-insensitively", async () => {
const plugin = pathEscapePlugin("/project");
const handler = pluginHandler(plugin, okHandler);
const { next, seen } = captureNext();
const handler = pluginHandler(plugin, next);
for (const key of ["FILE_PATH", "file-path"]) {
const result = await handler(
makeToolCall("read_file", { [key]: "../secret.txt" }),
neverAbort(),
);
expect(result.isError).toBe(true);
expect(result.content).toMatch(/escapes working directory/);
expect(result.isError).not.toBe(true);
expect(seen()).toEqual({ [key]: "../secret.txt" });
expect(
pathEscapeBlockReason(
{ [key]: "../secret.txt" },
Expand Down Expand Up @@ -529,13 +559,14 @@ describe("pathEscapePlugin", () => {
const plugin = pathEscapePlugin(cwd, () => [], {
trustedPluginRoots: () => [pluginDir],
});
const handler = pluginHandler(plugin, okHandler);
const handler = pluginHandler(plugin, echoArgsHandler);
const result = await handler(
makeToolCall("write_file", { path: target, content: "x" }),
neverAbort(),
);
expect(result.isError).toBe(true);
expect(result.content).toMatch(/escapes working directory/);
expect(result.isError).not.toBe(true);
const args = JSON.parse(String(result.content)) as { path: string };
expect(args.path).toBe(target);
expect(
pathEscapeBlockReason(
{ path: target, content: "x" },
Expand Down
16 changes: 9 additions & 7 deletions src/plugins/path-escape-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ function escapeArgs(
toolName: string,
trustedPluginRoots?: RootsProvider,
): Record<string, unknown> {
// One fused walk: sanitizePath throws the same deny strings
// pathEscapeBlockReason computes, so a surviving call is exactly one the
// block-reason leg would have passed.
// Virtual-ref denials still throw. Workspace escapes pass through unchanged
// when allowOutside is false; the permission gate denies them via
// pathEscapeBlockReason before allow and before ask.
const roots = rootsForEscape(toolName, rootsProvider, trustedPluginRoots);
return walkPathArgs(args, undefined, (value) =>
sanitizePath(value, cwd, roots, allowOutside, toolName),
Expand Down Expand Up @@ -200,9 +200,9 @@ export function virtualRefVerdict(
return undefined;
}

// Same sandbox pathEscapePlugin enforces at execution. The permission gate
// consults this at authorize time so it can deny instead of asking for a call
// the plugin will reject after Accept.
// The permission gate consults this before allow and before ask so an
// out-of-workspace path is denied instead of prompted. sanitizePath does not
// throw a second copy of that denial.
export function pathEscapeBlockReason(
args: Record<string, unknown>,
cwd: string,
Expand Down Expand Up @@ -297,5 +297,7 @@ function sanitizePath(
// so later plugins see a stable path, not a relative escape fragment.
return resolve(cwd, value);
}
throw new Error(`Path escapes working directory: ${value}`);
// The gate already hard-denies this escape. Leave the original path so a
// later permission check sees the same value the operator sent.
return value;
}
Loading