From 93c6e166aeeb7f4317bdfff27f41a05dfe02d5cb Mon Sep 17 00:00:00 2001 From: andyne13 Date: Thu, 2 Jul 2026 23:55:08 +0200 Subject: [PATCH 1/3] feat(api): pre-release security hardening bundle - Add SecurityHeadersMiddleware (X-Content-Type-Options, X-Frame-Options, Referrer-Policy, and HSTS over HTTPS) applied to every response. - Reject a wildcard CORS origin when credentials are enabled (sanitize_cors_origins), so a misconfigured CORS_EXTRA_ORIGINS=* cannot reflect credentialed any-origin access. - Parse RATE_LIMIT_* only when rate limiting is enabled, so a malformed value no longer crashes boot when the feature is off. Adds unit tests for each; full unit suite green. --- openrag/api/cors_config.py | 33 +++++++++++++ openrag/api/main.py | 9 ++++ openrag/api/middleware/__init__.py | 2 + openrag/api/middleware/rate_limit.py | 10 ++-- openrag/api/middleware/security_headers.py | 48 +++++++++++++++++++ tests/unit/api/middleware/test_rate_limit.py | 8 ++++ .../api/middleware/test_security_headers.py | 47 ++++++++++++++++++ tests/unit/api/test_cors_config.py | 23 +++++++++ 8 files changed, 177 insertions(+), 3 deletions(-) create mode 100644 openrag/api/cors_config.py create mode 100644 openrag/api/middleware/security_headers.py create mode 100644 tests/unit/api/middleware/test_security_headers.py create mode 100644 tests/unit/api/test_cors_config.py diff --git a/openrag/api/cors_config.py b/openrag/api/cors_config.py new file mode 100644 index 000000000..f57a7c56c --- /dev/null +++ b/openrag/api/cors_config.py @@ -0,0 +1,33 @@ +"""CORS origin sanitisation. + +A wildcard origin combined with credentials is unsafe: Starlette's +``CORSMiddleware`` reflects the request ``Origin`` *with* credentials in that +case, which defeats the allowlist and lets any site make credentialed calls. +This drops any ``"*"`` when credentials are enabled so a misconfigured +``CORS_EXTRA_ORIGINS=*`` cannot silently open that hole. +""" + +from __future__ import annotations + +from core.utils.logging import get_logger + +logger = get_logger() + + +def sanitize_cors_origins(origins: list[str], *, allow_credentials: bool) -> list[str]: + """Return ``origins`` with any wildcard removed when credentials are on. + + When ``allow_credentials`` is ``False`` the list is returned unchanged (a + wildcard is safe without credentials). When it is ``True`` and a ``"*"`` is + present, the wildcard is dropped and a warning is logged. + """ + if not allow_credentials or "*" not in origins: + return origins + logger.warning( + "Dropping wildcard '*' from CORS origins because credentials are enabled; " + "list explicit origins in CORS_EXTRA_ORIGINS" + ) + return [origin for origin in origins if origin != "*"] + + +__all__ = ["sanitize_cors_origins"] diff --git a/openrag/api/main.py b/openrag/api/main.py index cec7a8395..824139f28 100644 --- a/openrag/api/main.py +++ b/openrag/api/main.py @@ -28,6 +28,7 @@ import ray import uvicorn +from api.cors_config import sanitize_cors_origins from api.dependencies.auth import require_admin from api.error_handlers import register_error_handlers from api.middleware import ( @@ -36,6 +37,7 @@ RateLimitMiddleware, RequestIdMiddleware, RequestTimeoutMiddleware, + SecurityHeadersMiddleware, ) from api.routers.admin.cluster import router as actors_router from api.routers.admin.indexing import router as indexer_router @@ -279,6 +281,9 @@ def custom_openapi(): app.add_middleware(RequestIdMiddleware) app.add_middleware(RequestTimeoutMiddleware) app.add_middleware(InstrumentationMiddleware) +# Registered last among the app stack so it wraps outermost and stamps the +# baseline security headers on every response that flows out. +app.add_middleware(SecurityHeadersMiddleware) # Phase 10B centralises the OpenRAGError + generic Exception handlers in # api/error_handlers.py — the inline decorators that used to live here @@ -293,6 +298,10 @@ def custom_openapi(): *CORS_EXTRA_ORIGINS, ] +# Credentials + a wildcard origin is unsafe (Starlette reflects the Origin with +# credentials), so drop any "*" from a misconfigured CORS_EXTRA_ORIGINS=*. +allow_origins = sanitize_cors_origins(allow_origins, allow_credentials=True) + app.add_middleware( CORSMiddleware, allow_origins=allow_origins, diff --git a/openrag/api/middleware/__init__.py b/openrag/api/middleware/__init__.py index 44dcb10f6..7cbaee9a4 100644 --- a/openrag/api/middleware/__init__.py +++ b/openrag/api/middleware/__init__.py @@ -9,6 +9,7 @@ from api.middleware.rate_limit import RateLimitMiddleware from api.middleware.request_id import REQUEST_ID_HEADER, RequestIdMiddleware from api.middleware.request_timeout import RequestTimeoutMiddleware +from api.middleware.security_headers import SecurityHeadersMiddleware __all__ = [ "AuthMiddleware", @@ -17,4 +18,5 @@ "RequestIdMiddleware", "REQUEST_ID_HEADER", "RequestTimeoutMiddleware", + "SecurityHeadersMiddleware", ] diff --git a/openrag/api/middleware/rate_limit.py b/openrag/api/middleware/rate_limit.py index 9384ab6af..424e9670b 100644 --- a/openrag/api/middleware/rate_limit.py +++ b/openrag/api/middleware/rate_limit.py @@ -41,10 +41,14 @@ def __init__(self, app): super().__init__(app) self.enabled = _env_flag("RATE_LIMIT_ENABLED", True) self._limiter = MovingWindowRateLimiter(MemoryStorage()) - self._default = parse(os.environ.get("RATE_LIMIT_DEFAULT", "600/minute")) - self._auth = parse(os.environ.get("RATE_LIMIT_AUTH", "60/minute")) - self._chat = parse(os.environ.get("RATE_LIMIT_CHAT", "120/minute")) + # Only parse the limit configs when rate limiting is enabled: a malformed + # RATE_LIMIT_* value must not crash boot when the feature is turned off + # (``dispatch`` short-circuits before touching these when disabled). + self._default = self._auth = self._chat = None if self.enabled: + self._default = parse(os.environ.get("RATE_LIMIT_DEFAULT", "600/minute")) + self._auth = parse(os.environ.get("RATE_LIMIT_AUTH", "60/minute")) + self._chat = parse(os.environ.get("RATE_LIMIT_CHAT", "120/minute")) logger.info( "Rate limiting enabled", default=str(self._default), diff --git a/openrag/api/middleware/security_headers.py b/openrag/api/middleware/security_headers.py new file mode 100644 index 000000000..379098888 --- /dev/null +++ b/openrag/api/middleware/security_headers.py @@ -0,0 +1,48 @@ +"""Attach conservative security headers to every response. + +Sets baseline hardening headers app-wide: + +- ``X-Content-Type-Options: nosniff`` — stop browser MIME-sniffing. +- ``X-Frame-Options: SAMEORIGIN`` — clickjacking protection. ``SAMEORIGIN`` + (not ``DENY``) so same-origin embedding of the mounted UIs still works. +- ``Referrer-Policy: strict-origin-when-cross-origin`` — don't leak full URLs + (which can carry a ``?token=``) to third-party origins. +- ``Strict-Transport-Security`` — sent only when the request is already HTTPS + (browsers ignore it over plain HTTP anyway), so local HTTP development is + unaffected. + +A full Content-Security-Policy is intentionally omitted: the mounted admin UI +and Chainlit rely on inline scripts/styles, so a blanket script CSP would break +them and needs per-UI tuning. Frame protection is covered by X-Frame-Options. + +``setdefault`` is used so a route that sets a stricter value of its own (e.g. +the download route's ``X-Content-Type-Options``) is preserved. +""" + +from __future__ import annotations + +from fastapi import Request +from starlette.middleware.base import BaseHTTPMiddleware + +_HSTS_VALUE = "max-age=63072000; includeSubDomains" + + +def _is_https(request: Request) -> bool: + if request.url.scheme == "https": + return True + xfp = request.headers.get("x-forwarded-proto", "") + return xfp.split(",", 1)[0].strip().lower() == "https" + + +class SecurityHeadersMiddleware(BaseHTTPMiddleware): + """Add baseline security response headers to every response.""" + + async def dispatch(self, request: Request, call_next): + response = await call_next(request) + headers = response.headers + headers.setdefault("X-Content-Type-Options", "nosniff") + headers.setdefault("X-Frame-Options", "SAMEORIGIN") + headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin") + if _is_https(request): + headers.setdefault("Strict-Transport-Security", _HSTS_VALUE) + return response diff --git a/tests/unit/api/middleware/test_rate_limit.py b/tests/unit/api/middleware/test_rate_limit.py index f3d88dcea..915e1997a 100644 --- a/tests/unit/api/middleware/test_rate_limit.py +++ b/tests/unit/api/middleware/test_rate_limit.py @@ -78,6 +78,14 @@ def test_disabled_passes_through(monkeypatch): assert client.get("/v1/chat").status_code == 200 +def test_disabled_ignores_malformed_config(monkeypatch): + # A malformed RATE_LIMIT_* value must not crash boot when limiting is off: + # the middleware skips parsing the limits entirely when disabled. + app = _build_app(monkeypatch, RATE_LIMIT_ENABLED="false", RATE_LIMIT_DEFAULT="not-a-valid-limit") + client = TestClient(app) + assert client.get("/other").status_code == 200 + + def test_is_admin_true_for_admin_user_dict(): assert RateLimitMiddleware._is_admin(_make_request(user={"id": 1, "is_admin": True})) is True diff --git a/tests/unit/api/middleware/test_security_headers.py b/tests/unit/api/middleware/test_security_headers.py new file mode 100644 index 000000000..330945e72 --- /dev/null +++ b/tests/unit/api/middleware/test_security_headers.py @@ -0,0 +1,47 @@ +"""Tests for the baseline security-headers middleware.""" + +from api.middleware.security_headers import SecurityHeadersMiddleware +from starlette.applications import Starlette +from starlette.requests import Request +from starlette.responses import PlainTextResponse +from starlette.routing import Route +from starlette.testclient import TestClient + + +def _app(): + async def ok(_request: Request): + return PlainTextResponse("ok") + + app = Starlette(routes=[Route("/", ok)]) + app.add_middleware(SecurityHeadersMiddleware) + return TestClient(app) + + +def test_sets_baseline_headers(): + resp = _app().get("/") + assert resp.headers["X-Content-Type-Options"] == "nosniff" + assert resp.headers["X-Frame-Options"] == "SAMEORIGIN" + assert resp.headers["Referrer-Policy"] == "strict-origin-when-cross-origin" + + +def test_no_hsts_over_plain_http(): + # TestClient defaults to http:// — HSTS must not be sent so local HTTP dev + # (and http health probes) are unaffected. + resp = _app().get("/") + assert "Strict-Transport-Security" not in resp.headers + + +def test_hsts_sent_when_forwarded_proto_https(): + resp = _app().get("/", headers={"X-Forwarded-Proto": "https"}) + assert resp.headers["Strict-Transport-Security"] == "max-age=63072000; includeSubDomains" + + +def test_does_not_clobber_route_set_header(): + async def strict(_request: Request): + return PlainTextResponse("x", headers={"X-Frame-Options": "DENY"}) + + app = Starlette(routes=[Route("/strict", strict)]) + app.add_middleware(SecurityHeadersMiddleware) + resp = TestClient(app).get("/strict") + # setdefault must preserve a stricter value a route chose for itself. + assert resp.headers["X-Frame-Options"] == "DENY" diff --git a/tests/unit/api/test_cors_config.py b/tests/unit/api/test_cors_config.py new file mode 100644 index 000000000..1fc5de1f1 --- /dev/null +++ b/tests/unit/api/test_cors_config.py @@ -0,0 +1,23 @@ +"""Tests for CORS origin sanitisation.""" + +from api.cors_config import sanitize_cors_origins + + +def test_drops_wildcard_when_credentialed(): + origins = ["https://app.example.com", "*"] + assert sanitize_cors_origins(origins, allow_credentials=True) == ["https://app.example.com"] + + +def test_keeps_wildcard_when_no_credentials(): + origins = ["*"] + assert sanitize_cors_origins(origins, allow_credentials=False) == ["*"] + + +def test_noop_without_wildcard(): + origins = ["https://a.example.com", "https://b.example.com"] + assert sanitize_cors_origins(origins, allow_credentials=True) == origins + + +def test_drops_every_wildcard_occurrence(): + origins = ["*", "https://a.example.com", "*"] + assert sanitize_cors_origins(origins, allow_credentials=True) == ["https://a.example.com"] From af6593c1c57e4b46ea464515f342ef7195696a32 Mon Sep 17 00:00:00 2001 From: andyne13 Date: Fri, 3 Jul 2026 00:25:24 +0200 Subject: [PATCH 2/3] fix(api): extend security headers to preflights, chainlit, and 500s Addresses review feedback on the SecurityHeadersMiddleware coverage: - Register the middleware after CORS (outermost) so it also stamps CORS preflight responses. - Add the same middleware to the standalone Chainlit app so the Ray Serve Chainlit origin gets the baseline headers. - Apply the headers in the unhandled-500 handler, since Starlette generates those responses in the outer ServerErrorMiddleware, outside the user stack. Header logic is extracted into a shared apply_security_headers() used by both the middleware and the 500 handler. Adds a test for the 500 path. --- openrag/api/error_handlers.py | 7 ++++- openrag/api/main.py | 8 ++++-- openrag/api/middleware/security_headers.py | 28 +++++++++++++------ openrag/chainlit_api.py | 5 ++++ .../api/middleware/test_security_headers.py | 19 +++++++++++++ 5 files changed, 55 insertions(+), 12 deletions(-) diff --git a/openrag/api/error_handlers.py b/openrag/api/error_handlers.py index d5aa628ce..694c790c9 100644 --- a/openrag/api/error_handlers.py +++ b/openrag/api/error_handlers.py @@ -25,6 +25,7 @@ from __future__ import annotations +from api.middleware.security_headers import apply_security_headers from core.utils.exceptions import ( AuthenticationError, AuthError, @@ -136,10 +137,14 @@ async def unhandled_exception_handler(request: Request, exc: Exception) -> JSONR request_id = _get_request_id(request) if request_id is not None: extra["request_id"] = request_id - return JSONResponse( + response = JSONResponse( status_code=500, content={"detail": "[UNEXPECTED_ERROR]: An unexpected error occurred", "extra": extra}, ) + # Starlette generates unhandled-500s in its outer ServerErrorMiddleware, + # which sits outside the user middleware stack, so SecurityHeadersMiddleware + # never sees this response — set the baseline headers here too. + return apply_security_headers(response, request) def register_error_handlers(app: FastAPI) -> None: diff --git a/openrag/api/main.py b/openrag/api/main.py index 824139f28..0624ad8c0 100644 --- a/openrag/api/main.py +++ b/openrag/api/main.py @@ -281,9 +281,6 @@ def custom_openapi(): app.add_middleware(RequestIdMiddleware) app.add_middleware(RequestTimeoutMiddleware) app.add_middleware(InstrumentationMiddleware) -# Registered last among the app stack so it wraps outermost and stamps the -# baseline security headers on every response that flows out. -app.add_middleware(SecurityHeadersMiddleware) # Phase 10B centralises the OpenRAGError + generic Exception handlers in # api/error_handlers.py — the inline decorators that used to live here @@ -310,6 +307,11 @@ def custom_openapi(): allow_headers=["*"], ) +# Registered last so it wraps CORS and every other layer: this stamps the +# baseline security headers on all responses, including CORS preflights that +# CORSMiddleware short-circuits before they reach the inner stack. +app.add_middleware(SecurityHeadersMiddleware) + @app.get("/", include_in_schema=False) def root_redirect(): diff --git a/openrag/api/middleware/security_headers.py b/openrag/api/middleware/security_headers.py index 379098888..8d9aa5c37 100644 --- a/openrag/api/middleware/security_headers.py +++ b/openrag/api/middleware/security_headers.py @@ -21,7 +21,7 @@ from __future__ import annotations -from fastapi import Request +from fastapi import Request, Response from starlette.middleware.base import BaseHTTPMiddleware _HSTS_VALUE = "max-age=63072000; includeSubDomains" @@ -34,15 +34,27 @@ def _is_https(request: Request) -> bool: return xfp.split(",", 1)[0].strip().lower() == "https" +def apply_security_headers(response: Response, request: Request) -> Response: + """Set the baseline security headers on ``response`` (idempotent). + + Shared by :class:`SecurityHeadersMiddleware` and the 500 error handler: + Starlette generates unhandled-500s in its outer ``ServerErrorMiddleware``, + which is outside the user middleware stack, so those responses never pass + back through the middleware and must be covered explicitly. ``setdefault`` + preserves a stricter value a route set for itself. + """ + headers = response.headers + headers.setdefault("X-Content-Type-Options", "nosniff") + headers.setdefault("X-Frame-Options", "SAMEORIGIN") + headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin") + if _is_https(request): + headers.setdefault("Strict-Transport-Security", _HSTS_VALUE) + return response + + class SecurityHeadersMiddleware(BaseHTTPMiddleware): """Add baseline security response headers to every response.""" async def dispatch(self, request: Request, call_next): response = await call_next(request) - headers = response.headers - headers.setdefault("X-Content-Type-Options", "nosniff") - headers.setdefault("X-Frame-Options", "SAMEORIGIN") - headers.setdefault("Referrer-Policy", "strict-origin-when-cross-origin") - if _is_https(request): - headers.setdefault("Strict-Transport-Security", _HSTS_VALUE) - return response + return apply_security_headers(response, request) diff --git a/openrag/chainlit_api.py b/openrag/chainlit_api.py index 739b3f616..7f830f41e 100644 --- a/openrag/chainlit_api.py +++ b/openrag/chainlit_api.py @@ -20,6 +20,7 @@ from api.chainlit_assets import mount_chainlit_root_assets from api.middleware.auth import AuthMiddleware from api.middleware.request_id import RequestIdMiddleware +from api.middleware.security_headers import SecurityHeadersMiddleware from api.routers.user.download import router as download_router from chainlit.utils import mount_chainlit from core.config import load_config @@ -89,6 +90,10 @@ def _get_auth_service(request): # Registered after AuthMiddleware so it wraps it — i.e. runs first and sets # ``original_token`` before auth reads it (add_middleware adds outermost-last). app.add_middleware(RequestIdMiddleware) +# Ray Serve mode serves this Chainlit app on its own port, so it needs the same +# baseline security headers the mounted deployment gets from api.main. Added +# last → outermost, covering every response on this origin. +app.add_middleware(SecurityHeadersMiddleware) # Ray Serve mode runs the API and Chainlit on separate ports. Source previews # rewrite their file download links to the browser origin (the Chainlit host), diff --git a/tests/unit/api/middleware/test_security_headers.py b/tests/unit/api/middleware/test_security_headers.py index 330945e72..eab17fafa 100644 --- a/tests/unit/api/middleware/test_security_headers.py +++ b/tests/unit/api/middleware/test_security_headers.py @@ -45,3 +45,22 @@ async def strict(_request: Request): resp = TestClient(app).get("/strict") # setdefault must preserve a stricter value a route chose for itself. assert resp.headers["X-Frame-Options"] == "DENY" + + +def test_unhandled_500_still_gets_headers(): + # Unhandled exceptions are turned into a 500 by Starlette's outer + # ServerErrorMiddleware, which sits outside the user middleware stack — so + # the 500 handler must apply the headers itself. + from api.error_handlers import register_error_handlers + + async def boom(_request: Request): + raise RuntimeError("boom") + + app = Starlette(routes=[Route("/boom", boom)]) + register_error_handlers(app) + app.add_middleware(SecurityHeadersMiddleware) + + resp = TestClient(app, raise_server_exceptions=False).get("/boom") + assert resp.status_code == 500 + assert resp.headers["X-Content-Type-Options"] == "nosniff" + assert resp.headers["X-Frame-Options"] == "SAMEORIGIN" From 14d1b67f3472ab5fc5411fe2b0c09d7ffa3a42c9 Mon Sep 17 00:00:00 2001 From: andyne13 Date: Fri, 3 Jul 2026 13:27:04 +0200 Subject: [PATCH 3/3] fix(chainlit): apply security headers to unhandled 500s Register the shared error handlers on the standalone Chainlit app so its unhandled-500 responses (produced by Starlette's outer ServerErrorMiddleware, outside the user middleware stack) get the same baseline security headers via apply_security_headers. Also gives the Chainlit app consistent OpenRAGError/500 shaping. --- openrag/chainlit_api.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/openrag/chainlit_api.py b/openrag/chainlit_api.py index 7f830f41e..bbe1189b8 100644 --- a/openrag/chainlit_api.py +++ b/openrag/chainlit_api.py @@ -18,6 +18,7 @@ from contextlib import asynccontextmanager from api.chainlit_assets import mount_chainlit_root_assets +from api.error_handlers import register_error_handlers from api.middleware.auth import AuthMiddleware from api.middleware.request_id import RequestIdMiddleware from api.middleware.security_headers import SecurityHeadersMiddleware @@ -94,6 +95,11 @@ def _get_auth_service(request): # baseline security headers the mounted deployment gets from api.main. Added # last → outermost, covering every response on this origin. app.add_middleware(SecurityHeadersMiddleware) +# Unhandled 500s are produced by Starlette's outer ServerErrorMiddleware, which +# sits outside the user middleware stack — so the SecurityHeadersMiddleware above +# never sees them. register_error_handlers' 500 handler applies the same baseline +# headers (via apply_security_headers), covering that path on this origin too. +register_error_handlers(app) # Ray Serve mode runs the API and Chainlit on separate ports. Source previews # rewrite their file download links to the browser origin (the Chainlit host),