From 3587ce92cddfcbff17fbaa927bc8d33601855cfb 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 cross-origin requests (closes #261) The CORS middleware let every origin make credentialed requests, so a page on another subdomain of the server's domain could read and act as the signed-in user. Nothing needs cross-origin access, so the middleware and the go-chi/cors dependency go. A test checks that a request from another origin and a preflight get no Access-Control-Allow-* headers. Model: opus-5-5 --- README.md | 19 +++++---- go.mod | 1 - go.sum | 2 - internal/middleware/middleware.go | 25 ------------ internal/server/routes.go | 1 - internal/server/server_test.go | 65 +++++++++++++++++++++++++++++++ 6 files changed, 74 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 85155a9..261d489 100644 --- a/README.md +++ b/README.md @@ -168,8 +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`. -- 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 server allows no cross-origin requests. In a browser, only pages the + server itself serves, such as the web client, can use the API. ### The auth cookie @@ -1892,14 +1892,13 @@ 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. +- The server allows no cross-origin requests. No response carries an + `Access-Control-Allow-*` header. A preflight request gets `405`, like any + other `OPTIONS` request. ### Logging @@ -2002,8 +2001,8 @@ 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 `Content-Security-Policy` header and metrics. +- `internal/middleware/`: finding the client's address, request logging, 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 diff --git a/go.mod b/go.mod index 73baceb..324824a 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,6 @@ require ( github.com/gdamore/tcell/v2 v2.13.8 github.com/getsentry/sentry-go v0.42.0 github.com/go-chi/chi/v5 v5.2.1 - github.com/go-chi/cors v1.2.2 github.com/google/uuid v1.6.0 github.com/joho/godotenv v1.5.1 github.com/prometheus/client_golang v1.23.2 diff --git a/go.sum b/go.sum index e8c87ca..d80662c 100644 --- a/go.sum +++ b/go.sum @@ -20,8 +20,6 @@ github.com/getsentry/sentry-go v0.42.0 h1:eeFMACuZTbUQf90RE8dE4tXeSe4CZyfvR1MBL7 github.com/getsentry/sentry-go v0.42.0/go.mod h1:eRXCoh3uvmjQLY6qu63BjUZnaBu5L5WhMV1RwYO8W5s= github.com/go-chi/chi/v5 v5.2.1 h1:KOIHODQj58PmL80G2Eak4WdvUzjSJSm0vG72crDCqb8= github.com/go-chi/chi/v5 v5.2.1/go.mod h1:L2yAIGWB3H+phAw1NxKwWM+7eUH/lU8pOMm5hHcoops= -github.com/go-chi/cors v1.2.2 h1:Jmey33TE+b+rB7fT8MUy1u0I4L+NARQlK6LhzKPSyQE= -github.com/go-chi/cors v1.2.2/go.mod h1:sSbTewc+6wYHBBCW7ytsFSn836hqM7JxpglAy2Vzc58= github.com/go-errors/errors v1.4.2 h1:J6MZopCL4uSllY1OfXM374weqZFFItUbrImctkmUxIA= github.com/go-errors/errors v1.4.2/go.mod h1:sIVyrIiJhuEF+Pj9Ebtd6P/rEYROXFi3BopGUQ5a5Og= github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 52469ef..a7c47bb 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -12,7 +12,6 @@ import ( basicauth "github.com/99designs/basicauth-go" chimw "github.com/go-chi/chi/v5/middleware" - "github.com/go-chi/cors" metrics "github.com/slok/go-http-metrics/metrics/prometheus" ghmm "github.com/slok/go-http-metrics/middleware" "github.com/slok/go-http-metrics/middleware/std" @@ -24,8 +23,6 @@ import ( "sneak.berlin/go/neoirc/internal/logger" ) -const corsMaxAge = 300 - // Params defines the dependencies for creating Middleware. type Params struct { fx.In @@ -200,28 +197,6 @@ 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. -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 - }, - AllowedMethods: []string{ - "GET", "POST", "PUT", "DELETE", "OPTIONS", - }, - AllowedHeaders: []string{ - "Accept", "Content-Type", "X-CSRF-Token", - }, - ExposedHeaders: []string{"Link"}, - AllowCredentials: true, - MaxAge: corsMaxAge, - }) -} - // 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 diff --git a/internal/server/routes.go b/internal/server/routes.go index 98312d5..bf97492 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)) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 6a44326..efc22aa 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/http" + "net/http/httptest" "strconv" "strings" "testing" @@ -188,6 +189,70 @@ func TestServeAfterStopDoesNotStopApp(t *testing.T) { } } +// TestNoCrossOriginAccess checks that the server allows no +// cross-origin requests: neither a request from another origin +// nor a preflight gets Access-Control-Allow-Origin or +// Access-Control-Allow-Credentials, and a preflight gets 405 +// like any other OPTIONS request. +func TestNoCrossOriginAccess(t *testing.T) { + var srv *server.Server + + startApp(t, fx.Populate(&srv)) + + tests := []struct { + name string + method string + requestMethod string + status int + }{ + { + name: "request", + method: http.MethodGet, + status: http.StatusOK, + }, + { + name: "preflight", + method: http.MethodOptions, + requestMethod: http.MethodPost, + status: http.StatusMethodNotAllowed, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + request := httptest.NewRequestWithContext( + t.Context(), test.method, + "http://chat.example.com/api/v1/server", 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) + + if recorder.Code != test.status { + t.Errorf("status %d, want %d", recorder.Code, test.status) + } + + for _, header := range []string{ + "Access-Control-Allow-Origin", + "Access-Control-Allow-Credentials", + } { + value := recorder.Header().Get(header) + if value != "" { + t.Errorf("%s is %q, want none", header, value) + } + } + }) + } +} + // startApp starts the app newApp builds, with the HTTP port // set to 0, so the system gives the server a free port. func startApp(