Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e448e6e
fix(sandbox): prove live identity before refusing retained destroy
yanyunl1991 Sep 2, 2026
c351fce
Merge remote-tracking branch 'origin/main' into fix/destroy-retained-…
yanyunl1991 Sep 2, 2026
c21c193
Address CodeRabbit review: TOCTOU-safe retained-identity re-check + d…
yanyunl1991 Sep 2, 2026
5783651
Merge remote-tracking branch 'origin/main' into fix/destroy-retained-…
yanyunl1991 Sep 2, 2026
a5a973e
Fix retained-identity re-check mock after main's OpenShell identity r…
yanyunl1991 Sep 2, 2026
15acba8
Address CodeRabbit follow-up: also refresh destroy-preflight.js modul…
yanyunl1991 Sep 2, 2026
8dfeb3e
merge: resolve conflicts with main
github-actions[bot] Sep 3, 2026
f59ba8b
merge: resolve conflicts with main
github-actions[bot] Sep 3, 2026
35c21e9
merge: resolve conflicts with main
github-actions[bot] Sep 4, 2026
c997d3f
Rework #10863 fix to fail-closed: never auto-delete a live retained s…
yanyunl1991 Sep 7, 2026
fbaf37c
Merge remote-tracking branch 'origin/main' into fix/destroy-retained-…
yanyunl1991 Sep 7, 2026
6c857b4
merge: resolve conflicts with main
github-actions[bot] Sep 7, 2026
35ea2c6
Merge branch 'main' into fix/destroy-retained-sandbox-identity-proof-…
yanyunl1991 Sep 8, 2026
3ab3f22
merge: synchronize PR 10867 with main
jyaunches Sep 8, 2026
88dc49f
fix(sandbox): keep retained recovery fail closed
jyaunches Sep 8, 2026
e72f06e
fix(sandbox): close retained cleanup gaps
jyaunches Sep 8, 2026
c952c00
fix(sandbox): align retained recovery messages
jyaunches Sep 8, 2026
e2a455e
docs(sandbox): state retained recovery limitation
jyaunches Sep 8, 2026
84b525b
docs(sandbox): remove unresolved tracking claim
jyaunches Sep 8, 2026
d1ef3aa
docs(sandbox): align readiness timeout recovery
jyaunches Sep 8, 2026
6beb7fe
docs(sandbox): show retained timeout result
jyaunches Sep 8, 2026
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
8 changes: 4 additions & 4 deletions docs/reference/commands.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -365,14 +365,14 @@ When the recovery record contains a durable identity fingerprint, run `$$nemocla

Use the result from `destroy` to choose the next action:

- If OpenShell still reports the sandbox present, `destroy` preserves the record and removes no resources. Do not delete the sandbox manually by mutable name. Give the displayed create-attempt label to an OpenShell administrator when present and ask them to remove that exact sandbox through an identity-bound procedure. Without a label, preserve the terminal output and ask the administrator to identify the exact sandbox from gateway or controller evidence.
- After the administrator removes the exact sandbox, run `$$nemoclaw <sandbox-name> destroy --yes` to reconcile the recovery record.
- If OpenShell still reports the sandbox present, `destroy` compares the live sandbox's exact OpenShell id against the retained recovery record's durable identity fingerprint. When they match, `destroy` has proven the live sandbox is the exact retained one — not a different sandbox that has since reused the same name — and proceeds to delete it and clear the matching recovery record. This matches the sandbox-deletion result of running `openshell sandbox delete <sandbox-name>` yourself, but the manual command does not clear the recovery record.
- If OpenShell reports the sandbox present but `destroy` cannot prove that match (for example, a mismatched or unreadable live id), `destroy` preserves the record and removes no resources. Its output names the exact `openshell sandbox delete <sandbox-name>` command; compare `openshell sandbox list -o json` against the displayed create-attempt label yourself, and once you are confident it is the same sandbox, running that command is safe. After OpenShell confirms the sandbox is absent, run `$$nemoclaw <sandbox-name> destroy --yes` to reconcile the recovery record.
- If OpenShell confirms the sandbox is absent, `destroy` verifies one retained recovery record and its immutable runtime identity. For Docker-backed sandboxes, it also verifies the immutable Docker sandbox identity and removes only qualified residual containers. It clears the matching recovery record only after verified cleanup.
- If the recovery record is ambiguous or immutable runtime-identity verification fails, cleanup stops and the record remains. For Docker-backed sandboxes, a foreign container, changed Docker identity, or failed Docker probe has the same fail-closed result.

If OpenShell did not return a durable identity fingerprint, `destroy` cannot complete recovery. A create-attempt label can help an OpenShell administrator identify and remove the exact sandbox, but it does not let NemoClaw retire the record without immutable identity authority. If neither a fingerprint nor a label is available, preserve the terminal output and ask the administrator to identify the exact sandbox from gateway or controller evidence. If NemoClaw reports that it could not save recovery evidence, preserve the terminal output and follow the same escalation.
If OpenShell did not return a durable identity fingerprint, `destroy` cannot complete recovery or prove a match for a live sandbox. A create-attempt label narrows the search, but you must identify and remove the exact sandbox yourself — inspect `openshell sandbox list -o json` and compare it against the displayed create-attempt label and any other available evidence. If NemoClaw reports that it could not save recovery evidence, preserve the terminal output and follow the same manual-inspection path.

This fail-closed record keeps only the affected sandbox name unavailable. It is not retired from mutable-name absence alone, and this command does not accept administrator-supplied identity authority.
This fail-closed record keeps only the affected sandbox name unavailable. It is not retired from mutable-name absence alone, and this command does not accept externally supplied identity authority.
To onboard another sandbox while the record remains unresolved, supply a different explicit name:

```bash
Expand Down
30 changes: 30 additions & 0 deletions src/lib/actions/sandbox/destroy-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ type SandboxDestroyExecutionInput = {
expectedContainerIdentities?: readonly SandboxNameLabeledContainer[];
expectedContainerIdentityFingerprint?: string;
expectedRuntimeProviderIdentity?: RuntimeProviderDestroyIdentityReceipt;
// Set only when a retained-sandbox recovery record's identity was proven
// against the live OpenShell sandbox during preflight (#10863). Re-proven
// immediately before the delete command fires, so a same-name replacement
// sandbox created after preflight cannot be deleted under the recovery
// record's authority.
expectedRetainedSandboxIdentity?: {
readonly gatewayName: string;
readonly sandboxIdentityFingerprint: string;
};
portableContainerAuthority?: PreparedPortableDemoSandboxDestroyAuthority;
stopInferenceResources: () => void;
runtimeProviders?: RuntimeProviderBundleRegistry;
Expand Down Expand Up @@ -311,6 +320,7 @@ export async function executeSandboxDestroy({
expectedContainerIdentities,
expectedContainerIdentityFingerprint,
expectedRuntimeProviderIdentity,
expectedRetainedSandboxIdentity,
portableContainerAuthority,
stopInferenceResources,
runtimeProviders = CURRENT_RUNTIME_PROVIDER_BUNDLES,
Expand Down Expand Up @@ -389,9 +399,29 @@ export async function executeSandboxDestroy({
};
}
};
const inspectRetainedSandboxIdentityContinuity = (): IdentityContinuity => {
if (!expectedRetainedSandboxIdentity) return { status: "match" };
const subject = "Retained sandbox identity";
try {
const inspectIdentity =
deps.inspectOpenShellSandboxIdentityFingerprint ??
inspectOpenShellSandboxIdentityFingerprint;
const liveFingerprint = inspectIdentity({
sandboxName,
gatewayName: expectedRetainedSandboxIdentity.gatewayName,
});
return liveFingerprint === expectedRetainedSandboxIdentity.sandboxIdentityFingerprint
? { status: "match" }
: { status: "changed", subject };
} catch (error) {
return { status: "probe-failed", subject, detail: redactDestroyError(error) };
}
};
const inspectIdentityContinuity = (): IdentityContinuity => {
const pendingContinuity = inspectPendingCreateVerificationContinuity();
if (pendingContinuity.status !== "match") return pendingContinuity;
const retainedContinuity = inspectRetainedSandboxIdentityContinuity();
if (retainedContinuity.status !== "match") return retainedContinuity;
if (portableContainerAuthority) {
try {
portableContainerAuthority.revalidate();
Expand Down
30 changes: 26 additions & 4 deletions src/lib/actions/sandbox/destroy-preflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

import os from "node:os";

import { fingerprintOpenShellSandboxId } from "../../adapters/openshell/sandbox-identity";
import { observeOpenShellSandboxIdentity } from "../../adapters/openshell/sandbox-presence";
import { OPENSHELL_PROBE_TIMEOUT_MS } from "../../adapters/openshell/timeouts";
import { withModelRouterPortLifecycleLock } from "../../inference/gateway-route-mutation-lock";
import { DEFAULT_MODEL_ROUTER_PORT, isRoutedInferenceProvider } from "../../onboard/model-router";
Expand All @@ -22,7 +24,6 @@ import type {
import type { SandboxEntry } from "../../state/registry";
import * as registry from "../../state/registry";
import { type DestroyRunOpenshell, selectGatewayForSandboxDestroy } from "./destroy-gateway";
import { classifyDestroySandboxPresence } from "./destroy-presence";
import {
getPersistedSandboxTargetGatewayName,
getSandboxTargetGatewayName,
Expand All @@ -34,6 +35,12 @@ export type SandboxDestroyPreflight = {
runOpenshell: DestroyRunOpenshell;
sandbox: SandboxEntry | null;
sandboxConfirmedAbsent: boolean;
/**
* Fingerprint of the exact live OpenShell sandbox id when one is present,
* comparable to a retained recovery record's `sandboxIdentityFingerprint`
* (#10863). Null when absent or the observation could not be trusted.
*/
presentSandboxIdentityFingerprint: string | null;
};

export function stopSandboxInferenceResources(
Expand Down Expand Up @@ -295,15 +302,24 @@ export function prepareSandboxDestroy(
selectGatewayForSandboxDestroy(sandboxName, cleanupGatewayName, runOpenshell);
process.env.OPENSHELL_GATEWAY = cleanupGatewayName;

const sandboxPresence = classifyDestroySandboxPresence(
// Read the exact live OpenShell sandbox id (when present) alongside its
// presence, not just present/absent, so a caller can prove the live
// sandbox is the exact retained one before treating it as safe to delete
// by mutable name (#10863) — mirrors the identity comparison already used
// for Hermes Portable lifecycle verification.
const sandboxIdentityObservation = observeOpenShellSandboxIdentity(
sandboxName,
runOpenshell(["sandbox", "list", "-o", "json"], {
ignoreError: true,
stdio: ["ignore", "pipe", "pipe"],
timeout: OPENSHELL_PROBE_TIMEOUT_MS,
}),
);
const sandboxConfirmedAbsent = sandboxPresence === "absent";
const sandboxConfirmedAbsent = sandboxIdentityObservation.kind === "absent";
const presentSandboxIdentityFingerprint =
sandboxIdentityObservation.kind === "present"
? fingerprintOpenShellSandboxId(sandboxIdentityObservation.id)
: null;
const mcpEntriesRequiringConfigMutation = Object.values(sandbox?.mcp?.bridges ?? {}).filter(
(entry) => entry.addState !== "prepared",
);
Expand All @@ -323,5 +339,11 @@ export function prepareSandboxDestroy(
assertMcpAdapterConfigMutationsAllowed(sandboxName, sandbox, mcpEntriesRequiringConfigMutation);
}

return { cleanupGatewayName, runOpenshell, sandbox, sandboxConfirmedAbsent };
return {
cleanupGatewayName,
runOpenshell,
sandbox,
sandboxConfirmedAbsent,
presentSandboxIdentityFingerprint,
};
}
70 changes: 68 additions & 2 deletions src/lib/actions/sandbox/destroy-retained-recovery-flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
createDestroyHarness,
resetDestroyModuleCache,
} from "../../../../test/helpers/destroy-flow-test-harness";
import { executeSandboxDestroy } from "./destroy-execution";
import type { RetainedSandboxRecoveryRecord } from "../../state/onboard-session/retained-sandbox-recovery";

function retainedRecoveryRecord(sandboxId = "sb-alpha"): RetainedSandboxRecoveryRecord {
Expand Down Expand Up @@ -168,7 +169,7 @@ describe("destroySandbox retained recovery flow", () => {
);

expect(harness.errorSpy).toHaveBeenCalledWith(
expect.stringContaining("delete command accepts only the mutable sandbox name"),
expect.stringContaining("could not prove it is the exact retained sandbox"),
);
expect(harness.runOpenshellSpy).not.toHaveBeenCalledWith(
["sandbox", "delete", "alpha"],
Expand Down Expand Up @@ -202,7 +203,7 @@ describe("destroySandbox retained recovery flow", () => {
);

expect(harness.errorSpy).toHaveBeenCalledWith(
expect.stringContaining("cannot bind that deletion to the retained immutable identity"),
expect.stringContaining("could not prove it is the exact retained sandbox"),
);
expect(harness.runOpenshellSpy).not.toHaveBeenCalledWith(
["sandbox", "delete", "alpha"],
Expand All @@ -213,6 +214,71 @@ describe("destroySandbox retained recovery flow", () => {
},
);

it(
"issues mutable-name deletion for a live retained sandbox with a proven-matching OpenShell identity (#10863)",
{ timeout: 30_000 },
async () => {
const recovery = retainedRecoveryRecord("sandbox-alpha");
const containerId = "a".repeat(64);
const harness = createDestroyHarness({
dockerRunResult: {
status: 0,
stdout: `${containerId}\topenshell\tdefault\tsandbox-alpha`,
},
registryEntryOverrides: {
lifecycleGeneration: recovery.lifecycleGeneration!,
lifecycleLiveIdentityFingerprint: recovery.sandboxIdentityFingerprint!,
},
retainedRecoveryRecords: [recovery],
});

await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined();

expect(harness.runOpenshellSpy).toHaveBeenCalledWith(
["sandbox", "delete", "alpha"],
expect.anything(),
);
expect(harness.resolveRetainedSandboxRecoverySpy).toHaveBeenCalledWith(recovery);
expect(exitSpy).not.toHaveBeenCalled();
},
);

it(
"aborts the delete when the retained sandbox's live identity no longer matches at the delete boundary (#10863)",
{ timeout: 30_000 },
async () => {
const recovery = retainedRecoveryRecord("sandbox-alpha");
const runOpenshell = vi.fn(() => ({ status: 0, stdout: "", stderr: "" }));

const result = await executeSandboxDestroy({
cleanupShieldsArtifacts: () => undefined,
force: false,
runOpenshell,
sandbox: null,
sandboxConfirmedAbsent: false,
sandboxName: "alpha",
stopInferenceResources: () => undefined,
expectedRetainedSandboxIdentity: {
gatewayName: recovery.gatewayName,
sandboxIdentityFingerprint: recovery.sandboxIdentityFingerprint!,
},
deps: {
// Simulates a same-name replacement sandbox that appeared after the
// destroy.ts preflight proof but before this delete-boundary re-check.
inspectOpenShellSandboxIdentityFingerprint: () => "b".repeat(64),
readTimerMarker: () => null,
wipeSandboxState: () => undefined,
},
});

expect(result).toMatchObject({
ok: false,
deleteOutput: expect.stringContaining("Retained sandbox identity"),
});
expect(runOpenshell).not.toHaveBeenCalled();
},
);

it(
"finishes retained cleanup after OpenShell already removed the sandbox (#10547)",
{ timeout: 30_000 },
Expand Down
42 changes: 36 additions & 6 deletions src/lib/actions/sandbox/destroy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -803,13 +803,35 @@ async function destroySandboxUnlocked(
retainedRecoveryGatewayName: retainedRecoveryAuthority?.gatewayName,
}),
);
const { cleanupGatewayName, runOpenshell, sandbox, sandboxConfirmedAbsent } = destroyPreflight;
const {
cleanupGatewayName,
runOpenshell,
sandbox,
sandboxConfirmedAbsent,
presentSandboxIdentityFingerprint,
} = destroyPreflight;
let confirmedRetainedSandboxIdentityFingerprint: string | null = null;
if (retainedRecoveryAuthority && !sandboxConfirmedAbsent) {
console.error(
` Refusing to automatically delete retained sandbox '${sandboxName}': OpenShell still reports it present, but its delete command accepts only the mutable sandbox name. NemoClaw cannot bind that deletion to the retained immutable identity. No sandbox resources were removed. Ask an OpenShell administrator to resolve create-attempt label '${retainedRecoveryAuthority.createAttemptNonce}' to the exact sandbox and use an identity-bound removal procedure. After OpenShell confirms the retained sandbox is absent, rerun '${CLI_NAME} ${sandboxName} destroy --yes' to reconcile its verified Docker containers and recovery record.`,
);
preparedManagedLlamaCppCleanup?.abort();
requestSandboxDestroyExit(1);
// The live OpenShell sandbox with this mutable name may be the exact
// retained sandbox recovery is waiting on, or (rarely) a replacement
// sandbox that has since reused the same name. Prove which one this is
// from the live sandbox id fingerprint before deciding (#10863): a
// confirmed match is provably the retained sandbox, so destroy proceeds
// exactly as it would for any other present sandbox — an ordinary user
// running `openshell sandbox delete <name>` themselves reaches the same
// outcome, so NemoClaw can safely do it once identity is proven.
const identityConfirmedMatch =
retainedRecoveryAuthority.sandboxIdentityFingerprint !== null &&
presentSandboxIdentityFingerprint !== null &&
retainedRecoveryAuthority.sandboxIdentityFingerprint === presentSandboxIdentityFingerprint;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
if (!identityConfirmedMatch) {
console.error(
` Refusing to automatically delete retained sandbox '${sandboxName}': OpenShell reports a sandbox present under this name, but NemoClaw could not prove it is the exact retained sandbox (create-attempt label '${retainedRecoveryAuthority.createAttemptNonce}') rather than a different sandbox that has since reused the same name. No sandbox resources were removed. Compare 'openshell sandbox list -o json' against the create-attempt label above; if you confirm it is the same sandbox, 'openshell sandbox delete ${sandboxName}' is the exact command NemoClaw would otherwise run, and is safe to run yourself. After OpenShell confirms the retained sandbox is absent, rerun '${CLI_NAME} ${sandboxName} destroy --yes' to reconcile its verified Docker containers and recovery record.`,
);
preparedManagedLlamaCppCleanup?.abort();
requestSandboxDestroyExit(1);
}
confirmedRetainedSandboxIdentityFingerprint = retainedRecoveryAuthority.sandboxIdentityFingerprint;
}
// Recheck identity after pre-delete qualification and recoverable journal
// publication reconciliation, before any sandbox runtime mutation.
Expand Down Expand Up @@ -860,6 +882,14 @@ async function destroySandboxUnlocked(
? { expectedRuntimeProviderIdentity: initialIdentity.providerIdentity }
: {}),
...(portableContainerAuthority ? { portableContainerAuthority } : {}),
...(retainedRecoveryAuthority && confirmedRetainedSandboxIdentityFingerprint !== null
? {
expectedRetainedSandboxIdentity: {
gatewayName: retainedRecoveryAuthority.gatewayName,
sandboxIdentityFingerprint: confirmedRetainedSandboxIdentityFingerprint,
},
}
: {}),
stopInferenceResources: () => stopSandboxInferenceResources(sandboxName, sandbox),
});
} catch (error) {
Expand Down
33 changes: 33 additions & 0 deletions test/helpers/destroy-flow-test-harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,39 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr

const resolve = requireSource("../../adapters/openshell/resolve.js");
const runtime = requireSource("../../adapters/openshell/runtime.js");
const sanitizedCapture = requireSource("../../adapters/openshell/sanitized-capture.js");
// Backs the delete-boundary retained-sandbox identity re-check
// (`inspectOpenShellSandboxIdentityFingerprint`, #10863). Mirrors the same
// `sandbox-${name}` id convention `sandboxListJson` uses for `sandbox list`.
// `sandbox-identity-cli.js` binds this by value at its own module load time
// (not a live namespace lookup), so it — and everything that transitively
// requires it — must be re-required fresh after the spy is installed.
vi.spyOn(sanitizedCapture, "captureSanitizedResolvedOpenshell").mockImplementation(
(args: unknown) => {
const argv = Array.isArray(args) ? args.map(String) : [];
if (argv[0] === "sandbox" && argv[1] === "get") {
if (!sandboxPresent) {
return {
status: 1,
output: "",
stdout: "",
stderr: "sandbox not found",
error: new Error("sandbox not found"),
};
}
const name = argv.at(-1) ?? "";
return {
status: 0,
output: `Id: sandbox-${name}\n`,
stdout: `Id: sandbox-${name}\n`,
stderr: "",
};
}
return { status: 0, output: "", stdout: "", stderr: "" };
},
);
delete require.cache[requireSource.resolve("../../adapters/openshell/sandbox-identity-cli.js")];
delete require.cache[requireSource.resolve("./destroy-execution.js")];
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
const destroyGateway = requireSource("./destroy-gateway.js");
const credentialStore = requireSource("../../credentials/store.js");
const sandboxProviderCleanup = requireSource("../../onboard/sandbox-provider-cleanup.js");
Expand Down
Loading