Make the image prepare its own data directory (closes #262)

A host directory mounted at /var/lib/neoirc kept the host's owner, so
neoircd could not create state.db. The image now starts as root in
deploy/docker-entrypoint.sh, which creates /var/lib/neoirc, gives it to
neoirc with mode 0700, and runs neoircd as neoirc through su-exec.

Model: opus-5-5

README.md

@@ -157,10 +157,6 @@ either.
 ### Conventions
 
 - Request and response bodies are JSON.
-- Every `POST`, `PUT` and `DELETE` request under `/api/v1/` needs the header
-  `Content-Type: application/json`, even one with no body. A request with
-  another type, or none, gets `415` with
-  `{"error": "content type must be application/json"}`.
 - Endpoints marked "needs auth" need the `neoirc_auth` cookie.
   `POST /api/v1/session` and `POST /api/v1/login` set it. A request without a
   valid cookie gets `401` with `{"error": "not registered", "numeric": 451}`.
@@ -172,10 +168,8 @@ 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`.
-- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
-  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 server allows cross-origin requests from any origin, with credentials. The
+  cookie is `SameSite=Strict`, so a browser sends it only from the same site.
 
 ### The auth cookie
 
@@ -521,8 +515,7 @@ for every HTTP request when metrics are on.
 Every other `GET` path serves the web client. A path that names one of its files
 gets that file; any other path gets its `index.html`. An unknown path under
 `/api/v1/` gets a plain-text `404`. A request whose method the path does not
-take gets `405` with no body. Under `/api/v1/`, a `POST`, `PUT` or `DELETE`
-without `Content-Type: application/json` gets `415` instead.
+take gets `405` with no body.
 
 ### The message envelope
 
@@ -1346,9 +1339,8 @@ Every session has a hostmask in IRC's form, `nick!username@hostname`:
   from, or that IP address when the lookup gives no name. The lookup waits up to
   three seconds.
 
-Over the HTTP API, the client's IP address is the address of the TCP connection.
-When that connection comes from a trusted proxy, the server reads the address
-from `X-Forwarded-For` or `X-Real-IP` instead (see
+Over the HTTP API, the server takes the client's IP address from the first entry
+of the `X-Forwarded-For` header, then `X-Real-IP`, then the TCP connection (see
 [Reverse proxy](#reverse-proxy)).
 
 Every user can see a hostmask: in `RPL_WHOISUSER` (`311`), `RPL_WHOREPLY`
@@ -1458,9 +1450,10 @@ over the limit gets `429` `too many login attempts, try again later` with
 attempt is more than 15 minutes old. Session creation has no rate limit; the
 hashcash stamp is its cost.
 
-The IP address is the client's address as described in
-[Reverse proxy](#reverse-proxy). A client outside the trusted proxies cannot
-change it with `X-Forwarded-For` or `X-Real-IP`.
+The IP address comes from the first entry of `X-Forwarded-For`, else from
+`X-Real-IP`, else from the TCP connection. Without a reverse proxy that
+overwrites those headers, a client can set them itself and so pick any address
+(see [Reverse proxy](#reverse-proxy)).
 
 ### Writing a client
 
@@ -1721,16 +1714,11 @@ Every setting is an environment variable. The server also reads a `.env` file in
 its working directory, and a YAML file `neoirc.yaml` in `/etc/neoirc/` or
 `$HOME/.config/neoirc/` with the same names as keys. A variable from the
 environment wins over one in `.env`, and both win over `neoirc.yaml`. An
-environment variable set to the empty string counts as unset, with two
-exceptions:
-
-- For `IRC_LISTEN_ADDR`, the empty string turns the IRC listener off.
-- For `TRUSTED_PROXIES`, the empty string trusts no proxy.
-
-A `neoirc.yaml` that is not valid YAML stops the server at startup. A setting
-that does not parse or is out of range also stops the server at startup, and the
-error names the setting. The same holds for a setting given a list or a map in
-`neoirc.yaml`.
+environment variable set to the empty string counts as unset, except
+`IRC_LISTEN_ADDR`, where the empty string turns the IRC listener off. A
+`neoirc.yaml` that is not valid YAML stops the server at startup. A setting that
+does not parse or is out of range also stops the server at startup, and the
+error names the setting.
 
 - `PORT` (default `8080`): the HTTP port. `0` lets the system pick a free port.
 - `IRC_LISTEN_ADDR` (default `:6667`): the address of the IRC listener. The
@@ -1759,11 +1747,6 @@ error names the setting. The same holds for a setting given a list or a map in
   address. A fraction such as `0.5` is allowed. It must be above `0`.
 - `LOGIN_RATE_BURST` (default `5`): how many attempts an address may make at
   once before the rate applies. It must be at least `1`.
-- `TRUSTED_PROXIES` (default `10.0.0.0/8,172.16.0.0/12,192.168.0.0/16`): the
-  reverse proxies whose `X-Forwarded-For` and `X-Real-IP` headers the server
-  believes. It is a comma-separated list of networks, such as `10.0.0.0/8`, and
-  single addresses, such as `192.0.2.1`. A list replaces the default entirely.
-  The empty string trusts nothing. See [Reverse proxy](#reverse-proxy).
 - `SESSION_IDLE_TIMEOUT` (default `720h`, 30 days): a duration with a unit, `h`
   for hours, `m` for minutes or `s` for seconds, such as `720h` or `36h30m`. A
   client with no request for this long is removed, and a session left with no
@@ -1836,34 +1819,14 @@ Run the server behind a reverse proxy that terminates TLS. The server itself
 speaks plain HTTP, and the auth cookie is `Secure`, so browsers send it only
 over HTTPS. The proxy must:
 
-- Connect from an address in `TRUSTED_PROXIES`. The default trusts the private
-  ranges `10.0.0.0/8`, `172.16.0.0/12` and `192.168.0.0/16`. The setting must
-  hold the address the server sees the proxy connect from. For a proxy on the
-  same host:
-    - With the server outside Docker, that address is `127.0.0.1` or `::1`. The
-      default does not trust them, so set `TRUSTED_PROXIES=127.0.0.1,::1`.
-    - Through a published Docker port, that address is the Docker bridge
-      gateway, such as `172.17.0.1`. The default trusts it.
-- Add the client's address to `X-Forwarded-For`.
+- Set `X-Forwarded-For` to the client's address, replacing any value the client
+  sent. The server reads that header first and trusts it from anyone; it reads
+  `X-Real-IP` only when `X-Forwarded-For` is absent. Without a proxy that
+  overwrites the header, a client can choose its own address, which sets its
+  hostname and lets it dodge the login rate limit.
 - Wait at least 60 seconds for a response. A long poll can take 30 seconds, and
   the server's own write timeout is 60 seconds.
 
-The server finds the client's address in these steps:
-
-- A connection from outside `TRUSTED_PROXIES` is the client. The server ignores
-  its `X-Forwarded-For` and `X-Real-IP` headers.
-- For a connection from a trusted proxy, the server reads `X-Forwarded-For` from
-  right to left. It skips trusted addresses and takes the first untrusted one.
-  When every entry is trusted, it takes the leftmost.
-- Without `X-Forwarded-For`, the server takes `X-Real-IP`.
-- When an entry it reads is not an IP address, the server uses the proxy's
-  address.
-
-The client's address sets the session's hostname and counts toward the login
-rate limit. Any connection from a trusted address can choose its client address
-through `X-Forwarded-For`. If clients connect directly from private addresses,
-set `TRUSTED_PROXIES` to the proxy's address alone.
-
 Caddy:
 
 ```
@@ -1900,34 +1863,22 @@ 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 carries a `Content-Security-Policy`
-  header:
+- Every response to a well-formed request, except to a cross-origin preflight,
+  carries a `Content-Security-Policy` 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.
-- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
-  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.
+- Cross-origin requests are allowed from any origin, with credentials. The
+  allowed methods are `GET`, `POST`, `PUT`, `DELETE` and `OPTIONS`. The allowed
+  request headers are `Accept`, `Content-Type` and `X-CSRF-Token`. The cookie's
+  `SameSite=Strict` still keeps it to the same site.
 
 ### Logging
 
 The server logs with Go's `slog`, one line per event, to standard output. The
 lines are JSON, or plain text when standard output is a terminal. Every HTTP
-request is logged with:
-
-- start time
-- method
-- URL
-- protocol
-- status
-- latency
-- client address
-- user agent
-- referrer
-- request ID
-
-The client address is the one found as described in
-[Reverse proxy](#reverse-proxy). `DEBUG=true` adds debug-level lines.
+request is logged with its method, URL, protocol, status, latency, remote
+address, user agent, referrer and request ID. `DEBUG=true` adds debug-level
+lines.
 
 ### The database
 
@@ -2010,8 +1961,7 @@ and their sessions to end. Then it closes the database.
   gets a message.
 - `internal/hashcash/`: stamp checking and minting.
 - `internal/ratelimit/`: the per-address login rate limiter.
-- `internal/middleware/`: finding the client's address, request logging,
-  cross-origin requests, the API's `Content-Type` check, the
+- `internal/middleware/`: request logging, cross-origin requests, the
   `Content-Security-Policy` header and metrics.
 - `internal/healthcheck/` and `internal/stats/`: the health check and its
   counters.

go.mod

@@ -13,10 +13,8 @@ require (
 	github.com/prometheus/client_golang v1.23.2
 	github.com/rivo/tview v0.42.0
 	github.com/slok/go-http-metrics v0.13.0
-	github.com/spf13/cast v1.10.0
 	github.com/spf13/viper v1.21.0
 	go.uber.org/fx v1.24.0
-	go4.org/netipx v0.0.0-20260823151212-3075585bcbeb
 	golang.org/x/crypto v0.48.0
 	golang.org/x/time v0.6.0
 	modernc.org/sqlite v1.45.0
@@ -42,6 +40,7 @@ require (
 	github.com/sagikazarmark/locafero v0.11.0 // indirect
 	github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 // indirect
 	github.com/spf13/afero v1.15.0 // indirect
+	github.com/spf13/cast v1.10.0 // indirect
 	github.com/spf13/pflag v1.0.10 // indirect
 	github.com/subosito/gotenv v1.6.0 // indirect
 	go.uber.org/dig v1.19.0 // indirect

go.sum

@@ -111,8 +111,6 @@ go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI=
 go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU=
 go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc=
 go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
-go4.org/netipx v0.0.0-20260823151212-3075585bcbeb h1:XBM4hvfwGAttkkiTIFfeigdfcL1xIfdKXqFdgiHGtDs=
-go4.org/netipx v0.0.0-20260823151212-3075585bcbeb/go.mod h1:PLyyIXexvUFg3Owu6p/WfdlivPbZJsZdgWZlrGope/Y=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
 golang.org/x/crypto v0.48.0 h1:/VRzVqiRSggnhY7gNRxPauEQ5Drw9haKdM0jqfcCFts=

internal/config/config.go

@@ -6,16 +6,12 @@ import (
 	"fmt"
 	"log/slog"
 	"math"
-	"net/netip"
 	"os"
 	"strconv"
-	"strings"
 	"time"
 
-	"github.com/spf13/cast"
 	"github.com/spf13/viper"
 	"go.uber.org/fx"
-	"go4.org/netipx"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/logger"
 
@@ -36,16 +32,8 @@ 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
-// private address ranges of RFC 1918.
-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")
 
-var errNeverMatches = errors.New("entry can never match a connection")
-
-var errListOrMap = errors.New("a list or a map")
-
 // Params defines the dependencies for creating a Config.
 type Params struct {
 	fx.In
@@ -76,15 +64,13 @@ type Config struct {
 	LoginRateLimit     float64
 	LoginRateBurst     int
 	IRCListenAddr      string
-	TrustedProxies     *netipx.IPSet
 	params             *Params
 	log                *slog.Logger
 }
 
 // New creates a new Config by reading from files and environment variables.
-// It returns an error when the config file does not parse, when the file
-// gives a setting a list or a map, and when a number, true/false,
-// duration or address setting does not parse or is out of range.
+// It returns an error when a number, true/false or duration setting does
+// not parse or is out of range.
 func New(
 	_ fx.Lifecycle, params Params,
 ) (*Config, error) {
@@ -117,20 +103,32 @@ 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)
 
 	err := viper.ReadInConfig()
 	if err != nil {
 		var notFound viper.ConfigFileNotFoundError
 		if !errors.As(err, &notFound) {
-			return nil, fmt.Errorf("cannot read %s: %w",
-				viper.ConfigFileUsed(), err)
+			log.Error("config file malformed", "error", err)
+			panic(err)
 		}
 	}
 
 	cfg := &Config{
-		log:    log,
-		params: &params,
+		DBURL:              viper.GetString("DBURL"),
+		SentryDSN:          viper.GetString("SENTRY_DSN"),
+		MetricsUsername:    viper.GetString("METRICS_USERNAME"),
+		MetricsPassword:    viper.GetString("METRICS_PASSWORD"),
+		MessageMaxAge:      viper.GetString("MESSAGE_MAX_AGE"),
+		QueueMaxAge:        viper.GetString("QUEUE_MAX_AGE"),
+		MOTD:               viper.GetString("MOTD"),
+		ServerName:         viper.GetString("SERVER_NAME"),
+		FederationKey:      viper.GetString("FEDERATION_KEY"),
+		SessionIdleTimeout: viper.GetString("SESSION_IDLE_TIMEOUT"),
+		OperName:           viper.GetString("NEOIRC_OPER_NAME"),
+		OperPassword:       viper.GetString("NEOIRC_OPER_PASSWORD"),
+		IRCListenAddr:      viper.GetString("IRC_LISTEN_ADDR"),
+		log:                log,
+		params:             &params,
 	}
 
 	err = parseSettings(cfg)
@@ -138,6 +136,13 @@ func New(
 		return nil, err
 	}
 
+	// Viper counts an empty environment variable as unset, but an
+	// empty IRC_LISTEN_ADDR must turn the IRC listener off.
+	ircListenAddr, set := os.LookupEnv("IRC_LISTEN_ADDR")
+	if set {
+		cfg.IRCListenAddr = ircListenAddr
+	}
+
 	if cfg.Debug {
 		params.Logger.EnableDebugLogging()
 		cfg.log = params.Logger.Get()
@@ -146,24 +151,13 @@ func New(
 	return cfg, nil
 }
 
-// parseSettings reads every setting into cfg. Each number and duration
-// must be above zero. PORT and NEOIRC_HASHCASH_BITS may also be zero:
-// port 0 lets the system pick a port, and 0 bits turns proof of work off.
-// The error names each setting that is a list or a map, does not parse or
-// is out of range.
+// parseSettings reads the number and true/false settings into cfg and
+// checks the duration settings. Each number and duration must be above
+// zero. PORT and NEOIRC_HASHCASH_BITS may also be zero: port 0 lets the
+// system pick a port, and 0 bits turns proof of work off. The error
+// names each setting that does not parse or is out of range.
 func parseSettings(cfg *Config) error {
 	return errors.Join(
-		parseString("DBURL", &cfg.DBURL),
-		parseString("SENTRY_DSN", &cfg.SentryDSN),
-		parseString("METRICS_USERNAME", &cfg.MetricsUsername),
-		parseString("METRICS_PASSWORD", &cfg.MetricsPassword),
-		parseString("MOTD", &cfg.MOTD),
-		parseString("SERVER_NAME", &cfg.ServerName),
-		parseString("FEDERATION_KEY", &cfg.FederationKey),
-		parseString("NEOIRC_OPER_NAME", &cfg.OperName),
-		parseString("NEOIRC_OPER_PASSWORD", &cfg.OperPassword),
-		// The empty string turns the IRC listener off.
-		parseStringAllowEmptyEnv("IRC_LISTEN_ADDR", &cfg.IRCListenAddr),
 		parseInt("PORT", 0, &cfg.Port),
 		parseInt("MAX_MESSAGE_SIZE", 1, &cfg.MaxMessageSize),
 		parseInt("NEOIRC_HASHCASH_BITS", 0, &cfg.HashcashBits),
@@ -171,136 +165,16 @@ func parseSettings(cfg *Config) error {
 		parseFloat("LOGIN_RATE_LIMIT", &cfg.LoginRateLimit),
 		parseBool("DEBUG", &cfg.Debug),
 		parseBool("MAINTENANCE_MODE", &cfg.MaintenanceMode),
-		parseDuration("MESSAGE_MAX_AGE", &cfg.MessageMaxAge),
-		parseDuration("QUEUE_MAX_AGE", &cfg.QueueMaxAge),
-		parseDuration("SESSION_IDLE_TIMEOUT", &cfg.SessionIdleTimeout),
-		parseTrustedProxies(&cfg.TrustedProxies),
+		checkDuration("MESSAGE_MAX_AGE", cfg.MessageMaxAge),
+		checkDuration("QUEUE_MAX_AGE", cfg.QueueMaxAge),
+		checkDuration("SESSION_IDLE_TIMEOUT", cfg.SessionIdleTimeout),
 	)
 }
 
-// getString returns the setting key as a string. A number or a true/false
-// value in the config file becomes its text, as with viper.GetString. A
-// list or a map in the config file is an error, where viper.GetString
-// would return the empty string. The error leaves out the value, which
-// may be a password.
-func getString(key string) (string, error) {
-	raw, err := cast.ToStringE(viper.Get(key))
-	if err != nil {
-		return "", fmt.Errorf("%s is %w", key, errListOrMap)
-	}
-
-	return raw, nil
-}
-
-// parseString sets *value to the setting key.
-func parseString(key string, value *string) error {
-	raw, err := getString(key)
-	if err != nil {
-		return err
-	}
-
-	*value = raw
-
-	return nil
-}
-
-// parseStringAllowEmptyEnv sets *value to the setting key, as parseString
-// does, but an environment variable set to the empty string counts as
-// set. Viper counts such a variable as unset and uses the config file or
-// the default instead.
-func parseStringAllowEmptyEnv(key string, value *string) error {
-	fromEnv, set := os.LookupEnv(key)
-	if set {
-		*value = fromEnv
-
-		return nil
-	}
-
-	return parseString(key, value)
-}
-
-// parseTrustedProxies sets *value to the addresses in TRUSTED_PROXIES.
-func parseTrustedProxies(value **netipx.IPSet) error {
-	var raw string
-
-	err := parseStringAllowEmptyEnv("TRUSTED_PROXIES", &raw)
-	if err != nil {
-		return err
-	}
-
-	ipSet, err := ParseTrustedProxies(raw)
-	if err != nil {
-		return fmt.Errorf("cannot parse TRUSTED_PROXIES: %w", err)
-	}
-
-	*value = ipSet
-
-	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
-// address with a zone are errors, since no connection could match them.
-func addTrustedProxy(builder *netipx.IPSetBuilder, entry string) error {
-	if strings.Contains(entry, "/") {
-		prefix, err := netip.ParsePrefix(entry)
-		if err != nil {
-			return err //nolint:wrapcheck // the caller names the setting
-		}
-
-		// A connection from an IPv4 address is checked in IPv4 form.
-		if prefix.Addr().Is4In6() {
-			return fmt.Errorf("%w: %s is an IPv4 network in IPv6 form",
-				errNeverMatches, entry)
-		}
-
-		builder.AddPrefix(prefix)
-
-		return nil
-	}
-
-	addr, err := netip.ParseAddr(entry)
-	if err != nil {
-		return err //nolint:wrapcheck // the caller names the setting
-	}
-
-	// IPSet.Contains is false for every address with a zone.
-	if addr.Zone() != "" {
-		return fmt.Errorf("%w: %s has a zone", errNeverMatches, entry)
-	}
-
-	builder.Add(addr.Unmap())
-
-	return nil
-}
-
 // parseInt sets *value to the setting key read as a whole number. It
 // returns an error when the value does not parse or is below minimum.
 func parseInt(key string, minimum int, value *int) error {
-	raw, err := getString(key)
-	if err != nil {
-		return err
-	}
+	raw := viper.GetString(key)
 
 	parsed, err := strconv.Atoi(raw)
 	if err != nil {
@@ -321,10 +195,7 @@ func parseInt(key string, minimum int, value *int) error {
 // number. It returns an error when the value does not parse or is not a
 // finite number above zero.
 func parseFloat(key string, value *float64) error {
-	raw, err := getString(key)
-	if err != nil {
-		return err
-	}
+	raw := viper.GetString(key)
 
 	parsed, err := strconv.ParseFloat(raw, 64)
 	if err != nil {
@@ -344,12 +215,7 @@ func parseFloat(key string, value *float64) error {
 
 // parseBool sets *value to the setting key read as true or false.
 func parseBool(key string, value *bool) error {
-	raw, err := getString(key)
-	if err != nil {
-		return err
-	}
-
-	parsed, err := strconv.ParseBool(raw)
+	parsed, err := strconv.ParseBool(viper.GetString(key))
 	if err != nil {
 		return fmt.Errorf("cannot parse %s: %w", key, err)
 	}
@@ -359,25 +225,18 @@ func parseBool(key string, value *bool) error {
 	return nil
 }
 
-// parseDuration sets *value to the setting key. It returns an error when
-// the value is not a Go duration above zero.
-func parseDuration(key string, value *string) error {
-	raw, err := getString(key)
-	if err != nil {
-		return err
-	}
-
-	duration, err := time.ParseDuration(raw)
+// checkDuration returns an error naming the setting key when value is
+// not a Go duration above zero.
+func checkDuration(key, value string) error {
+	duration, err := time.ParseDuration(value)
 	if err != nil {
 		return fmt.Errorf("cannot parse %s: %w", key, err)
 	}
 
 	if duration <= 0 {
 		return fmt.Errorf("%w: %s is %s, but must be above zero",
-			errOutOfRange, key, raw)
+			errOutOfRange, key, value)
 	}
 
-	*value = raw
-
 	return nil
 }

internal/config/config_test.go

@@ -1,14 +1,11 @@
 package config_test
 
 import (
-	"net/netip"
 	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 	"time"
 
-	"github.com/spf13/viper"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/logger"
@@ -143,7 +140,8 @@ func settingsOf(cfg *config.Config) settings {
 }
 
 func TestBadSettingStopsStartup(t *testing.T) {
-	// Each value is invalid for its setting.
+	// For each setting, the first value does not parse and the others
+	// parse but are out of range.
 	tests := []struct {
 		key    string
 		values []string
@@ -160,10 +158,6 @@ func TestBadSettingStopsStartup(t *testing.T) {
 		{key: "MESSAGE_MAX_AGE", values: []string{"30d", "0", "-1h"}},
 		{key: "QUEUE_MAX_AGE", values: []string{"720", "0s", "-24h"}},
 		{key: "SESSION_IDLE_TIMEOUT", values: []string{"a month", "0", "-1m"}},
-		{key: "TRUSTED_PROXIES", values: []string{
-			"proxy.example.com", "10.0.0.0/33", "192.0.2.1:8080",
-			"::ffff:10.0.0.0/104", "fe80::1%eth0",
-		}},
 	}
 
 	for _, test := range tests {
@@ -212,228 +206,3 @@ func TestIRCListenAddrEmptyTurnsListenerOff(t *testing.T) {
 		t.Errorf("IRCListenAddr = %q, want empty", cfg.IRCListenAddr)
 	}
 }
-
-func TestTrustedProxies(t *testing.T) {
-	// Each case lists addresses that must be in the trusted set and
-	// addresses that must not.
-	tests := []struct {
-		name      string
-		set       bool
-		value     string
-		trusted   []string
-		untrusted []string
-	}{
-		{
-			name:  "unset trusts the RFC 1918 ranges",
-			set:   false,
-			value: "",
-			trusted: []string{
-				"10.1.2.3", "172.16.0.1", "172.31.255.254", "192.168.1.1",
-			},
-			untrusted: []string{
-				"127.0.0.1", "172.32.0.1", "192.0.2.1", "2001:db8::1",
-			},
-		},
-		{
-			name:      "a value replaces the default",
-			set:       true,
-			value:     "198.51.100.0/24, 192.0.2.7,2001:db8::/32",
-			trusted:   []string{"198.51.100.200", "192.0.2.7", "2001:db8::5"},
-			untrusted: []string{"10.4.5.6", "192.168.4.4", "192.0.2.8"},
-		},
-		{
-			name:      "a mapped IPv6 address is trusted as IPv4",
-			set:       true,
-			value:     "::ffff:192.0.2.7",
-			trusted:   []string{"192.0.2.7"},
-			untrusted: []string{"10.7.8.9"},
-		},
-		{
-			name:      "the empty string trusts nothing",
-			set:       true,
-			value:     "",
-			trusted:   nil,
-			untrusted: []string{"10.0.0.1", "192.168.0.1", "::1"},
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			// t.Setenv puts back whatever value was there after the
-			// test, including when Unsetenv removes it.
-			t.Setenv("TRUSTED_PROXIES", test.value)
-
-			if !test.set {
-				err := os.Unsetenv("TRUSTED_PROXIES")
-				if err != nil {
-					t.Fatal(err)
-				}
-			}
-
-			proxies := newConfig(t).TrustedProxies
-
-			for _, addr := range test.trusted {
-				if !proxies.Contains(netip.MustParseAddr(addr)) {
-					t.Errorf("%s is not trusted", addr)
-				}
-			}
-
-			for _, addr := range test.untrusted {
-				if proxies.Contains(netip.MustParseAddr(addr)) {
-					t.Errorf("%s is trusted", addr)
-				}
-			}
-		})
-	}
-}
-
-// unsetEnv removes the environment variable key for the rest of the
-// test, so that config.New reads key from the config file.
-func unsetEnv(t *testing.T, key string) {
-	t.Helper()
-
-	// t.Setenv puts back whatever value was there after the test,
-	// including when Unsetenv removes it.
-	t.Setenv(key, "")
-
-	err := os.Unsetenv(key)
-	if err != nil {
-		t.Fatal(err)
-	}
-}
-
-// writeConfigFile writes content to the config file config.New reads
-// when HOME is home, and returns the file's path.
-func writeConfigFile(t *testing.T, home, content string) string {
-	t.Helper()
-
-	// loadConfig names the app neoirc-test, so config.New reads
-	// $HOME/.config/neoirc-test/neoirc-test.yaml.
-	dir := filepath.Join(home, ".config", "neoirc-test")
-
-	err := os.MkdirAll(dir, 0o700)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	path := filepath.Join(dir, "neoirc-test.yaml")
-
-	err = os.WriteFile(path, []byte(content), 0o600)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	// Viper keeps the file and the values it read from it after the
-	// file is gone. Reset clears them for later tests.
-	t.Cleanup(viper.Reset)
-
-	return path
-}
-
-func TestListOrMapInConfigFileStopsStartup(t *testing.T) {
-	// YAML reads the first value as a list and the second as a map.
-	values := []string{`[":6667"]`, `{addr: ":6667"}`}
-	keys := []string{"IRC_LISTEN_ADDR", "MOTD", "TRUSTED_PROXIES"}
-
-	for _, key := range keys {
-		for _, value := range values {
-			t.Run(key+"="+value, func(t *testing.T) {
-				home := t.TempDir()
-				t.Setenv("HOME", home)
-				unsetEnv(t, key)
-				writeConfigFile(t, home, key+": "+value+"\n")
-
-				_, err := loadConfig(t)
-				if err == nil || !strings.Contains(err.Error(), key) {
-					t.Errorf("error = %v, want one naming %s", err, key)
-				}
-			})
-		}
-	}
-}
-
-func TestListOrMapErrorLeavesOutValue(t *testing.T) {
-	const key = "NEOIRC_OPER_PASSWORD"
-
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	unsetEnv(t, key)
-	// YAML reads an unquoted {hunter2} as a map.
-	writeConfigFile(t, home, key+": {hunter2}\n")
-
-	_, err := loadConfig(t)
-	if err == nil || !strings.Contains(err.Error(), key) {
-		t.Fatalf("error = %v, want one naming %s", err, key)
-	}
-
-	if strings.Contains(err.Error(), "hunter2") {
-		t.Errorf("error %q shows the password", err)
-	}
-}
-
-func TestEmptyEnvironmentValueWinsOverConfigFile(t *testing.T) {
-	const (
-		ircListenAddr  = "IRC_LISTEN_ADDR"
-		trustedProxies = "TRUSTED_PROXIES"
-	)
-
-	// Each setting is the empty string in the environment and a list in
-	// the config file. The environment wins, so the list is never read.
-	tests := []struct {
-		key   string
-		empty func(cfg *config.Config) bool
-	}{
-		{
-			key: ircListenAddr,
-			empty: func(cfg *config.Config) bool {
-				return cfg.IRCListenAddr == ""
-			},
-		},
-		{
-			key: trustedProxies,
-			empty: func(cfg *config.Config) bool {
-				return len(cfg.TrustedProxies.Prefixes()) == 0
-			},
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.key, func(t *testing.T) {
-			home := t.TempDir()
-			t.Setenv("HOME", home)
-			t.Setenv(test.key, "")
-			writeConfigFile(t, home, test.key+": [\":6667\"]\n")
-
-			cfg := newConfig(t)
-			if !test.empty(cfg) {
-				t.Errorf("%s is not the empty string", test.key)
-			}
-		})
-	}
-}
-
-func TestSingleValuesInConfigFileAreUsed(t *testing.T) {
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	unsetEnv(t, "PORT")
-	unsetEnv(t, "DEBUG")
-	unsetEnv(t, "MOTD")
-	writeConfigFile(t, home, "PORT: 9000\nDEBUG: true\nMOTD: hello\n")
-
-	cfg := newConfig(t)
-	if cfg.Port != 9000 || !cfg.Debug || cfg.MOTD != "hello" {
-		t.Errorf("Port, Debug, MOTD = %d, %t, %q, want 9000, true, %q",
-			cfg.Port, cfg.Debug, cfg.MOTD, "hello")
-	}
-}
-
-func TestConfigFileThatDoesNotParseStopsStartup(t *testing.T) {
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	path := writeConfigFile(t, home, "MOTD: [unclosed\n")
-
-	_, err := loadConfig(t)
-	if err == nil || !strings.Contains(err.Error(), path) {
-		t.Errorf("error = %v, want one naming %s", err, path)
-	}
-}

internal/db/auth.go

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

internal/db/db.go

@@ -15,7 +15,6 @@ import (
 	"strings"
 
 	"go.uber.org/fx"
-	"golang.org/x/crypto/bcrypt"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/logger"
 
@@ -40,19 +39,13 @@ 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.
 type Database struct {
-	conn         *sql.DB
-	log          *slog.Logger
-	params       *Params
-	passwordCost int
+	conn   *sql.DB
+	log    *slog.Logger
+	params *Params
 }
 
 // New creates a new Database and registers lifecycle hooks.
@@ -60,15 +53,9 @@ 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,
+		params: &params,
+		log:    params.Logger.Get(),
 	}
 
 	database.log.Info("Database instantiated")

internal/db/db_test.go

@@ -10,7 +10,6 @@ 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"
@@ -27,11 +26,7 @@ const (
 		(SELECT 1 UNION ALL SELECT x + 1 FROM n)
 		SELECT COUNT(*) FROM n`
 
-	// 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
+	endlessQueryTimeout = 100 * time.Millisecond
 )
 
 // TestDefaultDBURLSetsWALAndBusyTimeout opens a file database
@@ -241,85 +236,6 @@ 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,12 +8,10 @@ import (
 	"testing"
 	"time"
 
-	"golang.org/x/crypto/bcrypt"
 	"sneak.berlin/go/neoirc/internal/dbtest"
 )
 
-// NewTestDatabase creates an in-memory database for testing. It
-// hashes passwords at bcrypt.MinCost.
+// NewTestDatabase creates an in-memory database for testing.
 func NewTestDatabase(tb testing.TB) (*Database, error) {
 	tb.Helper()
 
@@ -25,9 +23,8 @@ func NewTestDatabase(tb testing.TB) (*Database, error) {
 	}
 
 	database := &Database{ //nolint:exhaustruct // test helper, params not needed
-		conn:         conn,
-		log:          slog.Default(),
-		passwordCost: bcrypt.MinCost,
+		conn: conn,
+		log:  slog.Default(),
 	}
 
 	err = database.runMigrations(context.Background())

internal/handlers/api.go

@@ -42,9 +42,25 @@ func (hdlr *Handlers) maxBodySize() int64 {
 	return defaultMaxBodySize
 }
 
-// clientIP returns the IP address in request.RemoteAddr, which the
-// ClientIP middleware has set to the client's address.
+// clientIP extracts the connecting client's IP address
+// from the request, checking X-Forwarded-For and
+// X-Real-IP headers before falling back to RemoteAddr.
 func clientIP(request *http.Request) string {
+	if forwarded := request.Header.Get("X-Forwarded-For"); forwarded != "" {
+		// X-Forwarded-For can contain a comma-separated list;
+		// the first entry is the original client.
+		parts := strings.SplitN(forwarded, ",", 2) //nolint:mnd
+		ip := strings.TrimSpace(parts[0])
+
+		if ip != "" {
+			return ip
+		}
+	}
+
+	if realIP := request.Header.Get("X-Real-IP"); realIP != "" {
+		return strings.TrimSpace(realIP)
+	}
+
 	host, _, err := net.SplitHostPort(request.RemoteAddr)
 	if err != nil {
 		return request.RemoteAddr

internal/handlers/api_test.go

@@ -145,46 +145,34 @@ type testServer struct {
 	handlers   *handlers.Handlers
 }
 
-const (
-	// maxMessageSize is MAX_MESSAGE_SIZE on every test server, the
-	// setting's default.
-	maxMessageSize = 4096
-
-	// 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
-)
+// viperMu makes tests start their servers one at a time,
+// because config.New writes the global viper instance and
+// the server reads it as it starts.
+//
+//nolint:gochecknoglobals // guards a global, so must be one
+var viperMu sync.Mutex
 
 func newTestServer(
 	t *testing.T,
 ) *testServer {
 	t.Helper()
 
-	return newTestServerWith(t, 0, "", "", config.DefaultTrustedProxies)
+	return newTestServerWith(t, 0, "", "")
 }
 
 func newTestServerWith(
 	t *testing.T,
 	hashcashBits int,
-	operName, operPassword, trustedProxies string,
+	operName, operPassword 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
@@ -196,25 +184,31 @@ func newTestServerWith(
 		fx.Provide(
 			newTestGlobals,
 			logger.New,
-			// Built here rather than by config.New, which writes
-			// the global viper instance, so servers start in
-			// parallel.
-			func() *config.Config {
-				return &config.Config{
-					DBURL: dbURL,
-					// One line: each line is a database write per new session.
-					MOTD:               "neoirc test server",
-					MaxMessageSize:     maxMessageSize,
-					HashcashBits:       hashcashBits,
-					OperName:           operName,
-					OperPassword:       operPassword,
-					MessageMaxAge:      thirtyDays,
-					QueueMaxAge:        thirtyDays,
-					SessionIdleTimeout: thirtyDays,
-					LoginRateLimit:     loginRateLimit,
-					LoginRateBurst:     loginRateBurst,
-					TrustedProxies:     trusted,
+			func(
+				lifecycle fx.Lifecycle,
+				globs *globals.Globals,
+				log *logger.Logger,
+			) (*config.Config, error) {
+				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
+				// One line: each line is a database write per new session.
+				cfg.MOTD = "neoirc test server"
+				cfg.HashcashBits = hashcashBits
+				cfg.OperName = operName
+				cfg.OperPassword = operPassword
+
+				return cfg, nil
 			},
 			newTestDB,
 			broker.New,
@@ -261,7 +255,7 @@ func newTestDB(
 	cfg *config.Config,
 ) (*db.Database, error) {
 	database, err := db.New(lifecycle, db.Params{ //nolint:exhaustruct
-		Logger: log, Config: cfg, PasswordCost: bcrypt.MinCost,
+		Logger: log, Config: cfg,
 	})
 	if err != nil {
 		return nil, fmt.Errorf("test db: %w", err)
@@ -415,7 +409,7 @@ func doRequestAuth(
 		return nil, fmt.Errorf("new request: %w", err)
 	}
 
-	if method != http.MethodGet {
+	if body != nil {
 		request.Header.Set(
 			"Content-Type", "application/json",
 		)
@@ -836,7 +830,10 @@ func (tserver *testServer) postBody(
 func TestRequestBodyLimit(t *testing.T) {
 	t.Parallel()
 
-	const nick = "alice_bodylimit"
+	const (
+		maxMessageSize = 4096 // the default MAX_MESSAGE_SIZE
+		nick           = "alice_bodylimit"
+	)
 
 	endpoints := []struct {
 		name, path, value string
@@ -903,131 +900,6 @@ func TestRequestBodyLimit(t *testing.T) {
 	}
 }
 
-// requestWithType sends a request with the Content-Type contentType,
-// or none when it is empty, and returns the status and the decoded
-// JSON body.
-func (tserver *testServer) requestWithType(
-	method, path, token, contentType, body string,
-) (int, map[string]any) {
-	tserver.t.Helper()
-
-	request, err := http.NewRequestWithContext(
-		tserver.t.Context(), method, tserver.url(path),
-		strings.NewReader(body),
-	)
-	if err != nil {
-		tserver.t.Fatalf("new request: %v", err)
-	}
-
-	if contentType != "" {
-		request.Header.Set("Content-Type", contentType)
-	}
-
-	request.AddCookie(&http.Cookie{ //nolint:exhaustruct,gosec // only name+value
-		Name:  authCookieName,
-		Value: token,
-	})
-
-	resp, err := http.DefaultClient.Do(request)
-	if err != nil {
-		tserver.t.Fatalf("%s %s: %v", method, path, err)
-	}
-
-	defer func() { _ = resp.Body.Close() }()
-
-	var result map[string]any
-
-	_ = json.NewDecoder(resp.Body).Decode(&result)
-
-	return resp.StatusCode, result
-}
-
-// TestRequireJSONContentType checks that a POST whose Content-Type is
-// not application/json gets 415 and changes nothing: the message is
-// not stored and the client is not logged out.
-func TestRequireJSONContentType(t *testing.T) {
-	t.Parallel()
-
-	tserver := newTestServer(t)
-	token := tserver.createSession("alice_ctype")
-
-	tserver.sendCommand(token, map[string]any{
-		commandKey: joinCmd, toKey: "#ctype",
-	})
-
-	status, _ := tserver.requestWithType(
-		http.MethodGet, apiState, token, "text/plain", "",
-	)
-	if status != http.StatusOK {
-		t.Errorf("GET with text/plain: expected 200, got %d", status)
-	}
-
-	tests := []struct {
-		name, contentType string
-		want              int
-	}{
-		{"text", "text/plain", http.StatusUnsupportedMediaType},
-		{
-			"form", "application/x-www-form-urlencoded",
-			http.StatusUnsupportedMediaType,
-		},
-		{
-			"multipart", "multipart/form-data; boundary=x",
-			http.StatusUnsupportedMediaType,
-		},
-		{"missing", "", http.StatusUnsupportedMediaType},
-		{"json", "application/json", http.StatusOK},
-		{"json with charset", "application/json; charset=utf-8", http.StatusOK},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
-			status, result := tserver.requestWithType(
-				http.MethodPost, apiMessages, token, test.contentType,
-				`{"command":"PRIVMSG","to":"#ctype","body":["`+test.name+`"]}`,
-			)
-			if status != test.want {
-				t.Fatalf("send: expected %d, got %d: %v", test.want, status, result)
-			}
-
-			if status == http.StatusUnsupportedMediaType &&
-				result["error"] != "content type must be application/json" {
-				t.Errorf("send: unexpected error body %v", result)
-			}
-
-			msgs, _ := tserver.pollMessages(token, 0)
-
-			stored := slices.ContainsFunc(msgs, func(msg map[string]any) bool {
-				body, _ := msg[bodyKey].([]any)
-
-				return msg[commandKey] == privmsgCmd &&
-					slices.Contains(body, any(test.name))
-			})
-			if stored != (test.want == http.StatusOK) {
-				t.Errorf("message stored: %v, want %v", stored, !stored)
-			}
-
-			if test.want == http.StatusOK {
-				return
-			}
-
-			status, _ = tserver.requestWithType(
-				http.MethodPost, apiLogout, token, test.contentType, "",
-			)
-			if status != http.StatusUnsupportedMediaType {
-				t.Errorf("logout: expected 415, got %d", status)
-			}
-
-			status, _ = tserver.getState(token)
-			if status != http.StatusOK {
-				t.Errorf("state after refused logout: expected 200, got %d", status)
-			}
-		})
-	}
-}
-
 func TestAuthNoCookie(t *testing.T) {
 	t.Parallel()
 
@@ -1878,67 +1750,52 @@ 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, at once, each on a new server, because
-// each trial logs in and logins are rate limited.
+// quitRaceTrials times, each on a new server, because 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"
 
-	firstToken := tserver.createSession(nick)
-	observerToken := tserver.createSession("raceobserver")
+	for trial := range quitRaceTrials {
+		tserver := newTestServer(t)
+		firstToken := tserver.createSession(nick)
+		observerToken := tserver.createSession("raceobserver")
 
-	tserver.setPassword(nick)
+		tserver.setPassword(nick)
 
-	tokens := []string{firstToken, tserver.login(nick)}
+		tokens := []string{firstToken, tserver.login(nick)}
 
-	for _, token := range []string{firstToken, observerToken} {
-		tserver.sendCommand(token, map[string]any{
-			commandKey: joinCmd, toKey: "#quitrace",
-		})
-	}
-
-	_, lastID := tserver.pollMessages(observerToken, 0)
-
-	leave(tserver, tokens)
-
-	msgs, _ := tserver.pollMessages(observerToken, lastID)
-
-	quits := countQuits(msgs, nick)
-	if quits != 1 {
-		t.Errorf("%d QUITs, want 1: %v", quits, msgs)
-	}
-
-	for _, token := range tokens {
-		status, _ := tserver.getState(token)
-		if status != http.StatusUnauthorized {
-			t.Errorf("state got %d, want 401", status)
+		for _, token := range []string{firstToken, observerToken} {
+			tserver.sendCommand(token, map[string]any{
+				commandKey: joinCmd, toKey: "#quitrace",
+			})
 		}
-	}
 
-	// createSession fails the test if the nick is taken.
-	tserver.createSession(nick)
+		_, lastID := tserver.pollMessages(observerToken, 0)
+
+		leave(tserver, tokens)
+
+		msgs, _ := tserver.pollMessages(observerToken, lastID)
+
+		quits := countQuits(msgs, nick)
+		if quits != 1 {
+			t.Errorf("trial %d: %d QUITs, want 1: %v", trial, 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)
+			}
+		}
+
+		// createSession fails the test if the nick is taken.
+		tserver.createSession(nick)
+	}
 }
 
 // TestLogoutOfOneClientKeepsSession checks that when one of
@@ -2076,28 +1933,11 @@ const (
 	// return once the session has left.
 	quitWakeDeadline = 5 * time.Second
 
-	// pollWaitStep is how often waitForLongPoll checks
+	// pollWaitStep is how often checkQuitWakesPoll 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.
@@ -2172,7 +2012,15 @@ func checkQuitWakesPoll(
 		msgs, pollErr = tserver.longPoll(peerToken, lastID, quitWakePollTimeout)
 	}()
 
-	tserver.waitForLongPoll(peerID)
+	deadline := time.Now().Add(quitWakeDeadline)
+
+	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)
 
@@ -3002,12 +2850,7 @@ func TestLongPoll(t *testing.T) {
 		pollMsgs = result.Messages
 	}()
 
-	bobID, err := tserver.database.GetSessionByNick(t.Context(), "lp_bob")
-	if err != nil {
-		t.Fatalf("get session of lp_bob: %v", err)
-	}
-
-	tserver.waitForLongPoll(bobID)
+	time.Sleep(200 * time.Millisecond)
 
 	tserver.sendCommand(aliceToken, map[string]any{
 		commandKey: privmsgCmd,
@@ -3166,18 +3009,18 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 		longTimeout = 10
 
 		// shortTimeout, in seconds, is the timeout of the
-		// request that waits it out.
+		// requests that wait it out.
 		shortTimeout = 1
 	)
 
 	// queued is whether a message is queued before the request,
-	// whose timeout is in seconds. failingRead and notified are
-	// as for getMessagesFailingRead. wantReads is how many times
-	// the handler reads the queue: once if it answers at once,
+	// whose timeout is in seconds. failingRead is as for
+	// getMessagesFailingRead. wantReads is how many times the
+	// handler reads the queue: once if it answers at once,
 	// twice if it waits first.
 	tests := []struct {
 		name                  string
-		queued, notified      bool
+		queued                bool
 		timeout, failingRead  int
 		wantReads, wantStatus int
 	}{
@@ -3198,9 +3041,9 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 			wantReads: 1, wantStatus: http.StatusInternalServerError,
 		},
 		{
-			name: "read after the wait failed", notified: true,
-			timeout: handlers.MaxLongPollTimeout, failingRead: 2,
-			wantReads: 2, wantStatus: http.StatusInternalServerError,
+			name: "read after the wait failed", timeout: shortTimeout,
+			failingRead: 2, wantReads: 2,
+			wantStatus: http.StatusInternalServerError,
 		},
 	}
 
@@ -3232,7 +3075,7 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 			}
 
 			status, reads := tserver.getMessagesFailingRead(
-				token, sessionID, lastID, tc.timeout, tc.failingRead, tc.notified,
+				token, lastID, tc.timeout, tc.failingRead,
 			)
 			if status != tc.wantStatus {
 				t.Errorf("status %d, want %d", status, tc.wantStatus)
@@ -3249,17 +3092,14 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 }
 
 // getMessagesFailingRead sends GET /api/v1/messages for the
-// client token of the session sessionID, with timeout and
-// after afterID. The handler's read of the queue numbered
-// failingRead, counting from 1, fails; with 0, none does.
-// With notified, it notifies the broker once the request
-// waits. It returns the response status and how many times
-// the handler read the queue.
+// client token with timeout and after afterID. The handler's
+// read of the queue numbered failingRead, counting from 1,
+// fails; with 0, none does. It returns the response status
+// and how many times the handler read the queue.
 func (tserver *testServer) getMessagesFailingRead(
 	token string,
-	sessionID, afterID int64,
+	afterID int64,
 	timeout, failingRead int,
-	notified bool,
 ) (int, int) {
 	tserver.t.Helper()
 
@@ -3281,45 +3121,20 @@ func (tserver *testServer) getMessagesFailingRead(
 		)
 	})
 
-	var (
-		status int
-		err    error
+	resp, err := doRequestAuth(
+		tserver.t, http.MethodGet,
+		tserver.url(fmt.Sprintf(
+			"%s?timeout=%d&after=%d", apiMessages, timeout, afterID,
+		)),
+		token, nil,
 	)
-
-	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)
 	}
 
-	return status, reads
+	_ = resp.Body.Close()
+
+	return resp.StatusCode, reads
 }
 
 // TestLongPollRemovesWaitChannelWhenClientGivesUp checks that
@@ -4466,116 +4281,6 @@ func TestLoginRateLimitExceeded(t *testing.T) {
 	}
 }
 
-func TestLoginRateLimitIgnoresForwardedHeaders(t *testing.T) {
-	t.Parallel()
-
-	// The test client connects from 127.0.0.1, which is outside the
-	// default trusted proxies, so the server must ignore these headers.
-	tests := []struct {
-		name   string
-		header string
-	}{
-		{name: "X-Forwarded-For", header: "X-Forwarded-For"},
-		{name: "X-Real-IP", header: "X-Real-IP"},
-	}
-
-	loginBody, err := json.Marshal(map[string]string{
-		nickKey: "unknownnick", passwordKey: "wrongpassword",
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
-			tserver := newTestServer(t)
-
-			// The default burst is 5 attempts, so the sixth gets 429
-			// although each claims a new address.
-			var status int
-
-			for attempt := range 6 {
-				request, rErr := http.NewRequestWithContext(
-					t.Context(), http.MethodPost,
-					tserver.url("/api/v1/login"),
-					bytes.NewReader(loginBody),
-				)
-				if rErr != nil {
-					t.Fatal(rErr)
-				}
-
-				request.Header.Set("Content-Type", "application/json")
-				request.Header.Set(
-					test.header, fmt.Sprintf("203.0.113.%d", attempt+1),
-				)
-
-				resp, dErr := http.DefaultClient.Do(request)
-				if dErr != nil {
-					t.Fatal(dErr)
-				}
-
-				status = resp.StatusCode
-				_ = resp.Body.Close()
-			}
-
-			if status != http.StatusTooManyRequests {
-				t.Fatalf("sixth attempt got %d, want 429", status)
-			}
-		})
-	}
-}
-
-func TestLoginRateLimitPerClientBehindTrustedProxy(t *testing.T) {
-	t.Parallel()
-
-	// 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.
-	loginFrom := func(forwardedFor string) int {
-		t.Helper()
-
-		request, err := http.NewRequestWithContext(
-			t.Context(), http.MethodPost,
-			tserver.url("/api/v1/login"), strings.NewReader("{}"),
-		)
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		request.Header.Set("Content-Type", "application/json")
-		request.Header.Set("X-Forwarded-For", forwardedFor)
-
-		resp, err := http.DefaultClient.Do(request)
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		_ = resp.Body.Close()
-
-		return resp.StatusCode
-	}
-
-	// The default burst is 5 attempts per address.
-	for range 5 {
-		loginFrom("203.0.113.1")
-	}
-
-	status := loginFrom("203.0.113.1")
-	if status != http.StatusTooManyRequests {
-		t.Errorf("sixth attempt from 203.0.113.1 got %d, want 429", status)
-	}
-
-	status = loginFrom("203.0.113.2")
-	if status == http.StatusTooManyRequests {
-		t.Error("first attempt from 203.0.113.2 got 429")
-	}
-}
-
 func TestLoginRateLimitAllowsNormalUse(t *testing.T) {
 	t.Parallel()
 
@@ -5263,9 +4968,7 @@ func TestRefusedSessionLeavesStampUnspent(t *testing.T) {
 		badNick = "bad nick"
 	)
 
-	tserver := newTestServerWith(
-		t, bits, "", "", config.DefaultTrustedProxies,
-	)
+	tserver := newTestServerWith(t, bits, "", "")
 
 	// A server with no SERVER_NAME takes stamps for "neoirc".
 	status := postSessionWithStamp(
@@ -5471,7 +5174,6 @@ func newTestServerWithOper(
 
 	return newTestServerWith(
 		t, 0, testOperName, testOperPassword,
-		config.DefaultTrustedProxies,
 	)
 }
 

internal/handlers/export_test.go

@@ -6,10 +6,6 @@ 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,10 +70,6 @@ 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.
@@ -87,7 +83,6 @@ func newConn(
 	brk *broker.Broker,
 	cfg *config.Config,
 	svc *service.Service,
-	poll time.Duration,
 ) *Conn {
 	host, _, _ := net.SplitHostPort(tcpConn.RemoteAddr().String())
 
@@ -106,8 +101,6 @@ 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, deadline and poll in place of
-// pingInterval, pongDeadline and pollInterval, and resolve in
-// place of resolveHost.
+// connections use interval and deadline in place of
+// pingInterval and pongDeadline, and resolve in 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, poll time.Duration,
+	interval, deadline time.Duration,
 	resolve func(ctx context.Context, addr string) string,
 ) *Server {
 	globs := &globals.Globals{
@@ -46,18 +46,10 @@ 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 {
@@ -107,6 +99,10 @@ 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(
@@ -115,7 +111,7 @@ func NewTestConn(
 	tcpConn net.Conn,
 	nick string,
 ) *Conn {
-	conn := newConn(tcpConn, log, nil, nil, cfg, nil, pollInterval)
+	conn := newConn(tcpConn, log, nil, nil, cfg, nil)
 	conn.nick = nick
 
 	return conn

internal/ircserver/integration_test.go

@@ -3,6 +3,7 @@ package ircserver_test
 import (
 	"strings"
 	"testing"
+	"time"
 
 	"sneak.berlin/go/neoirc/internal/config"
 )
@@ -1507,6 +1508,9 @@ 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(c.pollInterval)
+	ticker := time.NewTicker(pollInterval)
 	defer ticker.Stop()
 
 	for {

internal/ircserver/relay_test.go

@@ -9,7 +9,6 @@ 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"
@@ -59,11 +58,7 @@ func (env *testEnv) queueMessage(
 func TestRelayKeepsOneWaitChannel(t *testing.T) {
 	t.Parallel()
 
-	env := newTestEnvWithResolver(
-		t, &config.Config{ServerName: testServerName},
-		ircserver.PingInterval, ircserver.PongDeadline,
-		ircserver.PollInterval, ircserver.ResolveHost,
-	)
+	env := newTestEnv(t)
 
 	client := env.dial(t)
 	client.register("idle")
@@ -124,63 +119,6 @@ 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,11 +53,6 @@ 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
@@ -84,8 +79,6 @@ func New(
 
 		pingInterval: pingInterval,
 		pongDeadline: pongDeadline,
-		pollInterval: pollInterval,
-		stopTimeout:  stopTimeout,
 		resolveHost:  resolveHost,
 	}
 
@@ -161,10 +154,10 @@ func (s *Server) stop() {
 
 	select {
 	case <-done:
-	case <-time.After(s.stopTimeout):
+	case <-time.After(stopTimeout):
 		s.log.Error(
 			"irc connections not cleaned up before stop timeout",
-			"timeout", s.stopTimeout,
+			"timeout", stopTimeout,
 		)
 	}
 }
@@ -188,7 +181,7 @@ func (s *Server) acceptLoop(ctx context.Context) {
 
 		client := newConn(
 			tcpConn, s.log,
-			s.database, s.brk, s.cfg, s.svc, s.pollInterval,
+			s.database, s.brk, s.cfg, s.svc,
 		)
 
 		s.mu.Lock()

internal/ircserver/server_test.go

@@ -18,7 +18,6 @@ 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"
@@ -31,16 +30,7 @@ import (
 	_ "modernc.org/sqlite"
 )
 
-// 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
+const testTimeout = 5 * time.Second
 
 // testEnv holds the shared test infrastructure.
 type testEnv struct {
@@ -101,19 +91,16 @@ func newTestEnvWithPingTimes(
 	t.Helper()
 
 	return newTestEnvWithResolver(
-		t, cfg, interval, deadline, testPollInterval,
-		ircserver.ResolveHost,
+		t, cfg, interval, deadline, ircserver.ResolveHost,
 	)
 }
 
 // newTestEnvWithResolver is newTestEnvWithPingTimes, except
-// 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.
+// that the server looks up a client's hostname with resolve.
 func newTestEnvWithResolver(
 	t *testing.T,
 	cfg *config.Config,
-	interval, deadline, poll time.Duration,
+	interval, deadline time.Duration,
 	resolve func(ctx context.Context, addr string) string,
 ) *testEnv {
 	t.Helper()
@@ -129,9 +116,8 @@ func newTestEnvWithResolver(
 	}
 
 	database, err := db.New(lifecycle, db.Params{
-		Logger:       dbLog,
-		Config:       &config.Config{DBURL: dbURL},
-		PasswordCost: bcrypt.MinCost,
+		Logger: dbLog,
+		Config: &config.Config{DBURL: dbURL},
 	})
 	if err != nil {
 		t.Fatalf("new database: %v", err)
@@ -148,7 +134,7 @@ func newTestEnvWithResolver(
 	))
 
 	srv := ircserver.NewTestServer(
-		log, cfg, database, brk, interval, deadline, poll, resolve,
+		log, cfg, database, brk, interval, deadline, resolve,
 	)
 
 	// Port 0 makes the system pick a free port. dial reads it
@@ -444,8 +430,7 @@ func TestRegistrationWhileAnotherLookupIsPending(t *testing.T) {
 
 	env := newTestEnvWithResolver(
 		t, &config.Config{ServerName: testServerName},
-		ircserver.PingInterval, ircserver.PongDeadline,
-		testPollInterval, resolve,
+		ircserver.PingInterval, ircserver.PongDeadline, resolve,
 	)
 
 	env.dial(t)
@@ -568,49 +553,27 @@ func TestQuietClientAnsweringPingStaysConnected(t *testing.T) {
 func TestQuietClientNotAnsweringPingIsClosed(t *testing.T) {
 	t.Parallel()
 
-	// A read deadline of two ping intervals would close the
-	// 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.
+	// latest leaves room for a loaded host, but is sooner than
+	// a read deadline of two ping intervals would close it.
 	const (
-		interval = 4 * time.Second
-		earliest = interval + testPongDeadline
-		latest   = 2*interval - time.Second
+		earliest = testPingInterval + testPongDeadline
+		latest   = 2 * testPingInterval
 	)
 
-	env := newTestEnvWithPingTimes(
-		t,
-		&config.Config{ServerName: testServerName},
-		interval, testPongDeadline,
-	)
+	env := newTestEnvWithShortPings(t)
 	client := env.dial(t)
 
 	lastSent := time.Now()
 
 	client.register("silent")
-
-	// 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.readServerPing(lastSent)
 	client.readUntilClosed()
 
 	waited := time.Since(lastSent)
-	if waited < earliest {
+	if waited < earliest || waited >= latest {
 		t.Errorf(
-			"closed %v after the client's last line, want at least %v",
-			waited, earliest,
-		)
-	}
-
-	sinceRegistered := time.Since(registered)
-	if sinceRegistered >= latest {
-		t.Errorf(
-			"closed %v after registration, want less than %v",
-			sinceRegistered, latest,
+			"closed %v after the client's last line, want %v to %v",
+			waited, earliest, latest,
 		)
 	}
 }
@@ -1076,16 +1039,12 @@ func TestStopEndsSessions(t *testing.T) {
 }
 
 // TestStopReturnsWhileCleanupIsStuck checks that Stop stops
-// waiting for a connection's cleanup after its stop timeout,
-// so a cleanup that cannot finish does not hold up stopping.
+// waiting for a connection's cleanup after StopTimeout, 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")
@@ -1111,7 +1070,7 @@ func TestStopReturnsWhileCleanupIsStuck(t *testing.T) {
 
 	select {
 	case <-stopped:
-	case <-time.After(stopTimeout + testTimeout):
+	case <-time.After(ircserver.StopTimeout + testTimeout):
 		t.Error("Stop did not return while a cleanup was stuck")
 	}
 
@@ -1163,6 +1122,9 @@ 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

@@ -3,12 +3,8 @@ package middleware
 
 import (
 	"log/slog"
-	"mime"
 	"net"
 	"net/http"
-	"net/netip"
-	"slices"
-	"strings"
 	"time"
 
 	basicauth "github.com/99designs/basicauth-go"
@@ -19,7 +15,6 @@ import (
 	"github.com/slok/go-http-metrics/middleware/std"
 	"github.com/spf13/viper"
 	"go.uber.org/fx"
-	"go4.org/netipx"
 	"sneak.berlin/go/neoirc/internal/config"
 	"sneak.berlin/go/neoirc/internal/globals"
 	"sneak.berlin/go/neoirc/internal/logger"
@@ -67,77 +62,6 @@ func ipFromHostPort(hostPort string) string {
 	return host
 }
 
-// ClientIP returns middleware that replaces request.RemoteAddr with the
-// client's address and keeps the port of the TCP connection. The
-// X-Forwarded-For and X-Real-IP headers count only when the connection
-// comes from an address in TRUSTED_PROXIES.
-func (mware *Middleware) ClientIP() func(http.Handler) http.Handler {
-	trusted := mware.params.Config.TrustedProxies
-
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(
-			func(
-				writer http.ResponseWriter,
-				request *http.Request,
-			) {
-				peer, err := netip.ParseAddrPort(request.RemoteAddr)
-				if err == nil {
-					client := clientAddr(
-						peer.Addr().Unmap(), request.Header, trusted,
-					)
-					request.RemoteAddr = netip.AddrPortFrom(
-						client, peer.Port(),
-					).String()
-				}
-
-				next.ServeHTTP(writer, request)
-			})
-	}
-}
-
-// clientAddr returns the client's address for a request from peer. A
-// peer outside trusted is the client, whatever its headers say. A
-// trusted peer is a proxy. Its X-Forwarded-For list is read from the
-// right, past the trusted addresses, to the first untrusted one; when
-// all are trusted, the client is the leftmost. Without X-Forwarded-For,
-// the client is X-Real-IP. An entry that is not an IP address gives
-// peer.
-func clientAddr(
-	peer netip.Addr, header http.Header, trusted *netipx.IPSet,
-) netip.Addr {
-	if !trusted.Contains(peer) {
-		return peer
-	}
-
-	forwarded := strings.Join(header.Values("X-Forwarded-For"), ",")
-	if forwarded == "" {
-		realIP, err := netip.ParseAddr(
-			strings.TrimSpace(header.Get("X-Real-IP")),
-		)
-		if err != nil {
-			return peer
-		}
-
-		return realIP.Unmap()
-	}
-
-	client := peer
-
-	for _, entry := range slices.Backward(strings.Split(forwarded, ",")) {
-		addr, err := netip.ParseAddr(strings.TrimSpace(entry))
-		if err != nil {
-			return peer
-		}
-
-		client = addr.Unmap()
-		if !trusted.Contains(client) {
-			break
-		}
-	}
-
-	return client
-}
-
 type loggingResponseWriter struct {
 	http.ResponseWriter
 
@@ -201,74 +125,28 @@ func (mware *Middleware) Logging() func(http.Handler) http.Handler {
 	}
 }
 
-// CORS returns middleware that lets a page on any origin read the
-// response. Use it only on public, read-only routes.
+// CORS returns middleware that handles Cross-Origin Resource Sharing.
+// AllowCredentials is true so browsers include cookies in
+// cross-origin API requests.
 func (mware *Middleware) CORS() func(http.Handler) http.Handler {
 	return cors.Handler(cors.Options{ //nolint:exhaustruct // optional fields
-		AllowedOrigins: []string{"*"},
+		AllowOriginFunc: func(
+			_ *http.Request, _ string,
+		) bool {
+			return true
+		},
 		AllowedMethods: []string{
 			"GET", "POST", "PUT", "DELETE", "OPTIONS",
 		},
 		AllowedHeaders: []string{
-			"Accept", "Authorization", "Content-Type", "X-CSRF-Token",
+			"Accept", "Content-Type", "X-CSRF-Token",
 		},
 		ExposedHeaders:   []string{"Link"},
-		AllowCredentials: false,
+		AllowCredentials: true,
 		MaxAge:           corsMaxAge,
 	})
 }
 
-// notJSONError is the body of the 415 answer, in the API's usual
-// error format.
-const notJSONError = `{"error":"content type must be application/json"}` + "\n"
-
-// needsJSON reports whether a request with method must have the
-// Content-Type application/json, whether or not it has a body.
-func needsJSON(method string) bool {
-	return method == http.MethodPost ||
-		method == http.MethodPut ||
-		method == http.MethodDelete
-}
-
-// isJSON reports whether the request's Content-Type is
-// application/json, with or without parameters such as charset.
-func isJSON(request *http.Request) bool {
-	mediaType, _, err := mime.ParseMediaType(
-		request.Header.Get("Content-Type"),
-	)
-
-	return err == nil && mediaType == "application/json"
-}
-
-// RequireJSON returns middleware that answers 415 to a POST, PUT or
-// DELETE request whose Content-Type is not application/json, or that
-// has none. A browser sends a request with another type, such as
-// text/plain, to another origin of the same site without asking the
-// server first, and attaches the auth cookie. Refusing those types
-// stops a page on that origin from acting as the signed-in user.
-func (mware *Middleware) RequireJSON() func(http.Handler) http.Handler {
-	return func(next http.Handler) http.Handler {
-		return http.HandlerFunc(
-			func(
-				writer http.ResponseWriter,
-				request *http.Request,
-			) {
-				if needsJSON(request.Method) && !isJSON(request) {
-					writer.Header().Set(
-						"Content-Type",
-						"application/json; charset=utf-8",
-					)
-					writer.WriteHeader(http.StatusUnsupportedMediaType)
-					_, _ = writer.Write([]byte(notJSONError))
-
-					return
-				}
-
-				next.ServeHTTP(writer, request)
-			})
-	}
-}
-
 // Metrics returns middleware that records HTTP metrics.
 func (mware *Middleware) Metrics() func(http.Handler) http.Handler {
 	metricsMiddleware := ghmm.New(ghmm.Config{ //nolint:exhaustruct // optional fields

internal/middleware/middleware_test.go

@@ -1,256 +0,0 @@
-package middleware_test
-
-import (
-	"net/http"
-	"net/http/httptest"
-	"net/netip"
-	"testing"
-	"time"
-
-	"go4.org/netipx"
-	"sneak.berlin/go/neoirc/internal/config"
-	"sneak.berlin/go/neoirc/internal/globals"
-	"sneak.berlin/go/neoirc/internal/logger"
-	"sneak.berlin/go/neoirc/internal/middleware"
-)
-
-// The tests trust 10.0.0.0/8. Clients use the documentation ranges
-// 192.0.2.0/24, 198.51.100.0/24 and 203.0.113.0/24.
-const (
-	trustedNetwork = "10.0.0.0/8"
-	proxyPeer      = "10.0.0.1:4000"
-	directPeer     = "192.0.2.1:4000"
-	client         = "203.0.113.7"
-	clientWithPort = "203.0.113.7:4000"
-	realIPClient   = "203.0.113.8"
-)
-
-type clientIPTest struct {
-	name       string
-	remoteAddr string
-	forwarded  []string
-	realIP     string
-	want       string
-}
-
-func clientIPTests() []clientIPTest {
-	return []clientIPTest{
-		{
-			name:       "untrusted peer ignores forged headers",
-			remoteAddr: directPeer,
-			forwarded:  []string{client},
-			realIP:     realIPClient,
-			want:       directPeer,
-		},
-		{name: "trusted peer without headers", remoteAddr: proxyPeer, want: proxyPeer},
-		{
-			name:       "trusted peer gives rightmost untrusted entry",
-			remoteAddr: proxyPeer,
-			forwarded:  []string{"198.51.100.9, 203.0.113.7, 10.0.0.2"},
-			want:       clientWithPort,
-		},
-		{
-			name:       "entries split over several headers",
-			remoteAddr: proxyPeer,
-			forwarded:  []string{"198.51.100.9", client, "10.0.0.2"},
-			want:       clientWithPort,
-		},
-		{
-			name:       "all-trusted chain gives leftmost entry",
-			remoteAddr: proxyPeer,
-			forwarded:  []string{"10.0.0.3, 10.0.0.2"},
-			want:       "10.0.0.3:4000",
-		},
-		{
-			name:       "X-Real-IP without X-Forwarded-For",
-			remoteAddr: proxyPeer,
-			realIP:     realIPClient,
-			want:       "203.0.113.8:4000",
-		},
-		{
-			name:       "X-Forwarded-For wins over X-Real-IP",
-			remoteAddr: proxyPeer,
-			forwarded:  []string{client},
-			realIP:     realIPClient,
-			want:       clientWithPort,
-		},
-		{
-			name:       "unparseable X-Forwarded-For entry gives peer",
-			remoteAddr: proxyPeer,
-			forwarded:  []string{"203.0.113.7, not-an-ip, 10.0.0.2"},
-			want:       proxyPeer,
-		},
-		{
-			name:       "unparseable entry past the client is not read",
-			remoteAddr: proxyPeer,
-			forwarded:  []string{"not-an-ip, 203.0.113.7"},
-			want:       clientWithPort,
-		},
-		{
-			name:       "unparseable X-Real-IP gives peer",
-			remoteAddr: proxyPeer,
-			realIP:     "not-an-ip",
-			want:       proxyPeer,
-		},
-		{
-			name:       "mapped IPv6 peer is unmapped and trusted",
-			remoteAddr: "[::ffff:10.0.0.1]:4000",
-			forwarded:  []string{client},
-			want:       clientWithPort,
-		},
-		{
-			name:       "mapped IPv6 peer outside the set",
-			remoteAddr: "[::ffff:192.0.2.1]:4000",
-			forwarded:  []string{client},
-			want:       directPeer,
-		},
-		{
-			name:       "IPv6 client",
-			remoteAddr: proxyPeer,
-			forwarded:  []string{"2001:db8::1"},
-			want:       "[2001:db8::1]:4000",
-		},
-	}
-}
-
-func TestClientIP(t *testing.T) {
-	t.Parallel()
-
-	mware := newMiddleware(t, trustedNetwork)
-
-	for _, test := range clientIPTests() {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
-			var got string
-
-			handler := mware.ClientIP()(http.HandlerFunc(func(
-				_ http.ResponseWriter, request *http.Request,
-			) {
-				got = request.RemoteAddr
-			}))
-
-			request := httptest.NewRequestWithContext(
-				t.Context(), http.MethodGet, "/", nil,
-			)
-			request.RemoteAddr = test.remoteAddr
-
-			for _, value := range test.forwarded {
-				request.Header.Add("X-Forwarded-For", value)
-			}
-
-			if test.realIP != "" {
-				request.Header.Set("X-Real-IP", test.realIP)
-			}
-
-			handler.ServeHTTP(httptest.NewRecorder(), request)
-
-			if got != test.want {
-				t.Errorf("RemoteAddr = %q, want %q", got, test.want)
-			}
-		})
-	}
-}
-
-func TestRequireJSON(t *testing.T) {
-	t.Parallel()
-
-	mware := newMiddleware(t, trustedNetwork)
-
-	const (
-		jsonType = "application/json"
-		textType = "text/plain"
-		refused  = http.StatusUnsupportedMediaType
-	)
-
-	tests := []struct {
-		method, contentType string
-		want                int
-	}{
-		{http.MethodPost, jsonType, http.StatusOK},
-		{http.MethodPost, "application/json; charset=utf-8", http.StatusOK},
-		{http.MethodPost, "Application/JSON", http.StatusOK},
-		{http.MethodPost, textType, refused},
-		{http.MethodPost, "application/x-www-form-urlencoded", refused},
-		{http.MethodPost, "multipart/form-data; boundary=x", refused},
-		{http.MethodPost, "", refused},
-		{http.MethodPost, "application/json; charset", refused},
-		{http.MethodPut, textType, refused},
-		{http.MethodPut, jsonType, http.StatusOK},
-		{http.MethodDelete, "", refused},
-		{http.MethodDelete, jsonType, http.StatusOK},
-		{http.MethodGet, "", http.StatusOK},
-		{http.MethodGet, textType, http.StatusOK},
-	}
-
-	for _, test := range tests {
-		t.Run(test.method+" "+test.contentType, func(t *testing.T) {
-			t.Parallel()
-
-			handler := mware.RequireJSON()(http.HandlerFunc(func(
-				http.ResponseWriter, *http.Request,
-			) {
-			}))
-
-			request := httptest.NewRequestWithContext(
-				t.Context(), test.method, "/", nil,
-			)
-			if test.contentType != "" {
-				request.Header.Set("Content-Type", test.contentType)
-			}
-
-			recorder := httptest.NewRecorder()
-
-			handler.ServeHTTP(recorder, request)
-
-			if recorder.Code != test.want {
-				t.Fatalf("status %d, want %d", recorder.Code, test.want)
-			}
-
-			if test.want == http.StatusOK {
-				return
-			}
-
-			want := `{"error":"content type must be application/json"}` + "\n"
-			if recorder.Body.String() != want {
-				t.Errorf("body %q, want %q", recorder.Body.String(), want)
-			}
-		})
-	}
-}
-
-// newMiddleware returns a Middleware that trusts the network trusted.
-func newMiddleware(t *testing.T, trusted string) *middleware.Middleware {
-	t.Helper()
-
-	var builder netipx.IPSetBuilder
-
-	builder.AddPrefix(netip.MustParsePrefix(trusted))
-
-	trustedSet, err := builder.IPSet()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	globs := &globals.Globals{
-		Appname:   "neoirc-test",
-		Version:   "test",
-		StartTime: time.Now(),
-	}
-
-	log, err := logger.New(nil, logger.Params{Globals: globs})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	mware, err := middleware.New(nil, middleware.Params{
-		Logger:  log,
-		Globals: globs,
-		Config:  &config.Config{TrustedProxies: trustedSet},
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	return mware
-}

internal/server/routes.go

@@ -23,13 +23,13 @@ func (srv *Server) SetupRoutes() {
 
 	srv.router.Use(middleware.Recoverer)
 	srv.router.Use(middleware.RequestID)
-	srv.router.Use(srv.mw.ClientIP())
 	srv.router.Use(srv.mw.Logging())
 
 	if viper.GetString("METRICS_USERNAME") != "" {
 		srv.router.Use(srv.mw.Metrics())
 	}
 
+	srv.router.Use(srv.mw.CORS())
 	srv.router.Use(srv.mw.CSP())
 	srv.router.Use(middleware.Timeout(routeTimeout))
 
@@ -44,7 +44,7 @@ func (srv *Server) SetupRoutes() {
 	}
 
 	// Health check.
-	srv.router.With(srv.mw.CORS()).Get(
+	srv.router.Get(
 		"/.well-known/healthcheck.json",
 		srv.handlers.HandleHealthCheck(),
 	)
@@ -68,9 +68,7 @@ func (srv *Server) SetupRoutes() {
 }
 
 func (srv *Server) setupAPIv1(router chi.Router) {
-	router.Use(srv.mw.RequireJSON())
-
-	router.With(srv.mw.CORS()).Get(
+	router.Get(
 		"/server",
 		srv.handlers.HandleServerInfo(),
 	)

internal/server/server_test.go

@@ -6,10 +6,8 @@ package server_test
 
 import (
 	"fmt"
-	"maps"
 	"net"
 	"net/http"
-	"net/http/httptest"
 	"strconv"
 	"strings"
 	"testing"
@@ -190,88 +188,6 @@ 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,3 +1,7 @@
+// Tests use a global viper instance for configuration,
+// making parallel execution unsafe.
+//
+//nolint:paralleltest
 package service_test
 
 import (
@@ -55,12 +59,28 @@ func newTestEnv(t *testing.T) *testEnv {
 				}
 			},
 			logger.New,
-			func() *config.Config {
-				return &config.Config{
-					DBURL:        dbURL,
-					OperName:     "admin",
-					OperPassword: "secret",
+			func(
+				lifecycle fx.Lifecycle,
+				globs *globals.Globals,
+				log *logger.Logger,
+			) (*config.Config, error) {
+				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,
@@ -116,8 +136,6 @@ 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)
@@ -128,8 +146,6 @@ func TestEachTestEnvHasItsOwnDatabase(t *testing.T) {
 }
 
 func TestFanOut(t *testing.T) {
-	t.Parallel()
-
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -157,8 +173,6 @@ func TestFanOut(t *testing.T) {
 }
 
 func TestJoinChannel(t *testing.T) {
-	t.Parallel()
-
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -199,8 +213,6 @@ func TestJoinChannel(t *testing.T) {
 }
 
 func TestPartChannel(t *testing.T) {
-	t.Parallel()
-
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -301,8 +313,6 @@ 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)
@@ -365,8 +375,6 @@ 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()
 
@@ -418,8 +426,6 @@ 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()
 
@@ -485,8 +491,6 @@ 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()
 
@@ -565,95 +569,81 @@ 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. Each trial runs on a database of its own.
+// last.
 func TestQuitQueuedAheadOfNewNickHoldersJoin(t *testing.T) {
-	t.Parallel()
+	env := newTestEnv(t)
+	ctx := t.Context()
 
 	for trial := range concurrentJoinTrials {
-		t.Run(fmt.Sprintf("trial %d", trial), func(t *testing.T) {
-			t.Parallel()
+		nick := fmt.Sprintf("reused%d", trial)
+		crowd := fmt.Sprintf("#crowd%d", trial)
+		shared := fmt.Sprintf("#shared%d", trial)
+		quitterSID := createSession(ctx, t, env.db, nick)
 
-			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} {
-		_, err := env.svc.JoinChannel(ctx, quitterSID, 0, nick, channel, "")
-		if err != nil {
-			t.Fatalf("join %s: %v", channel, err)
+		for _, channel := range []string{crowd, shared} {
+			_, err := env.svc.JoinChannel(ctx, quitterSID, 0, nick, channel, "")
+			if err != nil {
+				t.Fatalf("join %s: %v", channel, err)
+			}
 		}
-	}
 
-	for member := range quitCrowdSize {
-		memberNick := fmt.Sprintf("crowd%d", member)
-		memberSID := createSession(ctx, t, env.db, memberNick)
+		for member := range quitCrowdSize {
+			memberNick := fmt.Sprintf("crowd%dx%d", trial, member)
+			memberSID := createSession(ctx, t, env.db, memberNick)
 
-		_, err := env.svc.JoinChannel(ctx, memberSID, 0, memberNick, crowd, "")
-		if err != nil {
-			t.Fatalf("join %s: %v", memberNick, err)
+			_, err := env.svc.JoinChannel(ctx, memberSID, 0, memberNick, crowd, "")
+			if err != nil {
+				t.Fatalf("join %s: %v", memberNick, err)
+			}
 		}
-	}
 
-	watcherSID, watcherClientID, _, err := env.db.CreateSession(
-		ctx, watcherNick, watcherNick, "localhost", "127.0.0.1",
-	)
-	if err != nil {
-		t.Fatalf("create session %s: %v", watcherNick, err)
-	}
+		watcherNick := fmt.Sprintf("watcher%d", trial)
 
-	_, err = env.svc.JoinChannel(ctx, watcherSID, 0, watcherNick, shared, "")
-	if err != nil {
-		t.Fatalf("join %s: %v", watcherNick, err)
-	}
-
-	_, lastQueueID, err := env.db.PollMessages(ctx, watcherClientID, 0, 0)
-	if err != nil {
-		t.Fatalf("poll: %v", err)
-	}
-
-	errs := runConcurrently(
-		func() error {
-			env.svc.BroadcastQuit(ctx, quitterSID, nick, "")
-
-			return nil
-		},
-		func() error {
-			return takeNickAndJoin(ctx, env, nick, shared)
-		},
-	)
-	if errs[1] != nil {
-		t.Fatalf("new session: %v", errs[1])
-	}
-
-	msgs, _, err := env.db.PollMessages(
-		ctx, watcherClientID, lastQueueID, 0,
-	)
-	if err != nil {
-		t.Fatalf("poll: %v", err)
-	}
-
-	commands := commandsFrom(msgs, nick)
-	if !slices.Equal(commands, []string{irc.CmdQuit, irc.CmdJoin}) {
-		t.Errorf(
-			"watcher got %v from %s, want QUIT then JOIN",
-			commands, nick,
+		watcherSID, watcherClientID, _, err := env.db.CreateSession(
+			ctx, watcherNick, watcherNick, "localhost", "127.0.0.1",
 		)
+		if err != nil {
+			t.Fatalf("create session %s: %v", watcherNick, err)
+		}
+
+		_, err = env.svc.JoinChannel(ctx, watcherSID, 0, watcherNick, shared, "")
+		if err != nil {
+			t.Fatalf("join %s: %v", watcherNick, err)
+		}
+
+		_, lastQueueID, err := env.db.PollMessages(ctx, watcherClientID, 0, 0)
+		if err != nil {
+			t.Fatalf("poll: %v", err)
+		}
+
+		errs := runConcurrently(
+			func() error {
+				env.svc.BroadcastQuit(ctx, quitterSID, nick, "")
+
+				return nil
+			},
+			func() error {
+				return takeNickAndJoin(ctx, env, nick, shared)
+			},
+		)
+		if errs[1] != nil {
+			t.Fatalf("trial %d: new session: %v", trial, errs[1])
+		}
+
+		msgs, _, err := env.db.PollMessages(
+			ctx, watcherClientID, lastQueueID, 0,
+		)
+		if err != nil {
+			t.Fatalf("poll: %v", err)
+		}
+
+		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",
+				trial, commands, nick,
+			)
+		}
 	}
 }
 
@@ -701,8 +691,6 @@ 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)
@@ -748,8 +736,6 @@ 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)
@@ -788,8 +774,6 @@ 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)
@@ -837,8 +821,6 @@ 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)
@@ -863,8 +845,6 @@ func TestKickOfLastMemberDeletesChannel(t *testing.T) {
 }
 
 func TestSendChannelMessage(t *testing.T) {
-	t.Parallel()
-
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -912,8 +892,6 @@ func TestSendChannelMessage(t *testing.T) {
 }
 
 func TestBroadcastQuit(t *testing.T) {
-	t.Parallel()
-
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -951,8 +929,6 @@ 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()
 
@@ -984,8 +960,6 @@ 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()
 
@@ -1017,8 +991,6 @@ func TestSendChannelMessageChannelDeleted(t *testing.T) {
 }
 
 func TestSendChannelMessage_Moderated(t *testing.T) {
-	t.Parallel()
-
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -1070,8 +1042,6 @@ 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
@@ -1097,8 +1067,6 @@ 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()
 
@@ -1151,8 +1119,6 @@ func TestRefusedChannelMessageLeavesStampUnspent(t *testing.T) {
 }
 
 func TestQueryUserMode(t *testing.T) {
-	t.Parallel()
-
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -1203,8 +1169,6 @@ func TestQueryUserMode(t *testing.T) {
 }
 
 func TestUserhostReply(t *testing.T) {
-	t.Parallel()
-
 	env := newTestEnv(t)
 	ctx := t.Context()
 
@@ -1310,14 +1274,10 @@ 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),