mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-10 14:27:00 -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.
101 lines
3.8 KiB
Python
101 lines
3.8 KiB
Python
"""CORS scoping tests.
|
|
|
|
The proxy binds on localhost and serves content endpoints (e.g. ``/v1/retrieve``
|
|
returns raw, uncompressed tool outputs). A wildcard CORS origin combined with
|
|
``allow_credentials=True`` let any web page the user had open read those
|
|
responses via a cross-origin fetch to ``127.0.0.1`` (CWE-346). The default
|
|
policy must allow only loopback origins — on *any* port, since the bound port
|
|
lives in the CLI/uvicorn layer, not in ``ProxyConfig`` — while still offering an
|
|
explicit override for Docker / remote-dashboard deployments. See #863 / #864.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import httpx
|
|
import pytest
|
|
from fastapi.testclient import TestClient
|
|
|
|
from headroom.proxy.server import ProxyConfig, create_app
|
|
|
|
|
|
def _make_client() -> TestClient:
|
|
config = 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,
|
|
)
|
|
return TestClient(create_app(config))
|
|
|
|
|
|
def _preflight(client: TestClient, origin: str) -> httpx.Response:
|
|
"""Send a CORS preflight; CORSMiddleware answers it directly."""
|
|
return client.options(
|
|
"/v1/messages",
|
|
headers={"Origin": origin, "Access-Control-Request-Method": "POST"},
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"origin",
|
|
[
|
|
"http://localhost:8787",
|
|
"http://127.0.0.1:8787",
|
|
"http://localhost:9000", # non-default port — must still be allowed
|
|
"http://127.0.0.1:54321",
|
|
"https://localhost:8787",
|
|
"http://[::1]:8787", # IPv6 loopback
|
|
"http://localhost", # no explicit port
|
|
],
|
|
)
|
|
def test_loopback_origins_allowed_on_any_port(monkeypatch: pytest.MonkeyPatch, origin: str) -> None:
|
|
monkeypatch.delenv("HEADROOM_CORS_ORIGINS", raising=False)
|
|
resp = _preflight(_make_client(), origin)
|
|
assert resp.status_code == 200, resp.text
|
|
assert resp.headers.get("access-control-allow-origin") == origin
|
|
# The original vulnerability was wildcard + credentials; credentials stay off.
|
|
assert resp.headers.get("access-control-allow-credentials") != "true"
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"origin",
|
|
[
|
|
"http://evil.com",
|
|
"https://attacker.example",
|
|
"http://localhost.evil.com", # suffix smuggling
|
|
"http://127.0.0.1.evil.com",
|
|
"http://notlocalhost", # prefix smuggling
|
|
],
|
|
)
|
|
def test_cross_origin_pages_rejected(monkeypatch: pytest.MonkeyPatch, origin: str) -> None:
|
|
monkeypatch.delenv("HEADROOM_CORS_ORIGINS", raising=False)
|
|
resp = _preflight(_make_client(), origin)
|
|
# A disallowed origin is never echoed back, so the browser blocks the read.
|
|
assert resp.headers.get("access-control-allow-origin") != origin
|
|
|
|
|
|
def test_explicit_allowlist_override(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
monkeypatch.setenv("HEADROOM_CORS_ORIGINS", "https://dash.example.com, http://10.0.0.5:3000")
|
|
client = _make_client()
|
|
|
|
allowed = _preflight(client, "https://dash.example.com")
|
|
assert allowed.status_code == 200
|
|
assert allowed.headers.get("access-control-allow-origin") == "https://dash.example.com"
|
|
|
|
# Once an explicit list is set, loopback is no longer implicitly trusted.
|
|
blocked = _preflight(client, "http://localhost:8787")
|
|
assert blocked.headers.get("access-control-allow-origin") != "http://localhost:8787"
|
|
|
|
|
|
def test_wildcard_optback(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
monkeypatch.setenv("HEADROOM_CORS_ORIGINS", "*")
|
|
resp = _preflight(_make_client(), "http://evil.com")
|
|
assert resp.status_code == 200
|
|
assert resp.headers.get("access-control-allow-origin") == "*"
|
|
# Wildcard opt-back must not silently re-enable credentialed reads.
|
|
assert resp.headers.get("access-control-allow-credentials") != "true"
|