Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion docs/reference/commands.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -1782,7 +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, the command refuses deletion and leaves the timer authority available to retry lockdown.
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.
Expand All @@ -1804,6 +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 <name> 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 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]
Expand Down
79 changes: 71 additions & 8 deletions src/lib/actions/sandbox/destroy-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,12 @@ export type SandboxDestroyExecutionResult =
gatewayUnreachable: boolean;
mcpOwnershipRequiresGateway: boolean;
mcpRecoveryFailure?: string;
shieldsRelockRequiresGateway: boolean;
};

type HardenedDeleteState = {
hardenedForDelete: boolean;
hardeningFailed: boolean;
timerProcessToken?: string;
};

Expand Down Expand Up @@ -103,23 +105,72 @@ function wipeAndHardenLiveSandbox(
sandboxConfirmedAbsent: boolean,
deps: NonNullable<SandboxDestroyExecutionInput["deps"]> = {},
): 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.
(deps.wipeSandboxState ?? wipeSandboxState)(sandboxName);
const timerMarker = (deps.readTimerMarker ?? 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) {
/**
* 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 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.
* 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
// 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}`,
);
console.warn(
` Continuing with delete — '${sandboxName}' and its unguarded config are removed together. ` +
"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 };
}
return { hardenedForDelete: true, hardeningFailed: false, timerProcessToken };
}

async function restoreMcpAfterDeleteAbort(
Expand Down Expand Up @@ -214,6 +265,7 @@ export async function executeSandboxDestroy({
exitCode: 1,
gatewayUnreachable: false,
mcpOwnershipRequiresGateway: false,
shieldsRelockRequiresGateway: false,
};
}
}
Expand Down Expand Up @@ -244,8 +296,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
Expand All @@ -258,6 +320,7 @@ export async function executeSandboxDestroy({
gatewayUnreachable,
mcpOwnershipRequiresGateway: gatewayUnreachable && hasMcpOwnership,
mcpRecoveryFailure,
shieldsRelockRequiresGateway: gatewayUnreachable && hardened.hardeningFailed,
};
}

Expand Down
43 changes: 38 additions & 5 deletions src/lib/actions/sandbox/destroy-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ import {
expectAbsentSandboxMcpFinalize,
expectActiveTimerDestroyOrder,
expectFailedDeletePreservesHostState,
expectFailedHardeningStopsDelete,
expectFailedHardeningMcpRestore,
expectFailedHardeningRefusesForcedCleanup,
expectFailedHardeningStillDeletes,
expectFailedMcpFinalizePreservesRegistry,
expectFailedMcpRestorePreservesDestroyFailure,
expectMcpFinalizeAfterDelete,
Expand Down Expand Up @@ -265,17 +267,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 () => {
Expand Down
9 changes: 8 additions & 1 deletion src/lib/actions/sandbox/destroy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,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.`,
);
Expand Down
8 changes: 7 additions & 1 deletion src/lib/actions/sandbox/mcp-bridge-status-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 = `
Expand Down
110 changes: 110 additions & 0 deletions src/lib/shields/flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,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;
Expand Down
Loading
Loading