fix(security): localhost auth bypass refuses requests with reverse-proxy forwarding headers

[scottwofford]

SECURITY: needs Scott's explicit review before merge.

Fixes the high-severity localhost auth bypass hole tracked in Trello: security: localhost auth bypass trusts TCP source IP — same-host reverse proxy exposes admin surface. Corroborated by finding F5 in the 2026-07-07 security/telemetry audit (luthien-org dev/2026-07-07_security-telemetry-data-audit.md).

Threat model

• is_localhost_request() decided the auth bypass purely from request.client.host (TCP source IP). It never looked at forwarding headers.
• The standard self-host topology puts a reverse proxy (Caddy, nginx, Traefik) on the same host as the gateway. Every external request then arrives at the gateway from 127.0.0.1.
• With LOCALHOST_AUTH_BYPASS=true (the default), the entire admin surface (/api/admin/*, history, request logs, debug, admin UI) was served unauthenticated to the public internet, regardless of whether ADMIN_API_KEY was set. That surface is exactly where stored conversation content is readable.
• Distinct from PR fix(auth): admin UI fails closed when ADMIN_API_KEY is unset #775 (admin UI fail-closed on unset key) and issue [from-pr-614-review] check_auth_or_redirect behavior change when ADMIN_API_KEY unset #767: the bypass short-circuits before those checks, so this hole was open even with an admin key configured.

Chosen behavior

The bypass now requires all three:

1. LOCALHOST_AUTH_BYPASS enabled (default unchanged: true — see below),
2. loopback TCP source address (127.0.0.1, ::1, ::ffff:127.0.0.1), and
3. no reverse-proxy forwarding headers on the request: Forwarded (RFC 7239), X-Forwarded-For, X-Forwarded-Host, X-Forwarded-Proto, X-Real-IP.

Why header presence rather than parsing the forwarded client IP: presence-detection fails safe. Caddy and Traefik always attach X-Forwarded-For; standard nginx configs attach X-Forwarded-For and/or X-Real-IP. A client can also set these headers themselves, but that only disables the bypass for that client and drops them into normal admin-key auth — an attacker cannot gain access by adding headers, and cannot strip the headers the proxy adds on the hop the gateway sees. A trusted-proxy CIDR config (option 1 on the card) would be strictly more machinery for no additional safety on this route, and gets the trust direction wrong if misconfigured.

Also added: a startup warning whenever the bypass is enabled, stating that any local process can read stored conversations and that reverse-proxy deployments should set LOCALHOST_AUTH_BYPASS=false (the gateway binds 0.0.0.0 unconditionally in main.py, so a bind-address startup guard — option 4 on the card — would fire on every deployment; the warning covers the same ground without a new config knob).

Residual risk (documented in auth.py): a same-host reverse proxy explicitly configured to strip/omit all forwarding headers still looks like a direct loopback client. The guidance to set LOCALHOST_AUTH_BYPASS=false behind any reverse proxy stays in the module docstring, the config-field description, and the startup warning.

Default NOT changed: LOCALHOST_AUTH_BYPASS still defaults to true. Flipping it to false (opt-in bypass) is the stronger long-term posture but is a breaking change to the local dev / dockerless browse-the-dashboard-without-logging-in workflow, and deserves an explicit product decision. Input for that decision: the luthien CLI does not depend on the bypass (onboard generates an ADMIN_API_KEY and gateway_client.py sends it as a Bearer token), so the flip would mainly cost browser UX — a one-time /login with the key from .env. Left as a follow-up decision on the Trello card rather than bundled here (one PR = one concern).

What still works (unchanged behavior)

• Direct curl http://localhost:8000/api/admin/... from the same box, no proxy: bypass applies as before.
• /v1/messages proxy auth (verify_token in gateway_routes.py): never consulted this module; unaffected.
• Railway: already disables the bypass at startup; unaffected.
• Authenticating with a valid admin key or session cookie through a reverse proxy: works (the proxied request falls through to normal auth).

RCA

• Root cause: the localhost check conflated "TCP peer is loopback" with "client is local." Behind a same-host reverse proxy those are different parties, and the check ignored the one signal the proxy provides to distinguish them (forwarding headers).
• Why it wasn't caught: the foot-gun was known and documented — the auth.py:15-20 docstring literally described this attack — but the mitigation was left to the operator reading source comments and flipping a non-obvious env var. No test modeled the proxied topology; every bypass test used a bare loopback request. A security posture that lives only in a docstring is not a control.
• Why it won't recur: the dangerous topology is now handled in code, fail-safe by construction (header presence can only narrow the bypass, never widen it). Regression tests pin the proxied case for both auth paths (verify_admin_token → 403, check_auth_or_redirect → login redirect) across each forwarding header, plus bare-loopback and valid-key-through-proxy cases, so a future refactor of _should_bypass_auth that drops the header check fails CI.

Tests

• New in tests/luthien_proxy/unit_tests/test_auth.py: TestHasForwardingHeaders, TestLocalhostBypassRefusesProxiedRequests, TestLocalhostBypassRefusesProxiedAdminApi — bypass works for bare localhost; refuses per forwarding header; admin API returns 403 in the proxied case; valid admin key still authenticates when proxied.
• ./scripts/dev_checks.sh fully green: ruff format + lint, pyright (0 errors, 0 warnings), full unit + integration pytest run.

🤖 Generated with Claude Code

[claude]

Review

Clean, well-scoped security fix. The threat model in the PR description is spot-on and the fail-safe design (presence-based header check that can only narrow the bypass) is the right primitive. Below are review notes — none are blocking; two are worth considering before merge.

Code quality & correctness

• _should_bypass_auth (auth.py:71) reads well and matches the docstring — three explicit conditions, easy to audit.
• _FORWARDING_HEADERS (auth.py:49-55) covers the mainstream set. Case-insensitive in on request.headers is guaranteed by Starlette's Headers, so lowercase constants are correct.
• Docstring at the top of auth.py was previously the only mitigation ("just flip an env var"); folding the header check into _should_bypass_auth moves the control out of the docstring and into code. This is exactly the RCA finding acted on.
• The added has_forwarding_headers is exported in __all__ and covered by dedicated tests — good.

Potential gaps

1. Via header not in the list. RFC 7230 § 5.7.1 defines Via explicitly for proxies. Reasonable proxies almost always also emit X-Forwarded-For, so the current set is sufficient for the stated threat (Caddy/nginx/Traefik same-host). But if a future exotic proxy sets only Via, this bypass would still trigger. Considering adding "via" to the tuple is cheap and pure win (fail-safe direction). Same argument, weaker, for X-Client-IP and True-Client-IP.

2. localhost_auth_bypass is db_settable=True with restart_required=False (config_fields.py:99). The startup warning at main.py:276 only fires at boot. If an operator flips the field to true at runtime via /api/admin/config, the same "any local process can read stored conversations" warning is silently skipped. Not part of this PR's scope, but worth a follow-up: emit the same warning when the DB value transitions to true, or make the field restart_required=True.

3. Runtime provenance vs. DB flip. Related: if the DB says localhost_auth_bypass=false but env says true, env wins per the config registry precedence. The docstring only mentions "Railway disables the bypass automatically at startup" — worth a one-line note that the DB-settable knob exists and takes normal precedence, so an operator toggling in the config UI knows their change is authoritative for non-env deployments.

Test coverage

• TestHasForwardingHeaders and both TestLocalhostBypass… classes cover the important cases: each header individually parameterized (test_auth.py:323-333), combined headers, redirect path, admin-API path, and the "valid key through proxy still authenticates" case (which prevents future refactors from over-blocking).
• The has_forwarding_headers unit tests use a MagicMock whose headers is a plain dict, so they don't exercise Starlette's case-insensitive matching. The TestLocalhostBypassRefusesProxiedAdminApi class does use a real TestClient, so end-to-end case handling is covered — but a one-liner test_uppercase_header_detected at the has_forwarding_headers layer would guard the helper itself if someone later replaces the tuple with a set() and forgets Starlette normalization.
• Consider parametrizing over a common header key with mixed case (X-Forwarded-For) in TestLocalhostBypassRefusesProxiedAdminApi.test_admin_api_403_when_forwarded_header_present to lock the Starlette contract in at both call sites.

Performance & security

• Header scan is O(5) per admin request; noise vs. existing per-request work is nothing.
• No new information is trusted from the client — headers are used only to deny bypass, never to grant it. That's the correct trust direction and it's preserved end-to-end. The only widening path is disabling the bypass entirely, which the startup warning surfaces.
• get_base_url (auth.py:179) already trusts x-forwarded-proto for redirect construction. Unchanged here, but worth noting: it does not need hardening because the affected surface (base URL for redirects) doesn't grant privilege, only affects the login-redirect URL scheme.

Nits

• changelog.d/localhost-bypass-forwarded-headers.md is a single very long paragraph. The changelog.d/README.md format allows multiline; a two-sentence lead + a short "unaffected: direct loopback / CLI" clause reads better in the assembled CHANGELOG, but not worth blocking on.
• The PR body notes the default of LOCALHOST_AUTH_BYPASS=true is intentionally preserved and calls out the follow-up decision. Good boundary — one PR, one concern.

Verdict

Ready to merge as-is. Recommended follow-ups (separate PRs / cards):

• add Via to _FORWARDING_HEADERS (5-line change, strictly fail-safer),
• emit the startup warning on DB toggle as well, or mark the field restart_required=True,
• product decision on flipping the default to opt-in (already noted in the PR body).

[scottwofford]

Claude-generated merge-queue triage of all open Luthien PRs, requested by Scott (Jul 7, 2026). Advisory only; Scott has not yet acted on these recommendations.

Recommendation: merge, after a human security read (as the PR itself requests).

The diff matches the description: the localhost bypass now requires both a loopback TCP source and the absence of Forwarded / X-Forwarded-* / X-Real-IP headers, so a same-host reverse proxy no longer silently exposes the admin surface. Header presence can only narrow the bypass, never widen it, and the default configuration is unchanged. Regression tests cover each header and the valid-key-through-proxy path. This is the one PR in the Jul 7 batch that reduces existing production risk, and it conflicts with nothing. Residual risk (a reverse proxy configured to strip forwarding headers) is documented in the PR body.