fix(access-policy): bwrap file-path tmpfs guard, seatbelt /tmp exec bit, mtime cache

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

@@ -4,6 +4,7 @@ import path from "node:path";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import {
   BROKEN_POLICY_FILE,
+  _resetFileCacheForTest,
   _resetNotFoundWarnedForTest,
   loadAccessPolicyFile,
   mergeAccessPolicy,
@@ -29,6 +30,7 @@ const FP_DIR = path.dirname(FP_FILE);
 beforeEach(() => {
   fs.mkdirSync(FP_DIR, { recursive: true });
   _resetNotFoundWarnedForTest();
+  _resetFileCacheForTest();
 });
 
 afterEach(() => {
@@ -254,6 +256,57 @@ describe("loadAccessPolicyFile", () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// loadAccessPolicyFile — mtime cache
+// ---------------------------------------------------------------------------
+
+describe("loadAccessPolicyFile — mtime cache", () => {
+  it("returns cached result on second call without re-reading the file", () => {
+    writeFile({ version: 1, base: { default: "r--" } });
+    const spy = vi.spyOn(fs, "readFileSync");
+    loadAccessPolicyFile(); // populate cache
+    loadAccessPolicyFile(); // should hit cache
+    // readFileSync should only be called once despite two loadAccessPolicyFile calls.
+    expect(spy.mock.calls.filter((c) => String(c[0]).includes("access-policy")).length).toBe(1);
+    spy.mockRestore();
+  });
+
+  it("re-reads when mtime changes (file updated)", () => {
+    writeFile({ version: 1, base: { default: "r--" } });
+    loadAccessPolicyFile(); // populate cache
+    // Rewrite the file — on most filesystems this bumps mtime. Force a detectable
+    // mtime change by setting it explicitly via utimesSync.
+    writeFile({ version: 1, base: { default: "rwx" } });
+    const future = Date.now() / 1000 + 1;
+    fs.utimesSync(FP_FILE, future, future);
+    const result = loadAccessPolicyFile();
+    expect(result).not.toBe(BROKEN_POLICY_FILE);
+    if (result === null || result === BROKEN_POLICY_FILE) {
+      throw new Error("unexpected");
+    }
+    expect(result.base?.default).toBe("rwx");
+  });
+
+  it("clears cache when file is deleted", () => {
+    writeFile({ version: 1, base: { default: "r--" } });
+    loadAccessPolicyFile(); // populate cache
+    fs.unlinkSync(FP_FILE);
+    expect(loadAccessPolicyFile()).toBeNull();
+  });
+
+  it("caches BROKEN_POLICY_FILE result for broken files", () => {
+    const spy = vi.spyOn(console, "error").mockImplementation(() => {});
+    fs.writeFileSync(FP_FILE, "not json {{ broken");
+    loadAccessPolicyFile(); // populate cache with BROKEN
+    const spy2 = vi.spyOn(fs, "readFileSync");
+    const result = loadAccessPolicyFile(); // should hit cache
+    expect(result).toBe(BROKEN_POLICY_FILE);
+    expect(spy2.mock.calls.filter((c) => String(c[0]).includes("access-policy")).length).toBe(0);
+    spy.mockRestore();
+    spy2.mockRestore();
+  });
+});
+
 // ---------------------------------------------------------------------------
 // resolveAccessPolicyForAgent
 // ---------------------------------------------------------------------------

src/infra/access-policy-file.ts

@@ -138,19 +138,57 @@ function validateAccessPolicyFileStructure(filePath: string, parsed: unknown): s
  */
 export const BROKEN_POLICY_FILE = Symbol("broken-policy-file");
 
+// In-process mtime cache — avoids re-parsing the file on every agent turn.
+// Cache is keyed by mtime so any write to the file automatically invalidates it.
+type _FileCacheEntry = {
+  mtimeMs: number;
+  result: AccessPolicyFile | typeof BROKEN_POLICY_FILE;
+};
+let _fileCache: _FileCacheEntry | undefined;
+
+/** Reset the in-process file cache. Only for use in tests. */
+export function _resetFileCacheForTest(): void {
+  _fileCache = undefined;
+}
+
 /**
  * Read and parse the sidecar file.
  * - Returns null if the file does not exist (opt-in not configured).
  * - Returns BROKEN_POLICY_FILE if the file exists but is malformed/unreadable
  *   (callers must treat this as default:"---" — fail-closed).
  * - Returns the parsed file on success.
+ *
+ * Result is cached in-process by mtime — re-parses only when the file changes.
  */
 export function loadAccessPolicyFile(): AccessPolicyFile | null | typeof BROKEN_POLICY_FILE {
   const filePath = resolveAccessPolicyPath();
-  if (!fs.existsSync(filePath)) {
+
+  // Single statSync per call: cheap enough and detects file changes.
+  let mtimeMs: number;
+  try {
+    mtimeMs = fs.statSync(filePath).mtimeMs;
+  } catch {
+    // File does not exist — clear cache and return null (feature is opt-in).
+    _fileCache = undefined;
     return null;
   }
 
+  // Cache hit: same mtime means same content — skip readFileSync + JSON.parse.
+  if (_fileCache && _fileCache.mtimeMs === mtimeMs) {
+    return _fileCache.result;
+  }
+
+  // Cache miss: parse fresh and store result (including BROKEN_POLICY_FILE).
+  const result = _parseAccessPolicyFile(filePath);
+  _fileCache = { mtimeMs, result: result ?? BROKEN_POLICY_FILE };
+  // _parseAccessPolicyFile returns null only for the non-existent case, which we
+  // handle above via statSync. If it somehow returns null here treat as broken.
+  return result ?? BROKEN_POLICY_FILE;
+}
+
+function _parseAccessPolicyFile(
+  filePath: string,
+): AccessPolicyFile | typeof BROKEN_POLICY_FILE | null {
   let parsed: unknown;
   try {
     const raw = fs.readFileSync(filePath, "utf8");

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

@@ -177,6 +177,23 @@ describe.skipIf(process.platform !== "linux")("generateBwrapArgs", () => {
     expect(bindMounts).not.toContain(`${HOME}/secret`);
   });
 
+  it("narrowing rule on an existing file does not emit --tmpfs (bwrap only accepts dirs)", () => {
+    // Reproducer: { "default": "r--", "rules": { "~/secrets.key": "---" } }
+    // where ~/secrets.key is an existing file. The old code emitted --tmpfs on the
+    // file path, causing bwrap to abort with "Not a directory". Fix: mirror the
+    // isDir guard already present in the deny[] branch.
+    // process.execPath is always an existing file — use it as the test target.
+    const filePath = process.execPath;
+    const config: AccessPolicyConfig = {
+      default: "r--",
+      rules: { [filePath]: "---" },
+    };
+    const args = generateBwrapArgs(config, HOME);
+    const tmpfsMounts = args.map((a, i) => (a === "--tmpfs" ? args[i + 1] : null)).filter(Boolean);
+    // Must NOT emit --tmpfs for a file path.
+    expect(tmpfsMounts).not.toContain(filePath);
+  });
+
   it('"--x" rule in permissive mode gets --tmpfs overlay to block reads', () => {
     // Execute-only rules have no read bit — same treatment as "---" in permissive mode.
     const config: AccessPolicyConfig = {

src/infra/exec-sandbox-bwrap.ts

@@ -205,7 +205,20 @@ export function generateBwrapArgs(
       // path is hidden even though --ro-bind / / made it readable by default.
       // This mirrors what deny[] does — without this, "---" rules under a permissive
       // default are silently ignored at the bwrap layer.
-      args.push("--tmpfs", p);
+      // 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).
+      let isDir = true;
+      try {
+        isDir = fs.statSync(p).isDirectory();
+      } catch {
+        // Non-existent — assume directory.
+      }
+      if (isDir) {
+        args.push("--tmpfs", p);
+      } else {
+        _warnBwrapFileDenyOnce(p);
+      }
     }
     // Permissive base + read-only rule: already covered by --ro-bind / /; no extra mount.
   }

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

@@ -183,6 +183,33 @@ describe("generateSeatbeltProfile", () => {
     expect(profile).not.toMatch(/allow file-read[^)]*\(subpath "\/private\/tmp"\)/);
   });
 
+  it("exec-only /tmp rule grants process-exec* on /private/tmp in restrictive mode", () => {
+    // Regression: when default:"---" and a rule grants exec on /tmp (e.g. "--x"),
+    // the seatbelt profile must emit (allow process-exec* (subpath "/private/tmp")).
+    // Without this fix, no exec allowance was emitted and binaries in /tmp could not
+    // be executed even though the policy explicitly permitted it.
+    const config: AccessPolicyConfig = {
+      default: "---",
+      rules: { "/tmp/**": "--x" },
+    };
+    const profile = generateSeatbeltProfile(config, HOME);
+    expect(profile).toMatch(/allow process-exec\*[^)]*\(subpath "\/private\/tmp"\)/);
+    // Read and write must NOT be granted.
+    expect(profile).not.toMatch(/allow file-read[^)]*\(subpath "\/private\/tmp"\)/);
+    expect(profile).not.toMatch(/allow file-write\*[^)]*\(subpath "\/private\/tmp"\)/);
+  });
+
+  it("r-x /tmp rule grants both read and exec on /private/tmp in restrictive mode", () => {
+    const config: AccessPolicyConfig = {
+      default: "---",
+      rules: { "/tmp/**": "r-x" },
+    };
+    const profile = generateSeatbeltProfile(config, HOME);
+    expect(profile).toMatch(/allow file-read[^)]*\(subpath "\/private\/tmp"\)/);
+    expect(profile).toMatch(/allow process-exec\*[^)]*\(subpath "\/private\/tmp"\)/);
+    expect(profile).not.toMatch(/allow file-write\*[^)]*\(subpath "\/private\/tmp"\)/);
+  });
+
   // ---------------------------------------------------------------------------
   // Symlink attack mitigation — profile ordering
   //

src/infra/exec-sandbox-seatbelt.ts

@@ -218,6 +218,9 @@ export function generateSeatbeltProfile(
     if (tmpPerm[1] === "w") {
       lines.push(`(allow file-write* (subpath "/private/tmp"))`);
     }
+    if (tmpPerm[2] === "x") {
+      lines.push(`(allow ${SEATBELT_EXEC_OPS} (subpath "/private/tmp"))`);
+    }
     lines.push(`(allow process-fork)`);
     lines.push(`(allow signal)`);
     lines.push(`(allow mach*)`);