From a5b0a8f4cc54d68afcf371a422b3a4a9635b7e7f Mon Sep 17 00:00:00 2001 From: Parideboy Date: Wed, 12 Aug 2026 06:05:22 +0200 Subject: [PATCH] fix(proxy): allow settings routes for trusted gateway/dashboard clients (#2491) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description `/settings`, `/settings/schema`, `/settings/apply`, and `/dashboard/settings` were gated by `_require_loopback`, which checks `request.client.host` directly and 404s for any non-loopback caller. When headroom-proxy runs behind a reverse-proxy/gateway (e.g. in a container), `request.client.host` is the gateway's IP, so these routes 404 unconditionally — even with `HEADROOM_PROXY_TRUSTED_GATEWAY_CIDRS`/`HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS` configured, a trust chain `/stats` and `/stats-lifetime` already use. Fixes #2466. ## Type of Change - [x] Bug fix ## Changes Made - Added `_require_loopback_or_trusted_dashboard_client` dependency in `headroom/proxy/server.py`, reusing the existing `_request_can_view_dashboard_metadata` trust chain (loopback check, IP-literal Host header check, same-origin check, trusted-gateway CIDR check). - Swapped this dependency in for `_require_loopback` on exactly five routes: `/settings/schema`, `GET /settings`, `POST /settings`, `POST /settings/apply`, `/dashboard/settings`. All other loopback-only admin/debug routes (`/admin/*`, `/debug/*`, `/cache/clear`, `/v1/retrieve*`) are untouched. - Added test coverage in `tests/test_proxy_loopback_gating.py`: non-loopback without trusted CIDR still 404s, loopback still allowed, trusted-gateway dashboard client is now allowed, and CIDR mismatch still 404s. ## Testing - [x] Added/updated tests - [x] Ran full test suite locally ``` $ python -m pytest tests/test_proxy_loopback_gating.py tests/test_proxy_settings_endpoints.py -q 73 passed, 1 warning in 28.80s $ ruff check headroom/proxy/server.py tests/test_proxy_loopback_gating.py All checks passed! $ ruff format --check headroom/proxy/server.py tests/test_proxy_loopback_gating.py 1 file already formatted, 1 file already formatted $ mypy headroom --ignore-missing-imports Success: no issues found in 506 source files ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.13.11, pytest 9.1.1, headroom repo local checkout - Exact command / steps: `python -m pytest tests/test_proxy_loopback_gating.py -q` after adding parametrized tests that set `HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS` and hit `/settings`, `/settings/schema`, `/dashboard/settings` from a simulated gateway-forwarded peer IP - Observed result: all 51 tests in the file pass, including new cases confirming trusted-gateway clients get 200 (previously 404) while unlisted/mismatched clients still get 404 - Not tested: did not manually deploy a real Docker container behind an actual reverse-proxy (e.g. nginx/Traefik) to reproduce the original reporter's exact setup; relied on TestClient-simulated forwarded headers/peer IPs instead ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review --------- Co-authored-by: Claude Sonnet 5 --- headroom/proxy/server.py | 56 +++++++++++++++-- tests/test_proxy_loopback_gating.py | 98 +++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 5 deletions(-) diff --git a/headroom/proxy/server.py b/headroom/proxy/server.py index 21d9fe26c..ea9615fcc 100644 --- a/headroom/proxy/server.py +++ b/headroom/proxy/server.py @@ -3342,6 +3342,40 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: from headroom.proxy.loopback_guard import require_loopback as _require_loopback from headroom.proxy.loopback_guard import require_same_origin as _require_same_origin + def _require_loopback_or_trusted_dashboard_client(request: Request) -> None: + """Allow loopback callers, or gateway-forwarded dashboard clients. + + Mirrors the trust chain already used by /stats and /stats-lifetime + (see _request_can_view_dashboard_metadata) so the settings UI works + the same way behind a reverse-proxy/gateway (issue #2466). + """ + if not _request_can_view_dashboard_metadata(request, trusted_dashboard_client_cidrs): + raise HTTPException(status_code=404) + + def _require_same_origin_or_trusted_dashboard_client(request: Request) -> None: + """Same-origin CSRF guard for settings writes, trusted-dashboard aware. + + ``require_same_origin`` only accepts an ``Origin`` that itself names a + loopback host, so a browser POST from a trusted-gateway dashboard + client was rejected even though the paired GET routes allow that same + caller (issue #2466). For non-loopback callers, accept an ``Origin`` + that matches this request's own Host header, provided the caller is + already an IP-literal-Host, CIDR-trusted dashboard client. Loopback + callers keep the stricter loopback-only origin check unchanged. + """ + if not _request_is_loopback(request): + origin = request.headers.get("origin") + host_header = request.headers.get("host") + if ( + origin + and origin != "null" + and host_header + and _request_has_same_origin_or_no_provenance(request, host_header) + and _request_can_view_dashboard_metadata(request, trusted_dashboard_client_cidrs) + ): + return + _require_same_origin(request) + @app.get("/admin/upstream", dependencies=[Depends(_require_loopback)]) async def get_upstream(): """Current Anthropic upstream + cc-switch reconciler state (loopback-only). @@ -3462,7 +3496,9 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: # (Phase 3's /settings/apply drives that). from headroom import settings_store - @app.get("/settings/schema", dependencies=[Depends(_require_loopback)]) + @app.get( + "/settings/schema", dependencies=[Depends(_require_loopback_or_trusted_dashboard_client)] + ) async def settings_schema(_request: Request): """Registry + grouped fields + effective values for the settings form.""" schema = settings_store.to_schema() @@ -3479,12 +3515,18 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: schema["supervised"] = False return JSONResponse(status_code=200, content=schema) - @app.get("/settings", dependencies=[Depends(_require_loopback)]) + @app.get("/settings", dependencies=[Depends(_require_loopback_or_trusted_dashboard_client)]) async def settings_get(_request: Request): """Return stored (file) values only; secret fields masked.""" return JSONResponse(status_code=200, content=settings_store.stored_values()) - @app.post("/settings", dependencies=[Depends(_require_loopback), Depends(_require_same_origin)]) + @app.post( + "/settings", + dependencies=[ + Depends(_require_loopback_or_trusted_dashboard_client), + Depends(_require_same_origin_or_trusted_dashboard_client), + ], + ) async def settings_post(request: Request): """Persist settings. Unknown key -> 400; bad type/enum/range -> 422. @@ -3529,7 +3571,11 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: ) @app.post( - "/settings/apply", dependencies=[Depends(_require_loopback), Depends(_require_same_origin)] + "/settings/apply", + dependencies=[ + Depends(_require_loopback_or_trusted_dashboard_client), + Depends(_require_same_origin_or_trusted_dashboard_client), + ], ) async def settings_apply(request: Request): """Persist settings (optional body) then restart the proxy to apply them. @@ -3592,7 +3638,7 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI: @app.get( "/dashboard/settings", response_class=HTMLResponse, - dependencies=[Depends(_require_loopback)], + dependencies=[Depends(_require_loopback_or_trusted_dashboard_client)], ) async def dashboard_settings(): """Serve the Headroom settings GUI.""" diff --git a/tests/test_proxy_loopback_gating.py b/tests/test_proxy_loopback_gating.py index 7e48dabf6..5e9a1c9cb 100644 --- a/tests/test_proxy_loopback_gating.py +++ b/tests/test_proxy_loopback_gating.py @@ -200,6 +200,104 @@ def test_ccr_retrieve_hash_route_blocks_valid_hash_for_non_loopback() -> None: reset_compression_store() +SETTINGS_GATED = [ + ("get", "/settings/schema"), + ("get", "/settings"), + ("get", "/dashboard/settings"), +] + + +@pytest.mark.parametrize("method,path", SETTINGS_GATED) +def test_settings_non_loopback_gets_404_without_trusted_cidr(method: str, path: str) -> None: + resp = TestClient(_make_app()).request(method, path) + assert resp.status_code == 404, resp.text + + +@pytest.mark.parametrize("method,path", SETTINGS_GATED) +def test_settings_loopback_caller_allowed(method: str, path: str) -> None: + resp = _loopback_client().request(method, path) + assert resp.status_code == 200, resp.text + + +@pytest.mark.parametrize("method,path", SETTINGS_GATED) +def test_settings_trusted_gateway_dashboard_client_allowed( + monkeypatch: pytest.MonkeyPatch, method: str, path: str +) -> None: + """Settings routes must follow the same trust chain as /stats so the + dashboard works behind a reverse-proxy/gateway (#2466).""" + monkeypatch.setenv("HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS", "100.90.0.5/32") + client = TestClient( + _make_app(), + base_url="http://100.82.0.2:8787", + client=("100.90.0.5", 12345), + ) + resp = client.request(method, path) + assert resp.status_code == 200, resp.text + + +def test_settings_trusted_gateway_cidr_mismatch_still_404s( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS", "100.90.0.5/32") + client = TestClient( + _make_app(), + base_url="http://100.82.0.2:8787", + client=("100.90.0.9", 12345), + ) + assert client.get("/settings").status_code == 404 + + +@pytest.mark.parametrize( + "path,body", + [("/settings", {"values": {}}), ("/settings/apply", None)], +) +def test_settings_post_trusted_gateway_client_same_origin_allowed( + monkeypatch: pytest.MonkeyPatch, path: str, body: dict | None +) -> None: + """Regression for #2491 review: a trusted-gateway dashboard client's real + same-origin browser POST (Origin matching this Host) must not be rejected + by the loopback-only same-origin guard.""" + monkeypatch.setenv("HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS", "100.90.0.5/32") + client = TestClient( + _make_app(), + base_url="http://100.82.0.2:8787", + client=("100.90.0.5", 12345), + ) + resp = client.post(path, json=body, headers={"origin": "http://100.82.0.2:8787"}) + assert resp.status_code != 403, resp.text + + +@pytest.mark.parametrize( + "path,body", + [("/settings", {"values": {}}), ("/settings/apply", None)], +) +def test_settings_post_trusted_gateway_client_mismatched_origin_rejected( + monkeypatch: pytest.MonkeyPatch, path: str, body: dict | None +) -> None: + """A trusted-gateway peer with a foreign Origin is still CSRF-rejected. + + The mismatched Origin also fails the first (loopback-or-trusted-client) + gate's own same-origin check, so this surfaces as 404, not 403 -- either + way the write must not go through.""" + monkeypatch.setenv("HEADROOM_PROXY_TRUSTED_DASHBOARD_CLIENT_CIDRS", "100.90.0.5/32") + client = TestClient( + _make_app(), + base_url="http://100.82.0.2:8787", + client=("100.90.0.5", 12345), + ) + resp = client.post(path, json=body, headers={"origin": "http://attacker.example"}) + assert resp.status_code in (403, 404), resp.text + + +def test_settings_post_loopback_null_origin_still_rejected() -> None: + """Loopback callers keep the stricter loopback-only origin check: a + sandboxed-iframe/file:// "null" Origin must still 403, unaffected by the + trusted-dashboard-client carve-out.""" + client = _loopback_client() + resp = client.post("/settings", json={"values": {}}, headers={"origin": "null"}) + assert resp.status_code == 403, resp.text + + def test_dns_rebinding_host_header_rejected() -> None: # Loopback peer IP but an attacker-controlled Host header (the DNS-rebinding # shape) must still be rejected by the second gate.