Stop allowing credentialed cross-origin requests (closes #261)

CORS() allowed every origin with credentials, so a page on another origin of the same site could read what the signed-in user sees.

CORS() now uses the CONVENTIONS.md template's options (AllowedOrigins "*", AllowCredentials false) and covers only the two public, read-only routes, GET /api/v1/server and GET /.well-known/healthcheck.json. Every other route sends no Access-Control-Allow-* header. A preflight to the public routes gets 405 like any other path.

Deviation: CORS() is not in the global middleware chain CONVENTIONS.md lists, because REPO_POLICIES.md forbids * on authenticated routes.

Model: opus-5-5

README.md

@@ -168,8 +168,10 @@ either.
 - Request bodies for `POST /api/v1/session`, `POST /api/v1/login` and
   `POST /api/v1/messages` are limited to `MAX_MESSAGE_SIZE` bytes (default
   `4096`). A larger body gets `400` `invalid request body`.
-- The server allows cross-origin requests from any origin, with credentials. The
+- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
-  cookie is `SameSite=Strict`, so a browser sends it only from the same site.
+  from any origin with `Access-Control-Allow-Origin: *`. Every other route sends
+  no `Access-Control-Allow-*` header, so a page on another origin cannot read
+  its responses.
 
 ### The auth cookie
 
@@ -1893,14 +1895,14 @@ The IRC listener has no TLS.
   seconds. A request still running after 60 seconds is cancelled, and the
   connection closes without a response. Headers may be up to 1 MiB.
 - A long poll waits at most 30 seconds.
-- Every response to a well-formed request, except to a cross-origin preflight,
+- Every response to a well-formed request carries a `Content-Security-Policy`
-  carries a `Content-Security-Policy` header:
+  header:
   `default-src 'self'; script-src 'self'; style-src 'self'; connect-src 'self'; img-src 'self'; font-src 'self'; object-src 'none'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'`.
   The web client works within it.
-- Cross-origin requests are allowed from any origin, with credentials. The
+- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
-  allowed methods are `GET`, `POST`, `PUT`, `DELETE` and `OPTIONS`. The allowed
+  from any origin with `Access-Control-Allow-Origin: *`. Every other route sends
-  request headers are `Accept`, `Content-Type` and `X-CSRF-Token`. The cookie's
+  no `Access-Control-Allow-*` header, so a page on another origin cannot read
-  `SameSite=Strict` still keeps it to the same site.
+  its responses.
 
 ### Logging
 

internal/config/config.go

@@ -36,9 +36,9 @@ Type /help for available commands.`
 const DefaultDBURL = "file:///var/lib/neoirc/state.db" +
 	"?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)"
 
-// defaultTrustedProxies is TRUSTED_PROXIES when it is not set: the
+// DefaultTrustedProxies is TRUSTED_PROXIES when it is not set: the
 // private address ranges of RFC 1918.
-const defaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
+const DefaultTrustedProxies = "10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"
 
 var errOutOfRange = errors.New("setting out of range")
 
@@ -117,7 +117,7 @@ func New(
 	viper.SetDefault("LOGIN_RATE_LIMIT", "1")
 	viper.SetDefault("LOGIN_RATE_BURST", "5")
 	viper.SetDefault("IRC_LISTEN_ADDR", ":6667")
-	viper.SetDefault("TRUSTED_PROXIES", defaultTrustedProxies)
+	viper.SetDefault("TRUSTED_PROXIES", DefaultTrustedProxies)
 
 	err := viper.ReadInConfig()
 	if err != nil {
@@ -219,9 +219,7 @@ func parseStringAllowEmptyEnv(key string, value *string) error {
 	return parseString(key, value)
 }
 
-// parseTrustedProxies sets *value to the addresses in TRUSTED_PROXIES, a
+// parseTrustedProxies sets *value to the addresses in TRUSTED_PROXIES.
-// comma-separated list of networks such as 10.0.0.0/8 and single
-// addresses such as 192.0.2.1. The empty string trusts nothing.
 func parseTrustedProxies(value **netipx.IPSet) error {
 	var raw string
 
@@ -230,18 +228,7 @@ func parseTrustedProxies(value **netipx.IPSet) error {
 		return err
 	}
 
-	var builder netipx.IPSetBuilder
+	ipSet, err := ParseTrustedProxies(raw)
-
-	if strings.TrimSpace(raw) != "" {
-		for entry := range strings.SplitSeq(raw, ",") {
-			err = addTrustedProxy(&builder, strings.TrimSpace(entry))
-			if err != nil {
-				return fmt.Errorf("cannot parse TRUSTED_PROXIES: %w", err)
-			}
-		}
-	}
-
-	ipSet, err := builder.IPSet()
 	if err != nil {
 		return fmt.Errorf("cannot parse TRUSTED_PROXIES: %w", err)
 	}
@@ -251,6 +238,25 @@ func parseTrustedProxies(value **netipx.IPSet) error {
 	return nil
 }
 
+// ParseTrustedProxies returns the addresses in list, which has the form
+// of TRUSTED_PROXIES: a comma-separated list of networks such as
+// 10.0.0.0/8 and single addresses such as 192.0.2.1. The empty string
+// trusts nothing.
+func ParseTrustedProxies(list string) (*netipx.IPSet, error) {
+	var builder netipx.IPSetBuilder
+
+	if strings.TrimSpace(list) != "" {
+		for entry := range strings.SplitSeq(list, ",") {
+			err := addTrustedProxy(&builder, strings.TrimSpace(entry))
+			if err != nil {
+				return nil, err
+			}
+		}
+	}
+
+	return builder.IPSet()
+}
+
 // addTrustedProxy adds entry, a network with a slash or a single
 // address, to builder. An IPv4 address written in IPv6 form, such as
 // ::ffff:192.0.2.1, is added as IPv4. A network in that form and an

internal/db/auth.go

@@ -22,7 +22,7 @@ func (database *Database) SetPassword(
 	password string,
 ) error {
 	hash, err := bcrypt.GenerateFromPassword(
-		[]byte(password), bcrypt.DefaultCost,
+		[]byte(password), database.passwordCost,
 	)
 	if err != nil {
 		return fmt.Errorf("hash password: %w", err)

internal/db/db.go

@@ -15,6 +15,7 @@ import (
 	"strings"
 
 	"go.uber.org/fx"
+	"golang.org/x/crypto/bcrypt"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/logger"
 
@@ -39,6 +40,11 @@ type Params struct {
 
 	Logger *logger.Logger
 	Config *config.Config
+
+	// PasswordCost is the bcrypt cost at which SetPassword hashes
+	// a password. The server leaves it unset, which means
+	// bcrypt.DefaultCost. Tests set a lower cost to save time.
+	PasswordCost int `optional:"true"`
 }
 
 // Database manages the SQLite connection and migrations.
@@ -46,6 +52,7 @@ type Database struct {
 	conn         *sql.DB
 	log          *slog.Logger
 	params       *Params
+	passwordCost int
 }
 
 // New creates a new Database and registers lifecycle hooks.
@@ -53,9 +60,15 @@ func New(
 	lifecycle fx.Lifecycle,
 	params Params,
 ) (*Database, error) {
+	passwordCost := params.PasswordCost
+	if passwordCost == 0 {
+		passwordCost = bcrypt.DefaultCost
+	}
+
 	database := &Database{ //nolint:exhaustruct // conn set in OnStart
 		params:       &params,
 		log:          params.Logger.Get(),
+		passwordCost: passwordCost,
 	}
 
 	database.log.Info("Database instantiated")

internal/db/db_test.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"go.uber.org/fx/fxtest"
+	"golang.org/x/crypto/bcrypt"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
 	"sneak.berlin/go/neoirc/internal/dbtest"
@@ -26,7 +27,11 @@ const (
 		(SELECT 1 UNION ALL SELECT x + 1 FROM n)
 		SELECT COUNT(*) FROM n`
 
-	endlessQueryTimeout = 100 * time.Millisecond
+	// endlessQueryTimeout is how long the endless query runs
+	// before its context ends. A query whose context ends before
+	// it starts is never run, and the pool keeps its connection,
+	// so the timeout leaves a loaded host time to start it.
+	endlessQueryTimeout = 2 * time.Second
 )
 
 // TestDefaultDBURLSetsWALAndBusyTimeout opens a file database
@@ -236,6 +241,85 @@ func TestDBURLSettingForeignKeysFailsStartup(t *testing.T) {
 	}
 }
 
+// TestPasswordCost checks the bcrypt cost of the hash that SetPassword
+// stores: bcrypt.DefaultCost when db.Params leaves PasswordCost unset,
+// as the server does, and PasswordCost when it is set.
+func TestPasswordCost(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name         string
+		passwordCost int
+		wantCost     int
+	}{
+		{name: "unset", wantCost: bcrypt.DefaultCost},
+		{
+			name:         "bcrypt.MinCost",
+			passwordCost: bcrypt.MinCost,
+			wantCost:     bcrypt.MinCost,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			lifecycle := fxtest.NewLifecycle(t)
+
+			log, err := logger.New(lifecycle, logger.Params{
+				Globals: &globals.Globals{},
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			database, err := db.New(lifecycle, db.Params{
+				Logger:       log,
+				Config:       &config.Config{DBURL: dbtest.NewURL(t)},
+				PasswordCost: test.passwordCost,
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			lifecycle.RequireStart()
+			t.Cleanup(lifecycle.RequireStop)
+
+			sessionID, _, _, err := database.CreateSession(
+				t.Context(), "costuser", "", "", "",
+			)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			err = database.SetPassword(t.Context(), sessionID, "password123")
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var hash string
+
+			err = database.GetDB().QueryRowContext(
+				t.Context(),
+				"SELECT password_hash FROM sessions WHERE id = ?",
+				sessionID,
+			).Scan(&hash)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			cost, err := bcrypt.Cost([]byte(hash))
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if cost != test.wantCost {
+				t.Errorf("cost = %d, want %d", cost, test.wantCost)
+			}
+		})
+	}
+}
+
 // startDatabase starts a Database for dbURL the way the server does.
 // It returns the error that stopped it from starting, if any.
 func startDatabase(t *testing.T, dbURL string) (*db.Database, error) {

internal/db/export_test.go

@@ -8,10 +8,12 @@ import (
 	"testing"
 	"time"
 
+	"golang.org/x/crypto/bcrypt"
 	"sneak.berlin/go/neoirc/internal/dbtest"
 )
 
-// NewTestDatabase creates an in-memory database for testing.
+// NewTestDatabase creates an in-memory database for testing. It
+// hashes passwords at bcrypt.MinCost.
 func NewTestDatabase(tb testing.TB) (*Database, error) {
 	tb.Helper()
 
@@ -25,6 +27,7 @@ func NewTestDatabase(tb testing.TB) (*Database, error) {
 	database := &Database{ //nolint:exhaustruct // test helper, params not needed
 		conn:         conn,
 		log:          slog.Default(),
+		passwordCost: bcrypt.MinCost,
 	}
 
 	err = database.runMigrations(context.Background())

internal/handlers/api_test.go

@@ -145,34 +145,46 @@ type testServer struct {
 	handlers   *handlers.Handlers
 }
 
-// viperMu makes tests start their servers one at a time,
+const (
-// because config.New writes the global viper instance and
+	// maxMessageSize is MAX_MESSAGE_SIZE on every test server, the
-// the server reads it as it starts.
+	// setting's default.
-//
+	maxMessageSize = 4096
-//nolint:gochecknoglobals // guards a global, so must be one
+
-var viperMu sync.Mutex
+	// loginRateLimit is LOGIN_RATE_LIMIT on every test server, the
+	// setting's default.
+	loginRateLimit = 1
+
+	// loginRateBurst is LOGIN_RATE_BURST on every test server, the
+	// setting's default.
+	loginRateBurst = 5
+)
 
 func newTestServer(
 	t *testing.T,
 ) *testServer {
 	t.Helper()
 
-	return newTestServerWith(t, 0, "", "")
+	return newTestServerWith(t, 0, "", "", config.DefaultTrustedProxies)
 }
 
 func newTestServerWith(
 	t *testing.T,
 	hashcashBits int,
-	operName, operPassword string,
+	operName, operPassword, trustedProxies string,
 ) *testServer {
 	t.Helper()
 
-	viperMu.Lock()
-	defer viperMu.Unlock()
-
 	dbURL := dbtest.NewURL(t)
 	copyTemplateDB(t, dbURL)
 
+	trusted, err := config.ParseTrustedProxies(trustedProxies)
+	if err != nil {
+		t.Fatalf("parse trusted proxies: %v", err)
+	}
+
+	// Each duration setting defaults to 30 days.
+	const thirtyDays = "720h"
+
 	var (
 		srv      *server.Server
 		brk      *broker.Broker
@@ -184,31 +196,25 @@ func newTestServerWith(
 		fx.Provide(
 			newTestGlobals,
 			logger.New,
-			func(
+			// Built here rather than by config.New, which writes
-				lifecycle fx.Lifecycle,
+			// the global viper instance, so servers start in
-				globs *globals.Globals,
+			// parallel.
-				log *logger.Logger,
+			func() *config.Config {
-			) (*config.Config, error) {
+				return &config.Config{
-				cfg, err := config.New(
+					DBURL: dbURL,
-					lifecycle, config.Params{ //nolint:exhaustruct
-						Globals: globs, Logger: log,
-					},
-				)
-				if err != nil {
-					return nil, fmt.Errorf(
-						"test config: %w", err,
-					)
-				}
-
-				cfg.DBURL = dbURL
-				cfg.Port = 0
 					// One line: each line is a database write per new session.
-				cfg.MOTD = "neoirc test server"
+					MOTD:               "neoirc test server",
-				cfg.HashcashBits = hashcashBits
+					MaxMessageSize:     maxMessageSize,
-				cfg.OperName = operName
+					HashcashBits:       hashcashBits,
-				cfg.OperPassword = operPassword
+					OperName:           operName,
-
+					OperPassword:       operPassword,
-				return cfg, nil
+					MessageMaxAge:      thirtyDays,
+					QueueMaxAge:        thirtyDays,
+					SessionIdleTimeout: thirtyDays,
+					LoginRateLimit:     loginRateLimit,
+					LoginRateBurst:     loginRateBurst,
+					TrustedProxies:     trusted,
+				}
 			},
 			newTestDB,
 			broker.New,
@@ -255,7 +261,7 @@ func newTestDB(
 	cfg *config.Config,
 ) (*db.Database, error) {
 	database, err := db.New(lifecycle, db.Params{ //nolint:exhaustruct
-		Logger: log, Config: cfg,
+		Logger: log, Config: cfg, PasswordCost: bcrypt.MinCost,
 	})
 	if err != nil {
 		return nil, fmt.Errorf("test db: %w", err)
@@ -830,10 +836,7 @@ func (tserver *testServer) postBody(
 func TestRequestBodyLimit(t *testing.T) {
 	t.Parallel()
 
-	const (
+	const nick = "alice_bodylimit"
-		maxMessageSize = 4096 // the default MAX_MESSAGE_SIZE
-		nick           = "alice_bodylimit"
-	)
 
 	endpoints := []struct {
 		name, path, value string
@@ -1750,18 +1753,34 @@ func TestConcurrentQuitsSendOneQuit(t *testing.T) {
 // that the second session then holds one QUIT from the
 // first, and that the first session is deleted: its tokens
 // are refused and its nick is free. It does all this
-// quitRaceTrials times, each on a new server, because each
+// quitRaceTrials times, at once, each on a new server, because
-// trial logs in and logins are rate limited.
+// each trial logs in and logins are rate limited.
 func checkQuitRace(
 	t *testing.T,
 	leave func(tserver *testServer, tokens []string),
 ) {
 	t.Helper()
 
+	for trial := range quitRaceTrials {
+		t.Run(fmt.Sprintf("trial %d", trial), func(t *testing.T) {
+			t.Parallel()
+
+			checkQuitRaceOnce(t, newTestServer(t), leave)
+		})
+	}
+}
+
+// checkQuitRaceOnce runs one trial of checkQuitRace on
+// tserver.
+func checkQuitRaceOnce(
+	t *testing.T,
+	tserver *testServer,
+	leave func(tserver *testServer, tokens []string),
+) {
+	t.Helper()
+
 	const nick = "racingquitter"
 
-	for trial := range quitRaceTrials {
-		tserver := newTestServer(t)
 	firstToken := tserver.createSession(nick)
 	observerToken := tserver.createSession("raceobserver")
 
@@ -1783,19 +1802,18 @@ func checkQuitRace(
 
 	quits := countQuits(msgs, nick)
 	if quits != 1 {
-			t.Errorf("trial %d: %d QUITs, want 1: %v", trial, quits, msgs)
+		t.Errorf("%d QUITs, want 1: %v", quits, msgs)
 	}
 
 	for _, token := range tokens {
 		status, _ := tserver.getState(token)
 		if status != http.StatusUnauthorized {
-				t.Errorf("trial %d: state got %d, want 401", trial, status)
+			t.Errorf("state got %d, want 401", status)
 		}
 	}
 
 	// createSession fails the test if the nick is taken.
 	tserver.createSession(nick)
-	}
 }
 
 // TestLogoutOfOneClientKeepsSession checks that when one of
@@ -1933,11 +1951,28 @@ const (
 	// return once the session has left.
 	quitWakeDeadline = 5 * time.Second
 
-	// pollWaitStep is how often checkQuitWakesPoll checks
+	// pollWaitStep is how often waitForLongPoll checks
 	// whether the long poll is waiting yet.
 	pollWaitStep = 10 * time.Millisecond
 )
 
+// waitForLongPoll waits until a long poll of the session
+// sessionID is waiting, and fails the test if none is within
+// quitWakeDeadline.
+func (tserver *testServer) waitForLongPoll(sessionID int64) {
+	tserver.t.Helper()
+
+	deadline := time.Now().Add(quitWakeDeadline)
+
+	for tserver.broker.ListenerCount(sessionID) == 0 {
+		if time.Now().After(deadline) {
+			tserver.t.Fatal("the long poll did not start waiting")
+		}
+
+		time.Sleep(pollWaitStep)
+	}
+}
+
 // TestQuitWakesPeerLongPoll checks that a session's QUIT
 // wakes the long poll of a user who shares a channel with
 // it.
@@ -2012,15 +2047,7 @@ func checkQuitWakesPoll(
 		msgs, pollErr = tserver.longPoll(peerToken, lastID, quitWakePollTimeout)
 	}()
 
-	deadline := time.Now().Add(quitWakeDeadline)
+	tserver.waitForLongPoll(peerID)
-
-	for tserver.broker.ListenerCount(peerID) == 0 {
-		if time.Now().After(deadline) {
-			t.Fatal("the long poll did not start waiting")
-		}
-
-		time.Sleep(pollWaitStep)
-	}
 
 	leave(tserver, token)
 
@@ -2850,7 +2877,12 @@ func TestLongPoll(t *testing.T) {
 		pollMsgs = result.Messages
 	}()
 
-	time.Sleep(200 * time.Millisecond)
+	bobID, err := tserver.database.GetSessionByNick(t.Context(), "lp_bob")
+	if err != nil {
+		t.Fatalf("get session of lp_bob: %v", err)
+	}
+
+	tserver.waitForLongPoll(bobID)
 
 	tserver.sendCommand(aliceToken, map[string]any{
 		commandKey: privmsgCmd,
@@ -3009,18 +3041,18 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 		longTimeout = 10
 
 		// shortTimeout, in seconds, is the timeout of the
-		// requests that wait it out.
+		// request that waits it out.
 		shortTimeout = 1
 	)
 
 	// queued is whether a message is queued before the request,
-	// whose timeout is in seconds. failingRead is as for
+	// whose timeout is in seconds. failingRead and notified are
-	// getMessagesFailingRead. wantReads is how many times the
+	// as for getMessagesFailingRead. wantReads is how many times
-	// handler reads the queue: once if it answers at once,
+	// the handler reads the queue: once if it answers at once,
 	// twice if it waits first.
 	tests := []struct {
 		name                  string
-		queued                bool
+		queued, notified      bool
 		timeout, failingRead  int
 		wantReads, wantStatus int
 	}{
@@ -3041,9 +3073,9 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 			wantReads: 1, wantStatus: http.StatusInternalServerError,
 		},
 		{
-			name: "read after the wait failed", timeout: shortTimeout,
+			name: "read after the wait failed", notified: true,
-			failingRead: 2, wantReads: 2,
+			timeout: handlers.MaxLongPollTimeout, failingRead: 2,
-			wantStatus: http.StatusInternalServerError,
+			wantReads: 2, wantStatus: http.StatusInternalServerError,
 		},
 	}
 
@@ -3075,7 +3107,7 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 			}
 
 			status, reads := tserver.getMessagesFailingRead(
-				token, lastID, tc.timeout, tc.failingRead,
+				token, sessionID, lastID, tc.timeout, tc.failingRead, tc.notified,
 			)
 			if status != tc.wantStatus {
 				t.Errorf("status %d, want %d", status, tc.wantStatus)
@@ -3092,14 +3124,17 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 }
 
 // getMessagesFailingRead sends GET /api/v1/messages for the
-// client token with timeout and after afterID. The handler's
+// client token of the session sessionID, with timeout and
-// read of the queue numbered failingRead, counting from 1,
+// after afterID. The handler's read of the queue numbered
-// fails; with 0, none does. It returns the response status
+// failingRead, counting from 1, fails; with 0, none does.
-// and how many times the handler read the queue.
+// With notified, it notifies the broker once the request
+// waits. It returns the response status and how many times
+// the handler read the queue.
 func (tserver *testServer) getMessagesFailingRead(
 	token string,
-	afterID int64,
+	sessionID, afterID int64,
 	timeout, failingRead int,
+	notified bool,
 ) (int, int) {
 	tserver.t.Helper()
 
@@ -3121,20 +3156,45 @@ func (tserver *testServer) getMessagesFailingRead(
 		)
 	})
 
-	resp, err := doRequestAuth(
+	var (
+		status int
+		err    error
+	)
+
+	answered := make(chan struct{})
+
+	go func() {
+		defer close(answered)
+
+		var resp *http.Response
+
+		resp, err = doRequestAuth(
 			tserver.t, http.MethodGet,
 			tserver.url(fmt.Sprintf(
 				"%s?timeout=%d&after=%d", apiMessages, timeout, afterID,
 			)),
 			token, nil,
 		)
+		if err != nil {
+			return
+		}
+
+		status = resp.StatusCode
+		_ = resp.Body.Close()
+	}()
+
+	if notified {
+		tserver.waitForLongPoll(sessionID)
+		tserver.broker.Notify(sessionID)
+	}
+
+	<-answered
+
 	if err != nil {
 		tserver.t.Fatalf("get messages: %v", err)
 	}
 
-	_ = resp.Body.Close()
+	return status, reads
-
-	return resp.StatusCode, reads
 }
 
 // TestLongPollRemovesWaitChannelWhenClientGivesUp checks that
@@ -4343,12 +4403,11 @@ func TestLoginRateLimitIgnoresForwardedHeaders(t *testing.T) {
 }
 
 func TestLoginRateLimitPerClientBehindTrustedProxy(t *testing.T) {
-	// The test client connects from 127.0.0.1. Trusted as a proxy, it
+	t.Parallel()
-	// passes each client's address in X-Forwarded-For. t.Setenv rules
-	// out t.Parallel.
-	t.Setenv("TRUSTED_PROXIES", "127.0.0.1")
 
-	tserver := newTestServer(t)
+	// The test client connects from 127.0.0.1. Trusted as a proxy, it
+	// passes each client's address in X-Forwarded-For.
+	tserver := newTestServerWith(t, 0, "", "", "127.0.0.1")
 
 	// An empty JSON object is refused after the rate limit counts the
 	// attempt, so no attempt checks a password or looks up a hostname.
@@ -5079,7 +5138,9 @@ func TestRefusedSessionLeavesStampUnspent(t *testing.T) {
 		badNick = "bad nick"
 	)
 
-	tserver := newTestServerWith(t, bits, "", "")
+	tserver := newTestServerWith(
+		t, bits, "", "", config.DefaultTrustedProxies,
+	)
 
 	// A server with no SERVER_NAME takes stamps for "neoirc".
 	status := postSessionWithStamp(
@@ -5285,6 +5346,7 @@ func newTestServerWithOper(
 
 	return newTestServerWith(
 		t, 0, testOperName, testOperPassword,
+		config.DefaultTrustedProxies,
 	)
 }
 

internal/handlers/export_test.go

@@ -6,6 +6,10 @@ import (
 	"sneak.berlin/go/neoirc/internal/db"
 )
 
+// MaxLongPollTimeout is the longest HandleGetMessages waits, in
+// seconds.
+const MaxLongPollTimeout = maxLongPollTimeout
+
 // SetPollMessages makes HandleGetMessages read the client
 // output queue with poll in place of Database.PollMessages.
 func (hdlr *Handlers) SetPollMessages(

internal/ircserver/conn.go

@@ -70,6 +70,10 @@ type Conn struct {
 	closed      bool
 	killed      bool
 
+	// pollInterval is how often the relay checks the client
+	// output queue when no broker notification arrives.
+	pollInterval time.Duration
+
 	// quitReason is the reason in the QUIT that peers are
 	// sent when the connection ends. The client's QUIT
 	// replaces it.
@@ -83,6 +87,7 @@ func newConn(
 	brk *broker.Broker,
 	cfg *config.Config,
 	svc *service.Service,
+	poll time.Duration,
 ) *Conn {
 	host, _, _ := net.SplitHostPort(tcpConn.RemoteAddr().String())
 
@@ -101,6 +106,8 @@ func newConn(
 		serverSfx:  srvName,
 		remoteIP:   host,
 		quitReason: "Connection closed",
+
+		pollInterval: poll,
 	}
 
 	conn.commands = conn.buildCommandMap()

internal/ircserver/export_test.go

@@ -14,16 +14,16 @@ import (
 )
 
 // NewTestServer creates a Server suitable for testing, whose
-// connections use interval and deadline in place of
+// connections use interval, deadline and poll in place of
-// pingInterval and pongDeadline, and resolve in place of
+// pingInterval, pongDeadline and pollInterval, and resolve in
-// resolveHost.
+// place of resolveHost.
 // The caller must call Stop() when finished.
 func NewTestServer(
 	log *slog.Logger,
 	cfg *config.Config,
 	database *db.Database,
 	brk *broker.Broker,
-	interval, deadline time.Duration,
+	interval, deadline, poll time.Duration,
 	resolve func(ctx context.Context, addr string) string,
 ) *Server {
 	globs := &globals.Globals{
@@ -46,10 +46,18 @@ func NewTestServer(
 
 		pingInterval: interval,
 		pongDeadline: deadline,
+		pollInterval: poll,
+		stopTimeout:  stopTimeout,
 		resolveHost:  resolve,
 	}
 }
 
+// SetStopTimeout makes Stop wait at most timeout for the
+// connections' cleanup.
+func (s *Server) SetStopTimeout(timeout time.Duration) {
+	s.stopTimeout = timeout
+}
+
 // ResolveHost is the reverse DNS lookup that a server made by
 // New does for each client.
 func ResolveHost(ctx context.Context, addr string) string {
@@ -99,10 +107,6 @@ const PingInterval = pingInterval
 // for a line before it closes the connection.
 const PongDeadline = pongDeadline
 
-// StopTimeout is the longest Stop waits for the connections'
-// cleanup.
-const StopTimeout = stopTimeout
-
 // NewTestConn returns a Conn for nick on tcpConn, without a
 // server, database or service behind it.
 func NewTestConn(
@@ -111,7 +115,7 @@ func NewTestConn(
 	tcpConn net.Conn,
 	nick string,
 ) *Conn {
-	conn := newConn(tcpConn, log, nil, nil, cfg, nil)
+	conn := newConn(tcpConn, log, nil, nil, cfg, nil, pollInterval)
 	conn.nick = nick
 
 	return conn

internal/ircserver/integration_test.go

@@ -3,7 +3,6 @@ package ircserver_test
 import (
 	"strings"
 	"testing"
-	"time"
 
 	"sneak.berlin/go/neoirc/internal/config"
 )
@@ -1508,9 +1507,6 @@ func TestIntegrationThirdClientObserver(t *testing.T) {
 	bob.joinAndDrain("#trio")
 	carol.joinAndDrain("#trio")
 
-	// Drain join notifications.
-	time.Sleep(100 * time.Millisecond)
-
 	// alice sends; both bob and carol should receive.
 	alice.send("PRIVMSG #trio :hello trio")
 

internal/ircserver/relay.go

@@ -17,7 +17,7 @@ import (
 func (c *Conn) relayMessages(ctx context.Context) {
 	// Use a ticker as a fallback; primary wakeup is via
 	// broker notification.
-	ticker := time.NewTicker(pollInterval)
+	ticker := time.NewTicker(c.pollInterval)
 	defer ticker.Stop()
 
 	for {

internal/ircserver/relay_test.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"golang.org/x/crypto/bcrypt"
+	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/dbtest"
 	"sneak.berlin/go/neoirc/internal/ircserver"
 	"sneak.berlin/go/neoirc/internal/service"
@@ -58,7 +59,11 @@ func (env *testEnv) queueMessage(
 func TestRelayKeepsOneWaitChannel(t *testing.T) {
 	t.Parallel()
 
-	env := newTestEnv(t)
+	env := newTestEnvWithResolver(
+		t, &config.Config{ServerName: testServerName},
+		ircserver.PingInterval, ircserver.PongDeadline,
+		ircserver.PollInterval, ircserver.ResolveHost,
+	)
 
 	client := env.dial(t)
 	client.register("idle")
@@ -119,6 +124,63 @@ func (env *testEnv) checkOneWaitChannel(
 	}
 }
 
+// TestRelayDeliversAfterFailedRead checks that when the relay
+// cannot read the client output queue, it waits for the next
+// notification, and delivers once the queue can be read again.
+func TestRelayDeliversAfterFailedRead(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	client := env.dial(t)
+	client.register("reader")
+
+	sessionID, err := env.database.GetSessionByNick(t.Context(), "reader")
+	if err != nil {
+		t.Fatalf("get session of reader: %v", err)
+	}
+
+	dbtest.Exec(t, env.dbURL,
+		"ALTER TABLE client_queues RENAME TO hidden_queues")
+
+	// Each pass of the relay takes a wait channel and then reads
+	// the queue. Once it holds a channel after the second
+	// notification, the read after the first one has failed.
+	for range 2 {
+		env.waitForWaitChannel(t, sessionID)
+		env.brk.Notify(sessionID)
+	}
+
+	env.waitForWaitChannel(t, sessionID)
+
+	dbtest.Exec(t, env.dbURL,
+		"ALTER TABLE hidden_queues RENAME TO client_queues")
+
+	env.queueMessage(
+		t, "reader", "PRIVMSG", "carol", "reader", []string{"readable"},
+	)
+
+	client.readUntil(func(l string) bool {
+		return strings.Contains(l, "readable")
+	})
+}
+
+// waitForWaitChannel waits until the session sessionID holds a
+// broker wait channel, and fails the test if it does not
+// within testTimeout.
+func (env *testEnv) waitForWaitChannel(t *testing.T, sessionID int64) {
+	t.Helper()
+
+	deadline := time.Now().Add(testTimeout)
+
+	for env.brk.ListenerCount(sessionID) == 0 {
+		if time.Now().After(deadline) {
+			t.Fatal("the relay is not waiting for a notification")
+		}
+
+		time.Sleep(relayCheckStep)
+	}
+}
+
 // TestRelayMultiLineBody checks that each line of a PRIVMSG
 // or NOTICE body sent over the HTTP API reaches an IRC
 // listener user as its own line.

internal/ircserver/server.go

@@ -53,6 +53,11 @@ type Server struct {
 	pingInterval time.Duration
 	pongDeadline time.Duration
 
+	// pollInterval and stopTimeout are the constants of the
+	// same names, except in tests, which change them.
+	pollInterval time.Duration
+	stopTimeout  time.Duration
+
 	// resolveHost is the function of the same name, except in
 	// tests, which replace it with one that blocks.
 	resolveHost func(ctx context.Context, addr string) string
@@ -79,6 +84,8 @@ func New(
 
 		pingInterval: pingInterval,
 		pongDeadline: pongDeadline,
+		pollInterval: pollInterval,
+		stopTimeout:  stopTimeout,
 		resolveHost:  resolveHost,
 	}
 
@@ -154,10 +161,10 @@ func (s *Server) stop() {
 
 	select {
 	case <-done:
-	case <-time.After(stopTimeout):
+	case <-time.After(s.stopTimeout):
 		s.log.Error(
 			"irc connections not cleaned up before stop timeout",
-			"timeout", stopTimeout,
+			"timeout", s.stopTimeout,
 		)
 	}
 }
@@ -181,7 +188,7 @@ func (s *Server) acceptLoop(ctx context.Context) {
 
 		client := newConn(
 			tcpConn, s.log,
-			s.database, s.brk, s.cfg, s.svc,
+			s.database, s.brk, s.cfg, s.svc, s.pollInterval,
 		)
 
 		s.mu.Lock()

internal/ircserver/server_test.go

@@ -18,6 +18,7 @@ import (
 	"time"
 
 	"go.uber.org/fx/fxtest"
+	"golang.org/x/crypto/bcrypt"
 	"sneak.berlin/go/neoirc/internal/broker"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/db"
@@ -30,7 +31,16 @@ import (
 	_ "modernc.org/sqlite"
 )
 
-const testTimeout = 5 * time.Second
+// testTimeout is how long a test waits for something before it
+// fails. A passing test never waits this long, so it is generous
+// enough for a heavily loaded host.
+const testTimeout = 30 * time.Second
+
+// testPollInterval is how often the relay of a test server
+// checks the client output queue without a broker
+// notification. It is so long that the relay delivers only on
+// notification, and a test fails when one is missing.
+const testPollInterval = time.Hour
 
 // testEnv holds the shared test infrastructure.
 type testEnv struct {
@@ -91,16 +101,19 @@ func newTestEnvWithPingTimes(
 	t.Helper()
 
 	return newTestEnvWithResolver(
-		t, cfg, interval, deadline, ircserver.ResolveHost,
+		t, cfg, interval, deadline, testPollInterval,
+		ircserver.ResolveHost,
 	)
 }
 
 // newTestEnvWithResolver is newTestEnvWithPingTimes, except
-// that the server looks up a client's hostname with resolve.
+// that the relay checks the client output queue every poll
+// without a broker notification, and the server looks up a
+// client's hostname with resolve.
 func newTestEnvWithResolver(
 	t *testing.T,
 	cfg *config.Config,
-	interval, deadline time.Duration,
+	interval, deadline, poll time.Duration,
 	resolve func(ctx context.Context, addr string) string,
 ) *testEnv {
 	t.Helper()
@@ -118,6 +131,7 @@ func newTestEnvWithResolver(
 	database, err := db.New(lifecycle, db.Params{
 		Logger:       dbLog,
 		Config:       &config.Config{DBURL: dbURL},
+		PasswordCost: bcrypt.MinCost,
 	})
 	if err != nil {
 		t.Fatalf("new database: %v", err)
@@ -134,7 +148,7 @@ func newTestEnvWithResolver(
 	))
 
 	srv := ircserver.NewTestServer(
-		log, cfg, database, brk, interval, deadline, resolve,
+		log, cfg, database, brk, interval, deadline, poll, resolve,
 	)
 
 	// Port 0 makes the system pick a free port. dial reads it
@@ -430,7 +444,8 @@ func TestRegistrationWhileAnotherLookupIsPending(t *testing.T) {
 
 	env := newTestEnvWithResolver(
 		t, &config.Config{ServerName: testServerName},
-		ircserver.PingInterval, ircserver.PongDeadline, resolve,
+		ircserver.PingInterval, ircserver.PongDeadline,
+		testPollInterval, resolve,
 	)
 
 	env.dial(t)
@@ -553,27 +568,49 @@ func TestQuietClientAnsweringPingStaysConnected(t *testing.T) {
 func TestQuietClientNotAnsweringPingIsClosed(t *testing.T) {
 	t.Parallel()
 
-	// latest leaves room for a loaded host, but is sooner than
+	// A read deadline of two ping intervals would close the
-	// a read deadline of two ping intervals would close it.
+	// connection a second after latest. The ping interval is
+	// longer than testPingInterval, so that the time between
+	// the expected close and latest leaves room for a loaded
+	// host.
 	const (
-		earliest = testPingInterval + testPongDeadline
+		interval = 4 * time.Second
-		latest   = 2 * testPingInterval
+		earliest = interval + testPongDeadline
+		latest   = 2*interval - time.Second
 	)
 
-	env := newTestEnvWithShortPings(t)
+	env := newTestEnvWithPingTimes(
+		t,
+		&config.Config{ServerName: testServerName},
+		interval, testPongDeadline,
+	)
 	client := env.dial(t)
 
 	lastSent := time.Now()
 
 	client.register("silent")
-	client.readServerPing(lastSent)
+
+	// The server starts both waits once it has handled the
+	// registration, so the time registration took does not
+	// count against latest.
+	registered := time.Now()
+
+	client.readUntil(isServerPing)
 	client.readUntilClosed()
 
 	waited := time.Since(lastSent)
-	if waited < earliest || waited >= latest {
+	if waited < earliest {
 		t.Errorf(
-			"closed %v after the client's last line, want %v to %v",
+			"closed %v after the client's last line, want at least %v",
-			waited, earliest, latest,
+			waited, earliest,
+		)
+	}
+
+	sinceRegistered := time.Since(registered)
+	if sinceRegistered >= latest {
+		t.Errorf(
+			"closed %v after registration, want less than %v",
+			sinceRegistered, latest,
 		)
 	}
 }
@@ -1039,12 +1076,16 @@ func TestStopEndsSessions(t *testing.T) {
 }
 
 // TestStopReturnsWhileCleanupIsStuck checks that Stop stops
-// waiting for a connection's cleanup after StopTimeout, so a
+// waiting for a connection's cleanup after its stop timeout,
-// cleanup that cannot finish does not hold up stopping.
+// so a cleanup that cannot finish does not hold up stopping.
 func TestStopReturnsWhileCleanupIsStuck(t *testing.T) {
 	t.Parallel()
 
+	const stopTimeout = 100 * time.Millisecond
+
 	env := newTestEnv(t)
+	env.srv.SetStopTimeout(stopTimeout)
+
 	client := env.dial(t)
 
 	client.register("stuck")
@@ -1070,7 +1111,7 @@ func TestStopReturnsWhileCleanupIsStuck(t *testing.T) {
 
 	select {
 	case <-stopped:
-	case <-time.After(ircserver.StopTimeout + testTimeout):
+	case <-time.After(stopTimeout + testTimeout):
 		t.Error("Stop did not return while a cleanup was stuck")
 	}
 
@@ -1122,9 +1163,6 @@ func TestDirectMessage(t *testing.T) {
 	receiver := env.dial(t)
 	receiver.register("dmreceiver")
 
-	// Give relay goroutines time to start.
-	time.Sleep(100 * time.Millisecond)
-
 	sender.send("PRIVMSG dmreceiver :hello privately")
 
 	lines := receiver.readUntil(func(line string) bool {

internal/middleware/middleware.go

@@ -200,24 +200,19 @@ func (mware *Middleware) Logging() func(http.Handler) http.Handler {
 	}
 }
 
-// CORS returns middleware that handles Cross-Origin Resource Sharing.
+// CORS returns middleware that lets a page on any origin read the
-// AllowCredentials is true so browsers include cookies in
+// response. Use it only on public, read-only routes.
-// cross-origin API requests.
 func (mware *Middleware) CORS() func(http.Handler) http.Handler {
 	return cors.Handler(cors.Options{ //nolint:exhaustruct // optional fields
-		AllowOriginFunc: func(
+		AllowedOrigins: []string{"*"},
-			_ *http.Request, _ string,
-		) bool {
-			return true
-		},
 		AllowedMethods: []string{
 			"GET", "POST", "PUT", "DELETE", "OPTIONS",
 		},
 		AllowedHeaders: []string{
-			"Accept", "Content-Type", "X-CSRF-Token",
+			"Accept", "Authorization", "Content-Type", "X-CSRF-Token",
 		},
 		ExposedHeaders:   []string{"Link"},
-		AllowCredentials: true,
+		AllowCredentials: false,
 		MaxAge:           corsMaxAge,
 	})
 }

internal/server/routes.go

@@ -30,7 +30,6 @@ func (srv *Server) SetupRoutes() {
 		srv.router.Use(srv.mw.Metrics())
 	}
 
-	srv.router.Use(srv.mw.CORS())
 	srv.router.Use(srv.mw.CSP())
 	srv.router.Use(middleware.Timeout(routeTimeout))
 
@@ -45,7 +44,7 @@ func (srv *Server) SetupRoutes() {
 	}
 
 	// Health check.
-	srv.router.Get(
+	srv.router.With(srv.mw.CORS()).Get(
 		"/.well-known/healthcheck.json",
 		srv.handlers.HandleHealthCheck(),
 	)
@@ -69,7 +68,7 @@ func (srv *Server) SetupRoutes() {
 }
 
 func (srv *Server) setupAPIv1(router chi.Router) {
-	router.Get(
+	router.With(srv.mw.CORS()).Get(
 		"/server",
 		srv.handlers.HandleServerInfo(),
 	)

internal/server/server_test.go

@@ -6,8 +6,10 @@ package server_test
 
 import (
 	"fmt"
+	"maps"
 	"net"
 	"net/http"
+	"net/http/httptest"
 	"strconv"
 	"strings"
 	"testing"
@@ -188,6 +190,88 @@ func TestServeAfterStopDoesNotStopApp(t *testing.T) {
 	}
 }
 
+// TestCORSHeaders checks the Access-Control-Allow-* headers on the
+// answers to requests from another origin. The public, read-only
+// routes answer with Access-Control-Allow-Origin: * alone. Routes
+// that need the cookie, and a preflight to one, answer with none.
+func TestCORSHeaders(t *testing.T) {
+	var srv *server.Server
+
+	startApp(t, fx.Populate(&srv))
+
+	tests := []struct {
+		name          string
+		method        string
+		path          string
+		requestMethod string
+		wantHeaders   map[string]string
+	}{
+		{
+			name:        "server information",
+			method:      http.MethodGet,
+			path:        "/api/v1/server",
+			wantHeaders: map[string]string{"Access-Control-Allow-Origin": "*"},
+		},
+		{
+			name:        "health check",
+			method:      http.MethodGet,
+			path:        "/.well-known/healthcheck.json",
+			wantHeaders: map[string]string{"Access-Control-Allow-Origin": "*"},
+		},
+		{
+			name:   "send",
+			method: http.MethodPost,
+			path:   "/api/v1/messages",
+		},
+		{
+			name:   "state",
+			method: http.MethodGet,
+			path:   "/api/v1/state",
+		},
+		{
+			name:          "preflight for send",
+			method:        http.MethodOptions,
+			path:          "/api/v1/messages",
+			requestMethod: http.MethodPost,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			request := httptest.NewRequestWithContext(
+				t.Context(), test.method,
+				"http://chat.example.com"+test.path, nil,
+			)
+			request.Header.Set("Origin", "http://blog.example.com")
+
+			if test.requestMethod != "" {
+				request.Header.Set(
+					"Access-Control-Request-Method", test.requestMethod,
+				)
+			}
+
+			recorder := httptest.NewRecorder()
+
+			srv.ServeHTTP(recorder, request)
+
+			gotHeaders := map[string]string{}
+
+			for name := range recorder.Header() {
+				if strings.HasPrefix(name, "Access-Control-Allow-") {
+					gotHeaders[name] = recorder.Header().Get(name)
+				}
+			}
+
+			if !maps.Equal(gotHeaders, test.wantHeaders) {
+				t.Errorf(
+					"Access-Control-Allow-* headers are %v, want %v",
+					gotHeaders, test.wantHeaders,
+				)
+			}
+		})
+	}
+}
+
 // startApp starts the app newApp builds, with the HTTP port
 // set to 0, so the system gives the server a free port.
 func startApp(

internal/service/service_test.go

@@ -1,7 +1,3 @@
-// Tests use a global viper instance for configuration,
-// making parallel execution unsafe.
-//
-//nolint:paralleltest
 package service_test
 
 import (
@@ -59,28 +55,12 @@ func newTestEnv(t *testing.T) *testEnv {
 				}
 			},
 			logger.New,
-			func(
+			func() *config.Config {
-				lifecycle fx.Lifecycle,
+				return &config.Config{
-				globs *globals.Globals,
+					DBURL:        dbURL,
-				log *logger.Logger,
+					OperName:     "admin",
-			) (*config.Config, error) {
+					OperPassword: "secret",
-				cfg, err := config.New(
-					lifecycle, config.Params{ //nolint:exhaustruct
-						Globals: globs, Logger: log,
-					},
-				)
-				if err != nil {
-					return nil, fmt.Errorf(
-						"test config: %w", err,
-					)
 				}
-
-				cfg.DBURL = dbURL
-				cfg.Port = 0
-				cfg.OperName = "admin"
-				cfg.OperPassword = "secret"
-
-				return cfg, nil
 			},
 			func(
 				lifecycle fx.Lifecycle,
@@ -136,6 +116,8 @@ func createSession(
 // environment starts on an empty database while another one
 // is still open.
 func TestEachTestEnvHasItsOwnDatabase(t *testing.T) {
+	t.Parallel()
+
 	ctx := t.Context()
 
 	first := newTestEnv(t)
@@ -146,6 +128,8 @@ func TestEachTestEnvHasItsOwnDatabase(t *testing.T) {
 }
 
 func TestFanOut(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -173,6 +157,8 @@ func TestFanOut(t *testing.T) {
 }
 
 func TestJoinChannel(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -213,6 +199,8 @@ func TestJoinChannel(t *testing.T) {
 }
 
 func TestPartChannel(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -313,6 +301,8 @@ func joinConcurrently(
 // concurrent JOINs of a +l 2 channel with one member, one
 // gets in and the other gets ERR_CHANNELISFULL.
 func TestConcurrentJoinsRespectUserLimit(t *testing.T) {
+	t.Parallel()
+
 	const userLimit = 2
 
 	env := newTestEnv(t)
@@ -375,6 +365,8 @@ func TestConcurrentJoinsRespectUserLimit(t *testing.T) {
 // two concurrent JOINs that create a channel, exactly one
 // makes its session the operator.
 func TestConcurrentFirstJoinsMakeOneOperator(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -426,6 +418,8 @@ func TestConcurrentFirstJoinsMakeOneOperator(t *testing.T) {
 // either joins a channel that still exists or gets
 // ERR_NOSUCHCHANNEL, and never fails otherwise.
 func TestJoinConcurrentWithLastPart(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -491,6 +485,8 @@ func TestJoinConcurrentWithLastPart(t *testing.T) {
 // quits, gets in only once the channel is deleted, into a
 // new channel of that name with no key.
 func TestJoinConcurrentWithLastQuit(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -569,15 +565,32 @@ const quitCrowdSize = 10
 // ahead of that JOIN. The quitting session is also on a
 // channel with quitCrowdSize other sessions, made before the
 // watcher's channel, so that its QUIT reaches the watcher
-// last.
+// last. Each trial runs on a database of its own.
 func TestQuitQueuedAheadOfNewNickHoldersJoin(t *testing.T) {
-	env := newTestEnv(t)
+	t.Parallel()
-	ctx := t.Context()
 
 	for trial := range concurrentJoinTrials {
-		nick := fmt.Sprintf("reused%d", trial)
+		t.Run(fmt.Sprintf("trial %d", trial), func(t *testing.T) {
-		crowd := fmt.Sprintf("#crowd%d", trial)
+			t.Parallel()
-		shared := fmt.Sprintf("#shared%d", trial)
+
+			checkQuitQueuedAheadOfJoin(t, newTestEnv(t))
+		})
+	}
+}
+
+// checkQuitQueuedAheadOfJoin runs one trial of
+// TestQuitQueuedAheadOfNewNickHoldersJoin on env.
+func checkQuitQueuedAheadOfJoin(t *testing.T, env *testEnv) {
+	t.Helper()
+
+	const (
+		nick        = "reused"
+		crowd       = "#crowd"
+		shared      = "#shared"
+		watcherNick = "watcher"
+	)
+
+	ctx := t.Context()
 	quitterSID := createSession(ctx, t, env.db, nick)
 
 	for _, channel := range []string{crowd, shared} {
@@ -588,7 +601,7 @@ func TestQuitQueuedAheadOfNewNickHoldersJoin(t *testing.T) {
 	}
 
 	for member := range quitCrowdSize {
-			memberNick := fmt.Sprintf("crowd%dx%d", trial, member)
+		memberNick := fmt.Sprintf("crowd%d", member)
 		memberSID := createSession(ctx, t, env.db, memberNick)
 
 		_, err := env.svc.JoinChannel(ctx, memberSID, 0, memberNick, crowd, "")
@@ -597,8 +610,6 @@ func TestQuitQueuedAheadOfNewNickHoldersJoin(t *testing.T) {
 		}
 	}
 
-		watcherNick := fmt.Sprintf("watcher%d", trial)
-
 	watcherSID, watcherClientID, _, err := env.db.CreateSession(
 		ctx, watcherNick, watcherNick, "localhost", "127.0.0.1",
 	)
@@ -627,7 +638,7 @@ func TestQuitQueuedAheadOfNewNickHoldersJoin(t *testing.T) {
 		},
 	)
 	if errs[1] != nil {
-			t.Fatalf("trial %d: new session: %v", trial, errs[1])
+		t.Fatalf("new session: %v", errs[1])
 	}
 
 	msgs, _, err := env.db.PollMessages(
@@ -640,11 +651,10 @@ func TestQuitQueuedAheadOfNewNickHoldersJoin(t *testing.T) {
 	commands := commandsFrom(msgs, nick)
 	if !slices.Equal(commands, []string{irc.CmdQuit, irc.CmdJoin}) {
 		t.Errorf(
-				"trial %d: watcher got %v from %s, want QUIT then JOIN",
+			"watcher got %v from %s, want QUIT then JOIN",
-				trial, commands, nick,
+			commands, nick,
 		)
 	}
-	}
 }
 
 // takeNickAndJoin creates a session with nick, trying again
@@ -691,6 +701,8 @@ func commandsFrom(msgs []db.IRCMessage, nick string) []string {
 // into a +i channel uses the invite up, so that a JOIN
 // after leaving gets ERR_INVITEONLYCHAN.
 func TestJoinUsesUpInvite(t *testing.T) {
+	t.Parallel()
+
 	const channel = "#club"
 
 	env := newTestEnv(t)
@@ -736,6 +748,8 @@ func TestJoinUsesUpInvite(t *testing.T) {
 // session's nick, username and hostname refuses its JOIN
 // with ERR_BANNEDFROMCHAN.
 func TestJoinRefusedByHostmaskBan(t *testing.T) {
+	t.Parallel()
+
 	const channel = "#club"
 
 	env := newTestEnv(t)
@@ -774,6 +788,8 @@ func TestJoinRefusedByHostmaskBan(t *testing.T) {
 // keeps a channel other members are still on and deletes
 // the channel when its last member parts.
 func TestPartDeletesChannelOnlyWhenEmpty(t *testing.T) {
+	t.Parallel()
+
 	const channel = "#room"
 
 	env := newTestEnv(t)
@@ -821,6 +837,8 @@ func TestPartDeletesChannelOnlyWhenEmpty(t *testing.T) {
 // TestKickOfLastMemberDeletesChannel checks that a KICK
 // that leaves a channel with no members deletes it.
 func TestKickOfLastMemberDeletesChannel(t *testing.T) {
+	t.Parallel()
+
 	const channel = "#room"
 
 	env := newTestEnv(t)
@@ -845,6 +863,8 @@ func TestKickOfLastMemberDeletesChannel(t *testing.T) {
 }
 
 func TestSendChannelMessage(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -892,6 +912,8 @@ func TestSendChannelMessage(t *testing.T) {
 }
 
 func TestBroadcastQuit(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -929,6 +951,8 @@ func TestBroadcastQuit(t *testing.T) {
 // error as a DM to an unknown nick. The trigger deletes the
 // target's session just before the DM is stored.
 func TestSendDirectMessageTargetQuits(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -960,6 +984,8 @@ func TestSendDirectMessageTargetQuits(t *testing.T) {
 // trigger deletes the channel just before the message is
 // stored.
 func TestSendChannelMessageChannelDeleted(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -991,6 +1017,8 @@ func TestSendChannelMessageChannelDeleted(t *testing.T) {
 }
 
 func TestSendChannelMessage_Moderated(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -1042,6 +1070,8 @@ func TestSendChannelMessage_Moderated(t *testing.T) {
 // not spend its hashcash stamp: the operator can then send
 // the same message with it.
 func TestRefusedChannelMessageLeavesStampUnspent(t *testing.T) {
+	t.Parallel()
+
 	const (
 		channel = "#paid"
 		bits    = 2
@@ -1067,6 +1097,8 @@ func TestRefusedChannelMessageLeavesStampUnspent(t *testing.T) {
 
 	for _, refusal := range refusals {
 		t.Run(refusal.name, func(t *testing.T) {
+			t.Parallel()
+
 			env := newTestEnv(t)
 			ctx := t.Context()
 
@@ -1119,6 +1151,8 @@ func TestRefusedChannelMessageLeavesStampUnspent(t *testing.T) {
 }
 
 func TestQueryUserMode(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -1169,6 +1203,8 @@ func TestQueryUserMode(t *testing.T) {
 }
 
 func TestUserhostReply(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -1274,10 +1310,14 @@ func applyUserModeTests() []applyUserModeTest {
 }
 
 func TestApplyUserMode(t *testing.T) {
+	t.Parallel()
+
 	env := newTestEnv(t)
 
 	for i, test := range applyUserModeTests() {
 		t.Run(test.modeStr, func(t *testing.T) {
+			t.Parallel()
+
 			ctx := t.Context()
 			sid := createSession(
 				ctx, t, env.db, fmt.Sprintf("user%d", i),