mirror of
https://github.com/MODSetter/SurfSense.git
synced 2026-04-25 00:36:31 +02:00
feat: enhance HTTP exception handling with 5xx sanitization policy
- Updated the HTTP exception handler to sanitize 500 responses, replacing them with a generic message to prevent sensitive information leakage. - Preserved detailed messages for other 5xx statuses (e.g., 503, 502) to provide user-friendly feedback while logging the errors server-side. - Added unit tests to ensure that specific 5xx responses retain their detail for frontend rendering.
This commit is contained in:
parent
2703fd4ebe
commit
dbcea0d50b
2 changed files with 42 additions and 1 deletions
|
|
@ -114,8 +114,19 @@ def _surfsense_error_handler(request: Request, exc: SurfSenseError) -> JSONRespo
|
||||||
|
|
||||||
|
|
||||||
def _http_exception_handler(request: Request, exc: HTTPException) -> JSONResponse:
|
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)
|
rid = _get_request_id(request)
|
||||||
|
should_sanitize = exc.status_code == 500
|
||||||
|
|
||||||
# Structured dict details (e.g. {"code": "CAPTCHA_REQUIRED", "message": "..."})
|
# Structured dict details (e.g. {"code": "CAPTCHA_REQUIRED", "message": "..."})
|
||||||
# are preserved so the frontend can parse them.
|
# are preserved so the frontend can parse them.
|
||||||
|
|
@ -130,6 +141,7 @@ def _http_exception_handler(request: Request, exc: HTTPException) -> JSONRespons
|
||||||
exc.status_code,
|
exc.status_code,
|
||||||
message,
|
message,
|
||||||
)
|
)
|
||||||
|
if should_sanitize:
|
||||||
message = GENERIC_5XX_MESSAGE
|
message = GENERIC_5XX_MESSAGE
|
||||||
err_code = "INTERNAL_ERROR"
|
err_code = "INTERNAL_ERROR"
|
||||||
body = {
|
body = {
|
||||||
|
|
@ -158,6 +170,7 @@ def _http_exception_handler(request: Request, exc: HTTPException) -> JSONRespons
|
||||||
exc.status_code,
|
exc.status_code,
|
||||||
detail,
|
detail,
|
||||||
)
|
)
|
||||||
|
if should_sanitize:
|
||||||
detail = GENERIC_5XX_MESSAGE
|
detail = GENERIC_5XX_MESSAGE
|
||||||
code = _status_to_code(exc.status_code, detail)
|
code = _status_to_code(exc.status_code, detail)
|
||||||
return _build_error_response(exc.status_code, detail, code=code, request_id=rid)
|
return _build_error_response(exc.status_code, detail, code=code, request_id=rid)
|
||||||
|
|
|
||||||
|
|
@ -70,6 +70,20 @@ def _make_test_app():
|
||||||
async def raise_http_500():
|
async def raise_http_500():
|
||||||
raise HTTPException(status_code=500, detail="secret db password leaked")
|
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")
|
@app.get("/surfsense-connector")
|
||||||
async def raise_connector():
|
async def raise_connector():
|
||||||
raise ConnectorError("GitHub API returned 401")
|
raise ConnectorError("GitHub API returned 401")
|
||||||
|
|
@ -184,6 +198,20 @@ class TestHTTPExceptionHandler:
|
||||||
assert body["error"]["message"] == GENERIC_5XX_MESSAGE
|
assert body["error"]["message"] == GENERIC_5XX_MESSAGE
|
||||||
assert body["error"]["code"] == "INTERNAL_ERROR"
|
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
|
# SurfSenseError hierarchy
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue