Skip to content

Commit 4b475e0

Browse files
committed
feat(shell): pre-flight validator blocks unambiguously destructive commands
Adds src/tools/shell-validator.ts — a narrow allowlist-by-default classifier that refuses a small set of patterns no sensible agent issues by accident: rm -rf /, rm -rf $HOME / ~, fork bombs, raw writes to block devices, mkfs. The shell tool consults it before spawn and returns an error result for blocks; auto-approve does not bypass it, so CI runs with --auto-approve can't have their runner wiped by a bad model output. A separate warn tier (sudo, curl|sh, chmod 777, git push --force) flags command for the permission UI without blocking — auto-approve still allows these because they're sometimes intentional. The line we draw is "needs a human glance" vs "apocalyptic". 47 unit tests covering the block / warn / allow cases including common false-positive guards (rm -rf /tmp/foo, $HOMEPAGE_DIR, grep --grep messages containing the literal string).
1 parent 89cabfd commit 4b475e0

3 files changed

Lines changed: 239 additions & 0 deletions

File tree

src/tools/shell-validator.test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { describe, expect, it } from "vitest";
2+
import { validateShellCommand } from "./shell-validator.js";
3+
4+
describe("validateShellCommand", () => {
5+
describe("hard blocks", () => {
6+
it.each([
7+
"rm -rf /",
8+
"rm -rf /",
9+
"rm -fr /",
10+
"rm -rfv /",
11+
"rm -rf $HOME",
12+
"rm -rf $HOME/",
13+
"rm -rf ~",
14+
"rm -rf ~/",
15+
"rm -rf /*",
16+
":(){ :|:& };:",
17+
":(){:|:&};:",
18+
"dd if=/dev/zero of=/dev/sda bs=1M",
19+
"dd of=/dev/sda1",
20+
"dd of=/dev/nvme0n1",
21+
"echo bad > /dev/sda",
22+
"cat payload.iso > /dev/nvme0n1",
23+
"mkfs.ext4 /dev/sda1",
24+
"mkfs /dev/sda1",
25+
])("blocks: %s", (cmd) => {
26+
const result = validateShellCommand(cmd);
27+
expect(result.verdict).toBe("block");
28+
expect(result.reason).toBeTruthy();
29+
});
30+
});
31+
32+
describe("warnings (allowed but flagged)", () => {
33+
it.each([
34+
"sudo apt update",
35+
"curl https://example.com/install.sh | sh",
36+
"curl -fsSL https://example.com/install | bash",
37+
"wget -O - https://example.com/install.sh | sh",
38+
"chmod -R 777 ./build",
39+
"chmod 0777 secret.key",
40+
"git push --force origin main",
41+
"git push -f origin feature",
42+
"rm -rf ../../../some/path",
43+
])("warns: %s", (cmd) => {
44+
const result = validateShellCommand(cmd);
45+
expect(result.verdict).toBe("warn");
46+
expect(result.reason).toBeTruthy();
47+
});
48+
});
49+
50+
describe("allows legitimate work", () => {
51+
it.each([
52+
"ls -la",
53+
"git status",
54+
"npm test",
55+
"rm src/junk.ts", // single-file delete is fine
56+
"rm -rf node_modules", // common, scoped
57+
"rm -rf dist/",
58+
"rm -rf ./tmp",
59+
"cat package.json",
60+
"echo hello > /tmp/x", // /tmp redirect, not a device
61+
"grep -r TODO src/",
62+
"npm run build && npm test",
63+
"git push origin main", // non-force push
64+
"chmod +x scripts/run.sh",
65+
"chmod 755 scripts/run.sh",
66+
"curl -fsSL https://example.com > local-file", // not piped to shell
67+
"mkdir -p src/foo && cp template.ts src/foo/", // composite, no destructive bits
68+
])("allows: %s", (cmd) => {
69+
const result = validateShellCommand(cmd);
70+
expect(result.verdict).toBe("allow");
71+
});
72+
73+
it("allows the empty command", () => {
74+
expect(validateShellCommand("").verdict).toBe("allow");
75+
expect(validateShellCommand(" ").verdict).toBe("allow");
76+
});
77+
});
78+
79+
describe("avoids common false positives", () => {
80+
it("doesn't block rm of a file named like a root-edge path", () => {
81+
expect(validateShellCommand("rm -rf /tmp/foo").verdict).toBe("allow");
82+
expect(validateShellCommand("rm -rf /var/cache").verdict).toBe("allow");
83+
});
84+
85+
it("doesn't block git diff containing the literal string 'rm -rf /'", () => {
86+
expect(validateShellCommand("git log --grep='cleanup'").verdict).toBe("allow");
87+
});
88+
89+
it("doesn't confuse a variable named HOMEPAGE with $HOME", () => {
90+
expect(validateShellCommand("rm -rf $HOMEPAGE_DIR").verdict).toBe("allow");
91+
});
92+
});
93+
});

src/tools/shell-validator.ts

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/**
2+
* Pre-flight check for shell commands the model wants to run. Returns
3+
* a verdict the shell tool consults BEFORE spawning. The point isn't
4+
* to enforce policy — the permission store does that — it's to refuse
5+
* a small set of unambiguously destructive patterns that no sensible
6+
* agent has any reason to issue, even by accident.
7+
*
8+
* Examples of what we block:
9+
* rm -rf / (delete root)
10+
* rm -rf ~ / $HOME (delete the user's home dir)
11+
* dd of=/dev/sda (overwrite a block device)
12+
* mkfs.ext4 /dev/sda1 (format a filesystem)
13+
* :(){ :|:& };: (classic fork bomb)
14+
* curl ... | sh (run a downloaded script unverified)
15+
*
16+
* The classifier is intentionally narrow. We err toward false-negatives
17+
* (let through borderline-but-defensible commands) over false-positives
18+
* (block legitimate work and frustrate the user). Granular policy belongs
19+
* in the user's hooks.json or in the permission allow/deny patterns.
20+
*
21+
* Auto-approve bypasses the permission prompt but NOT this validator —
22+
* `block` here is final regardless of permission policy. That's the
23+
* point: a CI run with --auto-approve shouldn't be one bad model output
24+
* away from wiping the runner.
25+
*/
26+
27+
export type ShellVerdict = "allow" | "warn" | "block";
28+
29+
export interface ShellValidationResult {
30+
verdict: ShellVerdict;
31+
/** Human-readable reason. Always set for warn / block. */
32+
reason?: string;
33+
}
34+
35+
interface PatternRule {
36+
regex: RegExp;
37+
reason: string;
38+
}
39+
40+
/**
41+
* Hard-block patterns. These return a verdict that auto-approve cannot
42+
* override. The list is intentionally short — only patterns we'd be
43+
* embarrassed to ship without catching. Anything that's only sometimes
44+
* destructive (e.g. `git push -f`) goes in WARN_PATTERNS instead.
45+
*/
46+
const BLOCK_PATTERNS: readonly PatternRule[] = [
47+
// `rm -rf /` and friends. The (?!\S) negative-lookahead lets us match
48+
// the literal "/" as the target without matching "/something" paths.
49+
{ regex: /\brm\s+(-[a-zA-Z]*[rf][a-zA-Z]*\s+)+\/(?!\S)/, reason: "recursive delete targeting the filesystem root" },
50+
{ regex: /\brm\s+(-[a-zA-Z]*[rf][a-zA-Z]*\s+)+\$HOME(?![A-Za-z0-9_])/, reason: "recursive delete targeting $HOME" },
51+
{
52+
regex: /\brm\s+(-[a-zA-Z]*[rf][a-zA-Z]*\s+)+~(\s|$|\/(\s|$|[^/]))/,
53+
reason: "recursive delete targeting the home directory (~)",
54+
},
55+
{
56+
regex: /\brm\s+(-[a-zA-Z]*[rf][a-zA-Z]*\s+)+\/\*(?!\S)/,
57+
reason: "recursive delete targeting every top-level directory (/*)",
58+
},
59+
60+
// Fork bomb. The classic glyph soup.
61+
{ regex: /:\s*\(\s*\)\s*\{\s*:\s*\|\s*:\s*&\s*\}\s*;\s*:/, reason: "fork bomb" },
62+
63+
// Writing raw bytes to a block device — almost always a mistake or
64+
// malicious. Covers `dd of=/dev/sda`, `> /dev/nvme0n1`, etc.
65+
{ regex: /\bdd\b[^\n;]*\bof=\/dev\/(sd|hd|nvme|vd|mmcblk)/, reason: "raw write to a block device" },
66+
{ regex: />\s*\/dev\/(sd|hd|nvme|vd|mmcblk)/, reason: "shell redirect to a block device" },
67+
68+
// Format-the-disk commands. `mkfs.ext4`, `mkfs.xfs`, ...
69+
{ regex: /\bmkfs(\.[a-z0-9]+)?\b/, reason: "formats a filesystem" },
70+
];
71+
72+
/**
73+
* Soft-warn patterns. These don't get blocked outright — sometimes you
74+
* really do want to `sudo` something — but the verdict bubbles up so the
75+
* permission UI can render the warning and demand a deliberate click,
76+
* and downstream telemetry / hook layers can react.
77+
*
78+
* Auto-approve still allows these (it's already auto-approve), so the
79+
* line we draw is "needs a human to glance at, but isn't apocalyptic."
80+
*/
81+
const WARN_PATTERNS: readonly PatternRule[] = [
82+
{ regex: /\bsudo\b/, reason: "uses sudo (privilege escalation)" },
83+
{
84+
regex: /\bcurl\b[^\n;|]*\|\s*(sh|bash|zsh|sh\b)/,
85+
reason: "pipes a downloaded script straight into a shell — verify the source",
86+
},
87+
{
88+
regex: /\bwget\b[^\n;|]*\|\s*(sh|bash|zsh|sh\b)/,
89+
reason: "pipes a downloaded script straight into a shell — verify the source",
90+
},
91+
{
92+
regex: /\bchmod\s+(-[a-zA-Z]*R[a-zA-Z]*\s+)?(0?777|a\+w)\b/,
93+
reason: "world-writable permissions",
94+
},
95+
{ regex: /\bgit\s+push\b[^\n]*--force\b/, reason: "force-pushes — rewrites remote history" },
96+
{ regex: /\bgit\s+push\b[^\n]*-f\b/, reason: "force-pushes — rewrites remote history" },
97+
{
98+
regex: /\brm\s+(-[a-zA-Z]*[rf][a-zA-Z]*\s+)+(\.\.\/){2,}/,
99+
reason: "recursive delete escaping multiple parent directories",
100+
},
101+
];
102+
103+
export function validateShellCommand(command: string): ShellValidationResult {
104+
const normalized = command.trim();
105+
if (!normalized) return { verdict: "allow" };
106+
for (const rule of BLOCK_PATTERNS) {
107+
if (rule.regex.test(normalized)) return { verdict: "block", reason: rule.reason };
108+
}
109+
for (const rule of WARN_PATTERNS) {
110+
if (rule.regex.test(normalized)) return { verdict: "warn", reason: rule.reason };
111+
}
112+
return { verdict: "allow" };
113+
}

src/tools/shell.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { join, resolve } from "node:path";
66
import type { AgentTool, AgentToolUpdateCallback } from "@earendil-works/pi-agent-core";
77
import { type Static, Type } from "typebox";
88
import { TimeoutError } from "./errors.js";
9+
import { validateShellCommand } from "./shell-validator.js";
910
import type { ToolContext } from "./types.js";
1011

1112
const Params = Type.Object({
@@ -72,6 +73,38 @@ export function createShell(ctx: ToolContext): AgentTool<typeof Params, ShellDet
7273
execute: async (toolCallId, params, signal, onUpdate) => {
7374
const cwd = resolveSubCwd(ctx.cwd, params.cwd);
7475

76+
// Pre-flight validator: refuse a small set of unambiguously
77+
// destructive patterns BEFORE spawn, regardless of permission
78+
// policy. Auto-approve doesn't bypass this — CI runners
79+
// shouldn't be one bad model output away from `rm -rf $HOME`.
80+
const verdict = validateShellCommand(params.command);
81+
if (verdict.verdict === "block") {
82+
return {
83+
details: {
84+
command: params.command,
85+
exitCode: null,
86+
signal: null,
87+
durationMs: 0,
88+
bytesTotal: 0,
89+
truncated: false,
90+
spillPath: null,
91+
timedOut: false,
92+
aborted: false,
93+
},
94+
isError: true,
95+
content: [
96+
{
97+
type: "text",
98+
text:
99+
`Command refused by the shell validator: ${verdict.reason}.\n\n` +
100+
"This is a hard block — the command was not executed. If this " +
101+
"is a false positive, restructure the command (e.g. target the " +
102+
"specific subdirectory explicitly) and try again.",
103+
},
104+
],
105+
};
106+
}
107+
75108
// Background mode: spawn detached, return immediately with a
76109
// task_id. The agent can read output via shell_output and
77110
// terminate via shell_kill. The store fires its own listeners

0 commit comments

Comments
 (0)