fix(server): only believe X-Forwarded-Proto from a trusted proxy (#474)

[edspencer]

Closes #474.

The bug

isSecureRequest() honoured X-Forwarded-Proto: https from any peer with no
trusted-proxy check, so the /mcp plaintext guard could be turned off by the
caller — with a header copy-pasted out of a smoke-test recipe.

Severity — calibrated deliberately

Not an auth bypass, and this PR does not claim to be one. A valid bearer
token is still required; spoofing the header buys an attacker nothing they could
not already do. The guard exists so an operator doesn't ship their own token
in clear over a network believing they were covered. What was broken is that the
one signal the guard relied on was set by the party it was supposed to protect
the operator from — including themselves.

So the goal was: make the guard rest on something the caller cannot set, without
turning a footgun-preventer into a thing that 403s working deployments.

The fix

X-Forwarded-Proto is believed only when the immediate peer — the socket
address, not any header — is a trusted proxy.

evaluateTransportSecurity(req, cfg) // → { secure, via, peer, assumedProxy }
//   tls        real TLS at this process
//   forwarded  a TRUSTED peer said https
//   loopback   client is on this host — nothing hit a wire
//   spoofable  an https claim from a peer that is NOT a trusted proxy → 403
//   plaintext  no claim at all → 403

New config managementApi.trustedProxies (YAML) / PADDOCK_MANAGEMENT_TRUSTED_PROXIES
(env wins, as everywhere else): IPs, CIDRs, the proxy-addr presets
loopback / linklocal / uniquelocal, or the words none / all. An
unparseable entry is dropped with a config error rather than failing the boot —
that direction can only make the guard stricter.

Default: loopback, linklocal, uniquelocal. In every recipe the TLS
terminator sits on the host or on a private container/pod network, so this keeps
auth-basic/ (Caddy + nginx sidecars on a Compose bridge), the k8s ingress, and
the docker/ recipe working untouched — while a public peer can no longer
switch the guard off, which it could before.

The default's honest limit is that it cannot tell your proxy at 172.18.0.5 from
a laptop at 192.168.1.50 — nothing in an IP packet says which one is a proxy.
So when the verdict rests on the default list rather than a proxy you named,
the server logs a one-per-peer warning naming the address and pointing at the
setting. Naming your terminator is what upgrades this from footgun-preventer to
control; none is the strict posture.

What I rejected, and why

• Fastify's trustProxy (what the issue suggests). It does not do what the
  issue expects in the version we pin: in fastify@4.28.1
  (lib/request.js:134), the trust-proxy protocol getter returns
  x-forwarded-proto whenever the header is present — the compiled trust
  function gates req.ip/req.ips only, never protocol. Building the guard on
  req.protocol would have reproduced the exact bug while looking fixed. It is
  also a server-wide switch, and I did not want a spoofable req.protocol/
  req.hostname sitting in the app for the next caller to trust. I do use
  proxy-addr directly (fastify's own CIDR matcher; now an explicit dependency)
  rather than hand-rolling IPv6/IPv4-mapped range logic in a security check.
• Treating the Docker bridge gateway (172.17.0.1) as loopback-equivalent.
  The issue asks for this to be decided on purpose: no. A request from
  another host to a 0.0.0.0-published port is SNAT'd to that same address, so
  "peer is the gateway" proves nothing about where the traffic came from. The
  documented smoke test now goes in through the container's own loopback
  (docker compose exec paddock curl http://127.0.0.1:4000/mcp …), which is a
  real loopback peer and needs no header at all — strictly better than the
  workaround paddock-deploy was about to bless.
• Defaulting to none (trust nothing). Correct on paper, and it would 403
  every existing sidecar deployment on upgrade for a low-severity,
  operator-protecting guard. The trade the issue explicitly asks for is the other
  way round.
• Inferring trust from Host matching publicUrl. Cute, and it separates
  "real proxy" from "copy-pasted curl" — but Host is equally client-settable,
  and it fails exactly when someone curls the public hostname over http, which is
  the footgun. Security-by-plausibility; dropped.
• A global PADDOCK_TRUSTED_PROXIES. Kept it namespaced to managementApi
  because that is the only decision it makes today. PADDOCK_AUTH_MODE=trusted-header
  still trusts its identity header from any peer by design — that mode assumes
  the proxy is the sole ingress, which is a network posture, not a header check.
  Documented rather than silently widened.

Threat model, stated plainly

• Protects against: an operator (or a script) disabling the plaintext guard by
  asserting a scheme the connection doesn't have, from anywhere but a peer the
  operator has vouched for — and, under any configuration, from a public peer.
• Does NOT protect against: anything token-related. A stolen token still
  works; a valid token was always required. It does not authenticate the proxy
  (any host at a trusted address is trusted — this is IP trust, not mTLS), and
  under the default list any private-network peer can still present a forwarded
  scheme. It is a transport-hygiene guard, not an access control.
• Residual risk: the default posture is compatibility-shaped, so a LAN peer
  can still assert https and leak a token in clear on that LAN. That is now
  visible (a warning naming the peer) and fixable in one line (name the
  proxy, or none) rather than silent.

Verification

Unit + integration (packages/server/test/unit/trusted-proxy.test.ts, and a new
/mcp — transport guard block in mcp-gate.test.ts driving real peer addresses
through inject({ remoteAddress })). Full server suite green.

Live, on a real server with real peers (pm-hosted dev instance, throwaway
token, Caddy in front). This box's TLS terminator is a genuinely separate host —
192.168.1.33 — which made the matrix honest:

peer	header	trustedProxies	result
127.0.0.1 (loopback)	—	default	200 MCP handshake
127.0.0.1	https	default	200 (loopback, not the header)
192.168.1.80 (off-host shape)	—	default	403 insecure_transport
192.168.1.80	https	default	200 + warn naming the peer
192.168.1.80	http	default	403
via Caddy 192.168.1.33 (real TLS)	https	default	200
192.168.1.80	https	[192.168.1.33]	403 — spoof refused
via Caddy 192.168.1.33	https	[192.168.1.33]	200 — legit proxy admitted
192.168.1.80	https	none	403
127.0.0.1	—	none	200 (loopback still loopback)
via Caddy	https	none	403 (as configured)

Server logs confirmed the reasons ("via":"spoofable", "via":"plaintext") and
that the default-list warning fires exactly once per distinct peer.

Not exercised live (covered by unit tests only, and by the identical code
path the refused rows above take): a peer with a public source address, and
a peer inside a Docker container. This box has no public source address to send
from, and its Docker daemon is remote, so a container there cannot reach the
test server. isTrustedProxy("203.0.113.9", default) === false is asserted in
trusted-proxy.test.ts; a public peer reaches the same via: "spoofable" → 403
branch that 192.168.1.80 demonstrably took under an explicit list.

Docs

docs/CONFIGURATION.md gains a Trusted proxies section: the value table, the
recommended posture, the Docker-NAT explanation, the in-container smoke-test
command, and the explicit note about trusted-header auth being a separate
question. Changeset included (patch).

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying paddock with    Cloudflare Pages

Latest commit:	16cecc9
Status:	✅  Deploy successful!
Preview URL:	https://f9a49971.paddock-7u2.pages.dev
Branch Preview URL:	https://fix-474-forwarded-proto-trus.paddock-7u2.pages.dev

View logs