fix(access-policy): scripts deep-merge, mid-path wildcard diagnostic, test reset exports

src/agents/bash-tools.exec-runtime.test.ts

@@ -10,7 +10,11 @@ vi.mock("../infra/system-events.js", () => ({
 
 import { requestHeartbeatNow } from "../infra/heartbeat-wake.js";
 import { enqueueSystemEvent } from "../infra/system-events.js";
-import { emitExecSystemEvent } from "./bash-tools.exec-runtime.js";
+import {
+  _resetBwrapUnavailableWarnedForTest,
+  _resetWindowsUnconfiguredWarnedForTest,
+  emitExecSystemEvent,
+} from "./bash-tools.exec-runtime.js";
 
 const requestHeartbeatNowMock = vi.mocked(requestHeartbeatNow);
 const enqueueSystemEventMock = vi.mocked(enqueueSystemEvent);
@@ -62,3 +66,21 @@ describe("emitExecSystemEvent", () => {
     expect(requestHeartbeatNowMock).not.toHaveBeenCalled();
   });
 });
+
+// ---------------------------------------------------------------------------
+// One-time warning reset helpers (exported for tests)
+// ---------------------------------------------------------------------------
+
+describe("_resetBwrapUnavailableWarnedForTest / _resetWindowsUnconfiguredWarnedForTest", () => {
+  it("exports _resetBwrapUnavailableWarnedForTest as a function", () => {
+    // Verify the export exists and is callable — the reset enables repeated
+    // warning tests without cross-test state leakage.
+    expect(typeof _resetBwrapUnavailableWarnedForTest).toBe("function");
+    expect(() => _resetBwrapUnavailableWarnedForTest()).not.toThrow();
+  });
+
+  it("exports _resetWindowsUnconfiguredWarnedForTest as a function", () => {
+    expect(typeof _resetWindowsUnconfiguredWarnedForTest).toBe("function");
+    expect(() => _resetWindowsUnconfiguredWarnedForTest()).not.toThrow();
+  });
+});

src/agents/bash-tools.exec-runtime.ts

@@ -311,6 +311,11 @@ function _warnBwrapUnavailableOnce(): void {
   );
 }
 
+/** Reset the one-time bwrap-unavailable warning flag. Only for use in tests. */
+export function _resetBwrapUnavailableWarnedForTest(): void {
+  _bwrapUnavailableWarned = false;
+}
+
 let _windowsUnconfiguredWarned = false;
 function _warnWindowsUnconfiguredOnce(): void {
   if (_windowsUnconfiguredWarned) {
@@ -322,6 +327,11 @@ function _warnWindowsUnconfiguredOnce(): void {
   );
 }
 
+/** Reset the one-time Windows-unconfigured warning flag. Only for use in tests. */
+export function _resetWindowsUnconfiguredWarnedForTest(): void {
+  _windowsUnconfiguredWarned = false;
+}
+
 export async function runExecProcess(opts: {
   command: string;
   // Execute this instead of `command` (which is kept for display/session/logging).

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

@@ -102,6 +102,61 @@ describe("mergeAccessPolicy", () => {
     expect(result?.deny).toBeUndefined();
     expect(result?.rules).toBeUndefined();
   });
+
+  it("scripts deep-merge: base sha256 is preserved when override supplies same script key", () => {
+    // Security regression: a shallow spread ({ ...base.scripts, ...override.scripts }) would
+    // silently drop the admin-configured sha256 hash check, defeating integrity enforcement.
+    const base = {
+      scripts: {
+        "/usr/local/bin/deploy.sh": {
+          sha256: "abc123",
+          rules: { "~/deploy/**": "rwx" as const },
+          deny: ["~/.ssh/**"],
+        },
+      },
+    };
+    const override = {
+      scripts: {
+        "/usr/local/bin/deploy.sh": {
+          // Agent block supplies same key — must NOT be able to drop sha256 or deny[].
+          rules: { "~/deploy/**": "r--" as const }, // narrower override — fine
+          deny: ["~/extra-deny/**"],
+        },
+      },
+    };
+    const result = mergeAccessPolicy(base, override);
+    const merged = result?.scripts?.["/usr/local/bin/deploy.sh"];
+    // sha256 from base must survive.
+    expect(merged?.sha256).toBe("abc123");
+    // deny[] must be additive — base deny cannot be removed.
+    expect(merged?.deny).toContain("~/.ssh/**");
+    expect(merged?.deny).toContain("~/extra-deny/**");
+    // rules: override key wins on collision.
+    expect(merged?.rules?.["~/deploy/**"]).toBe("r--");
+  });
+
+  it("scripts deep-merge: override-only script key is added verbatim", () => {
+    const base = { scripts: { "/bin/existing.sh": { sha256: "deadbeef" } } };
+    const override = {
+      scripts: { "/bin/new.sh": { rules: { "/tmp/**": "rwx" as const } } },
+    };
+    const result = mergeAccessPolicy(base, override);
+    // Base script untouched.
+    expect(result?.scripts?.["/bin/existing.sh"]?.sha256).toBe("deadbeef");
+    // New script from override is added.
+    expect(result?.scripts?.["/bin/new.sh"]?.rules?.["/tmp/**"]).toBe("rwx");
+  });
+
+  it("scripts deep-merge: base deny[] cannot be removed by override supplying empty deny[]", () => {
+    const base = {
+      scripts: { "/bin/s.sh": { deny: ["~/.secrets/**"] } },
+    };
+    const override = {
+      scripts: { "/bin/s.sh": { deny: [] } }, // empty override deny — base must survive
+    };
+    const result = mergeAccessPolicy(base, override);
+    expect(result?.scripts?.["/bin/s.sh"]?.deny).toContain("~/.secrets/**");
+  });
 });
 
 // ---------------------------------------------------------------------------

src/infra/access-policy-file.ts

@@ -50,8 +50,30 @@ export function mergeAccessPolicy(
   }
   const deny = [...(base.deny ?? []), ...(override.deny ?? [])];
   const rules = { ...base.rules, ...override.rules };
-  // scripts: shallow merge — override key wins (same semantics as rules)
-  const scripts = { ...base.scripts, ...override.scripts };
+  // scripts: deep-merge per key — base sha256 and deny[] are preserved regardless of
+  // what the agent override supplies. A plain spread ({ ...base.scripts, ...override.scripts })
+  // would silently drop the admin-configured hash integrity check and per-script deny list
+  // when an agent block supplies the same script key, defeating the security intent.
+  const mergedScripts: NonNullable<AccessPolicyConfig["scripts"]> = { ...base.scripts };
+  for (const [key, overrideEntry] of Object.entries(override.scripts ?? {})) {
+    const baseEntry = base.scripts?.[key];
+    if (!baseEntry) {
+      mergedScripts[key] = overrideEntry;
+      continue;
+    }
+    const entryDeny = [...(baseEntry.deny ?? []), ...(overrideEntry.deny ?? [])];
+    mergedScripts[key] = {
+      // sha256: base always wins — cannot be removed or replaced by an agent override.
+      ...(baseEntry.sha256 !== undefined ? { sha256: baseEntry.sha256 } : {}),
+      // rules: shallow-merge, override key wins on collision.
+      ...(Object.keys({ ...baseEntry.rules, ...overrideEntry.rules }).length > 0
+        ? { rules: { ...baseEntry.rules, ...overrideEntry.rules } }
+        : {}),
+      // deny: additive — base per-script deny cannot be removed.
+      ...(entryDeny.length > 0 ? { deny: entryDeny } : {}),
+    };
+  }
+  const scripts = Object.keys(mergedScripts).length > 0 ? mergedScripts : undefined;
   const result: AccessPolicyConfig = {};
   if (deny.length > 0) {
     result.deny = deny;
@@ -59,7 +81,7 @@ export function mergeAccessPolicy(
   if (Object.keys(rules).length > 0) {
     result.rules = rules;
   }
-  if (Object.keys(scripts).length > 0) {
+  if (scripts) {
     result.scripts = scripts;
   }
   if (override.default !== undefined) {
@@ -228,7 +250,7 @@ export function resolveAccessPolicyForAgent(agentId?: string): AccessPolicyConfi
       // Only print the footer when there are real permission-string errors —
       // auto-expand diagnostics ("rule auto-expanded to ...") are informational
       // and the footer would mislead operators into thinking the policy is broken.
-      if (errors.some((e) => !e.includes("auto-expanded"))) {
+      if (errors.some((e) => !e.includes("auto-expanded") && !e.includes("mid-path wildcard"))) {
         console.error(`[access-policy] Bad permission strings are treated as "---" (deny all).`);
       }
     }

src/infra/access-policy.test.ts

@@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it } from "vitest";
 import type { AccessPolicyConfig } from "../config/types.tools.js";
 import {
   _resetAutoExpandedWarnedForTest,
+  _resetMidPathWildcardWarnedForTest,
   applyScriptPolicyOverride,
   checkAccessPolicy,
   findBestRule,
@@ -26,6 +27,7 @@ const HOME = os.homedir();
 describe("validateAccessPolicyConfig", () => {
   beforeEach(() => {
     _resetAutoExpandedWarnedForTest();
+    _resetMidPathWildcardWarnedForTest();
   });
 
   it("returns no errors for a valid config", () => {
@@ -255,6 +257,48 @@ describe("validateAccessPolicyConfig", () => {
       }),
     ).toEqual([]);
   });
+
+  it("emits a one-time diagnostic for mid-path wildcard rules (OS-level enforcement skipped)", () => {
+    _resetMidPathWildcardWarnedForTest();
+    // "/home/*/secrets/**" has a wildcard in a non-final segment — bwrap and
+    // Seatbelt cannot derive a concrete mount path so they skip it silently.
+    // validateAccessPolicyConfig must surface this so operators know.
+    const errs = validateAccessPolicyConfig({
+      rules: { "/home/*/secrets/**": "---" },
+    });
+    expect(errs).toHaveLength(1);
+    expect(errs[0]).toMatch(/mid-path wildcard/);
+    expect(errs[0]).toMatch(/OS-level.*enforcement/);
+  });
+
+  it("deduplicates mid-path wildcard rule diagnostics across calls", () => {
+    _resetMidPathWildcardWarnedForTest();
+    const config = { rules: { "/home/*/secrets/**": "---" } };
+    const first = validateAccessPolicyConfig(config);
+    const second = validateAccessPolicyConfig(config);
+    expect(first.filter((e) => e.includes("mid-path wildcard"))).toHaveLength(1);
+    expect(second.filter((e) => e.includes("mid-path wildcard"))).toHaveLength(0);
+  });
+
+  it("emits a one-time diagnostic for mid-path wildcard deny[] entries", () => {
+    _resetMidPathWildcardWarnedForTest();
+    const errs = validateAccessPolicyConfig({
+      deny: ["/tmp/*/sensitive/**"],
+    });
+    expect(errs).toHaveLength(1);
+    expect(errs[0]).toMatch(/mid-path wildcard/);
+    expect(errs[0]).toMatch(/OS-level.*enforcement/);
+  });
+
+  it("does NOT emit mid-path wildcard diagnostic for final-segment wildcards", () => {
+    _resetMidPathWildcardWarnedForTest();
+    // "/home/user/**" — wildcard is in the final segment, no path separator follows.
+    const errs = validateAccessPolicyConfig({
+      rules: { "/home/user/**": "r--", "~/**": "rwx" },
+      deny: ["/tmp/**"],
+    });
+    expect(errs.filter((e) => e.includes("mid-path wildcard"))).toHaveLength(0);
+  });
 });
 
 // ---------------------------------------------------------------------------

src/infra/access-policy.ts

@@ -18,6 +18,28 @@ export function _resetAutoExpandedWarnedForTest(): void {
   _autoExpandedWarned.clear();
 }
 
+// Track mid-path wildcard patterns already warned about — one diagnostic per pattern.
+const _midPathWildcardWarned = new Set<string>();
+
+/** Reset the mid-path wildcard warning set. Only for use in tests. */
+export function _resetMidPathWildcardWarnedForTest(): void {
+  _midPathWildcardWarned.clear();
+}
+
+/**
+ * Returns true when a glob pattern has a wildcard character (*, ?, or bracket)
+ * in a non-final path segment (e.g. "/home/*\/secrets/**").
+ * bwrap and Seatbelt both skip such patterns at the OS layer because the
+ * concrete mount/deny path cannot be derived — only the tool layer enforces them.
+ */
+function hasMidPathWildcard(pattern: string): boolean {
+  const wildcardIdx = pattern.search(/[*?[]/);
+  if (wildcardIdx === -1) {
+    return false;
+  }
+  return /[/\\]/.test(pattern.slice(wildcardIdx));
+}
+
 /**
  * Validates and normalizes an AccessPolicyConfig for well-formedness.
  * Returns an array of human-readable diagnostic strings; empty = valid.
@@ -42,6 +64,12 @@ export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[]
           `access-policy.rules["${pattern}"] "${perm}" is invalid: must be exactly 3 chars (e.g. "rwx", "r--", "---")`,
         );
       }
+      if (hasMidPathWildcard(pattern) && !_midPathWildcardWarned.has(`rules:${pattern}`)) {
+        _midPathWildcardWarned.add(`rules:${pattern}`);
+        errors.push(
+          `access-policy.rules["${pattern}"] contains a mid-path wildcard — OS-level (bwrap/Seatbelt) enforcement cannot apply; tool-layer enforcement is still active.`,
+        );
+      }
       // If a bare path (no glob metacharacters, no trailing /) points to a real
       // directory it would match only the directory entry itself, not its
       // contents. Auto-expand to "/**" and notify — the fix is unambiguous.
@@ -105,6 +133,12 @@ export function validateAccessPolicyConfig(config: AccessPolicyConfig): string[]
         errors.push(`access-policy.deny[${i}] must be a non-empty glob pattern`);
         continue;
       }
+      if (hasMidPathWildcard(pattern) && !_midPathWildcardWarned.has(`deny:${pattern}`)) {
+        _midPathWildcardWarned.add(`deny:${pattern}`);
+        errors.push(
+          `access-policy.deny entry "${pattern}" contains a mid-path wildcard — OS-level (bwrap/Seatbelt) enforcement cannot apply; tool-layer enforcement is still active.`,
+        );
+      }
       // Bare-path auto-expand for directories: "~/.ssh" → "~/.ssh/**" so the
       // entire directory tree is denied, not just the directory inode itself.
       // For paths that exist and are confirmed files (statSync), keep the bare