diff --git a/CHANGELOG.md b/CHANGELOG.md index 2442ff8..13aa4f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,27 @@ All notable changes to `codex-plugin-doctor` are documented here. This changelog groups the shipped work into product-level release blocks instead of repeating every low-level git diff in isolation. +## [1.52.0] - 2026-07-25 + +### Added + +- added remote Streamable HTTP initialize readiness with explicit `--allow-network` and loopback-only `--allow-local-network` consent +- added GitHub Action inputs and documentation for remote runtime consent controls + +### Changed + +- extended runtime plans, policy, reports, scorecards, and output contracts with remote MCP readiness results +- limited OAuth handling to bounded metadata discovery without authentication or redirect following + +### Fixed + +- preserved release-evidence compatibility when runtime readiness data is present + +### Security + +- added SSRF-safe bounded client and peer/DNS checks for remote MCP readiness probes +- remediated development-only `postcss` and `nanoid` audit findings + ## [1.51.0] - 2026-07-24 ### Added diff --git a/README.md b/README.md index 14e1d48..2687291 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,17 @@ Runtime MCP validation with `--runtime`: - optional runtime approval gating with a precomputed `doctor runtime-plan` digest - optional Docker isolation for local Node.js stdio servers with `--sandbox docker` +### Remote MCP Readiness + +Remote MCP runtime probing is disabled until you explicitly pass `--allow-network`. Local endpoints also require `--allow-local-network`: + +```bash +codex-plugin-doctor check ./remote-mcp --runtime --allow-network +codex-plugin-doctor check ./remote-mcp --runtime --allow-network --allow-local-network +``` + +The probe makes only bounded, read-only protocol and OAuth metadata-discovery requests, redacts report output, and applies SSRF controls. It does not authenticate or follow redirects. See [Remote MCP Readiness](./docs/architecture/remote-mcp-readiness.md). + Output formats: - human text output @@ -427,9 +438,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 - - uses: Esquetta/CodexPluginDoctor@v1.51.0 + - uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" path: . runtime: "true" policy: codex-publish diff --git a/action.yml b/action.yml index 417d2b8..38eca5c 100644 --- a/action.yml +++ b/action.yml @@ -14,6 +14,14 @@ inputs: description: Run optional MCP runtime probing. required: false default: "false" + allow-network: + description: Explicitly allow remote MCP runtime probes to make outbound network requests. + required: false + default: "false" + allow-local-network: + description: Explicitly allow remote MCP runtime probes to contact loopback endpoints only (localhost, 127.0.0.0/8, or ::1). Private, link-local, multicast, unspecified, reserved, and NAT64 ranges remain blocked. + required: false + default: "false" installed: description: Validate plugins from the local Codex plugin cache. required: false @@ -160,6 +168,8 @@ runs: id: run-doctor shell: bash env: + ALLOW_NETWORK_INPUT: ${{ inputs['allow-network'] }} + ALLOW_LOCAL_NETWORK_INPUT: ${{ inputs['allow-local-network'] }} CORPUS_METRICS_MANIFEST_INPUT: ${{ inputs['corpus-metrics-manifest'] }} CORPUS_METRICS_BASELINE_INPUT: ${{ inputs['corpus-metrics-baseline'] }} CORPUS_METRICS_FAIL_ON_REGRESSION_INPUT: ${{ inputs['corpus-metrics-fail-on-regression'] }} @@ -199,6 +209,14 @@ runs: args+=(--runtime) fi + if [[ "$ALLOW_NETWORK_INPUT" == "true" ]]; then + args+=(--allow-network) + fi + + if [[ "$ALLOW_LOCAL_NETWORK_INPUT" == "true" ]]; then + args+=(--allow-local-network) + fi + if [[ -n "${{ inputs.config }}" ]]; then args+=(--config "${{ inputs.config }}") fi diff --git a/docs/README.md b/docs/README.md index 116e482..c1edf02 100644 --- a/docs/README.md +++ b/docs/README.md @@ -9,6 +9,7 @@ This directory contains public documentation for users, contributors, and securi - [Suppression Management](architecture/suppression-management.md) - [Runtime Sandbox and External Corpus](architecture/runtime-sandbox-and-external-corpus.md) - [MCP 2025-11 Conformance](architecture/mcp-2025-11-conformance.md) +- [Remote MCP Readiness](architecture/remote-mcp-readiness.md) - [Real-World Corpus Quality Metrics](architecture/real-world-corpus-quality-metrics.md) - [Corpus Metrics Regression Diff](architecture/corpus-metrics-regression-diff.md) diff --git a/docs/architecture/mcp-2025-11-conformance.md b/docs/architecture/mcp-2025-11-conformance.md index 0f5c264..552a406 100644 --- a/docs/architecture/mcp-2025-11-conformance.md +++ b/docs/architecture/mcp-2025-11-conformance.md @@ -27,7 +27,7 @@ This feature adds version-aware, read-only conformance checks to the existing ru - accepting or rejecting elicitation requests - handling URL-mode elicitation in a browser - servicing `sampling/createMessage` requests -- validating remote HTTP authorization or OAuth discovery +- performing remote OAuth metadata discovery as part of conformance; remote runtime readiness performs metadata discovery only - adding a new top-level CLI command - requiring older servers to implement capabilities introduced after their negotiated version - building a generic external rule-pack engine diff --git a/docs/architecture/remote-mcp-readiness.md b/docs/architecture/remote-mcp-readiness.md new file mode 100644 index 0000000..d6ec781 --- /dev/null +++ b/docs/architecture/remote-mcp-readiness.md @@ -0,0 +1,36 @@ +# Remote MCP Readiness + +## Purpose + +Codex Plugin Doctor can make bounded, read-only checks against a remote MCP endpoint. This is an opt-in readiness check, not a general remote MCP client. + +## Explicit Consent + +Remote requests require `--runtime --allow-network`. `--allow-local-network` is a second opt-in for loopback endpoints only (`localhost`, `127.0.0.0/8`, or `::1`). Private, link-local, multicast, unspecified, reserved, and NAT64 ranges remain blocked. + +```bash +codex-plugin-doctor check ./plugin --runtime --allow-network +codex-plugin-doctor check ./plugin --runtime --allow-network --allow-local-network +``` + +The same consent is available in the GitHub Action through `allow-network: "true"` and, only when needed, `allow-local-network: "true"`. Keep both inputs false for ordinary static validation. + +## Read-Only Scope + +The probe uses a bounded HTTP request for MCP initialization and only follows the OAuth metadata-discovery path advertised by an unauthenticated challenge. It never sends credentials or tokens, and reporting redacts sensitive values, response bodies, session identifiers, and authorization metadata. + +## SSRF Controls + +Before connecting, the CLI requires an absolute HTTP or HTTPS URL without credentials, query strings, fragments, or numeric IP literals. It resolves hostnames and rejects non-public targets. The local-network exception permits loopback only; private, link-local, multicast, unspecified, reserved, cloud-metadata, and NAT64 destinations remain blocked. Requests have fixed size and time limits and do not follow redirects. + +These checks reduce SSRF exposure but cannot account for every network topology. In particular, arbitrary network-specific NAT64 Pref64 mappings can change an address's effective route. Apply runner or host egress controls as the final boundary. + +## Out Of Scope + +- authenticated OAuth +- custom headers +- remote tool/resource/prompt/task calls +- GET SSE/resumability +- redirects + +Use a dedicated MCP client with its own authorization and network policy when any of these capabilities are required. diff --git a/docs/guides/github-action.md b/docs/guides/github-action.md index db5104f..5d052b2 100644 --- a/docs/guides/github-action.md +++ b/docs/guides/github-action.md @@ -6,6 +6,21 @@ Use the Codex Plugin Doctor GitHub Action when a plugin repository should fail p The action installs `codex-plugin-doctor` from npm, then runs the same CLI used locally. +## Remote MCP Runtime Probing + +Remote MCP checks are off by default. Set `runtime: "true"` and give explicit network consent only for endpoints you trust. Use `allow-local-network: "true"` for loopback endpoints only (`localhost`, `127.0.0.0/8`, or `::1`). Private, link-local, multicast, unspecified, reserved, and NAT64 ranges remain blocked. + +```yaml +- uses: ./ + with: + path: . + runtime: "true" + allow-network: "true" + allow-local-network: "true" # Remove for public endpoints. +``` + +The Action transfers these boolean inputs through environment-backed shell variables and a Bash argument array. Remote probes remain read-only and redact diagnostics; see [Remote MCP Readiness](../architecture/remote-mcp-readiness.md) for SSRF and OAuth metadata-discovery boundaries. + ## Recommended Workflow ```yaml @@ -22,9 +37,9 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 - - uses: Esquetta/CodexPluginDoctor@v1.51.0 + - uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" path: . runtime: "true" policy: codex-publish @@ -51,9 +66,9 @@ Every action run also writes `codex-plugin-doctor-action-manifest.json`. The man Use SARIF when repository security tooling should ingest validation findings. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" path: . sarif: "true" ``` @@ -65,9 +80,9 @@ The action writes `codex-plugin-doctor.sarif` into `output-dir`. Uploading it to Use artifact and summary controls when the workflow needs custom retention or wants to disable generated report uploads. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" path: . output-dir: doctor-ci-reports artifact-name: codex-plugin-doctor-reports @@ -102,11 +117,11 @@ The action also exposes these workflow outputs for follow-up steps: Use review bundle artifacts when a pull request or release workflow should preserve signed runtime approval, runtime policy, attestation, and release evidence handoff files. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 env: CODEX_PLUGIN_DOCTOR_SIGNING_KEY: ${{ secrets.CODEX_PLUGIN_DOCTOR_SIGNING_KEY }} with: - version: "1.51.0" + version: "1.52.0" path: . review-bundle: "true" review-bundle-verify: "true" @@ -137,9 +152,9 @@ The CLI can produce badge output for release notes, README automation, or a stat Use a private corpus metrics manifest to measure reviewed precision, recall, and false-positive share in CI. The action writes only the public-safe metrics report into its artifact directory; snapshots, manifest contents, local paths, and review notes are not copied. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" path: . corpus-metrics-manifest: ../private-corpus/metrics.json ``` @@ -147,9 +162,9 @@ Use a private corpus metrics manifest to measure reviewed precision, recall, and This writes `corpus-metrics.json`. To compare the result with a retained report and fail the job on regression: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" path: . corpus-metrics-manifest: ../private-corpus/metrics.json corpus-metrics-baseline: .doctor-baselines/corpus-metrics.json @@ -178,9 +193,9 @@ The history file is newline-delimited JSON. Store it as an artifact, cache, or r The composite action can also append history directly: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" path: . runtime: "true" history: validation-history.jsonl @@ -200,9 +215,9 @@ Use profiles when a consuming workflow needs a named validation policy instead o The composite action can pass profiles directly: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" path: . profile: publish ``` @@ -212,9 +227,9 @@ The composite action can pass profiles directly: Use policy presets when a workflow should apply one of the opinionated release gates without adding a local `.codex-doctor.json`. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" path: . policy: codex-publish ``` @@ -226,9 +241,9 @@ Supported policy values are `codex-publish`, `mcp-strict`, and `security`. The C Use installed-cache mode only in environments where Codex plugins are already available on the runner. ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" installed: "true" filter: github runtime: "false" @@ -239,9 +254,9 @@ Use installed-cache mode only in environments where Codex plugins are already av Pin both the action ref and npm package version for reproducible CI: ```yaml -- uses: Esquetta/CodexPluginDoctor@v1.51.0 +- uses: Esquetta/CodexPluginDoctor@v1.52.0 with: - version: "1.51.0" + version: "1.52.0" ``` Use `version: "latest"` only when the consuming repository intentionally wants automatic CLI upgrades. diff --git a/docs/rules/catalog.md b/docs/rules/catalog.md index 73e3dbe..72cec79 100644 --- a/docs/rules/catalog.md +++ b/docs/rules/catalog.md @@ -38,6 +38,8 @@ codex-plugin-doctor explain plugin.manifest.missing | `plugin.mcp.invalid_shape` | fail | MCP config does not contain a valid `mcpServers` object. | | `plugin.mcp.server.invalid` | fail | MCP server entry is not an object. | | `plugin.mcp.server.transport.missing` | fail | MCP server entry is missing both `command` and `url`. | +| `mcp.server.transport.conflict` | fail | An MCP server defines both command and URL transports. | +| `plugin.mcp.server.transport.conflict` | fail | A bundled MCP server defines both command and URL transports. | ## Security Rules @@ -52,7 +54,13 @@ codex-plugin-doctor explain plugin.manifest.missing | `plugin.security.path_traversal_risk` | fail | MCP server passes a package-external path to a path-like runtime argument. | | `plugin.security.dangerous_env_usage` | fail | MCP server sets an environment variable that can alter code loading. | | `plugin.security.cwd_outside_root` | fail | MCP server `cwd` resolves outside the plugin package root. | -| `plugin.security.insecure_http_url` | warn | MCP server uses a plain HTTP URL. | +| `plugin.security.insecure_http_url` | fail | MCP server uses a plain HTTP URL. | +| `plugin.security.remote_mcp_url.invalid` | fail | An MCP server URL is not an absolute HTTP or HTTPS URL. | +| `plugin.security.remote_mcp_url.unsupported_scheme` | fail | An MCP server URL uses an unsupported scheme. | +| `plugin.security.remote_mcp_url.credentials` | fail | An MCP server URL embeds credentials. | +| `plugin.security.remote_mcp_url.query` | fail | An MCP server URL contains a query string. | +| `plugin.security.remote_mcp_url.fragment` | fail | An MCP server URL contains a fragment. | +| `plugin.security.remote_mcp_url.ip_literal` | fail | An MCP server URL uses a numeric IP literal. | | `plugin.security.prompt_injection_text` | fail | Packaged text contains prompt-injection or secret-exfiltration instructions. | ## Runtime Rules diff --git a/docs/security/security-architecture.md b/docs/security/security-architecture.md index 0133711..f1b78d9 100644 --- a/docs/security/security-architecture.md +++ b/docs/security/security-architecture.md @@ -46,6 +46,12 @@ Run structural and config checks before any runtime command execution. This is an approval gate, not a sandbox. It reduces accidental or unreviewed execution, but it does not isolate the process after launch. +### Remote MCP Network Boundary + +Remote probing requires explicit network consent and separately requires `--allow-local-network` consent for loopback endpoints only. Private, link-local, multicast, unspecified, reserved, and NAT64 ranges remain blocked. Before each request, the CLI validates the URL and resolved addresses to block credentials, query and fragment components, numeric IP literals, loopback and private ranges, link-local and cloud-metadata ranges, and other SSRF targets. Requests are bounded, redirect-free, and redacted in reports. + +DNS and IP classification cannot eliminate arbitrary network-specific NAT64 Pref64 mappings. Use runner or host egress controls to limit the destinations that a CI job or workstation can reach. + ### Secret Hygiene - redact values that look like tokens in reports diff --git a/package-lock.json b/package-lock.json index bd849af..68e1f2a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "codex-plugin-doctor", - "version": "1.51.0", + "version": "1.52.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "codex-plugin-doctor", - "version": "1.51.0", + "version": "1.52.0", "license": "MIT", "bin": { "codex-plugin-doctor": "dist/cli.js" @@ -1218,9 +1218,9 @@ "license": "MIT" }, "node_modules/nanoid": { - "version": "3.3.11", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.11.tgz", - "integrity": "sha512-N8SpfPUnUp1bK+PMYW8qSWdl9U+wwNWI4QKxOYDy9JAro3WMX7p2OeVRF9v+347pnakNevPmiHhNmZ2HbFA76w==", + "version": "3.3.16", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz", + "integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==", "dev": true, "funding": [ { @@ -1274,9 +1274,9 @@ } }, "node_modules/postcss": { - "version": "8.5.10", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.10.tgz", - "integrity": "sha512-pMMHxBOZKFU6HgAZ4eyGnwXF/EvPGGqUr0MnZ5+99485wwW41kW91A4LOGxSHhgugZmSChL5AlElNdwlNgcnLQ==", + "version": "8.5.23", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.5.23.tgz", + "integrity": "sha512-g50586zr4bZmwFiTlflMu8E0bDTb5I5gertgwAKmsdUlTQIhZtunzUlD1WSzwcVWPoAVpsrA6vlfCD7oXvRwgg==", "dev": true, "funding": [ { @@ -1294,7 +1294,7 @@ ], "license": "MIT", "dependencies": { - "nanoid": "^3.3.11", + "nanoid": "^3.3.16", "picocolors": "^1.1.1", "source-map-js": "^1.2.1" }, diff --git a/package.json b/package.json index f9d0cd6..a6f296c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codex-plugin-doctor", - "version": "1.51.0", + "version": "1.52.0", "description": "CLI-first validator for Codex plugins, skills, and MCP package surfaces with runtime MCP protocol validation.", "type": "module", "main": "./dist/index.js", diff --git a/src/core/bounded-http-client.ts b/src/core/bounded-http-client.ts new file mode 100644 index 0000000..df6114b --- /dev/null +++ b/src/core/bounded-http-client.ts @@ -0,0 +1,320 @@ +import * as http from "node:http"; +import * as https from "node:https"; +import { BlockList, isIP } from "node:net"; + +import { resolveRemoteTarget, type RemoteLookup, type ResolvedRemoteTarget } from "./remote-network-policy.js"; + +const DEFAULT_TIMEOUT_MS = 3_000; +const DEFAULT_MAX_RESPONSE_BYTES = 1_024 * 1_024; +const allowedRequestHeaders = new Set([ + "accept", + "content-type", + "mcp-protocol-version", + "mcp-session-id", + "user-agent" +]); +const safeResponseHeaders = new Set([ + "content-type", + "www-authenticate", + "mcp-session-id", + "mcp-protocol-version", + "location" +]); + +export interface BoundedHttpRequestOptions { + allowLocalNetwork?: boolean; + lookup?: RemoteLookup; + timeoutMs?: number; + maxResponseBytes?: number; + method?: string; + body?: string | Buffer; + headers?: Record; + stopAfter?: (body: Buffer) => boolean; +} + +export interface BoundedHttpResponse { + statusCode: number; + headers: Record; + body: Buffer; +} + +export class BoundedHttpError extends Error { + constructor( + readonly code: + | "REMOTE_HTTP_ENCODING_UNSUPPORTED" + | "REMOTE_HTTP_HEADER_FORBIDDEN" + | "REMOTE_HTTP_OPTIONS_INVALID" + | "REMOTE_HTTP_PEER_MISMATCH" + | "REMOTE_HTTP_REDIRECT" + | "REMOTE_HTTP_REQUEST_FAILED" + | "REMOTE_HTTP_RESPONSE_TOO_LARGE" + | "REMOTE_HTTP_STOP_CONDITION_FAILED" + | "REMOTE_HTTP_TIMEOUT" + | "REMOTE_HTTP_URL_CREDENTIALS" + | "REMOTE_HTTP_URL_UNSUPPORTED", + message: string, + readonly statusCode?: number, + readonly headers?: Record + ) { + super(message); + this.name = "BoundedHttpError"; + } +} + +function validateHeaders(headers: BoundedHttpRequestOptions["headers"]): Record { + const validated: Record = {}; + for (const [name, value] of Object.entries(headers ?? {})) { + if (value === undefined) { + continue; + } + + const normalizedName = name.toLowerCase(); + if (!allowedRequestHeaders.has(normalizedName)) { + throw new BoundedHttpError( + "REMOTE_HTTP_HEADER_FORBIDDEN", + `Remote HTTP request header is not allowed: ${normalizedName}.` + ); + } + if (normalizedName in validated || typeof value !== "string") { + throw new BoundedHttpError( + "REMOTE_HTTP_HEADER_FORBIDDEN", + `Remote HTTP request header must be a single valid string: ${normalizedName}.` + ); + } + try { + http.validateHeaderValue(normalizedName, value); + } catch { + throw new BoundedHttpError( + "REMOTE_HTTP_HEADER_FORBIDDEN", + `Remote HTTP request header must be a single valid string: ${normalizedName}.` + ); + } + validated[normalizedName] = value; + } + return validated; +} + +function validateOptions(options: BoundedHttpRequestOptions): { + timeoutMs: number; + maxResponseBytes: number; +} { + const timeoutMs = options.timeoutMs ?? DEFAULT_TIMEOUT_MS; + const maxResponseBytes = options.maxResponseBytes ?? DEFAULT_MAX_RESPONSE_BYTES; + if (!Number.isInteger(timeoutMs) || timeoutMs < 1 || timeoutMs > DEFAULT_TIMEOUT_MS + || !Number.isInteger(maxResponseBytes) || maxResponseBytes < 1 || maxResponseBytes > DEFAULT_MAX_RESPONSE_BYTES) { + throw new BoundedHttpError( + "REMOTE_HTTP_OPTIONS_INVALID", + "Remote HTTP request options are invalid." + ); + } + return { timeoutMs, maxResponseBytes }; +} + +async function resolveWithinDeadline( + url: URL, + options: BoundedHttpRequestOptions, + timeoutMs: number +): Promise { + let timer: NodeJS.Timeout | undefined; + try { + return await Promise.race([ + resolveRemoteTarget(url, { + allowLocalNetwork: options.allowLocalNetwork, + lookup: options.lookup + }), + new Promise((_resolve, reject) => { + timer = setTimeout(() => { + reject(new BoundedHttpError("REMOTE_HTTP_TIMEOUT", "Remote HTTP request timed out.")); + }, timeoutMs); + }) + ]); + } finally { + if (timer !== undefined) { + clearTimeout(timer); + } + } +} + +function selectSafeHeaders(headers: http.IncomingHttpHeaders): Record { + const selected: Record = {}; + for (const name of safeResponseHeaders) { + const value = headers[name]; + if (value !== undefined) { + selected[name] = value; + } + } + return selected; +} + +function hasIdentityContentEncoding(headers: http.IncomingHttpHeaders): boolean { + const value = headers["content-encoding"]; + const values = Array.isArray(value) ? value : value === undefined ? [] : [value]; + return values.every((entry) => entry.toLowerCase() === "identity"); +} + +function matchesTargetPeer(target: ResolvedRemoteTarget, remoteAddress: string | undefined): boolean { + if (remoteAddress === undefined || isIP(remoteAddress) === 0) { + return false; + } + + const peers = new BlockList(); + peers.addAddress(target.address, target.family === 4 ? "ipv4" : "ipv6"); + if (target.family === 4) { + peers.addAddress(`::ffff:${target.address}`, "ipv6"); + } + + return peers.check(remoteAddress, isIP(remoteAddress) === 4 ? "ipv4" : "ipv6"); +} + +export async function requestBoundedHttp( + rawUrl: string, + options: BoundedHttpRequestOptions = {} +): Promise { + let url: URL; + try { + url = new URL(rawUrl); + } catch { + throw new BoundedHttpError("REMOTE_HTTP_URL_UNSUPPORTED", "Remote HTTP URL must be absolute HTTP or HTTPS."); + } + + if (url.protocol !== "http:" && url.protocol !== "https:") { + throw new BoundedHttpError("REMOTE_HTTP_URL_UNSUPPORTED", "Remote HTTP URL must be absolute HTTP or HTTPS."); + } + if (url.username || url.password) { + throw new BoundedHttpError("REMOTE_HTTP_URL_CREDENTIALS", "Remote HTTP URL must not include credentials."); + } + + const { timeoutMs, maxResponseBytes } = validateOptions(options); + const headers = validateHeaders(options.headers); + const deadline = Date.now() + timeoutMs; + const target = await resolveWithinDeadline(url, options, timeoutMs); + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) { + throw new BoundedHttpError("REMOTE_HTTP_TIMEOUT", "Remote HTTP request timed out."); + } + const transport = url.protocol === "https:" ? https : http; + + return new Promise((resolve, reject) => { + let request: http.ClientRequest | undefined; + let response: http.IncomingMessage | undefined; + let settled = false; + const timeout = setTimeout(() => { + fail(new BoundedHttpError("REMOTE_HTTP_TIMEOUT", "Remote HTTP request timed out.")); + }, remainingMs); + + const fail = (error: Error): void => { + if (settled) { + return; + } + settled = true; + clearTimeout(timeout); + response?.destroy(); + request?.destroy(); + reject(error); + }; + + const complete = (body: Buffer): void => { + if (settled) { + return; + } + settled = true; + clearTimeout(timeout); + response?.destroy(); + request?.destroy(); + resolve({ + statusCode: response?.statusCode ?? 0, + headers: response ? selectSafeHeaders(response.headers) : {}, + body + }); + }; + + try { + request = transport.request({ + protocol: url.protocol, + hostname: url.hostname, + port: url.port || undefined, + path: `${url.pathname}${url.search}`, + method: options.method ?? "GET", + headers, + agent: false, + lookup: (_hostname, lookupOptions, callback) => { + if (lookupOptions.all) { + callback(null, [{ address: target.address, family: target.family }]); + return; + } + callback(null, target.address, target.family); + } + }); + } catch { + fail(new BoundedHttpError("REMOTE_HTTP_REQUEST_FAILED", "Remote HTTP request failed.")); + return; + } + + request.once("error", () => { + fail(new BoundedHttpError("REMOTE_HTTP_REQUEST_FAILED", "Remote HTTP request failed.")); + }); + request.once("response", (incoming) => { + response = incoming; + const safeHeaders = selectSafeHeaders(incoming.headers); + + if (!matchesTargetPeer(target, incoming.socket.remoteAddress)) { + fail(new BoundedHttpError("REMOTE_HTTP_PEER_MISMATCH", "Remote HTTP peer did not match the resolved target.")); + return; + } + if ((incoming.statusCode ?? 0) >= 300 && (incoming.statusCode ?? 0) < 400) { + fail(new BoundedHttpError( + "REMOTE_HTTP_REDIRECT", + "Remote HTTP redirects are not allowed.", + incoming.statusCode, + safeHeaders + )); + return; + } + if (!hasIdentityContentEncoding(incoming.headers)) { + fail(new BoundedHttpError( + "REMOTE_HTTP_ENCODING_UNSUPPORTED", + "Remote HTTP response content encoding must be identity." + )); + return; + } + + const chunks: Buffer[] = []; + let receivedBytes = 0; + incoming.on("data", (chunk: Buffer) => { + receivedBytes += chunk.length; + if (receivedBytes > maxResponseBytes) { + fail(new BoundedHttpError( + "REMOTE_HTTP_RESPONSE_TOO_LARGE", + "Remote HTTP response exceeded the configured size limit." + )); + return; + } + chunks.push(chunk); + const body = Buffer.concat(chunks); + let shouldStop: boolean; + try { + shouldStop = options.stopAfter?.(body) ?? false; + } catch { + fail(new BoundedHttpError( + "REMOTE_HTTP_STOP_CONDITION_FAILED", + "Remote HTTP stop condition failed." + )); + return; + } + if (shouldStop) { + complete(body); + return; + } + }); + incoming.once("error", () => { + fail(new BoundedHttpError("REMOTE_HTTP_REQUEST_FAILED", "Remote HTTP request failed.")); + }); + incoming.once("end", () => { + if (!settled) { + complete(Buffer.concat(chunks)); + } + }); + }); + request.end(options.body); + }); +} diff --git a/src/core/output-contract.ts b/src/core/output-contract.ts index b546f89..c3e7d7f 100644 --- a/src/core/output-contract.ts +++ b/src/core/output-contract.ts @@ -81,6 +81,28 @@ const runtimeConformanceSchema = { additionalProperties: false }; +const remoteRuntimeScorecardSchema = { + type: "object", + properties: { + transport: runtimeCapabilityStatusSchema, + networkSafety: runtimeCapabilityStatusSchema, + initialize: runtimeCapabilityStatusSchema, + contentType: runtimeCapabilityStatusSchema, + session: { + type: "string", + enum: ["absent", "present-valid", "present-invalid"] + }, + protocolHeaders: runtimeCapabilityStatusSchema, + authorization: runtimeCapabilityStatusSchema, + overall: { + type: "string", + enum: ["pass", "warn", "fail", "skipped"] + } + }, + required: ["transport", "networkSafety", "initialize", "contentType", "session", "protocolHeaders", "authorization", "overall"], + additionalProperties: false +}; + const runtimeScorecardSchema = { type: "object", properties: { @@ -92,7 +114,8 @@ const runtimeScorecardSchema = { resourceTemplatesList: runtimeCapabilityStatusSchema, promptsList: runtimeCapabilityStatusSchema, promptGet: runtimeCapabilityStatusSchema, - conformance: runtimeConformanceSchema + conformance: runtimeConformanceSchema, + remote: remoteRuntimeScorecardSchema }, required: [ "initialize", diff --git a/src/core/release-check.ts b/src/core/release-check.ts index b4948a3..60fd61e 100644 --- a/src/core/release-check.ts +++ b/src/core/release-check.ts @@ -42,6 +42,8 @@ export interface BuildReleaseCheckOptions { env?: Record; platform?: NodeJS.Platform; runtime?: boolean; + allowNetwork?: boolean; + allowLocalNetwork?: boolean; runtimeSandbox?: RuntimeSandboxMode; runCheck?: (targetPath: string, options: CheckOptions) => Promise; } @@ -173,6 +175,8 @@ export async function buildReleaseCheck( const runtimeProbeEnabled = options.runtime ?? false; const validationResult = await (options.runCheck ?? validatePlugin)(resolvedPath, { runtime: runtimeProbeEnabled, + ...(options.allowNetwork ? { allowNetwork: true } : {}), + ...(options.allowLocalNetwork ? { allowLocalNetwork: true } : {}), ...(options.runtimeSandbox ? { runtimeSandbox: options.runtimeSandbox } : {}) }); const securityResult = await buildSecurityAudit(resolvedPath); diff --git a/src/core/release-evidence.ts b/src/core/release-evidence.ts index 3f8bef0..71031f5 100644 --- a/src/core/release-evidence.ts +++ b/src/core/release-evidence.ts @@ -32,6 +32,7 @@ import { buildDoctorRuntimePlan, evaluateRuntimeApproval, runtimeApprovalPassed, + type DoctorRuntimePlan, type RuntimeApprovalReport } from "./runtime-plan.js"; import type { CompatibilityEnvironment } from "../compatibility/compatibility-matrix.js"; @@ -158,6 +159,8 @@ export interface BuildDoctorReleaseEvidenceOptions { requireRuntimeApproval?: boolean; runtimeApprovalDigest?: string | null; runtime?: boolean; + allowNetwork?: boolean; + allowLocalNetwork?: boolean; sandbox?: RuntimeSandboxMode; environment?: CompatibilityEnvironment; runCheck?: (targetPath: string, options?: CheckOptions) => Promise; @@ -381,16 +384,138 @@ function signReleaseEvidence( }; } +function buildRuntimeApprovalFailureReport( + rootPath: string, + runtimePlan: DoctorRuntimePlan, + runtimeApproval: RuntimeApprovalReport, + options: BuildDoctorReleaseEvidenceOptions +): DoctorReleaseEvidenceReport { + const generatedAt = new Date().toISOString(); + const emptyDigest = sha256(""); + const failureMessage = "Release evidence checks were not run because runtime approval did not pass."; + const report: Omit = { + schemaVersion: "1.0.0", + kind: "doctor.release.evidence", + generatedAt, + version: packageVersion, + targetPath: rootPath, + status: "fail", + exitCode: 1, + releaseReady: false, + summary: { + attestation: "fail", + attestationVerification: "fail", + corpus: "fail", + performance: "fail", + releaseGates: "fail", + runtimeApproval: "fail", + security: "fail", + trust: "fail" + }, + releaseGates: { + status: "fail", + checks: [{ id: "runtime.approval", status: "fail", message: runtimeApproval.message }] + }, + runtimeApproval, + ...(options.runtime ? { execution: runtimePlan.execution } : {}), + package: { name: null, version: null, private: null }, + git: { commit: null, tag: null, dirty: null }, + attestation: { + schemaVersion: "1.0.0", + kind: "doctor.attestation", + generatedAt, + version: packageVersion, + targetPath: rootPath, + subject: { name: "unavailable", version: null, description: null }, + packageFingerprint: { algorithm: "sha256", digest: emptyDigest, files: { total: 0, bytes: 0 } }, + reportDigest: { algorithm: "sha256", digest: emptyDigest }, + summary: { + status: "fail", + validation: { status: "fail", findingCount: 0 }, + security: { status: "fail", score: 0, findingCount: 0 }, + compatibility: { failedClients: [] }, + trust: { status: "fail", score: 0, findingCount: 0 }, + recommendations: { actionCount: 0 } + }, + verification: { recomputeCommand: "", notes: [failureMessage] }, + signature: { status: "unsigned", reason: failureMessage } + }, + attestationVerification: { + schemaVersion: "1.0.0", + kind: "doctor.attestation.verification", + generatedAt, + artifactPath: "inline:doctor.release-evidence.attestation", + targetPath: rootPath, + status: "fail", + exitCode: 1, + summary: { packageFingerprint: "fail", reportDigest: "fail", signature: "fail" }, + unsignedFields: [], + checks: [{ id: "runtime.approval", status: "fail", message: runtimeApproval.message }] + }, + corpus: { + schemaVersion: "1.0.0", + kind: "doctor.validation.corpus", + generatedAt, + version: packageVersion, + summary: { status: "fail", caseCount: 0, passedExpectations: 0, failedExpectations: 0, runtimeCases: 0 }, + cases: [] + }, + performance: { + schemaVersion: "1.0.0", + kind: "doctor.perf", + generatedAt, + targetPath: rootPath, + status: "fail", + exitCode: 1, + summary: { + stageCount: 0, + slowestStage: "total", + totalDurationMs: 0, + validationStatus: "fail", + securityStatus: "fail", + trustScore: 0, + compatibilityFailures: 0, + thresholdFailures: 0 + }, + stages: [], + thresholds: [] + }, + security: { status: "fail", score: 0, findingCounts: { fail: 0, warn: 0, total: 0 } }, + trust: { status: "fail", score: 0, findingCounts: { fail: 0, warn: 0, total: 0 } } + }; + + return { + ...report, + evidenceSignature: signReleaseEvidence(report, options.signingKey, `env:${options.signingKeyEnv}`) + }; +} + export async function buildDoctorReleaseEvidenceReport( targetPath: string, options: BuildDoctorReleaseEvidenceOptions ): Promise { const rootPath = path.resolve(targetPath); + const runtimePlan = await buildDoctorRuntimePlan( + rootPath, + new Date().toISOString(), + options.sandbox ? { sandbox: options.sandbox } : {} + ); + const runtimeApproval = evaluateRuntimeApproval(runtimePlan, { + required: options.requireRuntimeApproval ?? false, + approvedDigest: options.runtimeApprovalDigest + }); + + if (!runtimeApprovalPassed(runtimeApproval)) { + return buildRuntimeApprovalFailureReport(rootPath, runtimePlan, runtimeApproval, options); + } + const security = await buildSecurityAudit(rootPath); const runCheck = options.runCheck ?? validatePlugin; const checkOptions: CheckOptions = options.runtime ? { runtime: true, + ...(options.allowNetwork ? { allowNetwork: true } : {}), + ...(options.allowLocalNetwork ? { allowLocalNetwork: true } : {}), ...(options.sandbox ? { runtimeSandbox: options.sandbox } : {}) } : {}; @@ -400,8 +525,7 @@ export async function buildDoctorReleaseEvidenceReport( performance, trust, packageMetadata, - git, - runtimePlan + git ] = await Promise.all([ buildDoctorAttestation(rootPath, { signingKey: options.signingKey, @@ -416,12 +540,7 @@ export async function buildDoctorReleaseEvidenceReport( }), buildTrustScore(rootPath, { securityAudit: security }), readPackageMetadata(rootPath), - readGitMetadata(rootPath), - buildDoctorRuntimePlan( - rootPath, - new Date().toISOString(), - options.sandbox ? { sandbox: options.sandbox } : {} - ) + readGitMetadata(rootPath) ]); const normalizedPackageMetadata = { name: packageMetadata.name ?? attestation.subject.name, @@ -437,10 +556,6 @@ export async function buildDoctorReleaseEvidenceReport( } ); const releaseGates = buildReleaseGateReport(git, options); - const runtimeApproval = evaluateRuntimeApproval(runtimePlan, { - required: options.requireRuntimeApproval ?? false, - approvedDigest: options.runtimeApprovalDigest - }); const partialReport = { schemaVersion: "1.0.0" as const, kind: "doctor.release.evidence" as const, diff --git a/src/core/remote-mcp-probe.ts b/src/core/remote-mcp-probe.ts new file mode 100644 index 0000000..a19fe22 --- /dev/null +++ b/src/core/remote-mcp-probe.ts @@ -0,0 +1,331 @@ +import { + BoundedHttpError, + requestBoundedHttp, + type BoundedHttpRequestOptions, + type BoundedHttpResponse +} from "./bounded-http-client.js"; +import { RemoteNetworkPolicyError, type RemoteLookup } from "./remote-network-policy.js"; +import { checkRemoteOAuthReadiness } from "./remote-oauth-readiness.js"; +import { inspectRemoteMcpUrl } from "./remote-url-policy.js"; +import type { Finding, RemoteRuntimeScorecard, RuntimeCapabilityStatus } from "../domain/types.js"; +import { packageVersion } from "../version.js"; + +const MCP_PROTOCOL_VERSION = "2025-11-25"; + +type JsonObject = Record; + +export type RemoteMcpRequest = ( + rawUrl: string, + options?: BoundedHttpRequestOptions +) => Promise; + +export interface RemoteMcpProbeOptions { + allowNetwork?: boolean; + allowLocalNetwork?: boolean; + requestTimeoutMs?: number; + lookup?: RemoteLookup; + request?: RemoteMcpRequest; +} + +export interface RemoteMcpProbeResult { + findings: Finding[]; + scorecard: RemoteRuntimeScorecard; +} + +function createScorecard(): RemoteRuntimeScorecard { + return { + transport: "skipped", + networkSafety: "skipped", + initialize: "skipped", + contentType: "skipped", + session: "absent", + protocolHeaders: "skipped", + authorization: "skipped", + overall: "skipped" + }; +} + +function failure( + id: string, + message: string, + impact: string, + suggestedFix: string +): Finding { + return { id, severity: "fail", message, impact, suggestedFix }; +} + +function finalize(scorecard: RemoteRuntimeScorecard, findings: Finding[]): RemoteMcpProbeResult { + scorecard.overall = findings.some((finding) => finding.severity === "fail") + ? "fail" + : findings.some((finding) => finding.severity === "warn") + ? "warn" + : scorecard.initialize === "pass" || scorecard.authorization === "pass" + ? "pass" + : "skipped"; + return { findings, scorecard }; +} + +function isPlainObject(value: unknown): value is JsonObject { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function responseHeader( + headers: BoundedHttpResponse["headers"], + name: string +): string | null { + const value = headers[name]; + return typeof value === "string" ? value : null; +} + +function mediaType(value: string | null): string | null { + return value?.split(";", 1)[0]?.trim().toLowerCase() ?? null; +} + +function parseJsonObject(source: string): JsonObject | null { + try { + const parsed: unknown = JSON.parse(source); + return isPlainObject(parsed) ? parsed : null; + } catch { + return null; + } +} + +function isInitializeResponseCandidate(message: JsonObject): boolean { + return message.id === 1; +} + +function findSseInitializeResponse(body: Buffer): JsonObject | null { + const text = body.toString("utf8").replace(/\r\n/g, "\n"); + let offset = 0; + while (offset < text.length) { + const boundary = text.indexOf("\n\n", offset); + if (boundary === -1) { + return null; + } + const data = text.slice(offset, boundary).split("\n") + .filter((line) => line.startsWith("data:")) + .map((line) => line.slice(5).replace(/^ /, "")); + const message = data.length > 0 ? parseJsonObject(data.join("\n")) : null; + if (message && isInitializeResponseCandidate(message)) { + return message; + } + offset = boundary + 2; + } + return null; +} + +function parseInitializeResponse(body: Buffer, contentType: string): JsonObject | null { + return contentType === "text/event-stream" + ? findSseInitializeResponse(body) + : parseJsonObject(body.toString("utf8")); +} + +function isValidInitializeResponse(message: JsonObject): boolean { + return ( + message.jsonrpc === "2.0" && + message.id === 1 && + isPlainObject(message.result) && + message.result.protocolVersion === MCP_PROTOCOL_VERSION && + isPlainObject(message.result.capabilities) && + isPlainObject(message.result.serverInfo) && + typeof message.result.serverInfo.name === "string" && + typeof message.result.serverInfo.version === "string" + ); +} + +function validSessionId(value: string | null): boolean { + return value !== null && /^[\x21-\x7e]+$/.test(value); +} + +function transportFailureId(error: unknown): string { + if (error instanceof BoundedHttpError) { + if (error.code === "REMOTE_HTTP_TIMEOUT") return "plugin.runtime.remote.transport.timeout"; + if (error.code === "REMOTE_HTTP_RESPONSE_TOO_LARGE") return "plugin.runtime.remote.transport.response_too_large"; + } + return "plugin.runtime.remote.transport.failed"; +} + +function transportStatus(error: unknown): RuntimeCapabilityStatus { + return error instanceof RemoteNetworkPolicyError + ? "skipped" + : "fail"; +} + +export async function probeRemoteMcpServer( + serverName: string, + rawUrl: string, + options: RemoteMcpProbeOptions = {} +): Promise { + const scorecard = createScorecard(); + const findings: Finding[] = []; + const request = options.request ?? requestBoundedHttp; + + if (!options.allowNetwork) { + scorecard.networkSafety = "fail"; + findings.push(failure( + "plugin.runtime.remote.network_not_approved", + `The remote MCP server \`${serverName}\` was not contacted because network probing is not approved.`, + "Remote MCP initialization can create outbound network traffic and must be explicitly approved.", + "Enable remote network probing only after reviewing the server endpoint." + )); + return finalize(scorecard, findings); + } + + const inspection = inspectRemoteMcpUrl(rawUrl); + if (inspection.issues.length > 0) { + scorecard.networkSafety = "fail"; + findings.push(failure( + "plugin.runtime.remote.url.invalid", + `The remote MCP server \`${serverName}\` has an unsafe or unsupported endpoint URL.`, + "Unsafe remote endpoint URLs can bypass network controls or expose credentials.", + "Use an absolute HTTP or HTTPS endpoint without credentials, query parameters, fragments, or IP literals." + )); + return finalize(scorecard, findings); + } + + scorecard.networkSafety = "pass"; + const initializeBody = JSON.stringify({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: MCP_PROTOCOL_VERSION, + capabilities: {}, + clientInfo: { name: "Codex Plugin Doctor", version: packageVersion } + } + }); + let initializeResponse: BoundedHttpResponse; + try { + initializeResponse = await request(rawUrl, { + allowLocalNetwork: options.allowLocalNetwork, + lookup: options.lookup, + timeoutMs: options.requestTimeoutMs, + method: "POST", + body: initializeBody, + headers: { + Accept: "application/json, text/event-stream", + "Content-Type": "application/json" + }, + stopAfter: (body) => findSseInitializeResponse(body) !== null + }); + } catch (error) { + scorecard.transport = transportStatus(error); + if (error instanceof RemoteNetworkPolicyError) { + scorecard.networkSafety = "fail"; + } + findings.push(failure( + transportFailureId(error), + `The remote MCP server \`${serverName}\` could not complete a bounded initialize request.`, + "A failed transport prevents safe MCP protocol negotiation.", + "Verify the endpoint is reachable and complies with the configured remote network policy." + )); + return finalize(scorecard, findings); + } + + scorecard.transport = "pass"; + if (initializeResponse.statusCode === 401) { + const challenge = initializeResponse.headers["www-authenticate"]; + const readiness = await checkRemoteOAuthReadiness(inspection.sanitizedUrl ?? rawUrl, challenge === undefined ? [] : [challenge], { + request, + requestOptions: { + allowLocalNetwork: options.allowLocalNetwork, + lookup: options.lookup, + timeoutMs: options.requestTimeoutMs + } + }); + scorecard.authorization = readiness.status; + findings.push(...readiness.findings); + return finalize(scorecard, findings); + } + if (initializeResponse.statusCode !== 200) { + scorecard.initialize = "fail"; + findings.push(failure( + "plugin.runtime.remote.http_status.invalid", + `The remote MCP server \`${serverName}\` returned an unexpected initialize HTTP status.`, + "Streamable HTTP MCP initialization requires a successful response before protocol negotiation can continue.", + "Return HTTP 200 for initialize, or configure authorization before probing a protected endpoint." + )); + return finalize(scorecard, findings); + } + + const contentType = mediaType(responseHeader(initializeResponse.headers, "content-type")); + if (contentType !== "application/json" && contentType !== "text/event-stream") { + scorecard.contentType = "fail"; + scorecard.initialize = "fail"; + findings.push(failure( + "plugin.runtime.remote.content_type.invalid", + `The remote MCP server \`${serverName}\` returned an unsupported initialize content type.`, + "MCP initialization responses must be JSON or Server-Sent Events so the JSON-RPC result can be validated.", + "Return application/json or text/event-stream with a JSON-RPC initialize response." + )); + return finalize(scorecard, findings); + } + scorecard.contentType = "pass"; + + const sessionId = responseHeader(initializeResponse.headers, "mcp-session-id"); + if (sessionId !== null && !validSessionId(sessionId)) { + scorecard.session = "present-invalid"; + scorecard.initialize = "fail"; + findings.push(failure( + "plugin.runtime.remote.session.invalid", + `The remote MCP server \`${serverName}\` returned an invalid MCP session header.`, + "Invalid session identifiers cannot be safely replayed on the initialized notification.", + "Return MCP-Session-Id only as visible ASCII characters." + )); + return finalize(scorecard, findings); + } + if (sessionId !== null) { + scorecard.session = "present-valid"; + } + + const initializeMessage = parseInitializeResponse(initializeResponse.body, contentType); + if (!initializeMessage || !isValidInitializeResponse(initializeMessage)) { + scorecard.initialize = "fail"; + findings.push(failure( + "plugin.runtime.remote.initialize.invalid", + `The remote MCP server \`${serverName}\` returned an invalid initialize JSON-RPC result.`, + "A malformed initialize result prevents protocol version negotiation.", + "Return a JSON-RPC 2.0 result with id 1 and protocol version 2025-11-25." + )); + return finalize(scorecard, findings); + } + scorecard.initialize = "pass"; + + try { + const initializedResponse = await request(rawUrl, { + allowLocalNetwork: options.allowLocalNetwork, + lookup: options.lookup, + timeoutMs: options.requestTimeoutMs, + method: "POST", + body: JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" }), + headers: { + Accept: "application/json, text/event-stream", + "Content-Type": "application/json", + "MCP-Protocol-Version": MCP_PROTOCOL_VERSION, + ...(sessionId === null ? {} : { "MCP-Session-Id": sessionId }) + } + }); + if (initializedResponse.statusCode < 200 || initializedResponse.statusCode >= 300) { + scorecard.protocolHeaders = "fail"; + findings.push(failure( + "plugin.runtime.remote.initialized.failed", + `The remote MCP server \`${serverName}\` did not acknowledge the initialized notification.`, + "The MCP session may not be ready for subsequent protocol traffic.", + "Accept a successful HTTP response to notifications/initialized at the same MCP endpoint." + )); + return finalize(scorecard, findings); + } + } catch { + scorecard.protocolHeaders = "fail"; + findings.push(failure( + "plugin.runtime.remote.initialized.failed", + `The remote MCP server \`${serverName}\` could not receive the initialized notification.`, + "The MCP session may not be ready for subsequent protocol traffic.", + "Accept a successful HTTP response to notifications/initialized at the same MCP endpoint." + )); + return finalize(scorecard, findings); + } + + scorecard.protocolHeaders = "pass"; + return finalize(scorecard, findings); +} diff --git a/src/core/remote-network-policy.ts b/src/core/remote-network-policy.ts new file mode 100644 index 0000000..37d4770 --- /dev/null +++ b/src/core/remote-network-policy.ts @@ -0,0 +1,185 @@ +import { lookup as dnsLookup } from "node:dns/promises"; +import { BlockList, isIP, SocketAddress } from "node:net"; + +export interface ResolvedRemoteTarget { + hostname: string; + address: string; + family: 4 | 6; + local: boolean; +} + +export interface RemoteLookup { + (hostname: string, options: { all: true; verbatim: true }): Promise>; +} + +export interface ResolveRemoteTargetOptions { + allowLocalNetwork?: boolean; + lookup?: RemoteLookup; +} + +export class RemoteNetworkPolicyError extends Error { + constructor( + readonly code: + | "REMOTE_TARGET_EMPTY" + | "REMOTE_TARGET_FORBIDDEN" + | "REMOTE_TARGET_INVALID_ADDRESS" + | "REMOTE_TARGET_LOOKUP_FAILED" + | "REMOTE_TARGET_URL_INVALID", + message: string + ) { + super(message); + this.name = "RemoteNetworkPolicyError"; + } +} + +const blockedAddresses = new BlockList(); +const loopbackAddresses = new BlockList(); +const mappedIpv4Addresses = new BlockList(); +const ipv4CompatibleAddresses = new BlockList(); + +function addIpv4Subnet(address: string, prefix: number): void { + blockedAddresses.addSubnet(address, prefix, "ipv4"); + blockedAddresses.addSubnet(`::ffff:${address}`, prefix + 96, "ipv6"); +} + +function addLoopbackSubnet(address: string, prefix: number): void { + loopbackAddresses.addSubnet(address, prefix, "ipv4"); + loopbackAddresses.addSubnet(`::ffff:${address}`, prefix + 96, "ipv6"); +} + +for (const [address, prefix] of [ + ["0.0.0.0", 8], + ["10.0.0.0", 8], + ["100.64.0.0", 10], + ["169.254.0.0", 16], + ["172.16.0.0", 12], + ["192.0.0.0", 24], + ["192.0.2.0", 24], + ["192.88.99.0", 24], + ["192.168.0.0", 16], + ["198.18.0.0", 15], + ["198.51.100.0", 24], + ["203.0.113.0", 24], + ["224.0.0.0", 4], + ["240.0.0.0", 4] +] as const) { + addIpv4Subnet(address, prefix); +} + +addLoopbackSubnet("127.0.0.0", 8); +blockedAddresses.addAddress("100.100.100.200", "ipv4"); +blockedAddresses.addAddress("100.100.100.100", "ipv4"); + +for (const [address, prefix] of [ + ["::", 128], + ["100::", 64], + ["2001::", 23], + ["2001:db8::", 32], + ["2002::", 16], + ["3fff::", 20], + ["5f00::", 16], + ["fc00::", 7], + ["fe80::", 10], + ["ff00::", 8] +] as const) { + blockedAddresses.addSubnet(address, prefix, "ipv6"); +} + +loopbackAddresses.addAddress("::1", "ipv6"); +mappedIpv4Addresses.addSubnet("::ffff:0:0", 96, "ipv6"); +ipv4CompatibleAddresses.addSubnet("::", 96, "ipv6"); + +for (const [address, prefix] of [ + ["64:ff9b::", 96], + ["64:ff9b:1::", 48], + ["100:0:0:1::", 64] +] as const) { + blockedAddresses.addSubnet(address, prefix, "ipv6"); +} + +function normalizeAddress(address: string, family: number): { address: string; family: 4 | 6 } { + if ((family !== 4 && family !== 6) || isIP(address) !== family) { + throw new RemoteNetworkPolicyError( + "REMOTE_TARGET_INVALID_ADDRESS", + "Remote target DNS resolution returned an invalid address." + ); + } + + const normalized = new SocketAddress({ + address, + port: 0, + family: family === 4 ? "ipv4" : "ipv6" + }); + + return { address: normalized.address, family }; +} + +function isListed(address: string, family: 4 | 6, list: BlockList): boolean { + return list.check(address, family === 4 ? "ipv4" : "ipv6"); +} + +export async function resolveRemoteTarget( + url: URL, + options: ResolveRemoteTargetOptions = {} +): Promise { + const hostname = url.hostname.startsWith("[") && url.hostname.endsWith("]") + ? url.hostname.slice(1, -1) + : url.hostname; + if ((url.protocol !== "http:" && url.protocol !== "https:") || !hostname || isIP(hostname) !== 0) { + throw new RemoteNetworkPolicyError( + "REMOTE_TARGET_URL_INVALID", + "Remote target URL must use HTTP or HTTPS with a hostname, not an IP address literal." + ); + } + + const lookup: RemoteLookup = options.lookup ?? (async (hostname, lookupOptions) => { + const answers = await dnsLookup(hostname, lookupOptions); + return answers.map((answer) => ({ + address: answer.address, + family: answer.family as 4 | 6 + })); + }); + let answers: Array<{ address: string; family: 4 | 6 }>; + try { + answers = await lookup(hostname, { all: true, verbatim: true }); + } catch { + throw new RemoteNetworkPolicyError( + "REMOTE_TARGET_LOOKUP_FAILED", + "Remote target DNS resolution failed." + ); + } + + if (answers.length === 0) { + throw new RemoteNetworkPolicyError( + "REMOTE_TARGET_EMPTY", + "Remote target DNS resolution returned no addresses." + ); + } + + const normalizedAnswers = answers.map((answer) => normalizeAddress(answer.address, answer.family)); + for (const answer of normalizedAnswers) { + const isLoopback = isListed(answer.address, answer.family, loopbackAddresses); + const isMappedIpv4 = answer.family === 6 && isListed(answer.address, answer.family, mappedIpv4Addresses); + const isIpv4Compatible = answer.family === 6 && isListed(answer.address, answer.family, ipv4CompatibleAddresses); + const isForbidden = isListed(answer.address, answer.family, blockedAddresses) + || isIpv4Compatible + || (isMappedIpv4 && !isLoopback); + if ((isForbidden && !isLoopback) || (isLoopback && !options.allowLocalNetwork)) { + throw new RemoteNetworkPolicyError( + "REMOTE_TARGET_FORBIDDEN", + "Remote target resolved to a forbidden address." + ); + } + } + + const selected = normalizedAnswers[0]; + return { + hostname, + address: selected.address, + family: selected.family, + local: isListed(selected.address, selected.family, loopbackAddresses) + }; +} diff --git a/src/core/remote-oauth-readiness.ts b/src/core/remote-oauth-readiness.ts new file mode 100644 index 0000000..5f49e85 --- /dev/null +++ b/src/core/remote-oauth-readiness.ts @@ -0,0 +1,339 @@ +import { isIP } from "node:net"; + +import { + requestBoundedHttp, + type BoundedHttpRequestOptions, + type BoundedHttpResponse +} from "./bounded-http-client.js"; +import type { Finding } from "../domain/types.js"; + +type JsonObject = Record; + +export type RemoteOAuthReadinessRequest = ( + rawUrl: string, + options?: BoundedHttpRequestOptions +) => Promise; + +export interface RemoteOAuthReadinessOptions { + request?: RemoteOAuthReadinessRequest; + requestOptions?: Pick; +} + +export interface RemoteOAuthReadinessResult { + status: "pass" | "fail"; + findings: Finding[]; +} + +type MetadataReply = + | { kind: "ok"; metadata: JsonObject } + | { kind: "invalid" } + | { kind: "not-found" } + | { kind: "unavailable" }; + +const MAX_DISCOVERY_CANDIDATES = 4; +const MAX_DISCOVERY_TIMEOUT_MS = 3_000; + +function failure(id: string): RemoteOAuthReadinessResult { + return { + status: "fail", + findings: [{ + id, + severity: "fail", + message: "The remote MCP server authorization metadata could not be validated.", + impact: "Protected MCP endpoints cannot be safely assessed without valid OAuth discovery metadata.", + suggestedFix: "Publish valid HTTPS protected-resource and authorization-server metadata without credentials, queries, or fragments." + }] + }; +} + +function isPlainObject(value: unknown): value is JsonObject { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function mediaType(headers: BoundedHttpResponse["headers"]): string | null { + const value = headers["content-type"]; + const source = Array.isArray(value) ? value[0] : value; + return typeof source === "string" ? source.split(";", 1)[0]?.trim().toLowerCase() ?? null : null; +} + +function parseJsonObject(body: Buffer): JsonObject | null { + try { + const parsed: unknown = JSON.parse(body.toString("utf8")); + return isPlainObject(parsed) ? parsed : null; + } catch { + return null; + } +} + +function safeHttpsUrl(value: unknown): string | null { + if (typeof value !== "string") return null; + try { + const url = new URL(value); + if ( + url.protocol !== "https:" || + !url.hostname || + url.username || + url.password || + url.search || + url.hash || + url.hostname.startsWith("[") || + isIP(url.hostname) !== 0 + ) { + return null; + } + return value; + } catch { + return null; + } +} + +function insertWellKnown(resourceUrl: string, suffix: string): string { + const url = new URL(resourceUrl); + const path = url.pathname === "/" ? "" : url.pathname.replace(/\/$/, ""); + return `${url.origin}/.well-known/${suffix}${path}`; +} + +function oidcDiscoveryUrl(issuer: string): string { + const url = new URL(issuer); + return `${url.origin}${url.pathname.replace(/\/$/, "")}/.well-known/openid-configuration`; +} + +function splitTopLevelCommas(value: string): string[] | null { + const parts: string[] = []; + let start = 0; + let quoted = false; + + for (let index = 0; index < value.length; index += 1) { + const character = value[index]!; + if (quoted && character === "\\") { + index += 1; + if (index >= value.length) return null; + continue; + } + if (character === "\"") { + quoted = !quoted; + continue; + } + if (!quoted && character === ",") { + parts.push(value.slice(start, index)); + start = index + 1; + } + } + + return quoted ? null : [...parts, value.slice(start)]; +} + +function parseAuthParam(value: string): { name: string; value: string } | null { + const match = /^([!#$%&'*+.^`|~\w-]+)[ \t]*=[ \t]*(.*)$/.exec(value); + if (match === null) return null; + + const name = match[1]!; + const rawValue = match[2]!.trim(); + if (rawValue.length === 0) return null; + if (!rawValue.startsWith("\"")) { + return /[\s"]/u.test(rawValue) ? null : { name, value: rawValue }; + } + if (!rawValue.endsWith("\"") || rawValue.length === 1) return null; + + let decoded = ""; + for (let index = 1; index < rawValue.length - 1; index += 1) { + const character = rawValue[index]!; + if (character === "\\") { + index += 1; + if (index >= rawValue.length - 1) return null; + decoded += rawValue[index]!; + continue; + } + if (character === "\"") return null; + decoded += character; + } + return { name, value: decoded }; +} + +function parseChallenge(value: string): { scheme: string; parameter: { name: string; value: string } | null } | null { + const match = /^([!#$%&'*+.^`|~\w-]+)(?:[ \t]+(.*))?$/.exec(value); + if (match === null) return null; + + const scheme = match[1]!; + const credentials = match[2]; + if (credentials === undefined || credentials.trim().length === 0) return { scheme, parameter: null }; + const parameter = parseAuthParam(credentials); + if (parameter !== null) return { scheme, parameter }; + return /^[A-Za-z0-9\-._~+/]+={0,}$/.test(credentials) ? { scheme, parameter: null } : null; +} + +function parseBearerResourceMetadata(headers: Array): { specified: boolean; values: string[] } | null { + const values: string[] = []; + let specified = false; + for (const header of headers.flatMap((value) => Array.isArray(value) ? value : [value])) { + const parts = splitTopLevelCommas(header); + if (parts === null) return null; + + let scheme: string | null = null; + for (const part of parts) { + const token = part.trim(); + if (token.length === 0) return null; + + const parameter = parseAuthParam(token); + if (parameter !== null) { + if (scheme === null) return null; + if (scheme.toLowerCase() === "bearer" && parameter.name.toLowerCase() === "resource_metadata") { + specified = true; + values.push(parameter.value); + } + continue; + } + + const challenge = parseChallenge(token); + if (challenge === null) return null; + scheme = challenge.scheme; + if (scheme.toLowerCase() === "bearer" && challenge.parameter?.name.toLowerCase() === "resource_metadata") { + specified = true; + values.push(challenge.parameter.value); + } + } + } + return { specified, values }; +} + +async function fetchMetadata( + url: string, + request: RemoteOAuthReadinessRequest, + requestOptions: RemoteOAuthReadinessOptions["requestOptions"], + deadline: number +): Promise { + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) return { kind: "unavailable" }; + + let timeout: NodeJS.Timeout | undefined; + try { + const response = await Promise.race([ + request(url, { + ...requestOptions, + timeoutMs: Math.max(1, Math.floor(remainingMs)), + method: "GET", + headers: { Accept: "application/json" } + }), + new Promise((_resolve, reject) => { + timeout = setTimeout(() => reject(new Error("OAuth discovery timed out.")), remainingMs); + }) + ]); + if (response.statusCode === 404) return { kind: "not-found" }; + if (response.statusCode !== 200 || mediaType(response.headers) !== "application/json") { + return { kind: "unavailable" }; + } + const metadata = parseJsonObject(response.body); + return metadata === null ? { kind: "invalid" } : { kind: "ok", metadata }; + } catch { + return { kind: "unavailable" }; + } finally { + if (timeout !== undefined) clearTimeout(timeout); + } +} + +function validProtectedResourceMetadata(metadata: JsonObject, resourceUrl: string): string[] | null { + if (metadata.resource !== resourceUrl || !Array.isArray(metadata.authorization_servers) || metadata.authorization_servers.length === 0) { + return null; + } + const issuers = metadata.authorization_servers.map(safeHttpsUrl); + if (!issuers.every((issuer): issuer is string => issuer !== null)) return null; + const uniqueIssuers = [...new Set(issuers)]; + return uniqueIssuers.length <= MAX_DISCOVERY_CANDIDATES ? uniqueIssuers : null; +} + +function validAuthorizationServerMetadata(metadata: JsonObject, issuer: string): boolean { + return ( + metadata.issuer === issuer && + safeHttpsUrl(metadata.authorization_endpoint) !== null && + safeHttpsUrl(metadata.token_endpoint) !== null + ); +} + +async function authorizationServerIsReady( + issuer: string, + request: RemoteOAuthReadinessRequest, + requestOptions: RemoteOAuthReadinessOptions["requestOptions"], + deadline: number +): Promise<"pass" | "invalid" | "unavailable"> { + const rfc8414 = await fetchMetadata(insertWellKnown(issuer, "oauth-authorization-server"), request, requestOptions, deadline); + if (rfc8414.kind === "ok") return validAuthorizationServerMetadata(rfc8414.metadata, issuer) ? "pass" : "invalid"; + if (rfc8414.kind === "unavailable") return "unavailable"; + + const oidcUrls = [...new Set([ + insertWellKnown(issuer, "openid-configuration"), + oidcDiscoveryUrl(issuer) + ])]; + for (const oidcUrl of oidcUrls) { + const oidc = await fetchMetadata(oidcUrl, request, requestOptions, deadline); + if (oidc.kind === "ok") return validAuthorizationServerMetadata(oidc.metadata, issuer) ? "pass" : "invalid"; + if (oidc.kind === "unavailable") return "unavailable"; + } + return "invalid"; +} + +export async function checkRemoteOAuthReadiness( + resourceUrl: string, + wwwAuthenticate: Array, + options: RemoteOAuthReadinessOptions = {} +): Promise { + const request = options.request ?? requestBoundedHttp; + const explicit = parseBearerResourceMetadata(wwwAuthenticate); + if (explicit === null) return failure("plugin.runtime.remote.authorization.metadata.invalid"); + const metadataCandidates = explicit.specified + ? explicit.values.map(safeHttpsUrl) + : [ + insertWellKnown(resourceUrl, "oauth-protected-resource"), + `${new URL(resourceUrl).origin}/.well-known/oauth-protected-resource` + ].map(safeHttpsUrl); + if (!metadataCandidates.every((url): url is string => url !== null)) { + return failure("plugin.runtime.remote.authorization.metadata.invalid"); + } + const metadataUrls = [...new Set(metadataCandidates)]; + if (metadataUrls.length > MAX_DISCOVERY_CANDIDATES) return failure("plugin.runtime.remote.authorization.metadata.invalid"); + const suppliedTimeoutMs = options.requestOptions?.timeoutMs; + const timeoutMs = typeof suppliedTimeoutMs === "number" && Number.isFinite(suppliedTimeoutMs) + ? Math.max(0, Math.min(suppliedTimeoutMs, MAX_DISCOVERY_TIMEOUT_MS)) + : MAX_DISCOVERY_TIMEOUT_MS; + const deadline = Date.now() + timeoutMs; + + let issuers: string[] | null = null; + let protectedMetadataFetchedInvalid = false; + let protectedMetadataUnavailable = false; + for (const metadataUrl of metadataUrls) { + const reply = await fetchMetadata(metadataUrl, request, options.requestOptions, deadline); + if (reply.kind === "ok") { + const candidateIssuers = validProtectedResourceMetadata(reply.metadata, resourceUrl); + if (candidateIssuers !== null) { + issuers = candidateIssuers; + break; + } + protectedMetadataFetchedInvalid = true; + if (!explicit.specified) return failure("plugin.runtime.remote.authorization.metadata.invalid"); + continue; + } + if (reply.kind === "invalid") { + protectedMetadataFetchedInvalid = true; + if (!explicit.specified) return failure("plugin.runtime.remote.authorization.metadata.invalid"); + continue; + } + protectedMetadataUnavailable = true; + if (!explicit.specified && reply.kind === "unavailable") { + return failure("plugin.runtime.remote.authorization.metadata.unavailable"); + } + } + if (issuers === null) return failure(protectedMetadataFetchedInvalid + ? "plugin.runtime.remote.authorization.metadata.invalid" + : protectedMetadataUnavailable + ? "plugin.runtime.remote.authorization.metadata.unavailable" + : "plugin.runtime.remote.authorization.metadata.invalid"); + + let unavailable = false; + for (const issuer of issuers) { + const readiness = await authorizationServerIsReady(issuer, request, options.requestOptions, deadline); + if (readiness === "pass") return { status: "pass", findings: [] }; + unavailable ||= readiness === "unavailable"; + } + return failure(unavailable + ? "plugin.runtime.remote.authorization.metadata.unavailable" + : "plugin.runtime.remote.authorization.metadata.invalid"); +} diff --git a/src/core/remote-url-policy.ts b/src/core/remote-url-policy.ts new file mode 100644 index 0000000..65549db --- /dev/null +++ b/src/core/remote-url-policy.ts @@ -0,0 +1,74 @@ +export interface RemoteUrlInspection { + parsedUrl: URL | null; + sanitizedUrl: string | null; + isLoopbackHost: boolean; + issues: Array< + | "invalid" + | "unsupported_scheme" + | "credentials" + | "query" + | "fragment" + | "ip_literal" + | "insecure_non_loopback" + >; +} + +function isIpLiteral(hostname: string): boolean { + return ( + /^\d{1,3}(?:\.\d{1,3}){3}$/.test(hostname) || + /^\[[0-9a-f:.]+\]$/i.test(hostname) + ); +} + +export function inspectRemoteMcpUrl(rawUrl: string): RemoteUrlInspection { + let parsedUrl: URL; + + try { + parsedUrl = new URL(rawUrl); + } catch { + return { + parsedUrl: null, + sanitizedUrl: null, + isLoopbackHost: false, + issues: ["invalid"] + }; + } + + const issues: RemoteUrlInspection["issues"] = []; + const isSupportedScheme = parsedUrl.protocol === "http:" || parsedUrl.protocol === "https:"; + const isAbsoluteHttpUrl = /^https?:\/\//i.test(rawUrl); + const isLoopbackHost = parsedUrl.hostname.toLowerCase() === "localhost"; + + if (!isSupportedScheme) { + issues.push("unsupported_scheme"); + } else if (!isAbsoluteHttpUrl || !parsedUrl.hostname) { + issues.push("invalid"); + } + + if (parsedUrl.username || parsedUrl.password) { + issues.push("credentials"); + } + + if (parsedUrl.search || rawUrl.includes("?")) { + issues.push("query"); + } + + if (parsedUrl.hash || rawUrl.includes("#")) { + issues.push("fragment"); + } + + if (isIpLiteral(parsedUrl.hostname)) { + issues.push("ip_literal"); + } + + if (parsedUrl.protocol === "http:" && !isLoopbackHost) { + issues.push("insecure_non_loopback"); + } + + return { + parsedUrl, + sanitizedUrl: `${parsedUrl.origin}${parsedUrl.pathname}`, + isLoopbackHost, + issues + }; +} diff --git a/src/core/runtime-plan.ts b/src/core/runtime-plan.ts index d964a28..4d671f8 100644 --- a/src/core/runtime-plan.ts +++ b/src/core/runtime-plan.ts @@ -11,10 +11,12 @@ import { import type { Finding } from "../domain/types.js"; import type { RuntimeExecutionEvidence, RuntimeSandboxMode } from "../domain/types.js"; import { DOCKER_RUNTIME_IMAGE } from "./runtime-sandbox.js"; +import { inspectRemoteMcpUrl } from "./remote-url-policy.js"; type RuntimePlanStatus = "pass" | "warn" | "fail"; type RuntimePlanRiskLevel = "low" | "medium" | "high"; type RuntimePlanTransport = "stdio" | "http"; +type RemoteNetworkClass = "public_https" | "loopback_http" | "loopback_https" | "invalid"; export interface RuntimePlanServer { name: string; @@ -23,7 +25,9 @@ export interface RuntimePlanServer { args: string[]; cwd: string | null; url: string | null; + networkClass?: RemoteNetworkClass; probeMethods: string[]; + approvalRequirements?: string[]; riskLevel: RuntimePlanRiskLevel; riskReasons: string[]; } @@ -117,6 +121,33 @@ function buildRisk( }; } +function remoteProbeMethods(): string[] { + return [ + "POST initialize", + "POST notifications/initialized", + "GET OAuth protected-resource metadata (401 only)", + "GET OAuth authorization-server metadata (401 only)" + ]; +} + +function remoteNetworkClass(rawUrl: string): RemoteNetworkClass { + const inspection = inspectRemoteMcpUrl(rawUrl); + + if ( + inspection.parsedUrl === null || + (inspection.parsedUrl.protocol !== "http:" && inspection.parsedUrl.protocol !== "https:") || + !inspection.parsedUrl.hostname + ) { + return "invalid"; + } + + if (inspection.isLoopbackHost) { + return inspection.parsedUrl.protocol === "http:" ? "loopback_http" : "loopback_https"; + } + + return "public_https"; +} + function planDigestPayload(plan: Omit): unknown { return { schemaVersion: plan.schemaVersion, @@ -204,6 +235,9 @@ export async function buildDoctorRuntimePlan( const url = typeof serverConfig.url === "string" ? serverConfig.url : null; const { riskLevel, riskReasons } = buildRisk(serverName, serverConfig, security.findings); + const networkClass = url ? remoteNetworkClass(url) : undefined; + const sanitizedUrl = url ? inspectRemoteMcpUrl(url).sanitizedUrl : null; + return { name: serverName, transport: command ? "stdio" as const : "http" as const, @@ -212,7 +246,8 @@ export async function buildDoctorRuntimePlan( ? serverConfig.args.filter((arg): arg is string => typeof arg === "string") : [], cwd: command ? normalizeCwd(discoveredPackage.rootPath, serverConfig.cwd) : null, - url, + url: sanitizedUrl, + ...(networkClass ? { networkClass } : {}), probeMethods: command ? [ "initialize", @@ -225,7 +260,14 @@ export async function buildDoctorRuntimePlan( "prompts/list", "prompts/get:first-prompt-only" ] - : [], + : url ? remoteProbeMethods() : [], + ...(url + ? { + approvalRequirements: networkClass === "loopback_http" || networkClass === "loopback_https" + ? ["--runtime", "--allow-network", "--allow-local-network"] + : ["--runtime", "--allow-network"] + } + : {}), riskLevel, riskReasons }; @@ -349,6 +391,7 @@ export function renderDoctorRuntimePlanMarkdown(plan: DoctorRuntimePlan): string "- This plan is non-executing.", "- Probe methods explicitly exclude task create, get, result, and cancel operations, plus sampling and elicitation requests.", "- Runtime probes require explicit operator approval before local MCP servers are started.", + "- Any remote target whose DNS resolves to loopback requires `--allow-local-network`, even if its URL looks public.", "- The approval digest changes when command, args, cwd, probe methods, risk reasons, or findings change.", "- Runtime approval is a review gate, not an OS, VM, or container sandbox.", "", diff --git a/src/core/runtime-policy.ts b/src/core/runtime-policy.ts index 6e719ab..a12b0ff 100644 --- a/src/core/runtime-policy.ts +++ b/src/core/runtime-policy.ts @@ -94,7 +94,9 @@ function buildRecommendation( decision: RuntimePolicyDecision, reasons: string[] ): RuntimePolicyRecommendation { - if (plan.summary.executableServerCount === 0) { + const hasRemoteServer = plan.servers.some((server) => server.transport === "http"); + + if (plan.summary.executableServerCount === 0 && !hasRemoteServer) { return { decision: "allow", reason: "No executable local MCP runtime servers were found.", @@ -137,6 +139,9 @@ function buildRecommendation( actions: [ "Review command, args, cwd, URL, probe methods, and risk reasons before execution.", "Approve the exact plan digest with `check --runtime --require-runtime-approval --runtime-approval-digest `.", + ...(hasRemoteServer + ? ["Remote MCP probing also requires `--runtime --allow-network`; any target that DNS resolves to loopback additionally requires `--allow-local-network`."] + : []), "Use `doctor runtime-plan --markdown` when the approval needs to be preserved with release evidence." ] }; diff --git a/src/core/runtime-probe.ts b/src/core/runtime-probe.ts index c76639a..b851312 100644 --- a/src/core/runtime-probe.ts +++ b/src/core/runtime-probe.ts @@ -13,11 +13,17 @@ import type { RuntimeConformanceScorecard, RuntimeExecutionEvidence, RuntimeProbeResult, + RemoteRuntimeScorecard, RuntimeSandboxMode, RuntimeScorecard, TasksListObservation } from "../domain/types.js"; import { evaluateMcpConformance } from "./mcp-conformance.js"; +import { + probeRemoteMcpServer, + type RemoteMcpRequest +} from "./remote-mcp-probe.js"; +import type { RemoteLookup } from "./remote-network-policy.js"; import { buildRuntimeLaunch, DOCKER_RUNTIME_STARTUP_TIMEOUT_MS, @@ -503,6 +509,9 @@ function mergeRuntimeScorecards( overall: worstRuntimeStatus(leftConformance.overall, rightConformance.overall) } : leftConformance ?? rightConformance; + const remote = left.remote && right.remote + ? mergeRemoteRuntimeScorecards(left.remote, right.remote) + : left.remote ?? right.remote; return { initialize: worstRuntimeStatus(left.initialize, right.initialize), @@ -516,7 +525,32 @@ function mergeRuntimeScorecards( ), promptsList: worstRuntimeStatus(left.promptsList, right.promptsList), promptGet: worstRuntimeStatus(left.promptGet, right.promptGet), - ...(conformance ? { conformance } : {}) + ...(conformance ? { conformance } : {}), + ...(remote ? { remote } : {}) + }; +} + +function mergeRemoteRuntimeScorecards( + left: RemoteRuntimeScorecard, + right: RemoteRuntimeScorecard +): RemoteRuntimeScorecard { + const sessionSeverity: Record = { + "present-invalid": 2, + "present-valid": 1, + absent: 0 + }; + + return { + transport: worstRuntimeStatus(left.transport, right.transport), + networkSafety: worstRuntimeStatus(left.networkSafety, right.networkSafety), + initialize: worstRuntimeStatus(left.initialize, right.initialize), + contentType: worstRuntimeStatus(left.contentType, right.contentType), + session: sessionSeverity[left.session] >= sessionSeverity[right.session] + ? left.session + : right.session, + protocolHeaders: worstRuntimeStatus(left.protocolHeaders, right.protocolHeaders), + authorization: worstRuntimeStatus(left.authorization, right.authorization), + overall: worstRuntimeStatus(left.overall, right.overall) }; } @@ -1939,6 +1973,11 @@ export interface RuntimeProbeOptions { startupTimeoutMs?: number; sandbox?: RuntimeSandboxMode; transcript?: (line: string) => void; + allowNetwork?: boolean; + allowLocalNetwork?: boolean; + remoteRequestTimeoutMs?: number; + remoteLookup?: RemoteLookup; + remoteRequest?: RemoteMcpRequest; } export async function probeRuntimeConfig( @@ -1990,46 +2029,64 @@ export async function probeRuntimeConfig( continue; } - const command = config.command; + const url = config.url; + let result: RuntimeProbeResult; - if (typeof command !== "string") { - continue; - } + if (typeof url === "string") { + const remote = await probeRemoteMcpServer(serverName, url, { + allowNetwork: options.allowNetwork, + allowLocalNetwork: options.allowLocalNetwork, + requestTimeoutMs: options.remoteRequestTimeoutMs, + lookup: options.remoteLookup, + request: options.remoteRequest + }); + const remoteScorecard = createRuntimeScorecard(); + remoteScorecard.remote = remote.scorecard; + result = { + findings: remote.findings.map((finding) => withRuntimeEvidence(finding, serverName)), + scorecard: remoteScorecard + }; + } else { + const command = config.command; + if (typeof command !== "string") { + continue; + } - const args = Array.isArray(config.args) - ? config.args.filter((value): value is string => typeof value === "string") - : []; - const cwd = await resolveRuntimeCwd(canonicalRootPath, config.cwd); - const result: RuntimeProbeResult = cwd === null - ? (() => { - const invalidCwdScorecard = createRuntimeScorecard(); - invalidCwdScorecard.initialize = "fail"; - - return { - findings: [ - withRuntimeEvidence( - buildFailure( - "plugin.runtime.startup.invalid_cwd", - `The MCP server \`${serverName}\` has an invalid runtime working directory.`, - "Runtime validation must not start a server from a missing, non-directory, or out-of-package working directory.", - "Set the MCP server cwd to an existing directory inside the plugin package root, or remove it." - ), - serverName - ) - ], - scorecard: invalidCwdScorecard - }; - })() - : await probeCommandServer({ - serverName, - packageRoot: canonicalRootPath, - command, - args, - cwd, - startupTimeoutMs, - sandbox: options.sandbox, - transcript: options.transcript - }); + const args = Array.isArray(config.args) + ? config.args.filter((value): value is string => typeof value === "string") + : []; + const cwd = await resolveRuntimeCwd(canonicalRootPath, config.cwd); + result = cwd === null + ? (() => { + const invalidCwdScorecard = createRuntimeScorecard(); + invalidCwdScorecard.initialize = "fail"; + + return { + findings: [ + withRuntimeEvidence( + buildFailure( + "plugin.runtime.startup.invalid_cwd", + `The MCP server \`${serverName}\` has an invalid runtime working directory.`, + "Runtime validation must not start a server from a missing, non-directory, or out-of-package working directory.", + "Set the MCP server cwd to an existing directory inside the plugin package root, or remove it." + ), + serverName + ) + ], + scorecard: invalidCwdScorecard + }; + })() + : await probeCommandServer({ + serverName, + packageRoot: canonicalRootPath, + command, + args, + cwd, + startupTimeoutMs, + sandbox: options.sandbox, + transcript: options.transcript + }); + } scorecard = hasProbedServer ? mergeRuntimeScorecards(scorecard, result.scorecard) diff --git a/src/core/validate-plugin.ts b/src/core/validate-plugin.ts index bab9676..0cee031 100644 --- a/src/core/validate-plugin.ts +++ b/src/core/validate-plugin.ts @@ -10,6 +10,7 @@ import type { } from "../domain/types.js"; import { withFindingFingerprints } from "../reporting/finding-fingerprint.js"; import { discoverPackage } from "./discover-package.js"; +import { inspectRemoteMcpUrl } from "./remote-url-policy.js"; import { probeRuntime } from "./runtime-probe.js"; function buildFailure( @@ -46,6 +47,18 @@ function buildWarning( }; } +function remoteUrlIssueFindingId(issue: string): string { + return issue === "insecure_non_loopback" + ? "plugin.security.insecure_http_url" + : `plugin.security.remote_mcp_url.${issue}`; +} + +function remoteUrlIssueMessage(issue: string): string { + return issue === "insecure_non_loopback" + ? "uses an insecure public HTTP URL" + : `uses a remote MCP URL with ${issue.replaceAll("_", " ")}`; +} + async function directoryExists(targetPath: string): Promise { try { const details = await stat(targetPath); @@ -676,6 +689,43 @@ async function validateMcpConfig( ); } + if (typeof command === "string" && typeof url === "string") { + findings.push( + buildFailure( + "plugin.mcp.server.transport.conflict", + `The MCP server \`${serverName}\` must not define both \`command\` and \`url\`.`, + "A server with two transports cannot be selected deterministically by MCP clients.", + `Keep either \`command\` or \`url\` for the \`${serverName}\` entry in \`${mcpConfigPath}\`.`, + { + configPath: relativePackagePath(rootPath, mcpConfigPath), + serverName, + field: "transport" + } + ) + ); + } + + if (typeof url === "string") { + const inspection = inspectRemoteMcpUrl(url); + + for (const issue of inspection.issues) { + findings.push( + buildFailure( + remoteUrlIssueFindingId(issue), + `The MCP server \`${serverName}\` ${remoteUrlIssueMessage(issue)}.`, + "Unsafe or ambiguous remote transport configuration can expose credentials or prevent reliable MCP connectivity.", + "Use an absolute HTTPS URL without credentials, query parameters, fragments, or numeric IP literals; HTTP is only supported for localhost development.", + { + configPath: relativePackagePath(rootPath, mcpConfigPath), + serverName, + field: "url", + url: inspection.sanitizedUrl + } + ) + ); + } + } + if (isPlainObject(env)) { for (const [envKey, envValue] of Object.entries(env)) { if ( @@ -739,7 +789,9 @@ export async function validatePlugin( ? await probeRuntime(discoveredPackage, { startupTimeoutMs: options.runtimeStartupTimeoutMs, sandbox: options.runtimeSandbox, - transcript: options.runtimeTranscript + transcript: options.runtimeTranscript, + allowNetwork: options.allowNetwork, + allowLocalNetwork: options.allowLocalNetwork }) : null; const findings = [ diff --git a/src/domain/types.ts b/src/domain/types.ts index d7142e8..e7ab7d4 100644 --- a/src/domain/types.ts +++ b/src/domain/types.ts @@ -50,6 +50,8 @@ export interface BaselineSummary { export interface CheckOptions { runtime?: boolean; + allowNetwork?: boolean; + allowLocalNetwork?: boolean; runtimeTranscript?: (line: string) => void; runtimeStartupTimeoutMs?: number; runtimeSandbox?: RuntimeSandboxMode; @@ -111,6 +113,18 @@ export interface RuntimeScorecard { promptsList: RuntimeCapabilityStatus; promptGet: RuntimeCapabilityStatus; conformance?: RuntimeConformanceScorecard; + remote?: RemoteRuntimeScorecard; +} + +export interface RemoteRuntimeScorecard { + transport: RuntimeCapabilityStatus; + networkSafety: RuntimeCapabilityStatus; + initialize: RuntimeCapabilityStatus; + contentType: RuntimeCapabilityStatus; + session: "absent" | "present-valid" | "present-invalid"; + protocolHeaders: RuntimeCapabilityStatus; + authorization: RuntimeCapabilityStatus; + overall: "pass" | "warn" | "fail" | "skipped"; } export type McpConformanceProfile = diff --git a/src/index.ts b/src/index.ts index 9498fe8..f8fb585 100644 --- a/src/index.ts +++ b/src/index.ts @@ -185,6 +185,26 @@ export { type RuntimePolicyDecision, type RuntimePolicyRecommendation } from "./core/runtime-policy.js"; +export { + inspectRemoteMcpUrl, + type RemoteUrlInspection +} from "./core/remote-url-policy.js"; +export { + resolveRemoteTarget, + RemoteNetworkPolicyError, + type ResolvedRemoteTarget, + type ResolveRemoteTargetOptions +} from "./core/remote-network-policy.js"; +export { + probeRemoteMcpServer, + type RemoteMcpProbeOptions, + type RemoteMcpProbeResult +} from "./core/remote-mcp-probe.js"; +export { + checkRemoteOAuthReadiness, + type RemoteOAuthReadinessOptions, + type RemoteOAuthReadinessResult +} from "./core/remote-oauth-readiness.js"; export { buildDoctorReviewBundle, diffDoctorReviewBundles, diff --git a/src/mcp/generic-mcp-doctor.ts b/src/mcp/generic-mcp-doctor.ts index 007f7dc..1a18166 100644 --- a/src/mcp/generic-mcp-doctor.ts +++ b/src/mcp/generic-mcp-doctor.ts @@ -40,6 +40,8 @@ export interface GenericMcpDoctorReport { export interface GenericMcpDoctorOptions { runtime?: boolean; + allowNetwork?: boolean; + allowLocalNetwork?: boolean; runtimeStartupTimeoutMs?: number; } @@ -154,7 +156,10 @@ function buildStaticMcpFindings( continue; } - if (typeof serverConfig.command !== "string" && typeof serverConfig.url !== "string") { + const command = serverConfig.command; + const url = serverConfig.url; + + if (typeof command !== "string" && typeof url !== "string") { findings.push( buildFinding( "fail", @@ -166,6 +171,20 @@ function buildStaticMcpFindings( ) ); } + + if (typeof command === "string" && typeof url === "string") { + findings.push( + buildFinding( + "fail", + "mcp.server.transport.conflict", + `The MCP server \`${serverName}\` must not define both \`command\` and \`url\`.`, + "A server with two transports cannot be selected deterministically by MCP clients.", + `Keep either \`command\` or \`url\` for the \`${serverName}\` entry in \`${configPath}\`.`, + { configPath, serverName, field: "transport" } + ) + ); + } + } return { @@ -255,7 +274,7 @@ export async function buildGenericMcpDoctor( const security = buildSecurityAuditFromFindings( rootPath, mcpConfigPath && parsedConfig !== null - ? auditMcpServerConfig(rootPath, parsedConfig) + ? auditMcpServerConfig(rootPath, parsedConfig, { configPath: mcpConfigPath }) : [] ); const runtimeResult = @@ -267,8 +286,10 @@ export async function buildGenericMcpDoctor( isPathWithinRoot(canonicalRootPath, canonicalMcpConfigPath) && !staticFindings.some((finding) => finding.severity === "fail") && security.status !== "fail" - ? await probeRuntimeConfig(canonicalRootPath, canonicalMcpConfigPath, { - startupTimeoutMs: options.runtimeStartupTimeoutMs + ? await probeRuntimeConfig(canonicalRootPath, canonicalMcpConfigPath, { + startupTimeoutMs: options.runtimeStartupTimeoutMs, + allowNetwork: options.allowNetwork, + allowLocalNetwork: options.allowLocalNetwork }) : null; const fingerprintedFindings = withFindingFingerprints( diff --git a/src/reporting/render-markdown-report.ts b/src/reporting/render-markdown-report.ts index bbe5757..d7a7700 100644 --- a/src/reporting/render-markdown-report.ts +++ b/src/reporting/render-markdown-report.ts @@ -22,20 +22,34 @@ function appendRuntimeScorecard(lines: string[], result: CheckResult) { const conformance = result.runtimeScorecard.conformance; - if (!conformance) { - return; + if (conformance) { + lines.push("", "## MCP Conformance", ""); + lines.push("| Check | Status |"); + lines.push("| --- | --- |"); + lines.push(`| Protocol version | ${conformance.protocolVersion ?? "unavailable"} |`); + lines.push(`| Profile | ${conformance.profile ?? "unavailable"} |`); + lines.push(`| Capability consistency | ${conformance.capabilityConsistency.toUpperCase()} |`); + lines.push(`| Task declarations | ${conformance.taskDeclarations.toUpperCase()} |`); + lines.push(`| Tasks list | ${conformance.tasksList.toUpperCase()} |`); + lines.push(`| Schema dialect | ${conformance.schemaDialect.toUpperCase()} |`); + lines.push(`| Overall | ${conformance.overall.toUpperCase()} |`); } - lines.push("", "## MCP Conformance", ""); - lines.push("| Check | Status |"); - lines.push("| --- | --- |"); - lines.push(`| Protocol version | ${conformance.protocolVersion ?? "unavailable"} |`); - lines.push(`| Profile | ${conformance.profile ?? "unavailable"} |`); - lines.push(`| Capability consistency | ${conformance.capabilityConsistency.toUpperCase()} |`); - lines.push(`| Task declarations | ${conformance.taskDeclarations.toUpperCase()} |`); - lines.push(`| Tasks list | ${conformance.tasksList.toUpperCase()} |`); - lines.push(`| Schema dialect | ${conformance.schemaDialect.toUpperCase()} |`); - lines.push(`| Overall | ${conformance.overall.toUpperCase()} |`); + const remote = result.runtimeScorecard.remote; + + if (remote) { + lines.push("", "## Remote MCP Scorecard", ""); + lines.push("| Check | Status |"); + lines.push("| --- | --- |"); + lines.push(`| Transport | ${remote.transport.toUpperCase()} |`); + lines.push(`| Network safety | ${remote.networkSafety.toUpperCase()} |`); + lines.push(`| Initialize | ${remote.initialize.toUpperCase()} |`); + lines.push(`| Content type | ${remote.contentType.toUpperCase()} |`); + lines.push(`| Session | ${remote.session.toUpperCase()} |`); + lines.push(`| Protocol headers | ${remote.protocolHeaders.toUpperCase()} |`); + lines.push(`| Authorization | ${remote.authorization.toUpperCase()} |`); + lines.push(`| Overall | ${remote.overall.toUpperCase()} |`); + } } export function buildMarkdownReport( diff --git a/src/reporting/render-text-report.ts b/src/reporting/render-text-report.ts index 6282225..e79153d 100644 --- a/src/reporting/render-text-report.ts +++ b/src/reporting/render-text-report.ts @@ -50,18 +50,30 @@ function appendRuntimeScorecard(lines: string[], result: CheckResult) { const conformance = result.runtimeScorecard.conformance; - if (!conformance) { - return; + if (conformance) { + lines.push("", "MCP Conformance", "---------------"); + lines.push(`Protocol version: ${conformance.protocolVersion ?? "unavailable"}`); + lines.push(`Profile: ${conformance.profile ?? "unavailable"}`); + lines.push(`Capability consistency: ${conformance.capabilityConsistency}`); + lines.push(`Task declarations: ${conformance.taskDeclarations}`); + lines.push(`Tasks list: ${conformance.tasksList}`); + lines.push(`Schema dialect: ${conformance.schemaDialect}`); + lines.push(`Overall: ${conformance.overall}`); } - lines.push("", "MCP Conformance", "---------------"); - lines.push(`Protocol version: ${conformance.protocolVersion ?? "unavailable"}`); - lines.push(`Profile: ${conformance.profile ?? "unavailable"}`); - lines.push(`Capability consistency: ${conformance.capabilityConsistency}`); - lines.push(`Task declarations: ${conformance.taskDeclarations}`); - lines.push(`Tasks list: ${conformance.tasksList}`); - lines.push(`Schema dialect: ${conformance.schemaDialect}`); - lines.push(`Overall: ${conformance.overall}`); + const remote = result.runtimeScorecard.remote; + + if (remote) { + lines.push("", "Remote MCP Scorecard", "--------------------"); + lines.push(`transport: ${remote.transport}`); + lines.push(`network safety: ${remote.networkSafety}`); + lines.push(`initialize: ${remote.initialize}`); + lines.push(`content type: ${remote.contentType}`); + lines.push(`session: ${remote.session}`); + lines.push(`protocol headers: ${remote.protocolHeaders}`); + lines.push(`authorization: ${remote.authorization}`); + lines.push(`overall: ${remote.overall}`); + } } export function renderTextReport( diff --git a/src/rules/rule-catalog.ts b/src/rules/rule-catalog.ts index b97bb2a..c0411b7 100644 --- a/src/rules/rule-catalog.ts +++ b/src/rules/rule-catalog.ts @@ -180,6 +180,24 @@ export const ruleCatalog: RuleDefinition[] = [ fix: "Add `command` for stdio servers or `url` for remote servers.", example: '{ "command": "node", "args": ["server.js"] }' }, + { + id: "mcp.server.transport.conflict", + category: "mcp", + defaultSeverity: "fail", + summary: "An MCP server defines both command and URL transports.", + why: "Clients cannot select a transport deterministically when a server defines both process and remote connection settings.", + fix: "Keep either command for stdio or url for remote MCP, but not both.", + example: '{ "url": "https://example.com/mcp" }' + }, + { + id: "plugin.mcp.server.transport.conflict", + category: "mcp", + defaultSeverity: "fail", + summary: "A bundled MCP server defines both command and URL transports.", + why: "Codex cannot select a transport deterministically when a bundled server defines both process and remote connection settings.", + fix: "Keep either command for stdio or url for remote MCP, but not both.", + example: '{ "url": "https://example.com/mcp" }' + }, { id: "plugin.security.path_traversal", category: "security", @@ -264,12 +282,66 @@ export const ruleCatalog: RuleDefinition[] = [ { id: "plugin.security.insecure_http_url", category: "security", - defaultSeverity: "warn", + defaultSeverity: "fail", summary: "An MCP server uses a plain HTTP URL.", why: "Plain HTTP can expose MCP traffic and does not verify endpoint identity on non-local networks.", fix: "Use HTTPS for remote MCP servers; reserve HTTP for explicit localhost development endpoints.", example: '{ "url": "https://example.com/mcp" }' }, + { + id: "plugin.security.remote_mcp_url.invalid", + category: "security", + defaultSeverity: "fail", + summary: "An MCP server URL is not an absolute HTTP or HTTPS URL.", + why: "Clients cannot reliably connect to malformed or relative remote MCP endpoints.", + fix: "Use an absolute HTTPS URL, or an explicit localhost HTTP development URL.", + example: '{ "url": "https://example.com/mcp" }' + }, + { + id: "plugin.security.remote_mcp_url.unsupported_scheme", + category: "security", + defaultSeverity: "fail", + summary: "An MCP server URL uses an unsupported scheme.", + why: "Remote MCP transport supports only HTTP and HTTPS endpoints.", + fix: "Use an HTTPS URL, or an explicit localhost HTTP development URL.", + example: '{ "url": "https://example.com/mcp" }' + }, + { + id: "plugin.security.remote_mcp_url.credentials", + category: "security", + defaultSeverity: "fail", + summary: "An MCP server URL embeds credentials.", + why: "URL credentials can leak through configuration, logs, reports, and package artifacts.", + fix: "Remove URL credentials and configure authentication outside the endpoint URL.", + example: '{ "url": "https://example.com/mcp" }' + }, + { + id: "plugin.security.remote_mcp_url.query", + category: "security", + defaultSeverity: "fail", + summary: "An MCP server URL contains a query string.", + why: "Query strings can carry secrets and make endpoint configuration ambiguous.", + fix: "Remove the query string from the MCP endpoint URL.", + example: '{ "url": "https://example.com/mcp" }' + }, + { + id: "plugin.security.remote_mcp_url.fragment", + category: "security", + defaultSeverity: "fail", + summary: "An MCP server URL contains a fragment.", + why: "Fragments are not sent to servers and can hide misleading endpoint configuration.", + fix: "Remove the fragment from the MCP endpoint URL.", + example: '{ "url": "https://example.com/mcp" }' + }, + { + id: "plugin.security.remote_mcp_url.ip_literal", + category: "security", + defaultSeverity: "fail", + summary: "An MCP server URL uses a numeric IP literal.", + why: "Numeric IP endpoints bypass hostname-based endpoint review and are not an accepted remote MCP shape.", + fix: "Use a reviewed hostname; use localhost only for local development.", + example: '{ "url": "https://example.com/mcp" }' + }, { id: "plugin.security.mcp_binds_all_interfaces", category: "security", @@ -395,6 +467,114 @@ export const ruleCatalog: RuleDefinition[] = [ why: "MCP stdio transport requires newline-delimited JSON-RPC messages on stdout.", fix: "Send logs to stderr and reserve stdout for JSON-RPC protocol messages only.", example: "Use `console.error` for diagnostics in Node stdio servers." + }, + { + id: "plugin.runtime.remote.network_not_approved", + category: "runtime", + defaultSeverity: "fail", + summary: "Remote MCP probing was not explicitly approved.", + why: "Remote initialization creates outbound network traffic.", + fix: "Review the plan, then use --runtime --allow-network; add --allow-local-network only for localhost HTTP.", + example: "codex-plugin-doctor mcp . --runtime --allow-network" + }, + { + id: "plugin.runtime.remote.url.invalid", + category: "runtime", + defaultSeverity: "fail", + summary: "A remote MCP endpoint URL is unsafe or unsupported.", + why: "Credentials, queries, fragments, IP literals, and unsupported schemes bypass the remote probe boundary.", + fix: "Use an absolute HTTPS URL without credentials, query parameters, fragments, or IP literals.", + example: "https://mcp.example/mcp" + }, + { + id: "plugin.runtime.remote.transport.timeout", + category: "runtime", + defaultSeverity: "fail", + summary: "The remote MCP initialize request timed out.", + why: "A bounded probe cannot safely negotiate an unavailable endpoint.", + fix: "Make the endpoint reachable and complete initialize within the configured timeout.", + example: "Return a valid initialize response promptly." + }, + { + id: "plugin.runtime.remote.transport.response_too_large", + category: "runtime", + defaultSeverity: "fail", + summary: "The remote MCP response exceeded the bounded probe limit.", + why: "Unbounded remote responses can exhaust local resources.", + fix: "Return a compact initialize response and keep discovery metadata bounded.", + example: "Return only the MCP initialize result." + }, + { + id: "plugin.runtime.remote.transport.failed", + category: "runtime", + defaultSeverity: "fail", + summary: "The remote MCP transport request failed.", + why: "Protocol negotiation cannot proceed without a safe bounded connection.", + fix: "Verify endpoint reachability, TLS, and remote network policy eligibility.", + example: "Use a public HTTPS endpoint or explicitly approved localhost HTTP." + }, + { + id: "plugin.runtime.remote.http_status.invalid", + category: "runtime", + defaultSeverity: "fail", + summary: "The remote MCP initialize response used an unexpected HTTP status.", + why: "Streamable HTTP initialization requires a successful response before negotiation.", + fix: "Return HTTP 200 for initialize or publish valid OAuth discovery metadata for protected endpoints.", + example: "HTTP/1.1 200 OK" + }, + { + id: "plugin.runtime.remote.content_type.invalid", + category: "runtime", + defaultSeverity: "fail", + summary: "The remote MCP initialize response used an unsupported content type.", + why: "The probe can only validate JSON or Server-Sent Events protocol responses.", + fix: "Return application/json or text/event-stream with a JSON-RPC response.", + example: "Content-Type: application/json" + }, + { + id: "plugin.runtime.remote.session.invalid", + category: "runtime", + defaultSeverity: "fail", + summary: "The remote MCP session header is invalid.", + why: "An invalid session identifier cannot be safely replayed on the initialized notification.", + fix: "Return MCP-Session-Id only as visible ASCII characters.", + example: "MCP-Session-Id: session-123" + }, + { + id: "plugin.runtime.remote.initialize.invalid", + category: "runtime", + defaultSeverity: "fail", + summary: "The remote MCP initialize JSON-RPC result is invalid.", + why: "Invalid negotiation results leave capabilities and protocol version unknown.", + fix: "Return a JSON-RPC 2.0 initialize result for protocol version 2025-11-25.", + example: '{ "jsonrpc": "2.0", "id": 1, "result": { "protocolVersion": "2025-11-25" } }' + }, + { + id: "plugin.runtime.remote.initialized.failed", + category: "runtime", + defaultSeverity: "fail", + summary: "The remote MCP server did not acknowledge notifications/initialized.", + why: "The session may not be ready for subsequent protocol traffic.", + fix: "Accept a successful initialized notification at the configured MCP endpoint.", + example: "Return HTTP 204 to notifications/initialized." + }, + { + id: "plugin.runtime.remote.authorization.metadata.invalid", + category: "runtime", + defaultSeverity: "fail", + summary: "Remote OAuth discovery metadata is invalid.", + why: "Protected MCP endpoints cannot be assessed safely without valid authorization metadata.", + fix: "Publish valid HTTPS protected-resource and authorization-server metadata.", + example: "https://mcp.example/.well-known/oauth-protected-resource" + }, + { + id: "plugin.runtime.remote.authorization.metadata.unavailable", + category: "runtime", + defaultSeverity: "fail", + summary: "Remote OAuth discovery metadata is unavailable.", + why: "Authorization readiness cannot be confirmed without bounded metadata responses.", + fix: "Make protected-resource and authorization-server metadata available over HTTPS.", + example: "Return application/json discovery metadata." } ]; diff --git a/src/run-cli.ts b/src/run-cli.ts index 19617ff..6c78436 100644 --- a/src/run-cli.ts +++ b/src/run-cli.ts @@ -321,9 +321,47 @@ function parseRuntimeSandbox( return value; } +function parseRemoteNetworkFlags( + flags: string[], + runtime: boolean +): { allowNetwork: boolean; allowLocalNetwork: boolean } | CliUsageError { + let allowNetwork = false; + let allowLocalNetwork = false; + + for (let index = 0; index < flags.length; index += 1) { + const flag = flags[index]; + + if (flag === "--allow-network") { + if (allowNetwork) return new CliUsageError("Duplicate runtime network flag: --allow-network."); + if (flags[index + 1] && !flags[index + 1]!.startsWith("--")) { + return new CliUsageError("--allow-network does not accept a value."); + } + allowNetwork = true; + } else if (flag === "--allow-local-network") { + if (allowLocalNetwork) return new CliUsageError("Duplicate runtime network flag: --allow-local-network."); + if (flags[index + 1] && !flags[index + 1]!.startsWith("--")) { + return new CliUsageError("--allow-local-network does not accept a value."); + } + allowLocalNetwork = true; + } else if (flag?.startsWith("--allow-network=") || flag?.startsWith("--allow-local-network=")) { + return new CliUsageError(`${flag.split("=", 1)[0]} does not accept a value.`); + } + } + + if ((allowNetwork || allowLocalNetwork) && !runtime) { + return new CliUsageError("--allow-network requires --runtime."); + } + + if (allowLocalNetwork && !allowNetwork) { + return new CliUsageError("--allow-local-network requires --allow-network."); + } + + return { allowNetwork, allowLocalNetwork }; +} + function printUsage(io: CliIo): void { io.writeStderr( - "Usage: codex-plugin-doctor check [filter] [--policy codex-publish|mcp-strict|security] [--compat] [--json|--markdown|--badge-json|--badge-markdown] [--output ] [--history ] [--runtime] [--sandbox docker] [--require-runtime-approval --runtime-approval-digest ] [--verbose-runtime] [--explain] [--no-animations] [--ascii] [--changed-since ]\n codex-plugin-doctor audit --installed [filter] [--policy codex-publish|mcp-strict|security] [--security] [--compat] [--json] [--output ] [--cache] [--changed]\n codex-plugin-doctor audit deps [--policy codex-publish|mcp-strict|security] [--recommend] [--json|--sarif] [--output ]\n codex-plugin-doctor mcp [--runtime] [--json] [--output ]\n codex-plugin-doctor security [--policy security] [--json|--scorecard]\n codex-plugin-doctor compat [--all|--client ] [--json] [--scorecard] [--output ] [--install-preview|--apply --backup]\n codex-plugin-doctor suppress add [--fingerprint --reason --expires-at YYYY-MM-DD] [--config ] [--json]\n codex-plugin-doctor suppress list [--config ] [--json]\n codex-plugin-doctor suppress remove [--fingerprint |--index ] [--config ] [--json]\n codex-plugin-doctor fix (--dry-run|--interactive --backup|--apply --backup)\n codex-plugin-doctor history [--json] [--fail-on-regression]\n codex-plugin-doctor watch [--runtime] [--json] [--output ] [--debounce-ms ] [--max-iterations ] [--fail-fast] [--accumulate-json ]\n codex-plugin-doctor doctor [npm |contract|corpus [--manifest ] [--json] [--output ]|corpus metrics --manifest [--json|--markdown] [--output ] [--min-precision <0..1>] [--min-recall <0..1>] [--max-false-positive-rate <0..1>]|runtime-plan [--runtime --sandbox docker] [--json|--markdown] [--output ]|runtime-policy [--runtime --sandbox docker] [--json] [--output ]|review-bundle --output --sign-key-env NAME [--json] [--allow-dirty] [--allow-untagged]|review-bundle verify --target --sign-key-env NAME [--json] [--output ] [--failures-only]|review-bundle diff --before --after [--json]|attest [--sign-key-env NAME]|attest verify --target --sign-key-env NAME|release-evidence --sign-key-env NAME [--runtime --sandbox docker] [--allow-dirty] [--allow-untagged] [--require-runtime-approval --runtime-approval-digest ]|release-evidence verify --target --sign-key-env NAME|release-evidence asset --tag --output --sign-key-env NAME [--upload]|mcp |inspector |diff --before --after |recommend |trust |perf [--max-total-ms ] [--max-stage-ms stage=ms]|export --bundle |snapshot|clients|--json|--update-check]\n codex-plugin-doctor init [path] [--template skill-only|mcp-stdio|mcp-http|full-runtime]\n codex-plugin-doctor init-ci [path]\n codex-plugin-doctor init-git-hooks [path] [--force] [--json]\n codex-plugin-doctor init-git-hooks [path] --remove [--json]\n codex-plugin-doctor completion bash|zsh|fish\n codex-plugin-doctor config validate [--json]\n codex-plugin-doctor release check [--json] [--runtime --sandbox docker]\n codex-plugin-doctor self-test\n codex-plugin-doctor list --installed\n codex-plugin-doctor explain \n codex-plugin-doctor --version\n\nFirst run:\n codex-plugin-doctor doctor\n codex-plugin-doctor self-test\n codex-plugin-doctor init my-plugin\n codex-plugin-doctor check . --runtime --explain" + "Usage: codex-plugin-doctor check [filter] [--policy codex-publish|mcp-strict|security] [--compat] [--json|--markdown|--badge-json|--badge-markdown] [--output ] [--history ] [--runtime [--allow-network [--allow-local-network]]] [--sandbox docker] [--require-runtime-approval --runtime-approval-digest ] [--verbose-runtime] [--explain] [--no-animations] [--ascii] [--changed-since ]\n codex-plugin-doctor audit --installed [filter] [--policy codex-publish|mcp-strict|security] [--security] [--compat] [--json] [--output ] [--cache] [--changed]\n codex-plugin-doctor audit deps [--policy codex-publish|mcp-strict|security] [--recommend] [--json|--sarif] [--output ]\n codex-plugin-doctor mcp [--runtime [--allow-network [--allow-local-network]]] [--json] [--output ]\n codex-plugin-doctor security [--policy security] [--json|--scorecard]\n codex-plugin-doctor compat [--all|--client ] [--json] [--scorecard] [--output ] [--install-preview|--apply --backup]\n codex-plugin-doctor suppress add [--fingerprint --reason --expires-at YYYY-MM-DD] [--config ] [--json]\n codex-plugin-doctor suppress list [--config ] [--json]\n codex-plugin-doctor suppress remove [--fingerprint |--index ] [--config ] [--json]\n codex-plugin-doctor fix (--dry-run|--interactive --backup|--apply --backup)\n codex-plugin-doctor history [--json] [--fail-on-regression]\n codex-plugin-doctor watch [--runtime] [--json] [--output ] [--debounce-ms ] [--max-iterations ] [--fail-fast] [--accumulate-json ]\n codex-plugin-doctor doctor [npm |contract|corpus [--manifest ] [--json] [--output ]|corpus metrics --manifest [--json|--markdown] [--output ] [--min-precision <0..1>] [--min-recall <0..1>] [--max-false-positive-rate <0..1>]|runtime-plan [--sandbox docker] [--json|--markdown] [--output ]|runtime-policy [--sandbox docker] [--json] [--output ]|review-bundle --output --sign-key-env NAME [--json] [--allow-dirty] [--allow-untagged]|review-bundle verify --target --sign-key-env NAME [--json] [--output ] [--failures-only]|review-bundle diff --before --after [--json]|attest [--sign-key-env NAME]|attest verify --target --sign-key-env NAME|release-evidence --sign-key-env NAME [--runtime [--allow-network [--allow-local-network]]] [--sandbox docker] [--allow-dirty] [--allow-untagged] [--require-runtime-approval --runtime-approval-digest ]|release-evidence verify --target --sign-key-env NAME|release-evidence asset --tag --output --sign-key-env NAME [--runtime [--allow-network [--allow-local-network]]] [--sandbox docker] [--allow-dirty] [--allow-untagged] [--require-runtime-approval --runtime-approval-digest ] [--upload]|mcp [--runtime [--allow-network [--allow-local-network]]]|inspector |diff --before --after |recommend |trust |perf [--max-total-ms ] [--max-stage-ms stage=ms]|export --bundle |snapshot|clients|--json|--update-check]\n codex-plugin-doctor init [path] [--template skill-only|mcp-stdio|mcp-http|full-runtime]\n codex-plugin-doctor init-ci [path]\n codex-plugin-doctor init-git-hooks [path] [--force] [--json]\n codex-plugin-doctor init-git-hooks [path] --remove [--json]\n codex-plugin-doctor completion bash|zsh|fish\n codex-plugin-doctor config validate [--json]\n codex-plugin-doctor release check [--json] [--runtime [--allow-network [--allow-local-network]]] [--sandbox docker]\n codex-plugin-doctor self-test\n codex-plugin-doctor list --installed\n codex-plugin-doctor explain \n codex-plugin-doctor --version\n\nFirst run:\n codex-plugin-doctor doctor\n codex-plugin-doctor self-test\n codex-plugin-doctor init my-plugin\n codex-plugin-doctor check . --runtime --explain" ); io.writeStderr( "Corpus quality regression: codex-plugin-doctor doctor corpus metrics diff --before --after [--fail-on-regression] [--json|--markdown] [--output ]" @@ -1145,6 +1183,8 @@ function buildGenericMcpDoctorCommandArgs(commandTarget: string, flags: string[] jsonOutput: boolean; outputPath: string | null; runtime: boolean; + allowNetwork: boolean; + allowLocalNetwork: boolean; } | string { if (!commandTarget || commandTarget.startsWith("--")) { return "Missing target path. Usage: codex-plugin-doctor mcp [--runtime] [--json] [--output ]"; @@ -1153,6 +1193,8 @@ function buildGenericMcpDoctorCommandArgs(commandTarget: string, flags: string[] let jsonOutput = false; let outputPath: string | null = null; let runtime = false; + let allowNetwork = false; + let allowLocalNetwork = false; for (let index = 0; index < flags.length; index += 1) { const flag = flags[index]; @@ -1175,6 +1217,18 @@ function buildGenericMcpDoctorCommandArgs(commandTarget: string, flags: string[] continue; } + if (flag === "--allow-network") { + if (allowNetwork) return "Duplicate MCP flag: --allow-network."; + allowNetwork = true; + continue; + } + + if (flag === "--allow-local-network") { + if (allowLocalNetwork) return "Duplicate MCP flag: --allow-local-network."; + allowLocalNetwork = true; + continue; + } + if (flag === "--output") { if (outputPath !== null) { return "Duplicate MCP flag: --output."; @@ -1196,11 +1250,19 @@ function buildGenericMcpDoctorCommandArgs(commandTarget: string, flags: string[] : `Unexpected MCP argument: ${flag}.`; } + const remoteNetwork = parseRemoteNetworkFlags(flags, runtime); + + if (remoteNetwork instanceof CliUsageError) { + return remoteNetwork.message; + } + return { targetPath: commandTarget, jsonOutput, outputPath, - runtime + runtime, + allowNetwork, + allowLocalNetwork }; } @@ -1590,7 +1652,9 @@ export async function runCli( env: terminalContext.env, platform: terminalContext.platform }, { - runtime: parsedMcpArgs.runtime + runtime: parsedMcpArgs.runtime, + allowNetwork: parsedMcpArgs.allowNetwork, + allowLocalNetwork: parsedMcpArgs.allowLocalNetwork }); const renderedReport = parsedMcpArgs.jsonOutput ? renderGenericMcpDoctorJson(report) @@ -1893,8 +1957,14 @@ export async function runCli( ? null : assetFlags[runtimeApprovalDigestIndex + 1]; const runtime = assetFlags.includes("--runtime"); + const remoteNetwork = parseRemoteNetworkFlags(assetFlags, runtime); let runtimeSandbox: RuntimeSandboxMode | null; + if (remoteNetwork instanceof CliUsageError) { + io.writeStderr(remoteNetwork.message); + return 2; + } + try { runtimeSandbox = parseRuntimeSandbox(assetFlags); } catch (error) { @@ -1973,6 +2043,8 @@ export async function runCli( requireRuntimeApproval, runtimeApprovalDigest, runtime, + ...(remoteNetwork.allowNetwork ? { allowNetwork: true } : {}), + ...(remoteNetwork.allowLocalNetwork ? { allowLocalNetwork: true } : {}), ...(runtimeSandbox ? { sandbox: runtimeSandbox } : {}), environment: { env: terminalContext.env, @@ -1981,6 +2053,11 @@ export async function runCli( runCheck: options.runCheckImpl ?? runCheck, performanceThresholds: parsedThresholds.thresholds }); + + if (!evidence.releaseReady) { + io.writeStdout(jsonOutput ? renderDoctorReleaseEvidenceJson(evidence) : renderDoctorReleaseEvidence(evidence)); + return evidence.exitCode; + } await writeFile(resolvedOutputPath, renderDoctorReleaseEvidenceJson(evidence), "utf8"); let uploaded = false; @@ -2096,8 +2173,14 @@ export async function runCli( ? null : evidenceFlags[runtimeApprovalDigestIndex + 1]; const runtime = evidenceFlags.includes("--runtime"); + const remoteNetwork = parseRemoteNetworkFlags(evidenceFlags, runtime); let runtimeSandbox: RuntimeSandboxMode | null; + if (remoteNetwork instanceof CliUsageError) { + io.writeStderr(remoteNetwork.message); + return 2; + } + try { runtimeSandbox = parseRuntimeSandbox(evidenceFlags); } catch (error) { @@ -2155,6 +2238,8 @@ export async function runCli( requireRuntimeApproval, runtimeApprovalDigest, runtime, + ...(remoteNetwork.allowNetwork ? { allowNetwork: true } : {}), + ...(remoteNetwork.allowLocalNetwork ? { allowLocalNetwork: true } : {}), ...(runtimeSandbox ? { sandbox: runtimeSandbox } : {}), environment: { env: terminalContext.env, @@ -3175,7 +3260,9 @@ export async function runCli( env: terminalContext.env, platform: terminalContext.platform }, { - runtime: parsedMcpArgs.runtime + runtime: parsedMcpArgs.runtime, + allowNetwork: parsedMcpArgs.allowNetwork, + allowLocalNetwork: parsedMcpArgs.allowLocalNetwork }); const renderedReport = parsedMcpArgs.jsonOutput ? renderGenericMcpDoctorJson(report) @@ -3515,8 +3602,14 @@ export async function runCli( : remainingArgs.slice(1); const jsonOutput = releaseFlags.includes("--json"); const runtimeProbeEnabled = releaseFlags.includes("--runtime"); + const remoteNetwork = parseRemoteNetworkFlags(releaseFlags, runtimeProbeEnabled); let runtimeSandbox: RuntimeSandboxMode | null; + if (remoteNetwork instanceof CliUsageError) { + io.writeStderr(remoteNetwork.message); + return 2; + } + try { runtimeSandbox = parseRuntimeSandbox(releaseFlags); } catch (error) { @@ -3564,6 +3657,8 @@ export async function runCli( env: terminalContext.env, platform: terminalContext.platform, runtime: runtimeProbeEnabled, + ...(remoteNetwork.allowNetwork ? { allowNetwork: true } : {}), + ...(remoteNetwork.allowLocalNetwork ? { allowLocalNetwork: true } : {}), ...(runtimeSandbox ? { runtimeSandbox } : {}), runCheck: options.runCheckImpl }); @@ -3746,6 +3841,12 @@ export async function runCli( runtimeProbeEnabled || checkProfile === "publish" || policyEnablesRuntime(policy); + const remoteNetwork = parseRemoteNetworkFlags(normalizedFlags, effectiveRuntimeProbeEnabled); + + if (remoteNetwork instanceof CliUsageError) { + io.writeStderr(remoteNetwork.message); + return 2; + } if (requireRuntimeApproval && !effectiveRuntimeProbeEnabled) { io.writeStderr("Runtime approval requires runtime probing. Add --runtime, --profile publish, or a runtime-enabled policy."); @@ -3829,6 +3930,8 @@ export async function runCli( result: applyDoctorConfig( await runCheckImpl(plugin.rootPath, { runtime: effectiveRuntimeProbeEnabled, + ...(remoteNetwork.allowNetwork ? { allowNetwork: true } : {}), + ...(remoteNetwork.allowLocalNetwork ? { allowLocalNetwork: true } : {}), runtimeTranscript: effectiveRuntimeProbeEnabled && verboseRuntime ? (line) => io.writeStderr(line) @@ -3929,7 +4032,9 @@ export async function runCli( const pluginResult = applyDoctorConfig( await runCheckImpl(pluginRoot, { - runtime: effectiveRuntimeProbeEnabled + runtime: effectiveRuntimeProbeEnabled, + ...(remoteNetwork.allowNetwork ? { allowNetwork: true } : {}), + ...(remoteNetwork.allowLocalNetwork ? { allowLocalNetwork: true } : {}) }), applyPolicyToDoctorConfig( applyCheckProfile(await loadDoctorConfig(pluginRoot, configPath), checkProfile), @@ -3963,6 +4068,8 @@ export async function runCli( const configuredResult = applyDoctorConfig( await runCheckImpl(targetPath, { runtime: effectiveRuntimeProbeEnabled, + ...(remoteNetwork.allowNetwork ? { allowNetwork: true } : {}), + ...(remoteNetwork.allowLocalNetwork ? { allowLocalNetwork: true } : {}), ...(runtimeSandbox ? { runtimeSandbox } : {}), ...(effectiveRuntimeProbeEnabled && verboseRuntime ? { runtimeTranscript: (line: string) => io.writeStderr(line) } diff --git a/src/security/security-audit.ts b/src/security/security-audit.ts index 4194c8f..a1ecc9d 100644 --- a/src/security/security-audit.ts +++ b/src/security/security-audit.ts @@ -3,6 +3,7 @@ import path from "node:path"; import { discoverPackage } from "../core/discover-package.js"; import { readJsonFile } from "../core/read-json-file.js"; +import { inspectRemoteMcpUrl } from "../core/remote-url-policy.js"; import { validatePlugin } from "../core/validate-plugin.js"; import type { DiscoveredPackage, Finding, FindingEvidence } from "../domain/types.js"; import { @@ -45,6 +46,18 @@ function isPlainObject(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } +function remoteUrlIssueFindingId(issue: string): string { + return issue === "insecure_non_loopback" + ? "plugin.security.insecure_http_url" + : `plugin.security.remote_mcp_url.${issue}`; +} + +function remoteUrlIssueMessage(issue: string): string { + return issue === "insecure_non_loopback" + ? "uses an insecure public HTTP URL" + : `uses a remote MCP URL with ${issue.replaceAll("_", " ")}`; +} + function isPathWithinRoot(rootPath: string, candidatePath: string): boolean { const relativePath = path.relative(rootPath, candidatePath); @@ -416,30 +429,42 @@ export function auditMcpServerConfig( } } - if (typeof url === "string" && /^http:\/\//i.test(url)) { - findings.push( - buildFinding( - "warn", - "plugin.security.insecure_http_url", - `The MCP server \`${serverName}\` uses an insecure HTTP URL.`, - "Plain HTTP transports can expose MCP traffic on non-local networks and make endpoint identity harder to verify.", - "Use HTTPS for remote MCP servers; reserve HTTP for explicit localhost development endpoints.", - { serverName, configPath, url } - ) - ); - } + if (typeof url === "string") { + const inspection = inspectRemoteMcpUrl(url); - if (typeof url === "string" && /\/\/0\.0\.0\.0[:/]/i.test(url)) { - findings.push( - buildFinding( - "warn", - "plugin.security.mcp_binds_all_interfaces", - `The MCP server \`${serverName}\` URL binds to \`0.0.0.0\`.`, - "Servers that listen on all interfaces can accept connections from external hosts, which is rarely intended for local MCP development.", - "Use `127.0.0.1` or `localhost` instead of `0.0.0.0` unless external access is explicitly required.", - { serverName, configPath, url } - ) - ); + for (const issue of inspection.issues) { + findings.push( + buildFinding( + "fail", + remoteUrlIssueFindingId(issue), + `The MCP server \`${serverName}\` ${remoteUrlIssueMessage(issue)}.`, + "Unsafe or ambiguous remote transport configuration can expose credentials or prevent reliable MCP connectivity.", + "Use an absolute HTTPS URL without credentials, query parameters, fragments, or numeric IP literals; HTTP is only supported for localhost development.", + { + serverName, + configPath, + url: inspection.sanitizedUrl + } + ) + ); + } + + if (inspection.parsedUrl?.hostname === "0.0.0.0") { + findings.push( + buildFinding( + "warn", + "plugin.security.mcp_binds_all_interfaces", + `The MCP server \`${serverName}\` URL binds to \`0.0.0.0\`.`, + "Servers that listen on all interfaces can accept connections from external hosts, which is rarely intended for local MCP development.", + "Use `127.0.0.1` or `localhost` instead of `0.0.0.0` unless external access is explicitly required.", + { + serverName, + configPath, + url: inspection.sanitizedUrl + } + ) + ); + } } } @@ -448,10 +473,16 @@ export function auditMcpServerConfig( const externalUrlPattern = /https?:\/\/[^\s`"'<>)]+/gi; -async function auditSkillExternalReferences(rootPath: string): Promise { +async function auditSkillExternalReferences( + rootPath: string, + mcpConfigPath: string | null +): Promise { const findings: Finding[] = []; for (const filePath of await collectPromptPoisoningScanFiles(rootPath)) { + if (mcpConfigPath !== null && path.resolve(filePath) === mcpConfigPath) { + continue; + } const content = await readFile(filePath, "utf8"); const matches = content.match(externalUrlPattern); @@ -734,7 +765,12 @@ export async function buildSecurityAudit(targetPath: string): Promise { expect(actionMetadata).toContain('exit "$status"'); }); + it("requires explicit, environment-backed consent for remote MCP network probing", async () => { + const actionMetadata = await readFile("action.yml", "utf8"); + + expect(actionMetadata).toContain("allow-network:"); + expect(actionMetadata).toContain("allow-local-network:"); + expect(actionMetadata).toMatch(/allow-network:[\s\S]*?default: "false"/); + expect(actionMetadata).toMatch(/allow-local-network:[\s\S]*?default: "false"/); + expect(actionMetadata).toContain('ALLOW_NETWORK_INPUT: ${{ inputs[\'allow-network\'] }}'); + expect(actionMetadata).toContain('ALLOW_LOCAL_NETWORK_INPUT: ${{ inputs[\'allow-local-network\'] }}'); + expect(actionMetadata).toContain('if [[ "$ALLOW_NETWORK_INPUT" == "true" ]]; then'); + expect(actionMetadata).toContain('args+=(--allow-network)'); + expect(actionMetadata).toContain('if [[ "$ALLOW_LOCAL_NETWORK_INPUT" == "true" ]]; then'); + expect(actionMetadata).toContain('args+=(--allow-local-network)'); + expect(actionMetadata).not.toContain('args+=(--allow-network "${{ inputs'); + expect(actionMetadata).not.toContain('args+=(--allow-local-network "${{ inputs'); + }); + + it("documents loopback-only consent without permitting private or reserved ranges", async () => { + const actionMetadata = await readFile("action.yml", "utf8"); + const actionUsage = await readFile("docs/guides/github-action.md", "utf8"); + const readiness = await readFile("docs/architecture/remote-mcp-readiness.md", "utf8"); + const securityArchitecture = await readFile("docs/security/security-architecture.md", "utf8"); + + for (const document of [actionMetadata, actionUsage, readiness, securityArchitecture]) { + expect(document).toContain("loopback endpoints only"); + expect(document).toContain("Private, link-local, multicast, unspecified, reserved, and NAT64 ranges remain blocked."); + } + }); + it("documents the public GitHub Action consumer workflow", async () => { const readme = await readFile("README.md", "utf8"); const actionUsage = await readFile("docs/guides/github-action.md", "utf8"); diff --git a/tests/bounded-http-client.test.ts b/tests/bounded-http-client.test.ts new file mode 100644 index 0000000..c0cde8d --- /dev/null +++ b/tests/bounded-http-client.test.ts @@ -0,0 +1,328 @@ +import { EventEmitter } from "node:events"; +import { createServer, type ClientRequest, type Server } from "node:http"; +import type { AddressInfo } from "node:net"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { requestBoundedHttp } from "../src/core/bounded-http-client.js"; +import type { RemoteLookup } from "../src/core/remote-network-policy.js"; + +const requestMock = vi.hoisted(() => vi.fn()); + +vi.mock("node:http", async (importOriginal) => { + const actual = await importOriginal(); + requestMock.mockImplementation(actual.request); + return { ...actual, request: requestMock }; +}); + +const servers: Server[] = []; + +afterEach(async () => { + await Promise.all(servers.splice(0).map((server) => new Promise((resolve, reject) => { + server.close((error) => error ? reject(error) : resolve()); + }))); +}); + +async function startServer(handler: Parameters[0]): Promise { + const server = createServer(handler); + servers.push(server); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + return (server.address() as AddressInfo).port; +} + +function localLookup(): RemoteLookup { + return async () => [{ address: "127.0.0.1", family: 4 }]; +} + +function options(port: number) { + return { + allowLocalNetwork: true, + lookup: localLookup(), + url: `http://mcp.test:${port}/mcp` + }; +} + +describe("requestBoundedHttp", () => { + it("returns only safe response headers and sends an allowed request body", async () => { + const port = await startServer((request, response) => { + expect(request.method).toBe("POST"); + expect(request.headers["mcp-session-id"]).toBe("session-1"); + expect(request.headers["content-type"]).toBe("application/json"); + let body = ""; + request.setEncoding("utf8"); + request.on("data", (chunk) => { body += chunk; }); + request.on("end", () => { + expect(body).toBe('{"ping":true}'); + response.writeHead(200, { + "content-type": "application/json", + "mcp-session-id": "session-2", + "mcp-protocol-version": "2025-11-25", + "set-cookie": "secret=value", + "x-internal": "hidden" + }); + response.end('{"ok":true}'); + }); + }); + + const result = await requestBoundedHttp(options(port).url, { + ...options(port), + method: "POST", + body: '{"ping":true}', + headers: { + Accept: "application/json", + "Content-Type": "application/json", + "MCP-Session-Id": "session-1" + } + }); + + expect(result).toEqual({ + statusCode: 200, + headers: { + "content-type": "application/json", + "mcp-session-id": "session-2", + "mcp-protocol-version": "2025-11-25" + }, + body: Buffer.from('{"ok":true}') + }); + }); + + it.each(["Authorization", "Proxy-Authorization", "Cookie", "Set-Cookie", "Host", "Connection", "Transfer-Encoding", "Content-Length", "X-Unapproved"])( + "rejects caller header %s", + async (header) => { + await expect( + requestBoundedHttp("http://mcp.test:1/mcp", { + allowLocalNetwork: true, + lookup: localLookup(), + headers: { [header]: "value" } + }) + ).rejects.toMatchObject({ + code: "REMOTE_HTTP_HEADER_FORBIDDEN", + message: `Remote HTTP request header is not allowed: ${header.toLowerCase()}.` + }); + } + ); + + it.each([ + ["a case-variant duplicate", { Accept: "application/json", accept: "text/plain" }], + ["an array value", { Accept: ["application/json"] }], + ["a CRLF value", { Accept: "application/json\r\nX-Injected: yes" }], + ["a non-string value", { Accept: 1 }] + ])("rejects %s request header", async (_name, headers) => { + await expect( + requestBoundedHttp("http://mcp.test:1/mcp", { + allowLocalNetwork: true, + lookup: localLookup(), + headers: headers as never + }) + ).rejects.toMatchObject({ code: "REMOTE_HTTP_HEADER_FORBIDDEN" }); + }); + + it("canonicalizes allowed request header names", async () => { + const port = await startServer((request, response) => { + expect(request.rawHeaders).toContain("accept"); + response.end("ok"); + }); + + await expect(requestBoundedHttp(options(port).url, { + ...options(port), + headers: { Accept: "application/json" } + })).resolves.toMatchObject({ body: Buffer.from("ok") }); + }); + + it.each([ + ["timeoutMs", 0], + ["timeoutMs", -1], + ["timeoutMs", 3_001], + ["timeoutMs", Infinity], + ["timeoutMs", Number.NaN], + ["maxResponseBytes", 0], + ["maxResponseBytes", -1], + ["maxResponseBytes", 1_048_577], + ["maxResponseBytes", Infinity], + ["maxResponseBytes", Number.NaN] + ])("rejects invalid %s values", async (name, value) => { + await expect(requestBoundedHttp("http://mcp.test:1/mcp", { + allowLocalNetwork: true, + lookup: localLookup(), + [name]: value + })).rejects.toMatchObject({ + code: "REMOTE_HTTP_OPTIONS_INVALID", + message: "Remote HTTP request options are invalid." + }); + }); + + it("uses the timeout as a wall-clock deadline while DNS is unresolved", async () => { + const startedAt = Date.now(); + + await expect(requestBoundedHttp("http://mcp.test/mcp", { + lookup: async () => new Promise(() => undefined), + timeoutMs: 20 + })).rejects.toMatchObject({ + code: "REMOTE_HTTP_TIMEOUT", + message: "Remote HTTP request timed out." + }); + + expect(Date.now() - startedAt).toBeLessThan(500); + }); + + it("uses one deadline across DNS and a stalled request", async () => { + const port = await startServer(() => undefined); + const timeoutMs = 150; + const dnsDelayMs = 100; + const startedAt = Date.now(); + + await expect(requestBoundedHttp(`http://mcp.test:${port}/mcp`, { + allowLocalNetwork: true, + lookup: async () => { + await new Promise((resolve) => setTimeout(resolve, dnsDelayMs)); + return [{ address: "127.0.0.1", family: 4 }]; + }, + timeoutMs + })).rejects.toMatchObject({ + code: "REMOTE_HTTP_TIMEOUT", + message: "Remote HTTP request timed out." + }); + + expect(Date.now() - startedAt).toBeLessThan(350); + }); + + it("rejects redirects without following them and retains only the safe location", async () => { + const port = await startServer((_request, response) => { + response.writeHead(302, { location: "https://elsewhere.test/mcp", "set-cookie": "secret=value" }); + response.end(); + }); + + await expect(requestBoundedHttp(options(port).url, options(port))).rejects.toMatchObject({ + code: "REMOTE_HTTP_REDIRECT", + message: "Remote HTTP redirects are not allowed.", + statusCode: 302, + headers: { location: "https://elsewhere.test/mcp" } + }); + }); + + it("rejects compressed responses", async () => { + const port = await startServer((_request, response) => { + response.writeHead(200, { "content-encoding": "gzip" }); + response.end("not decompressed"); + }); + + await expect(requestBoundedHttp(options(port).url, options(port))).rejects.toMatchObject({ + code: "REMOTE_HTTP_ENCODING_UNSUPPORTED", + message: "Remote HTTP response content encoding must be identity." + }); + }); + + it("aborts responses that exceed the configured byte limit", async () => { + const port = await startServer((_request, response) => { + response.writeHead(200); + response.end("too-large"); + }); + + await expect(requestBoundedHttp(options(port).url, { ...options(port), maxResponseBytes: 4 })).rejects.toMatchObject({ + code: "REMOTE_HTTP_RESPONSE_TOO_LARGE", + message: "Remote HTTP response exceeded the configured size limit." + }); + }); + + it("returns after a caller-recognized bounded response prefix without waiting for EOF", async () => { + const port = await startServer((_request, response) => { + response.writeHead(200, { "content-type": "text/event-stream" }); + response.write('data: {"ok":true}\n\n'); + }); + + await expect(requestBoundedHttp(options(port).url, { + ...options(port), + stopAfter: (body) => body.toString("utf8").endsWith("\n\n") + })).resolves.toMatchObject({ + body: Buffer.from('data: {"ok":true}\n\n') + }); + }); + + it("contains stop condition failures and closes the response", async () => { + let responseClosed: Promise | undefined; + const port = await startServer((_request, response) => { + responseClosed = new Promise((resolve) => response.once("close", resolve)); + response.writeHead(200, { "content-type": "text/event-stream" }); + response.write('data: {"ok":true}\n\n'); + }); + + await expect(requestBoundedHttp(options(port).url, { + ...options(port), + timeoutMs: 1_000, + stopAfter: () => { + throw new Error("stop condition failure"); + } + })).rejects.toMatchObject({ + code: "REMOTE_HTTP_STOP_CONDITION_FAILED", + message: "Remote HTTP stop condition failed." + }); + + await expect(responseClosed).resolves.toBeUndefined(); + }); + + it("times out a non-responsive request", async () => { + const port = await startServer(() => undefined); + + await expect(requestBoundedHttp(options(port).url, { ...options(port), timeoutMs: 20 })).rejects.toMatchObject({ + code: "REMOTE_HTTP_TIMEOUT", + message: "Remote HTTP request timed out." + }); + }); + + it("uses the DNS-pinned local target", async () => { + const port = await startServer((_request, response) => response.end("ok")); + + await expect(requestBoundedHttp(options(port).url, options(port))).resolves.toMatchObject({ + statusCode: 200, + body: Buffer.from("ok") + }); + }); + + it("rejects a connected peer that differs from the DNS-pinned address", async () => { + const request = new EventEmitter(); + Object.assign(request, { + destroy: () => undefined, + end: () => process.nextTick(() => request.emit("response", { + headers: {}, + statusCode: 200, + socket: { remoteAddress: "127.0.0.2" }, + destroy: () => undefined + })), + setTimeout: () => request + }); + requestMock.mockReturnValueOnce(request as unknown as ClientRequest); + + await expect(requestBoundedHttp("http://mcp.test/mcp", { + allowLocalNetwork: true, + lookup: async () => [{ address: "127.0.0.1", family: 4 }] + })).rejects.toMatchObject({ + code: "REMOTE_HTTP_PEER_MISMATCH", + message: "Remote HTTP peer did not match the resolved target." + }); + }); + + it("never reuses a loopback socket after DNS changes to a public target", async () => { + let requests = 0; + const port = await startServer((_request, response) => { + requests += 1; + response.end("loopback"); + }); + let lookupCalls = 0; + const rebindingLookup: RemoteLookup = async () => { + lookupCalls += 1; + return [{ address: lookupCalls === 1 ? "127.0.0.1" : "8.8.8.8", family: 4 }]; + }; + const url = `http://mcp.test:${port}/mcp`; + + await expect(requestBoundedHttp(url, { + allowLocalNetwork: true, + lookup: rebindingLookup + })).resolves.toMatchObject({ body: Buffer.from("loopback") }); + + await expect(requestBoundedHttp(url, { + lookup: rebindingLookup, + timeoutMs: 20, + body: "must-not-reach-loopback" + })).rejects.toBeDefined(); + expect(requests).toBe(1); + }); +}); diff --git a/tests/cli-command.test.ts b/tests/cli-command.test.ts index 039f5ce..1dc7051 100644 --- a/tests/cli-command.test.ts +++ b/tests/cli-command.test.ts @@ -2981,6 +2981,40 @@ describe("runCli", () => { expect(report.summary.runtimeProbeEnabled).toBe(true); }); + it.each([ + ["--profile publish", ["--profile", "publish"]], + ["a runtime-enabled policy", ["--policy", "codex-publish"]] + ])("accepts network approvals when %s enables runtime", async (_label, runtimeFlags) => { + const { io, stderr } = createIo(); + const runCheckImpl = vi.fn(async (targetPath: string) => ({ + targetPath, + status: "pass" as const, + exitCode: 0 as const, + findings: [] + })); + + const exitCode = await runCli( + [ + "check", + "tests/fixtures/valid-plugin", + ...runtimeFlags, + "--allow-network", + "--allow-local-network", + "--json" + ], + io, + { runCheckImpl } + ); + + expect(exitCode).toBe(0); + expect(stderr).toEqual([]); + expect(runCheckImpl).toHaveBeenCalledWith(expect.any(String), { + runtime: true, + allowNetwork: true, + allowLocalNetwork: true + }); + }); + it("initializes a minimal Codex plugin package", async () => { const targetPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-init-")); const { io, stdout, stderr } = createIo(); diff --git a/tests/contract-command.test.ts b/tests/contract-command.test.ts index e74ca37..5071ccf 100644 --- a/tests/contract-command.test.ts +++ b/tests/contract-command.test.ts @@ -265,6 +265,11 @@ describe("doctor contract command", () => { expect(mcpSchema.schema.properties.runtimeScorecard.description).toBe( "Present only when codex-plugin-doctor mcp --runtime is used." ); + expect(checkSchema.schema.properties.summary.properties.runtimeScorecard.properties.remote) + .toMatchObject({ + type: "object", + properties: { session: { enum: ["absent", "present-valid", "present-invalid"] } } + }); const suppressionSchemas = Object.fromEntries( output.schemas diff --git a/tests/json-runtime-scorecard.test.ts b/tests/json-runtime-scorecard.test.ts index b28f05b..4673daf 100644 --- a/tests/json-runtime-scorecard.test.ts +++ b/tests/json-runtime-scorecard.test.ts @@ -52,4 +52,47 @@ describe("runtime scorecard", () => { expect(report.summary).not.toHaveProperty("runtimeExecution"); }); + + it("includes the remote scorecard without changing stdio scorecard fields", () => { + const report = buildJsonReport( + { + targetPath: "/test/plugin", + status: "warn", + exitCode: 0, + findings: [], + runtimeScorecard: { + initialize: "skipped", + toolsList: "unsupported", + toolsCall: "unsupported", + resourcesList: "unsupported", + resourceRead: "unsupported", + resourceTemplatesList: "unsupported", + promptsList: "unsupported", + promptGet: "unsupported", + remote: { + transport: "pass", + networkSafety: "pass", + initialize: "skipped", + contentType: "skipped", + session: "absent", + protocolHeaders: "skipped", + authorization: "warn", + overall: "warn" + } + } + }, + { runtimeProbeEnabled: true } + ); + + expect(report.summary.runtimeScorecard?.remote).toEqual({ + transport: "pass", + networkSafety: "pass", + initialize: "skipped", + contentType: "skipped", + session: "absent", + protocolHeaders: "skipped", + authorization: "warn", + overall: "warn" + }); + }); }); diff --git a/tests/markdown-report.test.ts b/tests/markdown-report.test.ts index 14487b4..0aa1fff 100644 --- a/tests/markdown-report.test.ts +++ b/tests/markdown-report.test.ts @@ -84,6 +84,22 @@ describe("buildMarkdownReport", () => { expect(report).toContain("## Findings"); }); + it("renders the remote MCP scorecard with enum-only session state", () => { + const report = buildMarkdownReport({ + targetPath: "example", + status: "pass", + exitCode: 0, + findings: [], + runtimeScorecard: { + ...runtimeScorecard, + remote: { transport: "pass", networkSafety: "pass", initialize: "pass", contentType: "pass", session: "present-valid", protocolHeaders: "pass", authorization: "skipped", overall: "pass" } + } + }, { runtimeProbeEnabled: true }); + + expect(report).toContain("## Remote MCP Scorecard"); + expect(report).toContain("| Session | PRESENT-VALID |"); + }); + it("renders a CI-friendly markdown summary", async () => { const targetPath = path.resolve("tests/fixtures/heuristic-long-plugin-description"); const result = await runCheck(targetPath); diff --git a/tests/mcp-command.test.ts b/tests/mcp-command.test.ts index f0d65d8..dc6bc24 100644 --- a/tests/mcp-command.test.ts +++ b/tests/mcp-command.test.ts @@ -1,4 +1,5 @@ import { access, mkdir, mkdtemp, symlink, writeFile } from "node:fs/promises"; +import { createServer } from "node:http"; import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; @@ -35,7 +36,159 @@ async function createStandaloneMcpPackage(mcpConfig: unknown): Promise { return targetPath; } +async function startRemoteMcpServer(options: { invalidInitialize?: boolean } = {}): Promise<{ + url: string; + requests: string[]; + close(): Promise; +}> { + const requests: string[] = []; + const server = createServer((request, response) => { + const chunks: Buffer[] = []; + request.on("data", (chunk: Buffer) => chunks.push(chunk)); + request.on("end", () => { + const message = JSON.parse(Buffer.concat(chunks).toString("utf8")) as { method: string }; + requests.push(message.method); + + if (message.method === "initialize") { + if (options.invalidInitialize) { + response.writeHead(500, { "content-type": "application/json" }); + response.end(JSON.stringify({ error: "initialize failed" })); + return; + } + + response.writeHead(200, { "content-type": "application/json" }); + response.end(JSON.stringify({ + jsonrpc: "2.0", + id: 1, + result: { + protocolVersion: "2025-11-25", + capabilities: {}, + serverInfo: { name: "local", version: "1.0.0" } + } + })); + return; + } + + response.writeHead(204); + response.end(); + }); + }); + + await new Promise((resolve) => server.listen(0, resolve)); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("Expected a TCP listening address."); + } + + return { + url: `http://localhost:${address.port}/mcp`, + requests, + close: () => new Promise((resolve, reject) => server.close((error) => error ? reject(error) : resolve())) + }; +} + describe("mcp command", () => { + it("requires runtime and network approval flags before probing a local remote MCP server", async () => { + const remote = await startRemoteMcpServer(); + const targetPath = await createStandaloneMcpPackage({ + mcpServers: { local: { url: remote.url } } + }); + + try { + const missingRuntime = createIo(); + const missingNetwork = createIo(); + const approved = createIo(); + + expect(await runCli(["mcp", targetPath, "--allow-network"], missingRuntime.io)).toBe(2); + expect(missingRuntime.stderr.join("")).toContain("--allow-network requires --runtime"); + + expect(await runCli(["mcp", targetPath, "--runtime", "--allow-local-network"], missingNetwork.io)).toBe(2); + expect(missingNetwork.stderr.join("")).toContain("--allow-local-network requires --allow-network"); + + expect(await runCli([ + "mcp", targetPath, "--runtime", "--allow-network", "--allow-local-network", "--json" + ], approved.io)).toBe(0); + expect(JSON.parse(approved.stdout.join(""))).toMatchObject({ + runtimeScorecard: { + remote: { + networkSafety: "pass", + initialize: "pass", + protocolHeaders: "pass", + overall: "pass" + } + } + }); + expect(remote.requests).toEqual(["initialize", "notifications/initialized"]); + } finally { + await remote.close(); + } + }); + + it("forwards runtime network approvals through the doctor mcp alias", async () => { + const remote = await startRemoteMcpServer(); + const targetPath = await createStandaloneMcpPackage({ + mcpServers: { local: { url: remote.url } } + }); + const { io, stdout, stderr } = createIo(); + + try { + const exitCode = await runCli([ + "doctor", "mcp", targetPath, "--runtime", "--allow-network", "--allow-local-network", "--json" + ], io); + + expect(exitCode).toBe(0); + expect(stderr).toEqual([]); + expect(JSON.parse(stdout.join(""))).toMatchObject({ + runtimeScorecard: { remote: { networkSafety: "pass", overall: "pass" } } + }); + expect(remote.requests).toEqual(["initialize", "notifications/initialized"]); + } finally { + await remote.close(); + } + }); + + it("preserves the worst remote status when a later remote server passes", async () => { + const failing = await startRemoteMcpServer({ invalidInitialize: true }); + const passing = await startRemoteMcpServer(); + const targetPath = await createStandaloneMcpPackage({ + mcpServers: { + failing: { url: failing.url }, + passing: { url: passing.url } + } + }); + const { io, stdout, stderr } = createIo(); + + try { + const exitCode = await runCli([ + "mcp", targetPath, "--runtime", "--allow-network", "--allow-local-network", "--json" + ], io); + const output = JSON.parse(stdout.join("")); + + expect(exitCode).toBe(1); + expect(stderr).toEqual([]); + expect(output.runtimeScorecard.remote).toMatchObject({ + initialize: "fail", + overall: "fail" + }); + expect(failing.requests).toEqual(["initialize"]); + expect(passing.requests).toEqual(["initialize", "notifications/initialized"]); + } finally { + await failing.close(); + await passing.close(); + } + }); + + it("advertises doctor mcp and release evidence remote approval flags without dropping compat backups", async () => { + const { io, stderr } = createIo(); + + expect(await runCli([], io)).toBe(2); + + const usage = stderr.join(""); + expect(usage).toContain("doctor mcp [--runtime [--allow-network [--allow-local-network]]]"); + expect(usage).toContain("release-evidence asset --tag --output --sign-key-env NAME [--runtime [--allow-network [--allow-local-network]]]"); + expect(usage).toContain("[--install-preview|--apply --backup]"); + }); + it("renders finding fingerprints in text output", async () => { const targetPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-mcp-missing-")); const { io, stdout, stderr } = createIo(); @@ -76,6 +229,122 @@ describe("mcp command", () => { ); }); + it("accepts an explicit localhost HTTP development transport", async () => { + const targetPath = await createStandaloneMcpPackage({ + mcpServers: { + local: { url: "http://LOCALHOST:3000/mcp" } + } + }); + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["mcp", targetPath, "--json"], io); + const output = JSON.parse(stdout.join("")); + + expect(exitCode).toBe(0); + expect(stderr).toEqual([]); + expect(output.status).toBe("pass"); + expect(output.security.status).toBe("pass"); + }); + + it("fails conflicting remote transports without exposing URL credentials", async () => { + const rawUrl = "https://user:secret@example.com/mcp?token=secret"; + const targetPath = await createStandaloneMcpPackage({ + mcpServers: { + remote: { command: "node", url: rawUrl } + } + }); + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["mcp", targetPath, "--json"], io); + const serialized = stdout.join(""); + const output = JSON.parse(serialized); + + expect(exitCode).toBe(1); + expect(stderr).toEqual([]); + expect(output.findings).toHaveLength(3); + expect(output.findings.map((finding: { id: string }) => finding.id)).toEqual([ + "mcp.server.transport.conflict", + "plugin.security.remote_mcp_url.credentials", + "plugin.security.remote_mcp_url.query" + ]); + expect(serialized).not.toContain(rawUrl); + expect(serialized).not.toContain("secret"); + }); + + it("reports remote URL security evidence from a manifest-configured MCP file", async () => { + const targetPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-mcp-custom-config-")); + const mcpConfigPath = path.join(targetPath, "config", "remote.json"); + const rawUrl = "https://example.com/mcp?token=secret"; + + await mkdir(path.join(targetPath, ".codex-plugin"), { recursive: true }); + await mkdir(path.dirname(mcpConfigPath), { recursive: true }); + await writeFile( + path.join(targetPath, ".codex-plugin", "plugin.json"), + JSON.stringify({ + name: "custom-config", + version: "1.0.0", + description: "Custom MCP config fixture.", + mcpServers: "./config/remote.json" + }), + "utf8" + ); + await writeFile( + mcpConfigPath, + JSON.stringify({ + mcpServers: { + remote: { url: rawUrl } + } + }), + "utf8" + ); + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["mcp", targetPath, "--json"], io); + const serialized = stdout.join(""); + const output = JSON.parse(serialized); + + expect(exitCode).toBe(1); + expect(stderr).toEqual([]); + expect(output.security.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "plugin.security.remote_mcp_url.query", + evidence: expect.objectContaining({ + configPath: "config/remote.json", + serverName: "remote", + url: "https://example.com/mcp" + }) + }) + ]) + ); + expect(serialized).not.toContain(rawUrl); + expect(serialized).not.toContain("secret"); + }); + + it("reports empty query and fragment delimiters without exposing the remote URL", async () => { + const rawUrl = "https://example.com/mcp?#"; + const targetPath = await createStandaloneMcpPackage({ + mcpServers: { + remote: { url: rawUrl } + } + }); + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["mcp", targetPath, "--json"], io); + const serialized = stdout.join(""); + const output = JSON.parse(serialized); + + expect(exitCode).toBe(1); + expect(stderr).toEqual([]); + expect(output.findings.map((finding: { id: string }) => finding.id)).toEqual( + expect.arrayContaining([ + "plugin.security.remote_mcp_url.query", + "plugin.security.remote_mcp_url.fragment" + ]) + ); + expect(serialized).not.toContain(rawUrl); + }); + it("runs explicit runtime conformance for a valid task-capable MCP config", async () => { const { io, stdout, stderr } = createIo(); diff --git a/tests/policy-command.test.ts b/tests/policy-command.test.ts index 6bd9023..089a077 100644 --- a/tests/policy-command.test.ts +++ b/tests/policy-command.test.ts @@ -81,7 +81,7 @@ describe("policy packs", () => { ); }); - it("applies security policy to fail security warnings", async () => { + it("preserves blocking public HTTP findings under the security policy", async () => { const targetPath = await createHttpMcpPlugin(); const { io, stdout, stderr } = createIo(); @@ -93,7 +93,7 @@ describe("policy packs", () => { expect(output.status).toBe("fail"); expect(output.findings).toEqual( expect.arrayContaining([ - expect.objectContaining({ id: "plugin.security.insecure_http_url", severity: "warn" }) + expect.objectContaining({ id: "plugin.security.insecure_http_url", severity: "fail" }) ]) ); }); diff --git a/tests/public-readiness.test.ts b/tests/public-readiness.test.ts index d506f24..dee991b 100644 --- a/tests/public-readiness.test.ts +++ b/tests/public-readiness.test.ts @@ -66,4 +66,57 @@ describe("public repository readiness", () => { await expect(access("validation-sessions")).rejects.toThrow(); expect(`${readme}\n${docsReadme}`).not.toMatch(/validation-sessions|internal only/i); }); + + it("documents remote MCP transport and endpoint validation rules", async () => { + const catalog = await readText("docs/rules/catalog.md"); + + expect(catalog).toContain( + "| `mcp.server.transport.conflict` | fail | An MCP server defines both command and URL transports. |" + ); + expect(catalog).toContain( + "| `plugin.mcp.server.transport.conflict` | fail | A bundled MCP server defines both command and URL transports. |" + ); + expect(catalog).toContain( + "| `plugin.security.remote_mcp_url.invalid` | fail | An MCP server URL is not an absolute HTTP or HTTPS URL. |" + ); + expect(catalog).toContain( + "| `plugin.security.remote_mcp_url.unsupported_scheme` | fail | An MCP server URL uses an unsupported scheme. |" + ); + expect(catalog).toContain( + "| `plugin.security.remote_mcp_url.credentials` | fail | An MCP server URL embeds credentials. |" + ); + expect(catalog).toContain( + "| `plugin.security.remote_mcp_url.query` | fail | An MCP server URL contains a query string. |" + ); + expect(catalog).toContain( + "| `plugin.security.remote_mcp_url.fragment` | fail | An MCP server URL contains a fragment. |" + ); + expect(catalog).toContain( + "| `plugin.security.remote_mcp_url.ip_literal` | fail | An MCP server URL uses a numeric IP literal. |" + ); + }); + + it("publishes the remote MCP readiness boundary without exposing internal planning", async () => { + const readme = await readText("README.md"); + const actionGuide = await readText("docs/guides/github-action.md"); + const conformance = await readText("docs/architecture/mcp-2025-11-conformance.md"); + const readiness = await readText("docs/architecture/remote-mcp-readiness.md"); + const docsReadme = await readText("docs/README.md"); + const security = await readText("docs/security/security-architecture.md"); + expect(readme).toContain("Remote MCP Readiness"); + expect(actionGuide).toContain('allow-network: "true"'); + expect(actionGuide).toContain('allow-local-network: "true"'); + expect(conformance).toContain("OAuth metadata discovery"); + expect(readiness).toMatch(/explicit consent/i); + expect(readiness).toContain("SSRF"); + expect(readiness).toContain("NAT64 Pref64"); + expect(readiness).toContain("authenticated OAuth"); + expect(readiness).toContain("custom headers"); + expect(readiness).toContain("remote tool/resource/prompt/task calls"); + expect(readiness).toContain("GET SSE/resumability"); + expect(readiness).toContain("redirects"); + expect(docsReadme).toContain("Remote MCP Readiness"); + expect(security).toContain("runner or host egress controls"); + expect(readiness).not.toMatch(/internal (implementation )?plan/i); + }); }); diff --git a/tests/release-evidence-command.test.ts b/tests/release-evidence-command.test.ts index 5d5c3cc..741425e 100644 --- a/tests/release-evidence-command.test.ts +++ b/tests/release-evidence-command.test.ts @@ -283,6 +283,51 @@ describe("doctor release-evidence command", () => { expect(output.runtimeApproval.status).toBe("missing"); }); + it("rejects missing runtime approval before release evidence or asset scheduling", async () => { + const runCheckImpl = vi.fn(async () => { + throw new Error("runCheck must not be scheduled before runtime approval"); + }); + const outputPath = path.join( + await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-release-evidence-unapproved-")), + "release-evidence.json" + ); + const commands = [ + [ + "doctor", "release-evidence", "examples/codex-doctor-runtime", "--runtime", + "--require-runtime-approval", "--json", "--sign-key-env", "DOCTOR_SIGNING_KEY", + "--allow-dirty", "--allow-untagged" + ], + [ + "doctor", "release-evidence", "asset", "examples/codex-doctor-runtime", "--tag", "v1.1.0", + "--output", outputPath, "--runtime", "--require-runtime-approval", + "--runtime-approval-digest", "sha256:0000000000000000000000000000000000000000000000000000000000000000", "--json", + "--sign-key-env", "DOCTOR_SIGNING_KEY", "--allow-dirty", "--allow-untagged" + ] + ]; + + for (const command of commands) { + const { io, stdout, stderr } = createIo(); + const exitCode = await runCli(command, io, { + terminalContext: { + stdoutIsTTY: false, + stderrIsTTY: false, + env: { DOCTOR_SIGNING_KEY: "release-secret" }, + platform: "win32" + }, + runCheckImpl + }); + + const output = JSON.parse(stdout.join("")); + + expect(exitCode).toBe(1); + expect(stderr).toEqual([]); + expect(output.releaseReady).toBe(false); + expect(output.summary.runtimeApproval).toBe("fail"); + } + + expect(runCheckImpl).not.toHaveBeenCalled(); + }); + it("requires a signing key environment variable", async () => { const { io, stdout, stderr } = createIo(); diff --git a/tests/remote-mcp-probe.test.ts b/tests/remote-mcp-probe.test.ts new file mode 100644 index 0000000..651a5d3 --- /dev/null +++ b/tests/remote-mcp-probe.test.ts @@ -0,0 +1,318 @@ +import { createServer, type Server, type ServerResponse } from "node:http"; +import type { AddressInfo } from "node:net"; +import { afterEach, describe, expect, it } from "vitest"; + +import { probeRemoteMcpServer } from "../src/core/remote-mcp-probe.js"; +import type { BoundedHttpResponse } from "../src/core/bounded-http-client.js"; +import type { RemoteLookup } from "../src/core/remote-network-policy.js"; +import { packageVersion } from "../src/version.js"; + +const servers: Server[] = []; +const openResponses: ServerResponse[] = []; + +afterEach(async () => { + openResponses.splice(0).forEach((response) => response.destroy()); + await Promise.all(servers.splice(0).map((server) => new Promise((resolve, reject) => { + server.close((error) => error ? reject(error) : resolve()); + }))); +}); + +async function startServer(handler: Parameters[0]): Promise { + const server = createServer(handler); + servers.push(server); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + return (server.address() as AddressInfo).port; +} + +function localLookup(): RemoteLookup { + return async () => [{ address: "127.0.0.1", family: 4 }]; +} + +function options(port: number) { + return { + allowNetwork: true, + allowLocalNetwork: true, + lookup: localLookup(), + requestTimeoutMs: 100, + url: `http://localhost:${port}/mcp` + }; +} + +function initializedResponse(id: number) { + return JSON.stringify({ + jsonrpc: "2.0", + id, + result: { + protocolVersion: "2025-11-25", + capabilities: {}, + serverInfo: { name: "test", version: "1.0.0" } + } + }); +} + +function assertPrivate(value: unknown): void { + const serialized = JSON.stringify(value); + for (const sentinel of [ + "session-secret-sentinel", + "query-secret-sentinel", + "credential-secret-sentinel", + "challenge-secret-sentinel" + ]) { + expect(serialized).not.toContain(sentinel); + } +} + +describe("probeRemoteMcpServer", () => { + it("initializes with bounded JSON and sends initialized to the same endpoint", async () => { + const requests: Array<{ method: string; headers: Record; body: string }> = []; + const port = await startServer((request, response) => { + let body = ""; + request.setEncoding("utf8"); + request.on("data", (chunk) => { body += chunk; }); + request.on("end", () => { + requests.push({ method: request.method ?? "", headers: request.headers, body }); + const message = JSON.parse(body) as { id?: number; method: string }; + if (message.method === "initialize") { + response.writeHead(200, { + "content-type": "application/json; charset=utf-8", + "mcp-session-id": "session-secret-sentinel" + }); + response.end(initializedResponse(message.id ?? 1)); + return; + } + response.writeHead(202); + response.end(); + }); + }); + + const result = await probeRemoteMcpServer("remote", options(port).url, options(port)); + + expect(result.findings).toEqual([]); + expect(result.scorecard).toEqual({ + transport: "pass", + networkSafety: "pass", + initialize: "pass", + contentType: "pass", + session: "present-valid", + protocolHeaders: "pass", + authorization: "skipped", + overall: "pass" + }); + expect(requests).toHaveLength(2); + expect(requests.map((request) => request.method)).toEqual(["POST", "POST"]); + expect(requests.map((request) => JSON.parse(request.body).method)).toEqual([ + "initialize", + "notifications/initialized" + ]); + expect(JSON.parse(requests[0]?.body ?? "")).toEqual({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { + protocolVersion: "2025-11-25", + capabilities: {}, + clientInfo: { name: "Codex Plugin Doctor", version: packageVersion } + } + }); + expect(requests[0]?.headers.accept).toBe("application/json, text/event-stream"); + expect(requests[0]?.headers["content-type"]).toBe("application/json"); + expect(requests[0]?.headers.authorization).toBeUndefined(); + expect(requests[0]?.headers.cookie).toBeUndefined(); + expect(requests[1]?.headers["mcp-protocol-version"]).toBe("2025-11-25"); + expect(requests[1]?.headers["mcp-session-id"]).toBe("session-secret-sentinel"); + assertPrivate(result); + }); + + it("skips SSE primer events until the initialize response without waiting for the stream to close", async () => { + const port = await startServer((request, response) => { + let body = ""; + request.setEncoding("utf8"); + request.on("data", (chunk) => { body += chunk; }); + request.on("end", () => { + const message = JSON.parse(body) as { id?: number; method: string }; + if (message.method === "initialize") { + response.writeHead(200, { "content-type": "text/event-stream" }); + response.write("id: primer-event\ndata:\n\n"); + response.write("event: message\ndata: {\"jsonrpc\":\"2.0\",\"method\":\"notifications/progress\"}\n\n"); + response.write(`event: message\ndata: ${initializedResponse(message.id ?? 1)}\n\n`); + openResponses.push(response); + return; + } + response.writeHead(202); + response.end(); + }); + }); + + const result = await probeRemoteMcpServer("remote", options(port).url, options(port)); + + expect(result.findings).toEqual([]); + expect(result.scorecard.initialize).toBe("pass"); + expect(result.scorecard.contentType).toBe("pass"); + }); + + it("fails without network approval before issuing a request", async () => { + let requested = false; + + const result = await probeRemoteMcpServer("remote", "https://mcp.example/mcp", { + request: async () => { + requested = true; + throw new Error("must not run"); + } + }); + + expect(requested).toBe(false); + expect(result.scorecard.networkSafety).toBe("fail"); + expect(result.findings).toEqual([ + expect.objectContaining({ id: "plugin.runtime.remote.network_not_approved", severity: "fail" }) + ]); + }); + + it.each([ + ["an invalid content type", "text/plain", initializedResponse(1), "plugin.runtime.remote.content_type.invalid"], + ["malformed JSON-RPC", "application/json", "{", "plugin.runtime.remote.initialize.invalid"] + ])("fails %s deterministically", async (_name, contentType, body, findingId) => { + const port = await startServer((_request, response) => { + response.writeHead(200, { "content-type": contentType }); + response.end(body); + }); + + const result = await probeRemoteMcpServer("remote", options(port).url, options(port)); + + expect(result.findings).toEqual([ + expect.objectContaining({ id: findingId, severity: "fail" }) + ]); + }); + + it("fails a stalled or oversized bounded response", async () => { + const stalledPort = await startServer(() => undefined); + const stalled = await probeRemoteMcpServer("stalled", options(stalledPort).url, options(stalledPort)); + expect(stalled.findings).toEqual([ + expect.objectContaining({ id: "plugin.runtime.remote.transport.timeout", severity: "fail" }) + ]); + + const oversizedPort = await startServer((_request, response) => { + response.writeHead(200, { "content-type": "application/json" }); + response.end("x".repeat(1_024 * 1_024 + 1)); + }); + const oversized = await probeRemoteMcpServer("oversized", options(oversizedPort).url, options(oversizedPort)); + expect(oversized.findings).toEqual([ + expect.objectContaining({ id: "plugin.runtime.remote.transport.response_too_large", severity: "fail" }) + ]); + }); + + it("validates OAuth readiness after a 401 without sending an initialized notification", async () => { + const requests: Array<{ url: string; options: Record | undefined }> = []; + const request = async (url: string, requestOptions?: Record): Promise => { + requests.push({ url, options: requestOptions }); + if (url === "https://mcp.example/mcp") { + return { + statusCode: 401, + headers: { + "www-authenticate": 'Bearer resource_metadata="https://mcp.example/.well-known/oauth-protected-resource/mcp"' + }, + body: Buffer.alloc(0) + }; + } + if (url === "https://mcp.example/.well-known/oauth-protected-resource/mcp") { + return { + statusCode: 200, + headers: { "content-type": "application/json" }, + body: Buffer.from('{"resource":"https://mcp.example/mcp","authorization_servers":["https://auth.example"]}') + }; + } + return { + statusCode: 200, + headers: { "content-type": "application/json" }, + body: Buffer.from('{"issuer":"https://auth.example","authorization_endpoint":"https://auth.example/authorize","token_endpoint":"https://auth.example/token"}') + }; + }; + + const result = await probeRemoteMcpServer("remote", "https://mcp.example/mcp", { + allowNetwork: true, + request + }); + + expect(result.scorecard).toMatchObject({ authorization: "pass", initialize: "skipped", overall: "pass" }); + expect(result.findings).toEqual([]); + expect(requests.map((entry) => entry.url)).toEqual([ + "https://mcp.example/mcp", + "https://mcp.example/.well-known/oauth-protected-resource/mcp", + "https://auth.example/.well-known/oauth-authorization-server" + ]); + expect(requests).toHaveLength(3); + for (const requestEntry of requests) { + expect(JSON.stringify(requestEntry.options)).not.toMatch(/authorization|cookie|proxy-authorization/i); + } + }); + + it("fails authorization discovery after a 401 without initializing or notifying", async () => { + const requests: Array<{ url: string; options: Record | undefined }> = []; + const request = async (url: string, requestOptions?: Record): Promise => { + requests.push({ url, options: requestOptions }); + if (url === "https://mcp.example/mcp") { + return { + statusCode: 401, + headers: { + "www-authenticate": 'Bearer resource_metadata="https://mcp.example/.well-known/oauth-protected-resource/mcp"' + }, + body: Buffer.alloc(0) + }; + } + return { statusCode: 404, headers: { "content-type": "application/json" }, body: Buffer.alloc(0) }; + }; + + const result = await probeRemoteMcpServer("remote", "https://mcp.example/mcp", { + allowNetwork: true, + request + }); + + expect(result.scorecard).toMatchObject({ authorization: "fail", initialize: "skipped", overall: "fail" }); + expect(result.findings).toEqual([ + expect.objectContaining({ id: "plugin.runtime.remote.authorization.metadata.unavailable", severity: "fail" }) + ]); + expect(requests).toHaveLength(2); + expect(requests.some((entry) => JSON.stringify(entry.options).includes("notifications/initialized"))).toBe(false); + }); + + it("fails an unexpected HTTP status without exposing response details", async () => { + const port = await startServer((_request, response) => { + response.writeHead(500, { "content-type": "application/json" }); + response.end('{"error":"credential-secret-sentinel"}'); + }); + + const result = await probeRemoteMcpServer("remote", options(port).url, options(port)); + + expect(result.findings).toEqual([ + expect.objectContaining({ id: "plugin.runtime.remote.http_status.invalid", severity: "fail" }) + ]); + assertPrivate(result); + }); + + it.each([ + "https://credential-secret-sentinel@safe.example/mcp", + "https://safe.example/mcp?token=query-secret-sentinel" + ])("rejects unsafe URLs without retaining their sensitive component", async (url) => { + const result = await probeRemoteMcpServer("remote", url, { allowNetwork: true }); + + expect(result.scorecard.networkSafety).toBe("fail"); + assertPrivate(result); + }); + + it("rejects an invalid session header without retaining it", async () => { + const port = await startServer((_request, response) => { + response.writeHead(200, { + "content-type": "application/json", + "mcp-session-id": "invalid session-secret-sentinel" + }); + response.end(initializedResponse(1)); + }); + + const result = await probeRemoteMcpServer("remote", options(port).url, options(port)); + + expect(result.scorecard.session).toBe("present-invalid"); + expect(result.findings).toEqual([ + expect.objectContaining({ id: "plugin.runtime.remote.session.invalid", severity: "fail" }) + ]); + assertPrivate(result); + }); +}); diff --git a/tests/remote-network-policy.test.ts b/tests/remote-network-policy.test.ts new file mode 100644 index 0000000..f3292ea --- /dev/null +++ b/tests/remote-network-policy.test.ts @@ -0,0 +1,138 @@ +import { describe, expect, it } from "vitest"; + +import { + RemoteNetworkPolicyError, + resolveRemoteTarget, + type RemoteLookup +} from "../src/core/remote-network-policy.js"; + +function lookupFor(addresses: Array<{ address: string; family: 4 | 6 }>): RemoteLookup { + return async () => addresses; +} + +describe("resolveRemoteTarget", () => { + it.each([ + ["a non-HTTP protocol", new URL("file:///tmp/mcp")], + ["a URL without a hostname", new URL("file:///tmp/mcp")], + ["an IPv4 literal", new URL("http://127.0.0.1/mcp")], + ["an IPv6 literal", new URL("http://[::1]/mcp")] + ])("rejects %s before DNS lookup", async (_name, url) => { + await expect(resolveRemoteTarget(url)).rejects.toMatchObject({ + code: "REMOTE_TARGET_URL_INVALID", + message: "Remote target URL must use HTTP or HTTPS with a hostname, not an IP address literal." + }); + }); + + it("uses an all-address lookup and selects the first approved public IPv4 address", async () => { + let lookupOptions: { all?: boolean; verbatim?: boolean } | undefined; + const target = await resolveRemoteTarget(new URL("https://mcp.example/mcp"), { + lookup: async (_hostname, options) => { + lookupOptions = options; + return [ + { address: "8.8.8.8", family: 4 }, + { address: "1.1.1.1", family: 4 } + ]; + } + }); + + expect(lookupOptions).toMatchObject({ all: true, verbatim: true }); + expect(target).toEqual({ + hostname: "mcp.example", + address: "8.8.8.8", + family: 4, + local: false + }); + }); + + it("accepts public IPv6", async () => { + await expect( + resolveRemoteTarget(new URL("https://mcp.example/mcp"), { + lookup: lookupFor([{ address: "2606:4700:4700::1111", family: 6 }]) + }) + ).resolves.toMatchObject({ address: "2606:4700:4700::1111", family: 6, local: false }); + }); + + it("allows loopback only with explicit opt-in", async () => { + const options = { lookup: lookupFor([{ address: "127.0.0.1", family: 4 }]) }; + + await expect(resolveRemoteTarget(new URL("http://mcp.test/mcp"), options)).rejects.toMatchObject({ + code: "REMOTE_TARGET_FORBIDDEN", + message: "Remote target resolved to a forbidden address." + }); + await expect( + resolveRemoteTarget(new URL("http://mcp.test/mcp"), { ...options, allowLocalNetwork: true }) + ).resolves.toMatchObject({ address: "127.0.0.1", family: 4, local: true }); + }); + + it.each([ + ["IPv6 loopback", "::1", 6], + ["mapped IPv4 loopback", "::ffff:127.0.0.1", 6] + ])("allows %s only with explicit opt-in", async (_name, address, family) => { + const options = { lookup: lookupFor([{ address, family: family as 4 | 6 }]) }; + + await expect(resolveRemoteTarget(new URL("http://mcp.test/mcp"), options)).rejects.toMatchObject({ + code: "REMOTE_TARGET_FORBIDDEN" + }); + await expect( + resolveRemoteTarget(new URL("http://mcp.test/mcp"), { ...options, allowLocalNetwork: true }) + ).resolves.toMatchObject({ address, family, local: true }); + }); + + it.each([ + ["RFC1918", "10.0.0.1", 4], + ["link-local", "169.254.10.1", 4], + ["unspecified", "0.0.0.0", 4], + ["multicast", "239.1.2.3", 4], + ["reserved IPv4", "240.0.0.1", 4], + ["documentation IPv4", "198.51.100.10", 4], + ["metadata", "169.254.169.254", 4], + ["IPv6 unique local", "fc00::1", 6], + ["IPv6 link-local", "fe80::1", 6], + ["IPv6 multicast", "ff02::1", 6], + ["IPv6 unspecified", "::", 6], + ["IPv6 documentation", "2001:db8::1", 6], + ["mapped private IPv4", "::ffff:10.0.0.1", 6], + ["mapped public IPv4", "::ffff:8.8.8.8", 6], + ["IPv4-compatible IPv6", "::127.0.0.1", 6], + ["IPv6 well-known NAT64 prefix", "64:ff9b::a9fe:a9fe", 6], + ["IPv6 NAT64 local-use prefix", "64:ff9b:1::1", 6], + ["IPv6 discard-only prefix", "100:0:0:1::1", 6] + ])("rejects %s destinations", async (_name, address, family) => { + await expect( + resolveRemoteTarget(new URL("https://mcp.example/mcp"), { + lookup: lookupFor([{ address, family: family as 4 | 6 }]) + }) + ).rejects.toBeInstanceOf(RemoteNetworkPolicyError); + }); + + it("rejects a hostname when any DNS answer is forbidden", async () => { + await expect( + resolveRemoteTarget(new URL("https://mcp.example/mcp"), { + lookup: lookupFor([ + { address: "8.8.8.8", family: 4 }, + { address: "192.168.1.1", family: 4 } + ]) + }) + ).rejects.toMatchObject({ code: "REMOTE_TARGET_FORBIDDEN" }); + }); + + it("fails closed when DNS returns no answers", async () => { + await expect( + resolveRemoteTarget(new URL("https://mcp.example/mcp"), { lookup: lookupFor([]) }) + ).rejects.toMatchObject({ + code: "REMOTE_TARGET_EMPTY", + message: "Remote target DNS resolution returned no addresses." + }); + }); + + it("returns a deterministic error when DNS lookup fails", async () => { + await expect( + resolveRemoteTarget(new URL("https://mcp.example/mcp"), { + lookup: async () => { throw new Error("resolver-specific failure"); } + }) + ).rejects.toMatchObject({ + code: "REMOTE_TARGET_LOOKUP_FAILED", + message: "Remote target DNS resolution failed." + }); + }); +}); diff --git a/tests/remote-oauth-readiness.test.ts b/tests/remote-oauth-readiness.test.ts new file mode 100644 index 0000000..2397a94 --- /dev/null +++ b/tests/remote-oauth-readiness.test.ts @@ -0,0 +1,473 @@ +import { describe, expect, it, vi } from "vitest"; + +import { BoundedHttpError, type BoundedHttpResponse } from "../src/core/bounded-http-client.js"; +import { + checkRemoteOAuthReadiness, + type RemoteOAuthReadinessRequest +} from "../src/core/remote-oauth-readiness.js"; + +const resourceUrl = "https://mcp.example/v1/mcp"; +const resourceMetadataUrl = "https://mcp.example/.well-known/oauth-protected-resource/v1/mcp"; +const rootResourceMetadataUrl = "https://mcp.example/.well-known/oauth-protected-resource"; +const issuer = "https://auth.example/tenant"; +const authorizationMetadataUrl = "https://auth.example/.well-known/oauth-authorization-server/tenant"; +const oidcPathInsertionMetadataUrl = "https://auth.example/.well-known/openid-configuration/tenant"; +const oidcMetadataUrl = "https://auth.example/tenant/.well-known/openid-configuration"; + +function json(body: unknown, statusCode = 200): BoundedHttpResponse { + return { + statusCode, + headers: { "content-type": "application/json" }, + body: Buffer.from(JSON.stringify(body)) + }; +} + +function response(statusCode: number, contentType = "application/json", body = ""): BoundedHttpResponse { + return { statusCode, headers: { "content-type": contentType }, body: Buffer.from(body) }; +} + +function protectedMetadata(authorizationServers: unknown = [issuer]): BoundedHttpResponse { + return json({ resource: resourceUrl, authorization_servers: authorizationServers }); +} + +function authorizationMetadata(values: Record = {}): BoundedHttpResponse { + return json({ + issuer, + authorization_endpoint: "https://auth.example/authorize", + token_endpoint: "https://auth.example/token", + ...values + }); +} + +function requestFrom( + replies: Record +): { request: RemoteOAuthReadinessRequest; calls: Array<{ url: string; options: unknown }> } { + const calls: Array<{ url: string; options: unknown }> = []; + return { + calls, + request: async (url, options) => { + calls.push({ url, options }); + const reply = replies[url]; + if (reply === undefined) throw new Error("unexpected request"); + if (reply instanceof Error) throw reply; + return reply; + } + }; +} + +function assertPrivate(value: unknown): void { + const serialized = JSON.stringify(value); + for (const sentinel of [ + "challenge-secret-sentinel", + "scope-secret-sentinel", + "query-secret-sentinel", + "token-secret-sentinel", + "session-secret-sentinel" + ]) { + expect(serialized).not.toContain(sentinel); + } +} + +describe("checkRemoteOAuthReadiness", () => { + it("accepts quoted Bearer resource metadata within multiple challenges without sending credentials", async () => { + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata(), + [authorizationMetadataUrl]: authorizationMetadata() + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [ + 'Basic realm="challenge-secret-sentinel", bEaReR scope="scope-secret-sentinel", resource_metadata="https://mcp.example/.well-known/oauth-protected-resource/v1/mcp"' + ], { request }); + + expect(result).toEqual({ status: "pass", findings: [] }); + expect(calls.map((call) => call.url)).toEqual([resourceMetadataUrl, authorizationMetadataUrl]); + for (const call of calls) { + expect(call.options).toMatchObject({ method: "GET", headers: { Accept: "application/json" } }); + expect(JSON.stringify(call.options)).not.toMatch(/authorization|cookie|proxy-authorization/i); + } + assertPrivate(result); + }); + + it.each([ + ['a comma inside a Basic realm', 'Basic realm="x, Bearer resource_metadata=https://evil.example/"'], + ['an escaped quote inside a Basic realm', String.raw`Basic realm="x\", Bearer resource_metadata=https://evil.example/"`], + ['an escaped backslash inside a Basic realm', String.raw`Basic realm="x\\, Bearer resource_metadata=https://evil.example/"`] + ])("does not treat Bearer text in %s as a challenge", async (_name, header) => { + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata(), + [authorizationMetadataUrl]: authorizationMetadata() + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [header], { request }); + + expect(result).toEqual({ status: "pass", findings: [] }); + expect(calls.map((call) => call.url)).toEqual([resourceMetadataUrl, authorizationMetadataUrl]); + }); + + it("supports case-insensitive Bearer auth-params across comma-separated and separate header values", async () => { + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata(), + [authorizationMetadataUrl]: authorizationMetadata() + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [[ + `bEaReR scope="read,write", RESOURCE_METADATA="${resourceMetadataUrl}", error="invalid_token"`, + `Digest realm="other", BEARER error="invalid_token", resource_metadata="${resourceMetadataUrl}"` + ]], { request }); + + expect(result).toEqual({ status: "pass", findings: [] }); + expect(calls.map((call) => call.url)).toEqual([resourceMetadataUrl, authorizationMetadataUrl]); + }); + + it("fails closed without discovery when a Bearer challenge has an unterminated quoted auth-param", async () => { + let requested = false; + + const result = await checkRemoteOAuthReadiness(resourceUrl, [ + 'Bearer resource_metadata="https://metadata.example/oauth-protected-resource' + ], { request: async () => { + requested = true; + throw new Error("must not request"); + } }); + + expect(result.findings[0]).toMatchObject({ id: "plugin.runtime.remote.authorization.metadata.invalid" }); + expect(requested).toBe(false); + assertPrivate(result); + }); + + it("tries endpoint-specific resource metadata before the root fallback", async () => { + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: response(404), + [rootResourceMetadataUrl]: protectedMetadata(), + [authorizationMetadataUrl]: authorizationMetadata() + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result.status).toBe("pass"); + expect(calls.map((call) => call.url)).toEqual([ + resourceMetadataUrl, + rootResourceMetadataUrl, + authorizationMetadataUrl + ]); + }); + + it.each([ + ["malformed", response(200, "application/json", "{")], + ["resource-mismatched", json({ resource: "https://mcp.example/other", authorization_servers: [issuer] })] + ])("returns invalid when a root fallback is %s after an endpoint-specific 404", async (_name, rootReply) => { + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: response(404), + [rootResourceMetadataUrl]: rootReply + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result.findings[0]).toMatchObject({ id: "plugin.runtime.remote.authorization.metadata.invalid" }); + expect(calls.map((call) => call.url)).toEqual([resourceMetadataUrl, rootResourceMetadataUrl]); + }); + + it("refuses HTTP metadata discovery even when an injected request could reach localhost", async () => { + let requested = false; + + const result = await checkRemoteOAuthReadiness("http://localhost:8080/mcp", [], { + request: async () => { + requested = true; + throw new Error("must not request"); + } + }); + + expect(result.status).toBe("fail"); + expect(requested).toBe(false); + }); + + it("fails closed when protected-resource metadata omits authorization servers", async () => { + const { request } = requestFrom({ [resourceMetadataUrl]: protectedMetadata([]) }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result).toEqual(expect.objectContaining({ + status: "fail", + findings: [expect.objectContaining({ id: "plugin.runtime.remote.authorization.metadata.invalid", severity: "fail" })] + })); + }); + + it("uses a later advertised issuer when earlier issuer metadata is invalid", async () => { + const secondIssuer = "https://auth-two.example/issuer"; + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata([issuer, secondIssuer]), + [authorizationMetadataUrl]: response(404), + [oidcPathInsertionMetadataUrl]: response(404), + [oidcMetadataUrl]: response(404), + "https://auth-two.example/.well-known/oauth-authorization-server/issuer": json({ + issuer: secondIssuer, + authorization_endpoint: "https://auth-two.example/authorize", + token_endpoint: "https://auth-two.example/token" + }) + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result).toEqual({ status: "pass", findings: [] }); + expect(calls.map((call) => call.url)).toEqual([ + resourceMetadataUrl, + authorizationMetadataUrl, + oidcPathInsertionMetadataUrl, + oidcMetadataUrl, + "https://auth-two.example/.well-known/oauth-authorization-server/issuer" + ]); + }); + + it("follows RFC 8414 then both OIDC discovery locations", async () => { + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata(), + [authorizationMetadataUrl]: response(404), + [oidcPathInsertionMetadataUrl]: authorizationMetadata(), + [oidcMetadataUrl]: authorizationMetadata() + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result.status).toBe("pass"); + expect(calls.map((call) => call.url)).toEqual([ + resourceMetadataUrl, + authorizationMetadataUrl, + oidcPathInsertionMetadataUrl + ]); + }); + + it("tries appended OIDC discovery only after the path-insertion location is not found", async () => { + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata(), + [authorizationMetadataUrl]: response(404), + [oidcPathInsertionMetadataUrl]: response(404), + [oidcMetadataUrl]: authorizationMetadata() + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result.status).toBe("pass"); + expect(calls.map((call) => call.url)).toEqual([ + resourceMetadataUrl, + authorizationMetadataUrl, + oidcPathInsertionMetadataUrl, + oidcMetadataUrl + ]); + }); + + it("deduplicates equivalent OIDC discovery locations for a root issuer", async () => { + const rootIssuer = "https://auth.example"; + const rootAuthorizationMetadataUrl = "https://auth.example/.well-known/oauth-authorization-server"; + const rootOidcMetadataUrl = "https://auth.example/.well-known/openid-configuration"; + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata([rootIssuer]), + [rootAuthorizationMetadataUrl]: response(404), + [rootOidcMetadataUrl]: json({ + issuer: rootIssuer, + authorization_endpoint: "https://auth.example/authorize", + token_endpoint: "https://auth.example/token" + }) + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result.status).toBe("pass"); + expect(calls.map((call) => call.url)).toEqual([ + resourceMetadataUrl, + rootAuthorizationMetadataUrl, + rootOidcMetadataUrl + ]); + }); + + it.each([ + ["a resource mismatch", protectedMetadata(), authorizationMetadata(), { resource: "https://mcp.example/other", authorization_servers: [issuer] }], + ["an issuer mismatch", protectedMetadata(), authorizationMetadata({ issuer: "https://auth.example/other" }), undefined], + ["an unsafe authorization endpoint", protectedMetadata(), authorizationMetadata({ authorization_endpoint: "http://auth.example/authorize" }), undefined], + ["an unsafe token endpoint", protectedMetadata(), authorizationMetadata({ token_endpoint: "https://127.0.0.1/token" }), undefined] + ])("fails closed for %s", async (_name, protectedResponse, serverResponse, replacementProtectedMetadata) => { + const protectedReply = replacementProtectedMetadata === undefined + ? protectedResponse + : json(replacementProtectedMetadata); + const { request } = requestFrom({ + [resourceMetadataUrl]: protectedReply, + [authorizationMetadataUrl]: serverResponse, + [oidcMetadataUrl]: serverResponse + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result.status).toBe("fail"); + expect(result.findings[0]).toEqual(expect.objectContaining({ + id: "plugin.runtime.remote.authorization.metadata.invalid", + severity: "fail" + })); + }); + + it.each([ + ["a malformed JSON body", response(200, "application/json", "{"), "plugin.runtime.remote.authorization.metadata.invalid"], + ["a non-JSON content type", response(200, "text/plain", "{}"), "plugin.runtime.remote.authorization.metadata.unavailable"], + ["an oversized response", new BoundedHttpError("REMOTE_HTTP_RESPONSE_TOO_LARGE", "oversized token-secret-sentinel"), "plugin.runtime.remote.authorization.metadata.unavailable"], + ["a redirect", new BoundedHttpError("REMOTE_HTTP_REDIRECT", "redirect query-secret-sentinel"), "plugin.runtime.remote.authorization.metadata.unavailable"] + ])("returns a stable private failure for %s", async (_name, metadataReply, findingId) => { + const { request } = requestFrom({ [resourceMetadataUrl]: metadataReply }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result).toEqual(expect.objectContaining({ + status: "fail", + findings: [expect.objectContaining({ id: findingId, severity: "fail" })] + })); + assertPrivate(result); + }); + + it("rejects unsafe explicit metadata and advertised issuers without issuing those requests", async () => { + const explicit = await checkRemoteOAuthReadiness(resourceUrl, [ + 'Bearer resource_metadata="http://metadata.example/.well-known/oauth-protected-resource"' + ], { request: async () => { throw new Error("must not request"); } }); + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata([issuer, "https://auth.example/?query-secret-sentinel"]) + }); + const advertised = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(explicit.status).toBe("fail"); + expect(advertised.status).toBe("fail"); + expect(calls).toHaveLength(1); + assertPrivate({ explicit, advertised }); + }); + + it.each([ + ["a not-found document", response(404)], + ["an unavailable document", response(503)], + ["a malformed document", response(200, "application/json", "{")], + ["a resource-mismatched document", json({ resource: "https://mcp.example/other", authorization_servers: [issuer] })] + ])("deduplicates explicit metadata candidates and skips %s", async (_name, firstReply) => { + const alternateMetadataUrl = "https://metadata.example/oauth-protected-resource"; + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: firstReply, + [alternateMetadataUrl]: protectedMetadata(), + [authorizationMetadataUrl]: authorizationMetadata() + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [ + `Bearer resource_metadata="${resourceMetadataUrl}", Bearer resource_metadata="${resourceMetadataUrl}", Bearer resource_metadata="${alternateMetadataUrl}"` + ], { request }); + + expect(result).toEqual({ status: "pass", findings: [] }); + expect(calls.map((call) => call.url)).toEqual([ + resourceMetadataUrl, + alternateMetadataUrl, + authorizationMetadataUrl + ]); + }); + + it("rejects more than four unique explicit metadata candidates before requesting them", async () => { + const candidates = Array.from({ length: 5 }, (_, index) => `https://metadata-${index}.example/oauth-protected-resource`); + let requested = false; + + const result = await checkRemoteOAuthReadiness(resourceUrl, candidates.map((candidate) => `Bearer resource_metadata="${candidate}"`), { request: async () => { + requested = true; + throw new Error("must not request"); + } }); + + expect(result.findings[0]).toMatchObject({ id: "plugin.runtime.remote.authorization.metadata.invalid" }); + expect(requested).toBe(false); + }); + + it("rejects more than four unique advertised authorization servers before requesting them", async () => { + const authorizationServers = Array.from({ length: 5 }, (_, index) => `https://auth-${index}.example`); + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata(authorizationServers) + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result.findings[0]).toMatchObject({ id: "plugin.runtime.remote.authorization.metadata.invalid" }); + expect(calls.map((call) => call.url)).toEqual([resourceMetadataUrl]); + }); + + it("deduplicates advertised authorization servers before probing them", async () => { + const secondIssuer = "https://auth-two.example/issuer"; + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata([issuer, issuer, secondIssuer]), + [authorizationMetadataUrl]: response(404), + [oidcPathInsertionMetadataUrl]: response(404), + [oidcMetadataUrl]: response(404), + "https://auth-two.example/.well-known/oauth-authorization-server/issuer": json({ + issuer: secondIssuer, + authorization_endpoint: "https://auth-two.example/authorize", + token_endpoint: "https://auth-two.example/token" + }) + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [], { request }); + + expect(result).toEqual({ status: "pass", findings: [] }); + expect(calls.map((call) => call.url)).toEqual([ + resourceMetadataUrl, + authorizationMetadataUrl, + oidcPathInsertionMetadataUrl, + oidcMetadataUrl, + "https://auth-two.example/.well-known/oauth-authorization-server/issuer" + ]); + }); + + it.each([ + ["an explicit metadata URL", 'Bearer resource_metadata="https://[::1]/oauth-protected-resource"', undefined], + ["an advertised authorization server issuer", undefined, { authorization_servers: ["https://[::1]"] }], + ["an authorization endpoint", undefined, { authorization_endpoint: "https://[::1]/authorize" }], + ["a token endpoint", undefined, { token_endpoint: "https://[::1]/token" }] + ])("rejects bracketed IPv6 literals in %s without an unsafe request", async (_name, header, metadataOverrides) => { + const { request, calls } = requestFrom({ + [resourceMetadataUrl]: metadataOverrides?.authorization_servers === undefined + ? protectedMetadata() + : json({ resource: resourceUrl, ...metadataOverrides }), + [authorizationMetadataUrl]: authorizationMetadata(metadataOverrides) + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, header === undefined ? [] : [header], { request }); + + expect(result.status).toBe("fail"); + expect(calls.some((call) => call.url.includes("[::1]"))).toBe(false); + if (header !== undefined) { + expect(calls).toHaveLength(0); + } + }); + + it("uses one total deadline across delayed authorization discovery requests", async () => { + vi.useFakeTimers(); + try { + const calls: Array<{ url: string; timeoutMs: number | undefined; at: number }> = []; + const request: RemoteOAuthReadinessRequest = (url, requestOptions) => new Promise((resolve) => { + calls.push({ url, timeoutMs: requestOptions?.timeoutMs, at: Date.now() }); + setTimeout(() => resolve(url === resourceMetadataUrl ? protectedMetadata() : response(404)), 40); + }); + + const resultPromise = checkRemoteOAuthReadiness(resourceUrl, [], { + request, + requestOptions: { timeoutMs: 60 } + }); + + await vi.advanceTimersByTimeAsync(60); + const result = await resultPromise; + + expect(result.findings[0]).toMatchObject({ id: "plugin.runtime.remote.authorization.metadata.unavailable" }); + expect(calls.map((call) => call.url)).toEqual([resourceMetadataUrl, authorizationMetadataUrl]); + expect(calls.map((call) => call.timeoutMs)).toEqual([60, 20]); + expect(calls.map((call) => call.at - calls[0]!.at)).toEqual([0, 40]); + } finally { + vi.useRealTimers(); + } + }); + + it("accepts a standalone WWW-Authenticate header value", async () => { + const { request } = requestFrom({ + [resourceMetadataUrl]: protectedMetadata(), + [authorizationMetadataUrl]: authorizationMetadata() + }); + + const result = await checkRemoteOAuthReadiness(resourceUrl, [ + `Bearer resource_metadata="${resourceMetadataUrl}"` + ], { request }); + + expect(result).toEqual({ status: "pass", findings: [] }); + }); +}); diff --git a/tests/remote-url-policy.test.ts b/tests/remote-url-policy.test.ts new file mode 100644 index 0000000..479b932 --- /dev/null +++ b/tests/remote-url-policy.test.ts @@ -0,0 +1,113 @@ +import { mkdir, mkdtemp, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +import { inspectRemoteMcpUrl } from "../src/core/remote-url-policy.js"; +import { validatePlugin } from "../src/core/validate-plugin.js"; + +async function createPluginWithMcp(mcpConfig: unknown): Promise { + const targetPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-remote-url-")); + + await mkdir(path.join(targetPath, ".codex-plugin"), { recursive: true }); + await mkdir(path.join(targetPath, "skills", "hello"), { recursive: true }); + await writeFile( + path.join(targetPath, ".codex-plugin", "plugin.json"), + JSON.stringify({ + name: "remote-url-fixture", + version: "1.0.0", + description: "Fixture plugin for remote MCP URL validation.", + skills: "./skills", + mcpServers: "./.mcp.json" + }), + "utf8" + ); + await writeFile( + path.join(targetPath, "skills", "hello", "SKILL.md"), + "---\nname: hello\ndescription: Fixture skill.\n---\n", + "utf8" + ); + await writeFile(path.join(targetPath, ".mcp.json"), JSON.stringify(mcpConfig), "utf8"); + + return targetPath; +} + +describe("inspectRemoteMcpUrl", () => { + it("normalizes accepted HTTPS and localhost development URLs", () => { + expect(inspectRemoteMcpUrl("HTTPS://Example.COM:443/mcp")).toMatchObject({ + sanitizedUrl: "https://example.com/mcp", + isLoopbackHost: false, + issues: [] + }); + expect(inspectRemoteMcpUrl("http://LOCALHOST:3000/mcp")).toMatchObject({ + sanitizedUrl: "http://localhost:3000/mcp", + isLoopbackHost: true, + issues: [] + }); + }); + + it.each([ + ["not-a-url", ["invalid"]], + ["ftp://example.com/mcp", ["unsupported_scheme"]], + ["https://user:secret@example.com/mcp", ["credentials"]], + ["https://example.com/mcp?token=secret", ["query"]], + ["https://example.com/mcp#secret", ["fragment"]], + ["https://example.com/mcp?", ["query"]], + ["https://example.com/mcp#", ["fragment"]], + ["https://example.com/mcp?#", ["query", "fragment"]], + ["https://127.0.0.1/mcp", ["ip_literal"]], + ["https://[::1]/mcp", ["ip_literal"]], + ["http://example.com/mcp", ["insecure_non_loopback"]] + ])("classifies %s without retaining unsafe URL components", (rawUrl, issues) => { + const inspection = inspectRemoteMcpUrl(rawUrl); + + expect(inspection.issues).toEqual(issues); + expect(inspection.sanitizedUrl === null || !inspection.sanitizedUrl.includes("secret")).toBe(true); + }); + + it("reports empty query and fragment delimiters to plugin validation without exposing the URL", async () => { + const rawUrl = "https://example.com/mcp?#"; + const targetPath = await createPluginWithMcp({ + mcpServers: { + remote: { url: rawUrl } + } + }); + + const result = await validatePlugin(targetPath); + const serialized = JSON.stringify(result.findings); + + expect(result.status).toBe("fail"); + expect(result.findings.map((finding) => finding.id)).toEqual( + expect.arrayContaining([ + "plugin.security.remote_mcp_url.query", + "plugin.security.remote_mcp_url.fragment" + ]) + ); + expect(serialized).not.toContain(rawUrl); + }); +}); + +describe("plugin remote MCP validation", () => { + it("fails conflicting and credential-bearing remote transports without leaking the raw URL", async () => { + const rawUrl = "https://user:secret@example.com/mcp?token=secret"; + const targetPath = await createPluginWithMcp({ + mcpServers: { + remote: { command: "node", url: rawUrl } + } + }); + + const result = await validatePlugin(targetPath); + const serialized = JSON.stringify(result.findings); + + expect(result.status).toBe("fail"); + expect(result.findings.map((finding) => finding.id)).toEqual( + expect.arrayContaining([ + "plugin.mcp.server.transport.conflict", + "plugin.security.remote_mcp_url.credentials", + "plugin.security.remote_mcp_url.query" + ]) + ); + expect(serialized).not.toContain(rawUrl); + expect(serialized).not.toContain("secret"); + }); +}); diff --git a/tests/render-text-report.test.ts b/tests/render-text-report.test.ts index 70dc1e5..b191bcb 100644 --- a/tests/render-text-report.test.ts +++ b/tests/render-text-report.test.ts @@ -76,6 +76,23 @@ describe("renderTextReport", () => { expect(output).toContain("Failures"); }); + it("renders the remote MCP scorecard with enum-only session state", () => { + const output = renderTextReport({ + targetPath: "example", + status: "pass", + exitCode: 0, + findings: [], + runtimeScorecard: { + ...runtimeScorecard, + remote: { transport: "pass", networkSafety: "pass", initialize: "pass", contentType: "pass", session: "present-valid", protocolHeaders: "pass", authorization: "skipped", overall: "pass" } + } + }); + + expect(output).toContain("Remote MCP Scorecard"); + expect(output).toContain("network safety: pass"); + expect(output).toContain("session: present-valid"); + }); + it("renders a rich unicode summary for warn results", async () => { const result = await runCheck( path.resolve("tests/fixtures/heuristic-long-plugin-description") diff --git a/tests/rule-catalog.test.ts b/tests/rule-catalog.test.ts index 88a849f..575bdbe 100644 --- a/tests/rule-catalog.test.ts +++ b/tests/rule-catalog.test.ts @@ -77,6 +77,33 @@ const mcpConformanceRules = [ } ] as const; +const remoteMcpRules = [ + { id: "mcp.server.transport.conflict", category: "mcp", defaultSeverity: "fail" }, + { id: "plugin.mcp.server.transport.conflict", category: "mcp", defaultSeverity: "fail" }, + { id: "plugin.security.insecure_http_url", category: "security", defaultSeverity: "fail" }, + { id: "plugin.security.remote_mcp_url.invalid", category: "security", defaultSeverity: "fail" }, + { id: "plugin.security.remote_mcp_url.unsupported_scheme", category: "security", defaultSeverity: "fail" }, + { id: "plugin.security.remote_mcp_url.credentials", category: "security", defaultSeverity: "fail" }, + { id: "plugin.security.remote_mcp_url.query", category: "security", defaultSeverity: "fail" }, + { id: "plugin.security.remote_mcp_url.fragment", category: "security", defaultSeverity: "fail" }, + { id: "plugin.security.remote_mcp_url.ip_literal", category: "security", defaultSeverity: "fail" } +] as const; + +const remoteRuntimeRules = [ + "plugin.runtime.remote.network_not_approved", + "plugin.runtime.remote.url.invalid", + "plugin.runtime.remote.transport.timeout", + "plugin.runtime.remote.transport.response_too_large", + "plugin.runtime.remote.transport.failed", + "plugin.runtime.remote.http_status.invalid", + "plugin.runtime.remote.content_type.invalid", + "plugin.runtime.remote.session.invalid", + "plugin.runtime.remote.initialize.invalid", + "plugin.runtime.remote.initialized.failed", + "plugin.runtime.remote.authorization.metadata.invalid", + "plugin.runtime.remote.authorization.metadata.unavailable" +] as const; + describe("MCP 2025-11 conformance rule catalog", () => { it("resolves every evaluator finding with its public remediation contract", () => { expect(ruleCatalog.filter((rule) => rule.id.startsWith("mcp.conformance."))).toEqual( @@ -87,4 +114,16 @@ describe("MCP 2025-11 conformance rule catalog", () => { expect(findRuleDefinition(expectedRule.id)).toEqual(expectedRule); } }); + + it("resolves every remote MCP transport finding with a public remediation contract", () => { + for (const expectedRule of remoteMcpRules) { + expect(findRuleDefinition(expectedRule.id)).toMatchObject(expectedRule); + } + }); + + it("resolves every remote runtime finding with a fail remediation contract", () => { + for (const id of remoteRuntimeRules) { + expect(findRuleDefinition(id)).toMatchObject({ id, category: "runtime", defaultSeverity: "fail" }); + } + }); }); diff --git a/tests/runtime-plan-command.test.ts b/tests/runtime-plan-command.test.ts index 81f29aa..9f93cf8 100644 --- a/tests/runtime-plan-command.test.ts +++ b/tests/runtime-plan-command.test.ts @@ -24,6 +24,86 @@ function createIo() { } describe("doctor runtime-plan command", () => { + it("redacts remote URLs and records the remote approval boundary", async () => { + const targetPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-runtime-plan-remote-")); + const rawUrl = "https://user:credential-secret@example.com/mcp?query-secret=1#fragment-secret"; + + await (await import("node:fs/promises")).mkdir(path.join(targetPath, ".codex-plugin")); + await (await import("node:fs/promises")).writeFile( + path.join(targetPath, ".codex-plugin", "plugin.json"), + JSON.stringify({ name: "remote-plan", version: "1.0.0", description: "Remote plan test.", mcpServers: ".mcp.json" }), + "utf8" + ); + await (await import("node:fs/promises")).writeFile( + path.join(targetPath, ".mcp.json"), + JSON.stringify({ mcpServers: { remote: { url: rawUrl } } }), + "utf8" + ); + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["doctor", "runtime-plan", targetPath, "--json"], io); + const serialized = stdout.join(""); + const output = JSON.parse(serialized); + const markdown = createIo(); + const policy = createIo(); + + await runCli(["doctor", "runtime-plan", targetPath, "--markdown"], markdown.io); + await runCli(["doctor", "runtime-policy", targetPath, "--json"], policy.io); + + expect(exitCode).toBe(1); + expect(stderr).toEqual([]); + expect(serialized).not.toContain("credential-secret"); + expect(serialized).not.toContain("query-secret"); + expect(serialized).not.toContain("fragment-secret"); + for (const privateValue of ["credential-secret", "query-secret", "fragment-secret"]) { + expect(markdown.stdout.join("")).not.toContain(privateValue); + expect(policy.stdout.join("")).not.toContain(privateValue); + } + expect(output.servers).toEqual(expect.arrayContaining([ + expect.objectContaining({ + name: "remote", + url: "https://example.com/mcp", + networkClass: "public_https", + probeMethods: [ + "POST initialize", + "POST notifications/initialized", + "GET OAuth protected-resource metadata (401 only)", + "GET OAuth authorization-server metadata (401 only)" + ], + approvalRequirements: expect.arrayContaining(["--runtime", "--allow-network"]) + }) + ])); + }); + + it("classifies HTTPS localhost as loopback and documents DNS loopback approval", async () => { + const targetPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-runtime-plan-loopback-")); + + await (await import("node:fs/promises")).mkdir(path.join(targetPath, ".codex-plugin")); + await (await import("node:fs/promises")).writeFile( + path.join(targetPath, ".codex-plugin", "plugin.json"), + JSON.stringify({ name: "loopback-plan", version: "1.0.0", description: "Loopback plan test.", mcpServers: ".mcp.json" }), + "utf8" + ); + await (await import("node:fs/promises")).writeFile( + path.join(targetPath, ".mcp.json"), + JSON.stringify({ mcpServers: { local: { url: "https://localhost:3443/mcp" } } }), + "utf8" + ); + const json = createIo(); + const markdown = createIo(); + + await runCli(["doctor", "runtime-plan", targetPath, "--json"], json.io); + await runCli(["doctor", "runtime-plan", targetPath, "--markdown"], markdown.io); + + expect(JSON.parse(json.stdout.join("")).servers).toEqual(expect.arrayContaining([ + expect.objectContaining({ + networkClass: "loopback_https", + approvalRequirements: ["--runtime", "--allow-network", "--allow-local-network"] + }) + ])); + expect(markdown.stdout.join("")).toContain("DNS resolves to loopback"); + }); + it("renders a non-executing runtime plan as JSON", async () => { const { io, stdout, stderr } = createIo(); diff --git a/tests/runtime-policy-command.test.ts b/tests/runtime-policy-command.test.ts index c4d5822..91bc268 100644 --- a/tests/runtime-policy-command.test.ts +++ b/tests/runtime-policy-command.test.ts @@ -24,6 +24,30 @@ function createIo() { } describe("doctor runtime-policy command", () => { + it("does not allow a remote-only plan without explicit network approval", async () => { + const targetPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-runtime-policy-remote-")); + await (await import("node:fs/promises")).mkdir(path.join(targetPath, ".codex-plugin")); + await (await import("node:fs/promises")).writeFile( + path.join(targetPath, ".codex-plugin", "plugin.json"), + JSON.stringify({ name: "remote-policy", version: "1.0.0", description: "Remote policy test.", mcpServers: ".mcp.json" }), + "utf8" + ); + await (await import("node:fs/promises")).writeFile( + path.join(targetPath, ".mcp.json"), + JSON.stringify({ mcpServers: { remote: { url: "https://example.com/mcp" } } }), + "utf8" + ); + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["doctor", "runtime-policy", targetPath, "--json"], io); + const output = JSON.parse(stdout.join("")); + + expect(exitCode).toBe(0); + expect(stderr).toEqual([]); + expect(output.recommendation.decision).toBe("review"); + expect(output.recommendation.actions.join("\n")).toContain("--allow-network"); + }); + it("recommends review for a clean local stdio runtime server", async () => { const { io, stdout, stderr } = createIo(); diff --git a/tests/runtime-protocol.test.ts b/tests/runtime-protocol.test.ts index d29f8b1..01c0c26 100644 --- a/tests/runtime-protocol.test.ts +++ b/tests/runtime-protocol.test.ts @@ -111,6 +111,43 @@ describe("runtime protocol probing", () => { } }); + it("reports URL servers as unapproved without opening a remote connection by default", async () => { + const packageRoot = await mkdtemp( + path.join(os.tmpdir(), "codex-plugin-doctor-runtime-remote-") + ); + + try { + const manifestPath = path.join(packageRoot, ".codex-plugin", "plugin.json"); + await mkdir(path.dirname(manifestPath), { recursive: true }); + await writeFile(manifestPath, JSON.stringify({ + name: "runtime-remote", + version: "1.0.0", + description: "Remote runtime validation fixture.", + mcpServers: "./.mcp.json" + })); + await writeFile(path.join(packageRoot, ".mcp.json"), JSON.stringify({ + mcpServers: { remoteServer: { url: "https://mcp.example/mcp" } } + })); + + const result = await probeRuntime({ + rootPath: packageRoot, + manifestPath, + manifest: { mcpServers: "./.mcp.json" } + }); + + expect(result.findings).toEqual([ + expect.objectContaining({ id: "plugin.runtime.remote.network_not_approved", severity: "fail" }) + ]); + expect(result.scorecard.remote).toEqual(expect.objectContaining({ + networkSafety: "fail", + overall: "fail" + })); + expect(result.scorecard.initialize).toBe("skipped"); + } finally { + await rm(packageRoot, { recursive: true, force: true }); + } + }); + it.each(["missing-cwd", "not-a-directory"])( "rejects a %s runtime cwd before spawn", async (cwd) => { diff --git a/tests/security-command.test.ts b/tests/security-command.test.ts index d43e748..90d86e0 100644 --- a/tests/security-command.test.ts +++ b/tests/security-command.test.ts @@ -4,6 +4,11 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import { runCli } from "../src/run-cli.js"; +import { + auditMcpServerConfig, + buildSecurityAuditFromFindings, + renderSecurityAuditJson +} from "../src/security/security-audit.js"; function createIo() { const stdout: string[] = []; @@ -23,7 +28,10 @@ function createIo() { }; } -async function createPluginWithMcp(mcpConfig: unknown): Promise { +async function createPluginWithMcp( + mcpConfig: unknown, + mcpConfigPath = ".mcp.json" +): Promise { const targetPath = await mkdtemp(path.join(os.tmpdir(), "codex-plugin-doctor-security-")); await mkdir(path.join(targetPath, ".codex-plugin"), { recursive: true }); @@ -36,7 +44,7 @@ async function createPluginWithMcp(mcpConfig: unknown): Promise { version: "1.0.0", description: "Fixture package for security command tests.", skills: "./skills", - mcpServers: "./.mcp.json" + mcpServers: `./${mcpConfigPath}` }, null, 2 @@ -48,11 +56,9 @@ async function createPluginWithMcp(mcpConfig: unknown): Promise { "---\nname: hello\ndescription: Minimal fixture skill.\n---\n", "utf8" ); - await writeFile( - path.join(targetPath, ".mcp.json"), - JSON.stringify(mcpConfig, null, 2), - "utf8" - ); + const mcpConfigFilePath = path.join(targetPath, mcpConfigPath); + await mkdir(path.dirname(mcpConfigFilePath), { recursive: true }); + await writeFile(mcpConfigFilePath, JSON.stringify(mcpConfig, null, 2), "utf8"); return targetPath; } @@ -138,6 +144,100 @@ describe("security command", () => { expect(output.findings).toEqual([]); }); + it("fails query-bearing public HTTP without leaking URL secrets and permits localhost HTTP", async () => { + const publicMcpConfig = { + mcpServers: { + remote: { url: "http://example.com/mcp?token=secret" } + } + }; + const publicTargetPath = await createPluginWithMcp(publicMcpConfig, "config/remote.json"); + const localTargetPath = await createPluginWithMcp({ + mcpServers: { + local: { url: "http://LOCALHOST:3000/mcp" } + } + }); + const publicIo = createIo(); + const localIo = createIo(); + + const publicExitCode = await runCli(["security", publicTargetPath, "--json"], publicIo.io); + const localExitCode = await runCli(["security", localTargetPath, "--json"], localIo.io); + const publicSerialized = publicIo.stdout.join(""); + const publicOutput = JSON.parse(publicSerialized); + const rawAuditOutput = JSON.parse(renderSecurityAuditJson(buildSecurityAuditFromFindings( + publicTargetPath, + auditMcpServerConfig(publicTargetPath, publicMcpConfig, { + configPath: path.join(publicTargetPath, "config", "remote.json") + }) + ))); + const localOutput = JSON.parse(localIo.stdout.join("")); + + expect(publicExitCode).toBe(1); + expect(publicIo.stderr).toEqual([]); + expect(rawAuditOutput.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "plugin.security.insecure_http_url", + severity: "fail" + }) + ]) + ); + expect(publicOutput.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "plugin.security.insecure_http_url", + severity: "fail", + evidence: expect.objectContaining({ url: "http://example.com/mcp" }) + }), + expect.objectContaining({ + id: "plugin.security.remote_mcp_url.query", + severity: "fail" + }) + ]) + ); + expect(publicSerialized).not.toContain("token=secret"); + expect(publicSerialized).not.toContain("secret"); + expect(localExitCode).toBe(0); + expect(localIo.stderr).toEqual([]); + expect(localOutput.findings).toEqual([]); + }); + + it("preserves the all-interfaces finding alongside the IP-literal policy", async () => { + const targetPath = await createPluginWithMcp({ + mcpServers: { + remote: { url: "http://mcp-user:mcp-password@0.0.0.0:3000/mcp?token=secret" } + } + }); + const { io, stdout, stderr } = createIo(); + + const exitCode = await runCli(["security", targetPath, "--json"], io); + const serialized = stdout.join(""); + const output = JSON.parse(serialized); + + expect(exitCode).toBe(1); + expect(stderr).toEqual([]); + expect(output.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "plugin.security.mcp_binds_all_interfaces", + severity: "warn", + message: "The MCP server `remote` URL binds to `0.0.0.0`.", + impact: "Servers that listen on all interfaces can accept connections from external hosts, which is rarely intended for local MCP development.", + suggestedFix: "Use `127.0.0.1` or `localhost` instead of `0.0.0.0` unless external access is explicitly required.", + evidence: expect.objectContaining({ url: "http://0.0.0.0:3000/mcp" }) + }), + expect.objectContaining({ + id: "plugin.security.remote_mcp_url.ip_literal", + severity: "fail", + evidence: expect.objectContaining({ url: "http://0.0.0.0:3000/mcp" }) + }) + ]) + ); + expect(serialized).not.toContain("token=secret"); + expect(serialized).not.toContain("secret"); + expect(serialized).not.toContain("mcp-user"); + expect(serialized).not.toContain("mcp-password"); + }); + it("renders machine-readable security audit JSON", async () => { const targetPath = await createPluginWithMcp({ mcpServers: { @@ -249,6 +349,42 @@ describe("security command", () => { ); }); + it("scans nested non-active .mcp.json files for external URL references", async () => { + const targetPath = await createPluginWithMcp( + { + mcpServers: { + safe: { + command: "node", + args: ["server.js"] + } + } + }, + "config/active.json" + ); + const nestedConfigPath = path.join(targetPath, "skills", "hello", "references", ".mcp.json"); + + await mkdir(path.dirname(nestedConfigPath), { recursive: true }); + await writeFile(nestedConfigPath, JSON.stringify({ documentation: "https://example.com/reference" }), "utf8"); + + const { io, stdout, stderr } = createIo(); + const exitCode = await runCli(["security", targetPath, "--json"], io); + const output = JSON.parse(stdout.join("")); + + expect(exitCode).toBe(0); + expect(stderr).toEqual([]); + expect(output.findings).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "plugin.skill.external_http_reference", + evidence: { + filePath: "skills/hello/references/.mcp.json", + url: "https://example.com/reference" + } + }) + ]) + ); + }); + it("passes a valid MCP plugin with a perfect scorecard", async () => { const { io, stdout, stderr } = createIo();