From b7127d6c073bc5728083b33bd32d391d3f70f1ef Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Wed, 1 Jul 2026 11:52:37 -0700 Subject: [PATCH 1/3] refactor(shields): isolate mutable config repair Signed-off-by: Carlos Villela (cherry picked from commit 717d6ef70feb4df1b807a6597c9635eae5709368) --- scripts/lib/normalize_mutable_config_perms.py | 3 ++ src/lib/actions/sandbox/exec.ts | 9 ++++ src/lib/shields/index.ts | 26 ++-------- src/lib/shields/mutable-config-repair.ts | 52 +++++++++++++++++++ 4 files changed, 67 insertions(+), 23 deletions(-) create mode 100644 src/lib/shields/mutable-config-repair.ts diff --git a/scripts/lib/normalize_mutable_config_perms.py b/scripts/lib/normalize_mutable_config_perms.py index ac1e72d812..9f0d7ee01f 100755 --- a/scripts/lib/normalize_mutable_config_perms.py +++ b/scripts/lib/normalize_mutable_config_perms.py @@ -808,6 +808,9 @@ def lock_recovery_baseline( BASELINE_NAME, dir_fd=root_fd, follow_symlinks=False ) except FileNotFoundError: + # Expected before the first successful post-override capture. There + # is no recovery source to lock yet, and normal startup must remain + # quiet; capture mode creates the baseline through a fresh inode. before = None if before is not None: diff --git a/src/lib/actions/sandbox/exec.ts b/src/lib/actions/sandbox/exec.ts index d895f74062..a729d5d4fc 100644 --- a/src/lib/actions/sandbox/exec.ts +++ b/src/lib/actions/sandbox/exec.ts @@ -129,6 +129,15 @@ function repairFailureDetail( * `nemoclaw exec` command boundary. OpenShell executes the requested * process directly, so the sandbox entrypoint's one-shot cleanup does not run * on this path. Hermes and custom agents are deliberately left unchanged. + * + * Each production inspect/repair call takes the cross-process, timer-bound + * shields transition lock and rechecks posture while holding it. The repair is + * idempotent, so two CLI processes may interleave only between those protected + * steps: they can repeat a repair or make one caller report conservative drift, + * but host-side repair mutations cannot overlap or weaken shields-up. A + * process-local mutex would not serialize separate CLI invocations, while a + * lock inside the sandbox-owned config tree would put lock authority on the + * wrong trust side. */ export function cleanupOpenClawAfterExec( sandboxName: string, diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 05bc01bde6..0abeddd6ec 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -85,6 +85,9 @@ const { inspectMutableConfigPerms: inspectMutableConfigPermsCore, repairMutableConfigPerms: repairMutableConfigPermsCore, }: typeof import("./mutable-config-perms") = require("./mutable-config-perms"); +const { + normalizeMutableOpenClawConfig, +}: typeof import("./mutable-config-repair") = require("./mutable-config-repair"); type MutableConfigPermsInspection = import("./mutable-config-perms").MutableConfigPermsInspection; type MutableConfigRepairResult = import("./mutable-config-perms").MutableConfigRepairResult; type ProcessIdentity = import("./timer-control").ProcessIdentity; @@ -98,7 +101,6 @@ const HERMES_RUNTIME_CONFIG_GUARD = "/usr/local/lib/nemoclaw/hermes-runtime-conf const HERMES_PYTHON = "/opt/hermes/.venv/bin/python"; const HERMES_RESTART_SEAL_STATE = "/run/nemoclaw/hermes-restart-seal.json"; const HERMES_CONFIG_HASH = "/etc/nemoclaw/hermes.config-hash"; -const MUTABLE_CONFIG_NORMALIZER = "/usr/local/lib/nemoclaw/normalize_mutable_config_perms.py"; const STATE_DIR_GUARD_TIMEOUT_MS = 15 * 60 * 1000; const OPENCLAW_CONFIG_GUARD_TIMEOUT_MS = 6 * 60 * 1000; const HERMES_CONFIG_GUARD_TIMEOUT_MS = 11 * 60 * 1000; @@ -323,28 +325,6 @@ function privilegedSandboxExecCapture(sandboxName: string, cmd: string[], timeou }).trim(); } -function sandboxIdentityId(sandboxName: string, flag: "-u" | "-g"): string { - const id = privilegedSandboxExecCapture(sandboxName, ["/usr/bin/id", flag, "sandbox"]); - if (!/^[1-9][0-9]*$/.test(id)) { - const kind = flag === "-u" ? "UID" : "GID"; - throw new Error(`sandbox identity lookup returned an invalid ${kind}`); - } - return id; -} - -function normalizeMutableOpenClawConfig(sandboxName: string, configDir: string): void { - const sandboxUid = sandboxIdentityId(sandboxName, "-u"); - const sandboxGid = sandboxIdentityId(sandboxName, "-g"); - privilegedSandboxExec(sandboxName, [ - "/usr/bin/python3", - "-I", - MUTABLE_CONFIG_NORMALIZER, - configDir, - sandboxUid, - sandboxGid, - ]); -} - function hermesShieldsGuardArgs( action: string, target: AgentConfigTarget, diff --git a/src/lib/shields/mutable-config-repair.ts b/src/lib/shields/mutable-config-repair.ts new file mode 100644 index 0000000000..9645a42f86 --- /dev/null +++ b/src/lib/shields/mutable-config-repair.ts @@ -0,0 +1,52 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +const dockerExec: typeof import("../adapters/docker/exec") = require("../adapters/docker/exec"); +const privilegedExecModule: typeof import("../sandbox/privileged-exec") = require("../sandbox/privileged-exec"); + +const MUTABLE_CONFIG_NORMALIZER = "/usr/local/lib/nemoclaw/normalize_mutable_config_perms.py"; + +function runPrivileged(sandboxName: string, cmd: string[], timeout = 15000): void { + dockerExec.dockerExecFileSync( + privilegedExecModule.privilegedSandboxExecArgv(sandboxName, cmd, false, true), + { + stdio: ["ignore", "pipe", "pipe"], + timeout, + }, + ); +} + +function privilegedExecCapture(sandboxName: string, cmd: string[], timeout = 15000): string { + return dockerExec + .dockerExecFileSync( + privilegedExecModule.privilegedSandboxExecArgv(sandboxName, cmd, false, true), + { + stdio: ["ignore", "pipe", "pipe"], + timeout, + }, + ) + .trim(); +} + +function sandboxIdentityId(sandboxName: string, flag: "-u" | "-g"): string { + const id = privilegedExecCapture(sandboxName, ["/usr/bin/id", flag, "sandbox"]); + if (!/^[1-9][0-9]*$/.test(id)) { + const kind = flag === "-u" ? "UID" : "GID"; + throw new Error(`sandbox identity lookup returned an invalid ${kind}`); + } + return id; +} + +/** Apply the mutable OpenClaw contract through the image's trusted helper. */ +export function normalizeMutableOpenClawConfig(sandboxName: string, configDir: string): void { + const sandboxUid = sandboxIdentityId(sandboxName, "-u"); + const sandboxGid = sandboxIdentityId(sandboxName, "-g"); + runPrivileged(sandboxName, [ + "/usr/bin/python3", + "-I", + MUTABLE_CONFIG_NORMALIZER, + configDir, + sandboxUid, + sandboxGid, + ]); +} From 66546155f403631d5eb729730619c111cabd427e Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Wed, 1 Jul 2026 12:24:12 -0700 Subject: [PATCH 2/3] test(shields): pin mutable repair privilege boundary Signed-off-by: Carlos Villela --- src/lib/shields/mutable-config-repair.test.ts | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 src/lib/shields/mutable-config-repair.test.ts diff --git a/src/lib/shields/mutable-config-repair.test.ts b/src/lib/shields/mutable-config-repair.test.ts new file mode 100644 index 0000000000..7ee56f3d57 --- /dev/null +++ b/src/lib/shields/mutable-config-repair.test.ts @@ -0,0 +1,128 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { createRequire } from "node:module"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +const NORMALIZER = "/usr/local/lib/nemoclaw/normalize_mutable_config_perms.py"; +const requireSource = createRequire(import.meta.url); + +type DockerExecModule = typeof import("../adapters/docker/exec"); +type MutableConfigRepairModule = typeof import("./mutable-config-repair"); +type PrivilegedExecModule = typeof import("../sandbox/privileged-exec"); + +let dockerExec: DockerExecModule; +let normalizeMutableOpenClawConfig: MutableConfigRepairModule["normalizeMutableOpenClawConfig"]; +let privilegedExec: PrivilegedExecModule; + +function mockPrivilegedArgv() { + return vi + .spyOn(privilegedExec, "privilegedSandboxExecArgv") + .mockImplementation((_sandboxName, cmd) => ["privileged", ...cmd]); +} + +describe("mutable OpenClaw config repair", () => { + beforeEach(() => { + delete require.cache[requireSource.resolve("./mutable-config-repair.js")]; + dockerExec = requireSource("../adapters/docker/exec.js"); + privilegedExec = requireSource("../sandbox/privileged-exec.js"); + ({ normalizeMutableOpenClawConfig } = requireSource("./mutable-config-repair.js")); + }); + + afterEach(() => { + vi.restoreAllMocks(); + delete require.cache[requireSource.resolve("./mutable-config-repair.js")]; + }); + + it("sanitizes every privileged identity and normalizer invocation", () => { + const privilegedArgv = mockPrivilegedArgv(); + const dockerExecFileSync = vi + .spyOn(dockerExec, "dockerExecFileSync") + .mockReturnValueOnce("1000\n") + .mockReturnValueOnce("1001\n") + .mockReturnValue(""); + + normalizeMutableOpenClawConfig("alpha", "/sandbox/.openclaw"); + + expect(privilegedArgv.mock.calls).toEqual([ + ["alpha", ["/usr/bin/id", "-u", "sandbox"], false, true], + ["alpha", ["/usr/bin/id", "-g", "sandbox"], false, true], + [ + "alpha", + ["/usr/bin/python3", "-I", NORMALIZER, "/sandbox/.openclaw", "1000", "1001"], + false, + true, + ], + ]); + expect(dockerExecFileSync).toHaveBeenCalledTimes(3); + expect(dockerExecFileSync.mock.calls.map(([argv]) => argv)).toEqual([ + ["privileged", "/usr/bin/id", "-u", "sandbox"], + ["privileged", "/usr/bin/id", "-g", "sandbox"], + ["privileged", "/usr/bin/python3", "-I", NORMALIZER, "/sandbox/.openclaw", "1000", "1001"], + ]); + expect(dockerExecFileSync.mock.calls.map(([, options]) => options)).toEqual([ + { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, + { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, + { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, + ]); + }); + + it("rejects an invalid sandbox UID before the GID or normalizer runs", () => { + const privilegedArgv = mockPrivilegedArgv(); + const dockerExecFileSync = vi.spyOn(dockerExec, "dockerExecFileSync").mockReturnValue("0\n"); + + expect(() => normalizeMutableOpenClawConfig("alpha", "/sandbox/.openclaw")).toThrow( + "sandbox identity lookup returned an invalid UID", + ); + expect(privilegedArgv).toHaveBeenCalledOnce(); + expect(privilegedArgv).toHaveBeenCalledWith( + "alpha", + ["/usr/bin/id", "-u", "sandbox"], + false, + true, + ); + expect(dockerExecFileSync).toHaveBeenCalledOnce(); + }); + + it("rejects an invalid sandbox GID before the normalizer runs", () => { + const privilegedArgv = mockPrivilegedArgv(); + const dockerExecFileSync = vi + .spyOn(dockerExec, "dockerExecFileSync") + .mockReturnValueOnce("1000\n") + .mockReturnValueOnce("not-a-gid\n"); + + expect(() => normalizeMutableOpenClawConfig("alpha", "/sandbox/.openclaw")).toThrow( + "sandbox identity lookup returned an invalid GID", + ); + expect(privilegedArgv).toHaveBeenCalledTimes(2); + expect(privilegedArgv).not.toHaveBeenCalledWith( + "alpha", + expect.arrayContaining([NORMALIZER]), + false, + true, + ); + expect(dockerExecFileSync).toHaveBeenCalledTimes(2); + }); + + it("propagates a trusted normalizer execution failure", () => { + const privilegedArgv = mockPrivilegedArgv(); + const failure = new Error("docker exec failed"); + const dockerExecFileSync = vi + .spyOn(dockerExec, "dockerExecFileSync") + .mockReturnValueOnce("1000\n") + .mockReturnValueOnce("1001\n") + .mockImplementationOnce(() => { + throw failure; + }); + + expect(() => normalizeMutableOpenClawConfig("alpha", "/sandbox/.openclaw")).toThrow(failure); + expect(privilegedArgv).toHaveBeenLastCalledWith( + "alpha", + ["/usr/bin/python3", "-I", NORMALIZER, "/sandbox/.openclaw", "1000", "1001"], + false, + true, + ); + expect(dockerExecFileSync).toHaveBeenCalledTimes(3); + }); +}); From a32d29236b596b67ba8ffc1773b24ec72a3994dd Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Wed, 1 Jul 2026 14:46:32 -0700 Subject: [PATCH 3/3] fix(shields): bound mutable repair worker Signed-off-by: Carlos Villela --- src/lib/shields/mutable-config-repair.test.ts | 36 ++++++++++++++++--- src/lib/shields/mutable-config-repair.ts | 33 ++++++++++++----- src/lib/shields/openclaw-transition.test.ts | 6 +++- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/src/lib/shields/mutable-config-repair.test.ts b/src/lib/shields/mutable-config-repair.test.ts index 7ee56f3d57..e1ebcd1a97 100644 --- a/src/lib/shields/mutable-config-repair.test.ts +++ b/src/lib/shields/mutable-config-repair.test.ts @@ -6,6 +6,7 @@ import { createRequire } from "node:module"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const NORMALIZER = "/usr/local/lib/nemoclaw/normalize_mutable_config_perms.py"; +const NORMALIZER_WATCHDOG = ["/usr/bin/timeout", "--signal=TERM", "--kill-after=5s", "15s"]; const requireSource = createRequire(import.meta.url); type DockerExecModule = typeof import("../adapters/docker/exec"); @@ -35,7 +36,7 @@ describe("mutable OpenClaw config repair", () => { delete require.cache[requireSource.resolve("./mutable-config-repair.js")]; }); - it("sanitizes every privileged identity and normalizer invocation", () => { + it("sanitizes identity probes and watchdogs the privileged normalizer", () => { const privilegedArgv = mockPrivilegedArgv(); const dockerExecFileSync = vi .spyOn(dockerExec, "dockerExecFileSync") @@ -50,7 +51,15 @@ describe("mutable OpenClaw config repair", () => { ["alpha", ["/usr/bin/id", "-g", "sandbox"], false, true], [ "alpha", - ["/usr/bin/python3", "-I", NORMALIZER, "/sandbox/.openclaw", "1000", "1001"], + [ + ...NORMALIZER_WATCHDOG, + "/usr/bin/python3", + "-I", + NORMALIZER, + "/sandbox/.openclaw", + "1000", + "1001", + ], false, true, ], @@ -59,12 +68,21 @@ describe("mutable OpenClaw config repair", () => { expect(dockerExecFileSync.mock.calls.map(([argv]) => argv)).toEqual([ ["privileged", "/usr/bin/id", "-u", "sandbox"], ["privileged", "/usr/bin/id", "-g", "sandbox"], - ["privileged", "/usr/bin/python3", "-I", NORMALIZER, "/sandbox/.openclaw", "1000", "1001"], + [ + "privileged", + ...NORMALIZER_WATCHDOG, + "/usr/bin/python3", + "-I", + NORMALIZER, + "/sandbox/.openclaw", + "1000", + "1001", + ], ]); expect(dockerExecFileSync.mock.calls.map(([, options]) => options)).toEqual([ { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, - { stdio: ["ignore", "pipe", "pipe"], timeout: 15000 }, + { stdio: ["ignore", "pipe", "pipe"], timeout: 25000 }, ]); }); @@ -119,7 +137,15 @@ describe("mutable OpenClaw config repair", () => { expect(() => normalizeMutableOpenClawConfig("alpha", "/sandbox/.openclaw")).toThrow(failure); expect(privilegedArgv).toHaveBeenLastCalledWith( "alpha", - ["/usr/bin/python3", "-I", NORMALIZER, "/sandbox/.openclaw", "1000", "1001"], + [ + ...NORMALIZER_WATCHDOG, + "/usr/bin/python3", + "-I", + NORMALIZER, + "/sandbox/.openclaw", + "1000", + "1001", + ], false, true, ); diff --git a/src/lib/shields/mutable-config-repair.ts b/src/lib/shields/mutable-config-repair.ts index 9645a42f86..4494b496a1 100644 --- a/src/lib/shields/mutable-config-repair.ts +++ b/src/lib/shields/mutable-config-repair.ts @@ -5,6 +5,13 @@ const dockerExec: typeof import("../adapters/docker/exec") = require("../adapter const privilegedExecModule: typeof import("../sandbox/privileged-exec") = require("../sandbox/privileged-exec"); const MUTABLE_CONFIG_NORMALIZER = "/usr/local/lib/nemoclaw/normalize_mutable_config_perms.py"; +const MUTABLE_CONFIG_NORMALIZER_HOST_TIMEOUT_MS = 25000; +const MUTABLE_CONFIG_NORMALIZER_WATCHDOG = [ + "/usr/bin/timeout", + "--signal=TERM", + "--kill-after=5s", + "15s", +] as const; function runPrivileged(sandboxName: string, cmd: string[], timeout = 15000): void { dockerExec.dockerExecFileSync( @@ -30,6 +37,8 @@ function privilegedExecCapture(sandboxName: string, cmd: string[], timeout = 150 function sandboxIdentityId(sandboxName: string, flag: "-u" | "-g"): string { const id = privilegedExecCapture(sandboxName, ["/usr/bin/id", flag, "sandbox"]); + // Keep the ownership target non-root so privileged repair cannot become a + // confused-deputy path. if (!/^[1-9][0-9]*$/.test(id)) { const kind = flag === "-u" ? "UID" : "GID"; throw new Error(`sandbox identity lookup returned an invalid ${kind}`); @@ -41,12 +50,20 @@ function sandboxIdentityId(sandboxName: string, flag: "-u" | "-g"): string { export function normalizeMutableOpenClawConfig(sandboxName: string, configDir: string): void { const sandboxUid = sandboxIdentityId(sandboxName, "-u"); const sandboxGid = sandboxIdentityId(sandboxName, "-g"); - runPrivileged(sandboxName, [ - "/usr/bin/python3", - "-I", - MUTABLE_CONFIG_NORMALIZER, - configDir, - sandboxUid, - sandboxGid, - ]); + // The in-sandbox watchdog signals the Python process group and reaps its + // direct child before the longer host-side Docker timeout can release the + // shields transition lock. + runPrivileged( + sandboxName, + [ + ...MUTABLE_CONFIG_NORMALIZER_WATCHDOG, + "/usr/bin/python3", + "-I", + MUTABLE_CONFIG_NORMALIZER, + configDir, + sandboxUid, + sandboxGid, + ], + MUTABLE_CONFIG_NORMALIZER_HOST_TIMEOUT_MS, + ); } diff --git a/src/lib/shields/openclaw-transition.test.ts b/src/lib/shields/openclaw-transition.test.ts index 4e57615ab6..d258a4d56c 100644 --- a/src/lib/shields/openclaw-transition.test.ts +++ b/src/lib/shields/openclaw-transition.test.ts @@ -138,7 +138,7 @@ describe("OpenClaw shields top-config transaction", () => { switch (argv[0]) { case "/usr/bin/id": return "1000\n"; - case "/usr/bin/python3": + case "/usr/bin/timeout": return ""; default: throw new Error(`unexpected privileged command: ${argv.join(" ")}`); @@ -156,6 +156,10 @@ describe("OpenClaw shields top-config transaction", () => { ["/usr/bin/id", "-u", "sandbox"], ["/usr/bin/id", "-g", "sandbox"], [ + "/usr/bin/timeout", + "--signal=TERM", + "--kill-after=5s", + "15s", "/usr/bin/python3", "-I", "/usr/local/lib/nemoclaw/normalize_mutable_config_perms.py",