fix(access-policy): merge scripts[policy], isDir guard in bwrap overrides, permAllows format check, script entry shape check

src/infra/access-policy.test.ts

@@ -243,6 +243,18 @@ describe("checkAccessPolicy — malformed permission characters fail closed", ()
     const config = { policy: { "/tmp/**": "rWx" as unknown as "rwx" } };
     expect(checkAccessPolicy("/tmp/foo.txt", "write", config)).toBe("deny");
   });
+
+  it("treats 'aw-' (invalid first char) as deny for write even though index 1 is 'w'", () => {
+    // defense-in-depth: full format must be [r-][w-][x-]; 'a' at index 0 fails the regex
+    // so the entire string is rejected rather than accidentally granting write.
+    const config = { policy: { "/tmp/**": "aw-" as unknown as "r--" } };
+    expect(checkAccessPolicy("/tmp/foo.txt", "write", config)).toBe("deny");
+  });
+
+  it("treats a 4-char string as deny (wrong length)", () => {
+    const config = { policy: { "/tmp/**": "rwx!" as unknown as "rwx" } };
+    expect(checkAccessPolicy("/tmp/foo.txt", "exec", config)).toBe("deny");
+  });
 });
 
 // ---------------------------------------------------------------------------
@@ -1029,4 +1041,62 @@ describe("applyScriptPolicyOverride", () => {
       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.
+    const base: AccessPolicyConfig = {
+      policy: { "/**": "r--" },
+      scripts: {
+        policy: { [`${HOME}/.secrets/token`]: "r--" },
+        "/my/script.sh": { policy: { "/tmp/**": "rwx" } },
+      },
+    };
+    const { overrideRules } = applyScriptPolicyOverride(base, "/my/script.sh");
+    expect(overrideRules?.[`${HOME}/.secrets/token`]).toBe("r--");
+    expect(overrideRules?.["/tmp/**"]).toBe("rwx");
+  });
+
+  it("per-script policy wins over scripts['policy'] on conflict", () => {
+    const base: AccessPolicyConfig = {
+      policy: { "/**": "r--" },
+      scripts: {
+        policy: { "/tmp/**": "r--" },
+        "/my/script.sh": { policy: { "/tmp/**": "rwx" } },
+      },
+    };
+    const { overrideRules } = applyScriptPolicyOverride(base, "/my/script.sh");
+    expect(overrideRules?.["/tmp/**"]).toBe("rwx");
+  });
+
+  it("includes scripts['policy'] even when per-script entry has no policy key", () => {
+    // A script entry with only sha256 and no policy still gets scripts["policy"] applied.
+    const base: AccessPolicyConfig = {
+      policy: { "/**": "r--" },
+      scripts: {
+        policy: { [`${HOME}/.secrets/token`]: "r--" },
+        "/my/script.sh": {},
+      },
+    };
+    const { overrideRules } = applyScriptPolicyOverride(base, "/my/script.sh");
+    expect(overrideRules?.[`${HOME}/.secrets/token`]).toBe("r--");
+  });
+
+  it("returns base policy unchanged when script entry is a non-object truthy value", () => {
+    // A malformed entry like `true` or `"oops"` must not be treated as a valid override.
+    // Without the shape check, a truthy primitive would skip sha256 and mark hasScriptOverride=true.
+    const base: AccessPolicyConfig = {
+      policy: { "/**": "r--" },
+      scripts: {
+        "/my/script.sh": true as unknown as import("../config/types.tools.js").ScriptPolicyEntry,
+      },
+    };
+    const { policy, overrideRules, hashMismatch } = applyScriptPolicyOverride(
+      base,
+      "/my/script.sh",
+    );
+    expect(overrideRules).toBeUndefined();
+    expect(hashMismatch).toBeUndefined();
+    expect(policy).toBe(base);
+  });
 });

src/infra/access-policy.ts

@@ -220,6 +220,11 @@ const OP_GRANT_CHAR: Record<FsOp, string> = {
   exec: "x",
 };
 
+// Valid perm strings are exactly 3 chars: [r-][w-][x-].
+// Validated at parse time by validateAccessPolicyConfig, but also checked here
+// as defense-in-depth so a malformed value never accidentally grants access.
+const VALID_PERM_RE = /^[r-][w-][x-]$/;
+
 /**
  * Returns true if the given permission string grants the requested operation.
  * An absent or malformed string is treated as "---" (deny all).
@@ -227,7 +232,7 @@ const OP_GRANT_CHAR: Record<FsOp, string> = {
  * including typos fails closed rather than accidentally granting access.
  */
 function permAllows(perm: PermStr | undefined, op: FsOp): boolean {
-  if (!perm) {
+  if (!perm || !VALID_PERM_RE.test(perm)) {
     return false;
   }
   return perm[OP_INDEX[op]] === OP_GRANT_CHAR[op];
@@ -567,12 +572,18 @@ export function applyScriptPolicyOverride(
   // resolveArgv0 always returns the realpathSync result, so both forms must be
   // normalized the same way or the lookup silently misses, skipping sha256 verification.
   const scripts = policy.scripts;
-  const override = scripts
-    ? (Object.entries(scripts).find(
+  const rawOverride = scripts
+    ? Object.entries(scripts).find(
         ([k]) =>
           k !== "policy" && path.normalize(resolveScriptKey(k)) === path.normalize(resolvedArgv0),
-      )?.[1] as import("../config/types.tools.js").ScriptPolicyEntry | undefined)
+      )?.[1]
     : undefined;
+  // Reject non-object entries (e.g. true, "oops") — a truthy primitive would
+  // otherwise skip sha256 verification and act as an unchecked override grant.
+  const override =
+    rawOverride != null && typeof rawOverride === "object" && !Array.isArray(rawOverride)
+      ? (rawOverride as import("../config/types.tools.js").ScriptPolicyEntry)
+      : undefined;
   if (!override) {
     return { policy };
   }
@@ -610,9 +621,16 @@ export function applyScriptPolicyOverride(
   // a broadly denied subtree).
   const { scripts: _scripts, ...base } = policy;
   const merged: AccessPolicyConfig = { ...base };
+
+  // Merge scripts["policy"] (shared base for all matching scripts) with the
+  // per-script entry policy. Per-script wins on conflict (applied last).
+  const sharedPolicy = scripts?.["policy"];
+  const mergedOverride: Record<string, PermStr> = {
+    ...sharedPolicy,
+    ...override.policy,
+  };
   return {
     policy: merged,
-    overrideRules:
-      override.policy && Object.keys(override.policy).length > 0 ? override.policy : undefined,
+    overrideRules: Object.keys(mergedOverride).length > 0 ? mergedOverride : undefined,
   };
 }

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

@@ -325,6 +325,32 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
     expect(bindArgs[secretIdx]).toBe("--ro-bind-try");
   });
 
+  it('script override "---" rule targeting a file does not emit --tmpfs (bwrap rejects file paths)', () => {
+    // The base-rules loop has an isDir guard before --tmpfs; the scriptOverrideRules loop must too.
+    // /etc/hosts is a real file; emitting --tmpfs /etc/hosts would make bwrap fail at runtime.
+    const config: AccessPolicyConfig = { policy: { "/**": "r--" } };
+    const overrides = { "/etc/hosts": "---" as const };
+    const spy = vi.spyOn(console, "error").mockImplementation(() => {});
+    try {
+      const args = generateBwrapArgs(config, HOME, overrides);
+      const tmpfsMounts = args
+        .map((a, i) => (a === "--tmpfs" ? args[i + 1] : null))
+        .filter(Boolean);
+      expect(tmpfsMounts).not.toContain("/etc/hosts");
+      expect(spy).toHaveBeenCalledWith(expect.stringContaining("/etc/hosts"));
+    } finally {
+      spy.mockRestore();
+    }
+  });
+
+  it('script override "---" rule targeting a non-existent path emits --tmpfs (assumed directory)', () => {
+    const config: AccessPolicyConfig = { policy: { "/**": "r--" } };
+    const overrides = { "/nonexistent-path-for-test/**": "---" as const };
+    const args = generateBwrapArgs(config, HOME, overrides);
+    const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
+    expect(tmpfsMounts).toContain("/nonexistent-path-for-test");
+  });
+
   it('script override "-w-" under restrictive base emits --bind-try, not --tmpfs', () => {
     // Greptile: permAllowsWrite && (r || defaultR) condition was wrong — for -w- without /**
     // both flags are false so it fell to else → --tmpfs, silently blocking writes.

src/infra/exec-sandbox-bwrap.ts

@@ -255,7 +255,18 @@ export function generateBwrapArgs(
       } else if (perm[0] === "r") {
         args.push("--ro-bind-try", p, p);
       } else {
-        args.push("--tmpfs", p);
+        // Mirror the base-rules isDir guard — bwrap --tmpfs only accepts directories.
+        let isDir = true;
+        try {
+          isDir = fs.statSync(p).isDirectory();
+        } catch {
+          // Non-existent — assume directory (forward-protection).
+        }
+        if (isDir) {
+          args.push("--tmpfs", p);
+        } else {
+          _warnBwrapFileDenyOnce(p);
+        }
       }
     }
   }