Commit `7ac0710647c9`

Vincent Demeester <vincent@sbr.pm>

2026-03-17 16:27:40

feat(guardrails): add 3-way approval for commands

Replaced binary yes/no with Accept/Modify/Reject for bash command guardrails and soft-protected path writes. Reject tells the LLM not to retry. Modify tells it to ask the user what to change. Extracted rules and helpers into utils.ts for testability and added 34 tests.

main

1 parent 9f9c8ea

Changed files (3)

dots

agent

extensions

guardrails

guardrails.test.ts

index.ts

utils.ts

@@ -0,0 +1,250 @@
+/**
+ * Tests for Guardrails extension
+ *
+ * Run with: bun test guardrails.test.ts
+ */
+
+import { describe, expect, test } from "bun:test";
+import {
+	stripQuotedContent,
+	matchCommandRule,
+	commandRules,
+	dangerousBashWrites,
+	protectedPaths,
+	softProtectedPaths,
+	buildModifyReason,
+	buildRejectReason,
+	buildWriteModifyReason,
+	buildWriteRejectReason,
+} from "./utils";
+
+// ============================================================================
+// stripQuotedContent
+// ============================================================================
+
+describe("stripQuotedContent", () => {
+	test("strips double-quoted strings", () => {
+		expect(stripQuotedContent('git commit -m "rm -rf everything"')).toBe('git commit -m ""');
+	});
+
+	test("strips single-quoted strings", () => {
+		expect(stripQuotedContent("echo 'sudo rm -rf /'")).toBe("echo ''");
+	});
+
+	test("handles escaped quotes in double-quoted strings", () => {
+		expect(stripQuotedContent('echo "say \\"hello\\""')).toBe('echo ""');
+	});
+
+	test("preserves unquoted content", () => {
+		expect(stripQuotedContent("rm -rf /tmp/test")).toBe("rm -rf /tmp/test");
+	});
+
+	test("handles mixed quotes", () => {
+		const result = stripQuotedContent(`kubectl apply -f "config.yaml" --namespace 'prod'`);
+		expect(result).toBe(`kubectl apply -f "" --namespace ''`);
+	});
+});
+
+// ============================================================================
+// Command Rule Matching
+// ============================================================================
+
+describe("matchCommandRule", () => {
+	test("matches recursive rm", () => {
+		const match = matchCommandRule("rm -rf /tmp/test");
+		expect(match).not.toBeNull();
+		expect(match!.desc).toBe("recursive delete");
+		expect(match!.action).toBe("confirm");
+	});
+
+	test("matches sudo", () => {
+		const match = matchCommandRule("sudo apt update");
+		expect(match).not.toBeNull();
+		expect(match!.desc).toBe("sudo command");
+	});
+
+	test("matches kubectl", () => {
+		const match = matchCommandRule("kubectl get pods");
+		expect(match).not.toBeNull();
+		expect(match!.desc).toBe("kubectl command");
+		expect(match!.action).toBe("confirm");
+	});
+
+	test("blocks nixos-rebuild switch", () => {
+		const match = matchCommandRule("nixos-rebuild switch");
+		expect(match).not.toBeNull();
+		expect(match!.action).toBe("block");
+		expect(match!.suggestion).toContain("make");
+	});
+
+	test("blocks home-manager switch", () => {
+		const match = matchCommandRule("home-manager switch");
+		expect(match).not.toBeNull();
+		expect(match!.action).toBe("block");
+	});
+
+	test("matches gh pr", () => {
+		const match = matchCommandRule("gh pr list");
+		expect(match).not.toBeNull();
+		expect(match!.desc).toContain("gh CLI");
+		expect(match!.suggestion).toContain("github");
+	});
+
+	test("matches nix eval", () => {
+		const match = matchCommandRule("nix eval .#foo");
+		expect(match).not.toBeNull();
+		expect(match!.desc).toContain("nix eval");
+	});
+
+	test("matches nix flake update", () => {
+		const match = matchCommandRule("nix flake update");
+		expect(match).not.toBeNull();
+		expect(match!.desc).toContain("nix flake update");
+	});
+
+	test("matches nix run", () => {
+		const match = matchCommandRule("nix run nixpkgs#hello");
+		expect(match).not.toBeNull();
+		expect(match!.desc).toContain("nix run");
+	});
+
+	test("does not match safe commands", () => {
+		expect(matchCommandRule("ls -la")).toBeNull();
+		expect(matchCommandRule("cat README.md")).toBeNull();
+		expect(matchCommandRule("echo hello")).toBeNull();
+		expect(matchCommandRule("git status")).toBeNull();
+	});
+
+	test("does not match quoted content", () => {
+		// stripQuotedContent should be applied before matching
+		const stripped = stripQuotedContent('git commit -m "sudo rm -rf"');
+		expect(matchCommandRule(stripped)).toBeNull();
+	});
+});
+
+// ============================================================================
+// Dangerous Bash Writes
+// ============================================================================
+
+describe("dangerousBashWrites", () => {
+	test("detects redirect to .env", () => {
+		const matches = dangerousBashWrites.some((p) => p.test("> .env"));
+		expect(matches).toBe(true);
+	});
+
+	test("detects tee to .env", () => {
+		const matches = dangerousBashWrites.some((p) => p.test("tee .env"));
+		expect(matches).toBe(true);
+	});
+
+	test("detects redirect to .pem", () => {
+		const matches = dangerousBashWrites.some((p) => p.test("> cert.pem"));
+		expect(matches).toBe(true);
+	});
+
+	test("does not match safe writes", () => {
+		const matches = dangerousBashWrites.some((p) => p.test("> output.txt"));
+		expect(matches).toBe(false);
+	});
+});
+
+// ============================================================================
+// Protected Paths
+// ============================================================================
+
+describe("protectedPaths", () => {
+	test("matches .env files", () => {
+		const matches = protectedPaths.some((p) => p.pattern.test(".env"));
+		expect(matches).toBe(true);
+	});
+
+	test("matches .env.local but not .env.example", () => {
+		const matchesLocal = protectedPaths.some((p) => p.pattern.test(".env.local"));
+		expect(matchesLocal).toBe(true);
+		const matchesExample = protectedPaths.some((p) => p.pattern.test(".env.example"));
+		expect(matchesExample).toBe(false);
+	});
+
+	test("matches node_modules", () => {
+		const matches = protectedPaths.some((p) => p.pattern.test("node_modules/foo/bar.js"));
+		expect(matches).toBe(true);
+	});
+
+	test("matches .git directory", () => {
+		const matches = protectedPaths.some((p) => p.pattern.test(".git/config"));
+		expect(matches).toBe(true);
+	});
+
+	test("matches SSH keys", () => {
+		const matches = protectedPaths.some((p) => p.pattern.test("id_ed25519"));
+		expect(matches).toBe(true);
+	});
+
+	test("does not match normal files", () => {
+		const matches = protectedPaths.some((p) => p.pattern.test("src/main.ts"));
+		expect(matches).toBe(false);
+	});
+});
+
+describe("softProtectedPaths", () => {
+	test("matches package-lock.json", () => {
+		const matches = softProtectedPaths.some((p) => p.pattern.test("package-lock.json"));
+		expect(matches).toBe(true);
+	});
+
+	test("matches yarn.lock", () => {
+		const matches = softProtectedPaths.some((p) => p.pattern.test("yarn.lock"));
+		expect(matches).toBe(true);
+	});
+
+	test("matches pnpm-lock.yaml", () => {
+		const matches = softProtectedPaths.some((p) => p.pattern.test("pnpm-lock.yaml"));
+		expect(matches).toBe(true);
+	});
+
+	test("does not match package.json", () => {
+		const matches = softProtectedPaths.some((p) => p.pattern.test("package.json"));
+		expect(matches).toBe(false);
+	});
+});
+
+// ============================================================================
+// Approval Gate Integration
+// ============================================================================
+
+describe("buildModifyReason", () => {
+	test("tells LLM to ask user for changes", () => {
+		const reason = buildModifyReason("kubectl command");
+		expect(reason).toContain("modify");
+		expect(reason).toContain("Ask the user");
+		expect(reason).toContain("retry");
+		expect(reason).toContain("kubectl command");
+	});
+});
+
+describe("buildRejectReason", () => {
+	test("tells LLM not to retry", () => {
+		const reason = buildRejectReason("kubectl command");
+		expect(reason).toContain("rejected");
+		expect(reason).toContain("Do NOT retry");
+		expect(reason).toContain("kubectl command");
+	});
+});
+
+describe("buildWriteModifyReason", () => {
+	test("tells LLM to ask user for changes with file context", () => {
+		const reason = buildWriteModifyReason("package-lock.json", "/project/package-lock.json");
+		expect(reason).toContain("modify");
+		expect(reason).toContain("Ask the user");
+		expect(reason).toContain("package-lock.json");
+	});
+});
+
+describe("buildWriteRejectReason", () => {
+	test("tells LLM not to retry with file context", () => {
+		const reason = buildWriteRejectReason("yarn.lock", "/project/yarn.lock");
+		expect(reason).toContain("rejected");
+		expect(reason).toContain("Do NOT retry");
+		expect(reason).toContain("yarn.lock");
+	});
+});

@@ -19,6 +19,17 @@ import { existsSync } from "node:fs";
 import { readFile, writeFile } from "node:fs/promises";
 import { join, normalize } from "node:path";
 import { homedir } from "node:os";
+import {
+	stripQuotedContent,
+	commandRules,
+	dangerousBashWrites,
+	protectedPaths,
+	softProtectedPaths,
+	buildModifyReason,
+	buildRejectReason,
+	buildWriteModifyReason,
+	buildWriteRejectReason,
+} from "./utils";
 
 // ── Policy types ──────────────────────────────────────────────
 
@@ -41,93 +52,8 @@ interface PolicyConfig {
 	policies: PathPolicy[];
 }
 
-// ── Command rule types ────────────────────────────────────────
-
-interface CommandRule {
-	pattern: RegExp;
-	desc: string;
-	action: "confirm" | "block";
-	suggestion?: string;
-}
-
-// ── Dangerous & guarded command patterns ──────────────────────
-
-const commandRules: CommandRule[] = [
-	// General dangerous commands (confirm)
-	{ pattern: /\brm\s+(-[^\s]*r|--recursive)/, desc: "recursive delete", action: "confirm" },
-	{ pattern: /\bsudo\b/, desc: "sudo command", action: "confirm" },
-	{ pattern: /\b(chmod|chown)\b.*777/, desc: "dangerous permissions", action: "confirm" },
-	{ pattern: /\bmkfs\b/, desc: "filesystem format", action: "confirm" },
-	{ pattern: /\bdd\b.*\bof=\/dev\//, desc: "raw device write", action: "confirm" },
-	{ pattern: />\s*\/dev\/sd[a-z]/, desc: "raw device overwrite", action: "confirm" },
-	{ pattern: /\bkill\s+-9\s+-1\b/, desc: "kill all processes", action: "confirm" },
-	{ pattern: /:\(\)\s*\{\s*:\s*\|\s*:\s*&\s*\}\s*;/, desc: "fork bomb", action: "confirm" },
-	{ pattern: /\bkubectl\b/, desc: "kubectl command", action: "confirm" },
-
-	// Nix commands (block: must use make targets)
-	{ pattern: /\bnixos-rebuild\s+(switch|boot|test)/, desc: "direct nixos-rebuild", action: "block", suggestion: "Use 'make switch', 'make boot', or 'make host/<hostname>/switch' instead" },
-	{ pattern: /\bhome-manager\s+switch\b/, desc: "direct home-manager switch", action: "block", suggestion: "Use 'make switch' or appropriate make target instead" },
-
-	// gh CLI (confirm: use the github tool instead)
-	{ pattern: /\bgh\s+(pr|issue|run|release|repo)\b/, desc: "gh CLI (use the `github` tool instead for structured output and approval gates)", action: "confirm", suggestion: "Use the `github` tool with the appropriate resource/action instead of direct gh CLI" },
-
-	// Nix commands (confirm)
-	{ pattern: /\bnix\s+eval\b/, desc: "nix eval (arbitrary code execution)", action: "confirm" },
-	{ pattern: /\bnix-build\b/, desc: "nix-build (builds derivations)", action: "confirm" },
-	{ pattern: /\bnix\s+store\s+(delete|gc|optimise)\b/, desc: "nix store modification", action: "confirm" },
-	{ pattern: /\bnix-collect-garbage\b/, desc: "nix garbage collection", action: "confirm" },
-	{ pattern: /\bnix-channel\s+(--add|--remove|--update)\b/, desc: "nix-channel modification", action: "confirm" },
-	{ pattern: /\bnix\s+flake\s+update\b/, desc: "nix flake update", action: "confirm" },
-	{ pattern: /\bnix\s+flake\s+lock\b/, desc: "nix flake lock", action: "confirm" },
-	{ pattern: /\bnix\s+(copy|push)\b/, desc: "nix store transfer", action: "confirm" },
-	{ pattern: /\bnix\s+(develop|shell)\b/, desc: "nix development shell", action: "confirm" },
-	{ pattern: /\bnix\s+run\b/, desc: "nix run (execute package)", action: "confirm" },
-	{ pattern: /\bnix-env\s+-[ie]/, desc: "nix-env package installation", action: "confirm", suggestion: "Consider using home-manager or system configuration instead" },
-	{ pattern: /\bnix\s+profile\s+(install|remove|upgrade)\b/, desc: "nix profile modification", action: "confirm", suggestion: "Consider using home-manager or system configuration instead" },
-];
-
-// ── Protected path patterns (hard block via bash) ─────────────
-
-const dangerousBashWrites = [
-	/>\s*\.env/,
-	/>\s*\.dev\.vars/,
-	/>\s*.*\.pem/,
-	/>\s*.*\.key/,
-	/tee\s+.*\.env/,
-	/tee\s+.*\.dev\.vars/,
-	/cp\s+.*\s+\.env/,
-	/mv\s+.*\s+\.env/,
-];
-
-// ── Protected paths for write/edit tools ──────────────────────
-
-const protectedPaths = [
-	{ pattern: /\.env($|\.(?!example))/, desc: "environment file" },
-	{ pattern: /\.dev\.vars($|\.[^/]+$)/, desc: "dev vars file" },
-	{ pattern: /node_modules\//, desc: "node_modules" },
-	{ pattern: /^\.git\/|\/\.git\//, desc: "git directory" },
-	{ pattern: /\.pem$|\.key$/, desc: "private key file" },
-	{ pattern: /id_rsa|id_ed25519|id_ecdsa/, desc: "SSH key" },
-	{ pattern: /\.ssh\//, desc: ".ssh directory" },
-	{ pattern: /secrets?\.(json|ya?ml|toml)$/i, desc: "secrets file" },
-	{ pattern: /credentials/i, desc: "credentials file" },
-];
-
-const softProtectedPaths = [
-	{ pattern: /package-lock\.json$/, desc: "package-lock.json" },
-	{ pattern: /yarn\.lock$/, desc: "yarn.lock" },
-	{ pattern: /pnpm-lock\.yaml$/, desc: "pnpm-lock.yaml" },
-];
-
 // ── Helpers ───────────────────────────────────────────────────
 
-/** Strip content inside quotes to avoid false positives on e.g. commit messages */
-function stripQuotedContent(command: string): string {
-	return command
-		.replace(/"(?:[^"\\]|\\.)*"/g, '""')
-		.replace(/'[^']*'/g, "''");
-}
-
 function expandHome(path: string): string {
 	if (path.startsWith("~/")) {
 		return join(homedir(), path.slice(2));
@@ -381,10 +307,13 @@ export default function (pi: ExtensionAPI) {
 
 					const choice = await ctx.ui.select(
 						prompt,
-						["Yes", "Yes, for this turn", "Yes, for this session", "No"],
+						["Yes", "Yes, for this turn", "Yes, for this session", "✎ Modify", "✗ Reject"],
 					);
-					if (!choice || choice === "No") {
-						return { block: true, reason: `Blocked ${rule.desc} by user` };
+					if (choice === "✎ Modify") {
+						return { block: true, reason: buildModifyReason(rule.desc) };
+					}
+					if (!choice || choice === "✗ Reject") {
+						return { block: true, reason: buildRejectReason(rule.desc) };
 					}
 					if (choice === "Yes, for this turn") {
 						approvedCommands.push({ desc: rule.desc, scope: "turn" });
@@ -432,12 +361,15 @@ export default function (pi: ExtensionAPI) {
 				if (!ctx.hasUI) {
 					return { block: true, reason: `Protected path (no UI): ${desc}` };
 				}
-				const ok = await ctx.ui.confirm(
-					`⚠️ Modifying ${desc}`,
-					`Are you sure you want to modify ${filePath}?`,
+				const choice = await ctx.ui.select(
+					`⚠️ Modifying ${desc}\n\nAre you sure you want to modify ${filePath}?`,
+					["✓ Accept", "✎ Modify", "✗ Reject"],
 				);
-				if (!ok) {
-					return { block: true, reason: `User blocked write to ${desc}` };
+				if (choice === "✎ Modify") {
+					return { block: true, reason: buildWriteModifyReason(desc, filePath) };
+				}
+				if (!choice || choice === "✗ Reject") {
+					return { block: true, reason: buildWriteRejectReason(desc, filePath) };
 				}
 				break;
 			}

@@ -0,0 +1,140 @@
+/**
+ * Utility functions and rule definitions for Guardrails extension
+ *
+ * Extracted for testability.
+ */
+
+// ── Command rule types ────────────────────────────────────────
+
+export interface CommandRule {
+	pattern: RegExp;
+	desc: string;
+	action: "confirm" | "block";
+	suggestion?: string;
+}
+
+// ── Dangerous & guarded command patterns ──────────────────────
+
+export const commandRules: CommandRule[] = [
+	// General dangerous commands (confirm)
+	{ pattern: /\brm\s+(-[^\s]*r|--recursive)/, desc: "recursive delete", action: "confirm" },
+	{ pattern: /\bsudo\b/, desc: "sudo command", action: "confirm" },
+	{ pattern: /\b(chmod|chown)\b.*777/, desc: "dangerous permissions", action: "confirm" },
+	{ pattern: /\bmkfs\b/, desc: "filesystem format", action: "confirm" },
+	{ pattern: /\bdd\b.*\bof=\/dev\//, desc: "raw device write", action: "confirm" },
+	{ pattern: />\s*\/dev\/sd[a-z]/, desc: "raw device overwrite", action: "confirm" },
+	{ pattern: /\bkill\s+-9\s+-1\b/, desc: "kill all processes", action: "confirm" },
+	{ pattern: /:\(\)\s*\{\s*:\s*\|\s*:\s*&\s*\}\s*;/, desc: "fork bomb", action: "confirm" },
+	{ pattern: /\bkubectl\b/, desc: "kubectl command", action: "confirm" },
+
+	// Nix commands (block: must use make targets)
+	{ pattern: /\bnixos-rebuild\s+(switch|boot|test)/, desc: "direct nixos-rebuild", action: "block", suggestion: "Use 'make switch', 'make boot', or 'make host/<hostname>/switch' instead" },
+	{ pattern: /\bhome-manager\s+switch\b/, desc: "direct home-manager switch", action: "block", suggestion: "Use 'make switch' or appropriate make target instead" },
+
+	// gh CLI (confirm: use the github tool instead)
+	{ pattern: /\bgh\s+(pr|issue|run|release|repo)\b/, desc: "gh CLI (use the `github` tool instead for structured output and approval gates)", action: "confirm", suggestion: "Use the `github` tool with the appropriate resource/action instead of direct gh CLI" },
+
+	// Nix commands (confirm)
+	{ pattern: /\bnix\s+eval\b/, desc: "nix eval (arbitrary code execution)", action: "confirm" },
+	{ pattern: /\bnix-build\b/, desc: "nix-build (builds derivations)", action: "confirm" },
+	{ pattern: /\bnix\s+store\s+(delete|gc|optimise)\b/, desc: "nix store modification", action: "confirm" },
+	{ pattern: /\bnix-collect-garbage\b/, desc: "nix garbage collection", action: "confirm" },
+	{ pattern: /\bnix-channel\s+(--add|--remove|--update)\b/, desc: "nix-channel modification", action: "confirm" },
+	{ pattern: /\bnix\s+flake\s+update\b/, desc: "nix flake update", action: "confirm" },
+	{ pattern: /\bnix\s+flake\s+lock\b/, desc: "nix flake lock", action: "confirm" },
+	{ pattern: /\bnix\s+(copy|push)\b/, desc: "nix store transfer", action: "confirm" },
+	{ pattern: /\bnix\s+(develop|shell)\b/, desc: "nix development shell", action: "confirm" },
+	{ pattern: /\bnix\s+run\b/, desc: "nix run (execute package)", action: "confirm" },
+	{ pattern: /\bnix-env\s+-[ie]/, desc: "nix-env package installation", action: "confirm", suggestion: "Consider using home-manager or system configuration instead" },
+	{ pattern: /\bnix\s+profile\s+(install|remove|upgrade)\b/, desc: "nix profile modification", action: "confirm", suggestion: "Consider using home-manager or system configuration instead" },
+];
+
+// ── Protected path patterns (hard block via bash) ─────────────
+
+export const dangerousBashWrites = [
+	/>\s*\.env/,
+	/>\s*\.dev\.vars/,
+	/>\s*.*\.pem/,
+	/>\s*.*\.key/,
+	/tee\s+.*\.env/,
+	/tee\s+.*\.dev\.vars/,
+	/cp\s+.*\s+\.env/,
+	/mv\s+.*\s+\.env/,
+];
+
+// ── Protected paths for write/edit tools ──────────────────────
+
+export const protectedPaths = [
+	{ pattern: /\.env($|\.(?!example))/, desc: "environment file" },
+	{ pattern: /\.dev\.vars($|\.[^/]+$)/, desc: "dev vars file" },
+	{ pattern: /node_modules\//, desc: "node_modules" },
+	{ pattern: /^\.git\/|\/\.git\//, desc: "git directory" },
+	{ pattern: /\.pem$|\.key$/, desc: "private key file" },
+	{ pattern: /id_rsa|id_ed25519|id_ecdsa/, desc: "SSH key" },
+	{ pattern: /\.ssh\//, desc: ".ssh directory" },
+	{ pattern: /secrets?\.(json|ya?ml|toml)$/i, desc: "secrets file" },
+	{ pattern: /credentials/i, desc: "credentials file" },
+];
+
+export const softProtectedPaths = [
+	{ pattern: /package-lock\.json$/, desc: "package-lock.json" },
+	{ pattern: /yarn\.lock$/, desc: "yarn.lock" },
+	{ pattern: /pnpm-lock\.yaml$/, desc: "pnpm-lock.yaml" },
+];
+
+// ── Helpers ───────────────────────────────────────────────────
+
+/** Strip content inside quotes to avoid false positives on e.g. commit messages */
+export function stripQuotedContent(command: string): string {
+	return command
+		.replace(/"(?:[^"\\]|\\.)*"/g, '""')
+		.replace(/'[^']*'/g, "''");
+}
+
+/**
+ * Match a command string against the command rules.
+ * Returns the first matching rule or null.
+ * Apply stripQuotedContent before calling this for best results.
+ */
+export function matchCommandRule(command: string): CommandRule | null {
+	for (const rule of commandRules) {
+		if (rule.pattern.test(command)) {
+			return rule;
+		}
+	}
+	return null;
+}
+
+// ── Approval gate reason builders ─────────────────────────────
+
+/**
+ * Build the block reason when user wants to modify the command.
+ * Tells the LLM to ask the user what to change and retry.
+ */
+export function buildModifyReason(desc: string): string {
+	return `User wants to modify this command (${desc}). Ask the user what they would like to change, then retry with the updated command.`;
+}
+
+/**
+ * Build the block reason when user rejects the command.
+ * Tells the LLM NOT to retry.
+ */
+export function buildRejectReason(desc: string): string {
+	return `User rejected this command (${desc}). Do NOT retry or attempt this command again.`;
+}
+
+/**
+ * Build the block reason when user rejects a file write.
+ * Tells the LLM NOT to retry.
+ */
+export function buildWriteModifyReason(desc: string, filePath: string): string {
+	return `User wants to modify this write to ${desc} (${filePath}). Ask the user what they would like to change, then retry with the updated parameters.`;
+}
+
+/**
+ * Build the block reason when user rejects a file write.
+ * Tells the LLM NOT to retry.
+ */
+export function buildWriteRejectReason(desc: string, filePath: string): string {
+	return `User rejected write to ${desc} (${filePath}). Do NOT retry or attempt this write again.`;
+}

Commit 7ac0710647c9

Commit `7ac0710647c9`