From 28a6132eafc2f7297dce83f5631f74c67d0411e6 Mon Sep 17 00:00:00 2001
From: Jared Pleva <jpleva91@gmail.com>
Date: Sun, 29 Mar 2026 18:03:46 +0000
Subject: [PATCH] fix(p0): close governance fail-open vulnerabilities (#58,
 #62, #69, #75)

- govern-shell.sh: use jq --arg for safe JSON construction (fixes #75 printf injection)
- govern-shell.sh: use jq for output parsing, fail-closed on governance unavailable (fixes #67)
- cmd/shellforge/main.go: fail-closed on JSON unmarshal error and stdin read error (fixes #62)
- internal/governance/engine.go: wildcard-only timeout policy no longer matches every command (fixes #58)
- agentguard.yaml: block all rm (not just -rf/-fr), fix misleading mode comment (fixes #69, #59)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 agentguard.yaml               |  7 +++----
 cmd/shellforge/main.go        | 10 ++++++++--
 internal/governance/engine.go |  4 +++-
 scripts/govern-shell.sh       | 20 ++++++++++++++++----
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/agentguard.yaml b/agentguard.yaml
index 3fbd43d..a464d4b 100644
--- a/agentguard.yaml
+++ b/agentguard.yaml
@@ -1,5 +1,5 @@
 # ShellForge — AgentGuard Governance Policy
-# Mode: monitor (log but don't block) — switch to enforce when ready
+# Mode: enforce (block violations)
 
 mode: enforce
 
@@ -13,12 +13,11 @@ policies:
     message: "Force push is not allowed by governance policy"
 
   - name: no-destructive-rm
-    description: Block recursive force deletion
+    description: Block all rm invocations (plain rm, -r, -rf, etc.)
     match:
       command: "rm"
-      args_contain: ["-rf", "-fr", "--recursive --force", "--force --recursive", "-r -f", "-f -r"]
     action: deny
-    message: "Destructive rm blocked by policy"
+    message: "rm is not allowed by governance policy — use targeted file operations instead"
 
   - name: file-write-constraints
     description: Agents may only write to outputs/ and agents' own workspace
diff --git a/cmd/shellforge/main.go b/cmd/shellforge/main.go
index 55f2bea..805a359 100644
--- a/cmd/shellforge/main.go
+++ b/cmd/shellforge/main.go
@@ -801,7 +801,7 @@ func cmdEvaluate() {
 // Used by Crush fork to check actions before execution.
 data, err := io.ReadAll(os.Stdin)
 if err != nil {
-json.NewEncoder(os.Stdout).Encode(map[string]any{"allowed": true, "reason": "stdin read error"})
+json.NewEncoder(os.Stdout).Encode(map[string]any{"allowed": false, "reason": "stdin read error"})
 return
 }
 
@@ -810,7 +810,13 @@ Tool   string `json:"tool"`
 Action string `json:"action"`
 Path   string `json:"path"`
 }
-json.Unmarshal(data, &input)
+if err := json.Unmarshal(data, &input); err != nil {
+json.NewEncoder(os.Stdout).Encode(map[string]any{
+"allowed": false,
+"reason":  "malformed governance request: " + err.Error(),
+})
+return
+}
 
 configPath := findGovernanceConfig()
 if configPath == "" {
diff --git a/internal/governance/engine.go b/internal/governance/engine.go
index be6f943..baab1bb 100644
--- a/internal/governance/engine.go
+++ b/internal/governance/engine.go
@@ -120,7 +120,9 @@ if m.Command == "*" {
 if len(m.ArgsContain) > 0 {
 return containsAny(cmd, m.ArgsContain)
 }
-return p.Timeout > 0
+// Wildcard with no args_contain is a budget/monitoring policy only.
+// Timeout is enforced separately via GetTimeout(); don't match every command.
+return false
 }
 if !strings.Contains(cmd, m.Command) {
 return false
diff --git a/scripts/govern-shell.sh b/scripts/govern-shell.sh
index 7e62b6b..8412f46 100755
--- a/scripts/govern-shell.sh
+++ b/scripts/govern-shell.sh
@@ -22,10 +22,22 @@ if [ "${1:-}" = "-c" ]; then
     COMMAND="$*"
 
     # Evaluate through AgentGuard
-    RESULT=$(printf '{"tool":"run_shell","action":"%s","path":"."}' "$COMMAND" | shellforge evaluate 2>/dev/null || echo '{"allowed":true}')
-    
-    if echo "$RESULT" | grep -q '"allowed":false'; then
-        REASON=$(echo "$RESULT" | sed 's/.*"reason":"\([^"]*\)".*/\1/')
+    # Use jq --arg for safe JSON construction: handles quotes, backslashes, control chars in $COMMAND
+    if command -v jq &>/dev/null; then
+        JSON_PAYLOAD=$(jq -n --arg cmd "$COMMAND" '{"tool":"run_shell","action":$cmd,"path":"."}')
+    else
+        # Fallback: basic escaping (covers common cases; jq preferred)
+        ESCAPED=$(printf '%s' "$COMMAND" | sed 's/\\/\\\\/g; s/"/\\"/g')
+        JSON_PAYLOAD="{\"tool\":\"run_shell\",\"action\":\"$ESCAPED\",\"path\":\".\"}"
+    fi
+    RESULT=$(printf '%s' "$JSON_PAYLOAD" | shellforge evaluate 2>/dev/null || echo '{"allowed":false,"reason":"governance unavailable"}')
+
+    if printf '%s' "$RESULT" | grep -q '"allowed":false'; then
+        if command -v jq &>/dev/null; then
+            REASON=$(printf '%s' "$RESULT" | jq -r '.reason // "policy violation"')
+        else
+            REASON=$(printf '%s' "$RESULT" | sed 's/.*"reason":"\([^"]*\)".*/\1/')
+        fi
         echo "[AgentGuard] DENIED: $REASON" >&2
         exit 1
     fi