Skip to content

Commit 2a46cac

Browse files
ihor-sokoliukclaude
andcommitted
docs+test(http-security): clarify portless-Host rationale; deterministic 403 Host
Addresses PR #172 review: - Copilot: reword defaultAllowedHosts comment + CONFIGURATION.md to state bare hostnames are always kept (match a portless Host), not port-80-only. - Copilot: set an explicit disallowed Host in the mismatched-Host 403 test so it no longer relies on supertest's implicit ephemeral-port Host. - Codacy: reword the SECURITY.md rebinding bullet from an imperative 'must' to a descriptive match statement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent f89e797 commit 2a46cac

4 files changed

Lines changed: 7 additions & 5 deletions

File tree

CONFIGURATION.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ Opt-in security layer for when you expose the HTTP transport on a network. Defau
183183
| `MCP_HTTP_ALLOW_PRIVATE_URLS` | No | `false` | Allow `web_url_read` to fetch internal/private URLs, including hostnames that DNS-resolve to private/internal addresses. Private URL reads are blocked by default in all modes. |
184184
| `MCP_HTTP_EXPOSE_FULL_CONFIG` | No | `false` | Expose full config details in `/health` response (for debugging) |
185185

186-
`MCP_HTTP_ALLOWED_HOSTS` is compared against the raw `Host` header, which includes the port. The default already covers loopback access on the configured `MCP_HTTP_PORT` (`127.0.0.1:PORT`, `localhost:PORT`, `[::1]:PORT`) plus the bare hostnames for the port-80/443 case. When you set it explicitly, list the exact `Host` the client (or your reverse proxy) sends — e.g. `app.example.com` if the proxy forwards `Host: app.example.com` on 443, or `app.example.com:8443` if it forwards a port.
186+
`MCP_HTTP_ALLOWED_HOSTS` is compared against the raw `Host` header, which includes the port. The default already covers loopback access on the configured `MCP_HTTP_PORT` (`127.0.0.1:PORT`, `localhost:PORT`, `[::1]:PORT`) plus the bare hostnames, which match a portless `Host` — a client or reverse proxy that omits the port (as on ports 80/443). When you set it explicitly, list the exact `Host` the client (or your reverse proxy) sends — e.g. `app.example.com` if the proxy forwards `Host: app.example.com` on 443, or `app.example.com:8443` if it forwards a port.
187187

188188
## URL Reader Security
189189

SECURITY.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ Hardened mode enforces:
7070

7171
- **Bearer token authentication** on every request (`Authorization: Bearer <token>`)
7272
- **CORS origin allowlist** — requests from unlisted origins are rejected
73-
- **DNS rebinding protection** — the `Host` header is validated against `MCP_HTTP_ALLOWED_HOSTS`. The default allows loopback access on the configured port (`127.0.0.1`, `localhost`, `[::1]` and their `:PORT` forms). A custom list is matched exactly against `Host`, so each entry must include the port if the client or proxy sends one (e.g. `app.example.com:8443`).
73+
- **DNS rebinding protection** — the `Host` header is validated against `MCP_HTTP_ALLOWED_HOSTS`. The default allows loopback access on the configured port (`127.0.0.1`, `localhost`, `[::1]` and their `:PORT` forms). A custom list is matched exactly against `Host`: an entry matches only when it carries the same port the client or proxy sends (e.g. `app.example.com:8443`).
7474

7575
`MCP_HTTP_HARDEN=true` will fail to start if `MCP_HTTP_AUTH_TOKEN` or `MCP_HTTP_ALLOWED_ORIGINS` are missing.
7676

__tests__/integration/http-server.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,11 +361,12 @@ async function runTests() {
361361
envManager.set('MCP_HTTP_HARDEN', 'true');
362362
envManager.set('MCP_HTTP_AUTH_TOKEN', 'secret-token');
363363
envManager.set('MCP_HTTP_ALLOWED_ORIGINS', 'https://app.example.com');
364-
envManager.set('MCP_HTTP_ALLOWED_HOSTS', 'allowed.example.com'); // supertest sends 127.0.0.1:<ephemeral>, which will not match
364+
envManager.set('MCP_HTTP_ALLOWED_HOSTS', 'allowed.example.com');
365365

366366
const app = await createHttpServer(() => createTestMcpServer(), 3000);
367367
const res = await request(app)
368368
.post('/mcp')
369+
.set('Host', 'evil.example.com') // explicit disallowed Host so the 403 is deterministic across supertest/node versions
369370
.set('Origin', 'https://app.example.com')
370371
.set('Authorization', 'Bearer secret-token')
371372
.set('Content-Type', 'application/json')

src/http-security.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,9 @@ function parseTrustProxy(value: string | undefined): boolean | number | string {
4141
// Default DNS-rebinding allowlist when MCP_HTTP_ALLOWED_HOSTS is unset. The SDK
4242
// transport matches the raw Host header (port included) with exact Array.includes,
4343
// so the bind port must be baked into the loopback defaults or hardened mode 403s
44-
// every request on any non-80 port (BUG-012). Bare hostnames are kept for the
45-
// port-80/default-port case; [::1] mirrors the SDK's own localhost default.
44+
// every request on any non-80 port (BUG-012). The bare hostnames are always kept
45+
// too: they match a portless Host — a client or reverse proxy that omits the port
46+
// (as on default ports 80/443). [::1] mirrors the SDK's own localhost default.
4647
function defaultAllowedHosts(port?: number): string[] {
4748
const hosts = ["127.0.0.1", "localhost", "[::1]"];
4849
if (port !== undefined) {

0 commit comments

Comments
 (0)