From 680d44e719615b10bd3e30ed4ed26580d7533911 Mon Sep 17 00:00:00 2001 From: preved911 Date: Wed, 29 Jul 2026 11:30:21 +0000 Subject: [PATCH] feat: reject complex chains that require approval with readability instruction When a bash command has multiple top-level chain segments (&&, ||, ;, |) and the resolved permission would be 'ask', the plugin now rejects the command with a heredoc message telling the agent to rewrite it with newlines and comments for readability. - Add topLevelSegments to ChainResult to distinguish top-level chains from nested substitutions (, backticks, eval, sh -c) - Returns readabilityReject=true and stores deny decision for complex ask commands - Replaces the command with a heredoc error message instead of wrapping in { ... ; } - 18 new tests covering isComplexChain, readability rejection logic, and buildReadabilityMessage --- src/__tests__/enforce.test.ts | 119 +++++++++++++++++++++++++++++++++- src/chain.ts | 8 ++- src/enforce.ts | 55 ++++++++++++++-- src/index.ts | 17 +++-- 4 files changed, 183 insertions(+), 16 deletions(-) diff --git a/src/__tests__/enforce.test.ts b/src/__tests__/enforce.test.ts index 7db007f..d6774f7 100644 --- a/src/__tests__/enforce.test.ts +++ b/src/__tests__/enforce.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach } from "vitest"; -import { resolveSegment, resolveChain, beforeExecute, handlePermissionAsk, clearStoredDecision } from "../enforce.js"; +import { resolveSegment, resolveChain, beforeExecute, handlePermissionAsk, clearStoredDecision, isComplexChain, buildReadabilityMessage } from "../enforce.js"; import type { PluginConfig } from "../config.js"; const defaultConfig: PluginConfig = { @@ -172,3 +172,120 @@ describe("handlePermissionAsk", () => { expect(output.status).toBe("ask"); }); }); + +describe("isComplexChain", () => { + it("returns true for pipe chain", () => { + expect(isComplexChain("cat log.txt | grep error | sort")).toBe(true); + }); + + it("returns true for && chain", () => { + expect(isComplexChain("cd src && npm run build && npm test")).toBe(true); + }); + + it("returns false for single command", () => { + expect(isComplexChain("git status")).toBe(false); + }); + + it("returns false for single command with arguments", () => { + expect(isComplexChain("npm run build -- --watch")).toBe(false); + }); + + it("returns true for semicolon chain", () => { + expect(isComplexChain("echo hello; echo world")).toBe(true); + }); + + it("returns true for mixed operators", () => { + expect(isComplexChain("cd src && cat package.json | grep name")).toBe(true); + }); + + it("returns false for command with nested substitution only", () => { + expect(isComplexChain('cat $(find . -name "*.txt")')).toBe(false); + }); +}); + +describe("readabilityRejection", () => { + const askConfig: PluginConfig = { + bashRules: [ + { pattern: "*", action: "ask" }, + ], + externalDirectoryRules: [{ pattern: "./**", action: "allow" }], + externalDirectoryDefault: null, + enabled: true, + }; + + const gitConfig: PluginConfig = { + bashRules: [ + { pattern: "*", action: "ask" }, + { pattern: "git *", action: "allow" }, + { pattern: "npm *", action: "allow" }, + ], + externalDirectoryRules: [{ pattern: "./**", action: "allow" }], + externalDirectoryDefault: null, + enabled: true, + }; + + beforeEach(() => { + clearStoredDecision("test-readability"); + }); + + it("rejects complex chain with readabilityReject=true when action would be ask", () => { + const result = beforeExecute("Bash", "test-readability", "/project", { command: "echo hi && echo there" }, askConfig); + expect(result.readabilityReject).toBe(true); + expect(result.chainAction).toBe("deny"); + expect(result.shouldWrap).toBe(true); + }); + + it("rejects mixed chain with readabilityReject=true when one segment triggers ask", () => { + const result = beforeExecute("Bash", "test-readability", "/project", { command: "npm install good && wget evil.sh" }, gitConfig); + expect(result.readabilityReject).toBe(true); + expect(result.chainAction).toBe("deny"); + }); + + it("stores deny decision for readability-rejected command", () => { + beforeExecute("Bash", "test-readability", "/project", { command: "echo hi && echo there" }, askConfig); + const output = { status: "ask" as const }; + handlePermissionAsk({ callID: "test-readability" }, output); + expect(output.status).toBe("deny"); + }); + + it("does not reject single command that needs ask", () => { + const result = beforeExecute("Bash", "test-readability", "/project", { command: "wget evil.sh" }, gitConfig); + expect(result.readabilityReject).toBe(false); + expect(result.chainAction).toBe("ask"); + }); + + it("does not reject complex chain when all segments are denied anyway", () => { + const denyConfig: PluginConfig = { + bashRules: [{ pattern: "*", action: "deny" }], + externalDirectoryRules: [], + externalDirectoryDefault: null, + enabled: true, + }; + const result = beforeExecute("Bash", "test-readability", "/project", { command: "echo hi && echo there" }, denyConfig); + expect(result.readabilityReject).toBe(false); + expect(result.chainAction).toBe("deny"); + }); + + it("does not reject when command is parse error", () => { + const result = beforeExecute("Bash", "test-readability", "/project", { command: "echo \"hello" }, askConfig); + expect(result.readabilityReject).toBe(false); + expect(result.chainAction).toBe("deny"); + }); +}); + +describe("buildReadabilityMessage", () => { + it("includes the original command in the message", () => { + const msg = buildReadabilityMessage("echo hi && echo there"); + expect(msg).toContain("echo hi && echo there"); + }); + + it("starts with a heredoc", () => { + const msg = buildReadabilityMessage("echo hi && echo there"); + expect(msg).toContain("OPENGUARD"); + }); + + it("ends with exit 1", () => { + const msg = buildReadabilityMessage("echo hi && echo there"); + expect(msg).toContain("exit 1"); + }); +}); diff --git a/src/chain.ts b/src/chain.ts index bf632f8..802c440 100644 --- a/src/chain.ts +++ b/src/chain.ts @@ -8,6 +8,7 @@ export interface ChainSegment { export interface ChainResult { segments: ChainSegment[]; + topLevelSegments: ChainSegment[]; parseError: boolean; errors: string[]; } @@ -122,7 +123,7 @@ function parseMetaCommandArgs(command: string): string | null { export function parseChain(command: string): ChainResult { if (!command || command.trim().length === 0) { - return { segments: [], parseError: false, errors: [] }; + return { segments: [], topLevelSegments: [], parseError: false, errors: [] }; } const result = parse(command); @@ -136,7 +137,8 @@ export function parseChain(command: string): ChainResult { } } - const segments = extractCommandsFromScript(result); + const topLevelSegments = extractCommandsFromScript(result); + const segments = [...topLevelSegments]; const nestedCmds = extractNestedCommands(result); segments.push(...nestedCmds); @@ -158,5 +160,5 @@ export function parseChain(command: string): ChainResult { } } - return { segments, parseError, errors }; + return { segments, topLevelSegments, parseError, errors }; } diff --git a/src/enforce.ts b/src/enforce.ts index 1a63347..2d767a2 100644 --- a/src/enforce.ts +++ b/src/enforce.ts @@ -63,40 +63,81 @@ export function resolveChain(segments: Array<{ command: string; commandName: str return null; } +export interface BeforeExecuteResult { + shouldWrap: boolean; + chainAction: ChainAction; + readabilityReject: boolean; +} + +/** + * Check whether a command has multiple top-level chain segments (pipes, &&, ||, ;) + * that would benefit from formatting with newlines and comments, but exclude + * single-command invocations that merely contain nested substitutions. + */ +export function isComplexChain(command: string): boolean { + const chain = parseChain(command); + return chain.topLevelSegments.length > 1; +} + +/** Build a replacement command that prints a readability-formatting error. */ +export function buildReadabilityMessage(command: string): string { + return `cat <<'OPENGUARD' +\u2716 Command rejected: contains multiple chained operations. + +The agent must rewrite this command using line breaks and comments for readability: + + # Step 1: describe what this does + first-command + # Step 2: describe what this does + second-command + +Original command was: + + ${command} +OPENGUARD +exit 1`; +} + export function beforeExecute( tool: string, callID: string, cwd: string, args: any, config: PluginConfig, -): { shouldWrap: boolean; chainAction: ChainAction } { +): BeforeExecuteResult { if (tool.toLowerCase() !== "bash") { - return { shouldWrap: false, chainAction: null }; + return { shouldWrap: false, chainAction: null, readabilityReject: false }; } const command: string | undefined = args?.command; if (!command || command.trim().length === 0) { - return { shouldWrap: false, chainAction: null }; + return { shouldWrap: false, chainAction: null, readabilityReject: false }; } const chain = parseChain(command); if (chain.parseError || chain.segments.length === 0) { decisionStore.set(callID, { action: "deny" }); - return { shouldWrap: true, chainAction: "deny" }; + return { shouldWrap: true, chainAction: "deny", readabilityReject: false }; } const action = resolveChain(chain.segments, cwd, config); if (action === null || action === "allow") { - return { shouldWrap: false, chainAction: action }; + return { shouldWrap: false, chainAction: action, readabilityReject: false }; + } + + // Complex chain requiring approval → reject with readability instruction + if (action === "ask" && chain.topLevelSegments.length > 1) { + decisionStore.set(callID, { action: "deny" }); + return { shouldWrap: true, chainAction: "deny", readabilityReject: true }; } if (action === "deny" || action === "ask") { decisionStore.set(callID, { action }); - return { shouldWrap: true, chainAction: action }; + return { shouldWrap: true, chainAction: action, readabilityReject: false }; } - return { shouldWrap: false, chainAction: null }; + return { shouldWrap: false, chainAction: null, readabilityReject: false }; } export function handlePermissionAsk(input: { callID?: string }, output: { status: "ask" | "deny" | "allow" }): void { diff --git a/src/index.ts b/src/index.ts index c6e6ef4..6b3c9b6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,7 @@ import type { Plugin, Config, Hooks } from "@opencode-ai/plugin"; import type { Permission } from "@opencode-ai/sdk"; import { parseConfig } from "./config.js"; -import { beforeExecute, handlePermissionAsk } from "./enforce.js"; +import { beforeExecute, buildReadabilityMessage, handlePermissionAsk } from "./enforce.js"; let pluginConfig: ReturnType | null = null; @@ -30,10 +30,17 @@ const BashGuardPlugin: Plugin = async (input) => { if (result.shouldWrap && result.chainAction) { const originalCommand = toolOutput.args?.command || toolOutput.args?.args?.command; if (originalCommand && typeof originalCommand === "string") { - toolOutput.args = { - ...toolOutput.args, - command: `{ ${originalCommand}; }`, - }; + if (result.readabilityReject) { + toolOutput.args = { + ...toolOutput.args, + command: buildReadabilityMessage(originalCommand), + }; + } else { + toolOutput.args = { + ...toolOutput.args, + command: `{ ${originalCommand}; }`, + }; + } } } },