Skip to content

Commit 84a2ead

Browse files
fix(permissions): resolve advertised tool aliases in approval snapshots (#1223)
* CL-9477: add authz parity definitions for aliased tools The reactor authz snapshot is keyed by parked wire name, so an ask-tier bash/shell call without a parity copy throws a wiring-defect error instead of suspending. Emit one renamed copy per alias alongside every definition and wrap the leaf and chat bundles so the authz-facing set gains the copies while the advertised wire set stays byte-identical. * test(agent): add parity wrapper liveness and idempotency keepers * refactor(agent): reuse push dedup helper in parity loop * test(agent): add removeTools parity-reflection keeper
1 parent 67f2493 commit 84a2ead

6 files changed

Lines changed: 383 additions & 6 deletions

File tree

‎src/agent/tool-aliases.test.ts‎

Lines changed: 156 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
import { describe, expect, test } from "bun:test";
2-
import type { ToolDefinition } from "@intx/types/runtime";
2+
import type { ToolCall, ToolDefinition } from "@intx/types/runtime";
33
import { createDynamicToolRunner } from "../tui/dynamic-tool-runner.js";
44
import { advertisedTools } from "./tool-search.js";
55
import { canonicalToolName } from "./canonical-tool-name.js";
6-
import { advertisedToolName, WIRE_TO_ENGINE } from "./tool-aliases.js";
6+
import {
7+
advertisedToolName,
8+
authzParityDefinitions,
9+
withAuthzParityDefinitions,
10+
WIRE_TO_ENGINE,
11+
} from "./tool-aliases.js";
712
import { evaluateApprovals } from "../permission/authz-grants.js";
813

914
const noWorkspace = { resolvedCwd: "/repo", roots: ["/repo"] };
@@ -240,3 +245,152 @@ describe("hidden alias dispatch", () => {
240245
expect(new Set(names).size).toBe(names.length);
241246
});
242247
});
248+
249+
describe("authz parity definitions", () => {
250+
test("run_shell gains bash and shell copies that rename name only", () => {
251+
const native = posixDef("run_shell");
252+
const defs = authzParityDefinitions([native]);
253+
expect(defs.map((d) => d.name)).toEqual(["run_shell", "bash", "shell"]);
254+
const byName = new Map(defs.map((d) => [d.name, d]));
255+
expect(byName.get("run_shell")).toBe(native);
256+
expect(byName.get("bash")).toEqual({ ...native, name: "bash" });
257+
expect(byName.get("shell")).toEqual({ ...native, name: "shell" });
258+
});
259+
260+
test("read_file gains a read copy; MCP defs pass through untouched", () => {
261+
const mcp = posixDef("mcp__linear__save_issue");
262+
const defs = authzParityDefinitions([posixDef("read_file"), mcp]);
263+
expect(defs.map((d) => d.name)).toEqual([
264+
"read_file",
265+
"mcp__linear__save_issue",
266+
"read",
267+
]);
268+
expect(defs.find((d) => d.name === "mcp__linear__save_issue")).toBe(mcp);
269+
});
270+
271+
test("manage_tasks never gains an update_plan snapshot copy", () => {
272+
const defs = authzParityDefinitions([
273+
posixDef("manage_tasks"),
274+
posixDef("run_shell"),
275+
]);
276+
const names = defs.map((d) => d.name);
277+
expect(names).toContain("manage_tasks");
278+
expect(names).not.toContain("update_plan");
279+
expect(names).toContain("bash");
280+
expect(names).toContain("shell");
281+
});
282+
283+
test("already-aliased and duplicate defs dedup by name", () => {
284+
const defs = authzParityDefinitions([
285+
posixDef("run_shell"),
286+
posixDef("bash"),
287+
posixDef("run_shell"),
288+
]);
289+
expect(defs.map((d) => d.name)).toEqual(["run_shell", "bash", "shell"]);
290+
});
291+
292+
test("empty registry stays empty", () => {
293+
expect(authzParityDefinitions([])).toEqual([]);
294+
});
295+
296+
test("applied to its own output is a no-op (name set stable)", () => {
297+
const once = authzParityDefinitions([
298+
posixDef("run_shell"),
299+
posixDef("read_file"),
300+
posixDef("manage_tasks"),
301+
posixDef("mcp__acme__do"),
302+
]);
303+
const twice = authzParityDefinitions(once);
304+
expect(twice.map((d) => d.name)).toEqual(once.map((d) => d.name));
305+
expect(twice).toEqual(once);
306+
});
307+
});
308+
309+
describe("withAuthzParityDefinitions", () => {
310+
test("definitions getter returns parity over the live set; run delegates", async () => {
311+
let live: ToolDefinition[] = [posixDef("run_shell")];
312+
let ran: ToolCall | undefined;
313+
const bundle = {
314+
definitions: live,
315+
currentDefinitions: () => live,
316+
run: async (call: ToolCall, _signal: AbortSignal) => {
317+
ran = call;
318+
return { callId: call.id, content: "ok", isError: false };
319+
},
320+
addTools: () => undefined,
321+
setCallGate: () => undefined,
322+
};
323+
const wrapped = withAuthzParityDefinitions(bundle);
324+
expect(wrapped.definitions.map((d) => d.name)).toEqual([
325+
"run_shell",
326+
"bash",
327+
"shell",
328+
]);
329+
live = [...live, posixDef("mcp__acme__do")];
330+
expect(wrapped.definitions.map((d) => d.name)).toEqual([
331+
"run_shell",
332+
"mcp__acme__do",
333+
"bash",
334+
"shell",
335+
]);
336+
const call = {
337+
id: "1",
338+
name: "bash",
339+
arguments: { command: "echo hi" },
340+
};
341+
await wrapped.run(call, new AbortController().signal);
342+
expect(ran).toBe(call);
343+
});
344+
345+
test("falls back to definitions when currentDefinitions is absent", () => {
346+
const bundle = {
347+
definitions: [posixDef("read_file")] as readonly ToolDefinition[],
348+
run: async () => ({ callId: "1", content: "", isError: false }),
349+
};
350+
expect(
351+
withAuthzParityDefinitions(bundle).definitions.map((d) => d.name),
352+
).toEqual(["read_file", "read"]);
353+
});
354+
355+
test("addTools through the wrapper is reflected in definitions plus parity copies", () => {
356+
let live: ToolDefinition[] = [posixDef("run_shell")];
357+
const bundle = {
358+
definitions: live,
359+
currentDefinitions: () => live,
360+
addTools: (tools: ToolDefinition[]) => {
361+
live = [...live, ...tools];
362+
},
363+
run: async () => ({ callId: "1", content: "", isError: false }),
364+
};
365+
const wrapped = withAuthzParityDefinitions(bundle);
366+
expect(wrapped.addTools).toBe(bundle.addTools);
367+
wrapped.addTools([posixDef("read_file")]);
368+
expect(wrapped.definitions.map((d) => d.name)).toEqual([
369+
"run_shell",
370+
"read_file",
371+
"bash",
372+
"shell",
373+
"read",
374+
]);
375+
});
376+
377+
test("removeTools through the wrapper is reflected in definitions", () => {
378+
let live: ToolDefinition[] = [posixDef("run_shell"), posixDef("read_file")];
379+
const bundle = {
380+
definitions: live,
381+
currentDefinitions: () => live,
382+
removeTools: (names: string[]) => {
383+
live = live.filter((d) => !names.includes(d.name));
384+
},
385+
run: async () => ({ callId: "1", content: "", isError: false }),
386+
};
387+
const wrapped = withAuthzParityDefinitions(bundle);
388+
expect(wrapped.removeTools).toBe(bundle.removeTools);
389+
wrapped.removeTools(["read_file"]);
390+
expect(wrapped.definitions.map((d) => d.name)).toEqual([
391+
"run_shell",
392+
"bash",
393+
"shell",
394+
]);
395+
});
396+
});

‎src/agent/tool-aliases.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import { type } from "arktype";
1313
import type { ToolCall, ToolDefinition } from "@intx/types/runtime";
14+
import { canonicalToolName } from "./canonical-tool-name.js";
1415

1516
/** Advertised posix names → registry engine ids. 1:1, never dual-publish. */
1617
export const WIRE_TO_ENGINE = {
@@ -77,6 +78,67 @@ export function projectToolDefinitions(
7778
return defs.map(projectToolDefinition);
7879
}
7980

81+
/**
82+
* Authz parity definitions: every def unchanged, plus one `{...def, name:
83+
* alias}` copy for each alias in ALIAS_TO_ENGINE whose engine equals the
84+
* def's canonical name. The reactor authz snapshot is keyed by parked wire
85+
* name, so without these copies an ask-tier `bash`/`shell` call throws a
86+
* wiring-defect error instead of suspending.
87+
*
88+
* `update_plan` is never snapshotted: its grant is create-only narrow, so no
89+
* `update_plan`-named copy is emitted. Non-aliased defs (MCP, leaf-only)
90+
* pass through unchanged. Output is deduplicated by name.
91+
*/
92+
export function authzParityDefinitions(
93+
defs: readonly ToolDefinition[],
94+
): ToolDefinition[] {
95+
const seen = new Set<string>();
96+
const out: ToolDefinition[] = [];
97+
const push = (def: ToolDefinition): void => {
98+
if (seen.has(def.name)) return;
99+
seen.add(def.name);
100+
out.push(def);
101+
};
102+
for (const def of defs) push(def);
103+
for (const def of defs) {
104+
const engine = canonicalToolName(def.name);
105+
for (const [alias, aliasEngine] of Object.entries(ALIAS_TO_ENGINE)) {
106+
if (aliasEngine !== engine) continue;
107+
if (alias === "update_plan") continue;
108+
push({ ...def, name: alias });
109+
}
110+
}
111+
return out;
112+
}
113+
114+
/**
115+
* Thin wrapper over a tool bundle (e.g. DynamicToolRunner): identical except
116+
* the `definitions` getter returns `authzParityDefinitions` over the live
117+
* set. Run/dispatch and mutation entry points delegate verbatim — only the
118+
* authz-facing definition set gains parity copies. The advertised wire set
119+
* is untouched (advertise still projects through computeAdvertised).
120+
*/
121+
export function withAuthzParityDefinitions<
122+
T extends { readonly definitions: readonly ToolDefinition[] },
123+
>(bundle: T): T {
124+
const wrapped = { ...bundle };
125+
const liveSource = bundle as Partial<{
126+
currentDefinitions: () => readonly ToolDefinition[];
127+
}>;
128+
Object.defineProperty(wrapped, "definitions", {
129+
get() {
130+
const live =
131+
typeof liveSource.currentDefinitions === "function"
132+
? liveSource.currentDefinitions()
133+
: bundle.definitions;
134+
return authzParityDefinitions(live);
135+
},
136+
enumerable: true,
137+
configurable: true,
138+
});
139+
return wrapped;
140+
}
141+
80142
const SHELL_WRAPPERS = new Set(["bash", "sh", "zsh"]);
81143

82144
function shellQuote(arg: string): string {

‎src/permission/reactor-authorize.test.ts‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,19 @@ import { expect, test } from "bun:test";
22
import { mkdtempSync, realpathSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
5+
import { createAuthzExtension } from "@intx/inference";
6+
import type {
7+
ToolDefinition,
8+
ReactorState,
9+
TokenUsage,
10+
} from "@intx/types/runtime";
511
import { createPermissionGate } from "./gate.js";
612
import {
713
createReactorAuthorize,
814
createWorkerAuthorize,
915
workerPermissionGate,
1016
} from "./reactor-authorize.js";
17+
import { authzParityDefinitions } from "../agent/tool-aliases.js";
1118
import {
1219
runWithSubAgentIdentity,
1320
getSubAgentIdentity,
@@ -335,3 +342,125 @@ test("concurrent authorization preserves each worker cwd across awaited policy e
335342
expect(seen.sort()).toEqual(["/worker-a", "/worker-b"]);
336343
expect(getSubAgentIdentity()).toBeUndefined();
337344
});
345+
346+
const shellDef = (name: string): ToolDefinition => ({
347+
name,
348+
description: `${name} tool`,
349+
inputSchema: { type: "object", properties: {} },
350+
});
351+
352+
const emptyUsage = (): TokenUsage => ({
353+
input: 0,
354+
output: 0,
355+
cacheRead: 0,
356+
cacheWrite: 0,
357+
thinking: 0,
358+
});
359+
360+
const askState = (): ReactorState => ({
361+
sessionId: "parity-ask",
362+
turns: [],
363+
activeForks: [],
364+
pendingOperations: [],
365+
activeGates: [],
366+
tokenUsage: emptyUsage(),
367+
lastCycleUsage: null,
368+
lastCycleSource: null,
369+
});
370+
371+
const askSignal = new AbortController().signal;
372+
373+
test("ask-tier shell aliases suspend with a wire-named snapshot", async () => {
374+
const policy = gate({ interactive: true });
375+
const ext = createAuthzExtension({
376+
toolDefinitions: authzParityDefinitions([
377+
shellDef("run_shell"),
378+
shellDef("read_file"),
379+
]),
380+
authorize: createReactorAuthorize(policy),
381+
});
382+
for (const name of ["bash", "run_shell", "shell"]) {
383+
const toolCall = namedCall(name, { command: "sleep 30" });
384+
const outcome = await ext.beforeTool(toolCall, askState(), askSignal);
385+
if (outcome.type !== "suspend") throw new Error(`expected ${name} to park`);
386+
expect(outcome.pendingOp.approvalSnapshot?.name).toBe(name);
387+
expect(outcome.pendingOp.approvalSnapshot?.arguments).toEqual({
388+
command: "sleep 30",
389+
});
390+
expect(outcome.pendingOp.suspendedCall).toEqual(toolCall);
391+
}
392+
});
393+
394+
test("seeded run_shell grant allows bash, run_shell, and shell", async () => {
395+
const policy = gate({ interactive: true });
396+
policy.setSeededApprovals([{ tool: "run_shell", pattern: "echo *" }]);
397+
const ext = createAuthzExtension({
398+
toolDefinitions: authzParityDefinitions([shellDef("run_shell")]),
399+
authorize: createReactorAuthorize(policy),
400+
});
401+
for (const name of ["bash", "run_shell", "shell"]) {
402+
const outcome = await ext.beforeTool(
403+
namedCall(name, { command: "echo hi" }),
404+
askState(),
405+
askSignal,
406+
);
407+
expect(outcome.type).toBe("allow");
408+
}
409+
});
410+
411+
test("ask on a tool missing from the resolved set still throws", async () => {
412+
const policy = gate({ interactive: true });
413+
const ext = createAuthzExtension({
414+
toolDefinitions: authzParityDefinitions([shellDef("read_file")]),
415+
authorize: createReactorAuthorize(policy),
416+
});
417+
await expect(
418+
ext.beforeTool(
419+
namedCall("bash", { command: "sleep 30" }),
420+
askState(),
421+
askSignal,
422+
),
423+
).rejects.toThrow(/wiring defect/);
424+
});
425+
426+
test("approved ask resumes the exact parked call once via one-shot bypass", async () => {
427+
const policy = gate({ interactive: true });
428+
const reactorAuthorize = createReactorAuthorize(policy);
429+
const seen: ToolCall[] = [];
430+
const ext = createAuthzExtension<ToolCall>({
431+
toolDefinitions: authzParityDefinitions([shellDef("run_shell")]),
432+
authorize: (resource, action, call) => {
433+
seen.push(call);
434+
return reactorAuthorize(resource, action, call);
435+
},
436+
});
437+
for (const name of ["bash", "run_shell"]) {
438+
const toolCall = namedCall(name, { command: "sleep 30" });
439+
const parked = await ext.beforeTool(toolCall, askState(), askSignal);
440+
if (parked.type !== "suspend") throw new Error(`expected ${name} to park`);
441+
expect(parked.pendingOp.approvalSnapshot?.name).toBe(name);
442+
ext.grantOneShot?.(toolCall.id);
443+
const resumed = await ext.beforeTool(toolCall, askState(), askSignal);
444+
expect(resumed.type).toBe("allow");
445+
expect(seen[seen.length - 1]).toEqual(parked.pendingOp.suspendedCall);
446+
const reparked = await ext.beforeTool(toolCall, askState(), askSignal);
447+
expect(reparked.type).toBe("suspend");
448+
}
449+
});
450+
451+
test("denied shell call blocks with a policy error", async () => {
452+
const policy = gate({ interactive: false });
453+
const ext = createAuthzExtension({
454+
toolDefinitions: authzParityDefinitions([shellDef("run_shell")]),
455+
authorize: createReactorAuthorize(policy),
456+
});
457+
for (const name of ["bash", "run_shell"]) {
458+
const outcome = await ext.beforeTool(
459+
namedCall(name, { command: "sleep 30" }),
460+
askState(),
461+
askSignal,
462+
);
463+
if (outcome.type !== "block") throw new Error(`expected ${name} to block`);
464+
expect(outcome.reason).toMatch(/Denied by policy/);
465+
}
466+
});

0 commit comments

Comments
 (0)