From 6cbf1402b6c6e7a45ae397044b6bc9692b9a58fa Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 30 Jul 2026 07:10:42 +0000 Subject: [PATCH 1/8] fix(sandbox): continue destroy when pre-delete shields hardening fails (#7727) When `.config-hash` is removed from a locked OpenClaw sandbox, the config guard fails closed by design: `shields down` cannot unlock, its rollback cannot re-lock, and `shields up` refuses the tampered baseline. The pre-delete `shieldsUp` call in `wipeAndHardenLiveSandbox` ran with `throwOnError` and no failure handling, so that same failure escaped `executeSandboxDestroy` and `destroy --yes` exited 1. The sandbox stayed registered with no supported recovery path short of manually restoring trusted config state. Catch only that pre-delete hardening failure, warn with the guard's own detail, and continue with deletion. Tamper detection is unchanged: shields still fails closed, and `hardenedForDelete: false` keeps the delete-abort path from opening a bounded rollback window it never closed. A failed re-lock leaves the auto-restore timer as the only authority that can lock the config again, so `--force` no longer takes the local-cleanup shortcut when the gateway is unreachable after a failed re-lock. Discarding the record there would revoke that authority for a sandbox whose deletion the gateway never confirmed. Signed-off-by: Yimo Jiang Co-Authored-By: Claude Opus 5 (1M context) --- docs/reference/commands.mdx | 4 +- src/lib/actions/sandbox/destroy-execution.ts | 58 +++++++++++++++++--- src/lib/actions/sandbox/destroy-flow.test.ts | 43 +++++++++++++-- src/lib/actions/sandbox/destroy.ts | 9 ++- test/helpers/destroy-flow-test-assertions.ts | 46 ++++++++++++++-- test/helpers/destroy-flow-test-harness.ts | 4 +- 6 files changed, 144 insertions(+), 20 deletions(-) diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index f4d83aee124..3609f5f1637 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1745,7 +1745,8 @@ If sandbox deletion is refused after destroy has restored lockdown, NemoClaw ope If a shields auto-restore timer is active, `destroy` holds the same per-sandbox transition through state wipe and deletion. It restores and verifies lockdown and revokes the active timer before deletion. It clears the remaining local shields state only after deletion succeeds. -If hardening fails, the command refuses deletion and leaves the timer authority available to retry lockdown. +If hardening fails, for example because the tamper-evidence check rejects a missing `.config-hash`, the command warns and attempts deletion instead of refusing it. +If that deletion succeeds, the sandbox is removed with its unguarded config; if it fails, the config stays unlocked until you delete or rebuild the sandbox. If deletion fails after hardening, the command keeps the surviving sandbox's locked shields state instead of cleaning it up as though deletion succeeded. By default, unattended final-sandbox destroys (`--yes`, `--force`, or `NEMOCLAW_NON_INTERACTIVE=1`) remove the shared NemoClaw gateway on macOS so the host listener is released, while Linux preserves it for reuse. Pass `--cleanup-gateway` to force removal, or `--no-cleanup-gateway` to force preservation. @@ -1759,6 +1760,7 @@ If the OpenShell gateway is unreachable and the sandbox has no managed MCP owner Gateway-side deletion remains unconfirmed, shared host-service and gateway teardown are skipped, and the sandbox and retained volume may still exist if the gateway returns. Start the gateway with `$$nemoclaw status` and retry destroy when you need a confirmed deletion. Managed MCP ownership disables the local-only fallback because exact provider cleanup requires the retained ownership state, and other delete failures remain fatal. +A failed pre-delete lock also disables the local-only fallback, because the auto-restore timer is then the only authority that can lock the config again after the gateway returns. ```bash $$nemoclaw my-assistant destroy [--yes|-y|--force] [--cleanup-gateway|--no-cleanup-gateway] diff --git a/src/lib/actions/sandbox/destroy-execution.ts b/src/lib/actions/sandbox/destroy-execution.ts index 5bfb0d7cc96..40d2029d01d 100644 --- a/src/lib/actions/sandbox/destroy-execution.ts +++ b/src/lib/actions/sandbox/destroy-execution.ts @@ -46,10 +46,12 @@ export type SandboxDestroyExecutionResult = gatewayUnreachable: boolean; mcpOwnershipRequiresGateway: boolean; mcpRecoveryFailure?: string; + shieldsRelockRequiresGateway: boolean; }; type HardenedDeleteState = { hardenedForDelete: boolean; + hardeningFailed: boolean; timerProcessToken?: string; }; @@ -87,23 +89,52 @@ function wipeAndHardenLiveSandbox( sandboxName: string, sandboxConfirmedAbsent: boolean, ): HardenedDeleteState { - if (sandboxConfirmedAbsent) return { hardenedForDelete: false }; + if (sandboxConfirmedAbsent) return { hardenedForDelete: false, hardeningFailed: false }; // Wipe before delete while the retained volume is still mounted. The caller // holds the timer-bound lock across this phase and all following teardown. wipeSandboxState(sandboxName); const timerMarker = readTimerMarker(sandboxName); - if (!timerMarker) return { hardenedForDelete: false }; + if (!timerMarker) return { hardenedForDelete: false, hardeningFailed: false }; const timerProcessToken = /^[0-9a-f]{32}$/.test(timerMarker.processToken ?? "") ? timerMarker.processToken : undefined; const { shieldsUp } = require("../../shields") as typeof import("../../shields"); - shieldsUp(sandboxName, { - throwOnError: true, - allowLegacyHermesProtocol: true, - }); - return { hardenedForDelete: true, timerProcessToken }; + try { + shieldsUp(sandboxName, { + throwOnError: true, + allowLegacyHermesProtocol: true, + }); + } catch (error) { + // #7727: hardening before delete is a best-effort narrowing of the open + // shields-down window, not a precondition for removal. When the lock + // cannot be re-established — a deleted `.config-hash` in the locked + // posture makes the guard fail closed, and neither `shields up` nor + // `shields down` can repair it by design — refusing to delete stranded + // the sandbox the user explicitly asked to remove, with no supported + // recovery path. Deleting removes the unguarded config along with the + // sandbox, so report the failure and continue. `hardenedForDelete: false` + // keeps the delete-abort path honest: it will not open a bounded + // shields-down rollback window it never closed. + // + // The auto-restore timer stays authoritative until deletion succeeds, for + // the same reason `shieldsUp` keeps it through its own commit: revoking it + // here would turn a delete that then fails into an unbounded mutable + // window. It cannot preempt this process — a live transition-lock owner is + // never reclaimed, and the timer only stops a shields-down owner that + // published a `preparing` transition record, which destroy never does. + const detail = redact(error instanceof Error ? error.message : String(error)); + console.warn( + ` ${YW}⚠${R} Could not re-lock shields for '${sandboxName}' before delete: ${detail}`, + ); + console.warn( + ` Continuing with delete — '${sandboxName}' and its unguarded config are removed together. ` + + "If the delete fails, the config stays unlocked until the sandbox is deleted or rebuilt.", + ); + return { hardenedForDelete: false, hardeningFailed: true, timerProcessToken }; + } + return { hardenedForDelete: true, hardeningFailed: false, timerProcessToken }; } async function restoreMcpAfterDeleteAbort( @@ -204,8 +235,18 @@ export async function executeSandboxDestroy({ alreadyGone, gatewayUnreachable, } = getSandboxDeleteOutcome(deleteResult); + // #7727: a failed pre-delete re-lock leaves the auto-restore timer as the + // only authority that can lock the config again. Discarding the local + // record here would revoke it for a sandbox the gateway never confirmed + // deleting, so --force must not take the local-cleanup shortcut until the + // gateway is back and deletion is confirmed. const forcedLocalCleanup = - deleteResult.status !== 0 && !alreadyGone && gatewayUnreachable && force && !hasMcpOwnership; + deleteResult.status !== 0 && + !alreadyGone && + gatewayUnreachable && + force && + !hasMcpOwnership && + !hardened.hardeningFailed; if (deleteResult.status !== 0 && !alreadyGone && !forcedLocalCleanup) { const mcpRecoveryFailure = sandboxConfirmedAbsent @@ -218,6 +259,7 @@ export async function executeSandboxDestroy({ gatewayUnreachable, mcpOwnershipRequiresGateway: gatewayUnreachable && hasMcpOwnership, mcpRecoveryFailure, + shieldsRelockRequiresGateway: gatewayUnreachable && hardened.hardeningFailed, }; } diff --git a/src/lib/actions/sandbox/destroy-flow.test.ts b/src/lib/actions/sandbox/destroy-flow.test.ts index 0f34afa94d5..6a1c099086e 100644 --- a/src/lib/actions/sandbox/destroy-flow.test.ts +++ b/src/lib/actions/sandbox/destroy-flow.test.ts @@ -7,7 +7,9 @@ import { expectAbsentSandboxMcpFinalize, expectActiveTimerDestroyOrder, expectFailedDeletePreservesHostState, - expectFailedHardeningStopsDelete, + expectFailedHardeningMcpRestore, + expectFailedHardeningRefusesForcedCleanup, + expectFailedHardeningStillDeletes, expectFailedMcpFinalizePreservesRegistry, expectFailedMcpRestorePreservesDestroyFailure, expectMcpFinalizeAfterDelete, @@ -199,17 +201,48 @@ describe("destroySandbox flow", () => { expectActiveTimerDestroyOrder(harness); }); - it("does not delete when active-window hardening fails after the wipe", async () => { + it("warns and still deletes when active-window hardening fails after the wipe (#7727)", async () => { const harness = createDestroyHarness({ activeTimer: true, shieldsUpError: new Error("injected hardening failure"), }); - await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow( - "injected hardening failure", + await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); + + expectFailedHardeningStillDeletes(harness); + expect(exitSpy).not.toHaveBeenCalled(); + }); + + it("keeps the timer and local record when --force cannot confirm deletion after failed hardening (#7727)", async () => { + const harness = createDestroyHarness({ + activeTimer: true, + deleteStatus: 1, + deleteOutput: "error trying to connect: connection refused", + registeredSandboxCount: 1, + shieldsUpError: new Error("injected hardening failure"), + }); + + await expect(harness.destroySandbox("alpha", { force: true })).rejects.toThrow( + "process.exit(1)", ); - expectFailedHardeningStopsDelete(harness); + expectFailedHardeningRefusesForcedCleanup(harness); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + it("restores MCP runtime state without a rollback window when delete fails after failed hardening (#7727)", async () => { + const harness = createDestroyHarness({ + activeTimer: true, + deleteStatus: 7, + deleteOutput: "delete failed", + mcpServers: ["github"], + shieldsUpError: new Error("injected hardening failure"), + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(7)"); + + expectFailedHardeningMcpRestore(harness); + expect(exitSpy).toHaveBeenCalledWith(7); }); it("detaches MCP providers before delete and finalizes them only after delete succeeds", async () => { diff --git a/src/lib/actions/sandbox/destroy.ts b/src/lib/actions/sandbox/destroy.ts index 78a9483f8cc..e32fb3d7e83 100644 --- a/src/lib/actions/sandbox/destroy.ts +++ b/src/lib/actions/sandbox/destroy.ts @@ -359,7 +359,14 @@ async function destroySandboxUnlocked( } console.error(` Failed to destroy sandbox '${sandboxName}'.`); if (destructiveResult.gatewayUnreachable) { - if (destructiveResult.mcpOwnershipRequiresGateway) { + if (destructiveResult.shieldsRelockRequiresGateway) { + console.error( + ` The OpenShell gateway is unreachable and shields could not be re-locked before delete. Local state was preserved so the auto-restore timer can still lock the config when the gateway returns.`, + ); + console.error( + ` Start the gateway (run '${CLI_NAME} ${sandboxName} status'), then retry destroy; --force cannot safely discard a record whose config lock is unconfirmed.`, + ); + } else if (destructiveResult.mcpOwnershipRequiresGateway) { console.error( ` The OpenShell gateway is unreachable. Local state was preserved because it contains MCP ownership required for exact provider cleanup.`, ); diff --git a/test/helpers/destroy-flow-test-assertions.ts b/test/helpers/destroy-flow-test-assertions.ts index ac9b1bb5508..66b12c86aa0 100644 --- a/test/helpers/destroy-flow-test-assertions.ts +++ b/test/helpers/destroy-flow-test-assertions.ts @@ -112,11 +112,49 @@ export function expectActiveTimerDestroyOrder(harness: DestroyHarness): void { expect(harness.events.indexOf("delete")).toBeLessThan(harness.events.indexOf("timer-cleanup")); } -export function expectFailedHardeningStopsDelete(harness: DestroyHarness): void { - expect(harness.events).toContain("wipe"); - expect(harness.events).toContain("harden"); - expect(harness.events).not.toContain("delete"); +export function expectFailedHardeningStillDeletes(harness: DestroyHarness): void { + expect(harness.events).toEqual( + expect.arrayContaining(["wipe", "harden", "delete", "timer-cleanup"]), + ); + expect(harness.events.indexOf("wipe")).toBeLessThan(harness.events.indexOf("harden")); + expect(harness.events.indexOf("harden")).toBeLessThan(harness.events.indexOf("delete")); + expect(harness.events.indexOf("delete")).toBeLessThan(harness.events.indexOf("timer-cleanup")); + expect(harness.removeSandboxSpy).toHaveBeenCalledWith("alpha"); + expect(harness.killTimerSpy).toHaveBeenCalledTimes(1); + const warnOutput = harness.warnSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(warnOutput).toContain("Could not re-lock shields for 'alpha' before delete"); + expect(warnOutput).toContain("injected hardening failure"); + expect(warnOutput).toContain("Continuing with delete"); +} + +export function expectFailedHardeningRefusesForcedCleanup(harness: DestroyHarness): void { + expect(harness.events).toEqual(expect.arrayContaining(["harden", "delete"])); + // The auto-restore timer is the only remaining authority that can lock the + // config again, so an unconfirmed delete must keep it and the local record. expect(harness.killTimerSpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.stopAllSpy).not.toHaveBeenCalled(); + expect(harness.cleanupGatewaySpy).not.toHaveBeenCalled(); + const errorOutput = harness.errorSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(errorOutput).toContain("shields could not be re-locked before delete"); + expect(errorOutput).toContain("--force cannot safely discard a record whose config lock"); + expect(errorOutput).not.toContain("re-run with --force to remove the local sandbox record"); +} + +export function expectFailedHardeningMcpRestore(harness: DestroyHarness): void { + expect(harness.events).toEqual(expect.arrayContaining(["harden", "delete", "mcp-restore"])); + expect(harness.events.indexOf("harden")).toBeLessThan(harness.events.indexOf("delete")); + expect(harness.events.indexOf("delete")).toBeLessThan(harness.events.indexOf("mcp-restore")); + // No lock was re-established, so destroy must not open a bounded + // shields-down rollback window it cannot close again. + expect(harness.events).not.toContain("unlock"); + expect(harness.shieldsDownSpy).not.toHaveBeenCalled(); + expect(harness.restoreMcpBridgesAfterDestroyAbortSpy).toHaveBeenCalledWith( + "alpha", + expect.objectContaining({ entries: [{ server: "github" }] }), + ); + expect(harness.finalizeMcpBridgesAfterSandboxDeleteSpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); } export function expectMcpFinalizeAfterDelete(harness: DestroyHarness): void { diff --git a/test/helpers/destroy-flow-test-harness.ts b/test/helpers/destroy-flow-test-harness.ts index 1d16cc11b02..09785b5aaa8 100644 --- a/test/helpers/destroy-flow-test-harness.ts +++ b/test/helpers/destroy-flow-test-harness.ts @@ -37,6 +37,7 @@ export type DestroyHarness = { stopAllSpy: MockInstance; stopNimByNameSpy: MockInstance; unloadOllamaModelsSpy: MockInstance; + warnSpy: MockInstance; }; type DestroyHarnessOptions = { @@ -106,7 +107,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr const logSpy = vi.spyOn(console, "log").mockImplementation(() => undefined); const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); - vi.spyOn(console, "warn").mockImplementation(() => undefined); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); const resolve = requireDist("../../adapters/openshell/resolve.js"); const runtime = requireDist("../../adapters/openshell/runtime.js"); @@ -334,5 +335,6 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr stopAllSpy, stopNimByNameSpy, unloadOllamaModelsSpy, + warnSpy, }; } From ba66d0407f8f2c7eb8451c686820aeb492870b57 Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 30 Jul 2026 07:32:44 +0000 Subject: [PATCH 2/8] docs(sandbox): describe auto-restore retry after a failed pre-delete lock (#7727) The destroy reference and the CLI warning said the config stays unlocked until the sandbox is deleted or rebuilt when the delete fails after a failed pre-delete lock. That omitted the auto-restore timer this path deliberately preserves, which keeps retrying the lock and can restore it once the sandbox is reachable again (PR Review Advisor PRA-1). Signed-off-by: Yimo Jiang Co-Authored-By: Claude Opus 5 (1M context) --- docs/reference/commands.mdx | 4 +++- src/lib/actions/sandbox/destroy-execution.ts | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 3609f5f1637..11d49093e41 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1746,7 +1746,9 @@ If a shields auto-restore timer is active, `destroy` holds the same per-sandbox It restores and verifies lockdown and revokes the active timer before deletion. It clears the remaining local shields state only after deletion succeeds. If hardening fails, for example because the tamper-evidence check rejects a missing `.config-hash`, the command warns and attempts deletion instead of refusing it. -If that deletion succeeds, the sandbox is removed with its unguarded config; if it fails, the config stays unlocked until you delete or rebuild the sandbox. +If that deletion succeeds, the sandbox is removed with its unguarded config. +If it fails, NemoClaw keeps the local shields state and the auto-restore timer, which keeps retrying the lock and can restore it after the sandbox is reachable again. +The config stays unlocked until one of those retries succeeds, or until you delete or rebuild the sandbox. If deletion fails after hardening, the command keeps the surviving sandbox's locked shields state instead of cleaning it up as though deletion succeeded. By default, unattended final-sandbox destroys (`--yes`, `--force`, or `NEMOCLAW_NON_INTERACTIVE=1`) remove the shared NemoClaw gateway on macOS so the host listener is released, while Linux preserves it for reuse. Pass `--cleanup-gateway` to force removal, or `--no-cleanup-gateway` to force preservation. diff --git a/src/lib/actions/sandbox/destroy-execution.ts b/src/lib/actions/sandbox/destroy-execution.ts index 40d2029d01d..cfc1e991e79 100644 --- a/src/lib/actions/sandbox/destroy-execution.ts +++ b/src/lib/actions/sandbox/destroy-execution.ts @@ -130,7 +130,8 @@ function wipeAndHardenLiveSandbox( ); console.warn( ` Continuing with delete — '${sandboxName}' and its unguarded config are removed together. ` + - "If the delete fails, the config stays unlocked until the sandbox is deleted or rebuilt.", + "If the delete fails, the auto-restore timer keeps retrying the lock until it succeeds " + + "or the sandbox is deleted or rebuilt.", ); return { hardenedForDelete: false, hardeningFailed: true, timerProcessToken }; } From 4f631adc19fa3be534c1b47f495ca9bcc523e2e4 Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 30 Jul 2026 13:50:06 +0000 Subject: [PATCH 3/8] test(sandbox): budget the cross-agent MCP status state suite (#7727) Each case in this suite spawns a Node child that loads the registry and MCP bridge module graph. `ci/cli-test-timing-hints.json` records the file at 5515ms, above the 5000ms default `testTimeout`, so the suite fails on timing alone whenever its shard is busy. Adding two destroy-flow cases to the same shard was enough to tip it, failing `cli-test-shards (4)` three times in a row while other pull requests passed. Declare the same 15s budget the sibling `mcp-bridge-status-removal` suite already uses for its child-process cases. Signed-off-by: Yimo Jiang Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/actions/sandbox/mcp-bridge-status-state.test.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/lib/actions/sandbox/mcp-bridge-status-state.test.ts b/src/lib/actions/sandbox/mcp-bridge-status-state.test.ts index 59d095da66a..139bd9bc5c0 100644 --- a/src/lib/actions/sandbox/mcp-bridge-status-state.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-status-state.test.ts @@ -8,6 +8,8 @@ import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; +import { testTimeoutOptions } from "../../../../test/helpers/timeouts"; + const sourceRequireHook = path.resolve("test/helpers/onboard-script-mocks.cjs"); const sourceNodeOptions = [process.env.NODE_OPTIONS, `--require=${sourceRequireHook}`] .filter(Boolean) @@ -25,7 +27,11 @@ afterEach(() => { tempHomes.clear(); }); -describe("cross-agent MCP status state", () => { +// Each case spawns a Node child that loads the registry and MCP bridge module +// graph, which the recorded timing hints put at ~5.5s — over the 5s default. +// Use the same budget as the sibling `mcp-bridge-status-removal` suite so a +// loaded shard cannot fail these on timing alone. +describe("cross-agent MCP status state", testTimeoutOptions(15_000), () => { it("rejects duplicate static credential keys across bridges in one sandbox", () => { const home = createTempHome("nemoclaw-mcp-env-key-"); const script = ` From ee1bba9803eb1f7b9d1fa3032a0a050c947f9cff Mon Sep 17 00:00:00 2001 From: Yimo Jiang Date: Thu, 30 Jul 2026 14:16:52 +0000 Subject: [PATCH 4/8] docs(sandbox): record the source boundary for the best-effort delete (#7727) The failed-hardening fallback described the invalid state and the immediate behavior but not why the missing integrity sidecar cannot be repaired at its source, or when the fallback can be removed (PR Review Advisor PRA-1). State the boundary in the repository's SOURCE_OF_TRUTH form: host root removes the sidecar out of band, the locked-posture refusal belongs to the config guard and is deliberate tamper evidence that #7727 keeps, and the fallback goes away once the guard gains a supported authenticated repair for a missing sidecar in the locked posture. Signed-off-by: Yimo Jiang Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/actions/sandbox/destroy-execution.ts | 40 ++++++++++++++------ 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/src/lib/actions/sandbox/destroy-execution.ts b/src/lib/actions/sandbox/destroy-execution.ts index cfc1e991e79..c0002eafea1 100644 --- a/src/lib/actions/sandbox/destroy-execution.ts +++ b/src/lib/actions/sandbox/destroy-execution.ts @@ -107,17 +107,35 @@ function wipeAndHardenLiveSandbox( allowLegacyHermesProtocol: true, }); } catch (error) { - // #7727: hardening before delete is a best-effort narrowing of the open - // shields-down window, not a precondition for removal. When the lock - // cannot be re-established — a deleted `.config-hash` in the locked - // posture makes the guard fail closed, and neither `shields up` nor - // `shields down` can repair it by design — refusing to delete stranded - // the sandbox the user explicitly asked to remove, with no supported - // recovery path. Deleting removes the unguarded config along with the - // sandbox, so report the failure and continue. `hardenedForDelete: false` - // keeps the delete-abort path honest: it will not open a bounded - // shields-down rollback window it never closed. - // + /** + * SOURCE_OF_TRUTH + * Invalid state: the locked OpenClaw config pair lost its `.config-hash` + * integrity sidecar, so the guard refuses every lock transition and the + * sandbox cannot be re-hardened (#7727). + * Source boundary: the sidecar is removed out of band by host root inside + * the container (the reporter used `docker exec --user root rm -f`). The + * refusal itself belongs to `scripts/openclaw-config-guard.py`, which + * repairs an absent hash only in lock-from-mutable mode and fail-stops in + * the locked posture on purpose. + * Source-fix constraint: NemoClaw cannot stop host root from deleting a + * file inside the sandbox, and regenerating the hash from the current + * bytes in the locked posture is exactly the tamper laundering the guard + * exists to prevent — #7727 explicitly keeps that fail-closed. So destroy + * cannot repair this state at its source; it can only stop treating a + * best-effort hardening step as a precondition for removal. + * Regression proof: destroy-flow.test.ts covers delete-proceeds-after- + * failed-hardening, the emitted warning, timer cleanup ordering, and MCP + * restore when the delete then fails. + * Removal condition: drop this fallback when the config guard gains a + * supported authenticated repair for a missing sidecar in the locked + * posture, so a pre-delete `shieldsUp` can be required again. + * + * Hardening before delete narrows the open shields-down window; it is not + * a precondition for removal. Deleting removes the unguarded config along + * with the sandbox, so report the failure and continue. + * `hardenedForDelete: false` keeps the delete-abort path honest: it will + * not open a bounded shields-down rollback window it never closed. + */ // The auto-restore timer stays authoritative until deletion succeeds, for // the same reason `shieldsUp` keeps it through its own commit: revoking it // here would turn a delete that then fails into an unbounded mutable From 06a36ec6ca857b08749ae5c3266d425fd6de4f5c Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Thu, 30 Jul 2026 14:22:22 -0700 Subject: [PATCH 5/8] test(shields): prove destroy timer preemption Signed-off-by: Apurv Kumaria (cherry picked from commit a4f2b3433f0647f0b69ca009f9722ac146fa1aec) --- src/lib/actions/sandbox/destroy-execution.ts | 7 +- src/lib/shields/flow.test.ts | 110 +++++++++++++++++++ 2 files changed, 114 insertions(+), 3 deletions(-) diff --git a/src/lib/actions/sandbox/destroy-execution.ts b/src/lib/actions/sandbox/destroy-execution.ts index c0002eafea1..a8c2cda8a21 100644 --- a/src/lib/actions/sandbox/destroy-execution.ts +++ b/src/lib/actions/sandbox/destroy-execution.ts @@ -139,9 +139,10 @@ function wipeAndHardenLiveSandbox( // The auto-restore timer stays authoritative until deletion succeeds, for // the same reason `shieldsUp` keeps it through its own commit: revoking it // here would turn a delete that then fails into an unbounded mutable - // window. It cannot preempt this process — a live transition-lock owner is - // never reclaimed, and the timer only stops a shields-down owner that - // published a `preparing` transition record, which destroy never does. + // window. At the absolute deadline it may stop this exact token-bound + // destroy owner and its subprocesses before reclaiming the transition + // lock, then restore lockdown. The token and process identity checks make + // a mismatched owner fail closed instead of signaling an unrelated process. const detail = redact(error instanceof Error ? error.message : String(error)); console.warn( ` ${YW}⚠${R} Could not re-lock shields for '${sandboxName}' before delete: ${detail}`, diff --git a/src/lib/shields/flow.test.ts b/src/lib/shields/flow.test.ts index 7c9be44f48a..2e6915fc20c 100644 --- a/src/lib/shields/flow.test.ts +++ b/src/lib/shields/flow.test.ts @@ -807,6 +807,116 @@ describe("shields command flow", () => { } }); + it("lets an expired timer preempt its token-bound destroy owner and restore lockdown", { + timeout: 20_000, + }, async () => { + const transitionLockPath = path.join(import.meta.dirname, "transition-lock.ts"); + const stateDir = path.join(tmpDir, ".nemoclaw", "state"); + const sandboxName = "destroy-deadline"; + const processToken = "e".repeat(32); + const snapshotPath = path.join(stateDir, "policy-snapshot-destroy.yaml"); + const readyPath = path.join(stateDir, "destroy-owner.ready"); + const lockPath = path.join(stateDir, `shields-transition-lock-${sandboxName}.json`); + const markerPath = path.join(stateDir, `shields-timer-${sandboxName}.json`); + const statePath = path.join(stateDir, `shields-${sandboxName}.json`); + fs.mkdirSync(stateDir, { recursive: true }); + fs.writeFileSync(snapshotPath, "version: 1\nnetwork_policies:\n test: {}\n"); + fs.writeFileSync( + statePath, + JSON.stringify({ + shieldsDown: true, + shieldsDownAt: new Date(Date.now() - 60_000).toISOString(), + shieldsDownTimeout: 60, + shieldsDownReason: "destroy takeover coverage", + shieldsDownPolicy: "permissive", + shieldsPolicySnapshotPath: snapshotPath, + updatedAt: new Date().toISOString(), + }), + { mode: 0o600 }, + ); + fs.writeFileSync( + markerPath, + JSON.stringify({ + pid: 999_999, + sandboxName, + snapshotPath, + restoreAt: new Date(Date.now() - 1_000).toISOString(), + processToken, + }), + { mode: 0o600 }, + ); + + const owner = spawn( + process.execPath, + [ + "--import", + "tsx", + "-e", + [ + `const {withShieldsTransitionLock}=require(${JSON.stringify(transitionLockPath)})`, + "const fs=require('fs')", + "const [name,token,ready]=process.argv.slice(1)", + "withShieldsTransitionLock(name,'destroy sandbox',()=>{fs.writeFileSync(ready,'ready');Atomics.wait(new Int32Array(new SharedArrayBuffer(4)),0,0,10000)},{takeoverToken:token})", + ].join(";"), + sandboxName, + processToken, + readyPath, + ], + { env: { ...process.env, HOME: tmpDir }, stdio: "ignore" }, + ); + + try { + await vi.waitFor( + () => { + expect(fs.existsSync(readyPath)).toBe(true); + expect(fs.existsSync(lockPath)).toBe(true); + }, + { timeout: 5_000, interval: 10 }, + ); + const timerControl = requireDist("./timer-control.js"); + const ownerStartIdentity = timerControl.readProcessStartIdentity(owner.pid); + expect(ownerStartIdentity).toBeTypeOf("string"); + const harness = createHarness({ + dockerExecFileSync: (argv: unknown) => { + const args = Array.isArray(argv) ? argv.map(String) : []; + switch (true) { + case args.includes("sha256sum"): + return `${"a".repeat(64)} ${String(args.at(-1))}\n`; + case args.includes("lsattr"): + return `----i---------e----- ${String(args.at(-1))}\n`; + case args.includes("stat"): + return args.at(-1) === "/sandbox" + ? "1775 root:sandbox\n" + : args.at(-1) === "/sandbox/.openclaw" + ? "755 root:root\n" + : "444 root:root\n"; + default: + return ""; + } + }, + }); + + harness.shieldsStatus(sandboxName); + + const ownerState = timerControl.readProcessState(owner.pid); + expect(ownerState === null || ownerState.startsWith("Z")).toBe(true); + expect(fs.existsSync(lockPath)).toBe(false); + expect(JSON.parse(fs.readFileSync(statePath, "utf-8"))).toMatchObject({ + shieldsDown: false, + shieldsDownAt: null, + }); + expect(fs.existsSync(markerPath)).toBe(false); + expect(harness.runSpy).toHaveBeenCalledWith( + ["openshell", "policy", "set"], + expect.objectContaining({ ignoreError: true }), + ); + expect(harness.logSpy).toHaveBeenCalledWith(" Shields: UP (lockdown active)"); + } finally { + owner.kill("SIGCONT"); + owner.kill("SIGKILL"); + } + }); + it("publishes preparing recovery ownership before weakening and active only after unlock", () => { const stateDir = path.join(tmpDir, ".nemoclaw", "state"); let observedPreparingDuringPolicy = false; From 4c6be577522238d4eb369d77b8c1c34fc5576194 Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Mon, 3 Aug 2026 13:15:52 -0700 Subject: [PATCH 6/8] fix(sandbox): complete destroy failure result Signed-off-by: Apurv Kumaria --- src/lib/actions/sandbox/destroy-execution.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/actions/sandbox/destroy-execution.ts b/src/lib/actions/sandbox/destroy-execution.ts index 485896c61af..7968980af91 100644 --- a/src/lib/actions/sandbox/destroy-execution.ts +++ b/src/lib/actions/sandbox/destroy-execution.ts @@ -265,6 +265,7 @@ export async function executeSandboxDestroy({ exitCode: 1, gatewayUnreachable: false, mcpOwnershipRequiresGateway: false, + shieldsRelockRequiresGateway: false, }; } } From 5e584ea4fa83da6bb7cb323562c40b616609ad3e Mon Sep 17 00:00:00 2001 From: Apurv Kumaria Date: Mon, 3 Aug 2026 13:49:42 -0700 Subject: [PATCH 7/8] fix(sandbox): keep destroy orchestration provider-neutral Signed-off-by: Apurv Kumaria --- src/lib/actions/sandbox/destroy-execution.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/actions/sandbox/destroy-execution.ts b/src/lib/actions/sandbox/destroy-execution.ts index 7968980af91..91bf4cd38a5 100644 --- a/src/lib/actions/sandbox/destroy-execution.ts +++ b/src/lib/actions/sandbox/destroy-execution.ts @@ -129,7 +129,7 @@ function wipeAndHardenLiveSandbox( * integrity sidecar, so the guard refuses every lock transition and the * sandbox cannot be re-hardened (#7727). * Source boundary: the sidecar is removed out of band by host root inside - * the container (the reporter used `docker exec --user root rm -f`). The + * the sandbox (the reporter used a privileged runtime command). The * refusal itself belongs to `scripts/openclaw-config-guard.py`, which * repairs an absent hash only in lock-from-mutable mode and fail-stops in * the locked posture on purpose. From 99315ca9427679cea844ec67ea3e70c291ab4dcc Mon Sep 17 00:00:00 2001 From: Senthil Ravichandran Date: Mon, 3 Aug 2026 23:23:01 -0700 Subject: [PATCH 8/8] docs(sandbox): clarify failed re-lock recovery Signed-off-by: Senthil Ravichandran --- docs/reference/commands.mdx | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index e80bd5fd400..0fdd6ca11ad 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1782,10 +1782,11 @@ If sandbox deletion is refused after destroy has restored lockdown, NemoClaw ope If a shields auto-restore timer is active, `destroy` holds the same per-sandbox transition through state wipe and deletion. It restores and verifies lockdown and revokes the active timer before deletion. It clears the remaining local shields state only after deletion succeeds. -If hardening fails, for example because the tamper-evidence check rejects a missing `.config-hash`, the command warns and attempts deletion instead of refusing it. -If that deletion succeeds, the sandbox is removed with its unguarded config. -If it fails, NemoClaw keeps the local shields state and the auto-restore timer, which keeps retrying the lock and can restore it after the sandbox is reachable again. -The config stays unlocked until one of those retries succeeds, or until you delete or rebuild the sandbox. +If the pre-delete re-lock fails, the command warns and attempts to destroy the sandbox. +If the destroy operation succeeds, it destroys the sandbox and deletes its unguarded configuration. +If the destroy operation fails, NemoClaw keeps the local shields state and the auto-restore timer. +The timer keeps retrying lockdown and can restore it after the sandbox is reachable again. +NemoClaw records Shields down until a retry verifies lockdown, or until you destroy or rebuild the sandbox. If deletion fails after hardening, the command keeps the surviving sandbox's locked shields state instead of cleaning it up as though deletion succeeded. By default, unattended final-sandbox destroys (`--yes`, `--force`, or `NEMOCLAW_NON_INTERACTIVE=1`) remove the shared NemoClaw gateway on macOS so the host listener is released, while Linux preserves it for reuse. Pass `--cleanup-gateway` to force removal, or `--no-cleanup-gateway` to force preservation. @@ -1807,7 +1808,7 @@ If the OpenShell gateway is unreachable and the sandbox has no managed MCP owner Gateway-side deletion remains unconfirmed, shared host-service and gateway teardown are skipped, and the sandbox and retained volume may still exist if the gateway returns. Start the gateway with `$$nemoclaw status` and retry destroy when you need a confirmed deletion. Managed MCP ownership disables the local-only fallback because exact provider cleanup requires the retained ownership state, and other delete failures remain fatal. -A failed pre-delete lock also disables the local-only fallback, because the auto-restore timer is then the only authority that can lock the config again after the gateway returns. +A failed pre-delete re-lock also disables the local-only fallback, because the auto-restore timer is then the only authority that can lock the configuration again after the gateway returns. ```bash $$nemoclaw my-assistant destroy [--yes|-y|--force] [--cleanup-gateway|--no-cleanup-gateway]