Skip to content

Pin trust_proxy to a hop count so a forged X-Forwarded-For can't impersonate the allowlisted crawler - #148

Draft
jpr5 wants to merge 1 commit into
mainfrom
fix/trust-proxy-hop-count
Draft

Pin trust_proxy to a hop count so a forged X-Forwarded-For can't impersonate the allowlisted crawler#148
jpr5 wants to merge 1 commit into
mainfrom
fix/trust-proxy-hop-count

Conversation

@jpr5

@jpr5 jpr5 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

The problem

All three shipped deploy configs set trust_proxy: true:

deploy/copilotkit-docs.yaml:11
deploy/pathfinder-docs.yaml:11
deploy/aimock-docs.yaml:11

Express's boolean trust proxy = true does not mean "one proxy". It trusts every hop on the X-Forwarded-For chain and resolves req.ip to the leftmost entry — which is entirely client-supplied.

The same configs allowlist the Anthropic crawler IP to exempt it from the per-IP session cap:

allowlist: ["160.79.106.35"]

max_sessions_per_ip defaults to 20 (src/server.ts:4120), and the allowlist is checked against the resolved IP (src/ip-limiter.ts:271, [mcp] IP … on allowlist, bypassing session limit). So the two settings together mean any caller could send X-Forwarded-For: 160.79.106.35 and be attributed to the crawler — bypassing the per-IP session limiter outright, and poisoning query_log.client_ip (src/db/analytics.ts:371) with whatever value they chose.

src/ip-util.ts:23-33 already documents the hop count as the correct setting for a single-hop deployment. The shipped configs contradicted the code's own guidance.

Topology and the value chosen

trust_proxy: 1, not merely "something smaller than true".

Exactly one proxy sits in front of these containers:

  • railway.toml deploys the container directly; Dockerfile prod stage runs CMD ["node", "dist/cli.js", "serve"] — no sidecar, no in-container nginx/caddy/envoy (grepped: zero hits outside a prose mention).
  • Live response headers on the prod host show Railway's edge speaking straight to Express, with no CDN in front (no cf-ray, no second proxy signature):
$ curl -sS -D - https://mcp.pathfinder.copilotkit.dev/health
server: railway-hikari
x-railway-edge: sjc1
x-railway-request-id: AtWw3wwwSJeNMggMnPRhug
x-powered-by: Express
  • pathfinder.example.yaml states the value for this topology itself: <integer> — trust N hops (e.g. 1 for single-proxy Railway).

With a hop count, Express counts inward from the socket, so a client-forged XFF prefix is ignored — the edge appends the real peer to the right of anything the client sent. Note this is correct regardless of whether Railway's edge strips or appends client-supplied XFF, whereas true is only safe under the strip assumption that the old comment asserted without evidence.

RED — before the fix (trust_proxy: true)

Real server booted from source (npx tsx src/index.ts, NODE_ENV=production) with a config carrying the same allowlist: ["160.79.106.35"] and trust_proxy: true, and PATHFINDER_TRACE_CLAUDE_AI=1 so the trace middleware logs the resolved IP:

[startup] trust_proxy=true — honoring X-Forwarded-For (reverse proxy must strip/rewrite this header)
[startup] IP allowlist: 1 entry (bypasses session cap)
[startup] IP rate limit: 20 sessions/IP, TTL: 30m, unused TTL: 15m, global cap: 1000

Three POST /mcp initialize requests — forged multi-entry XFF, legitimate single hop, no XFF:

[trace] POST /mcp ip=160.79.106.35 ua=python-httpx/0.27 auth=none
[mcp] IP 160.79.106.35 on allowlist, bypassing session limit
[mcp] New session 238e87ca (1 active) [160.79.106.35]
[trace] POST /mcp ip=198.51.100.9 ua=python-httpx/0.27 auth=none
[mcp] New session e86080ee (2 active) [198.51.100.9]
[trace] POST /mcp ip=::ffff:127.0.0.1 ua=python-httpx/0.27 auth=none
[mcp] New session 2e263907 (3 active) [::ffff:127.0.0.1]

The first request sent X-Forwarded-For: 160.79.106.35, 203.0.113.7 — a forged allowlisted IP followed by the real peer as a fronting proxy would append it. The server attributed it to 160.79.106.35 and logged on allowlist, bypassing session limit. Full impersonation, limiter bypassed, and the session tagged with the forged IP.

GREEN — after the fix (trust_proxy: 1)

Same server, same three requests, byte-identical headers:

[startup] trust_proxy=1 — honoring X-Forwarded-For (reverse proxy must strip/rewrite this header)
[startup] IP allowlist: 1 entry (bypasses session cap)
[startup] IP rate limit: 20 sessions/IP, TTL: 30m, unused TTL: 15m, global cap: 1000

[trace] POST /mcp ip=203.0.113.7 ua=python-httpx/0.27 auth=none
[mcp] New session 080ee155 (1 active) [203.0.113.7]
[trace] POST /mcp ip=198.51.100.9 ua=python-httpx/0.27 auth=none
[mcp] New session 166e50f9 (2 active) [198.51.100.9]
[trace] POST /mcp ip=::ffff:127.0.0.1 ua=python-httpx/0.27 auth=none
[mcp] New session 163f960d (3 active) [::ffff:127.0.0.1]

Both halves hold:

  • Forgery neutralized — the forged 160.79.106.35 prefix is ignored; the request resolves to 203.0.113.7, the peer the proxy appended. The on allowlist, bypassing session limit line is gone, so the limiter now applies.
  • Legitimate proxy attribution intact — the single-hop X-Forwarded-For: 198.51.100.9 still resolves to 198.51.100.9, not to the proxy address. A fix that hardened forgery by collapsing every request onto the proxy IP would have broken rate-limit bucketing and analytics; this one doesn't.
  • No XFF still falls back to the socket peer.

Regression test

src/__tests__/deploy-trust-proxy.test.ts (18 tests) reads trust_proxy out of the shipped YAML rather than hard-coding it, so a revert to true fails the behavioral assertions and not just a shape check. Per deploy config it pins:

  1. forged multi-entry XFF → resolves to the proxy-appended peer, never the forged allowlisted IP
  2. legitimate single proxy hop → resolves to that IP
  3. no XFF → socket peer
  4. forged XFF does not win the allowlist session-cap bypass (cap 1 + shipped allowlist: second forged request gets 429, and getSessionCount("160.79.106.35") === 0)

Plus shape assertions that the value is a positive integer 1 and explicitly not true, and that the crawler IP is still on the allowlist so the forgery cases can't pass vacuously.

Mutation check

Reverted the production line (trust_proxy: 1true in deploy/copilotkit-docs.yaml) and re-ran — CAUGHT, 3 failures, including the vulnerability itself:

AssertionError: expected true not to be true            // shape
AssertionError: expected '160.79.106.35' to be '203.0.113.7'  // forgery wins
AssertionError: expected true to be false              // allowlist bypass fires
Test Files  1 failed (1)
     Tests  3 failed | 15 passed (18)

The 15 that still passed are the legitimate-hop and no-XFF cases, which are genuinely unaffected by the mutation — so the suite discriminates rather than failing wholesale.

No-op control: with trust_proxy: 1 restored, a comment-only edit to the same file → 18 passed (18). The CAUGHT verdict is attributable to the value, not to touching the file.

Gates

  • npx prettier --check "src/**/*.ts" — clean
  • npm run build — clean
  • npx tsc --noEmit -p tsconfig.scripts.json — clean
  • npm test — 3572 passed, 1 skipped (179 files)

Notes for the reviewer

  • scripts/atlas-harvest/SANDBOX.md documented the prod value as trust_proxy: true, which my change made stale. The one-token correction shortened a markdown table cell, so Prettier realigned the whole table — the 18-line diff there is alignment only, one token of real change. (CI's prettier job covers TS only, but the file was Prettier-clean on main and I kept it that way.)
  • pathfinder.example.yaml keeps its hardened trust_proxy: false default; only the allowlist note that pointed at true changed.
  • Deliberately out of scope, not touched: the analytics dashboard, scoring code, and docs/.

…rsonate the allowlisted crawler

All three shipped deploy configs set `trust_proxy: true`. Express's boolean
`trust proxy = true` does not mean "one proxy" — it trusts every hop on the
X-Forwarded-For chain and resolves `req.ip` to the LEFTMOST entry, which is
entirely client-supplied.

These deployments also allowlist the Anthropic crawler IP (160.79.106.35) to
exempt it from the per-IP session cap (default 20 sessions/IP). Combined, any
caller could send `X-Forwarded-For: 160.79.106.35` and be attributed to the
crawler: the per-IP session limiter was bypassed outright, and query_log
client_ip attribution was poisoned with whatever the caller chose.

Exactly one proxy fronts these containers — Railway's edge terminates TLS and
forwards to the Node process, with no CDN and no in-container reverse proxy —
so `trust_proxy: 1` is the correct depth, not merely a smaller one. With a hop
count Express counts inward from the socket, so a client-forged XFF prefix is
ignored because the edge appends the real peer to the right of it. Legitimate
single-hop proxy attribution is unchanged.

This is the setting src/ip-util.ts already recommended for single-hop
deployments; the shipped configs contradicted the code's own guidance.

Also corrects the two places that pointed operators at `true`:
pathfinder.example.yaml's allowlist note and the stale prod-value reference in
the Atlas sandbox doc.

Regression coverage in src/__tests__/deploy-trust-proxy.test.ts reads
trust_proxy out of the shipped YAML rather than hard-coding it, and pins the
resolved IP for a forged multi-entry XFF, a legitimate single hop, and no XFF
at all, plus the allowlist session-cap bypass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant