Report or refuse each unusable file webhooker reads (closes #290)

Audit of the files webhooker reads configuration or required state
from. A missing or zero-length database is reported with the "created
a new, empty database" warning and its path: webhooker.db at start,
and a per-webhook database at the latest at the next start, since
restart recovery now opens every webhook's database. The main
database's open errors name webhooker.db
(#459). webhooker resetpw
refuses a zero-length webhooker.db as it refuses a missing one. A
directory in place of a database file or its -wal or -shm is refused,
naming it; beside a -shm directory SQLite opened the database
read-only without a word. The README says how each case is treated.

Model: opus-5-5

README.md

@@ -569,8 +569,9 @@ A start that finds no `webhooker.db` in `DATA_DIR`, or a zero-length
 one (which SQLite opens as an empty database), also logs
 `created a new, empty database` at `WARN`, with the file's path,
 shortly before the banner. On a deployment that has run before, that
-line means `DATA_DIR` was empty, most often because its volume is not
+line means `webhooker.db` was lost: either `DATA_DIR` was empty, most
-mounted.
+often because its volume is not mounted, or the file was zero-length,
+as a truncated copy leaves it.
 
 #### Recovering a lost admin password
 
@@ -614,7 +615,8 @@ What it will not do:
   the old password, so a reset underneath it would report a change the
   service does not honour.
 - **Create anything.** A `DATA_DIR` that does not exist, or that holds
-  no `webhooker.db`, is an error rather than a new empty deployment —
+  no `webhooker.db` or a zero-length one, is an error naming the path
+  rather than a new empty deployment —
   a mistyped path must not be built out and then reported as a success.
 - **Create an account.** A username that does not exist is an error.
   `resetpw` changes an existing account's password and nothing else.
@@ -995,6 +997,16 @@ its sidecars; a killed or crashed instance leaves them, and they must be
 carried with the `.db`. An archive the service has not opened since a
 crash keeps that crash's sidecars, even across a later clean stop.
 
+A missing sidecar is therefore normal, and SQLite makes new ones, so a
+`-wal` lost from a copy cannot be reported: the transactions it held
+are simply gone. SQLite reads a `-wal` up to its first damaged frame,
+as after a crash, and rebuilds a damaged `-shm`. A sidecar with the
+wrong mode is set back to `0600` when its database is opened. A
+directory in place of either is refused then, with an error naming it:
+for `webhooker.db` the server and `webhooker resetpw` stop, and an event
+or archive database fails as a damaged one does (see
+[Database Architecture](#database-architecture)).
+
 Configuration is **not** in `DATA_DIR` — it comes from the environment
 and from a `.env` file read out of the process working directory. Back
 that up with your deployment config, separately.
@@ -1077,13 +1089,17 @@ with any `-wal`/`-shm` beside it, or wait until there are none.
 1. Stop the service.
 
 2. Restore the **whole set together**: `webhooker.db` *and* every
-   `events-*.db` *and* every `archive-*.db`. A partial restore fails
+   `events-*.db` *and* every `archive-*.db`. A partial restore is
-   quietly rather than loudly. Every database is opened `mode=rwc`, so a
+   reported, not refused. Every database is opened `mode=rwc`, so a
-   missing `events-{uuid}.db` is **created empty** on first access
+   missing `events-{uuid}.db` is **created empty**: the webhook comes
-   instead of erroring — the webhook comes back with its configuration
+   back with its configuration intact and its entire event history
-   intact and its entire event history silently gone. Event databases
+   gone. The first start after the restore logs
-   restored without `webhooker.db` are simply orphaned; nothing
+   `created a new, empty database` at `WARN` for each such file, with
-   references their UUIDs.
+   its path, as it does for a missing `webhooker.db`. A missing
+   `archive-*.db` is recreated at its target's next delivery without a
+   warning, since moving one away is a supported workflow. Event
+   databases restored without `webhooker.db` are simply orphaned;
+   nothing references their UUIDs.
 
 3. Carry any `*.db-wal` and `*.db-shm` files that are in the backup.
    They are part of the database, and dropping a `-wal` silently
@@ -1948,7 +1964,9 @@ A per-webhook database that is missing or zero-length later means its
 webhook's events and pending deliveries are gone. The next time it is
 opened, an empty one is created in its place, so the webhook keeps
 receiving, and `created a new, empty database` is logged at `WARN` with
-the file's path. A file there that SQLite cannot open fails that
+the file's path. Every webhook's database is opened when the service
+starts, so this appears at the latest at the first start after the
+file was lost. A file there that SQLite cannot open fails that
 webhook alone, with an `ERROR` naming the webhook on every access and a
 500 to its senders, so one damaged file does not stop the others.
 

internal/database/database.go

@@ -211,13 +211,15 @@ func (d *Database) connectTo(dataDir string) error {
 	if err != nil {
 		d.log.Error(
 			"failed to open database",
+			"path", dbPath,
 			"error", err,
 		)
 
 		return err
 	}
 
-	// Then use it with GORM
+	// Then use it with GORM. Its errors are SQLite's alone and name no
+	// file, so the path is added to them here.
 	db, err := gorm.Open(sqlite.Dialector{
 		Conn: sqlDB,
 	}, &gorm.Config{
@@ -227,10 +229,11 @@ func (d *Database) connectTo(dataDir string) error {
 	if err != nil {
 		d.log.Error(
 			"failed to connect to database",
+			"path", dbPath,
 			"error", err,
 		)
 
-		return err
+		return fmt.Errorf("connecting to %s: %w", dbPath, err)
 	}
 
 	d.db = db
@@ -241,8 +244,12 @@ func (d *Database) connectTo(dataDir string) error {
 		d.log.Info("connected to database", "path", dbPath)
 	}
 
-	// Run migrations
+	err = d.migrate()
-	return d.migrate()
+	if err != nil {
+		return fmt.Errorf("migrating %s: %w", dbPath, err)
+	}
+
+	return nil
 }
 
 func (d *Database) migrate() error {

internal/database/database_test.go

@@ -1,9 +1,15 @@
 package database_test
 
 import (
+	"bytes"
 	"context"
+	"log/slog"
+	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"go.uber.org/fx/fxtest"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/database"
@@ -100,3 +106,22 @@ func TestDatabaseConnection(t *testing.T) {
 		)
 	}
 }
+
+// TestOpen_UnreadableDatabaseIsNamed pins
+// https://git.eeqj.de/sneak/webhooker/issues/459: when SQLite cannot
+// read webhooker.db, the error that stops the server and `webhooker
+// resetpw` names the file, not only SQLite's own message.
+func TestOpen_UnreadableDatabaseIsNamed(t *testing.T) {
+	t.Parallel()
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, database.MainDBFileName)
+	require.NoError(t, os.WriteFile(
+		path, bytes.Repeat([]byte("junk"), 1024), database.SQLiteFilePerm,
+	))
+
+	_, err := database.Open(dir, slog.New(slog.DiscardHandler))
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), path)
+	assert.Contains(t, err.Error(), "file is not a database")
+}

internal/database/retention.go

@@ -184,8 +184,8 @@ func (r *RetentionReaper) sweep(ctx context.Context) {
 
 		wh := webhooks[i]
 
-		// Nothing to reap if the per-webhook database has never
+		// A missing database has nothing to reap. Restart recovery
-		// been created.
+		// reports a lost one (see WebhookDBManager.GetDB).
 		if !r.dbManager.DBExists(wh.ID) {
 			continue
 		}

internal/database/sqlite_mode_test.go

@@ -182,6 +182,29 @@ func TestOpenSQLiteTightensFilesLeftWorldReadable(t *testing.T) {
 	requireDatabaseSetOwnerOnly(t, path)
 }
 
+// TestOpenSQLiteRefusesADirectorySidecar covers a directory in place
+// of -wal or -shm. Beside a -shm directory SQLite opens the database
+// read-only without a word, and every write then fails naming no file,
+// so the open must stop instead, naming the directory.
+func TestOpenSQLiteRefusesADirectorySidecar(t *testing.T) {
+	t.Parallel()
+
+	for _, suffix := range []string{"-wal", "-shm"} {
+		t.Run(suffix, func(t *testing.T) {
+			t.Parallel()
+
+			path := filepath.Join(t.TempDir(), database.MainDBFileName)
+			require.NoError(t, os.Mkdir(path+suffix, 0o700))
+
+			_, err := database.OpenSQLite(
+				path, database.SQLiteModeCreate,
+			)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), path+suffix)
+		})
+	}
+}
+
 // TestOpenSQLiteExistingModeDoesNotCreateTheFile guards the mechanism
 // the fix uses: OpenSQLite now creates the database file itself, and
 // must not do so for a caller that asked for an existing database. An

internal/database/sqlite_open.go

@@ -7,6 +7,7 @@ import (
 	"io/fs"
 	"net/url"
 	"os"
+	"syscall"
 	"time"
 
 	_ "modernc.org/sqlite" // Pure Go SQLite driver
@@ -93,7 +94,8 @@ const (
 const SQLiteFilePerm fs.FileMode = 0o600
 
 // reserveSQLiteFile puts path at SQLiteFilePerm before the driver ever
-// touches it, and tightens any sidecar already on disk.
+// touches it, and tightens any sidecar already on disk. A directory in
+// place of any of them is an error naming it.
 //
 // The mode has to be settled here rather than by a chmod after opening,
 // because SQLite picks it: robust_open substitutes
@@ -143,7 +145,15 @@ func reserveSQLiteFile(path string, create bool) error {
 	for _, p := range append(
 		[]string{path}, sqliteSidecarPaths(path)...,
 	) {
-		err := os.Chmod(p, SQLiteFilePerm)
+		// Chmod accepts a directory, and SQLite opens a database whose
+		// -shm is one read-only, without a word: every write then
+		// fails naming no file.
+		info, err := os.Stat(p)
+		if err == nil && info.IsDir() {
+			return fmt.Errorf("securing %s: %w", p, syscall.EISDIR)
+		}
+
+		err = os.Chmod(p, SQLiteFilePerm)
 		if err != nil && !errors.Is(err, fs.ErrNotExist) {
 			return fmt.Errorf("securing %s: %w", p, err)
 		}

internal/delivery/engine.go

@@ -699,10 +699,9 @@ func (e *Engine) recoverInFlight(ctx context.Context) {
 		default:
 		}
 
-		if !e.dbManager.DBExists(webhookID) {
+		// Opened even when its file is missing, so that GetDB reports
-			continue
+		// a lost database at start, not when the webhook next receives
-		}
+		// an event, which for a quiet webhook may be never.
-
 		e.recoverWebhookDeliveries(ctx, webhookID)
 	}
 }

internal/delivery/engine_integration_test.go

@@ -1,6 +1,7 @@
 package delivery_test
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -1134,6 +1135,40 @@ func TestRecoverInFlight_WithPendingDeliveries(
 	}
 }
 
+// TestRecoverInFlight_ReportsAMissingWebhookDatabase covers a webhook
+// whose database file is gone, after a partial restore say. Restart
+// recovery opens every webhook's database, so the empty one made in its
+// place is reported at start, naming the file
+// (https://git.eeqj.de/sneak/webhooker/issues/290).
+func TestRecoverInFlight_ReportsAMissingWebhookDatabase(t *testing.T) {
+	t.Parallel()
+
+	mainDB := iMainDB(t)
+	webhookID := uuid.New().String()
+	iCreateWebhook(t, mainDB, webhookID, "lost-database")
+
+	var logs bytes.Buffer
+
+	dbMgr := database.NewTestWebhookDBManagerWithLogger(
+		t.TempDir(), slog.New(slog.NewTextHandler(&logs, nil)),
+	)
+	t.Cleanup(func() { _ = dbMgr.CloseAll() })
+
+	engine := delivery.NewTestEngineWithDB(
+		database.NewTestDatabase(mainDB), dbMgr,
+		slog.New(slog.DiscardHandler),
+		&http.Client{Timeout: 5 * time.Second}, 1,
+	)
+
+	engine.ExportRecoverInFlight(context.Background())
+
+	assert.Contains(
+		t, logs.String(),
+		`level=WARN msg="created a new, empty database" webhook_id=`+
+			webhookID+" path="+dbMgr.DBPath(webhookID),
+	)
+}
+
 // --- HTTP Config with custom headers ---
 
 func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {

internal/gormlog/scan_guard_test.go

@@ -14,18 +14,16 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// minNonTestFiles guards the walk below against passing because it
+// isRowProducer reports whether name is GORM's Row or database/sql's
-// found nothing to look at. The tree held 60 non-test .go files when
+// QueryRow or QueryRowContext, which return a *sql.Row whose Scan is
-// this was written.
+// database/sql's and not (*gorm.DB).Scan. GORM's Rows is not listed:
-const minNonTestFiles = 40
+// it also returns an error, so Scan is never called on its result
-
+// directly. It matches the method name only and resolves no types, so
-// isRowProducer reports whether name is a method that returns a
+// a repo-local method with one of these names that returns *gorm.DB
-// database/sql row handle. GORM's Row and Rows return *sql.Row and
+// gets past it: Scan on that method's result is not reported.
-// *sql.Rows, so Scan on the result of one of them is database/sql's
-// Scan and never (*gorm.DB).Scan.
 func isRowProducer(name string) bool {
 	switch name {
-	case "Row", "Rows", "QueryRow", "QueryRowContext":
+	case "Row", "QueryRow", "QueryRowContext":
 		return true
 	default:
 		return false
@@ -50,9 +48,14 @@ func receiverIsRowHandle(x ast.Expr) bool {
 }
 
 // unguardedScans returns the position of every Scan call in file whose
-// receiver is not a row handle. It fails closed: a receiver it cannot
+// receiver is not a call to a row producer. It fails closed: any other
-// resolve syntactically — a local variable, a struct field — is
+// receiver — a local variable, a struct field, a call to any other
-// reported rather than assumed safe.
+// method — is reported rather than assumed safe.
+//
+// It sees only calls written x.Scan(...). A method value, f := db.Scan
+// followed by f(&v), is out of scope: Scan is never the called
+// expression there, and nobody writes a query that way by accident,
+// which is the mistake this check exists to catch.
 func unguardedScans(
 	fset *token.FileSet, file *ast.File,
 ) []token.Position {
@@ -111,15 +114,15 @@ func skipDir(name string) bool {
 	}
 }
 
-// walkNonTestGo parses every non-test .go file under root and returns
+// walkNonTestGo parses every non-test .go file under root. It returns
-// how many it parsed along with every unguarded Scan it found.
+// the directories, relative to root, it parsed a file in, along with
-func walkNonTestGo(t *testing.T, root string) (int, []string) {
+// every unguarded Scan it found.
+func walkNonTestGo(t *testing.T, root string) (map[string]bool, []string) {
 	t.Helper()
 
-	var (
+	walked := map[string]bool{}
-		parsed int
+
-		hits   []string
+	var hits []string
-	)
 
 	fset := token.NewFileSet()
 
@@ -147,7 +150,12 @@ func walkNonTestGo(t *testing.T, root string) (int, []string) {
 				return err
 			}
 
-			parsed++
+			dir, err := filepath.Rel(root, filepath.Dir(path))
+			if err != nil {
+				return err
+			}
+
+			walked[dir] = true
 
 			for _, pos := range unguardedScans(fset, file) {
 				hits = append(hits, relPosition(root, pos))
@@ -157,7 +165,7 @@ func walkNonTestGo(t *testing.T, root string) (int, []string) {
 		},
 	))
 
-	return parsed, hits
+	return walked, hits
 }
 
 // isNonTestGo reports whether a file name is Go source this check
@@ -189,19 +197,39 @@ func relPosition(root string, pos token.Position) string {
 // logged with its values interpolated. The package comment states the
 // limit; this fails when someone adds a call site anyway.
 //
-// The current tree has one caller, internal/database/database_test.go,
+// Test files are not governed: what a test binds is fixture data.
-// which this check does not govern: it is test-only and its SELECT 1
-// binds nothing.
 func TestGormScanIsNeverCalledOutsideTests(t *testing.T) {
 	t.Parallel()
 
-	parsed, offenders := walkNonTestGo(t, moduleRoot(t))
+	root := moduleRoot(t)
+	walked, offenders := walkNonTestGo(t, root)
+
+	// The module's packages are static, templates, and every directory
+	// directly under cmd and internal. Each holds non-test code, so one
+	// the walk parsed nothing in was skipped, and a Scan there would
+	// pass unseen.
+	packages := []string{"static", "templates"}
+
+	for _, parent := range []string{"cmd", "internal"} {
+		entries, err := os.ReadDir(filepath.Join(root, parent))
+		require.NoError(t, err)
+
+		for _, entry := range entries {
+			if !entry.IsDir() {
+				continue
+			}
+
+			packages = append(packages, filepath.Join(parent, entry.Name()))
+		}
+	}
+
+	for _, dir := range packages {
+		require.True(
+			t, walked[dir],
+			"the walk parsed no non-test .go file in %s", dir,
+		)
+	}
 
-	require.GreaterOrEqual(
-		t, parsed, minNonTestFiles,
-		"parsed %d non-test .go files, so this check found "+
-			"nothing to look at", parsed,
-	)
 	require.Empty(
 		t, offenders,
 		"Scan called on a receiver this check cannot show is a "+
@@ -222,18 +250,51 @@ type scanGuardCase struct {
 	want int
 }
 
+// scanGuardCases covers each receiver form unguardedScans names, plus
+// each row producer isRowProducer lets through. Each body is valid Go
+// inside plantedFile.
 func scanGuardCases() []scanGuardCase {
 	return []scanGuardCase{
-		{"gorm chain", `db.DB().Raw("SELECT 1").Scan(&v)`, 1},
+		{"local variable", "q := gdb.Raw(\"SELECT 1\")\n\tq.Scan(&v)", 1},
-		{"gorm receiver", `gdb.Scan(&v)`, 1},
+		{"struct field", `s.db.Scan(&v)`, 1},
-		{"gorm via variable", "q := gdb.Raw(\"x\")\nq.Scan(&v)", 1},
+		{"gorm chain", `gdb.Raw("SELECT 1").Scan(&v)`, 1},
-		{"gorm model chain", `gdb.Model(&x).Scan(&v)`, 1},
+		{
-		{"sql row", `gdb.Raw("SELECT 1").Row().Scan(&v)`, 0},
+			"sql rows in a variable",
-		{"sql rows", `gdb.Raw("SELECT 1").Rows().Scan(&v)`, 0},
+			"rows, _ := gdb.Raw(\"SELECT 1\").Rows()\n\trows.Scan(&v)",
+			1,
+		},
+		{"gorm Row", `gdb.Raw("SELECT 1").Row().Scan(&v)`, 0},
+		{"sql QueryRow", `sqlDB.QueryRow("SELECT 1").Scan(&v)`, 0},
+		{
+			"sql QueryRowContext",
+			`sqlDB.QueryRowContext(ctx, "SELECT 1").Scan(&v)`,
+			0,
+		},
 		{"unrelated call", `gdb.Find(&v)`, 0},
 	}
 }
 
+// plantedFile wraps one case body in a function that declares every
+// name the bodies use, so each body is the Go it stands for. The result
+// is parsed, never compiled.
+const plantedFile = `package p
+
+import (
+	"context"
+	"database/sql"
+
+	"gorm.io/gorm"
+)
+
+type store struct{ db *gorm.DB }
+
+func f(ctx context.Context, gdb *gorm.DB, sqlDB *sql.DB, s store) {
+	var v int
+
+	%s
+}
+`
+
 // TestScanGuard_ReportsPlantedCalls proves the check fires. Without it
 // a detector that matched nothing would satisfy the walk above no
 // matter what the tree contained.
@@ -245,9 +306,7 @@ func TestScanGuard_ReportsPlantedCalls(t *testing.T) {
 			t.Parallel()
 
 			fset := token.NewFileSet()
-			src := fmt.Sprintf(
+			src := fmt.Sprintf(plantedFile, tc.body)
-				"package p\n\nfunc f() {\n\t%s\n}\n", tc.body,
-			)
 
 			file, err := parser.ParseFile(
 				fset, tc.name+".go", src, 0,

internal/handlers/event_resubmit.go

@@ -145,8 +145,9 @@ func (h *Handlers) resubmitEvent(
 // per-webhook database files — a sibling webhook's event is not in the
 // database being queried at all — and is there so the scoping survives
 // any future change that puts more than one webhook's events in one
-// file. Going through Model applies GORM's soft-delete scope, which is
+// file. A reaped event is not found because the retention reaper
-// what stops a reaped event being resubmitted.
+// deletes its row outright rather than marking it deleted; see
+// deleteEvents in internal/database/retention.go.
 func loadResubmitSource(
 	webhookDB *gorm.DB,
 	webhookID, eventID string,

internal/handlers/webhook.go

@@ -272,10 +272,12 @@ func requestEventSource(
 
 // createAndFanOut writes the event and one pending delivery per target,
 // and adds them to the webhook's running totals, in a single
-// transaction, then hands the tasks to the delivery engine. It is the
+// transaction, then hands the tasks to the delivery engine. Every
-// only path by which an event and its deliveries are created, so a
+// event is created here, received or resubmitted, so a resubmitted
-// resubmitted event is retried, SSRF-guarded and circuit-broken
+// event is retried, SSRF-guarded and circuit-broken exactly as a
-// exactly as a received one is.
+// received one is. Per-delivery replay is the one other path that
+// creates a delivery: it adds one to an existing event without
+// coming through here.
 //
 // The tasks are returned as well as queued, so a caller can report how
 // many targets the event went to.

internal/resetpw/resetpw.go

@@ -308,7 +308,7 @@ func checkDataDir(dir string) error {
 
 	dbPath := filepath.Join(dir, database.MainDBFileName)
 
-	_, err = os.Stat(dbPath)
+	dbInfo, err := os.Stat(dbPath)
 
 	switch {
 	case errors.Is(err, fs.ErrNotExist):
@@ -319,6 +319,15 @@ func checkDataDir(dir string) error {
 		)
 	case err != nil:
 		return fmt.Errorf("checking %s: %w", dbPath, err)
+	case dbInfo.Size() == 0:
+		// SQLite opens a zero-length file as an empty database, so
+		// it holds no deployment either, and opening it would write
+		// an empty schema into it.
+		return fmt.Errorf(
+			"%w: %s is zero-length. The admin account is created by "+
+				"the first server start",
+			ErrNoDatabase, dbPath,
+		)
 	}
 
 	return nil

internal/resetpw/resetpw_test.go

@@ -377,6 +377,33 @@ func TestMissingDatabaseCreatesNothing(t *testing.T) {
 	)
 }
 
+// TestZeroLengthDatabaseCreatesNothing covers a webhooker.db left at
+// zero length, as a truncated copy leaves it. SQLite would open it as
+// an empty database, so it is refused like a missing one and left as
+// it is.
+func TestZeroLengthDatabaseCreatesNothing(t *testing.T) {
+	dir := t.TempDir()
+	t.Setenv("DATA_DIR", dir)
+
+	dbPath := filepath.Join(dir, database.MainDBFileName)
+	require.NoError(
+		t, os.WriteFile(dbPath, nil, database.SQLiteFilePerm),
+	)
+
+	code, _, stderr := run(t, newPassword+"\n", operatorUser)
+
+	require.Equal(t, exitFailure, code)
+	assert.Contains(t, stderr, dbPath)
+
+	entries, err := os.ReadDir(dir)
+	require.NoError(t, err)
+	assert.Len(t, entries, 1, "nothing may be created beside it")
+
+	info, err := os.Stat(dbPath)
+	require.NoError(t, err)
+	assert.Zero(t, info.Size(), "nothing may be written into it")
+}
+
 // TestUnknownUserFails states the decision: resetpw changes an
 // existing account's password and never creates an account. A typo in
 // the username must say so rather than quietly adding a second user.

internal/server/resubmit_test.go

@@ -0,0 +1,215 @@
+package server_test
+
+import (
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// maxResubmits bounds the requests the tests below send to the
+// resubmit route. The route's rate limit belongs to the middleware;
+// this only has to sit well above it, so that a route without the
+// limiter fails its test instead of looping.
+const maxResubmits = 100
+
+// resubmitPath is the resubmit route for one stored event.
+func resubmitPath(webhookID, eventID string) string {
+	return "/hook/" + webhookID + "/events/" + eventID + "/resubmit"
+}
+
+// csrfForm is a resubmit form carrying the given CSRF token.
+func csrfForm(token string) url.Values {
+	form := url.Values{}
+	form.Set("csrf_token", token)
+
+	return form
+}
+
+// TestEventResubmit_SignedOutRequestsNeverReachTheRateLimit pins
+// RequireAuth on the resubmit route. The handler also turns away a
+// request without a session, with the same redirect, so a refusal
+// alone would pass without RequireAuth. What RequireAuth adds is that
+// it refuses such a request before the route's rate limit, so a
+// signed-out client cannot spend the budget a signed-in user
+// resubmits from. Each request carries a CSRF token valid for its own
+// cookie, so CSRF lets it through to RequireAuth.
+func TestEventResubmit_SignedOutRequestsNeverReachTheRateLimit(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	userID, _ := env.seedUser(t, "resubmitter", "somepassword")
+	wh := env.seedWebhook(t, userID)
+	evt := env.seedEvent(t, wh.ID, `{"resubmit":"me"}`)
+	path := resubmitPath(wh.ID, evt.ID)
+	logsPath := "/hook/" + wh.ID + "/events"
+
+	token, signedOut := env.csrfFrom(t, "/pages/login", nil)
+
+	for i := range maxResubmits {
+		w := env.post(path, csrfForm(token), signedOut)
+		require.Equal(t, http.StatusSeeOther, w.Code, "request %d", i)
+		require.Equal(
+			t, "/pages/login", w.Header().Get("Location"),
+			"request %d", i,
+		)
+	}
+
+	require.Equal(
+		t, int64(1), env.countEvents(t, wh.ID),
+		"a signed-out request must store nothing",
+	)
+
+	token, cookies := env.csrfFrom(
+		t, logsPath, env.authCookies(t, userID, "resubmitter"),
+	)
+
+	env.requireNotice(
+		t, env.post(path, csrfForm(token), cookies),
+		logsPath, "resubmit-no-targets",
+		"this source has no active targets", cookies,
+	)
+}
+
+// TestEventResubmit_RefusedWithoutAValidCSRFToken pins CSRF on the
+// resubmit route: a signed-in user's POST is refused with 403, and
+// stores nothing, unless it carries the token issued to that user's
+// own browser.
+func TestEventResubmit_RefusedWithoutAValidCSRFToken(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	userID, _ := env.seedUser(t, "resubmitter", "somepassword")
+	wh := env.seedWebhook(t, userID)
+	evt := env.seedEvent(t, wh.ID, `{"resubmit":"me"}`)
+	path := resubmitPath(wh.ID, evt.ID)
+	logsPath := "/hook/" + wh.ID + "/events"
+
+	token, cookies := env.csrfFrom(
+		t, logsPath, env.authCookies(t, userID, "resubmitter"),
+	)
+	otherBrowsers, _ := env.csrfFrom(t, "/pages/login", nil)
+
+	for name, form := range map[string]url.Values{
+		"no token":                {},
+		"a malformed token":       csrfForm("not-a-token"),
+		"another browser's token": csrfForm(otherBrowsers),
+	} {
+		assert.Equal(
+			t, http.StatusForbidden,
+			env.post(path, form, cookies).Code, name,
+		)
+	}
+
+	assert.Equal(
+		t, int64(1), env.countEvents(t, wh.ID),
+		"a refused request must store nothing",
+	)
+
+	// The same request with the user's own token goes through, so the
+	// refusals above were the token's doing.
+	env.requireNotice(
+		t, env.post(path, csrfForm(token), cookies),
+		logsPath, "resubmit-no-targets",
+		"this source has no active targets", cookies,
+	)
+}
+
+// TestEventResubmit_AnotherWebhooksEvent404s pins, on the route as
+// registered, that a signed-in user gets 404, and nothing is stored,
+// for an event of a webhook another user owns, which the handler's
+// ownership check refuses, and for another webhook's event posted
+// under a webhook the user does own, which the event lookup refuses.
+// The user's own event, posted the same way, is accepted, so the
+// second 404 comes from the lookup and not from a route that never
+// passed the event ID to the handler.
+func TestEventResubmit_AnotherWebhooksEvent404s(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	ownerID, _ := env.seedUser(t, "owner", "somepassword")
+	owners := env.seedWebhook(t, ownerID)
+	ownersEvent := env.seedEvent(t, owners.ID, `{"owner":"only"}`)
+
+	intruderID, _ := env.seedUser(t, "intruder", "somepassword")
+	intruders := env.seedWebhook(t, intruderID)
+	intrudersEvent := env.seedEvent(
+		t, intruders.ID, `{"intruder":"own"}`,
+	)
+	intrudersLogs := "/hook/" + intruders.ID + "/events"
+
+	token, cookies := env.csrfFrom(
+		t, intrudersLogs, env.authCookies(t, intruderID, "intruder"),
+	)
+
+	for name, path := range map[string]string{
+		"another user's webhook": resubmitPath(
+			owners.ID, ownersEvent.ID,
+		),
+		"another webhook's event": resubmitPath(
+			intruders.ID, ownersEvent.ID,
+		),
+	} {
+		w := env.post(path, csrfForm(token), cookies)
+		assert.Equal(t, http.StatusNotFound, w.Code, name)
+	}
+
+	assert.Equal(t, int64(1), env.countEvents(t, owners.ID))
+	assert.Equal(t, int64(1), env.countEvents(t, intruders.ID))
+
+	env.requireNotice(
+		t,
+		env.post(
+			resubmitPath(intruders.ID, intrudersEvent.ID),
+			csrfForm(token), cookies,
+		),
+		intrudersLogs, "resubmit-no-targets",
+		"this source has no active targets", cookies,
+	)
+}
+
+// TestEventResubmit_RateLimited pins the rate limit on the resubmit
+// route: a signed-in user's resubmits are accepted until the budget
+// is spent, and then refused with 429.
+func TestEventResubmit_RateLimited(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	userID, _ := env.seedUser(t, "resubmitter", "somepassword")
+	wh := env.seedWebhook(t, userID)
+	evt := env.seedEvent(t, wh.ID, `{"resubmit":"me"}`)
+	path := resubmitPath(wh.ID, evt.ID)
+
+	token, cookies := env.csrfFrom(
+		t, "/hook/"+wh.ID+"/events",
+		env.authCookies(t, userID, "resubmitter"),
+	)
+
+	limited := false
+
+	for range maxResubmits {
+		code := env.post(path, csrfForm(token), cookies).Code
+		if code == http.StatusTooManyRequests {
+			limited = true
+
+			break
+		}
+
+		require.Equal(
+			t, http.StatusSeeOther, code,
+			"a resubmit within the budget must be accepted",
+		)
+	}
+
+	assert.True(
+		t, limited, "repeated resubmits must eventually be refused",
+	)
+}

internal/server/routes_test.go

@@ -444,6 +444,23 @@ func (e *testEnv) countDeliveries(
 	return count
 }
 
+// countEvents reports how many events a webhook's database holds.
+func (e *testEnv) countEvents(t *testing.T, webhookID string) int64 {
+	t.Helper()
+
+	webhookDB, err := e.dbMgr.GetDB(webhookID)
+	require.NoError(t, err)
+
+	var count int64
+
+	require.NoError(
+		t,
+		webhookDB.Model(&database.Event{}).Count(&count).Error,
+	)
+
+	return count
+}
+
 // storedHash reads the current password hash for a username.
 func (e *testEnv) storedHash(t *testing.T, username string) string {
 	t.Helper()