diff --git a/docs/reference/commands-nemohermes.mdx b/docs/reference/commands-nemohermes.mdx index 3b78641cf8b..89cf7cb1c7b 100644 --- a/docs/reference/commands-nemohermes.mdx +++ b/docs/reference/commands-nemohermes.mdx @@ -1893,6 +1893,7 @@ These flags change defaults for commands that manage existing sandboxes. | `NEMOCLAW_CLEANUP_GATEWAY` | `1`, `true`, or `yes` to enable; `0`, `false`, or `no` to disable | Sets the default for whether `nemohermes destroy` removes the shared gateway when destroying the last sandbox. Command-line `--cleanup-gateway` and `--no-cleanup-gateway` still take precedence. | | `NEMOCLAW_DISABLE_INFERENCE_ROUTE_REPAIR` | `1` to enable | Skips the automatic DNS-proxy repair for stale `inference.local` routes during `nemohermes connect` and `nemohermes connect --probe-only`. Use only as a troubleshooting escape hatch. | | `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE` | `1` to opt in | Allows advanced immutable-config verification to trust the current on-disk bytes for older or partial content baselines. Use only after you have rebuilt or manually inspected the sandbox state and accepted that the baseline is operator-approved. | +| `NEMOCLAW_SHIELDS_SETTLE_MS` | milliseconds (default `750`, clamped to `0`–`10000`) | Settle window NemoClaw waits after re-applying a config lockdown (during shields auto-restore and `nemohermes shields up` drift remediation) before re-confirming the lock still holds. Detects when an in-sandbox reconciler changes config file permissions after lockdown and re-applies the lock; if NemoClaw cannot re-confirm the lock within the retry budget, shields stay down. This narrows the window in which a reconciler can revert permissions rather than eliminating it — the best-effort `chattr +i` immutable bit remains the only fully durable lock. Raise it on hosts where the gateway settles slowly. | ### Legacy `nemohermes setup` diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index c9182932424..7c62f31247a 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -2233,6 +2233,7 @@ These flags change defaults for commands that manage existing sandboxes. | `NEMOCLAW_CLEANUP_GATEWAY` | `1`, `true`, or `yes` to enable; `0`, `false`, or `no` to disable | Sets the default for whether `$$nemoclaw destroy` removes the shared gateway when destroying the last sandbox. Command-line `--cleanup-gateway` and `--no-cleanup-gateway` still take precedence. | | `NEMOCLAW_DISABLE_INFERENCE_ROUTE_REPAIR` | `1` to enable | Skips the automatic DNS-proxy repair for stale `inference.local` routes during `$$nemoclaw connect` and `$$nemoclaw connect --probe-only`. Use only as a troubleshooting escape hatch. | | `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE` | `1` to opt in | Allows advanced immutable-config verification to trust the current on-disk bytes for older or partial content baselines. Use only after you have rebuilt or manually inspected the sandbox state and accepted that the baseline is operator-approved. | +| `NEMOCLAW_SHIELDS_SETTLE_MS` | milliseconds (default `750`, clamped to `0`–`10000`) | Settle window NemoClaw waits after re-applying a config lockdown (during shields auto-restore and `$$nemoclaw shields up` drift remediation) before re-confirming the lock still holds. Detects when an in-sandbox reconciler changes config file permissions after lockdown and re-applies the lock; if NemoClaw cannot re-confirm the lock within the retry budget, shields stay down. This narrows the window in which a reconciler can revert permissions rather than eliminating it — the best-effort `chattr +i` immutable bit remains the only fully durable lock. Raise it on hosts where the gateway settles slowly. | ### Remote Deployment diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index f51d1e9beab..cea3a165676 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -42,6 +42,7 @@ const { }: typeof import("./permissive-runtime") = require("./permissive-runtime"); const { cleanupTempDir } = require("../onboard/temp-files"); const { verifyShieldsLockState }: typeof import("./verify-lock") = require("./verify-lock"); +const { relockAndReconfirm }: typeof import("./relock-reconfirm") = require("./relock-reconfirm"); const { parseSha256Output, isHashVerificationIssue, @@ -794,12 +795,18 @@ function rollbackShieldsDown( let rollbackChattrApplied: boolean | null = null; let rollbackFileHashes: { [path: string]: string } | null = null; if (rollbackResult.status === 0) { - try { - const lockResult = lockAgentConfig(sandboxName, target); - rollbackChattrApplied = lockResult.chattrApplied; - rollbackFileHashes = lockResult.fileHashes; - } catch { - console.error(" Warning: Rollback re-lock could not be verified. Check config manually."); + // Re-confirm after the settle window so a reconciler revert cannot leave + // the rolled-back config DRIFTED — same fail-closed treatment as the + // auto-restore path. Leaves the hashes null (→ "manual intervention" + // below) when the lock will not re-confirm. + const relock = relockAndReconfirm(() => lockAgentConfig(sandboxName, target)); + if (relock.ok && relock.lastResult) { + rollbackChattrApplied = relock.lastResult.chattrApplied; + rollbackFileHashes = relock.lastResult.fileHashes; + } else { + console.error( + " Warning: Rollback re-lock could not be re-confirmed. Check config manually.", + ); } } else { console.error(" Warning: Policy restore failed during rollback."); @@ -850,17 +857,23 @@ function activateLockdownFromSnapshot( } const target = resolveAgentConfig(sandboxName); - try { - const lockResult = lockAgentConfig(sandboxName, target); + // Re-confirm the lock after a settle window. This restore feeds the + // auto-restore inline recovery and the `shields up` snapshot path, both of + // which mark shields UP on this result — so a reconciler revert here would + // otherwise leave the same DRIFTED state #4663 is about. relockAndReconfirm + // fails closed (ok:false) when the lock will not hold past the settle window. + const relock = relockAndReconfirm(() => lockAgentConfig(sandboxName, target)); + if (!relock.ok || !relock.lastResult) { return { - ok: true, - chattrApplied: lockResult.chattrApplied, - fileHashes: lockResult.fileHashes, + ok: false, + error: relock.error ?? "config re-lock did not re-confirm after the settle window", }; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return { ok: false, error: message }; } + return { + ok: true, + chattrApplied: relock.lastResult.chattrApplied, + fileHashes: relock.lastResult.fileHashes, + }; } function recoverExpiredAutoRestoreInline( @@ -1256,15 +1269,20 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): // both cases re-applying the lock rewrites perms and captures a // fresh, complete seal. console.log(` Lockdown drifted — re-applying lock for ${sandboxName}...`); - let lockResult: { chattrApplied: boolean; fileHashes: { [path: string]: string } }; - try { - lockResult = lockAgentConfig(sandboxName, target); - } catch (err) { - const message = err instanceof Error ? err.message : String(err); + // #4663: re-confirm the lock held after the in-sandbox reconciler settles, + // re-applying if it reverts perms. A single re-apply here was also being + // reverted on DGX Station / DGX Spark, leaving the sandbox DRIFTED. This + // narrows (does not close) the revert window; the chattr +i immutable bit + // applied inside lockAgentConfig is the only fully durable defense. + const relock = relockAndReconfirm(() => lockAgentConfig(sandboxName, target)); + if (!relock.ok || !relock.lastResult) { + const message = relock.error ?? "Config re-lock did not re-confirm after settle window"; console.error(` ERROR: ${message}`); console.error(" Config remains drifted — manual intervention required."); return failShieldsCommand(message, opts.throwOnError); } + const lockResult: { chattrApplied: boolean; fileHashes: { [path: string]: string } } = + relock.lastResult; saveShieldsState(sandboxName, { shieldsDown: false, chattrApplied: lockResult.chattrApplied, diff --git a/src/lib/shields/relock-reconfirm.test.ts b/src/lib/shields/relock-reconfirm.test.ts new file mode 100644 index 00000000000..beabc6a03a9 --- /dev/null +++ b/src/lib/shields/relock-reconfirm.test.ts @@ -0,0 +1,158 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { relockAndReconfirm, resolveSettleMs } from "./relock-reconfirm"; + +const sealedHashes = { + "/sandbox/.openclaw/openclaw.json": + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + "/sandbox/.openclaw/.config-hash": + "fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210", +}; + +function okResult() { + return { chattrApplied: true, fileHashes: sealedHashes }; +} + +describe("relockAndReconfirm", () => { + it("returns ok with the re-confirmed result when the lock always succeeds", () => { + const lock = vi.fn(() => okResult()); + const sleep = vi.fn(); + + const result = relockAndReconfirm(lock, { sleep, settleMs: 5 }); + + expect(result.ok).toBe(true); + expect(result.attempts).toBe(1); + expect(result.lastResult).toEqual(okResult()); + // One apply + one re-confirm. + expect(lock).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenCalledTimes(1); + expect(sleep).toHaveBeenCalledWith(5); + }); + + it("fails closed (bounded) when the re-confirm always throws after a clean apply", () => { + // Apply succeeds every time, but the reconciler reverts before each + // re-confirm so every re-confirm throws. + const lock = vi + .fn() + .mockImplementationOnce(() => okResult()) // attempt 1 apply + .mockImplementationOnce(() => { + throw new Error("drift"); + }) // attempt 1 re-confirm + .mockImplementationOnce(() => okResult()) // attempt 2 apply + .mockImplementationOnce(() => { + throw new Error("drift"); + }) // attempt 2 re-confirm + .mockImplementationOnce(() => okResult()) // attempt 3 apply + .mockImplementationOnce(() => { + throw new Error("drift"); + }); // attempt 3 re-confirm + const sleep = vi.fn(); + + const result = relockAndReconfirm(lock, { sleep, settleMs: 0, maxAttempts: 3 }); + + expect(result.ok).toBe(false); + expect(result.attempts).toBe(3); + expect(result.lastResult).toBeNull(); + expect(result.error).toBe("drift"); + // Bounded: 2 calls per attempt × 3 attempts. + expect(lock).toHaveBeenCalledTimes(6); + expect(sleep).toHaveBeenCalledTimes(3); + }); + + it("retries the whole cycle when the lock reverts once then holds", () => { + const lock = vi + .fn() + .mockImplementationOnce(() => okResult()) // attempt 1 apply + .mockImplementationOnce(() => { + throw new Error("reverted during settle"); + }) // attempt 1 re-confirm — reverted + .mockImplementationOnce(() => okResult()) // attempt 2 apply + .mockImplementationOnce(() => okResult()); // attempt 2 re-confirm — holds + const sleep = vi.fn(); + + const result = relockAndReconfirm(lock, { sleep, settleMs: 0, maxAttempts: 3 }); + + expect(result.ok).toBe(true); + expect(result.attempts).toBe(2); + expect(result.lastResult).toEqual(okResult()); + expect(lock).toHaveBeenCalledTimes(4); + expect(sleep).toHaveBeenCalledTimes(2); + }); + + it("fails immediately on attempt 1 when the first apply throws, never sleeping", () => { + const lock = vi.fn(() => { + throw new Error("cannot apply lock"); + }); + const sleep = vi.fn(); + + const result = relockAndReconfirm(lock, { sleep, settleMs: 0, maxAttempts: 3 }); + + expect(result.ok).toBe(false); + expect(result.attempts).toBe(1); + expect(result.lastResult).toBeNull(); + expect(result.error).toBe("cannot apply lock"); + expect(lock).toHaveBeenCalledTimes(1); + expect(sleep).not.toHaveBeenCalled(); + }); +}); + +describe("resolveSettleMs", () => { + const originalVitest = process.env.VITEST; + const originalNodeEnv = process.env.NODE_ENV; + const originalSettle = process.env.NEMOCLAW_SHIELDS_SETTLE_MS; + + afterEach(() => { + if (originalVitest === undefined) delete process.env.VITEST; + else process.env.VITEST = originalVitest; + if (originalNodeEnv === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = originalNodeEnv; + if (originalSettle === undefined) delete process.env.NEMOCLAW_SHIELDS_SETTLE_MS; + else process.env.NEMOCLAW_SHIELDS_SETTLE_MS = originalSettle; + }); + + it("returns 0 under test (VITEST=true) so suites do not block", () => { + process.env.VITEST = "true"; + expect(resolveSettleMs()).toBe(0); + }); + + it("applies the real settle window when only NODE_ENV=test (VITEST is the sole test signal)", () => { + // Security: NODE_ENV=test must NOT collapse the durability wait to 0. Only + // Vitest (VITEST=true) is treated as a test runtime; a production + // deployment that happens to run with NODE_ENV=test keeps the real settle. + delete process.env.VITEST; + process.env.NODE_ENV = "test"; + delete process.env.NEMOCLAW_SHIELDS_SETTLE_MS; + expect(resolveSettleMs()).toBe(750); + }); + + it("defaults to 750ms outside test when env is unset", () => { + delete process.env.VITEST; + process.env.NODE_ENV = "production"; + delete process.env.NEMOCLAW_SHIELDS_SETTLE_MS; + expect(resolveSettleMs()).toBe(750); + }); + + it("clamps an over-range env value to the upper bound", () => { + delete process.env.VITEST; + process.env.NODE_ENV = "production"; + process.env.NEMOCLAW_SHIELDS_SETTLE_MS = "999999"; + expect(resolveSettleMs()).toBe(10_000); + }); + + it("clamps a negative env value to 0", () => { + delete process.env.VITEST; + process.env.NODE_ENV = "production"; + process.env.NEMOCLAW_SHIELDS_SETTLE_MS = "-500"; + expect(resolveSettleMs()).toBe(0); + }); + + it("honours a valid in-range env value", () => { + delete process.env.VITEST; + process.env.NODE_ENV = "production"; + process.env.NEMOCLAW_SHIELDS_SETTLE_MS = "1500"; + expect(resolveSettleMs()).toBe(1500); + }); +}); diff --git a/src/lib/shields/relock-reconfirm.ts b/src/lib/shields/relock-reconfirm.ts new file mode 100644 index 00000000000..9cbb266c828 --- /dev/null +++ b/src/lib/shields/relock-reconfirm.ts @@ -0,0 +1,146 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// Re-lock-and-reconfirm helper for shields (#4663). +// +// `lockAgentConfig` chmod 444 / chown root:root the config files and verifies +// the on-disk state once — a single instantaneous snapshot. On DGX Station / +// DGX Spark an in-sandbox privileged reconciler (OpenClaw gateway / doctor-style +// perm normalization) re-permissions `.config-hash` in place *after* the +// verified lock returns, reverting it to 660 sandbox:sandbox. The content is +// untouched (the SHA-256 seal still matches), so only mode/owner drift and the +// next `shields status` reports UP (DRIFTED). +// +// `relockAndReconfirm` runs a bounded "lock -> settle -> re-confirm -> re-lock +// if drifted" cycle and only declares the lock UP when a re-confirmation passes +// after the reconciler has had a chance to settle. +// +// IMPORTANT — this NARROWS the race window, it does NOT close it. After the +// final re-confirm returns, the same reconciler can revert perms one settle +// window later (the TOCTOU is shifted, not eliminated). The only fully durable +// defense is the `chattr +i` immutable bit set inside `lockAgentConfig`, which +// is best-effort and may be unavailable (e.g. no CAP_LINUX_IMMUTABLE). This +// helper is a fail-closed mitigation: when the lock will not re-confirm within +// the retry budget, callers leave shields DOWN rather than report a stale UP. +// It is synchronous (uses the blocking `sleepMs`) so callers such as the +// auto-restore timer stay synchronous. + +import { sleepMs } from "../core/wait"; + +const DEFAULT_MAX_ATTEMPTS = 3; +const DEFAULT_SETTLE_MS = 750; +const MIN_SETTLE_MS = 0; +const MAX_SETTLE_MS = 10_000; + +/** Result of a single `lockAgentConfig` call: apply + verify. */ +export interface LockResult { + chattrApplied: boolean; + fileHashes: { [path: string]: string }; +} + +/** A lock operation: applies the lock and verifies it, throwing on drift. */ +export type LockFn = () => LockResult; + +export interface RelockReconfirmOptions { + /** Maximum number of lock -> settle -> re-confirm cycles. Default 3. */ + maxAttempts?: number; + /** Override the settle window (ms) between apply and re-confirm. */ + settleMs?: number; + /** Injectable synchronous sleep, for unit tests. Defaults to `sleepMs`. */ + sleep?: (ms: number) => void; +} + +export interface RelockReconfirmResult { + /** True only when a re-confirmation after the settle window succeeded. */ + ok: boolean; + /** Number of full cycles attempted (1-based). */ + attempts: number; + /** The re-confirmed lock result when `ok`, else null. */ + lastResult: LockResult | null; + /** Failure message when `!ok`. */ + error?: string; +} + +/** + * Resolve the settle window (ms) between applying a lock and re-confirming it. + * + * Reads `NEMOCLAW_SHIELDS_SETTLE_MS`, defaulting to 750ms and clamping to + * [0, 10000]. Returns 0 only under Vitest so suites don't incur real blocking + * waits. + */ +export function resolveSettleMs(): number { + // VITEST is the precise test signal (Vitest always sets it). Do NOT key off + // NODE_ENV=test — the real settle window must apply in production regardless + // of NODE_ENV, or the re-confirm wait would silently collapse to 0. + if (process.env.VITEST === "true") { + return 0; + } + const raw = process.env.NEMOCLAW_SHIELDS_SETTLE_MS; + if (raw === undefined || raw === "") { + return DEFAULT_SETTLE_MS; + } + const parsed = Number(raw); + if (!Number.isFinite(parsed)) { + return DEFAULT_SETTLE_MS; + } + return Math.min(MAX_SETTLE_MS, Math.max(MIN_SETTLE_MS, Math.trunc(parsed))); +} + +/** + * Apply a config lock and re-confirm it holds after a settle window, retrying + * the whole cycle if an in-sandbox reconciler reverted perms during the wait. + * + * Each attempt: + * 1. `lock()` — apply + verify. If this throws, fail immediately (the lock + * could not even be applied/verified once). + * 2. `sleep(settleMs)` — give the gateway/reconciler time to settle. + * 3. `lock()` — re-confirm. Success => re-confirmed, return ok. Throw => the + * reconciler reverted during the settle window; retry the whole cycle. + * + * Returns ok:false (fail closed) when attempts are exhausted or the first + * apply of an attempt throws. NOTE: ok:true means the lock re-confirmed after + * the settle window — it does not guarantee the perms cannot be reverted again + * afterward (see the module header on the residual TOCTOU window). + */ +export function relockAndReconfirm( + lock: LockFn, + opts: RelockReconfirmOptions = {}, +): RelockReconfirmResult { + const maxAttempts = + opts.maxAttempts !== undefined && opts.maxAttempts > 0 + ? opts.maxAttempts + : DEFAULT_MAX_ATTEMPTS; + const settleMs = opts.settleMs !== undefined ? opts.settleMs : resolveSettleMs(); + const sleep = opts.sleep ?? sleepMs; + + let lastError = "Config re-lock did not re-confirm after settle window"; + + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + // Apply + verify. A failure here means the lock could not be established + // even momentarily — there is nothing to settle, so fail immediately. + try { + lock(); + } catch (error: unknown) { + return { + ok: false, + attempts: attempt, + lastResult: null, + error: error instanceof Error ? error.message : String(error), + }; + } + + // Let the in-sandbox reconciler settle, then re-confirm the lock held. + sleep(settleMs); + + try { + const confirmed = lock(); + return { ok: true, attempts: attempt, lastResult: confirmed }; + } catch (error: unknown) { + // The reconciler reverted perms during the settle window. Retry the + // whole cycle (re-apply, settle, re-confirm) up to maxAttempts. + lastError = error instanceof Error ? error.message : String(error); + } + } + + return { ok: false, attempts: maxAttempts, lastResult: null, error: lastError }; +} diff --git a/src/lib/shields/timer.test.ts b/src/lib/shields/timer.test.ts index 0248380f927..16480768f80 100644 --- a/src/lib/shields/timer.test.ts +++ b/src/lib/shields/timer.test.ts @@ -276,7 +276,10 @@ describe("shields timer authorization", () => { expect(exitCode).toBe(0); expect(runMock).toHaveBeenCalledTimes(1); - expect(lockMock).toHaveBeenCalledTimes(1); + // #4663: relockAndReconfirm applies then re-confirms after the settle + // window (0ms under test), so lockAgentConfig is invoked twice for a clean + // lock. + expect(lockMock).toHaveBeenCalledTimes(2); expect(updatedState.shieldsDown).toBe(false); expect(updatedState.chattrApplied).toBe(true); expect(updatedState.fileHashes).toEqual(sealedHashes); @@ -358,4 +361,176 @@ describe("shields timer authorization", () => { ); expect(fs.existsSync(markerPath)).toBe(false); }); + + // ------------------------------------------------------------------------- + // #4663 — auto-restore durability against post-lock perm revert + // + // On DGX Station / DGX Spark the auto-restore lock succeeds and verifies + // (444 root:root), but an in-sandbox reconciler (OpenClaw gateway / + // doctor-style perm normalization) re-touches `.config-hash` in place + // *after* the lock returns, reverting it to 660 sandbox:sandbox. Content is + // unchanged (the SHA-256 seal still matches), so only mode/owner drift, and + // the next `shields status` reports UP (DRIFTED). The auto-restore path + // performs a single instantaneous verify (inside lockAgentConfig) and never + // re-confirms after the gateway has had a chance to settle. + // + // Contract the fix must satisfy: after restoring policy, the timer must + // re-verify the lock held once the gateway has settled and re-apply if it + // drifted; it must only mark shields UP when a re-confirm passes 444 root:root + // after the settle window, otherwise leave shields DOWN with an audit warning. + // (This narrows the revert window; it does not close the TOCTOU.) + // ------------------------------------------------------------------------- + + it("#4663 re-verifies the auto-restore lock after settle so a reconciler reverting .config-hash perms is caught", async () => { + const stateDir = path.join(tmpHome, ".nemoclaw", "state"); + fs.mkdirSync(stateDir, { recursive: true }); + + const sandboxName = "alpha"; + const configPath = "/sandbox/.openclaw/openclaw.json"; + const configDir = "/sandbox/.openclaw"; + const sensitiveHashPath = `${configDir}/.config-hash`; + const snapshotPath = path.join(stateDir, "snapshot.yaml"); + const restoreAtIso = new Date(Date.now() + 60_000).toISOString(); + const markerPath = path.join(stateDir, `shields-timer-${sandboxName}.json`); + const stateFile = path.join(stateDir, `shields-${sandboxName}.json`); + + fs.writeFileSync(snapshotPath, "version: 1\nnetwork_policies:\n default: {}\n"); + fs.writeFileSync( + markerPath, + JSON.stringify({ + pid: process.pid, + sandboxName, + snapshotPath, + restoreAt: restoreAtIso, + processToken: "tok", + }), + ); + + const sealedHashes = { + [configPath]: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + [sensitiveHashPath]: "fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210", + }; + const lockMock = vi.fn(() => ({ chattrApplied: true, fileHashes: sealedHashes })); + + const sandboxConfigModule = await import("../sandbox/config"); + (sandboxConfigModule.resolveAgentConfig as ReturnType).mockReturnValue({ + agentName: "openclaw", + configPath, + configDir, + sensitiveFiles: [sensitiveHashPath], + }); + const indexModule = await import("./index"); + (indexModule.lockAgentConfig as ReturnType).mockImplementation(lockMock); + + const timer = await import("./timer"); + const args = timer.parseTimerArgs([ + sandboxName, + snapshotPath, + restoreAtIso, + configPath, + configDir, + "tok", + ]); + expect(args).not.toBeNull(); + + const exitCode = invokeTimerAndCaptureExit(timer.runRestoreTimer, args); + + // A single instantaneous lock+verify cannot prove the gateway didn't + // re-permission .config-hash afterward. The fix must re-confirm the lock + // held after the gateway settled, which re-invokes the verified lock path. + expect(lockMock).toHaveBeenCalledTimes(2); + expect(exitCode).toBe(0); + expect(JSON.parse(fs.readFileSync(stateFile, "utf-8")).shieldsDown).toBe(false); + }); + + it("#4663 leaves shields DOWN and audits when the post-settle re-lock cannot hold .config-hash perms", async () => { + const stateDir = path.join(tmpHome, ".nemoclaw", "state"); + fs.mkdirSync(stateDir, { recursive: true }); + + const sandboxName = "alpha"; + const configPath = "/sandbox/.openclaw/openclaw.json"; + const configDir = "/sandbox/.openclaw"; + const sensitiveHashPath = `${configDir}/.config-hash`; + const snapshotPath = path.join(stateDir, "snapshot.yaml"); + const restoreAtIso = new Date(Date.now() + 60_000).toISOString(); + const markerPath = path.join(stateDir, `shields-timer-${sandboxName}.json`); + const stateFile = path.join(stateDir, `shields-${sandboxName}.json`); + const auditFile = path.join(stateDir, "shields-audit.jsonl"); + + fs.writeFileSync(snapshotPath, "version: 1\nnetwork_policies:\n default: {}\n"); + fs.writeFileSync(stateFile, JSON.stringify({ shieldsDown: true }, null, 2)); + fs.writeFileSync( + markerPath, + JSON.stringify({ + pid: process.pid, + sandboxName, + snapshotPath, + restoreAt: restoreAtIso, + processToken: "tok", + }), + ); + + const sealedHashes = { + [configPath]: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + [sensitiveHashPath]: "fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210", + }; + // First lock succeeds and verifies; on re-confirmation the gateway has + // reverted .config-hash, so the verified lock path throws drift. + const lockMock = vi + .fn() + .mockImplementationOnce(() => ({ chattrApplied: true, fileHashes: sealedHashes })) + .mockImplementation(() => { + throw new Error( + "Config not locked: /sandbox/.openclaw/.config-hash mode=660 (expected 444), /sandbox/.openclaw/.config-hash owner=sandbox:sandbox (expected root:root)", + ); + }); + + const sandboxConfigModule = await import("../sandbox/config"); + (sandboxConfigModule.resolveAgentConfig as ReturnType).mockReturnValue({ + agentName: "openclaw", + configPath, + configDir, + sensitiveFiles: [sensitiveHashPath], + }); + const indexModule = await import("./index"); + (indexModule.lockAgentConfig as ReturnType).mockImplementation(lockMock); + + const timer = await import("./timer"); + const args = timer.parseTimerArgs([ + sandboxName, + snapshotPath, + restoreAtIso, + configPath, + configDir, + "tok", + ]); + expect(args).not.toBeNull(); + + const exitCode = invokeTimerAndCaptureExit(timer.runRestoreTimer, args); + const updatedState = JSON.parse(fs.readFileSync(stateFile, "utf-8")); + const auditEntries = fs + .readFileSync(auditFile, "utf-8") + .trim() + .split("\n") + .map((line) => JSON.parse(line)); + + expect(exitCode).toBe(1); + expect(updatedState.shieldsDown).toBe(true); + // Both audit outcomes must fire: the re-lock warning AND the terminal + // fail-closed entry that keeps shields DOWN. + expect(auditEntries).toContainEqual( + expect.objectContaining({ + action: "shields_auto_restore_lock_warning", + sandbox: sandboxName, + lock_verified: false, + }), + ); + expect(auditEntries).toContainEqual( + expect.objectContaining({ + action: "shields_up_failed", + sandbox: sandboxName, + error: "Config re-lock verification failed — shields remain DOWN", + }), + ); + }); }); diff --git a/src/lib/shields/timer.ts b/src/lib/shields/timer.ts index 9ebfcf3847f..e26c9f6f95d 100644 --- a/src/lib/shields/timer.ts +++ b/src/lib/shields/timer.ts @@ -17,6 +17,7 @@ import { resolveAgentConfig } from "../sandbox/config"; import { resolveNemoclawStateDir } from "../state/paths"; import { appendAuditEntry, type ShieldsAuditEntry } from "./audit"; import * as shields from "./index"; +import { relockAndReconfirm } from "./relock-reconfirm"; interface ShieldsStatePatch { shieldsDown?: boolean; @@ -243,9 +244,27 @@ function runRestoreTimer(args: TimerArgs): void { if (lockTarget) { try { const lockAgentConfig = resolveLockAgentConfig(); - const lockResult = lockAgentConfig(args.sandboxName, lockTarget); - lockedChattr = lockResult.chattrApplied; - lockedHashes = lockResult.fileHashes; + // #4663: a single instantaneous lock+verify cannot prove an + // in-sandbox reconciler didn't re-permission .config-hash after the + // verified lock returned. Re-confirm the lock held once the gateway + // has settled, re-applying if it drifted. This narrows (does not + // close) the revert window; fail closed (leave shields DOWN + audit) + // when the lock will not re-confirm within the retry budget. + const relock = relockAndReconfirm(() => lockAgentConfig(args.sandboxName, lockTarget)); + if (relock.ok && relock.lastResult) { + lockedChattr = relock.lastResult.chattrApplied; + lockedHashes = relock.lastResult.fileHashes; + } else { + lockVerified = false; + appendAudit({ + action: "shields_auto_restore_lock_warning", + sandbox: args.sandboxName, + timestamp: now, + restored_by: "auto_timer", + warning: relock.error ?? "Config re-lock did not re-confirm after settle window", + lock_verified: false, + }); + } } catch (error: unknown) { lockVerified = false; appendAudit({