fix(backend): rate-limit and cap report ingest, drop wildcard CORS (closes #20)

POST /api/v1/reports stays unauthenticated but is bounded. Each client
address, as the trusted-proxy logic resolves it, may send
REPORTS_PER_MINUTE reports a minute (default 60, counted by
go-chi/httprate over a sliding minute); past that it gets 429 with
Retry-After. reportbuf refuses a report that would take the report
files past DATA_DIR_MAX_BYTES (default 1 GiB), counting the files
already in DATA_DIR and unwritten reports at their uncompressed size;
the handler answers 507. CORS adds nothing unless CORS_ALLOWED_ORIGINS
lists origins. A limit that is not a positive number, or an origin
that is not a plain scheme://host[:port], stops the server from
starting.

Model: opus-5-5

TODO.md

@@ -30,8 +30,9 @@ latest run passes.
   `Retry-After`; the report files in `DATA_DIR`, counted from start with those
   already there, may total at most `DATA_DIR_MAX_BYTES` (default 1 GiB), past
   which reports get 507; and the wildcard CORS is gone: no CORS headers unless
-  `CORS_ALLOWED_ORIGINS` lists origins. Deleting report files frees room only at
+  `CORS_ALLOWED_ORIGINS` lists origins, and an entry that is not a plain
-  the next start; pruning is issue #54
+  `scheme://host[:port]` origin, `*` included, stops the server from starting.
+  Deleting report files frees room only at the next start; pruning is issue #54
 - 2026-09-28: one container image (issue #52): the root `Dockerfile` builds the
   only image, and `Dockerfile.backend` is gone. nginx serves the frontend on
   port 8080 and proxies `/api/` and `/.well-known/healthcheck` to the backend,

backend/README.md

@@ -80,7 +80,7 @@ Internal packages in `internal/` follow standard Go project layout:
 | `BIND_ADDRESS`         | empty                | IP address to listen on; empty listens on every interface                                                |
 | `PORT`                 | `8080`               | HTTP listen port                                                                                         |
 | `DATA_DIR`             | `./data/reports`     | Directory for compressed reports                                                                         |
-| `DATA_DIR_MAX_BYTES`   | `1073741824` (1 GiB) | Most the report files in `DATA_DIR` may total; see [Report limits](#report-limits)                       |
+| `DATA_DIR_MAX_BYTES`   | `1073741824` (1 GiB) | Largest total size of the report files in `DATA_DIR`; see [Report limits](#report-limits)                |
 | `DEBUG`                | `false`              | Enable debug logging                                                                                     |
 | `TRUSTED_PROXIES`      | loopback + RFC1918   | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution |
 | `REPORTS_PER_MINUTE`   | `60`                 | Reports each client address may send a minute; see [Report limits](#report-limits)                       |
@@ -136,7 +136,10 @@ The page calls the API from the origin it is served from, so by default the
 server sends no CORS headers, and browsers let no other origin's pages call it.
 To serve the page from elsewhere, list that origin in `CORS_ALLOWED_ORIGINS`
 (for example `https://netwatch.example.com`); pages from a listed origin may
-`GET` and `POST` with a `Content-Type` header.
+`GET` and `POST` with a `Content-Type` header. Each entry must be a plain
+origin, `scheme://host` with an optional `:port`, as browsers send it: no path,
+not even a trailing `/`, and no `*`. Any other entry stops the server from
+starting, with an error naming `CORS_ALLOWED_ORIGINS`.
 
 ## TODO
 

backend/internal/config/config.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"net/url"
 	"strings"
 
 	"sneak.berlin/go/netwatch/internal/globals"
@@ -31,7 +32,12 @@ const (
 	defaultDataDirMaxBytes  = 1 << 30 // 1 GiB
 )
 
-var errNotPositive = errors.New("must be a positive whole number")
+var (
+	errNotPositive = errors.New("must be a positive whole number")
+	errNotOrigin   = errors.New(
+		"must be an origin, scheme://host with an optional port",
+	)
+)
 
 // Params defines the dependencies for Config.
 type Params struct {
@@ -125,6 +131,11 @@ func New(
 			viper.GetString("DATA_DIR_MAX_BYTES"), errNotPositive)
 	}
 
+	err = checkOrigins(s.CORSAllowedOrigins)
+	if err != nil {
+		return nil, err
+	}
+
 	if s.Debug {
 		params.Logger.EnableDebugLogging()
 		s.log = params.Logger.Get()
@@ -133,6 +144,25 @@ func New(
 	return s, nil
 }
 
+// checkOrigins fails on the first CORS_ALLOWED_ORIGINS entry that is
+// not a plain origin, scheme://host with an optional port, as browsers
+// send it; anything more, such as a trailing "/", would match no page.
+// go-chi/cors reads a "*" anywhere in an entry as a wildcard, so no
+// entry may contain one.
+func checkOrigins(origins []string) error {
+	for _, origin := range origins {
+		u, err := url.Parse(origin)
+		if err != nil || u.Scheme == "" || u.Host == "" ||
+			strings.Contains(origin, "*") ||
+			origin != u.Scheme+"://"+u.Host {
+			return fmt.Errorf("CORS_ALLOWED_ORIGINS %q: %w",
+				origin, errNotOrigin)
+		}
+	}
+
+	return nil
+}
+
 // splitList turns a comma-separated setting into a trimmed
 // slice, dropping empty entries.
 func splitList(raw string) []string {

backend/internal/config/config_test.go

@@ -45,3 +45,20 @@ func TestDataDirMaxBytesMustBeANumber(t *testing.T) {
 
 	requireConfigError(t, "DATA_DIR_MAX_BYTES")
 }
+
+// TestCORSAllowedOriginsMustBeOrigins: "*" would let every origin in,
+// and an entry that is not a plain origin would match no page.
+func TestCORSAllowedOriginsMustBeOrigins(t *testing.T) {
+	for _, entry := range []string{
+		"*",
+		"https://*.netwatch.example",
+		"netwatch.example",
+		"https://netwatch.example/",
+	} {
+		t.Run(entry, func(t *testing.T) {
+			t.Setenv("CORS_ALLOWED_ORIGINS", entry)
+
+			requireConfigError(t, "CORS_ALLOWED_ORIGINS")
+		})
+	}
+}

backend/internal/middleware/middleware_test.go

@@ -367,6 +367,25 @@ func TestRateLimitRefusesPastAllowanceThenResets(t *testing.T) {
 	})
 }
 
+// postForwarded sends handler a report from peer that names client in
+// X-Forwarded-For, and returns the status.
+func postForwarded(
+	t *testing.T,
+	handler http.Handler,
+	peer, client string,
+) int {
+	t.Helper()
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequestWithContext(t.Context(),
+		http.MethodPost, "/api/v1/reports", http.NoBody)
+	req.RemoteAddr = peer
+	req.Header.Set("X-Forwarded-For", client)
+	handler.ServeHTTP(rec, req)
+
+	return rec.Code
+}
+
 // TestRateLimitIsPerForwardedClient checks that clients behind a
 // trusted proxy each get their own allowance: the limit is keyed on
 // the client address clientIP resolves, not on the proxy's.
@@ -378,32 +397,47 @@ func TestRateLimitIsPerForwardedClient(t *testing.T) {
 	mw := middleware.NewWithTrustedProxies(mustPrefixes(t, "127.0.0.1/32"))
 	handler := mw.RateLimit(1)(okHandler())
 
-	post := func(client string) int {
+	code := postForwarded(t, handler, loopbackPeer, forwardedIP)
-		rec := httptest.NewRecorder()
+	if code != http.StatusOK {
-		req := httptest.NewRequestWithContext(t.Context(),
-			http.MethodPost, "/api/v1/reports", http.NoBody)
-		req.RemoteAddr = loopbackPeer
-		req.Header.Set("X-Forwarded-For", client)
-		handler.ServeHTTP(rec, req)
-
-		return rec.Code
-	}
-
-	if code := post(forwardedIP); code != http.StatusOK {
 		t.Fatalf("first request: status = %d, want %d", code, http.StatusOK)
 	}
 
-	if code := post(forwardedIP); code != http.StatusTooManyRequests {
+	code = postForwarded(t, handler, loopbackPeer, forwardedIP)
+	if code != http.StatusTooManyRequests {
 		t.Fatalf("same client again: status = %d, want %d",
 			code, http.StatusTooManyRequests)
 	}
 
-	if code := post(otherClient); code != http.StatusOK {
+	code = postForwarded(t, handler, loopbackPeer, otherClient)
+	if code != http.StatusOK {
 		t.Fatalf("other client behind the same proxy: status = %d, want %d",
 			code, http.StatusOK)
 	}
 }
 
+// TestRateLimitIgnoresForwardedForFromUntrustedPeer checks that a
+// peer that is not a trusted proxy cannot get a fresh allowance by
+// naming a different client in X-Forwarded-For on each request.
+func TestRateLimitIgnoresForwardedForFromUntrustedPeer(t *testing.T) {
+	t.Parallel()
+
+	const untrustedPeer = "198.51.100.4:5000"
+
+	mw := middleware.NewWithTrustedProxies(mustPrefixes(t, "127.0.0.1/32"))
+	handler := mw.RateLimit(1)(okHandler())
+
+	code := postForwarded(t, handler, untrustedPeer, "203.0.113.8")
+	if code != http.StatusOK {
+		t.Fatalf("first request: status = %d, want %d", code, http.StatusOK)
+	}
+
+	code = postForwarded(t, handler, untrustedPeer, "203.0.113.9")
+	if code != http.StatusTooManyRequests {
+		t.Fatalf("same peer naming another client: status = %d, want %d",
+			code, http.StatusTooManyRequests)
+	}
+}
+
 // preflight sends cors the preflight request a browser makes before
 // it POSTs JSON from origin.
 func preflight(

backend/internal/reportbuf/reportbuf_test.go

@@ -8,7 +8,10 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
+	"sync/atomic"
 	"testing"
+	"time"
 
 	"sneak.berlin/go/netwatch/internal/config"
 	"sneak.berlin/go/netwatch/internal/globals"
@@ -207,12 +210,134 @@ func TestWrittenReportsCountAtFileSize(t *testing.T) {
 		t.Fatalf("flush: %v", err)
 	}
 
+	// The second report is written at shutdown, and must not land in
+	// the first file's millisecond (see TestWrittenReportsKeepCounting).
+	time.Sleep(time.Millisecond)
+
 	err = buf.Append(report)
 	if err != nil {
 		t.Fatalf("second report, after the first was written: %v", err)
 	}
 }
 
+// TestWrittenReportsKeepCounting writes one report file after another
+// under a small cap: each report must be taken while the files on disk
+// leave room for it, and refused once they do not.
+func TestWrittenReportsKeepCounting(t *testing.T) {
+	const maxBytes = 200
+
+	report := map[string]string{"id": "written"}
+	size := int64(lineBytes(t, report))
+	dir := t.TempDir()
+
+	t.Setenv("DATA_DIR", dir)
+	t.Setenv("DATA_DIR_MAX_BYTES", strconv.Itoa(maxBytes))
+
+	buf := startBuffer(t)
+
+	// Every file takes at least a byte, so they fill the cap within
+	// maxBytes rounds.
+	for range maxBytes {
+		used := reportFilesBytes(t, dir)
+
+		err := buf.Append(report)
+		if used+size > maxBytes {
+			if !errors.Is(err, reportbuf.ErrFull) {
+				t.Fatalf("with %d bytes of report files: error = %v, "+
+					"want ErrFull", used, err)
+			}
+
+			return
+		}
+
+		if err != nil {
+			t.Fatalf("with %d bytes of report files: %v", used, err)
+		}
+
+		// Report files are named to the millisecond; two in the same
+		// one collide (https://git.eeqj.de/sneak/netwatch/issues/61).
+		time.Sleep(time.Millisecond)
+
+		err = buf.Flush()
+		if err != nil {
+			t.Fatalf("flush: %v", err)
+		}
+	}
+
+	t.Fatal("the report files never filled the cap")
+}
+
+// TestConcurrentAppendsStopAtCap appends from many goroutines at once
+// with room for exactly roomFor reports: exactly that many must be
+// taken, which holds only if Append checks and counts each report
+// under one lock.
+func TestConcurrentAppendsStopAtCap(t *testing.T) {
+	const (
+		roomFor = 5
+		senders = 50
+	)
+
+	// Large, so each Append takes long enough for the senders to
+	// overlap while the cap is reached.
+	report := map[string]string{"id": strings.Repeat("a", 1_000_000)}
+
+	t.Setenv("DATA_DIR", t.TempDir())
+	t.Setenv("DATA_DIR_MAX_BYTES",
+		strconv.Itoa(roomFor*lineBytes(t, report)))
+
+	buf := startBuffer(t)
+
+	var (
+		taken atomic.Int64
+		wg    sync.WaitGroup
+	)
+
+	start := make(chan struct{})
+
+	for range senders {
+		wg.Go(func() {
+			<-start
+
+			err := buf.Append(report)
+			if err == nil {
+				taken.Add(1)
+			} else if !errors.Is(err, reportbuf.ErrFull) {
+				t.Errorf("append: %v", err)
+			}
+		})
+	}
+
+	close(start)
+	wg.Wait()
+
+	if got := taken.Load(); got != roomFor {
+		t.Fatalf("%d reports taken, want %d", got, roomFor)
+	}
+}
+
+// reportFilesBytes returns the total size of the report files in dir.
+func reportFilesBytes(t *testing.T, dir string) int64 {
+	t.Helper()
+
+	paths, err := filepath.Glob(filepath.Join(dir, "reports-*.jsonl.zst"))
+	if err != nil {
+		t.Fatalf("list report files: %v", err)
+	}
+
+	var total int64
+
+	for _, path := range paths {
+		info, statErr := os.Stat(path)
+		if statErr != nil {
+			t.Fatalf("stat %s: %v", path, statErr)
+		}
+
+		total += info.Size()
+	}
+
+	return total
+}
+
 func writeBytes(t *testing.T, path string, n int) {
 	t.Helper()
 

backend/internal/server/routes_test.go

@@ -81,6 +81,32 @@ func TestReportsAreRateLimited(t *testing.T) {
 	}
 }
 
+// TestCORSAllowedOriginsReachTheRouter checks that an origin listed in
+// CORS_ALLOWED_ORIGINS is allowed by the router, not only when handed
+// to the CORS middleware directly.
+func TestCORSAllowedOriginsReachTheRouter(t *testing.T) {
+	const origin = "https://netwatch.example:8443"
+
+	t.Setenv("CORS_ALLOWED_ORIGINS", origin)
+
+	srv := newServer(t)
+	srv.SetupRoutes()
+
+	// The preflight a browser sends before it POSTs JSON from origin.
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequestWithContext(t.Context(),
+		http.MethodOptions, "/api/v1/reports", http.NoBody)
+	req.Header.Set("Origin", origin)
+	req.Header.Set("Access-Control-Request-Method", http.MethodPost)
+	req.Header.Set("Access-Control-Request-Headers", "content-type")
+	srv.ServeHTTP(rec, req)
+
+	got := rec.Header().Get("Access-Control-Allow-Origin")
+	if got != origin {
+		t.Fatalf("Access-Control-Allow-Origin = %q, want %q", got, origin)
+	}
+}
+
 // TestHealthCheckRejectsOversizeBody sends the health check, which
 // never reads its body, a body one byte over the limit. Only the
 // router-wide body limit can reject it.