diff --git a/CHANGELOG.md b/CHANGELOG.md index b8eaaf3485..16a3e945ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +### Fixed + +- CSRF rejections now distinguish origin/proxy mismatches from expired session + tokens so provider-key removal and other protected requests show actionable + diagnostics instead of the generic cross-origin error. + ## [v0.51.124] — 2026-05-24 — Release CV (stage-batch6 — 3-PR Windows-only stack — agent paths / docs / port hardening) ### Added diff --git a/api/routes.py b/api/routes.py index eee4f166de..a207dd21f2 100644 --- a/api/routes.py +++ b/api/routes.py @@ -1260,8 +1260,37 @@ def _csrf_exempt_path(path: str) -> bool: return path in {"/api/auth/login", "/api/csp-report"} +_CSRF_FAILURE_ATTR = "_hermes_csrf_failure_reason" + + +def _set_csrf_failure_reason(handler, reason: str) -> bool: + try: + setattr(handler, _CSRF_FAILURE_ATTR, reason) + except Exception: + pass + return False + + +def _clear_csrf_failure_reason(handler) -> None: + try: + if hasattr(handler, _CSRF_FAILURE_ATTR): + delattr(handler, _CSRF_FAILURE_ATTR) + except Exception: + pass + + +def _csrf_rejection_error(handler) -> str: + reason = getattr(handler, _CSRF_FAILURE_ATTR, "") + if reason == "origin_mismatch": + return "Cross-origin mismatch - check reverse proxy headers" + if reason == "token_mismatch": + return "Session expired - reload the page" + return "Cross-origin request rejected" + + def _check_csrf(handler) -> bool: """Reject cross-origin or tokenless authenticated browser unsafe requests.""" + _clear_csrf_failure_reason(handler) origin = handler.headers.get("Origin", "") referer = handler.headers.get("Referer", "") host = handler.headers.get("Host", "") @@ -1271,7 +1300,7 @@ def _check_csrf(handler) -> bool: # Extract host:port from origin/referer m = _re.match(r"^https?://([^/]+)", target) if not m: - return False + return _set_csrf_failure_reason(handler, "origin_mismatch") origin_host = m.group(1) origin_scheme = m.group(0).split('://')[0].lower() # 'http' or 'https' origin_name, origin_port = _normalize_host_port(origin_host) @@ -1299,7 +1328,7 @@ def _check_csrf(handler) -> bool: origin_allowed = True break if not origin_allowed: - return False + return _set_csrf_failure_reason(handler, "origin_mismatch") from api.auth import CSRF_HEADER_NAME, is_auth_enabled, parse_cookie, verify_csrf_token @@ -1307,7 +1336,9 @@ def _check_csrf(handler) -> bool: return True cookie_val = parse_cookie(handler) submitted = handler.headers.get(CSRF_HEADER_NAME) or handler.headers.get("X-CSRF-Token") - return verify_csrf_token(cookie_val or "", submitted or "") + if verify_csrf_token(cookie_val or "", submitted or ""): + return True + return _set_csrf_failure_reason(handler, "token_mismatch") def _client_ip_for_rate_limit(handler) -> str: @@ -4614,7 +4645,7 @@ def handle_post(handler, parsed) -> bool: diag.stage("csrf") if not _csrf_exempt_path(parsed.path) and not _check_csrf(handler): try: - return j(handler, {"error": "Cross-origin request rejected"}, status=403) + return j(handler, {"error": _csrf_rejection_error(handler)}, status=403) finally: if diag: diag.finish() @@ -6194,7 +6225,7 @@ def _llm_update_summary(system_prompt: str, user_prompt: str) -> str: def handle_patch(handler, parsed) -> bool: """Handle all PATCH routes. Returns True if handled, False for 404.""" if not _check_csrf(handler): - return j(handler, {"error": "Cross-origin request rejected"}, status=403) + return j(handler, {"error": _csrf_rejection_error(handler)}, status=403) body = read_body(handler) if parsed.path.startswith("/api/kanban/"): from api.kanban_bridge import handle_kanban_patch @@ -6209,7 +6240,7 @@ def handle_patch(handler, parsed) -> bool: def handle_delete(handler, parsed) -> bool: """Handle all DELETE routes. Returns True if handled, False for 404.""" if not _check_csrf(handler): - return j(handler, {"error": "Cross-origin request rejected"}, status=403) + return j(handler, {"error": _csrf_rejection_error(handler)}, status=403) body = read_body(handler) if parsed.path.startswith("/api/kanban/"): from api.kanban_bridge import handle_kanban_delete diff --git a/tests/test_issue2572_csrf_diagnostics.py b/tests/test_issue2572_csrf_diagnostics.py new file mode 100644 index 0000000000..11b417b4b4 --- /dev/null +++ b/tests/test_issue2572_csrf_diagnostics.py @@ -0,0 +1,74 @@ +"""Regression tests for issue #2572 CSRF rejection diagnostics.""" + +import hmac +import io +import json +import time +from types import SimpleNamespace + +import api.auth as auth +import api.routes as routes + + +class _FakeHandler: + def __init__(self, headers=None, body=b"{}"): + self.headers = headers or {} + self.client_address = ("127.0.0.1", 12345) + self.rfile = io.BytesIO(body) + self.wfile = io.BytesIO() + self.status = None + self.sent_headers = {} + + def send_response(self, status): + self.status = status + + def send_header(self, key, value): + self.sent_headers[key] = value + + def end_headers(self): + pass + + +def _signed_cookie(raw_token: str) -> str: + sig = hmac.new(auth._signing_key(), raw_token.encode(), "sha256").hexdigest() + auth._sessions[raw_token] = time.time() + 60 + return f"{raw_token}.{sig}" + + +def _json_body(handler: _FakeHandler) -> dict: + return json.loads(handler.wfile.getvalue().decode("utf-8")) + + +def test_origin_mismatch_csrf_rejection_has_diagnostic_error(monkeypatch): + monkeypatch.setattr(auth, "is_auth_enabled", lambda: False) + handler = _FakeHandler( + { + "Origin": "https://evil.example", + "Host": "127.0.0.1:8787", + } + ) + + routes.handle_post(handler, SimpleNamespace(path="/api/providers/delete")) + + assert handler.status == 403 + assert _json_body(handler)["error"] == "Cross-origin mismatch - check reverse proxy headers" + + +def test_token_mismatch_csrf_rejection_has_reload_error(monkeypatch): + cookie = _signed_cookie("z" * 64) + monkeypatch.setattr(auth, "is_auth_enabled", lambda: True) + try: + handler = _FakeHandler( + { + "Origin": "http://127.0.0.1:8787", + "Host": "127.0.0.1:8787", + "Cookie": f"{auth.COOKIE_NAME}={cookie}", + } + ) + + routes.handle_post(handler, SimpleNamespace(path="/api/providers/delete")) + + assert handler.status == 403 + assert _json_body(handler)["error"] == "Session expired - reload the page" + finally: + auth._sessions.pop("z" * 64, None)