diff --git a/docs/deployment/set-up-mcp-bridge.mdx b/docs/deployment/set-up-mcp-bridge.mdx index a977379b18f..e3dfb732d1b 100644 --- a/docs/deployment/set-up-mcp-bridge.mdx +++ b/docs/deployment/set-up-mcp-bridge.mdx @@ -207,6 +207,20 @@ The `env.missing` field is an array of recorded host variable names that are cur An existing valid provider can remain ready when that host variable is unset because OpenShell retains the credential. The JSON value `support.mode: "bridge"` identifies the agent's config-adapter capability, not a host-side traffic bridge. +Provider presence, attachment, and credential-key checks read OpenShell metadata only; they cannot prove that OpenShell actually rewrites the `openshell:resolve:env:KEY` placeholder when a request leaves the sandbox. +`mcp status ` therefore requests a differential wire-level credential-resolution probe by default. +Before sending probe traffic, NemoClaw verifies that the exact generated `protocol: mcp` policy matches the effective gateway policy, the expected provider is attached, and the live provider has the recorded ID, generic type, a valid resource version, and exactly one credential key matching the recorded key; otherwise it reports `ok: null` with a `probe skipped` detail and sends no request. +When those readiness checks pass, the probe sends the same idempotent MCP `initialize` request twice from inside the sandbox, wrapped in the adapter runtime so the generated policy applies — once with the placeholder authorization header exactly as agent traffic carries it, and once with a deliberately-unresolvable literal control bearer that no gateway rewrite can touch. +A working rewrite makes the two requests reach the endpoint with different bearers; a dead rewrite forwards both literally. +Only a placeholder HTTP 2xx paired with a rejected control verifies resolution — an accepted request proves a valid credential was on the wire. +Every non-2xx placeholder outcome is inconclusive with the hypotheses named: identical rejections cannot separate "placeholder forwarded verbatim" from "resolved but expired or revoked credential" (or, for HTTP 400, endpoint request validation), and differing rejections cannot either, because an endpoint may reject two different literal bearer strings differently. +For identical HTTP 400, 401, or 403 rejections, the warning tells you to verify the stored credential first; for HTTP 401 or 403, a confirmed-valid credential means the host is not rewriting placeholders, while HTTP 400 remains inconclusive because the endpoint may reject the probe request itself. +The verdict appears as `provider.credentialResolution` in JSON (`ok` true or null with a `detail`; `httpStatus` and `controlHttpStatus` are present only when the relevant requests ran) and as a `credential resolution:` line in text output. +The probe never captures or prints endpoint response bodies — classification uses HTTP status and curl exit codes only — and it refuses to run against a persisted URL that no longer satisfies the current authenticated-endpoint boundary, so a legacy, private-alias, or plain-HTTP URL is never sent a header the gateway could rewrite into a live credential. +After `mcp add` commits, NemoClaw performs a fresh status inspection and either runs the same gated probe once or reports an inconclusive readiness skip, without failing the add. +Pass `--no-probe` to skip the probe, or `--probe` to request it for every server in the multi-server status form; readiness failures still skip traffic, and the bare `mcp list` and `mcp status` forms never probe so they stay fast. +Endpoints that accept both probes without enforcing authentication, differing non-auth rejections, endpoint outages, policy denials, timeouts, and unreachable sandboxes all report `ok: null` with the evidence in `detail`, so an endpoint problem is never blamed on the host's credential rewrite. + ### Rotate a Credential Export the replacement value under the same host environment name used by `mcp add`, then restart that managed server: @@ -259,6 +273,12 @@ For immediate revocation, revoke the upstream credential first, then run `$$nemo ## Troubleshooting +If `mcp status ` warns that credential resolution could not be verified because the placeholder probe and the unresolvable control probe were rejected identically, first confirm the stored credential is valid (rotate it with `mcp restart` if in doubt). +For an identical HTTP 401 or 403, a confirmed-valid credential settles it: the OpenShell gateway on this host is not rewriting the `openshell:resolve:env:KEY` placeholder on egress, and every agent request to that MCP server receives the same authentication failure even while provider presence, attachment, credential readiness, and adapter registration all report healthy. +An identical HTTP 400 stays inconclusive even with a valid credential, because the endpoint may reject the probe's `initialize` request itself; compare `mcp status` for the same server on a known-good host — if that host verifies, suspect this host's placeholder rewrite. +That is a host-side OpenShell defect rather than a NemoClaw registration problem: verify the OpenShell installation on the host (tracked upstream as OpenShell issue 2161). +A `credential resolution: unknown` verdict with an endpoint or policy detail means the probe could not reach a judgment; fix the reported endpoint or policy condition and rerun `mcp status `. + If `restart` reports a missing provider and the original credential is not registered in OpenShell, export the same variable name used during `add` and retry. If `status` reports an incomplete add transaction, rerun the original `mcp add` command with the same URL and environment-variable name. diff --git a/docs/reference/commands-nemohermes.mdx b/docs/reference/commands-nemohermes.mdx index deebd6cf86a..ce58d85f1ec 100644 --- a/docs/reference/commands-nemohermes.mdx +++ b/docs/reference/commands-nemohermes.mdx @@ -1256,6 +1256,9 @@ NemoClaw generates a narrow `protocol: mcp` policy for the destination, literal OpenShell `0.0.72` evaluates that policy before replacing the attached provider placeholder in the allowed request header. Static provider placeholders are sandbox-scoped rather than endpoint-exclusive, so do not grant broader inspected-HTTP routes to the same adapter runtime and credential key. The sandbox client connects directly through OpenShell's existing egress path, and NemoClaw does not run a host-side MCP data-plane bridge, proxy, relay, or listener. +After the add commits, NemoClaw freshly verifies the exact generated policy, expected provider attachment, recorded provider ID, generic type, valid resource version, and exactly one credential key matching the recorded key. +If those readiness checks pass, it sends a differential pair of wire-level MCP `initialize` requests from inside the sandbox — one with the placeholder header and one with an unresolvable control bearer — to verify that OpenShell resolves the credential on egress; otherwise it reports an inconclusive `probe skipped` result and sends no request. +Neither outcome fails the committed add, and `--no-probe` skips this check. For full setup details, see [Set Up MCP Servers](../manage-sandboxes/set-up-mcp-servers). Hermes MCP add, restart, and remove mutate managed config and are refused while shields are up. @@ -1274,14 +1277,24 @@ unset GITHUB_MCP_TOKEN Inspect MCP server state for one server or for all configured servers. Status includes OpenShell provider presence and credential-key shape, provider attachment, generated policy content match, adapter registration, current host-variable availability, and the selected agent's MCP support mode. While a managed provider is attached, text and JSON status warn that its credential is sandbox-scoped until OpenShell supports endpoint-exclusive binding plus Host, scheme, and query enforcement. +When a single server is named, status requests a differential wire-level credential-resolution probe. +It sends no probe traffic unless the exact generated policy matches the effective gateway policy, the expected provider attachment is confirmed, and the live provider has the recorded ID, generic type, a valid resource version, and exactly one credential key matching the recorded key; a readiness failure reports `unknown` with a `probe skipped` detail. +When ready, the same MCP `initialize` is sent from inside the sandbox once with the `openshell:resolve:env:KEY` placeholder header and once with a deliberately-unresolvable control bearer. +Classification uses the two HTTP status codes plus curl exit codes for transport, timeout, and policy-denial outcomes; response bodies are never captured or printed. +A `verified` verdict requires the placeholder request to be accepted (HTTP 2xx) while the control is rejected — the only outcome that proves a valid credential was on the wire. +Identical HTTP 400, 401, or 403 rejections raise a warning that names the hypotheses — the placeholder forwarded verbatim, an expired or revoked credential that resolved correctly, or (for HTTP 400) endpoint request validation — and tells you to verify the stored credential first. +For HTTP 401 or 403, a confirmed-valid credential means the host is not rewriting placeholders and agent runtimes receive the same auth failure and skip the server; HTTP 400 remains inconclusive because the endpoint may reject the probe request itself. +Every other outcome — differing rejections (an endpoint may reject two different literal bearers differently), endpoints that skip authentication, endpoint outages, policy denials, and unreachable sandboxes — reports as `unknown` rather than blaming the credential rewrite, and a persisted URL that fails the current authenticated-endpoint boundary is never probed. ```bash -nemohermes my-assistant mcp status [server] [--json] +nemohermes my-assistant mcp status [server] [--json] [--probe|--no-probe] ``` | Flag | Description | |------|-------------| | `--json` | Emit status as JSON without credential values | +| `--probe` | Request the wire-level credential-resolution probe for every listed server; entries that fail readiness checks are skipped | +| `--no-probe` | Skip the probe; it defaults on only when a single server is named | ### `nemohermes mcp restart` diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index d6356c5816c..a3808e859cc 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -1574,6 +1574,9 @@ NemoClaw generates a narrow `protocol: mcp` policy for the destination, literal OpenShell `0.0.72` evaluates that policy before replacing the attached provider placeholder in the allowed request header. Static provider placeholders are sandbox-scoped rather than endpoint-exclusive, so do not grant broader inspected-HTTP routes to the same adapter runtime and credential key. The sandbox client connects directly through OpenShell's existing egress path, and NemoClaw does not run a host-side MCP data-plane bridge, proxy, relay, or listener. +After the add commits, NemoClaw freshly verifies the exact generated policy, expected provider attachment, recorded provider ID, generic type, valid resource version, and exactly one credential key matching the recorded key. +If those readiness checks pass, it sends a differential pair of wire-level MCP `initialize` requests from inside the sandbox — one with the placeholder header and one with an unresolvable control bearer — to verify that OpenShell resolves the credential on egress; otherwise it reports an inconclusive `probe skipped` result and sends no request. +Neither outcome fails the committed add, and `--no-probe` skips this check. For full setup details, see [Set Up MCP Servers](../manage-sandboxes/set-up-mcp-servers). @@ -1596,14 +1599,24 @@ unset GITHUB_MCP_TOKEN Inspect MCP server state for one server or for all configured servers. Status includes OpenShell provider presence and credential-key shape, provider attachment, generated policy content match, adapter registration, current host-variable availability, and the selected agent's MCP support mode. While a managed provider is attached, text and JSON status warn that its credential is sandbox-scoped until OpenShell supports endpoint-exclusive binding plus Host, scheme, and query enforcement. +When a single server is named, status requests a differential wire-level credential-resolution probe. +It sends no probe traffic unless the exact generated policy matches the effective gateway policy, the expected provider attachment is confirmed, and the live provider has the recorded ID, generic type, a valid resource version, and exactly one credential key matching the recorded key; a readiness failure reports `unknown` with a `probe skipped` detail. +When ready, the same MCP `initialize` is sent from inside the sandbox once with the `openshell:resolve:env:KEY` placeholder header and once with a deliberately-unresolvable control bearer. +Classification uses the two HTTP status codes plus curl exit codes for transport, timeout, and policy-denial outcomes; response bodies are never captured or printed. +A `verified` verdict requires the placeholder request to be accepted (HTTP 2xx) while the control is rejected — the only outcome that proves a valid credential was on the wire. +Identical HTTP 400, 401, or 403 rejections raise a warning that names the hypotheses — the placeholder forwarded verbatim, an expired or revoked credential that resolved correctly, or (for HTTP 400) endpoint request validation — and tells you to verify the stored credential first. +For HTTP 401 or 403, a confirmed-valid credential means the host is not rewriting placeholders and agent runtimes receive the same auth failure and skip the server; HTTP 400 remains inconclusive because the endpoint may reject the probe request itself. +Every other outcome — differing rejections (an endpoint may reject two different literal bearers differently), endpoints that skip authentication, endpoint outages, policy denials, and unreachable sandboxes — reports as `unknown` rather than blaming the credential rewrite, and a persisted URL that fails the current authenticated-endpoint boundary is never probed. ```bash -$$nemoclaw my-assistant mcp status [server] [--json] +$$nemoclaw my-assistant mcp status [server] [--json] [--probe|--no-probe] ``` | Flag | Description | |------|-------------| | `--json` | Emit status as JSON without credential values | +| `--probe` | Request the wire-level credential-resolution probe for every listed server; entries that fail readiness checks are skipped | +| `--no-probe` | Skip the probe; it defaults on only when a single server is named | ### `$$nemoclaw mcp restart` diff --git a/src/lib/actions/sandbox/mcp-bridge-contracts.ts b/src/lib/actions/sandbox/mcp-bridge-contracts.ts index f4ad498cda1..01d38063b2e 100644 --- a/src/lib/actions/sandbox/mcp-bridge-contracts.ts +++ b/src/lib/actions/sandbox/mcp-bridge-contracts.ts @@ -50,6 +50,17 @@ export interface McpBridgeStatus { attached: boolean | null; credentialReady: boolean | null; detail?: string; + /** + * Wire-level placeholder-resolution probe outcome (#6379). Present only + * when the probe was requested for this entry; `ok: null` with a detail + * means the probe ran or was skipped without a verdict. + */ + credentialResolution?: { + ok: boolean | null; + httpStatus?: number; + controlHttpStatus?: number; + detail?: string; + }; }; policy: { name?: string; diff --git a/src/lib/actions/sandbox/mcp-bridge-input-runtime.test.ts b/src/lib/actions/sandbox/mcp-bridge-input-runtime.test.ts index cd2813feaeb..b3be6aa02f0 100644 --- a/src/lib/actions/sandbox/mcp-bridge-input-runtime.test.ts +++ b/src/lib/actions/sandbox/mcp-bridge-input-runtime.test.ts @@ -98,6 +98,30 @@ describe("MCP input runtime boundaries", () => { } }); + it("rejects the redundant undocumented --probe flag for add (#6379)", async () => { + const priorExitCode = process.exitCode; + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + process.exitCode = undefined; + await dispatchMcpBridgeCommand("missing-sandbox", [ + "add", + "github", + "--url", + "https://mcp.example.test/mcp", + "--env", + "GITHUB_TOKEN", + "--probe", + ]); + expect(process.exitCode).toBe(2); + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining("Usage: nemoclaw mcp add"), + ); + } finally { + errorSpy.mockRestore(); + process.exitCode = priorExitCode; + } + }); + it("documents force cleanup without promising residual registry removal", async () => { const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); try { diff --git a/src/lib/actions/sandbox/mcp-bridge-render.ts b/src/lib/actions/sandbox/mcp-bridge-render.ts index e7bc2d11b02..b565b2e80e8 100644 --- a/src/lib/actions/sandbox/mcp-bridge-render.ts +++ b/src/lib/actions/sandbox/mcp-bridge-render.ts @@ -78,6 +78,18 @@ export function renderMcpBridgeStatus( console.log( ` env: ${status.env.ready ? "ready" : status.env.missing.length > 0 ? `missing ${status.env.missing.join(", ")}` : "not ready"}`, ); + const resolution = status.provider.credentialResolution; + if (resolution) { + console.log( + ` credential resolution: ${ + resolution.ok === true + ? `verified (HTTP ${resolution.httpStatus})` + : resolution.ok === false + ? `FAILED (HTTP ${resolution.httpStatus})` + : `unknown${resolution.detail ? ` (${resolution.detail})` : ""}` + }`, + ); + } for (const warning of status.warnings) console.log(` warning: ${warning}`); } console.log(""); 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 new file mode 100644 index 00000000000..09be5d55ac9 --- /dev/null +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe-security.test.ts @@ -0,0 +1,216 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, expect, it } from "vitest"; + +import type { McpBridgeEntry } from "../../state/registry"; +import { + buildCredentialResolutionProbeCommand, + classifyCredentialResolutionProbe, + MCP_PROBE_CONTROL_BEARER, + MCP_PROBE_CONTROL_EXIT_MARKER, + MCP_PROBE_CONTROL_HTTP_MARKER, + MCP_PROBE_EXIT_MARKER, + MCP_PROBE_HTTP_MARKER, + PROBE_SANITIZED_ENV_VARS, +} from "./mcp-bridge-resolution-probe"; + +const baseEntry: McpBridgeEntry = { + server: "github", + agent: "openclaw", + adapter: "mcporter", + url: "https://api.githubcopilot.com/mcp/", + env: ["GITHUB_TOKEN"], + providerName: "alpha-mcp-github", + providerId: "11111111-2222-4333-8444-555555555555", + policyName: "mcp-bridge-github", + addedAt: new Date(0).toISOString(), +}; + +function probeStdout( + parts: { + httpStatus?: number; + curlExit: number; + controlHttpStatus?: number; + controlExit?: number; + }, + resultMarker?: string, +): string { + const nonce = resultMarker ? `${resultMarker}:` : ""; + return [ + ...(parts.httpStatus === undefined + ? [] + : [`${MCP_PROBE_HTTP_MARKER}${nonce}${parts.httpStatus}`]), + `${MCP_PROBE_EXIT_MARKER}${nonce}${parts.curlExit}`, + ...(parts.controlHttpStatus === undefined + ? [] + : [`${MCP_PROBE_CONTROL_HTTP_MARKER}${nonce}${parts.controlHttpStatus}`]), + ...(parts.controlExit === undefined + ? [] + : [`${MCP_PROBE_CONTROL_EXIT_MARKER}${nonce}${parts.controlExit}`]), + ].join("\n"); +} + +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"); + expect(built).not.toBeNull(); + const command = built?.command ?? ""; + const validationIndex = command.indexOf('[ -L "$proxy_env" ]'); + const sourceIndex = command.indexOf('. "$proxy_env"'); + const unsetIndex = command.indexOf(`unset ${PROBE_SANITIZED_ENV_VARS.join(" ")}`); + const frameIndex = command.indexOf(built?.resultMarker ?? "missing-result-marker"); + const firstChildIndex = command.indexOf("curl"); + + expect(command).toContain("expected regular root-owned mode 444 file"); + expect(command).toContain('. "$proxy_env" >/dev/null 2>&1'); + expect(validationIndex).toBeGreaterThan(-1); + expect(sourceIndex).toBeGreaterThan(validationIndex); + expect(unsetIndex).toBeGreaterThan(sourceIndex); + 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 ${MCP_PROBE_CONTROL_BEARER}'`); + expect(command).toContain('"method":"initialize"'); + expect(command).toContain(`${MCP_PROBE_HTTP_MARKER}${built?.resultMarker}:`); + expect(command).toContain(`${MCP_PROBE_CONTROL_HTTP_MARKER}${built?.resultMarker}:`); + expect(command.trimEnd().endsWith("exit 0")).toBe(true); + }); + + it("uses the selected adapter runtime without capturing endpoint bodies (#6379)", () => { + const mcporter = buildCredentialResolutionProbeCommand(baseEntry, "mcporter")?.command ?? ""; + const hermes = buildCredentialResolutionProbeCommand(baseEntry, "hermes-config")?.command ?? ""; + const deepagents = + buildCredentialResolutionProbeCommand(baseEntry, "deepagents-config")?.command ?? ""; + + expect(mcporter).toContain("nemoclaw-start node -e"); + expect(hermes).toContain("/opt/hermes/.venv/bin/python -c"); + expect(deepagents).toContain("/opt/venv/bin/python3 -c"); + for (const command of [mcporter, hermes, deepagents]) { + expect(command).toContain("'/dev/null'"); + expect(command).not.toContain("head -c"); + expect(command).not.toContain("mktemp"); + } + }); + + it("refuses missing credentials and unsafe persisted endpoints (#6379)", () => { + expect(buildCredentialResolutionProbeCommand({ ...baseEntry, env: [] }, "mcporter")).toBeNull(); + expect( + buildCredentialResolutionProbeCommand( + { ...baseEntry, url: "http://api.githubcopilot.com/mcp/" }, + "mcporter", + ), + ).toBeNull(); + expect( + buildCredentialResolutionProbeCommand( + { ...baseEntry, url: "https://host.openshell.internal:31337/mcp" }, + "mcporter", + ), + ).toBeNull(); + }); + + it("rejects duplicate and out-of-order result markers (#6379)", () => { + const built = buildCredentialResolutionProbeCommand(baseEntry, "mcporter"); + expect(built).not.toBeNull(); + const resultMarker = built?.resultMarker ?? "missing-result-marker"; + const duplicated = classifyCredentialResolutionProbe( + { + status: 0, + stdout: [ + resultMarker, + probeStdout( + { httpStatus: 200, curlExit: 0, controlHttpStatus: 401, controlExit: 0 }, + resultMarker, + ), + probeStdout( + { httpStatus: 401, curlExit: 0, controlHttpStatus: 401, controlExit: 0 }, + resultMarker, + ), + ].join("\n"), + stderr: "", + }, + baseEntry, + resultMarker, + ); + expect(duplicated.ok).toBeNull(); + + const outOfOrder = classifyCredentialResolutionProbe( + { + status: 0, + stdout: [ + resultMarker, + `${MCP_PROBE_EXIT_MARKER}${resultMarker}:0`, + `${MCP_PROBE_HTTP_MARKER}${resultMarker}:200`, + `${MCP_PROBE_CONTROL_HTTP_MARKER}${resultMarker}:401`, + `${MCP_PROBE_CONTROL_EXIT_MARKER}${resultMarker}:0`, + ].join("\n"), + stderr: "", + }, + baseEntry, + resultMarker, + ); + expect(outOfOrder).toEqual({ ok: null, detail: "probe output markers were out of order" }); + }); + + it("accepts only fresh nonce-bound markers after the trusted result frame (#6379)", () => { + const built = buildCredentialResolutionProbeCommand(baseEntry, "mcporter"); + expect(built).not.toBeNull(); + const resultMarker = built?.resultMarker ?? "missing-result-marker"; + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: [ + probeStdout({ httpStatus: 200, curlExit: 0, controlHttpStatus: 401, controlExit: 0 }), + resultMarker, + probeStdout( + { httpStatus: 401, curlExit: 0, controlHttpStatus: 401, controlExit: 0 }, + resultMarker, + ), + ].join("\n"), + stderr: "", + }, + baseEntry, + resultMarker, + ); + expect(probe.ok).toBeNull(); + expect(probe.httpStatus).toBe(401); + + const staleOnly = classifyCredentialResolutionProbe( + { + status: 0, + stdout: [ + resultMarker, + probeStdout({ httpStatus: 200, curlExit: 0, controlHttpStatus: 401, controlExit: 0 }), + ].join("\n"), + stderr: "", + }, + baseEntry, + resultMarker, + ); + expect(staleOnly).toEqual({ + ok: null, + detail: "probe output missing or ambiguous markers", + }); + + const duplicatedFrame = classifyCredentialResolutionProbe( + { + status: 0, + stdout: [ + resultMarker, + probeStdout( + { httpStatus: 200, curlExit: 0, controlHttpStatus: 401, controlExit: 0 }, + resultMarker, + ), + ].join("\n"), + stderr: resultMarker, + }, + baseEntry, + resultMarker, + ); + expect(duplicatedFrame).toEqual({ + ok: null, + detail: "probe output missing trusted result frame", + }); + }); +}); diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts new file mode 100644 index 00000000000..7592d7121ef --- /dev/null +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.test.ts @@ -0,0 +1,387 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import type { McpBridgeEntry } from "../../state/registry"; + +const mocks = vi.hoisted(() => ({ + executeSandboxCommand: vi.fn(), +})); + +vi.mock("./process-recovery", () => ({ + executeSandboxCommand: mocks.executeSandboxCommand, +})); + +import { + classifyCredentialResolutionProbe, + credentialResolutionWarning, + MCP_PROBE_CONTROL_BEARER, + MCP_PROBE_CONTROL_EXIT_MARKER, + MCP_PROBE_CONTROL_HTTP_MARKER, + MCP_PROBE_EXIT_MARKER, + MCP_PROBE_HTTP_MARKER, + probeCredentialResolution, +} from "./mcp-bridge-resolution-probe"; + +const baseEntry: McpBridgeEntry = { + server: "github", + agent: "openclaw", + adapter: "mcporter", + url: "https://api.githubcopilot.com/mcp/", + env: ["GITHUB_TOKEN"], + providerName: "alpha-mcp-github", + providerId: "11111111-2222-4333-8444-555555555555", + policyName: "mcp-bridge-github", + addedAt: new Date(0).toISOString(), +}; + +const readyProbe = { + policyGatewayPresent: true, + providerAttached: true, + providerCredentialReady: true, +} as const; + +function probeStdout( + parts: { + httpStatus?: number; + curlExit: number; + controlHttpStatus?: number; + controlExit?: number; + }, + resultMarker?: string, +): string { + const nonce = resultMarker ? `${resultMarker}:` : ""; + return [ + "", + ...(parts.httpStatus !== undefined + ? [`${MCP_PROBE_HTTP_MARKER}${nonce}${parts.httpStatus}`] + : []), + `${MCP_PROBE_EXIT_MARKER}${nonce}${parts.curlExit}`, + ...(parts.controlHttpStatus !== undefined + ? [`${MCP_PROBE_CONTROL_HTTP_MARKER}${nonce}${parts.controlHttpStatus}`] + : []), + ...(parts.controlExit !== undefined + ? [`${MCP_PROBE_CONTROL_EXIT_MARKER}${nonce}${parts.controlExit}`] + : []), + ].join("\n"); +} + +beforeEach(() => { + mocks.executeSandboxCommand.mockReset(); +}); + +describe("MCP credential-resolution probe classification", () => { + it("classifies placeholder 2xx with rejected control as resolved on the wire (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ + httpStatus: 200, + curlExit: 0, + controlHttpStatus: 401, + controlExit: 0, + }), + stderr: "", + }, + baseEntry, + ); + expect(probe).toEqual({ ok: true, httpStatus: 200, controlHttpStatus: 401 }); + }); + + it("classifies identical placeholder and control rejections as inconclusive with both hypotheses (#6379)", () => { + for (const httpStatus of [400, 401, 403]) { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ + httpStatus, + curlExit: 0, + controlHttpStatus: httpStatus, + controlExit: 0, + }), + stderr: "", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.httpStatus).toBe(httpStatus); + expect(probe.controlHttpStatus).toBe(httpStatus); + expect(probe.detail).toContain("rejected identically"); + expect(probe.detail).toContain("expired or revoked credential"); + } + }); + + it("classifies identical 5xx responses as indeterminate endpoint failure (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ + httpStatus: 500, + curlExit: 0, + controlHttpStatus: 500, + controlExit: 0, + }), + stderr: "", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("failed identically"); + }); + + it("never reports differing non-2xx rejections as verified resolution (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ + httpStatus: 401, + curlExit: 0, + controlHttpStatus: 400, + controlExit: 0, + }), + stderr: "", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("differing rejections do not prove resolution"); + }); + + it("names request validation among the hypotheses for identical 400 rejections (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ + httpStatus: 400, + curlExit: 0, + controlHttpStatus: 400, + controlExit: 0, + }), + stderr: "", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("request validation"); + }); + + it("classifies dual 2xx as an endpoint that does not enforce authentication (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ + httpStatus: 200, + curlExit: 0, + controlHttpStatus: 200, + controlExit: 0, + }), + stderr: "", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("does not enforce authentication"); + }); + + it("classifies differing non-auth statuses as indeterminate (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ + httpStatus: 400, + curlExit: 0, + controlHttpStatus: 401, + controlExit: 0, + }), + stderr: "", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("known-good host"); + }); + + it("classifies a failed control probe as indeterminate (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ httpStatus: 401, curlExit: 0, controlExit: 28 }), + stderr: "", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("control probe failed"); + }); + + it("classifies a CONNECT-level proxy 403 as an indeterminate policy denial (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: probeStdout({ curlExit: 56 }), + stderr: "curl: (56) CONNECT tunnel failed, response 403", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("CONNECT 403"); + }); + + it("classifies curl exit 28 as an indeterminate probe timeout (#6379)", () => { + const probe = classifyCredentialResolutionProbe( + { status: 0, stdout: probeStdout({ curlExit: 28 }), stderr: "" }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(probe.detail).toContain("timed out"); + }); + + it("classifies a missing command result as sandbox unreachable (#6379)", () => { + expect(classifyCredentialResolutionProbe(null, baseEntry)).toEqual({ + ok: null, + detail: "sandbox unreachable", + }); + }); + + it("never includes endpoint response text in the verdict (#6379)", () => { + const body = '{"error":"bad token ghp_super-secret-value-1234567890"}'; + const probe = classifyCredentialResolutionProbe( + { + status: 0, + stdout: `${probeStdout({ httpStatus: 401, curlExit: 0, controlHttpStatus: 401, controlExit: 0 })}\n${body}`, + stderr: "", + }, + baseEntry, + ); + expect(probe.ok).toBeNull(); + expect(JSON.stringify(probe)).not.toContain("ghp_super-secret-value-1234567890"); + }); +}); + +describe("MCP credential-resolution probe execution gates", () => { + it("fails closed before sandbox traffic unless policy and provider readiness are all true (#6379)", () => { + const cases = [ + [{ ...readyProbe, policyGatewayPresent: false }, "effective gateway policy"], + [{ ...readyProbe, policyGatewayPresent: null }, "could not be inspected"], + [{ ...readyProbe, providerAttached: false }, "not attached"], + [{ ...readyProbe, providerAttached: null }, "attachment could not be inspected"], + [{ ...readyProbe, providerCredentialReady: false }, "does not match"], + ] as const; + + for (const [readiness, expectedDetail] of cases) { + const probe = probeCredentialResolution("alpha", baseEntry, "mcporter", readiness); + expect(probe).toMatchObject({ ok: null }); + expect(probe.detail).toContain(expectedDetail); + } + expect(mocks.executeSandboxCommand).not.toHaveBeenCalled(); + }); + + it("skips without contacting the sandbox when the adapter is not declared (#6379)", () => { + const probe = probeCredentialResolution("alpha", baseEntry, undefined, readyProbe); + expect(probe).toEqual({ ok: null, detail: "MCP adapter is not declared" }); + expect(mocks.executeSandboxCommand).not.toHaveBeenCalled(); + }); + + it("skips without contacting the sandbox while an add transaction is incomplete (#6379)", () => { + const probe = probeCredentialResolution( + "alpha", + { ...baseEntry, addState: "preflighted" }, + "mcporter", + readyProbe, + ); + expect(probe).toEqual({ ok: null, detail: "add transaction incomplete" }); + expect(mocks.executeSandboxCommand).not.toHaveBeenCalled(); + }); + + it("skips without contacting the sandbox when the stored URL is unsafe (#6379)", () => { + const probe = probeCredentialResolution( + "alpha", + { ...baseEntry, url: "http://api.githubcopilot.com/mcp/" }, + "mcporter", + readyProbe, + ); + expect(probe).toEqual({ ok: null, detail: "no credential binding or safe endpoint to probe" }); + expect(mocks.executeSandboxCommand).not.toHaveBeenCalled(); + }); + + it("executes the probe in the sandbox and classifies the outcome (#6379)", () => { + 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); + 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(MCP_PROBE_CONTROL_BEARER); + }); +}); + +describe("MCP credential-resolution warning", () => { + it("warns on identical auth rejections with both hypotheses and the OpenShell host remediation (#6379)", () => { + const warning = credentialResolutionWarning("GITHUB_TOKEN", { + ok: null, + httpStatus: 403, + controlHttpStatus: 403, + }); + expect(warning).toContain("openshell:resolve:env:GITHUB_TOKEN"); + expect(warning).toContain("identically (HTTP 403)"); + expect(warning).toContain("If the stored credential is confirmed valid"); + expect(warning).toContain("OpenShell issue 2161"); + }); + + it("keeps the identical-400 warning explicitly inconclusive with the request-validation hypothesis (#6379)", () => { + const warning = credentialResolutionWarning("GITHUB_TOKEN", { + ok: null, + httpStatus: 400, + controlHttpStatus: 400, + }); + expect(warning).toContain("inconclusive even with a valid stored credential"); + expect(warning).toContain("request validation"); + expect(warning).toContain("known-good host"); + expect(warning).not.toContain("the OpenShell host is not rewriting"); + }); + + it("stays silent for verified, differing, non-auth 4xx, and 5xx outcomes (#6379)", () => { + expect( + credentialResolutionWarning("GITHUB_TOKEN", { + ok: true, + httpStatus: 200, + controlHttpStatus: 401, + }), + ).toBeUndefined(); + expect( + credentialResolutionWarning("GITHUB_TOKEN", { + ok: null, + httpStatus: 400, + controlHttpStatus: 401, + }), + ).toBeUndefined(); + expect( + credentialResolutionWarning("GITHUB_TOKEN", { + ok: null, + httpStatus: 404, + controlHttpStatus: 404, + }), + ).toBeUndefined(); + expect( + credentialResolutionWarning("GITHUB_TOKEN", { + ok: null, + httpStatus: 500, + controlHttpStatus: 500, + }), + ).toBeUndefined(); + }); +}); diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts new file mode 100644 index 00000000000..ccdedd26aa2 --- /dev/null +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-probe.ts @@ -0,0 +1,412 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Wire-level credential-resolution probe (#6379). + * + * 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). + * + * The probe is differential: it sends the same idempotent MCP `initialize` + * request twice from inside the sandbox — once with the placeholder + * authorization header exactly as agent traffic carries it, and once with a + * deliberately-unresolvable literal control bearer that no gateway rewrite can + * ever touch. A working rewrite makes the two requests reach the endpoint with + * different bearers; a dead rewrite forwards both literally. Only a + * placeholder 2xx paired with a rejected control verifies resolution — an + * accepted request proves a valid credential was on the wire while the control + * proves the endpoint rejects garbage. Every non-2xx placeholder outcome is + * inconclusive with the hypotheses named: identical rejections cannot separate + * "forwarded verbatim" from "resolved but expired or revoked", and differing + * rejections cannot either, because an endpoint may reject two different + * literal bearer strings differently. + * + * Response bodies are never captured or printed: they are untrusted + * authenticated endpoint output, and redaction cannot be guaranteed once the + * credential's host environment variable is absent. Classification uses HTTP + * status codes and curl exit codes only. + * + * Probing is gated on the stored URL still satisfying the current + * authenticated-endpoint boundary, so a persisted legacy, private-alias, or + * plain-HTTP URL is never sent a header that the gateway could rewrite into a + * live credential. + * + * Source-boundary note: + * - invalidState: provider metadata is healthy while OpenShell forwards the + * literal placeholder instead of resolving it. + * - sourceBoundary: OpenShell owns provider attachment/rewrite behavior; + * NemoClaw owns this bounded diagnostic and its fail-closed prerequisites. + * - whyNotSourceFix: supported OpenShell versions expose neither a + * resolution-or-deny guarantee nor an adequate machine-readable signal. + * - regressionTest: mcp-bridge-resolution-probe.test.ts pins classification + * and central no-send gates; mcp-bridge-status-resolution.test.ts pins status + * and post-add policy/provider prerequisites. + * - removalCondition: remove or replace this probe only when every supported + * OpenShell version guarantees resolution-or-deny before egress or exposes a + * reviewed signal for the exact attached provider revision and generated route. + */ + +import type { AgentMcpAdapter } from "../../agent/defs"; +import { shellQuote } from "../../core/shell-quote"; +import type { McpBridgeEntry } from "../../state/registry"; +import { authorizationValue } from "./mcp-bridge-adapter-status"; +import { redactBridgeSecretsForDisplay } from "./mcp-bridge-output"; +import { + type CredentialResolutionProbeReadiness, + credentialResolutionReadinessSkipDetail, +} from "./mcp-bridge-resolution-readiness"; +import { normalizeMcpServerUrl } from "./mcp-bridge-validation"; +import { executeSandboxCommand, type SandboxCommandResult } from "./process-recovery"; +import { + buildSandboxExecMarkedCommand, + createSandboxExecMarker, + extractSandboxExecCommandStdoutFromStreams, +} from "./sandbox-exec-output"; +import { buildTrustedProxyEnvSourceShell } from "./trusted-proxy-env"; + +export const MCP_PROBE_HTTP_MARKER = "NEMOCLAW_MCP_PROBE_HTTP_CODE="; +export const MCP_PROBE_EXIT_MARKER = "NEMOCLAW_MCP_PROBE_CURL_EXIT="; +export const MCP_PROBE_CONTROL_HTTP_MARKER = "NEMOCLAW_MCP_CONTROL_HTTP_CODE="; +export const MCP_PROBE_CONTROL_EXIT_MARKER = "NEMOCLAW_MCP_CONTROL_CURL_EXIT="; + +/** + * Literal control bearer. Not an `openshell:resolve:` reference, so the + * gateway forwards it untouched on healthy and broken hosts alike, and it is + * not a secret. It only has to be a value no endpoint would ever accept. + */ +export const MCP_PROBE_CONTROL_BEARER = "nemoclaw-mcp-probe-control-unresolvable"; + +// executeSandboxCommand enforces a 15s spawnSync timeout; two sequential curls +// must both fit comfortably below it so a slow endpoint classifies as a probe +// timeout instead of an ambiguous SSH failure. +const PROBE_CURL_MAX_TIME_SECONDS = 6; + +/** + * Sourcing /tmp/nemoclaw-proxy-env.sh can export the OpenClaw gateway + * credentials and break-glass toggles alongside the proxy variables the probe + * actually needs. These are unset immediately after sourcing, before the + * first child process, so neither the adapter runtime nor curl inherits them + * (same sanitize set nemoclaw-start uses for un-managed openclaw children). + */ +export const PROBE_SANITIZED_ENV_VARS = [ + "OPENCLAW_GATEWAY_URL", + "OPENCLAW_GATEWAY_PORT", + "OPENCLAW_GATEWAY_TOKEN", + "OPENCLAW_ALLOW_INSECURE_PRIVATE_WS", + "NEMOCLAW_OPENCLAW_ALLOW_INSECURE_PRIVATE_WS", +] as const; + +export interface CredentialResolutionProbe { + /** + * true = placeholder resolved on the wire; null = inconclusive or skipped + * (see detail). false is reserved for future evidence sources that can + * prove non-rewriting; wire statuses alone never can, so the current + * classifier never emits it. + */ + ok: boolean | null; + httpStatus?: number; + controlHttpStatus?: number; + detail?: string; +} + +export interface CredentialResolutionProbeCommand { + command: string; + resultMarker: string; +} + +interface ProbeOutputMarkers { + controlExit: string; + controlHttp: string; + placeholderExit: string; + placeholderHttp: string; +} + +function probeOutputMarkers(resultMarker?: string): ProbeOutputMarkers { + const nonce = resultMarker ? `${resultMarker}:` : ""; + return { + placeholderHttp: `${MCP_PROBE_HTTP_MARKER}${nonce}`, + placeholderExit: `${MCP_PROBE_EXIT_MARKER}${nonce}`, + controlHttp: `${MCP_PROBE_CONTROL_HTTP_MARKER}${nonce}`, + controlExit: `${MCP_PROBE_CONTROL_EXIT_MARKER}${nonce}`, + }; +} + +// "initialize" is idempotent and the first method allowed by the generated +// protocol: mcp policy, so the probe never mutates MCP server state. +const MCP_INITIALIZE_BODY = JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-06-18", + capabilities: {}, + clientInfo: { name: "nemoclaw-mcp-credential-probe", version: "1.0.0" }, + }, +}); + +/** + * OpenShell binds the generated MCP policy to /proc//exe and ancestors, + * so the curl child must keep the adapter's runtime binary as an ancestor. + * Same construction as the live E2E DNS-rebinding probe. + */ +function runtimeWrappedCommand(adapter: AgentMcpAdapter, quotedCurl: string): string { + switch (adapter) { + case "mcporter": { + const runner = + 'const { spawnSync } = require("node:child_process"); const result = spawnSync(process.argv[1], process.argv.slice(2), { stdio: "inherit" }); process.exit(result.status ?? 1);'; + return `nemoclaw-start node -e ${shellQuote(runner)} ${quotedCurl}`; + } + case "hermes-config": { + const runner = + "import subprocess, sys; raise SystemExit(subprocess.run(sys.argv[1:], check=False).returncode)"; + return `/opt/hermes/.venv/bin/python -c ${shellQuote(runner)} ${quotedCurl}`; + } + case "deepagents-config": { + const runner = + "import subprocess, sys; raise SystemExit(subprocess.run(sys.argv[1:], check=False).returncode)"; + return `/opt/venv/bin/python3 -c ${shellQuote(runner)} ${quotedCurl}`; + } + } +} + +function quotedCurlCommand(url: string, authorization: string, httpMarker: string): string { + const curlArgs = [ + "curl", + "-sS", + "--max-time", + String(PROBE_CURL_MAX_TIME_SECONDS), + // The response body is untrusted authenticated endpoint output and is + // never captured; classification uses status and exit codes only. + "-o", + "/dev/null", + "-w", + `\\n${httpMarker}%{http_code}\\n`, + "-X", + "POST", + url, + "-H", + "content-type: application/json", + "-H", + // mcporter itself synthesizes this accept header on every HTTP definition. + "accept: application/json, text/event-stream", + "-H", + `authorization: ${authorization}`, + "--data-binary", + MCP_INITIALIZE_BODY, + ]; + return curlArgs.map(shellQuote).join(" "); +} + +export function buildCredentialResolutionProbeCommand( + entry: Pick, + adapter: AgentMcpAdapter, +): CredentialResolutionProbeCommand | null { + const authorization = authorizationValue(entry); + if (!authorization) return null; + // Never probe a persisted URL that no longer satisfies the current + // authenticated-endpoint boundary: the gateway could rewrite the placeholder + // header into a live credential bound for a legacy or private endpoint. + try { + if (normalizeMcpServerUrl(entry.url) !== entry.url) return null; + } catch { + return null; + } + const resultMarker = createSandboxExecMarker(); + const markers = probeOutputMarkers(resultMarker); + const placeholderCurl = quotedCurlCommand(entry.url, authorization, markers.placeholderHttp); + const controlCurl = quotedCurlCommand( + entry.url, + `Bearer ${MCP_PROBE_CONTROL_BEARER}`, + markers.controlHttp, + ); + const probeBody = [ + runtimeWrappedCommand(adapter, placeholderCurl), + "rc=$?", + `printf '\\n${markers.placeholderExit}%s\\n' "$rc"`, + runtimeWrappedCommand(adapter, controlCurl), + "crc=$?", + `printf '\\n${markers.controlExit}%s\\n' "$crc"`, + // Always exit 0 so a nonzero SSH status unambiguously means transport + // failure, never a probe outcome. + "exit 0", + ].join("\n"); + return { + resultMarker, + command: [ + // SSH sessions can miss the sandbox proxy environment (#2704). Validate + // the cross-user file and suppress source-time output before framing any + // probe result, so preamble text cannot impersonate result markers. + buildTrustedProxyEnvSourceShell(), + // Must stay between the sourcing above and the first child below. + `unset ${PROBE_SANITIZED_ENV_VARS.join(" ")} || true`, + buildSandboxExecMarkedCommand(probeBody, resultMarker), + ].join("\n"), + }; +} + +function redactedProbeText(text: string, entry: Pick): string { + return redactBridgeSecretsForDisplay(text, entry).trim(); +} + +interface ProbeMarkerValue { + index: number; + value: number; +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +function markerValue(stdout: string, marker: string): ProbeMarkerValue | undefined { + const matches = [...stdout.matchAll(new RegExp(`^${escapeRegExp(marker)}([0-9]+)$`, "gm"))]; + if (matches.length !== 1) return undefined; + return { index: matches[0].index, value: Number(matches[0][1]) }; +} + +function transportDetail(curlExit: number, stderr: string): string | undefined { + if (curlExit === 56 && /CONNECT tunnel failed,\s*response 403/i.test(stderr)) { + return "OpenShell denied the probe connection (CONNECT 403); check the generated MCP policy"; + } + if (curlExit === 28) return `probe timed out after ${PROBE_CURL_MAX_TIME_SECONDS}s`; + return undefined; +} + +export function classifyCredentialResolutionProbe( + result: SandboxCommandResult | null, + entry: Pick, + resultMarker?: string, +): CredentialResolutionProbe { + if (result === null) return { ok: null, detail: "sandbox unreachable" }; + if (result.status !== 0) { + const detail = redactedProbeText(result.stderr || result.stdout, entry); + return { ok: null, detail: detail || "probe transport failed" }; + } + const framedStdout = resultMarker + ? extractSandboxExecCommandStdoutFromStreams( + { stdout: result.stdout, stderr: result.stderr }, + resultMarker, + ) + : result.stdout; + if (framedStdout === null) { + return { ok: null, detail: "probe output missing trusted result frame" }; + } + const markers = probeOutputMarkers(resultMarker); + const placeholderExit = markerValue(framedStdout, markers.placeholderExit); + if (placeholderExit === undefined) { + return { ok: null, detail: "probe output missing or ambiguous markers" }; + } + if (placeholderExit.value !== 0) { + const detail = transportDetail(placeholderExit.value, result.stderr); + return { ok: null, detail: detail ?? `probe curl exited ${placeholderExit.value}` }; + } + const httpStatus = markerValue(framedStdout, markers.placeholderHttp); + if (httpStatus === undefined) return { ok: null, detail: "probe output missing HTTP status" }; + const controlExit = markerValue(framedStdout, markers.controlExit); + const controlHttpStatus = + controlExit?.value === 0 ? markerValue(framedStdout, markers.controlHttp) : undefined; + if (controlExit === undefined || controlHttpStatus === undefined) { + return { + ok: null, + httpStatus: httpStatus.value, + detail: `the placeholder probe received HTTP ${httpStatus.value} but the unresolvable control probe failed, so resolved and unresolved credentials cannot be distinguished`, + }; + } + if ( + !( + httpStatus.index < placeholderExit.index && + placeholderExit.index < controlHttpStatus.index && + controlHttpStatus.index < controlExit.index + ) + ) { + return { ok: null, detail: "probe output markers were out of order" }; + } + const shared = { + httpStatus: httpStatus.value, + controlHttpStatus: controlHttpStatus.value, + }; + if (httpStatus.value >= 200 && httpStatus.value < 300) { + if (controlHttpStatus.value >= 200 && controlHttpStatus.value < 300) { + return { + ok: null, + ...shared, + detail: `the endpoint accepted both the placeholder probe and an unresolvable control bearer (HTTP ${httpStatus.value} / ${controlHttpStatus.value}), so it does not enforce authentication and credential resolution cannot be judged`, + }; + } + return { ok: true, ...shared }; + } + if (httpStatus.value === controlHttpStatus.value) { + // Identical statuses never prove non-rewriting: a correctly rewritten but + // expired or revoked credential and the bogus control can both draw the + // same 4xx, and an endpoint can fail both probes the same way. Report the + // evidence and let the operator rule out the credential. + if (httpStatus.value >= 400 && httpStatus.value < 500) { + const validationHypothesis = + httpStatus.value === 400 + ? ", or with an initialize request this endpoint does not accept (request validation)" + : ""; + return { + ok: null, + ...shared, + detail: `the placeholder probe and the unresolvable control probe were rejected identically (HTTP ${httpStatus.value}); this is consistent with the placeholder being forwarded verbatim, but also with an expired or revoked credential that resolved correctly${validationHypothesis} — verify the stored credential value first`, + }; + } + return { + ok: null, + ...shared, + detail: `both probes received HTTP ${httpStatus.value}; the endpoint failed identically and credential resolution could not be judged`, + }; + } + // Differing non-2xx statuses prove nothing either: a broken gateway forwards + // two different literal bearer strings, and an endpoint may reject those + // differently (e.g. placeholder 401, control 400) without rewriting one of + // them. Only a placeholder 2xx with a rejected control proves resolution. + return { + ok: null, + ...shared, + detail: `the placeholder probe received HTTP ${httpStatus.value} and the unresolvable control HTTP ${controlHttpStatus.value}; differing rejections do not prove resolution because the endpoint may reject two different literal bearers differently — verify the stored credential value and compare against a known-good host`, + }; +} + +/** + * Warning for the identical-4xx outcome. Wire evidence alone cannot separate + * "placeholder forwarded verbatim" from "resolved but expired or revoked + * credential", so the warning states the hypotheses and tells the operator + * which check rules out which. For identical 401/403 a confirmed-valid + * credential does settle it (a rewritten valid credential cannot draw the + * same auth rejection as garbage); for identical 400 it does not, because the + * endpoint may reject the probe's initialize request itself regardless of the + * bearer, so that warning stays explicitly inconclusive. + */ +export function credentialResolutionWarning( + envName: string | undefined, + probe: Pick, +): string | undefined { + if (probe.ok !== null) return undefined; + 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:"; + 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).`; + } + if (probe.httpStatus !== 401 && probe.httpStatus !== 403) return undefined; + return `Credential resolution could not be verified: a placeholder-bearing MCP initialize probe and a deliberately-unresolvable control probe were rejected identically (HTTP ${probe.httpStatus}). If the stored credential is confirmed valid, the OpenShell host is not rewriting the '${placeholder}' placeholder on egress and agent runtimes will hit the same auth failure and skip this MCP server (see NVIDIA/OpenShell issue 2161). Otherwise, rotate the credential with mcp restart and re-run mcp status.`; +} + +export function probeCredentialResolution( + sandboxName: string, + entry: McpBridgeEntry, + adapter: AgentMcpAdapter | undefined, + readiness: CredentialResolutionProbeReadiness, +): 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 }; + const result = executeSandboxCommand(sandboxName, probeCommand.command); + return classifyCredentialResolutionProbe(result, entry, probeCommand.resultMarker); +} diff --git a/src/lib/actions/sandbox/mcp-bridge-resolution-readiness.ts b/src/lib/actions/sandbox/mcp-bridge-resolution-readiness.ts new file mode 100644 index 00000000000..7aa14b489f1 --- /dev/null +++ b/src/lib/actions/sandbox/mcp-bridge-resolution-readiness.ts @@ -0,0 +1,29 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export interface CredentialResolutionProbeReadiness { + policyGatewayPresent: boolean | null; + providerAttached: boolean | null; + providerCredentialReady: boolean; +} + +export function credentialResolutionReadinessSkipDetail( + readiness: CredentialResolutionProbeReadiness, +): string | undefined { + if (readiness.policyGatewayPresent === null) { + return "probe skipped: the effective generated MCP policy could not be inspected"; + } + if (!readiness.policyGatewayPresent) { + return "probe skipped: the generated MCP policy does not match the effective gateway policy"; + } + if (readiness.providerAttached === null) { + return "probe skipped: provider attachment could not be inspected"; + } + if (!readiness.providerAttached) { + return "probe skipped: the credential provider is not attached to the sandbox"; + } + if (!readiness.providerCredentialReady) { + return "probe skipped: the OpenShell provider does not match the recorded credential binding"; + } + return undefined; +} diff --git a/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts b/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts new file mode 100644 index 00000000000..aba649976e9 --- /dev/null +++ b/src/lib/actions/sandbox/mcp-bridge-status-resolution.test.ts @@ -0,0 +1,495 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; + +const sourceRequireHook = path.resolve("test/helpers/onboard-script-mocks.cjs"); +const sourceNodeOptions = [process.env.NODE_OPTIONS, `--require=${sourceRequireHook}`] + .filter(Boolean) + .join(" "); +const tempHomes = new Set(); + +function createTempHome(prefix: string): string { + const home = fs.mkdtempSync(path.join(os.tmpdir(), prefix)); + tempHomes.add(home); + return home; +} + +afterEach(() => { + tempHomes.forEach((home) => fs.rmSync(home, { recursive: true, force: true })); + tempHomes.clear(); +}); + +// Shared subprocess prelude: a healthy committed bridge whose provider +// metadata is all-green, with the in-sandbox probe answering an identical +// rejection for the placeholder and control requests — the exact "status lies +// while the wire fails" shape from #6379. __PROBE_HTTP_STATUS__ is substituted +// per test so both the 401 (auth-shaped) and 400 (validation-ambiguous) +// warnings are exercised end-to-end. +const harnessPreludeTemplate = String.raw` +const registry = require("./src/lib/state/registry.js"); +const gatewayRuntime = require("./src/lib/gateway-runtime-action.js"); +const globalActions = require("./src/lib/actions/global.js"); +const policies = require("./src/lib/policy/index.js"); +const processRecovery = require("./src/lib/actions/sandbox/process-recovery.js"); +gatewayRuntime.recoverNamedGatewayRuntime = async () => ({ + recovered: true, + attempted: false, + before: { state: "healthy_named" }, + after: { state: "healthy_named" }, +}); +let providerAttachmentState = "attached"; +let providerCredentialKey = "GITHUB_TOKEN"; +globalActions.runOpenshellProviderCommand = (args) => { + if (args[0] === "provider" && args[1] === "get") { + return { + status: 0, + stdout: "Id: 11111111-2222-4333-8444-555555555555\nType: generic\nResource version: 4\nCredential keys: " + providerCredentialKey + "\n", + stderr: "", + }; + } + if (args[0] === "sandbox" && args[1] === "provider" && args[2] === "list") { + if (providerAttachmentState === "unknown") { + return { status: 1, stdout: "", stderr: "attachment inspection failed" }; + } + if (providerAttachmentState === "absent") { + return { status: 0, stdout: "No providers attached to sandbox alpha\n", stderr: "" }; + } + return { + status: 0, + stdout: "NAME TYPE CREDENTIAL_KEYS CONFIG_KEYS\nalpha-mcp-github generic 1 0\n", + stderr: "", + }; + } + throw new Error("Unexpected OpenShell call: " + args.join(" ")); +}; +let activePolicyState = "match"; +policies.getPresetContentGatewayState = () => activePolicyState; +const executedSandboxCommands = []; +processRecovery.executeSandboxCommand = (sandboxName, command) => { + executedSandboxCommands.push(command); + if (command.includes("NEMOCLAW_MCP_PROBE")) { + const resultMarker = command.match(/__NEMOCLAW_SANDBOX_EXEC_STARTED___[0-9a-f]{32}/)?.[0]; + if (!resultMarker) throw new Error("credential probe result marker missing"); + return { + status: 0, + stdout: [ + resultMarker, + "", + "NEMOCLAW_MCP_PROBE_HTTP_CODE=" + resultMarker + ":__PROBE_HTTP_STATUS__", + "NEMOCLAW_MCP_PROBE_CURL_EXIT=" + resultMarker + ":0", + "NEMOCLAW_MCP_CONTROL_HTTP_CODE=" + resultMarker + ":__PROBE_HTTP_STATUS__", + "NEMOCLAW_MCP_CONTROL_CURL_EXIT=" + resultMarker + ":0", + ].join("\n"), + stderr: "", + }; + } + return { status: 0, stdout: "registered", stderr: "" }; +}; +registry.registerSandbox({ + name: "alpha", + agent: "openclaw", + mcp: { bridges: { github: { + server: "github", + agent: "openclaw", + adapter: "mcporter", + url: "https://api.githubcopilot.com/mcp/", + env: ["GITHUB_TOKEN"], + providerName: "alpha-mcp-github", + providerId: "11111111-2222-4333-8444-555555555555", + policyName: "mcp-bridge-github", + addedAt: "2026-06-01T00:00:00.000Z", + } } }, +}); +registry.addCustomPolicy("alpha", { + name: "mcp-bridge-github", + content: "network_policies: {}\n", + sourcePath: "generated:nemoclaw-mcp-bridge", +}); +const bridge = require("./src/lib/actions/sandbox/mcp-bridge.js"); +const logLines = []; +const errorLines = []; +console.log = (...parts) => logLines.push(parts.join(" ")); +console.error = (...parts) => errorLines.push(parts.join(" ")); +`; + +function runHarness( + home: string, + body: string, + options: { probeHttpStatus?: number } = {}, +): { status: number | null; stdout: string } { + const prelude = harnessPreludeTemplate.replaceAll( + "__PROBE_HTTP_STATUS__", + String(options.probeHttpStatus ?? 401), + ); + const script = ` +process.env.HOME = ${JSON.stringify(home)}; +${prelude} +(async () => { +${body} +})().catch((error) => { + process.stderr.write(String(error && error.stack ? error.stack : error)); + process.exit(1); +}); +`; + const result = spawnSync(process.execPath, ["-e", script], { + cwd: process.cwd(), + encoding: "utf8", + env: { ...process.env, HOME: home, NODE_OPTIONS: sourceNodeOptions }, + }); + expect(result.status, `harness failed: ${result.stderr}`).toBe(0); + return { status: result.status, stdout: result.stdout }; +} + +describe("MCP status wire-level credential-resolution probe", () => { + it("probes by default for a single named server and surfaces the wire failure (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-single-"); + const { stdout } = runHarness( + home, + String.raw` + await bridge.dispatchMcpBridgeCommand("alpha", ["status", "github", "--json"]); + const status = JSON.parse(logLines.join("\n")); + process.stdout.write(JSON.stringify({ + status, + probed: executedSandboxCommands.some((c) => c.includes("NEMOCLAW_MCP_PROBE")), + exitCode: process.exitCode ?? 0, + })); +`, + ); + const payload = JSON.parse(stdout) as { + status: { + provider: { credentialResolution?: { ok: boolean | null; httpStatus?: number } }; + warnings: string[]; + }; + probed: boolean; + exitCode: number; + }; + expect(payload.probed).toBe(true); + expect(payload.status.provider.credentialResolution).toMatchObject({ + ok: null, + httpStatus: 401, + controlHttpStatus: 401, + }); + expect( + payload.status.warnings.some((warning) => + warning.includes("Credential resolution could not be verified"), + ), + ).toBe(true); + expect(payload.exitCode).toBe(0); + }); + + it("skips status probe traffic until exact policy and provider readiness are verified (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-readiness-"); + const { stdout } = runHarness( + home, + String.raw` + const outcomes = []; + for (const policyState of ["absent", "drift", null]) { + activePolicyState = policyState; + providerAttachmentState = "attached"; + providerCredentialKey = "GITHUB_TOKEN"; + executedSandboxCommands.length = 0; + const [status] = await bridge.statusMcpBridge("alpha", "github", { + probeCredentialResolution: true, + }); + outcomes.push({ + case: "policy:" + String(policyState), + gatewayPresent: status.policy.gatewayPresent, + resolution: status.provider.credentialResolution, + probed: executedSandboxCommands.some((c) => c.includes("NEMOCLAW_MCP_PROBE")), + }); + } + activePolicyState = "match"; + for (const attachmentState of ["absent", "unknown"]) { + providerAttachmentState = attachmentState; + providerCredentialKey = "GITHUB_TOKEN"; + executedSandboxCommands.length = 0; + const [status] = await bridge.statusMcpBridge("alpha", "github", { + probeCredentialResolution: true, + }); + outcomes.push({ + case: "attachment:" + attachmentState, + resolution: status.provider.credentialResolution, + probed: executedSandboxCommands.some((c) => c.includes("NEMOCLAW_MCP_PROBE")), + }); + } + providerAttachmentState = "attached"; + providerCredentialKey = "WRONG_TOKEN"; + executedSandboxCommands.length = 0; + const [wrongProvider] = await bridge.statusMcpBridge("alpha", "github", { + probeCredentialResolution: true, + }); + outcomes.push({ + case: "provider:wrong-shape", + resolution: wrongProvider.provider.credentialResolution, + probed: executedSandboxCommands.some((c) => c.includes("NEMOCLAW_MCP_PROBE")), + }); + process.stdout.write(JSON.stringify(outcomes)); +`, + ); + const outcomes = JSON.parse(stdout) as Array<{ + case: string; + gatewayPresent?: boolean | null; + resolution: { ok: boolean | null; detail?: string }; + probed: boolean; + }>; + expect(outcomes).toHaveLength(6); + expect(outcomes.map((outcome) => outcome.gatewayPresent).slice(0, 3)).toEqual([ + false, + false, + null, + ]); + for (const outcome of outcomes) { + expect(outcome.probed, outcome.case).toBe(false); + expect(outcome.resolution.ok, outcome.case).toBeNull(); + expect(outcome.resolution.detail, outcome.case).toContain("probe skipped"); + } + }); + + it("renders the identical-rejection probe in the human-readable status output (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-render-"); + const { stdout } = runHarness( + home, + String.raw` + await bridge.dispatchMcpBridgeCommand("alpha", ["status", "github"]); + process.stdout.write(JSON.stringify({ lines: logLines })); +`, + ); + const payload = JSON.parse(stdout) as { lines: string[] }; + expect(payload.lines.some((line) => line.includes("credential resolution: unknown"))).toBe( + true, + ); + expect( + payload.lines.some((line) => line.includes("Credential resolution could not be verified")), + ).toBe(true); + }); + + it("keeps the status warning for identical 400 explicitly inconclusive (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-400-"); + const { stdout } = runHarness( + home, + String.raw` + await bridge.dispatchMcpBridgeCommand("alpha", ["status", "github", "--json"]); + const status = JSON.parse(logLines.join("\n")); + process.stdout.write(JSON.stringify({ warnings: status.warnings })); +`, + { probeHttpStatus: 400 }, + ); + const payload = JSON.parse(stdout) as { warnings: string[] }; + const warning = payload.warnings.find((line) => + line.includes("Credential resolution could not be verified"), + ); + expect(warning).toBeDefined(); + expect(warning).toContain("inconclusive even with a valid stored credential"); + expect(warning).toContain("request validation"); + expect(warning).not.toContain("the OpenShell host is not rewriting"); + }); + + it("never probes from bare status or list so multi-server views stay fast (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-list-"); + const { stdout } = runHarness( + home, + String.raw` + await bridge.dispatchMcpBridgeCommand("alpha", ["status", "--json"]); + const bareStatus = JSON.parse(logLines.join("\n")); + logLines.length = 0; + await bridge.dispatchMcpBridgeCommand("alpha", ["list", "--json"]); + const list = JSON.parse(logLines.join("\n")); + process.stdout.write(JSON.stringify({ + probed: executedSandboxCommands.some((c) => c.includes("NEMOCLAW_MCP_PROBE")), + bareStatusResolution: bareStatus.bridges[0].provider.credentialResolution ?? null, + listResolution: list.bridges[0].provider.credentialResolution ?? null, + })); +`, + ); + const payload = JSON.parse(stdout) as { + probed: boolean; + bareStatusResolution: unknown; + listResolution: unknown; + }; + expect(payload.probed).toBe(false); + expect(payload.bareStatusResolution).toBeNull(); + expect(payload.listResolution).toBeNull(); + }); + + it("honors --no-probe on a named server and --probe on the multi-server form (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-flags-"); + const { stdout } = runHarness( + home, + String.raw` + await bridge.dispatchMcpBridgeCommand("alpha", ["status", "github", "--no-probe", "--json"]); + const skipped = JSON.parse(logLines.join("\n")); + const probesAfterSkip = executedSandboxCommands.filter((c) => c.includes("NEMOCLAW_MCP_PROBE")).length; + logLines.length = 0; + await bridge.dispatchMcpBridgeCommand("alpha", ["status", "--probe", "--json"]); + const forced = JSON.parse(logLines.join("\n")); + process.stdout.write(JSON.stringify({ + probesAfterSkip, + skippedResolution: skipped.provider.credentialResolution ?? null, + forcedResolution: forced.bridges[0].provider.credentialResolution ?? null, + })); +`, + ); + const payload = JSON.parse(stdout) as { + probesAfterSkip: number; + skippedResolution: unknown; + forcedResolution: { ok: boolean | null; httpStatus?: number } | null; + }; + expect(payload.probesAfterSkip).toBe(0); + expect(payload.skippedResolution).toBeNull(); + expect(payload.forcedResolution).toMatchObject({ + ok: null, + httpStatus: 401, + controlHttpStatus: 401, + }); + }); + + it("rejects combining --probe with --no-probe (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-conflict-"); + const { stdout } = runHarness( + home, + String.raw` + await bridge.dispatchMcpBridgeCommand("alpha", ["status", "github", "--probe", "--no-probe"]); + const observedExitCode = process.exitCode ?? 0; + process.exitCode = 0; + process.stdout.write(JSON.stringify({ errorLines, exitCode: observedExitCode })); +`, + ); + const payload = JSON.parse(stdout) as { errorLines: string[]; exitCode: number }; + expect(payload.exitCode).toBe(2); + expect(payload.errorLines.join("\n")).toContain("at most one of --probe / --no-probe"); + }); +}); + +describe("MCP add post-add credential-resolution probe", () => { + it("warns loudly on an identical-rejection probe without failing the committed add (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-add-"); + const { stdout } = runHarness( + home, + String.raw` + const addRestart = require("./src/lib/actions/sandbox/mcp-bridge-add-restart.js"); + addRestart.addMcpBridge = async () => {}; + await bridge.dispatchMcpBridgeCommand("alpha", [ + "add", "github", "--url", "https://api.githubcopilot.com/mcp/", "--env", "GITHUB_TOKEN", + ]); + process.stdout.write(JSON.stringify({ + logLines, + errorLines, + probed: executedSandboxCommands.some((c) => c.includes("NEMOCLAW_MCP_PROBE")), + exitCode: process.exitCode ?? 0, + })); +`, + ); + const payload = JSON.parse(stdout) as { + logLines: string[]; + errorLines: string[]; + probed: boolean; + exitCode: number; + }; + expect(payload.probed).toBe(true); + expect(payload.logLines.some((line) => line.includes("MCP server 'github' added"))).toBe(true); + expect( + payload.errorLines.some( + (line) => + line.includes("WARNING") && line.includes("Credential resolution could not be verified"), + ), + ).toBe(true); + expect(payload.exitCode).toBe(0); + }); + + it("skips post-add probe traffic when policy verification is absent, drifted, or unknown (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-add-policy-gate-"); + const { stdout } = runHarness( + home, + String.raw` + const addRestart = require("./src/lib/actions/sandbox/mcp-bridge-add-restart.js"); + addRestart.addMcpBridge = async () => {}; + const outcomes = []; + for (const policyState of ["absent", "drift", null]) { + activePolicyState = policyState; + executedSandboxCommands.length = 0; + logLines.length = 0; + errorLines.length = 0; + await bridge.dispatchMcpBridgeCommand("alpha", [ + "add", "github", "--url", "https://api.githubcopilot.com/mcp/", "--env", "GITHUB_TOKEN", + ]); + outcomes.push({ + policyState, + probed: executedSandboxCommands.some((c) => c.includes("NEMOCLAW_MCP_PROBE")), + output: [...logLines, ...errorLines].join("\n"), + exitCode: process.exitCode ?? 0, + }); + } + process.stdout.write(JSON.stringify(outcomes)); +`, + ); + const outcomes = JSON.parse(stdout) as Array<{ + policyState: "absent" | "drift" | null; + probed: boolean; + output: string; + exitCode: number; + }>; + expect(outcomes).toHaveLength(3); + for (const outcome of outcomes) { + expect(outcome.probed, String(outcome.policyState)).toBe(false); + expect(outcome.output).toContain("Credential resolution probe was inconclusive"); + expect(outcome.output).toContain("probe skipped"); + expect(outcome.exitCode).toBe(0); + } + }); + + it("keeps the post-add warning for identical 400 explicitly inconclusive (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-add-400-"); + const { stdout } = runHarness( + home, + String.raw` + const addRestart = require("./src/lib/actions/sandbox/mcp-bridge-add-restart.js"); + addRestart.addMcpBridge = async () => {}; + await bridge.dispatchMcpBridgeCommand("alpha", [ + "add", "github", "--url", "https://api.githubcopilot.com/mcp/", "--env", "GITHUB_TOKEN", + ]); + process.stdout.write(JSON.stringify({ errorLines, exitCode: process.exitCode ?? 0 })); +`, + { probeHttpStatus: 400 }, + ); + const payload = JSON.parse(stdout) as { errorLines: string[]; exitCode: number }; + const warning = payload.errorLines.find((line) => line.includes("WARNING")); + expect(warning).toBeDefined(); + expect(warning).toContain("inconclusive even with a valid stored credential"); + expect(warning).toContain("request validation"); + expect(warning).not.toContain("the OpenShell host is not rewriting"); + expect(payload.exitCode).toBe(0); + }); + + it("skips the post-add probe when --no-probe is passed (#6379)", () => { + const home = createTempHome("nemoclaw-mcp-resolution-add-skip-"); + const { stdout } = runHarness( + home, + String.raw` + const addRestart = require("./src/lib/actions/sandbox/mcp-bridge-add-restart.js"); + addRestart.addMcpBridge = async () => {}; + await bridge.dispatchMcpBridgeCommand("alpha", [ + "add", "github", "--url", "https://api.githubcopilot.com/mcp/", "--env", "GITHUB_TOKEN", "--no-probe", + ]); + process.stdout.write(JSON.stringify({ + errorLines, + probed: executedSandboxCommands.some((c) => c.includes("NEMOCLAW_MCP_PROBE")), + exitCode: process.exitCode ?? 0, + })); +`, + ); + const payload = JSON.parse(stdout) as { + errorLines: string[]; + probed: boolean; + exitCode: number; + }; + expect(payload.probed).toBe(false); + expect(payload.errorLines).toHaveLength(0); + expect(payload.exitCode).toBe(0); + }); +}); diff --git a/src/lib/actions/sandbox/mcp-bridge-status.ts b/src/lib/actions/sandbox/mcp-bridge-status.ts index f16018bef76..81ba60bddfd 100644 --- a/src/lib/actions/sandbox/mcp-bridge-status.ts +++ b/src/lib/actions/sandbox/mcp-bridge-status.ts @@ -21,6 +21,10 @@ import { providerMatchesCredential, providerShapeDetail, } from "./mcp-bridge-provider"; +import { + credentialResolutionWarning, + probeCredentialResolution, +} from "./mcp-bridge-resolution-probe"; import { bridgeState, ensureSandboxGatewaySelected, @@ -117,9 +121,19 @@ function getAdapterRegistration( }; } +export interface McpBridgeStatusOptions { + /** + * Run the wire-level credential-resolution probe for each entry (#6379). + * Costs one SSH round trip plus an in-sandbox MCP initialize per entry, so + * the dispatch layer enables it only where the operator asked for it. + */ + probeCredentialResolution?: boolean; +} + export async function statusMcpBridge( sandboxName: string, server?: string, + options: McpBridgeStatusOptions = {}, ): Promise { validateSandboxName(sandboxName); if (server !== undefined) validateMcpServerName(server); @@ -173,6 +187,7 @@ export async function statusMcpBridge( return entries.map(([name, entry]) => { const support = entry ? getPersistedBridgeSupport(entry) : getSupportSummary(agent); const registeredPolicy = getRegisteredGeneratedPolicy(sandboxName, entry); + const policyPresence = getPolicyPresence(sandboxName, entry); const hasCredentialBinding = !!entry && Array.isArray(entry.env) && @@ -208,6 +223,24 @@ export async function statusMcpBridge( } const unsafeCredentialMayBeAttached = !!credentialWarning && !!entry?.providerName && attached !== false; + const credentialResolution = + options.probeCredentialResolution && entry + ? unsafeCredentialMayBeAttached + ? { + ok: null, + detail: + "probe skipped: the unsupported legacy credential may still be attached to fresh sandbox children", + } + : probeCredentialResolution(sandboxName, entry, support.adapter, { + policyGatewayPresent: policyPresence, + providerAttached: attached, + providerCredentialReady, + }) + : undefined; + const resolutionWarning = credentialResolution + ? credentialResolutionWarning(entry?.env[0], credentialResolution) + : undefined; + if (resolutionWarning) warnings.push(resolutionWarning); return { server: name, agent: entry?.agent ?? agent.name, @@ -230,11 +263,12 @@ export async function statusMcpBridge( attached, credentialReady: entry ? providerCredentialReady : null, ...(providerDetail ? { detail: providerDetail } : {}), + ...(credentialResolution ? { credentialResolution } : {}), }, policy: { name: entry?.policyName, registryPresent: !!registeredPolicy, - gatewayPresent: getPolicyPresence(sandboxName, entry), + gatewayPresent: policyPresence, }, adapter: unsafeCredentialMayBeAttached ? { diff --git a/src/lib/actions/sandbox/mcp-bridge.ts b/src/lib/actions/sandbox/mcp-bridge.ts index f56d7965ef9..d31fdfaae1b 100644 --- a/src/lib/actions/sandbox/mcp-bridge.ts +++ b/src/lib/actions/sandbox/mcp-bridge.ts @@ -3,7 +3,11 @@ import type { McpBridgeEntry } from "../../state/registry"; import { addMcpBridge as addMcpBridgeLifecycle } from "./mcp-bridge-add-restart"; -import { type McpBridgeAddOptions, McpBridgeError } from "./mcp-bridge-contracts"; +import { + type McpBridgeAddOptions, + McpBridgeError, + type McpBridgeStatus, +} from "./mcp-bridge-contracts"; import { finalizeMcpBridgesAfterSandboxDelete as finalizeMcpBridgesAfterSandboxDeleteLifecycle, prepareMcpBridgesForAbsentSandboxDestroy as prepareMcpBridgesForAbsentSandboxDestroyLifecycle, @@ -19,6 +23,7 @@ import { } from "./mcp-bridge-rebuild"; import { removeMcpBridge as removeMcpBridgeLifecycle } from "./mcp-bridge-remove"; import { renderMcpBridgeList, renderMcpBridgeStatus } from "./mcp-bridge-render"; +import { credentialResolutionWarning } from "./mcp-bridge-resolution-probe"; import { restartMcpBridge as restartMcpBridgeLifecycle } from "./mcp-bridge-restart"; import { getSandboxAgent, getSandboxOrThrow } from "./mcp-bridge-state"; import { buildJsonSummary, statusMcpBridge } from "./mcp-bridge-status"; @@ -176,6 +181,53 @@ function parseJsonFlag(args: string[]): { json: boolean; rest: string[] } { }; } +function parseProbeFlags(args: string[]): { probe?: boolean; rest: string[] } { + if (args.includes("--probe") && args.includes("--no-probe")) { + throw new McpBridgeError("Pass at most one of --probe / --no-probe.", 2); + } + const probe = args.includes("--probe") ? true : args.includes("--no-probe") ? false : undefined; + return { probe, rest: args.filter((arg) => arg !== "--probe" && arg !== "--no-probe") }; +} + +/** + * Post-add wire probe (#6379). Reports only — the add transaction committed + * durably and a resolution failure is a host-side OpenShell defect, so a + * nonzero exit here would break scripted adds mid-remediation; `mcp status + * ` remains the authoritative recheck. + */ +async function reportAddCredentialResolution(sandboxName: string, server: string): Promise { + let probe: McpBridgeStatus["provider"]["credentialResolution"]; + let credentialEnvName: string | undefined; + try { + const [status] = await statusMcpBridge(sandboxName, server, { + probeCredentialResolution: true, + }); + probe = status?.provider.credentialResolution; + credentialEnvName = status?.env.names[0]; + } catch { + console.log( + " Credential resolution probe was inconclusive: post-add readiness inspection failed; run mcp status for this server to retry.", + ); + return; + } + if (!probe) { + console.log(" Credential resolution probe was inconclusive: post-add status unavailable."); + return; + } + const warning = credentialResolutionWarning(credentialEnvName, probe); + if (probe.ok === true) { + console.log( + ` Credential resolution verified on the wire (HTTP ${probe.httpStatus})${probe.detail ? `: ${probe.detail}` : "."}`, + ); + } else if (warning) { + console.error(` WARNING: ${warning}`); + } else { + console.log( + ` Credential resolution probe was inconclusive${probe.detail ? `: ${probe.detail}` : "."}`, + ); + } +} + function requireNoExtraArgs(args: string[], usage: string): void { if (args.length > 0) throw new McpBridgeError(usage, 2); } @@ -198,6 +250,7 @@ function renderMcpHelp(subcommand: string): void { FLAGS --url URL MCP Streamable HTTP endpoint --env KEY Required host credential reference registered with OpenShell + --no-probe Skip the post-add wire-level credential-resolution probe SECURITY Credentials are registered as an OpenShell provider and appear inside the @@ -213,10 +266,12 @@ FLAGS return; case "status": console.log(`USAGE - nemoclaw mcp status [server] [--json] + nemoclaw mcp status [server] [--json] [--probe|--no-probe] FLAGS - --json Emit MCP server status as JSON`); + --json Emit MCP server status as JSON + --probe Request the wire-level credential-resolution probe for every server + --no-probe Skip the probe (it defaults on only when a single server is named)`); return; case "restart": console.log(`USAGE @@ -251,9 +306,16 @@ export async function dispatchMcpBridgeCommand( } switch (subcommand) { case "add": { - const options = parseMcpAddArgs(rest); + const { probe, rest: addRest } = parseProbeFlags(rest); + if (probe === true) + throw new McpBridgeError( + "Usage: nemoclaw mcp add --url --env KEY [--no-probe]", + 2, + ); + const options = parseMcpAddArgs(addRest); await addMcpBridge(sandboxName, options); console.log(` MCP server '${options.server}' added to sandbox '${sandboxName}'.`); + if (probe !== false) await reportAddCredentialResolution(sandboxName, options.server); return; } case "list": { @@ -268,14 +330,17 @@ export async function dispatchMcpBridgeCommand( return; } case "status": { - const { json, rest: statusRest } = parseJsonFlag(rest); + const { json, rest: statusJsonRest } = parseJsonFlag(rest); + const { probe, rest: statusRest } = parseProbeFlags(statusJsonRest); const server = requireAtMostOneArg( statusRest, - "Usage: nemoclaw mcp status [server] [--json]", + "Usage: nemoclaw mcp status [server] [--json] [--probe|--no-probe]", ); const sandbox = getSandboxOrThrow(sandboxName); const agent = getSandboxAgent(sandbox); - const statuses = await statusMcpBridge(sandboxName, server); + const statuses = await statusMcpBridge(sandboxName, server, { + probeCredentialResolution: probe ?? server !== undefined, + }); if (json) { console.log( JSON.stringify( diff --git a/src/lib/actions/sandbox/sessions/gateway-rpc.ts b/src/lib/actions/sandbox/sessions/gateway-rpc.ts index 827824d90a7..f0702a379e4 100644 --- a/src/lib/actions/sandbox/sessions/gateway-rpc.ts +++ b/src/lib/actions/sandbox/sessions/gateway-rpc.ts @@ -6,6 +6,7 @@ import { captureOpenshell } from "../../../adapters/openshell/runtime"; import { CLI_NAME } from "../../../cli/branding"; import { redactFull } from "../../../security/redact"; import { runSandboxAutoPairApprovalPass } from "../auto-pair-approval"; +import { buildTrustedProxyEnvSourceShell } from "../trusted-proxy-env"; import { type GatewayCallPayload, parseGatewayCallPayload } from "./gateway-rpc-envelope"; export { type GatewayCallPayload, parseGatewayCallPayload } from "./gateway-rpc-envelope"; @@ -127,32 +128,10 @@ const GATEWAY_ADMIN_RPC_SCRIPT_B64 = Buffer.from(GATEWAY_ADMIN_RPC_SCRIPT, "utf8 "base64", ); -function shellSingleQuote(value: string): string { - return `'${value.replaceAll("'", `'"'"'`)}'`; -} - export function buildGatewayAdminRpcShell(proxyEnvPath = "/tmp/nemoclaw-proxy-env.sh"): string { return ` set -e -proxy_env=${shellSingleQuote(proxyEnvPath)} -if [ -e "$proxy_env" ] || [ -L "$proxy_env" ]; then - if [ -L "$proxy_env" ] || [ ! -f "$proxy_env" ]; then - echo "[SECURITY] $proxy_env is unsafe (expected regular root-owned mode 444 file)" >&2 - exit 126 - fi - perms="$(stat -c '%a' "$proxy_env" 2>/dev/null || stat -f '%Lp' "$proxy_env" 2>/dev/null || echo unknown)" - owner="$(stat -c '%U' "$proxy_env" 2>/dev/null || stat -f '%Su' "$proxy_env" 2>/dev/null || echo unknown)" - if [ "$(id -u)" -eq 0 ]; then - if [ "$owner" != "root" ] || [ "$perms" != "444" ]; then - echo "[SECURITY] $proxy_env has unsafe permissions: owner=$owner mode=$perms (expected root:444)" >&2 - exit 126 - fi - elif [ "$perms" != "444" ]; then - echo "[SECURITY] $proxy_env has unsafe permissions: mode=$perms (expected 444)" >&2 - exit 126 - fi - . "$proxy_env" >/dev/null 2>&1 -fi +${buildTrustedProxyEnvSourceShell(proxyEnvPath)} export NEMOCLAW_GATEWAY_RPC_METHOD="$3" export NEMOCLAW_GATEWAY_RPC_PARAMS_B64="$4" exec node --input-type=module --eval "$1" "$2" diff --git a/src/lib/actions/sandbox/trusted-proxy-env.test.ts b/src/lib/actions/sandbox/trusted-proxy-env.test.ts new file mode 100644 index 00000000000..4f2b03326b9 --- /dev/null +++ b/src/lib/actions/sandbox/trusted-proxy-env.test.ts @@ -0,0 +1,86 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { afterEach, describe, expect, it } from "vitest"; + +import { buildTrustedProxyEnvSourceShell } from "./trusted-proxy-env"; + +const tempRoots = new Set(); + +function tempPath(name: string): { file: string; root: string } { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-trusted-proxy-env-")); + tempRoots.add(root); + return { file: path.join(root, name), root }; +} + +function runSource(file: string) { + return spawnSync( + "bash", + [ + "--noprofile", + "--norc", + "-c", + `${buildTrustedProxyEnvSourceShell(file)}\nprintf 'PROXY=[%s]' "\${HTTP_PROXY:-}"`, + ], + { encoding: "utf8", env: { ...process.env, HTTP_PROXY: "" } }, + ); +} + +afterEach(() => { + tempRoots.forEach((root) => fs.rmSync(root, { recursive: true, force: true })); + tempRoots.clear(); +}); + +describe("trusted proxy env source shell", () => { + it("suppresses source output while preserving exported proxy variables", () => { + const { file } = tempPath("proxy-env.sh"); + fs.writeFileSync( + file, + "printf 'NEMOCLAW_MCP_PROBE_HTTP_CODE=200\\n'\nexport HTTP_PROXY=http://proxy.test:3128\n", + { mode: 0o444 }, + ); + fs.chmodSync(file, 0o444); + + const result = runSource(file); + + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toBe("PROXY=[http://proxy.test:3128]"); + expect(result.stdout).not.toContain("NEMOCLAW_MCP_PROBE_HTTP_CODE"); + }); + + it("fails closed for a writable proxy env file", () => { + const { file } = tempPath("proxy-env.sh"); + fs.writeFileSync(file, "export HTTP_PROXY=http://proxy.test:3128\n", { mode: 0o644 }); + fs.chmodSync(file, 0o644); + + const result = runSource(file); + + expect(result.status).toBe(126); + expect(result.stderr).toContain("unsafe permissions"); + expect(result.stdout).not.toContain("PROXY="); + }); + + it("fails closed for a symlink or a source-time error", () => { + const symlinkFixture = tempPath("proxy-env.sh"); + const target = path.join(symlinkFixture.root, "target.sh"); + fs.writeFileSync(target, "export HTTP_PROXY=http://proxy.test:3128\n", { mode: 0o444 }); + fs.symlinkSync(target, symlinkFixture.file); + + const symlinkResult = runSource(symlinkFixture.file); + expect(symlinkResult.status).toBe(126); + expect(symlinkResult.stderr).toContain("expected regular root-owned mode 444 file"); + + const failureFixture = tempPath("proxy-env.sh"); + fs.writeFileSync(failureFixture.file, "return 1\n", { mode: 0o444 }); + fs.chmodSync(failureFixture.file, 0o444); + + const failureResult = runSource(failureFixture.file); + expect(failureResult.status).toBe(126); + expect(failureResult.stderr).toContain("could not be sourced safely"); + }); +}); diff --git a/src/lib/actions/sandbox/trusted-proxy-env.ts b/src/lib/actions/sandbox/trusted-proxy-env.ts new file mode 100644 index 00000000000..6b46f7955bc --- /dev/null +++ b/src/lib/actions/sandbox/trusted-proxy-env.ts @@ -0,0 +1,39 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { shellQuote } from "../../core/shell-quote"; + +const DEFAULT_PROXY_ENV_PATH = "/tmp/nemoclaw-proxy-env.sh"; + +/** + * Validate the cross-user runtime env file before sourcing it and suppress all + * source-time output. Root-mode sandboxes require the root:444 trust posture; + * non-root mode can enforce only the repository's accepted mode-444 boundary + * because privilege separation is disabled there (scripts/lib/sandbox-init.sh). + */ +export function buildTrustedProxyEnvSourceShell(proxyEnvPath = DEFAULT_PROXY_ENV_PATH): string { + return ` +proxy_env=${shellQuote(proxyEnvPath)} +if [ -e "$proxy_env" ] || [ -L "$proxy_env" ]; then + if [ -L "$proxy_env" ] || [ ! -f "$proxy_env" ]; then + echo "[SECURITY] $proxy_env is unsafe (expected regular root-owned mode 444 file)" >&2 + exit 126 + fi + perms="$(stat -c '%a' "$proxy_env" 2>/dev/null || stat -f '%Lp' "$proxy_env" 2>/dev/null || echo unknown)" + owner="$(stat -c '%U' "$proxy_env" 2>/dev/null || stat -f '%Su' "$proxy_env" 2>/dev/null || echo unknown)" + if [ "$(id -u)" -eq 0 ]; then + if [ "$owner" != "root" ] || [ "$perms" != "444" ]; then + echo "[SECURITY] $proxy_env has unsafe permissions: owner=$owner mode=$perms (expected root:444)" >&2 + exit 126 + fi + elif [ "$perms" != "444" ]; then + echo "[SECURITY] $proxy_env has unsafe permissions: mode=$perms (expected 444)" >&2 + exit 126 + fi + if ! . "$proxy_env" >/dev/null 2>&1; then + echo "[SECURITY] $proxy_env could not be sourced safely" >&2 + exit 126 + fi +fi +`.trim(); +}