From 7afa33f0c2fde7e8130a80629534700f61eaec1c Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 21 May 2026 06:42:18 +0000 Subject: [PATCH 1/3] fix(shields): union live policy into permissive before applying (#3942) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenShell refuses to remove a `filesystem_policy.read_only` or `filesystem_policy.read_write` entry on a live sandbox. The static `openclaw-sandbox-permissive.yaml` baseline never sees the runtime enrichment that NemoClaw adds at create time, so applying it on top of a live sandbox can hit "filesystem read_write path '' cannot be removed on a live sandbox". This has reappeared with each new runtime-injected path: - #3168 — Hermes `/opt/hermes` (fixed by per-agent permissive YAML) - #3957 — OpenClaw `/home/linuxbrew` (fixed by patching the YAML) - #3942 — GPU sandboxes `/proc` (the current bug) Close the loop with a generic helper. Before `shields down` applies the permissive policy, fetch the live sandbox's policy via the snapshot we already capture and union its `filesystem_policy.read_write` and `filesystem_policy.read_only` into the base permissive YAML. The result is a strict superset of the live filesystem section, so OpenShell never sees a removal on transition. Resolution rules on overlap: a path that is `read_write` on the live side wins and is removed from `read_only` in the output — we never emit the same path in both lists, and we never downgrade a writable path to read-only as part of an unrelated transition. The helper takes its live-policy fetcher and base-policy reader as injected deps so it stays a pure function with no runtime cycle into the policy module, and so the regression test can drive it without mocking node_modules. When the live policy is empty, parses as an error, or omits the filesystem section, the helper degrades to the existing static path — matches prior behaviour rather than failing closed. Signed-off-by: Tinson Lai --- src/lib/shields/index.ts | 25 ++++- src/lib/shields/permissive-runtime.ts | 131 ++++++++++++++++++++++++ test/permissive-runtime.test.ts | 137 ++++++++++++++++++++++++++ 3 files changed, 291 insertions(+), 2 deletions(-) create mode 100644 src/lib/shields/permissive-runtime.ts create mode 100644 test/permissive-runtime.test.ts diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 9f8a7e315a4..b453891c08e 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -42,6 +42,10 @@ const { const { resolveNemoclawStateDir } = require("../state/paths"); const { appendAuditEntry } = require("./audit"); const { resolveAgentConfig } = require("../sandbox/config"); +const { + buildRuntimePermissivePolicy, +}: typeof import("./permissive-runtime") = require("./permissive-runtime"); +const { cleanupTempDir } = require("../onboard/temp-files"); const STATE_DIR = resolveNemoclawStateDir(); @@ -902,8 +906,19 @@ function shieldsDown(sandboxName: string, opts: ShieldsDownOpts = {}): void { // 2. Determine and apply relaxed policy let policyFile: string; + let policyFileIsTemp = false; if (policyName === "permissive") { - policyFile = resolvePermissivePolicyPath(sandboxName); + const basePath = resolvePermissivePolicyPath(sandboxName); + // Union the live sandbox's filesystem_policy into the static permissive + // baseline. OpenShell rejects removal of read_only / read_write paths + // on a live sandbox, and runtime-injected entries (/proc on GPU, + // /opt/hermes on Hermes, /home/linuxbrew on post-#3913 OpenClaw, etc.) + // are not present in the static YAML. See #3942, #3957, #3168. + policyFile = buildRuntimePermissivePolicy(sandboxName, basePath, { + fetchLivePolicy: () => rawPolicy, + readBasePolicy: () => fs.readFileSync(basePath, "utf-8"), + }); + policyFileIsTemp = policyFile !== basePath; } else if (fs.existsSync(policyName)) { policyFile = path.resolve(policyName); } else { @@ -914,7 +929,13 @@ function shieldsDown(sandboxName: string, opts: ShieldsDownOpts = {}): void { } console.log(` Applying ${policyName} policy...`); - run(buildPolicySetCommand(policyFile, sandboxName)); + try { + run(buildPolicySetCommand(policyFile, sandboxName)); + } finally { + if (policyFileIsTemp) { + cleanupTempDir(policyFile, "nemoclaw-permissive-runtime"); + } + } // 2b. Return config to default mutable state. // OpenClaw uses sandbox:sandbox 0660/2770 here so the gateway UID, which diff --git a/src/lib/shields/permissive-runtime.ts b/src/lib/shields/permissive-runtime.ts new file mode 100644 index 00000000000..0b2db1f0673 --- /dev/null +++ b/src/lib/shields/permissive-runtime.ts @@ -0,0 +1,131 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import YAML from "yaml"; + +import { secureTempFile } from "../onboard/temp-files"; + +/** + * Build a permissive policy YAML that is guaranteed to be a strict superset + * of the live sandbox's filesystem policy. + * + * Background (#3942, #3957, #3168): OpenShell refuses to remove a + * `filesystem_policy.read_only` or `filesystem_policy.read_write` entry on a + * live sandbox. The static `openclaw-sandbox-permissive.yaml` baseline does + * not see runtime-injected paths — `/proc` on GPU sandboxes, `/opt/hermes` + * on Hermes, `/home/linuxbrew` on post-#3913 OpenClaw, and any future + * agent- or feature-specific enrichment. Each of those past mismatches + * shipped its own permissive-YAML patch. This helper closes the loop by + * unioning whatever the live sandbox advertises into the permissive YAML + * before it is applied, so future runtime injections are absorbed + * automatically. + * + * Resolution rules when a path appears on both sides: + * - Live `read_write` is the more permissive of the two and takes priority: + * if the live state writes a path, the permissive transition keeps it + * writable, removing it from `read_only` first so we never emit a path + * in both lists. + * - Live `read_only` is merged into base `read_only` only when the same + * path is not already granted `read_write` (either by base or by live). + * + * Returns the path to a freshly created temp YAML file. Falls back to the + * base permissive path if the live policy can't be parsed or omits the + * filesystem section — degrading to the existing static behavior rather + * than failing closed. + */ +export interface PermissiveRuntimeDeps { + fetchLivePolicy: (sandboxName: string) => string; + readBasePolicy: () => string; +} + +export function buildRuntimePermissivePolicy( + sandboxName: string, + basePermissivePath: string, + deps: PermissiveRuntimeDeps, +): string { + const liveRaw = deps.fetchLivePolicy(sandboxName); + const liveYaml = parsePolicyBlock(liveRaw); + const live = liveYaml ? safeYamlObject(liveYaml) : null; + const liveRw = readStringList(live, "read_write"); + const liveRo = readStringList(live, "read_only"); + + // No live filesystem section to merge — keep the static path so the + // caller's apply path is unchanged. + if (liveRw.length === 0 && liveRo.length === 0) { + return basePermissivePath; + } + + const baseYaml = deps.readBasePolicy(); + const base = safeYamlObject(baseYaml); + if (!base) { + return basePermissivePath; + } + const fsPolicy = + base.filesystem_policy && typeof base.filesystem_policy === "object" + ? (base.filesystem_policy as Record) + : ((base.filesystem_policy = {} as Record), + base.filesystem_policy as Record); + + const baseRw = new Set(readStringList(base, "read_write")); + const baseRo = new Set(readStringList(base, "read_only")); + + // RW wins: a live write-path must stay writable in the new policy, and + // the same path cannot also live in read_only afterwards. + for (const p of liveRw) { + baseRo.delete(p); + baseRw.add(p); + } + for (const p of liveRo) { + if (!baseRw.has(p)) baseRo.add(p); + } + + fsPolicy.read_write = [...baseRw]; + fsPolicy.read_only = [...baseRo]; + + const tmpPath = secureTempFile("nemoclaw-permissive-runtime", ".yaml"); + fs.writeFileSync(tmpPath, YAML.stringify(base), { mode: 0o600 }); + return tmpPath; +} + +function safeYamlObject(text: string): Record | null { + try { + const parsed = YAML.parse(text); + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + return parsed as Record; + } + } catch { + return null; + } + return null; +} + +function readStringList( + root: Record | null, + key: "read_only" | "read_write", +): string[] { + const fsPolicy = root?.filesystem_policy; + if (!fsPolicy || typeof fsPolicy !== "object") return []; + const value = (fsPolicy as Record)[key]; + if (!Array.isArray(value)) return []; + return value.filter((entry): entry is string => typeof entry === "string"); +} + +// Lightweight clone of policy/index.ts:parseCurrentPolicy that strips the +// OpenShell header / error preamble before YAML.parse. Inlined to avoid a +// runtime cycle with the policy module. +function parsePolicyBlock(raw: string | null | undefined): string { + if (!raw) return ""; + const sep = raw.indexOf("---"); + const candidate = (sep === -1 ? raw : raw.slice(sep + 3)).trim(); + if (!candidate) return ""; + if (/^(error|failed|invalid|warning|status)\b/i.test(candidate)) return ""; + if (!/^[a-z_][a-z0-9_]*\s*:/m.test(candidate)) return ""; + try { + const parsed = YAML.parse(candidate); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return ""; + } catch { + return ""; + } + return candidate; +} diff --git a/test/permissive-runtime.test.ts b/test/permissive-runtime.test.ts new file mode 100644 index 00000000000..da1434d12ef --- /dev/null +++ b/test/permissive-runtime.test.ts @@ -0,0 +1,137 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; +import path from "node:path"; +import { describe, it, expect, afterEach } from "vitest"; +import YAML from "yaml"; + +import { buildRuntimePermissivePolicy } from "../dist/lib/shields/permissive-runtime.js"; + +const BASE_PERMISSIVE = YAML.stringify({ + filesystem_policy: { + read_only: ["/proc", "/etc"], + read_write: ["/tmp", "/sandbox/.openclaw"], + }, + landlock: { compatibility: "best_effort" }, +}); + +const tempFilesToClean: string[] = []; + +afterEach(() => { + while (tempFilesToClean.length > 0) { + const p = tempFilesToClean.pop(); + if (!p) continue; + try { + fs.rmSync(path.dirname(p), { recursive: true, force: true }); + } catch { + // best-effort + } + } +}); + +function withWrapper(yaml: string): string { + // Mirror `openshell policy get --full` output shape: a header line, then + // `---`, then the YAML body. parsePolicyBlock should strip the prefix. + return `policy: openclaw-sandbox\n---\n${yaml}`; +} + +describe("buildRuntimePermissivePolicy (#3942)", () => { + it("preserves /proc when the live GPU sandbox has it in read_write", () => { + const live = YAML.stringify({ + filesystem_policy: { + read_only: ["/etc", "/usr"], + // GPU enrichment from src/lib/onboard/initial-policy.ts:57. + read_write: ["/tmp", "/proc", "/home/linuxbrew"], + }, + }); + + const out = buildRuntimePermissivePolicy("alpha", "/unused-base.yaml", { + fetchLivePolicy: () => withWrapper(live), + readBasePolicy: () => BASE_PERMISSIVE, + }); + tempFilesToClean.push(out); + + const result = YAML.parse(fs.readFileSync(out, "utf-8")); + expect(result.filesystem_policy.read_write).toEqual( + expect.arrayContaining(["/tmp", "/sandbox/.openclaw", "/proc", "/home/linuxbrew"]), + ); + // /proc must NOT also appear in read_only; rw wins. + expect(result.filesystem_policy.read_only).not.toContain("/proc"); + }); + + it("merges live read_only paths into base read_only without clobbering rw", () => { + const live = YAML.stringify({ + filesystem_policy: { + // /tmp is in base read_write — live ro should NOT downgrade it. + read_only: ["/usr", "/tmp"], + read_write: [], + }, + }); + + const out = buildRuntimePermissivePolicy("alpha", "/unused-base.yaml", { + fetchLivePolicy: () => withWrapper(live), + readBasePolicy: () => BASE_PERMISSIVE, + }); + tempFilesToClean.push(out); + + const result = YAML.parse(fs.readFileSync(out, "utf-8")); + expect(result.filesystem_policy.read_write).toContain("/tmp"); + expect(result.filesystem_policy.read_only).toContain("/usr"); + expect(result.filesystem_policy.read_only).not.toContain("/tmp"); + }); + + it("deduplicates entries within each list and across lists", () => { + const live = YAML.stringify({ + filesystem_policy: { + read_only: ["/etc", "/etc"], // duplicate within live ro + read_write: ["/tmp", "/tmp", "/proc"], // duplicate within live rw + }, + }); + + const out = buildRuntimePermissivePolicy("alpha", "/unused-base.yaml", { + fetchLivePolicy: () => withWrapper(live), + readBasePolicy: () => BASE_PERMISSIVE, + }); + tempFilesToClean.push(out); + + const result = YAML.parse(fs.readFileSync(out, "utf-8")); + const rwCount = result.filesystem_policy.read_write.filter((p: string) => p === "/tmp").length; + const roCount = result.filesystem_policy.read_only.filter((p: string) => p === "/etc").length; + expect(rwCount).toBe(1); + expect(roCount).toBe(1); + // No path appears in both lists. + const rwSet = new Set(result.filesystem_policy.read_write); + for (const p of result.filesystem_policy.read_only) { + expect(rwSet.has(p)).toBe(false); + } + }); + + it("returns the static base path when live policy is empty / unparseable", () => { + const basePath = "/path/to/static.yaml"; + const out = buildRuntimePermissivePolicy("alpha", basePath, { + fetchLivePolicy: () => "", + readBasePolicy: () => BASE_PERMISSIVE, + }); + expect(out).toBe(basePath); + }); + + it("returns the static base path when openshell policy get errored", () => { + const basePath = "/path/to/static.yaml"; + const out = buildRuntimePermissivePolicy("alpha", basePath, { + fetchLivePolicy: () => "Error: sandbox not found", + readBasePolicy: () => BASE_PERMISSIVE, + }); + expect(out).toBe(basePath); + }); + + it("returns the static base path when live policy has no filesystem_policy section", () => { + const basePath = "/path/to/static.yaml"; + const live = YAML.stringify({ landlock: { compatibility: "best_effort" } }); + const out = buildRuntimePermissivePolicy("alpha", basePath, { + fetchLivePolicy: () => withWrapper(live), + readBasePolicy: () => BASE_PERMISSIVE, + }); + expect(out).toBe(basePath); + }); +}); From 73a4fe50d7df0bb727eff592579d7985ac09842a Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 21 May 2026 07:09:29 +0000 Subject: [PATCH 2/3] refactor(shields): tighten permissive-runtime helper per Codex review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Drop the local parsePolicyBlock clone: callers already invoke parseCurrentPolicy on the raw `openshell policy get` output, so the helper now takes the pre-parsed YAML body via `livePolicyYaml` instead of duplicating the header-stripping regex. shields/index.ts passes the `policyYaml` it already captured for the snapshot. * Wrap the boundary I/O — base-policy read, temp-file create, write — in try/catch. On any I/O failure the helper returns the static base path so shields-down keeps degrading to the existing apply path rather than aborting before the policy set ever runs. * Reword the doc comment: the helper unions filesystem path lists (read_only + read_write), not the entire filesystem_policy block. Other `filesystem_policy` fields such as `include_workdir` are preserved verbatim from the static base. Added a regression test to lock that behaviour in. Also: extra regression tests for the throwing-readBasePolicy and unparseable-base-YAML paths, and the include_workdir passthrough. Signed-off-by: Tinson Lai --- src/lib/shields/index.ts | 16 ++-- src/lib/shields/permissive-runtime.ts | 102 +++++++++++++------------- test/permissive-runtime.test.ts | 84 +++++++++++++-------- 3 files changed, 116 insertions(+), 86 deletions(-) diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index b453891c08e..ceba4524396 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -909,13 +909,15 @@ function shieldsDown(sandboxName: string, opts: ShieldsDownOpts = {}): void { let policyFileIsTemp = false; if (policyName === "permissive") { const basePath = resolvePermissivePolicyPath(sandboxName); - // Union the live sandbox's filesystem_policy into the static permissive - // baseline. OpenShell rejects removal of read_only / read_write paths - // on a live sandbox, and runtime-injected entries (/proc on GPU, - // /opt/hermes on Hermes, /home/linuxbrew on post-#3913 OpenClaw, etc.) - // are not present in the static YAML. See #3942, #3957, #3168. - policyFile = buildRuntimePermissivePolicy(sandboxName, basePath, { - fetchLivePolicy: () => rawPolicy, + // Union the live sandbox's filesystem_policy.read_only/read_write into + // the static permissive baseline. OpenShell rejects removal of those + // paths on a live sandbox, and runtime-injected entries (/proc on + // GPU, /opt/hermes on Hermes, /home/linuxbrew on post-#3913 OpenClaw, + // etc.) are not present in the static YAML. See #3942, #3957, #3168. + // policyYaml is the pre-parsed body we already captured for the + // snapshot above — reuse it instead of re-fetching. + policyFile = buildRuntimePermissivePolicy(basePath, { + livePolicyYaml: policyYaml, readBasePolicy: () => fs.readFileSync(basePath, "utf-8"), }); policyFileIsTemp = policyFile !== basePath; diff --git a/src/lib/shields/permissive-runtime.ts b/src/lib/shields/permissive-runtime.ts index 0b2db1f0673..bb6874b1013 100644 --- a/src/lib/shields/permissive-runtime.ts +++ b/src/lib/shields/permissive-runtime.ts @@ -7,46 +7,58 @@ import YAML from "yaml"; import { secureTempFile } from "../onboard/temp-files"; /** - * Build a permissive policy YAML that is guaranteed to be a strict superset - * of the live sandbox's filesystem policy. + * Build a permissive policy YAML whose filesystem path lists + * (`filesystem_policy.read_only` + `filesystem_policy.read_write`) are a + * superset of the live sandbox's, so OpenShell never has to remove a path + * on a live transition. + * + * Only the two path lists are unioned. Other `filesystem_policy` fields + * (e.g. `include_workdir`) are preserved verbatim from the static base — + * the bug class this helper exists for is path removal on a live sandbox, + * not policy shape changes. * * Background (#3942, #3957, #3168): OpenShell refuses to remove a - * `filesystem_policy.read_only` or `filesystem_policy.read_write` entry on a - * live sandbox. The static `openclaw-sandbox-permissive.yaml` baseline does - * not see runtime-injected paths — `/proc` on GPU sandboxes, `/opt/hermes` - * on Hermes, `/home/linuxbrew` on post-#3913 OpenClaw, and any future - * agent- or feature-specific enrichment. Each of those past mismatches - * shipped its own permissive-YAML patch. This helper closes the loop by - * unioning whatever the live sandbox advertises into the permissive YAML - * before it is applied, so future runtime injections are absorbed - * automatically. + * `filesystem_policy.read_only` or `filesystem_policy.read_write` entry + * on a live sandbox. The static `openclaw-sandbox-permissive.yaml` + * baseline does not see runtime-injected paths — `/proc` on GPU + * sandboxes, `/opt/hermes` on Hermes, `/home/linuxbrew` on post-#3913 + * OpenClaw, and any future agent- or feature-specific enrichment. Each + * past mismatch shipped its own permissive-YAML patch. This helper + * closes the loop by unioning whatever the live sandbox advertises into + * the permissive YAML before it is applied, so future runtime injections + * are absorbed automatically. * - * Resolution rules when a path appears on both sides: - * - Live `read_write` is the more permissive of the two and takes priority: - * if the live state writes a path, the permissive transition keeps it - * writable, removing it from `read_only` first so we never emit a path - * in both lists. + * Resolution rules when a path appears in both `read_only` and + * `read_write`: + * - Live `read_write` is the more permissive of the two and takes + * priority: if the live state writes a path, the permissive transition + * keeps it writable, removing it from `read_only` first so we never + * emit a path in both lists. * - Live `read_only` is merged into base `read_only` only when the same * path is not already granted `read_write` (either by base or by live). * - * Returns the path to a freshly created temp YAML file. Falls back to the - * base permissive path if the live policy can't be parsed or omits the - * filesystem section — degrading to the existing static behavior rather - * than failing closed. + * Returns the path to a freshly created temp YAML file when the live + * policy carries a filesystem section that needs merging. Falls back to + * the static base path when the live policy is empty / has no filesystem + * lists, when the base YAML cannot be parsed, or when temp-file I/O + * fails — degrading to the existing static apply path rather than + * aborting shields-down with an I/O error. */ export interface PermissiveRuntimeDeps { - fetchLivePolicy: (sandboxName: string) => string; + // Pre-parsed live policy YAML body (e.g. parseCurrentPolicy(rawPolicy) + // from the caller, which already strips the OpenShell header). Passed + // in rather than fetched here so this helper stays a pure transform. + livePolicyYaml: string; + // Lazy because callers may want to defer the read until the helper + // actually needs it. The returned string is parsed by YAML.parse. readBasePolicy: () => string; } export function buildRuntimePermissivePolicy( - sandboxName: string, basePermissivePath: string, deps: PermissiveRuntimeDeps, ): string { - const liveRaw = deps.fetchLivePolicy(sandboxName); - const liveYaml = parsePolicyBlock(liveRaw); - const live = liveYaml ? safeYamlObject(liveYaml) : null; + const live = deps.livePolicyYaml ? safeYamlObject(deps.livePolicyYaml) : null; const liveRw = readStringList(live, "read_write"); const liveRo = readStringList(live, "read_only"); @@ -56,7 +68,12 @@ export function buildRuntimePermissivePolicy( return basePermissivePath; } - const baseYaml = deps.readBasePolicy(); + let baseYaml: string; + try { + baseYaml = deps.readBasePolicy(); + } catch { + return basePermissivePath; + } const base = safeYamlObject(baseYaml); if (!base) { return basePermissivePath; @@ -70,8 +87,8 @@ export function buildRuntimePermissivePolicy( const baseRw = new Set(readStringList(base, "read_write")); const baseRo = new Set(readStringList(base, "read_only")); - // RW wins: a live write-path must stay writable in the new policy, and - // the same path cannot also live in read_only afterwards. + // RW wins: a live write-path must stay writable in the new policy, + // and the same path cannot also live in read_only afterwards. for (const p of liveRw) { baseRo.delete(p); baseRw.add(p); @@ -83,9 +100,13 @@ export function buildRuntimePermissivePolicy( fsPolicy.read_write = [...baseRw]; fsPolicy.read_only = [...baseRo]; - const tmpPath = secureTempFile("nemoclaw-permissive-runtime", ".yaml"); - fs.writeFileSync(tmpPath, YAML.stringify(base), { mode: 0o600 }); - return tmpPath; + try { + const tmpPath = secureTempFile("nemoclaw-permissive-runtime", ".yaml"); + fs.writeFileSync(tmpPath, YAML.stringify(base), { mode: 0o600 }); + return tmpPath; + } catch { + return basePermissivePath; + } } function safeYamlObject(text: string): Record | null { @@ -110,22 +131,3 @@ function readStringList( if (!Array.isArray(value)) return []; return value.filter((entry): entry is string => typeof entry === "string"); } - -// Lightweight clone of policy/index.ts:parseCurrentPolicy that strips the -// OpenShell header / error preamble before YAML.parse. Inlined to avoid a -// runtime cycle with the policy module. -function parsePolicyBlock(raw: string | null | undefined): string { - if (!raw) return ""; - const sep = raw.indexOf("---"); - const candidate = (sep === -1 ? raw : raw.slice(sep + 3)).trim(); - if (!candidate) return ""; - if (/^(error|failed|invalid|warning|status)\b/i.test(candidate)) return ""; - if (!/^[a-z_][a-z0-9_]*\s*:/m.test(candidate)) return ""; - try { - const parsed = YAML.parse(candidate); - if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return ""; - } catch { - return ""; - } - return candidate; -} diff --git a/test/permissive-runtime.test.ts b/test/permissive-runtime.test.ts index da1434d12ef..f3c24fa75cf 100644 --- a/test/permissive-runtime.test.ts +++ b/test/permissive-runtime.test.ts @@ -10,6 +10,7 @@ import { buildRuntimePermissivePolicy } from "../dist/lib/shields/permissive-run const BASE_PERMISSIVE = YAML.stringify({ filesystem_policy: { + include_workdir: true, read_only: ["/proc", "/etc"], read_write: ["/tmp", "/sandbox/.openclaw"], }, @@ -30,15 +31,9 @@ afterEach(() => { } }); -function withWrapper(yaml: string): string { - // Mirror `openshell policy get --full` output shape: a header line, then - // `---`, then the YAML body. parsePolicyBlock should strip the prefix. - return `policy: openclaw-sandbox\n---\n${yaml}`; -} - describe("buildRuntimePermissivePolicy (#3942)", () => { it("preserves /proc when the live GPU sandbox has it in read_write", () => { - const live = YAML.stringify({ + const liveYaml = YAML.stringify({ filesystem_policy: { read_only: ["/etc", "/usr"], // GPU enrichment from src/lib/onboard/initial-policy.ts:57. @@ -46,8 +41,8 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { }, }); - const out = buildRuntimePermissivePolicy("alpha", "/unused-base.yaml", { - fetchLivePolicy: () => withWrapper(live), + const out = buildRuntimePermissivePolicy("/unused-base.yaml", { + livePolicyYaml: liveYaml, readBasePolicy: () => BASE_PERMISSIVE, }); tempFilesToClean.push(out); @@ -60,8 +55,23 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { expect(result.filesystem_policy.read_only).not.toContain("/proc"); }); + it("preserves non-list filesystem_policy fields (e.g. include_workdir)", () => { + const liveYaml = YAML.stringify({ + filesystem_policy: { read_write: ["/proc"], read_only: ["/usr"] }, + }); + + const out = buildRuntimePermissivePolicy("/unused-base.yaml", { + livePolicyYaml: liveYaml, + readBasePolicy: () => BASE_PERMISSIVE, + }); + tempFilesToClean.push(out); + + const result = YAML.parse(fs.readFileSync(out, "utf-8")); + expect(result.filesystem_policy.include_workdir).toBe(true); + }); + it("merges live read_only paths into base read_only without clobbering rw", () => { - const live = YAML.stringify({ + const liveYaml = YAML.stringify({ filesystem_policy: { // /tmp is in base read_write — live ro should NOT downgrade it. read_only: ["/usr", "/tmp"], @@ -69,8 +79,8 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { }, }); - const out = buildRuntimePermissivePolicy("alpha", "/unused-base.yaml", { - fetchLivePolicy: () => withWrapper(live), + const out = buildRuntimePermissivePolicy("/unused-base.yaml", { + livePolicyYaml: liveYaml, readBasePolicy: () => BASE_PERMISSIVE, }); tempFilesToClean.push(out); @@ -82,15 +92,15 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { }); it("deduplicates entries within each list and across lists", () => { - const live = YAML.stringify({ + const liveYaml = YAML.stringify({ filesystem_policy: { - read_only: ["/etc", "/etc"], // duplicate within live ro - read_write: ["/tmp", "/tmp", "/proc"], // duplicate within live rw + read_only: ["/etc", "/etc"], + read_write: ["/tmp", "/tmp", "/proc"], }, }); - const out = buildRuntimePermissivePolicy("alpha", "/unused-base.yaml", { - fetchLivePolicy: () => withWrapper(live), + const out = buildRuntimePermissivePolicy("/unused-base.yaml", { + livePolicyYaml: liveYaml, readBasePolicy: () => BASE_PERMISSIVE, }); tempFilesToClean.push(out); @@ -100,37 +110,53 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { const roCount = result.filesystem_policy.read_only.filter((p: string) => p === "/etc").length; expect(rwCount).toBe(1); expect(roCount).toBe(1); - // No path appears in both lists. const rwSet = new Set(result.filesystem_policy.read_write); for (const p of result.filesystem_policy.read_only) { expect(rwSet.has(p)).toBe(false); } }); - it("returns the static base path when live policy is empty / unparseable", () => { + it("returns the static base path when live policy is empty", () => { const basePath = "/path/to/static.yaml"; - const out = buildRuntimePermissivePolicy("alpha", basePath, { - fetchLivePolicy: () => "", + const out = buildRuntimePermissivePolicy(basePath, { + livePolicyYaml: "", readBasePolicy: () => BASE_PERMISSIVE, }); expect(out).toBe(basePath); }); - it("returns the static base path when openshell policy get errored", () => { + it("returns the static base path when live policy has no filesystem_policy section", () => { const basePath = "/path/to/static.yaml"; - const out = buildRuntimePermissivePolicy("alpha", basePath, { - fetchLivePolicy: () => "Error: sandbox not found", + const liveYaml = YAML.stringify({ landlock: { compatibility: "best_effort" } }); + const out = buildRuntimePermissivePolicy(basePath, { + livePolicyYaml: liveYaml, readBasePolicy: () => BASE_PERMISSIVE, }); expect(out).toBe(basePath); }); - it("returns the static base path when live policy has no filesystem_policy section", () => { + it("returns the static base path when readBasePolicy throws (I/O failure)", () => { const basePath = "/path/to/static.yaml"; - const live = YAML.stringify({ landlock: { compatibility: "best_effort" } }); - const out = buildRuntimePermissivePolicy("alpha", basePath, { - fetchLivePolicy: () => withWrapper(live), - readBasePolicy: () => BASE_PERMISSIVE, + const liveYaml = YAML.stringify({ + filesystem_policy: { read_write: ["/proc"] }, + }); + const out = buildRuntimePermissivePolicy(basePath, { + livePolicyYaml: liveYaml, + readBasePolicy: () => { + throw new Error("ENOENT"); + }, + }); + expect(out).toBe(basePath); + }); + + it("returns the static base path when base YAML is unparseable", () => { + const basePath = "/path/to/static.yaml"; + const liveYaml = YAML.stringify({ + filesystem_policy: { read_write: ["/proc"] }, + }); + const out = buildRuntimePermissivePolicy(basePath, { + livePolicyYaml: liveYaml, + readBasePolicy: () => "::: not yaml :::", }); expect(out).toBe(basePath); }); From ac6b0355861b8424777d73d0f657b3200108de04 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 21 May 2026 07:46:03 +0000 Subject: [PATCH 3/3] fix(shields): tighten permissive-runtime write-failure path + tests * Track the mkdtemp directory and call cleanupTempDir in the writeFileSync catch branch so a partial temp-file failure no longer leaks a 0700 directory under /tmp. * Expose an injectable writeTempPolicy dep so the write-failure fallback can be driven from tests without monkey-patching node:fs. Default behaviour (omit dep) keeps using secureTempFile + fs.writeFileSync. * Add a regression test that drives the write-failure branch and asserts the helper returns the static base path. * Tighten the test cleanup helper so it refuses to enqueue any output whose dirname is not under os.tmpdir(). Earlier the tests could have done `rm -rf path.dirname("/unused-base.yaml")` (i.e. "/") if the helper ever degraded to the static path on a code regression. The guarded helper now also asserts out !== basePath in the success cases for an explicit early-warning signal. * Reword the dep doc comment: "stays a pure transform" was misleading because the helper does in fact do I/O (base read, temp file write). Say "live-policy acquisition stays outside this helper" instead. Signed-off-by: Tinson Lai --- src/lib/shields/permissive-runtime.ts | 29 +++++++++++++++--- test/permissive-runtime.test.ts | 44 ++++++++++++++++++++++++--- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/lib/shields/permissive-runtime.ts b/src/lib/shields/permissive-runtime.ts index bb6874b1013..46f523f60be 100644 --- a/src/lib/shields/permissive-runtime.ts +++ b/src/lib/shields/permissive-runtime.ts @@ -4,7 +4,9 @@ import fs from "node:fs"; import YAML from "yaml"; -import { secureTempFile } from "../onboard/temp-files"; +import { cleanupTempDir, secureTempFile } from "../onboard/temp-files"; + +const TEMP_FILE_PREFIX = "nemoclaw-permissive-runtime"; /** * Build a permissive policy YAML whose filesystem path lists @@ -47,11 +49,17 @@ import { secureTempFile } from "../onboard/temp-files"; export interface PermissiveRuntimeDeps { // Pre-parsed live policy YAML body (e.g. parseCurrentPolicy(rawPolicy) // from the caller, which already strips the OpenShell header). Passed - // in rather than fetched here so this helper stays a pure transform. + // in rather than fetched here so live-policy acquisition stays + // outside this helper — the helper itself still does I/O (base read, + // temp file write) but does not shell out to openshell. livePolicyYaml: string; // Lazy because callers may want to defer the read until the helper // actually needs it. The returned string is parsed by YAML.parse. readBasePolicy: () => string; + // Injectable temp-file writer. Defaults to fs.writeFileSync via + // secureTempFile when omitted. Exposed so tests can drive the + // write-failure fallback path without monkey-patching node:fs. + writeTempPolicy?: (yaml: string) => string; } export function buildRuntimePermissivePolicy( @@ -100,11 +108,24 @@ export function buildRuntimePermissivePolicy( fsPolicy.read_write = [...baseRw]; fsPolicy.read_only = [...baseRo]; + const yaml = YAML.stringify(base); + if (deps.writeTempPolicy) { + try { + return deps.writeTempPolicy(yaml); + } catch { + return basePermissivePath; + } + } + let tmpPath: string | null = null; try { - const tmpPath = secureTempFile("nemoclaw-permissive-runtime", ".yaml"); - fs.writeFileSync(tmpPath, YAML.stringify(base), { mode: 0o600 }); + tmpPath = secureTempFile(TEMP_FILE_PREFIX, ".yaml"); + fs.writeFileSync(tmpPath, yaml, { mode: 0o600 }); return tmpPath; } catch { + // secureTempFile may have created an mkdtemp directory before + // writeFileSync failed. Clean it up so we do not leak a 0700 dir + // on /tmp every time the write path errors. + if (tmpPath) cleanupTempDir(tmpPath, TEMP_FILE_PREFIX); return basePermissivePath; } } diff --git a/test/permissive-runtime.test.ts b/test/permissive-runtime.test.ts index f3c24fa75cf..ddbd8a26dda 100644 --- a/test/permissive-runtime.test.ts +++ b/test/permissive-runtime.test.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; import { describe, it, expect, afterEach } from "vitest"; import YAML from "yaml"; @@ -19,6 +20,19 @@ const BASE_PERMISSIVE = YAML.stringify({ const tempFilesToClean: string[] = []; +function trackTempForCleanup(out: string, basePath: string): void { + // Defensive: if the helper degrades to the static base path we must + // never try to `rm -rf` its parent dir — that would target the + // user's checkout. Only enqueue paths that the helper actually + // produced via mkdtemp. + if (out === basePath) return; + const tempRoot = path.resolve(os.tmpdir()); + const parent = path.resolve(path.dirname(out)); + const rel = path.relative(tempRoot, parent); + if (rel === "" || rel.startsWith("..") || path.isAbsolute(rel)) return; + tempFilesToClean.push(out); +} + afterEach(() => { while (tempFilesToClean.length > 0) { const p = tempFilesToClean.pop(); @@ -45,7 +59,8 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { livePolicyYaml: liveYaml, readBasePolicy: () => BASE_PERMISSIVE, }); - tempFilesToClean.push(out); + trackTempForCleanup(out, "/unused-base.yaml"); + expect(out).not.toBe("/unused-base.yaml"); const result = YAML.parse(fs.readFileSync(out, "utf-8")); expect(result.filesystem_policy.read_write).toEqual( @@ -64,7 +79,8 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { livePolicyYaml: liveYaml, readBasePolicy: () => BASE_PERMISSIVE, }); - tempFilesToClean.push(out); + trackTempForCleanup(out, "/unused-base.yaml"); + expect(out).not.toBe("/unused-base.yaml"); const result = YAML.parse(fs.readFileSync(out, "utf-8")); expect(result.filesystem_policy.include_workdir).toBe(true); @@ -83,7 +99,8 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { livePolicyYaml: liveYaml, readBasePolicy: () => BASE_PERMISSIVE, }); - tempFilesToClean.push(out); + trackTempForCleanup(out, "/unused-base.yaml"); + expect(out).not.toBe("/unused-base.yaml"); const result = YAML.parse(fs.readFileSync(out, "utf-8")); expect(result.filesystem_policy.read_write).toContain("/tmp"); @@ -103,7 +120,8 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { livePolicyYaml: liveYaml, readBasePolicy: () => BASE_PERMISSIVE, }); - tempFilesToClean.push(out); + trackTempForCleanup(out, "/unused-base.yaml"); + expect(out).not.toBe("/unused-base.yaml"); const result = YAML.parse(fs.readFileSync(out, "utf-8")); const rwCount = result.filesystem_policy.read_write.filter((p: string) => p === "/tmp").length; @@ -160,4 +178,22 @@ describe("buildRuntimePermissivePolicy (#3942)", () => { }); expect(out).toBe(basePath); }); + + it("returns the static base path when temp-file write throws", () => { + const basePath = "/path/to/static.yaml"; + const liveYaml = YAML.stringify({ + filesystem_policy: { read_write: ["/proc"] }, + }); + let writeAttempts = 0; + const out = buildRuntimePermissivePolicy(basePath, { + livePolicyYaml: liveYaml, + readBasePolicy: () => BASE_PERMISSIVE, + writeTempPolicy: () => { + writeAttempts += 1; + throw new Error("ENOSPC: simulated /tmp full"); + }, + }); + expect(out).toBe(basePath); + expect(writeAttempts).toBe(1); + }); });