From aa6c1d49fd8db201cb5619c90bee2899452dfed7 Mon Sep 17 00:00:00 2001 From: Dery Ferdika <114888155+DeryFerd@users.noreply.github.com> Date: Mon, 26 Oct 2026 21:55:10 +0700 Subject: [PATCH] fix(mcp)!: require allowed hosts for non-loopback HTTP binds (#7172) Require operator-declared hostnames before serving MCP HTTP outside loopback, preserving Host/Origin gates. Parse hostname identities once, share typed startup recovery across CLI/MCP, and announce an admitted client URL. Generated AgentEnv bundles declare their client/service names and validate DNS/HTTP name identity before export. Validated through real packaged HTTP requests, direct SDK connection, compiled export refusals, 184 focused tests, static and documentation checks on the refreshed main integration. Final CI and review evidence is recorded in the PR body. BREAKING CHANGE: non-loopback deployments require MCP_ALLOWED_HOSTS or rea mcp --http --allowed-hosts; bundle names must be unchanged HTTP hostnames within the 63-octet DNS label bound. Co-authored-by: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> --- docs/agent-environments.md | 15 ++- docs/cli.md | 27 ++++- docs/mcp-contracts.md | 7 +- .../environment-export/spec.md | 8 +- .../environment-serve/spec.md | 6 +- .../environment-serve/verification.md | 6 +- .../agent-environments/http-transport/spec.md | 7 +- .../http-transport/verification.md | 6 +- .../environment/environmentBuild.ts | 3 +- .../environment/environmentBundle.test.ts | 34 ++++++ .../environment/environmentBundle.ts | 55 +++++++-- src/cli/environmentCommands.ts | 47 +++----- src/config/mcpHttpEndpoint.ts | 48 +++++++- src/domain/httpHostname.ts | 37 ++++++ src/main.ts | 3 + src/main/mcpHttpArguments.ts | 32 ++++- src/main/transport.ts | 5 +- src/server/environment/serveEnvironment.ts | 6 + src/server/http/serveStreamableHttp.test.ts | 79 +++++++++++- src/server/http/serveStreamableHttp.ts | 56 +++++---- src/server/http/streamableHttpListenError.ts | 47 +++++++- .../boundary/cli/environmentExportCli.test.ts | 33 +++++ .../cli/environmentServeRemediation.test.ts | 20 ++++ tests/boundary/mcp/mainErrors.test.ts | 25 +++- tests/boundary/mcp/mcpHttpOptions.test.ts | 113 +++++++++++++++++- 25 files changed, 634 insertions(+), 91 deletions(-) create mode 100644 src/domain/httpHostname.ts diff --git a/docs/agent-environments.md b/docs/agent-environments.md index 2912c2c257..910063d985 100644 --- a/docs/agent-environments.md +++ b/docs/agent-environments.md @@ -86,8 +86,9 @@ name it `.har`. manifest's, unless the capture's origin is a loopback name or an IP literal: the capture file then names it (`grafana.har` and `grafana.capture.json` become `grafana`) and a `name-from-file` warning says so. A file name that - yields no portable label (kebab-case, at most 64 characters, not a Windows - device name) is refused; pass `--name`. + yields no portable label (kebab-case, at most 63 characters for a DNS service + label, not a Windows device name or a label interpreted as an IP address) is + refused; pass `--name`. - `--package-tarball` works as for the export, and `--capture-timeout-ms` as for `inspect-web-network-capture`. - `--dry-run` runs every stage and prints the report with `status: "planned"` @@ -982,7 +983,8 @@ serves replay. The complete tool catalog the bundle serves must also fit the server's advertised catalog budget before any files are written. Privately parsed JSON is sealed once so authentication and replay share immutable response trees rather than copying the capture repeatedly. `--name` replaces the manifest's portable kebab-case name as the env folder and env id -(Windows device names such as `con` and `lpt1` are refused); +(at most 63 characters for its DNS service label; Windows device names and +labels interpreted as IP addresses are refused); `--package-tarball ` copies a packed `rea-agents` tarball into the env folder and makes the Dockerfile install it instead of the registry package and remove it in the same layer, which is how a checkout rather than a release is @@ -1000,7 +1002,12 @@ Windows inherits ACLs and REA does not establish owner-only access there. Choose an already private Windows destination when captured content needs it. The Dockerfile sets `MCP_HOST=0.0.0.0` and `MCP_PORT=18765`, the port the -framework maps for an MCP server env; the framework's own deploy supplies +framework maps for an MCP server env, and `MCP_ALLOWED_HOSTS` naming the loopback +addresses and the environment name, because the container binds outside loopback +and the server refuses to serve on such a bind without an operator-declared +allowlist: the framework's published port is reached over loopback and its +gateway proxies to the env container by the compose service name (the +environment name). The framework's own deploy supplies `MCP_HOST`, `MCP_PORT` and `ENVIRONMENT_NAME`, so the served card carries the env's name. It installs Debian's `python3` because the framework's local sandbox probes a container's readiness with `python3` inside it, which the slim diff --git a/docs/cli.md b/docs/cli.md index 0e0259f267..367731119a 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -738,8 +738,18 @@ rea mcp --http --port 8765 ``` `--host` and `--port` default to `MCP_HOST` and `MCP_PORT` from the environment, -then to `127.0.0.1` and an ephemeral port. The bound URL is written to stderr as -one JSON log line (`"msg":"MCP transport listening"`) whatever `REA_LOG_LEVEL` +then to `127.0.0.1` and an ephemeral port. A bind outside loopback requires +`MCP_ALLOWED_HOSTS` or `--allowed-hosts`, a comma-separated list of client host +names or IP addresses without URLs or ports. For example: + +```sh +rea mcp --http --host 0.0.0.0 --allowed-hosts 127.0.0.1,rea.lan +``` + +Requests with a different `Host` or browser `Origin` receive `403`; a missing +`Origin` is accepted. The announced client URL uses the bound hostname when +admitted, otherwise the first declared hostname, with the actual bound port. +It is written to stderr as one JSON log line (`"msg":"MCP transport listening"`) whatever `REA_LOG_LEVEL` is set to; nothing is written to stdout. `rea mcp --http --help` lists the accepted options. The listener serves one client at a time: a session ends with the client's `DELETE`, and a client that connects while another session is open takes it over, so a client that @@ -777,9 +787,18 @@ rea environment-serve manifest.json --evidence capture.json --port 8765 ``` `--host` and `--port` resolve like `rea mcp --http`: flags, then `MCP_HOST` and -`MCP_PORT`, then `127.0.0.1` and an ephemeral port. `ENVIRONMENT_NAME` renames +`MCP_PORT`, then `127.0.0.1` and an ephemeral port. A non-loopback bind requires +`MCP_ALLOWED_HOSTS` with the same hostname rules and Host/Origin gates as above. +`environment-serve` uses this environment setting; `--allowed-hosts` belongs to +`rea mcp --http`. For example: + +```sh +MCP_ALLOWED_HOSTS=127.0.0.1,rea.lan rea environment-serve manifest.json --evidence capture.json --host 0.0.0.0 +``` + +`ENVIRONMENT_NAME` renames the served environment; set but blank, it is refused as `configuration_invalid` -like a blank `MCP_HOST`. The bound URL is written to stderr as one JSON log line +like a blank `MCP_HOST`. The admitted client URL is written to stderr as one JSON log line (`"msg":"MCP transport listening"`). See [agent environments](agent-environments.md) for what the state and replay return and do not establish. diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 6a67e5aea9..937a60d621 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -817,7 +817,12 @@ the bound address is local; names are added only after classifying the actual IP On such a bind, requests whose `Host` header is outside the SDK loopback allowlist plus the configured name and the bound address are refused with `403`, and a browser `Origin` outside that allowlist is refused the same way (a missing `Origin` -passes); a non-loopback bind relaxes both checks and logs the relaxation. A reply sent before a request body was read (a refused `Host` or `Origin`, an unknown route, the startup `503`) closes the connection after the reply, so an unread upload cannot keep a socket open. Request targets are taken verbatim as origin-form paths (`//host/path` is a path, not another origin); an absolute-form target, a target containing a raw fragment delimiter (`#`), or one whose path URL parsing would rewrite (dot segments such as `/public/../mcp`), is refused with `400`, and a method Fetch does not support with `405`. Percent-encoded `%23` remains valid query data. Only an `initialize` object can replace an open session. A sessionless array containing initialize, including a one-element array, is refused with HTTP `400` before session creation or handover. Scoped +passes). A non-loopback bind requires `MCP_ALLOWED_HOSTS`, or +`--allowed-hosts` for `rea mcp --http`, listing every client host name or IP +address. Configuration validates and canonicalizes complete tokens once; +URLs, ports and empty entries are refused. Both Host and Origin checks remain +active on that declared list. The announced URL uses the bound hostname if +admitted, otherwise the first declared hostname, with the actual bound port. A reply sent before a request body was read (a refused `Host` or `Origin`, an unknown route, the startup `503`) closes the connection after the reply, so an unread upload cannot keep a socket open. Request targets are taken verbatim as origin-form paths (`//host/path` is a path, not another origin); an absolute-form target, a target containing a raw fragment delimiter (`#`), or one whose path URL parsing would rewrite (dot segments such as `/public/../mcp`), is refused with `400`, and a method Fetch does not support with `405`. Percent-encoded `%23` remains valid query data. Only an `initialize` object can replace an open session. A sessionless array containing initialize, including a one-element array, is refused with HTTP `400` before session creation or handover. Scoped IPv6 literals (`fe80::1%eth0`) are refused before binding, because no URL can name them. Listener errors reach the runtime's connection-loss handler only after a successful bind; a bind failure prints the listen-failure message alone. Request bodies are bounded by the SDK's diff --git a/docs/specs/agent-environments/environment-export/spec.md b/docs/specs/agent-environments/environment-export/spec.md index 97fb829c24..e36a4c57f5 100644 --- a/docs/specs/agent-environments/environment-export/spec.md +++ b/docs/specs/agent-environments/environment-export/spec.md @@ -33,12 +33,14 @@ previous increment. copies of the inputs (mode `0600` on POSIX; Windows retains inherited ACLs and does not provide an owner-only guarantee), `artifacts/check_/verify.py` and `tasks/smoke.json`, where `` defaults to the manifest's kebab-case name - and `--name` must be a kebab-case label of at most 64 characters that is not a + and `--name` must be a kebab-case label of at most 63 characters for its DNS label that is not a device name Windows reserves (`con`, `prn`, `aux`, `nul`, `com1`–`com9`, - `lpt1`–`lpt9`), since the env folder could not be created there. + `lpt1`–`lpt9`). Its HTTP hostname identity must remain unchanged, so numeric + labels interpreted as IP addresses are refused before planning. - The Dockerfile uses the Node version pinned in `.nvmrc` with the `bookworm-slim` variant, sets - `MCP_HOST=0.0.0.0` and `MCP_PORT=18765`, installs Debian's `python3` for the + `MCP_HOST=0.0.0.0`, `MCP_PORT=18765`, and + `MCP_ALLOWED_HOSTS=127.0.0.1,localhost,`, installs Debian's `python3` for the framework's readiness probe, installs `rea-agents@` from the registry or, with `--package-tarball`, copies the tarball as `rea-agents.tgz`, installs it diff --git a/docs/specs/agent-environments/environment-serve/spec.md b/docs/specs/agent-environments/environment-serve/spec.md index b0f4ab8fb5..2a4356f353 100644 --- a/docs/specs/agent-environments/environment-serve/spec.md +++ b/docs/specs/agent-environments/environment-serve/spec.md @@ -104,8 +104,10 @@ unreadable path; the process-boundary test asserts a `143` exit with a released port. Replay validates no supplied values, performs no network activity and establishes no authorization, response semantics or state transitions. One MCP client is served at a time, as the shared listener -defines. Non-loopback binds relax the `Host` check and are not exercised; a -listener `error` after binding is wired but not provoked by a test. +defines. Non-loopback binds require `MCP_ALLOWED_HOSTS` and retain both Host +and Origin checks. Shared listener regressions exercise a wildcard bind, +configuration refusal, port release and admitted/foreign request headers; +the original environment-serving fixture runs below used loopback. ## Resource and cancellation boundaries diff --git a/docs/specs/agent-environments/environment-serve/verification.md b/docs/specs/agent-environments/environment-serve/verification.md index 5d223d5c54..bf797f1edd 100644 --- a/docs/specs/agent-environments/environment-serve/verification.md +++ b/docs/specs/agent-environments/environment-serve/verification.md @@ -235,8 +235,10 @@ parameters cannot mismatch, and a call without arguments is exact on the tool's first record. Request bodies on write tools are accepted and ignored. The post-bind listener `error` path is wired and type-checked but not provoked by a test, since the shared listener does not expose its -`node:http` server. Non-loopback binds and the relaxed `Host` check were not -exercised; the Python exchange, curl checks and every test used `127.0.0.1`. +`node:http` server. These recorded runs predate the required non-loopback Host/Origin allowlist; +the Python exchange, curl checks and tests recorded here used `127.0.0.1`. +The current contract and later non-loopback coverage are described in the +[transport specification](../http-transport/spec.md). Replay selection and body parsing were measured on the 28-record fixture only; no large-capture timing was taken. Resident-set figures are `ps` samples of the serving process before and after one request each. Token usage and diff --git a/docs/specs/agent-environments/http-transport/spec.md b/docs/specs/agent-environments/http-transport/spec.md index e4e9135833..3c59819310 100644 --- a/docs/specs/agent-environments/http-transport/spec.md +++ b/docs/specs/agent-environments/http-transport/spec.md @@ -67,8 +67,11 @@ initialize, tools/list, `binary_session` and `DELETE` with the SDK streamable HTTP client, then proves `binary_session` still answers in the next session and in a session that replaced one left open; a process-boundary test asserts `143`/`130` exits and a released port. Concurrent clients are not supported: -the newest `initialize` owns the server. The non-loopback relaxation is covered -only through its pure allowlist helper; no wide bind runs in tests. Response +the newest `initialize` owns the server. Non-loopback binds require the +operator's parsed hostname allowlist, retain Host/Origin gates, and announce a +client URL admitted by that list. Current listener regressions exercise a +wildcard bind, refusal/port release and a direct SDK connection to the +announced endpoint. Response budgets keep their stdio-derived floor; registrations, `rea mcp doctor` and `server.json` stay stdio. diff --git a/docs/specs/agent-environments/http-transport/verification.md b/docs/specs/agent-environments/http-transport/verification.md index 73dd3ff34a..44ce3ff297 100644 --- a/docs/specs/agent-environments/http-transport/verification.md +++ b/docs/specs/agent-environments/http-transport/verification.md @@ -145,9 +145,9 @@ The concurrent-`initialize` case is exercised at the slot with in-memory `Request` objects, where both calls advance in lockstep; over TCP two connections are served in separate event-loop turns, so the race is reachable only in-process and the listener-level behaviour remains "the newest client -owns the server, the other sees `400` or `404`". The non-loopback relaxation -path is covered only through the pure allowlist helper and the loopback smoke -runs; no wide bind was exercised. Sessions are served one at a time; +owns the server, the other sees `400` or `404`". Those recorded runs predate the mandatory non-loopback allowlist and did not +exercise a wide bind. The current requirement and later listener regression +coverage are described in the [specification](spec.md). Sessions are served one at a time; concurrent clients are not supported. The two failures that reproduce on the clean clone are environmental (macOS with Node 22.23.1 and npm 12.0.2) and are left for their owners; the load-induced timeouts were classified by isolated diff --git a/src/application/environment/environmentBuild.ts b/src/application/environment/environmentBuild.ts index f8325eb804..65a37b89cf 100644 --- a/src/application/environment/environmentBuild.ts +++ b/src/application/environment/environmentBuild.ts @@ -15,6 +15,7 @@ import type { EnvironmentBuildReport } from "../../domain/environmentBundleExpor import { err, ok, type Result } from "../../domain/result.js"; import { inspectAnalysisView } from "../analysisView/AnalysisViewService.js"; import { + BUNDLE_NAME_CONSTRAINT, ENVIRONMENT_EXPORT_OPERATION, isPortableBundleName, planEnvironmentBundle, @@ -191,7 +192,7 @@ const unnamed = (path: string): AnalysisInputError => new AnalysisInputError( ENVIRONMENT_BUILD_OPERATION, { - userMessage: `The capture's origin is a loopback or IP address, and ${path === "" ? "retained Evidence has no file name to take one from" : `${path} yields no portable environment name (a kebab-case label of at most 64 characters that is not a Windows device name)`}; name the environment.`, + userMessage: `The capture's origin is a loopback or IP address, and ${path === "" ? "retained Evidence has no file name to take one from" : `${path} yields no portable environment name (${BUNDLE_NAME_CONSTRAINT})`}; name the environment.`, }, [ { diff --git a/src/application/environment/environmentBundle.test.ts b/src/application/environment/environmentBundle.test.ts index 1a2e3a9895..c924f4a15c 100644 --- a/src/application/environment/environmentBundle.test.ts +++ b/src/application/environment/environmentBundle.test.ts @@ -15,6 +15,7 @@ import { PACKAGE_METADATA } from "../../generatedPackageMetadata.js"; import { BUNDLE_BASE_IMAGE, bundleDockerfile, + isPortableBundleName, ENVIRONMENT_EXPORT_OPERATION, planEnvironmentBundle, registryPackageSource, @@ -117,6 +118,9 @@ describe("planEnvironmentBundle", () => { expect(lines[0]).toBe(`FROM ${BUNDLE_BASE_IMAGE}`); expect(lines).toContain("ENV MCP_HOST=0.0.0.0"); expect(lines).toContain("ENV MCP_PORT=18765"); + expect(lines).toContain( + `ENV MCP_ALLOWED_HOSTS=127.0.0.1,localhost,grafana-example`, + ); expect(lines).toContain( "RUN apt-get update && apt-get install -y --no-install-recommends python3 && rm -rf /var/lib/apt/lists/*", ); @@ -235,6 +239,36 @@ describe("planEnvironmentBundle", () => { }); describe("planEnvironmentBundle portable names", () => { + it.each(["4294967296", "123", "0xffffffff"])( + "refuses bundle name %s when HTTP cannot preserve its host identity", + (name) => { + expect(nameRefusal(name)).toMatchObject({ + code: "invalid_request", + details: { + operation: "environment-export", + issues: [{ path: ["name"], reason: "invalid_value" }], + }, + }); + }, + ); + + it("keeps addressable numeric prefixes and admits the DNS label boundary", () => { + for (const name of ["123-replay", "0xffffffff-replay", "a".repeat(63)]) { + expect(isPortableBundleName(name)).toBe(true); + const planned = plan({ name }); + expect(planned.name).toBe(name); + expect(file(planned, `envs/${name}/Dockerfile`)).toContain( + `ENV MCP_ALLOWED_HOSTS=127.0.0.1,localhost,${name}`, + ); + } + expect(isPortableBundleName("a".repeat(64))).toBe(false); + expect(nameRefusal("a".repeat(64))).toMatchObject({ + code: "invalid_request", + }); + for (const name of ["123", "4294967296", "0xffffffff"]) + expect(isPortableBundleName(name)).toBe(false); + }); + it("accepts a kebab-case name override and refuses any other name as invalid input", () => { const renamed = plan({ name: "replay-2" }); expect(renamed.files.map((entry) => entry.path)).toContain( diff --git a/src/application/environment/environmentBundle.ts b/src/application/environment/environmentBundle.ts index 6ec75c8143..d5cf6fd7b7 100644 --- a/src/application/environment/environmentBundle.ts +++ b/src/application/environment/environmentBundle.ts @@ -1,3 +1,4 @@ +import { normalizeHttpHostname } from "../../domain/httpHostname.js"; import { PRODUCT_IDENTITY } from "../../identity.js"; import { PACKAGE_METADATA } from "../../generatedPackageMetadata.js"; import type { AnalysisError } from "../../domain/analysisErrorBase.js"; @@ -20,21 +21,23 @@ export const BUNDLE_BASE_IMAGE = `node:${PACKAGE_METADATA.pinnedNodeVersion}-boo /** The port the agentenv framework maps for an MCP server env; it sets the host but not the port. */ export const BUNDLE_MCP_PORT = 18765; +/** Host names the exported env container is addressed by: the published loopback port and the compose service name. */ +export const BUNDLE_ALLOWED_HOSTS = "127.0.0.1,localhost"; /** Name of the packed CLI inside the env folder when a tarball is exported. */ export const BUNDLE_TARBALL_NAME = "rea-agents.tgz"; /** Folder and env id rule: a kebab-case label the agentenv framework accepts as a local id. */ const BUNDLE_NAME = /^[a-z0-9]+(?:-[a-z0-9]+)*$/u; -const BUNDLE_NAME_MAX_LENGTH = 64; +// A compose service name is one DNS label, whose wire limit is 63 octets. +const BUNDLE_NAME_MAX_LENGTH = 63; +export const BUNDLE_NAME_CONSTRAINT = `a kebab-case label of at most ${String(BUNDLE_NAME_MAX_LENGTH)} characters, excluding Windows device names, that remains unchanged as an HTTP service hostname`; /** Win32 reserves these device names for files and folders, so the env folder can never take one. */ const WINDOWS_RESERVED_NAME = /^(?:con|prn|aux|nul|com[1-9]|lpt[1-9])$/u; /** Whether a name can be the env folder and env id: the checks the export applies. */ export const isPortableBundleName = (name: string): boolean => - BUNDLE_NAME.test(name) && - name.length <= BUNDLE_NAME_MAX_LENGTH && - !WINDOWS_RESERVED_NAME.test(name); + validateBundleName(name).ok; /** Where the Dockerfile installs the CLI from. */ export type EnvironmentPackageSource = @@ -121,6 +124,33 @@ const reservedNameError = (name: string): AnalysisInputError => ], ); +/** One owner admits the same folder, environment id and HTTP service hostname. */ +const validateBundleName = ( + name: string, +): Result => { + if (!BUNDLE_NAME.test(name) || name.length > BUNDLE_NAME_MAX_LENGTH) + return err(nameError(name)); + if (WINDOWS_RESERVED_NAME.test(name)) return err(reservedNameError(name)); + if (normalizeHttpHostname(name) !== name) + return err( + new AnalysisInputError( + ENVIRONMENT_EXPORT_OPERATION, + { + userMessage: `The bundle name ${JSON.stringify(name)} cannot be used unchanged as an HTTP host name. Choose a name such as replay-1.`, + }, + [ + { + path: ["name"], + reason: "invalid_value", + message: + "The service name must remain a hostname rather than be rejected or interpreted as an IP address.", + }, + ], + ), + ); + return ok(name); +}; + const installLine = (source: EnvironmentPackageSource): string => source.kind === "registry" ? `RUN npm install -g --ignore-scripts --no-fund --no-audit ${source.specifier}` @@ -131,11 +161,19 @@ const READINESS_PROBE_LAYER = "RUN apt-get update && apt-get install -y --no-install-recommends python3 && rm -rf /var/lib/apt/lists/*"; /** The Dockerfile text: pinned base image, readiness probe interpreter, pinned CLI, listener defaults, non-root user. */ -export const bundleDockerfile = (source: EnvironmentPackageSource): string => +export const bundleDockerfile = ( + source: EnvironmentPackageSource, + name?: string, +): string => [ `FROM ${BUNDLE_BASE_IMAGE}`, `ENV MCP_HOST=0.0.0.0`, `ENV MCP_PORT=${String(BUNDLE_MCP_PORT)}`, + // The env container binds outside loopback for the framework's published port and + // gateway proxy, so it names the hosts they address it by; the gateway reaches it + // as the compose service name (the environment name) and host-side clients go + // through the published loopback port. + `ENV MCP_ALLOWED_HOSTS=${BUNDLE_ALLOWED_HOSTS}${name === undefined ? "" : `,${name}`}`, READINESS_PROBE_LAYER, installLine(source), // Owned by the runtime user, so a private host umask cannot make the inputs unreadable. @@ -332,9 +370,8 @@ export const planEnvironmentBundle = ( options: EnvironmentBundleOptions, ): Result => { const name = options.name ?? source.manifest.name; - if (!BUNDLE_NAME.test(name) || name.length > BUNDLE_NAME_MAX_LENGTH) - return err(nameError(name)); - if (WINDOWS_RESERVED_NAME.test(name)) return err(reservedNameError(name)); + const admitted = validateBundleName(name); + if (!admitted.ok) return admitted; // Prepared only after the name checks, under the name the bundle's server serves. const { state, report } = options.served(name); // The catalog and answers the bundle's server will have, so the verifier and README predict them. @@ -370,7 +407,7 @@ export const planEnvironmentBundle = ( }, { path: `${envFolder}/Dockerfile`, - content: bundleDockerfile(options.packageSource), + content: bundleDockerfile(options.packageSource, name), }, { path: `artifacts/check_${name}/verify.py`, diff --git a/src/cli/environmentCommands.ts b/src/cli/environmentCommands.ts index abce912196..e2b0aefbaf 100644 --- a/src/cli/environmentCommands.ts +++ b/src/cli/environmentCommands.ts @@ -67,8 +67,11 @@ const untilAborted = (signal: AbortSignal): Promise => export const transportError = ( endpoint: McpHttpEndpoint, cause: StreamableHttpListenError, -): AnalysisResourceConstraintError => - new AnalysisResourceConstraintError( +): AnalysisResourceConstraintError => { + const failure = cause.classification.ok + ? cause.classification.value + : cause.classification.error; + return new AnalysisResourceConstraintError( CLI_COMMANDS.environmentServe, "transport", cause.message, @@ -77,38 +80,22 @@ export const transportError = ( cause, retryable: false, diagnostics: { - listener_failure: cause.classification.ok - ? { - kind: cause.classification.value.kind, - system_code: cause.classification.value.systemCode, - } - : { - kind: cause.classification.error.kind, - ...(cause.classification.error.systemCode === undefined - ? {} - : { system_code: cause.classification.error.systemCode }), - }, + listener_failure: { + kind: failure.kind, + ...("systemCode" in failure && failure.systemCode !== undefined + ? { system_code: failure.systemCode } + : {}), + ...(failure.kind === "host_allowlist" + ? { + reason: failure.reason, + } + : {}), + }, listener_phase: cause.phase, }, - remediationAction: listenerRemediation(cause.classification), + remediationAction: cause.remediationAction, }, ); - -const listenerRemediation = ( - classification: StreamableHttpListenError["classification"], -): string => { - if (!classification.ok) - return "Check the selected listener host, port, and binding permissions, then retry."; - switch (classification.value.kind) { - case "host_lookup": - return "Check --host or MCP_HOST and host name resolution; correct the value or resolve the lookup failure, then retry."; - case "host_unavailable": - return "Choose an address assigned to this machine with --host or MCP_HOST, then retry."; - case "port_in_use": - return "Choose another --port or MCP_PORT, or stop the process using it."; - case "permission_denied": - return "Choose a host and port this process is permitted to bind; if the selected port is restricted, use an allowed port."; - } }; interface ServeArguments { diff --git a/src/config/mcpHttpEndpoint.ts b/src/config/mcpHttpEndpoint.ts index 06980156ba..3263051000 100644 --- a/src/config/mcpHttpEndpoint.ts +++ b/src/config/mcpHttpEndpoint.ts @@ -1,4 +1,8 @@ import { z } from "zod"; +import { + httpHostnameSchema, + type HttpHostname, +} from "../domain/httpHostname.js"; import { ConfigurationError, @@ -27,11 +31,34 @@ export const MCP_HTTP_PORT_CONSTRAINT = "MCP_PORT must be a decimal integer between 0 and 65535."; export const MCP_HTTP_HOST_CONSTRAINT = "MCP_HOST must be a non-empty host name or IP address."; +export const MCP_ALLOWED_HOSTS_CONSTRAINT = + "MCP_ALLOWED_HOSTS must list at least one host name or IP address, separated by commas, without URL components or ports."; +export const MCP_ALLOWED_HOSTS_FLAG_CONSTRAINT = + "must list at least one host name or IP address, separated by commas, without URL components or ports."; + +/** Comma-separated operator-declared Host/Origin names for a non-loopback bind. */ +export const mcpAllowedHostsTextSchema = z + .string() + .transform((text, context) => { + const entries = text.split(",").map((entry) => entry.trim()); + if (entries.some((entry) => entry.length === 0)) { + context.addIssue({ + code: "custom", + message: MCP_ALLOWED_HOSTS_CONSTRAINT, + }); + return z.NEVER; + } + return entries; + }) + .pipe(z.array(httpHostnameSchema)) + .transform((names) => [...new Set(names)]); /** Where the streamable HTTP listener binds. */ export interface McpHttpEndpoint { readonly host: string; readonly port: number; + /** Operator-declared Host/Origin names; required before a non-loopback bind serves. */ + readonly allowedHosts?: readonly HttpHostname[]; } /** Resolve the endpoint: explicit values, then MCP_HOST/MCP_PORT, then loopback defaults. */ @@ -42,6 +69,7 @@ export const parseMcpHttpEndpoint = ( const issues: ConfigurationSettingIssue[] = []; let host = overrides.host ?? DEFAULT_MCP_HTTP_HOST; let port = overrides.port ?? DEFAULT_MCP_HTTP_PORT; + let allowedHosts = overrides.allowedHosts; if (overrides.host === undefined && environment.MCP_HOST !== undefined) { const parsed = mcpHttpHostSchema.safeParse(environment.MCP_HOST); if (parsed.success) host = parsed.data; @@ -60,6 +88,20 @@ export const parseMcpHttpEndpoint = ( constraint: MCP_HTTP_PORT_CONSTRAINT, }); } + if ( + overrides.allowedHosts === undefined && + environment.MCP_ALLOWED_HOSTS !== undefined + ) { + const parsed = mcpAllowedHostsTextSchema.safeParse( + environment.MCP_ALLOWED_HOSTS, + ); + if (parsed.success) allowedHosts = parsed.data; + else + issues.push({ + setting: "MCP_ALLOWED_HOSTS", + constraint: MCP_ALLOWED_HOSTS_CONSTRAINT, + }); + } if (issues.length > 0) return err( new ConfigurationError( @@ -67,5 +109,9 @@ export const parseMcpHttpEndpoint = ( { settings: issues }, ), ); - return ok({ host, port }); + return ok({ + host, + port, + ...(allowedHosts === undefined ? {} : { allowedHosts }), + }); }; diff --git a/src/domain/httpHostname.ts b/src/domain/httpHostname.ts new file mode 100644 index 0000000000..f0797e3a88 --- /dev/null +++ b/src/domain/httpHostname.ts @@ -0,0 +1,37 @@ +import { isIPv6 } from "node:net"; +import { z } from "zod"; + +/** Parse a complete hostname/IP, never URL components, into the SDK's hostname identity. */ +export const normalizeHttpHostname = (host: string): string | undefined => { + const unbracketed = + host.startsWith("[") && host.endsWith("]") ? host.slice(1, -1) : host; + const ipv6 = isIPv6(unbracketed); + // URL parsing alone discards userinfo, paths and default ports, and decodes escapes. + if ( + /[\s\x00-\x1F\x7F%\x2F\x5C?#@]/u.test(host) || + (!ipv6 && host.includes(":")) + ) + return undefined; + try { + return new URL(`http://${ipv6 ? `[${unbracketed}]` : host}`).hostname; + } catch { + return undefined; + } +}; + +/** Canonical HTTP hostname identity, parsed before consumers use it for lookup or matching. */ +export const httpHostnameSchema = z + .string() + .transform((source, context) => { + const hostname = normalizeHttpHostname(source); + if (hostname !== undefined) return hostname; + context.addIssue({ + code: "custom", + message: + "Expected a host name or IP address without URL components or ports", + }); + return z.NEVER; + }) + .brand<"HttpHostname">(); + +export type HttpHostname = z.infer; diff --git a/src/main.ts b/src/main.ts index 1954355635..3160649239 100644 --- a/src/main.ts +++ b/src/main.ts @@ -175,6 +175,9 @@ export const runMcpHttpEntrypoint = async ( const endpoint = parseMcpHttpEndpoint(snapshotEnvironment(process.env), { ...(parsed.host === undefined ? {} : { host: parsed.host }), ...(parsed.port === undefined ? {} : { port: parsed.port }), + ...(parsed.allowedHosts === undefined + ? {} + : { allowedHosts: [...parsed.allowedHosts] }), }); if (!endpoint.ok) { io.writeStderr(`${projectAnalysisError(endpoint.error).message}\n`); diff --git a/src/main/mcpHttpArguments.ts b/src/main/mcpHttpArguments.ts index 6072857c3d..013cf583b0 100644 --- a/src/main/mcpHttpArguments.ts +++ b/src/main/mcpHttpArguments.ts @@ -1,7 +1,10 @@ import { Help, z } from "incur"; +import type { HttpHostname } from "../domain/httpHostname.js"; import { + MCP_ALLOWED_HOSTS_FLAG_CONSTRAINT, MCP_HTTP_PORT_CONSTRAINT, + mcpAllowedHostsTextSchema, mcpHttpListenerPortSchema, mcpHttpPortSchema, } from "../config/mcpHttpEndpoint.js"; @@ -22,6 +25,12 @@ const MCP_HTTP_OPTIONS = z.object({ port: mcpHttpListenerPortSchema .optional() .describe("Listener port; default MCP_PORT, else 0 for an ephemeral port"), + allowedHosts: z + .array(z.string()) + .optional() + .describe( + "Host/Origin names a non-loopback bind may serve; default MCP_ALLOWED_HOSTS, else the bind is refused", + ), help: z.boolean().optional().describe("Show help"), }); @@ -30,7 +39,11 @@ const MCP_HTTP_COMMAND = "rea mcp --http"; const MCP_HTTP_DESCRIPTION = "Serve the MCP server over streamable HTTP for one local client"; const HELP_FLAGS: ReadonlySet = new Set(["--help", "-h"]); -const VALUED_FLAGS: ReadonlySet = new Set(["--host", "--port"]); +const VALUED_FLAGS: ReadonlySet = new Set([ + "--host", + "--port", + "--allowed-hosts", +]); /** Render help for the dispatcher entry, which accepts no Incur global options. */ export const renderMcpHttpHelp = (): string => @@ -44,7 +57,12 @@ export const renderMcpHttpHelp = (): string => /** Parsed `rea mcp --http` arguments; omitted values defer to the environment. */ export type McpHttpArguments = - | { readonly ok: true; readonly host?: string; readonly port?: number } + | { + readonly ok: true; + readonly host?: string; + readonly port?: number; + readonly allowedHosts?: readonly HttpHostname[]; + } | { readonly ok: false; readonly help: true } | { readonly ok: false; readonly help: false; readonly message: string }; @@ -93,9 +111,19 @@ export const parseMcpHttpArguments = ( portText === undefined ? undefined : mcpHttpPortSchema.safeParse(portText); if (port !== undefined && !port.success) return invalid(`Invalid --port: ${MCP_HTTP_PORT_CONSTRAINT}`); + const allowedHostsText = values.get("--allowed-hosts"); + const allowedHosts = + allowedHostsText === undefined + ? undefined + : mcpAllowedHostsTextSchema.safeParse(allowedHostsText); + if (allowedHosts !== undefined && !allowedHosts.success) + return invalid( + `Invalid --allowed-hosts: ${MCP_ALLOWED_HOSTS_FLAG_CONSTRAINT}`, + ); return { ok: true, ...(host === undefined ? {} : { host }), ...(port === undefined ? {} : { port: port.data }), + ...(allowedHosts === undefined ? {} : { allowedHosts: allowedHosts.data }), }; }; diff --git a/src/main/transport.ts b/src/main/transport.ts index d76a9b20ea..8735fce606 100644 --- a/src/main/transport.ts +++ b/src/main/transport.ts @@ -170,7 +170,10 @@ export const startMcpTransport = async ( if (!cancelled) { const startFailure = cause instanceof StreamableHttpListenError - ? MCP_HTTP_LISTEN_FAILED + ? cause.classification.ok && + cause.classification.value.kind === "host_allowlist" + ? cause.remediationAction + : MCP_HTTP_LISTEN_FAILED : MCP_CONNECTION_START_FAILED; serverLogger.error( { diff --git a/src/server/environment/serveEnvironment.ts b/src/server/environment/serveEnvironment.ts index 1931937047..91bb183ea8 100644 --- a/src/server/environment/serveEnvironment.ts +++ b/src/server/environment/serveEnvironment.ts @@ -1,3 +1,4 @@ +import type { HttpHostname } from "../../domain/httpHostname.js"; import { STATE_INTAKE_LIMIT_BYTES } from "../../application/environment/environmentStateDocument.js"; import { DEFAULT_MAX_REQUEST_BODY_SIZE, @@ -426,6 +427,8 @@ export interface ServeEnvironmentOptions { readonly source: EnvironmentSource; readonly host: string; readonly port: number; + /** Operator-declared Host/Origin names; a non-loopback bind refuses to serve without them. */ + readonly allowedHosts?: readonly HttpHostname[]; readonly signal?: AbortSignal; /** Served name; defaults to the manifest's environment name. */ readonly environmentName?: string; @@ -477,6 +480,9 @@ export const serveEnvironment = async ( const served = await serveStreamableHttp({ host, port, + ...(options.allowedHosts === undefined + ? {} + : { allowedHosts: options.allowedHosts }), ...(options.signal === undefined ? {} : { signal: options.signal }), createMcpServer: () => createEnvironmentMcpServer(name, tools, answer), routes: environmentRoutes( diff --git a/src/server/http/serveStreamableHttp.test.ts b/src/server/http/serveStreamableHttp.test.ts index 80ece61372..3044fdb574 100644 --- a/src/server/http/serveStreamableHttp.test.ts +++ b/src/server/http/serveStreamableHttp.test.ts @@ -4,7 +4,7 @@ import { } from "@modelcontextprotocol/client"; import { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/server"; import { Agent, request as httpRequest } from "node:http"; -import { connect as connectTcp } from "node:net"; +import { connect as connectTcp, createServer } from "node:net"; import pino from "pino"; import { afterEach, expect, it, vi } from "vitest"; import { z } from "zod"; @@ -20,6 +20,7 @@ import { type StreamableHttpServer, } from "./serveStreamableHttp.js"; import { StreamableHttpListenError } from "./streamableHttpListenError.js"; +import { mcpAllowedHostsTextSchema } from "../../config/mcpHttpEndpoint.js"; import { RequestTrackingMcpServer } from "../RequestTrackingMcpServer.js"; const servers: StreamableHttpServer[] = []; @@ -89,6 +90,21 @@ const connect = async (url: string) => { return { transport, client }; }; +/** Reserve an ephemeral port, release it, and return the number for a later bind. */ +const listenEphemeral = async (): Promise => + new Promise((resolve, reject) => { + const probe = createServer(); + probe.unref(); + probe.once("listening", () => { + const { port } = probe.address() as { port: number }; + probe.close(() => + port > 0 ? resolve(port) : reject(new Error("no port")), + ); + }); + probe.once("error", reject); + probe.listen(0, "127.0.0.1"); + }); + const echo = async (client: Client, text: string): Promise => { const result = await client.callTool({ name: "echo", arguments: { text } }); return result.content; @@ -284,6 +300,67 @@ it("refuses foreign Host headers on a loopback bind and frees the port on close" }); }); +it("refuses a non-loopback bind without an operator host allowlist as a typed listen error and frees the port", async () => { + const port = await listenEphemeral(); + const refused = start({ host: "0.0.0.0", port }); + await expect(refused).rejects.toBeInstanceOf(StreamableHttpListenError); + await expect(refused).rejects.toThrow(/MCP_ALLOWED_HOSTS/u); + // The refusal happens after the bind, so the staged close must have released it. + // `start` owns the re-bound listener: afterEach closes it even if an assertion above throws. + const rebind = start({ host: "127.0.0.1", port }); + await expect(rebind).resolves.toMatchObject({ + served: { close: expect.any(Function) }, + }); +}); + +it.each([ + ["REA.LAN", "rea.lan"], + ["rea.lan.", "rea.lan."], + ["bücher.example", "xn--bcher-kva.example"], + ["127.0.0.1", "127.0.0.1"], + ["::1", "[::1]"], + ["0:0:0:0:0:0:0:1", "[::1]"], + ["[0:0:0:0:0:0:0:1]", "[::1]"], +])("preserves declared hostname %j as %j", async (name, normalized) => { + const { served } = await start({ + host: "0.0.0.0", + allowedHosts: mcpAllowedHostsTextSchema.parse(name), + routes: [ + { + method: "GET", + path: "/probe", + handle: () => Response.json({ allowed: true }), + }, + ], + }); + const url = new URL(served.url); + url.hostname = "127.0.0.1"; + url.pathname = "/probe"; + const response = await rawHttpRequest(url, { + headers: { + host: `${normalized}:${url.port}`, + origin: `http://${normalized}:${url.port}`, + }, + }); + expect(response.status).toBe(200); + expect(JSON.parse(response.body)).toEqual({ allowed: true }); +}); + +it("announces an admitted non-loopback URL and gates it on the operator allowlist", async () => { + const { served } = await start({ + host: "0.0.0.0", + allowedHosts: mcpAllowedHostsTextSchema.parse("127.0.0.1"), + }); + const url = new URL(served.url); + expect(url.hostname).toBe("127.0.0.1"); + const { client } = await connect(url.toString()); + await expect(echo(client, "allowed")).resolves.toEqual([ + { type: "text", text: "allowed" }, + ]); + await client.close(); + await expectForeignHostRefused(url); +}); + it("reports a port that cannot be bound as a typed listen error naming the cause", async () => { const { served } = await start(); const failed = serveStreamableHttp({ diff --git a/src/server/http/serveStreamableHttp.ts b/src/server/http/serveStreamableHttp.ts index 7252463b7d..2f5547f370 100644 --- a/src/server/http/serveStreamableHttp.ts +++ b/src/server/http/serveStreamableHttp.ts @@ -16,8 +16,12 @@ import { type IncomingMessage, type ServerResponse, } from "node:http"; -import { isIPv4, isIPv6, type AddressInfo } from "node:net"; +import { isIPv4, type AddressInfo } from "node:net"; import type { Logger } from "pino"; +import { + normalizeHttpHostname, + type HttpHostname, +} from "../../domain/httpHostname.js"; import { toWebRequest, @@ -25,7 +29,10 @@ import { writeWebResponse, } from "./nodeHttpBridge.js"; import { listenHttpServer } from "./listenHttpServer.js"; -import { StreamableHttpListenError } from "./streamableHttpListenError.js"; +import { + StreamableHttpHostAllowlistError, + StreamableHttpListenError, +} from "./streamableHttpListenError.js"; import { NodeHttpRequestOwner, type HttpAdmissionLifetime, @@ -47,6 +54,8 @@ export interface StreamableHttpRoute { export interface ServeStreamableHttpOptions { readonly host: string; readonly port: number; + /** Operator-declared Host/Origin names; a non-loopback bind refuses to serve without them. */ + readonly allowedHosts?: readonly HttpHostname[]; /** Cancellation during listener startup; later shutdown remains owned by close(). */ readonly signal?: AbortSignal; /** Called once; the instance serves every session in turn on a fresh transport. */ @@ -73,14 +82,6 @@ export interface StreamableHttpServer { readonly close: () => Promise; } -/** The bind host as the SDK compares Host headers: lowercase, IPv6 bracketed and canonical. */ -const normalizedHostname = (host: string): string | undefined => { - try { - return new URL(`http://${isIPv6(host) ? `[${host}]` : host}`).hostname; - } catch { - return undefined; - } -}; const isLoopbackIpv4 = (address: string): boolean => isIPv4(address) && address.startsWith("127."); /** Classify the native bound IP; configured DNS names are not binding evidence. */ @@ -100,9 +101,9 @@ export const hostHeaderAllowlist = ( host: string, boundAddress: string, ): string[] | undefined => { - const bound = normalizedHostname(boundAddress); + const bound = normalizeHttpHostname(boundAddress); if (bound === undefined || !isLoopbackAddress(bound)) return undefined; - const names = [normalizedHostname(host), bound]; + const names = [normalizeHttpHostname(host), bound]; const known = names.filter((name): name is string => name !== undefined); return [...new Set([...localhostAllowedHostnames(), ...known])]; }; @@ -485,7 +486,7 @@ export const serveStreamableHttp = async ( admissionLifetime: "response" as const, }); if (stopped) return local(jsonError(503, -32000, "Server shutting down")); - // One loopback allowlist gates Host (DNS rebinding) and Origin (browser pages); a missing Origin passes. + // One resolved allowlist gates Host (DNS rebinding) and Origin (browser pages); a missing Origin passes. const refused = allowedHosts === undefined ? undefined @@ -550,9 +551,27 @@ export const serveStreamableHttp = async ( try { if (bound === null || typeof bound === "string") throw new TypeError("MCP HTTP listener has no TCP address"); - url = endpointUrl(bound); - origin = new URL(url).origin; - allowedHosts = hostHeaderAllowlist(host, bound.address); + const endpoint = new URL(endpointUrl(bound)); + const loopbackHosts = hostHeaderAllowlist(host, bound.address); + if (loopbackHosts !== undefined) { + allowedHosts = loopbackHosts; + } else { + // Configuration supplies canonical names; the listener does not reinterpret them. + if ( + options.allowedHosts === undefined || + options.allowedHosts.length === 0 + ) + throw new StreamableHttpHostAllowlistError(bound.address); + allowedHosts = [...options.allowedHosts]; + } + const announcedHost = allowedHosts.includes(endpoint.hostname) + ? endpoint.hostname + : allowedHosts[0]; + if (announcedHost === undefined) + throw new StreamableHttpHostAllowlistError(bound.address); + endpoint.hostname = announcedHost; + url = endpoint.href; + origin = endpoint.origin; } catch (cause: unknown) { await prepareTransportShutdown().catch(() => undefined); throw new StreamableHttpListenError(cause); @@ -598,11 +617,6 @@ export const serveStreamableHttp = async ( await close(); throw options.signal.reason; } - if (allowedHosts === undefined) - logger?.warn( - { host }, - "MCP HTTP Host header validation relaxed for a non-loopback bind", - ); // The URL is the only way to reach an ephemeral port, so the announcement ignores the log threshold. logger ?.child({}, { level: "info" }) diff --git a/src/server/http/streamableHttpListenError.ts b/src/server/http/streamableHttpListenError.ts index 6a6982173f..aebe1298ba 100644 --- a/src/server/http/streamableHttpListenError.ts +++ b/src/server/http/streamableHttpListenError.ts @@ -1,7 +1,11 @@ import { err, ok, type Result } from "../../domain/result.js"; -/** Parsed system failure facts for listener recovery guidance. */ +/** Parsed listener failure facts for recovery guidance. */ export type StreamableHttpListenFailure = + | { + readonly kind: "host_allowlist"; + readonly reason: "missing"; + } | { readonly kind: "host_lookup"; readonly systemCode: @@ -36,8 +40,10 @@ const systemCode = (cause: unknown): string | undefined => { return typeof code === "string" ? code : undefined; }; -/** Normalize recognized Node listener system codes once, at the listener boundary. */ +/** Classify configuration and native listener failures once, at their owning boundary. */ const classify = (cause: unknown): StreamableHttpListenClassification => { + if (cause instanceof StreamableHttpHostAllowlistError) + return ok(cause.failure); const code = systemCode(cause); switch (code) { case "ENOTFOUND": @@ -62,6 +68,41 @@ const classify = (cause: unknown): StreamableHttpListenClassification => { } }; +/** A declaration failure, distinct from an operating-system bind failure. */ +export class StreamableHttpHostAllowlistError extends Error { + readonly failure: Extract< + StreamableHttpListenFailure, + { kind: "host_allowlist" } + >; + + constructor(address: string) { + super( + `Refusing to serve ${address} outside loopback without an operator-declared host allowlist; set MCP_ALLOWED_HOSTS (or --allowed-hosts) to every host name clients will use, then retry`, + ); + this.failure = { kind: "host_allowlist", reason: "missing" }; + } +} + +/** Recovery belongs to the parsed failure owner, shared by CLI and MCP. */ +const listenerRemediation = ( + classification: StreamableHttpListenClassification, +): string => { + if (!classification.ok) + return "Check the selected listener host, port, and binding permissions, then retry."; + switch (classification.value.kind) { + case "host_allowlist": + return "Set MCP_ALLOWED_HOSTS to the host names or IP addresses clients will use, without URL components or ports, then retry. For rea mcp --http, --allowed-hosts accepts the same comma-separated list."; + case "host_lookup": + return "Check --host or MCP_HOST and host name resolution; correct the value or resolve the lookup failure, then retry."; + case "host_unavailable": + return "Choose an address assigned to this machine with --host or MCP_HOST, then retry."; + case "port_in_use": + return "Choose another --port or MCP_PORT, or stop the process using it."; + case "permission_denied": + return "Choose a host and port this process is permitted to bind; if the selected port is restricted, use an allowed port."; + } +}; + export type StreamableHttpListenPhase = "startup" | "serving"; /** Listener failure retaining its native cause, phase, and parsed recovery facts. */ @@ -69,6 +110,7 @@ export class StreamableHttpListenError extends Error { override readonly name = "StreamableHttpListenError"; readonly classification: StreamableHttpListenClassification; readonly phase: StreamableHttpListenPhase; + readonly remediationAction: string; constructor(cause: unknown, phase: StreamableHttpListenPhase = "startup") { super( @@ -79,5 +121,6 @@ export class StreamableHttpListenError extends Error { ); this.phase = phase; this.classification = classify(cause); + this.remediationAction = listenerRemediation(this.classification); } } diff --git a/tests/boundary/cli/environmentExportCli.test.ts b/tests/boundary/cli/environmentExportCli.test.ts index 23395c9db1..c540142b65 100644 --- a/tests/boundary/cli/environmentExportCli.test.ts +++ b/tests/boundary/cli/environmentExportCli.test.ts @@ -69,6 +69,39 @@ const BUNDLE_FILES = [ "tasks/smoke.json", ]; +cliTest( + "refuses unaddressable service names before writing a bundle", + async ({ cli }) => { + const root = await createTestTempDirectory("rea-bundle-hostname-cli-"); + const files = await writeEnvironmentServeFiles(root); + for (const name of ["4294967296", "123", "a".repeat(64)]) { + const directory = join(root, `bundle-${name}`); + const result = await cli.run({ + arguments: [ + "environment-export", + files.manifestPath, + "--evidence", + files.capturePath, + directory, + "--name", + name, + "--json", + ], + timeoutMs: 30_000, + }); + expect(result.exitCode).toBe(1); + expect(result.json).toMatchObject({ + code: "invalid_request", + details: { + operation: "environment-export", + issues: [{ path: ["name"] }], + }, + }); + await expect(stat(directory)).rejects.toMatchObject({ code: "ENOENT" }); + } + }, +); + cliTest( "writes the bundle folder from the fixture pair and prints the written files with sizes", async ({ cli }) => { diff --git a/tests/boundary/cli/environmentServeRemediation.test.ts b/tests/boundary/cli/environmentServeRemediation.test.ts index 449feec89d..77bbf7fd84 100644 --- a/tests/boundary/cli/environmentServeRemediation.test.ts +++ b/tests/boundary/cli/environmentServeRemediation.test.ts @@ -77,6 +77,26 @@ cliTest( }, ); +cliTest( + "reports the missing host allowlist instead of port advice for a non-loopback bind", + async ({ cli }) => { + const result = await serve(cli, await files(), "0.0.0.0", 0); + expect(result.exitCode).toBe(1); + expect(result.output).toMatchObject({ + code: "resource_constraint", + details: { + diagnostics: { + listener_failure: { kind: "host_allowlist", reason: "missing" }, + }, + }, + remediation: { action: expect.stringContaining("Set MCP_ALLOWED_HOSTS") }, + }); + expect(JSON.stringify(result.output.remediation)).not.toContain( + "Choose another --port", + ); + }, +); + cliTest( "retains port advice without closing the loopback listener it does not own", async ({ cli }) => { diff --git a/tests/boundary/mcp/mainErrors.test.ts b/tests/boundary/mcp/mainErrors.test.ts index b80f95785a..e448ef715d 100644 --- a/tests/boundary/mcp/mainErrors.test.ts +++ b/tests/boundary/mcp/mainErrors.test.ts @@ -12,7 +12,10 @@ import { SERVER_START_FAILED, } from "../../../src/main/messages.js"; import type { McpServeOptions } from "../../../src/main/types.js"; -import { StreamableHttpListenError } from "../../../src/server/http/streamableHttpListenError.js"; +import { + StreamableHttpHostAllowlistError, + StreamableHttpListenError, +} from "../../../src/server/http/streamableHttpListenError.js"; type RuntimeDependencies = NonNullable[0]>; @@ -201,6 +204,26 @@ describe("MCP runtime HTTP transport errors", () => { expect(closeCalls).toBe(1); }); + it("reports host allowlist recovery instead of port advice", async () => { + const output: string[] = []; + const refused = new StreamableHttpListenError( + new StreamableHttpHostAllowlistError("0.0.0.0"), + ); + expect( + await run( + dependencies({ + serve: () => Promise.reject(refused), + output, + shutdown: [], + exitCodes: [], + }), + ), + ).toBe(1); + expect(output).toEqual([`${refused.remediationAction}\n`]); + expect(output[0]).toContain("Set MCP_ALLOWED_HOSTS"); + expect(output[0]).not.toContain("Choose another --port"); + }); + it("reports an HTTP listener that cannot bind without its cause", async () => { const output: string[] = []; expect( diff --git a/tests/boundary/mcp/mcpHttpOptions.test.ts b/tests/boundary/mcp/mcpHttpOptions.test.ts index 243ef5682d..6081b3a7f5 100644 --- a/tests/boundary/mcp/mcpHttpOptions.test.ts +++ b/tests/boundary/mcp/mcpHttpOptions.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from "vitest"; -import { parseMcpHttpEndpoint } from "../../../src/config/mcpHttpEndpoint.js"; +import { + parseMcpHttpEndpoint, + mcpAllowedHostsTextSchema, +} from "../../../src/config/mcpHttpEndpoint.js"; import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; import { runMcpHttpEntrypoint } from "../../../src/main.js"; import { @@ -116,3 +119,111 @@ describe("MCP_HOST and MCP_PORT", () => { expect(projected).not.toContain("65536"); }); }); + +describe("MCP_ALLOWED_HOSTS", () => { + it.each([ + "not a host", + "127.0.0.1@evil.example", + "rea.lan/path", + "rea.lan\\path", + "rea.lan?query", + "rea.lan#fragment", + "rea.lan:80", + "https://rea.lan", + "rea.\tlan", + "rea.%6Can", + ])( + "rejects malformed hostname %j at both configuration inputs", + (invalidHost) => { + const endpoint = parseMcpHttpEndpoint({ + MCP_HOST: "0.0.0.0", + MCP_ALLOWED_HOSTS: invalidHost, + }); + expect(endpoint.ok).toBe(false); + if (endpoint.ok) throw new Error("Expected a configuration refusal"); + expect(endpoint.error.settings.map(({ setting }) => setting)).toEqual([ + "MCP_ALLOWED_HOSTS", + ]); + expect( + parseMcpHttpArguments(["--http", "--allowed-hosts", invalidHost]), + ).toMatchObject({ + ok: false, + message: expect.stringContaining("without URL components or ports"), + }); + }, + ); + + it("passes canonical host identities downstream and deduplicates after normalization", () => { + expect( + parseMcpHttpEndpoint({ + MCP_ALLOWED_HOSTS: + "REA.LAN,rea.lan,[0:0:0:0:0:0:0:1],::1,bücher.example", + }), + ).toEqual({ + ok: true, + value: { + host: "127.0.0.1", + port: 0, + allowedHosts: ["rea.lan", "[::1]", "xn--bcher-kva.example"], + }, + }); + }); + + it("parses comma-separated allowlist text and lets an explicit value win", () => { + expect( + parseMcpHttpEndpoint({ MCP_ALLOWED_HOSTS: "rea.lan, 10.0.0.5" }), + ).toEqual({ + ok: true, + value: { + host: "127.0.0.1", + port: 0, + allowedHosts: ["rea.lan", "10.0.0.5"], + }, + }); + expect( + parseMcpHttpEndpoint( + { MCP_ALLOWED_HOSTS: "env.example" }, + { allowedHosts: mcpAllowedHostsTextSchema.parse("flag.example") }, + ), + ).toEqual({ + ok: true, + value: { host: "127.0.0.1", port: 0, allowedHosts: ["flag.example"] }, + }); + }); + + it("rejects an empty or blank allowlist by setting name without echoing it", () => { + for (const text of ["", " ", "rea.lan,,10.0.0.5"]) { + const result = parseMcpHttpEndpoint({ MCP_ALLOWED_HOSTS: text }); + expect(result.ok).toBe(false); + if (result.ok) continue; + expect(result.error.settings.map(({ setting }) => setting)).toEqual([ + "MCP_ALLOWED_HOSTS", + ]); + expect(JSON.stringify(projectAnalysisError(result.error))).toContain( + "MCP_ALLOWED_HOSTS", + ); + } + }); + + it("accepts --allowed-hosts with either flag spelling and rejects empty entries", () => { + expect( + parseMcpHttpArguments(["--http", "--allowed-hosts=rea.lan, 10.0.0.5"]), + ).toEqual({ ok: true, allowedHosts: ["rea.lan", "10.0.0.5"] }); + expect( + parseMcpHttpArguments(["--http", "--allowed-hosts", "rea.lan"]), + ).toEqual({ ok: true, allowedHosts: ["rea.lan"] }); + for (const text of ["", " ", "rea.lan,,10.0.0.5"]) + expect( + parseMcpHttpArguments(["--http", `--allowed-hosts=${text}`]), + ).toMatchObject({ + ok: false, + message: expect.stringContaining("--allowed-hosts"), + }); + }); + + it("lists --allowed-hosts and MCP_ALLOWED_HOSTS in the rendered help", () => { + const help = renderMcpHttpHelp(); + expect(help).toContain("--allowed-hosts"); + expect(help).toContain("MCP_ALLOWED_HOSTS"); + }); +});