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
2026-09-29 04:22:19 +02:00
parent bbcc7d921d
commit ea66caf338
16 changed files with 908 additions and 54 deletions
@@ -4,7 +4,9 @@ package config

 import (
 	"errors"
+	"fmt"
 	"log/slog"
+	"net/url"
 	"strings"

 	"sneak.berlin/go/netwatch/internal/globals"
@@ -23,6 +25,20 @@ import (
 const defaultTrustedProxies = "127.0.0.1/32,::1/128," +
 	"10.0.0.0/8,172.16.0.0/12,192.168.0.0/16"

+// Default limits on stored reports; backend/README.md gives the
+// reasons for these values.
+const (
+	defaultReportsPerMinute = 60
+	defaultDataDirMaxBytes  = 1 << 30 // 1 GiB
+)
+
+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 {
 	fx.In
@@ -33,16 +49,19 @@ type Params struct {

 // Config holds the resolved application configuration.
 type Config struct {
-	BindAddress     string
-	DataDir         string
-	Debug           bool
-	MetricsPassword string
-	MetricsUsername string
-	Port            int
-	SentryDSN       string
-	TrustedProxies  []string
-	log             *slog.Logger
-	params          *Params
+	BindAddress        string
+	CORSAllowedOrigins []string
+	DataDir            string
+	DataDirMaxBytes    int64
+	Debug              bool
+	MetricsPassword    string
+	MetricsUsername    string
+	Port               int
+	ReportsPerMinute   int
+	SentryDSN          string
+	TrustedProxies     []string
+	log                *slog.Logger
+	params             *Params
 }

 // New loads configuration from env, .env files, and config
@@ -61,11 +80,15 @@ func New(

 	viper.AutomaticEnv()

+	// An empty CORS_ALLOWED_ORIGINS allows no other origin.
+	viper.SetDefault("CORS_ALLOWED_ORIGINS", "")
 	viper.SetDefault("DATA_DIR", "./data/reports")
+	viper.SetDefault("DATA_DIR_MAX_BYTES", defaultDataDirMaxBytes)
 	viper.SetDefault("DEBUG", "false")
 	// An empty BIND_ADDRESS listens on every interface.
 	viper.SetDefault("BIND_ADDRESS", "")
 	viper.SetDefault("PORT", "8080")
+	viper.SetDefault("REPORTS_PER_MINUTE", defaultReportsPerMinute)
 	viper.SetDefault("SENTRY_DSN", "")
 	viper.SetDefault("METRICS_USERNAME", "")
 	viper.SetDefault("METRICS_PASSWORD", "")
@@ -81,16 +104,36 @@ func New(
 	}

 	s := &Config{
-		BindAddress:     viper.GetString("BIND_ADDRESS"),
-		DataDir:         viper.GetString("DATA_DIR"),
-		Debug:           viper.GetBool("DEBUG"),
-		MetricsPassword: viper.GetString("METRICS_PASSWORD"),
-		MetricsUsername: viper.GetString("METRICS_USERNAME"),
-		Port:            viper.GetInt("PORT"),
-		SentryDSN:       viper.GetString("SENTRY_DSN"),
-		TrustedProxies:  splitList(viper.GetString("TRUSTED_PROXIES")),
-		log:             log,
-		params:          &params,
+		BindAddress:        viper.GetString("BIND_ADDRESS"),
+		CORSAllowedOrigins: splitList(viper.GetString("CORS_ALLOWED_ORIGINS")),
+		DataDir:            viper.GetString("DATA_DIR"),
+		DataDirMaxBytes:    viper.GetInt64("DATA_DIR_MAX_BYTES"),
+		Debug:              viper.GetBool("DEBUG"),
+		MetricsPassword:    viper.GetString("METRICS_PASSWORD"),
+		MetricsUsername:    viper.GetString("METRICS_USERNAME"),
+		Port:               viper.GetInt("PORT"),
+		ReportsPerMinute:   viper.GetInt("REPORTS_PER_MINUTE"),
+		SentryDSN:          viper.GetString("SENTRY_DSN"),
+		TrustedProxies:     splitList(viper.GetString("TRUSTED_PROXIES")),
+		log:                log,
+		params:             &params,
+	}
+
+	// viper reads a value that is not a number as 0, so this also
+	// catches a mistyped setting.
+	if s.ReportsPerMinute <= 0 {
+		return nil, fmt.Errorf("REPORTS_PER_MINUTE %q: %w",
+			viper.GetString("REPORTS_PER_MINUTE"), errNotPositive)
+	}
+
+	if s.DataDirMaxBytes <= 0 {
+		return nil, fmt.Errorf("DATA_DIR_MAX_BYTES %q: %w",
+			viper.GetString("DATA_DIR_MAX_BYTES"), errNotPositive)
+	}
+
+	err = checkOrigins(s.CORSAllowedOrigins)
+	if err != nil {
+		return nil, err
 	}

 	if s.Debug {
@@ -101,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 {
@@ -0,0 +1,64 @@
+package config_test
+
+import (
+	"strings"
+	"testing"
+
+	"sneak.berlin/go/netwatch/internal/config"
+	"sneak.berlin/go/netwatch/internal/globals"
+	"sneak.berlin/go/netwatch/internal/logger"
+
+	"go.uber.org/fx"
+)
+
+// requireConfigError builds the config as main does and fails the
+// test unless that fails with an error naming setting. It uses
+// fx.New, because fxtest.New fails the test itself on an error.
+func requireConfigError(t *testing.T, setting string) {
+	t.Helper()
+
+	app := fx.New(
+		fx.NopLogger,
+		fx.Provide(globals.New, logger.New, config.New),
+		fx.Invoke(func(*config.Config) {}),
+	)
+
+	err := app.Err()
+	if err == nil || !strings.Contains(err.Error(), setting) {
+		t.Fatalf("config error = %v, want one naming %s", err, setting)
+	}
+}
+
+// TestReportsPerMinuteMustBePositive: unchecked, zero would panic
+// when the routes are built, and a negative rate would lift the
+// limit.
+func TestReportsPerMinuteMustBePositive(t *testing.T) {
+	t.Setenv("REPORTS_PER_MINUTE", "0")
+
+	requireConfigError(t, "REPORTS_PER_MINUTE")
+}
+
+// TestDataDirMaxBytesMustBeANumber: viper reads a value that is not
+// a number, such as "1GB", as 0, which would refuse every report.
+func TestDataDirMaxBytesMustBeANumber(t *testing.T) {
+	t.Setenv("DATA_DIR_MAX_BYTES", "1GB")
+
+	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")
+		})
+	}
+}