Refuse API writes that are not application/json (closes #271)

A page on another origin of the same site could send a text/plain or
form POST with the user's cookie, since a browser sends such a request
without a preflight. RequireJSON on the /api/v1 router answers 415 to a
POST, PUT or DELETE whose Content-Type is not application/json, or that
has none, before the handler runs.

The web client and neoirc-cli already send the header on every request.

Model: opus-5-5

README.md

@@ -157,6 +157,10 @@ 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}`.
@@ -168,8 +172,10 @@ either.
 - Request bodies for `POST /api/v1/session`, `POST /api/v1/login` and
   `POST /api/v1/messages` are limited to `MAX_MESSAGE_SIZE` bytes (default
   `4096`). A larger body gets `400` `invalid request body`.
-- The server allows cross-origin requests from any origin, with credentials. The
-  cookie is `SameSite=Strict`, so a browser sends it only from the same site.
+- `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 auth cookie
 
@@ -515,7 +521,8 @@ 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.
+take gets `405` with no body. Under `/api/v1/`, a `POST`, `PUT` or `DELETE`
+without `Content-Type: application/json` gets `415` instead.
 
 ### The message envelope
 
@@ -1722,7 +1729,8 @@ exceptions:
 
 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.
+error names the setting. The same holds for a setting given a list or a map in
+`neoirc.yaml`.
 
 - `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
@@ -1892,14 +1900,14 @@ The IRC listener has no TLS.
   seconds. A request still running after 60 seconds is cancelled, and the
   connection closes without a response. Headers may be up to 1 MiB.
 - A long poll waits at most 30 seconds.
-- Every response to a well-formed request, except to a cross-origin preflight,
-  carries a `Content-Security-Policy` header:
+- Every response to a well-formed request 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.
-- 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.
+- `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.
 
 ### Logging
 
@@ -2003,7 +2011,8 @@ and their sessions to end. Then it closes the database.
 - `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 `Content-Security-Policy` header and metrics.
+  cross-origin requests, the API's `Content-Type` check, the
+  `Content-Security-Policy` header and metrics.
 - `internal/healthcheck/` and `internal/stats/`: the health check and its
   counters.
 - `internal/logger/` and `internal/globals/`: logging setup and build

go.mod

@@ -13,6 +13,7 @@ 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
@@ -41,7 +42,6 @@ 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

internal/config/config.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/spf13/cast"
 	"github.com/spf13/viper"
 	"go.uber.org/fx"
 	"go4.org/netipx"
@@ -43,7 +44,7 @@ var errOutOfRange = errors.New("setting out of range")
 
 var errNeverMatches = errors.New("entry can never match a connection")
 
-var errNotString = errors.New("not a string")
+var errListOrMap = errors.New("a list or a map")
 
 // Params defines the dependencies for creating a Config.
 type Params struct {
@@ -81,8 +82,9 @@ type Config struct {
 }
 
 // New creates a new Config by reading from files and environment variables.
-// It returns an error when a number, true/false, duration or address
-// setting does not parse or is out of range.
+// 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.
 func New(
 	_ fx.Lifecycle, params Params,
 ) (*Config, error) {
@@ -121,27 +123,14 @@ func New(
 	if err != nil {
 		var notFound viper.ConfigFileNotFoundError
 		if !errors.As(err, &notFound) {
-			log.Error("config file malformed", "error", err)
-			panic(err)
+			return nil, fmt.Errorf("cannot read %s: %w",
+				viper.ConfigFileUsed(), err)
 		}
 	}
 
 	cfg := &Config{
-		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,
+		log:    log,
+		params: &params,
 	}
 
 	err = parseSettings(cfg)
@@ -149,13 +138,6 @@ 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()
@@ -164,13 +146,24 @@ func New(
 	return cfg, nil
 }
 
-// parseSettings reads the number, true/false and address 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.
+// 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.
 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),
@@ -178,30 +171,61 @@ func parseSettings(cfg *Config) error {
 		parseFloat("LOGIN_RATE_LIMIT", &cfg.LoginRateLimit),
 		parseBool("DEBUG", &cfg.Debug),
 		parseBool("MAINTENANCE_MODE", &cfg.MaintenanceMode),
-		checkDuration("MESSAGE_MAX_AGE", cfg.MessageMaxAge),
-		checkDuration("QUEUE_MAX_AGE", cfg.QueueMaxAge),
-		checkDuration("SESSION_IDLE_TIMEOUT", cfg.SessionIdleTimeout),
+		parseDuration("MESSAGE_MAX_AGE", &cfg.MessageMaxAge),
+		parseDuration("QUEUE_MAX_AGE", &cfg.QueueMaxAge),
+		parseDuration("SESSION_IDLE_TIMEOUT", &cfg.SessionIdleTimeout),
 		parseTrustedProxies(&cfg.TrustedProxies),
 	)
 }
 
-// parseTrustedProxies sets *value to the addresses in TRUSTED_PROXIES.
-// Viper counts an empty environment variable as unset, so the
-// environment is read directly as well.
-func parseTrustedProxies(value **netipx.IPSet) error {
-	setting := viper.Get("TRUSTED_PROXIES")
-
-	fromEnv, set := os.LookupEnv("TRUSTED_PROXIES")
-	if set {
-		setting = fromEnv
+// 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)
 	}
 
-	// A YAML list or map in the config file is not a string, and
-	// viper.GetString would turn it into the empty string.
-	raw, isString := setting.(string)
-	if !isString {
-		return fmt.Errorf("cannot parse TRUSTED_PROXIES: %v is %w",
-			setting, errNotString)
+	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)
@@ -273,7 +297,10 @@ func addTrustedProxy(builder *netipx.IPSetBuilder, entry string) error {
 // 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 := viper.GetString(key)
+	raw, err := getString(key)
+	if err != nil {
+		return err
+	}
 
 	parsed, err := strconv.Atoi(raw)
 	if err != nil {
@@ -294,7 +321,10 @@ 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 := viper.GetString(key)
+	raw, err := getString(key)
+	if err != nil {
+		return err
+	}
 
 	parsed, err := strconv.ParseFloat(raw, 64)
 	if err != nil {
@@ -314,7 +344,12 @@ 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 {
-	parsed, err := strconv.ParseBool(viper.GetString(key))
+	raw, err := getString(key)
+	if err != nil {
+		return err
+	}
+
+	parsed, err := strconv.ParseBool(raw)
 	if err != nil {
 		return fmt.Errorf("cannot parse %s: %w", key, err)
 	}
@@ -324,18 +359,25 @@ func parseBool(key string, value *bool) error {
 	return nil
 }
 
-// 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)
+// 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)
 	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, value)
+			errOutOfRange, key, raw)
 	}
 
+	*value = raw
+
 	return nil
 }

internal/config/config_test.go

@@ -287,49 +287,153 @@ func TestTrustedProxies(t *testing.T) {
 	}
 }
 
-func TestTrustedProxiesNotAStringInYAMLStopsStartup(t *testing.T) {
+// 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{"[10.0.0.1]", "{proxy: 10.0.0.1}"}
+	values := []string{`[":6667"]`, `{addr: ":6667"}`}
+	keys := []string{"IRC_LISTEN_ADDR", "MOTD", "TRUSTED_PROXIES"}
 
-	for _, value := range values {
-		t.Run(value, func(t *testing.T) {
-			// t.Setenv puts back whatever value was there after the
-			// test, including when Unsetenv removes it.
-			t.Setenv("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 := os.Unsetenv("TRUSTED_PROXIES")
-			if err != nil {
-				t.Fatal(err)
-			}
+				_, err := loadConfig(t)
+				if err == nil || !strings.Contains(err.Error(), key) {
+					t.Errorf("error = %v, want one naming %s", err, key)
+				}
+			})
+		}
+	}
+}
 
-			// loadConfig names the app neoirc-test, so config.New reads
-			// $HOME/.config/neoirc-test/neoirc-test.yaml.
+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")
 
-			dir := filepath.Join(home, ".config", "neoirc-test")
-
-			err = os.MkdirAll(dir, 0o700)
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			err = os.WriteFile(
-				filepath.Join(dir, "neoirc-test.yaml"),
-				[]byte("TRUSTED_PROXIES: "+value+"\n"), 0o600,
-			)
-			if err != nil {
-				t.Fatal(err)
-			}
-
-			// Viper keeps the values it read from this file after the
-			// file is gone. Reset clears them for later tests.
-			t.Cleanup(viper.Reset)
-
-			_, err = loadConfig(t)
-			if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") {
-				t.Errorf("error = %v, want one naming TRUSTED_PROXIES", err)
+			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/db_test.go

@@ -27,7 +27,11 @@ const (
 		(SELECT 1 UNION ALL SELECT x + 1 FROM n)
 		SELECT COUNT(*) FROM n`
 
-	endlessQueryTimeout = 100 * time.Millisecond
+	// endlessQueryTimeout is how long the endless query runs
+	// before its context ends. A query whose context ends before
+	// it starts is never run, and the pool keeps its connection,
+	// so the timeout leaves a loaded host time to start it.
+	endlessQueryTimeout = 2 * time.Second
 )
 
 // TestDefaultDBURLSetsWALAndBusyTimeout opens a file database

internal/handlers/api_test.go

@@ -145,9 +145,19 @@ type testServer struct {
 	handlers   *handlers.Handlers
 }
 
-// maxMessageSize is MAX_MESSAGE_SIZE on every test server, the
-// setting's default.
-const maxMessageSize = 4096
+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
+)
 
 func newTestServer(
 	t *testing.T,
@@ -188,7 +198,7 @@ func newTestServerWith(
 			logger.New,
 			// Built here rather than by config.New, which writes
 			// the global viper instance, so servers start in
-			// parallel. Settings left out take their defaults.
+			// parallel.
 			func() *config.Config {
 				return &config.Config{
 					DBURL: dbURL,
@@ -201,6 +211,8 @@ func newTestServerWith(
 					MessageMaxAge:      thirtyDays,
 					QueueMaxAge:        thirtyDays,
 					SessionIdleTimeout: thirtyDays,
+					LoginRateLimit:     loginRateLimit,
+					LoginRateBurst:     loginRateBurst,
 					TrustedProxies:     trusted,
 				}
 			},
@@ -403,7 +415,7 @@ func doRequestAuth(
 		return nil, fmt.Errorf("new request: %w", err)
 	}
 
-	if body != nil {
+	if method != http.MethodGet {
 		request.Header.Set(
 			"Content-Type", "application/json",
 		)
@@ -891,6 +903,131 @@ 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()
 
@@ -3029,18 +3166,18 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 		longTimeout = 10
 
 		// shortTimeout, in seconds, is the timeout of the
-		// requests that wait it out.
+		// request that waits it out.
 		shortTimeout = 1
 	)
 
 	// queued is whether a message is queued before the request,
-	// whose timeout is in seconds. failingRead is as for
-	// getMessagesFailingRead. wantReads is how many times the
-	// handler reads the queue: once if it answers at once,
+	// 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,
 	// twice if it waits first.
 	tests := []struct {
 		name                  string
-		queued                bool
+		queued, notified      bool
 		timeout, failingRead  int
 		wantReads, wantStatus int
 	}{
@@ -3061,9 +3198,9 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 			wantReads: 1, wantStatus: http.StatusInternalServerError,
 		},
 		{
-			name: "read after the wait failed", timeout: shortTimeout,
-			failingRead: 2, wantReads: 2,
-			wantStatus: http.StatusInternalServerError,
+			name: "read after the wait failed", notified: true,
+			timeout: handlers.MaxLongPollTimeout, failingRead: 2,
+			wantReads: 2, wantStatus: http.StatusInternalServerError,
 		},
 	}
 
@@ -3095,7 +3232,7 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 			}
 
 			status, reads := tserver.getMessagesFailingRead(
-				token, lastID, tc.timeout, tc.failingRead,
+				token, sessionID, lastID, tc.timeout, tc.failingRead, tc.notified,
 			)
 			if status != tc.wantStatus {
 				t.Errorf("status %d, want %d", status, tc.wantStatus)
@@ -3112,14 +3249,17 @@ func TestGetMessagesRemovesWaitChannel(t *testing.T) {
 }
 
 // getMessagesFailingRead sends GET /api/v1/messages for the
-// 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.
+// 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.
 func (tserver *testServer) getMessagesFailingRead(
 	token string,
-	afterID int64,
+	sessionID, afterID int64,
 	timeout, failingRead int,
+	notified bool,
 ) (int, int) {
 	tserver.t.Helper()
 
@@ -3141,20 +3281,45 @@ func (tserver *testServer) getMessagesFailingRead(
 		)
 	})
 
-	resp, err := doRequestAuth(
-		tserver.t, http.MethodGet,
-		tserver.url(fmt.Sprintf(
-			"%s?timeout=%d&after=%d", apiMessages, timeout, afterID,
-		)),
-		token, nil,
+	var (
+		status int
+		err    error
 	)
+
+	answered := make(chan struct{})
+
+	go func() {
+		defer close(answered)
+
+		var resp *http.Response
+
+		resp, err = doRequestAuth(
+			tserver.t, http.MethodGet,
+			tserver.url(fmt.Sprintf(
+				"%s?timeout=%d&after=%d", apiMessages, timeout, afterID,
+			)),
+			token, nil,
+		)
+		if err != nil {
+			return
+		}
+
+		status = resp.StatusCode
+		_ = resp.Body.Close()
+	}()
+
+	if notified {
+		tserver.waitForLongPoll(sessionID)
+		tserver.broker.Notify(sessionID)
+	}
+
+	<-answered
+
 	if err != nil {
 		tserver.t.Fatalf("get messages: %v", err)
 	}
 
-	_ = resp.Body.Close()
-
-	return resp.StatusCode, reads
+	return status, reads
 }
 
 // TestLongPollRemovesWaitChannelWhenClientGivesUp checks that

internal/handlers/export_test.go

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

internal/ircserver/server_test.go

@@ -31,7 +31,10 @@ import (
 	_ "modernc.org/sqlite"
 )
 
-const testTimeout = 5 * time.Second
+// testTimeout is how long a test waits for something before it
+// fails. A passing test never waits this long, so it is generous
+// enough for a heavily loaded host.
+const testTimeout = 30 * time.Second
 
 // testPollInterval is how often the relay of a test server
 // checks the client output queue without a broker
@@ -565,27 +568,49 @@ func TestQuietClientAnsweringPingStaysConnected(t *testing.T) {
 func TestQuietClientNotAnsweringPingIsClosed(t *testing.T) {
 	t.Parallel()
 
-	// latest leaves room for a loaded host, but is sooner than
-	// a read deadline of two ping intervals would close it.
+	// 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.
 	const (
-		earliest = testPingInterval + testPongDeadline
-		latest   = 2 * testPingInterval
+		interval = 4 * time.Second
+		earliest = interval + testPongDeadline
+		latest   = 2*interval - time.Second
 	)
 
-	env := newTestEnvWithShortPings(t)
+	env := newTestEnvWithPingTimes(
+		t,
+		&config.Config{ServerName: testServerName},
+		interval, testPongDeadline,
+	)
 	client := env.dial(t)
 
 	lastSent := time.Now()
 
 	client.register("silent")
-	client.readServerPing(lastSent)
+
+	// The server starts both waits once it has handled the
+	// registration, so the time registration took does not
+	// count against latest.
+	registered := time.Now()
+
+	client.readUntil(isServerPing)
 	client.readUntilClosed()
 
 	waited := time.Since(lastSent)
-	if waited < earliest || waited >= latest {
+	if waited < earliest {
 		t.Errorf(
-			"closed %v after the client's last line, want %v to %v",
-			waited, earliest, latest,
+			"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,
 		)
 	}
 }

internal/middleware/middleware.go

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

@@ -152,6 +152,73 @@ func TestClientIP(t *testing.T) {
 	}
 }
 
+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()

internal/server/routes.go

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

internal/server/server_test.go

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