diff --git a/surfsense_backend/app/app.py b/surfsense_backend/app/app.py index 95aa1bf5d..a1795853a 100644 --- a/surfsense_backend/app/app.py +++ b/surfsense_backend/app/app.py @@ -114,8 +114,19 @@ def _surfsense_error_handler(request: Request, exc: SurfSenseError) -> JSONRespo def _http_exception_handler(request: Request, exc: HTTPException) -> JSONResponse: - """Wrap FastAPI/Starlette HTTPExceptions into the standard envelope.""" + """Wrap FastAPI/Starlette HTTPExceptions into the standard envelope. + + 5xx sanitization policy: + - 500 responses are sanitized (replaced with ``GENERIC_5XX_MESSAGE``) because + they usually wrap raw internal errors and may leak sensitive info. + - Other 5xx statuses (501, 502, 503, 504, ...) are raised explicitly by + route code to communicate a specific, user-safe operational state + (e.g. 503 "Page purchases are temporarily unavailable."). Those details + are preserved so the frontend can render them, but the error is still + logged server-side. + """ rid = _get_request_id(request) + should_sanitize = exc.status_code == 500 # Structured dict details (e.g. {"code": "CAPTCHA_REQUIRED", "message": "..."}) # are preserved so the frontend can parse them. @@ -130,6 +141,7 @@ def _http_exception_handler(request: Request, exc: HTTPException) -> JSONRespons exc.status_code, message, ) + if should_sanitize: message = GENERIC_5XX_MESSAGE err_code = "INTERNAL_ERROR" body = { @@ -158,6 +170,7 @@ def _http_exception_handler(request: Request, exc: HTTPException) -> JSONRespons exc.status_code, detail, ) + if should_sanitize: detail = GENERIC_5XX_MESSAGE code = _status_to_code(exc.status_code, detail) return _build_error_response(exc.status_code, detail, code=code, request_id=rid) diff --git a/surfsense_backend/tests/unit/test_error_contract.py b/surfsense_backend/tests/unit/test_error_contract.py index 8a1605dd1..81ec08b2d 100644 --- a/surfsense_backend/tests/unit/test_error_contract.py +++ b/surfsense_backend/tests/unit/test_error_contract.py @@ -70,6 +70,20 @@ def _make_test_app(): async def raise_http_500(): raise HTTPException(status_code=500, detail="secret db password leaked") + @app.get("/http-503") + async def raise_http_503(): + raise HTTPException( + status_code=503, + detail="Page purchases are temporarily unavailable.", + ) + + @app.get("/http-502") + async def raise_http_502(): + raise HTTPException( + status_code=502, + detail="Unable to create Stripe checkout session.", + ) + @app.get("/surfsense-connector") async def raise_connector(): raise ConnectorError("GitHub API returned 401") @@ -184,6 +198,20 @@ class TestHTTPExceptionHandler: assert body["error"]["message"] == GENERIC_5XX_MESSAGE assert body["error"]["code"] == "INTERNAL_ERROR" + def test_503_preserves_detail(self, client): + # Intentional 503s (e.g. feature flag off) must surface the developer + # message so the frontend can render actionable copy. + body = _assert_envelope(client.get("/http-503"), 503) + assert ( + body["error"]["message"] == "Page purchases are temporarily unavailable." + ) + assert body["error"]["message"] != GENERIC_5XX_MESSAGE + + def test_502_preserves_detail(self, client): + body = _assert_envelope(client.get("/http-502"), 502) + assert body["error"]["message"] == "Unable to create Stripe checkout session." + assert body["error"]["message"] != GENERIC_5XX_MESSAGE + # --------------------------------------------------------------------------- # SurfSenseError hierarchy