Plumb CSRF token to admin templates so POST forms work#26
Merged
Conversation
Every POST form on the admin web UI (Add GitLab connection, Add LLM,
Rotate passphrase, Create token, etc.) was failing at the middleware
with "csrf token missing or invalid". The templates didn't emit a
csrf_token form field and the render path didn't expose the plaintext
token to the page. The middleware only had the SHA-256 hash from
Lookup(), and the plaintext from Issue() was thrown away after the
session cookie was set.
Fix:
- session: set a NON-HttpOnly sieve_csrf cookie at Issue time carrying
the plaintext token (HttpOnly off is the documented synchronizer-
token pattern — the page script needs to read it). SameSite=Lax to
match the session cookie so OAuth callback flows still receive both.
/logout clears the cookie alongside the session cookie.
- web/auth.go: login + setup handlers SetCookie the new
sieve_csrf alongside sieve_session. The requireOperatorSession
middleware reads the sieve_csrf cookie and stashes the plaintext on
sess.CSRFToken so handlers/templates can echo it back. Missing
cookie is not fatal — the next POST fails closed and the operator
re-logs in.
- web/server.go: render() now injects CSRFToken into template data
from the session in context. Two shapes are supported:
* map[string]any: m["CSRFToken"] = token (when absent).
* *struct or struct with a string field named CSRFToken: set via
reflection (when zero). connectionEditData declares the field
to opt in.
- web/templates/nav.html: a single inline <script> seeds
window.SIEVE_CSRF from {{.CSRFToken}} (html/template JS-string-
escapes the value automatically), and a delegated submit handler
on the document injects an idempotent <input name="csrf_token">
into every same-origin POST form before submit. No per-form change
required; nav.html is included by every admin page so the handler
is always present.
Tests:
- csrf_form_integration_test.go: TestPOSTConnectionsAddPassesCSRF
proves a POST with the form-field token succeeds; the negative
control asserts a POST without it still 403s. Pins the security
property that this plumbing didn't accidentally relax the gate.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes broken CSRF handling in the admin UI by plumbing the plaintext CSRF token through the operator session → template render path → shared admin navigation template, enabling POST forms to consistently include csrf_token without per-form template edits.
Changes:
- Add a dedicated non-HttpOnly
sieve_csrfcookie to carry the plaintext CSRF token and clear it on logout. - Expose the plaintext CSRF token on the session-in-context and inject it into template data during render.
- Add a delegated JS submit handler in
nav.htmlto inject a hiddencsrf_tokeninput on same-origin POST forms; add integration tests for CSRF acceptance/rejection.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/web/templates/nav.html | Seeds window.SIEVE_CSRF from template data and injects csrf_token into POST forms at submit time. |
| internal/web/server.go | Updates render() signature and injects CSRF token into template data (map/struct). |
| internal/web/passphrase_rotation.go | Updates call site to new render(w, r, ...) signature. |
| internal/web/csrf_form_integration_test.go | Adds regression tests for CSRF acceptance/rejection on /connections/add. |
| internal/web/connection_edit.go | Adds CSRFToken field to view-model and updates render call signature. |
| internal/web/auth.go | Reads CSRF token from cookie into session context; sets/clears new CSRF cookie on login/setup/logout. |
| internal/session/session.go | Defines sieve_csrf cookie helpers for issuing and clearing plaintext CSRF cookie. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Four threads, four fixes:
1. render() now ALWAYS sets data["CSRFToken"], even to "" when no
session is in context (older session, lost cookie). nav.html
embeds the value as a JS string literal in
`window.SIEVE_CSRF = "{{.CSRFToken}}"`; leaving the key absent
would produce invalid JS (`= ;`) and break every script on the
page. The empty case is harmless: the submit handler skips
injection when SIEVE_CSRF is falsy, and the middleware fails
closed at the next POST anyway. injectCSRFToken docstring
updated to describe the new contract.
2. nav.html submit handler now actually verifies the form's action
resolves to the same origin as the page (the previous comment
claimed this, but the code didn't check). Uses URL() to resolve
action against window.location.href and compares .origin to
window.location.origin; malformed actions fail closed (skip).
A form with no action attribute resolves to the document URL
and passes — the common case for our forms.
3. csrf_form_integration_test.go file header rewritten to describe
what the tests actually do (POST + assertions) rather than the
misleading cookie/render claims from the first draft. Points
readers at auth_test.go for the cookie lifecycle coverage.
4. auth_test.go: TestLogin_HappyPath, TestLogin_WrongCredential, and
TestLogout_DeletesCookieAndSession now assert that
session.CSRFCookieName ("sieve_csrf") is set on successful login
(non-HttpOnly, non-empty value), NOT set on failed login, and
carries a deletion directive on logout. Pins the cookie
lifecycle alongside the existing sieve_session assertions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- nav.html: wrapped window.fetch so admin pages' fetch()-style POSTs
(/api/generate-script, /api/save-script, and any future callers)
automatically include X-CSRF-Token on same-origin state-changing
requests. Idempotent: a caller's existing X-CSRF-Token header is
left alone. Same-origin gate matches the form-submit handler's
resolution rule: input URL is resolved against the current document
and the request is skipped if the resulting .origin differs.
Non-state-changing methods (GET/HEAD/OPTIONS) and cross-origin
requests bypass the wrapper entirely so the token never leaks
off-site.
- server.go render() comment: the previous wording claimed nav.html
embeds the token as `window.SIEVE_CSRF = "{{...}}";` (with quotes
inside the template). nav.html actually writes
`window.SIEVE_CSRF = {{.CSRFToken}};` and lets html/template's
JS-context escaper emit a quoted literal automatically. Updated
the comment to match.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two fixes — one real bug, one docstring nit. 1. connection_edit.go: renderEditError(WithConfig) called t.ExecuteTemplate directly to set a 400 status, bypassing s.render() and therefore the CSRFToken injection. On validation / save errors the re-rendered edit page would carry an empty window.SIEVE_CSRF, and the operator's retry POST would 403 at the middleware. Plumbed *http.Request through both helpers and now inject data.CSRFToken from the session-in-context manually before executing the template. Annotated the bypass with a comment so future readers see why s.render is skipped here. 2. session.go: ClearCSRFCookie's docstring claimed the function returns a "Max-Age=0 cookie", which is true at the wire-header level but confusing given the implementation sets MaxAge=-1. (Go's net/http maps MaxAge<0 to the "Max-Age=0" Set-Cookie attribute — the field value is the deletion sentinel, not the wire value.) Reworded to spell out both layers so the next reader doesn't mistake the field for a bug. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every POST form on the admin web UI was failing at the middleware with
csrf token missing or invalid— reproduced when trying to add a GitLab connection but affects all 33 POST forms (Add LLM, Rotate passphrase, Create token, Add Slack OAuth credentials, etc.). Templates emitted nocsrf_tokenfield and the render path didn't expose the plaintext token to the page.Fix
sieve_csrfcookie at Issue time carrying the plaintext token.HttpOnly: falseis the documented synchronizer-token pattern — the page script needs to read it. SameSite=Lax matches the session cookie so OAuth callback flows still receive both. Logout clears it.SetCookiethe newsieve_csrfalongsidesieve_session. TherequireOperatorSessionmiddleware reads the cookie and stashes the plaintext onsess.CSRFTokenfor downstream use.render()— injectsCSRFTokeninto template data from the session in context. Supportsmap[string]any(most handlers) and structs with a stringCSRFTokenfield (justconnectionEditDatatoday, which now declares it).<script>seedswindow.SIEVE_CSRFfrom{{.CSRFToken}}(Go html/template JS-string-escapes the value), and a delegated submit handler on the document injects an idempotent<input name="csrf_token">into every same-origin POST form before submit. No per-form change required because nav.html is included by every admin page.Tests
csrf_form_integration_test.go:TestPOSTConnectionsAddPassesCSRFproves a POST carrying the form-field token reaches the handler (303 redirect).TestPOSTConnectionsAddMissingCSRFRejected(negative control) asserts a POST without it still 403s — pins the security property that this didn't accidentally relax the gate.go test ./...green.Security stance
Test plan
./sieve→ log in → Add GitLab connection succeeds (no CSRF error).sieve_csrfcookie in DevTools → next form POST fails 403 (proves the middleware is still enforcing).sieve_sessionandsieve_csrfcookies cleared.🤖 Generated with Claude Code