From e97cfaaa66a22ef54d555a8b9acec7aeb552832f Mon Sep 17 00:00:00 2001 From: Senthil Ravichandran Date: Mon, 10 Aug 2026 10:42:41 -0700 Subject: [PATCH 1/3] test(e2e): preserve MCP discovery diagnostics Signed-off-by: Senthil Ravichandran --- test/e2e/live/mcp-bridge-tool-discovery.ts | 42 ++++++++++++- test/e2e/live/mcp-bridge-trusted-private.ts | 3 + test/e2e/live/mcp-bridge.test.ts | 6 ++ .../support/mcp-bridge-tool-discovery.test.ts | 61 +++++++++++++++++++ 4 files changed, 109 insertions(+), 3 deletions(-) diff --git a/test/e2e/live/mcp-bridge-tool-discovery.ts b/test/e2e/live/mcp-bridge-tool-discovery.ts index e6902947bd5..51d508f216f 100644 --- a/test/e2e/live/mcp-bridge-tool-discovery.ts +++ b/test/e2e/live/mcp-bridge-tool-discovery.ts @@ -3,6 +3,7 @@ import { expect } from "vitest"; +import type { ArtifactSink } from "../fixtures/artifacts.ts"; import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts"; import { assertExitZero } from "../fixtures/clients/command.ts"; import type { HostCliClient } from "../fixtures/clients/host.ts"; @@ -39,7 +40,10 @@ export function shouldRetryMcpDiscoveryAfterRestart( } type McpToolDiscoveryStatusJson = { - provider: { credentialResolution?: unknown }; + provider: Record & { credentialResolution?: unknown }; + policy?: unknown; + adapter?: unknown; + trustedPrivateTarget?: unknown; toolDiscovery: { ok: boolean; count: number; @@ -49,6 +53,33 @@ type McpToolDiscoveryStatusJson = { }; }; +function buildMcpToolDiscoveryDiagnostics( + status: McpToolDiscoveryStatusJson, + requests: readonly FakeMcpRequest[], + expectedSecret: string, +): Record { + return { + provider: status.provider, + policy: status.policy, + adapter: status.adapter, + trustedPrivateTarget: status.trustedPrivateTarget, + toolDiscovery: status.toolDiscovery, + requests: requests.map((request) => ({ + httpMethod: request.method, + rpcMethod: request.rpcMethod ?? null, + responseStatus: request.responseStatus ?? null, + responseHasResult: request.responseHasResult ?? null, + sessionMetadataPresent: { + sessionId: Boolean(request.sessionId), + protocolVersion: Boolean(request.protocolVersion), + negotiatedSessionId: Boolean(request.negotiatedSessionId), + negotiatedProtocolVersion: Boolean(request.negotiatedProtocolVersion), + }, + credentialRewriteMatched: request.auth === `Bearer ${expectedSecret}`, + })), + }; +} + export async function assertAuthenticatedMcpRediscovery( target: AuthenticatedMcpDiscoveryTarget | undefined, requestOffset: number | undefined, @@ -174,6 +205,7 @@ export async function assertAuthenticatedMcpToolDiscovery( host: HostCliClient, fakeMcp: FakeMcpHttpsServer, options: { + artifacts: Pick; sandboxName: string; artifactPrefix: string; credentialKey?: string; @@ -202,7 +234,6 @@ export async function assertAuthenticatedMcpToolDiscovery( ); assertExitZero(status, `${options.artifactPrefix} mcp status --tools --json`); statusJson = JSON.parse(status.stdout) as McpToolDiscoveryStatusJson; - expect(statusJson.provider.credentialResolution).toBeUndefined(); if ( !shouldRetryMcpToolDiscoveryTransportFailure( statusJson.toolDiscovery, @@ -218,6 +249,12 @@ export async function assertAuthenticatedMcpToolDiscovery( await new Promise((resolve) => setTimeout(resolve, MCP_TOOL_DISCOVERY_RETRY_DELAY_MS)); } if (!status || !statusJson) throw new Error("MCP tool discovery did not run"); + const discoveryRequests = fakeMcp.requests.slice(requestOffset); + await options.artifacts.writeJson( + `${options.artifactPrefix}-mcp-tool-discovery-diagnostics.json`, + buildMcpToolDiscoveryDiagnostics(statusJson, discoveryRequests, options.hostSecret), + ); + expect(statusJson.provider.credentialResolution).toBeUndefined(); expect(statusJson.toolDiscovery).toMatchObject({ ok: true, count: 2, @@ -225,7 +262,6 @@ export async function assertAuthenticatedMcpToolDiscovery( truncated: false, }); expect(status.stdout).not.toContain(options.hostSecret); - const discoveryRequests = fakeMcp.requests.slice(requestOffset); const discoveryProtocolRequests = discoveryRequests.filter( (request) => (request.method === "POST" || request.method === "DELETE") && request.path === "/mcp", diff --git a/test/e2e/live/mcp-bridge-trusted-private.ts b/test/e2e/live/mcp-bridge-trusted-private.ts index 15787880593..9f248992a09 100644 --- a/test/e2e/live/mcp-bridge-trusted-private.ts +++ b/test/e2e/live/mcp-bridge-trusted-private.ts @@ -1,6 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import type { ArtifactSink } from "../fixtures/artifacts.ts"; import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts"; import type { CleanupRegistry } from "../fixtures/cleanup.ts"; import { assertExitZero as expectExitZero } from "../fixtures/clients/command.ts"; @@ -36,6 +37,7 @@ export async function assertTrustedPrivateMcpRebindingDenied( cleanup: CleanupRegistry, options: { adapter: McpDnsRebindingAdapter; + artifacts: Pick; artifactPrefix: string; assertSecretAbsent: ( sandbox: SandboxClient, @@ -164,6 +166,7 @@ export async function assertTrustedPrivateMcpRebindingDenied( `${options.artifactPrefix}-dns-rebinding-secret-absent-from-sandbox`, ); await assertAuthenticatedMcpToolDiscovery(host, rebindMcp, { + artifacts: options.artifacts, sandboxName: options.sandboxName, artifactPrefix: `${options.artifactPrefix}-trusted-private`, credentialKey: REBIND_CREDENTIAL_KEY, diff --git a/test/e2e/live/mcp-bridge.test.ts b/test/e2e/live/mcp-bridge.test.ts index 87afa2d0b30..ae4cb88c542 100644 --- a/test/e2e/live/mcp-bridge.test.ts +++ b/test/e2e/live/mcp-bridge.test.ts @@ -779,6 +779,7 @@ test("mcp-bridge", { artifactPrefix: "openclaw", }); await assertAuthenticatedMcpToolDiscovery(host, fakeMcp, { + artifacts, sandboxName: OPENCLAW_SANDBOX_NAME, artifactPrefix: "openclaw", hostSecret: HOST_SECRET, @@ -848,6 +849,7 @@ test("mcp-bridge", { await assertTrustedPrivateMcpRebindingDenied(host, sandbox, cleanup, { adapter: "mcporter", + artifacts, artifactPrefix: "openclaw", assertSecretAbsent: assertSecretAbsentFromSandbox, cleanupBridge: cleanupMcpBridge, @@ -1096,6 +1098,7 @@ mcpBridgeShardTest("hermes")( }, }); await assertAuthenticatedMcpToolDiscovery(host, fakeMcp, { + artifacts, sandboxName: HERMES_SANDBOX_NAME, artifactPrefix: "hermes", hostSecret: HOST_SECRET, @@ -1134,6 +1137,7 @@ mcpBridgeShardTest("hermes")( }; await assertTrustedPrivateMcpRebindingDenied(host, sandbox, cleanup, { adapter: "hermes-config", + artifacts, artifactPrefix: "hermes", assertSecretAbsent: assertSecretAbsentFromSandbox, cleanupBridge: cleanupMcpBridge, @@ -1321,6 +1325,7 @@ mcpBridgeShardTest("deepagents")( artifactPrefix: "deepagents", }); await assertAuthenticatedMcpToolDiscovery(host, fakeMcp, { + artifacts, sandboxName: DEEPAGENTS_SANDBOX_NAME, artifactPrefix: "deepagents", hostSecret: HOST_SECRET, @@ -1336,6 +1341,7 @@ mcpBridgeShardTest("deepagents")( await assertSecretAbsentFromSandbox(sandbox, DEEPAGENTS_SANDBOX_NAME, ["/sandbox/.deepagents"]); await assertTrustedPrivateMcpRebindingDenied(host, sandbox, cleanup, { adapter: "deepagents-config", + artifacts, artifactPrefix: "deepagents", assertSecretAbsent: assertSecretAbsentFromSandbox, cleanupBridge: cleanupMcpBridge, diff --git a/test/e2e/support/mcp-bridge-tool-discovery.test.ts b/test/e2e/support/mcp-bridge-tool-discovery.test.ts index 45818a6eebc..e76a4bd79c9 100644 --- a/test/e2e/support/mcp-bridge-tool-discovery.test.ts +++ b/test/e2e/support/mcp-bridge-tool-discovery.test.ts @@ -12,6 +12,7 @@ import { } from "../live/mcp-bridge-servers.ts"; import { assertAuthenticatedMcpDiscoveryWithOneRestart, + assertAuthenticatedMcpToolDiscovery, hasSuccessfulAuthenticatedMcpDiscovery, shouldRetryMcpDiscoveryAfterRestart, shouldRetryMcpToolDiscoveryTransportFailure, @@ -185,6 +186,66 @@ describe("authenticated MCP rediscovery evidence", () => { }); describe("authenticated MCP tool discovery transport retry", () => { + it("writes redacted boundary diagnostics before a discovery failure (#8746)", async () => { + const statusJson = { + provider: { attached: true, credentialReady: true }, + policy: { gatewayPresent: true }, + adapter: { registered: true }, + trustedPrivateTarget: { state: "match" }, + toolDiscovery: { + ok: false, + count: 0, + tools: [], + truncated: false, + detail: "MCP tool discovery request failed", + }, + }; + const fakeMcp = { requests: [] } as unknown as FakeMcpHttpsServer; + const host = { + nemoclaw: vi.fn(async () => { + fakeMcp.requests.push(successfulInitialize()); + return { exitCode: 0, stdout: JSON.stringify(statusJson), stderr: "" }; + }), + } as unknown as Parameters[0]; + const artifacts = { writeJson: vi.fn().mockResolvedValue("diagnostics.json") }; + + await expect( + assertAuthenticatedMcpToolDiscovery(host, fakeMcp, { + artifacts, + sandboxName: "sandbox", + artifactPrefix: "openclaw-trusted-private", + hostSecret: EXPECTED_SECRET, + progress: { event: vi.fn() }, + }), + ).rejects.toThrow(); + + expect(artifacts.writeJson).toHaveBeenCalledWith( + "openclaw-trusted-private-mcp-tool-discovery-diagnostics.json", + { + ...statusJson, + requests: [ + { + httpMethod: "POST", + rpcMethod: "initialize", + responseStatus: 200, + responseHasResult: true, + sessionMetadataPresent: { + sessionId: false, + protocolVersion: false, + negotiatedSessionId: true, + negotiatedProtocolVersion: true, + }, + credentialRewriteMatched: true, + }, + ], + }, + ); + const diagnostics = JSON.stringify(artifacts.writeJson.mock.calls[0]?.[1]); + expect(diagnostics).not.toContain(EXPECTED_SECRET); + expect(diagnostics).not.toContain(SESSION_ID); + expect(diagnostics).not.toContain(PROTOCOL_VERSION); + }); + it("retries one generic transport failure before any request reaches the fixture", () => { expect( shouldRetryMcpToolDiscoveryTransportFailure( From aa148a038329c799d565df6804cce1e436b13a68 Mon Sep 17 00:00:00 2001 From: Senthil Ravichandran Date: Mon, 10 Aug 2026 11:26:22 -0700 Subject: [PATCH 2/3] fix(e2e): sanitize MCP discovery diagnostics Signed-off-by: Senthil Ravichandran --- test/e2e/live/mcp-bridge-tool-discovery.ts | 58 ++++++++++-- .../support/mcp-bridge-tool-discovery.test.ts | 91 ++++++++++++++----- 2 files changed, 114 insertions(+), 35 deletions(-) diff --git a/test/e2e/live/mcp-bridge-tool-discovery.ts b/test/e2e/live/mcp-bridge-tool-discovery.ts index 51d508f216f..f0e2db0b76e 100644 --- a/test/e2e/live/mcp-bridge-tool-discovery.ts +++ b/test/e2e/live/mcp-bridge-tool-discovery.ts @@ -40,11 +40,26 @@ export function shouldRetryMcpDiscoveryAfterRestart( } type McpToolDiscoveryStatusJson = { - provider: Record & { credentialResolution?: unknown }; - policy?: unknown; - adapter?: unknown; - trustedPrivateTarget?: unknown; - toolDiscovery: { + provider: Record & { + registryPresent: boolean; + gatewayPresent: boolean | null; + attached: boolean | null; + credentialReady: boolean | null; + credentialResolution?: unknown; + }; + policy: Record & { + registryPresent: boolean; + gatewayPresent: boolean | null; + }; + adapter: Record & { + registered: boolean | null; + detail?: unknown; + }; + trustedPrivateTarget?: Record & { + state: "match" | "drift" | "unresolved"; + detail?: unknown; + }; + toolDiscovery: Record & { ok: boolean; count: number; tools: string[]; @@ -59,11 +74,34 @@ function buildMcpToolDiscoveryDiagnostics( expectedSecret: string, ): Record { return { - provider: status.provider, - policy: status.policy, - adapter: status.adapter, - trustedPrivateTarget: status.trustedPrivateTarget, - toolDiscovery: status.toolDiscovery, + provider: { + registryPresent: status.provider.registryPresent, + gatewayPresent: status.provider.gatewayPresent, + attached: status.provider.attached, + credentialReady: status.provider.credentialReady, + credentialResolutionPresent: status.provider.credentialResolution !== undefined, + }, + policy: { + registryPresent: status.policy.registryPresent, + gatewayPresent: status.policy.gatewayPresent, + }, + adapter: { + registered: status.adapter.registered, + detailPresent: status.adapter.detail !== undefined, + }, + trustedPrivateTarget: status.trustedPrivateTarget + ? { + state: status.trustedPrivateTarget.state, + detailPresent: status.trustedPrivateTarget.detail !== undefined, + } + : null, + toolDiscovery: { + ok: status.toolDiscovery.ok, + count: status.toolDiscovery.count, + tools: [...status.toolDiscovery.tools], + truncated: status.toolDiscovery.truncated, + ...(status.toolDiscovery.detail !== undefined ? { detail: status.toolDiscovery.detail } : {}), + }, requests: requests.map((request) => ({ httpMethod: request.method, rpcMethod: request.rpcMethod ?? null, diff --git a/test/e2e/support/mcp-bridge-tool-discovery.test.ts b/test/e2e/support/mcp-bridge-tool-discovery.test.ts index e76a4bd79c9..a32fd8f6139 100644 --- a/test/e2e/support/mcp-bridge-tool-discovery.test.ts +++ b/test/e2e/support/mcp-bridge-tool-discovery.test.ts @@ -1,7 +1,12 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; + import { afterEach, describe, expect, it, vi } from "vitest"; +import { ArtifactSink } from "../fixtures/artifacts.ts"; import { shouldRetryMcpMutationAfterConcurrencyConflict } from "../live/mcp-bridge-cleanup.ts"; import { type FakeMcpHttpsServer, @@ -22,6 +27,7 @@ const EXPECTED_SECRET = "expected-secret"; const EXPECTED_RESULT_TOKEN = "expected-result"; const SESSION_ID = "fake-session-1"; const PROTOCOL_VERSION = "2025-03-26"; +const STATUS_SECRET = "unregistered-sensitive-status-value"; function request(rpcMethod: string, overrides: Partial = {}): FakeMcpRequest { return { @@ -69,10 +75,13 @@ const BRIDGE_TOOLS = ["tool_search", "tool_describe", "tool_call"].map((name) => })); let compatibleMock: StartedHttpServer | undefined; +let artifactRoot: string | undefined; afterEach(async () => { await compatibleMock?.close(); compatibleMock = undefined; + if (artifactRoot) await fs.rm(artifactRoot, { recursive: true, force: true }); + artifactRoot = undefined; }); async function startDeferredCompatibleMock(): Promise { @@ -188,16 +197,29 @@ describe("authenticated MCP rediscovery evidence", () => { describe("authenticated MCP tool discovery transport retry", () => { it("writes redacted boundary diagnostics before a discovery failure (#8746)", async () => { const statusJson = { - provider: { attached: true, credentialReady: true }, - policy: { gatewayPresent: true }, - adapter: { registered: true }, - trustedPrivateTarget: { state: "match" }, + provider: { + registryPresent: true, + gatewayPresent: true, + attached: true, + credentialReady: true, + credentialResolution: { detail: STATUS_SECRET }, + token: STATUS_SECRET, + }, + policy: { registryPresent: true, gatewayPresent: true, token: STATUS_SECRET }, + adapter: { registered: true, detail: STATUS_SECRET, sessionId: STATUS_SECRET }, + trustedPrivateTarget: { + state: "match" as const, + host: STATUS_SECRET, + recordedPins: [STATUS_SECRET], + detail: STATUS_SECRET, + }, toolDiscovery: { ok: false, count: 0, tools: [], truncated: false, detail: "MCP tool discovery request failed", + credential: STATUS_SECRET, }, }; const fakeMcp = { requests: [] } as unknown as FakeMcpHttpsServer; @@ -207,7 +229,8 @@ describe("authenticated MCP tool discovery transport retry", () => { return { exitCode: 0, stdout: JSON.stringify(statusJson), stderr: "" }; }), } as unknown as Parameters[0]; - const artifacts = { writeJson: vi.fn().mockResolvedValue("diagnostics.json") }; + artifactRoot = await fs.mkdtemp(path.join(os.tmpdir(), "nemoclaw-mcp-diagnostics-")); + const artifacts = new ArtifactSink(artifactRoot); await expect( assertAuthenticatedMcpToolDiscovery(host, fakeMcp, { @@ -219,28 +242,46 @@ describe("authenticated MCP tool discovery transport retry", () => { }), ).rejects.toThrow(); - expect(artifacts.writeJson).toHaveBeenCalledWith( + const artifactPath = path.join( + artifactRoot, "openclaw-trusted-private-mcp-tool-discovery-diagnostics.json", - { - ...statusJson, - requests: [ - { - httpMethod: "POST", - rpcMethod: "initialize", - responseStatus: 200, - responseHasResult: true, - sessionMetadataPresent: { - sessionId: false, - protocolVersion: false, - negotiatedSessionId: true, - negotiatedProtocolVersion: true, - }, - credentialRewriteMatched: true, - }, - ], - }, ); - const diagnostics = JSON.stringify(artifacts.writeJson.mock.calls[0]?.[1]); + const diagnostics = await fs.readFile(artifactPath, "utf8"); + expect(JSON.parse(diagnostics)).toEqual({ + provider: { + registryPresent: true, + gatewayPresent: true, + attached: true, + credentialReady: true, + credentialResolutionPresent: true, + }, + policy: { registryPresent: true, gatewayPresent: true }, + adapter: { registered: true, detailPresent: true }, + trustedPrivateTarget: { state: "match", detailPresent: true }, + toolDiscovery: { + ok: false, + count: 0, + tools: [], + truncated: false, + detail: "MCP tool discovery request failed", + }, + requests: [ + { + httpMethod: "POST", + rpcMethod: "initialize", + responseStatus: 200, + responseHasResult: true, + sessionMetadataPresent: { + sessionId: false, + protocolVersion: false, + negotiatedSessionId: true, + negotiatedProtocolVersion: true, + }, + credentialRewriteMatched: true, + }, + ], + }); + expect(diagnostics).not.toContain(STATUS_SECRET); expect(diagnostics).not.toContain(EXPECTED_SECRET); expect(diagnostics).not.toContain(SESSION_ID); expect(diagnostics).not.toContain(PROTOCOL_VERSION); From 8c39ccdb52d2b22dfc4d6a76b262aaf29b1702f4 Mon Sep 17 00:00:00 2001 From: Senthil Ravichandran Date: Mon, 10 Aug 2026 11:29:48 -0700 Subject: [PATCH 3/3] test(e2e): keep diagnostic cleanup linear Signed-off-by: Senthil Ravichandran --- test/e2e/support/mcp-bridge-tool-discovery.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/e2e/support/mcp-bridge-tool-discovery.test.ts b/test/e2e/support/mcp-bridge-tool-discovery.test.ts index a32fd8f6139..72d328dd6de 100644 --- a/test/e2e/support/mcp-bridge-tool-discovery.test.ts +++ b/test/e2e/support/mcp-bridge-tool-discovery.test.ts @@ -75,13 +75,14 @@ const BRIDGE_TOOLS = ["tool_search", "tool_describe", "tool_call"].map((name) => })); let compatibleMock: StartedHttpServer | undefined; -let artifactRoot: string | undefined; +const artifactRoots: string[] = []; afterEach(async () => { await compatibleMock?.close(); compatibleMock = undefined; - if (artifactRoot) await fs.rm(artifactRoot, { recursive: true, force: true }); - artifactRoot = undefined; + await Promise.all( + artifactRoots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true })), + ); }); async function startDeferredCompatibleMock(): Promise { @@ -229,7 +230,8 @@ describe("authenticated MCP tool discovery transport retry", () => { return { exitCode: 0, stdout: JSON.stringify(statusJson), stderr: "" }; }), } as unknown as Parameters[0]; - artifactRoot = await fs.mkdtemp(path.join(os.tmpdir(), "nemoclaw-mcp-diagnostics-")); + const artifactRoot = await fs.mkdtemp(path.join(os.tmpdir(), "nemoclaw-mcp-diagnostics-")); + artifactRoots.push(artifactRoot); const artifacts = new ArtifactSink(artifactRoot); await expect(