diff --git a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts index 7e7fe356def..3fa21427fdc 100644 --- a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts +++ b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts @@ -44,6 +44,7 @@ import { observeMcpCredentialRevision, providerMatchesCredential, providerShapeDetail, + refreshMcpProviderEnvironment, upsertMcpProvider, waitForAttachedMcpCredential, waitForDetachedMcpCredential, @@ -434,6 +435,7 @@ async function addMcpBridgeUnlocked( providerAttachAttempted = true; attachProvider(sandboxName, entry); applyGeneratedPolicy(sandboxName, entry, target); + refreshMcpProviderEnvironment(entry); waitForAttachedMcpCredential(sandboxName, entry, { ...(providerResult.action === "updated" ? { 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 fae2b8884f1..c709c629d56 100644 --- a/src/lib/actions/sandbox/mcp-bridge-input-targets.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-input-targets.test.ts @@ -195,6 +195,7 @@ replace(provider, "upsertMcpProvider", () => ({ }, })); replace(provider, "attachProvider", () => {}); +replace(provider, "refreshMcpProviderEnvironment", () => {}); replace(provider, "waitForAttachedMcpCredential", () => {}); registry.registerSandbox({ name: "alpha", agent: "openclaw" }); require("./src/lib/actions/sandbox/mcp-bridge.js").addMcpBridge("alpha", { diff --git a/src/lib/actions/sandbox/mcp-bridge-provider-mutation.ts b/src/lib/actions/sandbox/mcp-bridge-provider-mutation.ts index 6b12ea62582..acf9893a53c 100644 --- a/src/lib/actions/sandbox/mcp-bridge-provider-mutation.ts +++ b/src/lib/actions/sandbox/mcp-bridge-provider-mutation.ts @@ -249,6 +249,49 @@ export function upsertMcpProvider( return { action: action === "create" ? "created" : "updated", inspection: after }; } +/** + * Republish an attached provider after its endpointless credential binding is + * active. OpenShell's Docker sidecar can observe the provider mutation before + * the bound policy generation; a no-field update advances the provider + * revision without reading or rotating the stored credential, giving the + * sidecar a post-policy generation to synchronize. + */ +export function refreshMcpProviderEnvironment(entry: McpBridgeEntry): McpProviderInspection { + assertPersistedAuthenticatedBridgeEntry(entry); + if (!entry.providerName || !entry.providerId) { + throw new McpBridgeError( + `MCP server '${entry.server}' has no stable OpenShell provider identity for credential synchronization.`, + ); + } + const before = inspectMcpProvider(entry.providerName); + if (!providerMatchesCredential(before, entry.env[0], entry.providerId)) { + throw new McpBridgeError( + `OpenShell provider '${entry.providerName}' changed before credential synchronization. ${providerShapeDetail(before, entry.env[0], entry.providerId)} Refusing to mutate it.`, + ); + } + const result = runOpenshellProviderCommand(["provider", "update", entry.providerName], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }) as OpenShellCommandResult; + if (result.status !== 0) { + throw new McpBridgeError( + commandOutput(result) || + `Failed to synchronize MCP provider '${entry.providerName}' after policy binding.`, + ); + } + const after = inspectMcpProvider(entry.providerName); + if ( + !providerMatchesCredential(after, entry.env[0], entry.providerId) || + !after.resourceVersion || + after.resourceVersion <= (before.resourceVersion ?? 0) + ) { + throw new McpBridgeError( + `OpenShell provider '${entry.providerName}' changed during credential synchronization. ${providerShapeDetail(after, entry.env[0], entry.providerId)} Refusing later MCP side effects.`, + ); + } + return after; +} + function inspectMcpProviderForDeletion( entry: McpBridgeEntry, options: { allowLegacyGeneric?: boolean; allowMissing?: boolean; bestEffort?: boolean } = {}, diff --git a/src/lib/actions/sandbox/mcp-bridge-provider.test.ts b/src/lib/actions/sandbox/mcp-bridge-provider.test.ts index e883d97456d..d2167180f98 100644 --- a/src/lib/actions/sandbox/mcp-bridge-provider.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-provider.test.ts @@ -22,6 +22,7 @@ import { import { assertMcpProviderRecoverable, observeMcpCredentialRevision, + refreshMcpProviderEnvironment, waitForAttachedMcpCredential, waitForDetachedMcpCredential, } from "./mcp-bridge-provider"; @@ -139,6 +140,50 @@ Provider: ); }); + it("republishes an exact provider only after policy binding without reading its credential", () => { + const id = "11111111-2222-4333-8444-555555555555"; + const providerResult = (resourceVersion: number) => ({ + pid: 1234, + status: 0, + signal: null, + output: [ + null, + `Id: ${id}\nType: nemoclaw-mcp-v1\nResource version: ${resourceVersion}\nCredential keys: GITHUB_TOKEN\n`, + "", + ], + stdout: `Id: ${id}\nType: nemoclaw-mcp-v1\nResource version: ${resourceVersion}\nCredential keys: GITHUB_TOKEN\n`, + stderr: "", + }); + const run = vi + .spyOn(providerCommand, "runOpenshellProviderCommand") + .mockReturnValueOnce(providerResult(7)) + .mockReturnValueOnce({ + pid: 1234, + status: 0, + signal: null, + output: [null, "", ""], + stdout: "", + stderr: "", + }) + .mockReturnValueOnce(providerResult(8)); + + expect( + refreshMcpProviderEnvironment({ + server: "github", + agent: "openclaw", + adapter: "mcporter", + url: "https://api.githubcopilot.com/mcp", + env: ["GITHUB_TOKEN"], + providerName: "alpha-mcp-github", + providerId: id, + policyName: "mcp-bridge-github", + addedAt: "2026-08-19T00:00:00.000Z", + }), + ).toMatchObject({ resourceVersion: 8 }); + expect(run.mock.calls[1]?.[0]).toEqual(["provider", "update", "alpha-mcp-github"]); + expect(run.mock.calls[1]?.[0]).not.toContain("--credential"); + }); + it("distinguishes a real detach from OpenShell's idempotent success", () => { expect( providerDetachChangedState(0, "✓ Detached provider alpha-mcp-github from sandbox alpha"), diff --git a/src/lib/actions/sandbox/mcp-bridge-provider.ts b/src/lib/actions/sandbox/mcp-bridge-provider.ts index 76bb508d3e4..42b991c6b10 100644 --- a/src/lib/actions/sandbox/mcp-bridge-provider.ts +++ b/src/lib/actions/sandbox/mcp-bridge-provider.ts @@ -30,6 +30,7 @@ export { detachMissingProviderReference, detachProvider, ensureMcpBridgeProviderProfile, + refreshMcpProviderEnvironment, providerDetachChangedState, upsertMcpProvider, } from "./mcp-bridge-provider-mutation"; diff --git a/src/lib/actions/sandbox/mcp-bridge-restart.ts b/src/lib/actions/sandbox/mcp-bridge-restart.ts index feb0ab79e0f..11d5d3d51e3 100644 --- a/src/lib/actions/sandbox/mcp-bridge-restart.ts +++ b/src/lib/actions/sandbox/mcp-bridge-restart.ts @@ -14,8 +14,9 @@ import { assertNoAttachedProviderCredentialCollisions, assertNoProviderCredentialCollisions, attachProvider, - ensureMcpBridgeProviderProfile, detachMissingProviderReference, + ensureMcpBridgeProviderProfile, + refreshMcpProviderEnvironment, type McpCredentialRevisionObservation, type McpProviderInspection, observeMcpCredentialRevision, @@ -171,6 +172,7 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P } attachProvider(sandboxName, entry); applyGeneratedPolicy(sandboxName, entry, target); + refreshMcpProviderEnvironment(entry); waitForAttachedMcpCredential(sandboxName, entry, { ...(providerResult.action === "updated" ? { previousRevision: previousCredentialRevision } @@ -239,6 +241,7 @@ export async function restoreExistingMcpBridgeRuntime( }); attachProvider(sandboxName, entry); applyGeneratedPolicy(sandboxName, entry, resolvedTargetPins(resolvedByServer, entry)); + refreshMcpProviderEnvironment(entry); waitForAttachedMcpCredential(sandboxName, entry); const adapter = (entry.adapter as AgentMcpAdapter | undefined) ?? defaultAdapter; registerAgentAdapter( diff --git a/test/deepagents-mcp-legacy-lifecycle.test.ts b/test/deepagents-mcp-legacy-lifecycle.test.ts index 38dd388a3d5..c7670e72d7a 100644 --- a/test/deepagents-mcp-legacy-lifecycle.test.ts +++ b/test/deepagents-mcp-legacy-lifecycle.test.ts @@ -57,6 +57,7 @@ type ProviderType = "generic" | "nemoclaw-mcp-v1"; let providerExists = true; let providerType: ProviderType = "generic"; +let providerResourceVersion = 1; let attached = true; let adapterRegistered = true; let adapterRemovalOutcome = ""; @@ -101,6 +102,7 @@ beforeEach(() => { providerExists = true; providerType = "generic"; + providerResourceVersion = 1; attached = true; adapterRegistered = true; adapterRemovalOutcome = ""; @@ -120,7 +122,7 @@ beforeEach(() => { return providerExists ? { status: 0, - stdout: `Id: ${providerId}\nType: ${providerType}\nResource version: 1\nCredential keys: GITHUB_TOKEN\n`, + stdout: `Id: ${providerId}\nType: ${providerType}\nResource version: ${providerResourceVersion}\nCredential keys: GITHUB_TOKEN\n`, stderr: "", } : { status: 1, stdout: "", stderr: "Provider not found" }; @@ -138,6 +140,12 @@ beforeEach(() => { case args[0] === "sandbox" && args[1] === "provider" && args[2] === "attach": attached = true; return { status: 0, stdout: "Attached provider", stderr: "" }; + case args[0] === "provider" && + args[1] === "update" && + args[2] === "alpha-mcp-github" && + args.length === 3: + providerResourceVersion += 1; + return { status: 0, stdout: "Updated provider", stderr: "" }; case args[0] === "provider" && args[1] === "delete": providerExists = false; attached = false; diff --git a/test/mcp-add-crash-consistency.test.ts b/test/mcp-add-crash-consistency.test.ts index 04a58734603..3a09a88bbbf 100644 --- a/test/mcp-add-crash-consistency.test.ts +++ b/test/mcp-add-crash-consistency.test.ts @@ -42,6 +42,8 @@ const crashAfter = ${JSON.stringify(crashAfter)}; const marker = (name) => path.join(process.env.HOME, name + ".marker"); const mark = (name) => fs.writeFileSync(marker(name), "yes\n", { mode: 0o600 }); const marked = (name) => fs.existsSync(marker(name)); +const providerVersion = () => Number.parseInt(marked("provider-version") ? fs.readFileSync(marker("provider-version"), "utf8") : "1", 10); +const setProviderVersion = (version) => fs.writeFileSync(marker("provider-version"), String(version), { mode: 0o600 }); const providerPresentAtStart = marked("provider"); const providerId = "11111111-2222-4333-8444-555555555555"; const foreignProviderId = "99999999-8888-4777-8666-555555555555"; @@ -84,17 +86,28 @@ 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: " + (marked("updated") ? "2" : "1") + "\nCredential keys: FAKE_MCP_SECRET\n", stderr: "" } + ? { status: 0, stdout: "Id: " + (marked("foreign-provider") ? foreignProviderId : 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 (!marked("policy")) { return { status: 1, stdout: "", stderr: "provider mutation preceded policy attestation" }; } - if (args[1] === "create") observedProviderName = args[args.indexOf("--name") + 1]; - if (args[1] === "update") observedProviderName = args[2]; + if (args[1] === "create") observedProviderName = args[args.indexOf("--name") + 1], setProviderVersion(1); + if (args[1] === "update") { + const isCredentialUpdate = + args.length === 5 && + args[2] === observedProviderName && + args[3] === "--credential" && + args[4] === "FAKE_MCP_SECRET"; + const isCredentialFreeRefresh = args.length === 3 && args[2] === observedProviderName; + if (!isCredentialUpdate && !isCredentialFreeRefresh) { + throw new Error("Unexpected provider update: " + args.join(" ")); + } + setProviderVersion(providerVersion() + 1); + if (isCredentialUpdate) mark("updated"); + } mark("provider"); - if (args[1] === "update") mark("updated"); if (crashAfter === "registered-late-collision") registry.addExtraProvider("foreign-registered"); if (crashAfter === "provider") process.exit(86); return { status: 0, stdout: args[1] === "create" ? "Created provider" : "Updated provider", stderr: "" }; @@ -168,7 +181,7 @@ processRecovery.executeSandboxExecCommand = (_sandbox, command) => { isPreupdateObservation && mark("observation"); return { status: crashAfter === "preupdate-observation-forbidden" && isPreupdateObservation ? 1 : 0, - stdout: isObservation ? (marked("updated") ? "v2" : marked("provider") ? "v1" : "absent") : "", + stdout: isObservation ? (marked("updated") ? "v" + providerVersion() : marked("provider") ? "v1" : "absent") : "", stderr: "", }; }; diff --git a/test/mcp-destroy-lifecycle.test.ts b/test/mcp-destroy-lifecycle.test.ts index a10ebdb2c23..68930e91ca7 100644 --- a/test/mcp-destroy-lifecycle.test.ts +++ b/test/mcp-destroy-lifecycle.test.ts @@ -184,7 +184,6 @@ async function captureMessage(action: () => Promise): Promise { return error instanceof Error ? error.message : String(error); } } - function registerAlphaGithubBridge(): void { registry.registerSandbox({ name: "alpha", @@ -303,6 +302,9 @@ 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; + return { status: 0, stdout: "Updated provider", stderr: "" }; case args[0] === "provider" && args[1] === "delete" && testState.failProviderDelete === args[2]: @@ -1236,15 +1238,12 @@ describe("authenticated MCP sandbox destroy lifecycle", () => { expect(process.env.GITHUB_TOKEN).toBe("ambient-value-that-must-not-rotate"); expect([...testState.providers.keys()]).toContain("alpha-mcp-github"); - expect( - testState.calls.some((call) => call === "sandbox provider attach alpha alpha-mcp-github"), - ).toBe(true); - expect(testState.calls.some((call) => /^provider (create|update) /.test(call))).toBe(false); + 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.policyApplyCalls).toBe(2); 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(); @@ -1320,7 +1319,8 @@ describe("authenticated MCP sandbox destroy lifecycle", () => { await bridge.restoreMcpBridgesAfterRebuild("alpha", [bridgeEntries.github]); expect(process.env.GITHUB_TOKEN).toBe("ambient-value-that-must-not-rotate"); - expect(testState.calls.some((call) => /^provider (create|update) /.test(call))).toBe(false); + 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.attachedProviders]).toContain("alpha-mcp-github"); expect(testState.adapterRegistered).toBe(true); expect(testState.policyApplyCalls).toBe(2); diff --git a/test/mcp-restart-policy-order.test.ts b/test/mcp-restart-policy-order.test.ts index 5001b71cac4..7f4360e6f3a 100644 --- a/test/mcp-restart-policy-order.test.ts +++ b/test/mcp-restart-policy-order.test.ts @@ -226,7 +226,7 @@ providerCommands.runOpenshellProviderCommand = (args) => { } if (args[0] === "provider" && args[1] === "update") { providerCalls.push(command); - resourceVersion = 2; + resourceVersion += 1; return { status: 0, stdout: "Updated provider", stderr: "" }; } if (args[0] === "sandbox" && args[1] === "provider" && args[2] === "list") { @@ -296,9 +296,10 @@ bridge.restartMcpBridge("alpha", "example").then( providerCalls: string[]; registeredProviderGets: number; }; - expect(payload.observations).toEqual(["v1", "v2"]); + expect(payload.observations).toEqual(["v1", "v3"]); expect(payload.providerCalls).toEqual([ "provider update alpha-mcp-example --credential MCP_TOKEN", + "provider update alpha-mcp-example", ]); expect(payload.registeredProviderGets).toBe(1); expect(payload.proofScripts).toHaveLength(2);