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}`.
@@ -517,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
 
@@ -2006,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

internal/handlers/api_test.go

@@ -415,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",
 		)
@@ -903,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()
 

internal/middleware/middleware.go

@@ -3,6 +3,7 @@ package middleware
 
 import (
 	"log/slog"
+	"mime"
 	"net"
 	"net/http"
 	"net/netip"
@@ -217,6 +218,57 @@ func (mware *Middleware) CORS() func(http.Handler) http.Handler {
 	})
 }
 
+// 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

@@ -68,6 +68,8 @@ func (srv *Server) SetupRoutes() {
 }
 
 func (srv *Server) setupAPIv1(router chi.Router) {
+	router.Use(srv.mw.RequireJSON())
+
 	router.With(srv.mw.CORS()).Get(
 		"/server",
 		srv.handlers.HandleServerInfo(),