Skip to content

Commit b44a74d

Browse files
feat(read): diagnose blocked file inspection with a next action (#1232)
* fix(auth): point store tests at the testkit mock helper The credential-lock tests imported a path that does not exist, so typecheck failed on main. Every other mock.module caller already uses testkit. * feat(read): diagnose blocked file inspection with a next action read_file used to dump PDFs as text or refuse binaries without saying whether the gap was a missing extractor, a worker that cannot run host commands, or a malformed file. The first failure now names that cause and the concrete unblock.
1 parent 08b4137 commit b44a74d

6 files changed

Lines changed: 450 additions & 7 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ tool call
415415
- **Authorization** (`run-shell-authz.ts`, enforced by the permission gate) — Denies catastrophic shell command patterns by regex, and hard-blocks shell `find`, head-position `rg`, and recursive `grep -r` (they can walk huge trees and OOM the host). Bounded `grep`/`search_files` tools remain practical alternatives (timeout + output caps); the patterns match those three command shapes only — an `ls -R`, `fd`, or scripted `os.walk` is just as unbounded and is not caught, so the block message tells the model not to substitute one. The gate hard-denies these at the top of its verdict path — before auto-allow, prompting, grants, and skipPermissions — so no mode or stored grant can admit them.
416416
- **Permission** (`permission-plugin.ts`) — Delegates consequential calls to the permission gate.
417417
- **Shell Guard** (`shell-guard-plugin.ts`) — Corbits Code-only replacement for stock `run_shell` (interchange stays unpatched): 120s foreground default (`settings.shell.timeoutMs` overrides that default only; a positive per-call `timeout` is the bound with no ceiling — `maxTimeoutMs` does not clamp it), background `run_shell` has no default (only a per-call timeout arms a timer), 512KB display cap with head+tail retention (the process keeps running when the cap is hit), process-group kill on timeout, abort, and plugin dispose (live children tracked in the plugin and reaped by `posixTools.dispose`), and `background: true` — the call returns a `shell_id` at once (registry in `src/shell/background-shell.ts`), the process group keeps running past the turn, completion is process-exit (stdio-close is not required), delivered on a later turn via `buildShellBackgroundMessage`, and `shell_collect` retrieves or cancels with a 300s wait cap (schema advertised by `advertiseShellGuardTimeout` only when `shell_collect` is mounted; evaluated by the permission chain at start time like any shell call). Foreground expiry is exit 124 + `timedOut:true` plus a nudge to retry with `background:true`. Also applies a 10s wall-clock budget to `grep`/`search_files` (budget expiry and budget-torn-down aborts surface as explicit timeout errors, never as empty results; an unbounded workspace-root `search_files` walk is refused up front with scope guidance instead of running to the budget). Ripgrep detached spawns are not tracked.
418-
- **Read File Guard** (`read-file-guard-plugin.ts`) — Corbits Code-only short-circuit for `read_file` on real filesystem paths and configured `tool-output://` URIs (interchange stays unpatched): streaming reads that never decode the whole file in one pass, caps model-facing output at 50KB, defaults to 2000 lines, truncates long lines with recovery hints, samples the first chunk to reject binary, and stops at an 8MB scan ceiling. Emits `offset` continuation notices so the model can page without losing file or spill content on disk.
418+
- **Read File Guard** (`read-file-guard-plugin.ts`) — Corbits Code-only short-circuit for `read_file` on real filesystem paths and configured `tool-output://` URIs (interchange stays unpatched): streaming reads that never decode the whole file in one pass, caps model-facing output at 50KB, defaults to 2000 lines, truncates long lines with recovery hints, samples the first chunk to reject binary and to diagnose blocked PDF inspection (missing `pdftotext` vs installed-but-worker-cannot-execute vs malformed/unreadable file, each with one next action), and stops at an 8MB scan ceiling. Emits `offset` continuation notices so the model can page without losing file or spill content on disk.
419419
- **Verify** (`verify-plugin.ts`) — Re-reads after `write_file` / `edit_file` and errors on mismatch. Per-path serialization (`file-mutation-lock.ts`) prevents parallel edits on one file from tripping verification.
420420
- **Edit file line range** (`edit-file-line-range-plugin.ts`) — Corbits Code-only short-circuit for `edit_file` mode B (`start_line`/`end_line`/`new_string`), same pattern as shell-guard; schema advertised via `advertiseEditFileLineRange`. Modes are mutually exclusive: a call supplying both `old_string` and `start_line`/`end_line` is rejected with a recoverable error naming which fields to omit (no file-content disambiguation).
421421
- **Edit file diagnostics** (`edit-file-diagnostics-plugin.ts`) — On stock substring mismatch (`old_string not found` / not unique), appends nearby file context (whitespace near-miss, occurrence lines) without changing match semantics.
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import { describe, expect, test } from "bun:test";
2+
import {
3+
chunkLooksLikePdf,
4+
diagnoseBlockedFileInspection,
5+
inspectionKindFromFirstChunk,
6+
pathLooksLikePdf,
7+
PDF_EXTRACTOR_BIN,
8+
} from "./file-inspection-diagnosis.js";
9+
10+
const PATH = "/tmp/report.pdf";
11+
12+
describe("PDF sniffing", () => {
13+
test("magic bytes classify as pdf regardless of suffix", () => {
14+
expect(chunkLooksLikePdf(Buffer.from("%PDF-1.7\n"))).toBe(true);
15+
expect(
16+
inspectionKindFromFirstChunk("notes.bin", Buffer.from("%PDF-1.4")),
17+
).toBe("pdf");
18+
});
19+
20+
test("a .pdf name without magic is malformed, not a capability miss", () => {
21+
expect(pathLooksLikePdf("report.PDF")).toBe(true);
22+
expect(
23+
inspectionKindFromFirstChunk("report.pdf", Buffer.from("not-a-pdf\0")),
24+
).toBe("pdf-malformed");
25+
expect(
26+
inspectionKindFromFirstChunk("report.pdf", Buffer.from("plain text")),
27+
).toBe("pdf-malformed");
28+
});
29+
30+
test("NUL without PDF markers is ordinary binary", () => {
31+
expect(
32+
inspectionKindFromFirstChunk("blob.bin", Buffer.from([0x41, 0x00, 0x42])),
33+
).toBe("binary");
34+
});
35+
36+
test("plain text is not an inspection block", () => {
37+
expect(
38+
inspectionKindFromFirstChunk("readme.txt", Buffer.from("hello\n")),
39+
).toBeUndefined();
40+
});
41+
});
42+
43+
describe("diagnoseBlockedFileInspection", () => {
44+
test("missing extractor names the capability and an install action", () => {
45+
const result = diagnoseBlockedFileInspection({
46+
path: PATH,
47+
kind: "pdf",
48+
extractorAvailable: false,
49+
canExecuteHostCommands: true,
50+
});
51+
expect(result.code).toBe("missing_extractor");
52+
expect(result.message).toContain("PDF");
53+
expect(result.message).toContain(PDF_EXTRACTOR_BIN);
54+
expect(result.message).toContain("missing");
55+
expect(result.message.toLowerCase()).toContain("poppler");
56+
expect(result.message).toContain("not a malformed file");
57+
expect(result.message).not.toContain("permission boundary");
58+
});
59+
60+
test("missing extractor wins over an unmounted shell (first gap)", () => {
61+
const result = diagnoseBlockedFileInspection({
62+
path: PATH,
63+
kind: "pdf",
64+
extractorAvailable: false,
65+
canExecuteHostCommands: false,
66+
});
67+
expect(result.code).toBe("missing_extractor");
68+
});
69+
70+
test("installed extractor with no run_shell is a permission boundary", () => {
71+
const result = diagnoseBlockedFileInspection({
72+
path: PATH,
73+
kind: "pdf",
74+
extractorAvailable: true,
75+
canExecuteHostCommands: false,
76+
});
77+
expect(result.code).toBe("permission_boundary");
78+
expect(result.message).toContain("installed");
79+
expect(result.message).toContain("permission boundary");
80+
expect(result.message).toContain("run_shell");
81+
expect(result.message).toContain("not a malformed file");
82+
expect(result.message.toLowerCase()).not.toContain("brew install");
83+
});
84+
85+
test("installed extractor the worker can run names the bash extract command", () => {
86+
const result = diagnoseBlockedFileInspection({
87+
path: PATH,
88+
kind: "pdf",
89+
extractorAvailable: true,
90+
canExecuteHostCommands: true,
91+
});
92+
expect(result.code).toBe("extractor_ready");
93+
expect(result.message).toContain("bash");
94+
expect(result.message).toContain(`${PDF_EXTRACTOR_BIN} ${PATH} -`);
95+
});
96+
97+
test("malformed PDF is not blamed on tooling or permission", () => {
98+
const result = diagnoseBlockedFileInspection({
99+
path: PATH,
100+
kind: "pdf-malformed",
101+
extractorAvailable: false,
102+
canExecuteHostCommands: false,
103+
});
104+
expect(result.code).toBe("malformed");
105+
expect(result.message).toContain("not a valid PDF");
106+
expect(result.message).toContain("not a missing extractor");
107+
expect(result.message).not.toContain("brew install");
108+
expect(result.message).not.toContain("grant this worker");
109+
});
110+
111+
test("unreadable file is filesystem permission, not a worker-tool gap", () => {
112+
const result = diagnoseBlockedFileInspection({
113+
path: PATH,
114+
kind: "unreadable",
115+
extractorAvailable: true,
116+
canExecuteHostCommands: true,
117+
});
118+
expect(result.code).toBe("unreadable");
119+
expect(result.message).toContain("unreadable");
120+
expect(result.message).toContain("filesystem permission");
121+
expect(result.message).not.toContain("grant this worker");
122+
expect(result.message.toLowerCase()).not.toContain("poppler");
123+
});
124+
125+
test("ordinary binary is not a missing-tool failure", () => {
126+
const result = diagnoseBlockedFileInspection({
127+
path: "/tmp/blob.bin",
128+
kind: "binary",
129+
extractorAvailable: false,
130+
canExecuteHostCommands: false,
131+
});
132+
expect(result.code).toBe("binary");
133+
expect(result.message).toContain("refusing to read binary file");
134+
expect(result.message).toContain("not a missing-tool");
135+
});
136+
});
Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
import { extname } from "node:path";
2+
3+
/** Host binary that extracts text from a PDF (poppler). */
4+
export const PDF_EXTRACTOR_BIN = "pdftotext";
5+
6+
const PDF_MAGIC = Buffer.from("%PDF");
7+
8+
export type FileInspectionKind =
9+
| "pdf"
10+
| "pdf-malformed"
11+
| "binary"
12+
| "unreadable";
13+
14+
export type FileInspectionBlockCode =
15+
| "missing_extractor"
16+
| "permission_boundary"
17+
| "extractor_ready"
18+
| "malformed"
19+
| "unreadable"
20+
| "binary";
21+
22+
export interface FileInspectionDiagnosis {
23+
code: FileInspectionBlockCode;
24+
message: string;
25+
}
26+
27+
export interface DiagnoseBlockedFileInspectionInput {
28+
path: string;
29+
kind: FileInspectionKind;
30+
extractorAvailable: boolean;
31+
canExecuteHostCommands: boolean;
32+
extractorName?: string;
33+
}
34+
35+
export function chunkLooksLikePdf(chunk: Uint8Array): boolean {
36+
return (
37+
chunk.length >= PDF_MAGIC.length &&
38+
Buffer.from(chunk.subarray(0, PDF_MAGIC.length)).equals(PDF_MAGIC)
39+
);
40+
}
41+
42+
export function pathLooksLikePdf(filePath: string): boolean {
43+
return extname(filePath).toLowerCase() === ".pdf";
44+
}
45+
46+
/**
47+
* Classify the first streamed chunk. PDF magic wins even without a .pdf
48+
* suffix. A .pdf name without magic is malformed, whether or not the chunk
49+
* looks binary. NUL without PDF markers is an ordinary binary file.
50+
*/
51+
export function inspectionKindFromFirstChunk(
52+
filePath: string,
53+
chunk: Uint8Array,
54+
): FileInspectionKind | undefined {
55+
if (chunkLooksLikePdf(chunk)) return "pdf";
56+
if (pathLooksLikePdf(filePath)) return "pdf-malformed";
57+
if (chunk.includes(0)) return "binary";
58+
return undefined;
59+
}
60+
61+
function extractorLabel(name: string | undefined): string {
62+
return name !== undefined && name.length > 0 ? name : PDF_EXTRACTOR_BIN;
63+
}
64+
65+
/**
66+
* Operator- and model-facing diagnosis when read_file cannot inspect a file.
67+
* One fact (capability vs permission vs bad file) plus one next action.
68+
*/
69+
export function diagnoseBlockedFileInspection(
70+
input: DiagnoseBlockedFileInspectionInput,
71+
): FileInspectionDiagnosis {
72+
const extractor = extractorLabel(input.extractorName);
73+
const path = input.path;
74+
75+
if (input.kind === "unreadable") {
76+
return {
77+
code: "unreadable",
78+
message:
79+
`Cannot inspect ${path}: the file is unreadable (filesystem permission denied), not a missing extractor or worker-tool gap. ` +
80+
`Check ownership and mode, or copy the file into a readable workspace path.`,
81+
};
82+
}
83+
84+
if (input.kind === "pdf-malformed") {
85+
return {
86+
code: "malformed",
87+
message:
88+
`Cannot inspect ${path}: the file is not a valid PDF (malformed or unreadable bytes), not a missing extractor or permission boundary. ` +
89+
`Open it in a PDF reader or re-export it, then retry.`,
90+
};
91+
}
92+
93+
if (input.kind === "binary") {
94+
return {
95+
code: "binary",
96+
message:
97+
`refusing to read binary file: ${path}. This is a binary file, not a missing-tool or permission failure. ` +
98+
`Use a format-specific extractor if you need text from it.`,
99+
};
100+
}
101+
102+
// kind === "pdf"
103+
if (!input.extractorAvailable) {
104+
return {
105+
code: "missing_extractor",
106+
message:
107+
`Cannot inspect PDF ${path}: ${extractor} is not installed (missing PDF-extraction capability), not a malformed file. ` +
108+
`Install poppler so ${extractor} is on PATH (for example \`brew install poppler\` or \`apt-get install poppler-utils\`), then retry.`,
109+
};
110+
}
111+
112+
if (!input.canExecuteHostCommands) {
113+
return {
114+
code: "permission_boundary",
115+
message:
116+
`Cannot inspect PDF ${path}: ${extractor} is installed, but this worker cannot execute it (run_shell is not mounted — a permission boundary, not a malformed file). ` +
117+
`Re-dispatch to a director that mounts run_shell, or grant this worker run_shell, then retry.`,
118+
};
119+
}
120+
121+
return {
122+
code: "extractor_ready",
123+
message:
124+
`Cannot inspect PDF ${path} as text via read_file. ${extractor} is installed and this session can run host commands. ` +
125+
`Run \`${extractor} ${path} -\` via bash to extract text.`,
126+
};
127+
}

‎src/plugins/read-file-guard-plugin.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,57 @@ describe("readFileBounded", () => {
126126
);
127127
expect(isError).toBe(true);
128128
expect(content).toContain("binary");
129+
expect(content).toContain("not a missing-tool");
130+
});
131+
132+
test("a PDF without pdftotext names the missing extractor, not a malformed file", async () => {
133+
const p = await fixture("report.pdf", Buffer.from("%PDF-1.4\n1 0 obj\n"));
134+
const { content, isError } = await readFileBounded(
135+
p,
136+
0,
137+
2000,
138+
neverAbort(),
139+
{ whichExtractor: () => null, canExecuteHostCommands: () => true },
140+
);
141+
expect(isError).toBe(true);
142+
expect(content).toContain("pdftotext");
143+
expect(content).toContain("missing");
144+
expect(content).toContain("not a malformed file");
145+
expect(content).not.toContain("permission boundary");
146+
});
147+
148+
test("a PDF with pdftotext installed but no run_shell names the permission boundary", async () => {
149+
const p = await fixture("locked.pdf", Buffer.from("%PDF-1.7\n"));
150+
const { content, isError } = await readFileBounded(
151+
p,
152+
0,
153+
2000,
154+
neverAbort(),
155+
{
156+
whichExtractor: () => "/usr/bin/pdftotext",
157+
canExecuteHostCommands: () => false,
158+
},
159+
);
160+
expect(isError).toBe(true);
161+
expect(content).toContain("installed");
162+
expect(content).toContain("permission boundary");
163+
expect(content).toContain("run_shell");
164+
expect(content).toContain("not a malformed file");
165+
});
166+
167+
test("a .pdf name without magic is malformed, not a missing extractor", async () => {
168+
const p = await fixture("fake.pdf", Buffer.from("not a pdf\0"));
169+
const { content, isError } = await readFileBounded(
170+
p,
171+
0,
172+
2000,
173+
neverAbort(),
174+
{ whichExtractor: () => null, canExecuteHostCommands: () => false },
175+
);
176+
expect(isError).toBe(true);
177+
expect(content).toContain("not a valid PDF");
178+
expect(content).toContain("not a missing extractor");
179+
expect(content).not.toContain("brew install");
129180
});
130181

131182
test("a NUL deep in an otherwise-valid file does not discard streamed content", async () => {
@@ -737,6 +788,42 @@ describe("readFileGuardPlugin", () => {
737788
expect(replay.isError).toBe(true);
738789
expect(String(replay.content)).toBe(String(first.content));
739790
});
791+
792+
test("PDF inspection: extractor unavailable then installed but worker cannot execute", async () => {
793+
await fixture("flow.pdf", Buffer.from("%PDF-1.4\ntrailer\n"));
794+
const call: ToolCall = {
795+
id: "pdf-flow",
796+
name: "read_file",
797+
arguments: { path: "flow.pdf" },
798+
};
799+
800+
const missing = readFileGuardPlugin(dir, {
801+
whichExtractor: () => null,
802+
canExecuteHostCommands: () => true,
803+
});
804+
const missingResult = await defined(missing.middleware)(fallback)(
805+
call,
806+
neverAbort(),
807+
);
808+
expect(missingResult.isError).toBe(true);
809+
expect(String(missingResult.content)).toContain("pdftotext");
810+
expect(String(missingResult.content)).toContain("missing");
811+
expect(String(missingResult.content)).toContain("not a malformed file");
812+
813+
const blocked = readFileGuardPlugin(dir, {
814+
whichExtractor: () => "/opt/homebrew/bin/pdftotext",
815+
canExecuteHostCommands: () => false,
816+
});
817+
const blockedResult = await defined(blocked.middleware)(fallback)(
818+
call,
819+
neverAbort(),
820+
);
821+
expect(blockedResult.isError).toBe(true);
822+
expect(String(blockedResult.content)).toContain("installed");
823+
expect(String(blockedResult.content)).toContain("permission boundary");
824+
expect(String(blockedResult.content)).toContain("run_shell");
825+
expect(String(blockedResult.content)).not.toContain("brew install");
826+
});
740827
});
741828

742829
describe("CL-8980 single-way path+offset resume", () => {

0 commit comments

Comments
 (0)