[SafairBru] /admin/* routes exempted from CSRF protection
main.py exempts /admin/.* from CSRF middleware (line 254). Admin POST endpoints include user creation (/admin/users/add), password reset (/admin/users/reset-password/{username})...
StateDoneNext ActionClosedOwnerLuciRuntimeClosedAge27d ago
Ticket is done; runtime is closed.·profile claude_opus_1m_medium · cwd /home/lucienne/workspace · uptime 27d 3h · last activity 27d 1h ago
Description
MC-3577
main.py exempts /admin/.* from CSRF middleware (line 254). Admin POST endpoints include user creation (/admin/users/add), password reset (/admin/users/reset-password/{username}), forecast locking (/admin/lock-actuals), and settings changes. A malicious site could submit forms to these endpoints when an authenticated admin visits. The login/logout exemptions have documented justification, but admin endpoints handle sensitive state-changing operations.
File: main.py:254
Complexity: simple
Found by: nightly code review (f2ecd69 fix(MC-3260): use _sast_now() in health endpoint timestamp)
Auto-fix skipped: Luci-side scheduled autofix is disabled by default; Larry owns SafairBru code changes.
Activity
done
INTERACTIVE
Luci is working...
Details —
Done
· High
· Luci
▼
SState
Done
Closed
PPeople
TTiming / Details▼
safairbru-review (scheduled)
SafairBru
27d ago
27d ago
Advanced / Operator evidence
RRouting owner
ROperator console
Ticket is done; runtime is closed.completedprofile claude_opus_1m_medium · cwd /home/lucienne/workspace · uptime 27d 3h · last activity 27d 1h agoMC is visibility-only. Hermes Luci launches and gates work outside MC, then mirrors evidence/status here.
28d32b36-0f60-4be6-b172-a96ba0304913 · luci · claude/anthropic/opus[1m]
WWorkflow
Start Dev Review + QA ▾
Select phases to include:
Agents
Review Gates
Decision
WAT routing: choose an agent, review gate, or decision. Buttons use the live runtime when one is attached.
Luci dispatching this ticket to Larry (background worker via SSH).
luci27d ago
Larry: Planning — reading main.py CSRF exemptionPROGRESS: Building — removing /admin/.* CSRF exemption (all admin forms use HTMX which injects CSRF header)PROGRESS: Testing — verifying syntax and module importsPROGRESS: Review gate — running broader test suitePROGRESS: Done — committingRemoved the `/admin/.*` CSRF exemption in main.py:254. All admin POST routes use HTMX, and the global `htmx:configRequest` handler in `static/js/app.js` already injects the CSRF token as the `X-CSRF-Token` header, so admin forms continue to work. Added regression test `tests/test_mc3577_admin_csrf.py`; full suite passes (one pre-existing datetime-related failure in `test_mc2685` is unrelated).
luci27d ago
Completed on Larry:
PROGRESS: Planning — reading main.py CSRF exemptionPROGRESS: Building — removing /admin/.* CSRF exemption (all admin forms use HTMX which injects CSRF header)PROGRESS: Testing — verifying syntax and module importsPROGRESS: Review gate — running broader test suitePROGRESS: Done — committingRemoved the `/admin/.*` CSRF exemption in main.py:254. All admin POST routes use HTMX, and the global `htmx:configRequest` handler in `static/js/app.js` already injects the CSRF token as the `X-CSRF-Token` header, so admin forms continue to work. Added regression test `tests/test_mc3577_admin_csrf.py`; full suite passes (one pre-existing datetime-related failure in `test_mc2685` is unrelated).
luci27d ago
Larry: Triggering post-completion code review...
luci27d ago
Code review (Larry reviewer via claude/opus):
verifies the exemption list, not that an actual POST to an `/admin/*` route without
a CSRF token is rejected. A request-level test via TestClient would more
directly prove the fix; the structural assertion is acceptable for a
Tier 1 fix.
Note (not a finding): the CLAUDE.md describes a Streamlit + F
astAPI stack but the diff touches a Starlette/FastAPI `main.py` with CSRFMiddleware — ass
uming this reflects the current code, the change is consistent with adm
in HTMX forms already sending the CSRF header via `htmx:configRequest`.
VERDICT
: PASS
Removes the `/admin/.*` CSRF exemption and adds a regression test asserting `/admin` is no longer in the exempt list.
MINOR: tests/test_mc3577_admin_csrf.py:30 — substring match `"/admin" in p` would also flag an unrelated pattern like `/administer`. Anchoring to `^/admin` would be tighter, but no such pattern exists today so this is harmless.
MINOR: tests/test_mc3577_admin_csrf.py — test only verifies the exemption list, not that an actual POST to an `/admin/*` route without a CSRF token is rejected. A request-level test via TestClient would more directly prove the fix; the structural assertion is acceptable for a Tier 1 fix.
Note (not a finding): the CLAUDE.md describes a Streamlit + FastAPI stack but the diff touches a Starlette/FastAPI `main.py` with CSRFMiddleware — assuming this reflects the current code, the change is consistent with admin HTMX forms already sending the CSRF header via `htmx:configRequest`.
VERDICT: PASS
luci27d ago
Larry shipped with clean code review. Auto-dispatching Luci-reviewer (verdict: clean). Next dispatch tick will spawn a single-session reviewer worker to verify, run Tessa subagent inline if UI-touching, merge the PR, and close.
luci27d ago
Completed:
DONE: MC-3577 verified — Larry's `/admin/*` CSRF exemption removal is correct. Confirmed `main.py` exempt list no longer includes `/admin/.*` (only static/health/robots/login/logout remain), regression test `tests/test_mc3577_admin_csrf.py` passes. Larry had committed (40f14e8) but not pushed; rebased local main onto origin (dropped a duplicate MC-3576 commit already upstream) and pushed — `origin/main` now at `fb5e948`. Admin HTMX forms unaffected since `htmx:configRequest` already injects the CSRF token header.