f929dfa252130cbe5b31245f4d1b3bf707c35bac

Author
TheEdgeOfRage <git@theedgeofrage.com>
Committer
TheEdgeOfRage <git@theedgeofrage.com>
Date

Message

Improve deterministic command matching

Diff

  1diff --git a/src/deterministic.ts b/src/deterministic.ts
  2index 7438429f5f9f823da0de51ba2d75f7cc2ab9ae65..4651187fd9a567b7d15ad4edb1ccf1ba83d308d2 100644
  3--- a/src/deterministic.ts
  4+++ b/src/deterministic.ts
  5@@ -1,5 +1,11 @@
  6 import type { Decision } from "./types"
  7-import { HARD_ALLOW_PATTERNS, CONFIG_ALLOW_PATTERNS, ASK_PATTERNS, SHELL_CONTROL_RE } from "./rules"
  8+import {
  9+  HARD_ALLOW_PATTERNS,
 10+  CONFIG_ALLOW_PATTERNS,
 11+  ASK_PATTERNS,
 12+  SHELL_CONTROL_RE,
 13+  STDERR_REDIRECT_RE,
 14+} from "./rules"
 15 
 16 function hasShellControl(cmd: string): boolean {
 17   return SHELL_CONTROL_RE.test(cmd.replace(/"[^"]*"|'[^']*'/g, '""'))
 18@@ -49,7 +55,10 @@ function checkConfigAllow(command: string): Decision | undefined {
 19 }
 20 
 21 function checkBash(command: string): Decision | undefined {
 22-  return checkHardAllow(command) ?? checkConfigAllow(command) ?? checkAskPatterns(command)
 23+  // Trailing `2>&1` / `2>/dev/null` are security-neutral; strip before matching
 24+  // so SHELL_CONTROL_RE's `>` check doesn't reject them.
 25+  const cleaned = command.replace(STDERR_REDIRECT_RE, "")
 26+  return checkHardAllow(cleaned) ?? checkConfigAllow(cleaned) ?? checkAskPatterns(cleaned)
 27 }
 28 
 29 export function checkDeterministic(
 30diff --git a/src/rules.ts b/src/rules.ts
 31index a2349b814b4dc0fcc729c758e47a9c052548e627..ed15612087d71846762316a34f120d7e3792129b 100644
 32--- a/src/rules.ts
 33+++ b/src/rules.ts
 34@@ -1,5 +1,10 @@
 35 // Bump to invalidate all cached decisions when rules change.
 36-export const POLICY_VERSION = 1;
 37+export const POLICY_VERSION = 2;
 38+
 39+// Trailing stderr redirections that are safe to strip before pattern matching.
 40+// `2>&1` and `2>/dev/null` have no security implication but would otherwise
 41+// trip SHELL_CONTROL_RE's `>` check.
 42+export const STDERR_REDIRECT_RE = /\s+2>(?:&1|\/dev\/null)\s*$/;
 43 
 44 // Matched with test() on anchored patterns (equivalent to Python fullmatch).
 45 // Purely a performance optimization — these would pass LLM review anyway.
 46@@ -9,12 +14,14 @@ export const HARD_ALLOW_PATTERNS: RegExp[] = [
 47   /^git\s+diff(?:\s+(--staged|--cached))?\s*$/,
 48   /^git\s+log(?:\s+--oneline)?(?:\s+-(?:\d{1,3})|\s+-n\s+\d{1,3})?\s*$/,
 49   /^git\s+show(?:\s+(HEAD(?:[~^]\d+)?|[0-9a-f]{7,40}))?\s*$/,
 50-  /^git\s+branch(?:\s+(-a|--all|-vv))?\s*$/,
 51+  /^git\s+branch(?:\s+(-a|--all|-vv|-l|--show-current))?\s*$/,
 52   /^git\s+remote(?:\s+-v)?\s*$/,
 53-  /^git\s+rev-parse\s+--abbrev-ref\s+HEAD\s*$/,
 54+  /^git\s+rev-parse\s+(?:--abbrev-ref\s+HEAD|HEAD)\s*$/,
 55+  /^git\s+reflog(?:\s+-\d+)?\s*$/,
 56   /^git\s+describe(?:\s+--tags)?\s*$/,
 57   /^git\s+tag\s+-l\s*$/,
 58   /^git\s+stash\s+list\s*$/,
 59+  /^git\s+stash\s+show\s+stash@\{\d+\}(?:\s+--stat)?\s*$/,
 60 
 61   // Filesystem + process inspection
 62   /^pwd\s*$/,
 63@@ -23,8 +30,9 @@ export const HARD_ALLOW_PATTERNS: RegExp[] = [
 64   /^which\s+[A-Za-z0-9._-]+\s*$/,
 65   /^whereis\s+[A-Za-z0-9._-]+\s*$/,
 66   /^type\s+[A-Za-z0-9._-]+\s*$/,
 67-  // ls with relative paths only (block hidden files, traversal, absolute paths)
 68-  /^ls(?:\s+-[A-Za-z]+)*(?:\s+(?!.*\/\.)(?!.*\.\.)[A-Za-z0-9][A-Za-z0-9._/-]*)?\s*$/,
 69+  // ls with relative or absolute paths (block hidden files, traversal).
 70+  // ls only prints filenames, no content leakage.
 71+  /^ls(?:\s+-[A-Za-z]+)*(?:\s+(?!.*\/\.)(?!.*\.\.)\/?[A-Za-z0-9][A-Za-z0-9._/-]*)*\s*$/,
 72   // cat with relative paths only
 73   /^cat\s+(?!.*\/\.)(?!.*\.\.)[A-Za-z0-9][A-Za-z0-9._/-]*(?:\s+(?!.*\/\.)(?!.*\.\.)[A-Za-z0-9][A-Za-z0-9._/-]*)*\s*$/,
 74   // head/tail with relative paths and line limits
 75@@ -61,9 +69,10 @@ export const CONFIG_ALLOW_PATTERNS: RegExp[] = [
 76   /^qmd\s/,
 77   /^mkdir\s/,
 78 
 79-  // Git ops not covered by strict HARD_ALLOW patterns
 80-  /^git\s+ls-files(?:\s|$)/,
 81-  /^git\s+merge-base\s/,
 82+  // Read-only git subcommands with optional `-C <path>` (multi-repo workflows).
 83+  // Destructive subcommands (branch -d, tag -d, stash drop, fetch --force, etc.)
 84+  // are intentionally excluded from this broad prefix — they have narrow rules.
 85+  /^git(?:\s+-C\s+\S+)?\s+(status|diff|log|show|rev-parse|reflog|shortlog|blame|describe|ls-tree|cat-file|rev-list|ls-files|merge-base)(?:\s|$)/,
 86 
 87   // Go toolchain
 88   /^(?:TZ=\S+\s+)?go\s+(doc|env|get|list|mod|test|vet)(?:\s|$)/,
 89diff --git a/test/deterministic.test.ts b/test/deterministic.test.ts
 90index cedaa4f9895e03db85e3f1144cc48128486f92e0..74a2577cd4b603ecf4c9de37ec8c7c9321f2ce60 100644
 91--- a/test/deterministic.test.ts
 92+++ b/test/deterministic.test.ts
 93@@ -18,6 +18,11 @@ describe("hard allow — accepts simple safe commands", () => {
 94     "type node",
 95     "ls -la",
 96     "ls src",
 97+    "ls query/environments/dev/ query/environments/prod/ query/environments/personal/",
 98+    // absolute paths allowed for ls (filenames only, no content)
 99+    "ls /home/user/project",
100+    "ls /tmp",
101+    "ls -la /home/user/project /tmp/other",
102     "cat package.json",
103     "cat README.md rules.md",
104     "ps",
105@@ -38,6 +43,14 @@ describe("hard allow — accepts simple safe commands", () => {
106     "grep -E 'foo|bar|baz'",
107     // find read-only
108     "find src -type f -name '*.ts'",
109+    // extended git read-only
110+    "git branch --show-current",
111+    "git branch -l",
112+    "git rev-parse HEAD",
113+    "git reflog",
114+    "git reflog -20",
115+    "git stash show stash@{0}",
116+    "git stash show stash@{1} --stat",
117   ]
118   for (const cmd of cases) {
119     test(cmd, () => {
120@@ -63,6 +76,34 @@ describe("shell operators block deterministic allow", () => {
121   }
122 })
123 
124+describe("trailing stderr redirection is stripped before matching", () => {
125+  const cases = [
126+    "ls -la 2>&1",
127+    "ls src 2>/dev/null",
128+    "git status 2>&1",
129+    "git log --oneline -10 2>&1",
130+    "git stash list 2>&1",
131+    "bun test 2>&1",
132+    "go test ./... 2>&1",
133+    "make check 2>&1",
134+    // -C combined with stderr strip
135+    "git -C /home/user/repo status 2>&1",
136+  ]
137+  for (const cmd of cases) {
138+    test(cmd, () => {
139+      const d = checkDeterministic("bash", { command: cmd })
140+      expect(d).toBeDefined()
141+      expect(d!.decision).toBe("allow")
142+    })
143+  }
144+
145+  // Non-trailing redirects still caught
146+  test("redirect before 2>&1 still blocked", () => {
147+    const d = checkDeterministic("bash", { command: "cat foo > /etc/passwd 2>&1" })
148+    expect(d?.decision !== "allow").toBe(true)
149+  })
150+})
151+
152 describe("config allow — accepts broad patterns from user config", () => {
153   const cases = [
154     "jq .key data.json",
155@@ -71,6 +112,25 @@ describe("config allow — accepts broad patterns from user config", () => {
156     "make test",
157     "git ls-files",
158     "git merge-base main feature",
159+    // extended read-only git with broader args
160+    "git status --short",
161+    "git status --porcelain=2 --branch",
162+    "git diff --stat",
163+    "git diff --shortstat main",
164+    "git diff --cached --stat",
165+    "git diff main..HEAD",
166+    "git diff HEAD -- path/to/file",
167+    "git log --oneline -30 -- path/to/file",
168+    "git log origin/main..HEAD",
169+    "git log -1 --format='%H %s'",
170+    "git show --stat HEAD",
171+    "git show 098d119b3 --stat",
172+    "git show 098d119b3 -- crates/foo/src/lib.rs",
173+    "git show origin/main:core-node/apps/analytics/src/file.ts",
174+    // git -C <path> multi-repo variants
175+    "git -C /home/user/repo status",
176+    "git -C /home/user/repo log --oneline -5",
177+    "git -C /home/user/repo diff main..HEAD",
178     // xargs with read-only commands (pipe targets)
179     "xargs grep -A2 snakeyaml",
180     "xargs head -20",