feat(admin): add CSRF protection to mutation endpoints by x3ek · Pull Request #83 · xeek-dev/squishmark

x3ek · 2026-05-23T11:53:14Z

Closes #79.

Summary

Admin POST/PUT/DELETE routes now require a server-validated CSRF token in addition to the session cookie
Token is per-session, minted lazily when the dashboard renders, rotated on OAuth login
Verified via a FastAPI dependency on each mutation route (dependencies=[Depends(verify_csrf_token)])
HTMX listener reads the token from a <meta name="csrf-token"> tag and attaches it as X-CSRF-Token to every request; csrf_token form-field fallback also accepted
dev_skip_auth mode bypasses CSRF to match the existing auth-bypass behavior

Changes

New services/csrf.py — get_or_create_csrf_token + verify_csrf_token (uses secrets.token_urlsafe(32) and secrets.compare_digest)
routers/admin.py — token threaded into dashboard render context; verify_csrf_token dep on create_note, update_note, delete_note, refresh_cache
routers/auth.py — pops csrf_token from session on successful OAuth callback so a fresh token mints on next read
themes/{default,blue-tech,terminal}/admin/admin.html — meta tag injected in <head>; HTMX configRequest listener added (inline in default/blue-tech; in themes/terminal/static/js/admin.js for terminal)
tests/test_csrf.py — 11 new unit tests covering mint/idempotency, header path, form fallback, missing-token/wrong-token rejection, dev-skip bypass, prod-mode enforcement

Test plan

python scripts/run-checks.py — 221 tests pass, ruff clean, 0 pyright errors
Playwright across all 3 themes (default, blue-tech, terminal): meta tag rendered, HTMX listener attaches X-CSRF-Token, mutations succeed
Manual verification with dev_skip_auth=true: dashboard loads, note CRUD via HTMX works
Unit tests cover rejection paths (missing session token → 403, wrong header → 403, no token submitted → 403)

🤖 Generated with Claude Code

Closes #79. Admin POST/PUT/DELETE routes now require a server-validated CSRF token in addition to the session cookie. Token is per-session, minted lazily when the dashboard renders, rotated on OAuth login, and validated via a FastAPI dependency on each mutation route. Clients send it in the X-CSRF-Token header (HTMX listener reads the token from a <meta> tag) with a csrf_token form-field fallback. Dev-skip-auth mode bypasses CSRF to match auth bypass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR adds CSRF protection for admin state-changing endpoints by minting a per-session CSRF token, validating it server-side on mutation routes, and propagating it to the client (via a <meta name="csrf-token"> tag + an HTMX htmx:configRequest listener).

Changes:

Introduces services/csrf.py with CSRF token minting (get_or_create_csrf_token) and validation (verify_csrf_token) logic.
Applies CSRF validation as a FastAPI dependency on admin mutation routes and threads the token into the admin template render context.
Updates bundled admin themes to emit the CSRF meta tag and attach X-CSRF-Token to HTMX requests; adds unit tests for CSRF behaviors.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/squishmark/services/csrf.py	Adds CSRF token generation + verification dependency used by admin mutation endpoints.
src/squishmark/routers/admin.py	Mints token during admin dashboard render and enforces CSRF validation on POST/PUT/DELETE routes.
src/squishmark/routers/auth.py	Rotates CSRF token on successful OAuth login by clearing the session CSRF entry.
themes/default/admin/admin.html	Renders CSRF token meta tag and adds HTMX request hook to send `X-CSRF-Token`.
themes/blue-tech/admin/admin.html	Renders CSRF token meta tag and adds HTMX request hook to send `X-CSRF-Token`.
themes/terminal/admin/admin.html	Renders CSRF token meta tag for the terminal theme’s admin UI.
themes/terminal/static/js/admin.js	Adds HTMX request hook to send `X-CSRF-Token` for the terminal theme.
tests/test_csrf.py	Adds unit tests for token minting, validation paths, and dev-skip bypass behavior.

…EY constant Address Copilot review on PR #83: 1. auth.py now uses SESSION_KEY constant from services.csrf instead of hardcoding 'csrf_token' — avoids key drift if the storage key ever changes. 2. New GET /admin/csrf endpoint returns the current token in JSON, matching the issue's acceptance criterion for non-HTML clients. The module docstring already promised JSON API support; this makes good on it. Tests added for the new endpoint (idempotent within session) and the auth rotation behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Address Copilot review pass 2 on PR #83: Route-level dependencies=[Depends(verify_csrf_token)] resolves before parameter deps (including AdminUser), so unauthenticated HTMX requests would get 403 (CSRF token missing) instead of the intended 401 + HX-Redirect. Fix by: 1. Move get_current_admin from routers/admin.py to dependencies.py so services/csrf.py can import it without a circular dep. Re-exported from admin.py so existing test patches keep working. 2. verify_csrf_token now depends on get_current_admin via Annotated[str, Depends(...)]. FastAPI resolves auth first; if it raises 401, CSRF never runs. Also rewrote test_oauth_callback_rotates_csrf_token to actually exercise oauth_callback with mocked httpx calls (per Copilot feedback — the previous version only tested dict.pop). Added a structural test that locks in the new CSRF→auth dep ordering. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Address Copilot review pass 3 on PR #83: 1. Centralize is_htmx in dependencies.py and import it from routers/admin.py. Previously the prior commit duplicated the helper (as _is_htmx in dependencies.py alongside the existing is_htmx in admin.py). 2. Update test patches that target squishmark.routers.admin.get_settings to squishmark.dependencies.get_settings. After moving get_current_admin to dependencies.py the old patches were silently no-ops — tests passed only because the real settings happened to produce the same outcome for an empty session. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…polish Independent code review pass on PR #83. Six changes: 1. tests/test_csrf_integration.py (new): five TestClient-based tests that actually exercise the route-level CSRF dep and the auth-before-CSRF ordering — previously all four 'dependencies=[Depends(verify_csrf_token)]' lines were uncovered (existing tests call handler functions directly and bypass FastAPI's DI). Includes the crucial 'unauth HTMX → 401+HX-Redirect, not 403' assertion. 2. services/csrf.py: collapse the two distinct error messages ('CSRF token missing from session' vs 'CSRF token invalid') into one generic 'CSRF validation failed'. The prior split was an information disclosure — it told an attacker whether they had a session cookie to bind to. 3. services/csrf.py: document the assumption that Starlette caches request.form() so the dep + route handler can both read it without losing the body. 4. routers/admin.py: drop the __all__ re-export of get_current_admin / AdminUser. Test imports updated to point at squishmark.dependencies directly — there were no production callers. 5. routers/admin.py: GET /admin/csrf now returns a typed CSRFTokenResponse Pydantic model instead of a raw dict, matching the convention of NoteResponse and CacheRefreshResponse. 6. tests/test_admin_notes.py: oauth_callback rotation test now also asserts a fresh token can be minted after rotation, not just that the stale one is gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Two structured logs added so production issues are diagnosable: 1. verify_csrf_token now emits WARNING on every rejection with reason (no-session-token / no-submitted-token / token-mismatch), method, path, admin, and htmx flag. The user-facing detail stays generic ('CSRF validation failed') to avoid info disclosure — the log is the only place the specific failure mode lives. Caplog test locks the format in. 2. oauth_callback emits INFO on CSRF token rotation, tying the rotation to the user that just logged in. Useful for audit trails and login-flow debugging. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

x3ek requested a review from Copilot May 23, 2026 11:55

Copilot started reviewing on behalf of x3ek May 23, 2026 11:56 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Comment thread src/squishmark/routers/auth.py Outdated

Comment thread src/squishmark/services/csrf.py Outdated

x3ek requested a review from Copilot May 23, 2026 12:02

Copilot started reviewing on behalf of x3ek May 23, 2026 12:02 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Comment thread src/squishmark/routers/admin.py

Comment thread src/squishmark/routers/admin.py

Comment thread src/squishmark/routers/admin.py

Comment thread src/squishmark/routers/admin.py

Comment thread tests/test_admin_notes.py

x3ek requested a review from Copilot May 23, 2026 12:10

Copilot started reviewing on behalf of x3ek May 23, 2026 12:10 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Comment thread src/squishmark/routers/admin.py Outdated

Comment thread src/squishmark/dependencies.py Outdated

x3ek and others added 3 commits May 23, 2026 07:40

x3ek merged commit f14b16b into main May 23, 2026
5 of 6 checks passed

x3ek deleted the feat/79-csrf-protection branch May 23, 2026 13:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(admin): add CSRF protection to mutation endpoints#83

feat(admin): add CSRF protection to mutation endpoints#83
x3ek merged 6 commits into
mainfrom
feat/79-csrf-protection

x3ek commented May 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

x3ek commented May 23, 2026

Summary

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants