fix: cross-layer enforcement gaps and cache mutation (vettri review)

- bwrap: '---' rules on SYSTEM_RO_BIND_PATHS (/etc /usr /bin /lib /sbin /opt) now emit
  --tmpfs in restrictive mode — previously the deny branch was gated to permissive mode
  only, leaving syscalls inside the sandbox able to read /etc/passwd etc. despite policy
- seatbelt: bracket globs [abc] now detected as wildcards (/[*?[]/ and strip regex updated);
  previously emitted as SBPL literals matching only a file literally named '[abc]'
- access-policy-file: mergeAccessPolicy fast-path (!base) returns shallow copy instead of
  reference — autoExpandBareDir was mutating the cached agents['*'].policy in-place,
  corrupting all subsequent resolveAccessPolicyForAgent calls in the same process
- access-policy: sha256 comparison normalizes to lowercase (.toLowerCase()) — validation
  regex accepts uppercase (/i) but crypto.digest always returns lowercase, causing uppercase
  sha256 in config to silently deny exec at runtime with no useful error
- Tests added for all four findings

src/infra/access-policy-file.test.ts

@@ -65,6 +65,22 @@ describe("mergeAccessPolicy", () => {
     expect(mergeAccessPolicy(undefined, override)).toEqual(override);
   });
 
+  it("returns a copy when base is undefined — mutations do not corrupt the original", () => {
+    // mergeAccessPolicy(undefined, x) was returning x by reference. validateAccessPolicyConfig
+    // calls autoExpandBareDir which mutates .policy in-place, permanently corrupting the cached
+    // agents["*"] object for all subsequent calls in the same process.
+    const override = { policy: { "/tmp": "rwx" as const } };
+    const result = mergeAccessPolicy(undefined, override);
+    // Simulate autoExpandBareDir mutating the result.
+    if (result?.policy) {
+      result.policy["/tmp/**"] = "rwx";
+      delete result.policy["/tmp"];
+    }
+    // The original override must be unchanged.
+    expect(override.policy).toEqual({ "/tmp": "rwx" });
+    expect(override.policy["/tmp/**" as keyof typeof override.policy]).toBeUndefined();
+  });
+
   it("rules are shallow-merged, override key wins on collision", () => {
     const result = mergeAccessPolicy(
       { policy: { "/**": "r--", "~/**": "rw-" } },
@@ -201,6 +217,22 @@ describe("loadAccessPolicyFile", () => {
     spy.mockRestore();
   });
 
+  it('returns BROKEN_POLICY_FILE when scripts["policy"] is a primitive (not an object)', () => {
+    // scripts["policy"] must be a Record<string, PermStr>; a primitive like `true`
+    // silently passes structural validation and is treated as an empty shared policy.
+    const spy = vi.spyOn(console, "error").mockImplementation(() => {});
+    writeFile({
+      version: 1,
+      agents: { "*": { scripts: { policy: true as unknown as Record<string, string> } } },
+    });
+    const result = loadAccessPolicyFile();
+    expect(result).toBe(BROKEN_POLICY_FILE);
+    expect(spy).toHaveBeenCalledWith(
+      expect.stringContaining('scripts["policy"] must be an object'),
+    );
+    spy.mockRestore();
+  });
+
   it("returns parsed file when valid", () => {
     const content: AccessPolicyFile = {
       version: 1,

src/infra/access-policy-file.ts

@@ -42,7 +42,15 @@ export function mergeAccessPolicy(
     return undefined;
   }
   if (!base) {
-    return override;
+    // Return a shallow copy so that validateAccessPolicyConfig → autoExpandBareDir
+    // does not mutate the cached agents["*"] object in _fileCache. Without this,
+    // the first call permanently corrupts policy entries for all subsequent calls
+    // in the same process.
+    return {
+      ...override,
+      policy: override.policy ? { ...override.policy } : undefined,
+      scripts: override.scripts ? { ...override.scripts } : undefined,
+    };
   }
   if (!override) {
     return base;
@@ -140,8 +148,17 @@ function validateAccessPolicyFileStructure(filePath: string, parsed: unknown): s
             for (const [scriptKey, scriptEntry] of Object.entries(
               scripts as Record<string, unknown>,
             )) {
+              if (scriptKey === "policy") {
+                // scripts["policy"] must be an object (Record<string, PermStr>), not a primitive.
                 if (
-                scriptKey !== "policy" &&
+                  scriptEntry != null &&
+                  (typeof scriptEntry !== "object" || Array.isArray(scriptEntry))
+                ) {
+                  errors.push(
+                    `${filePath}: agents["${agentId}"].scripts["policy"] must be an object (Record<string, PermStr>), got ${JSON.stringify(scriptEntry)}`,
+                  );
+                }
+              } else if (
                 scriptEntry != null &&
                 typeof scriptEntry === "object" &&
                 !Array.isArray(scriptEntry)

src/infra/access-policy.test.ts

@@ -125,6 +125,23 @@ describe("validateAccessPolicyConfig", () => {
     ).toBe(true);
   });
 
+  it('emits "---"-specific mid-path wildcard diagnostic for scripts["policy"] deny rules', () => {
+    // "---" with a mid-path wildcard cannot be enforced at the OS layer —
+    // the diagnostic must say "OS-level enforcement cannot apply", not the generic prefix-match message.
+    const config: AccessPolicyConfig = {
+      scripts: {
+        policy: { "/home/*/secrets/**": "---" },
+      },
+    };
+    const errs = validateAccessPolicyConfig(config);
+    expect(
+      errs.some(
+        (e) =>
+          e.includes("OS-level") && e.includes("cannot apply") && e.includes('scripts["policy"]'),
+      ),
+    ).toBe(true);
+  });
+
   it("emits mid-path wildcard diagnostic for per-script policy entries", () => {
     const config: AccessPolicyConfig = {
       scripts: {
@@ -137,6 +154,21 @@ describe("validateAccessPolicyConfig", () => {
     );
   });
 
+  it('emits "---"-specific mid-path wildcard diagnostic for per-script deny rules', () => {
+    // Same as scripts["policy"] — per-script "---" mid-path must get the stronger warning.
+    const config: AccessPolicyConfig = {
+      scripts: {
+        "/deploy.sh": { policy: { "/home/*/secrets/**": "---" } },
+      },
+    };
+    const errs = validateAccessPolicyConfig(config);
+    expect(
+      errs.some(
+        (e) => e.includes("OS-level") && e.includes("cannot apply") && e.includes("/deploy.sh"),
+      ),
+    ).toBe(true);
+  });
+
   it("validates scripts[].policy perm strings and emits diagnostics for bad ones", () => {
     // A typo like "rwX" in a script's policy must produce a diagnostic, not silently
     // fail closed (which would deny exec with no operator-visible error).
@@ -1124,6 +1156,28 @@ describe("applyScriptPolicyOverride", () => {
     }
   });
 
+  it("uppercase sha256 in config matches (case-normalized at comparison)", () => {
+    // Validation regex uses /i so uppercase passes; crypto.digest("hex") returns lowercase.
+    // Without .toLowerCase() at comparison, uppercase sha256 always fails at runtime — silent
+    // misconfiguration that denies exec with no useful error.
+    const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "ap-test-"));
+    const scriptPath = path.join(tmpDir, "script.sh");
+    const content = "#!/bin/sh\necho hi\n";
+    fs.writeFileSync(scriptPath, content);
+    const hashLower = crypto.createHash("sha256").update(Buffer.from(content)).digest("hex");
+    const hashUpper = hashLower.toUpperCase();
+    const realScriptPath = fs.realpathSync(scriptPath);
+    try {
+      const base: AccessPolicyConfig = {
+        scripts: { [scriptPath]: { sha256: hashUpper, policy: { "/tmp/**": "rwx" } } },
+      };
+      const { hashMismatch } = applyScriptPolicyOverride(base, realScriptPath);
+      expect(hashMismatch).toBeUndefined();
+    } finally {
+      fs.rmSync(tmpDir, { recursive: true });
+    }
+  });
+
   it("merges scripts['policy'] into overrideRules when a script matches", () => {
     // scripts["policy"] is the shared base for all named script entries.
     // It must appear in overrideRules so the tool layer and OS sandbox enforce it.

src/infra/access-policy.ts

@@ -137,10 +137,16 @@ export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[]
           !_midPathWildcardWarned.has(`scripts:policy:${pattern}`)
         ) {
           _midPathWildcardWarned.add(`scripts:policy:${pattern}`);
+          if (perm === "---") {
+            errors.push(
+              `access-policy.scripts["policy"]["${pattern}"] contains a mid-path wildcard with "---" — OS-level (bwrap/Seatbelt) enforcement cannot apply; tool-layer enforcement is still active.`,
+            );
+          } else {
             errors.push(
               `access-policy.scripts["policy"]["${pattern}"] contains a mid-path wildcard — OS-level enforcement uses prefix match (file-type filter is tool-layer only).`,
             );
           }
+        }
         autoExpandBareDir(sharedPolicy, pattern, perm, errors);
       }
     }
@@ -179,10 +185,16 @@ export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[]
             !_midPathWildcardWarned.has(`scripts:${scriptPath}:${pattern}`)
           ) {
             _midPathWildcardWarned.add(`scripts:${scriptPath}:${pattern}`);
+            if (perm === "---") {
+              errors.push(
+                `access-policy.scripts["${scriptPath}"].policy["${pattern}"] contains a mid-path wildcard with "---" — OS-level (bwrap/Seatbelt) enforcement cannot apply; tool-layer enforcement is still active.`,
+              );
+            } else {
               errors.push(
                 `access-policy.scripts["${scriptPath}"].policy["${pattern}"] contains a mid-path wildcard — OS-level enforcement uses prefix match (file-type filter is tool-layer only).`,
               );
             }
+          }
           autoExpandBareDir(scriptEntry.policy, pattern, perm, errors);
         }
       }
@@ -648,7 +660,10 @@ export function applyScriptPolicyOverride(
     } catch {
       return { policy, hashMismatch: true };
     }
-    if (actualHash !== override.sha256) {
+    // Normalize to lowercase: crypto.digest("hex") always returns lowercase, but
+    // the validation regex accepts uppercase (/i). Without normalization an uppercase
+    // sha256 in config passes validation and then silently fails here at runtime.
+    if (actualHash !== override.sha256.toLowerCase()) {
       return { policy, hashMismatch: true };
     }
   }

src/infra/exec-sandbox-bwrap.test.ts

@@ -193,6 +193,24 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
     expect(tmpfsMounts).toContain(`${HOME}/scripts`);
   });
 
+  it('"---" rule on SYSTEM_RO_BIND_PATHS path emits --tmpfs in restrictive mode', () => {
+    // SYSTEM_RO_BIND_PATHS (/etc, /usr, /bin, /lib, /lib64, /sbin, /opt) are unconditionally
+    // --ro-bind-try mounted in restrictive mode. Without a --tmpfs overlay, a "---" rule on
+    // e.g. "/etc/**" has no OS-level effect — syscalls inside the sandbox can still read
+    // /etc/passwd, /etc/shadow, etc. The fix: treat deny rules the same in both modes.
+    const config: AccessPolicyConfig = {
+      policy: { "/etc/**": "---" },
+    };
+    const args = generateBwrapArgs(config, HOME);
+    const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
+    expect(tmpfsMounts).toContain("/etc");
+    // Must NOT emit a read mount for a deny rule.
+    const roBound = args
+      .map((a, i) => (a === "--ro-bind-try" ? args[i + 1] : null))
+      .filter(Boolean);
+    expect(roBound).not.toContain("/etc");
+  });
+
   it('"---" rules do not create --ro-bind-try mounts in restrictive mode', () => {
     // A rule with "---" permission should NOT produce any bwrap mount — the
     // restrictive base already denies by not mounting. Emitting --ro-bind-try
@@ -410,6 +428,41 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
     expect(bindOf(withSlash)).toContain("/tmp");
     expect(bindOf(withSlash)).toEqual(bindOf(withGlob));
   });
+
+  it("malformed perm string in base rules emits no mount (fail closed, not --ro-bind-try)", () => {
+    // A malformed perm like "rwxoops" must not produce a --ro-bind-try mount.
+    // Previously the else-if branch accessed perm[0] without VALID_PERM_RE guard,
+    // which could emit --ro-bind-try for a rule meant to be restrictive.
+    const config: AccessPolicyConfig = {
+      policy: {
+        [`${HOME}/workspace/**`]:
+          "rwxoops" as unknown as import("../config/types.tools.js").PermStr,
+      },
+    };
+    const args = generateBwrapArgs(config, HOME);
+    const roBound = args
+      .map((a, i) => (a === "--ro-bind-try" ? args[i + 1] : null))
+      .filter(Boolean);
+    const rwBound = args.map((a, i) => (a === "--bind-try" ? args[i + 1] : null)).filter(Boolean);
+    // Malformed perm must not produce any mount for this path.
+    expect(roBound).not.toContain(`${HOME}/workspace`);
+    expect(rwBound).not.toContain(`${HOME}/workspace`);
+  });
+
+  it("malformed perm string in script override emits no --ro-bind-try (fail closed)", () => {
+    // Same VALID_PERM_RE guard required in the scriptOverrideRules loop.
+    const config: AccessPolicyConfig = { policy: { "/**": "r--" } };
+    const overrides = {
+      [`${HOME}/data/**`]: "rwxoops" as unknown as import("../config/types.tools.js").PermStr,
+    };
+    const args = generateBwrapArgs(config, HOME, overrides);
+    const roBound = args
+      .map((a, i) => (a === "--ro-bind-try" ? args[i + 1] : null))
+      .filter(Boolean);
+    const rwBound = args.map((a, i) => (a === "--bind-try" ? args[i + 1] : null)).filter(Boolean);
+    expect(roBound).not.toContain(`${HOME}/data`);
+    expect(rwBound).not.toContain(`${HOME}/data`);
+  });
 });
 
 describe("wrapCommandWithBwrap", () => {

src/infra/exec-sandbox-bwrap.ts

@@ -215,16 +215,22 @@ export function generateBwrapArgs(
       // a restrictive base will also permit reads at the OS layer. The tool layer still
       // denies read tool calls per the rule, so the practical exposure is exec-only paths.
       args.push("--bind-try", p, p);
-    } else if (catchAllPerm[0] !== "r" && perm[0] === "r") {
+    } else if (VALID_PERM_RE.test(perm) && catchAllPerm[0] !== "r" && perm[0] === "r") {
       // Restrictive base: only bind paths that the rule explicitly allows reads on.
       // Do NOT emit --ro-bind-try for "---" or "--x" rules — the base already denies
       // by not mounting; emitting a mount here would grant read access.
+      // VALID_PERM_RE guard: malformed perm falls through to no-op (fail closed).
       args.push("--ro-bind-try", p, p);
-    } else if (catchAllPerm[0] === "r" && perm[0] !== "r") {
+    } else if (VALID_PERM_RE.test(perm) && perm[0] !== "r") {
-      // Permissive base + narrowing rule (no read bit): overlay with tmpfs so the
+      // Deny/exec-only rule: overlay with --tmpfs to hide the path.
-      // path is hidden even though --ro-bind / / made it readable by default.
+      // Two cases handled identically:
-      // This mirrors what deny[] does — without this, "---" rules under a permissive
+      //   Permissive base (catchAllPerm[0] === "r"): --ro-bind / / made path readable;
-      // default are silently ignored at the bwrap layer.
+      //     --tmpfs hides it.
+      //   Restrictive base (catchAllPerm[0] !== "r"): SYSTEM_RO_BIND_PATHS unconditionally
+      //     mounts /etc, /usr, /bin, /lib, /lib64, /sbin, /opt; a "---" rule on those paths
+      //     had no effect without this branch because the three prior branches all require
+      //     perm[0] === "r". For non-system paths in restrictive mode, --tmpfs is a no-op
+      //     (nothing mounted there to overlay), so emitting it is harmless.
       // Guard: bwrap --tmpfs only accepts a directory as the mount point. If the
       // resolved path is a file, skip the mount and warn — same behaviour as deny[].
       // Non-existent paths are assumed to be directories (forward-protection).
@@ -241,6 +247,7 @@ export function generateBwrapArgs(
       }
     }
     // Permissive base + read-only rule: already covered by --ro-bind / /; no extra mount.
+    // Restrictive base + read-only rule: emitted as --ro-bind-try above.
   }
 
   // Script-specific override mounts — emitted after base rules so they can reopen
@@ -261,7 +268,8 @@ export function generateBwrapArgs(
         // and writes succeed. bwrap mounts are ordered; this override comes after
         // deny[] tmpfs entries, so --bind-try wins regardless of the base policy.
         args.push("--bind-try", p, p);
-      } else if (perm[0] === "r") {
+      } else if (VALID_PERM_RE.test(perm) && perm[0] === "r") {
+        // VALID_PERM_RE guard: malformed perm falls through to the deny branch below.
         args.push("--ro-bind-try", p, p);
       } else {
         // Mirror the base-rules isDir guard — bwrap --tmpfs only accepts directories.

src/infra/exec-sandbox-seatbelt.test.ts

@@ -226,6 +226,20 @@ describe("generateSeatbeltProfile", () => {
     expect(profile).not.toContain("**");
     expect(profile).toContain("/Users/kaveri/.ssh");
   });
+
+  it("bracket glob patterns are treated as wildcards, not SBPL literals", () => {
+    // Previously /[*?]/ missed [ — a pattern like "/usr/bin/[abc]" was emitted as
+    // sbplLiteral("/usr/bin/[abc]") which only matches a file literally named "[abc]".
+    // Fix: /[*?[]/ detects bracket globs and strips to the concrete prefix.
+    const config: AccessPolicyConfig = {
+      policy: { "/**": "rwx", "/usr/bin/[abc]": "---" as const },
+    };
+    const profile = generateSeatbeltProfile(config, HOME);
+    // Must NOT emit the literal bracket pattern.
+    expect(profile).not.toContain("[abc]");
+    // Must use the concrete prefix /usr/bin as an approximate subpath target.
+    expect(profile).toContain("/usr/bin");
+  });
 });
 
 describe("wrapCommandWithSeatbelt", () => {

src/infra/exec-sandbox-seatbelt.ts

@@ -108,12 +108,15 @@ function patternToSbplMatcher(pattern: string, homeDir: string, perm?: PermStr):
   // Both * and ? are wildcard characters in glob syntax; strip from whichever
   // appears first so patterns like "/tmp/file?.txt" don't embed a literal ?
   // in the SBPL literal matcher.
-  const withoutWild = expanded.replace(/[/\\]?[*?].*$/, "");
+  // Strip from the first glob metacharacter (*, ?, or [) to get the longest concrete prefix.
+  const withoutWild = expanded.replace(/[/\\]?[*?[].*$/, "");
   const base = withoutWild || "/";
 
   // If the original pattern had wildcards, use subpath (recursive match).
-  // Otherwise use literal (exact match).
+  // Includes bracket globs ([abc]) — previously only * and ? were detected,
-  if (/[*?]/.test(expanded)) {
+  // causing [abc] to be emitted as an SBPL literal that only matches a file
+  // literally named "[abc]", not the intended character-class targets.
+  if (/[*?[]/.test(expanded)) {
     const wildcardIdx = expanded.search(/[*?[]/);
     const afterWildcard = expanded.slice(wildcardIdx + 1);
     if (/[/\\]/.test(afterWildcard)) {