From d725c2e4e6d6b570c9007a9215612b29841f8b18 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 17 Aug 2026 00:10:34 +0000 Subject: [PATCH] feat(cors): always trust the service's own RAILWAY_PUBLIC_DOMAIN origin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The August 2026 cutover proved that a *configured* CORS allowlist can be worse than none: REGENGINE_CORS_ORIGINS was set — to the previous service's URL, via a Railway reference variable — so the new service rejected every browser request from its own domain for three days, and because auth_middleware gates state-changing requests on the same list, writes failed too (#80, #81). Append the platform-issued origin (https://$RAILWAY_PUBLIC_DOMAIN) to whatever cors_origins_from_env() resolves. A union rather than a fallback, deliberately: the incident had the variable set-but-stale, so a fallback that only applies when the variable is missing would have changed nothing. Trusting the platform domain widens nothing beyond the service's own canonical origin, and anyone who can forge that variable already controls the deployment. A malformed platform value degrades to "no extra origin" instead of raising — this path runs while the ASGI app is constructed, and a platform-injected string must never be able to crash startup. Explicit origins for third-party dashboard hosts still require configuration; the cutover checklist keeps that step and notes the self-trust behavior. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_014ykFQkKR1XmCtSkDRCT4sT --- DEPLOYMENT_PROFILES.md | 4 ++++ app/cors.py | 44 ++++++++++++++++++++++++++++++++++-------- tests/test_api.py | 37 +++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 8 deletions(-) diff --git a/DEPLOYMENT_PROFILES.md b/DEPLOYMENT_PROFILES.md index 3268f9a..722560d 100644 --- a/DEPLOYMENT_PROFILES.md +++ b/DEPLOYMENT_PROFILES.md @@ -263,6 +263,10 @@ next one starts. origin (`app/auth_middleware.py` gates writes on the same list). Verify: `curl -sD - -o /dev/null -H "Origin: https://" https:///api/healthz` must echo the origin back in `access-control-allow-origin`. + (On Railway the service now also trusts its own `RAILWAY_PUBLIC_DOMAIN` + origin *in addition to* this list, so a stale configured value can no + longer lock the service out of its own domain — but explicit origins for + any other dashboard host still need this step.) 2. **Replace secret reference variables with concrete values.** `REGENGINE_BASIC_AUTH_USERNAME`, `REGENGINE_BASIC_AUTH_PASSWORD`, and `REGENGINE_WEBHOOK_HMAC_SECRET` may reference the old service. Deleting diff --git a/app/cors.py b/app/cors.py index db97ea7..3a414c0 100644 --- a/app/cors.py +++ b/app/cors.py @@ -10,14 +10,42 @@ def cors_origins_from_env() -> list[str]: raw_origins = os.getenv("REGENGINE_CORS_ORIGINS") if not raw_origins or not raw_origins.strip(): - return list(DEFAULT_CORS_ORIGINS) - - origins: list[str] = [] - for raw_origin in raw_origins.split(","): - origin = _normalize_cors_origin(raw_origin) - if origin and origin not in origins: - origins.append(origin) - return origins or list(DEFAULT_CORS_ORIGINS) + origins = list(DEFAULT_CORS_ORIGINS) + else: + origins = [] + for raw_origin in raw_origins.split(","): + origin = _normalize_cors_origin(raw_origin) + if origin and origin not in origins: + origins.append(origin) + origins = origins or list(DEFAULT_CORS_ORIGINS) + + # The service always trusts its own platform-issued domain, IN ADDITION to + # whatever is configured. A union, not a fallback, on purpose: in the + # August 2026 cutover REGENGINE_CORS_ORIGINS was set — to the previous + # service's URL, via a Railway reference variable — so a fallback would + # have changed nothing, and the service rejected every browser request + # from its own domain for three days (see DEPLOYMENT_PROFILES.md, + # "Moving the demo to a new service"). Trusting RAILWAY_PUBLIC_DOMAIN is + # safe because whoever controls that variable controls the deployment + # itself; it widens nothing beyond the service's own canonical origin. + platform_origin = _platform_origin() + if platform_origin and platform_origin not in origins: + origins.append(platform_origin) + return origins + + +def _platform_origin() -> str | None: + domain = os.getenv("RAILWAY_PUBLIC_DOMAIN", "").strip().rstrip("/") + if not domain: + return None + if "://" not in domain: + domain = f"https://{domain}" + try: + return _normalize_cors_origin(domain) + except ValueError: + # A malformed platform value must degrade to "no extra origin", never + # crash startup — this runs while the ASGI app is being constructed. + return None def _normalize_cors_origin(raw_origin: str) -> str | None: diff --git a/tests/test_api.py b/tests/test_api.py index 33f498f..88e7f07 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -9,6 +9,7 @@ from fastapi.testclient import TestClient from app.build_info import BRANCH_ENV_VARS, COMMIT_SHA_ENV_VARS, DEPLOYMENT_ID_ENV_VARS +from app.cors import DEFAULT_CORS_ORIGINS from app.main import app, controller, cors_origins_from_env, scenario_saves from app.schemas.simulation import SimulationConfig from app.regengine_client import LiveIngestResult, LiveRegEngineDeliveryError @@ -201,6 +202,42 @@ def test_cors_origins_can_be_configured_without_wildcard_credentials(monkeypatch cors_origins_from_env() +def test_cors_allowlist_always_trusts_the_platform_domain(monkeypatch): + monkeypatch.setenv("RAILWAY_PUBLIC_DOMAIN", "demo.up.railway.app") + + # With no explicit config, the platform origin joins the local defaults. + monkeypatch.delenv("REGENGINE_CORS_ORIGINS", raising=False) + assert cors_origins_from_env() == [ + *DEFAULT_CORS_ORIGINS, + "https://demo.up.railway.app", + ] + + # An explicit-but-stale list can no longer lock the service out of its own + # domain — the regression that kept the nightly smokes red after the + # August 2026 cutover (issues #80/#81). + monkeypatch.setenv("REGENGINE_CORS_ORIGINS", "https://old.up.railway.app") + assert cors_origins_from_env() == [ + "https://old.up.railway.app", + "https://demo.up.railway.app", + ] + + # No duplicate when the platform origin is already configured. + monkeypatch.setenv("REGENGINE_CORS_ORIGINS", "https://demo.up.railway.app") + assert cors_origins_from_env() == ["https://demo.up.railway.app"] + + +def test_cors_platform_domain_never_crashes_startup(monkeypatch): + monkeypatch.delenv("REGENGINE_CORS_ORIGINS", raising=False) + + # A malformed platform value degrades to "no extra origin", not a raise — + # this path runs while the ASGI app is being constructed. + monkeypatch.setenv("RAILWAY_PUBLIC_DOMAIN", "demo.up.railway.app/?bad=1") + assert cors_origins_from_env() == list(DEFAULT_CORS_ORIGINS) + + monkeypatch.setenv("RAILWAY_PUBLIC_DOMAIN", " ") + assert cors_origins_from_env() == list(DEFAULT_CORS_ORIGINS) + + def test_basic_auth_is_optional_but_enforced_when_configured(monkeypatch): health_response = client.get("/api/health") assert health_response.status_code == 200