From 2fe019ccaed8d7f943ac66230aeed981cb88cda2 Mon Sep 17 00:00:00 2001
From: Peter Steinberger <steipete@gmail.com>
Date: Sun, 31 May 2026 14:00:04 +0100
Subject: [PATCH] fix(exec): allow predicate shell builtins in allowlist mode

---
 src/infra/exec-safe-builtins.test.ts | 50 ++++++++++++++++++++++++++++
 src/infra/exec-safe-builtins.ts      | 12 ++++++-
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/src/infra/exec-safe-builtins.test.ts b/src/infra/exec-safe-builtins.test.ts
index 708ee58ca81..e40efa1c4b7 100644
--- a/src/infra/exec-safe-builtins.test.ts
+++ b/src/infra/exec-safe-builtins.test.ts
@@ -65,6 +65,30 @@ describe("isSafeBuiltinSegment", () => {
     ).toBe(false);
   });
 
+  it("allows test and well-formed bracket predicates", () => {
+    expect(
+      isSafeBuiltinSegment({
+        segment: builtinSegment(["test", "-d", "/tmp"]),
+        platform: "linux",
+      }),
+    ).toBe(true);
+    expect(
+      isSafeBuiltinSegment({
+        segment: builtinSegment(["[", "-d", "/tmp", "]"]),
+        platform: "linux",
+      }),
+    ).toBe(true);
+  });
+
+  it("rejects malformed bracket predicates", () => {
+    expect(
+      isSafeBuiltinSegment({
+        segment: builtinSegment(["[", "-d", "/tmp"]),
+        platform: "linux",
+      }),
+    ).toBe(false);
+  });
+
   it("returns false on Windows hosts (PowerShell semantics differ)", () => {
     expect(
       isSafeBuiltinSegment({
@@ -113,6 +137,32 @@ describe("evaluateShellAllowlist with known safe builtins (regression for #46056
     expect(result.segmentSatisfiedBy).toContain("allowlist");
   });
 
+  it("'test -d /tmp && git status' passes with allowlist plus safe builtin handling", () => {
+    const result = evaluateShellAllowlist({
+      command: "test -d /tmp && git status",
+      allowlist: gitAllowlist,
+      safeBins: new Set(),
+      cwd: "/tmp",
+    });
+    expect(result.analysisOk).toBe(true);
+    expect(result.allowlistSatisfied).toBe(true);
+    expect(result.segmentSatisfiedBy).toContain("safeBuiltins");
+    expect(result.segmentSatisfiedBy).toContain("allowlist");
+  });
+
+  it("'[ -d /tmp ] && git status' passes with allowlist plus safe builtin handling", () => {
+    const result = evaluateShellAllowlist({
+      command: "[ -d /tmp ] && git status",
+      allowlist: gitAllowlist,
+      safeBins: new Set(),
+      cwd: "/tmp",
+    });
+    expect(result.analysisOk).toBe(true);
+    expect(result.allowlistSatisfied).toBe(true);
+    expect(result.segmentSatisfiedBy).toContain("safeBuiltins");
+    expect(result.segmentSatisfiedBy).toContain("allowlist");
+  });
+
   it("non-allowlisted binary still gates after a safe builtin", () => {
     const result = evaluateShellAllowlist({
       command: "cd /tmp && curl evil.com",
diff --git a/src/infra/exec-safe-builtins.ts b/src/infra/exec-safe-builtins.ts
index 363b7d40193..99fccf9d0f7 100644
--- a/src/infra/exec-safe-builtins.ts
+++ b/src/infra/exec-safe-builtins.ts
@@ -3,7 +3,14 @@ import { isWindowsPlatform, type ExecCommandSegment } from "./exec-approvals-ana
 // POSIX shell builtins that cannot execute external code or mutate environment state on their
 // own. Shell allowlist evaluation handles them as a closed internal set instead of path-based
 // safeBins matching.
-const DEFAULT_SAFE_BUILTINS: ReadonlySet<string> = new Set([":", "cd", "false", "pwd", "true"]);
+const DEFAULT_SAFE_BUILTINS: ReadonlySet<string> = new Set([
+  ":",
+  "cd",
+  "false",
+  "pwd",
+  "test",
+  "true",
+]);
 
 export function isSafeBuiltinSegment(params: {
   segment: ExecCommandSegment;
@@ -18,5 +25,8 @@ export function isSafeBuiltinSegment(params: {
   if (!head) {
     return false;
   }
+  if (head === "[") {
+    return params.segment.argv.at(-1) === "]";
+  }
   return DEFAULT_SAFE_BUILTINS.has(head);
 }