Skip to content
Merged
13 changes: 13 additions & 0 deletions docs/security/credential-storage.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,19 @@ Use this precedence to:

Managed MCP is an exception: `$$nemoclaw <name> mcp add` always creates and attaches an OpenShell provider, and `--env KEY` supplies only the transient input value.
For that credential boundary, refer to [About Managed MCP Servers](../manage-sandboxes/mcp-servers/about-managed-mcp-servers).
Before an ordinary live-sandbox rebuild or forced host-side recovery changes managed MCP state, NemoClaw compares the credential keys for every provider attached to the sandbox.
If another provider supplies a credential key that a managed MCP server reserves, rebuild stops before it changes the managed provider attachment, generated policy, or agent adapter.
The collision check does not delete either provider or its stored credential value.
Forced host-side recovery repeats the check before sandbox deletion.
Detach only the conflicting provider from the affected sandbox:

```bash
openshell sandbox provider detach <sandbox-name> <provider-name>
```

This command keeps the provider and stored credential in OpenShell and does not change its attachments to other sandboxes.
Rerun the original rebuild command.
Do not run `$$nemoclaw credentials reset <PROVIDER_NAME>` unless you intend to detach that provider from every sandbox and delete its stored credential from OpenShell.

When the host environment is empty, day-two operations such as `$$nemoclaw <name> rebuild` and remote-provider updates can reuse the credential already registered with the OpenShell gateway.
Export the credential only when you want to create, replace, or rotate the stored provider value.
Expand Down
6 changes: 3 additions & 3 deletions src/lib/actions/sandbox/mcp-bridge-add-restart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
} from "./mcp-bridge-policy";
import {
assertMcpProviderRecoverable,
assertNoAttachedProviderCredentialCollision,
assertNoAttachedProviderCredentialCollisions,
attachProvider,
deleteProvider,
detachMissingProviderReference,
Expand Down Expand Up @@ -372,7 +372,7 @@ async function addMcpBridgeUnlocked(
// Credential keys are sandbox-global. Prove this key is not already
// supplied by a foreign attachment before opening its MCP route, then check
// again after provider creation to close the intervening race.
assertNoAttachedProviderCredentialCollision(sandboxName, entry);
assertNoAttachedProviderCredentialCollisions(sandboxName, [entry]);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Loading the real protocol:mcp policy with --wait is the authoritative
// running-supervisor capability check. Do it before any host credential is
// created or updated so unsupported runtimes fail without that side effect.
Expand Down Expand Up @@ -407,7 +407,7 @@ async function addMcpBridgeUnlocked(
// adapter mutations. A process death before this write fails closed.
writeBridgeEntry(sandboxName, entry);
}
assertNoAttachedProviderCredentialCollision(sandboxName, entry);
assertNoAttachedProviderCredentialCollisions(sandboxName, [entry]);
if (providerResult.action === "updated" && previousCredentialRevision === undefined) {
throw new McpBridgeError(
`Could not retain the prior OpenShell credential revision for provider '${entry.providerName}'.`,
Expand Down
2 changes: 1 addition & 1 deletion src/lib/actions/sandbox/mcp-bridge-input-targets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ replace(adapters, "registerAgentAdapter", () => {});
replace(policy, "applyGeneratedPolicy", (_sandbox, _entry, target) => { admittedTarget = target; });
replace(state, "ensureSandboxGatewaySelected", async () => {});
replace(validation, "assertMcpCredentialBoundaryRuntimeVersion", () => {});
replace(provider, "assertNoAttachedProviderCredentialCollision", () => {});
replace(provider, "assertNoAttachedProviderCredentialCollisions", () => {});
replace(provider, "inspectMcpProvider", () => ({
credentialKeys: null, exists: false, id: null, resourceVersion: null, type: null,
}));
Expand Down
25 changes: 14 additions & 11 deletions src/lib/actions/sandbox/mcp-bridge-provider-inspection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,26 +164,29 @@ export function inspectMcpProviderAttachments(
}
}

export function assertNoAttachedProviderCredentialCollision(
export function assertNoAttachedProviderCredentialCollisions(
sandboxName: string,
entry: McpBridgeEntry,
entries: readonly McpBridgeEntry[],
): void {
if (entries.length === 0) return;
const inspection = inspectMcpProviderAttachments(sandboxName);
if (!inspection.attachments) {
throw new McpBridgeError(
inspection.error ?? `Could not inspect providers attached to sandbox '${sandboxName}'.`,
);
}
const credentialKey = entry.env[0];
const collision = inspection.attachments.find(
(attachment) =>
attachment.credentialKeys.includes(credentialKey) &&
!(attachment.name === entry.providerName && attachment.providerId === entry.providerId),
);
if (collision) {
throw new McpBridgeError(
`Credential key '${credentialKey}' is already supplied by attached provider '${collision.name}' with ID '${collision.providerId ?? "missing"}'. Refusing to reserve the key for MCP before provider activation.`,
for (const entry of entries) {
const credentialKey = entry.env[0];
const collision = inspection.attachments.find(
(attachment) =>
attachment.credentialKeys.includes(credentialKey) &&
!(attachment.name === entry.providerName && attachment.providerId === entry.providerId),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
);
if (collision) {
throw new McpBridgeError(
`Credential key '${credentialKey}' is already supplied by attached provider '${collision.name}' with ID '${collision.providerId ?? "missing"}'. Refusing to continue managed MCP while this sandbox receives that key from another provider.`,
);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/lib/actions/sandbox/mcp-bridge-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export type {
} from "./mcp-bridge-provider-inspection";
export {
assertMcpProviderRecoverable,
assertNoAttachedProviderCredentialCollision,
assertNoAttachedProviderCredentialCollisions,
inspectMcpProvider,
inspectMcpProviderAttachments,
parseMcpProviderAttachmentNames,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import {
inspectExactMcpDestroyProvider,
} from "./mcp-bridge-destroy-preflight";
import { assertGeneratedPolicyExactReadOnly } from "./mcp-bridge-policy";
import { preflightMcpEntryTargets } from "./mcp-bridge-provider";
import {
assertNoAttachedProviderCredentialCollisions,
preflightMcpEntryTargets,
} from "./mcp-bridge-provider";
import {
assertMcpDestroyNotPending,
bridgeState,
Expand Down Expand Up @@ -162,6 +165,7 @@ async function inspectReadOnlyRecoveryState(
providerByServer.set(entry.server, providerFingerprint(provider));
targetsByServer.set(entry.server, targetFingerprint(target));
}
assertNoAttachedProviderCredentialCollisions(sandboxName, entries);
return { policyByServer, providerByServer, targetsByServer };
}

Expand Down
2 changes: 2 additions & 0 deletions src/lib/actions/sandbox/mcp-bridge-rebuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
} from "./mcp-bridge-policy";
import {
assertMcpProviderRecoverable,
assertNoAttachedProviderCredentialCollisions,
attachProvider,
detachProvider,
preflightMcpEntryTargets,
Expand Down Expand Up @@ -129,6 +130,7 @@ export async function prepareMcpBridgesForRebuild(
for (const entry of entries) assertGeneratedPolicyMutationSafe(sandboxName, entry);
assertMcpAdapterTeardownRuntimeCapabilities(sandboxName, sandbox, entries);
for (const entry of entries) assertMcpProviderRecoverable(entry);
assertNoAttachedProviderCredentialCollisions(sandboxName, entries);
const detached: McpBridgeEntry[] = [];
const scrubbedAdapters: McpBridgeEntry[] = [];
try {
Expand Down
18 changes: 14 additions & 4 deletions src/lib/actions/sandbox/mcp-bridge-restart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { assertHermesMcpRuntimeIntent } from "./mcp-bridge-hermes-reconciliation
import { applyGeneratedPolicy, assertGeneratedPolicyMutationSafe } from "./mcp-bridge-policy";
import {
assertMcpProviderRecoverable,
assertNoAttachedProviderCredentialCollision,
assertNoAttachedProviderCredentialCollisions,
attachProvider,
detachMissingProviderReference,
type McpCredentialRevisionObservation,
Expand Down Expand Up @@ -117,6 +117,10 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P
for (const entry of missingProviderEntries) {
waitForDetachedMcpCredential(sandboxName, entry);
}
// Reject a collision on any target before the first policy/provider/adapter
// mutation. The per-entry checks below still close races at each mutation
// edge without allowing a later target to fail after an earlier update.
assertNoAttachedProviderCredentialCollisions(sandboxName, targetEntries);
for (const [name, storedEntry] of targets) {
// Validated as a complete authenticated entry before gateway side effects.
if (!storedEntry) continue;
Expand All @@ -125,7 +129,7 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P
const adapterEnvValues = resolveCredentialEnv(envRefs);
const target = resolvedTargetPins(resolvedByServer, entry);
let previousCredentialRevision: McpCredentialRevisionObservation | undefined;
assertNoAttachedProviderCredentialCollision(sandboxName, entry);
assertNoAttachedProviderCredentialCollisions(sandboxName, [entry]);
Comment thread
senthilr-nv marked this conversation as resolved.
// Revalidate the actual running supervisor before rotating, recreating,
// attaching, or re-registering an authenticated provider.
applyGeneratedPolicy(sandboxName, entry, target);
Expand All @@ -152,7 +156,7 @@ async function restartMcpBridgeUnlocked(sandboxName: string, server?: string): P
writeBridgeEntry(sandboxName, refreshedEntry);
entry = refreshedEntry;
}
assertNoAttachedProviderCredentialCollision(sandboxName, entry);
assertNoAttachedProviderCredentialCollisions(sandboxName, [entry]);
if (providerResult.action === "updated" && previousCredentialRevision === undefined) {
throw new McpBridgeError(
`Could not retain the prior OpenShell credential revision for provider '${entry.providerName}'.`,
Expand Down Expand Up @@ -213,7 +217,13 @@ export async function restoreExistingMcpBridgeRuntime(
`OpenShell provider '${entry.providerName}' is missing. Runtime restoration refuses to create or rotate credentials; run explicit MCP restart after exporting '${entry.env[0]}'.`,
);
}
assertNoAttachedProviderCredentialCollision(sandboxName, entry);
}
// Prove every restored entry is collision-free before the first mutation.
// The singleton check in the mutation loop still closes the race for each
// entry immediately before its policy and attachment are restored.
assertNoAttachedProviderCredentialCollisions(sandboxName, entries);
for (const entry of entries) {
assertNoAttachedProviderCredentialCollisions(sandboxName, [entry]);
applyGeneratedPolicy(sandboxName, entry, resolvedTargetPins(resolvedByServer, entry));
attachProvider(sandboxName, entry);
waitForAttachedMcpCredential(sandboxName, entry);
Expand Down
Loading
Loading