feat(api): pre-release security hardening bundle

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"]

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:

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
@@ -293,6 +295,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,
@@ -301,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():

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",
 ]

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),

openrag/api/middleware/security_headers.py

@@ -0,0 +1,60 @@
+"""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, Response
+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"
+
+
+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)
+        return apply_security_headers(response, request)

openrag/chainlit_api.py

@@ -18,8 +18,10 @@
 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
 from api.routers.user.download import router as download_router
 from chainlit.utils import mount_chainlit
 from core.config import load_config
@@ -89,6 +91,15 @@ 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)
+# 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),

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
 

tests/unit/api/middleware/test_security_headers.py

@@ -0,0 +1,66 @@
+"""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"
+
+
+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"

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"]