feat(coding-agent): support per-command bash approval rules

2026-07-14 14:45:09 +07:00
parent 38efea12ed
commit 6d7457663f
7 changed files with 262 additions and 14 deletions
@@ -147,6 +147,28 @@ tools:
    read: allow
 ```

+### Bash command approval patterns
+
+`tools.approval` sets default policy by tool name. For bash, you can add ordered command rules with `bash.patterns`; the first matching rule wins. Patterns support literal text plus `*` as a wildcard.
+
+```yaml
+tools:
+  approvalMode: write
+  approval:
+    bash: allow
+
+bash:
+  patterns:
+    - match: "git *"
+      approval: allow
+    - match: "rm -rf *"
+      approval: deny
+    - match: "*"
+      approval: allow
+```
+
+Valid rule approvals are `allow`, `prompt`, and `deny`. Critical bash commands still require confirmation unless a matching rule explicitly denies them; broad allow rules such as `match: "*"` do not bypass the critical-command guard.
+
 ### Worked example: global vs. project

 ```yaml
@@ -610,11 +610,14 @@ export type ToolLoadMode = "essential" | "discoverable";
 * - bare tier ("read" / "write" / "exec") — static classification.
 * - object form — adds a `reason` (shown in the prompt) and/or `override: true`
 *   (force-prompt even in modes that would otherwise auto-approve this tier).
+ *   `policy: "deny"` blocks the call at the approval gate.
 * - function — dynamic, given parsed args. Returns either form above.
 *
 * Omitted approvals are treated as "exec" by callers that enforce approvals.
 */
-export type ToolApprovalDecision = ToolTier | { tier: ToolTier; reason?: string; override?: boolean };
+export type ToolApprovalDecision =
+	| ToolTier
+	| { tier: ToolTier; reason?: string; override?: boolean; policy?: "allow" | "deny" | "prompt" };
 export type ToolApproval = ToolApprovalDecision | ((args: unknown) => ToolApprovalDecision);

 /**
@@ -600,6 +600,7 @@
 ### Changed

 - Enhanced Anthropic credential and usage management to support organization-scoped accounts, including displaying organization names in /usage, /logout, omp token --list, and OAuth login success messages, resolving active-account matching for shared organizations, and deduplicating identities during migration.
+- Added ordered `bash.patterns` command approval rules so selected bash commands can be allowed, prompted, or denied by command pattern.

 ## [16.5.0] - 2026-07-13

@@ -3287,6 +3287,17 @@ export const SETTINGS_SCHEMA = {
 			description: "Automatically background long-running bash commands and deliver the result later",
 		},
 	},
+	"bash.patterns": {
+		type: "array",
+		default: [],
+		ui: {
+			tab: "shell",
+			group: "Bash",
+			label: "Bash Approval Patterns",
+			description:
+				"Ordered bash command approval rules. Each item has match and approval fields; only '*' wildcards are supported.",
+		},
+	},

 	// Bash interceptor
 	"bashInterceptor.enabled": {
@@ -20,6 +20,7 @@ export interface ResolvedApproval {
 	tier: ToolTier;
 	reason?: string;
 	override: boolean;
+	source?: "tool" | "user" | "mode";
 }

 const POLICY_VALUES: ReadonlySet<ApprovalPolicy> = new Set(["allow", "deny", "prompt"]);
@@ -50,7 +51,7 @@ function isToolTier(value: unknown): value is ToolTier {
 	return typeof value === "string" && TIER_VALUES.has(value as ToolTier);
 }

-function normalizeDecision(value: unknown): Omit<ResolvedApproval, "policy"> {
+function normalizeDecision(value: unknown): Omit<ResolvedApproval, "policy"> & { policy?: ApprovalPolicy } {
 	if (isToolTier(value)) {
 		return { tier: value, override: false };
 	}
@@ -59,9 +60,11 @@ function normalizeDecision(value: unknown): Omit<ResolvedApproval, "policy"> {
 		const record = value as Record<string, unknown>;
 		const tier = isToolTier(record.tier) ? record.tier : "exec";
 		const reason = typeof record.reason === "string" && record.reason.length > 0 ? record.reason : undefined;
+		const policy = normalizePolicy(record.policy);
 		return {
 			tier,
 			override: record.override === true,
+			...(policy ? { policy } : {}),
 			...(reason ? { reason } : {}),
 		};
 	}
@@ -69,7 +72,10 @@ function normalizeDecision(value: unknown): Omit<ResolvedApproval, "policy"> {
 	return { tier: "exec", override: false };
 }

-function getToolDecision(tool: ApprovalSubject, args: unknown): Omit<ResolvedApproval, "policy"> {
+function getToolDecision(
+	tool: ApprovalSubject,
+	args: unknown,
+): Omit<ResolvedApproval, "policy"> & { policy?: ApprovalPolicy } {
 	const approval = tool.approval;
 	const decision: ToolApprovalDecision | undefined = typeof approval === "function" ? approval(args) : approval;
 	return normalizeDecision(decision);
@@ -110,34 +116,61 @@ export function resolveApproval(
 	const decision = getToolDecision(tool, args);
 	const userPolicy = Object.hasOwn(userConfig, tool.name) ? normalizePolicy(userConfig[tool.name]) : undefined;

+	if (decision.policy === "deny") {
+		return {
+			policy: "deny",
+			tier: decision.tier,
+			override: decision.override,
+			source: "tool",
+			...(decision.reason ? { reason: decision.reason } : {}),
+		};
+	}
+	if (userPolicy === "deny") {
+		return { policy: "deny", tier: decision.tier, override: decision.override, source: "user" };
+	}
+
 	if (mode === "yolo") {
-		return { policy: userPolicy ?? "allow", tier: decision.tier, override: false };
+		return {
+			policy: userPolicy ?? "allow",
+			tier: decision.tier,
+			override: false,
+			source: userPolicy ? "user" : "mode",
+		};
 	}

 	if (decision.override) {
-		if (userPolicy === "deny") {
-			return { policy: "deny", tier: decision.tier, override: true };
-		}
 		return {
-			policy: "prompt",
+			policy: decision.policy === "allow" ? "allow" : "prompt",
 			tier: decision.tier,
 			override: true,
+			source: "tool",
+			...(decision.reason ? { reason: decision.reason } : {}),
+		};
+	}
+
+	if (decision.policy === "allow" || decision.policy === "prompt") {
+		return {
+			policy: decision.policy,
+			tier: decision.tier,
+			override: false,
+			source: "tool",
 			...(decision.reason ? { reason: decision.reason } : {}),
 		};
 	}

 	if (userPolicy) {
-		return { policy: userPolicy, tier: decision.tier, override: false };
+		return { policy: userPolicy, tier: decision.tier, override: false, source: "user" };
 	}

 	if (modeApprovesTier(mode, decision.tier)) {
-		return { policy: "allow", tier: decision.tier, override: false };
+		return { policy: "allow", tier: decision.tier, override: false, source: "mode" };
 	}

 	return {
 		policy: "prompt",
 		tier: decision.tier,
 		override: false,
+		source: "mode",
 		...(decision.reason ? { reason: decision.reason } : {}),
 	};
 }
@@ -154,9 +187,12 @@ export function requiresApproval(
 	mode: ApprovalMode,
 	userConfig: Record<string, unknown> = {},
 ): { required: boolean; reason?: string } {
-	const { policy, reason } = resolveApproval(tool, args, mode, userConfig);
+	const { policy, reason, source } = resolveApproval(tool, args, mode, userConfig);

 	if (policy === "deny") {
+		if (source === "tool") {
+			throw new Error(`Tool "${tool.name}" is blocked by tool policy.${reason ? `\nReason: ${reason}` : ""}`);
+		}
 		throw new Error(
 			`Tool "${tool.name}" is blocked by user policy.\n` +
 				`To allow: remove "tools.approval.${tool.name}: deny" from config.`,
@@ -51,6 +51,8 @@ export const BASH_DEFAULT_PREVIEW_LINES = DEFAULT_TERMINAL_PREVIEW_LINES;

 const BASH_ENV_NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/;
 const DEFAULT_AUTO_BACKGROUND_THRESHOLD_MS = 60_000;
+const BASH_APPROVAL_SHELL_CONTROL_RE = /[\n\r;&|<>`$()]/u;
+const BASH_PATTERN_APPROVAL_VALUES = new Set(["allow", "deny", "prompt"]);

 /**
 * Shape a shell command line for an ACP-conformant `terminal/create` request.
@@ -127,6 +129,63 @@ export const CRITICAL_BASH_PATTERNS = [
 	/\bnc\b[^|;]*\s-[a-zA-Z]*[ec][a-zA-Z]*\s/i, // `nc -e` / `nc -c`.
 ] as const;

+type BashPatternApproval = "allow" | "deny" | "prompt";
+
+interface BashApprovalPatternRule {
+	match: string;
+	approval: BashPatternApproval;
+}
+
+function normalizeBashApprovalPattern(value: string): string {
+	return value.trim().replace(/\s+/gu, " ");
+}
+
+function bashApprovalPatternToRegExp(pattern: string): RegExp {
+	const escaped = normalizeBashApprovalPattern(pattern)
+		.split("*")
+		.map(part => part.replace(/[\\^$+?.()|[\]{}]/gu, "\\$&"))
+		.join(".*");
+	return new RegExp(`^${escaped}$`, "u");
+}
+
+function normalizeBashPatternApproval(value: unknown): BashPatternApproval | undefined {
+	if (typeof value !== "string") return undefined;
+	const normalized = value.trim().toLowerCase();
+	return BASH_PATTERN_APPROVAL_VALUES.has(normalized) ? (normalized as BashPatternApproval) : undefined;
+}
+
+function getBashApprovalPatternRules(value: unknown): BashApprovalPatternRule[] {
+	if (!Array.isArray(value)) return [];
+	return value
+		.map(item => {
+			if (!item || typeof item !== "object" || Array.isArray(item)) return undefined;
+			const record = item as Record<string, unknown>;
+			if (typeof record.match !== "string") return undefined;
+			const match = normalizeBashApprovalPattern(record.match);
+			const approval = normalizeBashPatternApproval(record.approval);
+			return match.length > 0 && approval ? { match, approval } : undefined;
+		})
+		.filter((rule): rule is BashApprovalPatternRule => !!rule);
+}
+
+function commandMatchesBashApprovalPattern(command: string, pattern: string): boolean {
+	const normalizedCommand = normalizeBashApprovalPattern(command);
+	if (normalizedCommand.length === 0) return false;
+	return bashApprovalPatternToRegExp(pattern).test(normalizedCommand);
+}
+
+function findBashApprovalPatternRule(
+	command: string,
+	rules: readonly BashApprovalPatternRule[],
+): BashApprovalPatternRule | undefined {
+	return rules.find(rule => {
+		if (rule.approval === "allow" && BASH_APPROVAL_SHELL_CONTROL_RE.test(normalizeBashApprovalPattern(command))) {
+			return false;
+		}
+		return commandMatchesBashApprovalPattern(command, rule.match);
+	});
+}
+
 async function saveBashOriginalArtifact(session: ToolSession, originalText: string): Promise<string | undefined> {
 	try {
 		const alloc = await session.allocateOutputArtifact?.("bash-original");
@@ -384,9 +443,29 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
 	readonly approval = (args: unknown): ToolApprovalDecision => {
 		const rawCommand = (args as Partial<BashToolInput>).command;
 		const command = typeof rawCommand === "string" ? rawCommand : "";
+		const patternRules = getBashApprovalPatternRules(this.session.settings.get("bash.patterns"));
+		const patternRule = patternRules.find(rule => commandMatchesBashApprovalPattern(command, rule.match));
+		if (patternRule?.approval === "deny") {
+			return {
+				tier: "exec",
+				override: true,
+				policy: "deny",
+				reason: `Blocked by bash pattern: ${patternRule.match}`,
+			};
+		}
 		if (command !== "" && CRITICAL_BASH_PATTERNS.some(pattern => pattern.test(command))) {
 			return { tier: "exec", override: true, reason: "Critical pattern detected" };
 		}
+		const safePatternRule = findBashApprovalPatternRule(command, patternRules);
+		if (safePatternRule?.approval === "allow") return { tier: "write", policy: "allow" };
+		if (safePatternRule?.approval === "prompt") {
+			return {
+				tier: "exec",
+				override: true,
+				policy: "prompt",
+				reason: `Prompt required by bash pattern: ${safePatternRule.match}`,
+			};
+		}
 		return "exec";
 	};
 	readonly formatApprovalDetails = (args: unknown): string[] => {
@@ -21,9 +21,10 @@ function tool(
 	return { name, approval, formatApprovalDetails };
 }

-function createBashTool(): BashTool {
+function createBashTool(settingsOverrides: Record<string, unknown> = {}): BashTool {
 	const settings = {
 		get(key: string): unknown {
+			if (Object.hasOwn(settingsOverrides, key)) return settingsOverrides[key];
 			switch (key) {
 				case "async.enabled":
 				case "bash.autoBackground.enabled":
@@ -42,8 +43,8 @@ function createBashTool(): BashTool {
 	return new BashTool({ settings } as unknown as ConstructorParameters<typeof BashTool>[0]);
 }

-function bashApproval(command: string) {
-	const approval = createBashTool().approval;
+function bashApproval(command: string, settingsOverrides: Record<string, unknown> = {}) {
+	const approval = createBashTool(settingsOverrides).approval;
 	if (typeof approval !== "function") throw new Error("Bash approval must be dynamic");
 	return approval({ command });
 }
@@ -94,6 +95,22 @@ describe("resolveApproval override and user policy", () => {
 		);
 	});

+	it("tool-owned deny policy blocks before mode and user allow policies", () => {
+		const blocked = tool("bash", {
+			tier: "exec",
+			override: true,
+			policy: "deny",
+			reason: "Blocked by bash pattern: rm -rf *",
+		});
+		expect(resolveApproval(blocked, {}, "yolo", { bash: "allow" })).toMatchObject({
+			policy: "deny",
+			source: "tool",
+		});
+		expect(() => requiresApproval(blocked, {}, "write", { bash: "allow" })).toThrow(
+			'Tool "bash" is blocked by tool policy',
+		);
+	});
+
 	it("valid user policy overrides mode and tier when no tool override is active", () => {
 		const writeTool = tool("write", "write");
 		expect(resolveApproval(writeTool, {}, "always-ask", { write: "allow" }).policy).toBe("allow");
@@ -178,6 +195,85 @@ describe("tool-owned dynamic approval declarations", () => {
 		}
 	});

+	it("classifies configured bash approval patterns", () => {
+		const settingsOverrides = {
+			"bash.patterns": [
+				{ match: "git *", approval: "allow" },
+				{ match: "rm -rf *", approval: "deny" },
+				{ match: "*", approval: "prompt" },
+			],
+		};
+
+		for (const command of ["git diff packages/coding-agent/src/tools/bash.ts", "git status", "git log --oneline"]) {
+			expect(bashApproval(command, settingsOverrides)).toEqual({ tier: "write", policy: "allow" });
+		}
+
+		expect(bashApproval("rm -rf build", settingsOverrides)).toEqual({
+			tier: "exec",
+			override: true,
+			policy: "deny",
+			reason: "Blocked by bash pattern: rm -rf *",
+		});
+		expect(
+			bashApproval("git diff packages/coding-agent/src/tools/bash.ts && rm file.txt", settingsOverrides),
+		).toEqual({
+			tier: "exec",
+			override: true,
+			policy: "prompt",
+			reason: "Prompt required by bash pattern: *",
+		});
+		expect(bashApproval("echo hello", settingsOverrides)).toEqual({
+			tier: "exec",
+			override: true,
+			policy: "prompt",
+			reason: "Prompt required by bash pattern: *",
+		});
+	});
+
+	it("keeps critical bash patterns prompt-gated unless explicitly denied", () => {
+		const settingsOverrides = {
+			"bash.patterns": [{ match: "*", approval: "allow" }],
+		};
+
+		expect(bashApproval("rm -rf /", settingsOverrides)).toEqual({
+			tier: "exec",
+			override: true,
+			reason: "Critical pattern detected",
+		});
+		expect(bashApproval("echo hello", settingsOverrides)).toEqual({
+			tier: "write",
+			policy: "allow",
+		});
+		expect(bashApproval("echo hello && rm file.txt", settingsOverrides)).toBe("exec");
+	});
+
+	it("applies the first matching bash approval pattern", () => {
+		const settingsOverrides = {
+			"bash.patterns": [
+				{ match: "*", approval: "allow" },
+				{ match: "git *", approval: "deny" },
+			],
+		};
+
+		expect(bashApproval("git status", settingsOverrides)).toEqual({
+			tier: "write",
+			policy: "allow",
+		});
+	});
+
+	it("allows a specific deny pattern to block a critical bash command", () => {
+		const settingsOverrides = {
+			"bash.patterns": [{ match: "rm -rf *", approval: "deny" }],
+		};
+
+		expect(bashApproval("rm -rf /", settingsOverrides)).toEqual({
+			tier: "exec",
+			override: true,
+			policy: "deny",
+			reason: "Blocked by bash pattern: rm -rf *",
+		});
+	});
+
 	it("exports LSP and debug read-only action sets from their owning tools", () => {
 		expect(LSP_READONLY_ACTIONS.has("diagnostics")).toBe(true);
 		expect(LSP_READONLY_ACTIONS.has("rename")).toBe(false);