diff --git a/src/lib/actions/sandbox/destroy-execution.ts b/src/lib/actions/sandbox/destroy-execution.ts index b9701599fe6..54d92a0b54c 100644 --- a/src/lib/actions/sandbox/destroy-execution.ts +++ b/src/lib/actions/sandbox/destroy-execution.ts @@ -74,6 +74,7 @@ type SandboxDestroyExecutionInput = { expectedContainerIdentity?: SandboxNameLabeledContainer | null; portableContainerAuthority?: PreparedPortableDemoSandboxDestroyAuthority; stopInferenceResources: () => void; + validateMcpPolicyAuthorityReceipt?: () => Promise; runtimeProviders?: RuntimeProviderBundleRegistry; deps?: { hostLocalInferenceLifecycleOptions?: HostLocalInferenceLifecycleOptions; @@ -131,13 +132,16 @@ async function prepareMcpDestroy( sandbox: SandboxEntry | null, sandboxConfirmedAbsent: boolean, force: boolean, + validateContainingPolicyReceipt?: () => Promise, ): Promise { if (Object.keys(sandbox?.mcp?.bridges ?? {}).length === 0) { return emptyMcpDestroyPreparation(); } const preparation = sandboxConfirmedAbsent ? await prepareMcpBridgesForAbsentSandboxDestroy(sandboxName, { force }) - : await prepareMcpBridgesForDestroy(sandboxName); + : validateContainingPolicyReceipt + ? await prepareMcpBridgesForDestroy(sandboxName, validateContainingPolicyReceipt) + : await prepareMcpBridgesForDestroy(sandboxName); if (sandboxConfirmedAbsent && preparation.entries.length > 0) { console.warn( ` ${YW}⚠${R} Sandbox '${sandboxName}' is already absent, so its retained-volume MCP adapter entry cannot be scrubbed in place. Exact OpenShell providers will be deleted so any stale credential placeholder cannot authenticate; same-name onboarding may need to replace stale MCP adapter config.`, @@ -225,6 +229,7 @@ async function restoreMcpAfterDeleteAbort( sandboxName: string, preparation: McpDestroyPreparation, hardened: HardenedDeleteState, + validateContainingPolicyReceipt?: () => Promise, ): Promise { let recoveryFailure: string | undefined; let openedRollbackWindow = false; @@ -246,7 +251,13 @@ async function restoreMcpAfterDeleteAbort( }); openedRollbackWindow = true; } - await restoreMcpBridgesAfterDestroyAbort(sandboxName, preparation); + await (validateContainingPolicyReceipt + ? restoreMcpBridgesAfterDestroyAbort( + sandboxName, + preparation, + validateContainingPolicyReceipt, + ) + : restoreMcpBridgesAfterDestroyAbort(sandboxName, preparation)); } catch (error) { recoveryFailure = redactDestroyError(error); } finally { @@ -287,6 +298,15 @@ async function finalizeMcpDestroy( } } +async function readMcpPolicyRefusal(revalidate?: () => Promise): Promise { + try { + await revalidate?.(); + return undefined; + } catch (error) { + return redactDestroyError(error); + } +} + export async function executeSandboxDestroy({ cleanupShieldsArtifacts, force, @@ -299,6 +319,7 @@ export async function executeSandboxDestroy({ expectedContainerIdentity, portableContainerAuthority, stopInferenceResources, + validateMcpPolicyAuthorityReceipt, runtimeProviders = CURRENT_RUNTIME_PROVIDER_BUNDLES, deps = {}, }: SandboxDestroyExecutionInput): Promise { @@ -440,7 +461,13 @@ export async function executeSandboxDestroy({ } let mcpPreparation: McpDestroyPreparation; try { - mcpPreparation = await prepareMcpDestroy(sandboxName, sandbox, sandboxConfirmedAbsent, force); + mcpPreparation = await prepareMcpDestroy( + sandboxName, + sandbox, + sandboxConfirmedAbsent, + force, + validateMcpPolicyAuthorityReceipt, + ); } catch (error) { if (error instanceof McpBridgeError) { return { @@ -468,7 +495,12 @@ export async function executeSandboxDestroy({ ): Promise => sandboxConfirmedAbsent ? undefined - : await restoreMcpAfterDeleteAbort(sandboxName, mcpPreparation, hardenedState); + : await restoreMcpAfterDeleteAbort( + sandboxName, + mcpPreparation, + hardenedState, + validateMcpPolicyAuthorityReceipt, + ); const preparedContinuity = inspectIdentityContinuity(); if (preparedContinuity.status !== "match") { const mcpRecoveryFailure = await restoreMcpForAbort(notHardened); @@ -566,6 +598,23 @@ export async function executeSandboxDestroy({ ` Managed inference cleanup and workspace wipe or hardening may already have run; inspect those resources before retrying.${detachedDetail}`, ); } + try { + await mcpPreparation.revalidateBeforeDelete?.(); + } catch (error) { + const mcpRecoveryFailure = await restoreMcpForAbort(hardened); + return { + ok: false as const, + deleteOutput: + `MCP policy authority changed at the sandbox delete boundary: ${redactDestroyError(error)}. ` + + "No sandbox delete was attempted.", + exitCode: error instanceof McpBridgeError ? error.exitCode : 1, + gatewayUnreachable: false, + hostLocalInferenceOwnershipRequiresGateway: false, + mcpOwnershipRequiresGateway: false, + mcpRecoveryFailure, + shieldsRelockRequiresGateway: false, + }; + } const deleteArgs = pendingPolicyVerification ? ["sandbox", "delete", "-g", pendingPolicyVerification.gatewayName, sandboxName] : ["sandbox", "delete", sandboxName]; @@ -603,12 +652,25 @@ export async function executeSandboxDestroy({ !hardened.hardeningFailed; if (deleteResult.status !== 0 && !alreadyGone && !forcedLocalCleanup) { + let policyRefusal: string | undefined; + try { + await mcpPreparation.revalidateBeforeDelete?.(); + } catch (error) { + policyRefusal = redactDestroyError(error); + } const mcpRecoveryFailure = sandboxConfirmedAbsent ? undefined - : await restoreMcpAfterDeleteAbort(sandboxName, mcpPreparation, hardened); + : await restoreMcpAfterDeleteAbort( + sandboxName, + mcpPreparation, + hardened, + validateMcpPolicyAuthorityReceipt, + ); return { ok: false as const, - deleteOutput, + deleteOutput: policyRefusal + ? `${deleteOutput}\nMCP policy authority revalidation also refused cleanup: ${policyRefusal}` + : deleteOutput, exitCode: deleteResult.status || 1, gatewayUnreachable, ...(timedOut ? { timedOut: true as const } : {}), @@ -622,6 +684,10 @@ export async function executeSandboxDestroy({ }; } + let finalPolicyRefusal = forcedLocalCleanup + ? undefined + : await readMcpPolicyRefusal(mcpPreparation.revalidateAfterDelete); + if (!forcedLocalCleanup && (portableContainerAuthority || expectedContainerIdentity)) { try { if (portableContainerAuthority) { @@ -702,6 +768,26 @@ export async function executeSandboxDestroy({ }; } } + if (!forcedLocalCleanup) { + const successEdgePolicyRefusal = await readMcpPolicyRefusal( + mcpPreparation.revalidateBeforeSuccess, + ); + finalPolicyRefusal ??= successEdgePolicyRefusal; + } + if (finalPolicyRefusal) { + return { + ok: false as const, + deleteOutput: + `OpenShell reported sandbox '${sandboxName}' absent, but final MCP policy authority ` + + `revalidation refused success publication: ${finalPolicyRefusal}`, + exitCode: 1, + gatewayUnreachable: false, + hostLocalInferenceOwnershipRequiresGateway: false, + mcpOwnershipRequiresGateway: false, + shieldsRelockRequiresGateway: false, + deleteConfirmed: true, + }; + } return { ok: true as const, detachOutcome, diff --git a/src/lib/actions/sandbox/destroy-flow.test.ts b/src/lib/actions/sandbox/destroy-flow.test.ts index 1ba9f6168f4..a17a183c860 100644 --- a/src/lib/actions/sandbox/destroy-flow.test.ts +++ b/src/lib/actions/sandbox/destroy-flow.test.ts @@ -1086,7 +1086,10 @@ describe("destroySandbox flow", () => { await expect(harness.destroySandbox("alpha", { yes: true })).resolves.toBeUndefined(); - expect(harness.prepareMcpBridgesForDestroySpy).toHaveBeenCalledWith("alpha"); + expect(harness.prepareMcpBridgesForDestroySpy).toHaveBeenCalledWith( + "alpha", + expect.any(Function), + ); }); it("does not require mutable Hermes config for absent-sandbox cleanup", async () => { @@ -1325,6 +1328,70 @@ describe("destroySandbox flow", () => { expectMcpFinalizeAfterDelete(harness); }); + it("restores MCP state and withholds delete when policy authority drifts after preparation (#9833)", async () => { + const harness = createDestroyHarness({ + mcpServers: ["github"], + revalidateMcpPolicyAuthority: async () => { + throw new Error("current MCP policy requirements changed"); + }, + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.events).toContain("mcp-revalidate"); + expect(harness.events).toContain("mcp-restore"); + expect(harness.events).not.toContain("delete"); + expect(harness.finalizeMcpBridgesAfterSandboxDeleteSpy).not.toHaveBeenCalled(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.errorSpy.mock.calls.map(([message]) => String(message)).join("\n")).toContain( + "current MCP policy requirements changed", + ); + }); + + it("finishes exact MCP cleanup but withholds success after a final authority refusal (#9833)", async () => { + const harness = createDestroyHarness({ + mcpPolicyAuthorityAfterDeleteError: "first retained authority refusal", + mcpServers: ["github"], + policyAuthority: "externally-managed", + policyAuthorityDuringMcpFinalization: "nemoclaw-managed", + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.events).toContain("delete"); + expect(harness.finalizeMcpBridgesAfterSandboxDeleteSpy).toHaveBeenCalledOnce(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.logSpy.mock.calls.map(([message]) => String(message)).join("\n")).not.toContain( + "Sandbox destroyed", + ); + expect(harness.errorSpy.mock.calls.map(([message]) => String(message)).join("\n")).toContain( + "first retained authority refusal", + ); + expect( + harness.errorSpy.mock.calls.map(([message]) => String(message)).join("\n"), + ).not.toContain("policy authority changed during destroy"); + }); + + it("withholds success when policy authority drifts during MCP finalization (#9833)", async () => { + const harness = createDestroyHarness({ + mcpServers: ["github"], + policyAuthority: "externally-managed", + policyAuthorityDuringMcpFinalization: "nemoclaw-managed", + }); + + await expect(harness.destroySandbox("alpha", { yes: true })).rejects.toThrow("process.exit(1)"); + + expect(harness.events).toContain("delete"); + expect(harness.finalizeMcpBridgesAfterSandboxDeleteSpy).toHaveBeenCalledOnce(); + expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); + expect(harness.logSpy.mock.calls.map(([message]) => String(message)).join("\n")).not.toContain( + "Sandbox destroyed", + ); + expect(harness.errorSpy.mock.calls.map(([message]) => String(message)).join("\n")).toContain( + "policy authority changed during destroy", + ); + }); + it("restores MCP runtime state when sandbox delete fails", async () => { const harness = createDestroyHarness({ activeTimer: true, diff --git a/src/lib/actions/sandbox/destroy.ts b/src/lib/actions/sandbox/destroy.ts index 0748dea7987..c22f8a6a1b8 100644 --- a/src/lib/actions/sandbox/destroy.ts +++ b/src/lib/actions/sandbox/destroy.ts @@ -486,6 +486,24 @@ function requestSandboxDestroyExit(exitCode: number): never { throw new SandboxDestroyExitRequest(exitCode); } +function bindMcpDestroyPolicyAuthority( + sandboxName: string, + sandbox: registry.SandboxEntry | null, +): (() => Promise) | undefined { + if (!sandbox || Object.keys(sandbox.mcp?.bridges ?? {}).length === 0) return undefined; + let authority = sandbox.policyAuthority; + return async () => { + const current = registry.getSandbox(sandboxName); + if (!current) { + throw new Error(`sandbox '${sandboxName}' is no longer registered`); + } + if (authority === undefined) authority = current.policyAuthority; + else if (current.policyAuthority !== authority) { + throw new Error(`sandbox '${sandboxName}' policy authority changed during destroy`); + } + }; +} + export async function destroySandbox( sandboxName: string, options: string[] | DestroySandboxOptions = {}, @@ -594,6 +612,10 @@ async function destroySandboxUnlocked( let destroyPreflight: ReturnType; destroyPreflight = abortPreparedCleanupOnError(() => prepareSandboxDestroy(sandboxName)); const { cleanupGatewayName, runOpenshell, sandbox, sandboxConfirmedAbsent } = destroyPreflight; + const validateMcpPolicyAuthorityReceipt = bindMcpDestroyPolicyAuthority( + sandboxName, + sandboxConfirmedAbsent ? null : sandbox, + ); // Recheck identity after pre-delete qualification and recoverable journal // publication reconciliation, before any sandbox runtime mutation. if (portableContainerAuthority) { @@ -637,6 +659,7 @@ async function destroySandboxUnlocked( expectedContainerIdentity: initialIdentity?.identity, ...(portableContainerAuthority ? { portableContainerAuthority } : {}), stopInferenceResources: () => stopSandboxInferenceResources(sandboxName, sandbox), + ...(validateMcpPolicyAuthorityReceipt ? { validateMcpPolicyAuthorityReceipt } : {}), }); } catch (error) { preparedManagedLlamaCppCleanup?.abort(); diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-teardown.test.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-teardown.test.ts index 5c861a226ea..0bd849589f0 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-teardown.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-teardown.test.ts @@ -15,7 +15,9 @@ const mocks = vi.hoisted(() => ({ getSandboxOrThrow: vi.fn(), inspectMcpProvider: vi.fn(), observeMcpCredentialRevision: vi.fn(), + preflightMcpEntryTargets: vi.fn(), removeGeneratedPolicy: vi.fn(), + revalidateContainingMcpPolicyAuthority: vi.fn(), registerAgentAdapterAtCurrentCredentialRevision: vi.fn(), restoreExistingMcpBridgeRuntime: vi.fn(), unregisterAgentAdapter: vi.fn(), @@ -42,21 +44,40 @@ vi.mock("./mcp-bridge-provider", () => ({ assertNoRegisteredProviderCredentialCollisions: vi.fn(), detachProvider: vi.fn(), inspectMcpProvider: mocks.inspectMcpProvider, - preflightMcpEntryTargets: vi.fn(), + preflightMcpEntryTargets: mocks.preflightMcpEntryTargets, waitForDetachedMcpCredential: vi.fn(), })); vi.mock("./mcp-bridge-destroy-preflight", () => ({ + assertMcpDestroySnapshotCurrent: vi.fn(), cloneMcpBridgeEntry: vi.fn((entry: McpBridgeEntry) => ({ ...entry, env: [...entry.env] })), discardSafeIncompleteMcpAdds: mocks.discardSafeIncompleteMcpAdds, inspectExactMcpDestroyProvider: vi.fn(), })); -vi.mock("./mcp-bridge-policy", () => ({ - assertGeneratedPolicyMutationSafe: vi.fn(), - assertGeneratedPolicyRegistrationMutationSafe: vi.fn(), - removeGeneratedPolicy: mocks.removeGeneratedPolicy, -})); +vi.mock("./mcp-bridge-policy", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + assertGeneratedPolicyMutationSafe: vi.fn(), + assertGeneratedPolicyRegistrationMutationSafe: vi.fn(), + buildRequiredMcpBridgePolicy: vi.fn(() => "required policy"), + qualifyMcpPolicyAuthorityReceipt: (options: object) => ({ + ...options, + authority: "nemoclaw-managed" as const, + }), + removeGeneratedPolicy: mocks.removeGeneratedPolicy, + revalidateContainingMcpPolicyAuthority: mocks.revalidateContainingMcpPolicyAuthority, + revalidateMcpPolicyAuthorityReceipt: async ( + _receipt: unknown, + validateContainingReceipt?: () => Promise, + assertCurrentState?: () => void, + ) => { + await validateContainingReceipt?.(); + assertCurrentState?.(); + }, + }; +}); vi.mock("./mcp-bridge-restart", () => ({ restoreExistingMcpBridgeRuntime: mocks.restoreExistingMcpBridgeRuntime, @@ -110,6 +131,10 @@ describe("MCP adapter teardown rollback", () => { mocks.getSandboxOrThrow.mockReset().mockReturnValue(sandbox); mocks.inspectMcpProvider.mockReset().mockReturnValue({ exists: false }); mocks.observeMcpCredentialRevision.mockReset().mockReturnValue("v12"); + mocks.preflightMcpEntryTargets.mockReset().mockResolvedValue( + new Map([[entry.server, { addresses: ["8.8.8.8"] }]]), + ); + mocks.revalidateContainingMcpPolicyAuthority.mockReset().mockResolvedValue(undefined); mocks.removeGeneratedPolicy.mockReset().mockImplementation(() => { throw new Error("forced lifecycle failure after adapter scrub"); }); @@ -134,18 +159,12 @@ describe("MCP adapter teardown rollback", () => { "forced lifecycle failure after adapter scrub", ); expect(mocks.unregisterAgentAdapter).toHaveBeenCalledOnce(); - expect(mocks.registerAgentAdapterAtCurrentCredentialRevision).toHaveBeenCalledWith( + expect(mocks.restoreExistingMcpBridgeRuntime).toHaveBeenCalledWith( "alpha", - "hermes-config", - expect.objectContaining({ ...entry, credentialRevision: "v12" }), - {}, - "v13", - { - replaceExisting: true, - teardownRollback: true, - }, + [expect.objectContaining({ ...entry, credentialRevision: "v12" })], + expect.objectContaining({ lifecyclePhase: "teardown-rollback" }), ); - expect(mocks.restoreExistingMcpBridgeRuntime).not.toHaveBeenCalled(); + expect(mocks.registerAgentAdapterAtCurrentCredentialRevision).not.toHaveBeenCalled(); }, ); diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-teardown.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-teardown.ts index c219ba7aa66..28c2fecb05c 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-teardown.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-teardown.ts @@ -57,14 +57,16 @@ export function scrubManagedMcpAdapterOrThrow( }; } -/** Restore scrubbed adapter entries without hiding failures from provider rollback. */ -export function rollbackScrubbedMcpAdapters( +/** Restore scrubbed adapters, but do not classify an authority refusal as a mutation failure. */ +export async function rollbackScrubbedMcpAdapters( sandboxName: string, sandbox: SandboxEntry, entries: readonly McpScrubbedAdapterEntry[], -): string[] { + validateBeforeMutation?: () => Promise, +): Promise { const failures: string[] = []; for (const entry of entries) { + await validateBeforeMutation?.(); let credentialRevision: McpAttachedCredentialRevision | undefined; try { const current = observeMcpCredentialRevision(sandboxName, entry); diff --git a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts index b5e189826a8..da82e673090 100644 --- a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts +++ b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts @@ -19,7 +19,7 @@ import { assertAgentMcpConfigMutationAllowed, assertAgentMcpMutationRuntimeCapability, inspectAgentAdapterRegistration, - registerAgentAdapter, + registerAgentAdapterAtCurrentCredentialRevision, unregisterAgentAdapter, } from "./mcp-bridge-adapters"; import { type McpBridgeAddOptions, McpBridgeError } from "./mcp-bridge-contracts"; @@ -29,6 +29,9 @@ import { buildMcpBridgePolicyKey, buildMcpBridgePolicyName, buildMcpBridgePolicyYaml, + buildRequiredMcpBridgePolicy, + McpPolicyAuthorityRefusalError, + preflightMcpPolicyAuthority, removeGeneratedPolicy, } from "./mcp-bridge-policy"; import { @@ -41,6 +44,7 @@ import { ensureMcpBridgeProviderProfile, inspectMcpProvider, type McpCredentialRevisionObservation, + type McpProviderInspection, observeMcpCredentialRevision, providerMatchesCredential, providerShapeDetail, @@ -96,6 +100,7 @@ function assertPreparedMcpAddResourcesAbsent( adapter: AgentMcpAdapter, entry: McpBridgeEntry, target: McpBridgeTargetValidation, + policyAuthority: "nemoclaw-managed" | "externally-managed" | "owner-unknown", ): void { const adapterInspection = inspectAgentAdapterRegistration(sandboxName, adapter, entry); if (adapterInspection.state !== "absent") { @@ -127,6 +132,7 @@ function assertPreparedMcpAddResourcesAbsent( `MCP add preflight for '${entry.server}' found an existing policy ownership record '${entry.policyName}'. The durable add manifest was preserved without claiming it.`, ); } + if (policyAuthority !== "nemoclaw-managed") return; const policyContent = buildMcpBridgePolicyYaml( entry.server, entry.url, @@ -249,15 +255,38 @@ async function addMcpBridgeUnlocked( 2, ); } - const providerName = - envNames.length > 0 - ? (existingEntry?.providerName ?? - buildMcpBridgeProviderName( - sandboxName, - options.server, - crypto.randomBytes(8).toString("hex"), - )) - : undefined; + let providerName = existingEntry?.providerName; + if (envNames.length > 0 && !providerName) { + let providerInstanceId: string; + if (sandbox.policyAuthority === "nemoclaw-managed") { + providerInstanceId = crypto.randomBytes(8).toString("hex"); + } else { + const lifecycleIdentity = sandbox.lifecycleGeneration ?? sandbox.createdAt; + if (!lifecycleIdentity) { + throw new McpBridgeError( + `Sandbox '${sandboxName}' has no durable lifecycle identity for MCP provider naming. Recreate the sandbox before adding an MCP server.`, + 2, + ); + } + // Bind external and unrecorded authority to durable lifecycle and + // provider intent. A refusal is retryable before a manifest exists, + // while sandbox recreation selects a different provider name. + providerInstanceId = crypto + .createHash("sha256") + .update( + JSON.stringify([ + "nemoclaw-mcp-provider-v1", + sandboxName, + lifecycleIdentity, + options.server, + envNames[0], + ]), + ) + .digest("hex") + .slice(0, 16); + } + providerName = buildMcpBridgeProviderName(sandboxName, options.server, providerInstanceId); + } const adapterEnvValues = resolveCredentialEnv(options.env); if (!existingEntry && !Object.hasOwn(adapterEnvValues, envNames[0])) { throw new McpBridgeError( @@ -306,6 +335,31 @@ async function addMcpBridgeUnlocked( 1, ); } + const requiredPolicyContent = buildRequiredMcpBridgePolicy(requestedEntry, target); + const recheckPolicyAuthority = () => { + try { + return preflightMcpPolicyAuthority({ + externalPolicy: "verify", + operation: `add MCP server '${requestedEntry.server}'`, + requiredPolicyContents: [requiredPolicyContent], + sandboxName, + }); + } catch (error) { + if (!(error instanceof McpPolicyAuthorityRefusalError) || !providerName) throw error; + throw new McpPolicyAuthorityRefusalError( + `${error.message} Required OpenShell provider name: '${providerName}'.`, + { + cause: error, + exitCode: error.exitCode, + reasonCode: error.reasonCode, + }, + ); + } + }; + // Qualify the exact external policy requirement before recording a bridge. + // A later refusal keeps the prepared manifest as retry intent, but the first + // refusal must leave the registry unchanged. + const policyAuthority = recheckPolicyAuthority(); // Hermes config posture is host-visible, so reject before even the durable // prepared manifest is written. The in-sandbox helper repeats the check at // the actual config write so a concurrent posture change still fails closed. @@ -320,9 +374,13 @@ async function addMcpBridgeUnlocked( // used by credentials add. Neither command can pass its collision check // before the other records its credential-key reservation. assertNoProviderCredentialCollisions(sandboxName, [entry]); + recheckPolicyAuthority(); writeBridgeEntry(sandboxName, entry); }); } + // From this point onward the prepared manifest owns the generated provider + // name, so later refusals leave stable retry intent. + recheckPolicyAuthority(); let providerCreated = false; let providerAttachAttempted = false; let policyApplied = false; @@ -345,6 +403,7 @@ async function addMcpBridgeUnlocked( // one recovery side effect that must precede the image capability // probe. It neither reads nor replaces credential material, and the // durable add manifest retains ownership if the later probe fails. + recheckPolicyAuthority(); detachMissingProviderReference(sandboxName, entry); detachedMissingProviderReference = true; } @@ -356,21 +415,25 @@ async function addMcpBridgeUnlocked( if (resumingPreflightedAdd && !Object.hasOwn(adapterEnvValues, entry.env[0])) { try { // A retry may reuse an exact provider without re-exporting its secret, - // but recreating a missing provider cannot. This check and any owned - // policy cleanup happen only after the running-image capability probe. + // but recreating a missing provider cannot. This check and any + // NemoClaw-owned policy cleanup happen only after the running-image + // capability probe. assertMcpProviderRecoverable(entry); } catch (error) { - removeGeneratedPolicy(sandboxName, entry, { bestEffort: true }); + if (policyAuthority === "nemoclaw-managed") { + removeGeneratedPolicy(sandboxName, entry, { bestEffort: true }); + } throw error; } } if (entry.addState === "prepared") { - assertPreparedMcpAddResourcesAbsent(sandboxName, adapter, entry, target); + assertPreparedMcpAddResourcesAbsent(sandboxName, adapter, entry, target, policyAuthority); entry = { ...entry, addState: "preflighted" }; // This second durable boundary proves the derived resource names and the // adapter slot were absent before any side effect. After a crash, retries // may therefore reuse only missing or exact resources, never drift. + recheckPolicyAuthority(); writeBridgeEntry(sandboxName, entry); } const adapterInspection = inspectAgentAdapterRegistration(sandboxName, adapter, entry); @@ -390,13 +453,17 @@ async function addMcpBridgeUnlocked( // supplied by a foreign attachment before opening its MCP route, then check // again after provider creation to close the intervening race. assertNoProviderCredentialCollisions(sandboxName, [entry]); + recheckPolicyAuthority(); ensureMcpBridgeProviderProfile(); // Load the real protocol:mcp policy without a credential binding before // provider mutation. OpenShell requires the endpointless provider to be // attached before it accepts credential_binding.provider, and withholds // that provider's static credential until the bound policy is active. - applyGeneratedPolicy(sandboxName, entry, target, { bindCredential: false }); - policyApplied = true; + recheckPolicyAuthority(); + if (policyAuthority === "nemoclaw-managed") { + applyGeneratedPolicy(sandboxName, entry, target, { bindCredential: false }); + policyApplied = true; + } const providerResult = upsertMcpProvider(providerName ?? "", options.env, { // A first mutation must still observe the absence proven above. Only a // retry of the durable preflighted transaction may encounter an exact @@ -404,6 +471,7 @@ async function addMcpBridgeUnlocked( allowExisting: resumingPreflightedAdd, expectedProviderId: entry.providerId, prepareMutation: (action) => { + recheckPolicyAuthority(); // A fresh create has no prior revision to compare. Observe only the // bounded placeholder classification for an actual update, after the // running supervisor has accepted the authenticated MCP policy. @@ -424,6 +492,7 @@ async function addMcpBridgeUnlocked( // The immutable OpenShell identity is the ownership boundary for every // later lifecycle action. Persist it before policy, attachment, or // adapter mutations. A process death before this write fails closed. + recheckPolicyAuthority(); writeBridgeEntry(sandboxName, entry); } assertNoProviderCredentialCollisions(sandboxName, [entry]); @@ -433,8 +502,11 @@ async function addMcpBridgeUnlocked( ); } providerAttachAttempted = true; + recheckPolicyAuthority(); attachProvider(sandboxName, entry); - applyGeneratedPolicy(sandboxName, entry, target); + if (policyAuthority === "nemoclaw-managed") { + applyGeneratedPolicy(sandboxName, entry, target); + } let refreshedAfterObservedAbsence = false; let credentialRevision = waitForAttachedMcpCredential(sandboxName, entry, { ...(providerResult.action === "updated" @@ -462,9 +534,13 @@ async function addMcpBridgeUnlocked( const republished = upsertMcpProvider(entry.providerName ?? "", options.env, { allowExisting: true, expectedProviderId: entry.providerId, + prepareMutation: recheckPolicyAuthority, requireExisting: true, }); - if (republished.action !== "updated") refreshMcpProviderEnvironment(entry); + if (republished.action !== "updated") { + recheckPolicyAuthority(); + refreshMcpProviderEnvironment(entry); + } }, }); if (Object.hasOwn(adapterEnvValues, entry.env[0]) && !refreshedAfterObservedAbsence) { @@ -485,23 +561,32 @@ async function addMcpBridgeUnlocked( // The adapter was proven absent above, so cleanup is safe even when a // command commits config and then fails during its runtime reload. adapterMutationAttempted = true; - registerAgentAdapter(sandboxName, adapter, entry, adapterEnvValues, { + recheckPolicyAuthority(); + registerAgentAdapterAtCurrentCredentialRevision( + sandboxName, + adapter, + entry, + adapterEnvValues, + credentialRevision, + { // An exact adapter entry is evidence of a post-commit process death. // Replacing it is idempotent and, for Hermes, re-verifies runtime reload. // The wait above already proved the same revision stable in consecutive // fresh execs, so repeating reconciliation here can outlive the caller's // bounded provider-synchronization contract. - replaceExisting: resumingPreflightedAdd && adapterInspection.state === "registered", - credentialRevision, - }); + replaceExisting: resumingPreflightedAdd && adapterInspection.state === "registered", + }, + ); if (adapter === "hermes-config") assertHermesMcpRuntimeIntent(sandboxName); const { addState: _completedAddState, ...committedEntry } = entry; + recheckPolicyAuthority(); writeBridgeEntry(sandboxName, committedEntry); + recheckPolicyAuthority(); } catch (error) { - const rollbackProviderInspection = - (providerAttachAttempted || providerCreated) && entry.providerId - ? inspectMcpProvider(providerName) - : undefined; + let rollbackProviderInspection: McpProviderInspection | undefined; + if ((providerAttachAttempted || providerCreated) && entry.providerId) { + rollbackProviderInspection = inspectMcpProvider(providerName); + } const rollbackProviderOwned = !!rollbackProviderInspection && providerMatchesCredential(rollbackProviderInspection, entry.env[0], entry.providerId); @@ -512,12 +597,24 @@ async function addMcpBridgeUnlocked( envValues: adapterEnvValues, }); } + let rollbackAuthorityRefusal: McpPolicyAuthorityRefusalError | undefined; if (policyApplied) { - removeGeneratedPolicy(sandboxName, entry, { bestEffort: true }); + try { + // Policy authority gates only this live policy mutation. Adapter, + // provider, credential, and durable-state cleanup continues below. + removeGeneratedPolicy(sandboxName, entry, { bestEffort: true }); + } catch (rollbackError) { + if (rollbackError instanceof McpPolicyAuthorityRefusalError) { + rollbackAuthorityRefusal = rollbackError; + } else { + throw rollbackError; + } + } + } + let detachOutcome: Awaited> = "absent"; + if (providerAttachAttempted) { + detachOutcome = await detachProvider(sandboxName, entry, { bestEffort: true }); } - const detachOutcome = providerAttachAttempted - ? detachProvider(sandboxName, entry, { bestEffort: true }) - : "absent"; let reservationCleanupProved = !providerAttachAttempted; if (providerAttachAttempted && detachOutcome !== "unknown") { try { @@ -536,6 +633,18 @@ async function addMcpBridgeUnlocked( // Exception rollback is best-effort and process death skips it entirely. // Keep the durable add manifest until a retry converges or `mcp remove` // proves and cleans each exact resource. + if (error instanceof McpPolicyAuthorityRefusalError) throw error; + if (rollbackAuthorityRefusal) { + throw new McpPolicyAuthorityRefusalError( + `${error instanceof Error ? error.message : String(error)}\n${rollbackAuthorityRefusal.message}`, + { + cause: error, + ...(error instanceof McpBridgeError + ? { exitCode: error.exitCode, reasonCode: error.reasonCode } + : {}), + }, + ); + } throw error; } } diff --git a/src/lib/actions/sandbox/mcp-bridge-contracts.ts b/src/lib/actions/sandbox/mcp-bridge-contracts.ts index 29d81e01b07..ffc21525c38 100644 --- a/src/lib/actions/sandbox/mcp-bridge-contracts.ts +++ b/src/lib/actions/sandbox/mcp-bridge-contracts.ts @@ -12,8 +12,9 @@ export class McpBridgeError extends Error { message: string, readonly exitCode = 1, readonly reasonCode?: McpBridgeErrorReasonCode, + options?: ErrorOptions, ) { - super(message); + super(message, options); this.name = "McpBridgeError"; } } diff --git a/src/lib/actions/sandbox/mcp-bridge-destroy-preflight.ts b/src/lib/actions/sandbox/mcp-bridge-destroy-preflight.ts index 0f65bb46ca3..d20869d046a 100644 --- a/src/lib/actions/sandbox/mcp-bridge-destroy-preflight.ts +++ b/src/lib/actions/sandbox/mcp-bridge-destroy-preflight.ts @@ -27,6 +27,12 @@ export interface McpDestroyPreparation { entries: McpBridgeEntry[]; detachedProviderEntries: McpBridgeEntry[]; scrubbedAdapterEntries: McpScrubbedAdapterEntry[]; + /** Recheck the exact retained MCP policy authority before sandbox deletion. */ + revalidateBeforeDelete?: () => Promise; + /** Recheck the retained authority and exact manifest after confirmed deletion. */ + revalidateAfterDelete?: () => Promise; + /** Recheck only retained authority after exact post-delete cleanup. */ + revalidateBeforeSuccess?: () => Promise; /** True when phase one was completed by an earlier destroy process. */ destroyAlreadyPrepared: boolean; /** True when a previous destroy already confirmed the sandbox was absent. */ diff --git a/src/lib/actions/sandbox/mcp-bridge-destroy.ts b/src/lib/actions/sandbox/mcp-bridge-destroy.ts index fcfa0b95ffb..4ccf2c5197f 100644 --- a/src/lib/actions/sandbox/mcp-bridge-destroy.ts +++ b/src/lib/actions/sandbox/mcp-bridge-destroy.ts @@ -9,7 +9,16 @@ import { type McpScrubbedAdapterEntry, } from "./mcp-bridge-adapter-teardown"; import { MCP_BRIDGE_POLICY_SOURCE, McpBridgeError } from "./mcp-bridge-contracts"; -import { removeGeneratedPolicy } from "./mcp-bridge-policy"; +import { + assertGeneratedPolicyMutationSafe, + buildRequiredMcpBridgePolicy, + McpPolicyAuthorityRefusalError, + qualifyMcpPolicyAuthorityReceipt, + removeGeneratedPolicy, + revalidateContainingMcpPolicyAuthority, + revalidateDeletedMcpPolicyAuthorityReceipt, + revalidateMcpPolicyAuthorityReceipt, +} from "./mcp-bridge-policy"; import type { McpDestroyPreparation } from "./mcp-bridge-destroy-preflight"; import { assertMcpDestroySnapshotCurrent, @@ -21,6 +30,7 @@ import { deleteProvider, detachProvider, inspectMcpProvider, + preflightMcpEntryTargets, waitForDetachedMcpCredential, } from "./mcp-bridge-provider"; import { restoreExistingMcpBridgeRuntime } from "./mcp-bridge-restart"; @@ -38,6 +48,7 @@ import { validateSandboxName } from "./mcp-bridge-validation"; export type { McpDestroyPreparation } from "./mcp-bridge-destroy-preflight"; export { + assertMcpDestroySnapshotCurrent, cloneMcpBridgeEntry, discardSafeIncompleteMcpAdds, inspectExactMcpDestroyProvider, @@ -53,6 +64,7 @@ export { */ export async function prepareMcpBridgesForDestroy( sandboxName: string, + validateContainingPolicyReceipt?: () => Promise, ): Promise { validateSandboxName(sandboxName); const currentSandbox = getSandboxOrThrow(sandboxName); @@ -68,6 +80,7 @@ export async function prepareMcpBridgesForDestroy( currentSandbox, entriesRequiringExternalCleanup, ); + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); const sandbox = await discardSafeIncompleteMcpAdds(sandboxName, currentSandbox); const entries = Object.values(bridgeState(sandbox)).map(cloneMcpBridgeEntry); const destroyAlreadyPrepared = !!sandbox.mcp?.destroyPreparedAt; @@ -105,35 +118,83 @@ export async function prepareMcpBridgesForDestroy( destroyAlreadyPending: true, }; } + const resolvedTargets = await preflightMcpEntryTargets(entries); + const policyAuthorityReceipt = qualifyMcpPolicyAuthorityReceipt({ + operation: `prepare MCP bridges before destroying sandbox '${sandboxName}'`, + requiredPolicyContents: entries.map((entry) => { + const target = resolvedTargets.get(entry.server); + if (!target) { + throw new McpBridgeError( + `MCP server '${entry.server}' has no validated address pins. Refusing destroy preparation.`, + ); + } + return buildRequiredMcpBridgePolicy(entry, target); + }), + sandboxName, + }); + const revalidateBeforeMutation = async (): Promise => { + await revalidateMcpPolicyAuthorityReceipt( + policyAuthorityReceipt, + validateContainingPolicyReceipt, + () => assertMcpDestroySnapshotCurrent(sandboxName, entries), + ); + }; + const revalidateAfterDelete = async (): Promise => { + await revalidateDeletedMcpPolicyAuthorityReceipt( + policyAuthorityReceipt, + validateContainingPolicyReceipt, + () => assertMcpDestroySnapshotCurrent(sandboxName, entries), + ); + }; + const revalidateBeforeSuccess = async (): Promise => { + await revalidateDeletedMcpPolicyAuthorityReceipt( + policyAuthorityReceipt, + validateContainingPolicyReceipt, + ); + }; if (destroyAlreadyPrepared) { - // Phase one completed before a prior process stopped. The sandbox may be - // live with its adapter scrubbed/provider detached, or it may already be - // gone. In either case, repeating delete is the next idempotent step. + // Phase one completed before a prior process stopped. Requalify and retain + // its exact current policy requirements before repeating sandbox delete. return { entries, detachedProviderEntries: entries.map(cloneMcpBridgeEntry), scrubbedAdapterEntries: entries.map(cloneMcpBridgeEntry), + revalidateBeforeDelete: revalidateBeforeMutation, + revalidateAfterDelete, + revalidateBeforeSuccess, destroyAlreadyPrepared: true, destroyAlreadyPending: false, }; } - await ensureSandboxGatewaySelected(sandboxName); + if (policyAuthorityReceipt.authority === "nemoclaw-managed") { + for (const entry of entries) assertGeneratedPolicyMutationSafe(sandboxName, entry); + } assertMcpAdapterTeardownRuntimeCapabilities(sandboxName, sandbox, entries); const detached: McpBridgeEntry[] = []; const scrubbedAdapters: McpScrubbedAdapterEntry[] = []; const removedPolicies: McpBridgeEntry[] = []; + let providerDetachAttempted = false; try { for (const entry of entries) { + await revalidateBeforeMutation(); scrubbedAdapters.push(scrubManagedMcpAdapterOrThrow(sandboxName, sandbox, entry)); } - for (const entry of entries) { - removeGeneratedPolicy(sandboxName, entry); - removedPolicies.push(entry); + if (policyAuthorityReceipt.authority === "nemoclaw-managed") { + for (const entry of entries) { + await revalidateBeforeMutation(); + removeGeneratedPolicy(sandboxName, entry); + removedPolicies.push(entry); + } } for (const entry of entries) { + await revalidateBeforeMutation(); inspectExactMcpDestroyProvider(entry, { allowMissing: false }); - const detachOutcome = detachProvider(sandboxName, entry, { allowLegacyGeneric: true }); + providerDetachAttempted = true; + const detachOutcome = await detachProvider(sandboxName, entry, { + allowLegacyGeneric: true, + prepareMutation: revalidateBeforeMutation, + }); if (detachOutcome === "unknown") { throw new McpBridgeError( `Could not prove provider detach for MCP server '${entry.server}'.`, @@ -146,6 +207,7 @@ export async function prepareMcpBridgesForDestroy( // detached one entry before a later entry fails. detached.push(entry); } + await revalidateBeforeMutation(); const marked = registry.updateSandbox(sandboxName, { mcp: { bridges: Object.fromEntries( @@ -165,9 +227,21 @@ export async function prepareMcpBridgesForDestroy( } catch (error) { const rollbackFailures: string[] = []; let runtimeRestored = false; - if (removedPolicies.length > 0) { + let snapshotCurrent = true; + try { + assertMcpDestroySnapshotCurrent(sandboxName, entries); + } catch (snapshotError) { + snapshotCurrent = false; + rollbackFailures.push( + snapshotError instanceof Error ? snapshotError.message : String(snapshotError), + ); + } + if (snapshotCurrent && scrubbedAdapters.length > 0) { try { - await restoreExistingMcpBridgeRuntime(sandboxName, removedPolicies, { + await restoreExistingMcpBridgeRuntime(sandboxName, scrubbedAdapters, { + ...(error instanceof McpPolicyAuthorityRefusalError + ? { teardownPolicyAuthorityRefusal: error } + : {}), lifecyclePhase: "teardown-rollback", }); runtimeRestored = true; @@ -177,13 +251,24 @@ export async function prepareMcpBridgesForDestroy( ); } } - if (!runtimeRestored) { + if ( + snapshotCurrent && + !runtimeRestored && + removedPolicies.length === 0 && + !providerDetachAttempted && + !(error instanceof McpPolicyAuthorityRefusalError) + ) { rollbackFailures.push( - ...rollbackScrubbedMcpAdapters(sandboxName, sandbox, scrubbedAdapters), + ...(await rollbackScrubbedMcpAdapters( + sandboxName, + sandbox, + scrubbedAdapters, + revalidateBeforeMutation, + )), ); } const current = registry.getSandbox(sandboxName); - if (current?.mcp?.destroyPreparedAt) { + if (runtimeRestored && current?.mcp?.destroyPreparedAt) { try { registry.updateSandbox(sandboxName, { mcp: { @@ -202,6 +287,13 @@ export async function prepareMcpBridgesForDestroy( } } const detail = error instanceof Error ? error.message : String(error); + if (error instanceof McpPolicyAuthorityRefusalError) { + throw new McpPolicyAuthorityRefusalError( + rollbackFailures.length > 0 + ? `${detail}\nMCP destroy compensation remains pending: ${rollbackFailures.join("; ")}` + : detail, + ); + } throw new McpBridgeError( rollbackFailures.length > 0 ? `${detail}\nMCP destroy rollback could not reattach: ${rollbackFailures.join("; ")}` @@ -212,22 +304,82 @@ export async function prepareMcpBridgesForDestroy( entries, detachedProviderEntries: detached, scrubbedAdapterEntries: scrubbedAdapters, + revalidateBeforeDelete: revalidateBeforeMutation, + revalidateAfterDelete, + revalidateBeforeSuccess, destroyAlreadyPrepared: false, destroyAlreadyPending: false, }; } +/** Recheck exact MCP policy requirements before opening a delete-abort recovery window. */ +export async function revalidateMcpDestroyAbortPolicyAuthority( + sandboxName: string, + preparation: McpDestroyPreparation, + validateContainingPolicyReceipt?: () => Promise, +): Promise { + if (preparation.entries.length === 0 || preparation.destroyAlreadyPending) return; + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); + const resolvedTargets = await preflightMcpEntryTargets(preparation.entries); + const receipt = qualifyMcpPolicyAuthorityReceipt({ + operation: `restore MCP bridges after a refused delete of sandbox '${sandboxName}'`, + requiredPolicyContents: preparation.entries.map((entry) => { + const target = resolvedTargets.get(entry.server); + if (!target) { + throw new McpBridgeError( + `MCP server '${entry.server}' has no validated address pins. Refusing delete-abort recovery.`, + ); + } + return buildRequiredMcpBridgePolicy(entry, target); + }), + sandboxName, + }); + await revalidateMcpPolicyAuthorityReceipt(receipt, undefined, () => + assertMcpDestroySnapshotCurrent(sandboxName, preparation.entries), + ); +} + /** Restore all MCP runtime state after OpenShell refused to delete the sandbox. */ export async function restoreMcpBridgesAfterDestroyAbort( sandboxName: string, preparation: McpDestroyPreparation, + validateContainingPolicyReceipt?: () => Promise, ): Promise { if (preparation.entries.length === 0 || preparation.destroyAlreadyPending) { return; } - const preparedSandbox = assertMcpDestroySnapshotCurrent(sandboxName, preparation.entries); + let authorityRefusal: McpPolicyAuthorityRefusalError | undefined; + try { + await revalidateMcpDestroyAbortPolicyAuthority( + sandboxName, + preparation, + validateContainingPolicyReceipt, + ); + } catch (error) { + if (!(error instanceof McpPolicyAuthorityRefusalError)) throw error; + authorityRefusal = error; + } + let preparedSandbox: ReturnType; + try { + preparedSandbox = assertMcpDestroySnapshotCurrent(sandboxName, preparation.entries); + } catch (snapshotError) { + if (!authorityRefusal) throw snapshotError; + const snapshotDetail = + snapshotError instanceof Error ? snapshotError.message : String(snapshotError); + throw new McpPolicyAuthorityRefusalError( + `${authorityRefusal.message}\nMCP destroy-abort snapshot validation also failed: ${snapshotDetail}`, + { + cause: new AggregateError( + [authorityRefusal, snapshotError], + "MCP destroy-abort authority and snapshot validation failed", + ), + exitCode: authorityRefusal.exitCode, + reasonCode: authorityRefusal.reasonCode, + }, + ); + } const destroyPreparedAt = preparedSandbox.mcp?.destroyPreparedAt ?? nowIso(); - const cleared = registry.updateSandbox(sandboxName, { + const transitioned = registry.updateSandbox(sandboxName, { mcp: { bridges: Object.fromEntries( preparation.entries.map((entry) => [entry.server, cloneMcpBridgeEntry(entry)]), @@ -235,11 +387,14 @@ export async function restoreMcpBridgesAfterDestroyAbort( ...(preparedSandbox.mcp?.managedServerNames ? { managedServerNames: preparedSandbox.mcp.managedServerNames } : {}), + ...(authorityRefusal ? { destroyPreparedAt } : {}), }, }); - if (!cleared) { + if (!transitioned) { throw new McpBridgeError( - `Could not clear prepared MCP destroy state for sandbox '${sandboxName}' before runtime restoration.`, + authorityRefusal + ? `Could not retain prepared MCP destroy state for sandbox '${sandboxName}' before runtime compensation.` + : `Could not clear prepared MCP destroy state for sandbox '${sandboxName}' before runtime restoration.`, ); } try { @@ -248,7 +403,9 @@ export async function restoreMcpBridgesAfterDestroyAbort( for (const entry of preparation.entries) inspectExactMcpDestroyProvider(entry, { allowMissing: false }); await restoreExistingMcpBridgeRuntime(sandboxName, preparation.entries, { + ...(authorityRefusal ? { teardownPolicyAuthorityRefusal: authorityRefusal } : {}), lifecyclePhase: "teardown-rollback", + ...(authorityRefusal ? {} : { validateContainingPolicyReceipt }), }); } catch (error) { let markerRestoreFailure = ""; @@ -270,12 +427,27 @@ export async function restoreMcpBridgesAfterDestroyAbort( restoreError instanceof Error ? restoreError.message : String(restoreError); } const detail = error instanceof Error ? error.message : String(error); + if (authorityRefusal) { + throw new McpPolicyAuthorityRefusalError( + `${authorityRefusal.message}\nMCP destroy-abort compensation remains pending: ${ + markerRestoreFailure ? `${detail}; ${markerRestoreFailure}` : detail + }`, + ); + } + if (error instanceof McpPolicyAuthorityRefusalError) { + throw new McpPolicyAuthorityRefusalError( + markerRestoreFailure + ? `${detail}; MCP destroy compensation remains pending: ${markerRestoreFailure}` + : detail, + ); + } throw new McpBridgeError( markerRestoreFailure ? `${detail}; could not restore the MCP destroy retry marker: ${markerRestoreFailure}` : detail, ); } + if (authorityRefusal) throw authorityRefusal; } /** diff --git a/src/lib/actions/sandbox/mcp-bridge-input-targets.test.ts b/src/lib/actions/sandbox/mcp-bridge-input-targets.test.ts index 67bd2660d55..f033c104ac0 100644 --- a/src/lib/actions/sandbox/mcp-bridge-input-targets.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-input-targets.test.ts @@ -181,6 +181,7 @@ replace(adapters, "assertAgentMcpMutationRuntimeCapability", () => {}); replace(adapters, "inspectAgentAdapterRegistration", () => ({ state: "absent" })); replace(adapters, "registerAgentAdapter", () => {}); replace(policy, "applyGeneratedPolicy", (_sandbox, _entry, target) => { admittedTarget = target; }); +replace(policy, "preflightMcpPolicyAuthority", () => "nemoclaw-managed"); replace(state, "ensureSandboxGatewaySelected", async () => {}); replace(validation, "assertMcpCredentialBoundaryRuntimeVersion", () => {}); replace(provider, "assertNoProviderCredentialCollisions", () => {}); diff --git a/src/lib/actions/sandbox/mcp-bridge-policy.test.ts b/src/lib/actions/sandbox/mcp-bridge-policy.test.ts index 23315de2c5a..f699c43bb4e 100644 --- a/src/lib/actions/sandbox/mcp-bridge-policy.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-policy.test.ts @@ -1,10 +1,11 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import YAML from "yaml"; import type { AgentMcpAdapter } from "../../agent/defs"; +import { isPolicyAuthorityRefusalError } from "../../adapters/openshell/policy-authority"; import * as policies from "../../policy"; import { replayTrustedPrivateEndpoint } from "../../security/trusted-private-endpoint"; import type { McpBridgeEntry } from "../../state/registry"; @@ -21,9 +22,13 @@ import { applyGeneratedPolicy, assertGeneratedPolicyExactReadOnly, assertGeneratedPolicyMutationSafe, + qualifyMcpPolicyAuthorityReceipt, + McpPolicyAuthorityRefusalError, removeGeneratedPolicy, + revalidateMcpPolicyAuthorityReceipt, } from "./mcp-bridge-policy"; import type { McpBridgeTargetValidation } from "./mcp-bridge-url-validation"; +import * as policyAuthority from "./policy-authority/preflight"; function buildMcpBridgePolicyYaml( server: string, @@ -49,10 +54,67 @@ function githubBridgeEntry(overrides: Partial = {}): McpBridgeEn } describe("MCP OpenShell policy", () => { + beforeEach(() => { + vi.spyOn(policyAuthority, "preflightSandboxPolicyAuthority").mockReturnValue( + "nemoclaw-managed", + ); + }); + afterEach(() => { vi.restoreAllMocks(); }); + it("classifies MCP authority refusals at cross-boundary recovery sites (#9833)", () => { + expect( + isPolicyAuthorityRefusalError( + new McpPolicyAuthorityRefusalError("External policy authority changed."), + ), + ).toBe(true); + }); + + it("retains and revalidates the exact external MCP policy requirement (#9833)", async () => { + vi.mocked(policyAuthority.preflightSandboxPolicyAuthority).mockReturnValue( + "externally-managed", + ); + const requiredPolicy = "network_policies:\n mcp_bridge_example: {}\n"; + + const receipt = qualifyMcpPolicyAuthorityReceipt({ + operation: "add MCP server 'example'", + requiredPolicyContents: [requiredPolicy], + sandboxName: "alpha", + }); + await revalidateMcpPolicyAuthorityReceipt(receipt); + + expect(receipt).toEqual({ + authority: "externally-managed", + operation: "add MCP server 'example'", + requiredPolicyContents: [requiredPolicy], + sandboxName: "alpha", + }); + expect(policyAuthority.preflightSandboxPolicyAuthority).toHaveBeenCalledTimes(2); + expect(policyAuthority.preflightSandboxPolicyAuthority).toHaveBeenLastCalledWith({ + externalPolicy: "verify", + operation: "add MCP server 'example'", + requiredPolicyContents: [requiredPolicy], + sandboxName: "alpha", + }); + }); + + it("refuses when an exact MCP receipt changes authority before mutation (#9833)", async () => { + vi.mocked(policyAuthority.preflightSandboxPolicyAuthority) + .mockReturnValueOnce("externally-managed") + .mockReturnValueOnce("nemoclaw-managed"); + const receipt = qualifyMcpPolicyAuthorityReceipt({ + operation: "remove MCP server 'example'", + requiredPolicyContents: ["network_policies:\n mcp_bridge_example: {}\n"], + sandboxName: "alpha", + }); + + await expect(revalidateMcpPolicyAuthorityReceipt(receipt)).rejects.toBeInstanceOf( + McpPolicyAuthorityRefusalError, + ); + }); + it("refuses to apply a generated policy without exact public address pins", () => { expect(() => applyGeneratedPolicy( @@ -301,6 +363,76 @@ describe("MCP OpenShell policy", () => { }); }); + it("stops before reserving generated policy ownership when authority changes after inspection (#9833)", () => { + vi.mocked(policyAuthority.preflightSandboxPolicyAuthority) + .mockReturnValueOnce("nemoclaw-managed") + .mockImplementationOnce(() => { + throw new Error("policy authority changed"); + }); + vi.spyOn(registry, "getCustomPolicies").mockReturnValue([]); + vi.spyOn(policies, "getPresetContentGatewayState").mockReturnValue("absent"); + const addPolicy = vi.spyOn(registry, "addCustomPolicy").mockReturnValue(true); + const applyPolicy = vi.spyOn(policies, "applyPresetContent").mockReturnValue(true); + + expect(() => + applyGeneratedPolicy("alpha", githubBridgeEntry(), { addresses: ["8.8.8.8"] }), + ).toThrow(/policy authority changed/); + expect(addPolicy).not.toHaveBeenCalled(); + expect(applyPolicy).not.toHaveBeenCalled(); + }); + + it("keeps a generated policy transition pending when authority changes after policy apply (#9833)", () => { + vi.mocked(policyAuthority.preflightSandboxPolicyAuthority) + .mockReturnValueOnce("nemoclaw-managed") + .mockReturnValueOnce("nemoclaw-managed") + .mockReturnValueOnce("nemoclaw-managed") + .mockImplementationOnce(() => { + throw new Error("policy authority changed"); + }); + vi.spyOn(registry, "getCustomPolicies").mockReturnValue([]); + vi.spyOn(policies, "getPresetContentGatewayState") + .mockReturnValueOnce("absent") + .mockReturnValueOnce("match"); + const addPolicy = vi.spyOn(registry, "addCustomPolicy").mockReturnValue(true); + const applyPolicy = vi.spyOn(policies, "applyPresetContent").mockReturnValue(true); + + expect(() => + applyGeneratedPolicy("alpha", githubBridgeEntry(), { addresses: ["8.8.8.8"] }), + ).toThrow(/policy authority changed/); + expect(applyPolicy).toHaveBeenCalledOnce(); + expect(addPolicy).toHaveBeenCalledOnce(); + expect(addPolicy.mock.calls[0]?.[1]).toEqual( + expect.objectContaining({ pendingContent: expect.any(String) }), + ); + }); + + it("does not swallow an authority change during best-effort generated policy removal (#9833)", () => { + const entry = githubBridgeEntry(); + const content = buildMcpBridgePolicyYaml(entry.server, entry.url, "mcporter", { + addresses: ["8.8.8.8"], + }); + vi.mocked(policyAuthority.preflightSandboxPolicyAuthority) + .mockReturnValueOnce("nemoclaw-managed") + .mockReturnValueOnce("nemoclaw-managed") + .mockImplementationOnce(() => { + throw new Error("policy authority changed"); + }); + vi.spyOn(registry, "getCustomPolicies").mockReturnValue([ + { name: entry.policyName, content, sourcePath: MCP_BRIDGE_POLICY_SOURCE }, + ]); + vi.spyOn(policies, "getPresetContentGatewayState") + .mockReturnValueOnce("match") + .mockReturnValueOnce("absent"); + const removePolicy = vi.spyOn(policies, "removePreset").mockReturnValue(true); + const removeOwnership = vi.spyOn(registry, "removeCustomPolicyByName"); + + expect(() => removeGeneratedPolicy("alpha", entry, { bestEffort: true })).toThrow( + /policy authority changed/, + ); + expect(removePolicy).toHaveBeenCalledOnce(); + expect(removeOwnership).not.toHaveBeenCalled(); + }); + it("accepts only the canonical generated policy for the exact bridge and DNS pins", () => { const entry = githubBridgeEntry(); const pins = ["2606:4700:4700::1111", "8.8.8.8"]; diff --git a/src/lib/actions/sandbox/mcp-bridge-policy.ts b/src/lib/actions/sandbox/mcp-bridge-policy.ts index c3805905258..478a8666d69 100644 --- a/src/lib/actions/sandbox/mcp-bridge-policy.ts +++ b/src/lib/actions/sandbox/mcp-bridge-policy.ts @@ -5,6 +5,7 @@ import { isIP } from "node:net"; import { isDeepStrictEqual } from "node:util"; import YAML from "yaml"; +import type { SandboxPolicyAuthority } from "../../adapters/openshell/policy-authority"; import type { AgentMcpAdapter } from "../../agent/defs"; import { diagnosticPreview } from "../../name-validation"; import * as policies from "../../policy"; @@ -19,6 +20,7 @@ import { isAgentMcpAdapter, MCP_BRIDGE_POLICY_SOURCE, McpBridgeError, + type McpBridgeErrorReasonCode, } from "./mcp-bridge-contracts"; import { buildMcpBridgeCapabilityPolicyYaml, @@ -26,9 +28,10 @@ import { buildMcpBridgePolicyName, buildMcpBridgePolicyYaml, } from "./mcp-bridge-policy-render"; +import { preflightSandboxPolicyAuthority } from "./policy-authority/preflight"; import type { McpBridgeTargetValidation } from "./mcp-bridge-url-validation"; -export { MCP_BRIDGE_POLICY_SOURCE } from "./mcp-bridge-contracts"; +export { isAgentMcpAdapter, MCP_BRIDGE_POLICY_SOURCE } from "./mcp-bridge-contracts"; export { buildMcpBridgePolicyKey, buildMcpBridgePolicyName, @@ -56,6 +59,140 @@ export interface ProvableManagedMcpPolicies { omissions: ManagedMcpPolicyOmission[]; } +export class McpPolicyAuthorityRefusalError extends McpBridgeError { + readonly code = "NEMOCLAW_POLICY_AUTHORITY_REFUSAL"; + + constructor( + message: string, + options: ErrorOptions & { + readonly exitCode?: number; + readonly reasonCode?: McpBridgeErrorReasonCode; + } = {}, + ) { + super(message, options.exitCode, options.reasonCode, options); + this.name = "McpPolicyAuthorityRefusalError"; + } +} + +export interface McpPolicyAuthorityReceipt { + readonly authority: SandboxPolicyAuthority; + readonly operation: string; + readonly requiredPolicyContents: readonly string[]; + readonly sandboxName: string; +} + +export function preflightMcpPolicyAuthority(options: { + readonly externalPolicy: "verify" | "refuse"; + readonly operation: string; + readonly requiredPolicyContents?: readonly string[]; + readonly sandboxName: string; +}): SandboxPolicyAuthority { + try { + return preflightSandboxPolicyAuthority(options); + } catch (error) { + throw new McpPolicyAuthorityRefusalError( + error instanceof Error ? error.message : String(error), + ); + } +} + +/** Retain the exact policy requirements and authority that qualified an MCP operation. */ +export function qualifyMcpPolicyAuthorityReceipt(options: { + readonly operation: string; + readonly requiredPolicyContents: readonly string[]; + readonly sandboxName: string; +}): McpPolicyAuthorityReceipt { + const requiredPolicyContents = [...options.requiredPolicyContents]; + const authority = preflightMcpPolicyAuthority({ + externalPolicy: "verify", + operation: options.operation, + requiredPolicyContents, + sandboxName: options.sandboxName, + }); + return { ...options, authority, requiredPolicyContents }; +} + +/** MCP rollback callers use this type to stop mutations after an enclosing authority refusal. */ +export async function revalidateContainingMcpPolicyAuthority( + validateContainingReceipt?: () => Promise, +): Promise { + try { + await validateContainingReceipt?.(); + } catch (error) { + if (error instanceof McpPolicyAuthorityRefusalError) throw error; + throw new McpPolicyAuthorityRefusalError( + error instanceof Error ? error.message : String(error), + ); + } +} + +/** Recheck an MCP receipt and its containing lifecycle receipt before one mutation. */ +export async function revalidateMcpPolicyAuthorityReceipt( + receipt: McpPolicyAuthorityReceipt, + validateContainingReceipt?: () => Promise, + assertCurrentState?: () => void, +): Promise { + try { + await revalidateContainingMcpPolicyAuthority(validateContainingReceipt); + const authority = preflightMcpPolicyAuthority({ + externalPolicy: "verify", + operation: receipt.operation, + requiredPolicyContents: receipt.requiredPolicyContents, + sandboxName: receipt.sandboxName, + }); + if (authority !== receipt.authority) { + throw new Error(`Policy authority changed while attempting to ${receipt.operation}.`); + } + assertCurrentState?.(); + } catch (error) { + if (error instanceof McpPolicyAuthorityRefusalError) throw error; + throw new McpPolicyAuthorityRefusalError( + error instanceof Error ? error.message : String(error), + ); + } +} + +/** Recheck the durable portion of an MCP receipt after its live sandbox was deleted. */ +export async function revalidateDeletedMcpPolicyAuthorityReceipt( + receipt: McpPolicyAuthorityReceipt, + validateContainingReceipt?: () => Promise, + assertCurrentState?: () => void, +): Promise { + try { + await revalidateContainingMcpPolicyAuthority(validateContainingReceipt); + const current = registry.getSandbox(receipt.sandboxName); + if (!current) { + throw new Error( + `Policy authority could not be revalidated after ${receipt.operation}: the sandbox is no longer registered.`, + ); + } + if (current.policyAuthority !== receipt.authority) { + throw new Error(`Policy authority changed while attempting to ${receipt.operation}.`); + } + assertCurrentState?.(); + } catch (error) { + if (error instanceof McpPolicyAuthorityRefusalError) throw error; + throw new McpPolicyAuthorityRefusalError( + error instanceof Error ? error.message : String(error), + ); + } +} + +export function buildRequiredMcpBridgePolicy( + entry: McpBridgeEntry, + target: McpBridgeTargetValidation, +): string { + assertMcpBridgePolicyTarget(entry, target); + const adapter = isAgentMcpAdapter(entry.adapter) ? entry.adapter : "mcporter"; + return buildMcpBridgePolicyYaml( + entry.server, + entry.url, + adapter, + target, + entry.providerName ?? "", + ); +} + type ManagedMcpPolicyInspectionDeps = { getSandbox: typeof registry.getSandbox; }; @@ -567,10 +704,22 @@ function withoutPendingContent( return { ...confirmed, content }; } +type GeneratedPolicyMutationAuthorityCheck = () => void; + +function assertGeneratedPolicyMutationAuthority(sandboxName: string, operation: string): void { + preflightMcpPolicyAuthority({ + externalPolicy: "refuse", + operation, + sandboxName, + }); +} + function persistGeneratedPolicyRegistration( sandboxName: string, policy: registry.CustomPolicyEntry, + beforePersist?: GeneratedPolicyMutationAuthorityCheck, ): void { + beforePersist?.(); if (!registry.addCustomPolicy(sandboxName, policy)) { throw new McpBridgeError( `Could not persist ownership for generated MCP policy '${policy.name}'.`, @@ -587,6 +736,7 @@ function persistGeneratedPolicyRegistration( function reconcileGeneratedPolicyRegistration( sandboxName: string, policy: registry.CustomPolicyEntry, + beforePersist?: GeneratedPolicyMutationAuthorityCheck, ): GeneratedPolicyRegistrationState { const pendingContent = policy.pendingContent; if (pendingContent === undefined) { @@ -603,7 +753,7 @@ function reconcileGeneratedPolicyRegistration( const pendingState = policies.getPresetContentGatewayState(sandboxName, pendingContent); if (pendingState === "match") { const confirmedPolicy = withoutPendingContent(policy, pendingContent); - persistGeneratedPolicyRegistration(sandboxName, confirmedPolicy); + persistGeneratedPolicyRegistration(sandboxName, confirmedPolicy, beforePersist); return { policy: confirmedPolicy, state: "match", confirmed: true }; } @@ -616,7 +766,7 @@ function reconcileGeneratedPolicyRegistration( const confirmedState = policies.getPresetContentGatewayState(sandboxName, policy.content); if (confirmedState === "match" || (confirmedState === "absent" && pendingState === "absent")) { const confirmedPolicy = withoutPendingContent(policy); - persistGeneratedPolicyRegistration(sandboxName, confirmedPolicy); + persistGeneratedPolicyRegistration(sandboxName, confirmedPolicy, beforePersist); return { policy: confirmedPolicy, state: confirmedState, confirmed: true }; } return { policy, state: confirmedState === null ? null : "drift", confirmed: false }; @@ -628,6 +778,12 @@ export function applyGeneratedPolicy( target: McpBridgeTargetValidation, options: { bindCredential?: boolean } = {}, ): void { + const recheckAuthority = () => + assertGeneratedPolicyMutationAuthority( + sandboxName, + `apply generated MCP policy '${entry.policyName}'`, + ); + recheckAuthority(); const resolvedAddresses = assertMcpBridgePolicyTarget(entry, target); if (resolvedAddresses.length === 0) { throw new McpBridgeError( @@ -638,13 +794,7 @@ export function applyGeneratedPolicy( const content = options.bindCredential === false ? buildMcpBridgeCapabilityPolicyYaml(entry.server, entry.url, adapter, target) - : buildMcpBridgePolicyYaml( - entry.server, - entry.url, - adapter, - target, - entry.providerName ?? "", - ); + : buildRequiredMcpBridgePolicy(entry, target); const policyKey = buildMcpBridgePolicyKey(entry.server); const sameNamePolicy = registry .getCustomPolicies(sandboxName) @@ -659,7 +809,11 @@ export function applyGeneratedPolicy( let previousPolicyConfirmed = false; let ownsExistingPolicyKey = false; if (registeredPolicy) { - const reconciled = reconcileGeneratedPolicyRegistration(sandboxName, registeredPolicy); + const reconciled = reconcileGeneratedPolicyRegistration( + sandboxName, + registeredPolicy, + recheckAuthority, + ); previousPolicy = reconciled.policy; previousPolicyConfirmed = reconciled.confirmed; const previousState = reconciled.state; @@ -693,7 +847,7 @@ export function applyGeneratedPolicy( reservation = previousPolicy; } else if (previousPolicy) { reservation = { ...withoutPendingContent(previousPolicy), pendingContent: content }; - persistGeneratedPolicyRegistration(sandboxName, reservation); + persistGeneratedPolicyRegistration(sandboxName, reservation, recheckAuthority); } else { reservation = { name: entry.policyName, @@ -701,11 +855,12 @@ export function applyGeneratedPolicy( pendingContent: content, sourcePath: MCP_BRIDGE_POLICY_SOURCE, }; - persistGeneratedPolicyRegistration(sandboxName, reservation); + persistGeneratedPolicyRegistration(sandboxName, reservation, recheckAuthority); } // `custom` denotes user-supplied preset content and intentionally rejects // `allowed_ips`. This content is generated from validated MCP inputs and the // ownership reservation above; `skipRegistryUpdate` avoids a second write. + recheckAuthority(); const ok = policies.applyPresetContent(sandboxName, entry.policyName, content, { expectedExistingNetworkPolicyContent: ownsExistingPolicyKey && previousPolicy ? previousPolicy.content : null, @@ -717,7 +872,11 @@ export function applyGeneratedPolicy( // Confirm that the effective policy still contains our exact generated entry. const activeState = policies.getPresetContentGatewayState(sandboxName, content); if (ok !== false && activeState === "match") { - persistGeneratedPolicyRegistration(sandboxName, withoutPendingContent(reservation, content)); + persistGeneratedPolicyRegistration( + sandboxName, + withoutPendingContent(reservation, content), + recheckAuthority, + ); return; } @@ -727,9 +886,14 @@ export function applyGeneratedPolicy( previousPolicy.content, ); if (previousState === "match" || (previousState === "absent" && activeState === "absent")) { - persistGeneratedPolicyRegistration(sandboxName, withoutPendingContent(previousPolicy)); + persistGeneratedPolicyRegistration( + sandboxName, + withoutPendingContent(previousPolicy), + recheckAuthority, + ); } } else if (activeState === "absent") { + recheckAuthority(); registry.removeCustomPolicyByName(sandboxName, entry.policyName); } const detail = @@ -802,10 +966,16 @@ export function assertGeneratedPolicyMutationSafe( sandboxName: string, entry: McpBridgeEntry, ): void { + const recheckAuthority = () => + assertGeneratedPolicyMutationAuthority( + sandboxName, + `inspect generated MCP policy '${entry.policyName}' before mutation`, + ); + recheckAuthority(); const registeredPolicy = assertGeneratedPolicyRegistrationMutationSafe(sandboxName, entry); const owned = registeredPolicy !== undefined; const reconciled = registeredPolicy - ? reconcileGeneratedPolicyRegistration(sandboxName, registeredPolicy) + ? reconcileGeneratedPolicyRegistration(sandboxName, registeredPolicy, recheckAuthority) : undefined; const state = reconciled?.state ?? getUnownedGeneratedPolicyState(sandboxName, entry); if (state === "absent") return; @@ -910,11 +1080,17 @@ export function assertGeneratedPolicyExactReadOnly( return { ...registeredPolicy }; } -export function removeGeneratedPolicy( +function removeGeneratedPolicyStrict( sandboxName: string, entry: McpBridgeEntry, options: { bestEffort?: boolean; preserveRegistryOwnership?: boolean } = {}, ): void { + const recheckAuthority = () => + assertGeneratedPolicyMutationAuthority( + sandboxName, + `remove generated MCP policy '${entry.policyName}'`, + ); + recheckAuthority(); const policyName = entry.policyName; const registeredPolicy = registry .getCustomPolicies(sandboxName) @@ -922,7 +1098,7 @@ export function removeGeneratedPolicy( const ownsRegistration = registeredPolicy?.sourcePath === MCP_BRIDGE_POLICY_SOURCE; const reconciled = registeredPolicy && ownsRegistration - ? reconcileGeneratedPolicyRegistration(sandboxName, registeredPolicy) + ? reconcileGeneratedPolicyRegistration(sandboxName, registeredPolicy, recheckAuthority) : undefined; const effectiveRegistration = reconciled?.policy ?? registeredPolicy; const content = effectiveRegistration?.content; @@ -933,6 +1109,7 @@ export function removeGeneratedPolicy( : getUnownedGeneratedPolicyState(sandboxName, entry)); if (gatewayState === "absent") { if (ownsRegistration && !options.preserveRegistryOwnership) { + recheckAuthority(); registry.removeCustomPolicyByName(sandboxName, policyName); } return; @@ -943,6 +1120,7 @@ export function removeGeneratedPolicy( `Generated MCP policy '${policyName}' is unowned, unreachable, or no longer matches its registered content. Refusing to delete same-key policy state.`, ); } + recheckAuthority(); const ok = policies.removePreset(sandboxName, policyName, { nonFatal: true, // Keep ownership durable across a crash or superseded OpenShell revision. @@ -960,6 +1138,7 @@ export function removeGeneratedPolicy( const activeState = policies.getPresetContentGatewayState(sandboxName, content); if (activeState === "absent") { if (!options.preserveRegistryOwnership) { + recheckAuthority(); registry.removeCustomPolicyByName(sandboxName, policyName); } return; @@ -967,13 +1146,27 @@ export function removeGeneratedPolicy( // Keep (or defensively restore) the last reconciled ownership record when // exact post-state is not proven. if (ownsRegistration && effectiveRegistration) { - persistGeneratedPolicyRegistration(sandboxName, effectiveRegistration); + persistGeneratedPolicyRegistration(sandboxName, effectiveRegistration, recheckAuthority); } if (options.bestEffort) return; const detail = ok ? `effective state: ${activeState}` : "the removal command failed"; throw new McpBridgeError(`Failed to remove generated MCP policy '${policyName}' (${detail}).`); } +export function removeGeneratedPolicy( + sandboxName: string, + entry: McpBridgeEntry, + options: { bestEffort?: boolean; preserveRegistryOwnership?: boolean } = {}, +): void { + try { + removeGeneratedPolicyStrict(sandboxName, entry, options); + } catch (error) { + if (error instanceof McpPolicyAuthorityRefusalError) throw error; + if (options.bestEffort && error instanceof McpBridgeError) return; + throw error; + } +} + export function getRegisteredGeneratedPolicy( sandboxName: string, entry: McpBridgeEntry | undefined, diff --git a/src/lib/actions/sandbox/mcp-bridge-provider-attachments.ts b/src/lib/actions/sandbox/mcp-bridge-provider-attachments.ts index eefa0691d02..85e467856af 100644 --- a/src/lib/actions/sandbox/mcp-bridge-provider-attachments.ts +++ b/src/lib/actions/sandbox/mcp-bridge-provider-attachments.ts @@ -117,11 +117,15 @@ function isRetryableSandboxMutationConflict(status: number | null, output: strin ); } -export function detachProvider( +export async function detachProvider( sandboxName: string, entry: McpBridgeEntry, - options: { allowLegacyGeneric?: boolean; bestEffort?: boolean } = {}, -): ProviderDetachOutcome { + options: { + allowLegacyGeneric?: boolean; + bestEffort?: boolean; + prepareMutation?: () => void | Promise; + } = {}, +): Promise { if (!entry.providerName) return "absent"; assertPersistedAuthenticatedBridgeEntry(entry); if (!entry.providerId) { @@ -156,6 +160,7 @@ export function detachProvider( `Provider attachment '${entry.providerName}' does not match MCP server '${entry.server}'. Expected stable provider ID '${entry.providerId}', found '${before.attachment.providerId ?? "missing"}', with credential keys '${before.attachment.credentialKeys.join(", ") || "none"}'.`, ); } + await options.prepareMutation?.(); const result = runOpenshellProviderCommand( ["sandbox", "provider", "detach", sandboxName, entry.providerName], { diff --git a/src/lib/actions/sandbox/mcp-bridge-provider-mutation.ts b/src/lib/actions/sandbox/mcp-bridge-provider-mutation.ts index 575223e5943..360c70c8638 100644 --- a/src/lib/actions/sandbox/mcp-bridge-provider-mutation.ts +++ b/src/lib/actions/sandbox/mcp-bridge-provider-mutation.ts @@ -8,7 +8,7 @@ * and mutation, and the server can merge against state NemoClaw did not approve. * A nonzero mutation result is ambiguous and always fails closed; NemoClaw never * infers success from a later resource-version increase. - * Randomized provider names, the MCP lifecycle lock, and mandatory + * Stable collision-resistant provider names, the MCP lifecycle lock, and mandatory * postinspection of immutable identity, credential shape, and resource version * constrain this TOCTOU boundary. Remove the compensation when OpenShell * exposes caller-supplied provider CAS or immutable provider IDs as mutation @@ -195,8 +195,8 @@ export function upsertMcpProvider( // compare-and-swap; v0.0.99 uses the version read inside the server but its // update CLI exposes no caller-supplied expected version. whyNotSourceFix: // NemoClaw cannot bind its preinspection to the upstream atomic mutation, so - // it uses randomized names, a lifecycle mutex, and immutable-ID/resource-version - // reinspection. + // it uses stable collision-resistant names, a lifecycle mutex, and + // immutable-ID/resource-version reinspection. // regressionTest: mcp-provider-ownership.test.ts simulates a concurrent // resource-version writer and requires the ambiguous update to fail closed. // removalCondition: use native immutable provider IDs or caller-supplied CAS diff --git a/src/lib/actions/sandbox/mcp-bridge-rebuild-exec-unavailable-policy-authority.test.ts b/src/lib/actions/sandbox/mcp-bridge-rebuild-exec-unavailable-policy-authority.test.ts new file mode 100644 index 00000000000..e817a395f00 --- /dev/null +++ b/src/lib/actions/sandbox/mcp-bridge-rebuild-exec-unavailable-policy-authority.test.ts @@ -0,0 +1,254 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { isPolicyAuthorityRefusalError } from "../../adapters/openshell/policy-authority"; +import type { McpBridgeEntry, SandboxEntry } from "../../state/registry"; + +const harness = vi.hoisted(() => ({ + assertAuthenticatedBridgeEntry: vi.fn(), + assertDestroyNotPending: vi.fn(), + assertDestroySnapshotCurrent: vi.fn(), + assertGeneratedPolicyExactReadOnly: vi.fn(), + assertNoProviderCredentialCollisions: vi.fn(), + bridgeState: vi.fn(), + buildRequiredPolicy: vi.fn(), + ensureSandboxGatewaySelected: vi.fn(), + getBridgeAdapter: vi.fn(), + getSandboxAgent: vi.fn(), + getSandboxOrThrow: vi.fn(), + inspectExactDestroyProvider: vi.fn(), + preflightEntryTargets: vi.fn(), + qualifyAuthority: vi.fn(), + resolveSandboxGatewayName: vi.fn(), + revalidateAuthority: vi.fn(), + validateSandboxName: vi.fn(), +})); + +vi.mock("../../onboard/gateway-binding", () => ({ + resolveSandboxGatewayName: harness.resolveSandboxGatewayName, +})); + +vi.mock("./mcp-bridge-destroy-preflight", () => ({ + assertMcpDestroySnapshotCurrent: harness.assertDestroySnapshotCurrent, + cloneMcpBridgeEntry: (entry: McpBridgeEntry) => structuredClone(entry), + inspectExactMcpDestroyProvider: harness.inspectExactDestroyProvider, +})); + +vi.mock("./mcp-bridge-policy", async (importOriginal) => { + const actual = await importOriginal(); + return { + assertGeneratedPolicyExactReadOnly: harness.assertGeneratedPolicyExactReadOnly, + buildRequiredMcpBridgePolicy: harness.buildRequiredPolicy, + McpPolicyAuthorityRefusalError: actual.McpPolicyAuthorityRefusalError, + qualifyMcpPolicyAuthorityReceipt: harness.qualifyAuthority, + revalidateContainingMcpPolicyAuthority: actual.revalidateContainingMcpPolicyAuthority, + revalidateMcpPolicyAuthorityReceipt: harness.revalidateAuthority, + }; +}); + +vi.mock("./mcp-bridge-provider", () => ({ + assertNoProviderCredentialCollisions: harness.assertNoProviderCredentialCollisions, + preflightMcpEntryTargets: harness.preflightEntryTargets, +})); + +vi.mock("./mcp-bridge-state", () => ({ + assertMcpDestroyNotPending: harness.assertDestroyNotPending, + bridgeState: harness.bridgeState, + ensureSandboxGatewaySelected: harness.ensureSandboxGatewaySelected, + getBridgeAdapter: harness.getBridgeAdapter, + getSandboxAgent: harness.getSandboxAgent, + getSandboxOrThrow: harness.getSandboxOrThrow, +})); + +vi.mock("./mcp-bridge-validation", () => ({ + assertAuthenticatedBridgeEntry: harness.assertAuthenticatedBridgeEntry, + validateSandboxName: harness.validateSandboxName, +})); + +const { prepareMcpBridgesForExecUnavailableRebuild } = + await import("./mcp-bridge-rebuild-exec-unavailable"); +const { McpPolicyAuthorityRefusalError } = await import("./mcp-bridge-policy"); + +const entry: McpBridgeEntry = { + server: "github", + agent: "openclaw", + adapter: "mcporter", + url: "https://mcp.example.test/server", + env: ["GITHUB_TOKEN"], + providerName: "alpha-mcp-github", + providerId: "11111111-2222-4333-8444-555555555555", + policyName: "mcp-bridge-github", + addedAt: "2026-08-24T00:00:00.000Z", +}; + +const sandbox: SandboxEntry = { + name: "alpha", + agent: "openclaw", + gatewayName: "nemoclaw", + policyAuthority: "externally-managed", + mcp: { bridges: { github: entry } }, +}; + +describe("exec-unavailable MCP rebuild policy authority", () => { + let authority: "externally-managed" | "nemoclaw-managed"; + let providerResourceVersion: number; + let targetAddress: string; + + beforeEach(() => { + vi.clearAllMocks(); + authority = "externally-managed"; + providerResourceVersion = 1; + targetAddress = "8.8.8.8"; + harness.getSandboxOrThrow.mockReturnValue(sandbox); + harness.bridgeState.mockImplementation((current: SandboxEntry) => current.mcp?.bridges ?? {}); + harness.getSandboxAgent.mockReturnValue({ name: "openclaw" }); + harness.getBridgeAdapter.mockReturnValue("mcporter"); + harness.resolveSandboxGatewayName.mockReturnValue("nemoclaw"); + harness.assertDestroySnapshotCurrent.mockReturnValue(sandbox); + harness.preflightEntryTargets.mockImplementation( + async () => new Map([[entry.server, { addresses: [targetAddress] }]]), + ); + harness.buildRequiredPolicy.mockImplementation( + (_entry: McpBridgeEntry, target: { addresses: readonly string[] }) => + `required:${target.addresses.join(",")}`, + ); + harness.qualifyAuthority.mockImplementation( + (options: { + operation: string; + requiredPolicyContents: readonly string[]; + sandboxName: string; + }) => ({ ...options, authority }), + ); + harness.revalidateAuthority.mockImplementation( + async ( + _receipt: unknown, + validateContainingReceipt?: () => Promise, + assertCurrentState?: () => void, + ) => { + await validateContainingReceipt?.(); + assertCurrentState?.(); + }, + ); + harness.assertGeneratedPolicyExactReadOnly.mockReturnValue({ + name: entry.policyName, + content: "managed policy", + sourcePath: "generated:nemoclaw-mcp-bridge", + }); + harness.inspectExactDestroyProvider.mockImplementation(() => ({ + exists: true, + id: entry.providerId, + resourceVersion: providerResourceVersion, + type: "nemoclaw-mcp-v1", + credentialKeys: entry.env, + })); + }); + + it("accepts exact external policy without NemoClaw attribution (#9833)", async () => { + const validateContainingPolicyReceipt = vi.fn(async () => undefined); + + const preparation = await prepareMcpBridgesForExecUnavailableRebuild( + "alpha", + validateContainingPolicyReceipt, + ); + await expect(preparation.revalidateBeforeDelete()).resolves.toBeUndefined(); + + expect(harness.qualifyAuthority).toHaveBeenCalledWith({ + operation: "preserve MCP bridges during host-side rebuild recovery for sandbox 'alpha'", + requiredPolicyContents: ["required:8.8.8.8"], + sandboxName: "alpha", + }); + expect(harness.assertGeneratedPolicyExactReadOnly).not.toHaveBeenCalled(); + expect(sandbox.customPolicies).toBeUndefined(); + expect(preparation.entries).toEqual([entry]); + expect(validateContainingPolicyReceipt).toHaveBeenCalled(); + }); + + it.each(["missing", "inconclusive"])( + "preserves canonical refusal when the external requirement is %s (#9833)", + async (state) => { + harness.qualifyAuthority.mockImplementationOnce(() => { + throw new McpPolicyAuthorityRefusalError(`external policy requirement is ${state}`); + }); + + let refusal: unknown; + try { + await prepareMcpBridgesForExecUnavailableRebuild("alpha"); + } catch (error) { + refusal = error; + } + + expect(isPolicyAuthorityRefusalError(refusal)).toBe(true); + expect(refusal).toEqual(expect.objectContaining({ message: expect.stringContaining(state) })); + expect(harness.inspectExactDestroyProvider).not.toHaveBeenCalled(); + expect(harness.assertGeneratedPolicyExactReadOnly).not.toHaveBeenCalled(); + }, + ); + + it("refuses external policy drift during delete-edge revalidation (#9833)", async () => { + const preparation = await prepareMcpBridgesForExecUnavailableRebuild("alpha"); + harness.revalidateAuthority.mockRejectedValueOnce( + new McpPolicyAuthorityRefusalError("external policy requirement drifted"), + ); + + await expect(preparation.revalidateBeforeDelete()).rejects.toSatisfy( + isPolicyAuthorityRefusalError, + ); + }); + + it("refuses target drift before host-side delete (#9833)", async () => { + const preparation = await prepareMcpBridgesForExecUnavailableRebuild("alpha"); + targetAddress = "9.9.9.9"; + + await expect(preparation.revalidateBeforeDelete()).rejects.toThrow( + "changed after host-side rebuild preflight", + ); + }); + + it("refuses provider drift before host-side delete (#9833)", async () => { + const preparation = await prepareMcpBridgesForExecUnavailableRebuild("alpha"); + providerResourceVersion = 2; + + await expect(preparation.revalidateBeforeDelete()).rejects.toThrow( + "changed after host-side rebuild preflight", + ); + }); + + it("refuses manifest drift before host-side delete (#9833)", async () => { + const preparation = await prepareMcpBridgesForExecUnavailableRebuild("alpha"); + harness.assertDestroySnapshotCurrent.mockImplementationOnce(() => { + throw new Error("MCP bridge definitions changed"); + }); + + await expect(preparation.revalidateBeforeDelete()).rejects.toThrow( + "MCP bridge definitions changed", + ); + }); + + it("retains generated ownership proof for managed authority (#9833)", async () => { + authority = "nemoclaw-managed"; + const managedSandbox = { ...sandbox, policyAuthority: authority }; + harness.getSandboxOrThrow.mockReturnValue(managedSandbox); + harness.assertDestroySnapshotCurrent.mockReturnValue(managedSandbox); + + const preparation = await prepareMcpBridgesForExecUnavailableRebuild("alpha"); + await expect(preparation.revalidateBeforeDelete()).resolves.toBeUndefined(); + + expect(harness.assertGeneratedPolicyExactReadOnly).toHaveBeenCalled(); + }); + + it("refuses managed recovery without exact generated ownership (#9833)", async () => { + authority = "nemoclaw-managed"; + const managedSandbox = { ...sandbox, policyAuthority: authority }; + harness.getSandboxOrThrow.mockReturnValue(managedSandbox); + harness.assertDestroySnapshotCurrent.mockReturnValue(managedSandbox); + harness.assertGeneratedPolicyExactReadOnly.mockImplementationOnce(() => { + throw new Error("generated policy ownership is missing"); + }); + + await expect(prepareMcpBridgesForExecUnavailableRebuild("alpha")).rejects.toThrow( + "generated policy ownership is missing", + ); + }); +}); diff --git a/src/lib/actions/sandbox/mcp-bridge-rebuild-exec-unavailable.ts b/src/lib/actions/sandbox/mcp-bridge-rebuild-exec-unavailable.ts index 2899a6dc2c7..92e401d312f 100644 --- a/src/lib/actions/sandbox/mcp-bridge-rebuild-exec-unavailable.ts +++ b/src/lib/actions/sandbox/mcp-bridge-rebuild-exec-unavailable.ts @@ -10,7 +10,14 @@ import { cloneMcpBridgeEntry, inspectExactMcpDestroyProvider, } from "./mcp-bridge-destroy-preflight"; -import { assertGeneratedPolicyExactReadOnly } from "./mcp-bridge-policy"; +import { + assertGeneratedPolicyExactReadOnly, + buildRequiredMcpBridgePolicy, + type McpPolicyAuthorityReceipt, + qualifyMcpPolicyAuthorityReceipt, + revalidateContainingMcpPolicyAuthority, + revalidateMcpPolicyAuthorityReceipt, +} from "./mcp-bridge-policy"; import { assertNoProviderCredentialCollisions, preflightMcpEntryTargets, @@ -140,27 +147,32 @@ async function inspectReadOnlyRecoveryState( sandboxName: string, entries: readonly McpBridgeEntry[], adapter: AgentMcpAdapter, + authority: McpPolicyAuthorityReceipt["authority"], + retainedTargets?: ReadonlyMap, ): Promise { - const resolvedTargets = await preflightMcpEntryTargets(entries); + const resolvedTargets = retainedTargets ?? (await preflightMcpEntryTargets(entries)); // This may start or recover the sandbox's recorded host gateway and select // it in CLI context. It does not mutate MCP ownership or sandbox contents; // the provider, policy, and target checks below remain inspection-only. - if (entries.length > 0) await ensureSandboxGatewaySelected(sandboxName); + if (!retainedTargets && entries.length > 0) await ensureSandboxGatewaySelected(sandboxName); const policyByServer = new Map(); const providerByServer = new Map(); const targetsByServer = new Map(); for (const entry of entries) { const target = resolvedTargets.get(entry.server); - const policy = assertGeneratedPolicyExactReadOnly( - sandboxName, - entry, - adapter, - target ?? { - addresses: [], - }, - ); - policyByServer.set(entry.server, policyFingerprint(policy)); + const validatedTarget = target ?? { addresses: [] }; + if (authority === "externally-managed") { + policyByServer.set(entry.server, buildRequiredMcpBridgePolicy(entry, validatedTarget)); + } else { + const policy = assertGeneratedPolicyExactReadOnly( + sandboxName, + entry, + adapter, + validatedTarget, + ); + policyByServer.set(entry.server, policyFingerprint(policy)); + } const provider = inspectExactMcpDestroyProvider(entry, { allowMissing: false }); providerByServer.set(entry.server, providerFingerprint(provider)); targetsByServer.set(entry.server, targetFingerprint(target)); @@ -220,20 +232,34 @@ async function revalidateBeforeDelete( expectedAgentName: string, expectedAdapter: AgentMcpAdapter, expectedValidation: ReadOnlyValidationSnapshot, + policyAuthorityReceipt: McpPolicyAuthorityReceipt, + validateContainingPolicyReceipt?: () => Promise, ): Promise { - assertDeleteEdgeUnchanged( - sandboxName, - expectedEntries, - expectedGatewayName, - expectedAgentName, - expectedAdapter, + const assertCurrentState = () => + assertDeleteEdgeUnchanged( + sandboxName, + expectedEntries, + expectedGatewayName, + expectedAgentName, + expectedAdapter, + ); + await revalidateMcpPolicyAuthorityReceipt( + policyAuthorityReceipt, + validateContainingPolicyReceipt, + assertCurrentState, ); const currentValidation = await inspectReadOnlyRecoveryState( sandboxName, expectedEntries, expectedAdapter, + policyAuthorityReceipt.authority, ); assertValidationSnapshotCurrent(expectedEntries, expectedValidation, currentValidation); + await revalidateMcpPolicyAuthorityReceipt( + policyAuthorityReceipt, + validateContainingPolicyReceipt, + assertCurrentState, + ); } /** @@ -246,13 +272,51 @@ async function revalidateBeforeDelete( */ export async function prepareMcpBridgesForExecUnavailableRebuild( sandboxName: string, + validateContainingPolicyReceipt?: () => Promise, ): Promise { const { entries, gatewayName, agentName, adapter } = snapshotCompleteEntries(sandboxName); const expectedEntries = entries.map(cloneMcpBridgeEntry); + if (expectedEntries.length === 0) { + return { + entries: [], + detachedProviderEntries: [], + scrubbedAdapterEntries: [], + revalidateBeforeDelete: async () => { + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); + assertDeleteEdgeUnchanged(sandboxName, expectedEntries, gatewayName, agentName, adapter); + }, + assertDeleteEdgeUnchanged: () => + assertDeleteEdgeUnchanged(sandboxName, expectedEntries, gatewayName, agentName, adapter), + }; + } + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); + const resolvedTargets = await preflightMcpEntryTargets(expectedEntries); + await ensureSandboxGatewaySelected(sandboxName); + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); + const policyAuthorityReceipt = qualifyMcpPolicyAuthorityReceipt({ + operation: `preserve MCP bridges during host-side rebuild recovery for sandbox '${sandboxName}'`, + requiredPolicyContents: expectedEntries.map((entry) => { + const target = resolvedTargets.get(entry.server); + if (!target) { + throw new McpBridgeError( + `MCP server '${entry.server}' has no validated address pins. Refusing host-side rebuild recovery.`, + ); + } + return buildRequiredMcpBridgePolicy(entry, target); + }), + sandboxName, + }); const expectedValidation = await inspectReadOnlyRecoveryState( sandboxName, expectedEntries, adapter, + policyAuthorityReceipt.authority, + resolvedTargets, + ); + await revalidateMcpPolicyAuthorityReceipt( + policyAuthorityReceipt, + validateContainingPolicyReceipt, + () => assertDeleteEdgeUnchanged(sandboxName, expectedEntries, gatewayName, agentName, adapter), ); return { entries: entries.map(cloneMcpBridgeEntry), @@ -266,6 +330,8 @@ export async function prepareMcpBridgesForExecUnavailableRebuild( agentName, adapter, expectedValidation, + policyAuthorityReceipt, + validateContainingPolicyReceipt, ), assertDeleteEdgeUnchanged: () => assertDeleteEdgeUnchanged(sandboxName, expectedEntries, gatewayName, agentName, adapter), diff --git a/src/lib/actions/sandbox/mcp-bridge-rebuild-policy-authority.test.ts b/src/lib/actions/sandbox/mcp-bridge-rebuild-policy-authority.test.ts index 4203f830675..4d7030ffe9d 100644 --- a/src/lib/actions/sandbox/mcp-bridge-rebuild-policy-authority.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-rebuild-policy-authority.test.ts @@ -9,6 +9,7 @@ const mocks = vi.hoisted(() => ({ assertAdapterConfigMutationsAllowed: vi.fn(), assertAdapterTeardownRuntimeCapabilities: vi.fn(), assertDestroyNotPending: vi.fn(), + assertDestroySnapshotCurrent: vi.fn(), assertGeneratedPolicyMutationSafe: vi.fn(), assertNoProviderCredentialCollisions: vi.fn(), assertProviderRecoverable: vi.fn(), @@ -36,16 +37,36 @@ vi.mock("./mcp-bridge-adapter-teardown", () => ({ })); vi.mock("./mcp-bridge-destroy", () => ({ + assertMcpDestroySnapshotCurrent: mocks.assertDestroySnapshotCurrent, cloneMcpBridgeEntry: (entry: McpBridgeEntry) => structuredClone(entry), discardSafeIncompleteMcpAdds: mocks.discardSafeIncompleteAdds, inspectExactMcpDestroyProvider: mocks.inspectExactDestroyProvider, })); -vi.mock("./mcp-bridge-policy", () => ({ - assertGeneratedPolicyMutationSafe: mocks.assertGeneratedPolicyMutationSafe, - assertGeneratedPolicyRegistrationMutationSafe: vi.fn(), - removeGeneratedPolicy: mocks.removeGeneratedPolicy, -})); +vi.mock("./mcp-bridge-policy", async (importOriginal) => { + const actual = await importOriginal(); + return { + assertGeneratedPolicyMutationSafe: mocks.assertGeneratedPolicyMutationSafe, + assertGeneratedPolicyRegistrationMutationSafe: vi.fn(), + buildRequiredMcpBridgePolicy: vi.fn(() => "required policy"), + McpPolicyAuthorityRefusalError: actual.McpPolicyAuthorityRefusalError, + qualifyMcpPolicyAuthorityReceipt: (options: { + operation: string; + requiredPolicyContents: readonly string[]; + sandboxName: string; + }) => ({ ...options, authority: "externally-managed" as const }), + removeGeneratedPolicy: mocks.removeGeneratedPolicy, + revalidateContainingMcpPolicyAuthority: actual.revalidateContainingMcpPolicyAuthority, + revalidateMcpPolicyAuthorityReceipt: async ( + _receipt: unknown, + validateContainingReceipt?: () => Promise, + assertCurrentState?: () => void, + ) => { + await actual.revalidateContainingMcpPolicyAuthority(validateContainingReceipt); + assertCurrentState?.(); + }, + }; +}); vi.mock("./mcp-bridge-provider", () => ({ assertMcpProviderRecoverable: mocks.assertProviderRecoverable, @@ -78,6 +99,7 @@ vi.mock("./mcp-bridge-state", () => ({ const { prepareMcpBridgesForRebuild, restoreMcpBridgesAfterRebuild } = await import("./mcp-bridge-rebuild"); +const { McpPolicyAuthorityRefusalError } = await import("./mcp-bridge-policy"); let entry: McpBridgeEntry; let sandbox: SandboxEntry; @@ -106,7 +128,11 @@ describe("MCP rebuild policy authority", () => { mocks.bridgeState.mockImplementation((current: SandboxEntry) => current.mcp?.bridges ?? {}); mocks.discardSafeIncompleteAdds.mockResolvedValue(sandbox); mocks.detachProvider.mockReturnValue("detached"); + mocks.preflightEntryTargets.mockImplementation(async (entries: readonly McpBridgeEntry[]) => + new Map(entries.map((current) => [current.server, { addresses: ["8.8.8.8"] }])), + ); mocks.rollbackScrubbedAdapters.mockReturnValue([]); + mocks.assertDestroySnapshotCurrent.mockReturnValue(sandbox); mocks.scrubAdapter.mockImplementation((_name, _sandbox, current: McpBridgeEntry) => ({ ...current, credentialRevision: "v1", @@ -134,15 +160,18 @@ describe("MCP rebuild policy authority", () => { ); }); - it("preserves the authority refusal before MCP teardown mutation (#9833)", async () => { + it("reports a containing authority refusal before MCP teardown mutation (#9833)", async () => { const refusal = new PolicyAuthorityRefusalError("policy authority changed"); const validatePolicyAuthority = vi .fn() .mockResolvedValueOnce(undefined) .mockRejectedValueOnce(refusal); - await expect(prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority)).rejects.toBe( - refusal, + await expect(prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority)).rejects.toEqual( + expect.objectContaining({ + message: refusal.message, + name: McpPolicyAuthorityRefusalError.name, + }), ); expect(mocks.scrubAdapter).not.toHaveBeenCalled(); @@ -159,16 +188,24 @@ describe("MCP rebuild policy authority", () => { .mockResolvedValueOnce(undefined) .mockRejectedValueOnce(refusal); - await expect(prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority)).rejects.toBe( - refusal, + await expect(prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority)).rejects.toEqual( + expect.objectContaining({ + message: refusal.message, + name: McpPolicyAuthorityRefusalError.name, + }), ); - expect(mocks.rollbackScrubbedAdapters).toHaveBeenCalledExactlyOnceWith("alpha", sandbox, [ - expect.objectContaining({ server: "github", credentialRevision: "v1" }), - ]); + expect(mocks.restoreRuntime).toHaveBeenCalledExactlyOnceWith( + "alpha", + [expect.objectContaining({ server: "github", credentialRevision: "v1" })], + expect.objectContaining({ + lifecyclePhase: "teardown-rollback", + teardownPolicyAuthorityRefusal: expect.objectContaining({ message: refusal.message }), + }), + ); + expect(mocks.rollbackScrubbedAdapters).not.toHaveBeenCalled(); expect(mocks.removeGeneratedPolicy).not.toHaveBeenCalled(); expect(mocks.detachProvider).not.toHaveBeenCalled(); - expect(mocks.restoreRuntime).not.toHaveBeenCalled(); }); it("reattaches a detached provider when authority changes before the next detach (#9833)", async () => { @@ -190,23 +227,27 @@ describe("MCP rebuild policy authority", () => { .mockResolvedValueOnce(undefined) .mockRejectedValueOnce(refusal); - await expect(prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority)).rejects.toBe( - refusal, + await expect(prepareMcpBridgesForRebuild("alpha", validatePolicyAuthority)).rejects.toEqual( + expect.objectContaining({ + message: refusal.message, + name: McpPolicyAuthorityRefusalError.name, + }), ); - expect(mocks.attachProvider).toHaveBeenCalledExactlyOnceWith("alpha", entry); - expect(mocks.refreshProviderEnvironment).toHaveBeenCalledExactlyOnceWith(entry); - expect(mocks.waitForAttachedCredential).toHaveBeenCalledExactlyOnceWith("alpha", entry); - expect(mocks.rollbackScrubbedAdapters).toHaveBeenCalledWith( + expect(mocks.restoreRuntime).toHaveBeenCalledWith( "alpha", - sandbox, expect.arrayContaining([ expect.objectContaining({ server: "github", credentialRevision: "v1" }), expect.objectContaining({ server: "gitlab", credentialRevision: "v1" }), ]), + expect.objectContaining({ + lifecyclePhase: "teardown-rollback", + teardownPolicyAuthorityRefusal: expect.objectContaining({ message: refusal.message }), + }), ); + expect(mocks.attachProvider).not.toHaveBeenCalled(); + expect(mocks.rollbackScrubbedAdapters).not.toHaveBeenCalled(); expect(mocks.removeGeneratedPolicy).not.toHaveBeenCalled(); - expect(mocks.restoreRuntime).not.toHaveBeenCalled(); }); it("revalidates between registry recovery and runtime restoration (#9833)", async () => { @@ -218,7 +259,12 @@ describe("MCP rebuild policy authority", () => { await expect( restoreMcpBridgesAfterRebuild("alpha", [entry], validatePolicyAuthority), - ).rejects.toBe(refusal); + ).rejects.toEqual( + expect.objectContaining({ + message: refusal.message, + name: McpPolicyAuthorityRefusalError.name, + }), + ); expect(mocks.setBridgeState).toHaveBeenCalledOnce(); expect(mocks.restoreRuntime).not.toHaveBeenCalled(); diff --git a/src/lib/actions/sandbox/mcp-bridge-rebuild.ts b/src/lib/actions/sandbox/mcp-bridge-rebuild.ts index 4ce8e992bce..08a9e113ded 100644 --- a/src/lib/actions/sandbox/mcp-bridge-rebuild.ts +++ b/src/lib/actions/sandbox/mcp-bridge-rebuild.ts @@ -2,10 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 import type { McpBridgeEntry } from "../../state/registry"; -import { - isPolicyAuthorityRefusalError, - PolicyAuthorityRefusalError, -} from "../../adapters/openshell/policy-authority"; import { rollbackScrubbedMcpAdapters, scrubManagedMcpAdapterOrThrow, @@ -13,6 +9,7 @@ import { } from "./mcp-bridge-adapter-teardown"; import { McpBridgeError } from "./mcp-bridge-contracts"; import { + assertMcpDestroySnapshotCurrent, cloneMcpBridgeEntry, discardSafeIncompleteMcpAdds, inspectExactMcpDestroyProvider, @@ -20,7 +17,12 @@ import { import { assertGeneratedPolicyMutationSafe, assertGeneratedPolicyRegistrationMutationSafe, + buildRequiredMcpBridgePolicy, + McpPolicyAuthorityRefusalError, + qualifyMcpPolicyAuthorityReceipt, removeGeneratedPolicy, + revalidateContainingMcpPolicyAuthority, + revalidateMcpPolicyAuthorityReceipt, } from "./mcp-bridge-policy"; import { assertMcpProviderRecoverable, @@ -57,32 +59,12 @@ export interface McpRebuildPreparation { assertDeleteEdgeUnchanged?: () => void; } -function compensatePolicyAuthorityRefusal( - sandboxName: string, - sandbox: ReturnType, - detachedEntries: readonly McpBridgeEntry[], - scrubbedAdapterEntries: readonly McpScrubbedAdapterEntry[], -): string[] { - const failures: string[] = []; - for (const entry of detachedEntries) { - try { - attachProvider(sandboxName, entry); - refreshMcpProviderEnvironment(entry); - waitForAttachedMcpCredential(sandboxName, entry); - } catch (error) { - failures.push(error instanceof Error ? error.message : String(error)); - } - } - failures.push(...rollbackScrubbedMcpAdapters(sandboxName, sandbox, scrubbedAdapterEntries)); - return failures; -} - export { prepareMcpBridgesForExecUnavailableRebuild } from "./mcp-bridge-rebuild-exec-unavailable"; async function getCompleteMcpRebuildEntries( sandboxName: string, options: { sandboxAbsent?: boolean } = {}, - validatePolicyAuthority?: () => Promise, + validateContainingPolicyReceipt?: () => Promise, ): Promise { validateSandboxName(sandboxName); const currentSandbox = getSandboxOrThrow(sandboxName); @@ -101,7 +83,7 @@ async function getCompleteMcpRebuildEntries( entriesRequiringExternalCleanup, ); } - await validatePolicyAuthority?.(); + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); const sandbox = await discardSafeIncompleteMcpAdds(sandboxName, currentSandbox, options); const entries = Object.values(bridgeState(sandbox)).map(cloneMcpBridgeEntry); const incompleteAdd = entries.find((entry) => entry.addState); @@ -146,13 +128,13 @@ export async function prepareMcpBridgesForAbsentSandboxRebuild( export async function prepareMcpBridgesForRebuild( sandboxName: string, - validatePolicyAuthority?: () => Promise, + validateContainingPolicyReceipt?: () => Promise, ): Promise { const sandbox = getSandboxOrThrow(sandboxName); const entries = await getCompleteMcpRebuildEntries( sandboxName, undefined, - validatePolicyAuthority, + validateContainingPolicyReceipt, ); if (entries.length === 0) { return { @@ -161,30 +143,53 @@ export async function prepareMcpBridgesForRebuild( scrubbedAdapterEntries: [], }; } - await preflightMcpEntryTargets(entries); + const resolvedTargets = await preflightMcpEntryTargets(entries); + const policyAuthorityReceipt = qualifyMcpPolicyAuthorityReceipt({ + operation: `prepare MCP bridges before rebuilding sandbox '${sandboxName}'`, + requiredPolicyContents: entries.map((entry) => { + const target = resolvedTargets.get(entry.server); + if (!target) { + throw new McpBridgeError( + `MCP server '${entry.server}' has no validated address pins. Refusing rebuild preparation.`, + ); + } + return buildRequiredMcpBridgePolicy(entry, target); + }), + sandboxName, + }); + const revalidateBeforeMutation = async (): Promise => { + await revalidateMcpPolicyAuthorityReceipt( + policyAuthorityReceipt, + validateContainingPolicyReceipt, + () => assertMcpDestroySnapshotCurrent(sandboxName, entries), + ); + }; await ensureSandboxGatewaySelected(sandboxName); - for (const entry of entries) assertGeneratedPolicyMutationSafe(sandboxName, entry); + if (policyAuthorityReceipt.authority === "nemoclaw-managed") { + for (const entry of entries) assertGeneratedPolicyMutationSafe(sandboxName, entry); + } assertMcpAdapterTeardownRuntimeCapabilities(sandboxName, sandbox, entries); for (const entry of entries) assertMcpProviderRecoverable(entry); assertNoProviderCredentialCollisions(sandboxName, entries); const detached: McpBridgeEntry[] = []; const scrubbedAdapters: McpScrubbedAdapterEntry[] = []; const removedPolicies: McpBridgeEntry[] = []; + let providerDetachAttempted = false; try { for (const entry of entries) { // `/sandbox` may be a retained PVC. Scrub before delete so a replacement // Hermes/agent cannot boot with a stale placeholder while its provider // is intentionally detached during recreate. - await validatePolicyAuthority?.(); + await revalidateBeforeMutation(); scrubbedAdapters.push(scrubManagedMcpAdapterOrThrow(sandboxName, sandbox, entry)); } - for (const entry of entries) { - // The same-name replacement journal fingerprints this source row before - // MCP teardown. Keep exact generated-policy ownership in that preserved - // row while removing only the live policy; inner onboarding excludes the - // generated name and post-rebuild restoration reuses this ownership. - if (sandbox.policyAuthority !== "externally-managed") { - await validatePolicyAuthority?.(); + if (policyAuthorityReceipt.authority === "nemoclaw-managed") { + for (const entry of entries) { + // The same-name replacement journal fingerprints this source row before + // MCP teardown. Keep exact generated-policy ownership in that preserved + // row while removing only the live policy; inner onboarding excludes the + // generated name and post-rebuild restoration reuses this ownership. + await revalidateBeforeMutation(); removeGeneratedPolicy(sandboxName, entry, { preserveRegistryOwnership: true }); removedPolicies.push(entry); } @@ -192,9 +197,12 @@ export async function prepareMcpBridgesForRebuild( for (const entry of entries) { // Keep the provider and its host-only credentials for the replacement // sandbox, but detach it before OpenShell deletes the old attachment. - await validatePolicyAuthority?.(); + await revalidateBeforeMutation(); inspectExactMcpDestroyProvider(entry, { allowMissing: false }); - const detachOutcome = detachProvider(sandboxName, entry); + providerDetachAttempted = true; + const detachOutcome = await detachProvider(sandboxName, entry, { + prepareMutation: revalidateBeforeMutation, + }); if (detachOutcome === "unknown") { throw new McpBridgeError( `Could not prove provider detach for MCP server '${entry.server}'.`, @@ -207,27 +215,23 @@ export async function prepareMcpBridgesForRebuild( detached.push(entry); } } catch (error) { - if (isPolicyAuthorityRefusalError(error)) { - const rollbackFailures = compensatePolicyAuthorityRefusal( - sandboxName, - sandbox, - detached, - scrubbedAdapters, - ); - if (rollbackFailures.length === 0) throw error; - const detail = error instanceof Error ? error.message : String(error); - throw new PolicyAuthorityRefusalError( - `${detail}\nMCP rebuild rollback could not reattach: ${rollbackFailures.join("; ")}`, - error instanceof PolicyAuthorityRefusalError ? error.observedAuthority : undefined, - { cause: error }, - ); - } const rollbackFailures: string[] = []; let runtimeRestored = false; - if (removedPolicies.length > 0) { + let snapshotCurrent = true; + try { + assertMcpDestroySnapshotCurrent(sandboxName, entries); + } catch (snapshotError) { + snapshotCurrent = false; + rollbackFailures.push( + snapshotError instanceof Error ? snapshotError.message : String(snapshotError), + ); + } + if (snapshotCurrent && scrubbedAdapters.length > 0) { try { - await validatePolicyAuthority?.(); - await restoreExistingMcpBridgeRuntime(sandboxName, removedPolicies, { + await restoreExistingMcpBridgeRuntime(sandboxName, scrubbedAdapters, { + ...(error instanceof McpPolicyAuthorityRefusalError + ? { teardownPolicyAuthorityRefusal: error } + : {}), lifecyclePhase: "teardown-rollback", }); runtimeRestored = true; @@ -237,19 +241,30 @@ export async function prepareMcpBridgesForRebuild( ); } } - if (!runtimeRestored) { - try { - await validatePolicyAuthority?.(); - rollbackFailures.push( - ...rollbackScrubbedMcpAdapters(sandboxName, sandbox, scrubbedAdapters), - ); - } catch (rollbackError) { - rollbackFailures.push( - rollbackError instanceof Error ? rollbackError.message : String(rollbackError), - ); - } + if ( + snapshotCurrent && + !runtimeRestored && + removedPolicies.length === 0 && + !providerDetachAttempted && + !(error instanceof McpPolicyAuthorityRefusalError) + ) { + rollbackFailures.push( + ...(await rollbackScrubbedMcpAdapters( + sandboxName, + sandbox, + scrubbedAdapters, + revalidateBeforeMutation, + )), + ); } const detail = error instanceof Error ? error.message : String(error); + if (error instanceof McpPolicyAuthorityRefusalError) { + throw new McpPolicyAuthorityRefusalError( + rollbackFailures.length > 0 + ? `${detail}\nMCP rebuild compensation remains pending: ${rollbackFailures.join("; ")}` + : detail, + ); + } throw new McpBridgeError( rollbackFailures.length > 0 ? `${detail}\nMCP rebuild rollback could not reattach: ${rollbackFailures.join("; ")}` @@ -267,9 +282,16 @@ export async function reattachMcpProvidersAfterRebuildAbort( sandboxName: string, entries: readonly McpBridgeEntry[], scrubbedAdapterEntries: readonly McpScrubbedAdapterEntry[] = [], - validatePolicyAuthority?: () => Promise, + validateContainingPolicyReceipt?: () => Promise, ): Promise { if (entries.length === 0 && scrubbedAdapterEntries.length === 0) return; + let authorityRefusal: McpPolicyAuthorityRefusalError | undefined; + try { + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); + } catch (error) { + if (!(error instanceof McpPolicyAuthorityRefusalError)) throw error; + authorityRefusal = error; + } await ensureSandboxGatewaySelected(sandboxName); const sandbox = getSandboxOrThrow(sandboxName); assertMcpAdapterTeardownRuntimeCapabilities(sandboxName, sandbox, [ @@ -281,28 +303,38 @@ export async function reattachMcpProvidersAfterRebuildAbort( let runtimeRestored = false; if (entries.length > 0) { try { - await validatePolicyAuthority?.(); await restoreExistingMcpBridgeRuntime(sandboxName, entries, { + ...(authorityRefusal ? { teardownPolicyAuthorityRefusal: authorityRefusal } : {}), lifecyclePhase: "teardown-rollback", + ...(authorityRefusal ? {} : { validateContainingPolicyReceipt }), }); runtimeRestored = true; } catch (error) { failures.push(error instanceof Error ? error.message : String(error)); } } - if (!runtimeRestored) { - await validatePolicyAuthority?.(); - failures.push(...rollbackScrubbedMcpAdapters(sandboxName, sandbox, scrubbedAdapterEntries)); + if (!runtimeRestored && !authorityRefusal) { + failures.push( + ...(await rollbackScrubbedMcpAdapters(sandboxName, sandbox, scrubbedAdapterEntries, () => + revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt), + )), + ); } if (failures.length > 0) { + if (authorityRefusal) { + throw new McpPolicyAuthorityRefusalError( + `${authorityRefusal.message}\nMCP rebuild-abort compensation remains pending: ${failures.join("; ")}`, + ); + } throw new McpBridgeError(failures.join("; ")); } + if (authorityRefusal) throw authorityRefusal; } export async function restoreMcpBridgesAfterRebuild( sandboxName: string, entries: readonly McpBridgeEntry[], - validatePolicyAuthority?: () => Promise, + validateContainingPolicyReceipt?: () => Promise, ): Promise { if (entries.length === 0) return; for (const entry of entries) assertAuthenticatedBridgeEntry(entry); @@ -311,9 +343,11 @@ export async function restoreMcpBridgesAfterRebuild( ); // Persist the recovery contract before touching the gateway. If refresh // fails, `mcp restart` remains retryable after the operator fixes the cause. - await validatePolicyAuthority?.(); + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); setBridgeState(sandboxName, bridges); - await validatePolicyAuthority?.(); - await restoreExistingMcpBridgeRuntime(sandboxName, entries); - await validatePolicyAuthority?.(); + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); + await restoreExistingMcpBridgeRuntime(sandboxName, entries, { + validateContainingPolicyReceipt, + }); + await revalidateContainingMcpPolicyAuthority(validateContainingPolicyReceipt); } diff --git a/src/lib/actions/sandbox/mcp-bridge-remove.ts b/src/lib/actions/sandbox/mcp-bridge-remove.ts index 6d7c956ed05..83b454c483c 100644 --- a/src/lib/actions/sandbox/mcp-bridge-remove.ts +++ b/src/lib/actions/sandbox/mcp-bridge-remove.ts @@ -1,6 +1,8 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import { isDeepStrictEqual } from "node:util"; + import type { AgentMcpAdapter } from "../../agent/defs"; import { withMcpLifecycleLock } from "../../state/mcp-lifecycle-lock"; import { assertHermesPortableCommandUnavailable } from "../../onboard/experimental/portable-agent-lifecycle"; @@ -12,12 +14,20 @@ import { } from "./mcp-bridge-adapters"; import { isAgentMcpAdapter, McpBridgeError } from "./mcp-bridge-contracts"; import { assertHermesMcpRuntimeIntent } from "./mcp-bridge-hermes-reconciliation"; -import { assertGeneratedPolicyMutationSafe, removeGeneratedPolicy } from "./mcp-bridge-policy"; +import { + buildMcpBridgePolicyKey, + buildRequiredMcpBridgePolicy, + McpPolicyAuthorityRefusalError, + qualifyMcpPolicyAuthorityReceipt, + removeGeneratedPolicy, + revalidateMcpPolicyAuthorityReceipt, +} from "./mcp-bridge-policy"; import { deleteProvider, detachMissingProviderReference, detachProvider, inspectMcpProvider, + preflightMcpEntryTargets, providerMatchesManagedCredential, providerShapeDetail, waitForDetachedMcpCredential, @@ -40,6 +50,73 @@ import { validateSandboxName, } from "./mcp-bridge-validation"; +function rethrowMcpPolicyAuthorityRefusal(error: unknown): void { + if (error instanceof McpPolicyAuthorityRefusalError) throw error; +} + +function assertMcpRemoveSnapshotCurrent( + sandboxName: string, + expectedBridges: Readonly>, +): void { + const currentBridges = bridgeState(getSandboxOrThrow(sandboxName)); + if (!isDeepStrictEqual(currentBridges, expectedBridges)) { + throw new McpBridgeError( + `MCP bridge definitions changed while server removal was in progress on sandbox '${sandboxName}'. The current manifest was preserved; retry removal against the current definition.`, + ); + } +} + +async function buildRequiredMcpRemovePolicy(entry: McpBridgeEntry): Promise { + const resolvedTargets = await preflightMcpEntryTargets([entry]); + const target = resolvedTargets.get(entry.server); + if (!target) { + throw new McpBridgeError( + `MCP server '${entry.server}' has no validated address pins. Refusing server removal.`, + ); + } + return buildRequiredMcpBridgePolicy(entry, target); +} + +function buildMcpRemoveAuthorityProbePolicy(entry: McpBridgeEntry): string { + return `network_policies:\n ${buildMcpBridgePolicyKey(entry.server)}: {}\n`; +} + +async function qualifyMcpRemovePolicyAuthority(sandboxName: string, entry: McpBridgeEntry) { + const operation = `remove MCP server '${entry.server}'`; + const qualifyExternalPolicy = async () => + qualifyMcpPolicyAuthorityReceipt({ + operation, + requiredPolicyContents: [await buildRequiredMcpRemovePolicy(entry)], + sandboxName, + }); + + if (getSandboxOrThrow(sandboxName).policyAuthority === "externally-managed") { + return qualifyExternalPolicy(); + } + + // This valid empty entry authorizes no endpoint or binary. Managed entries + // can qualify authority without resolving a URL they will only remove. + // Legacy external authority is persisted before requirement verification; + // retry that refusal with the exact derived requirement. + let receipt; + try { + receipt = qualifyMcpPolicyAuthorityReceipt({ + operation, + requiredPolicyContents: [buildMcpRemoveAuthorityProbePolicy(entry)], + sandboxName, + }); + } catch (error) { + if ( + !(error instanceof McpPolicyAuthorityRefusalError) || + getSandboxOrThrow(sandboxName).policyAuthority !== "externally-managed" + ) { + throw error; + } + return qualifyExternalPolicy(); + } + return receipt.authority === "externally-managed" ? qualifyExternalPolicy() : receipt; +} + function requiresProviderDetachBeforeAdapterCleanup(entry: McpBridgeEntry): boolean { assertPersistedAuthenticatedBridgeEntry(entry); try { @@ -186,12 +263,26 @@ async function removeMcpBridgeUnlocked( console.log(` No MCP server '${server}' is registered on sandbox '${sandboxName}'.`); return "noMatchingEntry"; } + const bridgeSnapshot = structuredClone(currentBridges); + const remainingBridgeSnapshot = structuredClone(currentBridges); + delete remainingBridgeSnapshot[server]; + const policyAuthorityReceipt = await qualifyMcpRemovePolicyAuthority(sandboxName, entry); + const revalidateBeforeMutation = () => + revalidateMcpPolicyAuthorityReceipt(policyAuthorityReceipt, undefined, () => + assertMcpRemoveSnapshotCurrent(sandboxName, bridgeSnapshot), + ); + const revalidateBeforeSuccess = () => + revalidateMcpPolicyAuthorityReceipt(policyAuthorityReceipt, undefined, () => + assertMcpRemoveSnapshotCurrent(sandboxName, remainingBridgeSnapshot), + ); if (entry.addState === "prepared") { // `prepared` is persisted before gateway selection and is advanced only // after adapter/provider/policy absence has been proven. It therefore owns // no external resources and can be cancelled without touching same-name // state another workflow may own. + await revalidateBeforeMutation(); removeBridgeEntry(sandboxName, server); + await revalidateBeforeSuccess(); console.log(` Cancelled incomplete MCP add for '${server}' on sandbox '${sandboxName}'.`); return "cancelledPreparedAdd"; } @@ -209,8 +300,8 @@ async function removeMcpBridgeUnlocked( // performs its host-side shields preflight here, before any provider, policy, // attachment, or adapter side effect. assertAgentMcpConfigMutationAllowed(sandboxName, adapter); + await revalidateBeforeMutation(); await ensureSandboxGatewaySelected(sandboxName); - assertGeneratedPolicyMutationSafe(sandboxName, entry); const failures: string[] = []; let providerOwnershipProved = !entry.providerName; let providerWasMissing = false; @@ -268,9 +359,11 @@ async function removeMcpBridgeUnlocked( detachBeforeAdapterCleanup ) { try { + await revalidateBeforeMutation(); detachMissingProviderReference(sandboxName, entry); missingProviderReferenceDetached = true; } catch (error) { + rethrowMcpPolicyAuthorityRefusal(error); const detail = error instanceof Error ? error.message : String(error); if (!options.force) throw new McpBridgeError(detail); failures.push(detail); @@ -284,7 +377,10 @@ async function removeMcpBridgeUnlocked( ? missingProviderReferenceDetached ? "detached" : "unknown" - : detachProvider(sandboxName, entry, { allowLegacyGeneric: true }); + : await detachProvider(sandboxName, entry, { + allowLegacyGeneric: true, + prepareMutation: revalidateBeforeMutation, + }); providerDetachedBeforeAdapterCleanup = detachOutcome !== "unknown"; if (!providerDetachedBeforeAdapterCleanup) { throw new McpBridgeError( @@ -292,6 +388,7 @@ async function removeMcpBridgeUnlocked( ); } } catch (error) { + rethrowMcpPolicyAuthorityRefusal(error); const detail = error instanceof Error ? error.message : String(error); if (!options.force) throw new McpBridgeError(detail); failures.push(detail); @@ -311,6 +408,7 @@ async function removeMcpBridgeUnlocked( // retains its helper/lifecycle validation; Deep Agents intentionally // skips only the marker that an older image cannot expose. assertAgentMcpTeardownRuntimeCapability(sandboxName, adapter); + await revalidateBeforeMutation(); const adapterRemoval = unregisterAgentAdapter( sandboxName, (entry.adapter as AgentMcpAdapter | undefined) ?? adapter, @@ -336,43 +434,51 @@ async function removeMcpBridgeUnlocked( }); } } catch (error) { + rethrowMcpPolicyAuthorityRefusal(error); const detail = error instanceof Error ? error.message : String(error); if (!options.force) throw new McpBridgeError(detail); adapterCleanupProved = false; failures.push(detail); } } - let policyCleanupProved = false; + let policyCleanupProved = policyAuthorityReceipt.authority === "externally-managed"; if (adapterCleanupProved) { - try { - removeGeneratedPolicy(sandboxName, entry); - policyCleanupProved = true; - } catch (error) { - const detail = error instanceof Error ? error.message : String(error); - if (!options.force) throw new McpBridgeError(detail); - failures.push(detail); + if (policyAuthorityReceipt.authority !== "externally-managed") { + try { + // This is the only cleanup step that mutates live policy. An authority + // refusal stops cleanup and preserves the bridge manifest for retry. + await revalidateBeforeMutation(); + removeGeneratedPolicy(sandboxName, entry); + policyCleanupProved = true; + } catch (error) { + rethrowMcpPolicyAuthorityRefusal(error); + const detail = error instanceof Error ? error.message : String(error); + if (!options.force) throw new McpBridgeError(detail); + failures.push(detail); + } } } - let reservationCleanupProved = !entry.providerName && adapterCleanupProved && policyCleanupProved; - if ( - adapterCleanupProved && - policyCleanupProved && - providerOwnershipProved && - entry.providerName - ) { + let reservationCleanupProved = !entry.providerName && adapterCleanupProved; + if (adapterCleanupProved && providerOwnershipProved && entry.providerName) { try { // OpenShell main cannot list a sandbox whose spec references a missing // provider. Remove that dangling name directly before using the normal // table-backed detach path for a provider that still exists. let detachOutcome; if (providerWasMissing) { - detachOutcome = missingProviderReferenceDetached - ? "detached" - : detachMissingProviderReference(sandboxName, entry); + if (missingProviderReferenceDetached) { + detachOutcome = "detached"; + } else { + await revalidateBeforeMutation(); + detachOutcome = detachMissingProviderReference(sandboxName, entry); + } } else { detachOutcome = providerDetachedBeforeAdapterCleanup ? "detached" - : detachProvider(sandboxName, entry, { allowLegacyGeneric: true }); + : await detachProvider(sandboxName, entry, { + allowLegacyGeneric: true, + prepareMutation: revalidateBeforeMutation, + }); } if (detachOutcome !== "unknown") { // A missing provider has no credential left to revoke. Its stock CLI @@ -385,6 +491,7 @@ async function removeMcpBridgeUnlocked( reservationCleanupProved = true; } } catch (error) { + rethrowMcpPolicyAuthorityRefusal(error); const detail = error instanceof Error ? error.message : String(error); if (!options.force) throw new McpBridgeError(detail); failures.push(detail); @@ -410,11 +517,13 @@ async function removeMcpBridgeUnlocked( allowMissing: false, force: options.force, }); + await revalidateBeforeMutation(); deleteProvider(entry, { allowLegacyGeneric: true, allowMissing: options.force === true || entry.addState === "preflighted", }); } catch (error) { + rethrowMcpPolicyAuthorityRefusal(error); const detail = error instanceof Error ? error.message : String(error); if (!options.force) throw new McpBridgeError(detail); failures.push(detail); @@ -422,16 +531,25 @@ async function removeMcpBridgeUnlocked( } if (failures.length > 0) { console.warn(` MCP force cleanup warnings:\n${failures.join("\n")}`); + } + if (!policyCleanupProved && (failures.length === 0 || options.allowResidual)) { + throw new McpBridgeError( + `Generated MCP policy cleanup for '${entry.policyName}' is incomplete. The bridge manifest was preserved so cleanup can be retried.`, + ); + } + if (failures.length > 0) { if (!options.allowResidual) { throw new McpBridgeError( - `MCP force cleanup left residual resources for '${server}'. The registry entry was preserved so cleanup can be retried.`, + `MCP force cleanup left residual resources for '${server}'. The bridge manifest was preserved so cleanup can be retried.`, ); } // allowResidual: the caller accepted leftover resources. This is NOT a proven // recovery — residual state remains — so the destroy marker must be preserved. return "residualPreserved"; } + await revalidateBeforeMutation(); removeBridgeEntry(sandboxName, server); + await revalidateBeforeSuccess(); console.log(` Removed MCP server '${server}' from sandbox '${sandboxName}'.`); return "removedTarget"; } diff --git a/src/lib/actions/sandbox/mcp-bridge-restart.ts b/src/lib/actions/sandbox/mcp-bridge-restart.ts index 23595deca94..3214d61bf3a 100644 --- a/src/lib/actions/sandbox/mcp-bridge-restart.ts +++ b/src/lib/actions/sandbox/mcp-bridge-restart.ts @@ -8,7 +8,16 @@ import type { McpBridgeEntry } from "../../state/registry"; import { registerAgentAdapterAtCurrentCredentialRevision } from "./mcp-bridge-adapters"; import { McpBridgeError } from "./mcp-bridge-contracts"; import { assertHermesMcpRuntimeIntent } from "./mcp-bridge-hermes-reconciliation"; -import { applyGeneratedPolicy, assertGeneratedPolicyMutationSafe } from "./mcp-bridge-policy"; +import { + applyGeneratedPolicy, + assertGeneratedPolicyMutationSafe, + buildRequiredMcpBridgePolicy, + McpPolicyAuthorityRefusalError, + preflightMcpPolicyAuthority, + qualifyMcpPolicyAuthorityReceipt, + revalidateContainingMcpPolicyAuthority, + revalidateMcpPolicyAuthorityReceipt, +} from "./mcp-bridge-policy"; import { assertMcpProviderRecoverable, assertNoAttachedProviderCredentialCollisions, @@ -99,12 +108,26 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P // recovery/selection, provider inspection, or any lifecycle mutation. assertMcpAdapterConfigMutationsAllowed(sandboxName, sandbox, targetEntries); const resolvedByServer = await preflightMcpEntryTargets(targetEntries); + const operation = server ? `restart MCP server '${server}'` : "restart managed MCP servers"; + const requiredPolicyContents = targetEntries.map((entry) => + buildRequiredMcpBridgePolicy(entry, resolvedTargetPins(resolvedByServer, entry)), + ); + const recheckPolicyAuthority = () => + preflightMcpPolicyAuthority({ + externalPolicy: "verify", + operation, + requiredPolicyContents, + sandboxName, + }); + const policyAuthority = recheckPolicyAuthority(); assertMcpCredentialBoundaryRuntimeVersion(); await ensureSandboxGatewaySelected(sandboxName); // Prove every policy key is absent or still matches its recorded ownership // before inspecting or updating any provider. `applyGeneratedPolicy` repeats // this check immediately before mutation to close the preflight-to-apply race. - for (const entry of targetEntries) assertGeneratedPolicyMutationSafe(sandboxName, entry); + if (policyAuthority === "nemoclaw-managed") { + for (const entry of targetEntries) assertGeneratedPolicyMutationSafe(sandboxName, entry); + } const providerInspectionByServer = new Map(); for (const entry of targetEntries) { providerInspectionByServer.set(entry.server, assertMcpProviderRecoverable(entry)); @@ -117,7 +140,9 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P // is still present in the sandbox spec. These references name providers // already proven absent; no live credential is removed before the runtime // capability probe, and the durable bridge manifest is retained on failure. + recheckPolicyAuthority(); for (const entry of missingProviderEntries) { + recheckPolicyAuthority(); detachMissingProviderReference(sandboxName, entry); } assertMcpAdapterMutationRuntimeCapabilities(sandboxName, sandbox, targetEntries); @@ -139,12 +164,16 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P // Revalidate the actual running supervisor before rotating or recreating // credentials. The temporary policy cannot bind the provider until an // endpointless profile is attached. + recheckPolicyAuthority(); ensureMcpBridgeProviderProfile(); - applyGeneratedPolicy(sandboxName, entry, target, { bindCredential: false }); + if (policyAuthority === "nemoclaw-managed") { + applyGeneratedPolicy(sandboxName, entry, target, { bindCredential: false }); + } const providerResult = upsertMcpProvider(entry.providerName ?? "", envRefs, { allowExisting: true, expectedProviderId: entry.providerId, prepareMutation: (action) => { + recheckPolicyAuthority(); if (action === "update") { previousCredentialRevision = observeMcpCredentialRevision(sandboxName, entry); } @@ -160,9 +189,11 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P providerId === entry.providerId ? entry : { ...entry, providerId, updatedAt: nowIso() }; if (refreshedEntry !== entry) { // A missing owned provider may be recreated during restart. Record the - // replacement object's immutable ID before policy/attach/adapter work. + // replacement object's immutable ID as recovery state before another + // authority check can refuse policy, attachment, or adapter work. writeBridgeEntry(sandboxName, refreshedEntry); entry = refreshedEntry; + recheckPolicyAuthority(); } assertNoAttachedProviderCredentialCollisions(sandboxName, [entry]); if (providerResult.action === "updated" && previousCredentialRevision === undefined) { @@ -170,8 +201,12 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P `Could not retain the prior OpenShell credential revision for provider '${entry.providerName}'.`, ); } + recheckPolicyAuthority(); attachProvider(sandboxName, entry); - applyGeneratedPolicy(sandboxName, entry, target); + if (policyAuthority === "nemoclaw-managed") { + applyGeneratedPolicy(sandboxName, entry, target); + } + recheckPolicyAuthority(); refreshMcpProviderEnvironment(entry); const entryAdapter = (entry.adapter as AgentMcpAdapter | undefined) ?? adapter; const credentialRevision = waitForAttachedMcpCredential(sandboxName, entry, { @@ -179,6 +214,7 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P ? { previousRevision: previousCredentialRevision } : {}), }); + recheckPolicyAuthority(); registerAgentAdapterAtCurrentCredentialRevision( sandboxName, entryAdapter, @@ -187,11 +223,13 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P credentialRevision, { replaceExisting: true }, ); + recheckPolicyAuthority(); writeBridgeEntry(sandboxName, { ...entry, adapter: (entry.adapter as AgentMcpAdapter | undefined) ?? adapter, updatedAt: nowIso(), }); + recheckPolicyAuthority(); console.log(` Refreshed MCP server '${name}'.`); } if (adapter === "hermes-config") assertHermesMcpRuntimeIntent(sandboxName); @@ -200,18 +238,91 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P export async function restoreExistingMcpBridgeRuntime( sandboxName: string, entries: readonly McpBridgeEntry[], - options: { lifecyclePhase?: "active-mutation" | "teardown-rollback" } = {}, + options: { + lifecyclePhase?: "active-mutation" | "teardown-rollback"; + /** Final refusal retained by the teardown caller while owned runtime compensation runs. */ + teardownPolicyAuthorityRefusal?: McpPolicyAuthorityRefusalError; + validateContainingPolicyReceipt?: () => Promise; + } = {}, ): Promise { if (entries.length === 0) return; for (const entry of entries) assertAuthenticatedBridgeEntry(entry); + const teardownRollback = options.lifecyclePhase === "teardown-rollback"; const resolvedByServer = await preflightMcpEntryTargets(entries); - if (options.lifecyclePhase !== "teardown-rollback") { + const sandbox = getSandboxOrThrow(sandboxName); + if (!teardownRollback || sandbox.mcp?.destroyPendingAt) { + assertMcpDestroyNotPending(sandbox); + } + let teardownAuthorityRefusal = teardownRollback + ? options.teardownPolicyAuthorityRefusal + : undefined; + let policyAuthorityReceipt: ReturnType | undefined; + if (!teardownAuthorityRefusal) { + try { + policyAuthorityReceipt = qualifyMcpPolicyAuthorityReceipt({ + operation: teardownRollback + ? "restore managed MCP runtime after teardown abort" + : "restore managed MCP runtime", + requiredPolicyContents: entries.map((entry) => + buildRequiredMcpBridgePolicy(entry, resolvedTargetPins(resolvedByServer, entry)), + ), + sandboxName, + }); + } catch (error) { + if (!teardownRollback || !(error instanceof McpPolicyAuthorityRefusalError)) throw error; + // Live policy must not change after this refusal. Provider and adapter + // compensation still runs below, then the refusal reports that policy + // restoration remains pending. + teardownAuthorityRefusal = error; + } + } + const revalidateBeforeMutation = () => + !teardownRollback && policyAuthorityReceipt + ? revalidateMcpPolicyAuthorityReceipt( + policyAuthorityReceipt, + options.validateContainingPolicyReceipt, + ) + : Promise.resolve(); + const observeTeardownContainingReceipt = async (): Promise => { + if (!teardownRollback || teardownAuthorityRefusal) return; + try { + await revalidateContainingMcpPolicyAuthority(options.validateContainingPolicyReceipt); + } catch (error) { + if (!(error instanceof McpPolicyAuthorityRefusalError)) throw error; + teardownAuthorityRefusal = error; + } + }; + const revalidateBeforeRuntimeMutation = async (): Promise => { + await revalidateBeforeMutation(); + await observeTeardownContainingReceipt(); + }; + const policyAuthority = policyAuthorityReceipt?.authority ?? sandbox.policyAuthority; + const runTeardownPolicyMutation = async (mutation: () => void): Promise => { + if (!teardownRollback) { + mutation(); + return true; + } + await observeTeardownContainingReceipt(); + if (teardownAuthorityRefusal || !policyAuthorityReceipt) return false; + try { + // The containing lifecycle receipt can already be the reason teardown + // aborted. Recheck the MCP receipt itself so owned compensation does not + // reuse a stale enclosing receipt. + await revalidateMcpPolicyAuthorityReceipt(policyAuthorityReceipt); + mutation(); + return true; + } catch (error) { + if (!(error instanceof McpPolicyAuthorityRefusalError)) throw error; + teardownAuthorityRefusal = error; + return false; + } + }; + if (!teardownRollback) { assertMcpCredentialBoundaryRuntimeVersion(); } + await revalidateBeforeMutation(); await ensureSandboxGatewaySelected(sandboxName); - const sandbox = getSandboxOrThrow(sandboxName); - assertMcpDestroyNotPending(sandbox); - if (options.lifecyclePhase === "teardown-rollback") { + if (teardownRollback) { // A failed delete/rebuild must be able to restore a backward-compatible // Deep Agents entry on the same old image it just scrubbed. New/rebuilt // images use the default path and must prove the current marker before any @@ -222,7 +333,15 @@ export async function restoreExistingMcpBridgeRuntime( } const defaultAdapter = getBridgeAdapter(getSandboxAgent(sandbox)); for (const entry of entries) { - assertGeneratedPolicyMutationSafe(sandboxName, entry); + if (policyAuthority === "nemoclaw-managed") { + if (teardownRollback) { + await runTeardownPolicyMutation(() => + assertGeneratedPolicyMutationSafe(sandboxName, entry), + ); + } else { + assertGeneratedPolicyMutationSafe(sandboxName, entry); + } + } const provider = assertMcpProviderRecoverable(entry); if (provider.exists !== true) { throw new McpBridgeError( @@ -237,15 +356,39 @@ export async function restoreExistingMcpBridgeRuntime( assertNoProviderCredentialCollisions(sandboxName, entries); for (const entry of entries) { assertNoAttachedProviderCredentialCollisions(sandboxName, [entry]); + await revalidateBeforeRuntimeMutation(); ensureMcpBridgeProviderProfile(); - applyGeneratedPolicy(sandboxName, entry, resolvedTargetPins(resolvedByServer, entry), { - bindCredential: false, - }); + if (policyAuthority === "nemoclaw-managed") { + if (teardownRollback) { + await runTeardownPolicyMutation(() => + applyGeneratedPolicy(sandboxName, entry, resolvedTargetPins(resolvedByServer, entry), { + bindCredential: false, + }), + ); + } else { + await revalidateBeforeMutation(); + applyGeneratedPolicy(sandboxName, entry, resolvedTargetPins(resolvedByServer, entry), { + bindCredential: false, + }); + } + } + await revalidateBeforeRuntimeMutation(); attachProvider(sandboxName, entry); - applyGeneratedPolicy(sandboxName, entry, resolvedTargetPins(resolvedByServer, entry)); - const adapter = (entry.adapter as AgentMcpAdapter | undefined) ?? defaultAdapter; + if (policyAuthority === "nemoclaw-managed") { + if (teardownRollback) { + await runTeardownPolicyMutation(() => + applyGeneratedPolicy(sandboxName, entry, resolvedTargetPins(resolvedByServer, entry)), + ); + } else { + await revalidateBeforeMutation(); + applyGeneratedPolicy(sandboxName, entry, resolvedTargetPins(resolvedByServer, entry)); + } + } + await revalidateBeforeRuntimeMutation(); refreshMcpProviderEnvironment(entry); const credentialRevision = waitForAttachedMcpCredential(sandboxName, entry); + const adapter = (entry.adapter as AgentMcpAdapter | undefined) ?? defaultAdapter; + await revalidateBeforeRuntimeMutation(); registerAgentAdapterAtCurrentCredentialRevision( sandboxName, adapter, @@ -254,10 +397,13 @@ export async function restoreExistingMcpBridgeRuntime( credentialRevision, { replaceExisting: true, - teardownRollback: options.lifecyclePhase === "teardown-rollback", + teardownRollback, }, ); - writeBridgeEntry(sandboxName, { ...entry, adapter, updatedAt: nowIso() }); + if (!teardownRollback) { + await revalidateBeforeMutation(); + writeBridgeEntry(sandboxName, { ...entry, adapter, updatedAt: nowIso() }); + } } if ( defaultAdapter === "hermes-config" || @@ -265,4 +411,10 @@ export async function restoreExistingMcpBridgeRuntime( ) { assertHermesMcpRuntimeIntent(sandboxName, { entries }); } + if (teardownRollback && policyAuthorityReceipt) { + await runTeardownPolicyMutation(() => undefined); + } + if (teardownAuthorityRefusal && !options.teardownPolicyAuthorityRefusal) { + throw teardownAuthorityRefusal; + } } diff --git a/src/lib/actions/sandbox/mcp-bridge-status-removal.test.ts b/src/lib/actions/sandbox/mcp-bridge-status-removal.test.ts index b417a039bcc..7e93fec7b14 100644 --- a/src/lib/actions/sandbox/mcp-bridge-status-removal.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-status-removal.test.ts @@ -36,6 +36,8 @@ const agentDefs = require("./src/lib/agent/defs.js"); const providerCommands = require("./src/lib/adapters/openshell/provider-command.js"); const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); agentDefs.loadAgent = () => { throw new Error("current agent must not be consulted"); }; gatewayRuntime.recoverNamedGatewayRuntime = async () => ({ @@ -56,6 +58,8 @@ providerCommands.runOpenshellProviderCommand = (args) => { }; policies.removePreset = () => true; policies.getPresetContentGatewayState = () => "absent"; +mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; processRecovery.executeSandboxCommand = () => ({ status: 0, stdout: "", stderr: "" }); processRecovery.executeSandboxExecCommand = () => ({ status: 0, stdout: "", stderr: "" }); registry.registerSandbox({ @@ -106,7 +110,7 @@ bridge.removeMcpBridge("legacy-sandbox", "github").then( }); }); - it("preserves the registry entry when force cleanup leaves residual policy state", () => { + it("preserves the bridge manifest when force cleanup leaves residual policy state", () => { const home = createTempHome("nemoclaw-mcp-residual-"); const script = ` process.env.HOME = ${JSON.stringify(home)}; @@ -115,6 +119,8 @@ const agentDefs = require("./src/lib/agent/defs.js"); const providerCommands = require("./src/lib/adapters/openshell/provider-command.js"); const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); agentDefs.loadAgent = () => { throw new Error("current agent must not be consulted"); }; gatewayRuntime.recoverNamedGatewayRuntime = async () => ({ @@ -135,6 +141,8 @@ providerCommands.runOpenshellProviderCommand = (args) => { }; policies.removePreset = () => false; policies.getPresetContentGatewayState = () => "match"; +mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; processRecovery.executeSandboxCommand = () => ({ status: 0, stdout: "", stderr: "" }); processRecovery.executeSandboxExecCommand = () => ({ status: 0, stdout: "", stderr: "" }); registry.registerSandbox({ @@ -180,7 +188,7 @@ bridge.removeMcpBridge("legacy-sandbox", "github", { force: true }).then( message: string; sandbox: { mcp?: { bridges?: Record } }; }; - expect(payload.message).toContain("registry entry was preserved"); + expect(payload.message).toContain("bridge manifest was preserved"); expect(payload.sandbox.mcp?.bridges).toHaveProperty("github"); }); }); diff --git a/src/lib/actions/sandbox/mcp-bridge.ts b/src/lib/actions/sandbox/mcp-bridge.ts index be85538880c..872c9973a82 100644 --- a/src/lib/actions/sandbox/mcp-bridge.ts +++ b/src/lib/actions/sandbox/mcp-bridge.ts @@ -11,8 +11,10 @@ import { } from "./mcp-bridge-contracts"; import { finalizeMcpBridgesAfterSandboxDelete as finalizeMcpBridgesAfterSandboxDeleteLifecycle, + type McpDestroyPreparation, prepareMcpBridgesForAbsentSandboxDestroy as prepareMcpBridgesForAbsentSandboxDestroyLifecycle, prepareMcpBridgesForDestroy as prepareMcpBridgesForDestroyLifecycle, + revalidateMcpDestroyAbortPolicyAuthority as revalidateMcpDestroyAbortPolicyAuthorityLifecycle, restoreMcpBridgesAfterDestroyAbort as restoreMcpBridgesAfterDestroyAbortLifecycle, } from "./mcp-bridge-destroy"; import { redactBridgeSecretsForDisplay } from "./mcp-bridge-output"; @@ -62,6 +64,7 @@ export { buildMcpBridgePolicyYaml, MCP_BRIDGE_ALLOWED_METHODS, MCP_BRIDGE_POLICY_MAX_BODY_BYTES, + McpPolicyAuthorityRefusalError, } from "./mcp-bridge-policy"; export { buildMcpBridgeProviderArgs, @@ -81,19 +84,9 @@ export { validateMcpCredentialEnvName, validateMcpServerName, } from "./mcp-bridge-validation"; -export type { McpRebuildPreparation }; +export type { McpDestroyPreparation, McpRebuildPreparation }; export { statusMcpBridge }; -export interface McpDestroyPreparation { - entries: McpBridgeEntry[]; - detachedProviderEntries: McpBridgeEntry[]; - scrubbedAdapterEntries: McpScrubbedAdapterEntry[]; - /** True when phase one was completed by an earlier destroy process. */ - destroyAlreadyPrepared: boolean; - /** True when a previous destroy already confirmed the sandbox was absent. */ - destroyAlreadyPending: boolean; -} - export async function addMcpBridge( sandboxName: string, options: McpBridgeAddOptions, @@ -122,15 +115,33 @@ export async function prepareMcpBridgesForAbsentSandboxDestroy( export async function prepareMcpBridgesForDestroy( sandboxName: string, + validateContainingPolicyReceipt?: () => Promise, ): Promise { - return prepareMcpBridgesForDestroyLifecycle(sandboxName); + return prepareMcpBridgesForDestroyLifecycle(sandboxName, validateContainingPolicyReceipt); } export async function restoreMcpBridgesAfterDestroyAbort( sandboxName: string, preparation: McpDestroyPreparation, + validateContainingPolicyReceipt?: () => Promise, +): Promise { + return restoreMcpBridgesAfterDestroyAbortLifecycle( + sandboxName, + preparation, + validateContainingPolicyReceipt, + ); +} + +export async function revalidateMcpDestroyAbortPolicyAuthority( + sandboxName: string, + preparation: McpDestroyPreparation, + validateContainingPolicyReceipt?: () => Promise, ): Promise { - return restoreMcpBridgesAfterDestroyAbortLifecycle(sandboxName, preparation); + return revalidateMcpDestroyAbortPolicyAuthorityLifecycle( + sandboxName, + preparation, + validateContainingPolicyReceipt, + ); } export async function finalizeMcpBridgesAfterSandboxDelete( @@ -149,31 +160,35 @@ export async function prepareMcpBridgesForAbsentSandboxRebuild( export async function prepareMcpBridgesForRebuild( sandboxName: string, - validatePolicyAuthority?: () => Promise, + validateContainingPolicyReceipt?: () => Promise, ): Promise { - return prepareMcpBridgesForRebuildLifecycle(sandboxName, validatePolicyAuthority); + return prepareMcpBridgesForRebuildLifecycle(sandboxName, validateContainingPolicyReceipt); } export async function reattachMcpProvidersAfterRebuildAbort( sandboxName: string, entries: readonly McpBridgeEntry[], scrubbedAdapterEntries: readonly McpScrubbedAdapterEntry[] = [], - validatePolicyAuthority?: () => Promise, + validateContainingPolicyReceipt?: () => Promise, ): Promise { return reattachMcpProvidersAfterRebuildAbortLifecycle( sandboxName, entries, scrubbedAdapterEntries, - validatePolicyAuthority, + validateContainingPolicyReceipt, ); } export async function restoreMcpBridgesAfterRebuild( sandboxName: string, entries: readonly McpBridgeEntry[], - validatePolicyAuthority?: () => Promise, + validateContainingPolicyReceipt?: () => Promise, ): Promise { - return restoreMcpBridgesAfterRebuildLifecycle(sandboxName, entries, validatePolicyAuthority); + return restoreMcpBridgesAfterRebuildLifecycle( + sandboxName, + entries, + validateContainingPolicyReceipt, + ); } function parseJsonFlag(args: string[]): { json: boolean; rest: string[] } { diff --git a/src/lib/actions/sandbox/policy-authority/mcp-remove.test.ts b/src/lib/actions/sandbox/policy-authority/mcp-remove.test.ts new file mode 100644 index 00000000000..a4ff36d541b --- /dev/null +++ b/src/lib/actions/sandbox/policy-authority/mcp-remove.test.ts @@ -0,0 +1,272 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { SandboxEntry } from "../../../state/registry"; + +const harness = vi.hoisted(() => ({ + actions: [] as string[], + afterAdapterRemoval: undefined as (() => void) | undefined, + afterQualification: undefined as (() => void) | undefined, + afterRegistryRemoval: undefined as (() => void) | undefined, + authority: "externally-managed" as "nemoclaw-managed" | "externally-managed", + buildRequiredPolicy: vi.fn(() => "network_policies:\n mcp_bridge_example:\n endpoints: []\n"), + detachProvider: vi.fn(), + policyRemove: vi.fn(), + qualifyAuthority: vi.fn(), + revalidateAuthority: vi.fn(), + sandbox: undefined as SandboxEntry | undefined, +})); + +vi.mock("../../../state/mcp-lifecycle-lock", () => ({ + withMcpLifecycleLock: async (_sandboxName: string, operation: () => Promise) => operation(), +})); + +vi.mock("../../../onboard/experimental/portable-agent-lifecycle", () => ({ + assertHermesPortableCommandUnavailable: vi.fn(), +})); + +vi.mock("../mcp-bridge-adapters", () => ({ + assertAgentMcpConfigMutationAllowed: vi.fn(), + assertAgentMcpTeardownRuntimeCapability: vi.fn(), + unregisterAgentAdapter: vi.fn(() => { + harness.actions.push("adapter:remove"); + harness.afterAdapterRemoval?.(); + return "removed"; + }), +})); + +vi.mock("../mcp-bridge-hermes-reconciliation", () => ({ + assertHermesMcpRuntimeIntent: vi.fn(), +})); + +vi.mock("../mcp-bridge-policy", async (importOriginal) => { + const actual = await importOriginal(); + harness.qualifyAuthority.mockImplementation((options) => { + const receipt = { ...options, authority: harness.authority }; + harness.afterQualification?.(); + return receipt; + }); + harness.revalidateAuthority.mockImplementation( + async ( + receipt: { authority: typeof harness.authority }, + _validateContainingReceipt: undefined, + assertCurrentState?: () => void, + ) => { + (receipt.authority === harness.authority + ? () => undefined + : () => { + throw new actual.McpPolicyAuthorityRefusalError("policy authority changed"); + })(); + assertCurrentState?.(); + }, + ); + return { + ...actual, + buildRequiredMcpBridgePolicy: harness.buildRequiredPolicy, + qualifyMcpPolicyAuthorityReceipt: harness.qualifyAuthority, + removeGeneratedPolicy: harness.policyRemove, + revalidateMcpPolicyAuthorityReceipt: harness.revalidateAuthority, + }; +}); + +vi.mock("../mcp-bridge-provider", () => ({ + deleteProvider: vi.fn(() => harness.actions.push("provider:delete")), + detachMissingProviderReference: vi.fn(() => "detached"), + detachProvider: harness.detachProvider, + inspectMcpProvider: vi.fn(() => ({ + credentialKeys: ["MCP_TOKEN"], + exists: true, + id: "11111111-2222-4333-8444-555555555555", + resourceVersion: "1", + type: "nemoclaw-mcp-v1", + })), + preflightMcpEntryTargets: vi.fn(async () => new Map([["example", { addresses: ["8.8.8.8"] }]])), + providerMatchesManagedCredential: vi.fn(() => true), + providerShapeDetail: vi.fn(() => null), + waitForDetachedMcpCredential: vi.fn(), +})); + +vi.mock("../mcp-bridge-state", () => ({ + assertMcpDestroyNotPending: vi.fn(), + bridgeState: (sandbox: SandboxEntry) => sandbox.mcp?.bridges ?? {}, + clearMcpDestroyMarkers: vi.fn(() => false), + ensureSandboxGatewaySelected: vi.fn(async () => { + harness.actions.push("gateway:select"); + }), + getBridgeAdapter: vi.fn(() => "mcporter"), + getSandboxAgent: vi.fn(() => ({ name: "openclaw" })), + getSandboxOrThrow: vi.fn(() => harness.sandbox), + removeBridgeEntry: vi.fn((_sandboxName: string, server: string) => { + harness.actions.push("registry:remove"); + delete harness.sandbox?.mcp?.bridges[server]; + harness.afterRegistryRemoval?.(); + }), +})); + +vi.mock("../mcp-bridge-validation", () => ({ + assertAuthenticatedBridgeEntry: vi.fn(), + assertPersistedAuthenticatedBridgeEntry: vi.fn(), + resolvePersistedCredentialEnvForRedaction: vi.fn(() => ({})), + validateMcpServerName: vi.fn(), + validateSandboxName: vi.fn(), +})); + +import { removeMcpBridge } from "../mcp-bridge-remove"; +import { McpPolicyAuthorityRefusalError } from "../mcp-bridge-policy"; + +const entry = { + server: "example", + agent: "openclaw", + adapter: "mcporter" as const, + url: "https://mcp.example.test/mcp", + env: ["MCP_TOKEN"], + providerName: "alpha-mcp-example-1234567890abcdef", + providerId: "11111111-2222-4333-8444-555555555555", + policyName: "mcp-bridge-example", + addedAt: "2026-08-24T00:00:00.000Z", +}; + +beforeEach(() => { + vi.clearAllMocks(); + harness.actions.length = 0; + harness.afterAdapterRemoval = undefined; + harness.afterQualification = undefined; + harness.afterRegistryRemoval = undefined; + harness.authority = "externally-managed"; + harness.sandbox = { + name: "alpha", + agent: "openclaw", + gatewayName: "nemoclaw", + policyAuthority: harness.authority, + mcp: { bridges: { example: { ...entry, env: [...entry.env] } } }, + }; + harness.detachProvider.mockImplementation(async (_sandboxName, _entry, options) => { + await options.prepareMutation?.(); + harness.actions.push("provider:detach"); + return "detached"; + }); +}); + +describe("standalone MCP remove policy authority", () => { + it("removes external runtime state without policy mutation or attribution (#9833)", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + + await removeMcpBridge("alpha", "example"); + + expect(harness.qualifyAuthority).toHaveBeenCalledWith({ + operation: "remove MCP server 'example'", + requiredPolicyContents: ["network_policies:\n mcp_bridge_example:\n endpoints: []\n"], + sandboxName: "alpha", + }); + expect(harness.qualifyAuthority).toHaveBeenCalledTimes(1); + expect(harness.actions).toEqual([ + "gateway:select", + "adapter:remove", + "provider:detach", + "provider:delete", + "registry:remove", + ]); + expect(harness.policyRemove).not.toHaveBeenCalled(); + expect(harness.sandbox?.customPolicies).toBeUndefined(); + expect(harness.sandbox?.mcp?.bridges).toEqual({}); + expect(harness.revalidateAuthority).toHaveBeenCalledTimes(6); + expect(log).toHaveBeenCalledWith(" Removed MCP server 'example' from sandbox 'alpha'."); + }); + + it("qualifies managed removal without resolving the discarded endpoint (#9833)", async () => { + harness.authority = "nemoclaw-managed"; + harness.sandbox = { ...harness.sandbox!, policyAuthority: "nemoclaw-managed" }; + + await removeMcpBridge("alpha", "example"); + + expect(harness.qualifyAuthority).toHaveBeenCalledWith({ + operation: "remove MCP server 'example'", + requiredPolicyContents: ["network_policies:\n mcp_bridge_example: {}\n"], + sandboxName: "alpha", + }); + expect(harness.buildRequiredPolicy).not.toHaveBeenCalled(); + expect(harness.policyRemove).toHaveBeenCalledTimes(1); + }); + + it("retries legacy external authority with the exact MCP requirement (#9833)", async () => { + harness.sandbox = { ...harness.sandbox!, policyAuthority: undefined }; + harness.qualifyAuthority.mockImplementationOnce(() => { + harness.sandbox!.policyAuthority = "externally-managed"; + throw new McpPolicyAuthorityRefusalError( + "the externally managed policy does not contain the authority probe", + ); + }); + + await removeMcpBridge("alpha", "example"); + + expect(harness.qualifyAuthority).toHaveBeenNthCalledWith(1, { + operation: "remove MCP server 'example'", + requiredPolicyContents: ["network_policies:\n mcp_bridge_example: {}\n"], + sandboxName: "alpha", + }); + expect(harness.qualifyAuthority).toHaveBeenNthCalledWith(2, { + operation: "remove MCP server 'example'", + requiredPolicyContents: ["network_policies:\n mcp_bridge_example:\n endpoints: []\n"], + sandboxName: "alpha", + }); + expect(harness.policyRemove).not.toHaveBeenCalled(); + }); + + it("refuses managed-to-external drift before the first mutation under force (#9833)", async () => { + harness.authority = "nemoclaw-managed"; + harness.sandbox = { ...harness.sandbox!, policyAuthority: "nemoclaw-managed" }; + harness.afterQualification = () => { + harness.authority = "externally-managed"; + }; + + await expect( + removeMcpBridge("alpha", "example", { allowResidual: true, force: true }), + ).rejects.toBeInstanceOf(McpPolicyAuthorityRefusalError); + + expect(harness.actions).toEqual([]); + expect(harness.policyRemove).not.toHaveBeenCalled(); + expect(harness.sandbox?.mcp?.bridges.example).toEqual(expect.objectContaining(entry)); + }); + + it("stops after adapter cleanup when managed authority changes under force (#9833)", async () => { + harness.authority = "nemoclaw-managed"; + harness.sandbox = { ...harness.sandbox!, policyAuthority: "nemoclaw-managed" }; + harness.afterAdapterRemoval = () => { + harness.authority = "externally-managed"; + }; + + await expect( + removeMcpBridge("alpha", "example", { allowResidual: true, force: true }), + ).rejects.toBeInstanceOf(McpPolicyAuthorityRefusalError); + + expect(harness.actions).toEqual(["gateway:select", "adapter:remove"]); + expect(harness.policyRemove).not.toHaveBeenCalled(); + expect(harness.detachProvider).not.toHaveBeenCalled(); + expect(harness.sandbox?.mcp?.bridges.example).toEqual(expect.objectContaining(entry)); + }); + + it("withholds removal success when the bridge manifest changes during removal (#9833)", async () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + harness.afterRegistryRemoval = () => { + harness.sandbox!.mcp!.bridges.concurrent = { + ...entry, + server: "concurrent", + }; + }; + + await expect(removeMcpBridge("alpha", "example")).rejects.toThrow( + "MCP bridge definitions changed while server removal was in progress", + ); + + expect(harness.actions).toEqual([ + "gateway:select", + "adapter:remove", + "provider:detach", + "provider:delete", + "registry:remove", + ]); + expect(log).not.toHaveBeenCalledWith(" Removed MCP server 'example' from sandbox 'alpha'."); + }); +}); diff --git a/src/lib/actions/sandbox/policy-authority/mcp-teardown.test.ts b/src/lib/actions/sandbox/policy-authority/mcp-teardown.test.ts new file mode 100644 index 00000000000..fc518f642b5 --- /dev/null +++ b/src/lib/actions/sandbox/policy-authority/mcp-teardown.test.ts @@ -0,0 +1,453 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import fs from "node:fs"; + +import { afterAll, beforeEach, describe, expect, it, vi } from "vitest"; + +const harness = vi.hoisted(() => { + const originalHome = process.env.HOME; + const stateHome = `/tmp/nemoclaw-mcp-teardown-authority-${process.pid}`; + process.env.HOME = stateHome; + return { + actions: [] as string[], + adapterRegistered: true, + appliedPolicyContents: [] as string[], + authority: "nemoclaw-managed" as "nemoclaw-managed" | "externally-managed", + detachOutcomes: [] as Array<"detached" | "unknown">, + originalHome, + policyRemoved: false, + preflightAuthority: vi.fn(), + providerAttached: true, + stateHome, + }; +}); + +vi.mock("../../../policy", () => ({ + applyPresetContent: vi.fn((_sandboxName: string, _policyName: string, content: string) => { + harness.actions.push("policy:restore"); + harness.appliedPolicyContents.push(content); + harness.policyRemoved = false; + return true; + }), + getLiveSandboxPolicyEntryDigest: vi.fn(() => (harness.policyRemoved ? null : "present")), + getPresetContentGatewayState: vi.fn(() => (harness.policyRemoved ? "absent" : "match")), + removePreset: vi.fn(() => { + harness.actions.push("policy:remove"); + harness.policyRemoved = true; + return true; + }), +})); + +vi.mock("./preflight", () => ({ + preflightSandboxPolicyAuthority: harness.preflightAuthority, +})); + +vi.mock("../mcp-bridge-adapter-teardown", () => ({ + rollbackScrubbedMcpAdapters: vi.fn(() => { + harness.actions.push("adapter:rollback"); + harness.adapterRegistered = true; + return []; + }), + scrubManagedMcpAdapterOrThrow: vi.fn(() => { + harness.actions.push("adapter:scrub"); + harness.adapterRegistered = false; + }), +})); + +vi.mock("../mcp-bridge-adapters", () => ({ + registerAgentAdapter: vi.fn(() => { + harness.actions.push("adapter:rollback"); + harness.adapterRegistered = true; + }), +})); + +vi.mock("../mcp-bridge-provider", () => ({ + assertMcpProviderRecoverable: vi.fn(() => ({ exists: true })), + assertNoAttachedProviderCredentialCollisions: vi.fn(), + assertNoProviderCredentialCollisions: vi.fn(), + assertNoRegisteredProviderCredentialCollisions: vi.fn(), + attachProvider: vi.fn(() => { + harness.actions.push("provider:attach"); + harness.providerAttached = true; + }), + deleteProvider: vi.fn(), + detachMissingProviderReference: vi.fn(), + detachProvider: vi.fn(() => { + const outcome = harness.detachOutcomes.shift() ?? "detached"; + harness.actions.push("provider:detach"); + harness.providerAttached = outcome === "detached" ? false : harness.providerAttached; + return outcome; + }), + inspectMcpProvider: vi.fn(() => ({ + credentialKeys: ["MCP_TOKEN"], + exists: true, + id: "11111111-2222-4333-8444-555555555555", + resourceVersion: "1", + type: "nemoclaw-mcp-v1", + })), + ensureMcpBridgeProviderProfile: vi.fn(), + observeMcpCredentialRevision: vi.fn(), + preflightMcpEntryTargets: vi.fn( + async (entries: Array<{ server: string }>) => + new Map(entries.map((entry) => [entry.server, { addresses: ["8.8.8.8"] }])), + ), + providerMatchesManagedCredential: vi.fn(() => true), + refreshMcpProviderEnvironment: vi.fn(), + upsertMcpProvider: vi.fn(), + waitForAttachedMcpCredential: vi.fn(), + waitForDetachedMcpCredential: vi.fn(), +})); + +vi.mock("../mcp-bridge-runtime-capabilities", () => ({ + assertMcpAdapterConfigMutationsAllowed: vi.fn(), + assertMcpAdapterMutationRuntimeCapabilities: vi.fn(), + assertMcpAdapterTeardownRuntimeCapabilities: vi.fn(), +})); + +vi.mock("../mcp-bridge-state", async (importOriginal) => ({ + ...(await importOriginal()), + ensureSandboxGatewaySelected: vi.fn(async () => undefined), +})); + +import * as registry from "../../../state/registry"; +import { + prepareMcpBridgesForDestroy, + restoreMcpBridgesAfterDestroyAbort, +} from "../mcp-bridge-destroy"; +import { + assertMcpProviderRecoverable, + ensureMcpBridgeProviderProfile, + preflightMcpEntryTargets, + refreshMcpProviderEnvironment, +} from "../mcp-bridge-provider"; +import { McpPolicyAuthorityRefusalError } from "../mcp-bridge-policy"; +import { buildMcpBridgePolicyYaml } from "../mcp-bridge-policy-render"; +import { prepareMcpBridgesForRebuild } from "../mcp-bridge-rebuild"; + +const bridgeEntry = { + server: "example", + agent: "openclaw", + adapter: "mcporter" as const, + url: "https://mcp.example.test/mcp", + env: ["MCP_TOKEN"], + providerName: "alpha-mcp-example", + providerId: "11111111-2222-4333-8444-555555555555", + policyName: "mcp-bridge-example", + addedAt: "2026-08-20T00:00:00.000Z", +}; + +function registerSandbox(authority: "nemoclaw-managed" | "externally-managed"): void { + registry.registerSandbox({ + name: "alpha", + agent: "openclaw", + gatewayName: "nemoclaw", + policyAuthority: authority, + mcp: { bridges: { example: bridgeEntry } }, + }); + authority === "nemoclaw-managed" && + registry.addCustomPolicy("alpha", { + name: bridgeEntry.policyName, + content: buildMcpBridgePolicyYaml( + bridgeEntry.server, + bridgeEntry.url, + bridgeEntry.adapter, + { addresses: ["8.8.8.8"] }, + bridgeEntry.providerName, + ), + sourcePath: "generated:nemoclaw-mcp-bridge", + }); +} + +beforeEach(() => { + fs.rmSync(harness.stateHome, { recursive: true, force: true }); + harness.actions.length = 0; + harness.adapterRegistered = true; + harness.appliedPolicyContents.length = 0; + harness.authority = "nemoclaw-managed"; + harness.detachOutcomes.length = 0; + harness.policyRemoved = false; + harness.providerAttached = true; + vi.mocked(ensureMcpBridgeProviderProfile).mockClear(); + vi.mocked(preflightMcpEntryTargets).mockClear(); + vi.mocked(refreshMcpProviderEnvironment).mockClear(); + harness.preflightAuthority.mockReset(); + harness.preflightAuthority.mockImplementation(() => harness.authority); +}); + +afterAll(() => { + fs.rmSync(harness.stateHome, { recursive: true, force: true }); + process.env.HOME = harness.originalHome; +}); + +describe("MCP teardown policy authority", () => { + it.each([ + ["destroy", prepareMcpBridgesForDestroy, 4], + ["rebuild", prepareMcpBridgesForRebuild, 3], + ] as const)( + "preserves externally managed policy during %s preparation (#9833)", + async (_operation, prepare, expectedAuthorityChecks) => { + harness.authority = "externally-managed"; + registerSandbox("externally-managed"); + + await prepare("alpha"); + + expect(harness.actions).toEqual(["adapter:scrub", "provider:detach"]); + expect(registry.getCustomPolicies("alpha")).toEqual([]); + expect(harness.preflightAuthority).toHaveBeenCalledTimes(expectedAuthorityChecks); + }, + ); + + it.each(["nemoclaw-managed", "externally-managed"] as const)( + "retains prepared destroy retry state after a containing refusal under %s authority (#9833)", + async (authority) => { + harness.authority = authority; + registerSandbox(authority); + const preparation = await prepareMcpBridgesForDestroy("alpha"); + const authorityChecksAfterPrepare = harness.preflightAuthority.mock.calls.length; + harness.actions.length = 0; + const validateContainingReceipt = vi + .fn<() => Promise>() + .mockRejectedValueOnce( + new McpPolicyAuthorityRefusalError("destroy policy authority changed"), + ); + + await expect( + restoreMcpBridgesAfterDestroyAbort("alpha", preparation, validateContainingReceipt), + ).rejects.toBeInstanceOf(McpPolicyAuthorityRefusalError); + + expect(harness.actions).toEqual(["provider:attach", "adapter:rollback"]); + expect(harness.actions).not.toContain("policy:restore"); + expect(harness.adapterRegistered).toBe(true); + expect(harness.providerAttached).toBe(true); + expect(harness.preflightAuthority).toHaveBeenCalledTimes(authorityChecksAfterPrepare); + expect(registry.getSandbox("alpha")?.mcp).toEqual( + expect.objectContaining({ + bridges: { example: bridgeEntry }, + destroyPreparedAt: expect.any(String), + }), + ); + }, + ); + + it("requalifies exact current MCP requirements before retrying a prepared destroy (#9833)", async () => { + harness.authority = "externally-managed"; + registerSandbox("externally-managed"); + await prepareMcpBridgesForDestroy("alpha"); + const preparationActions = [...harness.actions]; + vi.mocked(preflightMcpEntryTargets).mockResolvedValueOnce( + new Map([[bridgeEntry.server, { addresses: ["9.9.9.9"] }]]), + ); + harness.preflightAuthority.mockImplementationOnce( + (options: { requiredPolicyContents: readonly string[] }) => { + expect(options.requiredPolicyContents.join("\n")).toContain("9.9.9.9"); + throw new Error("current MCP policy no longer satisfies the prepared destroy"); + }, + ); + + await expect(prepareMcpBridgesForDestroy("alpha")).rejects.toBeInstanceOf( + McpPolicyAuthorityRefusalError, + ); + + expect(harness.actions).toEqual(preparationActions); + expect(preflightMcpEntryTargets).toHaveBeenCalledTimes(2); + expect(registry.getSandbox("alpha")?.mcp?.destroyPreparedAt).toEqual(expect.any(String)); + }); + + it("uses the retained destroy receipt after deletion without reading live policy (#9833)", async () => { + harness.authority = "externally-managed"; + registerSandbox("externally-managed"); + const preparation = await prepareMcpBridgesForDestroy("alpha"); + const liveAuthorityChecks = harness.preflightAuthority.mock.calls.length; + const current = registry.getSandbox("alpha"); + const getSandbox = vi + .spyOn(registry, "getSandbox") + .mockReturnValue(current ? { ...current, policyAuthority: "nemoclaw-managed" } : null); + + await expect(preparation.revalidateAfterDelete?.()).rejects.toBeInstanceOf( + McpPolicyAuthorityRefusalError, + ); + + expect(harness.preflightAuthority).toHaveBeenCalledTimes(liveAuthorityChecks); + getSandbox.mockRestore(); + expect(registry.getSandbox("alpha")?.mcp?.destroyPreparedAt).toEqual(expect.any(String)); + }); + + it("retains the prepared destroy marker when authority refusal compensation fails (#9833)", async () => { + harness.authority = "externally-managed"; + registerSandbox("externally-managed"); + const preparation = await prepareMcpBridgesForDestroy("alpha"); + vi.mocked(assertMcpProviderRecoverable).mockImplementationOnce(() => { + throw new Error("provider recovery failed"); + }); + + await expect( + restoreMcpBridgesAfterDestroyAbort("alpha", preparation, async () => { + throw new McpPolicyAuthorityRefusalError("destroy policy authority changed"); + }), + ).rejects.toBeInstanceOf(McpPolicyAuthorityRefusalError); + + expect(registry.getSandbox("alpha")?.mcp?.destroyPreparedAt).toEqual(expect.any(String)); + expect(registry.getSandbox("alpha")?.mcp?.bridges.example).toEqual(bridgeEntry); + }); + + it("continues non-policy compensation after containing authority drifts (#9833)", async () => { + registerSandbox("nemoclaw-managed"); + const preparation = await prepareMcpBridgesForDestroy("alpha"); + harness.actions.length = 0; + const validateContainingReceipt = vi + .fn<() => Promise>() + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockRejectedValue( + new McpPolicyAuthorityRefusalError( + "containing policy authority changed during compensation", + ), + ); + + await expect( + restoreMcpBridgesAfterDestroyAbort("alpha", preparation, validateContainingReceipt), + ).rejects.toBeInstanceOf(McpPolicyAuthorityRefusalError); + + expect(harness.actions).toEqual(["policy:restore", "provider:attach", "adapter:rollback"]); + expect(ensureMcpBridgeProviderProfile).toHaveBeenCalledOnce(); + expect(refreshMcpProviderEnvironment).toHaveBeenCalledOnce(); + expect(validateContainingReceipt).toHaveBeenCalledTimes(5); + expect(registry.getSandbox("alpha")?.mcp?.destroyPreparedAt).toEqual(expect.any(String)); + }); + + it.each([ + ["destroy", prepareMcpBridgesForDestroy], + ["rebuild", prepareMcpBridgesForRebuild], + ] as const)( + "compensates %s after authority drift follows the first owned mutation (#9833)", + async (_operation, prepare) => { + registerSandbox("nemoclaw-managed"); + const validateContainingReceipt = vi + .fn<() => Promise>() + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockImplementationOnce(async () => { + harness.authority = "externally-managed"; + throw new Error("policy authority changed after adapter cleanup"); + }); + + await expect(prepare("alpha", validateContainingReceipt)).rejects.toBeInstanceOf( + McpPolicyAuthorityRefusalError, + ); + + expect(harness.actions).toEqual(["adapter:scrub", "provider:attach", "adapter:rollback"]); + expect(harness.actions).not.toContain("policy:restore"); + expect(validateContainingReceipt).toHaveBeenCalledTimes(3); + expect(harness.adapterRegistered).toBe(true); + expect(harness.providerAttached).toBe(true); + expect(registry.getSandbox("alpha")?.mcp?.bridges.example).toEqual( + expect.objectContaining(bridgeEntry), + ); + }, + ); + + it.each([ + ["destroy", prepareMcpBridgesForDestroy], + ["rebuild", prepareMcpBridgesForRebuild], + ] as const)( + "restores the managed generated policy when %s provider detach cannot be proved (#9833)", + async (_operation, prepare) => { + harness.detachOutcomes.push("unknown"); + registerSandbox("nemoclaw-managed"); + + await expect(prepare("alpha")).rejects.toThrow("Could not prove provider detach"); + + const policyRemoval = harness.actions.indexOf("policy:remove"); + const firstPolicyRestore = harness.actions.indexOf("policy:restore"); + const providerAttach = harness.actions.indexOf("provider:attach"); + const finalPolicyRestore = harness.actions.lastIndexOf("policy:restore"); + const adapterRollback = harness.actions.indexOf("adapter:rollback"); + expect(policyRemoval).toBeGreaterThanOrEqual(0); + expect(firstPolicyRestore).toBeGreaterThan(policyRemoval); + expect(providerAttach).toBeGreaterThan(firstPolicyRestore); + expect(finalPolicyRestore).toBeGreaterThan(providerAttach); + expect(adapterRollback).toBeGreaterThan(finalPolicyRestore); + expect(harness.appliedPolicyContents.at(-1)).toContain("credential_binding:"); + expect(harness.policyRemoved).toBe(false); + expect(registry.getCustomPolicies("alpha")).toEqual([ + expect.objectContaining({ + name: bridgeEntry.policyName, + sourcePath: "generated:nemoclaw-mcp-bridge", + }), + ]); + expect(harness.adapterRegistered).toBe(true); + expect(harness.providerAttached).toBe(true); + }, + ); + + it.each([ + ["destroy", prepareMcpBridgesForDestroy], + ["rebuild", prepareMcpBridgesForRebuild], + ] as const)( + "restores MCP runtime after %s detach failure without consulting a stale containing receipt (#9833)", + async (_operation, prepare) => { + harness.authority = "externally-managed"; + harness.detachOutcomes.push("unknown"); + registerSandbox("externally-managed"); + const validateContainingReceipt = vi + .fn<() => Promise>() + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce(undefined) + .mockRejectedValue(new Error("containing policy authority changed")); + + await expect(prepare("alpha", validateContainingReceipt)).rejects.toThrow( + "Could not prove provider detach", + ); + + expect(validateContainingReceipt).toHaveBeenCalledTimes(3); + expect(harness.actions).toEqual([ + "adapter:scrub", + "provider:detach", + "provider:attach", + "adapter:rollback", + ]); + expect(harness.adapterRegistered).toBe(true); + expect(harness.providerAttached).toBe(true); + }, + ); + + it("reports both authority refusal and snapshot drift during destroy-abort recovery (#9833)", async () => { + harness.authority = "externally-managed"; + registerSandbox("externally-managed"); + const preparation = await prepareMcpBridgesForDestroy("alpha"); + const preparedMcp = registry.getSandbox("alpha")?.mcp; + registry.updateSandbox("alpha", { + mcp: { + ...preparedMcp, + bridges: { + example: { ...bridgeEntry, url: "https://changed.example.test/mcp" }, + }, + }, + }); + + let refusal: unknown; + try { + await restoreMcpBridgesAfterDestroyAbort("alpha", preparation, async () => { + throw new McpPolicyAuthorityRefusalError("destroy policy authority changed"); + }); + } catch (error) { + refusal = error; + } + + expect(refusal).toBeInstanceOf(McpPolicyAuthorityRefusalError); + expect(refusal).toEqual( + expect.objectContaining({ + message: expect.stringContaining("destroy policy authority changed"), + cause: expect.any(AggregateError), + }), + ); + expect((refusal as Error).message).toContain("snapshot validation also failed"); + expect((refusal as Error).message).toContain("changed"); + expect(harness.actions).toEqual(["adapter:scrub", "provider:detach"]); + expect(registry.getSandbox("alpha")?.mcp?.destroyPreparedAt).toEqual(expect.any(String)); + }); +}); diff --git a/src/lib/actions/sandbox/policy-channel.ts b/src/lib/actions/sandbox/policy-channel.ts index b7418ff62fe..d0605bc1138 100644 --- a/src/lib/actions/sandbox/policy-channel.ts +++ b/src/lib/actions/sandbox/policy-channel.ts @@ -980,6 +980,40 @@ async function applyChannelAddToGatewayAndRegistry( return true; } +export function revalidateMessagingProviderAttachmentTarget( + sandboxName: string, + gatewayName: string, +): void { + const expected = registry.getSandbox(sandboxName); + const lifecycleGeneration = expected?.lifecycleGeneration; + const expectedFingerprint = expected?.lifecycleLiveIdentityFingerprint; + if ( + !expected || + typeof lifecycleGeneration !== "string" || + typeof expectedFingerprint !== "string" || + (expected.gatewayName && expected.gatewayName !== gatewayName) + ) { + throw new Error( + `Sandbox '${sandboxName}' has incomplete lifecycle identity for messaging provider attachment.`, + ); + } + const liveFingerprint = policyChannelDependencies.inspectMessagingProviderAttachmentTarget( + sandboxName, + gatewayName, + ); + const confirmed = registry.getSandbox(sandboxName); + if ( + liveFingerprint !== expectedFingerprint || + confirmed?.lifecycleGeneration !== lifecycleGeneration || + confirmed.lifecycleLiveIdentityFingerprint !== expectedFingerprint || + (confirmed.gatewayName && confirmed.gatewayName !== gatewayName) + ) { + throw new Error( + `Sandbox '${sandboxName}' changed before messaging provider attachment completed.`, + ); + } +} + // Remove a channel's bridge providers from the gateway and drop it from the // compiled messaging plan. Mirrors applyChannelAddToGatewayAndRegistry. async function applyChannelRemoveToGatewayAndRegistry( @@ -1005,9 +1039,9 @@ async function applyChannelRemoveToGatewayAndRegistry( ...bridgeProviderNamesForChannel(sandboxName, channelName), ]), ]; + const gatewayName = getSandboxTargetGatewayName(sandboxName); if (providerNames.length > 0) { - const gatewayName = getSandboxTargetGatewayName(sandboxName); const recovery = await recoverNamedGatewayRuntime({ gatewayName }); if (!recovery.recovered) { console.error( diff --git a/src/lib/actions/sandbox/rebuild-destroy-phase.test.ts b/src/lib/actions/sandbox/rebuild-destroy-phase.test.ts index d9ceda0b10b..cd231682266 100644 --- a/src/lib/actions/sandbox/rebuild-destroy-phase.test.ts +++ b/src/lib/actions/sandbox/rebuild-destroy-phase.test.ts @@ -459,6 +459,63 @@ describe("rebuild destroy phase", () => { ); }); + it("withholds delete when read-only MCP state drifts during final policy validation (#9833)", async () => { + const revalidateBeforeDelete = vi.fn().mockResolvedValue(undefined); + const assertDeleteEdgeUnchanged = vi + .fn() + .mockReturnValueOnce(undefined) + .mockImplementationOnce(() => { + throw new Error("MCP bridge definitions changed"); + }); + const validateBeforeDeleteCommit = vi.fn().mockResolvedValue({ ok: true as const }); + const detachedProviderEntries = [{ providerName: "nemoclaw-mcp-alpha-github" }]; + const scrubbedAdapterEntries = [{ server: "github" }]; + mocks.prepareMcpForRebuild.mockResolvedValue({ + entries: [{ server: "github" }], + detachedProviderEntries, + scrubbedAdapterEntries, + revalidateBeforeDelete, + assertDeleteEdgeUnchanged, + }); + const recreateJournal = stubRecreateJournal(); + const relockShieldsIfNeeded = vi.fn(() => true); + + await expect( + runRebuildDestroyPhase({ + sandboxName: "alpha", + sandboxEntry: { name: "alpha", agent: "openclaw" }, + staleRecovery: false, + recreateJournal, + backupManifest: null, + force: true, + log: vi.fn(), + bail: vi.fn((message: string): never => { + throw new Error(message); + }), + relockShieldsIfNeeded, + validateBeforeDeleteCommit, + onDeleted: vi.fn(), + }), + ).rejects.toThrow( + "Failed to revalidate read-only MCP recovery before sandbox deletion: MCP bridge definitions changed", + ); + + expect(validateBeforeDeleteCommit).toHaveBeenCalledTimes(2); + expect(assertDeleteEdgeUnchanged).toHaveBeenCalledTimes(2); + expect(validateBeforeDeleteCommit.mock.invocationCallOrder[1]).toBeLessThan( + assertDeleteEdgeUnchanged.mock.invocationCallOrder[1]!, + ); + expect(recreateJournal.markDeleting).toHaveBeenCalledOnce(); + expect(mocks.reattachMcpAfterDeleteFailure).toHaveBeenCalledWith( + "alpha", + detachedProviderEntries, + scrubbedAdapterEntries, + undefined, + ); + expect(relockShieldsIfNeeded).toHaveBeenCalledWith(true); + expectNoSandboxDelete(mocks.runOpenshell); + }); + it("converges as deleted when a nonzero delete is followed by exact NotFound (#7062)", async () => { mocks.getSandbox.mockReturnValueOnce({ name: "alpha", @@ -756,7 +813,7 @@ describe("rebuild destroy phase", () => { }); expect(revalidateBeforeDelete).toHaveBeenCalledOnce(); - expect(assertDeleteEdgeUnchanged).toHaveBeenCalledOnce(); + expect(assertDeleteEdgeUnchanged).toHaveBeenCalledTimes(2); expect(mocks.stopNimContainer).not.toHaveBeenCalled(); expect(mocks.stopNimContainerByName).toHaveBeenCalledWith("nim-alpha"); expect(assertDeleteEdgeUnchanged.mock.invocationCallOrder[0]).toBeLessThan( diff --git a/src/lib/actions/sandbox/rebuild-destroy-phase.ts b/src/lib/actions/sandbox/rebuild-destroy-phase.ts index 8841d996f81..b2578cbb815 100644 --- a/src/lib/actions/sandbox/rebuild-destroy-phase.ts +++ b/src/lib/actions/sandbox/rebuild-destroy-phase.ts @@ -525,6 +525,28 @@ export async function runRebuildDestroyPhase( return null; } } + if (mcpPreparation.assertDeleteEdgeUnchanged) { + try { + // The final policy proof above can await external state. Close that + // window with a synchronous registry proof before the delete command. + mcpPreparation.assertDeleteEdgeUnchanged(); + } catch (error) { + const mcpRecoveryFailure = await reattachMcpAfterDeleteFailure( + sandboxName, + rebuildDetachedMcpProviderEntries, + rebuildScrubbedMcpAdapterEntries, + validateMcpPolicyAuthorityReceipt, + ); + relockShieldsIfNeeded(true); + const detail = error instanceof Error ? error.message : String(error); + bail( + mcpRecoveryFailure + ? `Failed to revalidate read-only MCP recovery before sandbox deletion: ${redactFull(detail)} MCP provider recovery also failed: ${mcpRecoveryFailure}` + : `Failed to revalidate read-only MCP recovery before sandbox deletion: ${redactFull(detail)}`, + ); + return null; + } + } if (sourcePresence === "missing") { log(`Skipping delete: gateway ${gatewayName} reports '${sandboxName}' already absent`); } else { diff --git a/src/lib/actions/sandbox/rebuild-mcp-phase.test.ts b/src/lib/actions/sandbox/rebuild-mcp-phase.test.ts index ff6a645267b..4d6194c1f2e 100644 --- a/src/lib/actions/sandbox/rebuild-mcp-phase.test.ts +++ b/src/lib/actions/sandbox/rebuild-mcp-phase.test.ts @@ -365,4 +365,26 @@ describe("MCP rebuild policy authority forwarding", () => { expect(relock).not.toHaveBeenCalled(); expect(bail).not.toHaveBeenCalled(); }); + + it("forwards and preserves policy authority during host-side recovery (#9833)", async () => { + const refusal = new PolicyAuthorityRefusalError("external MCP requirement changed"); + const validateContainingPolicyReceipt = vi.fn(async () => undefined); + mocks.executeSandboxExecCommand.mockReturnValue(null); + mocks.prepareExecUnavailable.mockRejectedValueOnce(refusal); + const relock = vi.fn(() => true); + const bail = vi.fn((message: string): never => { + throw new Error(message); + }); + + await expect( + prepareMcpForRebuild("alpha", false, true, relock, bail, validateContainingPolicyReceipt), + ).rejects.toBe(refusal); + + expect(mocks.prepareExecUnavailable).toHaveBeenCalledWith( + "alpha", + validateContainingPolicyReceipt, + ); + expect(relock).not.toHaveBeenCalled(); + expect(bail).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/actions/sandbox/rebuild-mcp-phase.ts b/src/lib/actions/sandbox/rebuild-mcp-phase.ts index 827e0f937e3..608996d89be 100644 --- a/src/lib/actions/sandbox/rebuild-mcp-phase.ts +++ b/src/lib/actions/sandbox/rebuild-mcp-phase.ts @@ -56,8 +56,11 @@ export async function prepareMcpForRebuild( if (force && !staleRecovery && !canExecuteMcpPreparation(sandboxName)) { console.error(` ${YW}⚠${R} MCP transport probe failed; --force using host-side MCP recovery`); try { - return await prepareMcpBridgesForExecUnavailableRebuild(sandboxName); + return await (validateContainingPolicyReceipt + ? prepareMcpBridgesForExecUnavailableRebuild(sandboxName, validateContainingPolicyReceipt) + : prepareMcpBridgesForExecUnavailableRebuild(sandboxName)); } catch (error) { + if (isPolicyAuthorityRefusalError(error)) throw error; relockShieldsIfNeeded(true); bail( `Failed to preserve MCP bridges before rebuild (--force host-side recovery): ${error instanceof Error ? error.message : String(error)}`, diff --git a/test/agents/deepagents/deepagents-mcp-legacy-lifecycle.test.ts b/test/agents/deepagents/deepagents-mcp-legacy-lifecycle.test.ts index f3ff7424759..53de6de5616 100644 --- a/test/agents/deepagents/deepagents-mcp-legacy-lifecycle.test.ts +++ b/test/agents/deepagents/deepagents-mcp-legacy-lifecycle.test.ts @@ -43,6 +43,10 @@ vi.mock("../../../src/lib/actions/sandbox/process-recovery", () => ({ executeSandboxExecCommand: mocks.executeSandboxExecCommand, })); +vi.mock("../src/lib/actions/sandbox/policy-authority/preflight", () => ({ + preflightSandboxPolicyAuthority: vi.fn(() => "nemoclaw-managed"), +})); + const MATCHING_OPENSHELL = path.resolve("test/fixtures/openshell-v0.0.106"); const ORIGINAL_HOME = process.env.HOME; const ORIGINAL_OPENSHELL_BIN = process.env.NEMOCLAW_OPENSHELL_BIN; diff --git a/test/agents/hermes/hermes-mcp-shields-order.test.ts b/test/agents/hermes/hermes-mcp-shields-order.test.ts index 6576f7a0564..cf881c079f2 100644 --- a/test/agents/hermes/hermes-mcp-shields-order.test.ts +++ b/test/agents/hermes/hermes-mcp-shields-order.test.ts @@ -18,6 +18,7 @@ const registry = require("./src/lib/state/registry.js"); const providerCommands = require("./src/lib/adapters/openshell/provider-command.js"); const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); const shields = require("./src/lib/shields/index.js"); @@ -50,6 +51,7 @@ providerCommands.runOpenshellProviderCommand = (args) => { policies.getPresetContentGatewayState = () => "match"; policies.applyPresetContent = () => { mutations.push("policy:apply"); return true; }; policies.removePreset = () => { mutations.push("policy:remove"); return true; }; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; processRecovery.executeSandboxCommand = (_sandboxName, command) => { mutations.push("adapter:" + command); return { status: 0, stdout: '{"ok":true}\n', stderr: "" }; @@ -138,8 +140,11 @@ const capture = async (operation) => { freshManifest?: unknown; }; expect(payload.messages).toHaveLength(4); - expect(payload.messages.every((message) => - message.includes("has shields up or an unreadable shields posture"))).toBe(true); + expect( + payload.messages.every((message) => + message.includes("has shields up or an unreadable shields posture"), + ), + ).toBe(true); expect(payload.mutations).toEqual([]); expect(payload.freshManifest).toBeUndefined(); }); diff --git a/test/helpers/destroy-flow-test-assertions.ts b/test/helpers/destroy-flow-test-assertions.ts index 72f5b457b45..0bc7719fc89 100644 --- a/test/helpers/destroy-flow-test-assertions.ts +++ b/test/helpers/destroy-flow-test-assertions.ts @@ -169,13 +169,17 @@ export function expectFailedHardeningMcpRestore(harness: DestroyHarness): void { expect(harness.restoreMcpBridgesAfterDestroyAbortSpy).toHaveBeenCalledWith( "alpha", expect.objectContaining({ entries: [{ server: "github" }] }), + expect.any(Function), ); expect(harness.finalizeMcpBridgesAfterSandboxDeleteSpy).not.toHaveBeenCalled(); expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); } export function expectMcpFinalizeAfterDelete(harness: DestroyHarness): void { - expect(harness.prepareMcpBridgesForDestroySpy).toHaveBeenCalledWith("alpha"); + expect(harness.prepareMcpBridgesForDestroySpy).toHaveBeenCalledWith( + "alpha", + expect.any(Function), + ); expect(harness.gatewayPinsAtMcpPrepare).toEqual(["nemoclaw-19080"]); const deleteCall = harness.runOpenshellSpy.mock.calls.findIndex( (call) => Array.isArray(call[0]) && call[0].join(" ") === "sandbox delete alpha", @@ -201,6 +205,7 @@ export function expectMcpRestoreAfterDeleteFailure(harness: DestroyHarness): voi expect(harness.restoreMcpBridgesAfterDestroyAbortSpy).toHaveBeenCalledWith( "alpha", expect.objectContaining({ entries: [{ server: "github" }] }), + expect.any(Function), ); expect(harness.finalizeMcpBridgesAfterSandboxDeleteSpy).not.toHaveBeenCalled(); expect(harness.removeSandboxSpy).not.toHaveBeenCalled(); diff --git a/test/helpers/destroy-flow-test-harness.ts b/test/helpers/destroy-flow-test-harness.ts index f390d1be22b..209f017c60b 100644 --- a/test/helpers/destroy-flow-test-harness.ts +++ b/test/helpers/destroy-flow-test-harness.ts @@ -93,7 +93,12 @@ type DestroyHarnessOptions = { onPrepareManagedLlamaCppRuntimeCleanup?: () => void; preparedManagedLlamaCppRuntimeCleanup?: PreparedManagedLlamaCppRuntimeCleanup | null; mcpAddState?: "prepared"; + mcpPolicyAuthorityAfterDeleteError?: string; mcpServers?: string[]; + policyAuthority?: "nemoclaw-managed" | "externally-managed"; + policyAuthorityAfterDelete?: "nemoclaw-managed" | "externally-managed"; + policyAuthorityDuringMcpFinalization?: "nemoclaw-managed" | "externally-managed"; + revalidateMcpPolicyAuthority?: () => void | Promise; openshellDriver?: string; portableCommandError?: string; portableDestroyAuthority?: boolean; @@ -279,7 +284,8 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr detected: true, sessions: [{ pid: 1 }], }); - vi.spyOn(registry, "getSandbox").mockReturnValue( + let policyAuthority = options.policyAuthority; + vi.spyOn(registry, "getSandbox").mockImplementation(() => options.registryEntryPresent === false ? null : { @@ -296,6 +302,7 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr ? { hostLocalInferenceProvenance: options.hostLocalInferenceProvenance } : {}), ...(options.workload ? { workload: options.workload } : {}), + ...(policyAuthority ? { policyAuthority } : {}), ...(options.mcpServers?.length ? { mcp: { @@ -414,6 +421,9 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr }; case "sandbox:delete": events.push("delete"); + if (options.policyAuthorityAfterDelete) { + policyAuthority = options.policyAuthorityAfterDelete; + } return { status: options.deleteStatus === undefined ? 0 : options.deleteStatus, stdout: options.deleteOutput ?? "", @@ -550,13 +560,34 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr const { McpBridgeError } = mcpBridge as any; const prepareMcpBridgesForDestroySpy = vi .spyOn(mcpBridge, "prepareMcpBridgesForDestroy") - .mockImplementation(async () => { + .mockImplementation(async (...args: unknown[]) => { + const validateContaining = args[1] as (() => Promise) | undefined; events.push("mcp-prepare"); if (options.prepareMcpBridgeError !== undefined) { throw new McpBridgeError(options.prepareMcpBridgeError); } gatewayPinsAtMcpPrepare.push(process.env.OPENSHELL_GATEWAY); - return mcpPreparation; + return { + ...mcpPreparation, + ...(options.revalidateMcpPolicyAuthority || validateContaining + ? { + revalidateBeforeDelete: async () => { + await validateContaining?.(); + if (options.revalidateMcpPolicyAuthority) { + events.push("mcp-revalidate"); + await options.revalidateMcpPolicyAuthority(); + } + }, + revalidateAfterDelete: async () => { + await validateContaining?.(); + if (options.mcpPolicyAuthorityAfterDeleteError) { + throw new Error(options.mcpPolicyAuthorityAfterDeleteError); + } + }, + revalidateBeforeSuccess: async () => validateContaining?.(), + } + : {}), + }; }); const prepareMcpBridgesForAbsentSandboxDestroySpy = vi .spyOn(mcpBridge, "prepareMcpBridgesForAbsentSandboxDestroy") @@ -578,6 +609,9 @@ export function createDestroyHarness(options: DestroyHarnessOptions = {}): Destr if (options.finalizeMcpBridgeError !== undefined) { return Promise.reject(new McpBridgeError(options.finalizeMcpBridgeError)); } + if (options.policyAuthorityDuringMcpFinalization) { + policyAuthority = options.policyAuthorityDuringMcpFinalization; + } return options.finalizeMcpError ? Promise.reject(new Error(options.finalizeMcpError)) : Promise.resolve(); diff --git a/test/helpers/mcp-destroy-lifecycle-support.ts b/test/helpers/mcp-destroy-lifecycle-support.ts new file mode 100644 index 00000000000..7942647be7f --- /dev/null +++ b/test/helpers/mcp-destroy-lifecycle-support.ts @@ -0,0 +1,30 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { vi } from "vitest"; + +import type { RebuildRecreateJournal } from "../../src/lib/actions/sandbox/rebuild-recreate-journal"; + +export function stubRecreateJournal(): RebuildRecreateJournal { + return { + id: "journal-1", + acceptedTarget: false, + sourceConfirmedAbsent: false, + gatewayAuthority: { + gatewayName: "nemoclaw", + gatewayPort: 8080, + mode: "nemoclaw-managed", + source: "standalone", + endpoint: null, + stateDir: null, + supervisor: null, + requiredCapabilities: [], + }, + targetGeneration: "generation-1", + targetIntentFingerprint: "intent-1", + markDeleting: vi.fn(), + observeSourceForDelete: vi.fn(() => "source" as const), + confirmDeleted: vi.fn(), + completeAcceptedTarget: vi.fn(), + }; +} diff --git a/test/mcp/mcp-adapter-teardown-rollback.test.ts b/test/mcp/mcp-adapter-teardown-rollback.test.ts index 8dc5303424a..f4ef5465b5a 100644 --- a/test/mcp/mcp-adapter-teardown-rollback.test.ts +++ b/test/mcp/mcp-adapter-teardown-rollback.test.ts @@ -50,11 +50,11 @@ describe("MCP adapter teardown rollback", () => { testState.registerAdapter.mockReset(); }); - it("restores the adapter with the fresh opaque credential revision (#10300)", () => { + it("restores the adapter with the fresh opaque credential revision (#10300)", async () => { const opaqueRevision = "v4067750153477477215"; testState.observeCredentialRevision.mockReturnValue(opaqueRevision); - const failures = rollbackScrubbedMcpAdapters("alpha", sandbox, [ + const failures = await rollbackScrubbedMcpAdapters("alpha", sandbox, [ { ...entry, credentialRevision: "v1" }, ]); @@ -71,10 +71,10 @@ describe("MCP adapter teardown rollback", () => { it.each(["absent", "canonical"] as const)( "reports rollback failure when fresh credential authority is %s (#10300)", - (observation) => { + async (observation) => { testState.observeCredentialRevision.mockReturnValue(observation); - const failures = rollbackScrubbedMcpAdapters("alpha", sandbox, [ + const failures = await rollbackScrubbedMcpAdapters("alpha", sandbox, [ { ...entry, credentialRevision: "v4067750153477477215" }, ]); diff --git a/test/mcp/mcp-add-crash-consistency.test.ts b/test/mcp/mcp-add-crash-consistency.test.ts index ba990bb7549..ce19d9d8557 100644 --- a/test/mcp/mcp-add-crash-consistency.test.ts +++ b/test/mcp/mcp-add-crash-consistency.test.ts @@ -8,6 +8,8 @@ import path from "node:path"; import { describe, expect, it, vi } from "vitest"; +import { managedSandboxEntry } from "../helpers/managed-policy-receipt-fixture"; + const MATCHING_OPENSHELL = path.resolve("test/fixtures/openshell-v0.0.106"); type CrashBoundary = @@ -23,11 +25,16 @@ type CrashBoundary = | "registered-credential-collision" | "registered-late-collision" | "adapter" + | "adapter-authority-refusal" | "adapter-mismatch" + | "authority-external" + | "gateway-authority-refusal" + | "authority-missing" | "attach-race" | "race" | "late-race" | "preupdate-observation-forbidden" + | "restart-provider-authority-refusal" | ""; function buildAddProcessScript( @@ -35,6 +42,7 @@ function buildAddProcessScript( crashAfter: CrashBoundary, includeSecret = true, initializeSandbox = true, + operation: "add" | "restart" = "add", ): string { return String.raw` process.env.HOME = ${JSON.stringify(home)}; @@ -44,6 +52,13 @@ includeSecret ? (process.env.FAKE_MCP_SECRET = "host-only-secret") : delete proc const fs = require("node:fs"); const path = require("node:path"); const crashAfter = ${JSON.stringify(crashAfter)}; +const operation = ${JSON.stringify(operation)}; +const externalAuthority = + crashAfter === "authority-external" || + crashAfter === "gateway-authority-refusal" || + crashAfter === "authority-missing" || + crashAfter === "adapter-authority-refusal" || + crashAfter === "restart-provider-authority-refusal"; const credentialProjectionScenario = crashAfter === "credential-projection-coalesced" || crashAfter === "credential-projection-unstable"; @@ -52,7 +67,7 @@ if (crashAfter === "credential-projection-coalesced") { } else if (crashAfter === "credential-projection-unstable") { process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "2"; } else if (crashAfter === "credential-projection-delayed-hostless") { - process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "2"; + process.env.NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS = "4"; } const marker = (name) => path.join(process.env.HOME, name + ".marker"); const mark = (name) => fs.writeFileSync(marker(name), "yes\n", { mode: 0o600 }); @@ -72,6 +87,7 @@ const advanceChildCredentialRevision = () => setChildCredentialRevision( const setProviderVersion = (version) => fs.writeFileSync(marker("provider-version"), String(version), { mode: 0o600 }); const providerPresentAtStart = marked("provider"); const providerId = "11111111-2222-4333-8444-555555555555"; +const replacementProviderId = "22222222-3333-4444-8555-666666666666"; const foreignProviderId = "99999999-8888-4777-8666-555555555555"; let providerGetCount = 0; let observedProviderName = null; @@ -89,6 +105,8 @@ const providerCommands = require("./src/lib/adapters/openshell/provider-command. const { mockManagedEndpointlessProviderProfileRun } = require("./test/helpers/onboard-script-mocks.cjs"); const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); const ownershipLocks = require("./src/lib/state/mcp-lifecycle-lock/credential-ownership.js"); @@ -100,12 +118,15 @@ if (crashAfter === "credential-command-race") { }; } -gatewayRuntime.recoverNamedGatewayRuntime = async () => ({ - recovered: true, - attempted: false, - before: { state: "healthy_named" }, - after: { state: "healthy_named" }, -}); +gatewayRuntime.recoverNamedGatewayRuntime = async () => { + if (crashAfter === "gateway-authority-refusal") mark("gateway-selected"); + return { + recovered: true, + attempted: false, + before: { state: "healthy_named" }, + after: { state: "healthy_named" }, + }; +}; providerCommands.runOpenshellProviderCommand = (args) => { const profileResult = mockManagedEndpointlessProviderProfileRun(args); @@ -122,19 +143,20 @@ providerCommands.runOpenshellProviderCommand = (args) => { if (crashAfter === "race" && providerGetCount === 2) mark("provider"); if (crashAfter === "late-race" && providerGetCount === 3) mark("provider"); return marked("provider") - ? { status: 0, stdout: "Id: " + (marked("foreign-provider") ? foreignProviderId : providerId) + "\nType: nemoclaw-mcp-v1\nResource version: " + providerVersion() + "\nCredential keys: FAKE_MCP_SECRET\n", stderr: "" } + ? { status: 0, stdout: "Id: " + (marked("foreign-provider") ? foreignProviderId : marked("replacement-provider") ? replacementProviderId : providerId) + "\nType: nemoclaw-mcp-v1\nResource version: " + providerVersion() + "\nCredential keys: FAKE_MCP_SECRET\n", stderr: "" } : { status: 1, stdout: "", stderr: "NotFound: provider" }; } if (args[0] === "provider" && (args[1] === "create" || args[1] === "update")) { if (credentialProjectionScenario) { fs.appendFileSync(marker("provider-mutation-log"), args[1] + "\n", { mode: 0o600 }); } - if (!marked("policy")) { + if (!marked("policy") && !externalAuthority) { return { status: 1, stdout: "", stderr: "provider mutation preceded policy attestation" }; } if (args[1] === "create") { observedProviderName = args[args.indexOf("--name") + 1]; setProviderVersion(1); + if (crashAfter === "restart-provider-authority-refusal") mark("replacement-provider"); setChildCredentialRevision(initialChildCredentialRevision); } if (args[1] === "update") { @@ -250,6 +272,34 @@ policies.removePreset = () => { fs.rmSync(marker("policy"), { force: true }); return true; }; +const inspectPolicyAuthority = (options) => { + if (crashAfter === "authority-missing") { + const requirement = options.requiredPolicyContents?.[0] ?? ""; + const providerName = /\n\s+provider:\s+(\S+)/u.exec(requirement)?.[1] ?? "missing"; + fs.appendFileSync(marker("authority-provider-name"), providerName + "\n", { mode: 0o600 }); + throw new mcpPolicy.McpPolicyAuthorityRefusalError( + "the externally managed policy has missing entries", + ); + } + if (crashAfter === "adapter-authority-refusal" && marked("adapter")) { + throw new mcpPolicy.McpPolicyAuthorityRefusalError( + "OpenShell policy authority changed after adapter registration", + ); + } + if (crashAfter === "gateway-authority-refusal" && marked("gateway-selected")) { + throw new mcpPolicy.McpPolicyAuthorityRefusalError( + "OpenShell policy authority changed during gateway selection", + ); + } + if (crashAfter === "restart-provider-authority-refusal" && marked("replacement-provider")) { + throw new mcpPolicy.McpPolicyAuthorityRefusalError( + "OpenShell policy authority changed after provider recreation", + ); + } + return externalAuthority ? "externally-managed" : "nemoclaw-managed"; +}; +mcpPolicy.preflightMcpPolicyAuthority = inspectPolicyAuthority; +policyAuthority.preflightSandboxPolicyAuthority = inspectPolicyAuthority; processRecovery.executeSandboxExecCommand = (_sandbox, command) => { const encoded = command.match(/printf '%s' '([A-Za-z0-9+/=]+)' \| base64 -d/)?.[1] || ""; @@ -355,21 +405,29 @@ processRecovery.executeSandboxCommand = (_sandbox, command) => { }; if (initializeSandbox && !registry.getSandbox("crash-test")) { - registry.registerSandbox({ - name: "crash-test", - agent: "openclaw", - gatewayName: "nemoclaw", - }); + registry.registerSandbox( + externalAuthority + ? { + name: "crash-test", + agent: "openclaw", + gatewayName: "nemoclaw", + policyAuthority: "externally-managed", + } + : ${JSON.stringify(managedSandboxEntry("crash-test"))}, + ); } if (crashAfter === "registered-credential-collision") { registry.addExtraProvider("foreign-registered"); } const bridge = require("./src/lib/actions/sandbox/mcp-bridge.js"); -bridge.addMcpBridge("crash-test", { +const bridgeOperation = operation === "restart" + ? bridge.restartMcpBridge("crash-test", "fake") + : bridge.addMcpBridge("crash-test", { server: "fake", url: "https://8.8.8.8/mcp", env: [{ name: "FAKE_MCP_SECRET" }], -}).then( +}); +bridgeOperation.then( async () => { try { if (crashAfter === "credential-projection-coalesced") { @@ -390,12 +448,29 @@ bridge.addMcpBridge("crash-test", { `; } -function initializeSandboxRegistry(home: string): void { +function initializeSandboxRegistry( + home: string, + policyAuthority = "nemoclaw-managed", + lifecycleGeneration?: string, + replace = false, +): void { + const sandboxEntry = + policyAuthority === "nemoclaw-managed" + ? managedSandboxEntry("crash-test", "openclaw", { + ...(lifecycleGeneration ? { lifecycleGeneration } : {}), + }) + : { + name: "crash-test", + agent: "openclaw", + gatewayName: "nemoclaw", + policyAuthority, + ...(lifecycleGeneration ? { lifecycleGeneration } : {}), + }; const result = spawnSync( process.execPath, [ "-e", - `process.env.HOME = ${JSON.stringify(home)}; const registry = require("./src/lib/state/registry.js"); registry.registerSandbox({ name: "crash-test", agent: "openclaw", gatewayName: "nemoclaw" });`, + `process.env.HOME = ${JSON.stringify(home)}; const registry = require("./src/lib/state/registry.js"); if (${JSON.stringify(replace)}) registry.removeSandbox("crash-test"); registry.registerSandbox(${JSON.stringify(sandboxEntry)});`, ], { cwd: process.cwd(), @@ -410,8 +485,13 @@ function initializeSandboxRegistry(home: string): void { ).toBe(0); } -function runAddProcess(home: string, crashAfter: CrashBoundary, includeSecret = true) { - const script = buildAddProcessScript(home, crashAfter, includeSecret); +function runAddProcess( + home: string, + crashAfter: CrashBoundary, + includeSecret = true, + operation: "add" | "restart" = "add", +) { + const script = buildAddProcessScript(home, crashAfter, includeSecret, true, operation); return spawnSync(process.execPath, ["-e", script], { cwd: process.cwd(), encoding: "utf8", @@ -420,6 +500,21 @@ function runAddProcess(home: string, crashAfter: CrashBoundary, includeSecret = }); } +function expectExternalAuthorityRefusal(result: ReturnType): void { + expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(2); + expect(result.stderr).toContain("missing entries"); + expect(`${result.stdout}\n${result.stderr}`).not.toContain("host-only-secret"); +} + +function expectNoAddMutationMarkers(home: string): void { + expect({ + adapter: fs.existsSync(path.join(home, "adapter.marker")), + attached: fs.existsSync(path.join(home, "attached.marker")), + policy: fs.existsSync(path.join(home, "policy.marker")), + provider: fs.existsSync(path.join(home, "provider.marker")), + }).toEqual({ adapter: false, attached: false, policy: false, provider: false }); +} + function spawnScript(home: string, script: string): ChildProcessWithoutNullStreams { return spawn(process.execPath, ["-e", script], { cwd: process.cwd(), @@ -521,6 +616,8 @@ const providerCommands = require("./src/lib/adapters/openshell/provider-command. const { mockManagedEndpointlessProviderProfileRun } = require("./test/helpers/onboard-script-mocks.cjs"); const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); gatewayRuntime.recoverNamedGatewayRuntime = async () => ({ @@ -585,6 +682,8 @@ policies.removePreset = () => { fs.rmSync(marker("policy"), { force: true }); return true; }; +mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; processRecovery.executeSandboxCommand = (_sandbox, command) => { if ( @@ -714,7 +813,9 @@ describe("MCP add crash consistency", () => { .join("\n---\n"); expect(results.map((result) => result.status).sort(), combinedOutput).toEqual([0, 2]); - expect(results.find((result) => result.status === 2)?.stderr).toContain("already exists"); + expect(results.find((result) => result.status === 2)?.stderr).toMatch( + /already exists|already supplied by attached provider/, + ); expect(combinedOutput).not.toContain("host-only-secret"); expect(fs.existsSync(path.join(home, "credential-observed-absent.marker"))).toBe(true); expect(fs.existsSync(path.join(home, "republish-before-observation.marker"))).toBe(false); @@ -730,17 +831,17 @@ describe("MCP add crash consistency", () => { .split("\n") .filter(Boolean), ).toEqual(["create", "update"]); + const credentialObservations = fs + .readFileSync(path.join(home, "credential-observation-log.marker"), "utf8") + .split("\n") + .filter(Boolean); + expect(credentialObservations[0]).toBe("v4067750153477477214"); + expect(credentialObservations.slice(1)).toHaveLength(6); expect( - fs - .readFileSync(path.join(home, "credential-observation-log.marker"), "utf8") - .split("\n") - .filter(Boolean), - ).toEqual([ - "v4067750153477477214", - "v4067750153477477215", - "v4067750153477477215", - "v4067750153477477215", - ]); + credentialObservations + .slice(1) + .every((revision) => revision === "v4067750153477477215"), + ).toBe(true); expect(fs.readFileSync(path.join(home, "adapter-revision.marker"), "utf8")).toBe( fs.readFileSync(path.join(home, "child-credential-revision.marker"), "utf8"), ); @@ -852,6 +953,163 @@ describe("MCP add crash consistency", () => { } }); + it("keeps the external provider binding stable across first refusal retries and changes it for a new sandbox lifecycle (#9833)", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-external-binding-")); + try { + initializeSandboxRegistry(home, "externally-managed", "external-generation-1"); + + const first = runAddProcess(home, "authority-missing"); + const second = runAddProcess(home, "authority-missing"); + expectExternalAuthorityRefusal(first); + expectExternalAuthorityRefusal(second); + + const providerNames = fs + .readFileSync(path.join(home, "authority-provider-name.marker"), "utf8") + .trim() + .split("\n"); + expect(providerNames).toHaveLength(2); + expect(providerNames[1]).toBe(providerNames[0]); + expect( + JSON.parse(fs.readFileSync(path.join(home, ".nemoclaw", "sandboxes.json"), "utf8")) + .sandboxes["crash-test"].mcp, + ).toBeUndefined(); + expectNoAddMutationMarkers(home); + + initializeSandboxRegistry(home, "externally-managed", "external-generation-2", true); + const nextLifecycle = runAddProcess(home, "authority-missing"); + expect(nextLifecycle.status, `${nextLifecycle.stdout}\n${nextLifecycle.stderr}`).toBe(2); + const nextProviderName = fs + .readFileSync(path.join(home, "authority-provider-name.marker"), "utf8") + .trim() + .split("\n") + .at(-1); + expect(nextProviderName).not.toBe(providerNames[0]); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + it("adds and restarts under matching external policy without mutation or attribution (#9833)", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-external-positive-")); + try { + initializeSandboxRegistry(home, "externally-managed", "external-generation"); + + const added = runAddProcess(home, "authority-external"); + expect(added.status, `${added.stdout}\n${added.stderr}`).toBe(0); + const restarted = runAddProcess(home, "authority-external", true, "restart"); + expect(restarted.status, `${restarted.stdout}\n${restarted.stderr}`).toBe(0); + + const registry = JSON.parse( + fs.readFileSync(path.join(home, ".nemoclaw", "sandboxes.json"), "utf8"), + ) as { + sandboxes: { + "crash-test": { customPolicies?: unknown; policyAuthority?: string }; + }; + }; + expect(registry.sandboxes["crash-test"]).toMatchObject({ + policyAuthority: "externally-managed", + }); + expect(registry.sandboxes["crash-test"].customPolicies).toBeUndefined(); + expect(readBridge(home).addState).toBeUndefined(); + expect(fs.existsSync(path.join(home, "adapter.marker"))).toBe(true); + expect(fs.existsSync(path.join(home, "attached.marker"))).toBe(true); + expect(fs.existsSync(path.join(home, "provider.marker"))).toBe(true); + expect(fs.existsSync(path.join(home, "policy.marker"))).toBe(false); + expect(fs.existsSync(path.join(home, "policy-apply-log.marker"))).toBe(false); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + it("removes adapter and provider mutations after external authority changes post-adapter (#9833)", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-adapter-authority-")); + try { + initializeSandboxRegistry(home, "externally-managed", "external-generation"); + + const refused = runAddProcess(home, "adapter-authority-refusal"); + + expect(refused.status, `${refused.stdout}\n${refused.stderr}`).toBe(2); + expect(refused.stderr).toContain("authority changed after adapter registration"); + expect(`${refused.stdout}\n${refused.stderr}`).not.toContain("host-only-secret"); + expect(readBridge(home)).toMatchObject({ addState: "preflighted" }); + expectNoAddMutationMarkers(home); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + it("refuses before the first manifest write when authority changes during gateway selection (#9833)", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-gateway-authority-")); + try { + initializeSandboxRegistry(home, "externally-managed", "external-generation"); + + const refused = runAddProcess(home, "gateway-authority-refusal"); + + expect(refused.status, `${refused.stdout}\n${refused.stderr}`).toBe(2); + expect(refused.stderr).toContain("authority changed during gateway selection"); + expect( + JSON.parse(fs.readFileSync(path.join(home, ".nemoclaw", "sandboxes.json"), "utf8")) + .sandboxes["crash-test"].mcp, + ).toBeUndefined(); + expectNoAddMutationMarkers(home); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + it("journals a recreated provider identity before reporting authority drift (#9833)", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-restart-authority-")); + try { + initializeSandboxRegistry(home, "externally-managed", "external-generation"); + const added = runAddProcess(home, "authority-external"); + expect(added.status, `${added.stdout}\n${added.stderr}`).toBe(0); + fs.rmSync(path.join(home, "provider.marker")); + fs.rmSync(path.join(home, "attached.marker")); + fs.rmSync(path.join(home, "adapter.marker")); + + const refused = runAddProcess(home, "restart-provider-authority-refusal", true, "restart"); + + expect(refused.status, `${refused.stdout}\n${refused.stderr}`).toBe(2); + expect(refused.stderr).toContain("authority changed after provider recreation"); + expect(readBridge(home)).toMatchObject({ + providerId: "22222222-3333-4444-8555-666666666666", + }); + expect(fs.existsSync(path.join(home, "provider.marker"))).toBe(true); + expect(fs.existsSync(path.join(home, "attached.marker"))).toBe(false); + expect(fs.existsSync(path.join(home, "adapter.marker"))).toBe(false); + expect(fs.existsSync(path.join(home, "policy.marker"))).toBe(false); + + const recovered = runAddProcess(home, "authority-external", true, "restart"); + expect(recovered.status, `${recovered.stdout}\n${recovered.stderr}`).toBe(0); + expect(fs.existsSync(path.join(home, "attached.marker"))).toBe(true); + expect(fs.existsSync(path.join(home, "adapter.marker"))).toBe(true); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + + it("uses a different randomized provider name after a completed remove (#566)", () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-provider-name-lifecycle-")); + try { + const firstAdd = runAddProcess(home, ""); + expect(firstAdd.status, `${firstAdd.stdout}\n${firstAdd.stderr}`).toBe(0); + const firstProviderName = String(readBridge(home).providerName); + + const removed = runRemoveProcess(home, false); + expect(removed.status, `${removed.stdout}\n${removed.stderr}`).toBe(0); + + const secondAdd = runAddProcess(home, ""); + expect(secondAdd.status, `${secondAdd.stdout}\n${secondAdd.stderr}`).toBe(0); + const secondProviderName = String(readBridge(home).providerName); + + expect(firstProviderName).toMatch(/^crash-test-mcp-fake-[a-f0-9]{16}$/u); + expect(secondProviderName).toMatch(/^crash-test-mcp-fake-[a-f0-9]{16}$/u); + expect(secondProviderName).not.toBe(firstProviderName); + } finally { + fs.rmSync(home, { recursive: true, force: true }); + } + }); + it("resumes an exact provider without a host credential or prior revision observation", () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-add-reuse-no-observation-")); try { @@ -1229,6 +1487,10 @@ describe("MCP add crash consistency", () => { const cancelScript = ` process.env.HOME = ${JSON.stringify(home)}; const registry = require("./src/lib/state/registry.js"); +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; const bridge = require("./src/lib/actions/sandbox/mcp-bridge.js"); bridge.removeMcpBridge("crash-test", "fake", { force: true }).then( () => process.exit(0), diff --git a/test/mcp/mcp-bridge-destroy-marker-recovery.test.ts b/test/mcp/mcp-bridge-destroy-marker-recovery.test.ts index b98fe63ea9c..f0eb163aa68 100644 --- a/test/mcp/mcp-bridge-destroy-marker-recovery.test.ts +++ b/test/mcp/mcp-bridge-destroy-marker-recovery.test.ts @@ -52,6 +52,19 @@ function runNodeScript( return { status: result.status, stdout: result.stdout || "", stderr: result.stderr || "" }; } +function runManagedMcpNodeScript( + home: string, + script: string, +): { status: number | null; stdout: string; stderr: string } { + const managedPolicyAuthority = ` +const __mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const __policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +__mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +__policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; +`; + return runNodeScript(home, `${managedPolicyAuthority}\n${script}`); +} + const GITHUB_BRIDGE = `{ server: "github", agent: "openclaw", @@ -197,7 +210,7 @@ bridge.removeMcpBridge("stuck-sandbox", "github", { force: true }).then( }, ); `; - const result = runNodeScript(home, script); + const result = runManagedMcpNodeScript(home, script); expect(result.status).toBe(0); expect(result.stdout).toContain( "Cleared incomplete MCP destroy transaction on sandbox 'stuck-sandbox'", @@ -338,7 +351,7 @@ bridge.removeMcpBridge("stuck-sandbox", "github", { force: true }).then( }, ); `; - const result = runNodeScript(home, script); + const result = runManagedMcpNodeScript(home, script); expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); const removedLog = result.stdout.indexOf("Removed MCP server 'github'"); const clearedLog = result.stdout.indexOf("Cleared incomplete MCP destroy transaction"); @@ -419,7 +432,7 @@ bridge.removeMcpBridge("stuck-sandbox", "not-registered", { force: true }).then( }, ); `; - const result = runNodeScript(home, script); + const result = runManagedMcpNodeScript(home, script); expect(result.status).toBe(0); const jsonMarker = "<>"; const parsed = JSON.parse( @@ -460,7 +473,7 @@ bridge.removeMcpBridge("deleted-sandbox", "github", { force: true }).then( }, ); `; - const result = runNodeScript(home, script); + const result = runManagedMcpNodeScript(home, script); expect(result.status).toBe(0); const parsed = JSON.parse(result.stdout) as { error: string; @@ -525,7 +538,7 @@ bridge.removeMcpBridge("stuck-sandbox", "github", { force: true }).then( }, ); `; - const result = runNodeScript(home, script); + const result = runManagedMcpNodeScript(home, script); expect(result.status).toBe(0); const jsonMarker = "<>"; const jsonPayload = result.stdout.slice(result.stdout.indexOf(jsonMarker) + jsonMarker.length); @@ -544,7 +557,7 @@ bridge.removeMcpBridge("stuck-sandbox", "github", { force: true }).then( expect(result.stdout).not.toContain("Cleared incomplete MCP destroy transaction"); }); - it("preserves the prepared marker and manifest when forced cleanup tolerates residuals", async () => { + it("preserves the prepared marker and manifest when policy cleanup is unproved", async () => { const home = createTempHome("nemoclaw-force-residual-preserve-"); const script = ` process.env.HOME = ${JSON.stringify(home)}; @@ -571,23 +584,26 @@ registry.registerSandbox({ const bridge = require("./src/lib/actions/sandbox/mcp-bridge.js"); bridge.removeMcpBridge("stuck-sandbox", "github", { force: true, allowResidual: true }).then( () => { - const after = registry.getSandbox("stuck-sandbox"); - process.stdout.write("<>" + JSON.stringify({ mcp: after && after.mcp })); - process.exit(0); + process.exit(1); }, (error) => { - process.stderr.write(String(error && error.message || error)); - process.exit(1); + const after = registry.getSandbox("stuck-sandbox"); + process.stdout.write("<>" + JSON.stringify({ + error: String(error && error.message || error), + mcp: after && after.mcp, + })); + process.exit(0); }, ); `; - const result = runNodeScript(home, script); - expect(result.status).toBe(0); + const result = runManagedMcpNodeScript(home, script); + expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); expect(result.stderr).toContain("adapter cleanup failed (injected)"); const jsonMarker = "<>"; const parsed = JSON.parse( result.stdout.slice(result.stdout.indexOf(jsonMarker) + jsonMarker.length), - ) as { mcp: SandboxMcpSnapshot | undefined }; + ) as { error: string; mcp: SandboxMcpSnapshot | undefined }; + expect(parsed.error).toContain("Generated MCP policy cleanup"); expect(parsed.mcp?.destroyPreparedAt).toBe("2026-06-27T01:00:00.000Z"); expect(parsed.mcp?.managedServerNames).toEqual(["github"]); expect(parsed.mcp?.bridges).toHaveProperty("github"); @@ -630,7 +646,7 @@ bridge.removeMcpBridge("stuck-sandbox", "github", { force: true }).then( }, ); `; - const result = runNodeScript(home, script); + const result = runManagedMcpNodeScript(home, script); expect(result.status).toBe(0); const jsonMarker = "<>"; const parsed = JSON.parse( @@ -745,7 +761,7 @@ bridge.removeMcpBridge("stuck-sandbox", "github", {}).then( }, ); `; - const result = runNodeScript(home, script); + const result = runManagedMcpNodeScript(home, script); expect(result.status).toBe(0); expect(result.stdout).toContain("incomplete MCP destroy transaction"); expect(result.stdout).toContain("mcp remove --force"); diff --git a/test/mcp/mcp-destroy-lifecycle.test.ts b/test/mcp/mcp-destroy-lifecycle.test.ts index 1b86f0fc95f..ba87926b107 100644 --- a/test/mcp/mcp-destroy-lifecycle.test.ts +++ b/test/mcp/mcp-destroy-lifecycle.test.ts @@ -8,6 +8,7 @@ import { afterAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { AgentMcpAdapter } from "../../src/lib/agent/defs"; import type { McpBridgeEntry } from "../../src/lib/state/registry"; +import { managedSandboxEntry } from "../helpers/managed-policy-receipt-fixture"; import { findObservedCredentialRevision } from "../helpers/mcp-provider-revision"; import { mockManagedEndpointlessProviderProfileRun } from "../helpers/onboard-script-mocks.cjs"; @@ -38,6 +39,7 @@ const testState = vi.hoisted(() => { home, originalEnv, policyApplyCalls: 0, + preflightSandboxPolicyAuthority: vi.fn(), removedPolicyKeys: new Set(), providers: new Map< string, @@ -86,6 +88,10 @@ vi.mock("../../src/lib/actions/sandbox/process-recovery", () => ({ executeSandboxExecCommand: testState.executeSandboxExecCommand, })); +vi.mock("../../src/lib/actions/sandbox/policy-authority/preflight", () => ({ + preflightSandboxPolicyAuthority: testState.preflightSandboxPolicyAuthority, +})); + vi.mock("../../src/lib/actions/sandbox/rebuild-flow-helpers", async (importOriginal) => ({ ...(await importOriginal()), warnUnpreservedUserManagedFiles: testState.warnUnpreservedUserManagedFiles, @@ -99,32 +105,8 @@ vi.mock("../../src/lib/inference/nim", () => ({ import * as bridge from "../../src/lib/actions/sandbox/mcp-bridge"; import { isAgentMcpAdapter } from "../../src/lib/actions/sandbox/mcp-bridge-contracts"; import { runRebuildDestroyPhase } from "../../src/lib/actions/sandbox/rebuild-destroy-phase"; -import type { RebuildRecreateJournal } from "../../src/lib/actions/sandbox/rebuild-recreate-journal"; import * as registry from "../../src/lib/state/registry"; - -function stubRecreateJournal(): RebuildRecreateJournal { - return { - id: "journal-1", - acceptedTarget: false, - sourceConfirmedAbsent: false, - gatewayAuthority: { - gatewayName: "nemoclaw", - gatewayPort: 8080, - mode: "nemoclaw-managed", - source: "standalone", - endpoint: null, - stateDir: null, - supervisor: null, - requiredCapabilities: [], - }, - targetGeneration: "generation-1", - targetIntentFingerprint: "intent-1", - markDeleting: vi.fn(), - observeSourceForDelete: vi.fn(() => "source" as const), - confirmDeleted: vi.fn(), - completeAcceptedTarget: vi.fn(), - }; -} +import { stubRecreateJournal } from "../helpers/mcp-destroy-lifecycle-support"; const MATCHING_OPENSHELL = path.resolve("test/fixtures/openshell-v0.0.106"); @@ -166,9 +148,15 @@ function ownedPolicy( const resolvedAddresses = options.resolvedAddresses ?? [new URL(entry.url).hostname]; return { name: entry.policyName, - content: bridge.buildMcpBridgePolicyYaml(entry.server, entry.url, adapter as AgentMcpAdapter, { - addresses: [...resolvedAddresses], - }, entry.providerName ?? ""), + content: bridge.buildMcpBridgePolicyYaml( + entry.server, + entry.url, + adapter as AgentMcpAdapter, + { + addresses: [...resolvedAddresses], + }, + entry.providerName ?? "", + ), sourcePath: "generated:nemoclaw-mcp-bridge", }; } @@ -191,9 +179,7 @@ async function captureMessage(action: () => Promise): Promise { } function registerAlphaGithubBridge(): void { registry.registerSandbox({ - name: "alpha", - agent: "openclaw", - gatewayName: "nemoclaw", + ...managedSandboxEntry("alpha"), mcp: { bridges: { github: bridgeEntries.github } }, }); registry.addCustomPolicy("alpha", ownedPolicy("github")); @@ -226,6 +212,7 @@ beforeEach(() => { testState.failProviderDelete = null; testState.failProviderDetach = null; vi.resetAllMocks(); + testState.preflightSandboxPolicyAuthority.mockReturnValue("nemoclaw-managed"); testState.recoverNamedGatewayRuntime.mockResolvedValue({ recovered: true, attempted: false, @@ -306,8 +293,12 @@ beforeEach(() => { case args[0] === "sandbox" && args[1] === "provider" && args[2] === "attach": testState.attachedProviders.add(args[4]); return { status: 0, stdout: "Attached provider", stderr: "" }; - case args[0] === "provider" && args[1] === "update" && args.length === 3 && testState.providers.has(args[2]): - testState.providers.get(args[2])!.resourceVersion = (testState.providers.get(args[2])!.resourceVersion ?? 1) + 1; + case args[0] === "provider" && + args[1] === "update" && + args.length === 3 && + testState.providers.has(args[2]): + testState.providers.get(args[2])!.resourceVersion = + (testState.providers.get(args[2])!.resourceVersion ?? 1) + 1; return { status: 0, stdout: "Updated provider", stderr: "" }; case args[0] === "provider" && args[1] === "delete" && @@ -1239,10 +1230,15 @@ describe("authenticated MCP sandbox destroy lifecycle", () => { expect([...testState.providers.keys()]).toContain("alpha-mcp-github"); expect(testState.calls).toContain("sandbox provider attach alpha alpha-mcp-github"); expect(testState.providers.get("alpha-mcp-github")?.resourceVersion).toBe(2); - expect(testState.calls.some((call) => /^provider (create|update) .*--credential/.test(call))).toBe(false); + expect( + testState.calls.some((call) => /^provider (create|update) .*--credential/.test(call)), + ).toBe(false); expect(testState.policyApplyCalls).toBe(2); + expect(testState.removedPolicyKeys).not.toContain("mcp_bridge_github"); expect(testState.adapterCalls).toContain("command -v mcporter"); - expect(testState.adapterCalls.some((call) => call.includes("openshell:resolve:env:GITHUB_TOKEN"))).toBe(true); + expect( + testState.adapterCalls.some((call) => call.includes("openshell:resolve:env:GITHUB_TOKEN")), + ).toBe(true); expect(sandbox?.mcp?.bridges).toHaveProperty("github"); expect(sandbox?.mcp?.managedServerNames).toEqual(["github", "retired"]); expect(sandbox?.mcp?.destroyPreparedAt).toBeUndefined(); @@ -1261,13 +1257,14 @@ describe("authenticated MCP sandbox destroy lifecycle", () => { registry.addCustomPolicy("alpha", ownedPolicy("github")); const preparation = await bridge.prepareMcpBridgesForDestroy("alpha"); - testState.applyPresetContent.mockReturnValue(false); + testState.providers.delete("alpha-mcp-github"); const error = await captureMessage(() => bridge.restoreMcpBridgesAfterDestroyAbort("alpha", preparation), ); const sandbox = registry.getSandbox("alpha"); - expect(error).toMatch(/failed to activate generated MCP policy/i); + expect(error).toMatch(/is missing/i); + expect(testState.policyApplyCalls).toBe(0); expect(sandbox?.mcp?.bridges).toHaveProperty("github"); expect(sandbox?.mcp?.managedServerNames).toEqual(["github", "retired"]); expect(sandbox?.mcp?.destroyPreparedAt).toBeTruthy(); @@ -1275,18 +1272,14 @@ describe("authenticated MCP sandbox destroy lifecycle", () => { expect(testState.adapterRegistered).toBe(false); }); - it("preserves credentials and bridge state until sandbox deletion is confirmed", async () => { - registry.registerSandbox({ - name: "alpha", - agent: "openclaw", - mcp: { bridges: { github: bridgeEntries.github } }, - }); - registry.addCustomPolicy("alpha", ownedPolicy("github")); + it("revalidates retained authority after real destroy finalization clears the MCP manifest (#9833)", async () => { + registerAlphaGithubBridge(); registry.addCustomPolicy("alpha", { name: "operator", content: "version: 1\n" }); const preparation = await bridge.prepareMcpBridgesForDestroy("alpha"); const afterPrepare = registry.getSandbox("alpha"); await bridge.finalizeMcpBridgesAfterSandboxDelete("alpha", preparation); + await preparation.revalidateBeforeSuccess?.(); const afterFinalize = registry.getSandbox("alpha"); expect(afterPrepare?.mcp?.bridges).toHaveProperty("github"); @@ -1319,7 +1312,9 @@ describe("authenticated MCP sandbox destroy lifecycle", () => { expect(process.env.GITHUB_TOKEN).toBe("ambient-value-that-must-not-rotate"); expect(testState.providers.get("alpha-mcp-github")?.resourceVersion).toBe(2); - expect(testState.calls.some((call) => /^provider (create|update) .*--credential/.test(call))).toBe(false); + expect( + testState.calls.some((call) => /^provider (create|update) .*--credential/.test(call)), + ).toBe(false); expect([...testState.attachedProviders]).toContain("alpha-mcp-github"); expect(testState.adapterRegistered).toBe(true); expect(testState.policyApplyCalls).toBe(2); diff --git a/test/mcp/mcp-policy-key-ownership.test.ts b/test/mcp/mcp-policy-key-ownership.test.ts index 0e714e1ad94..b509a94fc12 100644 --- a/test/mcp/mcp-policy-key-ownership.test.ts +++ b/test/mcp/mcp-policy-key-ownership.test.ts @@ -17,7 +17,11 @@ import { const MATCHING_OPENSHELL = path.resolve("test/fixtures/openshell-v0.0.106"); const MATCHING_OPENSHELL_VERSION_CLAUSE = `if [ "$1" = "--version" ]; then printf '%s\\n' 'openshell 0.0.106'; exit 0; fi`; -const MANAGED_POLICY_AUTHORITY_CLAUSE = `if [ "$1 $2" = "sandbox get" ]; then +const MANAGED_POLICY_AUTHORITY_CLAUSE = `if [ "$1 $2" = "gateway info" ]; then + printf 'Gateway endpoint: http://127.0.0.1:8080\n' + exit 0 +fi +if [ "$1 $2" = "sandbox get" ]; then printf 'Name: alpha\nId: ${SANDBOX_ID}\nPhase: Ready\n' exit 0 fi @@ -36,6 +40,16 @@ const PRESET = `network_policies: endpoints: [] `; +function managedSandboxPolicyMetadata(networkPolicies: Record): string { + return JSON.stringify({ + scope: "sandbox", + sandbox: "alpha", + status: "effective", + policy_source: "sandbox", + policy: { version: 1, network_policies: networkPolicies }, + }); +} + function runApply( expectedExistingNetworkPolicyContent: string | null, liveName: string | null = "operator-owned", @@ -43,6 +57,8 @@ function runApply( const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-mcp-policy-owner-")); const binDir = path.join(home, ".local", "bin"); const callsPath = path.join(home, "calls.log"); + const liveNetworkPolicies = + liveName === null ? {} : { example: { name: liveName, endpoints: [] } }; const appliedPolicyPath = path.join(home, "applied-policy.yaml"); fs.mkdirSync(binDir, { recursive: true }); fs.writeFileSync( @@ -51,6 +67,10 @@ function runApply( ${MATCHING_OPENSHELL_VERSION_CLAUSE} ${MANAGED_POLICY_AUTHORITY_CLAUSE} printf '%s\n' "$*" >> ${JSON.stringify(callsPath)} +if [ "$3" = "--full" ] || [ "$5" = "--full" ]; then + printf '%s\n' '${managedSandboxPolicyMetadata(liveNetworkPolicies)}' + exit 0 +fi if [ "$1 $2" = "policy get" ]; then if [ -f ${JSON.stringify(appliedPolicyPath)} ]; then cat ${JSON.stringify(appliedPolicyPath)} @@ -145,6 +165,12 @@ function runFailedPolicyMutation(operation: "apply" | "remove") { `#!/bin/sh ${MATCHING_OPENSHELL_VERSION_CLAUSE} ${MANAGED_POLICY_AUTHORITY_CLAUSE} +if [ "$3" = "--full" ] || [ "$5" = "--full" ]; then + printf '%s\n' '${managedSandboxPolicyMetadata({ + example: { name: "generated-policy", endpoints: [] }, + })}' + exit 0 +fi if [ "$1 $2" = "policy get" ]; then printf 'Version: 1\nHash: test\n---\nversion: 1\nnetwork_policies:\n example:\n name: generated-policy\n endpoints: []\n' exit 0 @@ -212,6 +238,12 @@ function runSuccessfulPolicyRemoval(skipRegistryUpdate: boolean) { `#!/bin/sh ${MATCHING_OPENSHELL_VERSION_CLAUSE} ${MANAGED_POLICY_AUTHORITY_CLAUSE} +if [ "$3" = "--full" ] || [ "$5" = "--full" ]; then + printf '%s\n' '${managedSandboxPolicyMetadata({ + example: { name: "generated-policy", endpoints: [] }, + })}' + exit 0 +fi if [ "$1 $2" = "policy get" ]; then if [ -f ${JSON.stringify(appliedPolicyPath)} ]; then cat ${JSON.stringify(appliedPolicyPath)} @@ -510,6 +542,10 @@ processRecovery.executeSandboxExecCommand = () => ({ }); ${managedRegistrationSource("alpha")} registry.addCustomPolicy = () => { throw new Error("injected registry write failure"); }; +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; const bridge = require("./src/lib/actions/sandbox/mcp-bridge.js"); bridge.addMcpBridge("alpha", { server: "reservation", @@ -595,6 +631,10 @@ processRecovery.executeSandboxCommand = () => ({ stderr: "", }); +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; const bridge = require("./src/lib/actions/sandbox/mcp-bridge.js"); const entry = { server: "example", diff --git a/test/mcp/mcp-policy-transition.test.ts b/test/mcp/mcp-policy-transition.test.ts index 71292f15e0e..c1875fb5584 100644 --- a/test/mcp/mcp-policy-transition.test.ts +++ b/test/mcp/mcp-policy-transition.test.ts @@ -18,6 +18,8 @@ const mode = ${JSON.stringify(mode)}; const registry = require("./src/lib/state/registry.js"); const policies = require("./src/lib/policy/index.js"); const generated = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; const entry = { server: "example", @@ -126,6 +128,8 @@ process.env.HOME = ${JSON.stringify(home)}; const registry = require("./src/lib/state/registry.js"); const policies = require("./src/lib/policy/index.js"); const generated = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; const entry = { server: "example", agent: "openclaw", @@ -177,6 +181,8 @@ process.env.HOME = ${JSON.stringify(home)}; const registry = require("./src/lib/state/registry.js"); const policies = require("./src/lib/policy/index.js"); const generated = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; const entry = { server: "example", agent: "openclaw", @@ -231,26 +237,26 @@ process.stdout.write(JSON.stringify({ } describe("generated MCP policy transitions", () => { - it.each([ - "assert", - "apply", - ] as const)("preserves an unowned same-name registry record during %s", (operation) => { - const result = runUnownedRegistryCollision(operation); - expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); - const payload = JSON.parse(result.stdout) as { - message: string; - applyCalled: boolean; - policies: Array<{ content: string; sourcePath: string }>; - }; - expect(payload.message).toMatch(/unowned same-name registry record/); - expect(payload.applyCalled).toBe(false); - expect(payload.policies).toEqual([ - expect.objectContaining({ - content: "operator-owned-content", - sourcePath: "/operator/policy.yaml", - }), - ]); - }); + it.each(["assert", "apply"] as const)( + "preserves an unowned same-name registry record during %s", + (operation) => { + const result = runUnownedRegistryCollision(operation); + expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); + const payload = JSON.parse(result.stdout) as { + message: string; + applyCalled: boolean; + policies: Array<{ content: string; sourcePath: string }>; + }; + expect(payload.message).toMatch(/unowned same-name registry record/); + expect(payload.applyCalled).toBe(false); + expect(payload.policies).toEqual([ + expect.objectContaining({ + content: "operator-owned-content", + sourcePath: "/operator/policy.yaml", + }), + ]); + }, + ); it("preserves the confirmed and desired policy across an interrupted refresh", () => { const result = runPolicyTransition("crash-retry"); @@ -335,18 +341,21 @@ describe("generated MCP policy transitions", () => { it.each([ ["absent", false], ["match", true], - ] as const)("requires exact post-removal state %s before dropping ownership", (postRemovalState, preservesOwnership) => { - const result = runGeneratedPolicyRemoval(postRemovalState); - expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); - const payload = JSON.parse(result.stdout) as { - message: string; - skipRegistryUpdate: boolean; - policies: Array<{ content: string; sourcePath: string }>; - }; - expect(payload.skipRegistryUpdate).toBe(true); - expect(payload.message).toMatch(preservesOwnership ? /effective state: match/ : /^$/); - expect(payload.policies.map((policy) => policy.sourcePath)).toEqual( - preservesOwnership ? ["generated:nemoclaw-mcp-bridge"] : [], - ); - }); + ] as const)( + "requires exact post-removal state %s before dropping ownership", + (postRemovalState, preservesOwnership) => { + const result = runGeneratedPolicyRemoval(postRemovalState); + expect(result.status, `${result.stdout}\n${result.stderr}`).toBe(0); + const payload = JSON.parse(result.stdout) as { + message: string; + skipRegistryUpdate: boolean; + policies: Array<{ content: string; sourcePath: string }>; + }; + expect(payload.skipRegistryUpdate).toBe(true); + expect(payload.message).toMatch(preservesOwnership ? /effective state: match/ : /^$/); + expect(payload.policies.map((policy) => policy.sourcePath)).toEqual( + preservesOwnership ? ["generated:nemoclaw-mcp-bridge"] : [], + ); + }, + ); }); diff --git a/test/mcp/mcp-provider-detach-retry.test.ts b/test/mcp/mcp-provider-detach-retry.test.ts index c96df17b054..10c9f27c9a0 100644 --- a/test/mcp/mcp-provider-detach-retry.test.ts +++ b/test/mcp/mcp-provider-detach-retry.test.ts @@ -5,7 +5,7 @@ import { spawnSync } from "node:child_process"; import { describe, expect, it } from "vitest"; -type DetachScenario = "success" | "drift" | "exhausted" | "other-error"; +type DetachScenario = "success" | "drift" | "exhausted" | "other-error" | "authority-refusal"; function runDetachScenario(scenario: DetachScenario) { const script = String.raw` @@ -61,12 +61,20 @@ const entry = { }; let outcome = null; let message = null; -try { - outcome = providerActions.detachProvider("alpha", entry); -} catch (error) { - message = error.message; -} -process.stdout.write(JSON.stringify({ outcome, message, detachCalls, attached, liveId })); +(async () => { + try { + outcome = await providerActions.detachProvider("alpha", entry, { + prepareMutation: async () => { + if (scenario === "authority-refusal" && detachCalls > 0) { + throw new Error("policy authority changed before detach retry"); + } + }, + }); + } catch (error) { + message = error.message; + } + process.stdout.write(JSON.stringify({ outcome, message, detachCalls, attached, liveId })); +})().catch((error) => { console.error(error); process.exit(1); }); `; const result = spawnSync(process.execPath, ["-e", script], { cwd: process.cwd(), @@ -115,4 +123,12 @@ describe("MCP provider detach retry", () => { expect(result.detachCalls).toBe(1); expect(result.attached).toBe(true); }); + + it("revalidates policy authority before a detach retry (#9833)", () => { + const result = runDetachScenario("authority-refusal"); + expect(result.outcome).toBeNull(); + expect(result.message).toContain("policy authority changed before detach retry"); + expect(result.detachCalls).toBe(1); + expect(result.attached).toBe(true); + }); }); diff --git a/test/mcp/mcp-provider-ownership.test.ts b/test/mcp/mcp-provider-ownership.test.ts index 5c206ef2d31..ca05f58701b 100644 --- a/test/mcp/mcp-provider-ownership.test.ts +++ b/test/mcp/mcp-provider-ownership.test.ts @@ -19,6 +19,10 @@ const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); const providerCommands = require("./src/lib/adapters/openshell/provider-command.js"); +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; const expectedId = "11111111-2222-4333-8444-555555555555"; const foreignId = "99999999-8888-4777-8666-555555555555"; let liveId = expectedId; @@ -122,6 +126,10 @@ const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); const providerCommands = require("./src/lib/adapters/openshell/provider-command.js"); +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; const expectedId = "11111111-2222-4333-8444-555555555555"; let providerExists = true; let attached = true; @@ -553,6 +561,10 @@ const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); const providerCommands = require("./src/lib/adapters/openshell/provider-command.js"); +const mcpPolicy = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +mcpPolicy.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; const calls = []; agentDefs.loadAgent = () => { throw new Error("persisted adapter must be used"); }; gatewayRuntime.recoverNamedGatewayRuntime = async () => ({ @@ -617,7 +629,7 @@ bridge.removeMcpBridge("alpha", "fake", { force: true }).then( calls: string[]; bridgePresent: boolean; }; - expect(payload.message).toContain("registry entry was preserved"); + expect(payload.message).toContain("bridge manifest was preserved"); expect(result.stderr).toContain("Expected stable provider ID"); expect(payload.calls.some((call) => call === "provider get alpha-mcp-fake")).toBe(true); expect(payload.bridgePresent).toBe(true); diff --git a/test/mcp/mcp-restart-policy-order.test.ts b/test/mcp/mcp-restart-policy-order.test.ts index 4301e5ad4fe..7f3b6258bee 100644 --- a/test/mcp/mcp-restart-policy-order.test.ts +++ b/test/mcp/mcp-restart-policy-order.test.ts @@ -26,6 +26,9 @@ const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); const generated = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; +generated.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; const providerCalls = []; let policyApplyCalls = 0; @@ -184,6 +187,9 @@ const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); const policies = require("./src/lib/policy/index.js"); const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); const generated = require("./src/lib/actions/sandbox/mcp-bridge-policy.js"); +const policyAuthority = require("./src/lib/actions/sandbox/policy-authority/preflight.js"); +policyAuthority.preflightSandboxPolicyAuthority = () => "nemoclaw-managed"; +generated.preflightMcpPolicyAuthority = () => "nemoclaw-managed"; let resourceVersion = 1; let registeredProviderGets = 0;