mirror of https://github.com/openclaw/openclaw.git
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.
This commit is contained in:
parent
8848bb82e9
commit
3c941eae23
|
|
@ -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<string, string> }
|
||||
| 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-" } },
|
||||
|
|
|
|||
|
|
@ -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<string, PermStr> — 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) {
|
||||
|
|
|
|||
|
|
@ -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"))`);
|
||||
|
|
|
|||
Loading…
Reference in New Issue