diff --git a/agents/hermes/mcp-config-transaction.py b/agents/hermes/mcp-config-transaction.py index a5b598e9c50..5b5f8da01e6 100755 --- a/agents/hermes/mcp-config-transaction.py +++ b/agents/hermes/mcp-config-transaction.py @@ -71,6 +71,10 @@ ENV_PLACEHOLDER_RE = re.compile( r"^Bearer openshell:resolve:env:([A-Za-z_][A-Za-z0-9_]{0,127})$" ) +REVISIONED_ENV_PLACEHOLDER_RE = re.compile( + r"^Bearer openshell:resolve:env:(v[0-9]{1,20})_([A-Za-z_][A-Za-z0-9_]{0,127})$" +) +OPENSHELL_CREDENTIAL_REVISION_RE = re.compile(r"^v[0-9]{1,20}$") OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE = re.compile(r"^v[0-9]+_[A-Za-z0-9_]+$") BOUNDARY_MANIFEST_NAME = "openshell-child-visible-credentials.v0.0.106.json" ANSI_ESCAPE_RE = re.compile( @@ -361,6 +365,8 @@ def _validate_payload(action: str, payload: dict[str, object]) -> None: raise ValueError("Unsupported MCP config action") allowed = {"server", "url", "headers"} allowed.add("replace_existing" if action == "add" else "force") + if action == "add": + allowed.update({"credential_name", "credential_revision"}) unexpected = sorted(set(payload) - allowed) if unexpected: raise ValueError( @@ -453,16 +459,38 @@ def _validate_payload(action: str, payload: dict[str, object]) -> None: if not isinstance(headers, dict) or set(headers) != {"Authorization"}: raise ValueError("MCP mutation payload must contain one Authorization header") authorization = headers.get("Authorization") - authorization_match = ( - ENV_PLACEHOLDER_RE.fullmatch(authorization) - if isinstance(authorization, str) - else None - ) + declared_credential_name = payload.get("credential_name") + credential_revision = payload.get("credential_revision") + if credential_revision is not None and ( + not isinstance(credential_revision, str) + or OPENSHELL_CREDENTIAL_REVISION_RE.fullmatch(credential_revision) is None + ): + raise ValueError("Hermes MCP credential revision is invalid") + authorization_match = None + credential_name = None + if isinstance(authorization, str) and credential_revision is not None: + authorization_match = REVISIONED_ENV_PLACEHOLDER_RE.fullmatch(authorization) + if ( + authorization_match is not None + and authorization_match.group(1) == credential_revision + and isinstance(declared_credential_name, str) + and authorization_match.group(2) == declared_credential_name + ): + credential_name = authorization_match.group(2) + else: + authorization_match = None + elif isinstance(authorization, str) and declared_credential_name is None: + authorization_match = ENV_PLACEHOLDER_RE.fullmatch(authorization) + if authorization_match is not None: + credential_name = authorization_match.group(1) if authorization_match is None: raise ValueError( "Hermes MCP Authorization must contain an OpenShell environment placeholder" ) - if action == "add" and _credential_name_is_reserved(authorization_match.group(1)): + if action == "add" and ( + not isinstance(credential_name, str) + or _credential_name_is_reserved(credential_name) + ): raise ValueError( "Hermes MCP Authorization uses a reserved credential environment name" ) @@ -484,6 +512,48 @@ def _managed_candidate(payload: dict[str, object]) -> dict[str, object]: return candidate +def _managed_candidate_matches( + actual: object, expected: dict[str, object], allow_revisioned: bool +) -> bool: + if actual == expected: + return True + if not allow_revisioned or not isinstance(actual, dict): + return False + if set(actual) != set(expected): + return False + for name, value in expected.items(): + if name != "headers" and actual.get(name) != value: + return False + actual_headers = actual.get("headers") + expected_headers = expected.get("headers") + if not isinstance(actual_headers, dict) or not isinstance(expected_headers, dict): + return False + if set(actual_headers) != {"Authorization"} or set(expected_headers) != { + "Authorization" + }: + return False + expected_authorization = expected_headers.get("Authorization") + actual_authorization = actual_headers.get("Authorization") + expected_match = ( + ENV_PLACEHOLDER_RE.fullmatch(expected_authorization) + if isinstance(expected_authorization, str) + else None + ) + if expected_match is None: + return False + expected_name = expected_match.group(1) + if OPENSHELL_REVISIONED_CREDENTIAL_NAME_RE.fullmatch(expected_name): + return False + if not isinstance(actual_authorization, str): + return False + prefix = "Bearer openshell:resolve:env:v" + suffix = f"_{expected_name}" + if not actual_authorization.startswith(prefix) or not actual_authorization.endswith(suffix): + return False + revision = actual_authorization[len(prefix) : -len(suffix)] + return revision.isdigit() and 1 <= len(revision) <= 20 + + _MANAGED_CANDIDATE_FIELDS = frozenset( {"url", "enabled", "timeout", "connect_timeout", "tools", "headers"} ) @@ -555,7 +625,10 @@ def inspect_managed_config(payload: dict[str, object]) -> dict[str, object]: absent = payload["absent"] if not isinstance(present, dict) or not isinstance(absent, list): raise RuntimeError("Hermes MCP config does not match persisted managed intent") - matches = all(servers.get(name) == expected for name, expected in present.items()) + matches = all( + _managed_candidate_matches(servers.get(name), expected, True) + for name, expected in present.items() + ) matches = matches and all(name not in servers for name in absent) if not matches: raise RuntimeError("Hermes MCP config does not match persisted managed intent") @@ -595,7 +668,7 @@ def _mutate(data: object, action: str, payload: dict[str, object]) -> tuple[dict return data, False if payload.get("force") is not True: current = servers.get(server_name) - if current != _managed_candidate(payload): + if not _managed_candidate_matches(current, _managed_candidate(payload), True): raise ValueError( f"Refusing to remove modified Hermes MCP server '{server_name}'. Use --force to remove it." ) diff --git a/agents/langchain-deepagents-code/validate-read-only-mcp-call.py b/agents/langchain-deepagents-code/validate-read-only-mcp-call.py index bd5888d2b00..6fd76b89393 100644 --- a/agents/langchain-deepagents-code/validate-read-only-mcp-call.py +++ b/agents/langchain-deepagents-code/validate-read-only-mcp-call.py @@ -273,7 +273,7 @@ def _write_config(url: str, *, ambiguous: bool = False) -> None: "type": "http", "url": url, "headers": { - "Authorization": "Bearer openshell:resolve:env:VALIDATION_MCP_TOKEN" + "Authorization": "Bearer openshell:resolve:env:v12_VALIDATION_MCP_TOKEN" }, } if ambiguous: diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-inspection.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-inspection.ts index 42f607827a7..bbe998a330d 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-inspection.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-inspection.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import type { McpBridgeEntry } from "../../state/registry"; +import type { McpAttachedCredentialRevision } from "./mcp-bridge-provider-readiness"; import { type AdapterRegistrationInspection, inspectAdapterRegistrationCommand, @@ -11,10 +12,11 @@ import { buildDeepAgentsMcpStatusCommand } from "./mcp-bridge-adapter-status"; export function inspectDeepAgentsAdapterRegistration( sandboxName: string, entry: McpBridgeEntry, + credentialRevision?: McpAttachedCredentialRevision, ): AdapterRegistrationInspection { return inspectAdapterRegistrationCommand( sandboxName, entry, - buildDeepAgentsMcpStatusCommand(entry), + buildDeepAgentsMcpStatusCommand(entry, credentialRevision), ); } diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-legacy.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-legacy.ts index 8e453b24ac1..50251a1de33 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-legacy.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-legacy.ts @@ -8,6 +8,11 @@ import { DEEPAGENTS_STRICT_JSON_HELPERS, } from "./mcp-bridge-adapter-deepagents-projection"; import { + DEEPAGENTS_LEGACY_CONFIG_HELPERS, + DEEPAGENTS_LEGACY_MCP_CONFIG_PATH, +} from "./mcp-bridge/deepagents-legacy-config"; +import { + MANAGED_HTTP_SERVER_MATCH_HELPERS, DEEPAGENTS_MCP_CONFIG_PATH, deepAgentsManagedServerConfig, pythonJsonLiteral, @@ -24,59 +29,13 @@ import { // and runtime-generation suites execute the rendered helper against real files. // removalCondition: delete this compatibility module after supported releases can // no longer contain registry-owned v1 entries and the migration window has ended. -export const DEEPAGENTS_LEGACY_MCP_CONFIG_PATH = "/sandbox/.deepagents/.mcp.json"; - -export const DEEPAGENTS_LEGACY_CONFIG_HELPERS = [ - "LEGACY_MCP_MAX_BYTES = 262144", - "def legacy_fingerprint(metadata):", - " return (metadata.st_dev, metadata.st_ino, metadata.st_size, metadata.st_mtime_ns, metadata.st_ctime_ns, metadata.st_mode, metadata.st_nlink, metadata.st_uid)", - "def read_legacy_config(path):", - " flags = os.O_RDONLY | os.O_CLOEXEC | os.O_NONBLOCK | os.O_NOFOLLOW", - " descriptor = os.open(path, flags)", - " try:", - " before = os.fstat(descriptor)", - " linked = os.stat(path, follow_symlinks=False)", - " safe = (stat.S_ISREG(before.st_mode) and before.st_uid == os.getuid() and stat.S_IMODE(before.st_mode) == 0o600 and before.st_nlink == 1 and (before.st_dev, before.st_ino) == (linked.st_dev, linked.st_ino))", - " if not safe:", - " raise ValueError('legacy MCP config has unsafe ownership, mode, type, or links')", - " if before.st_size <= 0 or before.st_size > LEGACY_MCP_MAX_BYTES:", - " raise ValueError('legacy MCP config has invalid size')", - " chunks = []", - " remaining = before.st_size", - " while remaining:", - " chunk = os.read(descriptor, remaining)", - " if not chunk:", - " break", - " chunks.append(chunk)", - " remaining -= len(chunk)", - " after = os.fstat(descriptor)", - " linked_after = os.stat(path, follow_symlinks=False)", - " stable = (legacy_fingerprint(before) == legacy_fingerprint(after) and legacy_fingerprint(after) == legacy_fingerprint(linked_after))", - " if remaining or not stable:", - " raise ValueError('legacy MCP config changed while reading')", - " finally:", - " os.close(descriptor)", - " raw = b''.join(chunks).decode('utf-8')", - " data = strict_json_loads(raw)", - " return data, legacy_fingerprint(before)", - "def assert_legacy_source_stable(path, identity):", - " if identity is None:", - " if os.path.lexists(path):", - " raise ValueError('legacy MCP config appeared during mutation')", - " return", - " current = os.stat(path, follow_symlinks=False)", - " safe = (stat.S_ISREG(current.st_mode) and current.st_uid == os.getuid() and stat.S_IMODE(current.st_mode) == 0o600 and current.st_nlink == 1 and legacy_fingerprint(current) == identity)", - " if not safe:", - " raise ValueError('legacy MCP config changed before mutation')", -]; - export function buildDeepAgentsMcpRollbackRegisterCommand( entry: McpBridgeEntry, expectedServers: Record>, ): string { const payload = { server: entry.server, - expected: deepAgentsManagedServerConfig(entry), + expected: expectedServers[entry.server] ?? deepAgentsManagedServerConfig(entry), expectedServers, }; return [ @@ -88,6 +47,7 @@ export function buildDeepAgentsMcpRollbackRegisterCommand( ...DEEPAGENTS_STRICT_JSON_HELPERS, ...DEEPAGENTS_MANAGED_PROJECTION_HELPERS, ...DEEPAGENTS_LEGACY_CONFIG_HELPERS, + ...MANAGED_HTTP_SERVER_MATCH_HELPERS, `runtime_kind = "auto" # NEMOCLAW_DEEPAGENTS_RUNTIME_TEST_ANCHOR`, "if runtime_kind == 'auto':", " runtime_kind = 'unknown'", @@ -131,9 +91,15 @@ export function buildDeepAgentsMcpRollbackRegisterCommand( " servers = data.get('mcpServers', {})", " if not isinstance(servers, dict):", " fail_rollback(f'Invalid managed MCP v2 server map at {config_path}')", - " if any(payload['expectedServers'].get(name) != current for name, current in servers.items()):", + " if any(not managed_http_server_matches(current, payload['expectedServers'].get(name), True) for name, current in servers.items()):", " fail_rollback(f'Refusing to overwrite drifted managed MCP v2 projection at {config_path}')", - " data = {'mcpServers': payload['expectedServers']}", + " next_servers = {}", + " for name, expected in payload['expectedServers'].items():", + " if name != payload['server'] and name in servers:", + " next_servers[name] = servers[name]", + " else:", + " next_servers[name] = expected", + " data = {'mcpServers': next_servers}", "else:", " servers = data.setdefault('mcpServers', {})", " if not isinstance(servers, dict):", @@ -173,7 +139,8 @@ export function buildDeepAgentsMcpRollbackRegisterCommand( "except (OSError, UnicodeDecodeError, ValueError) as exc:", " fail_rollback(f'Could not verify managed MCP rollback state at {config_path}: {exc}')", "if is_v2:", - " restored = persisted == {'mcpServers': payload['expectedServers']}", + " persisted_servers = persisted.get('mcpServers') if isinstance(persisted, dict) else None", + " restored = isinstance(persisted_servers, dict) and set(persisted_servers) == set(payload['expectedServers']) and all(managed_http_server_matches(persisted_servers.get(name), expected, True) for name, expected in payload['expectedServers'].items())", "else:", " persisted_servers = persisted.get('mcpServers') if isinstance(persisted, dict) else None", " restored = isinstance(persisted_servers, dict) and persisted_servers.get(payload['server']) == payload['expected']", diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts index 037c6b583e1..da848e4ff0f 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-projection.test.ts @@ -74,7 +74,7 @@ describe("Deep Agents managed MCP projection safety", () => { ); }); - it("keeps status inspection nonblocking and no-follow for hostile projection paths", () => { + it("fails status inspection closed without following hostile projection paths", () => { const statusCommand = buildDeepAgentsMcpStatusCommand(baseEntry); expect(statusCommand).toContain("os.O_NONBLOCK | os.O_NOFOLLOW"); expect(statusCommand).not.toContain("config_path.read_text"); @@ -86,15 +86,17 @@ describe("Deep Agents managed MCP projection safety", () => { 0o600, { symlink: true }, ); - expect(symlink.status, symlink.stderr).toBe(0); - expect(symlink.stdout.trim()).toBe("absent"); + expect(symlink.status).toBe(2); + expect(symlink.stdout.trim()).toBe(""); + expect(symlink.stderr).toContain("Could not inspect managed Deep Agents MCP state"); expect(symlink.managedSymlinkTargetText).toBe(`${JSON.stringify(emptyProjection, null, 2)}\n`); const fifo = runDeepAgentsConfigCommand(statusCommand, undefined, "v2", undefined, 0o600, { fifo: true, }); - expect(fifo.status, fifo.stderr).toBe(0); - expect(fifo.stdout.trim()).toBe("absent"); + expect(fifo.status).toBe(2); + expect(fifo.stdout.trim()).toBe(""); + expect(fifo.stderr).toContain("Could not inspect managed Deep Agents MCP state"); }); it.each([ diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.test.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.test.ts index 698b05de4e8..034a2827fa1 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.test.ts @@ -44,6 +44,26 @@ describe("Deep Agents MCP config adapter registration", () => { }); }); + it("publishes the readiness-proven credential revision without a raw credential", () => { + const registration = runDeepAgentsConfigCommand( + buildDeepAgentsMcpRegisterCommand(baseEntry, false, [baseEntry], false, "v12"), + ); + + expect(registration.status, registration.stderr).toBe(0); + expect(registration.config).toEqual({ + mcpServers: { + github: { + type: "http", + url: baseEntry.url, + headers: { + Authorization: "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + }, + }, + }, + }); + expect(registration.configText).not.toContain("host-only-secret"); + }); + it("rejects unowned config before registration mutates the file", () => { const initialConfig = { ui: { theme: "dark" } }; const registration = runDeepAgentsConfigCommand( @@ -56,7 +76,7 @@ describe("Deep Agents MCP config adapter registration", () => { expect(registration.config).toEqual(initialConfig); }); - it("renders the complete registry-owned server projection", () => { + it("preserves a sibling revision while revising the target server", () => { const jiraEntry: McpBridgeEntry = { ...baseEntry, server: "jira", @@ -66,14 +86,14 @@ describe("Deep Agents MCP config adapter registration", () => { policyName: "mcp-bridge-jira", }; const registration = runDeepAgentsConfigCommand( - buildDeepAgentsMcpRegisterCommand(jiraEntry, false, [baseEntry, jiraEntry]), + buildDeepAgentsMcpRegisterCommand(jiraEntry, false, [baseEntry, jiraEntry], false, "v12"), { mcpServers: { github: { type: "http", url: baseEntry.url, headers: { - Authorization: "Bearer openshell:resolve:env:GITHUB_TOKEN", + Authorization: "Bearer openshell:resolve:env:v11_GITHUB_TOKEN", }, }, }, @@ -86,29 +106,47 @@ describe("Deep Agents MCP config adapter registration", () => { github: { type: "http", url: baseEntry.url, - headers: { Authorization: "Bearer openshell:resolve:env:GITHUB_TOKEN" }, + headers: { Authorization: "Bearer openshell:resolve:env:v11_GITHUB_TOKEN" }, }, jira: { type: "http", url: jiraEntry.url, - headers: { Authorization: "Bearer openshell:resolve:env:JIRA_MCP_TOKEN" }, + headers: { Authorization: "Bearer openshell:resolve:env:v12_JIRA_MCP_TOKEN" }, }, }, }); }); - it("rejects a 65-server projection before rendering a mutation command", () => { - const managedEntries = Array.from( - { length: 65 }, - (_, index): McpBridgeEntry => ({ - ...baseEntry, - server: `server${String(index)}`, - env: [`SERVER_${String(index)}_TOKEN`], - providerName: `alpha-mcp-server-${String(index)}`, - policyName: `mcp-bridge-server-${String(index)}`, - }), + it("rejects a missing registry sibling before changing the target server", () => { + const jiraEntry: McpBridgeEntry = { + ...baseEntry, + server: "jira", + url: "https://mcp.atlassian.com/v1/", + env: ["JIRA_MCP_TOKEN"], + providerName: "alpha-mcp-jira", + policyName: "mcp-bridge-jira", + }; + const initialConfig = { mcpServers: {} }; + const registration = runDeepAgentsConfigCommand( + buildDeepAgentsMcpRegisterCommand(jiraEntry, false, [baseEntry, jiraEntry], false, "v12"), + initialConfig, ); + expect(registration.status).toBe(2); + expect(registration.stderr).toContain("registry-owned MCP sibling 'github' is absent"); + expect(registration.config).toEqual(initialConfig); + expect(registration.configText).not.toContain("openshell:resolve:env:GITHUB_TOKEN"); + }); + + it("rejects a 65-server projection before rendering a mutation command", () => { + const managedEntries = Array.from({ length: 65 }, (_, index): McpBridgeEntry => ({ + ...baseEntry, + server: `server${String(index)}`, + env: [`SERVER_${String(index)}_TOKEN`], + providerName: `alpha-mcp-server-${String(index)}`, + policyName: `mcp-bridge-server-${String(index)}`, + })); + expect(() => buildDeepAgentsMcpRegisterCommand(managedEntries[0], false, managedEntries), ).toThrow(/at most 64 servers.*refusing to render a 65-server mutation/); @@ -120,7 +158,7 @@ describe("Deep Agents MCP config adapter registration", () => { it("rejects an oversized rendered projection before truncating existing state", () => { const initialConfig = { mcpServers: {} }; const oversized = buildDeepAgentsMcpRegisterCommand(baseEntry).replace( - "data = {'mcpServers': payload['expectedServers']}", + "data = {'mcpServers': next_servers}", "data = {'mcpServers': {'oversized': {'blob': 'x' * 300000}}}", ); const registration = runDeepAgentsConfigCommand(oversized, initialConfig); diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.ts index 54d02108ae1..0b9365de1b0 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-registration.ts @@ -11,10 +11,12 @@ import { DEEPAGENTS_STRICT_JSON_HELPERS, } from "./mcp-bridge-adapter-deepagents-projection"; import { + MANAGED_HTTP_SERVER_MATCH_HELPERS, DEEPAGENTS_MCP_CONFIG_PATH, deepAgentsManagedServerConfig, pythonJsonLiteral, } from "./mcp-bridge-adapter-status"; +import type { McpAttachedCredentialRevision } from "./mcp-bridge-provider-readiness"; import { McpBridgeError } from "./mcp-bridge-contracts"; export function buildDeepAgentsMcpRegisterCommand( @@ -22,6 +24,7 @@ export function buildDeepAgentsMcpRegisterCommand( replaceExisting = false, managedEntries: readonly McpBridgeEntry[] = [entry], teardownRollback = false, + credentialRevision?: McpAttachedCredentialRevision, ): string { const expectedServers = Object.fromEntries( managedEntries @@ -31,6 +34,7 @@ export function buildDeepAgentsMcpRegisterCommand( ]) .sort(([left], [right]) => left.localeCompare(right)), ); + expectedServers[entry.server] = deepAgentsManagedServerConfig(entry, credentialRevision); const expectedServerCount = Object.keys(expectedServers).length; if (!teardownRollback && expectedServerCount > DEEPAGENTS_MCP_MAX_SERVERS) { throw new McpBridgeError( @@ -42,7 +46,7 @@ export function buildDeepAgentsMcpRegisterCommand( } const payload = { server: entry.server, - expected: deepAgentsManagedServerConfig(entry), + expected: deepAgentsManagedServerConfig(entry, credentialRevision), expectedServers, replaceExisting, }; @@ -53,6 +57,7 @@ export function buildDeepAgentsMcpRegisterCommand( `config_path = pathlib.Path(${JSON.stringify(DEEPAGENTS_MCP_CONFIG_PATH)})`, ...DEEPAGENTS_STRICT_JSON_HELPERS, ...DEEPAGENTS_MANAGED_PROJECTION_HELPERS, + ...MANAGED_HTTP_SERVER_MATCH_HELPERS, "source_descriptor = None", "def fail_registration(message):", " close_managed_projection_descriptor(source_descriptor)", @@ -71,12 +76,25 @@ export function buildDeepAgentsMcpRegisterCommand( ` fail_registration('Invalid ${DEEPAGENTS_MCP_CONFIG_PATH}: mcpServers must be an object')`, "if payload['server'] in servers and not payload['replaceExisting']:", ` fail_registration(f"MCP server '{payload['server']}' already exists in ${DEEPAGENTS_MCP_CONFIG_PATH} and is not managed by NemoClaw.")`, + "for name, expected in payload['expectedServers'].items():", + " if name == payload['server']:", + " continue", + " if name not in servers:", + ` fail_registration(f"Invalid ${DEEPAGENTS_MCP_CONFIG_PATH}: registry-owned MCP sibling '{name}' is absent")`, + " if not managed_http_server_matches(servers[name], expected, True):", + ` fail_registration(f"Invalid ${DEEPAGENTS_MCP_CONFIG_PATH}: registry-owned MCP sibling '{name}' is not exact registry-owned state")`, "for name, current in servers.items():", " if name == payload['server'] and payload['replaceExisting']:", " continue", - " if payload['expectedServers'].get(name) != current:", + " if not managed_http_server_matches(current, payload['expectedServers'].get(name), True):", ` fail_registration(f"Invalid ${DEEPAGENTS_MCP_CONFIG_PATH}: MCP server '{name}' is not exact registry-owned state")`, - "data = {'mcpServers': payload['expectedServers']}", + "next_servers = {}", + "for name, expected in payload['expectedServers'].items():", + " if name != payload['server'] and name in servers:", + " next_servers[name] = servers[name]", + " else:", + " next_servers[name] = expected", + "data = {'mcpServers': next_servers}", "config_path.parent.mkdir(parents=True, exist_ok=True)", "try:", " write_managed_projection(config_path, data, source_identity, source_descriptor)", @@ -97,8 +115,12 @@ function registryOwnedDeepAgentsEntries( return [...entries.values()]; } -function verifyDeepAgentsAdapterRegistration(sandboxName: string, entry: McpBridgeEntry): void { - const inspection = inspectDeepAgentsAdapterRegistration(sandboxName, entry); +function verifyDeepAgentsAdapterRegistration( + sandboxName: string, + entry: McpBridgeEntry, + credentialRevision?: McpAttachedCredentialRevision, +): void { + const inspection = inspectDeepAgentsAdapterRegistration(sandboxName, entry, credentialRevision); if (inspection.state === "registered") return; const detail = inspection.state === "error" ? inspection.detail : inspection.state; throw new McpBridgeError( @@ -112,6 +134,7 @@ export function registerDeepAgentsAdapter( envValues: Record = {}, replaceExisting = false, teardownRollback = false, + credentialRevision?: McpAttachedCredentialRevision, ): void { const stdout = runDeepAgentsAdapterCommand( sandboxName, @@ -121,6 +144,7 @@ export function registerDeepAgentsAdapter( replaceExisting, registryOwnedDeepAgentsEntries(sandboxName, entry), teardownRollback, + credentialRevision, ), `Deep Agents Code MCP config registration failed for '${entry.server}'.`, { envValues }, @@ -132,6 +156,6 @@ export function registerDeepAgentsAdapter( ); } } else { - verifyDeepAgentsAdapterRegistration(sandboxName, entry); + verifyDeepAgentsAdapterRegistration(sandboxName, entry, credentialRevision); } } diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-rollback.test.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-rollback.test.ts index c43034bf5a0..0cedb3f7642 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-rollback.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-rollback.test.ts @@ -69,16 +69,34 @@ describe("Deep Agents MCP config adapter rollback", () => { expect(rollback.legacyConfig).toEqual(legacyConfig); }); - it("does not apply the v2 server cap to a single-entry legacy rollback", () => { - const managedEntries = Array.from( - { length: 65 }, - (_, index): McpBridgeEntry => ({ - ...baseEntry, - server: `server${String(index)}`, - env: [`SERVER_${String(index)}_TOKEN`], - }), + it("restores the readiness-proven revision during v2 rollback", () => { + const rollback = runDeepAgentsConfigCommand( + buildDeepAgentsMcpRegisterCommand(baseEntry, true, [baseEntry], true, "v12"), + { mcpServers: {} }, + "v2", ); + expect(rollback.status, rollback.stderr).toBe(0); + expect(rollback.config).toEqual({ + mcpServers: { + github: { + type: "http", + url: baseEntry.url, + headers: { + Authorization: "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + }, + }, + }, + }); + }); + + it("does not apply the v2 server cap to a single-entry legacy rollback", () => { + const managedEntries = Array.from({ length: 65 }, (_, index): McpBridgeEntry => ({ + ...baseEntry, + server: `server${String(index)}`, + env: [`SERVER_${String(index)}_TOKEN`], + })); + const rollback = runDeepAgentsConfigCommand( buildDeepAgentsMcpRegisterCommand(managedEntries[0], true, managedEntries, true), undefined, diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-teardown.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-teardown.ts index cf81ca1fd52..c27d89dc2f3 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-teardown.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-teardown.ts @@ -6,7 +6,7 @@ import { runDeepAgentsAdapterCommand } from "./mcp-bridge-adapter-deepagents-com import { DEEPAGENTS_LEGACY_CONFIG_HELPERS, DEEPAGENTS_LEGACY_MCP_CONFIG_PATH, -} from "./mcp-bridge-adapter-deepagents-legacy"; +} from "./mcp-bridge/deepagents-legacy-config"; import { DEEPAGENTS_MANAGED_PROJECTION_HELPERS, DEEPAGENTS_STRICT_JSON_HELPERS, @@ -16,6 +16,7 @@ import type { AdapterRemovalOutcome, } from "./mcp-bridge-adapter-inspection"; import { + MANAGED_HTTP_SERVER_MATCH_HELPERS, DEEPAGENTS_MCP_CONFIG_PATH, deepAgentsManagedServerConfig, pythonJsonLiteral, @@ -40,6 +41,7 @@ export function buildDeepAgentsMcpRemoveCommand( ...DEEPAGENTS_STRICT_JSON_HELPERS, ...DEEPAGENTS_MANAGED_PROJECTION_HELPERS, ...DEEPAGENTS_LEGACY_CONFIG_HELPERS, + ...MANAGED_HTTP_SERVER_MATCH_HELPERS, `runtime_kind = "${adaptiveTeardown ? "auto" : "v2"}" # NEMOCLAW_DEEPAGENTS_RUNTIME_TEST_ANCHOR`, "if runtime_kind == 'auto':", " runtime_kind = 'unknown'", @@ -139,14 +141,14 @@ export function buildDeepAgentsMcpRemoveCommand( " repair_v2_projection(managed_identity, managed_descriptor)", " finish('removed')", " fail_teardown(f'Invalid managed MCP v2 projection at {config_path}: only mcpServers is allowed')", - " if present and not payload['force'] and current != payload['expected']:", + " if present and not payload['force'] and not managed_http_server_matches(current, payload['expected'], True):", " fail_teardown(f\"Refusing to remove modified MCP server '{payload['server']}' from {config_path}. Use --force to remove it.\")", " if not present:", " finish('absent')", "else:", " if not present:", " finish('absent')", - " if current != payload['expected'] and not payload['force']:", + " if not managed_http_server_matches(current, payload['expected'], True) and not payload['force']:", " finish('unowned')", "servers.pop(payload['server'])", "if is_v2:", diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-v2-removal.test.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-v2-removal.test.ts index 65397930c76..3a95e1f56ec 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-v2-removal.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-deepagents-v2-removal.test.ts @@ -10,6 +10,135 @@ import { buildDeepAgentsMcpRemoveCommand } from "./mcp-bridge-adapter-deepagents import { buildDeepAgentsMcpStatusCommand } from "./mcp-bridge-adapter-status"; describe("Deep Agents MCP config adapter v2 removal", () => { + it("inspects the installed legacy runtime projection", () => { + const legacyConfig = { + mcpServers: { + github: { + type: "http", + url: baseEntry.url, + headers: { + Authorization: "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + }, + }, + }, + }; + const status = runDeepAgentsConfigCommand( + buildDeepAgentsMcpStatusCommand(baseEntry), + undefined, + "legacy", + legacyConfig, + ); + + expect(status.status, status.stderr).toBe(0); + expect(status.stdout.trim()).toBe("registered"); + }); + + it("reports an unknown installed runtime as an inspection failure", () => { + const status = runDeepAgentsConfigCommand( + buildDeepAgentsMcpStatusCommand(baseEntry), + undefined, + "unknown", + ); + + expect(status.status).toBe(2); + expect(status.stderr).toContain("Could not identify the managed Deep Agents MCP runtime"); + expect(status.stdout.trim()).toBe(""); + }); + + it("reports unsafe legacy state as an inspection failure instead of absence", () => { + const legacyConfig = { + mcpServers: { + github: { + type: "http", + url: baseEntry.url, + headers: { + Authorization: "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + }, + }, + }, + }; + const status = runDeepAgentsConfigCommand( + buildDeepAgentsMcpStatusCommand(baseEntry), + undefined, + "legacy", + legacyConfig, + 0o644, + ); + + expect(status.status).toBe(2); + expect(status.stderr).toContain("legacy MCP config has unsafe ownership, mode, type, or links"); + expect(status.stdout.trim()).toBe(""); + }); + + it("recognizes and removes an exact revision-scoped managed credential", () => { + const revisionedConfig = { + mcpServers: { + github: { + type: "http", + url: baseEntry.url, + headers: { + Authorization: "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + }, + }, + }, + }; + + const status = runDeepAgentsConfigCommand( + buildDeepAgentsMcpStatusCommand(baseEntry), + revisionedConfig, + ); + expect(status.status, status.stderr).toBe(0); + expect(status.stdout.trim()).toBe("registered"); + + const removal = runDeepAgentsConfigCommand( + buildDeepAgentsMcpRemoveCommand(baseEntry), + revisionedConfig, + ); + expect(removal.status, removal.stderr).toBe(0); + expect(removal.config).toEqual({ mcpServers: {} }); + }); + + it("requires the exact revision when registration supplies one", () => { + const status = runDeepAgentsConfigCommand(buildDeepAgentsMcpStatusCommand(baseEntry, "v12"), { + mcpServers: { + github: { + type: "http", + url: baseEntry.url, + headers: { + Authorization: "Bearer openshell:resolve:env:v11_GITHUB_TOKEN", + }, + }, + }, + }); + + expect(status.status, status.stderr).toBe(0); + expect(status.stdout.trim()).toBe("mismatch"); + }); + + it.each([ + "Bearer openshell:resolve:env:v_GITHUB_TOKEN", + "Bearer openshell:resolve:env:v12_OTHER_TOKEN", + `Bearer openshell:resolve:env:v${"1".repeat(21)}_GITHUB_TOKEN`, + "Bearer raw-credential", + ])("rejects an invalid revision-scoped ownership claim: %s", (authorization) => { + const config = { + mcpServers: { + github: { + type: "http", + url: baseEntry.url, + headers: { Authorization: authorization }, + }, + }, + }; + const status = runDeepAgentsConfigCommand(buildDeepAgentsMcpStatusCommand(baseEntry), config); + const removal = runDeepAgentsConfigCommand(buildDeepAgentsMcpRemoveCommand(baseEntry), config); + + expect(status.status, status.stderr).toBe(0); + expect(status.stdout.trim()).toBe("mismatch"); + expect(removal.status).toBe(2); + expect(removal.config).toEqual(config); + }); + it("fails Deep Agents removal on corrupt config unless forced", () => { const corruptProjection = { mcpServers: [] }; const normal = runDeepAgentsConfigCommand( diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-hermes.test.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-hermes.test.ts index 8eb3efb8af8..653ba1c8801 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-hermes.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-hermes.test.ts @@ -64,4 +64,17 @@ describe("Hermes MCP config adapter", () => { "probe", ]); }); + + it("declares the readiness-proven credential revision for transaction validation (#10155)", () => { + const command = buildHermesMcpRegisterCommand(baseEntry, false, "v12"); + + expect(JSON.parse(command[3] ?? "{}")).toEqual({ + server: "github", + url: "https://api.githubcopilot.com/mcp/", + headers: { Authorization: "Bearer openshell:resolve:env:v12_GITHUB_TOKEN" }, + replace_existing: false, + credential_name: "GITHUB_TOKEN", + credential_revision: "v12", + }); + }); }); diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-hermes.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-hermes.ts index 6b3c840d9ab..a5acb4d53cc 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-hermes.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-hermes.ts @@ -17,6 +17,7 @@ import { import { buildHermesMcpStatusCommand, entryHeaders } from "./mcp-bridge-adapter-status"; import { McpBridgeError } from "./mcp-bridge-contracts"; import { commandOutput, redactBridgeSecretsForDisplay } from "./mcp-bridge-output"; +import type { McpAttachedCredentialRevision } from "./mcp-bridge-provider-readiness"; import { executeGatewaySupervisorAction } from "./process-recovery"; const HERMES_MCP_TRANSACTION_HELPER = "/usr/local/lib/nemoclaw/hermes-mcp-config-transaction.py"; @@ -32,12 +33,16 @@ const HERMES_MCP_LIFECYCLE_NOT_READY = export function buildHermesMcpRegisterCommand( entry: McpBridgeEntry, replaceExisting = false, + credentialRevision?: McpAttachedCredentialRevision, ): string[] { const payload = { server: entry.server, url: entry.url, - headers: entryHeaders(entry), + headers: entryHeaders(entry, credentialRevision), replace_existing: replaceExisting, + ...(credentialRevision + ? { credential_name: entry.env[0], credential_revision: credentialRevision } + : {}), }; return [HERMES_MCP_TRANSACTION_HELPER, "add", "--payload", JSON.stringify(payload)]; } @@ -77,8 +82,13 @@ export function buildHermesMcpProbeCommand(): string[] { export function inspectHermesAdapterRegistration( sandboxName: string, entry: McpBridgeEntry, + credentialRevision?: McpAttachedCredentialRevision, ): AdapterRegistrationInspection { - return inspectAdapterRegistrationCommand(sandboxName, entry, buildHermesMcpStatusCommand(entry)); + return inspectAdapterRegistrationCommand( + sandboxName, + entry, + buildHermesMcpStatusCommand(entry, credentialRevision), + ); } function parseLastJsonObject(output: string): Record | null { @@ -267,8 +277,12 @@ function runHermesAdapterCommand( } } -function verifyHermesAdapterRegistration(sandboxName: string, entry: McpBridgeEntry): void { - const inspection = inspectHermesAdapterRegistration(sandboxName, entry); +function verifyHermesAdapterRegistration( + sandboxName: string, + entry: McpBridgeEntry, + credentialRevision?: McpAttachedCredentialRevision, +): void { + const inspection = inspectHermesAdapterRegistration(sandboxName, entry, credentialRevision); if (inspection.state === "registered") return; const detail = inspection.state === "error" ? inspection.detail : inspection.state; throw new McpBridgeError( @@ -281,15 +295,16 @@ export function registerHermesAdapter( entry: McpBridgeEntry, envValues: Record = {}, replaceExisting = false, + credentialRevision?: McpAttachedCredentialRevision, ): void { runHermesAdapterCommand( sandboxName, entry, - buildHermesMcpRegisterCommand(entry, replaceExisting), + buildHermesMcpRegisterCommand(entry, replaceExisting, credentialRevision), `Hermes MCP config registration failed for '${entry.server}'.`, { envValues, requireReload: true }, ); - verifyHermesAdapterRegistration(sandboxName, entry); + verifyHermesAdapterRegistration(sandboxName, entry, credentialRevision); } export function unregisterHermesAdapter( diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-registration.test.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-registration.test.ts index 4b788a2c870..4c132bce1b1 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-registration.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-registration.test.ts @@ -9,6 +9,7 @@ import type { McpBridgeEntry } from "../../state/registry"; const mocks = vi.hoisted(() => ({ executeSandboxCommand: vi.fn(), executeGatewaySupervisorAction: vi.fn(), + getSandbox: vi.fn(), runOpenshellProviderCommand: vi.fn(), })); @@ -22,16 +23,18 @@ vi.mock("../../adapters/openshell/provider-command", () => ({ runOpenshellProviderCommand: mocks.runOpenshellProviderCommand, })); +vi.mock("../../state/registry", async (importOriginal) => ({ + ...(await importOriginal()), + getSandbox: mocks.getSandbox, +})); + import { buildDeepAgentsMcpStatusCommand, buildHermesMcpStatusCommand, registerAgentAdapter, } from "./mcp-bridge-adapters"; import { registerOpenClawAdapter } from "./mcp-bridge-adapter-openclaw"; -import { - entryHeaders, - mcporterHeadersMatchExpected, -} from "./mcp-bridge-adapter-status"; +import { entryHeaders, mcporterHeadersMatchExpected } from "./mcp-bridge-adapter-status"; const baseEntry: McpBridgeEntry = { server: "github", @@ -93,6 +96,7 @@ describe.each(adapterCases)("$name MCP adapter registration", (adapterCase) => { mocks.executeSandboxCommand.mockReset(); mocks.executeGatewaySupervisorAction.mockReset(); mocks.runOpenshellProviderCommand.mockReset(); + mocks.getSandbox.mockReset(); }); it("re-reads the persisted definition before registration succeeds", () => { @@ -124,6 +128,7 @@ describe.each(adapterCases)("$name MCP adapter registration", (adapterCase) => { describe("OpenClaw MCP adapter registration", () => { beforeEach(() => { mocks.executeSandboxCommand.mockReset(); + mocks.getSandbox.mockReset(); }); it("rejects a v11 post-write observation after registering the readiness-proven v12", () => { @@ -155,3 +160,72 @@ describe("OpenClaw MCP adapter registration", () => { ); }); }); + +describe("Deep Agents MCP adapter credential revision", () => { + beforeEach(() => { + mocks.executeSandboxCommand.mockReset(); + mocks.getSandbox.mockReset(); + }); + + it("writes and verifies the readiness-proven revision", () => { + const entry: McpBridgeEntry = { + ...baseEntry, + agent: "langchain-deepagents-code", + adapter: "deepagents-config", + }; + mocks.executeSandboxCommand.mockReturnValueOnce(commandSuccess).mockReturnValueOnce(registered); + + expect(() => + registerAgentAdapter( + "alpha", + "deepagents-config", + entry, + { GITHUB_TOKEN: "host-only-secret" }, + { credentialRevision: "v12" }, + ), + ).not.toThrow(); + + expect(mocks.executeSandboxCommand.mock.calls[0]?.[1]).toContain( + "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + ); + expect(mocks.executeSandboxCommand.mock.calls[1]?.[1]).toContain( + "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + ); + expect(JSON.stringify(mocks.executeSandboxCommand.mock.calls)).not.toContain( + "host-only-secret", + ); + }); +}); + +describe("Hermes MCP adapter credential revision", () => { + beforeEach(() => { + mocks.executeSandboxCommand.mockReset(); + mocks.runOpenshellProviderCommand.mockReset(); + mocks.getSandbox.mockReset(); + }); + + it("writes and verifies the readiness-proven revision", () => { + mocks.runOpenshellProviderCommand.mockReturnValue(lifecycleSuccess); + mocks.executeSandboxCommand.mockReturnValue(registered); + + expect(() => + registerAgentAdapter( + "alpha", + "hermes-config", + baseEntry, + { GITHUB_TOKEN: "host-only-secret" }, + { credentialRevision: "v12" }, + ), + ).not.toThrow(); + + expect(JSON.stringify(mocks.runOpenshellProviderCommand.mock.calls[0]?.[0])).toContain( + "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + ); + expect(mocks.executeSandboxCommand.mock.calls[0]?.[1]).toContain( + "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + ); + expect(JSON.stringify(mocks.runOpenshellProviderCommand.mock.calls)).not.toContain( + "host-only-secret", + ); + }); +}); diff --git a/src/lib/actions/sandbox/mcp-bridge-adapter-status.ts b/src/lib/actions/sandbox/mcp-bridge-adapter-status.ts index 6ed1286cfaa..3f38bb00504 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapter-status.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapter-status.ts @@ -3,6 +3,10 @@ import type { McpBridgeEntry } from "../../state/registry"; import type { McpAttachedCredentialRevision } from "./mcp-bridge-provider-readiness"; +import { + DEEPAGENTS_LEGACY_CONFIG_HELPERS, + DEEPAGENTS_LEGACY_MCP_CONFIG_PATH, +} from "./mcp-bridge/deepagents-legacy-config"; import { DEEPAGENTS_MANAGED_PROJECTION_READ_HELPERS, DEEPAGENTS_STRICT_JSON_HELPERS, @@ -28,8 +32,7 @@ function authPlaceholder( ): string | null { const envName = entry.env[0]; if (!envName) return null; - const revision = - credentialRevision && credentialRevision !== "canonical" ? `${credentialRevision}_` : ""; + const revision = credentialRevision ? `${credentialRevision}_` : ""; return `openshell:resolve:env:${revision}${envName}`; } @@ -109,8 +112,11 @@ export function mcporterHeaderMatcherSource(): string { return `const mcporterHeadersMatchExpected = ${mcporterHeadersMatchExpected.toString()};`; } -export function hermesManagedServerConfig(entry: McpBridgeEntry): Record { - const headers = entryHeaders(entry); +export function hermesManagedServerConfig( + entry: McpBridgeEntry, + credentialRevision?: McpAttachedCredentialRevision, +): Record { + const headers = entryHeaders(entry, credentialRevision); return { url: entry.url, enabled: true, @@ -130,18 +136,25 @@ export interface HermesMcpIntentPayload { export function buildHermesMcpIntentPayload( entries: readonly McpBridgeEntry[], managedServerNames: readonly string[], + credentialRevisions: ReadonlyMap = new Map(), ): HermesMcpIntentPayload { const sortedEntries = [...entries].sort((left, right) => left.server.localeCompare(right.server)); const present = Object.fromEntries( - sortedEntries.map((entry) => [entry.server, hermesManagedServerConfig(entry)]), + sortedEntries.map((entry) => [ + entry.server, + hermesManagedServerConfig(entry, credentialRevisions.get(entry.server)), + ]), ); const presentNames = new Set(Object.keys(present)); const absent = [...new Set(managedServerNames)].filter((name) => !presentNames.has(name)).sort(); return { present, absent }; } -export function deepAgentsManagedServerConfig(entry: McpBridgeEntry): Record { - const headers = entryHeaders(entry); +export function deepAgentsManagedServerConfig( + entry: McpBridgeEntry, + credentialRevision?: McpAttachedCredentialRevision, +): Record { + const headers = entryHeaders(entry, credentialRevision); return { type: "http", url: entry.url, @@ -149,46 +162,111 @@ export function deepAgentsManagedServerConfig(entry: McpBridgeEntry): Record 0 + ) { + credentialRevision = `v${provider.resourceVersion}`; + } + } + if (!credentialRevision) { + throw new McpBridgeError( + `Could not prove a revision-scoped credential before removing the managed adapter entry for MCP server '${entry.server}'.`, + ); + } const adapter = resolveManagedMcpAdapter(sandbox, entry); const removal = unregisterAgentAdapter(sandboxName, adapter, entry, { envValues: {}, @@ -33,16 +62,32 @@ export function scrubManagedMcpAdapterOrThrow( `Could not prove removal of the exact managed adapter entry for MCP server '${entry.server}'.`, ); } + return { ...entry, credentialRevision }; } /** Restore scrubbed adapter entries without hiding failures from provider rollback. */ export function rollbackScrubbedMcpAdapters( sandboxName: string, sandbox: SandboxEntry, - entries: readonly McpBridgeEntry[], + entries: readonly McpScrubbedAdapterEntry[], ): string[] { const failures: string[] = []; for (const entry of entries) { + let credentialRevision = entry.credentialRevision; + try { + const current = observeMcpCredentialRevision(sandboxName, entry); + if (current !== "absent" && current !== "canonical") credentialRevision = current; + if (current === "canonical") credentialRevision = undefined; + } catch (error) { + failures.push(error instanceof Error ? error.message : String(error)); + continue; + } + if (!credentialRevision) { + failures.push( + `Could not restore the managed adapter entry for MCP server '${entry.server}' without its observed credential revision.`, + ); + continue; + } try { registerAgentAdapter( sandboxName, @@ -52,6 +97,7 @@ export function rollbackScrubbedMcpAdapters( { replaceExisting: true, teardownRollback: true, + credentialRevision, }, ); } catch (error) { diff --git a/src/lib/actions/sandbox/mcp-bridge-adapters.ts b/src/lib/actions/sandbox/mcp-bridge-adapters.ts index d705b5341c2..10677f0a5fd 100644 --- a/src/lib/actions/sandbox/mcp-bridge-adapters.ts +++ b/src/lib/actions/sandbox/mcp-bridge-adapters.ts @@ -143,7 +143,13 @@ export function registerAgentAdapter( ); return; case "hermes-config": - registerHermesAdapter(sandboxName, entry, envValues, options.replaceExisting === true); + registerHermesAdapter( + sandboxName, + entry, + envValues, + options.replaceExisting === true, + options.credentialRevision, + ); return; case "deepagents-config": registerDeepAgentsAdapter( @@ -152,6 +158,7 @@ export function registerAgentAdapter( envValues, options.replaceExisting === true, options.teardownRollback === true, + options.credentialRevision, ); return; } diff --git a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts index 77f8274e02b..f8bcd7a5ff6 100644 --- a/src/lib/actions/sandbox/mcp-bridge-add-restart.ts +++ b/src/lib/actions/sandbox/mcp-bridge-add-restart.ts @@ -482,8 +482,9 @@ async function addMcpBridgeUnlocked( registerAgentAdapter(sandboxName, adapter, entry, adapterEnvValues, { // An exact adapter entry is evidence of a post-commit process death. // Replacing it is idempotent and, for Hermes, re-verifies runtime reload. - // Mcporter must project the same live revision OpenShell will recognize - // at egress; its canonical, unversioned placeholder is not sufficient. + // Credential-bearing adapters must project the same live revision + // OpenShell will recognize at egress. The canonical placeholder omits + // the provider identity required by revision-bound credentials. replaceExisting: resumingPreflightedAdd && adapterInspection.state === "registered", credentialRevision, }); diff --git a/src/lib/actions/sandbox/mcp-bridge-destroy-preflight.ts b/src/lib/actions/sandbox/mcp-bridge-destroy-preflight.ts index 5c40799bb19..0f65bb46ca3 100644 --- a/src/lib/actions/sandbox/mcp-bridge-destroy-preflight.ts +++ b/src/lib/actions/sandbox/mcp-bridge-destroy-preflight.ts @@ -3,6 +3,7 @@ import type { McpBridgeEntry, SandboxEntry } from "../../state/registry"; import * as registry from "../../state/registry"; +import type { McpScrubbedAdapterEntry } from "./mcp-bridge-adapter-teardown"; import { McpBridgeError } from "./mcp-bridge-contracts"; import { assertGeneratedPolicyRegistrationMutationSafe, @@ -25,7 +26,7 @@ import { assertAuthenticatedBridgeEntry, validateSandboxName } from "./mcp-bridg export interface McpDestroyPreparation { entries: McpBridgeEntry[]; detachedProviderEntries: McpBridgeEntry[]; - scrubbedAdapterEntries: 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. */ diff --git a/src/lib/actions/sandbox/mcp-bridge-destroy.ts b/src/lib/actions/sandbox/mcp-bridge-destroy.ts index e8f1deb67ce..fcfa0b95ffb 100644 --- a/src/lib/actions/sandbox/mcp-bridge-destroy.ts +++ b/src/lib/actions/sandbox/mcp-bridge-destroy.ts @@ -6,6 +6,7 @@ import * as registry from "../../state/registry"; import { rollbackScrubbedMcpAdapters, scrubManagedMcpAdapterOrThrow, + type McpScrubbedAdapterEntry, } from "./mcp-bridge-adapter-teardown"; import { MCP_BRIDGE_POLICY_SOURCE, McpBridgeError } from "./mcp-bridge-contracts"; import { removeGeneratedPolicy } from "./mcp-bridge-policy"; @@ -120,12 +121,11 @@ export async function prepareMcpBridgesForDestroy( await ensureSandboxGatewaySelected(sandboxName); assertMcpAdapterTeardownRuntimeCapabilities(sandboxName, sandbox, entries); const detached: McpBridgeEntry[] = []; - const scrubbedAdapters: McpBridgeEntry[] = []; + const scrubbedAdapters: McpScrubbedAdapterEntry[] = []; const removedPolicies: McpBridgeEntry[] = []; try { for (const entry of entries) { - scrubManagedMcpAdapterOrThrow(sandboxName, sandbox, entry); - scrubbedAdapters.push(entry); + scrubbedAdapters.push(scrubManagedMcpAdapterOrThrow(sandboxName, sandbox, entry)); } for (const entry of entries) { removeGeneratedPolicy(sandboxName, entry); diff --git a/src/lib/actions/sandbox/mcp-bridge-hermes-reconciliation.test.ts b/src/lib/actions/sandbox/mcp-bridge-hermes-reconciliation.test.ts index 496feda1393..b485c35c6c1 100644 --- a/src/lib/actions/sandbox/mcp-bridge-hermes-reconciliation.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-hermes-reconciliation.test.ts @@ -104,6 +104,19 @@ describe("Hermes MCP host reconciliation", () => { expect(options).toMatchObject({ ignoreError: true, timeout: 60_000 }); }); + it("requires the observed credential revision during status reconciliation (#10079)", () => { + expect( + inspectHermesMcpRuntimeIntent("alpha", { + credentialRevisions: new Map([["github", "v12"]]), + }), + ).toEqual({ ok: true, state: "matched" }); + + const payload = JSON.parse(mocks.runOpenshellProviderCommand.mock.calls[0]?.[0]?.[11]); + expect(payload.present.github.headers).toEqual({ + Authorization: "Bearer openshell:resolve:env:v12_GITHUB_TOKEN", + }); + }); + it("can inspect a removal intent while retaining the removed name as a tombstone", () => { expect( inspectHermesMcpRuntimeIntent("alpha", { diff --git a/src/lib/actions/sandbox/mcp-bridge-hermes-reconciliation.ts b/src/lib/actions/sandbox/mcp-bridge-hermes-reconciliation.ts index 754d924f0be..e3b177c776a 100644 --- a/src/lib/actions/sandbox/mcp-bridge-hermes-reconciliation.ts +++ b/src/lib/actions/sandbox/mcp-bridge-hermes-reconciliation.ts @@ -8,6 +8,7 @@ import * as registry from "../../state/registry"; import { buildHermesMcpIntentPayload } from "./mcp-bridge-adapter-status"; import { McpBridgeError } from "./mcp-bridge-contracts"; import { redactBridgeSecretsForDisplay } from "./mcp-bridge-output"; +import type { McpAttachedCredentialRevision } from "./mcp-bridge-provider-readiness"; import { sleepMcpBridgeRetry } from "./mcp-bridge/timing"; const HERMES_MCP_TRANSACTION_HELPER = "/usr/local/lib/nemoclaw/hermes-mcp-config-transaction.py"; @@ -29,6 +30,7 @@ export type HermesMcpReconciliationResult = export interface HermesMcpReconciliationOptions { entries?: readonly McpBridgeEntry[]; managedServerNames?: readonly string[]; + credentialRevisions?: ReadonlyMap; } export function hermesMcpReconciliationRemediationLines(sandboxName: string): readonly string[] { @@ -152,7 +154,11 @@ export function inspectHermesMcpRuntimeIntent( }; } - const payload = buildHermesMcpIntentPayload(entries, managedServerNames); + const payload = buildHermesMcpIntentPayload( + entries, + managedServerNames, + options.credentialRevisions, + ); let result: ReturnType; try { result = runOpenshellProviderCommand(buildInspectArgs(sandboxName, JSON.stringify(payload)), { diff --git a/src/lib/actions/sandbox/mcp-bridge-provider-readiness.ts b/src/lib/actions/sandbox/mcp-bridge-provider-readiness.ts index a7ad8d4dd6c..60eeadb8e73 100644 --- a/src/lib/actions/sandbox/mcp-bridge-provider-readiness.ts +++ b/src/lib/actions/sandbox/mcp-bridge-provider-readiness.ts @@ -15,7 +15,10 @@ import { executeSandboxExecCommand } from "./process-recovery"; const MCP_CREDENTIAL_REVISION_OBSERVATION_RE = /^(?:absent|canonical|v[0-9]{1,20})$/; export type McpCredentialRevisionObservation = "absent" | "canonical" | `v${number}`; -export type McpAttachedCredentialRevision = Exclude; +export type McpAttachedCredentialRevision = Exclude< + McpCredentialRevisionObservation, + "absent" | "canonical" +>; type McpCredentialRevisionAttempt = | { kind: "observation"; observation: McpCredentialRevisionObservation } @@ -182,9 +185,14 @@ export function waitForAttachedMcpCredential( lastAttempt = attempt; } const observation = attempt.kind === "observation" ? attempt.observation : null; + // The startup command can expose the identityless canonical placeholder + // before the process supervisor receives the attached provider snapshot. + // Endpoint-bound credentials become usable only when a fresh exec sees + // the revision-scoped placeholder issued by that snapshot. const attached = observation !== null && observation !== "absent" && + observation !== "canonical" && (options.previousRevision === undefined || observation !== options.previousRevision); if (attached) attachedRevision = observation; return attached; diff --git a/src/lib/actions/sandbox/mcp-bridge-provider.test.ts b/src/lib/actions/sandbox/mcp-bridge-provider.test.ts index e02a2434eb9..5bbdc2af88c 100644 --- a/src/lib/actions/sandbox/mcp-bridge-provider.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-provider.test.ts @@ -345,12 +345,11 @@ alpha-mcp-slack generic 1 0 ).toThrow(/Could not observe the current OpenShell credential revision/); }); - it("uses native multiline OpenShell exec for attachment readiness", () => { - const exec = vi.spyOn(processRecovery, "executeSandboxExecCommand").mockReturnValue({ - status: 0, - stdout: "canonical", - stderr: "", - }); + it("waits for native multiline OpenShell exec to expose an attached revision", () => { + const exec = vi + .spyOn(processRecovery, "executeSandboxExecCommand") + .mockReturnValueOnce({ status: 0, stdout: "canonical", stderr: "" }) + .mockReturnValue({ status: 0, stdout: "v11", stderr: "" }); const refreshAfterObservedAbsence = vi.fn(); const revision = waitForAttachedMcpCredential( @@ -375,7 +374,32 @@ alpha-mcp-slack generic 1 0 expect(proofCommand).toContain("GITHUB_TOKEN"); expect(proofCommand).not.toContain("base64 -d"); expect(refreshAfterObservedAbsence).not.toHaveBeenCalled(); - expect(revision).toBe("canonical"); + expect(exec).toHaveBeenCalledTimes(2); + expect(revision).toBe("v11"); + }); + + it("does not accept an identityless placeholder as attachment readiness", () => { + vi.stubEnv("NEMOCLAW_MCP_PROVIDER_SYNC_TIMEOUT_SECONDS", "1"); + vi.spyOn(processRecovery, "executeSandboxExecCommand").mockReturnValue({ + status: 0, + stdout: "canonical", + stderr: "", + }); + vi.spyOn(Date, "now").mockReturnValueOnce(0).mockReturnValueOnce(0).mockReturnValue(1_000); + + expect(() => + waitForAttachedMcpCredential("alpha", { + server: "github", + agent: "deepagents-code", + adapter: "deepagents-config", + url: "https://mcp.example.test/mcp", + env: ["GITHUB_TOKEN"], + providerName: "alpha-mcp-github-0123456789abcdef", + providerId: "11111111-2222-4333-8444-555555555555", + policyName: "mcp-bridge-github", + addedAt: "2026-06-01T00:00:00.000Z", + }), + ).toThrow(/last bounded observation: canonical/); }); it("refreshes once after a fresh exec reports the credential absent (#9764)", () => { diff --git a/src/lib/actions/sandbox/mcp-bridge-rebuild.ts b/src/lib/actions/sandbox/mcp-bridge-rebuild.ts index f5dcf11f8a7..027e4f6da86 100644 --- a/src/lib/actions/sandbox/mcp-bridge-rebuild.ts +++ b/src/lib/actions/sandbox/mcp-bridge-rebuild.ts @@ -5,6 +5,7 @@ import type { McpBridgeEntry } from "../../state/registry"; import { rollbackScrubbedMcpAdapters, scrubManagedMcpAdapterOrThrow, + type McpScrubbedAdapterEntry, } from "./mcp-bridge-adapter-teardown"; import { McpBridgeError } from "./mcp-bridge-contracts"; import { @@ -42,7 +43,7 @@ import { assertAuthenticatedBridgeEntry, validateSandboxName } from "./mcp-bridg export interface McpRebuildPreparation { entries: McpBridgeEntry[]; detachedProviderEntries: McpBridgeEntry[]; - scrubbedAdapterEntries: McpBridgeEntry[]; + scrubbedAdapterEntries: McpScrubbedAdapterEntry[]; /** Full read-only target, policy, provider, and registry proof before delete. */ revalidateBeforeDelete?: () => Promise; /** Final synchronous registry-only proof immediately before delete. */ @@ -133,15 +134,14 @@ export async function prepareMcpBridgesForRebuild( for (const entry of entries) assertMcpProviderRecoverable(entry); assertNoProviderCredentialCollisions(sandboxName, entries); const detached: McpBridgeEntry[] = []; - const scrubbedAdapters: McpBridgeEntry[] = []; + const scrubbedAdapters: McpScrubbedAdapterEntry[] = []; const removedPolicies: McpBridgeEntry[] = []; 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. - scrubManagedMcpAdapterOrThrow(sandboxName, sandbox, entry); - scrubbedAdapters.push(entry); + scrubbedAdapters.push(scrubManagedMcpAdapterOrThrow(sandboxName, sandbox, entry)); } for (const entry of entries) { // The same-name replacement journal fingerprints this source row before @@ -204,7 +204,7 @@ export async function prepareMcpBridgesForRebuild( export async function reattachMcpProvidersAfterRebuildAbort( sandboxName: string, entries: readonly McpBridgeEntry[], - scrubbedAdapterEntries: readonly McpBridgeEntry[] = [], + scrubbedAdapterEntries: readonly McpScrubbedAdapterEntry[] = [], ): Promise { if (entries.length === 0 && scrubbedAdapterEntries.length === 0) return; await ensureSandboxGatewaySelected(sandboxName); diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe-security.test.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe-security.test.ts index 4fe743e9489..1e1a41bc292 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe-security.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe-security.test.ts @@ -53,7 +53,7 @@ function probeStdout( describe("MCP credential-resolution probe command security", () => { it("validates and silences proxy env before framing nonce-bound runtime curls (#6379)", () => { - const built = buildCredentialResolutionProbeCommand(baseEntry, "mcporter"); + const built = buildCredentialResolutionProbeCommand(baseEntry, "mcporter", "v11"); expect(built).not.toBeNull(); const command = built?.command ?? ""; const validationIndex = command.indexOf('[ -L "$proxy_env" ]'); @@ -70,7 +70,8 @@ describe("MCP credential-resolution probe command security", () => { expect(frameIndex).toBeGreaterThan(unsetIndex); expect(firstChildIndex).toBeGreaterThan(frameIndex); expect(command).toContain("nemoclaw-start node -e"); - expect(command).toContain("'authorization: Bearer openshell:resolve:env:GITHUB_TOKEN'"); + expect(command).toContain("'authorization: Bearer openshell:resolve:env:v11_GITHUB_TOKEN'"); + expect(command).not.toContain("'authorization: Bearer openshell:resolve:env:GITHUB_TOKEN'"); expect(command).toContain(`'authorization: Bearer ${MCP_PROBE_CONTROL_BEARER}'`); expect(command).toContain('"method":"initialize"'); expect(command).toContain(`${MCP_PROBE_HTTP_MARKER}${built?.resultMarker}:`); @@ -85,7 +86,8 @@ describe("MCP credential-resolution probe command security", () => { ])( "uses the $adapter runtime without capturing endpoint bodies (#6379)", ({ adapter, runtime }) => { - const command = buildCredentialResolutionProbeCommand(baseEntry, adapter)?.command ?? ""; + const command = + buildCredentialResolutionProbeCommand(baseEntry, adapter, "v11")?.command ?? ""; expect(command).toContain(runtime); expect(command).toContain("'/dev/null'"); @@ -95,23 +97,27 @@ describe("MCP credential-resolution probe command security", () => { ); it("refuses missing credentials and unsafe persisted endpoints (#6379)", () => { - expect(buildCredentialResolutionProbeCommand({ ...baseEntry, env: [] }, "mcporter")).toBeNull(); + expect( + buildCredentialResolutionProbeCommand({ ...baseEntry, env: [] }, "mcporter", "v11"), + ).toBeNull(); expect( buildCredentialResolutionProbeCommand( { ...baseEntry, url: "http://api.githubcopilot.com/mcp/" }, "mcporter", + "v11", ), ).toBeNull(); expect( buildCredentialResolutionProbeCommand( { ...baseEntry, url: "https://host.openshell.internal:31337/mcp" }, "mcporter", + "v11", ), ).toBeNull(); }); it("rejects duplicate and out-of-order result markers (#6379)", () => { - const built = buildCredentialResolutionProbeCommand(baseEntry, "mcporter"); + const built = buildCredentialResolutionProbeCommand(baseEntry, "mcporter", "v11"); expect(built).not.toBeNull(); const resultMarker = built?.resultMarker ?? "missing-result-marker"; const duplicated = classifyCredentialResolutionProbe( @@ -154,7 +160,7 @@ describe("MCP credential-resolution probe command security", () => { }); it("accepts only fresh nonce-bound markers after the trusted result frame (#6379)", () => { - const built = buildCredentialResolutionProbeCommand(baseEntry, "mcporter"); + const built = buildCredentialResolutionProbeCommand(baseEntry, "mcporter", "v11"); expect(built).not.toBeNull(); const resultMarker = built?.resultMarker ?? "missing-result-marker"; const probe = classifyCredentialResolutionProbe( diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts index 5d2928353a5..ef401e8b80e 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts @@ -7,12 +7,17 @@ import type { McpBridgeEntry } from "../../state/registry"; const mocks = vi.hoisted(() => ({ executeSandboxCommand: vi.fn(), + observeMcpCredentialRevision: vi.fn(), })); vi.mock("./process-recovery", () => ({ executeSandboxCommand: mocks.executeSandboxCommand, })); +vi.mock("./mcp-bridge-provider", () => ({ + observeMcpCredentialRevision: mocks.observeMcpCredentialRevision, +})); + import { classifyCredentialResolutionProbe, credentialResolutionWarning, @@ -69,6 +74,8 @@ function probeStdout( beforeEach(() => { mocks.executeSandboxCommand.mockReset(); + mocks.observeMcpCredentialRevision.mockReset(); + mocks.observeMcpCredentialRevision.mockReturnValue("v11"); }); describe("MCP credential-resolution probe classification", () => { @@ -338,9 +345,48 @@ describe("MCP credential-resolution probe execution gates", () => { expect(probe).toEqual({ ok: true, httpStatus: 200, controlHttpStatus: 401 }); expect(mocks.executeSandboxCommand).toHaveBeenCalledTimes(1); const [, command] = mocks.executeSandboxCommand.mock.calls[0]; - expect(command).toContain("openshell:resolve:env:GITHUB_TOKEN"); + expect(command).toContain("openshell:resolve:env:v11_GITHUB_TOKEN"); + expect(command).not.toContain("authorization: Bearer openshell:resolve:env:GITHUB_TOKEN"); expect(command).toContain(MCP_PROBE_CONTROL_BEARER); }); + + it("reuses a status observation instead of starting a second revision check (#10079)", () => { + mocks.executeSandboxCommand.mockImplementation((_sandboxName: string, command: string) => { + const resultMarker = command.match(/__NEMOCLAW_SANDBOX_EXEC_STARTED___[0-9a-f]{32}/)?.[0]; + return { + status: 0, + stdout: [ + resultMarker, + probeStdout( + { httpStatus: 200, curlExit: 0, controlHttpStatus: 401, controlExit: 0 }, + resultMarker, + ), + ].join("\n"), + stderr: "", + }; + }); + + const probe = probeCredentialResolution("alpha", baseEntry, "mcporter", readyProbe, "v12"); + + expect(probe).toEqual({ ok: true, httpStatus: 200, controlHttpStatus: 401 }); + expect(mocks.observeMcpCredentialRevision).not.toHaveBeenCalled(); + expect(mocks.executeSandboxCommand.mock.calls[0]?.[1]).toContain( + "openshell:resolve:env:v12_GITHUB_TOKEN", + ); + }); + + it("does not probe with an identityless canonical placeholder (#10079)", () => { + mocks.observeMcpCredentialRevision.mockReturnValue("canonical"); + + const probe = probeCredentialResolution("alpha", baseEntry, "mcporter", readyProbe); + + expect(probe).toEqual({ + ok: null, + detail: + "probe skipped: a fresh OpenShell exec exposed an identityless credential placeholder instead of a revision-scoped placeholder", + }); + expect(mocks.executeSandboxCommand).not.toHaveBeenCalled(); + }); }); describe("MCP credential-resolution warning", () => { @@ -350,7 +396,7 @@ describe("MCP credential-resolution warning", () => { httpStatus: 403, controlHttpStatus: 403, }); - expect(warning).toContain("openshell:resolve:env:GITHUB_TOKEN"); + expect(warning).toContain("openshell:resolve:env:vN_GITHUB_TOKEN"); expect(warning).toContain("identically (HTTP 403)"); expect(warning).toContain("If the stored credential is confirmed valid"); expect(warning).toContain("OpenShell issue 2161"); diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts index 95eb4a17d86..401aaf466bb 100644 --- a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts @@ -7,7 +7,8 @@ * Provider metadata can be fully healthy while the OpenShell gateway never * rewrites the `openshell:resolve:env:` placeholder on egress, so every agent * request fails with the literal placeholder as the bearer token (see - * NVIDIA/OpenShell#2161). + * NVIDIA/OpenShell#2161). Identity-bound provider credentials require the + * revision-scoped placeholder observed through a fresh OpenShell exec. * * The probe is differential: it sends the same idempotent MCP `initialize` * request twice from inside the sandbox — once with the placeholder @@ -52,6 +53,11 @@ import type { AgentMcpAdapter } from "../../agent/defs"; import type { McpBridgeEntry } from "../../state/registry"; import { authorizationValue } from "./mcp-bridge-adapter-status"; import { redactBridgeSecretsForDisplay } from "./mcp-bridge-output"; +import { observeMcpCredentialRevision } from "./mcp-bridge-provider"; +import type { + McpAttachedCredentialRevision, + McpCredentialRevisionObservation, +} from "./mcp-bridge-provider-readiness"; import { type CredentialResolutionProbeReadiness, credentialResolutionReadinessSkipDetail, @@ -178,8 +184,9 @@ function curlCommand(url: string, authorization: string, httpMarker: string): st export function buildCredentialResolutionProbeCommand( entry: Pick, adapter: AgentMcpAdapter, + credentialRevision: McpAttachedCredentialRevision, ): CredentialResolutionProbeCommand | null { - const authorization = authorizationValue(entry); + const authorization = authorizationValue(entry, credentialRevision); if (!authorization) return null; // Never probe a persisted URL that no longer satisfies the current // authenticated-endpoint boundary: the gateway could rewrite the placeholder @@ -366,7 +373,9 @@ export function credentialResolutionWarning( if (probe.httpStatus === undefined || probe.httpStatus !== probe.controlHttpStatus) return undefined; if (probe.httpStatus < 400 || probe.httpStatus >= 500) return undefined; - const placeholder = envName ? `openshell:resolve:env:${envName}` : "openshell:resolve:env:"; + const placeholder = envName + ? `openshell:resolve:env:vN_${envName}` + : "openshell:resolve:env:vN_"; if (probe.httpStatus === 400) { return `Credential resolution could not be verified: a placeholder-bearing MCP initialize probe and a deliberately-unresolvable control probe were rejected identically (HTTP 400). This is inconclusive even with a valid stored credential — the endpoint may reject the probe's initialize request itself (request validation), the '${placeholder}' placeholder may have been forwarded verbatim, or the credential may be expired or revoked. Rotate the credential with mcp restart if in doubt, and compare mcp status for the same server on a known-good host; if that host verifies, suspect this host's OpenShell placeholder rewrite (see NVIDIA/OpenShell issue 2161).`; } @@ -379,13 +388,47 @@ export function probeCredentialResolution( entry: McpBridgeEntry, adapter: AgentMcpAdapter | undefined, readiness: CredentialResolutionProbeReadiness, + observedCredentialRevision?: McpCredentialRevisionObservation, ): CredentialResolutionProbe { if (!adapter) return { ok: null, detail: "MCP adapter is not declared" }; if (entry.addState) return { ok: null, detail: "add transaction incomplete" }; - const probeCommand = buildCredentialResolutionProbeCommand(entry, adapter); - if (!probeCommand) return { ok: null, detail: "no credential binding or safe endpoint to probe" }; const readinessSkipDetail = credentialResolutionReadinessSkipDetail(readiness); if (readinessSkipDetail) return { ok: null, detail: readinessSkipDetail }; + // Reject the entry before the fresh credential observation so an unsafe + // persisted URL cannot trigger either sandbox or endpoint traffic. + try { + if (!entry.env[0] || normalizeMcpServerUrl(entry.url) !== entry.url) { + return { ok: null, detail: "no credential binding or safe endpoint to probe" }; + } + } catch { + return { ok: null, detail: "no credential binding or safe endpoint to probe" }; + } + let credentialRevision = observedCredentialRevision; + if (credentialRevision === undefined) { + try { + credentialRevision = observeMcpCredentialRevision(sandboxName, entry); + } catch { + return { + ok: null, + detail: "probe skipped: the current OpenShell credential revision could not be observed", + }; + } + } + if (credentialRevision === "absent") { + return { + ok: null, + detail: "probe skipped: a fresh OpenShell exec did not expose the credential placeholder", + }; + } + if (credentialRevision === "canonical") { + return { + ok: null, + detail: + "probe skipped: a fresh OpenShell exec exposed an identityless credential placeholder instead of a revision-scoped placeholder", + }; + } + const probeCommand = buildCredentialResolutionProbeCommand(entry, adapter, credentialRevision); + if (!probeCommand) return { ok: null, detail: "no credential binding or safe endpoint to probe" }; const result = executeSandboxCommand(sandboxName, probeCommand.command); return classifyCredentialResolutionProbe(result, entry, probeCommand.resultMarker); } diff --git a/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts b/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts index 7dc0453826f..e2b6015943c 100644 --- a/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts @@ -46,6 +46,8 @@ gatewayRuntime.recoverNamedGatewayRuntime = async () => ({ let providerAttachmentState = "attached"; let providerInspectionState = "present"; let providerCredentialKey = "GITHUB_TOKEN"; +let persistedCredentialRevision = "v11"; +const hermesIntentPayloads = []; providerCommands.runOpenshellProviderCommand = (args) => { if (args[0] === "provider" && args[1] === "get") { if (providerInspectionState === "absent") { @@ -70,11 +72,31 @@ providerCommands.runOpenshellProviderCommand = (args) => { stderr: "", }; } + if (args[0] === "sandbox" && args[1] === "exec" && args.includes("inspect")) { + const payload = JSON.parse(args[args.length - 1]); + hermesIntentPayloads.push(payload); + const authorization = payload.present?.github?.headers?.Authorization; + const matches = authorization === + "Bearer openshell:resolve:env:" + persistedCredentialRevision + "_GITHUB_TOKEN"; + return matches + ? { status: 0, stdout: '{"ok":true,"state":"matched"}\n', stderr: "" } + : { status: 2, stdout: "", stderr: "Hermes MCP config does not match persisted managed intent" }; + } throw new Error("Unexpected OpenShell call: " + args.join(" ")); }; let activePolicyState = "match"; policies.getPresetContentGatewayState = () => activePolicyState; const executedSandboxCommands = []; +let providerCredentialObservation = "v11"; +let credentialObservationCount = 0; +processRecovery.executeSandboxExecCommand = () => { + credentialObservationCount += 1; + return { + status: 0, + stdout: providerCredentialObservation, + stderr: "", + }; +}; processRecovery.executeSandboxCommand = (sandboxName, command) => { executedSandboxCommands.push(command); if (command.includes("NEMOCLAW_MCP_PROBE")) { @@ -108,7 +130,13 @@ processRecovery.executeSandboxCommand = (sandboxName, command) => { stderr: "", }; } - return { status: 0, stdout: "registered", stderr: "" }; + const expectedAuthorization = + "openshell:resolve:env:" + persistedCredentialRevision + "_GITHUB_TOKEN"; + return { + status: 0, + stdout: command.includes(expectedAuthorization) ? "registered" : "mismatch", + stderr: "", + }; }; registry.registerSandbox({ name: "alpha", @@ -202,6 +230,119 @@ describe("MCP status wire-level credential-resolution probe", { timeout: 15_000 expect(payload.exitCode).toBe(0); }); + it("sends the observed revision and rejects canonical probe authority (#10079)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-revision-"); + const { stdout } = runHarness( + home, + String.raw` + const outcomes = []; + for (const observation of ["v19", "canonical"]) { + providerCredentialObservation = observation; + persistedCredentialRevision = observation === "v19" ? "v19" : "v11"; + credentialObservationCount = 0; + executedSandboxCommands.length = 0; + const [status] = await bridge.statusMcpBridge("alpha", "github", { + probeCredentialResolution: true, + }); + const probeCommand = executedSandboxCommands.find((command) => + command.includes("NEMOCLAW_MCP_PROBE"), + ); + outcomes.push({ + observation, + resolution: status.provider.credentialResolution, + probeCommand: probeCommand ?? null, + credentialObservationCount, + }); + } + process.stdout.write(JSON.stringify(outcomes)); +`, + ); + const outcomes = JSON.parse(stdout) as Array<{ + observation: string; + resolution: { ok: boolean | null; detail?: string }; + probeCommand: string | null; + credentialObservationCount: number; + }>; + + expect(outcomes[0]?.probeCommand).toContain( + "authorization: Bearer openshell:resolve:env:v19_GITHUB_TOKEN", + ); + expect(outcomes[0]?.probeCommand).not.toContain( + "authorization: Bearer openshell:resolve:env:GITHUB_TOKEN", + ); + expect(outcomes[1]?.probeCommand).toBeNull(); + expect(outcomes[1]?.resolution.detail).toContain("revision-scoped placeholder"); + expect(outcomes.map((outcome) => outcome.credentialObservationCount)).toEqual([1, 1]); + }); + + it("reports stale persisted revisions for every agent adapter (#10079)", () => { + const home = createTempHome("nemoclaw-mcp-status-stale-revision-"); + const { stdout } = runHarness( + home, + String.raw` + providerCredentialObservation = "v12"; + persistedCredentialRevision = "v11"; + const outcomes = []; + for (const [agent, adapter] of [ + ["openclaw", "mcporter"], + ["langchain-deepagents-code", "deepagents-config"], + ["hermes", "hermes-config"], + ]) { + const current = registry.getSandbox("alpha"); + const entry = { ...current.mcp.bridges.github, agent, adapter }; + registry.updateSandbox("alpha", { + agent, + mcp: { bridges: { github: entry }, managedServerNames: ["github"] }, + }); + credentialObservationCount = 0; + executedSandboxCommands.length = 0; + hermesIntentPayloads.length = 0; + const [status] = await bridge.statusMcpBridge("alpha", "github", { + probeCredentialResolution: true, + }); + outcomes.push({ + agent, + adapter: status.adapter, + resolution: status.provider.credentialResolution, + credentialObservationCount, + adapterCommand: executedSandboxCommands.find( + (command) => !command.includes("NEMOCLAW_MCP_PROBE"), + ) ?? null, + hermesIntent: hermesIntentPayloads[0] ?? null, + probed: executedSandboxCommands.some((command) => command.includes("NEMOCLAW_MCP_PROBE")), + }); + } + process.stdout.write(JSON.stringify(outcomes)); +`, + ); + const outcomes = JSON.parse(stdout) as Array<{ + agent: string; + adapter: { registered: boolean | null; detail?: string }; + resolution: { ok: boolean | null; detail?: string }; + credentialObservationCount: number; + adapterCommand: string | null; + hermesIntent: unknown; + probed: boolean; + }>; + + expect(outcomes.map((outcome) => outcome.adapter.registered)).toEqual([false, false, false]); + expect(outcomes.map((outcome) => outcome.credentialObservationCount)).toEqual([1, 1, 1]); + expect(outcomes.map((outcome) => outcome.probed)).toEqual([false, false, false]); + outcomes.forEach((outcome) => { + expect(outcome.resolution).toEqual({ + ok: null, + detail: + "probe skipped: the managed agent adapter does not match the current credential revision", + }); + }); + expect(outcomes[0]?.adapterCommand).toContain("openshell:resolve:env:v12_GITHUB_TOKEN"); + expect(outcomes[1]?.adapterCommand).toContain("openshell:resolve:env:v12_GITHUB_TOKEN"); + expect(JSON.stringify(outcomes[2]?.hermesIntent)).toContain( + "openshell:resolve:env:v12_GITHUB_TOKEN", + ); + expect(JSON.stringify(outcomes)).not.toContain("openshell:resolve:env:v11_GITHUB_TOKEN"); + }); + it("skips status probe traffic until exact policy and provider readiness are verified (#6379)", () => { const home = createTempHome("nemoclaw-mcp-resolution-readiness-"); const { stdout } = runHarness( @@ -584,6 +725,7 @@ describe("MCP add post-add credential-resolution probe", () => { logLines, errorLines, probed: executedSandboxCommands.some((c) => c.includes("NEMOCLAW_MCP_PROBE")), + probeCommand: executedSandboxCommands.find((c) => c.includes("NEMOCLAW_MCP_PROBE")), exitCode: process.exitCode ?? 0, })); `, @@ -592,9 +734,16 @@ describe("MCP add post-add credential-resolution probe", () => { logLines: string[]; errorLines: string[]; probed: boolean; + probeCommand?: string; exitCode: number; }; expect(payload.probed).toBe(true); + expect(payload.probeCommand).toContain( + "authorization: Bearer openshell:resolve:env:v11_GITHUB_TOKEN", + ); + expect(payload.probeCommand).not.toContain( + "authorization: Bearer openshell:resolve:env:GITHUB_TOKEN", + ); expect(payload.logLines.some((line) => line.includes("MCP server 'github' added"))).toBe(true); expect( payload.errorLines.some( diff --git a/src/lib/actions/sandbox/mcp-bridge-status.ts b/src/lib/actions/sandbox/mcp-bridge-status.ts index e9cf4cef617..a4580968b40 100644 --- a/src/lib/actions/sandbox/mcp-bridge-status.ts +++ b/src/lib/actions/sandbox/mcp-bridge-status.ts @@ -19,10 +19,15 @@ import { redactBridgeSecretsForDisplay } from "./mcp-bridge-output"; import { getPolicyPresence, getRegisteredGeneratedPolicy } from "./mcp-bridge-policy"; import { inspectMcpProvider, + observeMcpCredentialRevision, providerAttached, providerMatchesCredential, providerShapeDetail, } from "./mcp-bridge-provider"; +import type { + McpAttachedCredentialRevision, + McpCredentialRevisionObservation, +} from "./mcp-bridge-provider-readiness"; import { credentialResolutionWarning, probeCredentialResolution, @@ -86,9 +91,17 @@ function getAdapterRegistration( adapter: AgentMcpAdapter | undefined, entry: McpBridgeEntry | undefined, hermesReconciliation?: HermesMcpReconciliationResult, + credentialRevision?: McpAttachedCredentialRevision, + credentialObservationDetail?: string, ): McpBridgeStatus["adapter"] { if (!entry) return { registered: null }; if (!adapter) return { registered: null, detail: "MCP adapter is not declared" }; + if (credentialObservationDetail) { + return { + registered: null, + detail: `Adapter inspection was skipped because ${credentialObservationDetail}.`, + }; + } if (adapter === "hermes-config" && hermesReconciliation) { return hermesReconciliation.ok ? { registered: true } @@ -100,10 +113,11 @@ function getAdapterRegistration( entry, false, openClawMcporterRoot(getAgentConfigDir(entry.agent, DEFAULT_OPENCLAW_CONFIG_DIR)), + credentialRevision, ) : adapter === "hermes-config" - ? buildHermesMcpStatusCommand(entry) - : buildDeepAgentsMcpStatusCommand(entry); + ? buildHermesMcpStatusCommand(entry, credentialRevision) + : buildDeepAgentsMcpStatusCommand(entry, credentialRevision); const result = executeSandboxCommand(sandboxName, command); if (!result) return { registered: null, detail: "sandbox unreachable" }; if (result.status === 0) { @@ -136,6 +150,31 @@ export interface McpBridgeStatusOptions { discoverTools?: boolean; } +function attachedCredentialRevision( + observation: McpCredentialRevisionObservation | null | undefined, +): McpAttachedCredentialRevision | undefined { + return observation !== undefined && + observation !== null && + observation !== "absent" && + observation !== "canonical" + ? observation + : undefined; +} + +function credentialObservationDetail( + observation: McpCredentialRevisionObservation | null | undefined, +): string | undefined { + if (observation === null) + return "the current OpenShell credential revision could not be observed"; + if (observation === "absent") { + return "a fresh OpenShell exec did not expose the credential placeholder"; + } + if (observation === "canonical") { + return "a fresh OpenShell exec exposed an identityless credential placeholder instead of a revision-scoped placeholder"; + } + return undefined; +} + export async function statusMcpBridge( sandboxName: string, server?: string, @@ -189,11 +228,39 @@ export async function statusMcpBridge( ]; } + const credentialObservations = new Map(); + for (const [name, entry] of entries) { + if (!entry || storedCredentialWarning(entry) !== undefined) continue; + try { + credentialObservations.set(name, observeMcpCredentialRevision(sandboxName, entry)); + } catch { + credentialObservations.set(name, null); + } + } + const credentialRevisions = new Map(); + for (const [name, observation] of credentialObservations) { + const revision = attachedCredentialRevision(observation); + if (revision) credentialRevisions.set(name, revision); + } + const hermesCredentialObservationDetail = entries + .map(([name, entry]) => + entry && storedCredentialWarning(entry) === undefined + ? credentialObservationDetail(credentialObservations.get(name)) + : undefined, + ) + .find((detail) => detail !== undefined); + const hermesReconciliation = agent.name === "hermes" && (entries.length > 0 || (sandbox.mcp?.managedServerNames?.length ?? 0) > 0) && entries.every(([, entry]) => !entry || storedCredentialWarning(entry) === undefined) - ? inspectHermesMcpRuntimeIntent(sandboxName) + ? hermesCredentialObservationDetail + ? { + ok: false as const, + state: "error" as const, + detail: hermesCredentialObservationDetail, + } + : inspectHermesMcpRuntimeIntent(sandboxName, { credentialRevisions }) : undefined; if (entries.length === 0 && hermesReconciliation && !hermesReconciliation.ok) { throw new McpBridgeError( @@ -264,11 +331,28 @@ export async function statusMcpBridge( } const unsafeCredentialMayBeAttached = !!credentialWarning && !!entry?.providerName && attached !== false; + const credentialObservation = entry ? credentialObservations.get(name) : undefined; + const credentialRevision = attachedCredentialRevision(credentialObservation); + const observationDetail = credentialObservationDetail(credentialObservation); const readiness = { policyGatewayPresent: policyPresence, providerAttached: attached, providerCredentialReady, }; + const adapterRegistration = unsafeCredentialMayBeAttached + ? { + registered: null, + detail: + "Adapter inspection was skipped because the unsupported legacy credential may still be attached to fresh sandbox children.", + } + : getAdapterRegistration( + sandboxName, + support.adapter, + entry, + hermesReconciliation, + credentialRevision, + observationDetail, + ); const credentialResolution = options.probeCredentialResolution && entry ? unsafeCredentialMayBeAttached @@ -277,7 +361,21 @@ export async function statusMcpBridge( detail: "probe skipped: the unsupported legacy credential may still be attached to fresh sandbox children", } - : probeCredentialResolution(sandboxName, entry, support.adapter, readiness) + : observationDetail + ? { ok: null, detail: `probe skipped: ${observationDetail}` } + : adapterRegistration.registered !== true + ? { + ok: null, + detail: + "probe skipped: the managed agent adapter does not match the current credential revision", + } + : probeCredentialResolution( + sandboxName, + entry, + support.adapter, + readiness, + credentialRevision, + ) : undefined; const resolutionWarning = credentialResolution ? credentialResolutionWarning(entry?.env[0], credentialResolution) @@ -338,13 +436,7 @@ export async function statusMcpBridge( registryPresent: !!registeredPolicy, gatewayPresent: policyPresence, }, - adapter: unsafeCredentialMayBeAttached - ? { - registered: null, - detail: - "Adapter inspection was skipped because the unsupported legacy credential may still be attached to fresh sandbox children.", - } - : getAdapterRegistration(sandboxName, support.adapter, entry, hermesReconciliation), + adapter: adapterRegistration, ...(toolDiscovery ? { toolDiscovery } : {}), ...(entry?.addedAt ? { addedAt: entry.addedAt } : {}), ...(entry?.updatedAt ? { updatedAt: entry.updatedAt } : {}), diff --git a/src/lib/actions/sandbox/mcp-bridge.ts b/src/lib/actions/sandbox/mcp-bridge.ts index ff2b5a783e8..34f4c5ca6d7 100644 --- a/src/lib/actions/sandbox/mcp-bridge.ts +++ b/src/lib/actions/sandbox/mcp-bridge.ts @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 import type { McpBridgeEntry } from "../../state/registry"; +import type { McpScrubbedAdapterEntry } from "./mcp-bridge-adapter-teardown"; import { addMcpBridge as addMcpBridgeLifecycle } from "./mcp-bridge-add-restart"; import { type McpBridgeAddOptions, @@ -86,7 +87,7 @@ export { statusMcpBridge }; export interface McpDestroyPreparation { entries: McpBridgeEntry[]; detachedProviderEntries: McpBridgeEntry[]; - scrubbedAdapterEntries: 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. */ @@ -155,7 +156,7 @@ export async function prepareMcpBridgesForRebuild( export async function reattachMcpProvidersAfterRebuildAbort( sandboxName: string, entries: readonly McpBridgeEntry[], - scrubbedAdapterEntries: readonly McpBridgeEntry[] = [], + scrubbedAdapterEntries: readonly McpScrubbedAdapterEntry[] = [], ): Promise { return reattachMcpProvidersAfterRebuildAbortLifecycle( sandboxName, diff --git a/src/lib/actions/sandbox/mcp-bridge/deepagents-legacy-config.ts b/src/lib/actions/sandbox/mcp-bridge/deepagents-legacy-config.ts new file mode 100644 index 00000000000..248f4caa262 --- /dev/null +++ b/src/lib/actions/sandbox/mcp-bridge/deepagents-legacy-config.ts @@ -0,0 +1,48 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export const DEEPAGENTS_LEGACY_MCP_CONFIG_PATH = "/sandbox/.deepagents/.mcp.json"; + +export const DEEPAGENTS_LEGACY_CONFIG_HELPERS = [ + "LEGACY_MCP_MAX_BYTES = 262144", + "def legacy_fingerprint(metadata):", + " return (metadata.st_dev, metadata.st_ino, metadata.st_size, metadata.st_mtime_ns, metadata.st_ctime_ns, metadata.st_mode, metadata.st_nlink, metadata.st_uid)", + "def read_legacy_config(path):", + " flags = os.O_RDONLY | os.O_CLOEXEC | os.O_NONBLOCK | os.O_NOFOLLOW", + " descriptor = os.open(path, flags)", + " try:", + " before = os.fstat(descriptor)", + " linked = os.stat(path, follow_symlinks=False)", + " safe = (stat.S_ISREG(before.st_mode) and before.st_uid == os.getuid() and stat.S_IMODE(before.st_mode) == 0o600 and before.st_nlink == 1 and (before.st_dev, before.st_ino) == (linked.st_dev, linked.st_ino))", + " if not safe:", + " raise ValueError('legacy MCP config has unsafe ownership, mode, type, or links')", + " if before.st_size <= 0 or before.st_size > LEGACY_MCP_MAX_BYTES:", + " raise ValueError('legacy MCP config has invalid size')", + " chunks = []", + " remaining = before.st_size", + " while remaining:", + " chunk = os.read(descriptor, remaining)", + " if not chunk:", + " break", + " chunks.append(chunk)", + " remaining -= len(chunk)", + " after = os.fstat(descriptor)", + " linked_after = os.stat(path, follow_symlinks=False)", + " stable = (legacy_fingerprint(before) == legacy_fingerprint(after) and legacy_fingerprint(after) == legacy_fingerprint(linked_after))", + " if remaining or not stable:", + " raise ValueError('legacy MCP config changed while reading')", + " finally:", + " os.close(descriptor)", + " raw = b''.join(chunks).decode('utf-8')", + " data = strict_json_loads(raw)", + " return data, legacy_fingerprint(before)", + "def assert_legacy_source_stable(path, identity):", + " if identity is None:", + " if os.path.lexists(path):", + " raise ValueError('legacy MCP config appeared during mutation')", + " return", + " current = os.stat(path, follow_symlinks=False)", + " safe = (stat.S_ISREG(current.st_mode) and current.st_uid == os.getuid() and stat.S_IMODE(current.st_mode) == 0o600 and current.st_nlink == 1 and legacy_fingerprint(current) == identity)", + " if not safe:", + " raise ValueError('legacy MCP config changed before mutation')", +]; diff --git a/test/agents/deepagents/deepagents-mcp-legacy-lifecycle.test.ts b/test/agents/deepagents/deepagents-mcp-legacy-lifecycle.test.ts index 18e5404d017..f3ff7424759 100644 --- a/test/agents/deepagents/deepagents-mcp-legacy-lifecycle.test.ts +++ b/test/agents/deepagents/deepagents-mcp-legacy-lifecycle.test.ts @@ -199,13 +199,11 @@ beforeEach(() => { stderr: "", }; } - case command.includes("data = {'mcpServers': payload['expectedServers']}"): + case command.includes("NEMOCLAW_DEEPAGENTS_MCP_ROLLBACK_RESTORED=1"): adapterRegistered = true; return { status: 0, - stdout: command.includes("NEMOCLAW_DEEPAGENTS_MCP_ROLLBACK_RESTORED") - ? "NEMOCLAW_DEEPAGENTS_MCP_ROLLBACK_RESTORED=1\n" - : "", + stdout: "NEMOCLAW_DEEPAGENTS_MCP_ROLLBACK_RESTORED=1\n", stderr: "", }; case command.includes( @@ -231,7 +229,7 @@ beforeEach(() => { !isRevisionObservation && proof.includes('[ -z "${GITHUB_TOKEN+x}" ]'); return { status: isDetachedProof && attached ? 1 : 0, - stdout: attached ? "canonical" : "absent", + stdout: attached ? `v${String(providerResourceVersion)}` : "absent", stderr: "", }; }); @@ -408,6 +406,7 @@ describe("legacy Deep Agents managed MCP lifecycle", () => { providerExists: true, markerCalls: 0, }); + expect(adapterCalls.join("\n")).toContain("v2_GITHUB_TOKEN"); }); it("restores the old image when rebuild deletion aborts", async () => { @@ -425,5 +424,6 @@ describe("legacy Deep Agents managed MCP lifecycle", () => { providerExists: true, markerCalls: 0, }); + expect(adapterCalls.join("\n")).toContain("v2_GITHUB_TOKEN"); }); }); diff --git a/test/agents/deepagents/langchain-deepagents-code-managed-entrypoints.test.ts b/test/agents/deepagents/langchain-deepagents-code-managed-entrypoints.test.ts index 07f7bfb5b13..4b11e8f7dd8 100644 --- a/test/agents/deepagents/langchain-deepagents-code-managed-entrypoints.test.ts +++ b/test/agents/deepagents/langchain-deepagents-code-managed-entrypoints.test.ts @@ -131,6 +131,7 @@ describe("LangChain Deep Agents Code managed entrypoints", () => { expect(validator).toContain('"worker-broker_worker_task_context"'); expect(validator).toContain('"output_attestation"'); expect(validator).toContain('name="hanging"'); + expect(validator).toContain("openshell:resolve:env:v12_VALIDATION_MCP_TOKEN"); expect(validator).not.toContain("unittest.mock"); }); diff --git a/test/agents/hermes/hermes-mcp-credential-revision.test.ts b/test/agents/hermes/hermes-mcp-credential-revision.test.ts new file mode 100644 index 00000000000..b6f33f29561 --- /dev/null +++ b/test/agents/hermes/hermes-mcp-credential-revision.test.ts @@ -0,0 +1,79 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import path from "node:path"; + +import { describe, expect, it } from "vitest"; + +const TRANSACTION = path.resolve( + import.meta.dirname, + "../../..", + "agents/hermes/mcp-config-transaction.py", +); + +describe("Hermes MCP credential revision transaction", () => { + it("accepts only the exact bounded revision declared by the host (#10155)", () => { + const result = spawnSync( + "python3", + [ + "-c", + `import importlib.util, json, sys +spec = importlib.util.spec_from_file_location("mcp_tx", sys.argv[1]) +module = importlib.util.module_from_spec(spec) +sys.modules[spec.name] = module +spec.loader.exec_module(module) + +base = { + "server": "fake", + "url": "https://mcp.example.test/mcp", + "headers": {"Authorization": "Bearer openshell:resolve:env:v12_FAKE_TOKEN"}, + "replace_existing": True, + "credential_name": "FAKE_TOKEN", + "credential_revision": "v12", +} +canonical = { + "server": "fake", + "url": "https://mcp.example.test/mcp", + "headers": {"Authorization": "Bearer openshell:resolve:env:FAKE_TOKEN"}, + "replace_existing": True, +} +cases = { + "exact": base, + "canonical": canonical, + "missingRevision": {key: value for key, value in base.items() if key != "credential_revision"}, + "missingName": {key: value for key, value in base.items() if key != "credential_name"}, + "mismatchedRevision": {**base, "credential_revision": "v11"}, + "malformedRevision": {**base, "credential_revision": "v"}, + "overlongRevision": {**base, "credential_revision": "v" + "1" * 21}, + "wrongName": {**base, "headers": {"Authorization": "Bearer openshell:resolve:env:v12_OTHER_TOKEN"}}, + "metadataOnRemove": {**base, "force": False}, +} +cases["metadataOnRemove"].pop("replace_existing") +results = {} +for name, payload in cases.items(): + try: + module._validate_payload("remove" if name == "metadataOnRemove" else "add", payload) + results[name] = "accepted" + except ValueError: + results[name] = "rejected" +print(json.dumps(results))`, + TRANSACTION, + ], + { encoding: "utf8" }, + ); + + expect(result.status, result.stderr).toBe(0); + expect(JSON.parse(result.stdout)).toEqual({ + exact: "accepted", + canonical: "accepted", + missingRevision: "rejected", + missingName: "rejected", + mismatchedRevision: "rejected", + malformedRevision: "rejected", + overlongRevision: "rejected", + wrongName: "rejected", + metadataOnRemove: "rejected", + }); + }); +}); diff --git a/test/e2e/live/mcp-bridge-hermes-lifecycle.ts b/test/e2e/live/mcp-bridge-hermes-lifecycle.ts index 9bb12acb624..dcb09b3384e 100644 --- a/test/e2e/live/mcp-bridge-hermes-lifecycle.ts +++ b/test/e2e/live/mcp-bridge-hermes-lifecycle.ts @@ -33,13 +33,14 @@ export async function assertHermesConfig( const script = [ "set -eu", "/opt/hermes/.venv/bin/python - <<'PY'", - "import pathlib, yaml", + "import pathlib, re, yaml", "path = pathlib.Path('/sandbox/.hermes/config.yaml')", "text = path.read_text(encoding='utf-8')", "data = yaml.safe_load(text) or {}", `entry = data['mcp_servers'][${JSON.stringify(SERVER_NAME)}]`, `assert entry['url'] == ${JSON.stringify(mcpUrl)}`, - "assert entry['headers']['Authorization'] == 'Bearer openshell:resolve:env:FAKE_MCP_SECRET'", + "authorization = entry['headers']['Authorization']", + "assert re.fullmatch(r'Bearer openshell:resolve:env:v[0-9]{1,20}_FAKE_MCP_SECRET', authorization)", `assert ${JSON.stringify(HOST_SECRET)} not in text`, "PY", ].join("\n"); diff --git a/test/e2e/support/mcp-bridge-hermes-lifecycle.test.ts b/test/e2e/support/mcp-bridge-hermes-lifecycle.test.ts index 7dd433c85aa..136fc3c5875 100644 --- a/test/e2e/support/mcp-bridge-hermes-lifecycle.test.ts +++ b/test/e2e/support/mcp-bridge-hermes-lifecycle.test.ts @@ -10,6 +10,7 @@ import type { TrustedShellCommand, } from "../fixtures/shell-probe.ts"; import { + assertHermesConfig, assertHermesReloadRollback, lowerHermesShieldsForCleanup, reopenHermesMcpMaintenanceWindow, @@ -97,6 +98,19 @@ describe("Hermes MCP live rollback inspection", () => { }); }); +describe("Hermes MCP managed configuration assertion", () => { + it("requires the revision-scoped OpenShell credential placeholder (#10155)", async () => { + const runner = new RecordingRunner(); + const sandbox = new SandboxClient(runner); + + await assertHermesConfig(sandbox, "hermes-e2e", "https://mcp.example.test/mcp"); + + const command = runner.calls[0]?.args.at(-1) ?? ""; + expect(command).toContain("Bearer openshell:resolve:env:v[0-9]{1,20}_FAKE_MCP_SECRET"); + expect(command).not.toContain("== 'Bearer openshell:resolve:env:FAKE_MCP_SECRET'"); + }); +}); + describe("Hermes MCP post-rebuild maintenance", () => { it("opens a fresh Shields-down timer before the final config mutation", async () => { const runner = new RecordingRunner(); diff --git a/test/helpers/mcp-provider-revision.ts b/test/helpers/mcp-provider-revision.ts new file mode 100644 index 00000000000..b4dbbfbe7a6 --- /dev/null +++ b/test/helpers/mcp-provider-revision.ts @@ -0,0 +1,25 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +type ProviderWithCredentialRevision = { + credential: string; + resourceVersion?: number; +}; + +export function findObservedCredentialRevision( + proof: string, + attachedProviders: ReadonlySet, + providers: ReadonlyMap, +): string | null { + const credential = proof.includes("openshell:resolve:env:GITHUB_TOKEN") + ? "GITHUB_TOKEN" + : proof.includes("openshell:resolve:env:SLACK_TOKEN") + ? "SLACK_TOKEN" + : null; + if (credential === null) return null; + const providerName = [...attachedProviders].find( + (name) => providers.get(name)?.credential === credential, + ); + if (!providerName) return null; + return `v${String(providers.get(providerName)?.resourceVersion ?? 1)}`; +} diff --git a/test/helpers/vitest-watch-triggers.ts b/test/helpers/vitest-watch-triggers.ts index 091239a2c9b..9011be60530 100644 --- a/test/helpers/vitest-watch-triggers.ts +++ b/test/helpers/vitest-watch-triggers.ts @@ -161,9 +161,16 @@ export const vitestWatchTriggerPatterns: VitestWatchTriggerPattern[] = [ testsToRun: runTests("test/onboarding/effective-policy-contracts.test.ts"), }, { - pattern: /(?:^|\/)agents\/hermes\/(?:mcp-config-transaction|runtime-config-guard)\.py$/, + pattern: /(?:^|\/)agents\/hermes\/runtime-config-guard\.py$/, testsToRun: runTests("src/lib/actions/sandbox/gateway-restart-hermes-drift.test.ts"), }, + { + pattern: /(?:^|\/)agents\/hermes\/mcp-config-transaction\.py$/, + testsToRun: runTests( + "src/lib/actions/sandbox/gateway-restart-hermes-drift.test.ts", + "test/agents/hermes/hermes-mcp-credential-revision.test.ts", + ), + }, { pattern: /(?:^|\/)test\/e2e\/lib\/ci-compatible-inference\.sh$/, testsToRun: runTests("test/e2e/support/hosted-inference.test.ts"), diff --git a/test/mcp/mcp-destroy-lifecycle.test.ts b/test/mcp/mcp-destroy-lifecycle.test.ts index a905a42a90b..b90dee5c37b 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 { findObservedCredentialRevision } from "../helpers/mcp-provider-revision"; import { mockManagedEndpointlessProviderProfileRun } from "../helpers/onboard-script-mocks.cjs"; const testState = vi.hoisted(() => { @@ -347,16 +348,11 @@ beforeEach(() => { const encoded = command.match(/printf '%s' '([A-Za-z0-9+/=]+)' \| base64 -d/)?.[1] ?? ""; const proof = encoded ? Buffer.from(encoded, "base64").toString("utf8") : command; const isRevisionObservation = proof.includes("printf '%s\\n' absent"); - const observedCredential = proof.includes("openshell:resolve:env:GITHUB_TOKEN") - ? "GITHUB_TOKEN" - : proof.includes("openshell:resolve:env:SLACK_TOKEN") - ? "SLACK_TOKEN" - : null; - const credentialAttached = - observedCredential !== null && - [...testState.attachedProviders].some( - (providerName) => testState.providers.get(providerName)?.credential === observedCredential, - ); + const credentialRevision = findObservedCredentialRevision( + proof, + testState.attachedProviders, + testState.providers, + ); return { status: isNoopProbe || @@ -366,7 +362,7 @@ beforeEach(() => { proof.includes("openshell:resolve:env:SLACK_TOKEN") ? 0 : 1, - stdout: isRevisionObservation ? (credentialAttached ? "canonical" : "absent") : "", + stdout: isRevisionObservation ? (credentialRevision ?? "absent") : "", stderr: "", }; }); diff --git a/test/repository/vitest-watch-triggers.test.ts b/test/repository/vitest-watch-triggers.test.ts index 890cdad824d..22adc76792d 100644 --- a/test/repository/vitest-watch-triggers.test.ts +++ b/test/repository/vitest-watch-triggers.test.ts @@ -230,6 +230,7 @@ describe("Vitest opaque-input watch triggers", () => { ]); expect(triggeredBy("agents/hermes/mcp-config-transaction.py")).toEqual([ "src/lib/actions/sandbox/gateway-restart-hermes-drift.test.ts", + "test/agents/hermes/hermes-mcp-credential-revision.test.ts", ]); expect(triggeredBy("test/e2e/lib/ci-compatible-inference.sh")).toEqual([ "test/e2e/support/hosted-inference.test.ts",