diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 5a6174f648e..9dbbb943654 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -2243,6 +2243,42 @@ If you want to upgrade the sandbox while preserving state, use `$$nemoclaw +Do not remove or recreate a container until you verify its purpose, ownership, and data-retention requirements. +Removing or recreating a container can discard state that is not stored in a volume. + + +Resolve a conflict through the workflow that created the conflicting container. +Docker cannot change labels on an existing container. +Rerun the query after you resolve the conflict. +Rerun `destroy` only when the query returns one complete expected label set that you verified belongs to the target sandbox, or no containers after you independently confirm that the sandbox is absent. + If the Hermes sandbox has managed MCP entries, shields must be down before destroy can scrub their adapter configuration. diff --git a/src/lib/actions/sandbox/destroy-container-identity.test.ts b/src/lib/actions/sandbox/destroy-container-identity.test.ts new file mode 100644 index 00000000000..a63f1f61fbf --- /dev/null +++ b/src/lib/actions/sandbox/destroy-container-identity.test.ts @@ -0,0 +1,198 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { + classifyDestroyContainerIdentity, + type DestroyContainerIdentityVerdict, + formatAmbiguousDestroyIdentity, +} from "./destroy-presence"; + +type Row = { + id: string; + managedBy?: string; + workspace?: string; + sandboxId?: string; +}; + +function observeRows(rows: Row[], malformedRows = 0) { + return { + status: "observed" as const, + rows: rows.map((row) => ({ + id: row.id, + managedBy: row.managedBy ?? "", + workspace: row.workspace ?? "", + sandboxId: row.sandboxId ?? "", + })), + malformedRows, + }; +} + +const MANAGED = { + id: "aaaa000000000000", + managedBy: "openshell", + workspace: "default", + sandboxId: "sb-real", +} as const; + +const FOREIGN = { + id: "ffff000000000000", + managedBy: "", + workspace: "foreign", + sandboxId: "", +} as const; + +function expectAmbiguous( + verdict: DestroyContainerIdentityVerdict, +): Extract { + expect(verdict.status).toBe("ambiguous"); + return verdict as Extract; +} + +describe("classifyDestroyContainerIdentity", () => { + it("is clear when no container carries the sandbox-name label", () => { + expect(classifyDestroyContainerIdentity("destroytest", observeRows([]))).toEqual({ + status: "clear", + identity: null, + }); + }); + + it("is clear for exactly one managed container", () => { + expect(classifyDestroyContainerIdentity("destroytest", observeRows([MANAGED]))).toEqual({ + status: "clear", + identity: MANAGED, + }); + }); + + it("refuses when a foreign container shares the sandbox-name label (#8999)", () => { + // The exact repro: a real managed sandbox plus a busybox that borrows the + // sandbox-name label with a different workspace and no managed marker. + const verdict = expectAmbiguous( + classifyDestroyContainerIdentity("destroytest", observeRows([MANAGED, FOREIGN])), + ); + expect(verdict.foreign).toHaveLength(1); + expect(verdict.foreign[0].id).toBe(FOREIGN.id); + expect(verdict.managed).toHaveLength(1); + expect(verdict.reason).toContain("managed-by"); + }); + + it("refuses a foreign-only match with no managed container behind it", () => { + const verdict = expectAmbiguous( + classifyDestroyContainerIdentity("destroytest", observeRows([FOREIGN])), + ); + expect(verdict.managed).toHaveLength(0); + expect(verdict.foreign).toHaveLength(1); + }); + + it("refuses when managed containers span more than one workspace", () => { + const observe = vi.fn(() => + observeRows([ + MANAGED, + { + id: "bbbb000000000000", + managedBy: "openshell", + workspace: "other", + sandboxId: "sb-real", + }, + ]), + ); + const verdict = expectAmbiguous(classifyDestroyContainerIdentity("destroytest", observe())); + expect(verdict.reason).toContain("2 managed containers"); + }); + + it("refuses when managed containers span more than one sandbox-id", () => { + const observe = vi.fn(() => + observeRows([ + MANAGED, + { + id: "cccc000000000000", + managedBy: "openshell", + workspace: "default", + sandboxId: "sb-two", + }, + ]), + ); + expect(classifyDestroyContainerIdentity("destroytest", observe()).status).toBe("ambiguous"); + }); + + it("refuses multiple managed containers even when their mutable labels match", () => { + const verdict = expectAmbiguous( + classifyDestroyContainerIdentity( + "destroytest", + observeRows([MANAGED, { ...MANAGED, id: "dddd000000000000" }]), + ), + ); + expect(verdict.reason).toContain("2 managed containers"); + }); + + it.each([ + ["workspace", { ...MANAGED, workspace: "" }], + ["sandbox ID", { ...MANAGED, sandboxId: "" }], + ])("refuses a managed container with no %s", (_label, row) => { + const verdict = expectAmbiguous( + classifyDestroyContainerIdentity("destroytest", observeRows([row])), + ); + expect(verdict.reason).toContain("missing"); + }); + + it("refuses malformed Docker identity output", () => { + const verdict = expectAmbiguous( + classifyDestroyContainerIdentity("destroytest", observeRows([], 1)), + ); + expect(verdict.reason).toContain("malformed container identity"); + }); + + it("refuses terminal-control label output without rendering it", () => { + const verdict = expectAmbiguous( + classifyDestroyContainerIdentity("destroytest", observeRows([], 1)), + ); + expect(formatAmbiguousDestroyIdentity(verdict, "nemoclaw").join("\n")).not.toContain("\u001b"); + }); + + it("reports a failed Docker probe when identity cannot be proven", () => { + const verdict = classifyDestroyContainerIdentity("destroytest", { + status: "probe-failed", + detail: "Cannot connect to daemon", + }); + expect(verdict).toEqual({ + status: "probe-failed", + detail: expect.stringContaining("daemon"), + }); + }); +}); + +describe("formatAmbiguousDestroyIdentity", () => { + it("names the refusal, both container roles, and the recovery step", () => { + const verdict = expectAmbiguous( + classifyDestroyContainerIdentity("destroytest", observeRows([MANAGED, FOREIGN])), + ); + const lines = formatAmbiguousDestroyIdentity(verdict, "nemoclaw").join("\n"); + expect(lines).toContain("Refusing to destroy sandbox 'destroytest'"); + expect(lines).toContain("Conflicting container:"); + expect(lines).toContain("Managed sandbox container:"); + expect(lines).toContain('openshell.ai/sandbox-id="sb-real"'); + expect(lines).toContain("Resolve the conflict through the workflow that owns the container"); + expect(lines).toContain("nemoclaw destroytest destroy"); + expect(lines).not.toContain("--yes"); + }); + + it("quotes label values so printable delimiters cannot forge adjacent fields", () => { + const foreign = { + ...FOREIGN, + managedBy: 'foreign", openshell.ai/sandbox-workspace="default', + workspace: 'foo, bar"baz', + sandboxId: 'sb-foreign", openshell.ai/managed-by="openshell', + }; + const verdict = expectAmbiguous( + classifyDestroyContainerIdentity("destroytest", observeRows([MANAGED, foreign])), + ); + const lines = formatAmbiguousDestroyIdentity(verdict, "nemoclaw").join("\n"); + expect(lines).toContain(`openshell.ai/managed-by=${JSON.stringify(foreign.managedBy)}`); + expect(lines).toContain(`openshell.ai/sandbox-workspace=${JSON.stringify(foreign.workspace)}`); + expect(lines).toContain(`openshell.ai/sandbox-id=${JSON.stringify(foreign.sandboxId)}`); + expect(lines).not.toContain( + 'openshell.ai/managed-by="foreign", openshell.ai/sandbox-workspace="default"', + ); + }); +}); diff --git a/src/lib/actions/sandbox/destroy-execution.ts b/src/lib/actions/sandbox/destroy-execution.ts index 0629dc97ef7..3fc1c0b366a 100644 --- a/src/lib/actions/sandbox/destroy-execution.ts +++ b/src/lib/actions/sandbox/destroy-execution.ts @@ -18,6 +18,12 @@ import { redact, redactFull } from "../../security/redact"; import { withTimerBoundShieldsMutationLockAsync } from "../../shields/timer-bound-lock"; import { readTimerMarker } from "../../shields/timer-control"; import type { SandboxEntry } from "../../state/registry"; +import { + classifyDestroyContainerIdentity, + isSameDestroyContainerIdentity, + observeDestroyContainerIdentity, + type SandboxNameLabeledContainer, +} from "./destroy-presence"; import type { DestroyRunOpenshell } from "./destroy-gateway"; import { finalizeMcpBridgesAfterSandboxDelete, @@ -44,6 +50,11 @@ type SandboxDestroyExecutionInput = { sandbox: SandboxEntry | null; sandboxConfirmedAbsent: boolean; sandboxName: string; + // `undefined` delegates identity gating to the runtime provider. + // `null` records confirmed absence; an object records the one managed + // container observed by the pre-destroy guard. + expectedContainerIdentity?: SandboxNameLabeledContainer | null; + stopInferenceResources: () => void; runtimeProviders?: RuntimeProviderBundleRegistry; deps?: { readTimerMarker?: typeof readTimerMarker; @@ -206,7 +217,7 @@ async function restoreMcpAfterDeleteAbort( } await restoreMcpBridgesAfterDestroyAbort(sandboxName, preparation); } catch (error) { - recoveryFailure = error instanceof Error ? error.message : String(error); + recoveryFailure = redactDestroyError(error); } finally { if (openedRollbackWindow) { try { @@ -216,7 +227,7 @@ async function restoreMcpAfterDeleteAbort( allowLegacyHermesProtocol: true, }); } catch (error) { - const detail = error instanceof Error ? error.message : String(error); + const detail = redactDestroyError(error); recoveryFailure = recoveryFailure ? `${recoveryFailure}; shields re-lock failed: ${detail}` : `shields re-lock failed: ${detail}`; @@ -252,10 +263,57 @@ export async function executeSandboxDestroy({ sandbox, sandboxConfirmedAbsent, sandboxName, + expectedContainerIdentity, + stopInferenceResources, runtimeProviders = CURRENT_RUNTIME_PROVIDER_BUNDLES, deps = {}, }: SandboxDestroyExecutionInput): Promise { return withTimerBoundShieldsMutationLockAsync(sandboxName, "destroy sandbox", async () => { + type IdentityContinuity = + | { status: "match" } + | { status: "changed" } + | { status: "ambiguous"; detail: string } + | { status: "probe-failed"; detail: string }; + const inspectIdentityContinuity = (): IdentityContinuity => { + if (expectedContainerIdentity === undefined) return { status: "match" }; + const verdict = classifyDestroyContainerIdentity( + sandboxName, + observeDestroyContainerIdentity(sandboxName), + ); + if (isSameDestroyContainerIdentity(expectedContainerIdentity, verdict)) { + return { status: "match" }; + } + if (verdict.status === "probe-failed") { + return { status: "probe-failed", detail: redactDestroyError(verdict.detail) }; + } + if (verdict.status === "ambiguous") { + return { status: "ambiguous", detail: redactDestroyError(verdict.reason) }; + } + return { status: "changed" }; + }; + const identityRefusalResult = ( + phase: string, + continuity: Exclude, + mcpRecoveryFailure?: string, + earlierCleanupDetail = "", + ): SandboxDestroyExecutionResult => ({ + ok: false, + deleteOutput: + continuity.status === "probe-failed" + ? `Container identity could not be inspected ${phase}: ${continuity.detail}. No sandbox delete was attempted.${earlierCleanupDetail}` + : continuity.status === "ambiguous" + ? `Container identity became ambiguous ${phase}: ${continuity.detail}. No sandbox delete was attempted.${earlierCleanupDetail}` + : `Container identity changed ${phase}; no sandbox delete was attempted.${earlierCleanupDetail}`, + exitCode: 1, + gatewayUnreachable: false, + mcpOwnershipRequiresGateway: false, + mcpRecoveryFailure, + shieldsRelockRequiresGateway: false, + }); + const initialContinuity = inspectIdentityContinuity(); + if (initialContinuity.status !== "match") { + return identityRefusalResult("before destroy preparation", initialContinuity); + } let runtimeProvider: RuntimeProviderBundle | null = null; if (sandbox) { try { @@ -295,14 +353,86 @@ export async function executeSandboxDestroy({ // discarded during preparation. Remaining entries are the durable exact // provider ownership manifest and must survive an unconfirmed delete. const hasMcpOwnership = mcpPreparation.entries.length > 0; + const notHardened: HardenedDeleteState = { + hardenedForDelete: false, + hardeningFailed: false, + }; + const restoreMcpForAbort = async ( + hardenedState: HardenedDeleteState, + ): Promise => + sandboxConfirmedAbsent + ? undefined + : await restoreMcpAfterDeleteAbort(sandboxName, mcpPreparation, hardenedState); + const preparedContinuity = inspectIdentityContinuity(); + if (preparedContinuity.status !== "match") { + const mcpRecoveryFailure = await restoreMcpForAbort(notHardened); + return identityRefusalResult( + "during destroy preparation", + preparedContinuity, + mcpRecoveryFailure, + ); + } + try { + stopInferenceResources(); + } catch (error) { + const mcpRecoveryFailure = await restoreMcpForAbort(notHardened); + return { + ok: false, + deleteOutput: + `Could not stop managed inference resources before sandbox deletion: ${redactDestroyError(error)}. ` + + "No workspace wipe, provider cleanup, or sandbox deletion was attempted.", + exitCode: 1, + gatewayUnreachable: false, + mcpOwnershipRequiresGateway: false, + mcpRecoveryFailure, + shieldsRelockRequiresGateway: false, + }; + } + const postInferenceContinuity = inspectIdentityContinuity(); + if (postInferenceContinuity.status !== "match") { + const mcpRecoveryFailure = await restoreMcpForAbort(notHardened); + return identityRefusalResult( + "after managed inference cleanup", + postInferenceContinuity, + mcpRecoveryFailure, + " Managed inference cleanup may already be partial; inspect or restart its resources before retrying.", + ); + } const hardened = wipeAndHardenLiveSandbox(sandboxName, sandboxConfirmedAbsent, deps); const detachProviders = (): DetachSandboxProvidersResult => runSandboxProviderPreDeleteCleanup(sandboxName, { runOpenshell, redact }); + const preProviderContinuity = inspectIdentityContinuity(); + if (preProviderContinuity.status !== "match") { + const mcpRecoveryFailure = await restoreMcpForAbort(hardened); + return identityRefusalResult( + "before provider cleanup", + preProviderContinuity, + mcpRecoveryFailure, + " Managed inference cleanup and workspace wipe or hardening may already have run; inspect those resources before retrying.", + ); + } const detachOutcome: DetachSandboxProvidersResult = sandboxConfirmedAbsent ? { detached: [], failures: [] } : runtimeProvider?.cleanup.supported === true && sandbox ? runtimeProvider.cleanup.prepareDestroy({ sandbox, sandboxName }, { detachProviders }) : detachProviders(); + // The final identity proof runs immediately before OpenShell delete. A + // runtime administrator remains a trusted host authority; this closes the + // multi-step window without claiming a cross-runtime transaction. + const deleteBoundaryContinuity = inspectIdentityContinuity(); + if (deleteBoundaryContinuity.status !== "match") { + const detachedDetail = + detachOutcome.detached.length > 0 + ? ` Provider cleanup detached ${detachOutcome.detached.join(", ")}; rerun the owning setup workflow to restore those attachments.` + : ""; + const mcpRecoveryFailure = await restoreMcpForAbort(hardened); + return identityRefusalResult( + "at the delete boundary", + deleteBoundaryContinuity, + mcpRecoveryFailure, + ` Managed inference cleanup and workspace wipe or hardening may already have run; inspect those resources before retrying.${detachedDetail}`, + ); + } const deleteResult = runOpenshell(["sandbox", "delete", sandboxName], { ignoreError: true, stdio: ["ignore", "pipe", "pipe"], diff --git a/src/lib/actions/sandbox/destroy-flow.test.ts b/src/lib/actions/sandbox/destroy-flow.test.ts index 97986044577..40eac504650 100644 --- a/src/lib/actions/sandbox/destroy-flow.test.ts +++ b/src/lib/actions/sandbox/destroy-flow.test.ts @@ -23,6 +23,7 @@ import { import { createDestroyHarness, resetDestroyModuleCache, + traceDestroyBoundaryCalls, } from "../../../../test/helpers/destroy-flow-test-harness"; describe("destroySandbox flow", () => { @@ -88,18 +89,21 @@ describe("destroySandbox flow", () => { false, ], ["NEMOCLAW_NON_INTERACTIVE=1", "linux", {}, "1", false], - ] as const)("applies the final-gateway default for %s on %s (#4662)", async (_scenario, platform, options, nonInteractive, cleanupExpected) => { - vi.spyOn(process, "platform", "get").mockReturnValue(platform); - vi.stubEnv("NEMOCLAW_NON_INTERACTIVE", nonInteractive); - const harness = createDestroyHarness(); - - await expect(harness.destroySandbox("alpha", options)).resolves.toBeUndefined(); - - expect(harness.promptSpy).not.toHaveBeenCalled(); - expect(harness.cleanupGatewaySpy.mock.calls).toEqual( - cleanupExpected ? [["nemoclaw-19080", harness.runOpenshellSpy]] : [], - ); - }); + ] as const)( + "applies the final-gateway default for %s on %s (#4662)", + async (_scenario, platform, options, nonInteractive, cleanupExpected) => { + vi.spyOn(process, "platform", "get").mockReturnValue(platform); + vi.stubEnv("NEMOCLAW_NON_INTERACTIVE", nonInteractive); + const harness = createDestroyHarness(); + + await expect(harness.destroySandbox("alpha", options)).resolves.toBeUndefined(); + + expect(harness.promptSpy).not.toHaveBeenCalled(); + expect(harness.cleanupGatewaySpy.mock.calls).toEqual( + cleanupExpected ? [["nemoclaw-19080", harness.runOpenshellSpy]] : [], + ); + }, + ); it("stops before local cleanup when OpenShell fails to delete the live sandbox", async () => { const harness = createDestroyHarness({ @@ -113,6 +117,268 @@ describe("destroySandbox flow", () => { expect(harness.retirePortableLifecycleReceiptSpy).not.toHaveBeenCalled(); }); + it("refuses before destructive work when Docker identity cannot be inspected", async () => { + const harness = createDestroyHarness({ + dockerRunResult: { status: 1, stderr: "Docker daemon unavailable" }, + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.errorSpy.mock.calls.map((call) => String(call[0])).join("\n")).toContain( + "Docker container identity could not be inspected", + ); + expect(harness.runOpenshellSpy).not.toHaveBeenCalled(); + expect(harness.events).toEqual([]); + expect(harness.selectGatewaySpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.updateSessionSpy).not.toHaveBeenCalled(); + expect(harness.stopAllSpy).not.toHaveBeenCalled(); + expect(harness.killTimerSpy).not.toHaveBeenCalled(); + expect(harness.prepareMcpBridgesForDestroySpy).not.toHaveBeenCalled(); + expect(harness.stopNimByNameSpy).not.toHaveBeenCalled(); + expect(harness.killStaleProxySpy).not.toHaveBeenCalled(); + expect(harness.cleanupGatewaySpy).not.toHaveBeenCalled(); + expect(harness.retirePortableLifecycleReceiptSpy).not.toHaveBeenCalled(); + expect(harness.dockerRunSpy).toHaveBeenCalledOnce(); + }); + + it("refuses before destructive work when multiple Docker identities share the name", async () => { + const rows = [ + "aaaa000000000000\topenshell\tdefault\tsb-real", + "ffff000000000000\t\tforeign\t", + ].join("\n"); + const harness = createDestroyHarness({ + dockerRunResult: { status: 0, stdout: rows }, + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + const errorOutput = harness.errorSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(errorOutput).toContain("could not verify one complete container identity"); + expect(errorOutput).toContain("Managed sandbox container: aaaa00000000"); + expect(errorOutput).toContain("Conflicting container: ffff00000000"); + expect(errorOutput).toContain("sb-real"); + expect(harness.runOpenshellSpy).not.toHaveBeenCalled(); + expect(harness.events).toEqual([]); + expect(harness.events).not.toContain("wipe"); + expect(harness.events).not.toContain("detach"); + expect(harness.selectGatewaySpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.updateSessionSpy).not.toHaveBeenCalled(); + expect(harness.prepareMcpBridgesForDestroySpy).not.toHaveBeenCalled(); + expect(harness.stopNimByNameSpy).not.toHaveBeenCalled(); + expect(harness.killStaleProxySpy).not.toHaveBeenCalled(); + expect(harness.cleanupGatewaySpy).not.toHaveBeenCalled(); + expect(harness.retirePortableLifecycleReceiptSpy).not.toHaveBeenCalled(); + expect(harness.dockerRunSpy).toHaveBeenCalledOnce(); + }); + + it("refuses identity drift after read-only destroy preflight (#8999)", async () => { + const managed = "aaaa000000000000\topenshell\tdefault\tsb-alpha"; + const foreign = "ffff000000000000\t\tforeign\t"; + const harness = createDestroyHarness({ + dockerRunResultSequence: [ + { status: 0, stdout: managed }, + { status: 0, stdout: [managed, foreign].join("\n") }, + ], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.selectGatewaySpy).toHaveBeenCalledOnce(); + expect(harness.runOpenshellSpy).toHaveBeenCalledWith( + ["sandbox", "list", "-o", "json"], + expect.any(Object), + ); + expect(harness.events).toEqual([]); + expect(harness.stopNimByNameSpy).not.toHaveBeenCalled(); + expect(harness.killStaleProxySpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.updateSessionSpy).not.toHaveBeenCalled(); + expect(harness.prepareMcpBridgesForDestroySpy).not.toHaveBeenCalled(); + expect(harness.stopAllSpy).not.toHaveBeenCalled(); + expect(harness.cleanupGatewaySpy).not.toHaveBeenCalled(); + expect(harness.retirePortableLifecycleReceiptSpy).not.toHaveBeenCalled(); + expect(harness.dockerRunSpy).toHaveBeenCalledTimes(2); + }); + + it.each([ + [ + "a replacement identity", + { status: 0, stdout: "bbbb000000000000\topenshell\tdefault\tsb-beta" }, + ], + ["container absence", { status: 0, stdout: "" }], + ["a failed revalidation probe", { status: 1, stderr: "daemon unavailable" }], + ])("refuses %s before sandbox mutation", async (_scenario, changedIdentity) => { + const managed = { status: 0, stdout: "aaaa000000000000\topenshell\tdefault\tsb-alpha" }; + const harness = createDestroyHarness({ + dockerRunResultSequence: [managed, managed, changedIdentity], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.events).toEqual([]); + expect(harness.stopNimByNameSpy).not.toHaveBeenCalled(); + expect(harness.killStaleProxySpy).not.toHaveBeenCalled(); + expect(harness.prepareMcpBridgesForDestroySpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.retirePortableLifecycleReceiptSpy).not.toHaveBeenCalled(); + expect(harness.dockerRunSpy).toHaveBeenCalledTimes(3); + }); + + it("refuses an absent-to-present replacement before sandbox mutation", async () => { + const absent = { status: 0, stdout: "" }; + const replacement = { + status: 0, + stdout: "bbbb000000000000\topenshell\tdefault\tsb-beta", + }; + const harness = createDestroyHarness({ + sandboxPresent: false, + dockerRunResultSequence: [absent, absent, replacement], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.events).toEqual([]); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.retirePortableLifecycleReceiptSpy).not.toHaveBeenCalled(); + expect(harness.dockerRunSpy).toHaveBeenCalledTimes(3); + }); + + it("restores MCP preparation when identity changes before wipe", async () => { + const managed = { status: 0, stdout: "aaaa000000000000\topenshell\tdefault\tsb-alpha" }; + const replacement = { + status: 0, + stdout: "bbbb000000000000\topenshell\tdefault\tsb-beta", + }; + const harness = createDestroyHarness({ + mcpServers: ["github"], + dockerRunResultSequence: [managed, managed, managed, replacement], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.events).toEqual(["mcp-prepare", "mcp-restore"]); + expect(harness.stopNimByNameSpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.retirePortableLifecycleReceiptSpy).not.toHaveBeenCalled(); + expect(harness.dockerRunSpy).toHaveBeenCalledTimes(4); + }); + + it("restores MCP preparation when managed inference cleanup fails before wipe", async () => { + const inferenceSecretMarker = "inference-cleanup-secret"; + const recoverySecretMarker = "mcp-recovery-secret"; + const harness = createDestroyHarness({ + mcpServers: ["github"], + restoreMcpError: `OPENAI_API_KEY=${recoverySecretMarker}`, + stopInferenceError: `OPENAI_API_KEY=${inferenceSecretMarker}`, + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.events).toEqual(["mcp-prepare", "mcp-restore"]); + expect(harness.stopNimByNameSpy).toHaveBeenCalledOnce(); + expect(harness.runOpenshellSpy).not.toHaveBeenCalledWith( + ["sandbox", "exec", "alpha"], + expect.anything(), + ); + expect(harness.events).not.toContain("wipe"); + expect(harness.events).not.toContain("detach"); + expect(harness.events).not.toContain("delete"); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + const errorOutput = harness.errorSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(errorOutput).toContain("Could not stop managed inference resources"); + expect(errorOutput).not.toContain(inferenceSecretMarker); + expect(errorOutput).not.toContain(recoverySecretMarker); + }); + + it.each([ + [ + "a replacement identity", + { status: 0, stdout: "bbbb000000000000\topenshell\tdefault\tsb-beta" }, + "Container identity changed after managed inference cleanup", + ], + [ + "a failed Docker reinspection", + { status: 1, stderr: "daemon unavailable" }, + "Container identity could not be inspected after managed inference cleanup: daemon unavailable", + ], + ])( + "restores MCP preparation and refuses workspace wipe after %s", + async (_scenario, changedIdentity, expectedMessage) => { + const managed = { status: 0, stdout: "aaaa000000000000\topenshell\tdefault\tsb-alpha" }; + const harness = createDestroyHarness({ + mcpServers: ["github"], + dockerRunResultSequence: [managed, managed, managed, managed, changedIdentity], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow( + "process.exit(1)", + ); + + expect(harness.events).toEqual(["mcp-prepare", "mcp-restore"]); + expect(harness.stopNimByNameSpy).toHaveBeenCalledOnce(); + expect(harness.events).not.toContain("wipe"); + expect(harness.events).not.toContain("detach"); + expect(harness.events).not.toContain("delete"); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + const errorOutput = harness.errorSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(errorOutput).toContain(expectedMessage); + expect(errorOutput).toContain("Managed inference cleanup may already be partial"); + }, + ); + + it("revalidates immediately before delete and reports partial provider preparation", async () => { + const managed = { status: 0, stdout: "aaaa000000000000\topenshell\tdefault\tsb-alpha" }; + const replacement = { + status: 0, + stdout: "bbbb000000000000\topenshell\tdefault\tsb-beta", + }; + const harness = createDestroyHarness({ + detachedProviders: ["provider-a"], + dockerRunResultSequence: [ + managed, // Initial guard before read-only preflight. + managed, // Guard after preflight and before mutation. + managed, // Execution-entry continuity guard. + managed, // Guard after MCP destroy preparation. + managed, // Guard after managed inference cleanup. + managed, // Guard before provider cleanup. + replacement, // Final guard immediately before sandbox deletion. + ], + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.events).toEqual(["wipe", "detach", "mcp-restore"]); + expect( + harness.runOpenshellSpy.mock.calls.some( + ([args]) => Array.isArray(args) && args[0] === "sandbox" && args[1] === "delete", + ), + ).toBe(false); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.retirePortableLifecycleReceiptSpy).not.toHaveBeenCalled(); + expect(harness.dockerRunSpy).toHaveBeenCalledTimes(7); + const errorOutput = harness.errorSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(errorOutput).toContain("Provider cleanup detached provider-a"); + }); + + it("keeps the final exact probe immediately adjacent to sandbox delete", async () => { + const managed = { status: 0, stdout: "aaaa000000000000\topenshell\tdefault\tsb-alpha" }; + const trace: string[] = []; + const harness = createDestroyHarness({ + dockerRunResult: managed, + onDockerRun: (call) => trace.push(`probe:${String(call)}`), + }); + traceDestroyBoundaryCalls(harness, trace); + + await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); + + expect(trace.slice(-2)).toEqual([ + `probe:${String(harness.dockerRunSpy.mock.calls.length)}`, + "delete", + ]); + }); + it("preserves provider and registry ownership when runtime authority is unknown", async () => { const harness = createDestroyHarness({ openshellDriver: "unknown-runtime", diff --git a/src/lib/actions/sandbox/destroy-preflight.ts b/src/lib/actions/sandbox/destroy-preflight.ts index 2b8fb1ce449..dc2b688d726 100644 --- a/src/lib/actions/sandbox/destroy-preflight.ts +++ b/src/lib/actions/sandbox/destroy-preflight.ts @@ -16,7 +16,10 @@ export type SandboxDestroyPreflight = { sandboxConfirmedAbsent: boolean; }; -function stopSandboxInferenceResources(sandboxName: string, sandbox: SandboxEntry | null): void { +export function stopSandboxInferenceResources( + sandboxName: string, + sandbox: SandboxEntry | null, +): void { const nim = require("../../inference/nim") as { stopNimContainer: (name: string, opts?: { silent?: boolean }) => void; stopNimContainerByName: (name: string) => void; @@ -76,6 +79,5 @@ export function prepareSandboxDestroy(sandboxName: string): SandboxDestroyPrefli assertMcpAdapterConfigMutationsAllowed(sandboxName, sandbox, mcpEntriesRequiringConfigMutation); } - stopSandboxInferenceResources(sandboxName, sandbox); return { cleanupGatewayName, runOpenshell, sandbox, sandboxConfirmedAbsent }; } diff --git a/src/lib/actions/sandbox/destroy-presence.ts b/src/lib/actions/sandbox/destroy-presence.ts index a03dad14f70..0760c5a28c3 100644 --- a/src/lib/actions/sandbox/destroy-presence.ts +++ b/src/lib/actions/sandbox/destroy-presence.ts @@ -1,6 +1,236 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { + OPENSHELL_MANAGED_BY_LABEL, + OPENSHELL_MANAGED_BY_VALUE, + OPENSHELL_SANDBOX_ID_LABEL, + OPENSHELL_SANDBOX_NAME_LABEL, +} from "../../onboard/openshell-docker-sandbox-containers"; +import { sanitizeReadinessText } from "../../readiness/sanitize"; +import { + type DockerSandboxIdentityObservation, + inspectDockerSandboxIdentities, +} from "../../adapters/docker/inspect"; + +/** Workspace label OpenShell stamps on every managed sandbox container. */ +export const OPENSHELL_SANDBOX_WORKSPACE_LABEL = "openshell.ai/sandbox-workspace"; + +const IDENTITY_VALUE_MAX_LENGTH = 256; +const IDENTITY_DIAGNOSTIC_MAX_LENGTH = 500; + +/** One container carrying the destroy target's `sandbox-name` label. */ +export type SandboxNameLabeledContainer = { + id: string; + managedBy: string; + workspace: string; + sandboxId: string; +}; + +/** Verdict for whether destroy resolved one complete managed container identity. */ +export type DestroyContainerIdentityVerdict = + | { status: "clear"; identity: SandboxNameLabeledContainer | null } + | { status: "probe-failed"; detail: string } + | { + status: "ambiguous"; + sandboxName: string; + reason: string; + foreign: SandboxNameLabeledContainer[]; + managed: SandboxNameLabeledContainer[]; + }; + +export type AssertUnambiguousDestroyIdentityDeps = { + providerId: string; + redact: (detail: string) => string; + cliName?: string; + classify?: (sandboxName: string) => DestroyContainerIdentityVerdict; + error?: (message: string) => void; +}; + +export type DestroyContainerIdentityProof = { + identity: SandboxNameLabeledContainer | null | undefined; +}; + +function observeDockerSandboxIdentities(sandboxName: string): DockerSandboxIdentityObservation { + return inspectDockerSandboxIdentities(`${OPENSHELL_SANDBOX_NAME_LABEL}=${sandboxName}`, { + managedBy: OPENSHELL_MANAGED_BY_LABEL, + workspace: OPENSHELL_SANDBOX_WORKSPACE_LABEL, + sandboxId: OPENSHELL_SANDBOX_ID_LABEL, + }); +} + +/** Read the host observation consumed by the pure identity classifier. */ +export function observeDestroyContainerIdentity( + sandboxName: string, +): DockerSandboxIdentityObservation { + return observeDockerSandboxIdentities(sandboxName); +} + +/** + * Classify every Docker container carrying `openshell.ai/sandbox-name=`. + * The query intentionally does not filter by managed-by so a foreign container + * borrowing the mutable name remains visible and makes destroy fail closed. + */ +export function classifyDestroyContainerIdentity( + sandboxName: string, + observation: DockerSandboxIdentityObservation, +): DestroyContainerIdentityVerdict { + if (observation.status === "probe-failed") { + return { + status: "probe-failed", + detail: + sanitizeReadinessText(observation.detail, IDENTITY_DIAGNOSTIC_MAX_LENGTH) || + "docker ps did not complete successfully", + }; + } + + const { malformedRows, rows } = observation; + const managed = rows.filter((row) => row.managedBy === OPENSHELL_MANAGED_BY_VALUE); + const foreign = rows.filter((row) => row.managedBy !== OPENSHELL_MANAGED_BY_VALUE); + + if (malformedRows > 0) { + return { + status: "ambiguous", + sandboxName, + reason: `Docker returned ${String(malformedRows)} malformed container identity row(s)`, + foreign, + managed, + }; + } + if (rows.length === 0) return { status: "clear", identity: null }; + if (foreign.length > 0) { + return { + status: "ambiguous", + sandboxName, + reason: + `${String(foreign.length)} container(s) carry the '${OPENSHELL_SANDBOX_NAME_LABEL}=` + + `${sandboxName}' label without the '${OPENSHELL_MANAGED_BY_LABEL}=` + + `${OPENSHELL_MANAGED_BY_VALUE}' marker`, + foreign, + managed, + }; + } + if (managed.length !== 1) { + return { + status: "ambiguous", + sandboxName, + reason: + `${String(managed.length)} managed containers carry the ` + + `'${OPENSHELL_SANDBOX_NAME_LABEL}=${sandboxName}' label; expected exactly one`, + foreign, + managed, + }; + } + + const [identity] = managed; + if (!identity.workspace || !identity.sandboxId) { + const missingLabels = [ + identity.workspace ? null : OPENSHELL_SANDBOX_WORKSPACE_LABEL, + identity.sandboxId ? null : OPENSHELL_SANDBOX_ID_LABEL, + ].filter((label): label is string => label !== null); + return { + status: "ambiguous", + sandboxName, + reason: `the managed container is missing ${missingLabels.join(" and ")}`, + foreign, + managed, + }; + } + return { status: "clear", identity }; +} + +/** Require the same immutable container row, including the already-absent state. */ +export function isSameDestroyContainerIdentity( + expected: SandboxNameLabeledContainer | null, + verdict: DestroyContainerIdentityVerdict, +): boolean { + if (verdict.status !== "clear") return false; + if (expected === null || verdict.identity === null) return expected === verdict.identity; + return ( + expected.id === verdict.identity.id && + expected.managedBy === verdict.identity.managedBy && + expected.workspace === verdict.identity.workspace && + expected.sandboxId === verdict.identity.sandboxId + ); +} + +/** Human-readable lines describing an ambiguous-identity refusal. */ +export function formatAmbiguousDestroyIdentity( + verdict: Extract, + cliName: string, +): string[] { + const display = (value: string, fallback = ""): string => + sanitizeReadinessText(value || fallback, IDENTITY_VALUE_MAX_LENGTH); + const displayLabel = (value: string): string => JSON.stringify(display(value)); + const describe = (row: SandboxNameLabeledContainer): string => + `${display(row.id).slice(0, 12)} (${OPENSHELL_MANAGED_BY_LABEL}=${displayLabel(row.managedBy)}, ` + + `${OPENSHELL_SANDBOX_WORKSPACE_LABEL}=${displayLabel(row.workspace)}, ` + + `${OPENSHELL_SANDBOX_ID_LABEL}=${displayLabel(row.sandboxId)})`; + const sandboxName = display(verdict.sandboxName); + const lines = [ + `Refusing to destroy sandbox '${sandboxName}': ${sanitizeReadinessText(verdict.reason, IDENTITY_DIAGNOSTIC_MAX_LENGTH)}.`, + "NemoClaw could not verify one complete container identity for this sandbox name, so destroy fails closed.", + ]; + for (const row of verdict.foreign) { + lines.push(` Conflicting container: ${describe(row)}`); + } + for (const row of verdict.managed) { + lines.push(` Managed sandbox container: ${describe(row)}`); + } + lines.push( + "Inspect containers with the sandbox-name label. Resolve the conflict through the workflow " + + `that owns the container, then rerun '${display(cliName)} ${sandboxName} destroy'.`, + ); + return lines; +} + +/** + * Fail closed when a Docker sandbox name does not resolve to one complete + * container identity. Other runtime providers own their identity checks. + */ +export function assertUnambiguousDestroyContainerIdentity( + sandboxName: string, + deps: AssertUnambiguousDestroyIdentityDeps, +): DestroyContainerIdentityProof | false { + const classify = + deps.classify ?? + ((name: string) => + classifyDestroyContainerIdentity(name, observeDestroyContainerIdentity(name))); + const error = deps.error ?? ((message: string) => console.error(` ${message}`)); + if (deps.providerId !== "docker") return { identity: undefined }; + + const verdict = classify(sandboxName); + if (verdict.status === "ambiguous") { + for (const line of formatAmbiguousDestroyIdentity(verdict, deps.cliName ?? "nemoclaw")) { + error(line); + } + return false; + } + if (verdict.status === "probe-failed") { + error( + `Refusing to destroy sandbox '${sandboxName}': Docker container identity could not be ` + + `inspected (${deps.redact(verdict.detail)}). No sandbox resources were removed. ` + + "Correct the reported Docker error, then rerun the destroy command.", + ); + return false; + } + return { identity: verdict.identity }; +} + +/** Compare provider-owned identity proofs across two destroy checkpoints. */ +export function isSameDestroyContainerIdentityProof( + expected: DestroyContainerIdentityProof, + actual: DestroyContainerIdentityProof, +): boolean { + if (expected.identity === undefined || actual.identity === undefined) { + return expected.identity === actual.identity; + } + return isSameDestroyContainerIdentity(expected.identity, { + status: "clear", + identity: actual.identity, + }); +} + export type DestroySandboxPresence = "present" | "absent" | "unknown"; function isStrictSandboxListJsonRow(value: unknown): value is { name: string } { diff --git a/src/lib/actions/sandbox/destroy.test.ts b/src/lib/actions/sandbox/destroy.test.ts index db2f370c534..f621f220f87 100644 --- a/src/lib/actions/sandbox/destroy.test.ts +++ b/src/lib/actions/sandbox/destroy.test.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; -import { cleanupSandboxServices } from "./destroy"; +import { assertUnambiguousDestroyContainerIdentity, cleanupSandboxServices } from "./destroy"; const SANDBOX = "mybox"; const mainPidDir = path.resolve("/tmp", `nemoclaw-services-${SANDBOX}`); @@ -70,3 +70,82 @@ describe("cleanupSandboxServices Google Chat tunnel cleanup (#7317)", () => { expect(rmSync).toHaveBeenCalledWith(googlechatPidDir, { recursive: true, force: true }); }); }); + +describe("assertUnambiguousDestroyContainerIdentity (#8999)", () => { + const dockerSandbox = { openshellDriver: "docker" } as { openshellDriver: string | null }; + + it("refuses destroy when a foreign container shares the sandbox-name label", () => { + const error = vi.fn(); + const classify = vi.fn(() => ({ + status: "ambiguous" as const, + sandboxName: "destroytest", + reason: "a foreign container carries the label", + foreign: [{ id: "ffff", managedBy: "", workspace: "foreign", sandboxId: "" }], + managed: [{ id: "aaaa", managedBy: "openshell", workspace: "default", sandboxId: "sb" }], + })); + + const proceed = assertUnambiguousDestroyContainerIdentity("destroytest", { + providerId: dockerSandbox.openshellDriver ?? "docker", + redact: String, + classify: classify as never, + error, + }); + + expect(proceed).toBe(false); + expect(classify).toHaveBeenCalledWith("destroytest"); + expect(error).toHaveBeenCalled(); + }); + + it("proceeds for a clear single managed identity", () => { + const identity = { + id: "aaaa000000000000", + managedBy: "openshell", + workspace: "default", + sandboxId: "sb", + }; + const classify = vi.fn(() => ({ status: "clear" as const, identity })); + expect( + assertUnambiguousDestroyContainerIdentity("destroytest", { + providerId: dockerSandbox.openshellDriver ?? "docker", + redact: String, + classify: classify as never, + }), + ).toEqual({ identity }); + }); + + it("does not probe or block a non-Docker runtime provider", () => { + const classify = vi.fn(); + const proceed = assertUnambiguousDestroyContainerIdentity("destroytest", { + providerId: "podman", + redact: String, + classify: classify as never, + }); + expect(proceed).toEqual({ identity: undefined }); + expect(classify).not.toHaveBeenCalled(); + }); + + it("refuses when the Docker probe cannot prove identity", () => { + const error = vi.fn(); + const proceed = assertUnambiguousDestroyContainerIdentity("destroytest", { + providerId: dockerSandbox.openshellDriver ?? "docker", + redact: String, + classify: vi.fn(() => ({ status: "probe-failed" as const, detail: "daemon down" })) as never, + error, + }); + expect(proceed).toBe(false); + expect(error).toHaveBeenCalledWith(expect.stringContaining("daemon down")); + expect(error).toHaveBeenCalledWith( + expect.stringContaining("No sandbox resources were removed"), + ); + }); + + it("treats an unknown/null driver as Docker (the default) and still guards", () => { + const classify = vi.fn(() => ({ status: "clear" as const, identity: null })); + assertUnambiguousDestroyContainerIdentity("destroytest", { + providerId: "docker", + redact: String, + classify: classify as never, + }); + expect(classify).toHaveBeenCalledWith("destroytest"); + }); +}); diff --git a/src/lib/actions/sandbox/destroy.ts b/src/lib/actions/sandbox/destroy.ts index 0f4c55dea67..710dc66988b 100644 --- a/src/lib/actions/sandbox/destroy.ts +++ b/src/lib/actions/sandbox/destroy.ts @@ -48,10 +48,15 @@ import { } from "./destroy-execution"; import { cleanupGatewayAfterLastSandbox } from "./destroy-gateway"; import { shouldCleanupGatewayAfterConfirmedFinalDestroy } from "./destroy-gateway-cleanup"; -import { prepareSandboxDestroy } from "./destroy-preflight"; +import { + assertUnambiguousDestroyContainerIdentity, + classifyDestroySandboxPresence, + isSameDestroyContainerIdentityProof, +} from "./destroy-presence"; +import { prepareSandboxDestroy, stopSandboxInferenceResources } from "./destroy-preflight"; import { type WipeSandboxStateDeps, wipeSandboxState } from "./wipe-state"; -export { classifyDestroySandboxPresence } from "./destroy-presence"; +export { assertUnambiguousDestroyContainerIdentity, classifyDestroySandboxPresence }; type RemoveSandboxImageDeps = { getSandbox?: typeof registry.getSandbox; @@ -465,8 +470,34 @@ async function destroySandboxUnlocked( const normalized = normalizeDestroySandboxOptions(options); if (!(await confirmSandboxDestroy(sandboxName, normalized))) return; + const inspectContainerIdentity = () => + assertUnambiguousDestroyContainerIdentity(sandboxName, { + cliName: CLI_NAME, + providerId: normalizeRuntimeProviderIdentity( + registry.getSandbox(sandboxName)?.openshellDriver, + ), + redact: redactDestroyError, + }); + const initialIdentity = inspectContainerIdentity(); + if (initialIdentity === false) { + process.exit(1); + } + const { cleanupGatewayName, runOpenshell, sandbox, sandboxConfirmedAbsent } = prepareSandboxDestroy(sandboxName); + // Recheck identity after read-only preflight and before local mutation. + const preMutationIdentity = inspectContainerIdentity(); + if ( + preMutationIdentity === false || + !isSameDestroyContainerIdentityProof(initialIdentity, preMutationIdentity) + ) { + if (preMutationIdentity !== false) { + console.error( + ` Refusing to destroy sandbox '${sandboxName}': Container identity changed during preflight. No sandbox resources were removed.`, + ); + } + process.exit(1); + } const priorHttpsPinRouteId = parseHttpsPinRouteId(sandbox?.endpointUrl); const destructiveResult = await executeSandboxDestroy({ cleanupShieldsArtifacts: cleanupShieldsDestroyArtifacts, @@ -475,6 +506,8 @@ async function destroySandboxUnlocked( sandbox, sandboxConfirmedAbsent, sandboxName, + expectedContainerIdentity: initialIdentity.identity, + stopInferenceResources: () => stopSandboxInferenceResources(sandboxName, sandbox), }); if (!destructiveResult.ok) { if (destructiveResult.deleteOutput) { diff --git a/src/lib/adapters/docker/inspect-identity.test.ts b/src/lib/adapters/docker/inspect-identity.test.ts new file mode 100644 index 00000000000..7eff8c7afb8 --- /dev/null +++ b/src/lib/adapters/docker/inspect-identity.test.ts @@ -0,0 +1,60 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it, vi } from "vitest"; + +import { inspectDockerSandboxIdentities } from "./inspect"; + +const LABELS = { + managedBy: "openshell.ai/managed-by", + workspace: "openshell.ai/sandbox-workspace", + sandboxId: "openshell.ai/sandbox-id", +}; + +describe("inspectDockerSandboxIdentities", () => { + it("queries only the sandbox-name label and parses one exact row", () => { + const inspect = vi.fn((_args: readonly string[]) => ({ + status: 0, + stdout: "aaaa000000000000\topenshell\tdefault\tsb-real", + })); + + expect( + inspectDockerSandboxIdentities("openshell.ai/sandbox-name=alpha", LABELS, inspect), + ).toEqual({ + status: "observed", + malformedRows: 0, + rows: [ + { + id: "aaaa000000000000", + managedBy: "openshell", + workspace: "default", + sandboxId: "sb-real", + }, + ], + }); + expect(inspect.mock.calls[0][0]).toContain("-a"); + expect(inspect.mock.calls[0][0]).toContain("label=openshell.ai/sandbox-name=alpha"); + expect(inspect.mock.calls[0][0]).not.toContain("label=openshell.ai/managed-by=openshell"); + }); + + it.each([ + ["trailing vertical tab", "openshell\u000b"], + ["leading whitespace", " openshell"], + ])("rejects %s instead of normalizing a trusted label", (_label, managedBy) => { + expect( + inspectDockerSandboxIdentities("openshell.ai/sandbox-name=alpha", LABELS, () => ({ + status: 0, + stdout: `aaaa000000000000\t${managedBy}\tdefault\tsb-real`, + })), + ).toEqual({ status: "observed", rows: [], malformedRows: 1 }); + }); + + it("preserves probe diagnostics for caller-side sanitization", () => { + expect( + inspectDockerSandboxIdentities("openshell.ai/sandbox-name=alpha", LABELS, () => ({ + status: 1, + stderr: "daemon unavailable", + })), + ).toEqual({ status: "probe-failed", detail: "daemon unavailable" }); + }); +}); diff --git a/src/lib/adapters/docker/inspect.ts b/src/lib/adapters/docker/inspect.ts index b542118be95..437af7ce009 100644 --- a/src/lib/adapters/docker/inspect.ts +++ b/src/lib/adapters/docker/inspect.ts @@ -3,6 +3,86 @@ import { type DockerCaptureOptions, type DockerRunOptions, dockerCapture, dockerRun } from "./run"; +export type DockerSandboxIdentityRow = { + id: string; + managedBy: string; + workspace: string; + sandboxId: string; +}; + +export type DockerSandboxIdentityObservation = + | { status: "observed"; rows: DockerSandboxIdentityRow[]; malformedRows: number } + | { status: "probe-failed"; detail: string }; + +type DockerSandboxIdentityInspect = ( + args: readonly string[], + opts?: DockerRunOptions, +) => Partial, "error" | "status" | "stderr" | "stdout">>; + +const DOCKER_IDENTITY_PROBE_TIMEOUT_MS = 30_000; +const DOCKER_IDENTITY_PROBE_MAX_BUFFER_BYTES = 256 * 1024; +const IDENTITY_VALUE_MAX_LENGTH = 256; +const DOCKER_CONTAINER_ID_PATTERN = /^[0-9a-f]{12,64}$/iu; +const UNSAFE_IDENTITY_TEXT_PATTERN = + /[\u0000-\u001f\u007f-\u009f\u061c\u200e\u200f\u2028-\u202e\u2066-\u2069]/u; + +function isExactBoundedIdentityText(value: string): boolean { + return ( + value === value.trim() && + value.length <= IDENTITY_VALUE_MAX_LENGTH && + !UNSAFE_IDENTITY_TEXT_PATTERN.test(value) + ); +} + +/** Inspect and parse the exact Docker rows used by sandbox destroy. */ +export function inspectDockerSandboxIdentities( + sandboxNameLabel: string, + labelKeys: { managedBy: string; workspace: string; sandboxId: string }, + inspect: DockerSandboxIdentityInspect = dockerRun, +): DockerSandboxIdentityObservation { + const format = [ + "{{.ID}}", + `{{.Label "${labelKeys.managedBy}"}}`, + `{{.Label "${labelKeys.workspace}"}}`, + `{{.Label "${labelKeys.sandboxId}"}}`, + ].join("\t"); + const result = inspect( + ["ps", "-a", "--no-trunc", "--filter", `label=${sandboxNameLabel}`, "--format", format], + { + ignoreError: true, + maxBuffer: DOCKER_IDENTITY_PROBE_MAX_BUFFER_BYTES, + suppressOutput: true, + timeout: DOCKER_IDENTITY_PROBE_TIMEOUT_MS, + }, + ); + if (Number(result.status ?? 1) !== 0) { + const detail = [result.error?.message, result.stderr, result.stdout] + .filter((value) => value !== undefined && value !== null && String(value).length > 0) + .map(String) + .join(" ") + .trim(); + return { status: "probe-failed", detail }; + } + + const rows: DockerSandboxIdentityRow[] = []; + let malformedRows = 0; + for (const rawLine of String(result.stdout ?? "").split(/\r?\n/u)) { + if (rawLine.length === 0) continue; + const fields = rawLine.split("\t"); + if ( + fields.length !== 4 || + !DOCKER_CONTAINER_ID_PATTERN.test(fields[0] ?? "") || + !fields.every(isExactBoundedIdentityText) + ) { + malformedRows += 1; + continue; + } + const [id, managedBy, workspace, sandboxId] = fields as [string, string, string, string]; + rows.push({ id, managedBy, workspace, sandboxId }); + } + return { status: "observed", rows, malformedRows }; +} + export function dockerInspect(args: readonly string[], opts: DockerRunOptions = {}) { return dockerRun(["inspect", ...args], opts); } diff --git a/src/lib/onboard/runtime-provider/runtime-provider-contract.test.ts b/src/lib/onboard/runtime-provider/runtime-provider-contract.test.ts index 92f92e1ac89..8f05ecf3aff 100644 --- a/src/lib/onboard/runtime-provider/runtime-provider-contract.test.ts +++ b/src/lib/onboard/runtime-provider/runtime-provider-contract.test.ts @@ -993,6 +993,7 @@ describe("socket-free MXC action contract", () => { sandbox: entry, sandboxConfirmedAbsent: false, sandboxName, + stopInferenceResources: vi.fn(), runtimeProviders: providers, deps: { readTimerMarker: () => null, diff --git a/test/helpers/destroy-flow-test-harness.ts b/test/helpers/destroy-flow-test-harness.ts index cd7b37f6aa7..7ca7a023302 100644 --- a/test/helpers/destroy-flow-test-harness.ts +++ b/test/helpers/destroy-flow-test-harness.ts @@ -6,7 +6,7 @@ import { createRequire } from "node:module"; import { expect, type MockInstance, vi } from "vitest"; import type { SandboxWorkloadReceipt } from "../../src/lib/state/registry"; -type DestroySandbox = typeof import("../../src/lib/actions/sandbox/destroy")["destroySandbox"]; +type DestroySandbox = (typeof import("../../src/lib/actions/sandbox/destroy"))["destroySandbox"]; const requireDist = createRequire( new URL("../../src/lib/actions/sandbox/destroy-flow.test.ts", import.meta.url), @@ -51,6 +51,14 @@ type DestroyHarnessOptions = { deleteOutput?: string; deleteStatus?: number; dockerPsOutput?: string; + dockerRunResult?: { status: number | null; stdout?: string; stderr?: string }; + dockerRunResultSequence?: Array<{ + status: number | null; + stdout?: string; + stderr?: string; + }>; + onDockerRun?: (call: number) => void; + detachedProviders?: string[]; endpointUrl?: string; finalizeMcpBridgeError?: string; finalizeMcpError?: string; @@ -66,6 +74,7 @@ type DestroyHarnessOptions = { sandboxPresent?: boolean; shieldsDown?: boolean; shieldsUpError?: Error; + stopInferenceError?: string; workload?: SandboxWorkloadReceipt; }; @@ -111,6 +120,24 @@ export function loadDestroySandboxPresenceClassifier(): DestroySandboxPresenceCl return destroyModule.classifyDestroySandboxPresence; } +export function traceDestroyBoundaryCalls( + harness: Pick, + trace: string[], +): void { + harness.runOpenshellSpy.mockImplementation((args: unknown) => { + const argv = Array.isArray(args) ? args : []; + switch (`${String(argv[0])}:${String(argv[1])}`) { + case "sandbox:delete": + trace.push("delete"); + return { status: 0, stdout: "", stderr: "" }; + case "sandbox:list": + return { status: 0, stdout: "[]", stderr: "" }; + default: + return { status: 0, stdout: "", stderr: "" }; + } + }); +} + export function createDestroyHarness(options: DestroyHarnessOptions = {}): DestroyHarness { resetDestroyModuleCache(); const events: string[] = []; @@ -240,9 +267,27 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr : names; return matchedNames.length > 0 ? `${matchedNames.join("\n")}\n` : ""; }); - const dockerRunSpy = vi - .spyOn(dockerRun, "dockerRun") - .mockReturnValue({ status: 0 } as ReturnType); + let identityProbeCall = 0; + const dockerRunSpy = vi.spyOn(dockerRun, "dockerRun").mockImplementation((args: unknown) => { + const argv = Array.isArray(args) ? args.map(String) : []; + const filterIndex = argv.indexOf("--filter"); + const isIdentityProbe = + argv[0] === "ps" && + argv.includes("-a") && + argv.includes("--no-trunc") && + filterIndex >= 0 && + argv[filterIndex + 1]?.startsWith("label=openshell.ai/sandbox-name=") === true; + if (!isIdentityProbe) { + return (options.dockerRunResult ?? { status: 0 }) as ReturnType; + } + identityProbeCall += 1; + options.onDockerRun?.(identityProbeCall); + const result = + options.dockerRunResultSequence?.[identityProbeCall - 1] ?? + options.dockerRunResult ?? + { status: 0 }; + return result as ReturnType; + }); const selectGatewaySpy = vi .spyOn(destroyGateway, "selectGatewayForSandboxDestroy") .mockImplementation(() => undefined); @@ -251,14 +296,18 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr .mockImplementation(() => undefined); vi.spyOn(sandboxProviderCleanup, "runSandboxProviderPreDeleteCleanup").mockImplementation(() => { events.push("detach"); - return { failures: [] }; + return { detached: options.detachedProviders ?? [], failures: [] }; }); vi.spyOn(sandboxProviderCleanup, "emitProviderDetachResidualHint").mockImplementation( () => undefined, ); const stopNimByNameSpy = vi .spyOn(nim, "stopNimContainerByName") - .mockImplementation(() => undefined); + .mockImplementation(() => { + if (options.stopInferenceError !== undefined) { + throw new Error(options.stopInferenceError); + } + }); vi.spyOn(nim, "stopNimContainer").mockImplementation(() => undefined); const killStaleProxySpy = vi .spyOn(ollamaProxy, "killStaleProxy") @@ -308,6 +357,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr const prepareMcpBridgesForDestroySpy = vi .spyOn(mcpBridge, "prepareMcpBridgesForDestroy") .mockImplementation(async () => { + events.push("mcp-prepare"); if (options.prepareMcpBridgeError !== undefined) { throw new McpBridgeError(options.prepareMcpBridgeError); }