Escape the database path in the SQLite connection string (closes #518)

SQLite reads a file: URI's path only up to the first ? or #, and
decodes %XX escapes in it. With a data directory such as
/srv/hooks#1, every database opened the one file /srv/hooks. The
connection string is now built with url.URL, which escapes the path,
and OmitHost keeps a relative path relative.

Tests open databases in a directory whose name holds #, ? and %, and
at a relative path, and check through PRAGMA database_list that each
opened the file asked for. A handlers test comment that blamed # in
the temporary directory's path is dropped, as that is no longer true.

Model: opus-5-5

Dockerfile

@@ -214,22 +214,16 @@ RUN version="$(make version VERSION="$VERSION")"; \
 # (Alpine.js, extracted from its tarball in 3p/) first.
 RUN make build VERSION="$VERSION"
 
-# Rebuild with static linking for the Alpine runtime. The cgo binary the
-# step above builds is dynamically linked against glibc, which Alpine
-# (musl) does not have.
+# Rebuild with static linking for Alpine runtime.
+# make build already verified compilation.
+# The CGO binary from `make build` is dynamically linked against glibc,
+# which doesn't exist on Alpine (musl). Rebuild with static linking so
+# the binary runs on Alpine without glibc.
 #
-# A statically linked glibc still loads shared libraries at run time for
-# getaddrinfo and dlopen, and Alpine has none to load, so the build tags
-# keep both out of the binary: netgo and osusergo select Go's own DNS
-# resolver and user lookup instead of glibc's, and
-# sqlite_omit_load_extension leaves out SQLite extension loading, which
-# webhooker never uses.
-#
-# The static flags and tags go in through GO_LDFLAGS and GO_TAGS rather
-# than a -ldflags of their own: the build target composes them with the
-# -X that stamps the version, so this relink cannot silently drop the
-# stamp.
-RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_TAGS=netgo,osusergo,sqlite_omit_load_extension GO_LDFLAGS='-extldflags "-static"'
+# The static flags go in through GO_LDFLAGS rather than a -ldflags of
+# their own: the build target composes them with the -X that stamps the
+# version, so this relink cannot silently drop the stamp.
+RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'
 
 # Runtime stage
 # alpine:3.21, 2026-03-17

Makefile

@@ -16,16 +16,14 @@ VERSION ?= $(shell script/version)
 # exists to correct.
 override VERSION := $(or $(strip $(VERSION)),$(shell script/version))
 
-# Extra linker flags and build tags for the build target. The static
-# relink in the Dockerfile passes -extldflags and its build tags through
-# these rather than a -ldflags of its own, so composing flags cannot drop
-# the version stamp.
+# Extra linker flags for the build target. The static relink in the
+# Dockerfile adds -extldflags here rather than passing its own -ldflags,
+# so composing flags cannot drop the version stamp.
 #
 # The build target itself always passes -trimpath and -s -w, as the Go
 # Dockerfile in REPO_POLICIES.md does: no build paths, symbol table or
 # debug information in the binary.
 GO_LDFLAGS ?=
-GO_TAGS ?=
 
 bootstrap:
 	@script/bootstrap
@@ -55,7 +53,7 @@ check:
 	@script/check
 
 build: assets
-	go build -trimpath -tags '$(GO_TAGS)' -ldflags '$(strip -s -w -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
+	go build -trimpath -ldflags '$(strip -s -w -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
 
 run: build
 	./bin/webhooker

README.md

@@ -3419,14 +3419,12 @@ fixed independently of the compiler's:
    phases and the `css-check` and `markdown-check` stages passing (it copies a
    file from each), runs `make build` (which extracts Alpine.js from `3p/`
    first), and then rebuilds the binary with `CGO_ENABLED=1` and static linking
-   so it runs on musl, with the build tags `netgo`, `osusergo` and
-   `sqlite_omit_load_extension` so it needs none of glibc's shared libraries at
-   run time. Both builds go through `make build`, the relink adding its
-   `-extldflags` via `GO_LDFLAGS` and its tags via `GO_TAGS`, so neither can
-   drop the `-X` that stamps the version. The version is the `VERSION` build arg
-   if one is given, otherwise derived from the `.git` in the context, and the
-   stage fails if a context with `.git` would stamp an empty version, `dev` or
-   `unknown` (see [Version stamping](#version-stamping)).
+   so it runs on musl. Both builds go through `make build`, the relink adding
+   its `-extldflags` via `GO_LDFLAGS`, so neither can drop the `-X` that stamps
+   the version. The version is the `VERSION` build arg if one is given,
+   otherwise derived from the `.git` in the context, and the stage fails if a
+   context with `.git` would stamp an empty version, `dev` or `unknown` (see
+   [Version stamping](#version-stamping)).
 7. **Runtime stage** (`alpine:3.21`) — copies the static binary and
    `deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` directory for
    all SQLite databases, exposes port 8080, and includes a health check against

internal/database/model_event.go

@@ -45,7 +45,8 @@ type Event struct {
 
 	// ResubmittedFromID names the event this one was copied from by
 	// an operator resubmit. It is nil for an event that arrived on
-	// the receiver. It is not a foreign key: the source event can be
+	// the receiver, which is every event created before the column
+	// existed. It is not a foreign key: the source event can be
 	// reaped by retention while its copies remain, and the id is
 	// kept as the record of where the copy came from either way.
 	ResubmittedFromID *string `gorm:"type:uuid;index:idx_events_resubmitted_from_id,priority:1;index:idx_events_entrypoint_id,priority:3" json:"resubmittedFromId,omitempty"`

internal/database/model_webhook.go

@@ -82,7 +82,7 @@ type Webhook struct {
 // value while building the insert statement, which runs after
 // BeforeSave; rewriting any later than this loses that race and the
 // row lands at 30 days. Living on the model also means a future call
-// site, such as a REST API or a fixture, cannot bypass it.
+// site — a REST API, a fixture, a migration — cannot bypass it.
 func (w *Webhook) BeforeSave(_ *gorm.DB) error {
 	if w.RetentionDays <= 0 {
 		w.RetentionDays = RetentionForeverDays
@@ -98,8 +98,8 @@ func (w *Webhook) BeforeSave(_ *gorm.DB) error {
 // are exempt from reaping.
 //
 // It accepts the RetentionForeverDays sentinel written by BeforeSave
-// and, defensively, any non-positive value, which BeforeSave treats
-// as a request to retain forever.
+// and, defensively, the non-positive values that rows written before
+// the sentinel existed may still carry.
 func retainsForever(retentionDays int) bool {
 	return retentionDays <= 0 ||
 		retentionDays >= RetentionForeverDays

internal/database/retention.go

@@ -253,7 +253,7 @@ func (r *RetentionReaper) reapWebhook(
 // row in the database: the sweep then deletes every event, delivery,
 // and delivery result, including ones created seconds ago. Rejecting
 // out-of-range input at the form is the primary guard; saturating here
-// means a value that reaches the row by any other path cannot turn a
+// means an old row, a migration, or a future call site cannot turn a
 // too-large retention into total data loss.
 func retentionCutoff(
 	now time.Time,

internal/database/sqlite_open.go

@@ -118,8 +118,9 @@ const SQLiteFilePerm fs.FileMode = 0o600
 // SQLite still reports the absence rather than this function
 // materializing an empty database the caller asked not to create.
 //
-// The mode is set on every open, whoever created the file, so a
-// database or sidecar found at a wider mode is tightened too.
+// Chmod of a file that already exists is what tightens a data
+// directory an earlier build left at 0644 — including a developer's
+// own scratch directory — without any migration machinery.
 func reserveSQLiteFile(path string, create bool) error {
 	if create {
 		// gosec G304: the path is the database file the caller asked

internal/delivery/archive_sweeper.go

@@ -201,7 +201,8 @@ func (s *ArchiveSweeper) sweep(ctx context.Context) {
 
 // sweepTarget prunes the archive of a single database target.
 // A missing, empty, or "never" expiry parses as a zero duration
-// and is skipped entirely, so those archives are never pruned.
+// and is skipped entirely, so those archives keep exactly the
+// behaviour they had before the sweep existed.
 func (s *ArchiveSweeper) sweepTarget(target *database.Target) {
 	expiry, err := parseArchiveExpiry(target.Config)
 	if err != nil {

internal/delivery/target_headers.go

@@ -82,8 +82,8 @@ func isReservedTargetHeader(name string) bool {
 // one pair silently overwriting the other.
 //
 // An input with no pairs yields an empty map, which omitempty drops
-// from the stored config, so a target configured with no headers
-// stores no headers key at all.
+// from the stored config: a target configured with no headers keeps
+// the same config JSON it had before this field existed.
 func ParseTargetHeaders(raw string) (map[string]string, error) {
 	headers := make(map[string]string)
 

internal/delivery/target_redact.go

@@ -13,9 +13,9 @@ import (
 const RedactionMarker = "(redacted)"
 
 // Redactor removes one target's own credential material from
-// a delivery response body, whose text the target's remote
-// peer chose, and from a stored delivery error as a second
-// line behind the delivery path's own URL masking.
+// text that target's remote peer chose: a delivery response
+// body, or a delivery error stored before the delivery path
+// learned to mask the URLs it embeds.
 //
 // It removes byte-identical echoes of strings taken from the
 // target's stored configuration, and nothing else. Anything

internal/handlers/delivery_result_view.go

@@ -12,12 +12,13 @@ import (
 //
 // The bound is enforced here and in SQL, because this page's
 // memory profile must not depend on a constant in another
-// package staying where it is, or on every stored row having
-// been written by the delivery engine under its own cap.
+// package staying where it is, and because rows predating the
+// delivery engine's own cap or restored from an archive are
+// not covered by it at all.
 //
 // It happens to equal that engine cap, so a row written by the
-// engine reaches this one exactly and is never cut twice.
-// Nothing here may assume the two differ: see view.
+// current engine reaches this one exactly and is never cut
+// twice. Nothing here may assume the two differ: see view.
 const maxRenderedResponseBytes = 4096
 
 // deliveryResultColumns is the delivery attempt projection.
@@ -39,8 +40,9 @@ const deliveryResultColumns = "delivery_id, attempt_num, created_at, " +
 //
 // Both Error and ResponseBody have been through the target's
 // Redactor. The engine already masks the URL out of the
-// errors it stores, so for errors this is a second line; for
-// response bodies it is the only line, and its reach is what
+// errors it stores, so for errors this is a second line
+// covering rows written before it did; for response bodies it
+// is the only line, and its reach is what
 // delivery.Redactor documents.
 type DeliveryResultView struct {
 	AttemptNum int

internal/handlers/target_retries.go

@@ -49,6 +49,13 @@ var (
 // working retry configuration over a typo. A default answers a
 // question that was not asked; it never answers one that was asked
 // badly.
+//
+// A target stored with a count above the ceiling before this
+// validation existed keeps rendering and keeps delivering — nothing
+// clamps the row. Re-saving it from the edit form does have to bring
+// it into range, because the form submits the pre-filled value back
+// and accepting it would be the ceiling not applying to the edit
+// path. The 400 names the ceiling, so the fix is one field.
 func parseMaxRetries(raw string, fallback int) (int, error) {
 	raw = strings.TrimSpace(raw)
 	if raw == "" {

internal/session/session.go

@@ -441,9 +441,10 @@ func (s *Session) Regenerate(
 //
 // Whichever comes first ends the session.
 //
-// A session that claims to be authenticated but lacks the timestamp
-// a deadline is measured from is treated as expired, so the user
-// re-authenticates rather than being granted an unbounded session.
+// A session that claims to be authenticated but carries no
+// timestamps predates this check; it is treated as expired so the
+// user re-authenticates rather than being granted an unbounded
+// session.
 func (s *Session) expired(sess *sessions.Session) bool {
 	now := s.now()
 

internal/versionscript/version_script_test.go

@@ -156,8 +156,7 @@ func TestVersion_IsStableAcrossInvocations(t *testing.T) {
 // The build target is the only place composing linker flags. If a
 // future edit drops either half, the binary silently reports "dev"
 // again (the defect this package exists for) or fails to link on
-// Alpine. If it stops passing GO_TAGS, the image's static binary links
-// glibc's getaddrinfo and dlopen again.
+// Alpine.
 func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
 	t.Parallel()
 
@@ -165,7 +164,6 @@ func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
 
 	require.Contains(t, makefile, "-X main.version=$(VERSION)")
 	require.Contains(t, makefile, "$(GO_LDFLAGS)")
-	require.Contains(t, makefile, "-tags '$(GO_TAGS)'")
 	require.Contains(t, makefile, "VERSION ?= $(shell script/version)")
 }
 
@@ -235,9 +233,7 @@ func TestDockerfile_BuildsThroughTheMakeTarget(t *testing.T) {
 		"a raw go build bypasses the Makefile's -X flag")
 	require.Contains(t, dockerfile, "ARG VERSION")
 	require.Contains(t, dockerfile,
-		`make build VERSION="$VERSION"`+
-			` GO_TAGS=netgo,osusergo,sqlite_omit_load_extension`+
-			` GO_LDFLAGS='-extldflags "-static"'`)
+		`make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'`)
 }
 
 func read(t *testing.T, path string) string {