mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description Locks down the proxy's browser- and network-facing attack surface, which matters most under a `--host 0.0.0.0` bind (the Docker default). The wildcard CORS policy (`allow_origins=["*"]` + `allow_credentials=True`) let any web page the user had open read the proxy's content endpoints — `/v1/retrieve` returns raw, uncompressed tool outputs (source, secrets) — via a cross-origin fetch to `127.0.0.1` (CWE-346). Several operator endpoints additionally leaked sensitive data or allowed unauthenticated state mutation to any network-reachable client. This PR scopes CORS to loopback origins and extends the project's existing `require_loopback` trust boundary (already used for `/admin/*` and `/debug/*`) to the remaining exposed endpoints. Closes #863. Supersedes #864 and #758 — see "Additional Notes". ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - **CORS**: replaced `allow_origins=["*"]` + `allow_credentials=True` with a port-agnostic loopback origin regex (`https?://(localhost|127\.0\.0\.1|\[::1\])(:\d+)?`), `allow_credentials=False`, and methods/headers narrowed to `GET/POST` + `Content-Type/Authorization`. `HEADROOM_CORS_ORIGINS` (comma-separated) pins an explicit allowlist for Docker/remote dashboards; `*` opts back into the old wildcard. - **`/transformations/feed`** and **`/cache/clear`** gated behind `require_loopback` → 404 for non-loopback callers. The feed returns full prompt/completion bodies when `log_full_messages` is on; `/cache/clear` is unauthenticated state mutation (cache-eviction DoS / cost amplification). - **`/health`**: the `config` block (upstream API URLs, savings profile) is now served only to loopback callers; network callers get the `/readyz`-shape body (status/checks). `/livez` and `/readyz` remain unauthenticated probes for orchestration. - **`/stats`**: `recent_requests` / `request_logs` (per-request ids, providers, models, errors) and `config` are served only to loopback callers; aggregate counters stay public for remote monitoring. - Added `_request_is_loopback()` helper mirroring `require_loopback`'s two-gate check (loopback peer IP + loopback `Host` header, the DNS-rebinding defence) but degrading the payload instead of returning 404, so monitors keep the non-sensitive fields. - Tests: new `tests/test_proxy_cors.py` and `tests/test_proxy_loopback_gating.py`; updated 4 existing tests that assert the now-loopback-only data to use loopback clients. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ ruff check headroom/proxy/server.py tests/test_proxy_cors.py \ tests/test_proxy_loopback_gating.py tests/test_proxy_healthchecks.py \ tests/test_proxy_stats_recent_requests.py tests/test_proxy/test_transformations_feed.py All checks passed! $ mypy headroom --ignore-missing-imports Success: no issues found in 380 source files $ pytest tests/test_proxy_cors.py tests/test_proxy_loopback_gating.py \ tests/test_proxy/test_transformations_feed.py tests/test_proxy_healthchecks.py \ tests/test_proxy_stats_recent_requests.py tests/test_proxy_dashboard_stats_cache.py \ tests/test_proxy_compression_executor.py tests/test_header_isolation.py -q ======================== 82 passed, 1 skipped in 17.75s ======================== ``` ## Real Behavior Proof - Environment: macOS (darwin 25.4.0), Python 3.12 venv; FastAPI `TestClient` driving the real `create_app()` ASGI app - Exact command / steps: issued requests as a non-loopback caller (`client.host=testclient`) vs a loopback caller (`base_url=http://127.0.0.1`, `client=("127.0.0.1", 9999)`), plus CORS preflights with varying `Origin` headers - Observed result: CORS — `http://evil.com` → no `access-control-allow-origin`; `http://localhost:8787` and `http://localhost:9000` → echoed (loopback allowed on any port); `access-control-allow-credentials` → absent. `/cache/clear` and `/transformations/feed` → 404 (network) / 200 (loopback). `/health` `config` block present for loopback only. `/stats` `recent_requests` present for loopback only, while the aggregate `tokens` block stays present for network callers. - Not tested: a live `headroom proxy` process bound on `0.0.0.0` reached from a second host (simulated via ASGI peer/Host instead); end-to-end browser DNS-rebinding (covered by the `Host`-header gate and its unit test) ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) N/A — proxy/middleware change; behavior is captured under "Real Behavior Proof". ## Additional Notes **Supersedes two stale PRs that target the same issue but have drifted from `main`:** - **#864** (`fix(proxy): scope CORS to localhost`, @gabiudrescu) — correct instinct and the source of the tighter `GET/POST` + `Content-Type/Authorization` scoping kept here, but it derived the allowlist from the `HEADROOM_PORT` env var (wrong when `--port` is passed as a CLI flag), carried ~40 lines of unrelated punctuation churn, and is ~125 commits behind `main`. The port-agnostic regex used here resolves the reviewer's port concern. - **#758** (`security: adversarial review`, @neogenix) — bundled these same application-layer fixes with a large CI/CD + Docker supply-chain pass. It is a ~160-commit-behind draft whose `server.py` no longer merges cleanly (`main` independently adopted the same `require_loopback` pattern). The application-layer fixes are rebased onto current `main` here; the CI/Docker/supply-chain hardening from #758 is still valuable and would be welcome as a separate, rebased PR. Thanks to @gabiudrescu and @neogenix for the original analysis (#863). **Deliberate scope / follow-ups (not in this PR):** - `/stats` aggregate counters and the basic `/health` body remain readable on a `0.0.0.0` bind by design, so remote monitoring keeps working. Full lock-down is a one-line `Depends(require_loopback)` each if preferred. - The `/v1/retrieve*` family stays network-reachable; it can't be loopback-gated without breaking legitimate remote/containerized agents and needs auth instead — tracked separately. - `ruff check .` is scoped to changed paths above because the dashboard HTML template trips ruff's `invalid-syntax` (a known repo false-positive); `mypy` is run over the full `headroom` package.
108 lines
4 KiB
Python
108 lines
4 KiB
Python
"""Loopback-gating tests for state-mutating / content-leaking endpoints.
|
|
|
|
``/transformations/feed`` can return full prompt + completion bodies (when
|
|
``log_full_messages`` is on) and ``/cache/clear`` mutates server state. With the
|
|
default ``--host 0.0.0.0`` Docker bind, neither should be reachable by an
|
|
arbitrary network client — they are gated to the loopback interface via
|
|
``require_loopback`` (the same guard already used for ``/admin/*`` and
|
|
``/debug/*``). See #863.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import pytest
|
|
from fastapi import FastAPI
|
|
from fastapi.testclient import TestClient
|
|
|
|
from headroom.proxy.server import ProxyConfig, create_app
|
|
|
|
GATED = [
|
|
("get", "/transformations/feed"),
|
|
("post", "/cache/clear"),
|
|
]
|
|
|
|
|
|
def _make_app() -> FastAPI:
|
|
return create_app(
|
|
ProxyConfig(
|
|
optimize=False,
|
|
cache_enabled=False,
|
|
rate_limit_enabled=False,
|
|
cost_tracking_enabled=False,
|
|
log_requests=False,
|
|
ccr_inject_tool=False,
|
|
ccr_handle_responses=False,
|
|
ccr_context_tracking=False,
|
|
image_optimize=False,
|
|
)
|
|
)
|
|
|
|
|
|
def _loopback_client() -> TestClient:
|
|
# A real loopback peer + a loopback Host header — passes both guard gates
|
|
# (client-IP check and the DNS-rebinding Host-header check).
|
|
return TestClient(_make_app(), base_url="http://127.0.0.1", client=("127.0.0.1", 12345))
|
|
|
|
|
|
@pytest.mark.parametrize("method,path", GATED)
|
|
def test_non_loopback_caller_gets_404(method: str, path: str) -> None:
|
|
# A vanilla TestClient presents client.host="testclient", which is not a
|
|
# loopback IP, so the guard returns 404 (invisible, not 403).
|
|
client = TestClient(_make_app())
|
|
resp = client.request(method, path)
|
|
assert resp.status_code == 404, resp.text
|
|
|
|
|
|
@pytest.mark.parametrize("method,path", GATED)
|
|
def test_loopback_caller_allowed(method: str, path: str) -> None:
|
|
client = _loopback_client()
|
|
resp = client.request(method, path)
|
|
assert resp.status_code == 200, 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.
|
|
client = TestClient(_make_app(), base_url="http://127.0.0.1", client=("127.0.0.1", 12345))
|
|
resp = client.get("/transformations/feed", headers={"host": "attacker.example"})
|
|
assert resp.status_code == 404, resp.text
|
|
|
|
|
|
def _client(*, loopback: bool) -> TestClient:
|
|
app = _make_app()
|
|
if loopback:
|
|
return TestClient(app, base_url="http://127.0.0.1", client=("127.0.0.1", 12345))
|
|
# Default TestClient presents client.host="testclient" — not loopback.
|
|
return TestClient(app)
|
|
|
|
|
|
def test_health_config_block_is_loopback_only(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
"""/health stays reachable for monitors but hides the `config` block (which
|
|
echoes upstream API URLs + backend settings) from non-loopback callers."""
|
|
monkeypatch.setenv("HEADROOM_SKIP_UPSTREAM_CHECK", "1")
|
|
|
|
network = _client(loopback=False).get("/health")
|
|
assert network.status_code == 200
|
|
assert "config" not in network.json()
|
|
# Basic health is still visible to monitors.
|
|
assert network.json()["status"] in {"healthy", "unhealthy"}
|
|
|
|
local = _client(loopback=True).get("/health")
|
|
assert local.status_code == 200
|
|
assert "config" in local.json()
|
|
|
|
|
|
def test_stats_per_request_metadata_is_loopback_only() -> None:
|
|
"""/stats keeps aggregate counters public but restricts per-request metadata
|
|
(recent_requests / request_logs) and `config` to loopback callers."""
|
|
network = _client(loopback=False).get("/stats")
|
|
assert network.status_code == 200
|
|
payload = network.json()
|
|
assert "tokens" in payload # aggregate counters still served
|
|
assert "recent_requests" not in payload
|
|
assert "request_logs" not in payload
|
|
assert "config" not in payload
|
|
|
|
local = _client(loopback=True).get("/stats").json()
|
|
assert "recent_requests" in local
|
|
assert "config" in local
|