diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 614f37f56bf..9196a487472 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1202,6 +1202,12 @@ $$nemoclaw my-assistant shields down --timeout 5m --reason "maintenance" | `shields up` | Lock the sandbox config and restore the saved restrictive policy | | `shields down` | Temporarily unlock the sandbox config. Supports `--timeout`, `--reason`, and `--policy` | +If OpenShell rejects the permissive policy before it is applied, `shields down` returns an error and keeps the sandbox in the Shields up state. +The command clears the provisional Shields down record and timer, and `shields status` remains `UP`. +If that record cannot be cleared and NemoClaw writes the rejection marker, `shields status` derives `UP` from that marker. +The auto-restore timer and transition remain the recovery authority. +If the rejection marker also cannot be written, `shields status` reports the incomplete transition as an error. + If `shields up` reports that the config remains unlocked or drifted, confirm that the sandbox is running and ready, then retry `$$nemoclaw shields up`. If the retry still fails, rebuild a known-good baseline with `$$nemoclaw rebuild --yes`. diff --git a/src/lib/shields/index.ts b/src/lib/shields/index.ts index 5d7fc5999d0..e65175ce17d 100644 --- a/src/lib/shields/index.ts +++ b/src/lib/shields/index.ts @@ -135,7 +135,7 @@ const HERMES_CONFIG_GUARD_TIMEOUT_MS = 11 * 60 * 1000; type ShieldsDownTransition = { version: 1; - phase: "preparing" | "active"; + phase: "preparing" | "active" | "policy_rejected"; ownerPid: number; ownerStartIdentity: string; processToken: string; @@ -182,7 +182,9 @@ function isShieldsDownTransition(value: unknown): value is ShieldsDownTransition if (!isObjectRecord(value)) return false; return ( value.version === 1 && - (value.phase === "preparing" || value.phase === "active") && + (value.phase === "preparing" || + value.phase === "active" || + value.phase === "policy_rejected") && typeof value.ownerPid === "number" && Number.isInteger(value.ownerPid) && value.ownerPid > 0 && @@ -219,6 +221,13 @@ function readShieldsDownTransition( } } +function readTimerBoundShieldsDownTransition(sandboxName: string): ShieldsDownTransition | null { + const marker = readTimerMarker(sandboxName); + if (!marker?.processToken || !/^[0-9a-f]{32}$/.test(marker.processToken)) return null; + const transition = readShieldsDownTransition(sandboxName, marker.processToken); + return transition?.snapshotPath === marker.snapshotPath ? transition : null; +} + function writeShieldsDownTransition( transition: ShieldsDownTransition, expectedPhase: ShieldsDownTransition["phase"] | null, @@ -930,8 +939,27 @@ function getShieldsPostureWithoutHostLock( allowInlineRecovery = false, ): ShieldsPosture { const state = recoverExpiredAutoRestoreGate(sandboxName, allowInlineRecovery); - const mode = state._isCorrupt ? "error" : deriveShieldsMode(state, state._hasStateFile); - return { ...describeShieldsMode(mode), state }; + const timerBoundTransition = + !state._isCorrupt && state.shieldsDown === true + ? readTimerBoundShieldsDownTransition(sandboxName) + : null; + const transitionDeniesMutability = + timerBoundTransition?.phase === "policy_rejected" || + timerBoundTransition?.phase === "preparing"; + const effectiveState: LoadedShieldsState = transitionDeniesMutability + ? { + ...state, + shieldsDown: false, + shieldsDownAt: null, + shieldsDownTimeout: null, + shieldsDownReason: null, + shieldsDownPolicy: null, + } + : state; + const mode = effectiveState._isCorrupt + ? "error" + : deriveShieldsMode(effectiveState, effectiveState._hasStateFile); + return { ...describeShieldsMode(mode), state: effectiveState }; } type ExpiredAutoRestoreTakeover = { @@ -2962,6 +2990,8 @@ interface ShieldsPolicySnapshotRestoreOptions { transitionProcessToken?: string; deadlineAuthoritative?: boolean; expiredTimerRecovery?: boolean; + buildPolicySet?: typeof buildPolicySetCommand; + runPolicySet?: typeof run; } type ShieldsPolicySnapshotRestoreResult = ReturnType & { @@ -2973,6 +3003,8 @@ function applyShieldsPolicySnapshot( snapshotPath: string, options: ShieldsPolicySnapshotRestoreOptions = {}, ): ShieldsPolicySnapshotRestoreResult { + const buildPolicySet = options.buildPolicySet ?? buildPolicySetCommand; + const runPolicySet = options.runPolicySet ?? run; const state = loadShieldsState(sandboxName); let transition: ShieldsDownTransition | null = null; if (options.transitionProcessToken !== undefined) { @@ -3061,7 +3093,7 @@ function applyShieldsPolicySnapshot( fs.readFileSync(snapshotPath, "utf-8"), hasManagedMcpPolicyClaims(sandboxName), ); - return run(buildPolicySetCommand(snapshotPath, sandboxName), { + return runPolicySet(buildPolicySet(snapshotPath, sandboxName), { ignoreError: true, }); } @@ -3087,7 +3119,7 @@ function applyShieldsPolicySnapshot( } const runtimePolicyIsTemp = runtimePolicyPath !== snapshotPath; try { - const result = run(buildPolicySetCommand(runtimePolicyPath, sandboxName), { + const result = runPolicySet(buildPolicySet(runtimePolicyPath, sandboxName), { ignoreError: true, }); return managedMcpOmissions.length > 0 ? { ...result, managedMcpOmissions } : result; @@ -3658,11 +3690,66 @@ function shieldsDownWithoutHostLock(sandboxName: string, opts: ShieldsDownOpts = } console.log(` Applying ${policyName} policy...`); + let policySetResult: ReturnType; try { - run(buildPolicySetCommand(policyFile, sandboxName)); + policySetResult = run(buildPolicySetCommand(policyFile, sandboxName), { + ignoreError: true, + }); } finally { cleanupRuntimePolicyFile(); } + if (policySetResult.status !== 0) { + // The permissive policy was rejected before it applied — for example, + // OpenShell refuses a live Landlock change on a sandbox whose policy is + // sealed at startup (Deep Agents). Nothing was weakened: configuration is + // still locked and the restrictive policy is unchanged. The provisional + // Shields down record written above therefore conflicts with the actual + // posture. Clear it, cancel the now-pointless timer and transition, and + // fail closed. Otherwise `shields status` would report `DOWN`/permissive + // for an unlock that never happened. + // See #8198. + try { + saveShieldsState(sandboxName, { + shieldsDown: false, + shieldsDownAt: null, + shieldsDownTimeout: null, + shieldsDownReason: null, + shieldsDownPolicy: null, + shieldsPolicySnapshotPath: null, + }); + } catch (stateErr) { + // Clearing the provisional Shields down record failed, so on disk the + // record still says `DOWN`. Mark the retained transition as rejected so + // status derives the restrictive posture instead of treating the + // provisional record as a completed unlock. The timer and transition + // remain the recovery authority and reclaim the restrictive snapshot. + if (transition) { + try { + transition = { ...transition, phase: "policy_rejected" }; + writeShieldsDownTransition(transition, "preparing"); + } catch (transitionErr) { + const transitionMessage = + transitionErr instanceof Error ? transitionErr.message : String(transitionErr); + console.error( + ` The rejected Shields down transition could not be recorded: ${transitionMessage}`, + ); + } + } + const stateMessage = stateErr instanceof Error ? stateErr.message : String(stateErr); + console.error( + ` ERROR: Could not apply the ${policyName} policy, and clearing the provisional Shields down record failed: ${stateMessage}`, + ); + console.error(" The scheduled auto-restore remains authoritative."); + return failShieldsCommand(`Could not apply ${policyName} policy`, opts.throwOnError); + } + if (transition) clearShieldsDownTransition(sandboxName, transition.processToken); + killTimer(sandboxName); + console.error( + ` ERROR: Could not apply the ${policyName} policy; the sandbox remains in the Shields up state.`, + ); + console.error(" Shields down did not take effect. `shields status` continues to report `UP`."); + return failShieldsCommand(`Could not apply ${policyName} policy`, opts.throwOnError); + } // 2b. Return config to default mutable state. // OpenClaw uses sandbox:sandbox 0660/2770 here so the gateway UID, which @@ -4122,6 +4209,13 @@ function shieldsStatusWithoutHostLock( throw new DeferredShieldsExit("Shields state is corrupt", 1); } + const transition = readTimerBoundShieldsDownTransition(sandboxName); + if (transition?.phase === "preparing") { + console.error(" Shields: ERROR (Shields down transition incomplete)"); + console.error(" The scheduled auto-restore remains authoritative."); + throw new DeferredShieldsExit("Shields down transition is incomplete", 1); + } + switch (posture.mode) { case "mutable_default": // NC-2227-02: Fresh sandbox with no shields history — do NOT claim locked @@ -4260,12 +4354,8 @@ function shieldsStatus( * "not configured" instead of "down". */ function isShieldsDown(sandboxName: string, allowInlineRecovery = false): boolean { - const state = allowInlineRecovery - ? getShieldsPosture(sandboxName, true).state - : recoverExpiredAutoRestoreGate(sandboxName, false); - if (state._isCorrupt) return false; - const mode = deriveShieldsMode(state, state._hasStateFile); - return mode !== "locked"; + const posture = getShieldsPosture(sandboxName, allowInlineRecovery); + return posture.mode !== "error" && posture.mode !== "locked"; } /** diff --git a/src/lib/shields/policy-transition.test.ts b/src/lib/shields/policy-transition.test.ts index 81c95c845f8..a0f6eadbbb6 100644 --- a/src/lib/shields/policy-transition.test.ts +++ b/src/lib/shields/policy-transition.test.ts @@ -101,6 +101,184 @@ describe("shields policy transition", () => { }); }); +describe("shields down policy rejection", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "shields-policy-rejection-")); + vi.stubEnv("HOME", tmpDir); + }); + + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); + + function createRejectedPolicyHarness() { + return createShieldsFlowHarness(requireSource, tmpDir, { + run: (cmd) => ({ + status: Array.isArray(cmd) && cmd.includes("policy") && cmd.includes("set") ? 1 : 0, + }), + }); + } + + it("keeps `shields status` at `UP` when OpenShell rejects the permissive policy (#8198)", () => { + const harness = createRejectedPolicyHarness(); + + expect(() => + harness.shieldsDown("openclaw", { + reason: "verify", + skipTimer: true, + throwOnError: true, + }), + ).toThrow(/Could not apply/); + expect(harness.isShieldsDown("openclaw")).toBe(false); + + const state = JSON.parse( + fs.readFileSync(path.join(tmpDir, ".nemoclaw/state/shields-openclaw.json"), "utf-8"), + ); + expect(state).toMatchObject({ shieldsDown: false, shieldsDownAt: null }); + }); + + it("retains auto-restore authority when rejected policy state cleanup fails (#8198)", () => { + const stateDir = path.join(tmpDir, ".nemoclaw", "state"); + const statePath = path.join(stateDir, "shields-openclaw.json"); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + statePath, + JSON.stringify({ + shieldsDown: false, + fileHashes: { "/sandbox/.openclaw/openclaw.json": "a".repeat(64) }, + updatedAt: "2026-08-05T00:00:00.000Z", + }), + ); + const timerKill = vi.fn(() => true); + const harness = createShieldsFlowHarness(requireSource, tmpDir, { + failPolicyRejectionStateClear: true, + initialOpenClawPosture: "locked", + processStartIdentity: "test-process-start-identity", + fork: () => ({ + pid: 4242, + disconnect: vi.fn(), + unref: vi.fn(), + send: vi.fn(() => true), + kill: timerKill, + }), + run: (cmd) => ({ + status: Array.isArray(cmd) && cmd.includes("policy") && cmd.includes("set") ? 1 : 0, + }), + }); + expect(() => + harness.shieldsDown("openclaw", { + reason: "verify recovery authority", + throwOnError: true, + }), + ).toThrow(/Could not apply/); + + expect(JSON.parse(fs.readFileSync(statePath, "utf-8"))).toMatchObject({ + shieldsDown: true, + shieldsDownReason: "verify recovery authority", + }); + expect(fs.existsSync(path.join(stateDir, "shields-timer-openclaw.json"))).toBe(true); + expect( + fs.readdirSync(stateDir).some((name) => name.startsWith("shields-transition-openclaw-")), + ).toBe(true); + const transitionName = fs + .readdirSync(stateDir) + .find((name) => name.startsWith("shields-transition-openclaw-")); + expect( + JSON.parse(fs.readFileSync(path.join(stateDir, transitionName!), "utf-8")), + ).toMatchObject({ phase: "policy_rejected" }); + expect(timerKill).not.toHaveBeenCalled(); + expect(harness.errorSpy.mock.calls.flat().join("\n")).toContain( + "The scheduled auto-restore remains authoritative.", + ); + + harness.logSpy.mockClear(); + harness.errorSpy.mockClear(); + harness.shieldsStatus("openclaw", true, { + verifyLockState: () => ({ ok: true, issues: [] }), + resolveConfig: () => ({ + agentName: "openclaw", + configPath: "/sandbox/.openclaw/openclaw.json", + configDir: "/sandbox/.openclaw", + }), + }); + + expect(harness.isShieldsDown("openclaw")).toBe(false); + expect(harness.logSpy).toHaveBeenCalledWith(" Shields: UP (lockdown active)"); + expect(harness.logSpy.mock.calls.flat().join("\n")).not.toMatch(/DOWN|permissive|unlocked/); + }); + + it("denies mutations when rejected policy state and transition updates both fail (#8198)", () => { + const stateDir = path.join(tmpDir, ".nemoclaw", "state"); + const statePath = path.join(stateDir, "shields-openclaw.json"); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync( + statePath, + JSON.stringify({ + shieldsDown: false, + fileHashes: { "/sandbox/.openclaw/openclaw.json": "a".repeat(64) }, + updatedAt: "2026-08-05T00:00:00.000Z", + }), + ); + const harness = createShieldsFlowHarness(requireSource, tmpDir, { + failPolicyRejectionStateClear: true, + failPolicyRejectionTransitionWrite: true, + initialOpenClawPosture: "locked", + processStartIdentity: "test-process-start-identity", + fork: () => ({ + pid: 4242, + disconnect: vi.fn(), + unref: vi.fn(), + send: vi.fn(() => true), + kill: vi.fn(() => true), + }), + run: (cmd) => ({ + status: Array.isArray(cmd) && cmd.includes("policy") && cmd.includes("set") ? 1 : 0, + }), + }); + + expect(() => + harness.shieldsDown("openclaw", { + reason: "verify incomplete rejection", + throwOnError: true, + }), + ).toThrow(/Could not apply/); + + const transitionName = fs + .readdirSync(stateDir) + .find((name) => name.startsWith("shields-transition-openclaw-")); + expect( + JSON.parse(fs.readFileSync(path.join(stateDir, transitionName!), "utf-8")), + ).toMatchObject({ phase: "preparing" }); + expect(harness.getShieldsPosture("openclaw", false)).toMatchObject({ + locked: true, + mutable: false, + }); + expect(harness.isShieldsDown("openclaw")).toBe(false); + + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((code?: number) => { + throw new Error(`process exit ${String(code)}`); + }) as typeof process.exit); + expect(() => + harness.shieldsStatus("openclaw", true, { + verifyLockState: () => ({ ok: true, issues: [] }), + resolveConfig: () => ({ + agentName: "openclaw", + configPath: "/sandbox/.openclaw/openclaw.json", + configDir: "/sandbox/.openclaw", + }), + }), + ).toThrow("process exit 1"); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(harness.errorSpy.mock.calls.flat().join("\n")).toContain( + "Shields: ERROR (Shields down transition incomplete)", + ); + }); +}); + describe("shields config lock without a shipped config hash", () => { const CONFIG_DIR = "/sandbox/.deepagents"; const CONFIG_PATH = `${CONFIG_DIR}/config.toml`; diff --git a/test/helpers/shields-flow-harness.ts b/test/helpers/shields-flow-harness.ts index cf4f83792ff..9721ca324ff 100644 --- a/test/helpers/shields-flow-harness.ts +++ b/test/helpers/shields-flow-harness.ts @@ -16,6 +16,7 @@ export type ShieldsFlowHarness = { auditSpy: MockInstance; cleanupTempDirSpy: MockInstance; errorSpy: MockInstance; + getShieldsPosture: typeof import("../../src/lib/shields/index.js").getShieldsPosture; getOpenClawPosture: () => "locked" | "mutable"; logSpy: MockInstance; policySetBodies: string[]; @@ -34,6 +35,8 @@ export type ShieldsFlowHarnessOptions = { directSandboxUnavailable?: boolean; dockerExecFileSync?: (argv: unknown) => string; failOpenClawGuardActions?: Array<"lock" | "unlock">; + failPolicyRejectionStateClear?: boolean; + failPolicyRejectionTransitionWrite?: boolean; failStateSave?: boolean; initialOpenClawPosture?: "locked" | "mutable"; invokedAs?: "nemoclaw" | "nemohermes"; @@ -47,6 +50,7 @@ export type ShieldsFlowHarnessOptions = { path: string; detail: string; }>; + processStartIdentity?: string; fork?: (...args: unknown[]) => { pid: number; disconnect: () => void; @@ -124,6 +128,14 @@ export function createShieldsFlowHarness( delete require.cache[requireDist.resolve("../actions/sandbox/mcp-bridge-policy.js")]; delete require.cache[requireDist.resolve("../sandbox/privileged-exec.js")]; delete require.cache[requireDist.resolve("../cli/branding.js")]; + const timerControl = requireDist( + "./timer-control.js", + ) as typeof import("../../src/lib/shields/timer-control.js"); + if (options.processStartIdentity !== undefined) { + vi.spyOn(timerControl, "readProcessStartIdentity").mockReturnValue( + options.processStartIdentity, + ); + } const lifecycleLock = requireDist( "../state/mcp-lifecycle-lock.js", ) as typeof import("../../src/lib/state/mcp-lifecycle-lock.js"); @@ -143,7 +155,6 @@ export function createShieldsFlowHarness( const privilegedExec = requireDist("../sandbox/privileged-exec.js"); const dockerExec = requireDist("../adapters/docker/exec.js"); const audit = requireDist("./audit.js"); - const timerControl = requireDist("./timer-control.js"); const tempFiles = requireDist("../onboard/temp-files.js"); const childProcess = requireDist("node:child_process"); const policySetBodies: string[] = []; @@ -311,6 +322,35 @@ export function createShieldsFlowHarness( }, ); + if (options.failPolicyRejectionStateClear) { + const statePath = path.join(tmpDir, ".nemoclaw", "state", "shields-openclaw.json"); + const originalWriteFileSync = fs.writeFileSync.bind(fs); + let stateWrites = 0; + vi.spyOn(fs, "writeFileSync").mockImplementation(((file, data, writeOptions) => { + if (String(file) === statePath && ++stateWrites === 2) { + throw new Error("state cleanup denied"); + } + return originalWriteFileSync(file, data, writeOptions); + }) as typeof fs.writeFileSync); + } + + if (options.failPolicyRejectionTransitionWrite) { + const transitionPrefix = path.join( + tmpDir, + ".nemoclaw", + "state", + "shields-transition-openclaw-", + ); + const originalRenameSync = fs.renameSync.bind(fs); + let transitionWrites = 0; + vi.spyOn(fs, "renameSync").mockImplementation((oldPath, newPath) => { + if (String(newPath).startsWith(transitionPrefix) && ++transitionWrites === 2) { + throw new Error("transition update denied"); + } + return originalRenameSync(oldPath, newPath); + }); + } + const shields = requireDist(shieldsModulePath); logSpy.mockClear(); errorSpy.mockClear(); @@ -321,6 +361,7 @@ export function createShieldsFlowHarness( auditSpy, cleanupTempDirSpy, errorSpy, + getShieldsPosture: shields.getShieldsPosture, getOpenClawPosture: () => openClawPosture, logSpy, policySetBodies,