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.

A test opens two databases in a directory whose name holds #, ? and %,
and checks through PRAGMA database_list that each opened its own file.

Model: opus-5-5

README.md

@@ -465,7 +465,10 @@ idle timestamp is rewritten at most once per tenth of the idle window rather
 than on every request, which means a session may expire up to 10% early relative
 to the user's true last request, but never late.
 
-Both clocks are anchored by timestamps stored in the session cookie.
+Both clocks are anchored by timestamps stored in the session cookie. Sessions
+issued before this feature existed carry neither, so they are treated as
+expired: upgrading to a build that has it logs every existing session out once,
+and those users sign in again.
 
 #### Invalid values abort startup
 
@@ -699,11 +702,11 @@ app as that user.
 `webhooker.db` holds target configuration in plaintext — bearer tokens, API
 keys, Slack webhook URLs — along with the session encryption key, so webhooker
 creates every SQLite file it owns `0600`: each database and both of its `-wal`
-and `-shm` sidecars, across all three tiers. Any of them found at another mode
+and `-shm` sidecars, across all three tiers. Files an earlier build left `0644`
-is set to `0600` when the database is opened. The directory's `0750` is defence
+are tightened when they are opened. The directory's `0750` is defence in depth —
-in depth — it stops other local users listing the directory and learning your
+it stops other local users listing the directory and learning your webhook UUIDs
-webhook UUIDs from the `events-{uuid}.db` filenames — not the barrier protecting
+from the `events-{uuid}.db` filenames — not the barrier protecting the
-the credentials.
+credentials.
 
 ### Running under upaas
 
@@ -915,7 +918,7 @@ replaces the `max-age` it stored, so to end the policy sooner, set a short
 `HSTS_MAX_AGE` such as `1s` and keep it until returning browsers have picked it
 up, or have a reverse proxy send `max-age=0`.
 
-## Backup and Restore
+## Backup, Restore, and Upgrades
 
 ### What to back up
 
@@ -1068,14 +1071,60 @@ beside it, or wait until there are none.
 
 4. Start the service. The container gives the directory and the restored files
    to the `webhooker` user before the app starts, whoever restored them (see
-   [Running with Docker](#running-with-docker)).
+   [Running with Docker](#running-with-docker)). Each restored database gets any
+   of its SQL migrations it has not had as it is opened (see
+   [Upgrades](#upgrades)).
+
+### Upgrades
+
+**Back up before every upgrade.** Each kind of database file has its schema in
+SQL files built into the binary, one directory per kind under
+`internal/db/migrations/`. In each, `000_migration.sql` creates the
+`schema_migrations` table, which records the number of every file the database
+has had, and `001_schema.sql` holds the whole schema. Opening a database runs,
+in one transaction, every file in its directory that `schema_migrations` does
+not list:
+
+- `main/` for the main database, on connect — settings, users, API keys,
+  webhooks, entrypoints and targets
+- `events/` for each event database, when it is lazily opened — events,
+  deliveries, delivery results and their running totals
+- `archive/` for each archive database, on every open and reopen
+
+There are no down migrations.
+
+Upgrade procedure:
+
+1. Stop the service.
+2. Back up `DATA_DIR` using one of the procedures above.
+3. Pull the new image and start it.
+4. Confirm `database migrations completed` in the logs before putting traffic
+   back on it.
+5. Confirm the new build is the one running:
+   `curl -s http://host:8080/.well-known/healthcheck` reports the version it was
+   stamped with (see [Version stamping](#version-stamping)).
+
+**Upgrading past the introduction of `BIND_ADDRESS`:** earlier versions always
+bound every interface. **Container deployments are unaffected** — the image
+ships `ENV BIND_ADDRESS=0.0.0.0`, so a `docker run` or Compose service that
+worked before still works with nothing changed.
+
+A **bare binary** is the case that changes: the listener now binds `127.0.0.1`
+unless `BIND_ADDRESS` says otherwise, so a deployment that relied on reaching it
+from another host becomes unreachable until it sets the address the proxy
+connects to. Check the `bindAddress` field of the startup log to see what a
+running process bound. See [Bind address](#bind-address).
+
+**Downgrade is unsupported.** Once a newer binary has migrated the files there
+is no way to move them back. The only supported way back to an older version is
+restoring the pre-upgrade backup, which discards everything received since that
+backup was taken.
 
 ### Version stamping
 
 The binary reports its version at `/.well-known/healthcheck` (the `version`
 field), in the UI footer, and in the startup log line (`msg=starting`,
 `version=...`). It is also the Sentry release name, as `webhooker-{version}`.
-`curl -s http://host:8080/.well-known/healthcheck` shows which build is running.
 The value is stamped in at build time by the linker; it is not read from a file
 at runtime, so it identifies the build itself.
 
@@ -1534,7 +1583,8 @@ The cap is not arbitrary: the reaper computes its cutoff as a `time.Duration`,
 an int64 nanosecond count, and a longer period overflows it. An overflowed
 cutoff lands in the future, where it matches every row, so the sweep would
 delete every event the webhook has instead of none. The reaper also clamps the
-value it reads to the cap, so no stored value can trigger that either.
+value it is given, so a row written by an older version cannot trigger that
+either.
 
 #### Entrypoint
 
@@ -1917,7 +1967,7 @@ and application state:
 - **APIKeys** — programmatic access credentials
 
 On first startup the main database is created from its SQL files (see
-[Schema](#schema)), a session encryption key is generated and stored, and an
+[Upgrades](#upgrades)), a session encryption key is generated and stored, and an
 `admin` user is created.
 
 **Per-Webhook Event Databases** (`{DATA_DIR}/events-{webhook_uuid}.db`) — each
@@ -2114,21 +2164,6 @@ The database uses the
 though CGO is required at build time due to the transitive `mattn/go-sqlite3`
 dependency from `gorm.io/driver/sqlite`.
 
-#### Schema
-
-Each kind of database file has its schema in SQL files built into the binary,
-one directory per kind under `internal/db/migrations/`. In each,
-`000_migration.sql` creates the `schema_migrations` table, which records the
-number of every file the database has had, and `001_schema.sql` holds the whole
-schema. Opening a database runs, in one transaction, every file in its directory
-that `schema_migrations` does not list:
-
-- `main/` for the main database, on connect — settings, users, API keys,
-  webhooks, entrypoints and targets
-- `events/` for each event database, when it is lazily opened — events,
-  deliveries, delivery results and their running totals
-- `archive/` for each archive database, on every open and reopen
-
 ### Request Flow
 
 ```

internal/database/sqlite_open_test.go

@@ -2,7 +2,6 @@ package database_test
 
 import (
 	"context"
-	"database/sql"
 	"os"
 	"path/filepath"
 	"strings"
@@ -121,6 +120,7 @@ func TestMainDBAppliesPragmasOnALiveHandle(t *testing.T) {
 func TestOpenSQLiteOpensThePathAskedFor(t *testing.T) {
 	t.Parallel()
 
+	ctx := context.Background()
 	dir := filepath.Join(t.TempDir(), "hooks#1?x%41")
 	require.NoError(t, os.Mkdir(dir, 0o700))
 
@@ -137,56 +137,30 @@ func TestOpenSQLiteOpensThePathAskedFor(t *testing.T) {
 
 		t.Cleanup(func() { assert.NoError(t, sqlDB.Close()) })
 
-		assertOpenedFile(t, sqlDB, path)
+		// database_list names the file SQLite actually opened, with
+		// symlinks resolved, so the files are compared, not the names.
+		var (
+			seq          int
+			schema, file string
+		)
+
+		require.NoError(t, sqlDB.
+			QueryRowContext(ctx, "pragma database_list").
+			Scan(&seq, &schema, &file))
+
+		asked, err := os.Stat(path)
+		require.NoError(t, err)
+
+		opened, err := os.Stat(file)
+		require.NoError(t, err)
+
+		assert.True(
+			t, os.SameFile(asked, opened),
+			"asked for %s, SQLite opened %s", path, file,
+		)
 	}
 }
 
-// TestOpenSQLiteOpensARelativePath opens a database at a path relative
-// to the working directory, which is what a relative DATA_DIR such as
-// the README's DATA_DIR=../webhooker-data gives OpenSQLite.
-//
-//nolint:paralleltest // t.Chdir moves the whole process.
-func TestOpenSQLiteOpensARelativePath(t *testing.T) {
-	t.Chdir(t.TempDir())
-	require.NoError(t, os.Mkdir("data", 0o700))
-
-	path := filepath.Join("data", database.MainDBFileName)
-
-	sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
-	require.NoError(t, err)
-
-	defer func() { require.NoError(t, sqlDB.Close()) }()
-
-	assertOpenedFile(t, sqlDB, path)
-}
-
-// assertOpenedFile checks that sqlDB is the SQLite file at path.
-// database_list names the file SQLite actually opened, as an absolute
-// path with symlinks resolved, so the files are compared, not the names.
-func assertOpenedFile(t *testing.T, sqlDB *sql.DB, path string) {
-	t.Helper()
-
-	var (
-		seq          int
-		schema, file string
-	)
-
-	require.NoError(t, sqlDB.
-		QueryRowContext(t.Context(), "pragma database_list").
-		Scan(&seq, &schema, &file))
-
-	asked, err := os.Stat(path)
-	require.NoError(t, err)
-
-	opened, err := os.Stat(file)
-	require.NoError(t, err)
-
-	assert.True(
-		t, os.SameFile(asked, opened),
-		"asked for %s, SQLite opened %s", path, file,
-	)
-}
-
 // TestConcurrentReaderDoesNotBlockWrites is the unit-scale form of the
 // reproduction in
 // https://git.eeqj.de/sneak/webhooker/issues/256: an operator's

internal/handlers/delivery_attempts_test.go

@@ -42,6 +42,9 @@ func TestDeliveryAttempts_ReadInTheTargetTypesOwnTerms(t *testing.T) {
 		{database.TargetTypeLog, true, 0, "", "written to the log", ""},
 	}
 
+	// Named for type and outcome, so that no two cases share a name:
+	// Go adds "#01" to a repeated one, and a "#" in the temporary
+	// directory's path cuts the path SQLiteDSN gives SQLite short.
 	for _, tc := range cases {
 		t.Run(string(tc.targetType)+" "+tc.outcome, func(t *testing.T) {
 			t.Parallel()