From 3c941eae23be6c33a3a15910b6156e38b4f27b94 Mon Sep 17 00:00:00 2001 From: subrih Date: Sat, 14 Mar 2026 16:20:02 -0700 Subject: [PATCH] fix: deep-copy nested scripts policy objects in mergeAccessPolicy, use SEATBELT_WRITE_OPS - mergeAccessPolicy !base fast-path: scripts["policy"] and per-script entry.policy were shallow-copied, leaving them as references into the cached _fileCache object. autoExpandBareDir mutations would propagate back into the cache, violating the invariant established by the policy-copy fix. Now deep-copied via Object.fromEntries map. - exec-sandbox-seatbelt: replace hardcoded "file-write*" with SEATBELT_WRITE_OPS constant in the /tmp write allowance branch, consistent with all other allowance lines in the file. - Tests added for nested scripts deep-copy invariant. --- src/infra/access-policy-file.test.ts | 28 ++++++++++++++++++++++++++++ src/infra/access-policy-file.ts | 23 ++++++++++++++++++++++- src/infra/exec-sandbox-seatbelt.ts | 2 +- 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/infra/access-policy-file.test.ts b/src/infra/access-policy-file.test.ts index b1db78f86d9..273241e5d55 100644 --- a/src/infra/access-policy-file.test.ts +++ b/src/infra/access-policy-file.test.ts @@ -81,6 +81,34 @@ describe("mergeAccessPolicy", () => { expect(override.policy["/tmp/**" as keyof typeof override.policy]).toBeUndefined(); }); + it("deep-copies nested scripts policy maps — mutations do not corrupt cache", () => { + // Shallow-copying scripts is not enough: scripts["policy"] and per-script entry.policy + // are nested objects that would still be references into the cached _fileCache object. + const scriptPolicy = { "/tmp/**": "rwx" as const }; + const entryPolicy = { "/data/**": "r--" as const }; + const override = { + scripts: { + policy: scriptPolicy, + "/deploy.sh": { policy: entryPolicy }, + }, + }; + const result = mergeAccessPolicy(undefined, override); + // Mutate the returned scripts["policy"] and per-script policy. + const sp = result?.scripts?.["policy"]; + if (sp) { + sp["/added/**"] = "---"; + } + const entry = result?.scripts?.["/deploy.sh"] as + | { policy?: Record } + | undefined; + if (entry?.policy) { + entry.policy["/added/**"] = "---"; + } + // Originals must be unchanged. + expect(scriptPolicy).toEqual({ "/tmp/**": "rwx" }); + expect(entryPolicy).toEqual({ "/data/**": "r--" }); + }); + it("rules are shallow-merged, override key wins on collision", () => { const result = mergeAccessPolicy( { policy: { "/**": "r--", "~/**": "rw-" } }, diff --git a/src/infra/access-policy-file.ts b/src/infra/access-policy-file.ts index 4878f2a5beb..ff91897d241 100644 --- a/src/infra/access-policy-file.ts +++ b/src/infra/access-policy-file.ts @@ -46,10 +46,31 @@ export function mergeAccessPolicy( // 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. + // Deep-copy nested policy records so validateAccessPolicyConfig → autoExpandBareDir + // cannot mutate the cached _fileCache object. Shallow-copying `scripts` is not enough: + // `scripts["policy"]` (shared rules map) and per-script `entry.policy` maps are nested + // objects that would still be references into the cache. + const overrideScripts = override.scripts; + const scriptsCopy: AccessPolicyConfig["scripts"] | undefined = overrideScripts + ? Object.fromEntries( + Object.entries(overrideScripts).map(([k, v]) => { + if (k === "policy") { + // scripts["policy"] is a Record — shallow copy is sufficient. + return [k, v != null && typeof v === "object" ? { ...v } : v]; + } + // Per-script entries: copy the entry and its nested policy map. + if (v != null && typeof v === "object" && !Array.isArray(v)) { + const entry = v as import("../config/types.tools.js").ScriptPolicyEntry; + return [k, { ...entry, policy: entry.policy ? { ...entry.policy } : undefined }]; + } + return [k, v]; + }), + ) + : undefined; return { ...override, policy: override.policy ? { ...override.policy } : undefined, - scripts: override.scripts ? { ...override.scripts } : undefined, + scripts: scriptsCopy, }; } if (!override) { diff --git a/src/infra/exec-sandbox-seatbelt.ts b/src/infra/exec-sandbox-seatbelt.ts index d77a0bc3176..9c0f4477911 100644 --- a/src/infra/exec-sandbox-seatbelt.ts +++ b/src/infra/exec-sandbox-seatbelt.ts @@ -248,7 +248,7 @@ export function generateSeatbeltProfile( lines.push(`(allow ${SEATBELT_READ_OPS} (subpath "/private/tmp"))`); } if (tmpPerm[1] === "w") { - lines.push(`(allow file-write* (subpath "/private/tmp"))`); + lines.push(`(allow ${SEATBELT_WRITE_OPS} (subpath "/private/tmp"))`); } if (tmpPerm[2] === "x") { lines.push(`(allow ${SEATBELT_EXEC_OPS} (subpath "/private/tmp"))`);