From b5b9df01a43bcb8454273c1044b6846d6d39adc7 Mon Sep 17 00:00:00 2001
From: Arthur Breitman <arthur@nospam.spam>
Date: Thu, 18 Jun 2026 12:32:15 +0000
Subject: [PATCH 1/4] fix(web): plumb CSRF token to admin templates so POST
 forms work
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 internal/session/session.go                | 36 +++++++++
 internal/web/auth.go                       | 18 +++++
 internal/web/connection_edit.go            |  8 +-
 internal/web/csrf_form_integration_test.go | 85 ++++++++++++++++++++++
 internal/web/passphrase_rotation.go        |  2 +-
 internal/web/server.go                     | 70 ++++++++++++++----
 internal/web/templates/nav.html            | 31 ++++++++
 7 files changed, 234 insertions(+), 16 deletions(-)
 create mode 100644 internal/web/csrf_form_integration_test.go

diff --git a/internal/session/session.go b/internal/session/session.go
index 400b430..65c94f5 100644
--- a/internal/session/session.go
+++ b/internal/session/session.go
@@ -21,6 +21,16 @@ import (
 // see that helper for the Lax-vs-Strict rationale.
 const CookieName = "sieve_session"
 
+// CSRFCookieName carries the plaintext CSRF token. Unlike the session
+// cookie this one is intentionally NOT HttpOnly: the page-side script
+// has to read it to inject `csrf_token` form fields into POST forms
+// and `X-CSRF-Token` headers into fetch calls. The plaintext is only
+// useful if the attacker already has the session cookie (the server
+// verifies token+session as a pair), so cookie-readability adds no
+// new surface. SameSite=Lax matches the session cookie so the OAuth
+// callback flow still receives both.
+const CSRFCookieName = "sieve_csrf"
+
 // DefaultIdleTimeout is the documented sliding-window expiry. Settings
 // can override via the session.idle_timeout_minutes key.
 const DefaultIdleTimeout = 8 * time.Hour
@@ -293,6 +303,32 @@ func ClearCookie(secure bool) *http.Cookie {
 	return c
 }
 
+// NewCSRFCookie returns the http.Cookie that carries the plaintext CSRF
+// token to the browser so admin templates can echo it back into
+// state-changing requests. HttpOnly is deliberately false — the page-
+// side script reads this cookie to inject the `csrf_token` hidden input
+// into every form before submit and to set the `X-CSRF-Token` header on
+// fetch calls. SameSite=Lax matches the session cookie so the OAuth
+// callback flow still receives both.
+func NewCSRFCookie(plaintext string, secure bool) *http.Cookie {
+	return &http.Cookie{
+		Name:     CSRFCookieName,
+		Value:    plaintext,
+		Path:     "/",
+		HttpOnly: false,
+		SameSite: http.SameSiteLaxMode,
+		Secure:   secure,
+	}
+}
+
+// ClearCSRFCookie returns a Max-Age=0 cookie used by /logout to clear
+// the plaintext CSRF cookie alongside the session cookie.
+func ClearCSRFCookie(secure bool) *http.Cookie {
+	c := NewCSRFCookie("", secure)
+	c.MaxAge = -1
+	return c
+}
+
 // --- helpers ---
 
 func randString(n int) (string, error) {
diff --git a/internal/web/auth.go b/internal/web/auth.go
index 36b415f..2a065f3 100644
--- a/internal/web/auth.go
+++ b/internal/web/auth.go
@@ -158,6 +158,21 @@ func (s *Server) requireOperatorSession(next http.Handler) http.Handler {
 			}
 		}
 
+		// Surface the plaintext CSRF token from its cookie onto the
+		// session-in-context so handlers/templates can echo it into
+		// forms. Lookup() returns the session with CSRFToken empty
+		// (only the hash is in the DB); the plaintext lives in the
+		// CSRFCookieName cookie set at Issue time. Missing cookie is
+		// not fatal here — the cookie is independent of the session
+		// cookie and may legitimately be absent on older sessions
+		// issued before this code was deployed; templates render with
+		// an empty CSRFToken in that case and the next form submit
+		// will fail closed at the verify gate above. Operator just
+		// re-logs in.
+		if csrfCookie, err := r.Cookie(session.CSRFCookieName); err == nil && csrfCookie.Value != "" {
+			sess.CSRFToken = csrfCookie.Value
+		}
+
 		ctx := context.WithValue(r.Context(), sessionCtxKey{}, sess)
 		next.ServeHTTP(w, r.WithContext(ctx))
 	})
@@ -307,6 +322,7 @@ func (s *Server) handleLoginPost(w http.ResponseWriter, r *http.Request) {
 	// so the cookie is still sent on plaintext loopback.
 	secure := r.TLS != nil
 	http.SetCookie(w, session.NewCookie(sess.Plaintext, secure))
+	http.SetCookie(w, session.NewCSRFCookie(sess.CSRFToken, secure))
 	http.Redirect(w, r, "/", http.StatusSeeOther)
 }
 
@@ -332,6 +348,7 @@ func (s *Server) handleLogout(w http.ResponseWriter, r *http.Request) {
 	}
 	secure := r.TLS != nil
 	http.SetCookie(w, session.ClearCookie(secure))
+	http.SetCookie(w, session.ClearCSRFCookie(secure))
 	http.Redirect(w, r, "/login", http.StatusSeeOther)
 }
 
@@ -420,6 +437,7 @@ func (s *Server) handleSetupPost(w http.ResponseWriter, r *http.Request) {
 	s.logLoginAttempt(name, "setup.ok", ip)
 	secure := r.TLS != nil
 	http.SetCookie(w, session.NewCookie(sess.Plaintext, secure))
+	http.SetCookie(w, session.NewCSRFCookie(sess.CSRFToken, secure))
 	http.Redirect(w, r, "/", http.StatusSeeOther)
 }
 
diff --git a/internal/web/connection_edit.go b/internal/web/connection_edit.go
index 6ab4e09..7fd595a 100644
--- a/internal/web/connection_edit.go
+++ b/internal/web/connection_edit.go
@@ -43,6 +43,12 @@ type connectionEditData struct {
 	// HTTPProxyBaseline carries the static deny-list info displayed
 	// alongside the http_proxy edit form. Read-only; not a form field.
 	HTTPProxyBaseline *httpProxyBaselineView
+
+	// CSRFToken is the plaintext token used by nav.html to seed
+	// window.SIEVE_CSRF, which the delegated submit handler echoes
+	// back into every POST form as `csrf_token`. Populated by render()
+	// from the session-in-context — handlers don't set it directly.
+	CSRFToken string
 }
 
 // editFieldView is the template-friendly projection of a single editable
@@ -111,7 +117,7 @@ func (s *Server) handleConnectionEditPage(w http.ResponseWriter, r *http.Request
 	if r.URL.Query().Get("saved") == "1" {
 		data.Success = true
 	}
-	s.render(w, "connection_edit", data)
+	s.render(w, r, "connection_edit", data)
 }
 
 // handleConnectionEditSave validates and persists the edit form.
diff --git a/internal/web/csrf_form_integration_test.go b/internal/web/csrf_form_integration_test.go
new file mode 100644
index 0000000..ac33b92
--- /dev/null
+++ b/internal/web/csrf_form_integration_test.go
@@ -0,0 +1,85 @@
+package web_test
+
+// Regression test for the end-to-end CSRF token plumbing on admin POST
+// forms. Background: connections form templates (gitlab, http_proxy,
+// slack, etc.) historically didn't include a csrf_token hidden input
+// and the render path didn't expose the plaintext token to nav.html, so
+// every basic Add-Connection POST failed at the middleware with
+// "csrf token missing or invalid". This test pins three properties:
+//
+//   - GET /connections sets a non-HttpOnly sieve_csrf cookie carrying
+//     the plaintext token (so the page script can read it).
+//   - The rendered page exposes the token to nav.html's
+//     window.SIEVE_CSRF script tag.
+//   - A POST to /connections/add carrying the csrf_token form field
+//     (the value the nav.html submit handler echoes back) is accepted
+//     by the middleware and reaches the handler. We check for a 303
+//     redirect (success) or a 400 (request rejected for a NON-CSRF
+//     reason, e.g. unknown connector type in the smoke seed) — both
+//     prove CSRF was satisfied. A 403 fails the test.
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+)
+
+// TestPOSTConnectionsAddPassesCSRF asserts that an admin POST with the
+// session cookie + csrf_token form field passes the middleware. Uses
+// the connector_type=mock seed (mock is registered by newTestWebServer)
+// so the handler runs to completion and we exercise the real success
+// path rather than a 400-on-unknown-connector path.
+func TestPOSTConnectionsAddPassesCSRF(t *testing.T) {
+	handler, env := newTestWebServer(t)
+
+	form := url.Values{}
+	form.Set("connector_type", "mock")
+	form.Set("id", "smoke-csrf")
+	form.Set("display_name", "CSRF smoke")
+	form.Set("csrf_token", env.CSRFToken())
+
+	req := httptest.NewRequest(http.MethodPost, "/connections/add", strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	if c := env.SessionCookie(); c != nil {
+		req.AddCookie(c)
+	}
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	if rec.Code == http.StatusForbidden && strings.Contains(rec.Body.String(), "csrf") {
+		t.Fatalf("CSRF middleware rejected request that carried the form-field token (body: %s)", rec.Body.String())
+	}
+	if rec.Code != http.StatusSeeOther {
+		t.Fatalf("expected 303 redirect after Add, got %d (body: %s)", rec.Code, rec.Body.String())
+	}
+}
+
+// TestPOSTConnectionsAddMissingCSRFRejected is the negative control:
+// without the csrf_token form field (and without the X-CSRF-Token
+// header), the middleware MUST reject with 403. This pins the security
+// property that the new plumbing didn't accidentally relax.
+func TestPOSTConnectionsAddMissingCSRFRejected(t *testing.T) {
+	handler, env := newTestWebServer(t)
+
+	form := url.Values{}
+	form.Set("connector_type", "mock")
+	form.Set("id", "smoke-no-csrf")
+	form.Set("display_name", "CSRF negative control")
+
+	req := httptest.NewRequest(http.MethodPost, "/connections/add", strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	if c := env.SessionCookie(); c != nil {
+		req.AddCookie(c)
+	}
+	rec := httptest.NewRecorder()
+	handler.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusForbidden {
+		t.Fatalf("expected 403 for missing CSRF token, got %d (body: %s)", rec.Code, rec.Body.String())
+	}
+	if !strings.Contains(rec.Body.String(), "csrf") {
+		t.Errorf("expected csrf-related error body, got: %s", rec.Body.String())
+	}
+}
diff --git a/internal/web/passphrase_rotation.go b/internal/web/passphrase_rotation.go
index 5e8a99c..a9c3d5d 100644
--- a/internal/web/passphrase_rotation.go
+++ b/internal/web/passphrase_rotation.go
@@ -168,7 +168,7 @@ func (s *Server) renderRotationError(w http.ResponseWriter, r *http.Request, sta
 	// machine.
 	w.Header().Set("Cache-Control", "no-store")
 	w.WriteHeader(status)
-	s.render(w, "settings", data)
+	s.render(w, r, "settings", data)
 }
 
 // checkRotationOrigin verifies that the request's Origin header (or
diff --git a/internal/web/server.go b/internal/web/server.go
index dc0bb36..0f47903 100644
--- a/internal/web/server.go
+++ b/internal/web/server.go
@@ -36,6 +36,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strconv"
 	"strings"
@@ -504,12 +505,53 @@ func (s *Server) adminAuthWrapper(next http.Handler) http.Handler {
 	})
 }
 
-func (s *Server) render(w http.ResponseWriter, page string, data any) {
+// injectCSRFToken sets data["CSRFToken"] (for maps) or data.CSRFToken
+// (for structs that declare the field) when the value is currently
+// zero. Used by render() to surface the token to nav.html. Reflection
+// is fine here — admin renders are not on a hot path.
+func injectCSRFToken(data any, token string) {
+	if m, ok := data.(map[string]any); ok {
+		if _, present := m["CSRFToken"]; !present {
+			m["CSRFToken"] = token
+		}
+		return
+	}
+	v := reflect.ValueOf(data)
+	if v.Kind() == reflect.Ptr {
+		v = v.Elem()
+	}
+	if !v.IsValid() || v.Kind() != reflect.Struct {
+		return
+	}
+	f := v.FieldByName("CSRFToken")
+	if !f.IsValid() || !f.CanSet() || f.Kind() != reflect.String {
+		return
+	}
+	if f.String() == "" {
+		f.SetString(token)
+	}
+}
+
+func (s *Server) render(w http.ResponseWriter, r *http.Request, page string, data any) {
 	t, ok := s.templates[page]
 	if !ok {
 		http.Error(w, "template not found", http.StatusInternalServerError)
 		return
 	}
+	// Inject the plaintext CSRF token into the template data so
+	// nav.html (included on every admin page) can expose it to the
+	// page script that injects `csrf_token` hidden inputs into every
+	// POST form on submit. Two shapes are supported:
+	//   - map[string]any: set m["CSRFToken"] when missing.
+	//   - struct (or *struct) with a settable CSRFToken string field:
+	//     set via reflection when zero. Typed view-models like
+	//     connectionEditData declare the field so the same nav.html
+	//     access works uniformly.
+	// Handlers that have already populated CSRFToken (manually or by
+	// other means) are left alone.
+	if sess := sessionFromContext(r); sess != nil && sess.CSRFToken != "" {
+		injectCSRFToken(data, sess.CSRFToken)
+	}
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	if err := t.ExecuteTemplate(w, page, data); err != nil {
 		http.Error(w, fmt.Sprintf("render error: %v", err), http.StatusInternalServerError)
@@ -659,7 +701,7 @@ func (s *Server) handleConnections(w http.ResponseWriter, r *http.Request) {
 		// flag and runtime behavior never diverge.
 		"SlackOAuthConfigured": s.slackOAuthIsConfigured(),
 	}
-	s.render(w, "connections", data)
+	s.render(w, r, "connections", data)
 }
 
 // pendingOAuth holds info for a connection being added via OAuth.
@@ -1205,7 +1247,7 @@ func (s *Server) handleTokens(w http.ResponseWriter, r *http.Request) {
 		"RoleNames": roleNames,
 		"Filter":    filter,
 	}
-	s.render(w, "tokens", data)
+	s.render(w, r, "tokens", data)
 }
 
 func (s *Server) handleTokenCreate(w http.ResponseWriter, r *http.Request) {
@@ -1281,7 +1323,7 @@ func (s *Server) handleTokenCreate(w http.ResponseWriter, r *http.Request) {
 		"PlaintextToken": result.PlaintextToken,
 		"CreatedToken":   result.Token,
 	}
-	s.render(w, "tokens", data)
+	s.render(w, r, "tokens", data)
 }
 
 func (s *Server) handleTokenRevoke(w http.ResponseWriter, r *http.Request) {
@@ -1321,7 +1363,7 @@ func (s *Server) handleRoles(w http.ResponseWriter, r *http.Request) {
 		"Connections": conns,
 		"Policies":    pols,
 	}
-	s.render(w, "roles", data)
+	s.render(w, r, "roles", data)
 }
 
 func (s *Server) handleRoleCreate(w http.ResponseWriter, r *http.Request) {
@@ -1393,7 +1435,7 @@ func (s *Server) handleRoleEdit(w http.ResponseWriter, r *http.Request) {
 		"Connections": conns,
 		"Policies":    pols,
 	}
-	s.render(w, "role_edit", data)
+	s.render(w, r, "role_edit", data)
 }
 
 func (s *Server) handleRoleUpdate(w http.ResponseWriter, r *http.Request) {
@@ -1474,7 +1516,7 @@ func (s *Server) handlePolicies(w http.ResponseWriter, r *http.Request) {
 		"Policies": pols,
 		"Scope":    scope,
 	}
-	s.render(w, "policies", data)
+	s.render(w, r, "policies", data)
 }
 
 func (s *Server) handlePolicyCreate(w http.ResponseWriter, r *http.Request) {
@@ -1605,7 +1647,7 @@ func (s *Server) handlePolicyEdit(w http.ResponseWriter, r *http.Request) {
 		"Policy": pol,
 		"Scope":  scope,
 	}
-	s.render(w, "policy_edit", data)
+	s.render(w, r, "policy_edit", data)
 }
 
 // inferPolicyScope guesses a policy's connector scope from the shape of its
@@ -1902,7 +1944,7 @@ func (s *Server) handleApprovals(w http.ResponseWriter, r *http.Request) {
 		"Approvals":  items,
 		"TokenNames": tokenNames,
 	}
-	s.render(w, "approvals", data)
+	s.render(w, r, "approvals", data)
 }
 
 // The historical rejectIfAgentToken helper has been removed. The
@@ -2030,7 +2072,7 @@ func (s *Server) handleAudit(w http.ResponseWriter, r *http.Request) {
 		"After":        r.URL.Query().Get("after"),
 		"Before":       r.URL.Query().Get("before"),
 	}
-	s.render(w, "audit", data)
+	s.render(w, r, "audit", data)
 }
 
 // --- Settings handlers ---
@@ -2107,7 +2149,7 @@ func (s *Server) handleSettings(w http.ResponseWriter, r *http.Request) {
 		"RotationSuccess": rotationSuccess,
 		"RotationCount":   rotationCount,
 	}
-	s.render(w, "settings", data)
+	s.render(w, r, "settings", data)
 }
 
 func (s *Server) handleSettingsSave(w http.ResponseWriter, r *http.Request) {
@@ -2522,7 +2564,7 @@ func (s *Server) handleDocsIndex(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	idx.Breadcrumbs = []Breadcrumb{{Label: "Documentation"}}
-	s.render(w, "docs", map[string]any{
+	s.render(w, r, "docs", map[string]any{
 		"Active": "docs",
 		"Index":  idx,
 	})
@@ -2586,7 +2628,7 @@ func (s *Server) handleDocs(w http.ResponseWriter, r *http.Request) {
 		{Label: title},
 	}
 
-	s.render(w, "docs", map[string]any{
+	s.render(w, r, "docs", map[string]any{
 		"Active": "docs",
 		"Index":  idx,
 	})
@@ -2610,7 +2652,7 @@ func (s *Server) handleDocsCategory(w http.ResponseWriter, r *http.Request) {
 		{Label: "Documentation", Href: "/docs"},
 		{Label: cat.Title},
 	}
-	s.render(w, "docs", map[string]any{
+	s.render(w, r, "docs", map[string]any{
 		"Active": "docs",
 		"Index":  idx,
 	})
diff --git a/internal/web/templates/nav.html b/internal/web/templates/nav.html
index 5c22e9a..6369056 100644
--- a/internal/web/templates/nav.html
+++ b/internal/web/templates/nav.html
@@ -198,6 +198,37 @@
 </nav>
 
 <script>
+// CSRF token injected by the server (render() pulls it from the
+// session-in-context). Empty string means the operator's session was
+// issued before the CSRF cookie existed or the cookie was lost — the
+// next POST will fail closed at the middleware and the operator can
+// re-log in. Templates do not echo it back into forms by hand; the
+// delegated submit handler below adds a hidden `csrf_token` input
+// at submit time, and any fetch-style admin JS callers should read
+// `window.SIEVE_CSRF` and pass it as the X-CSRF-Token header.
+window.SIEVE_CSRF = {{.CSRFToken}};
+
+// Delegated form-submit handler. Capture phase so it runs even if a
+// deeper listener calls stopPropagation. For every same-origin POST
+// form (GET forms don't need CSRF, off-site forms — none today —
+// would leak the token cross-origin so are explicitly skipped) we
+// inject a hidden `csrf_token` input carrying the token from
+// window.SIEVE_CSRF. Idempotent: if a hand-crafted form already has
+// a `csrf_token` input we leave it alone.
+document.addEventListener("submit", function(e) {
+  var form = e.target;
+  if (!form || form.tagName !== "FORM") return;
+  if ((form.method || "get").toLowerCase() !== "post") return;
+  if (form.querySelector('input[name="csrf_token"]')) return;
+  var token = window.SIEVE_CSRF || "";
+  if (!token) return;
+  var input = document.createElement("input");
+  input.type = "hidden";
+  input.name = "csrf_token";
+  input.value = token;
+  form.appendChild(input);
+}, true);
+
 function toggleSub(id) {
   var el = document.getElementById(id);
   var chevron = document.getElementById(id + '-chevron');

From dca800fbb3e4aac747ab38ef5c87ac6e9a5c856c Mon Sep 17 00:00:00 2001
From: Arthur Breitman <arthur@nospam.spam>
Date: Thu, 18 Jun 2026 12:50:00 +0000
Subject: [PATCH 2/4] fix(pr26): address Copilot round 1
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 internal/web/auth_test.go                  | 50 +++++++++++++++++++---
 internal/web/csrf_form_integration_test.go | 37 +++++++++-------
 internal/web/server.go                     | 27 ++++++++----
 internal/web/templates/nav.html            | 26 ++++++++---
 4 files changed, 105 insertions(+), 35 deletions(-)

diff --git a/internal/web/auth_test.go b/internal/web/auth_test.go
index b0579b0..e9181a4 100644
--- a/internal/web/auth_test.go
+++ b/internal/web/auth_test.go
@@ -135,17 +135,29 @@ func TestLogin_HappyPath(t *testing.T) {
 		body := readAll(t, resp.Body)
 		t.Fatalf("status=%d body=%s", resp.StatusCode, body)
 	}
-	// Session cookie set; redirect to /.
-	hasCookie := false
+	// Session cookie + sieve_csrf cookie set; redirect to /.
+	var sessionCookie, csrfCookie *http.Cookie
 	for _, c := range resp.Cookies() {
-		if c.Name == session.CookieName {
-			hasCookie = true
-			break
+		switch c.Name {
+		case session.CookieName:
+			sessionCookie = c
+		case session.CSRFCookieName:
+			csrfCookie = c
 		}
 	}
-	if !hasCookie {
+	if sessionCookie == nil {
 		t.Error("login did not set session cookie")
 	}
+	if csrfCookie == nil {
+		t.Error("login did not set sieve_csrf cookie")
+	} else {
+		if csrfCookie.HttpOnly {
+			t.Error("sieve_csrf cookie must be non-HttpOnly (page script needs to read it)")
+		}
+		if csrfCookie.Value == "" {
+			t.Error("sieve_csrf cookie value must not be empty after a successful login")
+		}
+	}
 	if resp.Header.Get("Location") != "/" {
 		t.Errorf("Location = %q, want /", resp.Header.Get("Location"))
 	}
@@ -169,6 +181,9 @@ func TestLogin_WrongCredential(t *testing.T) {
 		if c.Name == session.CookieName {
 			t.Error("failed login must NOT set session cookie")
 		}
+		if c.Name == session.CSRFCookieName {
+			t.Error("failed login must NOT set sieve_csrf cookie")
+		}
 	}
 }
 
@@ -208,6 +223,29 @@ func TestLogout_DeletesCookieAndSession(t *testing.T) {
 	if _, err := env.Session.Lookup(cookie.Value); err == nil {
 		t.Error("logout did not delete the session row")
 	}
+	// Both the session cookie and the sieve_csrf cookie should carry
+	// a deletion directive (MaxAge<0 / empty value) so the browser
+	// drops them. Without clearing sieve_csrf, a subsequent login
+	// would race against the stale cookie still being sent up.
+	var sessionCleared, csrfCleared bool
+	for _, c := range resp.Cookies() {
+		switch c.Name {
+		case session.CookieName:
+			if c.MaxAge < 0 || c.Value == "" {
+				sessionCleared = true
+			}
+		case session.CSRFCookieName:
+			if c.MaxAge < 0 || c.Value == "" {
+				csrfCleared = true
+			}
+		}
+	}
+	if !sessionCleared {
+		t.Error("logout did not clear the session cookie")
+	}
+	if !csrfCleared {
+		t.Error("logout did not clear the sieve_csrf cookie")
+	}
 }
 
 // --- Middleware tests ---
diff --git a/internal/web/csrf_form_integration_test.go b/internal/web/csrf_form_integration_test.go
index ac33b92..8c85144 100644
--- a/internal/web/csrf_form_integration_test.go
+++ b/internal/web/csrf_form_integration_test.go
@@ -1,22 +1,27 @@
 package web_test
 
-// Regression test for the end-to-end CSRF token plumbing on admin POST
-// forms. Background: connections form templates (gitlab, http_proxy,
-// slack, etc.) historically didn't include a csrf_token hidden input
-// and the render path didn't expose the plaintext token to nav.html, so
-// every basic Add-Connection POST failed at the middleware with
-// "csrf token missing or invalid". This test pins three properties:
+// Regression tests for the end-to-end CSRF token plumbing on admin
+// POST forms. Background: connections form templates (gitlab,
+// http_proxy, slack, etc.) historically didn't include a csrf_token
+// hidden input and the render path didn't expose the plaintext token
+// to nav.html, so every basic Add-Connection POST failed at the
+// middleware with "csrf token missing or invalid".
 //
-//   - GET /connections sets a non-HttpOnly sieve_csrf cookie carrying
-//     the plaintext token (so the page script can read it).
-//   - The rendered page exposes the token to nav.html's
-//     window.SIEVE_CSRF script tag.
-//   - A POST to /connections/add carrying the csrf_token form field
-//     (the value the nav.html submit handler echoes back) is accepted
-//     by the middleware and reaches the handler. We check for a 303
-//     redirect (success) or a 400 (request rejected for a NON-CSRF
-//     reason, e.g. unknown connector type in the smoke seed) — both
-//     prove CSRF was satisfied. A 403 fails the test.
+// This file pins two properties:
+//
+//   - TestPOSTConnectionsAddPassesCSRF: a POST to /connections/add
+//     carrying the csrf_token form field (the value the nav.html
+//     submit handler echoes back from window.SIEVE_CSRF) is accepted
+//     by the middleware and reaches the handler. We assert the
+//     documented 303 redirect on success.
+//   - TestPOSTConnectionsAddMissingCSRFRejected: the negative control
+//     — the same POST without any CSRF token still 403s with a
+//     csrf-themed body. Pins that the new plumbing didn't accidentally
+//     relax the gate.
+//
+// The new sieve_csrf cookie's set/clear lifecycle is covered alongside
+// the session cookie in auth_test.go (TestLogin_HappyPath,
+// TestLogin_WrongCredential, TestLogout_DeletesCookieAndSession).
 
 import (
 	"net/http"
diff --git a/internal/web/server.go b/internal/web/server.go
index 0f47903..ad9f97f 100644
--- a/internal/web/server.go
+++ b/internal/web/server.go
@@ -507,8 +507,12 @@ func (s *Server) adminAuthWrapper(next http.Handler) http.Handler {
 
 // injectCSRFToken sets data["CSRFToken"] (for maps) or data.CSRFToken
 // (for structs that declare the field) when the value is currently
-// zero. Used by render() to surface the token to nav.html. Reflection
-// is fine here — admin renders are not on a hot path.
+// unset/zero. Used by render() to surface the token to nav.html.
+// Reflection is fine here — admin renders are not on a hot path. The
+// caller passes "" when no session is present; we still need to set
+// the key so the template renders a valid JS string literal rather
+// than the bare `;` that an absent map key produces inside
+// `{{.CSRFToken}}`.
 func injectCSRFToken(data any, token string) {
 	if m, ok := data.(map[string]any); ok {
 		if _, present := m["CSRFToken"]; !present {
@@ -527,7 +531,7 @@ func injectCSRFToken(data any, token string) {
 	if !f.IsValid() || !f.CanSet() || f.Kind() != reflect.String {
 		return
 	}
-	if f.String() == "" {
+	if f.String() == "" && token != "" {
 		f.SetString(token)
 	}
 }
@@ -547,11 +551,18 @@ func (s *Server) render(w http.ResponseWriter, r *http.Request, page string, dat
 	//     set via reflection when zero. Typed view-models like
 	//     connectionEditData declare the field so the same nav.html
 	//     access works uniformly.
-	// Handlers that have already populated CSRFToken (manually or by
-	// other means) are left alone.
-	if sess := sessionFromContext(r); sess != nil && sess.CSRFToken != "" {
-		injectCSRFToken(data, sess.CSRFToken)
-	}
+	// Always set the key — even to "" when the session is missing the
+	// CSRF cookie (older session, lost cookie). nav.html embeds the
+	// value as a JS string literal (`window.SIEVE_CSRF = "{{...}}";`);
+	// leaving the key absent would produce invalid JS (`= ;`) and
+	// break every script on the page. The empty string is harmless:
+	// the form-submit handler skips injecting when SIEVE_CSRF is
+	// falsy, and the middleware fails closed at the next POST anyway.
+	var token string
+	if sess := sessionFromContext(r); sess != nil {
+		token = sess.CSRFToken
+	}
+	injectCSRFToken(data, token)
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	if err := t.ExecuteTemplate(w, page, data); err != nil {
 		http.Error(w, fmt.Sprintf("render error: %v", err), http.StatusInternalServerError)
diff --git a/internal/web/templates/nav.html b/internal/web/templates/nav.html
index 6369056..04fab16 100644
--- a/internal/web/templates/nav.html
+++ b/internal/web/templates/nav.html
@@ -210,11 +210,20 @@
 
 // Delegated form-submit handler. Capture phase so it runs even if a
 // deeper listener calls stopPropagation. For every same-origin POST
-// form (GET forms don't need CSRF, off-site forms — none today —
-// would leak the token cross-origin so are explicitly skipped) we
-// inject a hidden `csrf_token` input carrying the token from
-// window.SIEVE_CSRF. Idempotent: if a hand-crafted form already has
-// a `csrf_token` input we leave it alone.
+// form (GET forms don't need CSRF; off-site forms would leak the
+// token cross-origin so are explicitly skipped) we inject a hidden
+// `csrf_token` input carrying the token from window.SIEVE_CSRF.
+// Idempotent: if a hand-crafted form already has a `csrf_token`
+// input we leave it alone.
+//
+// The same-origin check is intentionally strict: we resolve the
+// form's action against the current document and require the
+// resulting origin to equal window.location.origin. A form with no
+// action attribute (action == "") submits to the current document
+// URL, so it resolves to the same origin and passes. A form whose
+// action points off-site, or whose action becomes invalid for any
+// reason, is skipped — better to let an off-site POST fail at the
+// destination than to leak the token cross-origin.
 document.addEventListener("submit", function(e) {
   var form = e.target;
   if (!form || form.tagName !== "FORM") return;
@@ -222,6 +231,13 @@
   if (form.querySelector('input[name="csrf_token"]')) return;
   var token = window.SIEVE_CSRF || "";
   if (!token) return;
+  try {
+    var resolved = new URL(form.getAttribute("action") || "", window.location.href);
+    if (resolved.origin !== window.location.origin) return;
+  } catch (err) {
+    // Malformed action — don't risk it.
+    return;
+  }
   var input = document.createElement("input");
   input.type = "hidden";
   input.name = "csrf_token";

From 11a34783b7b1199c25144504c80e799b7a220772 Mon Sep 17 00:00:00 2001
From: Arthur Breitman <arthur@nospam.spam>
Date: Thu, 18 Jun 2026 12:59:25 +0000
Subject: [PATCH 3/4] fix(pr26): address Copilot round 2

- 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>
---
 internal/web/server.go          | 15 ++++++++-----
 internal/web/templates/nav.html | 40 +++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/internal/web/server.go b/internal/web/server.go
index ad9f97f..41e4c68 100644
--- a/internal/web/server.go
+++ b/internal/web/server.go
@@ -552,12 +552,15 @@ func (s *Server) render(w http.ResponseWriter, r *http.Request, page string, dat
 	//     connectionEditData declare the field so the same nav.html
 	//     access works uniformly.
 	// Always set the key — even to "" when the session is missing the
-	// CSRF cookie (older session, lost cookie). nav.html embeds the
-	// value as a JS string literal (`window.SIEVE_CSRF = "{{...}}";`);
-	// leaving the key absent would produce invalid JS (`= ;`) and
-	// break every script on the page. The empty string is harmless:
-	// the form-submit handler skips injecting when SIEVE_CSRF is
-	// falsy, and the middleware fails closed at the next POST anyway.
+	// CSRF cookie (older session, lost cookie). nav.html writes the
+	// value as `window.SIEVE_CSRF = {{.CSRFToken}};` and relies on
+	// html/template's JS-context escaping to emit a valid quoted
+	// string (or `""` when the value is empty). Leaving the key
+	// absent from the data map would produce invalid JS (`= ;`) and
+	// break every script on the page. The empty-string case is
+	// harmless: the form-submit handler and fetch wrapper both skip
+	// injecting when SIEVE_CSRF is falsy, and the middleware fails
+	// closed at the next POST anyway.
 	var token string
 	if sess := sessionFromContext(r); sess != nil {
 		token = sess.CSRFToken
diff --git a/internal/web/templates/nav.html b/internal/web/templates/nav.html
index 04fab16..de14006 100644
--- a/internal/web/templates/nav.html
+++ b/internal/web/templates/nav.html
@@ -245,6 +245,46 @@
   form.appendChild(input);
 }, true);
 
+// fetch() wrapper. Admin pages also drive POST requests through
+// fetch() (e.g. /api/generate-script, /api/save-script), and those
+// don't go through the form-submit path above. Intercept window.fetch
+// and inject the X-CSRF-Token header on every same-origin
+// state-changing request. Idempotent: if a caller already set
+// X-CSRF-Token we leave it alone. Cross-origin requests are left
+// untouched so the token never leaks off-site.
+(function() {
+  if (typeof window.fetch !== "function") return;
+  var origFetch = window.fetch.bind(window);
+  window.fetch = function(input, init) {
+    init = init || {};
+    var method = (init.method || (input && input.method) || "GET").toUpperCase();
+    // Skip non-state-changing methods.
+    if (method !== "POST" && method !== "PUT" && method !== "PATCH" && method !== "DELETE") {
+      return origFetch(input, init);
+    }
+    var token = window.SIEVE_CSRF || "";
+    if (!token) return origFetch(input, init);
+    // Same-origin gate: resolve the request URL against the current
+    // document and refuse to add the token to cross-origin requests.
+    var urlStr = (typeof input === "string") ? input : (input && input.url) || "";
+    try {
+      var resolved = new URL(urlStr, window.location.href);
+      if (resolved.origin !== window.location.origin) return origFetch(input, init);
+    } catch (err) {
+      return origFetch(input, init);
+    }
+    // Build a Headers object so case-insensitive lookup works, and
+    // copy any existing init.headers into it before checking for an
+    // operator-supplied X-CSRF-Token (which we leave alone).
+    var headers = new Headers(init.headers || (input && input.headers) || {});
+    if (!headers.has("X-CSRF-Token")) {
+      headers.set("X-CSRF-Token", token);
+    }
+    init.headers = headers;
+    return origFetch(input, init);
+  };
+})();
+
 function toggleSub(id) {
   var el = document.getElementById(id);
   var chevron = document.getElementById(id + '-chevron');

From 9658e226a7814fa62d135465a94f0f2ed72c0334 Mon Sep 17 00:00:00 2001
From: Arthur Breitman <arthur@nospam.spam>
Date: Thu, 18 Jun 2026 13:10:48 +0000
Subject: [PATCH 4/4] fix(pr26): address Copilot round 3
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 internal/session/session.go     |  7 +++++--
 internal/web/connection_edit.go | 22 ++++++++++++++--------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/internal/session/session.go b/internal/session/session.go
index 65c94f5..6debbf8 100644
--- a/internal/session/session.go
+++ b/internal/session/session.go
@@ -321,8 +321,11 @@ func NewCSRFCookie(plaintext string, secure bool) *http.Cookie {
 	}
 }
 
-// ClearCSRFCookie returns a Max-Age=0 cookie used by /logout to clear
-// the plaintext CSRF cookie alongside the session cookie.
+// ClearCSRFCookie returns a deletion cookie for the plaintext CSRF
+// cookie. Used by /logout alongside ClearCookie() for the session.
+// Implementation note: Go's net/http maps MaxAge < 0 to a
+// "Max-Age=0" Set-Cookie attribute, which is the wire-level signal
+// browsers treat as immediate deletion. The Go field value isn't 0.
 func ClearCSRFCookie(secure bool) *http.Cookie {
 	c := NewCSRFCookie("", secure)
 	c.MaxAge = -1
diff --git a/internal/web/connection_edit.go b/internal/web/connection_edit.go
index 7fd595a..61a50a9 100644
--- a/internal/web/connection_edit.go
+++ b/internal/web/connection_edit.go
@@ -145,7 +145,7 @@ func (s *Server) handleConnectionEditSave(w http.ResponseWriter, r *http.Request
 
 	meta, ok := s.registry.Meta(conn.ConnectorType)
 	if !ok {
-		s.renderEditError(w, conn, "unknown connector type "+conn.ConnectorType)
+		s.renderEditError(w, r, conn, "unknown connector type "+conn.ConnectorType)
 		return
 	}
 
@@ -160,7 +160,7 @@ func (s *Server) handleConnectionEditSave(w http.ResponseWriter, r *http.Request
 		// Render from cfg (the in-progress config carrying every
 		// field parsed successfully so far) so the operator's typed
 		// values are preserved in the form.
-		s.renderEditErrorWithConfig(w, conn, cfg, msg)
+		s.renderEditErrorWithConfig(w, r, conn, cfg, msg)
 		return
 	}
 
@@ -170,7 +170,7 @@ func (s *Server) handleConnectionEditSave(w http.ResponseWriter, r *http.Request
 	// here gives a clearer banner that preserves the rest of the form.
 	if conn.ConnectorType == "http_proxy" {
 		if aqp, _ := cfg["auth_query_param"].(string); aqp != "" && !authQueryParamPattern.MatchString(aqp) {
-			s.renderEditErrorWithConfig(w, conn, cfg,
+			s.renderEditErrorWithConfig(w, r, conn, cfg,
 				"auth_query_param must contain only letters, digits, _, -, or . (got "+aqp+")")
 			return
 		}
@@ -179,7 +179,7 @@ func (s *Server) handleConnectionEditSave(w http.ResponseWriter, r *http.Request
 	if err := s.connections.UpdateConfig(id, cfg); err != nil {
 		// Render from cfg so the operator's attempted values are
 		// preserved on the retry.
-		s.renderEditErrorWithConfig(w, conn, cfg, "save failed: "+err.Error())
+		s.renderEditErrorWithConfig(w, r, conn, cfg, "save failed: "+err.Error())
 		return
 	}
 
@@ -331,8 +331,8 @@ func joinStringSliceField(v any) string {
 // Use this only when the in-progress config isn't meaningful (e.g.,
 // unknown connector type, where there's nothing successfully parsed
 // to preserve).
-func (s *Server) renderEditError(w http.ResponseWriter, conn *connections.Connection, msg string) {
-	s.renderEditErrorWithConfig(w, conn, conn.Config, msg)
+func (s *Server) renderEditError(w http.ResponseWriter, r *http.Request, conn *connections.Connection, msg string) {
+	s.renderEditErrorWithConfig(w, r, conn, conn.Config, msg)
 }
 
 // renderEditErrorWithConfig re-renders the edit page using a specific
@@ -344,10 +344,16 @@ func (s *Server) renderEditError(w http.ResponseWriter, conn *connections.Connec
 // Sets Content-Type and the 400 status on the ResponseWriter directly
 // rather than calling s.render's helper, so we don't trip the
 // "superfluous WriteHeader" warning when render writes its own
-// content-type header.
-func (s *Server) renderEditErrorWithConfig(w http.ResponseWriter, conn *connections.Connection, cfg map[string]any, msg string) {
+// content-type header. The CSRFToken is injected manually here too,
+// since we bypass s.render — without it, the rendered error page
+// would carry an empty window.SIEVE_CSRF and the operator's retry
+// POST would 403 at the middleware.
+func (s *Server) renderEditErrorWithConfig(w http.ResponseWriter, r *http.Request, conn *connections.Connection, cfg map[string]any, msg string) {
 	data := s.connectionEditViewFromConfig(conn, cfg)
 	data.Error = msg
+	if sess := sessionFromContext(r); sess != nil {
+		data.CSRFToken = sess.CSRFToken
+	}
 	w.Header().Set("Content-Type", "text/html; charset=utf-8")
 	w.WriteHeader(http.StatusBadRequest)
 	t, ok := s.templates["connection_edit"]