bc22f3dbc8d1f48083c9b67f071851c619d02ec9
- Author
- TheEdgeOfRage <git@theedgeofrage.com>
- Committer
- TheEdgeOfRage <git@theedgeofrage.com>
- Date
Message
Parse Bash redirects before policy checks
Diff
This diff is truncated to protect this page.
1diff --git a/src/core/deterministic.ts b/src/core/deterministic.ts
2index 257422432e44234e867e5d8ebc4c5ddd8083cac2..20a72afd5e0218752d668d4fa0f278e04a4d53f7 100644
3--- a/src/core/deterministic.ts
4+++ b/src/core/deterministic.ts
5@@ -2,6 +2,12 @@ import type { Decision } from "./types";
6 import { ALLOW_COMMANDS, ASK_PATTERNS, SHELL_CONTROL_RE, STDERR_REDIRECT_RE, DEVNULL_REDIRECT_RE } from "./rules";
7 import { stripRtkPrefix, expandHome } from "./normalize";
8
9+export type BashRedirect = {
10+ path: string;
11+ writes: boolean;
12+ dynamic: boolean;
13+};
14+
15 const DESTRUCTIVE_RM_TARGETS = new Set(["/", "/*", "~", "~/*", "/etc", "/usr", "/var", "/home", "/root"]);
16 const SYSTEM_DIRECTORIES = [
17 "/",
18@@ -21,9 +27,55 @@ const SYSTEM_DIRECTORIES = [
19 "/var",
20 ];
21 const LOCAL_MUTATION_COMMANDS = new Set(["chmod", "chown", "cp", "ln", "mkdir", "mv", "rm", "rmdir", "tee", "touch"]);
22+const FIND_OPERATORS = new Set(["!", "-not", "-a", "-and", "-o", "-or", "(", ")"]);
23+const FIND_UNARY_EXPRESSIONS = new Set([
24+ "-depth",
25+ "-empty",
26+ "-ls",
27+ "-print",
28+ "-print0",
29+ "-prune",
30+ "-readable",
31+ "-writable",
32+ "-executable",
33+]);
34+
35+function hasShellControl(command: string): boolean {
36+ let quote: "'" | '"' | undefined;
37+ for (let index = 0; index < command.length; index += 1) {
38+ const character = command[index];
39+ const nextCharacter = command[index + 1];
40+
41+ if (quote === "'") {
42+ if (character === "'") quote = undefined;
43+ continue;
44+ }
45+ if (quote === '"') {
46+ if (character === '"') {
47+ quote = undefined;
48+ continue;
49+ }
50+ if (character === "`" || (character === "$" && ["(", "{"].includes(nextCharacter ?? ""))) return true;
51+ if (character === "\\") index += 1;
52+ continue;
53+ }
54
55-function hasShellControl(cmd: string): boolean {
56- return SHELL_CONTROL_RE.test(cmd);
57+ if (character === "'") {
58+ quote = "'";
59+ continue;
60+ }
61+ if (character === '"') {
62+ quote = '"';
63+ continue;
64+ }
65+ if (character === "\\") {
66+ index += 1;
67+ continue;
68+ }
69+ if (character === "$" && ["(", "{"].includes(nextCharacter ?? "")) return true;
70+ if (SHELL_CONTROL_RE.test(character)) return true;
71+ }
72+ return quote !== undefined;
73 }
74
75 function shellWords(command: string): string[] {
76@@ -93,7 +145,7 @@ function hasOption(arguments_: string[], optionNames: ReadonlySet<string>): bool
77
78 function hasSecretPathOperand(command: string): boolean {
79 const [program, ...arguments_] = shellWords(command);
80- if (!program) return false;
81+ if (!program || program === "cd") return false;
82
83 if (program === "grep" || program === "rg") {
84 const fileOptions =
85@@ -153,6 +205,63 @@ function isDestructiveRmTarget(word: string): boolean {
86 return DESTRUCTIVE_RM_TARGETS.has(path) || path === process.env.HOME;
87 }
88
89+function findToken(argument: string): string {
90+ return argument === "\\(" ? "(" : argument === "\\)" ? ")" : argument;
91+}
92+
93+function findRootPaths(arguments_: string[]): string[] {
94+ const roots: string[] = [];
95+ for (const argument of arguments_) {
96+ const token = findToken(argument);
97+ if (token.startsWith("-") || FIND_OPERATORS.has(token)) break;
98+ roots.push(argument);
99+ }
100+ return roots;
101+}
102+
103+function isSafeFind(arguments_: string[]): boolean {
104+ let index = findRootPaths(arguments_).length;
105diff --git a/src/core/rules.ts b/src/core/rules.ts
106index aad9bd762c4f227568189658a9881452595c7921..9d1594461d63255105b7885b777a4bb235ee8018 100644
107--- a/src/core/rules.ts
108+++ b/src/core/rules.ts
109@@ -1,7 +1,7 @@
110 import { DECISION_CATEGORIES } from "./types";
111
112 // Bump to invalidate all cached decisions when rules change.
113-export const POLICY_VERSION = 35;
114+export const POLICY_VERSION = 36;
115
116 // Trailing stderr redirections that are safe to strip before pattern matching.
117 // `2>&1` and `2>/dev/null` have no security implication but would otherwise
118@@ -22,6 +22,7 @@ export const ALLOW_COMMANDS: ReadonlySet<string> = new Set([
119 "bunx",
120 "cargo",
121 "cat",
122+ "cd",
123 "chmod",
124 "chown",
125 "composer",
126@@ -115,8 +116,7 @@ export const ASK_PATTERNS: RegExp[] = [
127 /^git(?:\s+-C\s+\S+)*\s+config\s+--system(?:\s|$)/,
128 ];
129
130-// Reject shell syntax from deterministic allow patterns so compound commands
131-// cannot receive a deterministic allow before their command nodes are evaluated.
132+// Raw core calls reject shell syntax. Parsed Pi commands provide structured redirects instead.
133 export const SHELL_CONTROL_RE = /[|;&`<>\r\n]|\$\(|\$\{/;
134
135 const EXTERNAL_DIRECTORIES_PLACEHOLDER = "<<EXTERNAL_DIRECTORIES>>";
136diff --git a/src/pi/bash-split.ts b/src/pi/bash-split.ts
137index dff2c9e7733dfbfb9207f8a9e2d884014dc0f78d..c5ecaa153fc3d6d9bb337b5776630424aca66dc1 100644
138--- a/src/pi/bash-split.ts
139+++ b/src/pi/bash-split.ts
140@@ -1,8 +1,14 @@
141 import { createRequire } from "node:module";
142 import { Language, Parser, type Node } from "web-tree-sitter";
143+import type { BashRedirect } from "../core/deterministic";
144+
145+export type BashSplitCommand = {
146+ source: string;
147+ redirects: BashRedirect[];
148+};
149
150 export type BashSplitResult = {
151- commands: string[];
152+ commands: BashSplitCommand[];
153 parsed: boolean;
154 };
155
156@@ -15,8 +21,36 @@ type BashParserLoader = () => Promise<BashParser>;
157 const require = createRequire(import.meta.url);
158 let parserPromise: Promise<BashParser> | undefined;
159
160-function source(node: Node): string {
161- return (node.parent?.type === "redirected_statement" ? node.parent.text : node.text).trim();
162+function redirectNodes(command: Node): Node[] {
163+ const redirects = [...command.childrenForFieldName("redirect")];
164+ for (let node = command.parent; node; node = node.parent) {
165+ if (node.type === "command_substitution" || node.type === "process_substitution") break;
166+ if (node.type === "redirected_statement") redirects.push(...node.childrenForFieldName("redirect"));
167+ }
168+ return redirects;
169+}
170+
171+function redirectPath(node: Node): string | undefined {
172+ const destination = node.childForFieldName("destination")?.text.trim();
173+ if (!destination || /[$`]/.test(destination)) return undefined;
174+ if (
175+ (destination.startsWith('"') && destination.endsWith('"')) ||
176+ (destination.startsWith("'") && destination.endsWith("'"))
177+ ) {
178+ return destination.slice(1, -1);
179+ }
180+ return destination;
181+}
182+
183+function redirects(command: Node): BashRedirect[] {
184+ return redirectNodes(command).map((node) => {
185+ const path = redirectPath(node);
186+ return {
187+ path: path ?? "",
188+ writes: node.text.includes(">"),
189+ dynamic: path === undefined,
190+ };
191+ });
192 }
193
194 async function loadBashParser(): Promise<BashParser> {
195@@ -35,7 +69,7 @@ async function loadBashParser(): Promise<BashParser> {
196 }
197
198 function raw(command: string): BashSplitResult {
199- return { commands: [command], parsed: false };
200+ return { commands: [{ source: command, redirects: [] }], parsed: false };
201 }
202
203 export async function splitBashCommand(
204@@ -47,14 +81,14 @@ export async function splitBashCommand(
205 const tree = parser.parse(command);
206 if (!tree || tree.rootNode.hasError) return raw(command);
207 try {
208- const commands: string[] = [];
209+ const commands: BashSplitCommand[] = [];
210 const seen = new Set<string>();
211 for (const node of tree.rootNode.descendantsOfType("command")) {
212 if (!node) continue;
213- const text = source(node);
214- if (text && !seen.has(text)) {
215- seen.add(text);
216- commands.push(text);
217+ const source = node.text.trim();
218+ if (source && !seen.has(source)) {
219+ seen.add(source);
220+ commands.push({ source, redirects: redirects(node) });
221 }
222 }
223 return { commands, parsed: true };
224diff --git a/src/pi/index.ts b/src/pi/index.ts
225index a1c3450e103cecfd0db861637adf7162e71f789a..f5d91cb42317e712c33c1128f132aa3ff7ef7165 100644
226--- a/src/pi/index.ts
227+++ b/src/pi/index.ts
228@@ -2,7 +2,7 @@ import type { UserMessage } from "@earendil-works/pi-ai";
229 import type { ExtensionAPI, ExtensionContext } from "@earendil-works/pi-coding-agent";
230 import { createJsonlDecisionAudit } from "../core/audit";
231 import { createJsonlDecisionCache } from "../core/cache";
232-import { checkDeterministic } from "../core/deterministic";
233+import { checkDeterministic, checkParsedBash, type BashRedirect } from "../core/deterministic";
234 import { auditInputSummary, cacheKey, normalizeRequest } from "../core/normalize";
235 import { createPolicyPipeline } from "../core/pipeline";
236 import { createPolicyReviewer, type PiReviewerCaller } from "../core/review";
237@@ -91,6 +91,7 @@ function createPiPipeline(
238 sessionBashAllowOverride: SessionBashAllowOverride | undefined,
239 cwd: string,
240 ctx: ExtensionContext,
241+ bashRedirects: WeakMap<object, readonly BashRedirect[]>,
242 ) {
243 const config = loadPiPolicyConfig(cwd);
244 const paths = piPolicyPaths();
245@@ -117,7 +118,15 @@ function createPiPipeline(
246 decide: async (request) => checkStaticPermission(request.toolName, request.input, cwd, config),
247 },
248 ],
249- deterministic: (request) => checkDeterministic(request.toolName, request.input),
250+ deterministic: (request) => {
251+ if (request.toolName !== "bash" || typeof request.input.command !== "string") {
252+ return checkDeterministic(request.toolName, request.input);
253+ }
254+ const redirects = bashRedirects.get(request.input);
255+ return redirects
256+ ? checkParsedBash(request.input.command, redirects)
257+ : checkDeterministic(request.toolName, request.input);
258+ },
259 cache: createJsonlDecisionCache(paths.cacheFile),
260 audit: createJsonlDecisionAudit(paths.auditFile),
261 reviewer: createPolicyReviewer(config.reviewer, piReviewerCaller(ctx), config.externalDirectories),
262@@ -137,7 +146,8 @@ export async function evaluatePiToolCall(
263 ctx?: ExtensionContext,
264 ): Promise<PolicyEvaluation> {
265 if (!ctx) throw new Error("Pi extension context is required");
266- const pipeline = createPiPipeline(sessionBashAllowOverride, context.cwd ?? process.cwd(), ctx);
267+ const bashRedirects = new WeakMap<object, readonly BashRedirect[]>();
268+ const pipeline = createPiPipeline(sessionBashAllowOverride, context.cwd ?? process.cwd(), ctx, bashRedirects);
269 const evaluationOptions = {
270 skipPrechecks: true,
271 skipLLMReview: skipPermissions,
272@@ -151,10 +161,11 @@ export async function evaluatePiToolCall(
273 }
274
275 const split = await splitBash(input.command);
276- const requests = split.commands.map((command) => ({
277- toolName,
278- input: { ...input, command },
279- }));
280+ const requests = split.commands.map((splitCommand) => {
281+ const commandInput = { ...input, command: splitCommand.source };
282+ bashRedirects.set(commandInput, splitCommand.redirects);
283+ return { toolName, input: commandInput };
284+ });
285 return pipeline.evaluateMany(requests, context, {
286 ...evaluationOptions,
287 skipDeterministic: !split.parsed,
288diff --git a/test/core/deterministic.test.ts b/test/core/deterministic.test.ts
289index a15f51e0ccb548840eb0d4dfa0c613139f3ddcc3..942730e0d09d1330697c5568eeebb3594af07669 100644
290--- a/test/core/deterministic.test.ts
291+++ b/test/core/deterministic.test.ts
292@@ -1,5 +1,5 @@
293 import { expect, test } from "bun:test";
294-import { checkDeterministic } from "../../src/core/deterministic";
295+import { checkDeterministic, checkParsedBash } from "../../src/core/deterministic";
296
297 test("allows sh syntax checks without restricting the source path", () => {
298 for (const command of [
299@@ -20,6 +20,9 @@ test("keeps shell execution subject to confirmation", () => {
300 test("allows recognized deterministic command grammars", () => {
301 for (const command of [
302 "command -v bun node",
303+ "cd ~/.ssh",
304+ 'grep -rn "namespace\\|Namespace" ~/go/pkg/mod/github.com/jessevdk/go-flags@v1.6.1/option.go',
305+ "echo 'a|b'",
306 "git add src/core/rules.ts",
307 "git commit -m 'Allow deterministic Git changes'",
308 "git commit -m 'sudo rm -rf /'",
309@@ -70,6 +73,22 @@ test("allows recognized deterministic command grammars", () => {
310 }
311 });
312
313+test("checks parsed Bash redirects", () => {
314+ expect(checkParsedBash("echo value", [{ path: "output.txt", writes: true, dynamic: false }])).toMatchObject({
315+ decision: "allow",
316+ });
317+ expect(checkParsedBash("cat .golangci.yml", [{ path: "/dev/null", writes: true, dynamic: false }])).toMatchObject({
318+ decision: "allow",
319+ });
320+ for (const redirect of [
321+ { path: ".env", writes: true, dynamic: false },
322+ { path: "/etc/policy-engine", writes: true, dynamic: false },
323+ { path: "", writes: true, dynamic: true },
324+ ]) {
325+ expect(checkParsedBash("echo value", [redirect])?.decision).not.toBe("allow");
326+ }
327+});
328+
329 test("allows local Docker operations except volume deletion", () => {
330 for (const command of [
331 "docker compose up -d",
332@@ -124,6 +143,24 @@ test("does not treat search terms as secret paths", () => {
333 expect(checkDeterministic("bash", { command: "rg '.env' README.md" })).toMatchObject({ decision: "allow" });
334 });
335
336+test("allows read-only find expressions", () => {
337+ for (const command of [
338+ "find .",
339+ "find src -type f -name '*.ts'",
340+ "find . -maxdepth 2 -type f -print",
341+ "find . -name '*.ts' -o -name '*.md'",
342+ 'find . -type f \\( -name "*.go" -o -name "*.md" -o -name "*.json" \\) -not -path "./.git/*"',
343+ ]) {
344+ expect(checkDeterministic("bash", { command })).toMatchObject({ decision: "allow" });
345+ }
346+
347+ for (const command of ["find . -delete", "find . -exec cat {} \\;", "find . -fprint results.txt"]) {
348+ expect(checkDeterministic("bash", { command })?.decision).not.toBe("allow");
349+ }
350+
351+ expect(checkDeterministic("bash", { command: "find ~ -name '*.ts'" })).toMatchObject({ decision: "ask" });
352+});
353+
354 test("asks before destructive operations", () => {
355 for (const command of [
356 "git reset --hard",
357@@ -148,6 +185,7 @@ test("does not deterministically allow non-toolchain mutation or shell-control v
358 "kustomize build deploy",
359 "kubectl kustomize deploy",
360 "command -v node; rm -rf /",
361+ 'echo "$(pwd)"',
362 ]) {
363 expect(checkDeterministic("bash", { command })?.decision).not.toBe("allow");
364 }