morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #7386] [Bug] MCP HTTP array-option help accepts repeated host flags but drops earlier values #4392

New issue

Closed

opened 2026-10-31 00:49:27 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-31 00:49:27 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 27, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/7386

Before submitting

I searched the open and closed issues for a related report.
I removed credentials, tokens, private paths, and sensitive target data.

Area

MCP or agent integration

Observed behavior

The HTTP entry's rendered help advertises --allowed-hosts <array>. Passing --allowed-hosts a.example --allowed-hosts b.example to its production argument parser succeeds but retains only b.example. In a local production-listener replay using that parsed endpoint, a request with Host a.example received HTTP 403 with Invalid Host: a.example, while Host b.example reached the fixture route and received 200. A single --allowed-hosts a.example,b.example preserved both names and both requests received 200. This was a source-level production-boundary replay; the packaged CLI and a real MCP client were not launched.

Reproduction

Use Node 24.22.1 and a source checkout at the revision below, with locked dependencies installed using npm ci. From the repository root, run this self-contained probe. It creates an in-memory MCP fixture, opens one ephemeral listener at a time, sends requests only to the local machine, and closes each listener before proceeding. The example domain names are Host header values and require no DNS setup.

node --input-type=module <<'JS'
import { importVerifierSource } from './scripts/lib/verifier-source-loader.mjs';
import { request } from 'node:http';
const source = path => importVerifierSource(path, new URL('./', import.meta.url));
const { parseMcpHttpArguments, renderMcpHttpHelp } =
  await source('./src/main/mcpHttpArguments.ts');
const { parseMcpHttpEndpoint } = await source('./src/config/mcpHttpEndpoint.ts');
const { serveStreamableHttp } =
  await source('./src/server/http/serveStreamableHttp.ts');
const { RequestTrackingMcpServer } =
  await source('./src/server/RequestTrackingMcpServer.ts');
const get = (url, host) => new Promise((resolve, reject) => {
  const req = request(url, { headers: { host }, agent: false }, res => {
    res.resume();
    res.on('end', () => resolve(res.statusCode));
    res.on('error', reject);
  });
  req.on('error', reject);
  req.end();
});
console.log(renderMcpHttpHelp().split('\n').find(x => x.includes('--allowed-hosts')));
for (const flags of [
  ['--allowed-hosts', 'a.example', '--allowed-hosts', 'b.example'],
  ['--allowed-hosts', 'a.example,b.example'],
]) {
  const parsed = parseMcpHttpArguments(['--http', '--host', '0.0.0.0', ...flags]);
  if (!parsed.ok) throw new Error(JSON.stringify(parsed));
  const endpoint = parseMcpHttpEndpoint({}, parsed);
  if (!endpoint.ok) throw endpoint.error;
  console.log(JSON.stringify(parsed.allowedHosts));
  const served = await serveStreamableHttp({
    ...endpoint.value,
    createMcpServer: () => new RequestTrackingMcpServer({ name: 'probe', version: '1' }),
    routes: [{ method: 'GET', path: '/probe', handle: () => Response.json({ allowed: true }) }],
  });
  try {
    const url = new URL(served.url);
    url.hostname = '127.0.0.1';
    url.pathname = '/probe';
    for (const host of ['a.example', 'b.example'])
      console.log(host, await get(url, host));
  } finally {
    await served.close();
  }
}
JS

The repeated form prints only ["b.example"] and returns 403 for the earlier name. The comma-separated control prints both names and returns 200 for each. This behavior reproduced in the local replay; a separate focused source test also confirmed the help/parser discrepancy.

Expected behavior

The accepted input should agree with the array type shown by the command's own help. Repeating an array-valued option should preserve its elements, or the entry should reject repetition and explain the supported comma-separated form. Silent acceptance followed by loss of earlier values gives the operator no indication that a declared name will be refused. Expected source: src/main/mcpHttpArguments.ts:28 declares the help option as an array. The guide explicitly specifies the working comma-separated syntax at docs/cli.md:756; it does not document repetition, so this report concerns the mismatch with the help rather than a failure of that guide example.

Impact and scope

P2: an operator relying on the array-shaped help can configure multiple names without a parsing error, then find that clients using earlier names cannot reach the HTTP endpoint. The impact is bounded and reversible. Passing all names in one comma-separated option is a verified practical workaround that preserves the intended allowlist. The observed behavior removes names; no undeclared name was admitted. The replay covered two valid names on a non-loopback bind, with local requests. Other platforms, browser Origin requests, and packaged-client integration were not exercised.

REA version

6.8.0; source revision f8f7ac96de

Runtime and platform

Node v24.22.1; macOS 27.0.1; arm64

Target and artifact context

No analysis target. MCP HTTP endpoint configuration with an in-memory server fixture and a local GET route.

Interface

CLI

Earliest failure stage

Request parsing or validation

Relevant output or evidence

The production-boundary probe produced this relevant output:

  --allowed-hosts <array>  Host/Origin names a non-loopback bind may serve; default MCP_ALLOWED_HOSTS, else the bind is refused
["b.example"]
a.example 403
b.example 200
["a.example","b.example"]
a.example 200
b.example 200

Failure source: src/main/mcpHttpArguments.ts:103 overwrites earlier flag values, and src/main/mcpHttpArguments.ts:114 retrieves the final value only. Reachability: scripts/rea.mjs:67 invokes the HTTP entry, src/main.ts:164 parses these arguments, and src/main.ts:178 supplies the resulting list to endpoint resolution. The production HTTP adapter passes that endpoint to the listener at src/main.ts:59. The non-loopback listener consumes the list at src/server/http/serveStreamableHttp.ts:565 and checks requests against it at src/server/http/serveStreamableHttp.ts:493. These locators establish the CLI path; the runtime replay exercised the actual parser, endpoint resolver, and listener with the fixture above.

Typed outcome or recovery expected

Either retain both requested names or return an argument error explaining that the option requires a single comma-separated list. The help should state the accepted syntax explicitly.

Additional context

Related: #7172 (merged) made --allowed-hosts required for non-loopback HTTP binds. No security-boundary bypass was identified: the defect only removes declared names.

Originally created by @coygeek on GitHub (Oct 27, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/7386 ### Before submitting - [x] I searched the open and closed issues for a related report. - [x] I removed credentials, tokens, private paths, and sensitive target data. ### Area MCP or agent integration ### Observed behavior The HTTP entry's rendered help advertises `--allowed-hosts <array>`. Passing `--allowed-hosts a.example --allowed-hosts b.example` to its production argument parser succeeds but retains only `b.example`. In a local production-listener replay using that parsed endpoint, a request with Host `a.example` received HTTP 403 with `Invalid Host: a.example`, while Host `b.example` reached the fixture route and received 200. A single `--allowed-hosts a.example,b.example` preserved both names and both requests received 200. This was a source-level production-boundary replay; the packaged CLI and a real MCP client were not launched. ### Reproduction Use Node 24.22.1 and a source checkout at the revision below, with locked dependencies installed using `npm ci`. From the repository root, run this self-contained probe. It creates an in-memory MCP fixture, opens one ephemeral listener at a time, sends requests only to the local machine, and closes each listener before proceeding. The example domain names are Host header values and require no DNS setup. ```sh node --input-type=module <<'JS' import { importVerifierSource } from './scripts/lib/verifier-source-loader.mjs'; import { request } from 'node:http'; const source = path => importVerifierSource(path, new URL('./', import.meta.url)); const { parseMcpHttpArguments, renderMcpHttpHelp } = await source('./src/main/mcpHttpArguments.ts'); const { parseMcpHttpEndpoint } = await source('./src/config/mcpHttpEndpoint.ts'); const { serveStreamableHttp } = await source('./src/server/http/serveStreamableHttp.ts'); const { RequestTrackingMcpServer } = await source('./src/server/RequestTrackingMcpServer.ts'); const get = (url, host) => new Promise((resolve, reject) => { const req = request(url, { headers: { host }, agent: false }, res => { res.resume(); res.on('end', () => resolve(res.statusCode)); res.on('error', reject); }); req.on('error', reject); req.end(); }); console.log(renderMcpHttpHelp().split('\n').find(x => x.includes('--allowed-hosts'))); for (const flags of [ ['--allowed-hosts', 'a.example', '--allowed-hosts', 'b.example'], ['--allowed-hosts', 'a.example,b.example'], ]) { const parsed = parseMcpHttpArguments(['--http', '--host', '0.0.0.0', ...flags]); if (!parsed.ok) throw new Error(JSON.stringify(parsed)); const endpoint = parseMcpHttpEndpoint({}, parsed); if (!endpoint.ok) throw endpoint.error; console.log(JSON.stringify(parsed.allowedHosts)); const served = await serveStreamableHttp({ ...endpoint.value, createMcpServer: () => new RequestTrackingMcpServer({ name: 'probe', version: '1' }), routes: [{ method: 'GET', path: '/probe', handle: () => Response.json({ allowed: true }) }], }); try { const url = new URL(served.url); url.hostname = '127.0.0.1'; url.pathname = '/probe'; for (const host of ['a.example', 'b.example']) console.log(host, await get(url, host)); } finally { await served.close(); } } JS ``` The repeated form prints only `["b.example"]` and returns 403 for the earlier name. The comma-separated control prints both names and returns 200 for each. This behavior reproduced in the local replay; a separate focused source test also confirmed the help/parser discrepancy. ### Expected behavior The accepted input should agree with the array type shown by the command's own help. Repeating an array-valued option should preserve its elements, or the entry should reject repetition and explain the supported comma-separated form. Silent acceptance followed by loss of earlier values gives the operator no indication that a declared name will be refused. Expected source: src/main/mcpHttpArguments.ts:28 declares the help option as an array. The guide explicitly specifies the working comma-separated syntax at docs/cli.md:756; it does not document repetition, so this report concerns the mismatch with the help rather than a failure of that guide example. ### Impact and scope P2: an operator relying on the array-shaped help can configure multiple names without a parsing error, then find that clients using earlier names cannot reach the HTTP endpoint. The impact is bounded and reversible. Passing all names in one comma-separated option is a verified practical workaround that preserves the intended allowlist. The observed behavior removes names; no undeclared name was admitted. The replay covered two valid names on a non-loopback bind, with local requests. Other platforms, browser Origin requests, and packaged-client integration were not exercised. ### REA version 6.8.0; source revision f8f7ac96de8a66a4ab9a23d87e04779bfcd11045 ### Runtime and platform Node v24.22.1; macOS 27.0.1; arm64 ### Target and artifact context No analysis target. MCP HTTP endpoint configuration with an in-memory server fixture and a local GET route. ### Interface CLI ### Earliest failure stage Request parsing or validation ### Relevant output or evidence The production-boundary probe produced this relevant output: ```text --allowed-hosts <array> Host/Origin names a non-loopback bind may serve; default MCP_ALLOWED_HOSTS, else the bind is refused ["b.example"] a.example 403 b.example 200 ["a.example","b.example"] a.example 200 b.example 200 ``` Failure source: src/main/mcpHttpArguments.ts:103 overwrites earlier flag values, and src/main/mcpHttpArguments.ts:114 retrieves the final value only. Reachability: scripts/rea.mjs:67 invokes the HTTP entry, src/main.ts:164 parses these arguments, and src/main.ts:178 supplies the resulting list to endpoint resolution. The production HTTP adapter passes that endpoint to the listener at src/main.ts:59. The non-loopback listener consumes the list at src/server/http/serveStreamableHttp.ts:565 and checks requests against it at src/server/http/serveStreamableHttp.ts:493. These locators establish the CLI path; the runtime replay exercised the actual parser, endpoint resolver, and listener with the fixture above. ### Typed outcome or recovery expected Either retain both requested names or return an argument error explaining that the option requires a single comma-separated list. The help should state the accepted syntax explicitly. ### Additional context Related: #7172 (merged) made `--allowed-hosts` required for non-loopback HTTP binds. No security-boundary bypass was identified: the defect only removes declared names.

bsakaguchi closed this issue

2026-10-31 00:49:27 +00:00

bsakaguchi commented

2026-10-31 00:49:27 +00:00

Author

Owner

@z2Ace0107 commented on GitHub (Oct 28, 2026):

I would like to take this one.

Root cause, located in this repository: parseMcpHttpArguments in
src/main/mcpHttpArguments.ts collects flag values into a
Map<string, string> and stores each occurrence with values.set(flag, value).
A repeated flag therefore overwrites the earlier one, so
--allowed-hosts a.example --allowed-hosts b.example keeps only
b.example. The comma-separated form survives because
mcpAllowedHostsTextSchema splits on commas, which is exactly the asymmetry the
report measured.

My plan is to accumulate repeated --allowed-hosts occurrences instead of
overwriting them, then feed the joined value through the existing
mcpAllowedHostsTextSchema so both spellings agree and the existing validation
and constraint message still apply unchanged. --host and --port stay
single-valued, and repeating them keeps its current behaviour rather than
silently accumulating, because their schemas declare scalars.

I also want to check the rendered help, since the report notes it advertises
--allowed-hosts <array> without saying the flag may be repeated. If the help
can state that, the fix should include it; otherwise I will say so explicitly in
the pull request rather than leave the two spellings looking equally supported.

Scope: the dispatcher argument parser and its help text. I am not changing the
allowed-host admission rules, the loopback policy, or what a non-loopback bind
may serve.

I have not started implementation yet; I will post the report's own
before/after measurements alongside my own evidence in the pull request.

@z2Ace0107 commented on GitHub (Oct 28, 2026): I would like to take this one. Root cause, located in this repository: `parseMcpHttpArguments` in `src/main/mcpHttpArguments.ts` collects flag values into a `Map<string, string>` and stores each occurrence with `values.set(flag, value)`. A repeated flag therefore overwrites the earlier one, so `--allowed-hosts a.example --allowed-hosts b.example` keeps only `b.example`. The comma-separated form survives because `mcpAllowedHostsTextSchema` splits on commas, which is exactly the asymmetry the report measured. My plan is to accumulate repeated `--allowed-hosts` occurrences instead of overwriting them, then feed the joined value through the existing `mcpAllowedHostsTextSchema` so both spellings agree and the existing validation and constraint message still apply unchanged. `--host` and `--port` stay single-valued, and repeating them keeps its current behaviour rather than silently accumulating, because their schemas declare scalars. I also want to check the rendered help, since the report notes it advertises `--allowed-hosts <array>` without saying the flag may be repeated. If the help can state that, the fix should include it; otherwise I will say so explicitly in the pull request rather than leave the two spellings looking equally supported. Scope: the dispatcher argument parser and its help text. I am not changing the allowed-host admission rules, the loopback policy, or what a non-loopback bind may serve. I have not started implementation yet; I will post the report's own before/after measurements alongside my own evidence in the pull request.

bsakaguchi referenced this issue

2026-10-31 00:51:13 +00:00

[GH-ISSUE #7681] [Bug] Session redirect rows omit the promised prior-hop response context #4603

bsakaguchi referenced this issue

2026-10-31 01:12:43 +00:00

[PR #4391] [CLOSED] fix(browser): bound observe_web_session windows to the Node.js timer limit #6211