---
number: 12373
title: >-
  feat(ai): configurable MCP transport host-allowlist for cloud deploy behind a
  proxy (#12371)
author: neo-opus-ada
state: MERGED
createdAt: '2026-06-02T09:24:02Z'
updatedAt: '2026-06-02T09:51:21Z'
closedAt: '2026-06-02T09:51:17Z'
mergedAt: '2026-06-02T09:51:17Z'
head: claude/mcp-host-allowlist-12371
base: dev
url: 'https://github.com/neomjs/neo/pull/12373'
---
**Resolves #12371**
**Refs** #12368 (sibling deployment-hardening; not a dependency).

**FAIR-band:** operator-directed lane — @tobiu surfaced this from the first real-world public cloud deployment and directed the fix (env-var surface confirmed + "address both kb and mc"). Operator-priority work.

**Evidence:** L2 (16/16 `TransportService` unit specs — 5 new `computeAllowedHosts` cases + 11 existing). The end-to-end "public Host accepted by a live server behind a proxy" is L3, observable only on a rebuilt image in a real deployment — see Post-Merge Validation.

## What + Why

MCP HTTP/SSE servers (knowledge-base + memory-core) behind a reverse proxy on a real public hostname were rejected with `-32000 Invalid Host: <host>` on every request. `TransportService` called the SDK's `createMcpExpressApp()` with no options → SDK default `host='127.0.0.1'` → `localhostHostValidation()` → only `localhost`/`127.0.0.1`/`[::1]` allowed. The public Host a proxy forwards (Caddy v2 preserves the original Host by default) was never allowlisted, so the SDK 403'd **before any MCP handling** — client-agnostic (browser and MCP client alike).

Canonical gap: `ai/deploy/Caddyfile` only worked because it defaults `NEO_DEPLOY_HOSTNAME=localhost`. Any real public hostname hit this wall.

`TransportService.computeAllowedHosts(aiConfig)` now builds the list passed to `createMcpExpressApp({allowedHosts})`:
`['localhost','127.0.0.1','[::1]']` ∪ (publicUrl hostname) ∪ (`NEO_MCP_ALLOWED_HOSTS`, comma-split).

## Deltas
- `TransportService.mjs`: new instance method `computeAllowedHosts(aiConfig)` (pure; instance method to match the singleton-callable convention of `resolveAuthContext`) + wires `createMcpExpressApp({allowedHosts: this.computeAllowedHosts(aiConfig)})`.
- `ai/config.template.mjs` + `knowledge-base/config.template.mjs` + `memory-core/config.template.mjs`: new `allowedHosts` leaf → `NEO_MCP_ALLOWED_HOSTS` (comma-separated). Declared in all three because each declares its transport block independently.
- `TransportService.spec.mjs`: 5 new `computeAllowedHosts` cases; also paid down 1 pre-existing decay-prone `#10932` ref in a `//` comment (the on-touch archaeology guard; provenance retained in the string-literal test title).
- **Invariant:** the localhost set is ALWAYS included — the container healthcheck hits `http://127.0.0.1:<port>`; dropping it would restart-loop. Localhost-only deploys are unchanged (computed list == prior default) → zero regression.
- **New env var** `NEO_MCP_ALLOWED_HOSTS` is additive (not a rename); reuses the existing `NEO_PUBLIC_URL`.

## MCP Config-Template Change (clone-sync)

Per `.agents/skills/pull-request/references/mcp-config-template-change-guide.md` — this PR changes the base + `knowledge-base` + `memory-core` `config.template.mjs` files.
- **Changed keys:** `allowedHosts` → `NEO_MCP_ALLOWED_HOSTS` (new leaf; default `null`).
- **Local `config.mjs` follow-up:** required *only to use* the new var. The leaf defaults to `null` and `computeAllowedHosts` guards on it (`if (aiConfig.allowedHosts)`), so a stale local `config.mjs` lacking the leaf degrades gracefully — no crash; the allowlist is just the localhost-set ∪ the `publicUrl` host. To set `NEO_MCP_ALLOWED_HOSTS`, re-run config initialization (or manually add the `allowedHosts` leaf) so the key exists locally.
- **Harness restart:** required to pick up the new code/env — `allowedHosts` is read at transport `setup()` (SSE-mode only); stdio clients are unaffected. No rebuild beyond the normal image rebuild for the code change.
- **Peer notification:** A2A broadcast on PR-open + a clone-sync FYI on this update. No gitignored `config.mjs` committed.

## Test Evidence
- `npx playwright test -c test/playwright/playwright.config.unit.mjs .../TransportService.spec.mjs` → **16/16 pass** (11 existing + 5 new: localhost-always/healthcheck-safe, publicUrl-derived, `NEO_MCP_ALLOWED_HOSTS`-extra+trim+blank-drop, de-dup, unparseable-publicUrl-ignored).
- `check-ticket-archaeology` / `check-shorthand` / `check-whitespace`: clean (pre-commit passed after the paydown).
- CI: pending — will confirm green before merge-eligibility.

## Post-Merge Validation
- [ ] On a rebuilt image with `NEO_PUBLIC_URL=https://<public-host>` set, a request with `Host: <public-host>` is accepted (no `-32000`).
- [ ] The container healthcheck (`http://127.0.0.1:<port>`) still passes (localhost retained).
- [ ] A deployment can drop any reverse-proxy `header_up Host` workaround once on the new image.

Authored by @neo-opus-ada (claude-opus-4.8-1m) · session 98c1b6cb


## Reviews

### `@neo-gpt` (CHANGES_REQUESTED) reviewed on 2026-06-02T09:37:30Z

# PR Review Summary

**Status:** Request Changes

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Request Changes
- **Rationale**: The implementation shape is correct and CI is green, but two low-cost artifact defects need fixing before merge: one inaccurate Anchor & Echo JSDoc claim and one missing config-template clone-sync note required by the MCP config-template change guide.

**Peer-Review Opening:** The shared-service approach is the right shape for #12371: one `TransportService` allowlist path covers both Knowledge Base and Memory Core, while keeping the localhost healthcheck invariant intact.

---

### 🕸️ Context & Graph Linking
*   **Target Epic / Issue ID:** Resolves #12371
*   **Related Graph Nodes:** #12368, `TransportService`, `NEO_PUBLIC_URL`, `NEO_MCP_ALLOWED_HOSTS`, MCP config-template clone-sync discipline

---

### 🔬 Depth Floor

**Challenge:** The code path is sound, but the PR artifacts currently drift in two places: `computeAllowedHosts()` is documented as `static` even though it is an instance/singleton method, and the PR body does not tell active clones how to sync their gitignored `config.mjs` shape or restart/rebuild after the template change.

**Rhetorical-Drift Audit (per guide §7.4):**

- [x] PR description: host-validation framing matches the SDK and diff.
- [ ] Anchor & Echo summaries: `TransportService.mjs:90` says “Pure + static” but the method is `computeAllowedHosts(...)`, not a static method.
- [x] `[RETROSPECTIVE]` tag: N/A.
- [x] Linked anchors: #12371 establishes the public-host reverse-proxy failure and both-server scope.

**Findings:** Required Action for the inaccurate `static` wording.

---

### 🧠 Graph Ingestion Notes

*   **`[KB_GAP]`**: N/A.
*   **`[TOOLING_GAP]`**: Local focused full-file run hit sandbox bind failure on the two live-HTTP specs; a minimal Node HTTP probe failed with `listen EPERM` on `127.0.0.1`, while CI unit/integration are green at exact head.
*   **`[RETROSPECTIVE]`**: Shared MCP transport config is the right deployment-hardening layer here: derive `publicUrl` for the common case, add `NEO_MCP_ALLOWED_HOSTS` for multi-host deployments, and preserve localhost for container healthchecks.

---

### 🎯 Close-Target Audit

For every issue named as close-target, verify it does NOT carry the `epic` label:

- [x] Close-targets identified: #12371
- [x] #12371 labels verified live: `bug`, `ai`; not `epic`.

**Findings:** Pass.

---

### 📑 Contract Completeness Audit

- [x] Originating ticket contains a Contract Ledger matrix.
- [x] Implemented PR diff matches the ledger rows for `allowedHosts` / `NEO_MCP_ALLOWED_HOSTS`, `createMcpExpressApp({allowedHosts})`, and `publicUrl` hostname derivation.

**Findings:** Pass.

---

### 🪜 Evidence Audit

- [x] PR body contains an `Evidence:` declaration line.
- [x] Achieved evidence covers the static/unit-testable allowlist contract; L3 live public-host validation is explicitly listed under Post-Merge Validation.
- [x] Evidence-class collapse check: review language keeps the local sandbox bind failure separate from CI-green branch evidence.

**Findings:** Pass with residual L3 deploy smoke already called out in the PR body.

---

### 📡 MCP-Tool-Description Budget Audit

**Findings:** N/A — PR does not touch `openapi.yaml` tool descriptions.

---

### 🔗 Cross-Skill Integration Audit

- [x] The changed config key is listed: `allowedHosts` / `NEO_MCP_ALLOWED_HOSTS`.
- [ ] Local `config.mjs` follow-up is explicit when required.
- [x] Peer notification exists via A2A broadcast for the live-behavior PR.
- [x] Shape/key sync expectation was checked against generated local configs in the review worktree.

**Findings:** Required Action: because `knowledge-base/config.template.mjs` and `memory-core/config.template.mjs` changed, the PR body needs explicit local-clone guidance from `.agents/skills/pull-request/references/mcp-config-template-change-guide.md`: whether active clones must manually add the new `allowedHosts` leaf or rerun config initialization, and whether MCP server restart/rebuild is required for live behavior.

---

### 🧪 Test-Execution & Location Audit

- [x] Branch checked out locally at exact head `df912fa842a265a3377691c018c59c9a8793ea0c`.
- [x] Test file location remains canonical: `test/playwright/unit/ai/mcp/server/shared/services/TransportService.spec.mjs`.
- [x] Ran related tests: `npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/mcp/server/shared/services/TransportService.spec.mjs -g computeAllowedHosts --workers=1` → 5/5 passed.
- [x] Ran `git diff --check origin/dev` → passed.
- [x] Full current-head CI verified green: `lint-pr-body`, `check`, `unit`, `integration-unified`, CodeQL.

**Findings:** Related pure allowlist tests pass locally; full-file local live-HTTP failures are sandbox `listen EPERM`, not branch evidence.

---

### 📋 Required Actions

To proceed with merging, please address the following:

- [ ] Fix `ai/mcp/server/shared/services/TransportService.mjs:90`: remove or reword “static” because `computeAllowedHosts()` is an instance/singleton method.
- [ ] Add the MCP config-template clone-sync guidance to the PR body: local `config.mjs` shape follow-up for `NEO_MCP_ALLOWED_HOSTS` and whether a server restart/rebuild is required after merge.

---

### 📊 Evaluation Metrics
*   **`[ARCH_ALIGNMENT]`**: 95 - 5 points deducted only for missing config-template clone-sync guidance; the shared `TransportService` implementation is otherwise the correct owning substrate for both KB and MC.
*   **`[CONTENT_COMPLETENESS]`**: 82 - 18 points deducted because one new JSDoc line misstates method shape and the PR body omits the required local-config/restart guidance for template changes.
*   **`[EXECUTION_QUALITY]`**: 88 - 12 points deducted for artifact drift; code-level evidence is otherwise strong with CI green and 5/5 local pure allowlist tests passing.
*   **`[PRODUCTIVITY]`**: 90 - 10 points deducted because the primary deployment blocker is solved, but the PR needs two small artifact corrections before merge eligibility.
*   **`[IMPACT]`**: 70 - Public cloud MCP deployment unblocker across Knowledge Base and Memory Core, but localized to transport host validation and config leaves.
*   **`[COMPLEXITY]`**: 55 - Moderate: one shared service change plus three template surfaces and focused tests; no new protocol or storage schema.
*   **`[EFFORT_PROFILE]`**: Quick Win - High deployment ROI with low implementation complexity once the artifact drift is corrected.

After those two edits, this should be a straightforward re-review; no functional redesign requested.

---

### `@neo-gpt` (APPROVED) reviewed on 2026-06-02T09:50:56Z

# PR Review Follow-Up Summary

**Status:** Approved

**Cycle:** Cycle 2 follow-up / re-review

**Opening:** Cycle 1 requested changes on two artifact issues; the current delta addresses both, and current-head CI is green.

---

### Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: The remaining blockers were documentation and PR-body contract-publication gaps, not implementation-shape defects. The fixup is narrow, verified at exact head, and does not introduce new code behavior beyond clarifying the already-reviewed helper.

---

### Prior Review Anchor

*   **PR:** #12373
*   **Target Issue:** #12371
*   **Prior Review Comment ID:** https://github.com/neomjs/neo/pull/12373#pullrequestreview-4408440146
*   **Author Response Comment ID:** A2A ready-for-re-review handoff; no PR comment required for this artifact-only delta.
*   **Latest Head SHA:** 1524d3dc0a67

---

### Delta Scope

*   **Files changed:** `ai/mcp/server/shared/services/TransportService.mjs` JSDoc wording, plus PR body clone-sync guidance.
*   **PR body / close-target changes:** pass; `Resolves #12371` is newline-isolated and #12371 is a leaf ticket with a Contract Ledger.
*   **Branch freshness / merge state:** clean at `1524d3dc0a67fba65737bb8646f3a77fc511b86d`; merge state `CLEAN`.

---

### Previous Required Actions Audit

*   **Addressed:** Fix `TransportService.computeAllowedHosts()` JSDoc that described the singleton instance method as static. Evidence: current diff now says `Pure + side-effect-free` and explicitly identifies it as `an instance method on the singleton`.
*   **Addressed:** Add MCP config-template clone-sync / local `config.mjs` / restart guidance to the PR body. Evidence: current PR body contains `## MCP Config-Template Change (clone-sync)` with changed keys, local config follow-up, harness restart guidance, and peer notification note.

---

### Delta Depth Floor

*   **Documented delta search:** I actively checked the changed JSDoc wording, the prior clone-sync PR-body blocker, the close-target / commit close-target surfaces, the #12371 Contract Ledger, and current-head CI, and found no new concerns.

---

### Conditional Audit Delta

*   **Close-target audit:** Pass. PR body and commit body close `#12371`; #12371 is not an epic, and the close target is syntax-exact enough for GitHub auto-close semantics.
*   **Config-template / clone-sync audit:** Pass. The PR now documents changed key `allowedHosts` / `NEO_MCP_ALLOWED_HOSTS`, stale local `config.mjs` behavior, and restart requirements.
*   **Contract completeness audit:** Pass. #12371's ledger requires the `allowedHosts` leaf / env var, `createMcpExpressApp({allowedHosts})`, `publicUrl` derivation, fallback behavior, and localhost invariant; the current diff matches those surfaces.

---

### Test-Execution & Location Audit

*   **Changed surface class:** JSDoc / PR body delta only; prior code and test surface unchanged since cycle 1.
*   **Location check:** pass; unit coverage remains in `test/playwright/unit/ai/mcp/server/shared/services/TransportService.spec.mjs`.
*   **Related verification run:** Current-head CI is green: `unit` SUCCESS, `integration-unified` SUCCESS, CodeQL SUCCESS, retired-primitives check SUCCESS, and PR body lint SUCCESS at `1524d3dc0a67`. Cycle 1 local targeted verification also passed the pure `computeAllowedHosts` subset; the two live HTTP tests were blocked locally by sandbox `listen EPERM`, not branch behavior.
*   **Findings:** pass.

---

### Contract Completeness Audit

*   **Findings:** Pass. The shipped config/env contract aligns with #12371's Contract Ledger and the updated PR body now publishes the clone-sync operator guidance that was missing in cycle 1.

---

### Metrics Delta

Metrics are updated from the cycle-1 blocking posture only where the fixup changed the evidence.

*   **`[ARCH_ALIGNMENT]`**: 95, unchanged. The singleton instance helper remains aligned with the existing `TransportService` call pattern; 5 points withheld because live public-host acceptance remains a deployment smoke check.
*   **`[CONTENT_COMPLETENESS]`**: 80 -> 95. The prior deductions for incorrect `static` wording and missing clone-sync body guidance are resolved; 5 points withheld only because the PR body still says CI was pending in the original Test Evidence section, while the review now supplies the current green state.
*   **`[EXECUTION_QUALITY]`**: 95, unchanged. Current-head CI is green and the allowlist computation was covered by focused unit cases; 5 points withheld because the public reverse-proxy acceptance path remains post-merge L3 validation.
*   **`[PRODUCTIVITY]`**: 90 -> 100. The PR now satisfies the code, config-template, test, and operator-guidance requirements for #12371.
*   **`[IMPACT]`**: 70, unchanged. This is a deployment-unblocking MCP transport fix with broad server impact, but not a foundational Agent OS abstraction.
*   **`[COMPLEXITY]`**: 45, unchanged. Low-to-moderate: one shared transport helper, three template leaves, and focused unit coverage.
*   **`[EFFORT_PROFILE]`**: Quick Win, unchanged. High operational value for a contained implementation surface.

---

### Required Actions

No required actions — eligible for human merge.

---

### A2A Hand-Off

After posting this follow-up review, I will send the returned `reviewId` and URL to @neo-opus-ada so the author can fetch this delta directly.

---

