Record the gomodguard_v2 migration in TODO.md (closes #57)

Adds the Completed Steps entry for re-vendoring the canonical
.golangci.yml, which switches off the deprecated gomodguard and runs
gomodguard_v2 and depguard in its place.

Model: opus-5-5

.golangci.yml

@@ -10,14 +10,20 @@ run:
 
 linters:
   default: all
+  enable:
+    # Successor to the deprecated gomodguard. Named explicitly, rather than
+    # left to `default: all`, because it carries the module policy below.
+    - gomodguard_v2
   disable:
     # Genuinely incompatible with project patterns
     - exhaustruct      # Requires all struct fields
-    - depguard         # Dependency allow/block lists
     - godot            # Requires comments to end with periods
-    - wsl              # Deprecated, replaced by wsl_v5
     - wrapcheck        # Too verbose for internal packages
     - varnamelen       # Short names like db, id are idiomatic Go
+    # Deprecated: the warning is attached to the old name, so it is
+    # silenced by disabling that name, not by enabling the successor.
+    - wsl              # Deprecated, replaced by wsl_v5
+    - gomodguard       # Deprecated, replaced by gomodguard_v2
   settings:
     lll:
       line-length: 88
@@ -28,6 +34,64 @@ linters:
       max-complexity: 15
     dupl:
       threshold: 100
+    depguard:
+      # Test-support code must not be compiled into the shipped binary. A
+      # test-support package exists to hand a test privileges the program
+      # itself must never have, so a file that is not a test must not import
+      # one. Test files, and the files inside a package whose directory name
+      # ends in `test`, are where that code belongs, and are exempt.
+      #
+      # The deny list below is the one part of this file a repository is
+      # expected to extend, and the only part it may. depguard matches an
+      # import path against a list of prefixes, so it cannot be told "any path
+      # whose last segment ends in test"; a repository's own test-support
+      # packages have to be named here one at a time, by full import path,
+      # under a module path that differs from repository to repository. Add
+      # them; change nothing else.
+      rules:
+        test-support:
+          list-mode: lax
+          files:
+            - "$all"
+            - "!$test"
+            - "!**/*test/**"
+          deny:
+            - pkg: net/http/httptest
+              desc: >-
+                Test-support code belongs in test files and in packages whose
+                directory name ends in test, not in the shipped binary.
+    # Only decisions already recorded in the Go package defaults are
+    # listed here. Every entry matches the module path exactly.
+    gomodguard_v2:
+      blocked:
+        - module: github.com/rs/zerolog
+          recommendations:
+            - log/slog
+          reason: "Structured logging is stdlib log/slog."
+        # One entry per pre-fork module path, because the later releases
+        # are separate paths. A prefix match would be shorter but would
+        # also reach github.com/go-redis/redismock, the test double for
+        # the successor these entries recommend.
+        - module: github.com/go-redis/redis
+          recommendations:
+            - github.com/redis/go-redis/v9
+          reason: "Pre-fork module; use the maintained go-redis v9."
+        - module: github.com/go-redis/redis/v7
+          recommendations:
+            - github.com/redis/go-redis/v9
+          reason: "Pre-fork module; use the maintained go-redis v9."
+        - module: github.com/go-redis/redis/v8
+          recommendations:
+            - github.com/redis/go-redis/v9
+          reason: "Pre-fork module; use the maintained go-redis v9."
+        - module: github.com/sergi/go-diff
+          recommendations:
+            - github.com/aymanbagabas/go-udiff
+          reason: "No unified diff output; use go-udiff."
+        - module: github.com/hexops/gotextdiff
+          recommendations:
+            - github.com/aymanbagabas/go-udiff
+          reason: "Unmaintained fork; use go-udiff."
 
 issues:
   max-issues-per-linter: 0

TODO.md

@@ -29,26 +29,11 @@ P2: security: referer blacklist
 
 # Completed Steps
 
-- 2026-10-04 logging in, logging out, the URL generator and `/v1/e/` have
-  handler tests (closes #77): new tests in `internal/handlers`, with no
-  network, check that `GET /` without a login session shows the login form; a
-  wrong key shows it again with an error and sets no session cookie; the right
-  key answers 303 to `/` with a session cookie marked `Secure`, `HttpOnly` and
-  `SameSite=Strict`, with which `GET /` shows the generator page; `GET /logout`
-  answers 303 to `/` with an empty session cookie sent with `Max-Age=0`;
-  `POST /generate` without a login session answers 303 to `/`; `/v1/e/` serves
-  the image for a valid token, answers 410 for an expired one and 400 for one
-  with a character changed, cut short or made with another signing key; and a
-  URL made on the generator page is served by `/v1/e/`. No code changes.
-- 2026-10-04 `TestEvictionRunsOnPeriodicSchedule` no longer races the evictor
-  (closes #183): it wrote each variant file and then inserted its accounting row
-  by hand, and a reconciliation pass between the two adopted the file first, so
-  the insert failed. It now writes the files only, while holding the test
-  database's only connection so the evictor's startup pass waits after walking
-  the empty variant directory; a periodic reconciliation pass then adopts the
-  files and the eviction pass after it evicts them. No other test in
-  `internal/imgcache` inserts a row by hand after starting the evictor. Test
-  only.
+- 2026-10-04 `.golangci.yml` re-vendored from the canonical copy (closes #57):
+  the deprecated `gomodguard` is switched off, so lint runs print no
+  deprecation warning; its successor `gomodguard_v2` runs with the shared
+  module block list, and `depguard` keeps `net/http/httptest` out of files that
+  are not tests. The tree needed no code changes.
 - 2026-10-04 deployment guide and example Caddy config (closes #89):
   "Deployment" in `README.md` says what the reverse proxy in front of pixa must
   do (terminate TLS; pass `Host`, `Origin` and `Referer` on unchanged; set

internal/handlers/auth_session_internal_test.go

@@ -1,243 +0,0 @@
-package handlers
-
-import (
-	"log/slog"
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"regexp"
-	"strings"
-	"testing"
-
-	"sneak.berlin/go/pixa/internal/imgcache"
-	"sneak.berlin/go/pixa/internal/session"
-)
-
-// formatField is the generator form's format field name.
-const formatField = "format"
-
-// Markers telling the login page from the generator page.
-const (
-	loginForm     = `action="/"`
-	loginKeyInput = `name="key"`
-	generatorForm = `action="/generate"`
-)
-
-// generatedURLPattern extracts the path of the URL the generator page shows.
-// The test router runs with debug on, so the URL starts with http, and its
-// host is httptest's default request host.
-var generatedURLPattern = regexp.MustCompile(
-	`value="http://example\.com(/v1/e/[^"]+)"`)
-
-// findSessionCookie returns the session cookie rec sets, or nil if it sets
-// none.
-func findSessionCookie(rec *httptest.ResponseRecorder) *http.Cookie {
-	for _, c := range rec.Result().Cookies() {
-		if c.Name == session.CookieName {
-			return c
-		}
-	}
-
-	return nil
-}
-
-// TestHandleRoot_NoSession_ShowsLoginForm verifies that GET / without a
-// login session shows the login form.
-func TestHandleRoot_NoSession_ShowsLoginForm(t *testing.T) {
-	t.Parallel()
-
-	_, srv := newCSRFTestRouter(t)
-
-	rec := httptest.NewRecorder()
-	srv.ServeHTTP(rec, httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/", nil))
-
-	if rec.Code != http.StatusOK {
-		t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
-	}
-
-	body := rec.Body.String()
-	if !strings.Contains(body, loginForm) || !strings.Contains(body, loginKeyInput) {
-		t.Errorf("page is not the login form: %s", body)
-	}
-}
-
-// TestLoginPost_WrongKey_ShowsErrorWithoutSession verifies that a wrong key
-// shows the login form again with an error, and sets no session cookie.
-func TestLoginPost_WrongKey_ShowsErrorWithoutSession(t *testing.T) {
-	t.Parallel()
-
-	_, srv := newCSRFTestRouter(t)
-
-	cookies, token := csrfCredentials(t, srv, nil)
-
-	rec := postForm(srv, "/", cookies, url.Values{
-		loginKeyField:  {"wrong-signing-key-fedcba9876543210"},
-		csrfTokenField: {token},
-	})
-
-	if rec.Code != http.StatusOK {
-		t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK)
-	}
-
-	body := rec.Body.String()
-	if !strings.Contains(body, loginForm) || !strings.Contains(body, loginKeyInput) {
-		t.Errorf("page is not the login form: %s", body)
-	}
-
-	if !strings.Contains(body, "Invalid signing key") {
-		t.Error("login form does not show the error")
-	}
-
-	if c := findSessionCookie(rec); c != nil {
-		t.Errorf("wrong key set a session cookie: %s", c)
-	}
-}
-
-// TestLoginPost_RightKey_SetsSessionCookie verifies that the right key answers
-// 303 to / with a session cookie marked Secure, HttpOnly and SameSite=Strict,
-// and that GET / with that cookie shows the generator page.
-func TestLoginPost_RightKey_SetsSessionCookie(t *testing.T) {
-	t.Parallel()
-
-	_, srv := newCSRFTestRouter(t)
-
-	cookies, token := csrfCredentials(t, srv, nil)
-
-	rec := postForm(srv, "/", cookies, url.Values{
-		loginKeyField:  {testSigningKey},
-		csrfTokenField: {token},
-	})
-
-	if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/" {
-		t.Fatalf("status = %d, Location = %q, want %d to /",
-			rec.Code, rec.Header().Get("Location"), http.StatusSeeOther)
-	}
-
-	sessionCookie := findSessionCookie(rec)
-	if sessionCookie == nil {
-		t.Fatal("right key set no session cookie")
-	}
-
-	t.Logf("Set-Cookie: %s", sessionCookie)
-
-	if !sessionCookie.Secure {
-		t.Error("session cookie is not Secure")
-	}
-
-	if !sessionCookie.HttpOnly {
-		t.Error("session cookie is not HttpOnly")
-	}
-
-	if sessionCookie.SameSite != http.SameSiteStrictMode {
-		t.Errorf("session cookie SameSite = %v, want Strict", sessionCookie.SameSite)
-	}
-
-	req := httptest.NewRequestWithContext(t.Context(), http.MethodGet, "/", nil)
-	req.AddCookie(sessionCookie)
-
-	rec = httptest.NewRecorder()
-	srv.ServeHTTP(rec, req)
-
-	if rec.Code != http.StatusOK ||
-		!strings.Contains(rec.Body.String(), generatorForm) {
-		t.Errorf("GET / with the session cookie: status = %d, "+
-			"want %d and the generator page", rec.Code, http.StatusOK)
-	}
-}
-
-// TestHandleLogout_ClearsSessionCookie verifies that GET /logout answers 303
-// to / and replaces the session cookie with an empty one sent with
-// Max-Age=0, which makes the browser delete it.
-func TestHandleLogout_ClearsSessionCookie(t *testing.T) {
-	t.Parallel()
-
-	h, _ := newCSRFTestRouter(t)
-
-	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, "/logout", nil)
-	req.AddCookie(newSessionCookie(t, h))
-
-	rec := httptest.NewRecorder()
-	h.HandleLogout().ServeHTTP(rec, req)
-
-	if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/" {
-		t.Fatalf("status = %d, Location = %q, want %d to /",
-			rec.Code, rec.Header().Get("Location"), http.StatusSeeOther)
-	}
-
-	t.Logf("Set-Cookie: %s", rec.Header().Get("Set-Cookie"))
-
-	sessionCookie := findSessionCookie(rec)
-	if sessionCookie == nil {
-		t.Fatal("logout did not set the session cookie")
-	}
-
-	if sessionCookie.Value != "" {
-		t.Errorf("session cookie value = %q, want empty", sessionCookie.Value)
-	}
-
-	// net/http reads a Max-Age=0 attribute back as MaxAge -1.
-	if sessionCookie.MaxAge != -1 {
-		t.Errorf("session cookie MaxAge = %d, want -1 (Max-Age=0)",
-			sessionCookie.MaxAge)
-	}
-}
-
-// TestGeneratePost_NoSession_RedirectsToLogin verifies that POST /generate
-// with a valid CSRF token but no login session answers 303 to / and makes no
-// URL.
-func TestGeneratePost_NoSession_RedirectsToLogin(t *testing.T) {
-	t.Parallel()
-
-	_, srv := newCSRFTestRouter(t)
-
-	cookies, token := csrfCredentials(t, srv, nil)
-
-	rec := postForm(srv, "/generate", cookies, url.Values{
-		sourceURLField: {testSourceURL},
-		csrfTokenField: {token},
-	})
-
-	if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/" {
-		t.Fatalf("status = %d, Location = %q, want %d to /",
-			rec.Code, rec.Header().Get("Location"), http.StatusSeeOther)
-	}
-
-	if strings.Contains(rec.Body.String(), "/v1/e/") {
-		t.Error("a URL was made without a login session")
-	}
-}
-
-// TestGeneratePost_URLServesImage verifies that the URL the generator page
-// makes is served by /v1/e/. The image route runs on handlers of its own,
-// made with the same signing key.
-func TestGeneratePost_URLServesImage(t *testing.T) {
-	t.Parallel()
-
-	_, imageSrv := newSignedHostServer(t, slog.New(slog.DiscardHandler))
-
-	rec := generatePost(t, url.Values{
-		sourceURLField: {"https://" + signedHost + photoPath},
-		widthField:     {"50"},
-		heightField:    {"50"},
-		formatField:    {string(imgcache.FormatJPEG)},
-	})
-
-	if rec.Code != http.StatusOK {
-		t.Fatalf("POST /generate status = %d, want %d", rec.Code, http.StatusOK)
-	}
-
-	match := generatedURLPattern.FindStringSubmatch(rec.Body.String())
-	if match == nil {
-		t.Fatalf("generator page shows no URL: %s", rec.Body.String())
-	}
-
-	t.Logf("generated URL path: %s", match[1])
-
-	imageRec := httptest.NewRecorder()
-	imageSrv.ServeHTTP(imageRec, httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, match[1], nil))
-
-	requireServedPhoto(t, imageRec)
-}

internal/handlers/imageenc_token_internal_test.go

@@ -1,126 +0,0 @@
-package handlers
-
-import (
-	"image/jpeg"
-	"log/slog"
-	"net/http"
-	"net/http/httptest"
-	"testing"
-	"time"
-
-	"sneak.berlin/go/pixa/internal/encurl"
-)
-
-// requireServedPhoto requires that rec answers 200 with the JPEG at photoPath
-// on signedHost at the 50x50 that encPhotoURL and the generator tests ask for.
-func requireServedPhoto(t *testing.T, rec *httptest.ResponseRecorder) {
-	t.Helper()
-
-	if rec.Code != http.StatusOK {
-		t.Fatalf("status = %d, want %d; body %q",
-			rec.Code, http.StatusOK, rec.Body.String())
-	}
-
-	contentType := rec.Header().Get("Content-Type")
-	if contentType != "image/jpeg" {
-		t.Errorf("Content-Type = %q, want image/jpeg", contentType)
-	}
-
-	img, err := jpeg.DecodeConfig(rec.Body)
-	if err != nil {
-		t.Fatalf("body is not a JPEG: %v", err)
-	}
-
-	if img.Width != 50 || img.Height != 50 {
-		t.Errorf("image is %dx%d, want 50x50", img.Width, img.Height)
-	}
-}
-
-// TestHandleImageEnc_ValidToken_ServesImage verifies that a token made with
-// the signing key serves the image it asks for.
-func TestHandleImageEnc_ValidToken_ServesImage(t *testing.T) {
-	t.Parallel()
-
-	h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler))
-
-	rec := httptest.NewRecorder()
-	srv.ServeHTTP(rec, httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, encPhotoURL(t, h), nil))
-
-	requireServedPhoto(t, rec)
-}
-
-// TestHandleImageEnc_RejectedToken verifies that a token that has expired
-// answers 410, and that a token with one character changed, a token cut
-// short, and a token made with another signing key answer 400. The server
-// would serve the photo for a token it accepted.
-func TestHandleImageEnc_RejectedToken(t *testing.T) {
-	t.Parallel()
-
-	h, srv := newSignedHostServer(t, slog.New(slog.DiscardHandler))
-
-	photo := encurl.Payload{
-		SourceHost: signedHost,
-		SourcePath: photoPath,
-		Width:      50,
-		Height:     50,
-	}
-
-	valid, err := h.encGen.Generate(&photo)
-	if err != nil {
-		t.Fatalf("Generate() error = %v", err)
-	}
-
-	expiredPhoto := photo
-	expiredPhoto.ExpiresAt = time.Now().Add(-time.Minute).Unix()
-
-	expired, err := h.encGen.Generate(&expiredPhoto)
-	if err != nil {
-		t.Fatalf("Generate() error = %v", err)
-	}
-
-	otherGen, err := encurl.NewGenerator("another-signing-key-fedcba9876543210")
-	if err != nil {
-		t.Fatalf("encurl.NewGenerator() error = %v", err)
-	}
-
-	otherKey, err := otherGen.Generate(&photo)
-	if err != nil {
-		t.Fatalf("Generate() error = %v", err)
-	}
-
-	// Changing a character in the middle always changes the decoded bytes;
-	// the last character of unpadded base64 can carry unused bits.
-	middle := len(valid) / 2
-
-	replacement := "A"
-	if valid[middle] == 'A' {
-		replacement = "B"
-	}
-
-	changed := valid[:middle] + replacement + valid[middle+1:]
-
-	tests := []struct {
-		name       string
-		token      string
-		wantStatus int
-	}{
-		{"expired", expired, http.StatusGone},
-		{"one character changed", changed, http.StatusBadRequest},
-		{"cut short", valid[:middle], http.StatusBadRequest},
-		{"another signing key", otherKey, http.StatusBadRequest},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			rec := getEncToken(srv, tt.token)
-			t.Logf("GET /v1/e/%s/img.jpg: %d %s", tt.token, rec.Code, rec.Body)
-
-			if rec.Code != tt.wantStatus {
-				t.Errorf("status = %d, want %d", rec.Code, tt.wantStatus)
-			}
-		})
-	}
-}

internal/imgcache/eviction_internal_test.go

@@ -681,11 +681,6 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
 	assertNoDanglingReferences(t, cache)
 }
 
-// TestEvictionRunsOnPeriodicSchedule writes three variant files straight
-// to disk, bypassing StoreVariant, so they have no accounting rows and no
-// write-pressure notification fires. Only a periodic reconciliation pass
-// can then adopt them, and only the eviction pass that follows it can
-// evict them.
 func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
 	t.Parallel()
 
@@ -693,29 +688,13 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
 
 	cache, _ := newEvictionTestCache(t, limit)
 
-	// Hold the test database's only connection, so the startup pass
-	// waits for it after walking the still empty variant directory: the
-	// files written while it waits are first seen by a periodic pass.
-	conn, err := cache.db.Conn(t.Context())
-	if err != nil {
-		t.Fatalf("failed to take the database connection: %v", err)
-	}
-
-	defer func() { _ = conn.Close() }()
-
+	// Start the evictor while the cache is empty, then create tracked
+	// over-limit state WITHOUT going through the store methods, so no
+	// write-pressure notification fires and only the periodic ticker
+	// can trigger eviction.
 	cache.StartEviction(100 * time.Millisecond)
 	defer func() { _ = cache.StopEviction(t.Context()) }()
 
-	deadline := time.Now().Add(5 * time.Second)
-
-	for cache.db.Stats().WaitCount == 0 {
-		if time.Now().After(deadline) {
-			t.Fatal("the startup pass never waited for the database")
-		}
-
-		time.Sleep(10 * time.Millisecond)
-	}
-
 	keys := []VariantKey{
 		testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree,
 	}
@@ -724,43 +703,25 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
 	for i, key := range keys {
 		content := bytes.Repeat([]byte{fills[i]}, 1000)
 
-		_, err = cache.variants.Store(key, bytes.NewReader(content), "image/webp")
+		_, err := cache.variants.Store(key, bytes.NewReader(content), "image/webp")
 		if err != nil {
 			t.Fatalf("failed to store variant file: %v", err)
 		}
-	}
 
-	_ = conn.Close()
-
-	// Only one of the 1000-byte files fits under the limit: wait until
-	// the evictor has removed the other two.
-	stored := len(keys)
-	deadline = time.Now().Add(5 * time.Second)
-
-	for stored > 1 && time.Now().Before(deadline) {
-		time.Sleep(25 * time.Millisecond)
-
-		stored = 0
-
-		for _, key := range keys {
-			if cache.variants.Exists(key) {
-				stored++
-			}
+		_, err = cache.db.ExecContext(t.Context(),
+			`INSERT INTO variant_content (cache_key, size_bytes, content_type)
+			 VALUES (?, ?, ?)`,
+			string(key), len(content), "image/webp",
+		)
+		if err != nil {
+			t.Fatalf("failed to insert variant accounting row: %v", err)
 		}
 	}
 
-	if stored > 1 {
-		t.Fatalf("periodic schedule did not trigger eviction: %d of %d "+
-			"variant files still on disk, want at most 1", stored, len(keys))
-	}
-
-	usage, err := cache.UsageBytes(t.Context())
-	if err != nil {
-		t.Fatalf("UsageBytes failed: %v", err)
-	}
-
+	usage := waitForUsageAtOrBelow(t, cache, limit, 5*time.Second)
 	if usage > limit {
-		t.Errorf("usage after eviction = %d, want <= %d", usage, limit)
+		t.Errorf("periodic schedule did not trigger eviction: usage = %d, want <= %d",
+			usage, limit)
 	}
 
 	assertNoDanglingReferences(t, cache)