From b6607b200942472999e9fe1605bbdaadb561ef1a Mon Sep 17 00:00:00 2001 From: Tyler Date: Sun, 21 Jun 2026 16:29:59 -0600 Subject: [PATCH] feat(api): harden CORS and surface 500/409 errors with proper CORS headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent improvements that fix real browser-facing bugs: 1. CORS: allow 127.0.0.1:5173 in addition to localhost:5173. Both resolve to the same Vite dev server but CORS treats them as distinct origins, so tabs opened via the IP form silently break. 2. CORS: support CYCLONE_ALLOWED_ORIGINS env var (comma-separated) for LAN / staging hosts. The middleware reads it at module import. 3. Catch-all exception handler: returns JSON 500 with CORS headers instead of a bare Uvicorn text/plain response. Without this, any unhandled exception is misreported by browsers as a CORS error because the body can't be read without the allow-origin header. 4. IntegrityError → 409: when (batch_id, patient_control_number) is UNIQUE-constrained and a duplicate collides, return 409 with the batch id instead of letting the exception 500. Same problem as (3) for the most common ingest failure mode. Tests added: - test_cors_headers_present_for_loopback_ip - test_cors_extra_origins_via_env (uses importlib.reload because the allow-list is built at module import) --- backend/src/cyclone/api.py | 95 +++++++++++++++++++++++++++++++++++--- backend/tests/test_api.py | 48 +++++++++++++++++++ 2 files changed, 137 insertions(+), 6 deletions(-) diff --git a/backend/src/cyclone/api.py b/backend/src/cyclone/api.py index 7e558ca..0a7dd6f 100644 --- a/backend/src/cyclone/api.py +++ b/backend/src/cyclone/api.py @@ -9,8 +9,11 @@ The frontend (Vite/React on http://localhost:5173) uploads an X12 file via — one envelope, one line per claim/payout, then one summary — so the UI can render records incrementally as they're produced. -CORS is configured to allow the Vite dev origin (``http://localhost:5173``) -plus GET/POST with any header. +CORS is configured to allow the Vite dev origins (``http://localhost:5173`` +and ``http://127.0.0.1:5173`` — both forms are valid dev-server addresses +but CORS treats them as distinct origins) plus GET/POST with any header. +Additional origins (LAN IPs, staging hosts) can be appended via the +``CYCLONE_ALLOWED_ORIGINS`` env var as a comma-separated list. """ from __future__ import annotations @@ -19,6 +22,7 @@ import csv import io import json import logging +import os import uuid from datetime import datetime, timezone from contextlib import asynccontextmanager @@ -33,6 +37,7 @@ from pydantic import ValidationError from cyclone import __version__, db from cyclone.db import Claim, ClaimState, Remittance from sqlalchemy import desc, or_ +from sqlalchemy.exc import IntegrityError from cyclone.inbox_state import apply_999_rejections from cyclone.inbox_state_277ca import apply_277ca_rejections from cyclone.audit_log import AuditEvent, append_event, verify_chain @@ -214,7 +219,17 @@ PAYER_FACTORIES_835: dict[str, Any] = { "generic_835": PayerConfig835.generic_835, } -VITE_DEV_ORIGIN = "http://localhost:5173" +# Allow both common dev-server origins. ``localhost`` and ``127.0.0.1`` +# resolve to the same Vite dev server but CORS treats them as distinct +# origins, so the allow-list has to list both — otherwise a tab opened +# via the IP form gets blocked even though the dev server is identical. +VITE_DEV_ORIGINS: list[str] = [ + "http://localhost:5173", + "http://127.0.0.1:5173", +] +_extra_origins = os.environ.get("CYCLONE_ALLOWED_ORIGINS", "").strip() +if _extra_origins: + VITE_DEV_ORIGINS.extend(o.strip() for o in _extra_origins.split(",") if o.strip()) app = FastAPI( title="Cyclone 837P / 835 Parser API", @@ -225,7 +240,7 @@ app = FastAPI( app.add_middleware( CORSMiddleware, - allow_origins=[VITE_DEV_ORIGIN], + allow_origins=VITE_DEV_ORIGINS, allow_credentials=False, allow_methods=["GET", "POST"], allow_headers=["*"], @@ -282,6 +297,35 @@ def _resolve_payer_835(name: str) -> PayerConfig835: return PAYER_FACTORIES_835[name]() +# --------------------------------------------------------------------------- # +# Catch-all exception handler +# --------------------------------------------------------------------------- # +# +# Without this, any exception that escapes a route handler is rendered by +# Uvicorn as a bare ``500 Internal Server Error`` text/plain response with +# NO CORS headers. Browsers reading such a response report it as a CORS +# error (``Origin ... is not allowed by Access-Control-Allow-Origin``) +# because they cannot read the body without the CORS allow-origin header +# — even though the actual failure was on the server. We catch everything +# here, log it, and return a JSON error with the request's Origin echoed +# back so the browser can surface the real message. +@app.exception_handler(Exception) +async def _unhandled_exception_handler(request: Request, exc: Exception) -> JSONResponse: + log.exception( + "Unhandled exception in %s %s", request.method, request.url.path + ) + headers: dict[str, str] = {} + origin = request.headers.get("origin", "") + if origin: + headers["Access-Control-Allow-Origin"] = origin + headers["Vary"] = "Origin" + return JSONResponse( + status_code=500, + content={"error": "Internal server error", "detail": str(exc)}, + headers=headers, + ) + + @app.post("/api/parse-837") async def parse_837( request: Request, @@ -345,7 +389,30 @@ async def parse_837( parsed_at=utcnow(), result=result, ) - store.add(rec, event_bus=request.app.state.event_bus) + try: + store.add(rec, event_bus=request.app.state.event_bus) + except IntegrityError as exc: + # ``(batch_id, patient_control_number)`` is UNIQUE — fires when a + # single batch file contains the same CLM01 control number twice, + # or when the same claim id has already been ingested in a prior + # batch. Surface as 409 with the batch id so the caller can look + # it up; do NOT 500 (a 500 without CORS headers is misreported by + # browsers as a CORS error and hides the real cause). + log.warning("Duplicate claim while persisting batch %s: %s", rec.id, exc) + return JSONResponse( + status_code=409, + content={ + "error": "Duplicate claim", + "detail": ( + "This file (or one previously ingested with the same " + "claim control number) collides with an existing " + "record. Inspect the file for duplicate CLM01 " + "control numbers, or remove the existing batch " + "before retrying." + ), + "batch_id": rec.id, + }, + ) if _client_wants_json(request): body = json.loads(result.model_dump_json()) @@ -516,7 +583,23 @@ async def parse_835_endpoint( parsed_at=utcnow(), result=result, ) - store.add(rec, event_bus=request.app.state.event_bus) + try: + store.add(rec, event_bus=request.app.state.event_bus) + except IntegrityError as exc: + log.warning("Duplicate remittance while persisting batch %s: %s", rec.id, exc) + return JSONResponse( + status_code=409, + content={ + "error": "Duplicate remittance", + "detail": ( + "This 835 file (or one previously ingested with the " + "same payer claim control number) collides with an " + "existing record. Remove the existing remittance " + "before retrying." + ), + "batch_id": rec.id, + }, + ) if _client_wants_json(request): body = json.loads(result.model_dump_json()) diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 6517d20..29f98e1 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -160,3 +160,51 @@ def test_cors_headers_present(client: TestClient): ) assert resp.headers.get("access-control-allow-origin") == "http://localhost:5173" assert "POST" in resp.headers.get("access-control-allow-methods", "").upper() + + +def test_cors_headers_present_for_loopback_ip(client: TestClient): + # ``http://127.0.0.1:5173`` is a distinct origin from + # ``http://localhost:5173`` per the CORS spec, even though both resolve + # to the same Vite dev server. Both must be allow-listed or tabs opened + # via the IP form silently break. + resp = client.options( + "/api/parse-837", + headers={ + "Origin": "http://127.0.0.1:5173", + "Access-Control-Request-Method": "POST", + "Access-Control-Request-Headers": "content-type", + }, + ) + assert resp.headers.get("access-control-allow-origin") == "http://127.0.0.1:5173" + + +def test_cors_extra_origins_via_env(client: TestClient, monkeypatch): + # LAN / staging hosts opt in via CYCLONE_ALLOWED_ORIGINS. The env var + # is a comma-separated list; the middleware must reflect each entry. + # The allow-list is built at module import, so we re-execute the + # module under the env var and build a TestClient against the + # reloaded app. + monkeypatch.setenv( + "CYCLONE_ALLOWED_ORIGINS", "http://192.168.1.42:5173,https://staging.example.com" + ) + import importlib + from cyclone import api as api_module + from fastapi.testclient import TestClient as _TC + importlib.reload(api_module) + try: + with _TC(api_module.app) as tc: + for origin in ("http://192.168.1.42:5173", "https://staging.example.com"): + resp = tc.options( + "/api/parse-837", + headers={ + "Origin": origin, + "Access-Control-Request-Method": "POST", + "Access-Control-Request-Headers": "content-type", + }, + ) + assert resp.headers.get("access-control-allow-origin") == origin + finally: + monkeypatch.delenv("CYCLONE_ALLOWED_ORIGINS", raising=False) + # Reload once more so the module-level allow-list returns to its + # default for any test that imports `cyclone.api` after this one. + importlib.reload(api_module)