Skip to content

Commit 96675e4

Browse files
TheGreatAxiosTheGreatAxios
committed
Harden archive path bounds and completeness-gate covering
Independent review found archive: prefixes skipped workspace sanitization, workers fell through to the filesystem, and derived handoffs fail-closed the second fold. Bound refs to well-formed archive:/// paths, fail-close workers without an archive, and skip foldable units so covering matches live history.
1 parent 12418f6 commit 96675e4

19 files changed

Lines changed: 732 additions & 36 deletions

‎src/agent/archive-tool-result.test.ts‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
import { mkdtempSync, writeFileSync } from "node:fs";
2+
import { tmpdir } from "node:os";
3+
import { join } from "node:path";
4+
import { describe, expect, test } from "bun:test";
5+
import { stringTool } from "@intx/agent";
6+
7+
import { withAuthorizedArchiveResult } from "./archive-tool-result.js";
8+
import { createPermissionGate } from "../permission/gate.js";
9+
import { createCompactionArchive, type CompactionArchive } from "../session/compaction-archive.js";
10+
11+
function memoryArchive(sessionId: string): CompactionArchive {
12+
const dir = mkdtempSync(join(tmpdir(), "archive-tool-result-"));
13+
const blobs = new Map<string, Uint8Array>();
14+
return createCompactionArchive({
15+
sessionId,
16+
contextDir: dir,
17+
writeBlob: async (key, bytes) => {
18+
blobs.set(key, bytes);
19+
},
20+
readBlob: async (key) => {
21+
const bytes = blobs.get(key);
22+
if (bytes === undefined) throw new Error(`missing blob ${key}`);
23+
return bytes;
24+
},
25+
});
26+
}
27+
28+
describe("withAuthorizedArchiveResult", () => {
29+
test("records string-tool results when an archive is present", async () => {
30+
const archive = memoryArchive("sess-agent");
31+
const tool = withAuthorizedArchiveResult(
32+
stringTool({
33+
definition: {
34+
name: "manage_tasks",
35+
description: "tasks",
36+
inputSchema: { type: "object", properties: {} },
37+
},
38+
handler: async () => "listed 1 task",
39+
}),
40+
() => archive,
41+
);
42+
expect(tool.kind).toBe("full");
43+
if (tool.kind !== "full") return;
44+
const result = await tool.handler(
45+
{ id: "call-mt", name: "manage_tasks", arguments: { action: "create", tasks: [] } },
46+
new AbortController().signal,
47+
);
48+
expect(result.content).toBe("listed 1 task");
49+
const occs = await archive.listOccurrences();
50+
const hits = occs.filter((occ) => occ.kind === "tool_result" && occ.callId === "call-mt");
51+
expect(hits).toHaveLength(1);
52+
const recorded = hits[0];
53+
expect(recorded?.provenance).toBe("agent:post-policy");
54+
expect(recorded).toBeDefined();
55+
if (recorded === undefined) return;
56+
expect(await archive.readAuthorizedPayload(recorded.occurrenceId)).toContain("listed 1 task");
57+
});
58+
59+
test("does not record when the archive getter is omitted", async () => {
60+
const inner = stringTool({
61+
definition: {
62+
name: "use_skill",
63+
description: "skill",
64+
inputSchema: { type: "object", properties: {} },
65+
},
66+
handler: async () => "ok",
67+
});
68+
expect(withAuthorizedArchiveResult(inner, undefined)).toBe(inner);
69+
});
70+
});
71+
72+
describe("createAgentToolset authorized result capture", () => {
73+
test("records manage_tasks and does not double-record posix read_file", async () => {
74+
const cwd = mkdtempSync(join(tmpdir(), "corbits-archive-capture-"));
75+
writeFileSync(join(cwd, "note.txt"), "hello");
76+
const archive = memoryArchive("sess-capture");
77+
const { createAgentToolset } = await import("./tools.js");
78+
const permissionGate = createPermissionGate({
79+
approvals: [],
80+
interactive: false,
81+
skipPermissions: true,
82+
reactorGated: false,
83+
cwd,
84+
});
85+
const toolset = await createAgentToolset({
86+
cwd,
87+
permissionGate,
88+
onOperatorGate: async () => ({ kind: "option", index: 0 }),
89+
getEvidenceArchive: () => archive,
90+
});
91+
try {
92+
const tasks = await toolset.dynamicRunner.run(
93+
{
94+
id: "call-mt",
95+
name: "manage_tasks",
96+
arguments: {
97+
action: "create",
98+
tasks: [{ id: "t1", title: "one" }],
99+
},
100+
},
101+
new AbortController().signal,
102+
);
103+
expect(tasks.isError).toBeFalsy();
104+
const posix = await toolset.dynamicRunner.run(
105+
{
106+
id: "call-rf",
107+
name: "read_file",
108+
arguments: { path: join(cwd, "note.txt") },
109+
},
110+
new AbortController().signal,
111+
);
112+
expect(posix.isError).toBeFalsy();
113+
const occs = await archive.listOccurrences();
114+
const mt = occs.filter((occ) => occ.kind === "tool_result" && occ.callId === "call-mt");
115+
const rf = occs.filter((occ) => occ.kind === "tool_result" && occ.callId === "call-rf");
116+
expect(mt).toHaveLength(1);
117+
expect(mt[0]?.provenance).toBe("agent:post-policy");
118+
expect(rf).toHaveLength(1);
119+
expect(rf[0]?.provenance).toBe("posix:post-policy-pre-truncation");
120+
} finally {
121+
await toolset.dispose();
122+
}
123+
});
124+
});

‎src/agent/archive-tool-result.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import type { AgentTool } from "@intx/agent";
2+
import {
3+
applyRecordingPolicyToValue,
4+
type CompactionArchive,
5+
} from "../session/compaction-archive.js";
6+
7+
async function recordAuthorizedToolResult(
8+
getArchive: () => CompactionArchive | undefined,
9+
callId: string,
10+
content: unknown,
11+
isError: boolean,
12+
): Promise<void> {
13+
const archive = getArchive();
14+
if (archive === undefined) return;
15+
await archive.recordAuthorizedPayload({
16+
kind: "tool_result",
17+
payload: applyRecordingPolicyToValue(content),
18+
callId,
19+
provenance: isError ? "agent:error" : "agent:post-policy",
20+
});
21+
}
22+
23+
/** Record authorized results for non-posix AgentTools. Posix tools already record via resultTruncationPlugin. */
24+
export function withAuthorizedArchiveResult(
25+
tool: AgentTool,
26+
getArchive: (() => CompactionArchive | undefined) | undefined,
27+
): AgentTool {
28+
if (getArchive === undefined) return tool;
29+
if (tool.kind === "string") {
30+
const inner = tool.handler;
31+
return {
32+
kind: "full",
33+
definition: tool.definition,
34+
handler: async (call, signal) => {
35+
try {
36+
const content = await inner(call.arguments, signal);
37+
await recordAuthorizedToolResult(getArchive, call.id, content, false);
38+
return { callId: call.id, content };
39+
} catch (err) {
40+
const content = err instanceof Error ? err.message : String(err);
41+
await recordAuthorizedToolResult(getArchive, call.id, content, true);
42+
return { callId: call.id, content, isError: true };
43+
}
44+
},
45+
};
46+
}
47+
const inner = tool.handler;
48+
return {
49+
kind: "full",
50+
definition: tool.definition,
51+
handler: async (call, signal) => {
52+
const result = await inner(call, signal);
53+
await recordAuthorizedToolResult(
54+
getArchive,
55+
call.id,
56+
result.content,
57+
result.isError === true,
58+
);
59+
return result;
60+
},
61+
};
62+
}

‎src/agent/posix-tool-plugins.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,42 @@ describe("buildCorePosixToolPlugins", () => {
350350
}
351351
});
352352

353+
test("always mounts the archive search plugin so workers fail-close on archive:///", async () => {
354+
const cwd = await mkdtemp(join(tmpdir(), "ic-posix-plugins-"));
355+
try {
356+
const gate = createPermissionGate({
357+
approvals: [],
358+
interactive: false,
359+
skipPermissions: true,
360+
reactorGated: false,
361+
cwd,
362+
});
363+
const plugins = buildCorePosixToolPlugins({ cwd, permissionGate: gate });
364+
const archiveIndex = findMiddlewareIndex(
365+
plugins,
366+
"evidence archive is not available in this session",
367+
);
368+
expect(archiveIndex).toBeGreaterThanOrEqual(0);
369+
370+
const runner = createPosixTools({
371+
cwd,
372+
plugins,
373+
});
374+
const result = await runner.run(
375+
{
376+
id: "call-archive",
377+
name: "read_file",
378+
arguments: { path: "archive:///occ-missing" },
379+
},
380+
new AbortController().signal,
381+
);
382+
expect(result.isError).toBe(true);
383+
expect(String(result.content)).toContain("evidence archive is not available");
384+
} finally {
385+
await rm(cwd, { recursive: true, force: true });
386+
}
387+
});
388+
353389
test("a real line-range edit_file call verifies as success through the wired plugin chain", async () => {
354390
const cwd = await mkdtemp(join(tmpdir(), "ic-posix-plugins-"));
355391
try {

‎src/agent/posix-tool-plugins.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,14 @@ export function buildCorePosixToolPlugins(args: CorePosixToolPluginsArgs): ToolP
9696
resultTruncationPlugin(truncationOptions),
9797
toolResultSecretScrubPlugin(),
9898
pathEscapePlugin(cwd, createWorktreeRootsProvider(cwd), { allowOutside }),
99-
evidenceArchivePathGuardPlugin(),
99+
evidenceArchivePathGuardPlugin(() => getEvidenceArchive?.() !== undefined),
100100
deleteFilePlugin(cwd, { allowOutside }),
101101
toolOutputUriPlugin(),
102102
secretGuardPlugin(),
103103
authzPlugin(),
104104
permissionPlugin(permissionGate),
105105
shellGuardPlugin(cwd, shellTimeout, shellEnv, { allowOutsideCwd: allowOutside }),
106-
...(getEvidenceArchive !== undefined ? [evidenceArchiveSearchPlugin(getEvidenceArchive)] : []),
106+
evidenceArchiveSearchPlugin(getEvidenceArchive ?? (() => undefined)),
107107
readFileGuardPlugin(cwd, readFileGuard),
108108
ripgrepPlugin(cwd),
109109
// Verify wraps the line-range short-circuit (composeMiddleware runs plugins

‎src/agent/tools.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ import {
8484
type CodexRunTool,
8585
} from "./codex-tool-proxies.js";
8686
import { createCodexReadRawFile } from "./codex-read-raw-file.js";
87+
import { withAuthorizedArchiveResult } from "./archive-tool-result.js";
8788
import type { ReactorEmittedEvent } from "@intx/inference";
8889

8990
const AskOperatorArgs = type({
@@ -345,6 +346,8 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
345346
}),
346347
});
347348

349+
const posixToolNames = new Set(posixTools.definitions.map((definition) => definition.name));
350+
348351
// Codex apply_patch proxy forwards ops through posixTools.run so permission
349352
// plugins (gate, path policy, etc.) still apply — same call shape as
350353
// posix-tool-plugins.test.ts.
@@ -577,7 +580,13 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
577580
}),
578581
);
579582

580-
const primaryTools = baseTools.filter((tool) => tool.definition.name !== "apply_patch");
583+
const primaryTools = baseTools
584+
.filter((tool) => tool.definition.name !== "apply_patch")
585+
.map((tool) =>
586+
posixToolNames.has(tool.definition.name)
587+
? tool
588+
: withAuthorizedArchiveResult(tool, getEvidenceArchive),
589+
);
581590

582591
const dynamicRunner = createDynamicToolRunner(primaryTools, toolWatchdog);
583592
runnerHolder.current = dynamicRunner;

‎src/plugins/evidence-archive-path-guard.test.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ describe("isProtectedEvidenceLocation", () => {
2828
});
2929

3030
describe("evidenceArchivePathGuardPlugin", () => {
31-
test("denies path tools targeting evidence-archive or tool-output/archive-*", async () => {
31+
test("denies dump-path tools without coaching archive:/// when the archive is unavailable", async () => {
3232
const plugin = evidenceArchivePathGuardPlugin();
3333
const handler = plugin.middleware ? plugin.middleware(nextHandler) : nextHandler;
3434
const denied = [
@@ -43,11 +43,23 @@ describe("evidenceArchivePathGuardPlugin", () => {
4343
for (const call of denied) {
4444
const result = await handler(call, new AbortController().signal);
4545
expect(result.isError).toBe(true);
46-
expect(String(result.content)).toContain("search_files");
47-
expect(String(result.content)).toContain("archive:///");
46+
expect(String(result.content)).toContain("Cannot read evidence-archive");
47+
expect(String(result.content)).not.toContain("archive:///");
4848
}
4949
});
5050

51+
test("coaches archive:/// refs when the archive is available", async () => {
52+
const plugin = evidenceArchivePathGuardPlugin(() => true);
53+
const handler = plugin.middleware ? plugin.middleware(nextHandler) : nextHandler;
54+
const result = await handler(
55+
makeCall("read_file", { path: "evidence-archive/index.jsonl" }),
56+
new AbortController().signal,
57+
);
58+
expect(result.isError).toBe(true);
59+
expect(String(result.content)).toContain("search_files");
60+
expect(String(result.content)).toContain("archive:///");
61+
});
62+
5163
test("does not deny a grep pattern that mentions evidence-archive", async () => {
5264
const plugin = evidenceArchivePathGuardPlugin();
5365
const handler = plugin.middleware ? plugin.middleware(nextHandler) : nextHandler;

‎src/plugins/evidence-archive-path-guard.ts‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ const PATH_TOOLS = new Set([
1212
"delete_file",
1313
]);
1414

15-
const DENY_MESSAGE =
16-
"Cannot read evidence-archive or tool-output/archive-* dumps. Use search_files, grep, or read_file with archive:/// refs.";
15+
const DUMP_DENY = "Cannot read evidence-archive or tool-output/archive-* dumps.";
16+
const ARCHIVE_HINT = " Use search_files, grep, or read_file with archive:/// refs.";
1717

1818
export function isProtectedEvidenceLocation(value: string): boolean {
1919
const normalized = value.replaceAll("\\", "/");
@@ -23,13 +23,16 @@ export function isProtectedEvidenceLocation(value: string): boolean {
2323
return false;
2424
}
2525

26-
export function evidenceArchivePathGuardPlugin(): ToolPlugin {
26+
export function evidenceArchivePathGuardPlugin(
27+
archiveAvailable: () => boolean = () => false,
28+
): ToolPlugin {
2729
return {
2830
middleware: (next) => async (call, signal) => {
2931
if (!PATH_TOOLS.has(call.name)) return next(call, signal);
3032
for (const [key, value] of Object.entries(call.arguments)) {
3133
if (typeof value === "string" && looksLikePath(key) && isProtectedEvidenceLocation(value)) {
32-
return { callId: call.id, content: DENY_MESSAGE, isError: true };
34+
const content = archiveAvailable() ? `${DUMP_DENY}${ARCHIVE_HINT}` : DUMP_DENY;
35+
return { callId: call.id, content, isError: true };
3336
}
3437
}
3538
return next(call, signal);

‎src/plugins/evidence-archive-search-plugin.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,48 @@ describe("evidenceArchiveSearchPlugin", () => {
321321
expect(content).toContain(`${firstRef}-1-before-one`);
322322
expect(content).toContain(`${secondRef}-1-before-two`);
323323
});
324+
325+
test("search_files glob does not match formatArchiveRef URI strings", async () => {
326+
const archive = memoryArchive("sess-glob");
327+
const occ = await archive.recordAuthorizedPayload({
328+
kind: "user_message",
329+
payload: "plain-payload",
330+
});
331+
const plugin = evidenceArchiveSearchPlugin(() => archive);
332+
const handler = plugin.middleware ? plugin.middleware(nextHandler) : nextHandler;
333+
const uriHits = await handler(
334+
makeCall("search_files", { pattern: "*archive*", path: "archive:///" }),
335+
new AbortController().signal,
336+
);
337+
expect(String(uriHits.content)).not.toContain(formatArchiveRef(occ.occurrenceId));
338+
expect(String(uriHits.content)).toContain("No evidence-archive occurrences matched");
339+
340+
const idHits = await handler(
341+
makeCall("search_files", { pattern: occ.occurrenceId, path: "archive:///" }),
342+
new AbortController().signal,
343+
);
344+
expect(String(idHits.content)).toContain(formatArchiveRef(occ.occurrenceId));
345+
346+
const kindHits = await handler(
347+
makeCall("search_files", { pattern: "user_message", path: "archive:///" }),
348+
new AbortController().signal,
349+
);
350+
expect(String(kindHits.content)).toContain(formatArchiveRef(occ.occurrenceId));
351+
});
352+
353+
test("returns an explicit error when the archive getter is undefined", async () => {
354+
const plugin = evidenceArchiveSearchPlugin(() => undefined);
355+
const handler = plugin.middleware ? plugin.middleware(nextHandler) : nextHandler;
356+
for (const call of [
357+
makeCall("read_file", { path: "archive:///occ-abc" }),
358+
makeCall("grep", { pattern: "foo", path: "archive:///" }),
359+
makeCall("search_files", { pattern: "*", path: "archive:///" }),
360+
]) {
361+
const result = await handler(call, new AbortController().signal);
362+
expect(result.isError).toBe(true);
363+
expect(String(result.content)).toContain("evidence archive is not available");
364+
}
365+
});
324366
});
325367

326368
describe("createAgentToolset archive mount", () => {

0 commit comments

Comments
 (0)