Repository navigation
fix(shields): union live policy into permissive before applying #3976
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
7afa33f
fix(shields): union live policy into permissive before applying (#3942)
laitingsheng 73a4fe5
refactor(shields): tighten permissive-runtime helper per Codex review
laitingsheng ac6b035
fix(shields): tighten permissive-runtime write-failure path + tests
laitingsheng File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<string, unknown>) | ||
| : ((base.filesystem_policy = {} as Record<string, unknown>), | ||
| base.filesystem_policy as Record<string, unknown>); | ||
|
|
||
| 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<string, unknown> | null { | ||
| try { | ||
| const parsed = YAML.parse(text); | ||
| if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { | ||
| return parsed as Record<string, unknown>; | ||
| } | ||
| } catch { | ||
| return null; | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| function readStringList( | ||
| root: Record<string, unknown> | null, | ||
| key: "read_only" | "read_write", | ||
| ): string[] { | ||
| const fsPolicy = root?.filesystem_policy; | ||
| if (!fsPolicy || typeof fsPolicy !== "object") return []; | ||
| const value = (fsPolicy as Record<string, unknown>)[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; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.