From 9c8b70ed36ae9b585d56ef74f055f26e92913955 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 10 Nov 2026 21:50:10 +0000 Subject: [PATCH] Stop allowing credentialed cross-origin requests (closes #261) CORS() allowed every origin with credentials, so a page on another origin of the same site could read what the signed-in user sees. It now uses the options of the CONVENTIONS.md template: every origin as *, no credentials. It applies only to GET /api/v1/server and GET /.well-known/healthcheck.json, the public read-only routes, since REPO_POLICIES.md forbids * on authenticated routes. Every other route sends no Access-Control-Allow-* header. Model: opus-5-5 --- README.md | 18 ++++--- internal/middleware/middleware.go | 15 ++---- internal/server/routes.go | 5 +- internal/server/server_test.go | 84 +++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 2b3e501..4f5c114 100644 --- a/README.md +++ b/README.md @@ -168,8 +168,10 @@ either. - Request bodies for `POST /api/v1/session`, `POST /api/v1/login` and `POST /api/v1/messages` are limited to `MAX_MESSAGE_SIZE` bytes (default `4096`). A larger body gets `400` `invalid request body`. -- The server allows cross-origin requests from any origin, with credentials. The - 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 @@ -1893,14 +1895,14 @@ The IRC listener has no TLS. seconds. A request still running after 60 seconds is cancelled, and the connection closes without a response. Headers may be up to 1 MiB. - A long poll waits at most 30 seconds. -- Every response to a well-formed request, except to a cross-origin preflight, - 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 diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 52469ef..a4e67c6 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -200,24 +200,19 @@ func (mware *Middleware) Logging() func(http.Handler) http.Handler { } } -// CORS returns middleware that handles Cross-Origin Resource Sharing. -// 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, }) } diff --git a/internal/server/routes.go b/internal/server/routes.go index 98312d5..82bbac5 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -30,7 +30,6 @@ func (srv *Server) SetupRoutes() { srv.router.Use(srv.mw.Metrics()) } - srv.router.Use(srv.mw.CORS()) srv.router.Use(srv.mw.CSP()) srv.router.Use(middleware.Timeout(routeTimeout)) @@ -45,7 +44,7 @@ func (srv *Server) SetupRoutes() { } // Health check. - srv.router.Get( + srv.router.With(srv.mw.CORS()).Get( "/.well-known/healthcheck.json", srv.handlers.HandleHealthCheck(), ) @@ -69,7 +68,7 @@ func (srv *Server) SetupRoutes() { } func (srv *Server) setupAPIv1(router chi.Router) { - router.Get( + router.With(srv.mw.CORS()).Get( "/server", srv.handlers.HandleServerInfo(), ) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 6a44326..11b27dd 100644 --- a/internal/server/server_test.go +++ b/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(