Skip to content

Commit 3d61cd1

Browse files
committed
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.
1 parent 5ca559f commit 3d61cd1

4 files changed

Lines changed: 178 additions & 20 deletions

File tree

‎src/permission/gate.ts‎

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
} from "./auto-shell-policy.js";
2323
import { commandReferencesSensitivePath } from "../plugins/secret-guard-plugin.js";
2424
import {
25-
looksLikePath,
25+
normalizePathArguments,
2626
pathEscapeBlockReason,
2727
} from "../plugins/path-escape-plugin.js";
2828
import { runShellAuthzBlockReason } from "../shell/run-shell-authz.js";
@@ -38,10 +38,7 @@ import {
3838
tokenize,
3939
stripCommentLines,
4040
} from "./command.js";
41-
import {
42-
createPathRestriction,
43-
resolveWorkspacePath,
44-
} from "./path-restriction.js";
41+
import { createPathRestriction } from "./path-restriction.js";
4542
import {
4643
createWorktreeRootsProvider,
4744
type RootsProvider,
@@ -475,23 +472,15 @@ function canSafelyMintPerSegment(pattern: string): boolean {
475472
}
476473

477474
// posix pathEscapePlugin rewrites path-like args to resolveWorkspacePath before
478-
// gateToolCall. Cache identity must use that same resolution so an authorizeCall
479-
// allow is not treated as a different call (and re-decided) at execution.
475+
// gateToolCall. Cache identity must use that same resolution — deep, like the
476+
// plugin's escapeValue walk — so an authorizeCall allow is not treated as a
477+
// different call (and re-decided) at execution.
480478
function identityArguments(
481479
args: ToolCall["arguments"],
482480
cwd: string,
483481
rootsProvider: RootsProvider,
484482
): string {
485-
const normalized: Record<string, unknown> = {};
486-
for (const [key, value] of Object.entries(args)) {
487-
if (typeof value === "string" && looksLikePath(key)) {
488-
normalized[key] =
489-
resolveWorkspacePath(cwd, value, rootsProvider) ?? value;
490-
} else {
491-
normalized[key] = value;
492-
}
493-
}
494-
return JSON.stringify(normalized);
483+
return JSON.stringify(normalizePathArguments(args, cwd, rootsProvider));
495484
}
496485

497486
export function createPermissionGate(

‎src/permission/permission.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,6 +1069,47 @@ describe("gate denies path tools path-escape will reject", () => {
10691069
});
10701070
});
10711071

1072+
describe("gate cache identity matches the plugin rewrite for nested paths", () => {
1073+
// authorizeCall caches by identityArguments; executionVerdict must hit that
1074+
// cache when execution hands it the plugin-rewritten (workspace-absolute)
1075+
// arguments. A grant seeded after authorize changes what a fresh decide
1076+
// would say, so a miss visibly flips to allow while a hit reuses the ask.
1077+
test("nested in-bounds call authorizes once and executes without re-decide", async () => {
1078+
const cwd = realpathSync(mkdtempSync(join(tmpdir(), "corbits-identity-")));
1079+
const gate = createPermissionGate({
1080+
approvals: [],
1081+
cwd,
1082+
requestApproval: async () => ({ allow: false }),
1083+
interactive: true,
1084+
skipPermissions: false,
1085+
reactorGated: true,
1086+
});
1087+
const call: ToolCall = {
1088+
id: "c",
1089+
name: "write_file",
1090+
arguments: {
1091+
path: "notes.txt",
1092+
options: { path: "notes.txt" },
1093+
content: "x",
1094+
},
1095+
};
1096+
const authorized = await gate.authorizeCall(call);
1097+
expect(authorized.effect).toBe("ask");
1098+
gate.setSeededApprovals([
1099+
{ tool: "write_file", pattern: join(cwd, "notes.txt") },
1100+
]);
1101+
const executed = await gate.executionVerdict({
1102+
...call,
1103+
arguments: {
1104+
path: join(cwd, "notes.txt"),
1105+
options: { path: join(cwd, "notes.txt") },
1106+
content: "x",
1107+
},
1108+
});
1109+
expect(executed.effect).toBe("ask");
1110+
});
1111+
});
1112+
10721113
describe("createPermissionGate", () => {
10731114
test("allow-tier tools pass without asking", async () => {
10741115
let asked = 0;

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

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ import { existsSync, realpathSync } from "node:fs";
1111
import { tmpdir } from "node:os";
1212
import { join } from "node:path";
1313

14-
import { pathEscapePlugin } from "./path-escape-plugin.js";
14+
import {
15+
normalizePathArguments,
16+
pathEscapeBlockReason,
17+
pathEscapePlugin,
18+
} from "./path-escape-plugin.js";
1519
import type { ToolCall, ToolResult } from "@intx/types/runtime";
1620

1721
function makeCall(name: string, args: Record<string, unknown>): ToolCall {
@@ -334,7 +338,6 @@ describe("pathEscapePlugin", () => {
334338
});
335339

336340
test("pathEscapeBlockReason agrees with execution time on nested escapes", async () => {
337-
const { pathEscapeBlockReason } = await import("./path-escape-plugin.js");
338341
expect(
339342
pathEscapeBlockReason(
340343
{ options: { path: "../secret.txt" } },
@@ -351,5 +354,79 @@ describe("pathEscapePlugin", () => {
351354
),
352355
).toMatch(/escapes working directory/);
353356
});
357+
358+
test("nested non-path keys pass through untouched (allowlist policy)", async () => {
359+
const plugin = pathEscapePlugin("/project");
360+
const { next, seen } = captureNext();
361+
const handler = plugin.middleware ? plugin.middleware(next) : next;
362+
const result = await handler(
363+
makeCall("custom_tool", { options: { command: "../secret.txt" } }),
364+
new AbortController().signal,
365+
);
366+
expect(result.isError).not.toBe(true);
367+
expect(seen()).toEqual({ options: { command: "../secret.txt" } });
368+
expect(
369+
pathEscapeBlockReason(
370+
{ options: { command: "../secret.txt" } },
371+
"/project",
372+
),
373+
).toBeUndefined();
374+
});
375+
376+
test("FILE_PATH and file-path match case- and separator-insensitively", async () => {
377+
const plugin = pathEscapePlugin("/project");
378+
const handler = plugin.middleware
379+
? plugin.middleware(nextHandler)
380+
: nextHandler;
381+
for (const key of ["FILE_PATH", "file-path"]) {
382+
const result = await handler(
383+
makeCall("read_file", { [key]: "../secret.txt" }),
384+
new AbortController().signal,
385+
);
386+
expect(result.isError).toBe(true);
387+
expect(result.content).toMatch(/escapes working directory/);
388+
expect(
389+
pathEscapeBlockReason({ [key]: "../secret.txt" }, "/project"),
390+
).toMatch(/escapes working directory/);
391+
}
392+
});
393+
394+
test("xpath, jsonpath, and classpath keys pass through untouched", async () => {
395+
const plugin = pathEscapePlugin("/project");
396+
const { next, seen } = captureNext();
397+
const handler = plugin.middleware ? plugin.middleware(next) : next;
398+
const args = {
399+
xpath: "../../title",
400+
jsonpath: "$.store.book",
401+
classpath: "src/Main",
402+
};
403+
const result = await handler(
404+
makeCall("custom_tool", args),
405+
new AbortController().signal,
406+
);
407+
expect(result.isError).not.toBe(true);
408+
expect(seen()).toEqual(args);
409+
expect(pathEscapeBlockReason(args, "/project")).toBeUndefined();
410+
});
411+
412+
test("normalizePathArguments shares the plugin rewrite identity", () => {
413+
expect(
414+
normalizePathArguments(
415+
{ options: { path: "src/index.ts" } },
416+
"/project",
417+
() => [],
418+
),
419+
).toEqual({ options: { path: "/project/src/index.ts" } });
420+
expect(
421+
normalizePathArguments(
422+
{ options: { command: "../secret.txt" } },
423+
"/project",
424+
() => [],
425+
),
426+
).toEqual({ options: { command: "../secret.txt" } });
427+
expect(
428+
normalizePathArguments({ xpath: "src/index.ts" }, "/project", () => []),
429+
).toEqual({ xpath: "src/index.ts" });
430+
});
354431
});
355432
});

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

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,24 @@ function escapeValue(
9797
// Explicit allowlist of argument keys treated as filesystem paths. Keys are
9898
// matched case- and separator-insensitively, so `filePath`, `FILE_PATH`,
9999
// and `file-path` all count alongside `file_path`; any key ending in
100-
// `path`/`paths` (e.g. `somepath`, `outputPaths`) counts too. Anything else
100+
// `path`/`paths` (e.g. `somepath`, `outputPaths`) counts too, except query-
101+
// language and JVM keys (`xpath`, `jsonpath`, `classpath` and their plurals)
102+
// whose values are expressions, not filesystem paths. Anything else
101103
// passes through untouched by design: MCP and custom tools may use arbitrary
102104
// keys whose values only their server interprets, so unknown keys are that
103105
// server's contract, not this sandbox's.
104106
export function looksLikePath(key: string): boolean {
105107
const normalized = key.toLowerCase().replace(/[-_]/g, "");
108+
if (
109+
normalized.endsWith("xpath") ||
110+
normalized.endsWith("xpaths") ||
111+
normalized.endsWith("jsonpath") ||
112+
normalized.endsWith("jsonpaths") ||
113+
normalized.endsWith("classpath") ||
114+
normalized.endsWith("classpaths")
115+
) {
116+
return false;
117+
}
106118
return (
107119
normalized === "path" ||
108120
normalized === "paths" ||
@@ -134,6 +146,45 @@ export function pathEscapeBlockReason(
134146
return blockReasonFor(args, cwd, rootsProvider);
135147
}
136148

149+
// Deep-walk identity for the permission gate's authorize/execution cache.
150+
// Same key propagation as escapeValue (innermost key wins; array entries
151+
// inherit the array key), but non-throwing: in-bounds paths resolve to their
152+
// workspace-absolute form while escapes and non-path values pass through
153+
// untouched. Both cache sides compute it, so a fail-closed re-decide still
154+
// agrees — the point is only that authorize-time relative and execution-time
155+
// rewritten arguments share one identity.
156+
export function normalizePathArguments(
157+
args: Record<string, unknown>,
158+
cwd: string,
159+
rootsProvider: RootsProvider = () => [],
160+
): Record<string, unknown> {
161+
return normalizeValue(args, cwd, rootsProvider) as Record<string, unknown>;
162+
}
163+
164+
function normalizeValue(
165+
value: unknown,
166+
cwd: string,
167+
rootsProvider: RootsProvider,
168+
key?: string,
169+
): unknown {
170+
if (typeof value === "string") {
171+
if (key === undefined || !looksLikePath(key)) return value;
172+
if (isToolOutputLike(value) || isArchiveLike(value)) return value;
173+
return resolveWorkspacePath(cwd, value, rootsProvider) ?? value;
174+
}
175+
if (Array.isArray(value)) {
176+
return value.map((entry) => normalizeValue(entry, cwd, rootsProvider, key));
177+
}
178+
if (typeof value === "object" && value !== null) {
179+
const out: Record<string, unknown> = {};
180+
for (const [entryKey, entryValue] of Object.entries(value)) {
181+
out[entryKey] = normalizeValue(entryValue, cwd, rootsProvider, entryKey);
182+
}
183+
return out;
184+
}
185+
return value;
186+
}
187+
137188
function blockReasonFor(
138189
value: unknown,
139190
cwd: string,

0 commit comments

Comments
 (0)