From b2c042779b67f7719aed1a2ff8cd1b6126694383 Mon Sep 17 00:00:00 2001 From: Shriraj Date: Thu, 30 Jul 2026 23:25:07 +0530 Subject: [PATCH 1/2] feat(middleware): add failure recovery to security middleware (#920) --- backend/middleware/csrf.py | 34 ++++++++++++++++++++++------------ backend/tests/test_csrf.py | 24 ++++++++++++++++++++++++ docs/csrf-protection.md | 9 +++++++++ 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/backend/middleware/csrf.py b/backend/middleware/csrf.py index 798ed3e0..7685f1cb 100644 --- a/backend/middleware/csrf.py +++ b/backend/middleware/csrf.py @@ -88,22 +88,32 @@ async def dispatch(self, request: Request, call_next): if request.method in _SAFE_METHODS: return await call_next(request) - origin = _origin_from_header(request) - - if origin is None: - # No Origin / Referer — allow (same-origin or non-browser client). - return await call_next(request) - - if origin not in self._allowed: - logger.warning( - "CSRF check failed: method=%s path=%s origin=%r not in allowlist", + try: + origin = _origin_from_header(request) + + if origin is not None and origin not in self._allowed: + logger.warning( + "CSRF check failed: method=%s path=%s origin=%r not in allowlist", + request.method, + request.url.path, + origin, + ) + return JSONResponse( + {"detail": "CSRF check failed: origin not allowed"}, + status_code=403, + ) + except Exception as exc: + logger.error( + "Failure recovery triggered in security middleware: method=%s path=%s error=%s", request.method, request.url.path, - origin, + exc, + exc_info=True, ) return JSONResponse( - {"detail": "CSRF check failed: origin not allowed"}, - status_code=403, + {"detail": "Security verification error: middleware failure recovery triggered"}, + status_code=500, ) return await call_next(request) + diff --git a/backend/tests/test_csrf.py b/backend/tests/test_csrf.py index 0b46b03d..5039f375 100644 --- a/backend/tests/test_csrf.py +++ b/backend/tests/test_csrf.py @@ -204,3 +204,27 @@ def test_plugin_run_attacker_origin_blocked(): headers={"Origin": "https://evil.com"}, ) assert r.status_code == 403 + + +# ── Failure Recovery Tests ─────────────────────────────────────────────────── + +from unittest.mock import patch + + +def test_security_middleware_failure_recovery_on_exception(): + """When an exception occurs during origin verification, middleware returns 500 fail-closed.""" + with patch("middleware.csrf._origin_from_header", side_effect=RuntimeError("Header extraction fault")): + r = client.post( + "/api/sessions/", + json={"title": "Failure recovery test"}, + ) + assert r.status_code == 500 + assert "Security verification error" in r.json().get("detail", "") + + +def test_security_middleware_failure_recovery_safe_methods(): + """Safe HTTP methods bypass origin processing and succeed even during header faults.""" + with patch("middleware.csrf._origin_from_header", side_effect=RuntimeError("Header extraction fault")): + r = client.get("/api/sessions/") + assert r.status_code == 200 + diff --git a/docs/csrf-protection.md b/docs/csrf-protection.md index 62229d87..f6e854fa 100644 --- a/docs/csrf-protection.md +++ b/docs/csrf-protection.md @@ -38,6 +38,11 @@ This significantly reduces the CSRF surface, but a residual risk exists: 3. **`Referer` fallback** — when `Origin` is absent but `Referer` is present, the header is normalised to `scheme://host` and checked against the same list. +4. **Failure recovery** — if an unexpected exception occurs during origin parsing + or validation, the middleware catches the exception, logs an error traceback, + and returns `HTTP 500` (`Security verification error: middleware failure recovery triggered`) + to guarantee a fail-closed security posture. + ### Integration: `backend/app.py` @@ -115,6 +120,10 @@ requests would break the application when the frontend is served by the same FastAPI process (production mode). It would also break direct API access from `curl` and the `pytest` test client — neither of which is a CSRF attack. +### Why Fail-Closed Failure Recovery? + +If header processing or origin validation fails unexpectedly (e.g. malformed headers or runtime errors during header inspection), allowing the request to proceed without validation could bypass security checks. Failing closed with an HTTP 500 error response and logging the traceback guarantees that security checks are strictly enforced. + ## References - [OWASP CSRF Prevention Cheat Sheet — Verifying Origin With Standard Headers](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#verifying-origin-with-standard-headers) From a162fe9cbab7f6eeb8d6c9600d898a1424137ba3 Mon Sep 17 00:00:00 2001 From: Shriraj Date: Thu, 30 Jul 2026 23:35:26 +0530 Subject: [PATCH 2/2] fix(middleware): use logger.exception for ruff lint compliance --- backend/middleware/csrf.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/backend/middleware/csrf.py b/backend/middleware/csrf.py index 7685f1cb..6f79a5e0 100644 --- a/backend/middleware/csrf.py +++ b/backend/middleware/csrf.py @@ -102,13 +102,11 @@ async def dispatch(self, request: Request, call_next): {"detail": "CSRF check failed: origin not allowed"}, status_code=403, ) - except Exception as exc: - logger.error( - "Failure recovery triggered in security middleware: method=%s path=%s error=%s", + except Exception: + logger.exception( + "Failure recovery triggered in security middleware: method=%s path=%s", request.method, request.url.path, - exc, - exc_info=True, ) return JSONResponse( {"detail": "Security verification error: middleware failure recovery triggered"},