diff --git a/drb-c2-core/.env.example b/drb-c2-core/.env.example index 0ae79c4..b0b1067 100644 --- a/drb-c2-core/.env.example +++ b/drb-c2-core/.env.example @@ -33,6 +33,13 @@ SUMMARY_INTERVAL_MINUTES=15 CORRELATION_WINDOW_HOURS=4 EMBEDDING_SIMILARITY_THRESHOLD=0.82 +# Browser origins allowed to call this API cross-origin (JSON list). The only +# browser caller is the frontend's Archive page (GET /calls/search). Set this +# to the exact origin the frontend is served from — scheme + host, no path. +# Defaults to https://drb.cusano.net. A "*" entry works for local dev but is +# logged as a probable misconfiguration and never gets a credentialed response. +CORS_ORIGINS=["https://drb.cusano.net"] + # Fleet-wide token edge nodes present as X-Enrollment-Token on first boot # (POST /nodes/enroll). Shared across every node — NOT a per-node secret. # Generate with: openssl rand -hex 32 diff --git a/drb-c2-core/app/config.py b/drb-c2-core/app/config.py index 4a17f2c..3b8b1b7 100644 --- a/drb-c2-core/app/config.py +++ b/drb-c2-core/app/config.py @@ -180,16 +180,18 @@ class Settings(BaseSettings): # between genuinely separate transmissions on a busy dispatch channel. duplicate_window_seconds: int = 10 - # CORS — set to your frontend origin(s) in production, e.g. ["https://app.example.com"] - # Defaults to "*" for local development only. + # Browser origins allowed to call this API cross-origin. The only browser + # caller is the frontend's Archive page (GET /calls/search) — every other + # page reads Firestore directly. The frontend is served on the BARE domain + # (see infra Caddyfile.j2 — only drb. and api. have DNS records), so the + # default is that origin, not app.. Override via CORS_ORIGINS (JSON + # list) if the frontend ever moves; keep infra/.../c2-core.env.j2 in sync. # - # Leaving this as "*" is not merely permissive: main.py turns OFF - # allow_credentials when it sees a wildcard, because Starlette would - # otherwise reflect each caller's origin back WITH - # Access-Control-Allow-Credentials. So a production deployment that - # forgets to set this gets a loud ERROR at startup and loses credentialed - # cross-origin requests, rather than silently accepting every origin. - cors_origins: list[str] = ["*"] + # A "*" entry here still works for local dev but is refused a credentialed + # response: main.py never enables allow_credentials (auth is a Bearer + # header, not a cookie), and it logs a loud ERROR when it sees a wildcard + # in a deployment so a forgotten override is visible. + cors_origins: list[str] = ["https://drb.cusano.net"] # Discord webhook URL that app/internal/ai_health.py posts to when an AI # tier (transcription/correlation) transitions into or out of degraded diff --git a/drb-c2-core/app/main.py b/drb-c2-core/app/main.py index ac9ea4d..eca5a4f 100644 --- a/drb-c2-core/app/main.py +++ b/drb-c2-core/app/main.py @@ -78,33 +78,40 @@ async def lifespan(app: FastAPI): app = FastAPI(title="DRB C2 Core", lifespan=lifespan) -# "*" plus allow_credentials=True is not the permissive-but-harmless setting it -# looks like. Starlette does not refuse the combination -- it reflects the -# caller's Origin back and still sends Access-Control-Allow-Credentials: true, -# so the effective policy becomes "any origin, with credentials", the opposite -# of what a wildcard normally means. Rather than trust every deployment to -# remember to override CORS_ORIGINS, make the dangerous pair unrepresentable. +# The browser needs CORS to reach this API at all: the frontend's Archive page +# calls GET /calls/search with Authorization + Content-Type headers, which +# forces a preflight. Without this middleware the OPTIONS gets a bare 405 and +# the fetch fails (#110). allow_origins is an explicit list -- never "*" in a +# deployment -- so name every host the frontend is served from in CORS_ORIGINS. +# +# allow_credentials stays False on purpose: auth here is a Bearer header, not a +# cookie, so credentialed CORS is never needed, and keeping it False is what +# lets an explicit-origin allowlist work without Starlette's "*"-only +# restriction. "*" + credentials is the dangerous pair (Starlette reflects the +# caller's Origin back WITH Access-Control-Allow-Credentials: true); this code +# cannot produce it because credentials are hard-off. def cors_allows_credentials(origins: list[str]) -> bool: - """False when any entry is a wildcard. Extracted so it can be tested - without re-importing this module, which drags in every router.""" - return "*" not in origins + """Always False -- credentialed CORS is never enabled here (Bearer auth, + not cookies). Kept as a named predicate so a future edit that wants to + turn credentials on has to go through here and confront the "*" case. + A wildcard entry would additionally be refused a credentialed response.""" + return False -_cors_is_wildcard = not cors_allows_credentials(settings.cors_origins) +_cors_is_wildcard = "*" in settings.cors_origins if _cors_is_wildcard: logger.error( - "CORS_ORIGINS is '*', so credentialed cross-origin requests are being " - "DISABLED to avoid reflecting every caller's origin back with " - "Access-Control-Allow-Credentials. Set CORS_ORIGINS to your frontend " - "origin(s) in production, e.g. [\"https://app.example.com\"]." + "CORS_ORIGINS contains '*'. That is fine for local dev but is almost " + "certainly a misconfigured deployment -- set CORS_ORIGINS to your " + "frontend origin(s), e.g. [\"https://drb.cusano.net\"]." ) app.add_middleware( CORSMiddleware, allow_origins=settings.cors_origins, - allow_methods=["*"], - allow_headers=["*"], - allow_credentials=not _cors_is_wildcard, + allow_methods=["GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS"], + allow_headers=["authorization", "content-type"], + allow_credentials=False, ) app.include_router(nodes.router, dependencies=[Depends(require_service_or_firebase_token)]) diff --git a/drb-c2-core/tests/test_cors.py b/drb-c2-core/tests/test_cors.py new file mode 100644 index 0000000..dae4a55 --- /dev/null +++ b/drb-c2-core/tests/test_cors.py @@ -0,0 +1,66 @@ +""" +End-to-end CORS wiring for the one browser-facing REST surface. + +The frontend's Archive page calls GET /calls/search with Authorization + +Content-Type headers, which forces the browser to send a CORS preflight +first. Before #110 that OPTIONS got a bare 405 with no Access-Control-* +headers and the fetch failed with "TypeError: Failed to fetch". These +tests drive the real app through TestClient so a regression in the +middleware wiring (not just the helper) is caught. + +TestClient is NOT used as a context manager on purpose: that would run the +lifespan (mqtt_handler.connect(), the sweeper loops, dynsec bootstrap), +none of which is needed here -- CORSMiddleware answers a preflight before +routing or dependencies run. +""" +from fastapi.testclient import TestClient + +from app.config import settings +from app.main import app + +client = TestClient(app) + +ALLOWED_ORIGIN = "https://drb.cusano.net" +DISALLOWED_ORIGIN = "https://evil.example.com" + + +def test_default_allowed_origin_matches_the_deployed_frontend(): + # The frontend is served on the bare domain (infra Caddyfile.j2), so the + # default must allow exactly that origin without any env override. + assert ALLOWED_ORIGIN in settings.cors_origins + + +def test_preflight_for_calls_search_is_allowed(): + resp = client.options( + "/calls/search", + headers={ + "Origin": ALLOWED_ORIGIN, + "Access-Control-Request-Method": "GET", + "Access-Control-Request-Headers": "authorization,content-type", + }, + ) + assert resp.status_code == 200 + assert resp.headers.get("access-control-allow-origin") == ALLOWED_ORIGIN + allow_methods = resp.headers.get("access-control-allow-methods", "").upper() + assert "GET" in allow_methods + # Bearer auth, not cookies -- credentials must never be advertised. + assert "access-control-allow-credentials" not in resp.headers + + +def test_preflight_from_disallowed_origin_gets_no_allow_origin(): + resp = client.options( + "/calls/search", + headers={ + "Origin": DISALLOWED_ORIGIN, + "Access-Control-Request-Method": "GET", + }, + ) + assert resp.headers.get("access-control-allow-origin") is None + + +def test_simple_get_from_allowed_origin_is_annotated(): + # Even a non-preflight GET must carry Access-Control-Allow-Origin or the + # browser hides the response body from the page. + resp = client.get("/health", headers={"Origin": ALLOWED_ORIGIN}) + assert resp.status_code == 200 + assert resp.headers.get("access-control-allow-origin") == ALLOWED_ORIGIN diff --git a/drb-c2-core/tests/test_cors_policy.py b/drb-c2-core/tests/test_cors_policy.py index aa889e2..e652ed5 100644 --- a/drb-c2-core/tests/test_cors_policy.py +++ b/drb-c2-core/tests/test_cors_policy.py @@ -5,8 +5,9 @@ Starlette does not reject `allow_origins=["*"]` combined with `allow_credentials=True`. It reflects the caller's Origin back in Access-Control-Allow-Origin and still sends Access-Control-Allow-Credentials: true, so the effective policy is the -opposite of what a wildcard usually means. main.py defuses that by turning -credentials off whenever it sees a wildcard; these tests hold it to that. +opposite of what a wildcard usually means. main.py never enables +credentials at all (auth is a Bearer header, not a cookie), which makes +that pair unrepresentable; these tests hold it to that. The policy lives in a pure function so it can be exercised directly -- reloading app.main to vary settings drags every router back through import @@ -28,11 +29,11 @@ def test_wildcard_among_real_origins_still_disables_credentials(): assert cors_allows_credentials(["https://app.example.com", "*"]) is False -def test_named_origins_keep_credentials(): - # Naming your origins is how you ask for credentialed requests, so a - # correctly configured deployment must not be penalised. - assert cors_allows_credentials(["https://app.example.com"]) is True - assert cors_allows_credentials([]) is True +def test_credentials_never_enabled_even_for_named_origins(): + # Auth here is a Bearer header, not a cookie, so credentialed CORS is + # never needed. The predicate is hard-off regardless of the origin list. + assert cors_allows_credentials(["https://app.example.com"]) is False + assert cors_allows_credentials([]) is False def test_the_app_actually_mounted_that_policy(): @@ -42,6 +43,7 @@ def test_the_app_actually_mounted_that_policy(): (mw.kwargs for mw in app.user_middleware if mw.cls is CORSMiddleware), None ) assert opts is not None, "CORSMiddleware is not mounted at all" + assert opts["allow_credentials"] is False assert opts["allow_credentials"] is cors_allows_credentials(settings.cors_origins)