Test a pending delivery whose target is found only on lookup

When the batch's target map lacks a delivery's target, as it does
for every delivery when the batch query fails, sendRecoveredDeliveries
looks the target up on its own. The new test hands it an empty map
and checks the healthy delivery is queued once, with its real target
id and type.

Model: opus-5-5

README.md

@@ -157,11 +157,6 @@ private and reserved ranges — RFC 1918, loopback, CGNAT, link-local and
 the rest — are refused, which stops a target from being used to make
 webhooker probe the network it sits in.
 
-Besides the private and reserved ranges, the default blocklist refuses
-public cloud metadata addresses: currently only `168.63.129.16`, Azure's
-WireServer, which serves an Azure VM its credentials. Because it is a
-public address, listing it in `ALLOWED_EGRESS_CIDRS` reopens it.
-
 That default is also inconvenient for the thing webhooker is mostly
 for: taking a public webhook and forwarding it to something on your own
 network. A container on the same Docker network, a box on `10.x`, a
@@ -200,16 +195,15 @@ Two things this setting cannot do:
   the list is always an allowlist; an empty list (the default) means
   every private and reserved range stays refused. Note that
   `0.0.0.0/0` gets you most of the way there anyway, per above.
-- **It cannot open link-local, or a cloud metadata endpoint at a
+- **It cannot open link-local, or a cloud metadata endpoint that
-  non-public address that discloses credentials or user data.** An
+  discloses credentials or user data.** An address is on the list below
-  address is on the list below when it is not a public address and both
+  when both of these hold: the provider fixes it, so it cannot collide
-  of these hold: the provider fixes it, so it cannot collide with
+  with anything you run; and reaching it hands out credentials, user
-  anything you run; and reaching it hands out credentials, user data or
+  data or bootstrap material. Those stay blocked no matter what you
-  bootstrap material. Those stay blocked no matter what you list,
+  list, including when you list them outright or list a supernet such
-  including when you list them outright or list a supernet such as
+  as `0.0.0.0/0`, `::/0`, `fd00::/8` or `100.64.0.0/10`. Treat this as
-  `0.0.0.0/0`, `::/0`, `fd00::/8` or `100.64.0.0/10`. Treat this as best
+  best effort rather than a guarantee — it is a hand-maintained list
-  effort rather than a guarantee — it is a hand-maintained list and the
+  and the caveat below the table applies:
-  caveat below the table applies:
 
   | Blocked unconditionally | What it is |
   | ----------------------- | ---------- |
@@ -248,8 +242,7 @@ Two things this setting cannot do:
   encodings, which the default blocklist does not match. A publicly
   routable metadata address is not listed here, because nothing on this
   list can be reopened and blocking one that way would leave you no
-  escape hatch at all; Azure's `168.63.129.16` is refused by the default
+  escape hatch at all.
-  blocklist instead, as described above.
 
   This list is not exhaustive of every cloud's metadata address — if
   yours is not here, do not allowlist the block that contains it.
@@ -975,10 +968,15 @@ scratch file**: it holds committed transactions that are not yet in the
 have no readable schema at all. `-shm` is regenerable, but there is no
 reason to separate the two — copy the directory and you have them.
 
-A clean shutdown closes every database, which checkpoints and removes
+A clean shutdown closes `webhooker.db` and every `events-*.db`, which
-its sidecars; a killed or crashed instance leaves them, and they must be
+checkpoints and removes their sidecars; a killed or crashed instance
-carried with the `.db`. An archive the service has not opened since a
+leaves them, and they must be carried with the `.db`. **Archive
-crash keeps that crash's sidecars, even across a later clean stop.
+databases are different**: their handle is not closed at shutdown, so
+`archive-*.db-wal` and `-shm` normally survive a clean stop and the
+`-wal` can hold every row the archive has. Measured on a stopped
+instance: `archive-….db` 4096 bytes with no table, its `-wal` 157 KB
+holding all 8 archived events. Copying `DATA_DIR` in full is what makes
+this a non-issue; copying `.db` files out of it by name is not.
 
 Configuration is **not** in `DATA_DIR` — it comes from the environment
 and from a `.env` file read out of the process working directory. Back
@@ -1053,9 +1051,10 @@ The file becomes self-contained again when the handle closes, which
 happens on the next write past the debounce window, when the connection
 pool retires the idle connection (about a minute after the last write),
 or at the idle archive sweep — measured, the same file was a complete
-20 KB `.db` with no sidecars about a minute after its last write. A
+20 KB `.db` with no sidecars about a minute after its last write.
-clean stop closes it too. So either move `archive-{uuid}.db` together
+Shutdown is **not** on that list: the archive handle is not closed when
-with any `-wal`/`-shm` beside it, or wait until there are none.
+the service stops. So either move `archive-{uuid}.db` together with any
+`-wal`/`-shm` beside it, or wait until there are none.
 
 ### Restore
 
@@ -1074,10 +1073,12 @@ with any `-wal`/`-shm` beside it, or wait until there are none.
    They are part of the database, and dropping a `-wal` silently
    discards every transaction it still holds. An `.backup` set will not
    contain any: it writes a single consolidated file per database. A
-   stop-and-copy set normally has none, because a clean stop closes
+   stop-and-copy set has none for `webhooker.db` or the `events-*.db`,
-   every database and checkpoints its sidecars away; the exception is an
+   because a clean stop closes those and checkpoints their sidecars
-   archive not opened since a crash. A copy salvaged from a crashed
+   away — but it will normally have them for `archive-*.db`, whose
-   instance has them for everything, and needs all of them.
+   handle stays open across shutdown, and those carry the archive's
+   rows. A copy salvaged from a crashed instance has them for
+   everything, and needs all of them.
 
 4. **Fix ownership.** The container runs as the non-root `webhooker`
    user, UID 1000 / GID 1000. Restored files must be owned by (or
@@ -2721,7 +2722,7 @@ abuse limit later; they are tracked as future work.
 | ------ | --------------------------- | ----------- |
 | `GET`  | `/`                         | Root redirect, 303 (authenticated → `/sources`, unauthenticated → `/pages/login`) |
 | `GET`  | `/.well-known/healthcheck`  | Health check (JSON: `status`, `now`, `uptimeSeconds`, `uptimeHuman`, `version`, `appname`, `maintenanceMode`) |
-| `GET`, `HEAD` | `/s/*`              | Static file serving (embedded CSS, JS). `GET` and `HEAD` only — `POST`, `PUT`, `PATCH`, `DELETE`, `OPTIONS`, `TRACE` and `CONNECT` are answered `405 Method Not Allowed` with `Allow: GET, HEAD`. Any other method (such as `PROPFIND`) is refused by chi before it reaches this route, and gets `405` without an `Allow` header. Pinned by `TestStaticServesOnlyGetAndHead` |
+| any    | `/s/*`                      | Static file serving (embedded CSS, JS). Mounted for every method, not just `GET`/`HEAD`: chi's `Mount` registers all methods and `http.FileServer` special-cases only `HEAD` (by omitting the body), so a `POST` or `DELETE` to an asset is answered `200` with the file. Pinned by `TestStaticServesEveryMethod` |
 | `POST` | `/webhook/{uuid}`           | Webhook receiver endpoint. `POST` only — every other method is answered `405 Method Not Allowed` with `Allow: POST`. Rate limited (see [Rate Limiting](#rate-limiting)) |
 
 #### Authentication Endpoints
@@ -3087,8 +3088,7 @@ each hook. The order, read off the fx stop-hook log:
 3. `server` — the HTTP drain, bounded separately by
    `server.ShutdownTimeout` (**3 seconds**), then a Sentry flush if
    `SENTRY_DSN` is set
-4. `delivery.Engine` — waits for its workers, then closes the archive
+4. `delivery.Engine`
-   databases
 5. `healthcheck`
 6. `WebhookDBManager`
 7. the database close
@@ -3280,5 +3280,3 @@ MIT
 ## Author
 
 [@sneak](https://sneak.berlin)
-
-

go.mod

@@ -4,7 +4,6 @@ go 1.26.1
 
 require (
 	github.com/99designs/basicauth-go v0.0.0-20230316000542-bf6f9cbbf0f8
-	github.com/dustin/go-humanize v1.0.1
 	github.com/getsentry/sentry-go v0.25.0
 	github.com/go-chi/chi v1.5.5
 	github.com/go-chi/cors v1.2.1
@@ -30,6 +29,7 @@ require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
+	github.com/dustin/go-humanize v1.0.1 // indirect
 	github.com/gorilla/securecookie v1.1.2 // indirect
 	github.com/jinzhu/inflection v1.0.0 // indirect
 	github.com/jinzhu/now v1.1.5 // indirect

internal/config/config.go

@@ -192,10 +192,9 @@ type Config struct {
 	// alwaysBlockedNetworks stays blocked no matter what is listed
 	// here. That set is link-local plus the cloud metadata
 	// endpoints outside it that disclose credentials or user data
-	// at a provider-fixed, non-public address; it is not
+	// at a provider-fixed address; it is not exhaustive of every
-	// exhaustive of every cloud's metadata address. See
+	// cloud's metadata address. See alwaysBlockedNetworks for the
-	// alwaysBlockedNetworks for the authoritative list and the
+	// authoritative list and the criterion it is built from.
-	// criterion it is built from.
 	AllowedEgressCIDRs []netip.Prefix
 
 	params *ConfigParams
@@ -747,14 +746,12 @@ func (c *Config) warnEgressAllowlist(log *slog.Logger) {
 
 	log.Warn(
 		"ALLOWED_EGRESS_CIDRS lets delivery targets reach these "+
-			"otherwise-blocked networks. Anyone who can create a "+
+			"otherwise-blocked private/reserved networks. Anyone "+
-			"delivery target can now make this process issue "+
+			"who can create a delivery target can now make this "+
-			"requests into them, and read back the response. Only "+
+			"process issue requests into them, and read back the "+
-			"the addresses the README lists as blocked "+
+			"response. Link-local and the known cloud instance "+
-			"unconditionally stay blocked regardless of what is "+
+			"metadata endpoints outside it stay blocked "+
-			"listed here; a public cloud metadata address such as "+
+			"regardless of what is listed here.",
-			"168.63.129.16 is reachable once it, or a block "+
-			"covering it, is listed.",
 		"allowedEgressCIDRs",
 		strings.Join(PrefixStrings(c.AllowedEgressCIDRs), ","),
 	)

internal/config/config_test.go

@@ -834,13 +834,12 @@ func TestEgressAllowlistWarning(t *testing.T) {
 			// to be able to read back which networks are open.
 			assert.Contains(t, logged, "10.0.0.0/8")
 			assert.Contains(t, logged, "127.0.0.0/8")
-			// What stays shut is the whole unconditional set, not
+			// What stays shut. Asserted on the clause naming the
-			// link-local alone; a public metadata address is not in
+			// wider set rather than on "Link-local" alone, so the
-			// it, so a listed block covering it opens it.
+			// string cannot narrow back to link-local only while
-			assert.Contains(t, logged, "blocked unconditionally")
+			// the always-blocked set covers ULA, CGNAT and two
-			assert.Contains(t, logged, "168.63.129.16 is reachable")
+			// public metadata addresses as well.
-			// The listed blocks need not be private or reserved.
+			assert.Contains(t, logged, "metadata endpoints outside it")
-			assert.NotContains(t, logged, "private/reserved")
 		})
 	}
 }

internal/delivery/engine.go

@@ -362,15 +362,6 @@ func (e *Engine) start() {
 // stop cancels the worker pool's context and waits for the pool
 // to drain, bounded by the stop hook's context: a wedged worker
 // must not hang the process past fx's stop timeout.
-//
-// Once the pool has drained it closes the archive writers, so a
-// clean stop leaves no archive -wal behind. Nothing else holds a
-// writer for long by then: the archive sweeper stops before the
-// engine, and deleting a webhook only closes one. If the pool did
-// not drain in time, the writers are left open, as a kill would
-// leave them. Closing them would wait for any write in progress,
-// and a worker still running would then open new writers that
-// nothing closes, so it gains nothing over a kill.
 func (e *Engine) stop(ctx context.Context) error {
 	e.log.Info("delivery engine stopping")
 
@@ -385,8 +376,6 @@ func (e *Engine) stop(ctx context.Context) error {
 		return err
 	}
 
-	e.dbTarget.evictAll()
-
 	e.log.Info("delivery engine stopped")
 
 	return nil

internal/delivery/engine_lifecycle_test.go

@@ -2,8 +2,6 @@ package delivery_test
 
 import (
 	"context"
-	"fmt"
-	"path/filepath"
 	"testing"
 	"time"
 
@@ -271,88 +269,3 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
 
 	requireStopHookExpires(t, lc.hooks[0], "delivery engine")
 }
-
-// deliverToArchive runs one delivery to a database target through
-// the running engine and returns the webhook's archive file path.
-// The archive writer holds the file open afterwards.
-func deliverToArchive(t *testing.T, s iSetup) string {
-	t.Helper()
-
-	deliveryID, task := seedLogTask(t, s)
-	task.TargetType = database.TargetTypeDatabase
-
-	s.Engine.Notify([]delivery.Task{task})
-
-	iWaitForDelivered(t, s.WebhookDB, deliveryID)
-
-	return filepath.Join(
-		filepath.Dir(s.DBMgr.DBPath(s.WebhookID)),
-		fmt.Sprintf("archive-%s.db", s.WebhookID),
-	)
-}
-
-// TestEngine_StopHookClosesArchives is the regression test for an
-// archive split across two files by a clean stop. The engine never
-// closed its archive writers, so after a stop the archived rows
-// could sit in archive-{id}.db-wal while archive-{id}.db held no
-// table at all, and copying the .db on its own gave an empty
-// database.
-func TestEngine_StopHookClosesArchives(t *testing.T) {
-	t.Parallel()
-
-	s := newISetup(t)
-
-	lc := startEngineViaHook(t, s.Engine)
-
-	path := deliverToArchive(t, s)
-	require.FileExists(
-		t, path+"-wal",
-		"an open archive should have a -wal for the stop to remove",
-	)
-
-	require.NoError(t, lc.hooks[0].OnStop(context.Background()))
-
-	wals, err := filepath.Glob(
-		filepath.Join(filepath.Dir(path), "archive-*.db-wal"),
-	)
-	require.NoError(t, err)
-	require.Empty(
-		t, wals, "a clean stop must leave no archive -wal behind",
-	)
-
-	// With no -wal beside it, the row can only be in the .db.
-	count, err := countArchivedRows(path)
-	require.NoError(t, err)
-	require.Equal(t, int64(1), count)
-}
-
-// TestEngine_StopHookTimeoutLeavesArchivesOpen covers a stop whose
-// budget runs out while a worker is still running. The archive
-// writers are left open, as a kill would leave them: closing them
-// would wait for any write in progress, and that worker would then
-// open new writers that nothing closes.
-func TestEngine_StopHookTimeoutLeavesArchivesOpen(t *testing.T) {
-	t.Parallel()
-
-	s := newISetup(t)
-
-	lc := startEngineViaHook(t, s.Engine)
-
-	deliverToArchive(t, s)
-
-	release := make(chan struct{})
-
-	t.Cleanup(func() {
-		close(release)
-		s.Engine.EvictWebhook(s.WebhookID)
-	})
-
-	s.Engine.ExportWedgeWorker(release)
-
-	requireStopHookExpires(t, lc.hooks[0], "delivery engine")
-
-	require.True(
-		t, s.Engine.ExportArchiveHandleOpen(s.WebhookID),
-		"a stop that timed out must not close archive writers",
-	)
-}

internal/delivery/engine_test.go

@@ -1166,10 +1166,6 @@ func TestIsForwardableHeader(t *testing.T) {
 	assert.False(t,
 		delivery.ExportIsForwardableHeader("Content-Length"),
 	)
-
-	assert.False(t,
-		delivery.ExportIsForwardableHeader("Content-Type"),
-	)
 }
 
 func TestTruncate(t *testing.T) {
@@ -1251,81 +1247,6 @@ func TestDoHTTPRequest_ForwardsHeaders(t *testing.T) {
 	)
 }
 
-// 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
-// on the target winning, then the event's ContentType.
-func TestApplyRequestHeaders_SendsOneContentType(t *testing.T) {
-	t.Parallel()
-
-	cases := map[string]struct {
-		inbound    string
-		event      string
-		configured string
-		want       []string
-	}{
-		"inbound and event agree": {
-			inbound: testContentType,
-			event:   testContentType,
-			want:    []string{testContentType},
-		},
-		"inbound and event disagree": {
-			inbound: "text/plain",
-			event:   testContentType,
-			want:    []string{testContentType},
-		},
-		"event has none": {
-			inbound: testContentType,
-			want:    nil,
-		},
-		"target configures its own": {
-			inbound:    testContentType,
-			event:      testContentType,
-			configured: "application/xml",
-			want:       []string{"application/xml"},
-		},
-	}
-
-	for name, tc := range cases {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			inbound, err := json.Marshal(map[string][]string{
-				headerContentType: {tc.inbound},
-			})
-			require.NoError(t, err)
-
-			cfg := &delivery.HTTPTargetConfig{}
-			if tc.configured != "" {
-				cfg.Headers = map[string]string{
-					headerContentType: tc.configured,
-				}
-			}
-
-			req, err := http.NewRequestWithContext(
-				context.Background(),
-				http.MethodPost,
-				"https://target.example.com/hook",
-				http.NoBody,
-			)
-			require.NoError(t, err)
-
-			delivery.ExportApplyRequestHeaders(
-				req,
-				&database.Event{
-					Headers:     string(inbound),
-					ContentType: tc.event,
-				},
-				cfg,
-			)
-
-			assert.Equal(t,
-				tc.want, req.Header.Values(headerContentType),
-			)
-		})
-	}
-}
-
 func TestProcessDelivery_RoutesToCorrectHandler(
 	t *testing.T,
 ) {

internal/delivery/redirect_test.go

@@ -339,11 +339,10 @@ func TestRedirectPolicy_StopsAtHopCap(t *testing.T) {
 // The set the redirect policy strips is whatever the delivery path
 // actually put on the wire, so a header added to the forward set is
 // covered without a second edit. A header the event never carried
-// is not in the set, and neither is the inbound Content-Type, because
+// is not in the set, and the delivery path's own two are deliberately
-// it is not forwarded. Two more are deliberately excluded: a
+// excluded: Content-Type describes the body, which a 307 carries
-// Content-Type configured on the target describes the body, which a
+// across hosts, and the inbound User-Agent every real sender supplies
-// 307 carries across hosts, and the inbound User-Agent every real
+// is overwritten before the request goes out.
-// sender supplies is overwritten before the request goes out.
 func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
 	t.Parallel()
 
@@ -372,7 +371,6 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
 		&delivery.HTTPTargetConfig{
 			Headers: map[string]string{
 				probeHeaderName: probeHeaderValue,
-				"Content-Type":  testContentType,
 			},
 		},
 	)
@@ -380,11 +378,7 @@ func TestApplyRequestHeaders_ReportsOriginScopedNames(t *testing.T) {
 	assert.Equal(t,
 		[]string{probeHeaderName, inboundHeaderName}, names,
 		"both header classes are reported, and only those: "+
-			"Host and the inbound Content-Type are never "+
+			"Host is never forwarded, Content-Type and "+
-			"forwarded, User-Agent is the delivery path's own",
+			"User-Agent are the delivery path's own",
-	)
-	assert.NotContains(t, names, "Content-Type",
-		"a Content-Type configured on the target must survive "+
-			"a cross-origin 307/308 with the body it describes",
 	)
 }

internal/delivery/ssrf.go

@@ -26,7 +26,7 @@ var (
 		"hostname resolved to no IP addresses",
 	)
 	errBlockedIP = errors.New(
-		"blocked private, reserved or cloud metadata address",
+		"blocked private/reserved IP range",
 	)
 	errBlockedMetadata = errors.New(
 		"blocked link-local or cloud instance metadata " +
@@ -37,10 +37,9 @@ var (
 	)
 )
 
-// blockedNetworks is the default blocklist: the private and
+// blockedNetworks contains all private/reserved IP ranges
-// reserved IP ranges, plus the public cloud metadata addresses,
+// that should be blocked to prevent SSRF attacks. An operator
-// that are blocked to prevent SSRF attacks. An operator can
+// can permit specific blocks out of this set with
-// permit specific blocks out of this set with
 // ALLOWED_EGRESS_CIDRS; see Guard.
 //
 //nolint:gochecknoglobals // package-level network list is appropriate here
@@ -123,8 +122,6 @@ func init() {
 		"::1/128",
 		"fc00::/7",
 		"fe80::/10",
-		// Azure WireServer, a public address that serves VM credentials.
-		"168.63.129.16/32",
 	})
 
 	// Every entry is named. The set must not grow or shrink
@@ -219,8 +216,8 @@ func matchesAny(networks []*net.IPNet, ip net.IP) bool {
 }
 
 // isBlockedIP checks whether an IP address falls within
-// the default blocklist, before any operator allowlist is
+// any blocked private/reserved network range, before any
-// considered.
+// operator allowlist is considered.
 func isBlockedIP(ip net.IP) bool {
 	return matchesAny(blockedNetworks, ip)
 }
@@ -323,7 +320,7 @@ func (g *Guard) allows(ip net.IP) bool {
 //
 //  1. alwaysBlockedNetworks is refused before the allowlist is
 //     consulted, so no configured CIDR reaches link-local or a
-//     cloud metadata endpoint at a non-public address.
+//     cloud instance metadata endpoint.
 //  2. The allowlist is consulted next, so a listed private
 //     network becomes reachable.
 //  3. Everything else keeps the default blocklist's answer.

internal/delivery/ssrf_allowlist_test.go

@@ -390,41 +390,6 @@ func TestGuardAllowlist_PublicUnaffected(t *testing.T) {
 	}
 }
 
-// TestGuardAllowlist_AzureWireServerReopenable covers Azure's
-// WireServer, a public address that serves VM credentials. The
-// default guard refuses it, but because it is public it sits in
-// the default blocklist rather than the unconditional set, so an
-// operator who lists it can reach it.
-func TestGuardAllowlist_AzureWireServerReopenable(t *testing.T) {
-	t.Parallel()
-
-	const wireServerIP = "168.63.129.16"
-
-	target := "http://" + wireServerIP + "/?comp=versions"
-
-	defaultGuard := delivery.NewTestGuard()
-
-	err := defaultGuard.ValidateTargetURL(context.Background(), target)
-	require.Error(t, err,
-		"WireServer must be refused with no allowlist set",
-	)
-	assert.NotContains(t, err.Error(), metadataRefusalClause,
-		"WireServer must be refused by the default blocklist, "+
-			"which an allowlist can override",
-	)
-
-	assertDialRefused(t, defaultGuard, target)
-
-	listed := delivery.NewTestGuard(
-		netip.MustParsePrefix(wireServerIP + "/32"),
-	)
-
-	assert.NoError(t,
-		listed.ValidateTargetURL(context.Background(), target),
-		"an operator who lists WireServer must be able to reach it",
-	)
-}
-
 // TestGuardCheckIP_BothPathsShareOneDecision asserts that the
 // validator and the dialer are not two policies that happen to
 // agree: both are defined in terms of checkIP, so the exported

internal/delivery/target_database.go

@@ -277,24 +277,6 @@ func (t *databaseTarget) evict(webhookID string) {
 	)
 }
 
-// evictAll evicts every cached archive writer, exactly as evict
-// does for one webhook. The engine calls it at shutdown, once its
-// workers have returned. Closing the last handle on an archive
-// moves the contents of its -wal into the .db and removes the
-// -wal, so a clean stop leaves each archive as a single file.
-func (t *databaseTarget) evictAll() {
-	t.mu.Lock()
-
-	writers := t.writers
-	t.writers = nil
-
-	t.mu.Unlock()
-
-	for _, w := range writers {
-		w.evict()
-	}
-}
-
 // sweepWebhook prunes one webhook's archive of rows older than
 // expiry, without requiring a write. It returns nil (nothing to
 // do) when the archive file does not exist, so a sweep never

internal/delivery/target_database_evict_test.go

@@ -1,7 +1,6 @@
 package delivery_test
 
 import (
-	"context"
 	"errors"
 	"fmt"
 	"net/http"
@@ -362,46 +361,3 @@ func TestEvictWebhook_LaterDeliveryRecreatesWriter(t *testing.T) {
 		"a later delivery should recreate the writer",
 	)
 }
-
-// TestEngineStop_WriteAfterStopIsRefused proves the engine's stop
-// closes each archive writer the way deleting its webhook does: a
-// write that reaches a writer after the stop is refused, reopens
-// nothing and adds no row.
-func TestEngineStop_WriteAfterStopIsRefused(t *testing.T) {
-	t.Parallel()
-
-	eng, _ := evictTestEngine(t)
-
-	webhookDB := testWebhookDB(t)
-	event := seedEvent(t, webhookDB, `{"archived":true}`)
-	d := seedDatabaseTargetDelivery(t, webhookDB, event, "")
-
-	eng.ExportDeliverDatabase(webhookDB, d)
-
-	w := eng.ExportArchiveWriterFor(event.WebhookID)
-	require.NotNil(t, w)
-	require.True(t, w.HandleOpen())
-
-	require.NoError(t, eng.ExportStop(context.Background()))
-
-	err := w.Write(evictTestRow("ev-after-stop"), 0)
-
-	require.ErrorIs(
-		t, err, delivery.ErrExportArchiveWriterEvicted,
-		"a write after the stop must be refused",
-	)
-	assert.False(
-		t, w.HandleOpen(),
-		"a refused write must not reopen the archive",
-	)
-	assert.False(
-		t, eng.ExportHasArchiveWriter(event.WebhookID),
-		"the stop should empty the registry",
-	)
-
-	count, err := countArchivedRows(w.Path())
-	require.NoError(t, err)
-	assert.Equal(
-		t, int64(1), count, "the refused row must not be written",
-	)
-}

internal/delivery/target_headers_test.go

@@ -11,11 +11,10 @@ import (
 	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
-// Literals these tests repeat, named so that the header names and the
+// Literals these tests repeat, named so that the header name and the
 // keep-forever archive config each have one definition.
 const (
 	headerAuthorization = "Authorization"
-	headerContentType   = "Content-Type"
 	bearerValue         = "Bearer abc"
 	archiveConfigNever  = "{\"expiry\":\"never\"}"
 )

internal/delivery/target_http.go

@@ -541,11 +541,6 @@ func isForwardableHeader(name string) bool {
 		"Upgrade", "Proxy-Authorization",
 		"Proxy-Connection", "Content-Length":
 		return false
-	case "Content-Type":
-		// applyRequestHeaders sets Content-Type itself. The receiver
-		// already stored this inbound value as the event's
-		// ContentType, so forwarding it too would send it twice.
-		return false
 	default:
 		return true
 	}
@@ -558,10 +553,6 @@ func isForwardableHeader(name string) bool {
 // policy strips exactly that set on a hop that leaves the origin,
 // so the forward set is decided here and only here — a header added
 // to it is covered off-origin without a second edit elsewhere.
-//
-// Content-Type goes out once: a Content-Type configured on the target
-// wins, otherwise the event's ContentType, otherwise none. The inbound
-// Content-Type in the event's headers is never forwarded.
 func applyRequestHeaders(
 	req *http.Request,
 	event *database.Event,
@@ -582,10 +573,10 @@ func applyRequestHeaders(
 
 	req.Header.Set("User-Agent", "webhooker/1.0")
 
-	// A Content-Type configured on the target describes the body
+	// Content-Type describes the body being sent rather than the
-	// being sent rather than the sender. A 307/308 preserves the
+	// sender, and the delivery path sets it from the event itself.
-	// body across hosts, so stripping it would send that body
+	// A 307/308 preserves the body across hosts, so stripping it
-	// untyped.
+	// would send that body untyped.
 	delete(originScoped, "Content-Type")
 
 	// User-Agent is overwritten just above, so an inbound one never

internal/handlers/delivery_result_view.go

@@ -1,8 +1,6 @@
 package handlers
 
 import (
-	"time"
-
 	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
@@ -27,7 +25,7 @@ const maxRenderedResponseBytes = 4096
 // cut, so an oversized stored response never becomes a Go
 // string at all.
 const deliveryResultColumns = "delivery_id, attempt_num, success, " +
-	"status_code, error, duration, created_at, " +
+	"status_code, error, duration, " +
 	"substr(cast(response_body as blob), 1, ?) AS response_body, " +
 	"length(cast(response_body as blob)) AS response_bytes"
 
@@ -108,10 +106,6 @@ type deliveryResultRow struct {
 	Duration      int64
 	ResponseBody  []byte
 	ResponseBytes int64
-
-	// CreatedAt is when the attempt's result was recorded, which
-	// is when the attempt finished.
-	CreatedAt time.Time
 }
 
 // view projects a loaded row for rendering, stripping the

internal/handlers/handlers.go

@@ -28,7 +28,7 @@ const (
 	// maxBodyShift is the bit shift for 1 MB body limit.
 	maxBodyShift = 20
 	// recentEventLimit is the number of recent events to show.
-	recentEventLimit = 50
+	recentEventLimit = 20
 	// paginationPerPage is the number of items per page.
 	paginationPerPage = 25
 

internal/handlers/recent_events.go

@@ -1,248 +0,0 @@
-package handlers
-
-import (
-	"net/http"
-	"strconv"
-	"time"
-
-	"github.com/dustin/go-humanize"
-	"gorm.io/gorm"
-	"sneak.berlin/go/webhooker/internal/database"
-)
-
-// recentEventColumns is the recent events list's projection. It
-// reads the body's size and never the body itself, for the reason
-// maxRenderedBodyBytes gives; the cast to blob makes length count
-// bytes rather than characters.
-const recentEventColumns = "id, created_at, method, content_type, " +
-	"resubmitted_from_id, length(cast(body as blob)) AS body_bytes"
-
-// RecentEventView is one row of the recent events list on a
-// webhook's page.
-type RecentEventView struct {
-	Method      string
-	ContentType string
-
-	// ResubmittedFromID names the event this one was copied from,
-	// empty for an event that arrived on the receiver.
-	ResubmittedFromID string
-
-	// Received is how long ago the event arrived, and ReceivedUTC
-	// the full timestamp the page shows on hover.
-	Received    string
-	ReceivedUTC string
-
-	// Size is the size of the stored body.
-	Size string
-
-	// ProcessingTime is how long the event's slowest delivery
-	// took; see processingTime.
-	ProcessingTime string
-
-	// Status is what the webhook's HTTP target answered, and
-	// StatusClass its colour; see targetStatus. Both are empty
-	// unless the webhook has exactly one HTTP target.
-	Status      string
-	StatusClass string
-}
-
-// recentEventRow is one row of recentEventColumns.
-type recentEventRow struct {
-	ID                string
-	CreatedAt         time.Time
-	Method            string
-	ContentType       string
-	ResubmittedFromID *string
-	BodyBytes         uint64
-}
-
-// singleHTTPTargetID returns the ID of the webhook's HTTP target
-// when it has exactly one, and "" when it has none or several.
-func singleHTTPTargetID(targets []database.Target) string {
-	id := ""
-	count := 0
-
-	for i := range targets {
-		if targets[i].Type == database.TargetTypeHTTP {
-			id = targets[i].ID
-			count++
-		}
-	}
-
-	if count != 1 {
-		return ""
-	}
-
-	return id
-}
-
-// loadRecentEvents loads the webhook's recentEventLimit newest
-// events for its page, newest first. statusTargetID is the
-// webhook's only HTTP target, or "" when the list shows no status.
-func (h *Handlers) loadRecentEvents(
-	webhookDB *gorm.DB, webhookID, statusTargetID string,
-) ([]RecentEventView, error) {
-	var rows []recentEventRow
-
-	err := webhookDB.Model(&database.Event{}).
-		Select(recentEventColumns).
-		Where("webhook_id = ?", webhookID).
-		Order("created_at DESC").
-		Limit(recentEventLimit).
-		Find(&rows).Error
-	if err != nil {
-		return nil, err
-	}
-
-	eventIDs := make([]string, len(rows))
-	for i := range rows {
-		eventIDs[i] = rows[i].ID
-	}
-
-	// Oldest first, so an event's last delivery to a target is its
-	// newest: a replay adds a delivery rather than changing the
-	// earlier one.
-	var deliveries []database.Delivery
-
-	err = webhookDB.
-		Select("id, event_id, target_id, status, created_at").
-		Where("event_id IN ?", eventIDs).
-		Order("created_at ASC").
-		Find(&deliveries).Error
-	if err != nil {
-		return nil, err
-	}
-
-	byEvent := make(map[string][]database.Delivery, len(rows))
-	deliveryIDs := make([]string, len(deliveries))
-
-	for i := range deliveries {
-		eventID := deliveries[i].EventID
-		byEvent[eventID] = append(byEvent[eventID], deliveries[i])
-		deliveryIDs[i] = deliveries[i].ID
-	}
-
-	attempts, err := h.loadDeliveryResults(webhookDB, deliveryIDs)
-	if err != nil {
-		return nil, err
-	}
-
-	views := make([]RecentEventView, len(rows))
-	for i := range rows {
-		views[i] = rows[i].view(
-			byEvent[rows[i].ID], attempts, statusTargetID,
-		)
-	}
-
-	return views, nil
-}
-
-// view projects a loaded row for rendering. deliveries is the
-// event's deliveries, oldest first, and attempts their recorded
-// attempts keyed by delivery ID.
-func (r *recentEventRow) view(
-	deliveries []database.Delivery,
-	attempts map[string][]deliveryResultRow,
-	statusTargetID string,
-) RecentEventView {
-	v := RecentEventView{
-		Method:         r.Method,
-		ContentType:    r.ContentType,
-		Received:       humanize.Time(r.CreatedAt),
-		ReceivedUTC:    r.CreatedAt.UTC().Format(time.DateTime) + " UTC",
-		Size:           humanize.Bytes(r.BodyBytes),
-		ProcessingTime: processingTime(deliveries, attempts),
-	}
-
-	if r.ResubmittedFromID != nil {
-		v.ResubmittedFromID = *r.ResubmittedFromID
-	}
-
-	if statusTargetID != "" {
-		v.Status, v.StatusClass = targetStatus(
-			deliveries, attempts, statusTargetID,
-		)
-	}
-
-	return v
-}
-
-// processingTime is how long the event's slowest delivery took,
-// from being queued to its last recorded attempt, time spent
-// waiting between retries included. A delivery is queued when its
-// event is received, or when an operator replays it, so a replay
-// is timed from the replay rather than from the event's arrival.
-// It is "in progress" while any delivery is pending or retrying,
-// and empty for an event with no deliveries.
-func processingTime(
-	deliveries []database.Delivery,
-	attempts map[string][]deliveryResultRow,
-) string {
-	if len(deliveries) == 0 {
-		return ""
-	}
-
-	var slowest time.Duration
-
-	for i := range deliveries {
-		if !deliveries[i].Status.Terminal() {
-			return "in progress"
-		}
-
-		tries := attempts[deliveries[i].ID]
-		if len(tries) == 0 {
-			continue
-		}
-
-		last := tries[len(tries)-1].CreatedAt
-		slowest = max(slowest, last.Sub(deliveries[i].CreatedAt))
-	}
-
-	return slowest.Round(time.Millisecond).String()
-}
-
-// targetStatus is what the target answered for the event, and the
-// colour to show it in: the HTTP status code of the last attempt of
-// the event's newest delivery to the target. Without a code it is
-// "no response" when that attempt failed before a response
-// arrived, the delivery's status ("pending") before any attempt,
-// and "not sent" when the event has no delivery to the target.
-func targetStatus(
-	deliveries []database.Delivery,
-	attempts map[string][]deliveryResultRow,
-	targetID string,
-) (string, string) {
-	newest := -1
-
-	for i := range deliveries {
-		if deliveries[i].TargetID == targetID {
-			newest = i
-		}
-	}
-
-	if newest < 0 {
-		return "not sent", "text-gray-400"
-	}
-
-	tries := attempts[deliveries[newest].ID]
-	if len(tries) == 0 {
-		return string(deliveries[newest].Status), "text-gray-400"
-	}
-
-	code := tries[len(tries)-1].StatusCode
-
-	switch {
-	case code == 0:
-		return "no response", "text-red-600"
-	case code >= http.StatusInternalServerError:
-		return strconv.Itoa(code), "text-red-600"
-	case code >= http.StatusBadRequest:
-		return strconv.Itoa(code), "text-yellow-600"
-	case code >= http.StatusMultipleChoices:
-		return strconv.Itoa(code), "text-gray-500"
-	case code >= http.StatusOK:
-		return strconv.Itoa(code), "text-green-600"
-	default:
-		return strconv.Itoa(code), "text-gray-500"
-	}
-}

internal/handlers/recent_events_test.go

@@ -1,295 +0,0 @@
-package handlers_test
-
-import (
-	"fmt"
-	"net/http"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"gorm.io/gorm"
-	"gorm.io/gorm/clause"
-	"sneak.berlin/go/webhooker/internal/database"
-	"sneak.berlin/go/webhooker/internal/handlers"
-	"sneak.berlin/go/webhooker/internal/session"
-)
-
-// statusTitle marks the status column's cell in a recent events
-// row; it is absent from the page when the column is not shown.
-const statusTitle = `title="HTTP status from the HTTP target"`
-
-// recentEventsFixture is one started app and a webhook whose
-// recent events list a test fills.
-type recentEventsFixture struct {
-	h         *handlers.Handlers
-	sess      *session.Session
-	db        *database.Database
-	webhook   *database.Webhook
-	webhookDB *gorm.DB
-}
-
-func newRecentEventsFixture(t *testing.T) *recentEventsFixture {
-	t.Helper()
-
-	f := &recentEventsFixture{}
-
-	var dbMgr *database.WebhookDBManager
-
-	app := newTestApp(t, &f.h, &f.sess, &f.db, &dbMgr)
-	app.RequireStart()
-
-	t.Cleanup(app.RequireStop)
-
-	f.webhook = seedWebhook(t, f.db)
-
-	webhookDB, err := dbMgr.GetDB(f.webhook.ID)
-	require.NoError(t, err)
-
-	f.webhookDB = webhookDB
-
-	return f
-}
-
-func (f *recentEventsFixture) render(t *testing.T) string {
-	t.Helper()
-
-	return renderSourceDetailPage(t, f.h, f.sess, f.webhook.ID)
-}
-
-// event records an event received at receivedAt.
-func (f *recentEventsFixture) event(
-	t *testing.T, contentType, body string, receivedAt time.Time,
-) *database.Event {
-	t.Helper()
-
-	event := &database.Event{
-		WebhookID:   f.webhook.ID,
-		Method:      http.MethodPost,
-		Body:        body,
-		ContentType: contentType,
-	}
-	event.CreatedAt = receivedAt
-
-	require.NoError(t, f.webhookDB.Omit(
-		clause.Associations,
-	).Create(event).Error)
-
-	return event
-}
-
-// delivery records a delivery of the event to the target, queued
-// when the event was received.
-func (f *recentEventsFixture) delivery(
-	t *testing.T,
-	event *database.Event,
-	targetID string,
-	status database.DeliveryStatus,
-) *database.Delivery {
-	t.Helper()
-
-	return f.deliveryQueuedAt(
-		t, event, targetID, status, event.CreatedAt,
-	)
-}
-
-// deliveryQueuedAt records a delivery of the event to the target,
-// queued at queuedAt, as a replay is.
-func (f *recentEventsFixture) deliveryQueuedAt(
-	t *testing.T,
-	event *database.Event,
-	targetID string,
-	status database.DeliveryStatus,
-	queuedAt time.Time,
-) *database.Delivery {
-	t.Helper()
-
-	dlv := &database.Delivery{
-		EventID:  event.ID,
-		TargetID: targetID,
-		Status:   status,
-	}
-	dlv.CreatedAt = queuedAt
-
-	require.NoError(t, f.webhookDB.Omit(
-		clause.Associations,
-	).Create(dlv).Error)
-
-	return dlv
-}
-
-// attempt records one attempt of the delivery that finished took
-// after the delivery was queued, with HTTP status code (0 for no
-// response).
-func (f *recentEventsFixture) attempt(
-	t *testing.T, dlv *database.Delivery, code int, took time.Duration,
-) {
-	t.Helper()
-
-	result := &database.DeliveryResult{
-		DeliveryID: dlv.ID,
-		AttemptNum: 1,
-		StatusCode: code,
-	}
-	result.CreatedAt = dlv.CreatedAt.Add(took)
-
-	require.NoError(t, f.webhookDB.Omit(
-		clause.Associations,
-	).Create(result).Error)
-}
-
-// statusCell is the status column's cell as the page renders it.
-func statusCell(class, text string) string {
-	return `<span class="font-medium ` + class + `" ` + statusTitle +
-		`>` + text + `</span>`
-}
-
-// TestHandleSourceDetail_ShowsFiftyNewestEvents proves the list
-// holds the 50 newest events, newest first, and not one more.
-func TestHandleSourceDetail_ShowsFiftyNewestEvents(t *testing.T) {
-	t.Parallel()
-
-	f := newRecentEventsFixture(t)
-	base := time.Now().Add(-time.Hour)
-
-	for i := range 51 {
-		f.event(
-			t, fmt.Sprintf("application/x-recent-%02d", i), "{}",
-			base.Add(time.Duration(i)*time.Second),
-		)
-	}
-
-	body := f.render(t)
-
-	assert.Equal(t, 50, strings.Count(body, `title="Body size"`))
-	assert.NotContains(t, body, "application/x-recent-00")
-	assert.Contains(t, body, "application/x-recent-01")
-	assert.Less(
-		t,
-		strings.Index(body, "application/x-recent-50"),
-		strings.Index(body, "application/x-recent-49"),
-	)
-}
-
-// TestHandleSourceDetail_RecentEventColumns proves a row shows its
-// time relative with the UTC timestamp on hover, its body size,
-// and its processing time once every delivery has finished.
-func TestHandleSourceDetail_RecentEventColumns(t *testing.T) {
-	t.Parallel()
-
-	f := newRecentEventsFixture(t)
-	logTarget := seedTarget(t, f.db, f.webhook.ID, database.TargetTypeLog)
-
-	receivedAt := time.Now().Add(-210 * time.Second).
-		UTC().Truncate(time.Second)
-
-	done := f.event(
-		t, contentTypeJSON, strings.Repeat("x", 2048), receivedAt,
-	)
-	f.attempt(
-		t,
-		f.delivery(t, done, logTarget.ID, database.DeliveryStatusDelivered),
-		0, 1500*time.Millisecond,
-	)
-
-	waiting := f.event(t, "text/plain", "{}", receivedAt)
-	f.delivery(t, waiting, logTarget.ID, database.DeliveryStatusPending)
-
-	body := f.render(t)
-
-	assert.Contains(
-		t, body,
-		`<span title="`+receivedAt.Format(time.DateTime)+
-			` UTC">3 minutes ago</span>`,
-	)
-	assert.Contains(t, body, `<span title="Body size">2.0 kB</span>`)
-	assert.Contains(t, body, ">1.5s</span>")
-	assert.Contains(t, body, ">in progress</span>")
-}
-
-// TestHandleSourceDetail_StatusWithSingleHTTPTarget proves that a
-// webhook with exactly one HTTP target shows, colour-coded, what
-// that target answered for each event. The log target beside it
-// does not count against "exactly one".
-func TestHandleSourceDetail_StatusWithSingleHTTPTarget(t *testing.T) {
-	t.Parallel()
-
-	f := newRecentEventsFixture(t)
-	target := seedTarget(t, f.db, f.webhook.ID, database.TargetTypeHTTP)
-	seedTarget(t, f.db, f.webhook.ID, database.TargetTypeLog)
-
-	now := time.Now()
-
-	for _, code := range []int{204, 302, 404, 503, 0} {
-		dlv := f.delivery(
-			t, f.event(t, contentTypeJSON, "{}", now), target.ID,
-			database.DeliveryStatusDelivered,
-		)
-		f.attempt(t, dlv, code, time.Second)
-	}
-
-	f.delivery(
-		t, f.event(t, contentTypeJSON, "{}", now), target.ID,
-		database.DeliveryStatusPending,
-	)
-	f.event(t, contentTypeJSON, "{}", now)
-
-	// A replay is a newer delivery, and its answer is the one shown.
-	replayed := f.event(t, contentTypeJSON, "{}", now)
-	f.attempt(t, f.delivery(
-		t, replayed, target.ID, database.DeliveryStatusFailed,
-	), 502, time.Second)
-	f.attempt(t, f.deliveryQueuedAt(
-		t, replayed, target.ID, database.DeliveryStatusDelivered,
-		now.Add(time.Minute),
-	), 200, time.Second)
-
-	body := f.render(t)
-
-	assert.Contains(t, body, statusCell("text-green-600", "204"))
-	assert.Contains(t, body, statusCell("text-gray-500", "302"))
-	assert.Contains(t, body, statusCell("text-yellow-600", "404"))
-	assert.Contains(t, body, statusCell("text-red-600", "503"))
-	assert.Contains(t, body, statusCell("text-red-600", "no response"))
-	assert.Contains(t, body, statusCell("text-gray-400", "pending"))
-	assert.Contains(t, body, statusCell("text-gray-400", "not sent"))
-	assert.Contains(t, body, statusCell("text-green-600", "200"))
-	assert.NotContains(t, body, ">502<")
-}
-
-// TestHandleSourceDetail_NoStatusWithoutSingleHTTPTarget proves the
-// status column is absent when the webhook has no HTTP target or
-// more than one.
-func TestHandleSourceDetail_NoStatusWithoutSingleHTTPTarget(
-	t *testing.T,
-) {
-	t.Parallel()
-
-	cases := map[string][]database.TargetType{
-		"none":    {database.TargetTypeLog},
-		"several": {database.TargetTypeHTTP, database.TargetTypeHTTP},
-	}
-
-	for name, types := range cases {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			f := newRecentEventsFixture(t)
-			event := f.event(t, contentTypeJSON, "{}", time.Now())
-
-			for _, tt := range types {
-				target := seedTarget(t, f.db, f.webhook.ID, tt)
-				f.attempt(t, f.delivery(
-					t, event, target.ID,
-					database.DeliveryStatusDelivered,
-				), 200, time.Second)
-			}
-
-			body := f.render(t)
-
-			assert.Contains(t, body, `title="Body size"`)
-			assert.NotContains(t, body, statusTitle)
-		})
-	}
-}

internal/handlers/source_management.go

@@ -415,21 +415,16 @@ func (h *Handlers) renderSourceDetail(
 		"webhook_id = ?", webhook.ID,
 	).Find(&targets)
 
-	var events []RecentEventView
+	var events []database.Event
 
 	if h.dbMgr.DBExists(webhook.ID) {
 		webhookDB, dbErr := h.dbMgr.GetDB(webhook.ID)
 		if dbErr == nil {
-			events, dbErr = h.loadRecentEvents(
+			webhookDB.Where(
-				webhookDB, webhook.ID, singleHTTPTargetID(targets),
+				"webhook_id = ?", webhook.ID,
-			)
+			).Order("created_at DESC").Limit(
-		}
+				recentEventLimit,
-
+			).Find(&events)
-		if dbErr != nil {
-			h.log.Error(
-				"failed to load recent events",
-				"webhook_id", webhook.ID, "error", dbErr,
-			)
 		}
 	}
 

internal/middleware/recoverer.go

@@ -133,11 +133,6 @@ func (w *recoverResponseWriter) Unwrap() http.ResponseWriter {
 // what the access log records and the metrics count, and outside the
 // sentryhttp handler, whose Repanic option depends on something
 // further out recovering what it re-raises.
-//
-// Unlike http.Error on its own, it deletes any Set-Cookie the handler
-// set before panicking, because a request that failed must not hand
-// the client a credential; every other header is left to http.Error.
-// See https://git.eeqj.de/sneak/webhooker/issues/193.
 func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(
@@ -169,8 +164,6 @@ func (s *Middleware) Recoverer() func(http.Handler) http.Handler {
 					return
 				}
 
-				rw.Header().Del("Set-Cookie")
-
 				http.Error(
 					rw,
 					http.StatusText(

internal/middleware/recoverer_test.go

@@ -304,44 +304,16 @@ func TestRecovererRepanicsErrAbortHandler(t *testing.T) {
 	)
 }
 
-// TestRecovererDropsSetCookieFromTheRecovered500 covers a handler that
-// sets a cookie and a redirect target and then panics before sending
-// anything. A request that failed must not hand the client a
-// credential, so the 500 carries no cookie; Location is left alone.
-func TestRecovererDropsSetCookieFromTheRecovered500(t *testing.T) {
-	t.Parallel()
-
-	probe := newRecovererProbe(
-		t, false,
-		func(w http.ResponseWriter, _ *http.Request) {
-			w.Header().Set("Set-Cookie", "session=x")
-			w.Header().Set("Location", "/after")
-
-			panic(panicMarker)
-		},
-	)
-
-	resp, err := probe.get(t)
-	require.NoError(t, err)
-	require.NoError(t, resp.Body.Close())
-
-	assert.Equal(t, http.StatusInternalServerError, resp.StatusCode)
-	assert.Empty(t, resp.Cookies())
-	assert.Equal(t, "/after", resp.Header.Get("Location"))
-}
-
 // TestRecovererKeepsAnAlreadyCommittedResponse covers a handler that
 // panics after sending its status. The bytes are already on the wire,
-// cookie included, so a second WriteHeader would change nothing the
+// so a second WriteHeader would change nothing the client sees and
-// client sees and would draw net/http's "superfluous
+// would draw net/http's "superfluous response.WriteHeader" report.
-// response.WriteHeader" report.
 func TestRecovererKeepsAnAlreadyCommittedResponse(t *testing.T) {
 	t.Parallel()
 
 	probe := newRecovererProbe(
 		t, false,
 		func(w http.ResponseWriter, _ *http.Request) {
-			w.Header().Set("Set-Cookie", "session=x")
 			w.WriteHeader(committedStatus)
 			_, _ = w.Write([]byte("partial"))
 
@@ -359,7 +331,6 @@ func TestRecovererKeepsAnAlreadyCommittedResponse(t *testing.T) {
 
 	assert.Equal(t, committedStatus, resp.StatusCode)
 	assert.Equal(t, "partial", string(body))
-	assert.Len(t, resp.Cookies(), 1)
 
 	record := probe.panicRecord(t)
 	assert.Equal(t, panicMarker, record["panic"])

internal/server/routes.go

@@ -92,25 +92,11 @@ func (s *Server) setupGlobalMiddleware() {
 func (s *Server) setupRoutes() {
 	s.router.Get("/", s.h.HandleIndex())
 
-	// Static assets answer GET and HEAD only. chi's default 405
+	s.router.Mount(
-	// carries no Allow header, so this group supplies its own.
+		"/s",
-	staticFiles := http.StripPrefix(
+		http.StripPrefix("/s", http.FileServer(http.FS(static.Static))),
-		"/s", http.FileServer(http.FS(static.Static)),
 	)
 
-	s.router.Route("/s", func(r chi.Router) {
-		r.MethodNotAllowed(func(w http.ResponseWriter, _ *http.Request) {
-			w.Header().Set("Allow", "GET, HEAD")
-			http.Error(
-				w,
-				"Method Not Allowed",
-				http.StatusMethodNotAllowed,
-			)
-		})
-		r.Method(http.MethodGet, "/*", staticFiles)
-		r.Method(http.MethodHead, "/*", staticFiles)
-	})
-
 	s.router.Route("/api/v1", func(_ chi.Router) {
 		// API routes will be added here.
 	})

internal/server/routes_test.go

@@ -396,15 +396,13 @@ func (e *testEnv) storedHash(t *testing.T, username string) string {
 
 // --- /s static group ---
 
-// TestStaticServesOnlyGetAndHead pins the methods the static group
+// TestStaticServesEveryMethod pins what the static mount actually
-// answers: GET and HEAD are served the asset, and the other methods
+// answers. chi's Mount registers the handler for all methods and
-// chi routes (POST, PUT, DELETE and the rest) are refused with 405
+// http.FileServer only special-cases HEAD (by suppressing the body),
-// and an Allow header naming those two. A method chi does not route,
+// so a POST or a DELETE to an asset is served the file rather than
-// such as PROPFIND, is refused with 405 by the top-level router
+// refused. The README documents this; the test is what keeps the two
-// before it reaches the static group, so it gets no Allow header.
+// from drifting.
-// The README documents this; the test is what keeps the two from
+func TestStaticServesEveryMethod(t *testing.T) {
-// drifting.
-func TestStaticServesOnlyGetAndHead(t *testing.T) {
 	t.Parallel()
 
 	env := newTestEnv(t)
@@ -419,7 +417,6 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) {
 		http.MethodPost,
 		http.MethodPut,
 		http.MethodDelete,
-		"PROPFIND",
 	} {
 		t.Run(method, func(t *testing.T) {
 			t.Parallel()
@@ -431,38 +428,18 @@ func TestStaticServesOnlyGetAndHead(t *testing.T) {
 			w := httptest.NewRecorder()
 			env.router.ServeHTTP(w, req)
 
-			switch method {
+			assert.Equal(t, http.StatusOK, w.Code,
-			case http.MethodGet:
+				"static mount answers every method")
-				assert.Equal(t, http.StatusOK, w.Code)
+
-				assert.Equal(t, body, w.Body.Bytes(),
+			if method == http.MethodHead {
-					"the asset itself is returned")
-			case http.MethodHead:
-				assert.Equal(t, http.StatusOK, w.Code)
 				assert.Empty(t, w.Body.Bytes(),
 					"HEAD must not carry a body")
-			case "PROPFIND":
+
-				assert.Equal(
+				return
-					t, http.StatusMethodNotAllowed, w.Code,
-				)
-				assert.Empty(t, w.Header().Get("Allow"),
-					"chi refuses a method it does not route "+
-						"before the static group runs")
-				assert.NotContains(
-					t, w.Body.String(), string(body),
-					"a refused method must not get the asset",
-				)
-			default:
-				assert.Equal(
-					t, http.StatusMethodNotAllowed, w.Code,
-				)
-				assert.Equal(
-					t, "GET, HEAD", w.Header().Get("Allow"),
-				)
-				assert.NotContains(
-					t, w.Body.String(), string(body),
-					"a refused method must not get the asset",
-				)
 			}
+
+			assert.Equal(t, body, w.Body.Bytes(),
+				"the asset itself is returned")
 		})
 	}
 }

templates/source_detail.html

@@ -187,24 +187,12 @@
         <div class="divide-y divide-gray-100">
             {{range .Events}}
             <div class="p-4">
-                <div class="flex flex-wrap items-center justify-between gap-3">
+                <div class="flex items-center justify-between">
-                    <div class="flex flex-wrap items-center gap-3">
+                    <div class="flex items-center gap-3">
                         <span class="badge-info">{{.Method}}</span>
-                        <span class="text-sm text-gray-500 break-all">{{.ContentType}}</span>
+                        <span class="text-sm text-gray-500">{{.ContentType}}</span>
-                        {{if .ResubmittedFromID}}
-                        <span class="text-xs text-gray-500" title="This event is a copy of {{.ResubmittedFromID}}">resubmitted copy</span>
-                        {{end}}
-                    </div>
-                    <div class="flex flex-wrap items-center gap-3 text-xs text-gray-400">
-                        <span title="Body size">{{.Size}}</span>
-                        {{if .ProcessingTime}}
-                        <span title="Processing time: how long the slowest delivery took, from being queued to its last attempt">{{.ProcessingTime}}</span>
-                        {{end}}
-                        {{if .Status}}
-                        <span class="font-medium {{.StatusClass}}" title="HTTP status from the HTTP target">{{.Status}}</span>
-                        {{end}}
-                        <span title="{{.ReceivedUTC}}">{{.Received}}</span>
                     </div>
+                    <span class="text-xs text-gray-400">{{.CreatedAt.Format "2006-01-02 15:04:05 UTC"}}</span>
                 </div>
             </div>
             {{else}}