From 07ef8ff2d48b4856f4abb8c62489420742543caa Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 13 Sep 2026 13:56:54 -0700 Subject: [PATCH 1/3] Add failing tests for nested and alternate path keys --- src/plugins/path-escape-plugin.test.ts | 88 ++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/src/plugins/path-escape-plugin.test.ts b/src/plugins/path-escape-plugin.test.ts index 9734eb624..bf0bff306 100644 --- a/src/plugins/path-escape-plugin.test.ts +++ b/src/plugins/path-escape-plugin.test.ts @@ -264,4 +264,92 @@ describe("pathEscapePlugin", () => { await rm(outside, { recursive: true, force: true }); }); }); + + describe("nested and alternate path keys (CL-6730)", () => { + const captureNext = () => { + let seen: Record = {}; + const next = async (call: ToolCall): Promise => { + seen = call.arguments as Record; + return { + callId: call.id, + content: JSON.stringify(call.arguments), + }; + }; + return { next, seen: () => seen }; + }; + + test("blocks escape in a nested object under a path-like key", async () => { + const plugin = pathEscapePlugin("/project"); + const handler = plugin.middleware + ? plugin.middleware(nextHandler) + : nextHandler; + const result = await handler( + makeCall("read_file", { options: { path: "../secret.txt" } }), + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(result.content).toMatch(/escapes working directory/); + }); + + test("resolves nested in-bounds paths instead of passing them through", async () => { + const plugin = pathEscapePlugin("/project"); + const { next, seen } = captureNext(); + const handler = plugin.middleware ? plugin.middleware(next) : next; + const result = await handler( + makeCall("read_file", { options: { path: "src/index.ts" } }), + new AbortController().signal, + ); + expect(result.isError).not.toBe(true); + expect(seen()).toEqual({ + options: { path: "/project/src/index.ts" }, + }); + }); + + test("blocks escape via the filepath spelling", async () => { + const plugin = pathEscapePlugin("/project"); + const handler = plugin.middleware + ? plugin.middleware(nextHandler) + : nextHandler; + const result = await handler( + makeCall("read_file", { filepath: "../secret.txt" }), + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(result.content).toMatch(/escapes working directory/); + }); + + test("blocks escape in a string array under a path-like key", async () => { + const plugin = pathEscapePlugin("/project"); + const handler = plugin.middleware + ? plugin.middleware(nextHandler) + : nextHandler; + const result = await handler( + makeCall("read_file", { + paths: ["src/index.ts", "../secret.txt"], + }), + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(result.content).toMatch(/escapes working directory/); + }); + + test("pathEscapeBlockReason agrees with execution time on nested escapes", async () => { + const { pathEscapeBlockReason } = await import("./path-escape-plugin.js"); + expect( + pathEscapeBlockReason( + { options: { path: "../secret.txt" } }, + "/project", + ), + ).toMatch(/escapes working directory/); + expect( + pathEscapeBlockReason({ filepath: "../secret.txt" }, "/project"), + ).toMatch(/escapes working directory/); + expect( + pathEscapeBlockReason( + { paths: ["src/index.ts", "../secret.txt"] }, + "/project", + ), + ).toMatch(/escapes working directory/); + }); + }); }); From 5ca559f9de67557126161a63c59b828b634e84e9 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 13 Sep 2026 13:56:55 -0700 Subject: [PATCH 2/3] Sanitize nested objects, arrays, and alternate path-key spellings --- src/plugins/path-escape-plugin.ts | 107 ++++++++++++++++++++++++------ 1 file changed, 85 insertions(+), 22 deletions(-) diff --git a/src/plugins/path-escape-plugin.ts b/src/plugins/path-escape-plugin.ts index 5a33d7f76..5679aedb5 100644 --- a/src/plugins/path-escape-plugin.ts +++ b/src/plugins/path-escape-plugin.ts @@ -55,31 +55,71 @@ function escapeArgs( const reason = pathEscapeBlockReason(args, cwd, rootsProvider); if (reason !== undefined) throw new Error(reason); } - const out: Record = {}; - for (const [key, value] of Object.entries(args)) { - if (typeof value === "string" && looksLikePath(key)) { - out[key] = sanitizePath(value, cwd, rootsProvider, allowOutside); - } else { - out[key] = value; + return escapeValue(args, cwd, rootsProvider, allowOutside) as Record< + string, + unknown + >; +} + +function escapeValue( + value: unknown, + cwd: string, + rootsProvider: RootsProvider, + allowOutside: boolean, + key?: string, +): unknown { + if (typeof value === "string") { + return key !== undefined && looksLikePath(key) + ? sanitizePath(value, cwd, rootsProvider, allowOutside) + : value; + } + if (Array.isArray(value)) { + return value.map((entry) => + escapeValue(entry, cwd, rootsProvider, allowOutside, key), + ); + } + if (typeof value === "object" && value !== null) { + const out: Record = {}; + for (const [entryKey, entryValue] of Object.entries(value)) { + out[entryKey] = escapeValue( + entryValue, + cwd, + rootsProvider, + allowOutside, + entryKey, + ); } + return out; } - return out; + return value; } +// Explicit allowlist of argument keys treated as filesystem paths. Keys are +// matched case- and separator-insensitively, so `filePath`, `FILE_PATH`, +// and `file-path` all count alongside `file_path`; any key ending in +// `path`/`paths` (e.g. `somepath`, `outputPaths`) counts too. Anything else +// passes through untouched by design: MCP and custom tools may use arbitrary +// keys whose values only their server interprets, so unknown keys are that +// server's contract, not this sandbox's. export function looksLikePath(key: string): boolean { + const normalized = key.toLowerCase().replace(/[-_]/g, ""); return ( - key === "path" || - key === "file_path" || - key === "target" || - key === "cwd" || - key === "directory" || - key === "dir" || - key === "dest" || - key === "source" || - key === "from" || - key === "to" || - key === "filename" || - key.endsWith("Path") + normalized === "path" || + normalized === "paths" || + normalized === "filepath" || + normalized === "filepaths" || + normalized === "target" || + normalized === "cwd" || + normalized === "directory" || + normalized === "dir" || + normalized === "dest" || + normalized === "source" || + normalized === "from" || + normalized === "to" || + normalized === "filename" || + normalized === "filenames" || + normalized.endsWith("path") || + normalized.endsWith("paths") ); } @@ -91,12 +131,35 @@ export function pathEscapeBlockReason( cwd: string, rootsProvider: RootsProvider = () => [], ): string | undefined { - for (const [key, value] of Object.entries(args)) { - if (typeof value !== "string" || !looksLikePath(key)) continue; - if (isToolOutputLike(value) || isArchiveLike(value)) continue; + return blockReasonFor(args, cwd, rootsProvider); +} + +function blockReasonFor( + value: unknown, + cwd: string, + rootsProvider: RootsProvider, + key?: string, +): string | undefined { + if (typeof value === "string") { + if (key === undefined || !looksLikePath(key)) return undefined; + if (isToolOutputLike(value) || isArchiveLike(value)) return undefined; if (resolveWorkspacePath(cwd, value, rootsProvider) === undefined) { return `Path escapes working directory: ${value}`; } + return undefined; + } + if (Array.isArray(value)) { + for (const entry of value) { + const reason = blockReasonFor(entry, cwd, rootsProvider, key); + if (reason !== undefined) return reason; + } + return undefined; + } + if (typeof value === "object" && value !== null) { + for (const [entryKey, entryValue] of Object.entries(value)) { + const reason = blockReasonFor(entryValue, cwd, rootsProvider, entryKey); + if (reason !== undefined) return reason; + } } return undefined; } From 3d61cd1a9c25d2c3311ee8bf8fadf45fadd23b3b Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sun, 13 Sep 2026 14:12:51 -0700 Subject: [PATCH 3/3] Deepen gate cache identity and carve out non-path key suffixes Authorize-time relative and execution-time rewritten nested paths shared no cache identity, so nested in-bounds calls re-decided at execution. Query-language keys ending in path also over-blocked foreign tool contracts. --- src/permission/gate.ts | 23 ++------ src/permission/permission.test.ts | 41 +++++++++++++ src/plugins/path-escape-plugin.test.ts | 81 +++++++++++++++++++++++++- src/plugins/path-escape-plugin.ts | 53 ++++++++++++++++- 4 files changed, 178 insertions(+), 20 deletions(-) diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 2b0bbbec2..8354a87ff 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -22,7 +22,7 @@ import { } from "./auto-shell-policy.js"; import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js"; import { - looksLikePath, + normalizePathArguments, pathEscapeBlockReason, } from "../plugins/path-escape-plugin.js"; import { runShellAuthzBlockReason } from "../shell/run-shell-authz.js"; @@ -38,10 +38,7 @@ import { tokenize, stripCommentLines, } from "./command.js"; -import { - createPathRestriction, - resolveWorkspacePath, -} from "./path-restriction.js"; +import { createPathRestriction } from "./path-restriction.js"; import { createWorktreeRootsProvider, type RootsProvider, @@ -475,23 +472,15 @@ function canSafelyMintPerSegment(pattern: string): boolean { } // posix pathEscapePlugin rewrites path-like args to resolveWorkspacePath before -// gateToolCall. Cache identity must use that same resolution so an authorizeCall -// allow is not treated as a different call (and re-decided) at execution. +// gateToolCall. Cache identity must use that same resolution — deep, like the +// plugin's escapeValue walk — so an authorizeCall allow is not treated as a +// different call (and re-decided) at execution. function identityArguments( args: ToolCall["arguments"], cwd: string, rootsProvider: RootsProvider, ): string { - const normalized: Record = {}; - for (const [key, value] of Object.entries(args)) { - if (typeof value === "string" && looksLikePath(key)) { - normalized[key] = - resolveWorkspacePath(cwd, value, rootsProvider) ?? value; - } else { - normalized[key] = value; - } - } - return JSON.stringify(normalized); + return JSON.stringify(normalizePathArguments(args, cwd, rootsProvider)); } export function createPermissionGate( diff --git a/src/permission/permission.test.ts b/src/permission/permission.test.ts index f7d213c65..48d6cd515 100644 --- a/src/permission/permission.test.ts +++ b/src/permission/permission.test.ts @@ -1069,6 +1069,47 @@ describe("gate denies path tools path-escape will reject", () => { }); }); +describe("gate cache identity matches the plugin rewrite for nested paths", () => { + // authorizeCall caches by identityArguments; executionVerdict must hit that + // cache when execution hands it the plugin-rewritten (workspace-absolute) + // arguments. A grant seeded after authorize changes what a fresh decide + // would say, so a miss visibly flips to allow while a hit reuses the ask. + test("nested in-bounds call authorizes once and executes without re-decide", async () => { + const cwd = realpathSync(mkdtempSync(join(tmpdir(), "corbits-identity-"))); + const gate = createPermissionGate({ + approvals: [], + cwd, + requestApproval: async () => ({ allow: false }), + interactive: true, + skipPermissions: false, + reactorGated: true, + }); + const call: ToolCall = { + id: "c", + name: "write_file", + arguments: { + path: "notes.txt", + options: { path: "notes.txt" }, + content: "x", + }, + }; + const authorized = await gate.authorizeCall(call); + expect(authorized.effect).toBe("ask"); + gate.setSeededApprovals([ + { tool: "write_file", pattern: join(cwd, "notes.txt") }, + ]); + const executed = await gate.executionVerdict({ + ...call, + arguments: { + path: join(cwd, "notes.txt"), + options: { path: join(cwd, "notes.txt") }, + content: "x", + }, + }); + expect(executed.effect).toBe("ask"); + }); +}); + describe("createPermissionGate", () => { test("allow-tier tools pass without asking", async () => { let asked = 0; diff --git a/src/plugins/path-escape-plugin.test.ts b/src/plugins/path-escape-plugin.test.ts index bf0bff306..c27676824 100644 --- a/src/plugins/path-escape-plugin.test.ts +++ b/src/plugins/path-escape-plugin.test.ts @@ -11,7 +11,11 @@ import { existsSync, realpathSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { pathEscapePlugin } from "./path-escape-plugin.js"; +import { + normalizePathArguments, + pathEscapeBlockReason, + pathEscapePlugin, +} from "./path-escape-plugin.js"; import type { ToolCall, ToolResult } from "@intx/types/runtime"; function makeCall(name: string, args: Record): ToolCall { @@ -334,7 +338,6 @@ describe("pathEscapePlugin", () => { }); test("pathEscapeBlockReason agrees with execution time on nested escapes", async () => { - const { pathEscapeBlockReason } = await import("./path-escape-plugin.js"); expect( pathEscapeBlockReason( { options: { path: "../secret.txt" } }, @@ -351,5 +354,79 @@ describe("pathEscapePlugin", () => { ), ).toMatch(/escapes working directory/); }); + + test("nested non-path keys pass through untouched (allowlist policy)", async () => { + const plugin = pathEscapePlugin("/project"); + const { next, seen } = captureNext(); + const handler = plugin.middleware ? plugin.middleware(next) : next; + const result = await handler( + makeCall("custom_tool", { options: { command: "../secret.txt" } }), + new AbortController().signal, + ); + expect(result.isError).not.toBe(true); + expect(seen()).toEqual({ options: { command: "../secret.txt" } }); + expect( + pathEscapeBlockReason( + { options: { command: "../secret.txt" } }, + "/project", + ), + ).toBeUndefined(); + }); + + test("FILE_PATH and file-path match case- and separator-insensitively", async () => { + const plugin = pathEscapePlugin("/project"); + const handler = plugin.middleware + ? plugin.middleware(nextHandler) + : nextHandler; + for (const key of ["FILE_PATH", "file-path"]) { + const result = await handler( + makeCall("read_file", { [key]: "../secret.txt" }), + new AbortController().signal, + ); + expect(result.isError).toBe(true); + expect(result.content).toMatch(/escapes working directory/); + expect( + pathEscapeBlockReason({ [key]: "../secret.txt" }, "/project"), + ).toMatch(/escapes working directory/); + } + }); + + test("xpath, jsonpath, and classpath keys pass through untouched", async () => { + const plugin = pathEscapePlugin("/project"); + const { next, seen } = captureNext(); + const handler = plugin.middleware ? plugin.middleware(next) : next; + const args = { + xpath: "../../title", + jsonpath: "$.store.book", + classpath: "src/Main", + }; + const result = await handler( + makeCall("custom_tool", args), + new AbortController().signal, + ); + expect(result.isError).not.toBe(true); + expect(seen()).toEqual(args); + expect(pathEscapeBlockReason(args, "/project")).toBeUndefined(); + }); + + test("normalizePathArguments shares the plugin rewrite identity", () => { + expect( + normalizePathArguments( + { options: { path: "src/index.ts" } }, + "/project", + () => [], + ), + ).toEqual({ options: { path: "/project/src/index.ts" } }); + expect( + normalizePathArguments( + { options: { command: "../secret.txt" } }, + "/project", + () => [], + ), + ).toEqual({ options: { command: "../secret.txt" } }); + expect( + normalizePathArguments({ xpath: "src/index.ts" }, "/project", () => []), + ).toEqual({ xpath: "src/index.ts" }); + }); }); }); diff --git a/src/plugins/path-escape-plugin.ts b/src/plugins/path-escape-plugin.ts index 5679aedb5..55796cfd6 100644 --- a/src/plugins/path-escape-plugin.ts +++ b/src/plugins/path-escape-plugin.ts @@ -97,12 +97,24 @@ function escapeValue( // Explicit allowlist of argument keys treated as filesystem paths. Keys are // matched case- and separator-insensitively, so `filePath`, `FILE_PATH`, // and `file-path` all count alongside `file_path`; any key ending in -// `path`/`paths` (e.g. `somepath`, `outputPaths`) counts too. Anything else +// `path`/`paths` (e.g. `somepath`, `outputPaths`) counts too, except query- +// language and JVM keys (`xpath`, `jsonpath`, `classpath` and their plurals) +// whose values are expressions, not filesystem paths. Anything else // passes through untouched by design: MCP and custom tools may use arbitrary // keys whose values only their server interprets, so unknown keys are that // server's contract, not this sandbox's. export function looksLikePath(key: string): boolean { const normalized = key.toLowerCase().replace(/[-_]/g, ""); + if ( + normalized.endsWith("xpath") || + normalized.endsWith("xpaths") || + normalized.endsWith("jsonpath") || + normalized.endsWith("jsonpaths") || + normalized.endsWith("classpath") || + normalized.endsWith("classpaths") + ) { + return false; + } return ( normalized === "path" || normalized === "paths" || @@ -134,6 +146,45 @@ export function pathEscapeBlockReason( return blockReasonFor(args, cwd, rootsProvider); } +// Deep-walk identity for the permission gate's authorize/execution cache. +// Same key propagation as escapeValue (innermost key wins; array entries +// inherit the array key), but non-throwing: in-bounds paths resolve to their +// workspace-absolute form while escapes and non-path values pass through +// untouched. Both cache sides compute it, so a fail-closed re-decide still +// agrees — the point is only that authorize-time relative and execution-time +// rewritten arguments share one identity. +export function normalizePathArguments( + args: Record, + cwd: string, + rootsProvider: RootsProvider = () => [], +): Record { + return normalizeValue(args, cwd, rootsProvider) as Record; +} + +function normalizeValue( + value: unknown, + cwd: string, + rootsProvider: RootsProvider, + key?: string, +): unknown { + if (typeof value === "string") { + if (key === undefined || !looksLikePath(key)) return value; + if (isToolOutputLike(value) || isArchiveLike(value)) return value; + return resolveWorkspacePath(cwd, value, rootsProvider) ?? value; + } + if (Array.isArray(value)) { + return value.map((entry) => normalizeValue(entry, cwd, rootsProvider, key)); + } + if (typeof value === "object" && value !== null) { + const out: Record = {}; + for (const [entryKey, entryValue] of Object.entries(value)) { + out[entryKey] = normalizeValue(entryValue, cwd, rootsProvider, entryKey); + } + return out; + } + return value; +} + function blockReasonFor( value: unknown, cwd: string,