server: limit wildcard CORS to the public routes (closes #100)

The CORS wildcard was global, so it also covered the Basic-Auth
protected /metrics, which REPO_POLICIES.md forbids, and it allowed
POST, PUT and DELETE, which no route serves, plus the Authorization
and X-CSRF-Token headers. CORS now sits on a router holding only the
public routes and allows GET and OPTIONS with the Accept and
Content-Type headers. /metrics gets no CORS at all.

Both are mounted routers rather than a Group: chi answers OPTIONS on
a Group's route with 405 before its middleware runs, and any method
/metrics does not register would otherwise fall through to the public
router. So every method on /metrics now meets Basic Auth first, and
/metrics/ is served like /metrics.

Model: opus-5-5

TODO.md

@@ -21,10 +21,6 @@ Rationale, Design, TODO, License, Author) if any are still missing.
 
 - 2026-10-01: wildcard CORS now applies only to the public routes, not to
   `/metrics`, and allows only the methods they serve (closes #100).
-- 2026-10-01: `internal/state` and `internal/watcher` no longer export test-only
-  constructors: two moved to `export_test.go`, one is deleted (closes #111).
-- 2026-10-01: notify shutdown tests use one timing constant per meaning, name
-  the bound they check, and require the drain's debug line (closes #116).
 - 2026-09-29: the live-DNS test package is renamed `internal/livednstest` and
   added to the `test-support` `deny` list in `.golangci.yml`, so `make lint`
   fails when program code imports it (closes #164).

internal/middleware/middleware_test.go

@@ -276,18 +276,10 @@ func newTestHandlers(t *testing.T) *handlers.Handlers {
 		t.Fatalf("notify.New: %v", err)
 	}
 
-	st, err := state.New(fxtest.NewLifecycle(t), state.Params{
-		Logger: log,
-		Config: &config.Config{DataDir: t.TempDir()},
-	})
-	if err != nil {
-		t.Fatalf("state.New: %v", err)
-	}
-
 	hnd, err := handlers.New(nil, handlers.Params{
 		Logger:  log,
 		Globals: glob,
-		State:   st,
+		State:   state.NewForTest(),
 		Notify:  notifier,
 	})
 	if err != nil {

internal/notify/shutdown_test.go

@@ -33,29 +33,10 @@ const (
 	// out.
 	drainDeadline = 50 * time.Millisecond
 
-	// timeoutDrainBound is how long a drain given drainDeadline
-	// may take to return before the test gives up on it.  At
-	// forty times drainDeadline it leaves ample room for
-	// scheduling delay on a loaded box under -race, yet it is far
-	// below the test binary's -timeout, so a drain that its
-	// deadline does not bound fails that one test instead of
-	// hanging the package.
-	timeoutDrainBound = 2 * time.Second
-
-	// longDrainDeadline is the deadline given to a drain that is
-	// expected to finish well before it: when the in-flight
-	// delivery completes after inFlightHold, or at once when
-	// nothing is in flight.  It is far above inFlightHold, so
-	// those drains never reach it, and four times
-	// idleDrainBound, so an idle drain that waited for its
-	// deadline instead of returning fails that bound.
-	longDrainDeadline = 2 * time.Second
-
-	// reachEndpointTimeout is how long a submitted delivery may
-	// take to reach the test server.  That normally takes a few
-	// milliseconds; the margin is for a loaded box under -race,
-	// and only a failing run ever waits this long.
-	reachEndpointTimeout = 2 * time.Second
+	// drainSlack is the upper bound on how long a bounded
+	// drain may take; generous enough for a loaded CI box,
+	// still far below the 20s test ceiling.
+	drainSlack = 2 * time.Second
 
 	// settleDelay is how long to wait before asserting that
 	// something did *not* happen.
@@ -65,11 +46,10 @@ const (
 	// nothing in flight.  It is deliberately far above the cost
 	// of the goroutine hop through inFlight.Wait() — which
 	// reached 57ms on a loaded box under -race with the package's
-	// parallel tests — and far below longDrainDeadline, the
-	// deadline such a drain is given.  A drain that blocked until
-	// its deadline instead of returning on the WaitGroup
-	// therefore still fails this bound, but scheduling delay
-	// alone cannot.
+	// parallel tests — and far below drainSlack, the deadline
+	// such a drain is given.  A drain that blocked until its
+	// deadline instead of returning on the WaitGroup therefore
+	// still fails this bound, but scheduling delay alone cannot.
 	idleDrainBound = 500 * time.Millisecond
 )
 
@@ -95,14 +75,12 @@ func (sb *syncBuffer) String() string {
 }
 
 // newLoggingService returns a Service writing JSON logs into
-// the returned buffer, debug level included.
+// the returned buffer.
 func newLoggingService(
 	transport http.RoundTripper,
 ) (*notify.Service, *syncBuffer) {
 	logs := &syncBuffer{}
-	handler := slog.NewJSONHandler(
-		logs, &slog.HandlerOptions{Level: slog.LevelDebug},
-	)
+	handler := slog.NewJSONHandler(logs, nil)
 
 	return notify.NewTestServiceWithLogger(transport, handler),
 		logs
@@ -156,7 +134,7 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) {
 	// drain begins.
 	select {
 	case <-entered:
-	case <-time.After(reachEndpointTimeout):
+	case <-time.After(drainSlack):
 		t.Fatal("delivery never reached the endpoint")
 	}
 
@@ -173,7 +151,7 @@ func TestDrainWaitsForInFlightDelivery(t *testing.T) {
 	defer timer.Stop()
 
 	ctx, cancel := context.WithTimeout(
-		context.Background(), longDrainDeadline,
+		context.Background(), drainSlack,
 	)
 	defer cancel()
 
@@ -279,11 +257,11 @@ func TestDrainBoundedByContextDeadline(t *testing.T) {
 
 	select {
 	case <-returned:
-	case <-time.After(timeoutDrainBound):
+	case <-time.After(drainSlack):
 		t.Fatalf(
 			"drain did not return within %v; its %v deadline "+
 				"did not bound it",
-			timeoutDrainBound, drainDeadline,
+			drainSlack, drainDeadline,
 		)
 	}
 
@@ -355,7 +333,7 @@ func TestDrainRefusesNewDeliveries(t *testing.T) {
 	svc.SetMattermostWebhookURL(target)
 
 	ctx, cancel := context.WithTimeout(
-		context.Background(), longDrainDeadline,
+		context.Background(), drainSlack,
 	)
 	defer cancel()
 
@@ -468,7 +446,7 @@ func TestNewRegistersDrainingStopHook(t *testing.T) {
 
 	select {
 	case <-entered:
-	case <-time.After(reachEndpointTimeout):
+	case <-time.After(drainSlack):
 		t.Fatal("delivery never reached the endpoint")
 	}
 
@@ -478,7 +456,7 @@ func TestNewRegistersDrainingStopHook(t *testing.T) {
 	defer timer.Stop()
 
 	ctx, cancel := context.WithTimeout(
-		context.Background(), longDrainDeadline,
+		context.Background(), drainSlack,
 	)
 	defer cancel()
 
@@ -509,7 +487,7 @@ func TestDrainWithoutDeliveriesReturnsImmediately(t *testing.T) {
 	start := time.Now()
 
 	ctx, cancel := context.WithTimeout(
-		context.Background(), longDrainDeadline,
+		context.Background(), drainSlack,
 	)
 	defer cancel()
 
@@ -517,9 +495,9 @@ func TestDrainWithoutDeliveriesReturnsImmediately(t *testing.T) {
 
 	if elapsed := time.Since(start); elapsed > idleDrainBound {
 		t.Errorf(
-			"drain of an idle service took %v, want at most "+
-				"%v; its deadline was %v",
-			elapsed, idleDrainBound, longDrainDeadline,
+			"drain of an idle service took %v, want well "+
+				"under its %v deadline",
+			elapsed, drainSlack,
 		)
 	}
 }
@@ -527,11 +505,10 @@ func TestDrainWithoutDeliveriesReturnsImmediately(t *testing.T) {
 // TestDrainWithCancelledContextDoesNotWarn verifies that an
 // OnStop context that is already dead on entry does not produce
 // an "abandoning them" warning when there was nothing in flight
-// to abandon, and that the drain returns and says at debug level
-// that nothing was in flight.  The expired context wins the
-// select immediately, so only the outstanding count can tell the
-// difference between a genuine timeout and a shutdown that had
-// simply already run out of time with no work left.
+// to abandon.  The expired context wins the select immediately,
+// so only the outstanding count can tell the difference between
+// a genuine timeout and a shutdown that had simply already run
+// out of time with no work left.
 func TestDrainWithCancelledContextDoesNotWarn(t *testing.T) {
 	t.Parallel()
 
@@ -540,43 +517,11 @@ func TestDrainWithCancelledContextDoesNotWarn(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	cancel()
 
-	// A watchdog, as in TestDrainBoundedByContextDeadline, so
-	// that a drain which never returns fails here instead of
-	// hanging the package.
-	returned := make(chan struct{})
-
-	go func() {
-		defer close(returned)
-
 	svc.Drain(ctx)
-	}()
 
-	select {
-	case <-returned:
-	case <-time.After(idleDrainBound):
-		t.Fatalf(
-			"drain with nothing in flight and a cancelled "+
-				"context did not return within %v",
-			idleDrainBound,
-		)
-	}
-
-	output := logs.String()
-
-	// The absence of a warning alone would also pass if the drain
-	// logged nothing at all, so require the debug line it writes
-	// when it finds nothing outstanding.
-	if !strings.Contains(
-		output, "all in-flight notifications completed",
+	if output := logs.String(); strings.Contains(
+		output, `"level":"WARN"`,
 	) {
-		t.Errorf(
-			"drain did not log that nothing was in flight; "+
-				"log output: %s",
-			output,
-		)
-	}
-
-	if strings.Contains(output, `"level":"WARN"`) {
 		t.Errorf(
 			"drain with nothing in flight warned about "+
 				"abandoned deliveries; log output: %s",

internal/state/export_test.go

@@ -1,23 +0,0 @@
-package state
-
-import (
-	"log/slog"
-
-	"sneak.berlin/go/dnswatcher/internal/config"
-)
-
-// NewForTestWithDataDir creates an empty State that saves to dataDir,
-// without the fx lifecycle.
-func NewForTestWithDataDir(dataDir string) *State {
-	return &State{
-		log: slog.Default(),
-		snapshot: &Snapshot{
-			Version:      stateVersion,
-			Domains:      make(map[string]*DomainState),
-			Hostnames:    make(map[string]*HostnameState),
-			Ports:        make(map[string]*PortState),
-			Certificates: make(map[string]*CertificateState),
-		},
-		config: &config.Config{DataDir: dataDir},
-	}
-}

internal/state/state_test.go

@@ -739,7 +739,7 @@ func TestPortStateUnmarshalJSON_BothFormats(t *testing.T) {
 func TestGetSnapshot_ReturnsCopy(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTestWithDataDir(t.TempDir())
+	s := state.NewForTest()
 
 	populateState(t, s)
 
@@ -761,7 +761,7 @@ func TestGetSnapshot_ReturnsCopy(t *testing.T) {
 func TestDomainState_GetSet(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTestWithDataDir(t.TempDir())
+	s := state.NewForTest()
 
 	// Get on missing key returns false.
 	_, ok := s.GetDomainState("nonexistent.com")
@@ -812,7 +812,7 @@ func TestDomainState_GetSet(t *testing.T) {
 func TestHostnameState_GetSet(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTestWithDataDir(t.TempDir())
+	s := state.NewForTest()
 
 	_, ok := s.GetHostnameState("missing.example.com")
 	if ok {
@@ -857,7 +857,7 @@ func TestHostnameState_GetSet(t *testing.T) {
 func TestPortState_GetSetDelete(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTestWithDataDir(t.TempDir())
+	s := state.NewForTest()
 
 	_, ok := s.GetPortState("1.2.3.4:80")
 	if ok {
@@ -895,7 +895,7 @@ func TestPortState_GetSetDelete(t *testing.T) {
 func TestGetAllPortKeys(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTestWithDataDir(t.TempDir())
+	s := state.NewForTest()
 
 	keys := s.GetAllPortKeys()
 	if len(keys) != 0 {
@@ -937,7 +937,7 @@ func TestGetAllPortKeys(t *testing.T) {
 func TestCertificateState_GetSet(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTestWithDataDir(t.TempDir())
+	s := state.NewForTest()
 
 	_, ok := s.GetCertificateState("1.2.3.4:443:www.example.com")
 	if ok {
@@ -1158,7 +1158,7 @@ func TestLoadPreservesExistingStateOnMissingFile(t *testing.T) {
 func TestConcurrentGetSet(t *testing.T) {
 	t.Parallel()
 
-	s := state.NewForTestWithDataDir(t.TempDir())
+	s := state.NewForTest()
 
 	const goroutines = 20
 
@@ -1368,6 +1368,35 @@ func TestMultipleSavesOverwrite(t *testing.T) {
 	}
 }
 
+// TestNewForTest verifies the test helper creates a valid empty state.
+func TestNewForTest(t *testing.T) {
+	t.Parallel()
+
+	s := state.NewForTest()
+
+	snap := s.GetSnapshot()
+
+	if snap.Version != 1 {
+		t.Errorf("version: got %d, want 1", snap.Version)
+	}
+
+	if snap.Domains == nil {
+		t.Error("Domains map should be initialized")
+	}
+
+	if snap.Hostnames == nil {
+		t.Error("Hostnames map should be initialized")
+	}
+
+	if snap.Ports == nil {
+		t.Error("Ports map should be initialized")
+	}
+
+	if snap.Certificates == nil {
+		t.Error("Certificates map should be initialized")
+	}
+}
+
 // TestSaveFilePermissions verifies the saved file has restricted permissions.
 func TestSaveFilePermissions(t *testing.T) {
 	t.Parallel()

internal/state/state_test_helper.go

@@ -0,0 +1,38 @@
+package state
+
+import (
+	"log/slog"
+
+	"sneak.berlin/go/dnswatcher/internal/config"
+)
+
+// NewForTest creates a State for unit testing with no persistence.
+func NewForTest() *State {
+	return &State{
+		log: slog.Default(),
+		snapshot: &Snapshot{
+			Version:      stateVersion,
+			Domains:      make(map[string]*DomainState),
+			Hostnames:    make(map[string]*HostnameState),
+			Ports:        make(map[string]*PortState),
+			Certificates: make(map[string]*CertificateState),
+		},
+		config: &config.Config{DataDir: ""},
+	}
+}
+
+// NewForTestWithDataDir creates a State backed by the given directory
+// for tests that need file persistence.
+func NewForTestWithDataDir(dataDir string) *State {
+	return &State{
+		log: slog.Default(),
+		snapshot: &Snapshot{
+			Version:      stateVersion,
+			Domains:      make(map[string]*DomainState),
+			Hostnames:    make(map[string]*HostnameState),
+			Ports:        make(map[string]*PortState),
+			Certificates: make(map[string]*CertificateState),
+		},
+		config: &config.Config{DataDir: dataDir},
+	}
+}

internal/watcher/export_test.go

@@ -2,35 +2,10 @@ package watcher
 
 import (
 	"context"
-	"log/slog"
-	"time"
 
-	"sneak.berlin/go/dnswatcher/internal/config"
 	"sneak.berlin/go/dnswatcher/internal/state"
 )
 
-// NewForTest creates a Watcher without fx for unit testing.
-func NewForTest(
-	cfg *config.Config,
-	st *state.State,
-	res DNSResolver,
-	pc PortChecker,
-	tc TLSChecker,
-	n Notifier,
-) *Watcher {
-	return &Watcher{
-		log:            slog.Default(),
-		config:         cfg,
-		state:          st,
-		resolver:       res,
-		portCheck:      pc,
-		tlsCheck:       tc,
-		notify:         n,
-		firstRun:       true,
-		expiryNotified: make(map[string]time.Time),
-	}
-}
-
 // NewlyDisagreeingPairs exports newlyDisagreeingPairs for testing.
 func NewlyDisagreeingPairs(
 	prev *state.HostnameState,

internal/watcher/watcher.go

@@ -102,6 +102,28 @@ func New(
 	return w, nil
 }
 
+// NewForTest creates a Watcher without fx for unit testing.
+func NewForTest(
+	cfg *config.Config,
+	st *state.State,
+	res DNSResolver,
+	pc PortChecker,
+	tc TLSChecker,
+	n Notifier,
+) *Watcher {
+	return &Watcher{
+		log:            slog.Default(),
+		config:         cfg,
+		state:          st,
+		resolver:       res,
+		portCheck:      pc,
+		tlsCheck:       tc,
+		notify:         n,
+		firstRun:       true,
+		expiryNotified: make(map[string]time.Time),
+	}
+}
+
 // Run starts the monitoring loop with periodic scheduling.
 func (w *Watcher) Run(ctx context.Context) {
 	w.log.Info(

internal/watcher/watcher_test.go

@@ -9,12 +9,8 @@ import (
 	"testing"
 	"time"
 
-	"go.uber.org/fx/fxtest"
-
 	"sneak.berlin/go/dnswatcher/internal/config"
-	"sneak.berlin/go/dnswatcher/internal/globals"
 	"sneak.berlin/go/dnswatcher/internal/livednstest"
-	"sneak.berlin/go/dnswatcher/internal/logger"
 	"sneak.berlin/go/dnswatcher/internal/portcheck"
 	"sneak.berlin/go/dnswatcher/internal/resolver"
 	"sneak.berlin/go/dnswatcher/internal/state"
@@ -152,24 +148,7 @@ func newTestWatcher(
 		config:   cfg,
 	}
 
-	g, err := globals.New(nil)
-	if err != nil {
-		t.Fatalf("globals.New: %v", err)
-	}
-
-	log, err := logger.New(nil, logger.Params{Globals: g})
-	if err != nil {
-		t.Fatalf("logger.New: %v", err)
-	}
-
-	// The watcher saves state after every check, into cfg.DataDir.
-	deps.state, err = state.New(fxtest.NewLifecycle(t), state.Params{
-		Logger: log,
-		Config: cfg,
-	})
-	if err != nil {
-		t.Fatalf("state.New: %v", err)
-	}
+	deps.state = state.NewForTest()
 
 	w := watcher.NewForTest(
 		deps.config,