From fa29a3959f61a1e1c1cf8bdd96915cdae33430ac Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 28 May 2026 19:35:32 +0000 Subject: [PATCH 1/8] fix(shields): seal locked files with SHA-256 and detect content drift Signed-off-by: Tinson Lai --- docs/security/best-practices.mdx | 1 + src/lib/shields/index.test.ts | 103 ++++++++++++++ src/lib/shields/index.ts | 147 +++++++++++++++++-- src/lib/shields/verify-lock.test.ts | 190 +++++++++++++++++++++++++ src/lib/shields/verify-lock.ts | 60 +++++++- test/repro-2681-group-writable.test.ts | 7 + 6 files changed, 491 insertions(+), 17 deletions(-) diff --git a/docs/security/best-practices.mdx b/docs/security/best-practices.mdx index 11539658778..43ecf68fed5 100644 --- a/docs/security/best-practices.mdx +++ b/docs/security/best-practices.mdx @@ -204,6 +204,7 @@ For sensitive workloads, use a reviewed host-side immutability workflow after in - **DAC permissions (default).** The sandbox user owns `/sandbox/.openclaw` with mode `2770` (setgid `sandbox:sandbox`) and `openclaw.json` with mode `660`, so the agent and its group can read and write config directly. A reviewed host-side immutability workflow should compare the intended ownership and mode with the live sandbox filesystem before treating the config tree as locked. - **Config integrity hash.** The image includes a SHA256 hash of `openclaw.json`. In the default mutable state, `.config-hash` is sandbox-owned and is not a tamper-proof trust anchor, so startup does not fail closed on that hash. When the hash is root-owned and read-only, startup enforces it and refuses to start if the hash does not match. +- **Content seal under shields up.** When `nemoclaw shields up` runs, it captures a SHA-256 seal of `openclaw.json` and any other locked files into the host-side shields state file. Every `shields status` call recomputes the hash inside the sandbox and surfaces drift on any mismatch, so a host-root tamper that flips perms back to `444 root:root` after rewriting the file is still flagged. `shields up` refuses to re-seal a tampered baseline; restore the original file or rebuild the sandbox before re-running. - **Gateway token environment.** The gateway exports `OPENCLAW_GATEWAY_TOKEN` and writes it to `/tmp/nemoclaw-proxy-env.sh` for interactive sandbox sessions. Keep this in mind when deciding whether a workload should run with mutable config or an immutable config posture. | Aspect | Detail | diff --git a/src/lib/shields/index.test.ts b/src/lib/shields/index.test.ts index 4e0ea6ed636..8c436ee4ed7 100644 --- a/src/lib/shields/index.test.ts +++ b/src/lib/shields/index.test.ts @@ -753,6 +753,109 @@ describe("shields — unit logic", () => { expect(errorSpy).not.toHaveBeenCalled(); }); + it("passes the persisted fileHashes seal to the verifier when present (#4243)", async () => { + const sandboxName = "openclaw"; + const fileHashes = { + "/sandbox/.openclaw/openclaw.json": + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + }; + fs.mkdirSync(stateDir(), { recursive: true }); + fs.writeFileSync( + path.join(stateDir(), `shields-${sandboxName}.json`), + JSON.stringify( + { + shieldsDown: false, + chattrApplied: true, + fileHashes, + updatedAt: new Date().toISOString(), + }, + null, + 2, + ), + { mode: 0o600 }, + ); + let receivedExpectedHashes: + | { [path: string]: string } + | undefined; + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + const { shieldsStatus } = await loadShieldsModule(); + shieldsStatus(sandboxName, true, { + verifyLockState: ( + _name: string, + _target: unknown, + options: { expectedHashes?: { [path: string]: string } }, + ) => { + receivedExpectedHashes = options.expectedHashes; + return { ok: true, issues: [] }; + }, + resolveConfig: () => ({ + agentName: "openclaw", + configPath: "/sandbox/.openclaw/openclaw.json", + configDir: "/sandbox/.openclaw", + }), + }); + + expect(receivedExpectedHashes).toEqual(fileHashes); + // No legacy-state notice when a seal is recorded. + expect( + logSpy.mock.calls.map((args) => args[0]).join("\n"), + ).not.toContain("no content seal recorded"); + expect(errorSpy).not.toHaveBeenCalled(); + }); + + it("logs a legacy-state notice when locked but no fileHashes seal is recorded", async () => { + const sandboxName = "openclaw"; + writeLockedState(sandboxName); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + + const { shieldsStatus } = await loadShieldsModule(); + shieldsStatus(sandboxName, true, { + verifyLockState: () => ({ ok: true, issues: [] }), + resolveConfig: () => ({ + agentName: "openclaw", + configPath: "/sandbox/.openclaw/openclaw.json", + configDir: "/sandbox/.openclaw", + }), + }); + + const lines = logSpy.mock.calls.map((args) => args[0]).join("\n"); + expect(lines).toContain( + `Notice: no content seal recorded; re-run \`nemoclaw ${sandboxName} shields up\` to capture one for drift detection.`, + ); + }); + + it("surfaces content-drift entries from the verifier without re-locking (#4243)", async () => { + const sandboxName = "openclaw"; + writeLockedState(sandboxName); + const driftIssues = [ + "/sandbox/.openclaw/openclaw.json content drifted (sha256 fff... != sealed 012...)", + ]; + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((code?: string | number | null) => { + throw new Error(`exit ${String(code)}`); + }); + + const { shieldsStatus } = await loadShieldsModule(); + expect(() => + shieldsStatus(sandboxName, true, { + verifyLockState: () => ({ ok: false, issues: driftIssues }), + resolveConfig: () => ({ + agentName: "openclaw", + configPath: "/sandbox/.openclaw/openclaw.json", + configDir: "/sandbox/.openclaw", + }), + }), + ).toThrow("exit 2"); + + const allErrors = errorSpy.mock.calls.map((args) => args[0]).join("\n"); + expect(allErrors).toContain("content drifted"); + expect(exitSpy).toHaveBeenCalledWith(2); + }); + it("treats a resolveConfig throw as drift so the locked status cannot mask a setup gap", async () => { const sandboxName = "openclaw"; writeLockedState(sandboxName); diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 7c27a465061..93480386fca 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -105,6 +105,13 @@ interface ShieldsState { shieldsDownPolicy?: string | null; shieldsPolicySnapshotPath?: string | null; chattrApplied?: boolean; + // SHA-256 seal of each locked file, captured by `shields up` after the + // lock verification passes. `shields status` re-hashes the same files + // inside the sandbox and flags drift on any mismatch. This catches the + // host-root tamper pattern that defeats perm-only checks: chmod to + // mutable -> write -> chmod back to 444 leaves mode/owner identical to + // the locked baseline but produces a new content hash (#4243). + fileHashes?: { [path: string]: string }; updatedAt?: string; } @@ -282,6 +289,17 @@ function isOptionalNullableNumber( ); } +function isOptionalHashMap( + value: unknown, +): value is { [path: string]: string } | undefined { + if (value === undefined) return true; + if (!isObjectRecord(value)) return false; + for (const v of Object.values(value)) { + if (typeof v !== "string") return false; + } + return true; +} + function isShieldsState(value: unknown): value is ShieldsState { return ( isObjectRecord(value) && @@ -292,6 +310,7 @@ function isShieldsState(value: unknown): value is ShieldsState { isOptionalNullableString(value.shieldsDownPolicy) && isOptionalNullableString(value.shieldsPolicySnapshotPath) && isOptionalBoolean(value.chattrApplied) && + isOptionalHashMap(value.fileHashes) && isOptionalString(value.updatedAt) ); } @@ -589,10 +608,43 @@ function unlockAgentConfig( // in case the runtime environment supports it. // --------------------------------------------------------------------------- +// SHA-256-hex parser shared with verify-lock. Centralised so the lock-path +// seal write and the status-path drift check stay byte-for-byte identical. +const SHA256_HEX_RE = /^[0-9a-f]{64}$/i; + +function parseSha256Hex(raw: string): string | null { + const trimmed = raw.trim(); + if (!trimmed) return null; + const token = trimmed.split(/\s+/, 1)[0]; + return SHA256_HEX_RE.test(token) ? token.toLowerCase() : null; +} + +function captureSealHashes( + sandboxName: string, + filesToHash: string[], +): { [path: string]: string } { + const hashes: { [path: string]: string } = {}; + for (const f of filesToHash) { + let raw: string; + try { + raw = privilegedSandboxExecCapture(sandboxName, ["sha256sum", f]); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + throw new Error(`sha256sum ${f} failed: ${msg}`); + } + const hex = parseSha256Hex(raw); + if (!hex) { + throw new Error(`sha256sum ${f} returned unparsable output: ${raw}`); + } + hashes[f] = hex; + } + return hashes; +} + function lockAgentConfig( sandboxName: string, target: AgentConfigTarget, -): { chattrApplied: boolean } { +): { chattrApplied: boolean; fileHashes: { [path: string]: string } } { const errors: string[] = []; const filesToLock = [target.configPath, ...(target.sensitiveFiles || [])]; @@ -672,7 +724,12 @@ function lockAgentConfig( throw new Error(`Config not locked: ${issues.join(", ")}`); } - return { chattrApplied: chattrSucceeded }; + // Mode + ownership are clean; capture the SHA-256 seal of each locked + // file so `shields status` can detect content drift even when an + // attacker restores the mode/owner after writing (#4243). + const fileHashes = captureSealHashes(sandboxName, filesToLock); + + return { chattrApplied: chattrSucceeded, fileHashes }; } function rollbackShieldsDown( @@ -685,10 +742,12 @@ function rollbackShieldsDown( ignoreError: true, }); 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.", @@ -697,7 +756,7 @@ function rollbackShieldsDown( } else { console.error(" Warning: Policy restore failed during rollback."); } - if (rollbackChattrApplied !== null) { + if (rollbackChattrApplied !== null && rollbackFileHashes !== null) { saveShieldsState(sandboxName, { shieldsDown: false, shieldsDownAt: null, @@ -705,6 +764,7 @@ function rollbackShieldsDown( shieldsDownReason: null, shieldsDownPolicy: null, chattrApplied: rollbackChattrApplied, + fileHashes: rollbackFileHashes, }); console.error(" Lockdown restored. Config was never left unguarded."); } else { @@ -718,6 +778,8 @@ function rollbackShieldsDown( interface LockdownActivationResult { ok: boolean; error?: string; + chattrApplied?: boolean; + fileHashes?: { [path: string]: string }; } function activateLockdownFromSnapshot( @@ -742,13 +804,16 @@ function activateLockdownFromSnapshot( const target = resolveAgentConfig(sandboxName); try { - lockAgentConfig(sandboxName, target); + const lockResult = lockAgentConfig(sandboxName, target); + return { + ok: true, + chattrApplied: lockResult.chattrApplied, + fileHashes: lockResult.fileHashes, + }; } catch (error) { const message = error instanceof Error ? error.message : String(error); return { ok: false, error: message }; } - - return { ok: true }; } function recoverExpiredAutoRestoreInline( @@ -810,6 +875,12 @@ function recoverExpiredAutoRestoreInline( shieldsDownTimeout: null, shieldsDownReason: null, shieldsDownPolicy: null, + ...(activation.fileHashes && typeof activation.chattrApplied === "boolean" + ? { + chattrApplied: activation.chattrApplied, + fileHashes: activation.fileHashes, + } + : {}), }); clearTimerMarker(sandboxName); appendAuditEntry({ @@ -1072,23 +1143,43 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): // undefined (no state file) means fresh sandbox — mutable default, allow shields-up. if (state.shieldsDown === false) { // Verify the sandbox filesystem still matches the locked posture. If a - // host-root tamper has reverted protected perms, re-apply the lock so - // the recovery hint surfaced by `shields status` actually works. + // host-root tamper has reverted protected perms or rewritten file + // content (even when the mode/owner is restored), re-apply the lock + // so the recovery hint surfaced by `shields status` actually works. const target = resolveAgentConfig(sandboxName); const { issues } = verifyShieldsLockState(sandboxName, target, { verifyChattr: state.chattrApplied === true, exec: (cmd: string[]) => privilegedSandboxExecCapture(sandboxName, cmd), assertLegacyLayout: assertNoLegacyStateLayout, + expectedHashes: state.fileHashes, }); if (issues.length === 0) { clearTimerMarker(sandboxName); console.log(" Lockdown is already active."); return; } + // Content drift means a host-root tamper rewrote a locked file even + // while keeping the mode/owner correct. Re-locking would launder the + // tampered content into a fresh seal, so refuse and ask the operator + // to restore the file (or rebuild the sandbox) before re-running. + const contentDrift = issues.filter((entry) => entry.includes("content drifted")); + if (contentDrift.length > 0) { + console.error(" ERROR: locked file content has drifted:"); + for (const entry of contentDrift) { + console.error(` - ${entry}`); + } + console.error( + " Refusing to re-seal a tampered baseline. Restore the file or rebuild the sandbox, then re-run shields up.", + ); + return failShieldsCommand( + `Locked file content drifted: ${contentDrift.join("; ")}`, + opts.throwOnError, + ); + } console.log( ` Lockdown drifted — re-applying lock for ${sandboxName}...`, ); - let lockResult: { chattrApplied: boolean }; + let lockResult: { chattrApplied: boolean; fileHashes: { [path: string]: string } }; try { lockResult = lockAgentConfig(sandboxName, target); } catch (err) { @@ -1102,6 +1193,7 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): saveShieldsState(sandboxName, { shieldsDown: false, chattrApplied: lockResult.chattrApplied, + fileHashes: lockResult.fileHashes, }); clearTimerMarker(sandboxName); appendAuditEntry({ @@ -1133,6 +1225,9 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): ); return failShieldsCommand("Saved policy snapshot is missing", opts.throwOnError); } + let snapshotLockResult: + | { chattrApplied: boolean; fileHashes: { [path: string]: string } } + | null = null; if (snapshotPath) { console.log(" Restoring restrictive policy from snapshot..."); const activation = activateLockdownFromSnapshot(sandboxName, snapshotPath); @@ -1146,6 +1241,12 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): ); return failShieldsCommand(activation.error ?? "unknown restore error", opts.throwOnError); } + if (activation.fileHashes && typeof activation.chattrApplied === "boolean") { + snapshotLockResult = { + chattrApplied: activation.chattrApplied, + fileHashes: activation.fileHashes, + }; + } } else { // 2b. Lock config file to read-only. // Uses kubectl exec to bypass Landlock (same as shields down). @@ -1155,7 +1256,7 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): console.log( ` Locking ${target.agentName} config (${target.configPath})...`, ); - let lockResult: { chattrApplied: boolean }; + let lockResult: { chattrApplied: boolean; fileHashes: { [path: string]: string } }; try { lockResult = lockAgentConfig(sandboxName, target); } catch (err) { @@ -1169,7 +1270,10 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): ); return failShieldsCommand(message, opts.throwOnError); } - saveShieldsState(sandboxName, { chattrApplied: lockResult.chattrApplied }); + saveShieldsState(sandboxName, { + chattrApplied: lockResult.chattrApplied, + fileHashes: lockResult.fileHashes, + }); } // 3. Calculate duration @@ -1179,14 +1283,22 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): const now = new Date(); const durationSeconds = Math.floor((now.getTime() - downAt.getTime()) / 1000); - // 4. Update state + // 4. Update state. When the snapshot-restore branch ran, fold its + // captured chattrApplied + fileHashes into the persisted state so + // drift detection on the next `shields status` has a seal to compare + // against. The non-snapshot branch already persisted those above. saveShieldsState(sandboxName, { shieldsDown: false, shieldsDownAt: null, shieldsDownTimeout: null, shieldsDownReason: null, shieldsDownPolicy: null, - // Keep snapshotPath + chattrApplied for forensics / drift re-verify + ...(snapshotLockResult + ? { + chattrApplied: snapshotLockResult.chattrApplied, + fileHashes: snapshotLockResult.fileHashes, + } + : {}), }); clearTimerMarker(sandboxName); @@ -1262,6 +1374,7 @@ function shieldsStatus( verifyChattr: state.chattrApplied === true, exec: (cmd: string[]) => privilegedSandboxExecCapture(sandboxName, cmd), assertLegacyLayout: assertNoLegacyStateLayout, + expectedHashes: state.fileHashes, }).issues; } catch (err) { const msg = err instanceof Error ? err.message : String(err); @@ -1290,6 +1403,14 @@ function shieldsStatus( if (state.shieldsDownAt) { console.log(` Last unlocked: ${state.shieldsDownAt}`); } + if (!state.fileHashes) { + // Legacy state file pre-dates the content seal — perm-only + // verification cannot catch a host-root chmod-write-chmod tamper + // cycle. Recommend re-locking to capture a SHA-256 seal. + console.log( + ` Notice: no content seal recorded; re-run \`nemoclaw ${sandboxName} shields up\` to capture one for drift detection.`, + ); + } return; } diff --git a/src/lib/shields/verify-lock.test.ts b/src/lib/shields/verify-lock.test.ts index d40093689ca..2be1f63ec22 100644 --- a/src/lib/shields/verify-lock.test.ts +++ b/src/lib/shields/verify-lock.test.ts @@ -189,4 +189,194 @@ describe("verifyShieldsLockState", () => { ) => unknown; expect(() => call("openclaw", target)).toThrow(/requires options\.exec/); }); + + // --------------------------------------------------------------------------- + // Content-seal drift (#4243). Perm-only verification cannot catch + // chmod-write-chmod cycles because the mode/owner end up identical to + // the locked baseline. The hash compare is the only way to flag a + // content tamper that restores the perms afterwards. + // --------------------------------------------------------------------------- + + const CLEAN_OPENCLAW_HASH = + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + const CLEAN_CONFIG_HASH_HASH = + "fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210"; + const expectedHashes = { + "/sandbox/.openclaw/openclaw.json": CLEAN_OPENCLAW_HASH, + "/sandbox/.openclaw/.config-hash": CLEAN_CONFIG_HASH_HASH, + }; + + function makeStatPlusSha(perms: StatLookup, hashes: StatLookup) { + return (cmd: string[]): string => { + const file = cmd[cmd.length - 1]; + if (cmd[0] === "stat" && file in perms) return perms[file]; + if (cmd[0] === "sha256sum" && file in hashes) { + // sha256sum prints " "; mirror that shape. + return `${hashes[file]} ${file}`; + } + return ""; + }; + } + + it("flags content drift when chmod-write-chmod tamper leaves perms clean but hash changes", async () => { + const { verifyShieldsLockState } = await loadVerifier(); + const exec = makeStatPlusSha( + { + "/sandbox/.openclaw/openclaw.json": "444 root:root", + "/sandbox/.openclaw/.config-hash": "444 root:root", + "/sandbox/.openclaw": "755 root:root", + }, + { + // openclaw.json hash differs from the seal — this is the #4243 repro. + "/sandbox/.openclaw/openclaw.json": + "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", + "/sandbox/.openclaw/.config-hash": CLEAN_CONFIG_HASH_HASH, + }, + ); + + const result = verifyShieldsLockState("openclaw", target, { + exec, + expectedHashes, + }); + + expect(result.ok).toBe(false); + expect( + result.issues.some( + (issue: string) => + issue.startsWith("/sandbox/.openclaw/openclaw.json content drifted"), + ), + ).toBe(true); + // The clean file must not show up as drifted. + expect( + result.issues.some( + (issue: string) => + issue.startsWith("/sandbox/.openclaw/.config-hash content drifted"), + ), + ).toBe(false); + }); + + it("passes when perms are clean and hashes match the seal", async () => { + const { verifyShieldsLockState } = await loadVerifier(); + const exec = makeStatPlusSha( + { + "/sandbox/.openclaw/openclaw.json": "444 root:root", + "/sandbox/.openclaw/.config-hash": "444 root:root", + "/sandbox/.openclaw": "755 root:root", + }, + { + "/sandbox/.openclaw/openclaw.json": CLEAN_OPENCLAW_HASH, + "/sandbox/.openclaw/.config-hash": CLEAN_CONFIG_HASH_HASH, + }, + ); + + const result = verifyShieldsLockState("openclaw", target, { + exec, + expectedHashes, + }); + + expect(result.ok).toBe(true); + expect(result.issues).toEqual([]); + }); + + it("flags a missing seal entry rather than silently passing when expectedHashes is given but a path is absent", async () => { + const { verifyShieldsLockState } = await loadVerifier(); + const exec = makeStatPlusSha( + { + "/sandbox/.openclaw/openclaw.json": "444 root:root", + "/sandbox/.openclaw/.config-hash": "444 root:root", + "/sandbox/.openclaw": "755 root:root", + }, + { + "/sandbox/.openclaw/openclaw.json": CLEAN_OPENCLAW_HASH, + "/sandbox/.openclaw/.config-hash": CLEAN_CONFIG_HASH_HASH, + }, + ); + + const result = verifyShieldsLockState("openclaw", target, { + exec, + expectedHashes: { + // .config-hash deliberately omitted. + "/sandbox/.openclaw/openclaw.json": CLEAN_OPENCLAW_HASH, + }, + }); + + expect(result.ok).toBe(false); + expect(result.issues).toContain( + "/sandbox/.openclaw/.config-hash no seal recorded (expected SHA-256)", + ); + }); + + it("flags sha256sum failures as drift instead of swallowing them", async () => { + const { verifyShieldsLockState } = await loadVerifier(); + const exec = (cmd: string[]): string => { + if (cmd[0] === "stat") { + if (cmd[cmd.length - 1] === "/sandbox/.openclaw") return "755 root:root"; + return "444 root:root"; + } + if (cmd[0] === "sha256sum") { + throw new Error("sha256sum: I/O error"); + } + return ""; + }; + + const result = verifyShieldsLockState("openclaw", target, { + exec, + expectedHashes, + }); + + expect(result.ok).toBe(false); + expect( + result.issues.some((issue: string) => + issue.includes("sha256sum failed: sha256sum: I/O error"), + ), + ).toBe(true); + }); + + it("flags unparsable sha256sum output rather than treating it as a match", async () => { + const { verifyShieldsLockState } = await loadVerifier(); + const exec = (cmd: string[]): string => { + if (cmd[0] === "stat") { + if (cmd[cmd.length - 1] === "/sandbox/.openclaw") return "755 root:root"; + return "444 root:root"; + } + if (cmd[0] === "sha256sum") return "garbage output"; + return ""; + }; + + const result = verifyShieldsLockState("openclaw", target, { + exec, + expectedHashes, + }); + + expect(result.ok).toBe(false); + expect( + result.issues.some((issue: string) => + issue.includes("sha256sum output unparsable"), + ), + ).toBe(true); + }); + + it("skips hash verification entirely when expectedHashes is undefined (legacy state)", async () => { + const { verifyShieldsLockState } = await loadVerifier(); + // sha256sum is wired to throw — if the verifier were to invoke it + // anyway, the call would surface in issues. + let sha256Calls = 0; + const exec = (cmd: string[]): string => { + if (cmd[0] === "stat") { + if (cmd[cmd.length - 1] === "/sandbox/.openclaw") return "755 root:root"; + return "444 root:root"; + } + if (cmd[0] === "sha256sum") { + sha256Calls++; + throw new Error("should not be called"); + } + return ""; + }; + + const result = verifyShieldsLockState("openclaw", target, { exec }); + + expect(sha256Calls).toBe(0); + expect(result.ok).toBe(true); + expect(result.issues).toEqual([]); + }); }); diff --git a/src/lib/shields/verify-lock.ts b/src/lib/shields/verify-lock.ts index 8f1fee5ee57..9e3fbd93c9a 100644 --- a/src/lib/shields/verify-lock.ts +++ b/src/lib/shields/verify-lock.ts @@ -4,10 +4,17 @@ // Re-verify that the sandbox filesystem still matches what `shields up` // established: 444 root:root on each locked file, 755 root:root on the // config directory, no legacy state layout, and (when the caller knows -// chattr was applied) the immutable bit. Returns the list of mismatches -// so callers can either fail the lock operation or surface drift after a -// host-root tamper. Stat/lsattr failures are folded into `issues` so the -// caller can decide whether to treat them as drift. +// chattr was applied) the immutable bit. When the caller supplies the +// SHA-256 seal that was captured at lock time, also re-hash each file +// and surface a content-drift entry on any mismatch. This catches the +// host-root tamper pattern that defeats perm-only verification: chmod +// to mutable -> write -> chmod back to 444 leaves mode/owner identical +// to the locked baseline but produces a new content hash (#4243). +// +// Returns the list of mismatches so callers can either fail the lock +// operation or surface drift after a host-root tamper. Stat/lsattr/hash +// failures are folded into `issues` so the caller can decide whether to +// treat them as drift. export type LockTarget = { configPath: string; @@ -19,6 +26,7 @@ export type VerifyShieldsLockOptions = { verifyChattr?: boolean; exec: (cmd: string[]) => string; assertLegacyLayout?: (sandboxName: string, configDir: string) => void; + expectedHashes?: { [path: string]: string }; }; export type VerifyShieldsLockResult = { @@ -30,11 +38,23 @@ const EXPECTED_FILE_MODE = "444"; const EXPECTED_DIR_MODE = "755"; const EXPECTED_OWNER = "root:root"; +const SHA256_HEX = /^[0-9a-f]{64}$/i; + function noopAssertLegacyLayout(_sandboxName: string, _configDir: string): void { // Production callers replace this with the real legacy-layout assertion; // when omitted, the verifier treats legacy-layout state as "no issue". } +function parseSha256Output(raw: string): string | null { + // `sha256sum ` prints ``. + // Tolerate leading whitespace so callers that pipe through `tr -d ' \t'` + // or trim themselves still get a clean comparison. + const trimmed = raw.trim(); + if (!trimmed) return null; + const token = trimmed.split(/\s+/, 1)[0]; + return SHA256_HEX.test(token) ? token.toLowerCase() : null; +} + export function verifyShieldsLockState( sandboxName: string, target: LockTarget, @@ -88,6 +108,38 @@ export function verifyShieldsLockState( } } + if (options.expectedHashes) { + const expected = options.expectedHashes; + for (const f of filesToVerify) { + const want = expected[f]; + if (!want) { + // Seal was missing for this file — flag explicitly rather than + // silently passing. Callers that genuinely lack a seal pass + // `expectedHashes: undefined` instead of an empty record. + issues.push(`${f} no seal recorded (expected SHA-256)`); + continue; + } + let raw: string; + try { + raw = exec(["sha256sum", f]); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + issues.push(`${f} sha256sum failed: ${msg}`); + continue; + } + const got = parseSha256Output(raw); + if (!got) { + issues.push(`${f} sha256sum output unparsable: ${raw.trim()}`); + continue; + } + if (got !== want.toLowerCase()) { + issues.push( + `${f} content drifted (sha256 ${got} != sealed ${want.toLowerCase()})`, + ); + } + } + } + try { assertLegacyLayout(sandboxName, target.configDir); } catch (err) { diff --git a/test/repro-2681-group-writable.test.ts b/test/repro-2681-group-writable.test.ts index d2b05409619..b476a1da4a9 100644 --- a/test/repro-2681-group-writable.test.ts +++ b/test/repro-2681-group-writable.test.ts @@ -235,6 +235,13 @@ Module._load = function patchedLoad(request, parent, isMain) { if (command[0] === "lsattr") { return "----i----------------- " + command.at(-1) + "\n"; } + if (command[0] === "sha256sum") { + return ( + "0000000000000000000000000000000000000000000000000000000000000001 " + + command.at(-1) + + "\n" + ); + } return ""; }, }; From ff201d587721fd9f391782bbd53cf5085d9c502b Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 29 May 2026 02:04:27 +0000 Subject: [PATCH 2/8] fix(shields): seal legacy lockdowns, refuse to launder hash failures Signed-off-by: Tinson Lai --- docs/security/best-practices.mdx | 2 +- src/lib/shields/index.test.ts | 4 +- src/lib/shields/index.ts | 64 ++++++++++++------- src/lib/shields/seal.test.ts | 95 +++++++++++++++++++++++++++++ src/lib/shields/seal.ts | 29 +++++++++ src/lib/shields/timer.ts | 15 ++++- src/lib/shields/verify-lock.test.ts | 6 +- src/lib/shields/verify-lock.ts | 16 +---- test/e2e/test-shields-config.sh | 81 ++++++++++++++++++++++++ 9 files changed, 269 insertions(+), 43 deletions(-) create mode 100644 src/lib/shields/seal.test.ts create mode 100644 src/lib/shields/seal.ts diff --git a/docs/security/best-practices.mdx b/docs/security/best-practices.mdx index 43ecf68fed5..6ecde79d152 100644 --- a/docs/security/best-practices.mdx +++ b/docs/security/best-practices.mdx @@ -204,7 +204,7 @@ For sensitive workloads, use a reviewed host-side immutability workflow after in - **DAC permissions (default).** The sandbox user owns `/sandbox/.openclaw` with mode `2770` (setgid `sandbox:sandbox`) and `openclaw.json` with mode `660`, so the agent and its group can read and write config directly. A reviewed host-side immutability workflow should compare the intended ownership and mode with the live sandbox filesystem before treating the config tree as locked. - **Config integrity hash.** The image includes a SHA256 hash of `openclaw.json`. In the default mutable state, `.config-hash` is sandbox-owned and is not a tamper-proof trust anchor, so startup does not fail closed on that hash. When the hash is root-owned and read-only, startup enforces it and refuses to start if the hash does not match. -- **Content seal under shields up.** When `nemoclaw shields up` runs, it captures a SHA-256 seal of `openclaw.json` and any other locked files into the host-side shields state file. Every `shields status` call recomputes the hash inside the sandbox and surfaces drift on any mismatch, so a host-root tamper that flips perms back to `444 root:root` after rewriting the file is still flagged. `shields up` refuses to re-seal a tampered baseline; restore the original file or rebuild the sandbox before re-running. +- **Content seal under shields up.** When `nemoclaw shields up` runs against a clean lock, it captures a SHA-256 seal of `openclaw.json` and any other locked files into the host-side shields state file. On sealed sandboxes, every `shields status` call recomputes the hash inside the sandbox and surfaces drift on any mismatch, so a host-root tamper that flips perms back to `444 root:root` after rewriting the file is still flagged. Sandboxes that were already locked before this seal landed have no recorded hash; the first `shields up` after upgrade captures one even when the verifier reports a clean lock, and `shields status` shows a one-line notice until that happens. `shields up` refuses to re-seal a tampered baseline; restore the original file or rebuild the sandbox before re-running. - **Gateway token environment.** The gateway exports `OPENCLAW_GATEWAY_TOKEN` and writes it to `/tmp/nemoclaw-proxy-env.sh` for interactive sandbox sessions. Keep this in mind when deciding whether a workload should run with mutable config or an immutable config posture. | Aspect | Detail | diff --git a/src/lib/shields/index.test.ts b/src/lib/shields/index.test.ts index 8c436ee4ed7..be75530fb07 100644 --- a/src/lib/shields/index.test.ts +++ b/src/lib/shields/index.test.ts @@ -753,7 +753,7 @@ describe("shields — unit logic", () => { expect(errorSpy).not.toHaveBeenCalled(); }); - it("passes the persisted fileHashes seal to the verifier when present (#4243)", async () => { + it("passes the persisted fileHashes seal to the verifier when present", async () => { const sandboxName = "openclaw"; const fileHashes = { "/sandbox/.openclaw/openclaw.json": @@ -826,7 +826,7 @@ describe("shields — unit logic", () => { ); }); - it("surfaces content-drift entries from the verifier without re-locking (#4243)", async () => { + it("surfaces content-drift entries from the verifier without re-locking", async () => { const sandboxName = "openclaw"; writeLockedState(sandboxName); const driftIssues = [ diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 93480386fca..9a96cf678cf 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -48,6 +48,10 @@ const { cleanupTempDir } = require("../onboard/temp-files"); const { verifyShieldsLockState, }: typeof import("./verify-lock") = require("./verify-lock"); +const { + parseSha256Output, + isHashVerificationIssue, +}: typeof import("./seal") = require("./seal"); const STATE_DIR = resolveNemoclawStateDir(); @@ -110,7 +114,9 @@ interface ShieldsState { // inside the sandbox and flags drift on any mismatch. This catches the // host-root tamper pattern that defeats perm-only checks: chmod to // mutable -> write -> chmod back to 444 leaves mode/owner identical to - // the locked baseline but produces a new content hash (#4243). + // the locked baseline but produces a new content hash. Absent on state + // files captured before the seal landed; the first `shields up` after + // upgrade captures one even when the verifier reports a clean lock. fileHashes?: { [path: string]: string }; updatedAt?: string; } @@ -608,17 +614,6 @@ function unlockAgentConfig( // in case the runtime environment supports it. // --------------------------------------------------------------------------- -// SHA-256-hex parser shared with verify-lock. Centralised so the lock-path -// seal write and the status-path drift check stay byte-for-byte identical. -const SHA256_HEX_RE = /^[0-9a-f]{64}$/i; - -function parseSha256Hex(raw: string): string | null { - const trimmed = raw.trim(); - if (!trimmed) return null; - const token = trimmed.split(/\s+/, 1)[0]; - return SHA256_HEX_RE.test(token) ? token.toLowerCase() : null; -} - function captureSealHashes( sandboxName: string, filesToHash: string[], @@ -632,7 +627,7 @@ function captureSealHashes( const msg = err instanceof Error ? err.message : String(err); throw new Error(`sha256sum ${f} failed: ${msg}`); } - const hex = parseSha256Hex(raw); + const hex = parseSha256Output(raw); if (!hex) { throw new Error(`sha256sum ${f} returned unparsable output: ${raw}`); } @@ -726,7 +721,7 @@ function lockAgentConfig( // Mode + ownership are clean; capture the SHA-256 seal of each locked // file so `shields status` can detect content drift even when an - // attacker restores the mode/owner after writing (#4243). + // attacker restores the mode/owner after writing. const fileHashes = captureSealHashes(sandboxName, filesToLock); return { chattrApplied: chattrSucceeded, fileHashes }; @@ -1154,25 +1149,50 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): expectedHashes: state.fileHashes, }); if (issues.length === 0) { + // Legacy locked state predates the content seal. Capture one now so + // future `shields status` calls can detect content drift instead of + // relying on perm-only verification. We only do this when the + // verifier was already happy with the on-disk state. + if (!state.fileHashes) { + try { + const filesToHash = [ + target.configPath, + ...(target.sensitiveFiles || []), + ]; + const newHashes = captureSealHashes(sandboxName, filesToHash); + saveShieldsState(sandboxName, { fileHashes: newHashes }); + console.log( + " Captured SHA-256 content seal for existing lockdown.", + ); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + console.error(` ERROR: ${message}`); + console.error( + " Could not capture content seal — sandbox filesystem may be unreachable.", + ); + return failShieldsCommand(message, opts.throwOnError); + } + } clearTimerMarker(sandboxName); console.log(" Lockdown is already active."); return; } - // Content drift means a host-root tamper rewrote a locked file even - // while keeping the mode/owner correct. Re-locking would launder the - // tampered content into a fresh seal, so refuse and ask the operator + // Any hash-verification failure (content drift, sha256sum failure, + // unparsable output, missing seal entry) means the seal cannot be + // trusted. Re-locking on such a state would launder the tampered or + // unknown content into a fresh seal, so refuse and ask the operator // to restore the file (or rebuild the sandbox) before re-running. - const contentDrift = issues.filter((entry) => entry.includes("content drifted")); - if (contentDrift.length > 0) { - console.error(" ERROR: locked file content has drifted:"); - for (const entry of contentDrift) { + const hashIssues = issues.filter(isHashVerificationIssue); + if (hashIssues.length > 0) { + console.error(" ERROR: locked file seal cannot be trusted:"); + for (const entry of hashIssues) { console.error(` - ${entry}`); } console.error( " Refusing to re-seal a tampered baseline. Restore the file or rebuild the sandbox, then re-run shields up.", ); return failShieldsCommand( - `Locked file content drifted: ${contentDrift.join("; ")}`, + `Locked file seal cannot be trusted: ${hashIssues.join("; ")}`, opts.throwOnError, ); } diff --git a/src/lib/shields/seal.test.ts b/src/lib/shields/seal.test.ts new file mode 100644 index 00000000000..8a6cbf8e55a --- /dev/null +++ b/src/lib/shields/seal.test.ts @@ -0,0 +1,95 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; +import path from "node:path"; + +async function loadSeal(): Promise { + const distModulePath = path.join( + process.cwd(), + "dist", + "lib", + "shields", + "seal.js", + ); + return import(distModulePath); +} + +describe("parseSha256Output", () => { + it("returns the hex hash from a standard `sha256sum ` line", async () => { + const { parseSha256Output } = await loadSeal(); + const line = + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef /sandbox/.openclaw/openclaw.json"; + expect(parseSha256Output(line)).toBe( + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + ); + }); + + it("returns null for empty or whitespace-only input", async () => { + const { parseSha256Output } = await loadSeal(); + expect(parseSha256Output("")).toBeNull(); + expect(parseSha256Output(" \n\t ")).toBeNull(); + }); + + it("returns null when the first token is not a 64-char hex string", async () => { + const { parseSha256Output } = await loadSeal(); + expect(parseSha256Output("garbage output line")).toBeNull(); + expect(parseSha256Output("0123 /sandbox/.openclaw/openclaw.json")).toBeNull(); + // 65 chars + expect( + parseSha256Output( + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdefx /file", + ), + ).toBeNull(); + }); + + it("normalises uppercase hex to lowercase", async () => { + const { parseSha256Output } = await loadSeal(); + expect( + parseSha256Output( + "ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789ABCDEF0123456789 /file", + ), + ).toBe("abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"); + }); +}); + +describe("isHashVerificationIssue", () => { + it("matches every emitted hash-failure prefix so callers refuse to re-seal", async () => { + const { isHashVerificationIssue } = await loadSeal(); + expect( + isHashVerificationIssue( + "/sandbox/.openclaw/openclaw.json content drifted (sha256 ff != sealed 01)", + ), + ).toBe(true); + expect( + isHashVerificationIssue( + "/sandbox/.openclaw/openclaw.json sha256sum failed: I/O error", + ), + ).toBe(true); + expect( + isHashVerificationIssue( + "/sandbox/.openclaw/openclaw.json sha256sum output unparsable: garbage", + ), + ).toBe(true); + expect( + isHashVerificationIssue( + "/sandbox/.openclaw/openclaw.json no seal recorded (expected SHA-256)", + ), + ).toBe(true); + }); + + it("rejects unrelated perm-only entries so they remain launderable by re-lock", async () => { + const { isHashVerificationIssue } = await loadSeal(); + expect( + isHashVerificationIssue( + "/sandbox/.openclaw/openclaw.json mode=660 (expected 444)", + ), + ).toBe(false); + expect( + isHashVerificationIssue( + "/sandbox/.openclaw/openclaw.json owner=sandbox:sandbox (expected root:root)", + ), + ).toBe(false); + expect(isHashVerificationIssue("dir mode=2770 (expected 755)")).toBe(false); + }); +}); diff --git a/src/lib/shields/seal.ts b/src/lib/shields/seal.ts new file mode 100644 index 00000000000..f5814360139 --- /dev/null +++ b/src/lib/shields/seal.ts @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Shared helpers for the shields-up content seal. Centralised so the lock +// path that writes the seal and the status path that re-checks it share +// the same input contract (sha256sum output shape, hex normalisation). + +const SHA256_HEX = /^[0-9a-f]{64}$/i; + +export function parseSha256Output(raw: string): string | null { + const trimmed = raw.trim(); + if (!trimmed) return null; + const token = trimmed.split(/\s+/, 1)[0]; + return SHA256_HEX.test(token) ? token.toLowerCase() : null; +} + +// Issue-string prefixes the verifier emits for hash-related failures. +// Used by callers that need to classify whether drift is launderable +// (perms-only) or non-launderable (any hash-verification failure). +export const HASH_ISSUE_PATTERNS: readonly string[] = [ + "content drifted", + "sha256sum failed", + "sha256sum output unparsable", + "no seal recorded", +]; + +export function isHashVerificationIssue(entry: string): boolean { + return HASH_ISSUE_PATTERNS.some((p) => entry.includes(p)); +} diff --git a/src/lib/shields/timer.ts b/src/lib/shields/timer.ts index 42dec0b5118..9207a960751 100644 --- a/src/lib/shields/timer.ts +++ b/src/lib/shields/timer.ts @@ -24,6 +24,8 @@ interface ShieldsStatePatch { shieldsDownTimeout?: number | null; shieldsDownReason?: string | null; shieldsDownPolicy?: string | null; + chattrApplied?: boolean; + fileHashes?: { [path: string]: string }; } interface TimerArgs { @@ -187,6 +189,8 @@ function runRestoreTimer(args: TimerArgs): void { // that interactive `shields up` uses. Fall back to the bare configPath/ // configDir from argv if resolution fails (e.g., registry unavailable). let lockVerified = true; + let lockedChattr: boolean | null = null; + let lockedHashes: { [path: string]: string } | null = null; if (args.configPath) { let lockTarget: { agentName?: string; @@ -220,7 +224,9 @@ function runRestoreTimer(args: TimerArgs): void { } if (lockTarget) { try { - lockAgentConfig(args.sandboxName, lockTarget); + const lockResult = lockAgentConfig(args.sandboxName, lockTarget); + lockedChattr = lockResult.chattrApplied; + lockedHashes = lockResult.fileHashes; } catch (error: unknown) { lockVerified = false; appendAudit({ @@ -237,13 +243,16 @@ function runRestoreTimer(args: TimerArgs): void { // Only mark shields as UP if the lock was verified (or no config path). if (lockVerified) { - updateState(args.stateFile, { + const patch: ShieldsStatePatch = { shieldsDown: false, shieldsDownAt: null, shieldsDownTimeout: null, shieldsDownReason: null, shieldsDownPolicy: null, - }); + }; + if (lockedChattr !== null) patch.chattrApplied = lockedChattr; + if (lockedHashes !== null) patch.fileHashes = lockedHashes; + updateState(args.stateFile, patch); appendAudit({ action: "shields_auto_restore", diff --git a/src/lib/shields/verify-lock.test.ts b/src/lib/shields/verify-lock.test.ts index 2be1f63ec22..b7cc65408e3 100644 --- a/src/lib/shields/verify-lock.test.ts +++ b/src/lib/shields/verify-lock.test.ts @@ -191,7 +191,7 @@ describe("verifyShieldsLockState", () => { }); // --------------------------------------------------------------------------- - // Content-seal drift (#4243). Perm-only verification cannot catch + // Content-seal drift. Perm-only verification cannot catch // chmod-write-chmod cycles because the mode/owner end up identical to // the locked baseline. The hash compare is the only way to flag a // content tamper that restores the perms afterwards. @@ -227,7 +227,9 @@ describe("verifyShieldsLockState", () => { "/sandbox/.openclaw": "755 root:root", }, { - // openclaw.json hash differs from the seal — this is the #4243 repro. + // openclaw.json hash differs from the seal — the host-root tamper + // restored the perms after writing so only the content-seal check + // can catch it. "/sandbox/.openclaw/openclaw.json": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", "/sandbox/.openclaw/.config-hash": CLEAN_CONFIG_HASH_HASH, diff --git a/src/lib/shields/verify-lock.ts b/src/lib/shields/verify-lock.ts index 9e3fbd93c9a..811df241646 100644 --- a/src/lib/shields/verify-lock.ts +++ b/src/lib/shields/verify-lock.ts @@ -9,13 +9,15 @@ // and surface a content-drift entry on any mismatch. This catches the // host-root tamper pattern that defeats perm-only verification: chmod // to mutable -> write -> chmod back to 444 leaves mode/owner identical -// to the locked baseline but produces a new content hash (#4243). +// to the locked baseline but produces a new content hash. // // Returns the list of mismatches so callers can either fail the lock // operation or surface drift after a host-root tamper. Stat/lsattr/hash // failures are folded into `issues` so the caller can decide whether to // treat them as drift. +import { parseSha256Output } from "./seal"; + export type LockTarget = { configPath: string; configDir: string; @@ -38,23 +40,11 @@ const EXPECTED_FILE_MODE = "444"; const EXPECTED_DIR_MODE = "755"; const EXPECTED_OWNER = "root:root"; -const SHA256_HEX = /^[0-9a-f]{64}$/i; - function noopAssertLegacyLayout(_sandboxName: string, _configDir: string): void { // Production callers replace this with the real legacy-layout assertion; // when omitted, the verifier treats legacy-layout state as "no issue". } -function parseSha256Output(raw: string): string | null { - // `sha256sum ` prints ``. - // Tolerate leading whitespace so callers that pipe through `tr -d ' \t'` - // or trim themselves still get a clean comparison. - const trimmed = raw.trim(); - if (!trimmed) return null; - const token = trimmed.split(/\s+/, 1)[0]; - return SHA256_HEX.test(token) ? token.toLowerCase() : null; -} - export function verifyShieldsLockState( sandboxName: string, target: LockTarget, diff --git a/test/e2e/test-shields-config.sh b/test/e2e/test-shields-config.sh index 076c63f92bb..d04a070f061 100755 --- a/test/e2e/test-shields-config.sh +++ b/test/e2e/test-shields-config.sh @@ -10,6 +10,7 @@ # Phase 3: shields up — verify config becomes immutable # Phase 4: config get — read-only inspection # Phase 5: shields status — shows UP +# Phase 5b: Content-seal drift detection (chmod-write-chmod tamper) # Phase 6: shields down — verify config returns to writable # Phase 7: shields status — shows DOWN # Phase 8: Audit trail completeness @@ -313,6 +314,86 @@ else fail "shields status should show UP: ${STATUS_OUTPUT}" fi +# ══════════════════════════════════════════════════════════════════ +# Phase 5b: content-seal drift detection — host-root chmod-write-chmod +# ══════════════════════════════════════════════════════════════════ +# Verifies the SHA-256 content seal: a host-root tamper that rewrites a +# locked file and restores 444 root:root afterwards leaves mode/owner +# clean but produces a new content hash. `shields status` must flag this +# as drift, and `shields up` must refuse to launder the tampered +# baseline into a fresh seal. +section "Phase 5b: content-seal drift detection" + +CTR=$(docker ps --filter "name=openshell-${SANDBOX_NAME}" -q | head -n1) +if [ -z "$CTR" ]; then + fail "Could not find sandbox container for ${SANDBOX_NAME}" +else + ORIG_CONTENT=$(docker exec -u 0 "$CTR" cat "$CONFIG_PATH" 2>/dev/null || true) + if [ -z "$ORIG_CONTENT" ]; then + fail "Could not read original ${CONFIG_PATH} content as host root" + else + docker exec -u 0 "$CTR" sh -c \ + "chmod 644 ${CONFIG_PATH} && printf ' ' >> ${CONFIG_PATH} && chmod 444 ${CONFIG_PATH}" \ + >/dev/null 2>&1 + PERMS_AFTER_TAMPER=$(docker exec "$CTR" stat -c '%a %U:%G' "$CONFIG_PATH" 2>/dev/null || true) + info "Config perms after chmod-write-chmod tamper: ${PERMS_AFTER_TAMPER}" + if [ "$PERMS_AFTER_TAMPER" = "444 root:root" ]; then + pass "Tamper restored 444 root:root (mode/owner alone cannot detect drift)" + else + fail "Expected tamper to leave 444 root:root, got: ${PERMS_AFTER_TAMPER}" + fi + + set +e + STATUS_TAMPER_OUTPUT=$(nemoclaw "${SANDBOX_NAME}" shields status 2>&1) + STATUS_TAMPER_EXIT=$? + set -e + echo "$STATUS_TAMPER_OUTPUT" + if [ "$STATUS_TAMPER_EXIT" = "2" ]; then + pass "shields status exits 2 on content drift" + else + fail "shields status should exit 2 on content drift, got ${STATUS_TAMPER_EXIT}" + fi + if echo "$STATUS_TAMPER_OUTPUT" | grep -q "UP (DRIFTED"; then + pass "shields status surfaces DRIFTED on content drift" + else + fail "shields status should surface DRIFTED line on content drift" + fi + if echo "$STATUS_TAMPER_OUTPUT" | grep -q "content drifted"; then + pass "shields status names the drifted file" + else + fail "shields status should name the drifted file" + fi + + set +e + REUP_OUTPUT=$(nemoclaw "${SANDBOX_NAME}" shields up 2>&1) + REUP_EXIT=$? + set -e + echo "$REUP_OUTPUT" + if [ "$REUP_EXIT" != "0" ]; then + pass "shields up refuses to re-seal a tampered baseline (exit ${REUP_EXIT})" + else + fail "shields up should refuse to re-seal a tampered baseline" + fi + if echo "$REUP_OUTPUT" | grep -q "Refusing to re-seal"; then + pass "shields up surfaces the refuse-to-re-seal message" + else + fail "shields up should surface the refuse-to-re-seal message" + fi + + # Restore the original content as host root so the rest of the suite + # can continue against a clean lock. + docker exec -u 0 "$CTR" sh -c \ + "chmod 644 ${CONFIG_PATH} && cat > ${CONFIG_PATH} && chmod 444 ${CONFIG_PATH}" \ + <<<"$ORIG_CONTENT" >/dev/null 2>&1 + POST_RESTORE_OUTPUT=$(nemoclaw "${SANDBOX_NAME}" shields status 2>&1 || true) + if echo "$POST_RESTORE_OUTPUT" | grep -q "Shields: UP (lockdown active)"; then + pass "shields status clean after content restore" + else + fail "shields status should report clean UP after content restore: ${POST_RESTORE_OUTPUT}" + fi + fi +fi + # ══════════════════════════════════════════════════════════════════ # Phase 6: shields down — config returns to writable # ══════════════════════════════════════════════════════════════════ From 8b241be8f416396ebe4ba00f5914a04b0be279a9 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 29 May 2026 02:37:50 +0000 Subject: [PATCH 3/8] fix(shields): gate legacy seal, split recovery hint, harden e2e backup Signed-off-by: Tinson Lai --- docs/security/best-practices.mdx | 8 ++++- src/lib/shields/index.test.ts | 31 ++++++++++++++++ src/lib/shields/index.ts | 61 +++++++++++++++++++++++++++----- test/e2e/test-shields-config.sh | 28 ++++++++++----- 4 files changed, 109 insertions(+), 19 deletions(-) diff --git a/docs/security/best-practices.mdx b/docs/security/best-practices.mdx index 6ecde79d152..26525ed43dd 100644 --- a/docs/security/best-practices.mdx +++ b/docs/security/best-practices.mdx @@ -204,7 +204,13 @@ For sensitive workloads, use a reviewed host-side immutability workflow after in - **DAC permissions (default).** The sandbox user owns `/sandbox/.openclaw` with mode `2770` (setgid `sandbox:sandbox`) and `openclaw.json` with mode `660`, so the agent and its group can read and write config directly. A reviewed host-side immutability workflow should compare the intended ownership and mode with the live sandbox filesystem before treating the config tree as locked. - **Config integrity hash.** The image includes a SHA256 hash of `openclaw.json`. In the default mutable state, `.config-hash` is sandbox-owned and is not a tamper-proof trust anchor, so startup does not fail closed on that hash. When the hash is root-owned and read-only, startup enforces it and refuses to start if the hash does not match. -- **Content seal under shields up.** When `nemoclaw shields up` runs against a clean lock, it captures a SHA-256 seal of `openclaw.json` and any other locked files into the host-side shields state file. On sealed sandboxes, every `shields status` call recomputes the hash inside the sandbox and surfaces drift on any mismatch, so a host-root tamper that flips perms back to `444 root:root` after rewriting the file is still flagged. Sandboxes that were already locked before this seal landed have no recorded hash; the first `shields up` after upgrade captures one even when the verifier reports a clean lock, and `shields status` shows a one-line notice until that happens. `shields up` refuses to re-seal a tampered baseline; restore the original file or rebuild the sandbox before re-running. +- **Content seal under shields up.** + When `nemoclaw shields up` runs against a clean lock, it captures a SHA-256 seal of `openclaw.json` and any other locked files into the host-side shields state file. + On sealed sandboxes, every `shields status` call recomputes the hash inside the sandbox and surfaces drift on any mismatch, so a host-root tamper that flips perms back to `444 root:root` after rewriting the file is still flagged. + Sandboxes locked before this seal landed have no recorded hash; perm-only verification cannot prove their bytes match the image-original, so the seal is **not** a retroactive proof of integrity for legacy state. + By default, `shields up` refuses to seal such a baseline and asks the operator to rebuild the sandbox first for a known-good baseline. + Operators who explicitly trust the current bytes can opt in via `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1`, which captures a seal over the current files and is acknowledged in the log line. + Once a sandbox is sealed, `shields up` refuses to re-seal a tampered baseline; restore the original file or rebuild the sandbox before re-running. - **Gateway token environment.** The gateway exports `OPENCLAW_GATEWAY_TOKEN` and writes it to `/tmp/nemoclaw-proxy-env.sh` for interactive sandbox sessions. Keep this in mind when deciding whether a workload should run with mutable config or an immutable config posture. | Aspect | Detail | diff --git a/src/lib/shields/index.test.ts b/src/lib/shields/index.test.ts index be75530fb07..e8acf9b30e2 100644 --- a/src/lib/shields/index.test.ts +++ b/src/lib/shields/index.test.ts @@ -633,6 +633,37 @@ describe("shields — unit logic", () => { ); }); + it("rejects state files whose fileHashes entries are not SHA-256 hex strings", async () => { + const sandboxName = "openclaw"; + fs.mkdirSync(stateDir(), { recursive: true }); + // Hash value is the right length but contains non-hex chars, + // and another value is far too short. Either alone should fail + // the isOptionalHashMap guard. + fs.writeFileSync( + path.join(stateDir(), `shields-${sandboxName}.json`), + JSON.stringify({ + shieldsDown: false, + fileHashes: { + "/sandbox/.openclaw/openclaw.json": "not-a-real-hash", + }, + updatedAt: new Date().toISOString(), + }), + ); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((code?: string | number | null) => { + throw new Error(`exit ${String(code)}`); + }); + + const { shieldsStatus } = await loadShieldsModule(); + expect(() => shieldsStatus(sandboxName)).toThrow("exit 1"); + expect(errorSpy).toHaveBeenCalledWith( + " Shields: ERROR (state file is corrupt)", + ); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + it("status fails fast on corrupt shields state instead of reporting NOT CONFIGURED", async () => { const sandboxName = "openclaw"; fs.mkdirSync(stateDir(), { recursive: true }); diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 9a96cf678cf..e1201e9a4b9 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -295,13 +295,19 @@ function isOptionalNullableNumber( ); } +// SHA-256 hex strings are 64 lowercase or uppercase hex chars. The seal +// helper normalises to lowercase before persisting; accept either case +// here so manually edited state files and legacy uppercase entries still +// load, and reject anything that cannot be a real digest. +const SHA256_HEX_RE = /^[0-9a-f]{64}$/i; + function isOptionalHashMap( value: unknown, ): value is { [path: string]: string } | undefined { if (value === undefined) return true; if (!isObjectRecord(value)) return false; for (const v of Object.values(value)) { - if (typeof v !== "string") return false; + if (typeof v !== "string" || !SHA256_HEX_RE.test(v)) return false; } return true; } @@ -1149,11 +1155,37 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): expectedHashes: state.fileHashes, }); if (issues.length === 0) { - // Legacy locked state predates the content seal. Capture one now so - // future `shields status` calls can detect content drift instead of - // relying on perm-only verification. We only do this when the - // verifier was already happy with the on-disk state. + // Legacy locked state predates the content seal. Capturing a seal + // now would treat the *current* file content as the trusted + // baseline, but perm-only verification gives no proof that the + // bytes match the image-original. A pre-existing content tamper + // would be sealed in as the new "trusted" content. + // + // Refuse by default and ask the operator to either: + // 1. rebuild the sandbox (clean baseline), then `shields up`; + // 2. explicitly opt in via `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1` + // to acknowledge that the current bytes are trusted. + // Once the operator opts in, the seal is captured and subsequent + // `shields up`/`shields status` runs detect any future drift. if (!state.fileHashes) { + if (process.env.NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE !== "1") { + console.error( + " ERROR: locked sandbox has no content seal (state predates the seal).", + ); + console.error( + " Perm-only verification cannot prove the locked files have not already been tampered with.", + ); + console.error( + ` Recovery: rebuild the sandbox for a known-good baseline, then run \`nemoclaw ${sandboxName} shields up\`.`, + ); + console.error( + ` Or accept the current bytes as the trusted baseline by setting NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1 and rerunning.`, + ); + return failShieldsCommand( + "Locked sandbox has no content seal; refusing to seal a legacy baseline without explicit operator acknowledgement", + opts.throwOnError, + ); + } try { const filesToHash = [ target.configPath, @@ -1162,7 +1194,7 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): const newHashes = captureSealHashes(sandboxName, filesToHash); saveShieldsState(sandboxName, { fileHashes: newHashes }); console.log( - " Captured SHA-256 content seal for existing lockdown.", + " Captured SHA-256 content seal for existing lockdown (current bytes accepted as baseline).", ); } catch (err) { const message = err instanceof Error ? err.message : String(err); @@ -1413,9 +1445,20 @@ function shieldsStatus( for (const issue of driftIssues) { console.error(` - ${issue}`); } - console.error( - ` Recovery: nemoclaw ${sandboxName} shields up # re-lock and re-verify`, - ); + // Hash-trust failures cannot be repaired by re-locking — re-up + // would just seal the tampered or unverifiable content. Perm + // drift (mode/owner/chattr/legacy-layout) is launderable by + // re-up. Surface the right recovery for the failure mode. + const hasHashTrouble = driftIssues.some(isHashVerificationIssue); + if (hasHashTrouble) { + console.error( + ` Recovery: restore the original file content from a trusted source, or rebuild the sandbox, then run \`nemoclaw ${sandboxName} shields up\` to re-seal.`, + ); + } else { + console.error( + ` Recovery: nemoclaw ${sandboxName} shields up # re-lock and re-verify`, + ); + } process.exit(2); } console.log(` Shields: ${posture.statusText}`); diff --git a/test/e2e/test-shields-config.sh b/test/e2e/test-shields-config.sh index d04a070f061..0f1eca60ae1 100755 --- a/test/e2e/test-shields-config.sh +++ b/test/e2e/test-shields-config.sh @@ -328,9 +328,16 @@ CTR=$(docker ps --filter "name=openshell-${SANDBOX_NAME}" -q | head -n1) if [ -z "$CTR" ]; then fail "Could not find sandbox container for ${SANDBOX_NAME}" else - ORIG_CONTENT=$(docker exec -u 0 "$CTR" cat "$CONFIG_PATH" 2>/dev/null || true) - if [ -z "$ORIG_CONTENT" ]; then + # Use a byte-preserving temp file for backup/restore. Bash command + # substitution `$(...)` strips trailing newlines, which would change + # the file's SHA-256 between backup and restore and create false + # drift after the post-restore status check. + ORIG_CONTENT_FILE=$(mktemp -t nemoclaw-shields-orig.XXXXXX) + trap 'rm -f "$ORIG_CONTENT_FILE"' EXIT + if ! docker exec -u 0 "$CTR" cat "$CONFIG_PATH" >"$ORIG_CONTENT_FILE" 2>/dev/null; then fail "Could not read original ${CONFIG_PATH} content as host root" + elif [ ! -s "$ORIG_CONTENT_FILE" ]; then + fail "Original ${CONFIG_PATH} read returned an empty file" else docker exec -u 0 "$CTR" sh -c \ "chmod 644 ${CONFIG_PATH} && printf ' ' >> ${CONFIG_PATH} && chmod 444 ${CONFIG_PATH}" \ @@ -343,10 +350,13 @@ else fail "Expected tamper to leave 444 root:root, got: ${PERMS_AFTER_TAMPER}" fi - set +e + # The script runs with `set -uo pipefail` (no -e), so `$?` after a + # command substitution gives that command's exit code without + # aborting the script. Toggling `set -e` here would interact badly + # with the `fail()` helper, whose `((FAIL++))` returns a non-zero + # exit when FAIL is 0 and would abort under -e. STATUS_TAMPER_OUTPUT=$(nemoclaw "${SANDBOX_NAME}" shields status 2>&1) STATUS_TAMPER_EXIT=$? - set -e echo "$STATUS_TAMPER_OUTPUT" if [ "$STATUS_TAMPER_EXIT" = "2" ]; then pass "shields status exits 2 on content drift" @@ -364,10 +374,8 @@ else fail "shields status should name the drifted file" fi - set +e REUP_OUTPUT=$(nemoclaw "${SANDBOX_NAME}" shields up 2>&1) REUP_EXIT=$? - set -e echo "$REUP_OUTPUT" if [ "$REUP_EXIT" != "0" ]; then pass "shields up refuses to re-seal a tampered baseline (exit ${REUP_EXIT})" @@ -381,10 +389,12 @@ else fi # Restore the original content as host root so the rest of the suite - # can continue against a clean lock. - docker exec -u 0 "$CTR" sh -c \ + # can continue against a clean lock. `docker exec -i` keeps stdin + # open and we stream the backup file straight in — no command + # substitution that would strip trailing newlines. + docker exec -i -u 0 "$CTR" sh -c \ "chmod 644 ${CONFIG_PATH} && cat > ${CONFIG_PATH} && chmod 444 ${CONFIG_PATH}" \ - <<<"$ORIG_CONTENT" >/dev/null 2>&1 + <"$ORIG_CONTENT_FILE" >/dev/null 2>&1 POST_RESTORE_OUTPUT=$(nemoclaw "${SANDBOX_NAME}" shields status 2>&1 || true) if echo "$POST_RESTORE_OUTPUT" | grep -q "Shields: UP (lockdown active)"; then pass "shields status clean after content restore" From 44bd0eb2364ec42c23ca02ae8a122952004343c7 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 29 May 2026 02:52:43 +0000 Subject: [PATCH 4/8] fix(shields): document legacy-baseline env var, prefix hash failures Signed-off-by: Tinson Lai --- docs/reference/commands.mdx | 1 + src/lib/shields/verify-lock.test.ts | 2 +- src/lib/shields/verify-lock.ts | 13 ++++++++++--- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 1b058d80fe6..590813e7a49 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1396,6 +1396,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 | When `nemoclaw shields up` runs against a sandbox that was locked before the SHA-256 content seal landed, the existing on-disk bytes have no independently verified baseline. By default, `shields up` refuses to capture a seal in that state and asks the operator to rebuild the sandbox for a known-good baseline. Set this to `1` to accept the current bytes as the trusted baseline and let the seal be captured anyway. Once captured, subsequent `shields status` runs detect any future drift. | ## NemoHermes Alias diff --git a/src/lib/shields/verify-lock.test.ts b/src/lib/shields/verify-lock.test.ts index b7cc65408e3..220acf1cfc5 100644 --- a/src/lib/shields/verify-lock.test.ts +++ b/src/lib/shields/verify-lock.test.ts @@ -304,7 +304,7 @@ describe("verifyShieldsLockState", () => { expect(result.ok).toBe(false); expect(result.issues).toContain( - "/sandbox/.openclaw/.config-hash no seal recorded (expected SHA-256)", + "/sandbox/.openclaw/.config-hash content drifted (no seal recorded; expected SHA-256)", ); }); diff --git a/src/lib/shields/verify-lock.ts b/src/lib/shields/verify-lock.ts index 811df241646..56541a02142 100644 --- a/src/lib/shields/verify-lock.ts +++ b/src/lib/shields/verify-lock.ts @@ -106,7 +106,12 @@ export function verifyShieldsLockState( // Seal was missing for this file — flag explicitly rather than // silently passing. Callers that genuinely lack a seal pass // `expectedHashes: undefined` instead of an empty record. - issues.push(`${f} no seal recorded (expected SHA-256)`); + // Prefix with "content drifted" so callers that filter on that + // substring (`shieldsUp` re-seal refusal) treat every hash-trust + // failure as non-launderable. + issues.push( + `${f} content drifted (no seal recorded; expected SHA-256)`, + ); continue; } let raw: string; @@ -114,12 +119,14 @@ export function verifyShieldsLockState( raw = exec(["sha256sum", f]); } catch (err) { const msg = err instanceof Error ? err.message : String(err); - issues.push(`${f} sha256sum failed: ${msg}`); + issues.push(`${f} content drifted (sha256sum failed: ${msg})`); continue; } const got = parseSha256Output(raw); if (!got) { - issues.push(`${f} sha256sum output unparsable: ${raw.trim()}`); + issues.push( + `${f} content drifted (sha256sum output unparsable: ${raw.trim()})`, + ); continue; } if (got !== want.toLowerCase()) { From 2647579239ff365c48b7fcb8430b213c6a7c1e3a Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 29 May 2026 03:07:48 +0000 Subject: [PATCH 5/8] fix(shields): preserve sensitiveFiles in timer auto-restore lock target Signed-off-by: Tinson Lai --- src/lib/shields/timer.ts | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/lib/shields/timer.ts b/src/lib/shields/timer.ts index 9207a960751..ff82be4b5d0 100644 --- a/src/lib/shields/timer.ts +++ b/src/lib/shields/timer.ts @@ -13,7 +13,7 @@ import path from "node:path"; import { isRecord, type UnknownRecord } from "../core/json-types"; import { buildPolicySetCommand } from "../policy"; import { run } from "../runner"; -import { DEFAULT_AGENT_CONFIG, resolveAgentConfig } from "../sandbox/config"; +import { resolveAgentConfig } from "../sandbox/config"; import { resolveNemoclawStateDir } from "../state/paths"; import { appendAuditEntry, type ShieldsAuditEntry } from "./audit"; import { lockAgentConfig } from "./index"; @@ -199,17 +199,22 @@ function runRestoreTimer(args: TimerArgs): void { sensitiveFiles?: string[]; } | null = null; try { - const resolvedTarget = resolveAgentConfig(args.sandboxName); - if (resolvedTarget === DEFAULT_AGENT_CONFIG && args.configDir) { - lockTarget = { configPath: args.configPath, configDir: args.configDir }; - } else { - lockTarget = resolvedTarget; - } + // Always prefer the resolved target — even DEFAULT_AGENT_CONFIG + // carries the OpenClaw sensitiveFiles (.config-hash) that + // shields-up locks and that the content seal hashes. Dropping + // them here would persist a partial fileHashes map and the next + // `shields status` would flag the missing entries as drift. + lockTarget = resolveAgentConfig(args.sandboxName); } catch { - // Fall back to argv-supplied paths without sensitive files — - // better to lock the main config than nothing at all. + // Resolver itself threw (registry unavailable). Fall back to + // argv-supplied paths, but still infer sensitiveFiles from + // configDir so the locked set matches what shields-up uses. if (args.configDir) { - lockTarget = { configPath: args.configPath, configDir: args.configDir }; + lockTarget = { + configPath: args.configPath, + configDir: args.configDir, + sensitiveFiles: [`${args.configDir}/.config-hash`], + }; } else { lockVerified = false; appendAudit({ From 7b6923608619130a297030267850dbe00687b32f Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 29 May 2026 03:56:21 +0000 Subject: [PATCH 6/8] fix(shields): align legacy notice + dedupe SHA regex, harden timer test Signed-off-by: Tinson Lai --- docs/reference/commands.mdx | 2 +- docs/security/best-practices.mdx | 4 +- src/lib/shields/index.test.ts | 5 ++- src/lib/shields/index.ts | 24 +++++++---- src/lib/shields/seal.ts | 11 ++++- src/lib/shields/timer.test.ts | 70 ++++++++++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 14 deletions(-) diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 590813e7a49..a92a31c0c60 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1396,7 +1396,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 | When `nemoclaw shields up` runs against a sandbox that was locked before the SHA-256 content seal landed, the existing on-disk bytes have no independently verified baseline. By default, `shields up` refuses to capture a seal in that state and asks the operator to rebuild the sandbox for a known-good baseline. Set this to `1` to accept the current bytes as the trusted baseline and let the seal be captured anyway. Once captured, subsequent `shields status` runs detect any future drift. | +| `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE` | `1` to opt in | When `nemoclaw shields up` runs against a sandbox that was locked before the SHA-256 content seal landed, the existing on-disk bytes have no independently verified baseline. By default, `shields up` refuses to capture a seal in that state and asks you to rebuild the sandbox for a known-good baseline. Set this to `1` to accept the current bytes as the trusted baseline and let the seal be captured anyway. Once captured, subsequent `shields status` runs detect any future drift. | ## NemoHermes Alias diff --git a/docs/security/best-practices.mdx b/docs/security/best-practices.mdx index 26525ed43dd..5e0e81473f3 100644 --- a/docs/security/best-practices.mdx +++ b/docs/security/best-practices.mdx @@ -208,8 +208,8 @@ For sensitive workloads, use a reviewed host-side immutability workflow after in When `nemoclaw shields up` runs against a clean lock, it captures a SHA-256 seal of `openclaw.json` and any other locked files into the host-side shields state file. On sealed sandboxes, every `shields status` call recomputes the hash inside the sandbox and surfaces drift on any mismatch, so a host-root tamper that flips perms back to `444 root:root` after rewriting the file is still flagged. Sandboxes locked before this seal landed have no recorded hash; perm-only verification cannot prove their bytes match the image-original, so the seal is **not** a retroactive proof of integrity for legacy state. - By default, `shields up` refuses to seal such a baseline and asks the operator to rebuild the sandbox first for a known-good baseline. - Operators who explicitly trust the current bytes can opt in via `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1`, which captures a seal over the current files and is acknowledged in the log line. + By default, `shields up` refuses to seal such a baseline and asks you to rebuild the sandbox first for a known-good baseline. + If you explicitly trust the current bytes, opt in via `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1`, which captures a seal over the current files and is acknowledged in the log line. Once a sandbox is sealed, `shields up` refuses to re-seal a tampered baseline; restore the original file or rebuild the sandbox before re-running. - **Gateway token environment.** The gateway exports `OPENCLAW_GATEWAY_TOKEN` and writes it to `/tmp/nemoclaw-proxy-env.sh` for interactive sandbox sessions. Keep this in mind when deciding whether a workload should run with mutable config or an immutable config posture. diff --git a/src/lib/shields/index.test.ts b/src/lib/shields/index.test.ts index e8acf9b30e2..767fe68bf3f 100644 --- a/src/lib/shields/index.test.ts +++ b/src/lib/shields/index.test.ts @@ -853,7 +853,10 @@ describe("shields — unit logic", () => { const lines = logSpy.mock.calls.map((args) => args[0]).join("\n"); expect(lines).toContain( - `Notice: no content seal recorded; re-run \`nemoclaw ${sandboxName} shields up\` to capture one for drift detection.`, + "Notice: no content seal recorded; rebuild the sandbox for a known-good baseline", + ); + expect(lines).toContain( + `or set NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1 and re-run \`nemoclaw ${sandboxName} shields up\` to seal the current bytes.`, ); }); diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index e1201e9a4b9..16a57fe9b4f 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -51,6 +51,7 @@ const { const { parseSha256Output, isHashVerificationIssue, + isSha256Hex, }: typeof import("./seal") = require("./seal"); const STATE_DIR = resolveNemoclawStateDir(); @@ -115,8 +116,10 @@ interface ShieldsState { // host-root tamper pattern that defeats perm-only checks: chmod to // mutable -> write -> chmod back to 444 leaves mode/owner identical to // the locked baseline but produces a new content hash. Absent on state - // files captured before the seal landed; the first `shields up` after - // upgrade captures one even when the verifier reports a clean lock. + // files captured before the seal landed; on those legacy lockdowns + // `shields up` refuses to seal an unverified baseline by default and + // asks the operator to rebuild the sandbox, or to opt in via + // `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1`. fileHashes?: { [path: string]: string }; updatedAt?: string; } @@ -298,16 +301,16 @@ function isOptionalNullableNumber( // SHA-256 hex strings are 64 lowercase or uppercase hex chars. The seal // helper normalises to lowercase before persisting; accept either case // here so manually edited state files and legacy uppercase entries still -// load, and reject anything that cannot be a real digest. -const SHA256_HEX_RE = /^[0-9a-f]{64}$/i; - +// load, and reject anything that cannot be a real digest. Uses the same +// `isSha256Hex` predicate as the verifier so the persisted-state and +// runtime contracts stay aligned. function isOptionalHashMap( value: unknown, ): value is { [path: string]: string } | undefined { if (value === undefined) return true; if (!isObjectRecord(value)) return false; for (const v of Object.values(value)) { - if (typeof v !== "string" || !SHA256_HEX_RE.test(v)) return false; + if (typeof v !== "string" || !isSha256Hex(v)) return false; } return true; } @@ -1469,9 +1472,14 @@ function shieldsStatus( if (!state.fileHashes) { // Legacy state file pre-dates the content seal — perm-only // verification cannot catch a host-root chmod-write-chmod tamper - // cycle. Recommend re-locking to capture a SHA-256 seal. + // cycle. `shields up` refuses to seal an unverified baseline by + // default, so point at the recovery paths instead of suggesting + // a bare re-up that will abort. + console.log( + " Notice: no content seal recorded; rebuild the sandbox for a known-good baseline", + ); console.log( - ` Notice: no content seal recorded; re-run \`nemoclaw ${sandboxName} shields up\` to capture one for drift detection.`, + ` or set NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1 and re-run \`nemoclaw ${sandboxName} shields up\` to seal the current bytes.`, ); } return; diff --git a/src/lib/shields/seal.ts b/src/lib/shields/seal.ts index f5814360139..ddb201ad160 100644 --- a/src/lib/shields/seal.ts +++ b/src/lib/shields/seal.ts @@ -5,13 +5,20 @@ // path that writes the seal and the status path that re-checks it share // the same input contract (sha256sum output shape, hex normalisation). -const SHA256_HEX = /^[0-9a-f]{64}$/i; +// Single source of truth for the SHA-256 hex shape used across the +// shields module: by the verifier, the lock-time seal capture, and the +// `ShieldsState.fileHashes` schema guard. +export const SHA256_HEX_RE = /^[0-9a-f]{64}$/i; + +export function isSha256Hex(value: string): boolean { + return SHA256_HEX_RE.test(value); +} export function parseSha256Output(raw: string): string | null { const trimmed = raw.trim(); if (!trimmed) return null; const token = trimmed.split(/\s+/, 1)[0]; - return SHA256_HEX.test(token) ? token.toLowerCase() : null; + return isSha256Hex(token) ? token.toLowerCase() : null; } // Issue-string prefixes the verifier emits for hash-related failures. diff --git a/src/lib/shields/timer.test.ts b/src/lib/shields/timer.test.ts index 4650c1bbc8b..4257876d1ed 100644 --- a/src/lib/shields/timer.test.ts +++ b/src/lib/shields/timer.test.ts @@ -211,4 +211,74 @@ describe("shields timer authorization", () => { expect(updatedState.shieldsDownAt).toBeNull(); expect(fs.existsSync(markerPath)).toBe(false); }); + + it("persists chattrApplied and fileHashes from the auto-restore lock result", 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); + const updatedState = JSON.parse(fs.readFileSync(stateFile, "utf-8")); + + expect(exitCode).toBe(0); + expect(runMock).toHaveBeenCalledTimes(1); + expect(lockMock).toHaveBeenCalledTimes(1); + expect(updatedState.shieldsDown).toBe(false); + expect(updatedState.chattrApplied).toBe(true); + expect(updatedState.fileHashes).toEqual(sealedHashes); + expect(updatedState.fileHashes[sensitiveHashPath]).toBeDefined(); + expect(fs.existsSync(markerPath)).toBe(false); + }); }); From c385270fe92cbe9f715cfb467b2f9b59e424ceb4 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 29 May 2026 04:18:52 +0000 Subject: [PATCH 7/8] fix(shields): cover partial seal opt-in, drop e2e EXIT trap clash Signed-off-by: Tinson Lai --- src/lib/shields/index.ts | 114 ++++++++++++++++++-------------- test/e2e/test-shields-config.sh | 7 +- 2 files changed, 70 insertions(+), 51 deletions(-) diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 16a57fe9b4f..a02901ac033 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -1157,38 +1157,69 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): assertLegacyLayout: assertNoLegacyStateLayout, expectedHashes: state.fileHashes, }); + // Classify the verifier output. "no seal recorded" entries mean the + // verifier wanted a hash for a file that has no recorded baseline — + // this happens both for legacy lockdowns (no fileHashes at all) and + // for partial lockdowns whose seal predates a newly added sensitive + // file. Everything else under `isHashVerificationIssue` is a real + // content-trust failure (drift, sha256sum failure, unparsable + // output) and never launderable. + const hashIssues = issues.filter(isHashVerificationIssue); + const realHashDrift = hashIssues.filter( + (entry) => !entry.includes("no seal recorded"), + ); + if (realHashDrift.length > 0) { + console.error(" ERROR: locked file seal cannot be trusted:"); + for (const entry of realHashDrift) { + console.error(` - ${entry}`); + } + console.error( + " Refusing to re-seal a tampered baseline. Restore the file or rebuild the sandbox, then re-run shields up.", + ); + return failShieldsCommand( + `Locked file seal cannot be trusted: ${realHashDrift.join("; ")}`, + opts.throwOnError, + ); + } + + // Legacy lockdown (no fileHashes at all) or partial lockdown (some + // sealed, some missing because the locked-file set grew between + // releases). Both cases would seal the *current* bytes as the new + // trusted baseline, which perm-only verification cannot prove are + // untampered. Require explicit operator opt-in via the env var. + const hasMissingSeals = hashIssues.length > realHashDrift.length; + const requiresLegacyOptIn = !state.fileHashes || hasMissingSeals; + if ( + requiresLegacyOptIn && + process.env.NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE !== "1" + ) { + console.error( + state.fileHashes + ? " ERROR: locked sandbox seal is missing entries (locked file set grew after the existing seal was captured)." + : " ERROR: locked sandbox has no content seal (state predates the seal).", + ); + console.error( + " Perm-only verification cannot prove the unsealed files have not already been tampered with.", + ); + console.error( + ` Recovery: rebuild the sandbox for a known-good baseline, then run \`nemoclaw ${sandboxName} shields up\`.`, + ); + console.error( + ` Or accept the current bytes as the trusted baseline by setting NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1 and rerunning.`, + ); + return failShieldsCommand( + state.fileHashes + ? "Locked sandbox seal is incomplete; refusing to seal the missing entries without explicit operator acknowledgement" + : "Locked sandbox has no content seal; refusing to seal a legacy baseline without explicit operator acknowledgement", + opts.throwOnError, + ); + } + if (issues.length === 0) { - // Legacy locked state predates the content seal. Capturing a seal - // now would treat the *current* file content as the trusted - // baseline, but perm-only verification gives no proof that the - // bytes match the image-original. A pre-existing content tamper - // would be sealed in as the new "trusted" content. - // - // Refuse by default and ask the operator to either: - // 1. rebuild the sandbox (clean baseline), then `shields up`; - // 2. explicitly opt in via `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1` - // to acknowledge that the current bytes are trusted. - // Once the operator opts in, the seal is captured and subsequent - // `shields up`/`shields status` runs detect any future drift. + // Verifier saw a clean lock. If the legacy-baseline opt-in was + // required (no fileHashes), capture the seal now so future + // `shields status` runs can detect content drift. if (!state.fileHashes) { - if (process.env.NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE !== "1") { - console.error( - " ERROR: locked sandbox has no content seal (state predates the seal).", - ); - console.error( - " Perm-only verification cannot prove the locked files have not already been tampered with.", - ); - console.error( - ` Recovery: rebuild the sandbox for a known-good baseline, then run \`nemoclaw ${sandboxName} shields up\`.`, - ); - console.error( - ` Or accept the current bytes as the trusted baseline by setting NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1 and rerunning.`, - ); - return failShieldsCommand( - "Locked sandbox has no content seal; refusing to seal a legacy baseline without explicit operator acknowledgement", - opts.throwOnError, - ); - } try { const filesToHash = [ target.configPath, @@ -1212,25 +1243,10 @@ function shieldsUp(sandboxName: string, opts: { throwOnError?: boolean } = {}): console.log(" Lockdown is already active."); return; } - // Any hash-verification failure (content drift, sha256sum failure, - // unparsable output, missing seal entry) means the seal cannot be - // trusted. Re-locking on such a state would launder the tampered or - // unknown content into a fresh seal, so refuse and ask the operator - // to restore the file (or rebuild the sandbox) before re-running. - const hashIssues = issues.filter(isHashVerificationIssue); - if (hashIssues.length > 0) { - console.error(" ERROR: locked file seal cannot be trusted:"); - for (const entry of hashIssues) { - console.error(` - ${entry}`); - } - console.error( - " Refusing to re-seal a tampered baseline. Restore the file or rebuild the sandbox, then re-run shields up.", - ); - return failShieldsCommand( - `Locked file seal cannot be trusted: ${hashIssues.join("; ")}`, - opts.throwOnError, - ); - } + // At this point the verifier still flagged something: perm drift, or + // missing-seal entries that the operator has just opted in to. In + // both cases re-applying the lock rewrites perms and captures a + // fresh, complete seal. console.log( ` Lockdown drifted — re-applying lock for ${sandboxName}...`, ); diff --git a/test/e2e/test-shields-config.sh b/test/e2e/test-shields-config.sh index 0f1eca60ae1..e8d42643cf3 100755 --- a/test/e2e/test-shields-config.sh +++ b/test/e2e/test-shields-config.sh @@ -331,9 +331,11 @@ else # Use a byte-preserving temp file for backup/restore. Bash command # substitution `$(...)` strips trailing newlines, which would change # the file's SHA-256 between backup and restore and create false - # drift after the post-restore status check. + # drift after the post-restore status check. The temp file is cleaned + # up at the end of the phase — do not install an EXIT trap here + # because `sandbox-teardown.sh` already owns the EXIT trap and a bare + # `trap '...' EXIT` would clobber the sandbox cleanup. ORIG_CONTENT_FILE=$(mktemp -t nemoclaw-shields-orig.XXXXXX) - trap 'rm -f "$ORIG_CONTENT_FILE"' EXIT if ! docker exec -u 0 "$CTR" cat "$CONFIG_PATH" >"$ORIG_CONTENT_FILE" 2>/dev/null; then fail "Could not read original ${CONFIG_PATH} content as host root" elif [ ! -s "$ORIG_CONTENT_FILE" ]; then @@ -402,6 +404,7 @@ else fail "shields status should report clean UP after content restore: ${POST_RESTORE_OUTPUT}" fi fi + rm -f "$ORIG_CONTENT_FILE" fi # ══════════════════════════════════════════════════════════════════ From 3b2fb476bbe42a36566a84a21812498c1b77639d Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Fri, 29 May 2026 04:37:00 +0000 Subject: [PATCH 8/8] fix(shields): UNSEALED status exit 2, harden e2e tamper around chattr Signed-off-by: Tinson Lai --- docs/reference/commands.mdx | 2 +- docs/security/best-practices.mdx | 4 ++- src/lib/shields/index.test.ts | 52 +++++++++++++++++++++++--------- src/lib/shields/index.ts | 35 +++++++++++++-------- test/e2e/test-shields-config.sh | 33 ++++++++++++++++++-- 5 files changed, 93 insertions(+), 33 deletions(-) diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index a92a31c0c60..db12681e725 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1396,7 +1396,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 | When `nemoclaw shields up` runs against a sandbox that was locked before the SHA-256 content seal landed, the existing on-disk bytes have no independently verified baseline. By default, `shields up` refuses to capture a seal in that state and asks you to rebuild the sandbox for a known-good baseline. Set this to `1` to accept the current bytes as the trusted baseline and let the seal be captured anyway. Once captured, subsequent `shields status` runs detect any future drift. | +| `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE` | `1` to opt in | Applies in two cases: (1) sandboxes that were locked before the SHA-256 content seal landed (no `fileHashes` in shields state), and (2) partial seals where the locked file set grew after the existing seal was captured (some entries sealed, some missing). In both cases the existing on-disk bytes for the unsealed files have no independently verified baseline. By default, `shields up` refuses to capture a seal and asks you to rebuild the sandbox for a known-good baseline. Set this to `1` to accept the current bytes as the trusted baseline and let the seal be captured anyway. Once captured, subsequent `shields status` runs detect any future drift. | ## NemoHermes Alias diff --git a/docs/security/best-practices.mdx b/docs/security/best-practices.mdx index 5e0e81473f3..19cf3a37f9b 100644 --- a/docs/security/best-practices.mdx +++ b/docs/security/best-practices.mdx @@ -208,7 +208,9 @@ For sensitive workloads, use a reviewed host-side immutability workflow after in When `nemoclaw shields up` runs against a clean lock, it captures a SHA-256 seal of `openclaw.json` and any other locked files into the host-side shields state file. On sealed sandboxes, every `shields status` call recomputes the hash inside the sandbox and surfaces drift on any mismatch, so a host-root tamper that flips perms back to `444 root:root` after rewriting the file is still flagged. Sandboxes locked before this seal landed have no recorded hash; perm-only verification cannot prove their bytes match the image-original, so the seal is **not** a retroactive proof of integrity for legacy state. - By default, `shields up` refuses to seal such a baseline and asks you to rebuild the sandbox first for a known-good baseline. + The same refusal applies to partial seals where the locked file set grew after the existing seal was captured (some entries sealed, some missing). + By default, `shields up` refuses to seal in either case and asks you to rebuild the sandbox first for a known-good baseline. + `shields status` on a legacy lockdown surfaces `UP (UNSEALED — content integrity unknown for legacy lockdown)` and exits with status 2 so scripts treat it as a failure until the operator seals an explicit baseline. If you explicitly trust the current bytes, opt in via `NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1`, which captures a seal over the current files and is acknowledged in the log line. Once a sandbox is sealed, `shields up` refuses to re-seal a tampered baseline; restore the original file or rebuild the sandbox before re-running. - **Gateway token environment.** The gateway exports `OPENCLAW_GATEWAY_TOKEN` and writes it to `/tmp/nemoclaw-proxy-env.sh` for interactive sandbox sessions. Keep this in mind when deciding whether a workload should run with mutable config or an immutable config posture. diff --git a/src/lib/shields/index.test.ts b/src/lib/shields/index.test.ts index 767fe68bf3f..3bed4a786ce 100644 --- a/src/lib/shields/index.test.ts +++ b/src/lib/shields/index.test.ts @@ -706,7 +706,10 @@ describe("shields — unit logic", () => { return path.join(tmpDir, ".nemoclaw", "state"); } - function writeLockedState(sandboxName: string): void { + function writeLockedState( + sandboxName: string, + extra: Record = {}, + ): void { fs.mkdirSync(stateDir(), { recursive: true }); fs.writeFileSync( path.join(stateDir(), `shields-${sandboxName}.json`), @@ -714,6 +717,7 @@ describe("shields — unit logic", () => { { shieldsDown: false, updatedAt: new Date().toISOString(), + ...extra, }, null, 2, @@ -722,6 +726,16 @@ describe("shields — unit logic", () => { ); } + const SEAL_HASH = + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + + function writeSealedLockedState(sandboxName: string): void { + writeLockedState(sandboxName, { + chattrApplied: true, + fileHashes: { "/sandbox/.openclaw/openclaw.json": SEAL_HASH }, + }); + } + it("prints DRIFTED with the issue list and exits 2 when the verifier reports drift", async () => { const sandboxName = "openclaw"; writeLockedState(sandboxName); @@ -765,7 +779,7 @@ describe("shields — unit logic", () => { it("prints a clean locked status when the verifier reports no drift", async () => { const sandboxName = "openclaw"; - writeLockedState(sandboxName); + writeSealedLockedState(sandboxName); const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); @@ -836,28 +850,36 @@ describe("shields — unit logic", () => { expect(errorSpy).not.toHaveBeenCalled(); }); - it("logs a legacy-state notice when locked but no fileHashes seal is recorded", async () => { + it("exits 2 with an UNSEALED line when locked but no fileHashes seal is recorded", async () => { const sandboxName = "openclaw"; writeLockedState(sandboxName); - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const exitSpy = vi + .spyOn(process, "exit") + .mockImplementation((code?: string | number | null) => { + throw new Error(`exit ${String(code)}`); + }); const { shieldsStatus } = await loadShieldsModule(); - shieldsStatus(sandboxName, true, { - verifyLockState: () => ({ ok: true, issues: [] }), - resolveConfig: () => ({ - agentName: "openclaw", - configPath: "/sandbox/.openclaw/openclaw.json", - configDir: "/sandbox/.openclaw", + expect(() => + shieldsStatus(sandboxName, true, { + verifyLockState: () => ({ ok: true, issues: [] }), + resolveConfig: () => ({ + agentName: "openclaw", + configPath: "/sandbox/.openclaw/openclaw.json", + configDir: "/sandbox/.openclaw", + }), }), - }); + ).toThrow("exit 2"); - const lines = logSpy.mock.calls.map((args) => args[0]).join("\n"); - expect(lines).toContain( - "Notice: no content seal recorded; rebuild the sandbox for a known-good baseline", + const errors = errorSpy.mock.calls.map((args) => args[0]).join("\n"); + expect(errors).toContain( + "Shields: UP (UNSEALED — content integrity unknown for legacy lockdown)", ); - expect(lines).toContain( + expect(errors).toContain( `or set NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1 and re-run \`nemoclaw ${sandboxName} shields up\` to seal the current bytes.`, ); + expect(exitSpy).toHaveBeenCalledWith(2); }); it("surfaces content-drift entries from the verifier without re-locking", async () => { diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index a02901ac033..86f195cf6d2 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -1480,23 +1480,32 @@ function shieldsStatus( } process.exit(2); } - console.log(` Shields: ${posture.statusText}`); - console.log(policyLine); - if (state.shieldsDownAt) { - console.log(` Last unlocked: ${state.shieldsDownAt}`); - } if (!state.fileHashes) { - // Legacy state file pre-dates the content seal — perm-only - // verification cannot catch a host-root chmod-write-chmod tamper - // cycle. `shields up` refuses to seal an unverified baseline by - // default, so point at the recovery paths instead of suggesting - // a bare re-up that will abort. - console.log( - " Notice: no content seal recorded; rebuild the sandbox for a known-good baseline", + // Legacy state file pre-dates the content seal. Perm-only + // verification cannot prove the locked bytes were not already + // tampered before the upgrade, so we cannot honestly call this + // a clean lockdown. Surface integrity-unknown and exit with + // status 2 (same code as drifted) so scripts treat it as a + // failure until the operator seals an explicit baseline. + console.error( + " Shields: UP (UNSEALED — content integrity unknown for legacy lockdown)", ); - console.log( + console.error(policyLine); + if (state.shieldsDownAt) { + console.error(` Last unlocked: ${state.shieldsDownAt}`); + } + console.error( + " Recovery: rebuild the sandbox for a known-good baseline,", + ); + console.error( ` or set NEMOCLAW_SHIELDS_ACCEPT_LEGACY_BASELINE=1 and re-run \`nemoclaw ${sandboxName} shields up\` to seal the current bytes.`, ); + process.exit(2); + } + console.log(` Shields: ${posture.statusText}`); + console.log(policyLine); + if (state.shieldsDownAt) { + console.log(` Last unlocked: ${state.shieldsDownAt}`); } return; } diff --git a/test/e2e/test-shields-config.sh b/test/e2e/test-shields-config.sh index e8d42643cf3..7ed586e7c9c 100755 --- a/test/e2e/test-shields-config.sh +++ b/test/e2e/test-shields-config.sh @@ -341,9 +341,30 @@ else elif [ ! -s "$ORIG_CONTENT_FILE" ]; then fail "Original ${CONFIG_PATH} read returned an empty file" else + # When shields-up applied `chattr +i`, `chmod 644` alone would EPERM + # and the tamper would no-op — masking the seal check. Drop the + # immutable bit best-effort before the tamper, then restore it after + # so the post-tamper file is indistinguishable from the locked + # baseline by `stat`/`lsattr` alone. Track whether `+i` was applied + # via `lsattr -d` so we only re-apply when it was set before. + LSATTR_BEFORE=$(docker exec -u 0 "$CTR" lsattr -d "$CONFIG_PATH" 2>/dev/null | awk '{print $1}' || true) + HAD_IMMUTABLE_BIT=false + if echo "$LSATTR_BEFORE" | grep -q "i"; then + HAD_IMMUTABLE_BIT=true + fi docker exec -u 0 "$CTR" sh -c \ - "chmod 644 ${CONFIG_PATH} && printf ' ' >> ${CONFIG_PATH} && chmod 444 ${CONFIG_PATH}" \ + "chattr -i ${CONFIG_PATH} 2>/dev/null || true; \ + chmod 644 ${CONFIG_PATH} && printf ' ' >> ${CONFIG_PATH} && chmod 444 ${CONFIG_PATH}" \ >/dev/null 2>&1 + TAMPER_EXIT=$? + if [ "$HAD_IMMUTABLE_BIT" = "true" ]; then + docker exec -u 0 "$CTR" chattr +i "$CONFIG_PATH" >/dev/null 2>&1 || true + fi + if [ "$TAMPER_EXIT" = "0" ]; then + pass "Tamper command executed (chmod-write-chmod) without error" + else + fail "Tamper command failed (exit ${TAMPER_EXIT}); cannot validate drift detection" + fi PERMS_AFTER_TAMPER=$(docker exec "$CTR" stat -c '%a %U:%G' "$CONFIG_PATH" 2>/dev/null || true) info "Config perms after chmod-write-chmod tamper: ${PERMS_AFTER_TAMPER}" if [ "$PERMS_AFTER_TAMPER" = "444 root:root" ]; then @@ -391,12 +412,18 @@ else fi # Restore the original content as host root so the rest of the suite - # can continue against a clean lock. `docker exec -i` keeps stdin + # can continue against a clean lock. Drop the immutable bit (if any) + # before the write and re-apply it after so the file ends in the + # same chattr posture it started in. `docker exec -i` keeps stdin # open and we stream the backup file straight in — no command # substitution that would strip trailing newlines. docker exec -i -u 0 "$CTR" sh -c \ - "chmod 644 ${CONFIG_PATH} && cat > ${CONFIG_PATH} && chmod 444 ${CONFIG_PATH}" \ + "chattr -i ${CONFIG_PATH} 2>/dev/null || true; \ + chmod 644 ${CONFIG_PATH} && cat > ${CONFIG_PATH} && chmod 444 ${CONFIG_PATH}" \ <"$ORIG_CONTENT_FILE" >/dev/null 2>&1 + if [ "$HAD_IMMUTABLE_BIT" = "true" ]; then + docker exec -u 0 "$CTR" chattr +i "$CONFIG_PATH" >/dev/null 2>&1 || true + fi POST_RESTORE_OUTPUT=$(nemoclaw "${SANDBOX_NAME}" shields status 2>&1 || true) if echo "$POST_RESTORE_OUTPUT" | grep -q "Shields: UP (lockdown active)"; then pass "shields status clean after content restore"