diff --git a/scripts/openclaw-config-guard.py b/scripts/openclaw-config-guard.py index 078420276d1..926cd572ef1 100755 --- a/scripts/openclaw-config-guard.py +++ b/scripts/openclaw-config-guard.py @@ -2941,6 +2941,32 @@ def _verify_locked_files( ) +def _is_resealable_config_hash_permissions_drift( + snapshots: tuple[FileSnapshot, FileSnapshot], identity: Identity +) -> bool: + config, hash_record = snapshots + blocking_flags = FS_IMMUTABLE_FL | FS_APPEND_FL + config_stays_locked = ( + config.uid == identity.root_uid + and config.gid == identity.root_gid + and config.mode == 0o444 + and not ( + config.inode_flags is not None + and config.inode_flags & blocking_flags + ) + ) + hash_has_known_reconciler_posture = ( + hash_record.uid == identity.sandbox_uid + and hash_record.gid == identity.sandbox_gid + and hash_record.mode == 0o660 + and not ( + hash_record.inode_flags is not None + and hash_record.inode_flags & blocking_flags + ) + ) + return config_stays_locked and hash_has_known_reconciler_posture + + def _verify_locked_posture( opened: OpenConfig, snapshots: tuple[FileSnapshot, FileSnapshot], @@ -3266,6 +3292,12 @@ def _preflight_restart(opened: OpenConfig, identity: Identity) -> None: _hash_synthesized = False +# Set True when a lock transition re-seals a perms-only drift on an already-locked +# config (an in-sandbox reconciler re-permissioned a canonical file after the +# lock: #4663 / #7985). Surfaced in the result JSON so the host can report the +# self-heal instead of leaving it invisible. +_resealed_drift = False + def _write_hash_record(opened: OpenConfig, config_data: bytes, identity: Identity) -> None: digest = hashlib.sha256(config_data).hexdigest() @@ -3426,6 +3458,7 @@ def _transition( *, quarantine_untrusted: bool = False, ) -> None: + global _resealed_drift if action == "lock": if _has_clamped_locked_dir_posture(opened, identity): pair = _snapshot_pair(opened) @@ -3435,8 +3468,30 @@ def _transition( # A restart journal may still need rootfs-authenticated cleanup. _settle_pending_transaction_for_lock(opened, identity) pair = _snapshot_pair(opened) - _verify_locked_files(opened, pair, identity) - return + try: + _verify_locked_files(opened, pair, identity) + return + except GuardError as verify_error: + if verify_error.code != "config-not-locked" or not ( + _is_resealable_config_hash_permissions_drift(pair, identity) + ): + raise + # The config remains root-owned and read-only, while the + # in-sandbox reconciler re-permissioned only .config-hash after + # the lock (#4663 / #7985). Re-seal below instead of failing + # closed: the root-owned directory prevents replacement, the + # config cannot be written, and _snapshot_pair verified that the + # sidecar still authenticates those config bytes. + # Source: the writer is the upstream OpenClaw in-sandbox gateway/ + # doctor perm-normalizer, which NemoClaw does not own, so the + # correct fix is this host-authenticated relock re-seal, not a + # change to that writer, which this repo cannot make. Removal + # condition: delete this path once the lock is durably immutable on + # every platform (chattr +i, unavailable on overlayfs today) or the + # upstream reconciler stops re-permissioning an already-locked + # config; either fully closes the #4663 relock settle-window race + # that this branch only narrows. + _resealed_drift = True freeze_started = False try: freeze_started = True @@ -4273,6 +4328,7 @@ def main(argv: list[str] | None = None) -> int: "chattrApplied": False, **({"configSha256": new_digest} if new_digest is not None else {}), **({"hashSynthesized": True} if _hash_synthesized else {}), + **({"resealedDrift": True} if _resealed_drift else {}), **({"recovery": recovery} if recovery is not None else {}), **( {"originalLocked": original_locked} diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 83b664d6337..cf96a9594df 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -1112,6 +1112,15 @@ function transitionOpenClawTopConfig( `Config not ${action === "unlock" ? "unlocked" : "locked"}: ${result.issues.join(", ")}`, ); } + if (result.resealedDrift) { + // The guard found an already-locked config whose canonical file had drifted + // perms-only (a reconciler re-permissioned it after the lock) and re-sealed + // it in place instead of failing closed (#4663 / #7985). Surface the + // self-heal so a rebuild/relock does not fix drift invisibly. + console.log( + " Re-sealed a perms-only config-lock drift (config dir stays root-owned; contents intact).", + ); + } return result.chattrApplied; } diff --git a/src/lib/shields/openclaw-config-lock.test.ts b/src/lib/shields/openclaw-config-lock.test.ts index 4a584aadd74..8f6e43a4bb5 100644 --- a/src/lib/shields/openclaw-config-lock.test.ts +++ b/src/lib/shields/openclaw-config-lock.test.ts @@ -367,4 +367,33 @@ describe("OpenClaw top-config guard host wiring", () => { expect(parsed.hashSynthesized).toBe(true); expect(parseOpenClawConfigGuardOutput("lock", plain).hashSynthesized).toBeUndefined(); }); + + it("propagates the guard's re-sealed-drift marker on a successful lock", () => { + const resealed: PrivilegedExecResult = { + status: 0, + signal: null, + stdout: `${JSON.stringify({ + type: "result", + action: "lock", + status: "ok", + configDir: OPENCLAW_CONFIG_DIR, + files: ["openclaw.json", ".config-hash"], + chattrApplied: false, + resealedDrift: true, + })}\n`, + stderr: "", + }; + const plain: PrivilegedExecResult = { + status: 0, + signal: null, + stdout: `${success("lock")}\n`, + stderr: "", + }; + + const parsed = parseOpenClawConfigGuardOutput("lock", resealed); + + expect(parsed.issues).toEqual([]); + expect(parsed.resealedDrift).toBe(true); + expect(parseOpenClawConfigGuardOutput("lock", plain).resealedDrift).toBeUndefined(); + }); }); diff --git a/src/lib/shields/openclaw-config-lock.ts b/src/lib/shields/openclaw-config-lock.ts index 94e15f4ea3b..1321c90888b 100644 --- a/src/lib/shields/openclaw-config-lock.ts +++ b/src/lib/shields/openclaw-config-lock.ts @@ -65,6 +65,7 @@ type GuardSummary = { chattrApplied?: boolean; configSha256?: string; hashSynthesized?: boolean; + resealedDrift?: boolean; recovery?: string; originalLocked?: boolean; }; @@ -74,6 +75,7 @@ export type OpenClawConfigGuardResult = { chattrApplied: boolean; configSha256?: string; hashSynthesized?: boolean; + resealedDrift?: boolean; recovery?: string; originalLocked?: boolean; }; @@ -230,6 +232,7 @@ export function parseOpenClawConfigGuardOutput( (record.chattrApplied === undefined || typeof record.chattrApplied === "boolean") && (record.configSha256 === undefined || typeof record.configSha256 === "string") && (record.hashSynthesized === undefined || typeof record.hashSynthesized === "boolean") && + (record.resealedDrift === undefined || typeof record.resealedDrift === "boolean") && (record.recovery === undefined || typeof record.recovery === "string") && (record.originalLocked === undefined || typeof record.originalLocked === "boolean") ) { @@ -302,6 +305,7 @@ export function parseOpenClawConfigGuardOutput( ...(summary?.status === "ok" && summary.hashSynthesized === true ? { hashSynthesized: true } : {}), + ...(summary?.status === "ok" && summary.resealedDrift === true ? { resealedDrift: true } : {}), ...(summary?.status === "ok" && summary.recovery ? { recovery: summary.recovery } : {}), ...(summary?.status === "ok" && typeof summary.originalLocked === "boolean" ? { originalLocked: summary.originalLocked } diff --git a/test/e2e/live/shields-config.test.ts b/test/e2e/live/shields-config.test.ts index 08771262b23..8e58e981ef1 100644 --- a/test/e2e/live/shields-config.test.ts +++ b/test/e2e/live/shields-config.test.ts @@ -230,6 +230,7 @@ test("shields-config: live shields up/down locks config and detects drift", { "establish the mutable unified OpenClaw config", "lock config and workspace and inspect redaction", "detect host-root config drift and refuse resealing", + "re-seal a perms-only .config-hash drift instead of failing closed", "unlock shields and inspect the audit trail", "recover shields after a dead restore timer", "reject duplicate shields transitions", @@ -246,6 +247,7 @@ test("shields-config: live shields up/down locks config and detects drift", { "documented nemoclaw exec doctor path preserves 2770/660 and gateway writes", "shields up locks config/workspace and config get redacts secrets", "host-root chmod-write-chmod tamper is detected as content drift", + "a perms-only .config-hash drift is re-sealed by shields up, not failed closed", "shields down restores mutable modes and records audit JSONL", "dead auto-restore timer inline recovery re-locks config and .config-hash", "double shields-up/down operations are rejected", @@ -578,6 +580,55 @@ test("shields-config: live shields up/down locks config and detects drift", { expect(statusRestored.exitCode, resultText(statusRestored)).toBe(0); expect(statusRestored.stdout).toContain("Shields: UP (lockdown active)"); + progress.phase("re-seal a perms-only .config-hash drift instead of failing closed"); + // #7985/#4663: an in-sandbox privileged reconciler (OpenClaw gateway / doctor + // perm-normalization) can re-permission .config-hash back to group-writable + // AFTER the lock without touching its bytes. That perms-only drift must be + // re-sealed by the drift-repair `shields up`, not fail closed with + // "restart seal requires the exact shields-locked file posture" (which + // stranded host state UNLOCKED while the tree stayed root-locked). The bytes + // are untouched here, so it is a launderable perms drift, not content drift. + const permsDrift = await host.command( + "bash", + [ + "-lc", + `docker exec -u 0 ${containerId} sh -c 'chattr -i ${CONFIG_HASH_PATH} 2>/dev/null || true; chmod 660 ${CONFIG_HASH_PATH} && chown sandbox:sandbox ${CONFIG_HASH_PATH}'`, + ], + { + artifactName: "phase-5c-config-hash-perms-only-drift", + env: commandEnv(), + timeoutMs: 30_000, + }, + ); + expect(permsDrift.exitCode, resultText(permsDrift)).toBe(0); + + const hashDrifted = await statPath(sandbox, CONFIG_HASH_PATH, "phase-5c-hash-perms-after-drift"); + expect(hashDrifted).toMatchObject({ mode: "660", owner: "sandbox:sandbox" }); + + // The drift-repair relock reaches the OpenClaw config guard's already-locked + // branch. The fix re-seals the perms-only drift; before it, the guard + // rejected with config-not-locked and the relock could never re-apply. + const reseal = await runNemoclaw(host, [SANDBOX_NAME, "shields", "up"], { + artifactName: "phase-5c-shields-up-reseals-perms-drift", + }); + expect(reseal.exitCode, resultText(reseal)).toBe(0); + // The guard surfaces the self-heal so a relock/rebuild does not fix a + // perms-only drift invisibly (#4663 / #7985 observability). + expect(resultText(reseal)).toContain("Re-sealed a perms-only config-lock drift"); + + const hashResealed = await statPath( + sandbox, + CONFIG_HASH_PATH, + "phase-5c-hash-perms-after-reseal", + ); + expect(hashResealed).toMatchObject({ mode: "444", owner: "root:root" }); + + const statusResealed = await runNemoclaw(host, [SANDBOX_NAME, "shields", "status"], { + artifactName: "phase-5c-shields-status-after-reseal", + }); + expect(statusResealed.exitCode, resultText(statusResealed)).toBe(0); + expect(statusResealed.stdout).toContain("Shields: UP (lockdown active)"); + progress.phase("unlock shields and inspect the audit trail"); const shieldsDown = await runNemoclaw( host, @@ -730,6 +781,7 @@ test("shields-config: live shields up/down locks config and detects drift", { shieldsUpLock: true, configGetRedaction: true, contentDriftDetection: true, + permsOnlyDriftReseal: true, shieldsDownMutableRestore: true, auditTrail: true, deadTimerInlineAutoRestore: true, diff --git a/test/openclaw-config-guard-lock-reseal.test.ts b/test/openclaw-config-guard-lock-reseal.test.ts new file mode 100644 index 00000000000..efac5be9953 --- /dev/null +++ b/test/openclaw-config-guard-lock-reseal.test.ts @@ -0,0 +1,197 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// _transition("lock") re-seals a perms-only drifted locked pair (#4663 / #7985). +// +// When a rebuild's closing shields relock re-confirms an already-locked +// /sandbox/.openclaw, an in-sandbox privileged reconciler (OpenClaw gateway / +// doctor perm-normalization) may have re-permissioned .config-hash back to +// 660 sandbox:sandbox in the meantime. The locked *directory* posture then +// routes into the verify-only branch, which used to raise config-not-locked and +// strand the sandbox with host shields state UNLOCKED while the state +// directories stayed root-locked (#7985: skill install fails, agent turns +// EACCES, `shields down` refuses). The fix lets that perms-only drift fall +// through to the freeze/install re-seal path. Content or structural drift is +// caught earlier by _snapshot_pair's hash check and still fails closed. +// +// The guard needs euid 0 and the fixed /sandbox/.openclaw path, so it cannot run +// for real here. This drives _transition through a Python harness that replaces +// every filesystem helper with a recording stub, asserting which branch runs. + +import { spawnSync } from "node:child_process"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +const guardPath = path.join(import.meta.dirname, "..", "scripts", "openclaw-config-guard.py"); + +// Loads the guard module, neutralizes the filesystem helpers _transition("lock") +// delegates to (recording which ran), then exercises one scenario named by argv. +// Prints "OK" and exits 0 on success; raises (non-zero exit) on any mismatch. +const HARNESS = String.raw` +import importlib.util, sys + +guard_path, scenario = sys.argv[1], sys.argv[2] + +spec = importlib.util.spec_from_file_location("openclaw_config_guard", guard_path) +g = importlib.util.module_from_spec(spec) +# KW_ONLY dataclasses in the guard resolve their own module from sys.modules +# during class creation, so register before exec. +sys.modules[spec.name] = g +spec.loader.exec_module(g) + +ran = [] +CONFIG_HASH = "/sandbox/.openclaw/.config-hash" +classify_resealable_drift = g._is_resealable_config_hash_permissions_drift +identity = g.Identity(root_uid=0, root_gid=0, sandbox_uid=1000, sandbox_gid=1000) + +class Opened: + config_path = "/sandbox/.openclaw" + +opened = Opened() + +def snapshot(name, uid, gid, mode, flags=0): + return g.FileSnapshot(name, 1, 1, uid, gid, mode, 0, 0, 0, 1, b"x", (), flags) + +locked_pair = ( + snapshot("openclaw.json", 0, 0, 0o444), + snapshot(".config-hash", 0, 0, 0o444), +) +drifted_hash_pair = ( + locked_pair[0], + snapshot(".config-hash", 1000, 1000, 0o660), +) +writable_config_pair = ( + snapshot("openclaw.json", 1000, 1000, 0o660), + drifted_hash_pair[1], +) + +def stub(name, result=None): + def _run(*a, **k): + ran.append(name) + return result + return _run + +def raising(name, code): + def _run(*a, **k): + ran.append(name) + raise g.GuardError(code, CONFIG_HASH, code) + return _run + +def check(cond, msg): + if not cond: + raise SystemExit("CHECK FAILED [%s]: %s ran=%r" % (scenario, msg, ran)) + +# Locked directory posture; every filesystem helper neutralized. +g._has_clamped_locked_dir_posture = lambda *a, **k: False +g._has_locked_dir_posture = lambda *a, **k: True +g._settle_pending_transaction_for_lock = stub("settle") +pair_results = iter( + [drifted_hash_pair, locked_pair] + if scenario == "perms-drift-reseals" + else [writable_config_pair] + if scenario == "unsafe-file-posture-reraised" + else [locked_pair] +) +def snapshot_pair(*a, **k): + ran.append("snapshot_pair") + return next(pair_results) +g._snapshot_pair = snapshot_pair +g._freeze = stub("freeze") +g._repair_absent_hash_for_lock = stub("repair_hash") +g._snapshot_raw_pair = stub("snapshot_raw", ("raw-a", "raw-b")) +g._canonical_targets = stub("canonical", (("t-a", "t-b"), "digest")) +g._install_stored_pair = stub("install") +g._commit_locked_dirs = stub("commit") +g._force_fail_closed_lock = stub("fail_closed", []) + +def lock(): + return g._transition("lock", opened, identity) + +if scenario == "perms-drift-reseals": + lock() # The real classifier must route only the known drift to re-seal. + check("freeze" in ran and "install" in ran and "commit" in ran, "expected re-seal") + check(g._resealed_drift is True, "expected resealedDrift flag set for the result JSON") + check("fail_closed" not in ran, "re-seal path must not fall into fail-closed") +elif scenario == "other-error-reraised": + g._verify_locked_files = raising("verify", "startup-not-ready") + code = None + try: + lock() + except g.GuardError as e: + code = e.code + check(code == "startup-not-ready", "expected re-raise, got %r" % code) + check("install" not in ran, "must not re-seal") +elif scenario == "unsafe-file-posture-reraised": + code = None + try: + lock() + except g.GuardError as e: + code = e.code + check(code == "config-not-locked", "expected unsafe posture rejection, got %r" % code) + check("install" not in ran, "must not re-seal an unsafe file posture") +elif scenario == "classifies-only-known-hash-drift": + locked_config, drifted_hash = drifted_hash_pair + check(classify_resealable_drift((locked_config, drifted_hash), identity), "expected known drift") + writable_config = snapshot("openclaw.json", 1000, 1000, 0o660) + check(not classify_resealable_drift((writable_config, drifted_hash), identity), "writable config") + unexpected_hash = snapshot(".config-hash", 0, 0, 0o644) + check(not classify_resealable_drift((locked_config, unexpected_hash), identity), "unknown hash") + flagged_hash = snapshot(".config-hash", 1000, 1000, 0o660, g.FS_IMMUTABLE_FL) + check(not classify_resealable_drift((locked_config, flagged_hash), identity), "flagged hash") +elif scenario == "content-drift-fails-closed": + g._snapshot_pair = raising("snapshot_pair", "config-hash-mismatch") + code = None + try: + lock() + except g.GuardError as e: + code = e.code + check(code == "config-hash-mismatch", "expected fail-closed, got %r" % code) + check("verify" not in ran and "install" not in ran, "must not verify or re-seal") +elif scenario == "clean-verify-only": + lock() # clean locked pair: verify-only, no raise + check("snapshot_pair" in ran and "freeze" not in ran and "install" not in ran, "expected verify-only") + check(g._resealed_drift is False, "clean pair must not flag a reseal") +else: + raise SystemExit("unknown scenario: " + scenario) + +print("OK") +`; + +function runScenario(scenario: string) { + const result = spawnSync("python3", ["-c", HARNESS, guardPath, scenario], { + encoding: "utf8", + env: { ...process.env, PYTHONDONTWRITEBYTECODE: "1" }, + }); + expect( + result.status, + `guard harness '${scenario}' exited ${String(result.status)}:\n${result.stderr}${result.stdout}`, + ).toBe(0); + return result.stdout.trim(); +} + +describe("openclaw-config-guard lock re-seal on perms-only drift (#7985)", () => { + it("re-seals a perms-only .config-hash drift instead of failing closed", () => { + expect(runScenario("perms-drift-reseals")).toBe("OK"); + }); + + it("re-raises a guard error that is not config-not-locked", () => { + expect(runScenario("other-error-reraised")).toBe("OK"); + }); + + it("rejects file-level drift outside the known .config-hash posture", () => { + expect(runScenario("unsafe-file-posture-reraised")).toBe("OK"); + }); + + it("classifies only the known hash-sidecar permission drift as recoverable", () => { + expect(runScenario("classifies-only-known-hash-drift")).toBe("OK"); + }); + + it("still fails closed on content or structural drift", () => { + expect(runScenario("content-drift-fails-closed")).toBe("OK"); + }); + + it("leaves an unmodified locked pair verify-only", () => { + expect(runScenario("clean-verify-only")).toBe("OK"); + }); +});