From 0ab546a2dcb78c1c2e8e0e050be264bddd76cc7f Mon Sep 17 00:00:00 2001 From: magi Date: Sun, 6 Sep 2026 22:32:12 +0530 Subject: [PATCH] fix(csrf): CSRF cookie was HttpOnly, silently defeating the double-submit pattern The double-submit cookie pattern relies on client-side JavaScript reading the CSRF cookie and echoing its value back in a request header (header_name, e.g. X-CSRF-Token) -- that round-trip is what proves the request came from same-origin script, since a cross-site attacker can trigger a cookie-carrying request but can't read the cookie's value to set the matching header. _set_csrf_cookie() set HttpOnly on that cookie, which makes it invisible to JavaScript entirely. Every real browser client would be structurally unable to complete the round-trip: every state-changing request would 403 with "CSRF token missing" until an app author noticed and manually worked around the framework. Found reading the source while building a session-cookie app; not caught by any existing test. Removed HttpOnly. Added a regression test. 261/261 tests, mypy clean, ruff clean. --- tests/test_security_csrf.py | 14 ++++++++++++++ velocix/security/csrf.py | 13 ++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/test_security_csrf.py b/tests/test_security_csrf.py index 0605c63..178704b 100644 --- a/tests/test_security_csrf.py +++ b/tests/test_security_csrf.py @@ -52,6 +52,20 @@ async def scenario(): _run(scenario()) +def test_csrf_cookie_is_not_httponly(): + """The double-submit pattern requires client-side JS to read this cookie + and echo it back as a header; HttpOnly would make that impossible, + silently breaking CSRF protection for every real browser client.""" + app = _app_with_csrf() + + async def scenario(): + async with TestClient(app) as client: + resp = await client.get("/page") + assert "httponly" not in resp.headers["set-cookie"].lower() + + _run(scenario()) + + def test_get_with_existing_valid_cookie_does_not_reset(): app = _app_with_csrf() diff --git a/velocix/security/csrf.py b/velocix/security/csrf.py index 0691b78..10394ac 100644 --- a/velocix/security/csrf.py +++ b/velocix/security/csrf.py @@ -141,7 +141,15 @@ def _validate_token(self, token: str) -> bool: return False def _set_csrf_cookie(self, response: Response, token: str) -> None: - """Set the CSRF cookie on the response.""" + """Set the CSRF cookie on the response. + + Deliberately NOT HttpOnly: the double-submit pattern requires + client-side JavaScript to read this cookie and echo its value back + in the request header (self._header_name) on state-changing + requests -- that's what proves the request came from same-origin + script rather than a cross-site form/link. An HttpOnly cookie here + can never be read by that script, which defeats the entire scheme. + """ response.raw_headers.append(( b"set-cookie", ( @@ -149,8 +157,7 @@ def _set_csrf_cookie(self, response: Response, token: str) -> None: f"Path={self._cookie_path}; " f"SameSite={self._cookie_samesite}; " f"{'Secure; ' if self._cookie_secure else ''}" - f"HttpOnly" - ).encode("latin-1"), + ).rstrip("; ").encode("latin-1"), )) async def _on_request(self, request: Request) -> Response: