From d1ecb46076145deb188abcba8f0699709ea17198 Mon Sep 17 00:00:00 2001
From: Peter Steinberger <steipete@gmail.com>
Date: Mon, 2 Feb 2026 16:53:09 -0800
Subject: [PATCH] fix: harden exec allowlist parsing

---
 docs/tools/exec-approvals.md     |  2 ++
 src/infra/exec-approvals.test.ts | 42 ++++++++++++++++++++++++++++++++
 src/infra/exec-approvals.ts      | 33 +++++++++++++++++++++++++
 3 files changed, 77 insertions(+)

diff --git a/docs/tools/exec-approvals.md b/docs/tools/exec-approvals.md
index b582bcd185..4fd847b7f3 100644
--- a/docs/tools/exec-approvals.md
+++ b/docs/tools/exec-approvals.md
@@ -128,6 +128,8 @@ Shell chaining and redirections are not auto-allowed in allowlist mode.
 
 Shell chaining (`&&`, `||`, `;`) is allowed when every top-level segment satisfies the allowlist
 (including safe bins or skill auto-allow). Redirections remain unsupported in allowlist mode.
+Command substitution (`$()` / backticks) is rejected during allowlist parsing, including inside
+double quotes; use single quotes if you need literal `$()` text.
 
 Default safe bins: `jq`, `grep`, `cut`, `sort`, `uniq`, `head`, `tail`, `tr`, `wc`.
 
diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts
index 091f34e144..635ba97113 100644
--- a/src/infra/exec-approvals.test.ts
+++ b/src/infra/exec-approvals.test.ts
@@ -131,6 +131,36 @@ describe("exec approvals shell parsing", () => {
     expect(res.ok).toBe(true);
     expect(res.segments[0]?.argv).toEqual(["/bin/echo", "ok"]);
   });
+
+  it("rejects command substitution inside double quotes", () => {
+    const res = analyzeShellCommand({ command: 'echo "output: $(whoami)"' });
+    expect(res.ok).toBe(false);
+    expect(res.reason).toBe("unsupported shell token: $()");
+  });
+
+  it("rejects backticks inside double quotes", () => {
+    const res = analyzeShellCommand({ command: 'echo "output: `id`"' });
+    expect(res.ok).toBe(false);
+    expect(res.reason).toBe("unsupported shell token: `");
+  });
+
+  it("rejects command substitution outside quotes", () => {
+    const res = analyzeShellCommand({ command: "echo $(whoami)" });
+    expect(res.ok).toBe(false);
+    expect(res.reason).toBe("unsupported shell token: $()");
+  });
+
+  it("allows escaped command substitution inside double quotes", () => {
+    const res = analyzeShellCommand({ command: 'echo "output: \\$(whoami)"' });
+    expect(res.ok).toBe(true);
+    expect(res.segments[0]?.argv[0]).toBe("echo");
+  });
+
+  it("allows command substitution syntax inside single quotes", () => {
+    const res = analyzeShellCommand({ command: "echo 'output: $(whoami)'" });
+    expect(res.ok).toBe(true);
+    expect(res.segments[0]?.argv[0]).toBe("echo");
+  });
 });
 
 describe("exec approvals shell allowlist (chained commands)", () => {
@@ -185,6 +215,18 @@ describe("exec approvals shell allowlist (chained commands)", () => {
     expect(result.analysisOk).toBe(true);
     expect(result.allowlistSatisfied).toBe(true);
   });
+
+  it("respects escaped quotes when splitting chains", () => {
+    const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/echo" }];
+    const result = evaluateShellAllowlist({
+      command: '/usr/bin/echo "foo\\" && bar"',
+      allowlist,
+      safeBins: new Set(),
+      cwd: "/tmp",
+    });
+    expect(result.analysisOk).toBe(true);
+    expect(result.allowlistSatisfied).toBe(true);
+  });
 });
 
 describe("exec approvals safe bins", () => {
diff --git a/src/infra/exec-approvals.ts b/src/infra/exec-approvals.ts
index 61a757ba8a..a510bd72ab 100644
--- a/src/infra/exec-approvals.ts
+++ b/src/infra/exec-approvals.ts
@@ -585,6 +585,11 @@ export type ExecCommandAnalysis = {
 };
 
 const DISALLOWED_PIPELINE_TOKENS = new Set([">", "<", "`", "\n", "\r", "(", ")"]);
+const DOUBLE_QUOTE_ESCAPES = new Set(["\\", '"', "$", "`", "\n", "\r"]);
+
+function isDoubleQuoteEscape(next: string | undefined): next is string {
+  return Boolean(next && DOUBLE_QUOTE_ESCAPES.has(next));
+}
 
 type IteratorAction = "split" | "skip" | "include" | { reject: string };
 
@@ -637,6 +642,21 @@ function iterateQuoteAware(
       continue;
     }
     if (inDouble) {
+      if (ch === "\\" && isDoubleQuoteEscape(next)) {
+        buf += ch;
+        buf += next;
+        i += 1;
+        continue;
+      }
+      if (ch === "$" && next === "(") {
+        return { ok: false, reason: "unsupported shell token: $()" };
+      }
+      if (ch === "`") {
+        return { ok: false, reason: "unsupported shell token: `" };
+      }
+      if (ch === "\n" || ch === "\r") {
+        return { ok: false, reason: "unsupported shell token: newline" };
+      }
       if (ch === '"') {
         inDouble = false;
       }
@@ -749,6 +769,12 @@ function tokenizeShellSegment(segment: string): string[] | null {
       continue;
     }
     if (inDouble) {
+      const next = segment[i + 1];
+      if (ch === "\\" && isDoubleQuoteEscape(next)) {
+        buf += next;
+        i += 1;
+        continue;
+      }
       if (ch === '"') {
         inDouble = false;
       } else {
@@ -1067,6 +1093,7 @@ function splitCommandChain(command: string): string[] | null {
 
   for (let i = 0; i < command.length; i += 1) {
     const ch = command[i];
+    const next = command[i + 1];
     if (escaped) {
       buf += ch;
       escaped = false;
@@ -1085,6 +1112,12 @@ function splitCommandChain(command: string): string[] | null {
       continue;
     }
     if (inDouble) {
+      if (ch === "\\" && isDoubleQuoteEscape(next)) {
+        buf += ch;
+        buf += next;
+        i += 1;
+        continue;
+      }
       if (ch === '"') {
         inDouble = false;
       }