Pin the HTTP target's unpinned error checks (closes #285)

Seven error checks in internal/delivery/target_http.go could be removed without any test noticing, among them withRetry's check on writing the delivery result, the branch that leaves a sent delivery retrying and recoverable when its bookkeeping write fails. Each now fails a test when removed. The "send succeeded" case starts from a tripped circuit breaker, so a probe whose send succeeds but whose result write fails must still close the breaker. The checks in remainingBackoff and backoffElapsed stay unpinned: removing them gives the same answer, and they state a rule a reader needs. Test change only.

Model: opus-5-5

README.md

@@ -2031,29 +2031,6 @@ Because each `database` target has its own archive file, a target's
 webhook with different expiries keep two archives, each pruned on its
 own schedule.
 
-Each `database` target on the webhook page has a **Download** button,
-which returns its archive as one gzipped JSON file,
-`archive-{webhook_name}-{target_name}-{YYYYMMDDTHHMMSSZ}.json.gz`, the
-names made safe as above and the time in UTC. The file holds one
-object: `webhook` and `target`, each an `id` and a `name`;
-`exported_at`; and `archived_events`, one object per archived row with
-every column, keyed by column name. A body that is not valid UTF-8 is
-written in base64, with `"body_encoding": "base64"` beside it. An
-archive that does not exist yet, or was moved away, downloads with an
-empty `archived_events`; the download never creates the file.
-
-The download streams: each row is read and written out compressed
-before the next is read, so neither the archive nor the JSON is held in
-memory. It reads on a connection of its own, inside one read-only
-transaction, so the file holds the archive as it stood when the
-download started, and archive writes go on meanwhile, since under WAL a
-reader never blocks a writer. While it runs, the `-wal` cannot be
-checkpointed past what it reads, so a long download lets the `-wal`
-grow. It finds the file by the stored names under the lock that webhook
-edits, target edits and target creation hold, and lets go once the file
-is open: a rename during the download moves the file without affecting
-it.
-
 Deleting a webhook releases its archives: the delivery engine's cached
 archive writers are dropped and their file handles closed, so nothing
 lingers after the webhook is gone. The archive **files themselves are
@@ -2921,7 +2898,6 @@ returns to the page that was asked for.
 | `POST` | `/hook/{id}/targets`     | Add target to webhook |
 | `GET`  | `/hook/{id}/targets/{targetID}/edit` | Edit target form. The one page that renders a target's destination URL and header values in full, rather than masked |
 | `POST` | `/hook/{id}/targets/{targetID}/edit` | Edit target submission |
-| `GET`  | `/hook/{id}/targets/{targetID}/download` | Download a `database` target's archive as one gzipped JSON file. See [Database Architecture](#database-architecture) |
 | `POST` | `/hook/{id}/targets/{targetID}/delete` | Delete a target |
 | `POST` | `/hook/{id}/targets/{targetID}/toggle` | Enable or disable a target |
 
@@ -3003,7 +2979,6 @@ webhooker/
 │   │   ├── target_slack.go    # Slack/Mattermost incoming-webhook target
 │   │   ├── target_database.go # Database archive target
 │   │   ├── target_database_archive.go  # Archive file lifecycle and pruning
-│   │   ├── target_database_export.go   # Archive download as gzipped JSON
 │   │   ├── target_log.go      # Log target (stdout)
 │   │   ├── target_config_view.go  # Masked target config for templates
 │   │   ├── archive_sweeper.go # Periodic pruning of idle archives

internal/delivery/engine_integration_test.go

@@ -1425,6 +1425,32 @@ func TestDeliverHTTP_InvalidConfig(t *testing.T) {
 	)
 }
 
+// TestDeliverHTTP_InvalidConfigUnrecordedStaysPending: a delivery is
+// failed for an invalid config only once the reason is recorded.
+// Unrecorded, it stays pending, where the sweep finds it again.
+func TestDeliverHTTP_InvalidConfigUnrecordedStaysPending(t *testing.T) {
+	t.Parallel()
+
+	db := testWebhookDB(t)
+	e := testEngine(t, 1)
+
+	event, del := iSeedEventAndDelivery(
+		t, db, `{"config":"invalid"}`, "",
+	)
+
+	task, d := iHTTPTaskAndDelivery(
+		event, del, "bad-config", `not-json`, 0, 1,
+	)
+
+	require.NoError(t, db.Exec("drop table delivery_results").Error)
+
+	e.ExportDeliverHTTP(context.TODO(), db, d, task)
+
+	iAssertStatus(t, db, del.ID,
+		database.DeliveryStatusPending,
+	)
+}
+
 // --- Notify batching ---
 
 func TestNotify_MultipleTasks(t *testing.T) {

internal/delivery/engine_test.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
@@ -1056,6 +1057,21 @@ func TestParseHTTPConfig_MissingURL(t *testing.T) {
 	)
 }
 
+func TestParseHTTPConfig_Undecodable(t *testing.T) {
+	t.Parallel()
+
+	e := testEngine(t, 1)
+
+	_, err := e.ExportParseHTTPConfig(
+		`{"url":"https://example.com/hook","timeout":"soon"}`,
+	)
+
+	assert.Error(t, err,
+		"config that does not decode should return error, "+
+			"even when the part that did names a URL",
+	)
+}
+
 func TestScheduleRetry_SendsToRetryChannel(
 	t *testing.T,
 ) {
@@ -1241,6 +1257,33 @@ func TestDoHTTPRequest_ForwardsHeaders(t *testing.T) {
 	)
 }
 
+// A response that ends before the length it announced is an error, not
+// a short body.
+func TestDoHTTPRequest_CutShortResponseIsAnError(t *testing.T) {
+	t.Parallel()
+
+	ts := httptest.NewServer(
+		http.HandlerFunc(
+			func(w http.ResponseWriter, _ *http.Request) {
+				w.Header().Set("Content-Length", "100")
+				_, _ = w.Write([]byte("cut short"))
+			},
+		),
+	)
+	defer ts.Close()
+
+	e := testEngine(t, 1)
+
+	_, body, _, err := e.ExportDoHTTPRequest(
+		context.TODO(),
+		&delivery.HTTPTargetConfig{URL: ts.URL},
+		&database.Event{},
+	)
+
+	require.ErrorIs(t, err, io.ErrUnexpectedEOF)
+	assert.Empty(t, body)
+}
+
 // The event's stored inbound headers carry the same Content-Type the
 // receiver saved as the event's ContentType, so a delivery could send
 // it twice. It must go out exactly once, with a Content-Type configured
@@ -1317,6 +1360,34 @@ func TestApplyRequestHeaders_SendsOneContentType(t *testing.T) {
 	}
 }
 
+// Stored inbound headers that do not decode forward nothing, not the
+// part of them that happened to decode.
+func TestApplyRequestHeaders_UndecodableInboundForwardsNothing(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	req, err := http.NewRequestWithContext(
+		context.Background(),
+		http.MethodPost,
+		"https://target.example.com/hook",
+		http.NoBody,
+	)
+	require.NoError(t, err)
+
+	names := delivery.ExportApplyRequestHeaders(
+		req,
+		&database.Event{
+			Headers: `{"X-Custom":["value1"],"X-Broken":"not a list"}`,
+		},
+		&delivery.HTTPTargetConfig{},
+		"webhooker/dev",
+	)
+
+	assert.Empty(t, names)
+	assert.Empty(t, req.Header.Get("X-Custom"))
+}
+
 func TestProcessDelivery_RoutesToCorrectHandler(
 	t *testing.T,
 ) {

internal/delivery/recovery_durability_test.go

@@ -376,3 +376,97 @@ func TestFailedResultWriteLeavesDeliveryRecoverable(
 		database.DeliveryStatusPending,
 	)
 }
+
+// TestFailedResultWriteWithRetriesLeavesDeliveryRecoverable is the same
+// rule for a target with retries: whatever the receiver answered, the
+// delivery stays pending and no retry is scheduled. The circuit breaker
+// still learns the answer, because it describes the target's health,
+// not the database's.
+func TestFailedResultWriteWithRetriesLeavesDeliveryRecoverable(
+	t *testing.T,
+) {
+	t.Parallel()
+
+	// The "send succeeded" case starts with the breaker tripped open,
+	// so the delivery goes out as its probe and only a recorded
+	// success closes it again.
+	tests := []struct {
+		name        string
+		answer      int
+		tripped     bool
+		wantBreaker delivery.CircuitState
+	}{
+		{"send succeeded", http.StatusOK, true, delivery.CircuitClosed},
+		{"send failed", http.StatusBadGateway, false, delivery.CircuitOpen},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			s := newISetup(t)
+			targetID := uuid.New().String()
+
+			ts := httptest.NewServer(http.HandlerFunc(
+				func(w http.ResponseWriter, _ *http.Request) {
+					w.WriteHeader(tc.answer)
+				},
+			))
+			defer ts.Close()
+
+			event := iSeedEvent(
+				t, s.WebhookDB, s.WebhookID, `{"unwritable":true}`,
+			)
+
+			d := iSeedDelivery(
+				t, s.WebhookDB, event.ID, targetID,
+				database.DeliveryStatusPending,
+			)
+
+			require.NoError(
+				t,
+				s.WebhookDB.Exec("drop table delivery_results").Error,
+			)
+
+			// A single failure opens this breaker, and with no
+			// cooldown an open breaker lets the next delivery
+			// through as a probe.
+			cb := delivery.NewTestCircuitBreaker(1, 0)
+			if tc.tripped {
+				cb.RecordFailure()
+			}
+
+			s.Engine.ExportSetCircuitBreaker(targetID, cb)
+
+			full := &database.Delivery{
+				EventID:  event.ID,
+				TargetID: targetID,
+				Status:   database.DeliveryStatusPending,
+				Event:    event,
+				Target: database.Target{
+					Name:       "unwritable",
+					Type:       database.TargetTypeHTTP,
+					Config:     iHTTPConfig(ts.URL),
+					MaxRetries: 3,
+				},
+			}
+			full.ID = d.ID
+
+			sched := &recordingScheduler{}
+
+			s.Engine.ExportDeliverHTTPWithScheduler(
+				context.Background(), s.WebhookDB, full,
+				&delivery.Task{
+					DeliveryID: d.ID,
+					TargetID:   targetID,
+					AttemptNum: 1,
+				},
+				sched,
+			)
+
+			iAssertStatus(t, s.WebhookDB, d.ID, database.DeliveryStatusPending)
+			assert.Empty(t, sched.delays, "no retry may be scheduled")
+			assert.Equal(t, tc.wantBreaker, cb.State())
+		})
+	}
+}

internal/delivery/target_database.go

@@ -3,6 +3,7 @@ package delivery
 import (
 	"context"
 	"fmt"
+	"path/filepath"
 	"strings"
 	"sync"
 	"time"
@@ -276,9 +277,10 @@ func (t *databaseTarget) releaseSweepWriter(
 }
 
 // newWriter builds the writer for a database target's archive. The
-// file is the one ArchivePath gives for the webhook and the target as
-// the main database names them now; from then on only rename changes
-// the name the writer uses. It does not touch the archive file.
+// file lives beside the webhook's event database in the data
+// directory and is named for the webhook and the target as the main
+// database has them now; from then on only rename changes the name
+// the writer uses. It does not touch the archive file.
 func (t *databaseTarget) newWriter(
 	targetID string,
 ) (*archiveWriter, error) {
@@ -297,10 +299,12 @@ func (t *databaseTarget) newWriter(
 		)
 	}
 
-	w := newArchiveWriter(
-		ArchivePath(t.eng.dbManager, &target.Webhook, &target),
-		t.eng.log,
+	dir := filepath.Dir(t.eng.dbManager.DBPath(target.WebhookID))
+	name := ArchiveFileName(
+		target.Webhook.Name, target.Name, target.ID,
 	)
+
+	w := newArchiveWriter(filepath.Join(dir, name), t.eng.log)
 	w.webhookID = target.WebhookID
 
 	return w, nil

internal/delivery/target_database_export.go

@@ -1,275 +0,0 @@
-package delivery
-
-import (
-	"compress/gzip"
-	"context"
-	"database/sql"
-	"encoding/base64"
-	"encoding/json"
-	"fmt"
-	"io"
-	"log/slog"
-	"path/filepath"
-	"time"
-	"unicode/utf8"
-
-	"gorm.io/driver/sqlite"
-	"gorm.io/gorm"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/gormlog"
-)
-
-// archiveTableQuery counts the archive's table: 0 when the archive
-// writer has created the file but not yet the table in it.
-const archiveTableQuery = "SELECT count(*) FROM sqlite_master " +
-	"WHERE type = 'table' AND name = 'archived_events'"
-
-// ArchivePath returns where a database target's archive file is: in
-// the data directory, beside the webhook's event database, under the
-// name ArchiveFileName gives it.
-func ArchivePath(
-	dbMgr *database.WebhookDBManager,
-	webhook *database.Webhook,
-	target *database.Target,
-) string {
-	return filepath.Join(
-		filepath.Dir(dbMgr.DBPath(webhook.ID)),
-		ArchiveFileName(webhook.Name, target.Name, target.ID),
-	)
-}
-
-// ArchiveExportFileName returns the name a database target's archive
-// downloads under:
-// archive-WEBHOOKNAME-TARGETNAME-YYYYMMDDTHHMMSSZ.json.gz, the names
-// made safe as in ArchiveFileName and the time in UTC.
-func ArchiveExportFileName(
-	webhookName, targetName string, at time.Time,
-) string {
-	return "archive-" + archiveNamePart(webhookName) + "-" +
-		archiveNamePart(targetName) + "-" +
-		at.UTC().Format("20060102T150405Z") + ".json.gz"
-}
-
-// ArchiveExport is a database target's archive opened for download.
-// It reads the file on its own connection, inside one read-only
-// transaction, so it writes out the archive as it stood when
-// OpenArchiveExport returned.
-//
-// Archives are in WAL mode, where a reader works from a snapshot and
-// never blocks a writer: archive writes go on while an export is open,
-// and the export does not see them. SQLite cannot checkpoint the -wal
-// past an open snapshot, so the -wal grows until the export is closed.
-type ArchiveExport struct {
-	db *sql.DB
-	tx *gorm.DB
-
-	// empty is true when there is nothing to read: no file, or a file
-	// without the archive's table yet.
-	empty bool
-}
-
-// exportedName is how an export names its webhook and its target.
-type exportedName struct {
-	ID   string `json:"id"`
-	Name string `json:"name"`
-}
-
-// OpenArchiveExport opens the archive file at path for export and
-// takes the snapshot the export reads. It never creates the file: with
-// no file at path, the export has no rows.
-//
-// Once it has returned, the file is open, so a rename or a move of it
-// does not affect the export, which reads the same file under its new
-// name.
-//
-// The transaction lasts as long as ctx does, so ctx must last for the
-// whole export.
-func OpenArchiveExport(
-	ctx context.Context, path string, log *slog.Logger,
-) (*ArchiveExport, error) {
-	if !fileExists(path) {
-		return &ArchiveExport{empty: true}, nil
-	}
-
-	db, err := database.OpenSQLite(path, archiveModeExisting)
-	if err != nil {
-		return nil, fmt.Errorf("opening archive %s: %w", path, err)
-	}
-
-	gdb, err := gorm.Open(
-		sqlite.Dialector{Conn: db}, &gorm.Config{
-			// Never leave this at GORM's default. See
-			// internal/gormlog.
-			Logger: gormlog.New(log),
-		},
-	)
-	if err != nil {
-		_ = db.Close()
-
-		return nil, fmt.Errorf("opening archive %s: %w", path, err)
-	}
-
-	// ReadOnly makes the driver begin a deferred transaction in place
-	// of the BEGIN IMMEDIATE the connection string asks for, so the
-	// export never takes the archive's write lock.
-	tx := gdb.WithContext(ctx).Begin(&sql.TxOptions{ReadOnly: true})
-	if tx.Error != nil {
-		_ = db.Close()
-
-		return nil, fmt.Errorf("reading archive %s: %w", path, tx.Error)
-	}
-
-	// The transaction's first read is what takes the snapshot.
-	var tables int
-
-	err = tx.Raw(archiveTableQuery).Row().Scan(&tables)
-	if err != nil {
-		_ = tx.Rollback()
-		_ = db.Close()
-
-		return nil, fmt.Errorf("reading archive %s: %w", path, err)
-	}
-
-	return &ArchiveExport{db: db, tx: tx, empty: tables == 0}, nil
-}
-
-// WriteGzipJSON writes the export to w as one gzipped JSON object:
-// webhook and target, each an id and a name; exported_at; and
-// archived_events, one object per archived row, keyed by column name.
-// A body that is not valid UTF-8 cannot be a JSON string, so it is
-// written in base64, with "body_encoding": "base64" beside it.
-//
-// Each row is written out before the next is read, so neither the
-// archive nor its JSON is ever held in memory whole. After an error
-// the gzip stream is left unfinished, so what was written does not
-// decompress as a whole file.
-func (x *ArchiveExport) WriteGzipJSON(
-	ctx context.Context,
-	w io.Writer,
-	webhook *database.Webhook,
-	target *database.Target,
-	exportedAt time.Time,
-) error {
-	head, err := json.Marshal(map[string]any{
-		"webhook":     exportedName{ID: webhook.ID, Name: webhook.Name},
-		"target":      exportedName{ID: target.ID, Name: target.Name},
-		"exported_at": exportedAt.UTC(),
-	})
-	if err != nil {
-		return fmt.Errorf("encoding archive export: %w", err)
-	}
-
-	zw := gzip.NewWriter(w)
-
-	err = x.writeJSON(ctx, zw, head)
-	if err != nil {
-		return fmt.Errorf("writing archive export: %w", err)
-	}
-
-	return zw.Close()
-}
-
-// Close ends the export's transaction and closes its connection.
-func (x *ArchiveExport) Close() error {
-	if x.db == nil {
-		return nil
-	}
-
-	_ = x.tx.Rollback()
-
-	return x.db.Close()
-}
-
-// writeJSON writes head with archived_events added as its last key,
-// the rows going into it one at a time.
-func (x *ArchiveExport) writeJSON(
-	ctx context.Context, w io.Writer, head []byte,
-) error {
-	// head goes out without its closing brace, so that
-	// archived_events can follow it.
-	_, err := w.Write(head[:len(head)-1])
-	if err != nil {
-		return err
-	}
-
-	_, err = io.WriteString(w, `,"archived_events":[`)
-	if err != nil {
-		return err
-	}
-
-	err = x.writeRows(ctx, w)
-	if err != nil {
-		return err
-	}
-
-	_, err = io.WriteString(w, "\n]}\n")
-
-	return err
-}
-
-// writeRows writes each archived row to w, oldest first, one per line,
-// separated by commas.
-func (x *ArchiveExport) writeRows(ctx context.Context, w io.Writer) error {
-	if x.empty {
-		return nil
-	}
-
-	rows, err := x.tx.WithContext(ctx).
-		Model(&archivedEvent{}).Order("id").Rows()
-	if err != nil {
-		return err
-	}
-
-	defer func() { _ = rows.Close() }()
-
-	for sep := "\n"; rows.Next(); sep = ",\n" {
-		var ev archivedEvent
-
-		err = x.tx.ScanRows(rows, &ev)
-		if err != nil {
-			return err
-		}
-
-		_, err = io.WriteString(w, sep)
-		if err != nil {
-			return err
-		}
-
-		err = writeRow(w, &ev)
-		if err != nil {
-			return err
-		}
-	}
-
-	return rows.Err()
-}
-
-// writeRow writes an archived row to w as a JSON object keyed by
-// column name, its body in base64 when it is not valid UTF-8.
-func writeRow(w io.Writer, ev *archivedEvent) error {
-	row := map[string]any{
-		"id":            ev.ID,
-		"event_id":      ev.EventID,
-		"webhook_id":    ev.WebhookID,
-		"entrypoint_id": ev.EntrypointID,
-		"method":        ev.Method,
-		"headers":       ev.Headers,
-		"body":          ev.Body,
-		"content_type":  ev.ContentType,
-		"archived_at":   ev.ArchivedAt.UTC(),
-	}
-
-	if !utf8.ValidString(ev.Body) {
-		row["body"] = base64.StdEncoding.EncodeToString([]byte(ev.Body))
-		row["body_encoding"] = "base64"
-	}
-
-	line, err := json.Marshal(row)
-	if err != nil {
-		return err
-	}
-
-	_, err = w.Write(line)
-
-	return err
-}

internal/delivery/target_database_export_test.go

@@ -1,412 +0,0 @@
-package delivery_test
-
-import (
-	"bufio"
-	"bytes"
-	"compress/gzip"
-	"crypto/rand"
-	"encoding/base64"
-	"encoding/json"
-	"fmt"
-	"io"
-	"os"
-	"path/filepath"
-	"runtime"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/delivery"
-)
-
-// The webhook and the target the export tests' archives belong to.
-const (
-	exportWebhookID   = "wh-export"
-	exportWebhookName = "Orders (EU)"
-	exportTargetID    = "tgt-export"
-	exportTargetName  = "Long-term archive"
-)
-
-const (
-	// binaryBody is a body that is not valid UTF-8.
-	binaryBody = "\xff\xfe\x00\x01binary\x80"
-
-	// openedEventID is the event the snapshot tests archive before
-	// they open the export.
-	openedEventID = "opened"
-)
-
-// writeExportTo writes export to w as the archive of the export tests'
-// webhook and target, exported at 2026-10-02T12:03:04Z.
-func writeExportTo(
-	t *testing.T, export *delivery.ArchiveExport, w io.Writer,
-) error {
-	t.Helper()
-
-	return export.WriteGzipJSON(
-		t.Context(), w,
-		&database.Webhook{
-			BaseModel: database.BaseModel{ID: exportWebhookID},
-			Name:      exportWebhookName,
-		},
-		&database.Target{
-			BaseModel: database.BaseModel{ID: exportTargetID},
-			Name:      exportTargetName,
-		},
-		time.Date(2026, 10, 2, 12, 3, 4, 0, time.UTC),
-	)
-}
-
-// exportArchive runs a whole export of the archive at path and returns
-// its JSON, decompressed and parsed.
-func exportArchive(t *testing.T, path string) map[string]any {
-	t.Helper()
-
-	export, err := delivery.OpenArchiveExport(
-		t.Context(), path, archiveTestLogger(),
-	)
-	require.NoError(t, err)
-
-	defer func() { require.NoError(t, export.Close()) }()
-
-	return writeExport(t, export)
-}
-
-// writeExport writes an opened export and returns its JSON,
-// decompressed and parsed. Reading to the end makes the gzip reader
-// check that the stream was finished.
-func writeExport(
-	t *testing.T, export *delivery.ArchiveExport,
-) map[string]any {
-	t.Helper()
-
-	var buf bytes.Buffer
-
-	require.NoError(t, writeExportTo(t, export, &buf))
-
-	zr, err := gzip.NewReader(&buf)
-	require.NoError(t, err)
-
-	raw, err := io.ReadAll(zr)
-	require.NoError(t, err)
-
-	var got map[string]any
-
-	require.NoError(t, json.Unmarshal(raw, &got))
-
-	return got
-}
-
-// exportedEvents returns an export's archived_events.
-func exportedEvents(t *testing.T, got map[string]any) []map[string]any {
-	t.Helper()
-
-	list, ok := got["archived_events"].([]any)
-	require.True(t, ok, "archived_events must be an array: %v", got)
-
-	events := make([]map[string]any, len(list))
-
-	for i, v := range list {
-		events[i], ok = v.(map[string]any)
-		require.True(t, ok, "an archived event must be an object: %v", v)
-	}
-
-	return events
-}
-
-// exportedEventIDs returns the event_id of each of an export's
-// archived_events.
-func exportedEventIDs(t *testing.T, got map[string]any) []string {
-	t.Helper()
-
-	events := exportedEvents(t, got)
-	ids := make([]string, 0, len(events))
-
-	for _, ev := range events {
-		ids = append(ids, fmt.Sprint(ev["event_id"]))
-	}
-
-	return ids
-}
-
-// TestArchiveExport_MatchesStoredRows proves an export holds the
-// webhook, the target, the time, and every column of every stored
-// row: a body that is valid UTF-8 as a string, and one that is not in
-// base64, marked as such.
-func TestArchiveExport_MatchesStoredRows(t *testing.T) {
-	t.Parallel()
-
-	path := filepath.Join(t.TempDir(), "archive.db")
-	w := delivery.NewExportArchiveWriter(path, archiveTestLogger(), 0)
-	bodies := []string{`{"order":1}`, "plain text", "", binaryBody}
-
-	for i, body := range bodies {
-		require.NoError(t, w.Write(delivery.ExportArchivedEvent{
-			EventID:      fmt.Sprintf("ev-%d", i),
-			WebhookID:    exportWebhookID,
-			EntrypointID: "ep-1",
-			Method:       "POST",
-			Headers:      `{"X-Test":["yes"]}`,
-			Body:         body,
-			ContentType:  testContentType,
-		}, 0))
-	}
-
-	var stored []delivery.ExportArchivedEvent
-
-	require.NoError(t, openArchiveDBForRead(t, path).
-		Order("id").Find(&stored).Error)
-
-	got := exportArchive(t, path)
-
-	assert.Equal(t,
-		map[string]any{"id": exportWebhookID, "name": exportWebhookName},
-		got["webhook"],
-	)
-	assert.Equal(t,
-		map[string]any{"id": exportTargetID, "name": exportTargetName},
-		got["target"],
-	)
-	assert.Equal(t, "2026-10-02T12:03:04Z", got["exported_at"])
-
-	events := exportedEvents(t, got)
-	require.Len(t, events, len(bodies))
-
-	for i, row := range stored {
-		assertExportedRow(t, row, events[i])
-	}
-}
-
-// assertExportedRow checks that ev, from an export, holds every column
-// of the stored row.
-func assertExportedRow(
-	t *testing.T, row delivery.ExportArchivedEvent, ev map[string]any,
-) {
-	t.Helper()
-
-	archivedAt, err := time.Parse(
-		time.RFC3339Nano, fmt.Sprint(ev["archived_at"]),
-	)
-	require.NoError(t, err)
-	assert.True(t, archivedAt.Equal(row.ArchivedAt))
-
-	assert.EqualValues(t, row.ID, ev["id"])
-	assert.Equal(t, row.EventID, ev["event_id"])
-	assert.Equal(t, row.WebhookID, ev["webhook_id"])
-	assert.Equal(t, row.EntrypointID, ev["entrypoint_id"])
-	assert.Equal(t, row.Method, ev["method"])
-	assert.Equal(t, row.Headers, ev["headers"])
-	assert.Equal(t, row.ContentType, ev["content_type"])
-
-	if row.Body != binaryBody {
-		assert.Equal(t, row.Body, ev["body"])
-		assert.Len(t, ev, 9, "the nine columns and nothing else: %v", ev)
-
-		return
-	}
-
-	body, err := base64.StdEncoding.DecodeString(fmt.Sprint(ev["body"]))
-	require.NoError(t, err)
-	assert.Equal(t, binaryBody, string(body))
-	assert.Equal(t, "base64", ev["body_encoding"])
-	assert.Len(t, ev, 10, "the nine columns and body_encoding: %v", ev)
-}
-
-// TestArchiveExport_Empty proves an archive with nothing in it exports
-// as an empty archived_events: no file, which the export must not
-// create; a file the archive writer has not yet put its table in; and
-// a table with no rows.
-func TestArchiveExport_Empty(t *testing.T) {
-	t.Parallel()
-
-	dir := t.TempDir()
-	missing := filepath.Join(dir, "missing.db")
-	noTable := filepath.Join(dir, "no-table.db")
-	noRows := filepath.Join(dir, "no-rows.db")
-
-	require.NoError(t, os.WriteFile(noTable, nil, 0o600))
-	require.NoError(t,
-		delivery.NewExportArchiveWriter(noRows, archiveTestLogger(), 0).
-			Open(0),
-	)
-
-	for _, path := range []string{missing, noTable, noRows} {
-		assert.Empty(t, exportedEvents(t, exportArchive(t, path)), path)
-	}
-
-	for _, suffix := range archiveFileSuffixes() {
-		assert.NoFileExists(t, missing+suffix)
-	}
-}
-
-// TestArchiveExport_ReadsOneSnapshot proves an export writes the
-// archive as it was when it was opened, and holds up no archive
-// write: a row written while the export is open is stored, and is not
-// in the export. A write held up for the whole busy timeout would
-// fail.
-func TestArchiveExport_ReadsOneSnapshot(t *testing.T) {
-	t.Parallel()
-
-	path := filepath.Join(t.TempDir(), "archive.db")
-	w := delivery.NewExportArchiveWriter(path, archiveTestLogger(), 0)
-
-	require.NoError(t, w.Write(delivery.ExportArchivedEvent{EventID: openedEventID}, 0))
-
-	export, err := delivery.OpenArchiveExport(
-		t.Context(), path, archiveTestLogger(),
-	)
-	require.NoError(t, err)
-
-	defer func() { require.NoError(t, export.Close()) }()
-
-	require.NoError(t, w.Write(delivery.ExportArchivedEvent{EventID: "during"}, 0))
-
-	assert.Equal(t,
-		[]string{openedEventID}, exportedEventIDs(t, writeExport(t, export)),
-	)
-
-	var stored int64
-
-	require.NoError(t, openArchiveDBForRead(t, path).
-		Model(&delivery.ExportArchivedEvent{}).Count(&stored).Error)
-	assert.Equal(t, int64(2), stored)
-}
-
-// TestArchiveExport_SurvivesRename proves that renaming the archive
-// while an export of it is open, as renaming its webhook or target
-// does, leaves the export reading the same file.
-func TestArchiveExport_SurvivesRename(t *testing.T) {
-	t.Parallel()
-
-	path := filepath.Join(t.TempDir(), "archive-old.db")
-	w := delivery.NewExportArchiveWriter(path, archiveTestLogger(), 0)
-
-	require.NoError(t, w.Write(delivery.ExportArchivedEvent{EventID: openedEventID}, 0))
-
-	export, err := delivery.OpenArchiveExport(
-		t.Context(), path, archiveTestLogger(),
-	)
-	require.NoError(t, err)
-
-	defer func() { require.NoError(t, export.Close()) }()
-
-	require.NoError(t, w.Rename("archive-new.db"))
-	require.NoError(t, w.Write(delivery.ExportArchivedEvent{EventID: "after"}, 0))
-	require.NoFileExists(t, path)
-
-	assert.Equal(t,
-		[]string{openedEventID}, exportedEventIDs(t, writeExport(t, export)),
-	)
-}
-
-// heapPeak is an io.Writer that discards what it is given and records
-// the largest heap it saw at a write. It collects garbage before each
-// reading, so the heap it reads is what is still held.
-type heapPeak struct {
-	max uint64
-}
-
-func (p *heapPeak) Write(b []byte) (int, error) {
-	var m runtime.MemStats
-
-	runtime.GC()
-	runtime.ReadMemStats(&m)
-	p.max = max(p.max, m.HeapAlloc)
-
-	return len(b), nil
-}
-
-// exportHeapGrowth exports an archive of rows random bodies, each
-// bodySize bytes of base64, and returns how far the heap rose above
-// where it stood when the export began, at its highest.
-func exportHeapGrowth(t *testing.T, rows, bodySize int) uint64 {
-	t.Helper()
-
-	path := filepath.Join(t.TempDir(), "archive.db")
-	w := delivery.NewExportArchiveWriter(path, archiveTestLogger(), 0)
-
-	// Base64 makes four characters of every three bytes.
-	random := make([]byte, bodySize/4*3)
-
-	for range rows {
-		_, _ = rand.Read(random)
-
-		require.NoError(t, w.Write(delivery.ExportArchivedEvent{
-			Body: base64.StdEncoding.EncodeToString(random),
-		}, 0))
-	}
-
-	export, err := delivery.OpenArchiveExport(
-		t.Context(), path, archiveTestLogger(),
-	)
-	require.NoError(t, err)
-
-	defer func() { require.NoError(t, export.Close()) }()
-
-	runtime.GC()
-
-	var start runtime.MemStats
-
-	runtime.ReadMemStats(&start)
-
-	// Through a buffer, the heap is read once per 8 KiB of output
-	// rather than at each of gzip's small writes, which takes far
-	// longer.
-	peak := &heapPeak{max: start.HeapAlloc}
-	buffered := bufio.NewWriterSize(peak, 8<<10)
-
-	require.NoError(t, writeExportTo(t, export, buffered))
-	require.NoError(t, buffered.Flush())
-
-	return peak.max - start.HeapAlloc
-}
-
-// TestArchiveExport_Streams proves an export holds neither the archive
-// nor its output in memory whole: exporting 384 KiB more of archive
-// raises the heap's peak by less than half of that. The export's own
-// memory, mostly gzip's compressor, is the same for both archives, so
-// it cancels out. The bodies are random bytes in base64, which gzip
-// shrinks by only a quarter, so an export that read every row before
-// writing, or built the JSON or the gzipped file before writing it,
-// would raise the peak by at least three quarters of the difference.
-//
-// The smaller archive has two rows so that its export, too, writes
-// out more than the 8 KiB buffer in exportHeapGrowth before it ends:
-// the heap must be read while the export's own memory is held.
-//
-//nolint:paralleltest // It measures the heap, which tests share.
-func TestArchiveExport_Streams(t *testing.T) {
-	const (
-		bodySize  = 16 << 10
-		smallRows = 2
-		largeRows = smallRows + 24
-		limit     = (largeRows - smallRows) * bodySize / 2
-	)
-
-	small := exportHeapGrowth(t, smallRows, bodySize)
-	large := exportHeapGrowth(t, largeRows, bodySize)
-
-	assert.Less(t, large, small+limit,
-		"the heap rose by %d for %d rows and by %d for %d rows",
-		small, smallRows, large, largeRows,
-	)
-}
-
-// TestArchiveExportFileName proves the download is named for the
-// webhook and the target, with the names made safe as for the archive
-// file, and the export time in UTC.
-func TestArchiveExportFileName(t *testing.T) {
-	t.Parallel()
-
-	cest := time.FixedZone("CEST", int((2 * time.Hour).Seconds()))
-
-	assert.Equal(t,
-		"archive-orders-eu-long-term-archive-20261002T120304Z.json.gz",
-		delivery.ArchiveExportFileName(
-			exportWebhookName, exportTargetName,
-			time.Date(2026, 10, 2, 14, 3, 4, 0, cest),
-		),
-	)
-}

internal/delivery/url_mask_test.go

@@ -179,6 +179,27 @@ func TestDoHTTPRequest_TransportErrorMasksURL(t *testing.T) {
 	)
 }
 
+// TestDoHTTPRequest_UnparsableURLIsMasked is the same for an HTTP
+// target URL that no request can be built from.
+func TestDoHTTPRequest_UnparsableURLIsMasked(t *testing.T) {
+	t.Parallel()
+
+	e := testEngine(t, 1)
+
+	statusCode, _, _, reqErr := e.ExportDoHTTPRequest(
+		context.TODO(),
+		&delivery.HTTPTargetConfig{
+			URL: "https://hooks.example.com" + maskSecretPath + "\n",
+		},
+		&database.Event{},
+	)
+	require.Error(t, reqErr)
+	assert.Zero(t, statusCode)
+
+	assertNoCredential(t, reqErr.Error())
+	assert.Contains(t, reqErr.Error(), "invalid control character")
+}
+
 // TestValidateTargetURL_UnparsableURLIsMasked proves the SSRF
 // validator's error does not carry the submitted URL, which
 // the handler both logs and shows.

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
-// found nothing to look at. The tree held 60 non-test .go files when
-// this was written.
-const minNonTestFiles = 40
-
-// isRowProducer reports whether name is a method that returns a
-// database/sql row handle. GORM's Row and Rows return *sql.Row and
-// *sql.Rows, so Scan on the result of one of them is database/sql's
-// Scan and never (*gorm.DB).Scan.
+// isRowProducer reports whether name is GORM's Row or database/sql's
+// QueryRow or QueryRowContext, which return a *sql.Row whose Scan is
+// database/sql's and not (*gorm.DB).Scan. GORM's Rows is not listed:
+// 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
+// a repo-local method with one of these names that returns *gorm.DB
+// gets past it: Scan on that method's result is not reported.
 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
-// resolve syntactically — a local variable, a struct field — is
-// reported rather than assumed safe.
+// receiver is not a call to a row producer. It fails closed: any other
+// receiver — a local variable, a struct field, a call to any other
+// 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
-// how many it parsed along with every unguarded Scan it found.
-func walkNonTestGo(t *testing.T, root string) (int, []string) {
+// walkNonTestGo parses every non-test .go file under root. It returns
+// the directories, relative to root, it parsed a file in, along with
+// every unguarded Scan it found.
+func walkNonTestGo(t *testing.T, root string) (map[string]bool, []string) {
 	t.Helper()
 
-	var (
-		parsed int
-		hits   []string
-	)
+	walked := map[string]bool{}
+
+	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,
-// which this check does not govern: it is test-only and its SELECT 1
-// binds nothing.
+// Test files are not governed: what a test binds is fixture data.
 func TestGormScanIsNeverCalledOutsideTests(t *testing.T) {
 	t.Parallel()
 
-	parsed, offenders := walkNonTestGo(t, moduleRoot(t))
+	root := moduleRoot(t)
+	walked, offenders := walkNonTestGo(t, root)
 
-	require.GreaterOrEqual(
-		t, parsed, minNonTestFiles,
-		"parsed %d non-test .go files, so this check found "+
-			"nothing to look at", parsed,
+	// 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.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},
-		{"gorm receiver", `gdb.Scan(&v)`, 1},
-		{"gorm via variable", "q := gdb.Raw(\"x\")\nq.Scan(&v)", 1},
-		{"gorm model chain", `gdb.Model(&x).Scan(&v)`, 1},
-		{"sql row", `gdb.Raw("SELECT 1").Row().Scan(&v)`, 0},
-		{"sql rows", `gdb.Raw("SELECT 1").Rows().Scan(&v)`, 0},
+		{"local variable", "q := gdb.Raw(\"SELECT 1\")\n\tq.Scan(&v)", 1},
+		{"struct field", `s.db.Scan(&v)`, 1},
+		{"gorm chain", `gdb.Raw("SELECT 1").Scan(&v)`, 1},
+		{
+			"sql rows in a variable",
+			"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(
-				"package p\n\nfunc f() {\n\t%s\n}\n", tc.body,
-			)
+			src := fmt.Sprintf(plantedFile, tc.body)
 
 			file, err := parser.ParseFile(
 				fset, tc.name+".go", src, 0,

internal/handlers/event_body.go

@@ -15,10 +15,12 @@ import (
 // eventBodyQuery reads one event's stored body as bytes. The cast
 // to blob is what makes the driver hand back the stored bytes
 // rather than a string conversion, so Content-Length taken from
-// the result matches what goes on the wire. The soft-delete
-// predicate is spelled out because Raw bypasses GORM's default
-// scope, and it is what stops a reaped event still being
-// downloadable.
+// the result matches what goes on the wire. The retention reaper
+// deletes event rows outright, so a reaped event is simply gone
+// and the query finds no row. The deleted_at predicate repeats
+// the soft-delete scope GORM adds to its own queries, which Raw
+// bypasses; nothing soft-deletes an event, so today it excludes
+// nothing.
 const eventBodyQuery = "SELECT cast(body as blob) " +
 	"FROM events WHERE id = ? AND webhook_id = ? AND deleted_at IS NULL"
 

internal/handlers/event_body_test.go

@@ -405,10 +405,11 @@ func TestHandleEventBodyDownload_UnknownEvent404s(t *testing.T) {
 // route. The body is read in one query before any header is
 // written, so a reaped event cannot produce a partial download:
 // it is a clean 404 with no Content-Length and no
-// Content-Disposition. Both removals the codebase performs are
-// covered — the reaper hard-deletes, and a soft-deleted row is
-// excluded by the query's own deleted_at predicate rather than
-// by GORM's default scope, which Raw bypasses.
+// Content-Disposition. The reaper deletes event rows outright,
+// which is the "hard deleted" case. The "soft deleted" case
+// covers a row no code produces today: it only pins the query's
+// own deleted_at predicate, the soft-delete condition Raw would
+// otherwise skip.
 func TestHandleEventBodyDownload_ReapedEvent404s(t *testing.T) {
 	t.Parallel()
 

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
-// what stops a reaped event being resubmitted.
+// file. A reaped event is not found because the retention reaper
+// 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/handlers.go

@@ -97,8 +97,7 @@ type Handlers struct {
 	// names through the archive rename, the save and any move back.
 	// Interleaved, one could rename an archive between another's
 	// rename and save, leaving the file named for one edit and the
-	// stored names from the other. An archive download holds it while
-	// it reads the stored names and opens the file they give.
+	// stored names from the other.
 	renameMu sync.Mutex
 
 	// dummyVerifications counts the equivalent-cost verifications

internal/handlers/source_management_test.go

@@ -191,7 +191,6 @@ func storedRetentionDays(
 type sourceTestEnv struct {
 	handlers *handlers.Handlers
 	db       *database.Database
-	dbMgr    *database.WebhookDBManager
 	archives *recordingArchives
 	cookies  []*http.Cookie
 }
@@ -205,11 +204,9 @@ func setupSourceTest(t *testing.T) *sourceTestEnv {
 
 	var db *database.Database
 
-	var dbMgr *database.WebhookDBManager
-
 	var archives *recordingArchives
 
-	app := newTestApp(t, &h, &sess, &db, &dbMgr, &archives)
+	app := newTestApp(t, &h, &sess, &db, &archives)
 	app.RequireStart()
 
 	t.Cleanup(app.RequireStop)
@@ -217,7 +214,6 @@ func setupSourceTest(t *testing.T) *sourceTestEnv {
 	return &sourceTestEnv{
 		handlers: h,
 		db:       db,
-		dbMgr:    dbMgr,
 		archives: archives,
 		cookies: authenticatedCookies(
 			t, sess, sourceTestUserID, "sourceuser",

internal/handlers/target_download.go

@@ -1,121 +0,0 @@
-package handlers
-
-import (
-	"context"
-	"errors"
-	"net/http"
-	"time"
-
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/delivery"
-)
-
-// downloadWriteTimeout is how long one write of a download may wait
-// for a client that has stopped reading.
-const downloadWriteTimeout = 60 * time.Second
-
-// HandleTargetDownload serves a database target's archive as one
-// gzipped JSON file, named for the webhook, the target and the time;
-// see delivery.ArchiveExport.WriteGzipJSON for what it holds. Other
-// target types have no archive and are a 404.
-//
-// A download runs for as long as the client keeps reading: it reads
-// under a context the request limit does not cancel, and gives each
-// write its own deadline in place of the server's write timeout. It
-// stops when a write fails.
-func (h *Handlers) HandleTargetDownload() http.HandlerFunc {
-	return func(w http.ResponseWriter, r *http.Request) {
-		ctx := context.WithoutCancel(r.Context())
-
-		webhook, target, export, ok := h.openTargetArchive(ctx, w, r)
-		if !ok {
-			return
-		}
-
-		defer func() { _ = export.Close() }()
-
-		now := time.Now()
-
-		w.Header().Set("Content-Type", "application/gzip")
-		w.Header().Set(
-			"Content-Disposition",
-			`attachment; filename="`+delivery.ArchiveExportFileName(
-				webhook.Name, target.Name, now,
-			)+`"`,
-		)
-
-		err := export.WriteGzipJSON(
-			ctx,
-			downloadWriter{w: w, rc: http.NewResponseController(w)},
-			&webhook, target, now,
-		)
-		if err != nil {
-			h.log.Error(
-				"failed to export archive",
-				"target_id", target.ID,
-				"error", err,
-			)
-
-			// The 200 has gone out. Aborting the connection is what
-			// tells the client the file is incomplete.
-			panic(http.ErrAbortHandler)
-		}
-	}
-}
-
-// downloadWriter writes a download to the client, giving each write
-// downloadWriteTimeout to finish.
-type downloadWriter struct {
-	w  http.ResponseWriter
-	rc *http.ResponseController
-}
-
-func (d downloadWriter) Write(b []byte) (int, error) {
-	// A writer that has no write deadline, such as a test's recorder,
-	// answers http.ErrNotSupported and needs none extended.
-	err := d.rc.SetWriteDeadline(time.Now().Add(downloadWriteTimeout))
-	if err != nil && !errors.Is(err, http.ErrNotSupported) {
-		return 0, err
-	}
-
-	return d.w.Write(b)
-}
-
-// openTargetArchive opens the archive of the request's database target
-// for export, with its reads under ctx. It reports false once it has
-// written the response.
-//
-// It holds renameMu, which every archive rename runs under, while it
-// reads the stored names and opens the file, so the file it opens is
-// the one those names give. It lets go before the export is streamed:
-// once the file is open, a rename does not affect the export.
-func (h *Handlers) openTargetArchive(
-	ctx context.Context,
-	w http.ResponseWriter,
-	r *http.Request,
-) (database.Webhook, *database.Target, *delivery.ArchiveExport, bool) {
-	h.renameMu.Lock()
-	defer h.renameMu.Unlock()
-
-	webhook, target, ok := h.ownedTarget(w, r)
-	if !ok {
-		return database.Webhook{}, nil, nil, false
-	}
-
-	if target.Type != database.TargetTypeDatabase {
-		h.renderError(w, r, http.StatusNotFound)
-
-		return database.Webhook{}, nil, nil, false
-	}
-
-	export, err := delivery.OpenArchiveExport(
-		ctx, delivery.ArchivePath(h.dbMgr, &webhook, target), h.log,
-	)
-	if err != nil {
-		h.serverError(w, r, "failed to open archive for export", err)
-
-		return database.Webhook{}, nil, nil, false
-	}
-
-	return webhook, target, export, true
-}

internal/handlers/target_download_test.go

@@ -1,379 +0,0 @@
-package handlers_test
-
-import (
-	"bytes"
-	"compress/gzip"
-	"context"
-	"crypto/rand"
-	"encoding/json"
-	"errors"
-	"io"
-	"log/slog"
-	"net"
-	"net/http"
-	"net/http/httptest"
-	"net/url"
-	"sync"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/config"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/delivery"
-	"sneak.berlin/go/webhooker/internal/middleware"
-)
-
-// errClientGone is the write failure of a client that has gone away.
-var errClientGone = errors.New("client gone")
-
-// downloadPath is the archive download route of a target.
-func downloadPath(webhookID, targetID string) string {
-	return "/hook/" + webhookID + "/targets/" + targetID + "/download"
-}
-
-// renameTarget submits the edit form renaming a target to Renamed.
-func renameTarget(
-	env *sourceTestEnv, webhookID, targetID string,
-) *httptest.ResponseRecorder {
-	form := url.Values{}
-	form.Set("name", "Renamed")
-
-	return submitTargetEdit(env, webhookID, targetID, form)
-}
-
-// TestHandleTargetDownload proves a database target's archive
-// downloads as a gzipped JSON attachment named for the webhook, the
-// target and the time, here with no archive file yet, so with no
-// rows; and that a target of another type has no download.
-func TestHandleTargetDownload(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh := seedWebhookWithRetention(t, env.db, 7)
-	archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
-	logTarget := seedTarget(t, env.db, wh.ID, database.TargetTypeLog)
-
-	w := serveTarget(
-		env, http.MethodGet, downloadPath(wh.ID, archive.ID), nil,
-	)
-	require.Equal(t, http.StatusOK, w.Code, w.Body.String())
-	assert.Equal(t, "application/gzip", w.Header().Get("Content-Type"))
-	assert.Regexp(t,
-		`^attachment; filename="archive-seeded-t-database-`+
-			`\d{8}T\d{6}Z\.json\.gz"$`,
-		w.Header().Get("Content-Disposition"),
-	)
-
-	zr, err := gzip.NewReader(w.Body)
-	require.NoError(t, err)
-
-	var got map[string]json.RawMessage
-
-	require.NoError(t, json.NewDecoder(zr).Decode(&got))
-	assert.JSONEq(t,
-		`{"id":"`+archive.ID+`","name":"t-database"}`,
-		string(got["target"]),
-	)
-	assert.JSONEq(t, `[]`, string(got["archived_events"]))
-
-	w = serveTarget(
-		env, http.MethodGet, downloadPath(wh.ID, logTarget.ID), nil,
-	)
-	assert.Equal(t, http.StatusNotFound, w.Code)
-}
-
-// TestHandleTargetDownload_WaitsForRename proves a download reads the
-// target's names and opens its archive under the lock a rename holds:
-// started while an edit is renaming the archive, it waits, and is
-// named for the target's new name.
-func TestHandleTargetDownload_WaitsForRename(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh := seedWebhookWithRetention(t, env.db, 7)
-	archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
-
-	renaming, release := env.archives.BlockNextRename()
-	edited := make(chan *httptest.ResponseRecorder, 1)
-
-	go func() {
-		edited <- renameTarget(env, wh.ID, archive.ID)
-	}()
-
-	<-renaming
-
-	downloaded := make(chan *httptest.ResponseRecorder, 1)
-
-	go func() {
-		downloaded <- serveTarget(
-			env, http.MethodGet, downloadPath(wh.ID, archive.ID), nil,
-		)
-	}()
-
-	select {
-	case <-downloaded:
-		release()
-		t.Fatal("the download did not wait for the rename")
-	case <-time.After(100 * time.Millisecond):
-	}
-
-	release()
-	require.Equal(t, http.StatusSeeOther, (<-edited).Code)
-
-	w := <-downloaded
-	require.Equal(t, http.StatusOK, w.Code)
-	assert.Contains(t,
-		w.Header().Get("Content-Disposition"), "archive-seeded-renamed-",
-	)
-}
-
-// stalledWriter is a response writer whose first write waits until
-// resume is closed, closing writing when it starts to wait.
-type stalledWriter struct {
-	*httptest.ResponseRecorder
-
-	once    sync.Once
-	writing chan struct{}
-	resume  chan struct{}
-}
-
-func (s *stalledWriter) Write(b []byte) (int, error) {
-	s.once.Do(func() {
-		close(s.writing)
-		<-s.resume
-	})
-
-	return s.ResponseRecorder.Write(b)
-}
-
-// TestHandleTargetDownload_StreamsWithoutTheLock proves a download
-// lets go of the rename lock once its archive is open: while the
-// download is stalled writing, an edit can still rename the target.
-func TestHandleTargetDownload_StreamsWithoutTheLock(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh := seedWebhookWithRetention(t, env.db, 7)
-	archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
-
-	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, downloadPath(wh.ID, archive.ID), nil,
-	)
-	for _, c := range env.cookies {
-		req.AddCookie(c)
-	}
-
-	sw := &stalledWriter{
-		ResponseRecorder: httptest.NewRecorder(),
-		writing:          make(chan struct{}),
-		resume:           make(chan struct{}),
-	}
-	downloaded := make(chan struct{})
-
-	go func() {
-		targetRouter(env).ServeHTTP(sw, req)
-		close(downloaded)
-	}()
-
-	<-sw.writing
-
-	edited := make(chan *httptest.ResponseRecorder, 1)
-
-	go func() {
-		edited <- renameTarget(env, wh.ID, archive.ID)
-	}()
-
-	select {
-	case w := <-edited:
-		assert.Equal(t, http.StatusSeeOther, w.Code)
-	case <-time.After(10 * time.Second):
-		t.Error("the rename waited for the download")
-	}
-
-	close(sw.resume)
-	<-downloaded
-	assert.Equal(t, http.StatusOK, sw.Code)
-}
-
-// seedArchive writes rows to the archive file at path, each with a
-// body of bodySize random bytes, which do not compress. Its table has
-// only the columns the test fills; an export writes the others empty.
-func seedArchive(t *testing.T, path string, rows, bodySize int) {
-	t.Helper()
-
-	db, err := database.OpenSQLite(path, database.SQLiteModeCreate)
-	require.NoError(t, err)
-
-	defer func() { require.NoError(t, db.Close()) }()
-
-	_, err = db.ExecContext(t.Context(),
-		"CREATE TABLE archived_events (id INTEGER PRIMARY KEY, body TEXT)",
-	)
-	require.NoError(t, err)
-
-	body := make([]byte, bodySize)
-
-	for range rows {
-		_, _ = rand.Read(body)
-
-		_, err = db.ExecContext(t.Context(),
-			"INSERT INTO archived_events (body) VALUES (?)", string(body),
-		)
-		require.NoError(t, err)
-	}
-}
-
-// limitedServer serves the target routes as the server does, behind the
-// access log, whose lines it returns, and the request limit, here
-// limit, which is also its write timeout. Each connection's send buffer
-// is a few KiB, so a larger response is still being written while its
-// client is not reading.
-func limitedServer(
-	t *testing.T, env *sourceTestEnv, limit time.Duration,
-) (*httptest.Server, *bytes.Buffer) {
-	t.Helper()
-
-	const sendBuffer = 4 << 10
-
-	logBuf := new(bytes.Buffer)
-	mw := middleware.NewForTest(
-		slog.New(slog.NewJSONHandler(logBuf, nil)),
-		&config.Config{Environment: config.EnvironmentDev},
-		nil,
-	)
-
-	srv := httptest.NewUnstartedServer(
-		mw.Logging()(mw.Timeout(limit)(targetRouter(env))),
-	)
-	srv.Config.WriteTimeout = limit
-	srv.Config.ConnContext = func(
-		ctx context.Context, c net.Conn,
-	) context.Context {
-		tcp, ok := c.(*net.TCPConn)
-		if assert.True(t, ok) {
-			assert.NoError(t, tcp.SetWriteBuffer(sendBuffer))
-		}
-
-		return ctx
-	}
-	srv.Start()
-	t.Cleanup(srv.Close)
-
-	return srv, logBuf
-}
-
-// TestHandleTargetDownload_OutlastsTheRequestLimit proves a download
-// runs for as long as the client keeps reading, and is logged as the
-// 200 it was. Behind a request limit and a server write timeout of a
-// tenth of a second, the client stops reading once the response has
-// started, waits three times as long, and still gets the whole file.
-// The archive is larger than the connection holds, so the download is
-// still being written while the client waits.
-func TestHandleTargetDownload_OutlastsTheRequestLimit(t *testing.T) {
-	t.Parallel()
-
-	const (
-		limit    = 100 * time.Millisecond
-		rows     = 8
-		bodySize = 64 << 10
-	)
-
-	env := setupSourceTest(t)
-	wh := seedWebhookWithRetention(t, env.db, 7)
-	archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
-	seedArchive(
-		t, delivery.ArchivePath(env.dbMgr, &wh, archive), rows, bodySize,
-	)
-
-	srv, accessLog := limitedServer(t, env, limit)
-
-	req, err := http.NewRequestWithContext(
-		t.Context(), http.MethodGet,
-		srv.URL+downloadPath(wh.ID, archive.ID), nil,
-	)
-	require.NoError(t, err)
-
-	for _, c := range env.cookies {
-		req.AddCookie(c)
-	}
-
-	resp, err := srv.Client().Do(req)
-	require.NoError(t, err)
-
-	defer func() { _ = resp.Body.Close() }()
-
-	require.Equal(t, http.StatusOK, resp.StatusCode)
-
-	time.Sleep(3 * limit)
-
-	zr, err := gzip.NewReader(resp.Body)
-	require.NoError(t, err)
-
-	var (
-		got    map[string]json.RawMessage
-		events []json.RawMessage
-	)
-
-	require.NoError(t, json.NewDecoder(zr).Decode(&got))
-	require.NoError(t, json.Unmarshal(got["archived_events"], &events))
-	assert.Len(t, events, rows)
-
-	// Reading to the end makes the gzip reader check that the file was
-	// finished.
-	_, err = io.ReadAll(zr)
-	require.NoError(t, err)
-
-	// Close waits for the handler, so the access log line is written.
-	srv.Close()
-
-	var access map[string]any
-
-	require.NoError(t, json.Unmarshal(accessLog.Bytes(), &access))
-	assert.EqualValues(t, http.StatusOK, access["status"])
-	assert.GreaterOrEqual(t,
-		access["latency_ms"], float64(limit.Milliseconds()),
-		"the download must outlast the request limit",
-	)
-}
-
-// brokenWriter is a response writer whose writes fail once the
-// response has started, as they do when the client goes away.
-type brokenWriter struct {
-	*httptest.ResponseRecorder
-}
-
-func (b brokenWriter) Write(p []byte) (int, error) {
-	if b.Body.Len() > 0 {
-		return 0, errClientGone
-	}
-
-	return b.ResponseRecorder.Write(p)
-}
-
-// TestHandleTargetDownload_AbortsWhenItFails proves a download that
-// fails after its response has started aborts the connection, so the
-// client sees a failed download rather than a file that looks
-// complete and does not decompress.
-func TestHandleTargetDownload_AbortsWhenItFails(t *testing.T) {
-	t.Parallel()
-
-	env := setupSourceTest(t)
-	wh := seedWebhookWithRetention(t, env.db, 7)
-	archive := seedTarget(t, env.db, wh.ID, database.TargetTypeDatabase)
-
-	req := httptest.NewRequestWithContext(
-		t.Context(), http.MethodGet, downloadPath(wh.ID, archive.ID), nil,
-	)
-	for _, c := range env.cookies {
-		req.AddCookie(c)
-	}
-
-	w := brokenWriter{ResponseRecorder: httptest.NewRecorder()}
-
-	assert.PanicsWithValue(t, http.ErrAbortHandler, func() {
-		targetRouter(env).ServeHTTP(w, req)
-	})
-	assert.Equal(t, http.StatusOK, w.Code)
-}

internal/handlers/target_edit_test.go

@@ -37,18 +37,14 @@ const (
 	editAuthHeader   = "Authorization: Bearer " + editBearerSecret
 )
 
-// targetRouter mounts the target create, edit and download routes on
-// a chi router so the handlers see the URL parameters they read.
+// targetRouter mounts the target create and edit routes on a chi
+// router so the handlers see the URL parameters they read.
 func targetRouter(env *sourceTestEnv) *chi.Mux {
 	router := chi.NewRouter()
 	router.Post(
 		"/hook/{sourceID}/targets",
 		env.handlers.HandleTargetCreate(),
 	)
-	router.Get(
-		"/hook/{sourceID}/targets/{targetID}/download",
-		env.handlers.HandleTargetDownload(),
-	)
 	router.Get(
 		"/hook/{sourceID}/targets/{targetID}/edit",
 		env.handlers.HandleTargetEdit(),

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
-// only path by which an event and its deliveries are created, so a
-// resubmitted event is retried, SSRF-guarded and circuit-broken
-// exactly as a received one is.
+// transaction, then hands the tasks to the delivery engine. Every
+// event is created here, received or resubmitted, so a resubmitted
+// event is retried, SSRF-guarded and circuit-broken exactly as a
+// 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/middleware/timeout.go

@@ -1,71 +0,0 @@
-package middleware
-
-import (
-	"context"
-	"errors"
-	"net/http"
-	"time"
-)
-
-// Timeout returns middleware that gives each request limit to finish:
-// it cancels the request's context once limit has passed, and answers
-// 504 when the handler then returns without having started its
-// response.
-//
-// It replaces chi's middleware.Timeout, which writes that 504 even
-// after the handler has sent its own status. A download that outlasts
-// the limit has already sent its 200 and the whole file, so the late
-// 504 changes nothing for the client: the access log and the metrics
-// would record it in place of the 200, and net/http would complain of
-// a superfluous WriteHeader.
-func (s *Middleware) Timeout(
-	limit time.Duration,
-) func(http.Handler) http.Handler {
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(func(
-			w http.ResponseWriter,
-			r *http.Request,
-		) {
-			ctx, cancel := context.WithTimeout(r.Context(), limit)
-			defer cancel()
-
-			tw := &timeoutResponseWriter{ResponseWriter: w}
-
-			next.ServeHTTP(tw, r.WithContext(ctx))
-
-			if !tw.started &&
-				errors.Is(ctx.Err(), context.DeadlineExceeded) {
-				w.WriteHeader(http.StatusGatewayTimeout)
-			}
-		})
-	}
-}
-
-// timeoutResponseWriter records whether the handler has started its
-// response.
-type timeoutResponseWriter struct {
-	http.ResponseWriter
-
-	started bool
-}
-
-func (w *timeoutResponseWriter) WriteHeader(code int) {
-	w.started = true
-
-	w.ResponseWriter.WriteHeader(code)
-}
-
-func (w *timeoutResponseWriter) Write(b []byte) (int, error) {
-	// A Write without a WriteHeader starts the response too: net/http
-	// sends 200 in front of it.
-	w.started = true
-
-	//nolint:wrapcheck // Pass the writer's own error through unchanged.
-	return w.ResponseWriter.Write(b)
-}
-
-// Unwrap lets http.ResponseController reach the writer underneath, so
-// a handler can still set a write deadline through this wrapper.
-func (w *timeoutResponseWriter) Unwrap() http.ResponseWriter {
-	return w.ResponseWriter
-}

internal/middleware/timeout_test.go

@@ -1,56 +0,0 @@
-package middleware_test
-
-import (
-	"net/http"
-	"net/http/httptest"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-// TestTimeout proves the request limit answers 504 to a handler that
-// outlasts it without starting its response, and leaves a response the
-// handler has started with the status it sent. Both are what the
-// access log records.
-func TestTimeout(t *testing.T) {
-	t.Parallel()
-
-	const limit = 10 * time.Millisecond
-
-	for _, tc := range []struct {
-		name string
-		sent int // the status the handler sends, or 0 for none
-		want int
-	}{
-		{name: "not started", sent: 0, want: http.StatusGatewayTimeout},
-		{name: "started", sent: http.StatusOK, want: http.StatusOK},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			m, buf := capturingMiddleware(t)
-			handler := m.Logging()(m.Timeout(limit)(http.HandlerFunc(
-				func(w http.ResponseWriter, r *http.Request) {
-					if tc.sent != 0 {
-						w.WriteHeader(tc.sent)
-					}
-
-					<-r.Context().Done()
-				},
-			)))
-
-			w := httptest.NewRecorder()
-			handler.ServeHTTP(w, httptest.NewRequestWithContext(
-				t.Context(), http.MethodGet, "/", nil,
-			))
-
-			assert.Equal(t, tc.want, w.Code)
-
-			entries := accessLogEntries(t, buf)
-			require.Len(t, entries, 1)
-			assert.EqualValues(t, tc.want, entries[0]["status"])
-		})
-	}
-}

internal/server/archive_download_test.go

@@ -1,69 +0,0 @@
-package server_test
-
-import (
-	"compress/gzip"
-	"encoding/json"
-	"net/http"
-	"regexp"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gorm.io/gorm/clause"
-	"sneak.berlin/go/webhooker/internal/database"
-)
-
-// TestHook_DownloadArchive follows the Download link the webhook page
-// shows for a database target, and only for it, and gets the archive
-// as a gzipped JSON file. Signed out, the link leads to the login page.
-func TestHook_DownloadArchive(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "archivist", "somepassword")
-	cookies := env.authCookies(t, userID, "archivist")
-	wh := env.seedWebhook(t, userID)
-	env.seedTarget(t, wh.ID)
-
-	archive := &database.Target{
-		WebhookID: wh.ID,
-		Name:      "kept",
-		Type:      database.TargetTypeDatabase,
-		Active:    true,
-	}
-	require.NoError(t,
-		env.db.DB().Omit(clause.Associations).Create(archive).Error,
-	)
-
-	page := env.get("/hook/"+wh.ID, cookies)
-	require.Equal(t, http.StatusOK, page.Code)
-
-	links := regexp.MustCompile(
-		`href="(/hook/[^/"]+/targets/[^/"]+/download)"`,
-	).FindAllStringSubmatch(page.Body.String(), -1)
-	require.Len(t, links, 1, "only the database target has a Download")
-
-	link := links[0][1]
-	assert.Equal(t,
-		"/hook/"+wh.ID+"/targets/"+archive.ID+"/download", link,
-	)
-
-	w := env.get(link, cookies)
-	require.Equal(t, http.StatusOK, w.Code)
-	assert.Equal(t, "application/gzip", w.Header().Get("Content-Type"))
-
-	zr, err := gzip.NewReader(w.Body)
-	require.NoError(t, err)
-
-	var got map[string]json.RawMessage
-
-	require.NoError(t, json.NewDecoder(zr).Decode(&got))
-	assert.JSONEq(t,
-		`{"id":"`+wh.ID+`","name":"routed"}`, string(got["webhook"]),
-	)
-
-	w = env.get(link, nil)
-	assert.Equal(t, http.StatusSeeOther, w.Code)
-	assert.Contains(t, w.Header().Get("Location"), "/pages/login")
-}

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.go

@@ -73,7 +73,7 @@ func (s *Server) setupGlobalMiddleware() {
 	}
 
 	s.router.Use(s.mw.CORS())
-	s.router.Use(s.mw.Timeout(requestTimeout))
+	s.router.Use(middleware.Timeout(requestTimeout))
 
 	// Panic recovery, deliberately here rather than first. It has to
 	// run inside every middleware that observes the response, so the
@@ -312,10 +312,6 @@ func (s *Server) setupSourceRoutes() {
 			"/targets/{targetID}/edit",
 			s.h.HandleTargetEditSubmit(),
 		)
-		r.Get(
-			"/targets/{targetID}/download",
-			s.h.HandleTargetDownload(),
-		)
 		r.Post(
 			"/targets/{targetID}/delete",
 			s.h.HandleTargetDelete(),

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()

templates/source_detail.html

@@ -157,9 +157,6 @@
                             {{else}}
                             <span class="badge-error">Inactive</span>
                             {{end}}
-                            {{if eq .Type "database"}}
-                            <a href="/hook/{{$.Webhook.ID}}/targets/{{.ID}}/download" class="btn-small" title="Download the archive as gzipped JSON">Download</a>
-                            {{end}}
                             <a href="/hook/{{$.Webhook.ID}}/targets/{{.ID}}/edit" class="btn-small" title="Edit">Edit</a>
                             <form method="POST" action="/hook/{{$.Webhook.ID}}/targets/{{.ID}}/toggle" class="inline">
                                 <input type="hidden" name="csrf_token" value="{{$.CSRFToken}}">