Move to the layout REPO_POLICIES.md gives (closes #30)

The Go module moves from backend/ to the root (go.mod, go.sum,
.golangci.yml, cmd/, internal/), the page's source to web/, and nginx's
two files to configs/. backend/ is gone: its LICENSE and .dockerignore
are deleted, and its .gitignore entries, README and scripts are folded
into the root ones. script/build builds the server binary after the
frontend; script/run and script/clean move up from backend/script/;
make test, lint, fmt and fmt-check do what the backend's did. Vite's
root is web/ and it still builds into dist/. go.mod ignores
node_modules/, whose npm packages can ship .go files. script/bootstrap
no longer installs gcc, which only the host Go tests needed.

Model: opus-5-5

internal/server/export_test.go

@@ -0,0 +1,23 @@
+package server
+
+import "github.com/go-chi/chi/v5"
+
+// Router exposes the router to the external tests, which add routes
+// of their own to it after SetupRoutes.
+func (s *Server) Router() *chi.Mux {
+	return s.router
+}
+
+// MaxRequestBodyBytes exposes the router-wide body limit to the
+// external tests.
+const MaxRequestBodyBytes = maxRequestBodyBytes
+
+// MetricsRequestsPerMinute exposes the /metrics rate limit to the
+// external tests.
+const MetricsRequestsPerMinute = metricsRequestsPerMinute
+
+// ListenAddr exposes the address the server listens on to the
+// external tests.
+func (s *Server) ListenAddr() string {
+	return s.newHTTPServer().Addr
+}

internal/server/http.go

@@ -0,0 +1,71 @@
+package server
+
+import (
+	"errors"
+	"net"
+	"net/http"
+	"runtime"
+	"strconv"
+	"time"
+
+	"go.uber.org/fx"
+)
+
+const (
+	readTimeout       = 10 * time.Second
+	readHeaderTimeout = 5 * time.Second
+	idleTimeout       = 60 * time.Second
+	maxHeaderBytes    = 1 << 20 // 1 MiB
+
+	// requestTimeout (routes.go) is the single per-request
+	// processing budget, enforced by chi's middleware.Timeout.
+	// writeTimeout must exceed that budget so a handler can write
+	// its 503 when the chi timeout fires; if it were shorter the
+	// server would abort the write first and the chi budget would
+	// be unreachable dead configuration.
+	writeTimeout = requestTimeout + 5*time.Second
+)
+
+// newHTTPServer constructs the http.Server. It performs no I/O
+// and does not start listening.
+func (s *Server) newHTTPServer() *http.Server {
+	listenAddr := net.JoinHostPort(
+		s.params.Config.BindAddress,
+		strconv.Itoa(s.params.Config.Port),
+	)
+
+	return &http.Server{
+		Addr:              listenAddr,
+		Handler:           s,
+		MaxHeaderBytes:    maxHeaderBytes,
+		ReadTimeout:       readTimeout,
+		ReadHeaderTimeout: readHeaderTimeout,
+		WriteTimeout:      writeTimeout,
+		IdleTimeout:       idleTimeout,
+	}
+}
+
+// listenAndServe runs the listener until the server is shut
+// down. A genuine listen failure (not the expected
+// ErrServerClosed from a clean shutdown) requests process
+// shutdown through fx with a non-zero exit code, so the failure
+// is visible to any supervisor.
+func (s *Server) listenAndServe() {
+	s.log.Info("http begin listen",
+		"listenaddr", s.httpServer.Addr,
+		"version", s.params.Globals.Version,
+		"arch", runtime.GOARCH,
+	)
+
+	err := s.httpServer.ListenAndServe()
+	if err == nil || errors.Is(err, http.ErrServerClosed) {
+		return
+	}
+
+	s.log.Error("listen error", "error", err)
+
+	shutdownErr := s.shutdowner.Shutdown(fx.ExitCode(1))
+	if shutdownErr != nil {
+		s.log.Error("request shutdown failed", "error", shutdownErr)
+	}
+}

internal/server/http_test.go

@@ -0,0 +1,32 @@
+package server_test
+
+import "testing"
+
+// TestListenAddress checks that the server listens on BIND_ADDRESS
+// and PORT, and on port 8080 on every interface when neither is set.
+// The container image sets both, to keep the backend on loopback
+// behind nginx.
+func TestListenAddress(t *testing.T) {
+	tests := []struct {
+		bindAddress string
+		port        string
+		want        string
+	}{
+		{bindAddress: "", port: "", want: ":8080"},
+		{bindAddress: "127.0.0.1", port: "8081", want: "127.0.0.1:8081"},
+		{bindAddress: "::1", port: "8081", want: "[::1]:8081"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.want, func(t *testing.T) {
+			// t.Setenv rules out t.Parallel.
+			t.Setenv("BIND_ADDRESS", tt.bindAddress)
+			t.Setenv("PORT", tt.port)
+
+			got := newServer(t).ListenAddr()
+			if got != tt.want {
+				t.Errorf("listen address = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}

internal/server/routes.go

@@ -0,0 +1,85 @@
+package server
+
+import (
+	"time"
+
+	sentryhttp "github.com/getsentry/sentry-go/http"
+	"github.com/go-chi/chi/v5"
+	"github.com/go-chi/chi/v5/middleware"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/collectors"
+	"github.com/prometheus/client_golang/prometheus/promhttp"
+)
+
+const (
+	requestTimeout = 60 * time.Second
+
+	// maxRequestBodyBytes caps every request body. A route group
+	// can mount s.mw.MaxBodyBytes with a smaller value to lower
+	// its bound, but cannot raise it: this cap runs first.
+	maxRequestBodyBytes int64 = 1 << 20 // 1 MiB
+
+	// metricsRequestsPerMinute is how many requests to /metrics each
+	// client address may make a minute, whatever their credentials. A
+	// scraper polling every 2 seconds sends half of it, which httprate
+	// never refuses.
+	metricsRequestsPerMinute = 60
+)
+
+// SetupRoutes configures the chi router with middleware and
+// all application routes.
+func (s *Server) SetupRoutes() {
+	s.router = chi.NewRouter()
+
+	s.router.Use(s.mw.Recoverer())
+	s.router.Use(middleware.RequestID)
+	s.router.Use(s.mw.Logging())
+	s.router.Use(s.mw.SecurityHeaders())
+	s.router.Use(s.mw.CORS(s.params.Config.CORSAllowedOrigins))
+	s.router.Use(s.mw.MaxBodyBytes(maxRequestBodyBytes))
+	s.router.Use(middleware.Timeout(requestTimeout))
+
+	// Sentry reports a panic, then panics again, so that s.mw.Recoverer
+	// still answers 500.
+	if s.params.Config.SentryDSN != "" {
+		s.router.Use(sentryhttp.New(sentryhttp.Options{Repanic: true}).Handle)
+	}
+
+	// The metrics go in a registry of this server's own, not in
+	// Prometheus' default one, which takes them only once per process.
+	registry := prometheus.NewRegistry()
+	registry.MustRegister(
+		collectors.NewGoCollector(),
+		collectors.NewProcessCollector(collectors.ProcessCollectorOpts{}),
+	)
+
+	// Requests are measured only once chi has matched them to one of
+	// these routes, by path and method. The metrics are labelled with
+	// both, which any client can make up, so measuring every request
+	// would let clients add labels without bound. A Route here would
+	// be matched by its path prefix alone, so each path is given in
+	// full.
+	s.router.Group(func(r chi.Router) {
+		// config.New refuses one of the two credentials without the
+		// other.
+		if s.params.Config.MetricsUsername != "" {
+			r.Use(s.mw.Metrics(registry))
+		}
+
+		r.Get("/.well-known/healthcheck", s.h.HandleHealthCheck())
+
+		r.With(s.mw.RateLimit(s.params.Config.ReportsPerMinute)).
+			Post("/api/v1/reports", s.h.HandleReport())
+	})
+
+	// The rate limit comes before the basic auth, so a client past it
+	// gets 429 and its password is not checked.
+	if s.params.Config.MetricsUsername != "" {
+		s.router.With(
+			s.mw.RateLimit(metricsRequestsPerMinute),
+			s.mw.MetricsAuth(),
+		).Get("/metrics", promhttp.HandlerFor(
+			registry, promhttp.HandlerOpts{},
+		).ServeHTTP)
+	}
+}

internal/server/routes_test.go

@@ -0,0 +1,342 @@
+package server_test
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"sneak.berlin/go/netwatch/internal/config"
+	"sneak.berlin/go/netwatch/internal/globals"
+	"sneak.berlin/go/netwatch/internal/handlers"
+	"sneak.berlin/go/netwatch/internal/healthcheck"
+	"sneak.berlin/go/netwatch/internal/logger"
+	"sneak.berlin/go/netwatch/internal/middleware"
+	"sneak.berlin/go/netwatch/internal/reportbuf"
+	"sneak.berlin/go/netwatch/internal/server"
+
+	"github.com/getsentry/sentry-go"
+	"go.uber.org/fx"
+	"go.uber.org/fx/fxtest"
+)
+
+// newServer builds a Server from the same constructors as main,
+// configured from the environment. It is never started, so nothing
+// listens.
+func newServer(t *testing.T) *server.Server {
+	t.Helper()
+
+	var srv *server.Server
+
+	app := fxtest.New(t,
+		fx.Provide(
+			config.New,
+			globals.New,
+			handlers.New,
+			healthcheck.New,
+			logger.New,
+			middleware.New,
+			reportbuf.New,
+			server.New,
+		),
+		fx.Populate(&srv),
+	)
+
+	err := app.Err()
+	if err != nil {
+		t.Fatalf("build server: %v", err)
+	}
+
+	return srv
+}
+
+// TestReportsAreRateLimited checks that POST /api/v1/reports is
+// behind the per-address rate limit, set here to two a minute.
+func TestReportsAreRateLimited(t *testing.T) {
+	t.Setenv("REPORTS_PER_MINUTE", "2")
+
+	srv := newServer(t)
+	srv.SetupRoutes()
+
+	post := func() int {
+		rec := httptest.NewRecorder()
+		req := httptest.NewRequestWithContext(t.Context(),
+			http.MethodPost, "/api/v1/reports",
+			strings.NewReader(`{"clientId":"c1","hosts":[]}`),
+		)
+		srv.ServeHTTP(rec, req)
+
+		return rec.Code
+	}
+
+	for i := range 2 {
+		if code := post(); code != http.StatusOK {
+			t.Fatalf("report %d: status = %d, want %d",
+				i+1, code, http.StatusOK)
+		}
+	}
+
+	if code := post(); code != http.StatusTooManyRequests {
+		t.Fatalf("third report in a minute: status = %d, want %d",
+			code, http.StatusTooManyRequests)
+	}
+}
+
+// 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)
+	}
+}
+
+// TestNoMetricsWithoutCredentials: with neither metrics setting set,
+// there is no /metrics.
+func TestNoMetricsWithoutCredentials(t *testing.T) {
+	t.Setenv("METRICS_USERNAME", "")
+	t.Setenv("METRICS_PASSWORD", "")
+
+	srv := newServer(t)
+	srv.SetupRoutes()
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequestWithContext(t.Context(),
+		http.MethodGet, "/metrics", http.NoBody)
+	srv.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusNotFound {
+		t.Fatalf("status = %d, want %d", rec.Code, http.StatusNotFound)
+	}
+}
+
+// TestMetricsBehindBasicAuth: with both metrics settings set, /metrics
+// answers only with them as basic auth credentials, and shows a
+// request to a route but not one to a path no route has.
+func TestMetricsBehindBasicAuth(t *testing.T) {
+	t.Setenv("METRICS_USERNAME", "prometheus")
+	t.Setenv("METRICS_PASSWORD", "right")
+
+	srv := newServer(t)
+	srv.SetupRoutes()
+
+	get := func(path, username, password string) *httptest.ResponseRecorder {
+		rec := httptest.NewRecorder()
+		req := httptest.NewRequestWithContext(t.Context(),
+			http.MethodGet, path, http.NoBody)
+
+		if username != "" {
+			req.SetBasicAuth(username, password)
+		}
+
+		srv.ServeHTTP(rec, req)
+
+		return rec
+	}
+
+	get("/.well-known/healthcheck", "", "")
+	get("/api/v1/no-such-route", "", "")
+
+	for _, creds := range [][2]string{
+		{"", ""},
+		{"prometheus", "wrong"},
+		{"someone", "right"},
+	} {
+		rec := get("/metrics", creds[0], creds[1])
+		if rec.Code != http.StatusUnauthorized {
+			t.Errorf("credentials %q: status = %d, want %d",
+				creds, rec.Code, http.StatusUnauthorized)
+		}
+	}
+
+	rec := get("/metrics", "prometheus", "right")
+	if rec.Code != http.StatusOK {
+		t.Fatalf("right credentials: status = %d, want %d",
+			rec.Code, http.StatusOK)
+	}
+
+	body := rec.Body.String()
+	if !strings.Contains(body, `handler="/.well-known/healthcheck"`) {
+		t.Errorf("metrics show no health check request:\n%s", body)
+	}
+
+	if strings.Contains(body, "no-such-route") {
+		t.Errorf("metrics show a request to a path no route has:\n%s", body)
+	}
+
+	if !strings.Contains(body, "go_goroutines") {
+		t.Errorf("metrics show no Go runtime metrics:\n%s", body)
+	}
+}
+
+// TestMetricsAreRateLimited: a client that has used up its /metrics
+// allowance on wrong passwords gets 429 even with the right one, which
+// is then not checked, while another client behind the same nginx
+// still gets in.
+func TestMetricsAreRateLimited(t *testing.T) {
+	t.Setenv("METRICS_USERNAME", "prometheus")
+	t.Setenv("METRICS_PASSWORD", "right")
+	// As in the container: nginx connects from loopback and names the
+	// client in X-Forwarded-For.
+	t.Setenv("TRUSTED_PROXIES", "127.0.0.1/32")
+
+	srv := newServer(t)
+	srv.SetupRoutes()
+
+	get := func(client, password string) int {
+		rec := httptest.NewRecorder()
+		req := httptest.NewRequestWithContext(t.Context(),
+			http.MethodGet, "/metrics", http.NoBody)
+		req.RemoteAddr = "127.0.0.1:40000"
+		req.Header.Set("X-Forwarded-For", client)
+		req.SetBasicAuth("prometheus", password)
+		srv.ServeHTTP(rec, req)
+
+		return rec.Code
+	}
+
+	for i := range server.MetricsRequestsPerMinute {
+		if code := get("203.0.113.7", "wrong"); code != http.StatusUnauthorized {
+			t.Fatalf("guess %d: status = %d, want %d",
+				i+1, code, http.StatusUnauthorized)
+		}
+	}
+
+	if code := get("203.0.113.7", "right"); code != http.StatusTooManyRequests {
+		t.Fatalf("right password past the limit: status = %d, want %d",
+			code, http.StatusTooManyRequests)
+	}
+
+	if code := get("203.0.113.8", "right"); code != http.StatusOK {
+		t.Fatalf("another client: status = %d, want %d",
+			code, http.StatusOK)
+	}
+}
+
+// TestMetricsInTwoServers: two servers in one process can both have
+// metrics on.
+func TestMetricsInTwoServers(t *testing.T) {
+	t.Setenv("METRICS_USERNAME", "prometheus")
+	t.Setenv("METRICS_PASSWORD", "right")
+
+	for range 2 {
+		newServer(t).SetupRoutes()
+	}
+}
+
+// TestSentry: with SENTRY_DSN empty there is no Sentry client. With it
+// pointing at a local server standing in for Sentry, a panic in a
+// handler reaches that server, and the request still gets the 500 from
+// the panic recovery.
+func TestSentry(t *testing.T) {
+	const panicMessage = "handler panic for TestSentry"
+
+	// sentry.Init sets the client for the whole process; take it away
+	// again so that no other test reports to Sentry.
+	t.Cleanup(func() { sentry.CurrentHub().BindClient(nil) })
+
+	t.Setenv("SENTRY_DSN", "")
+	newServer(t)
+
+	if sentry.CurrentHub().Client() != nil {
+		t.Fatal("a Sentry client exists with SENTRY_DSN empty")
+	}
+
+	// The body of the first request the stand-in for Sentry receives.
+	received := make(chan string, 1)
+
+	sentryServer := httptest.NewServer(http.HandlerFunc(
+		func(_ http.ResponseWriter, r *http.Request) {
+			body, _ := io.ReadAll(r.Body)
+
+			select {
+			case received <- string(body):
+			default:
+			}
+		},
+	))
+	defer sentryServer.Close()
+
+	t.Setenv("SENTRY_DSN",
+		"http://key@"+sentryServer.Listener.Addr().String()+"/1")
+
+	srv := newServer(t)
+	srv.SetupRoutes()
+	srv.Router().Get("/panic", func(http.ResponseWriter, *http.Request) {
+		panic(panicMessage)
+	})
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequestWithContext(t.Context(),
+		http.MethodGet, "/panic", http.NoBody)
+	srv.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusInternalServerError {
+		t.Fatalf("status = %d, want %d",
+			rec.Code, http.StatusInternalServerError)
+	}
+
+	// Sentry sends from a goroutine of its own.
+	select {
+	case body := <-received:
+		if !strings.Contains(body, panicMessage) {
+			t.Fatalf("the Sentry server received no report of the panic:\n%s",
+				body)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("nothing reached the Sentry server")
+	}
+}
+
+// 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.
+func TestHealthCheckRejectsOversizeBody(t *testing.T) {
+	t.Parallel()
+
+	srv := newServer(t)
+	srv.SetupRoutes()
+
+	rec := httptest.NewRecorder()
+	req := httptest.NewRequestWithContext(t.Context(),
+		http.MethodGet, "/.well-known/healthcheck",
+		strings.NewReader(
+			strings.Repeat("x", int(server.MaxRequestBodyBytes)+1),
+		),
+	)
+
+	srv.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusRequestEntityTooLarge {
+		t.Fatalf("status = %d, want %d",
+			rec.Code, http.StatusRequestEntityTooLarge)
+	}
+
+	if got := rec.Body.String(); got != "{\"status\":\"error\"}\n" {
+		t.Errorf("body = %q, want %q", got, "{\"status\":\"error\"}\n")
+	}
+
+	got := rec.Header().Get("Content-Type")
+	if got != "application/json; charset=utf-8" {
+		t.Errorf("Content-Type = %q, want a JSON content type", got)
+	}
+}

internal/server/server.go

@@ -0,0 +1,142 @@
+// Package server provides the HTTP server lifecycle,
+// including startup, routing, and graceful shutdown. The
+// process lifetime is owned by fx: shutdown is requested
+// through fx.Shutdowner so every component's OnStop hook runs
+// in dependency order.
+package server
+
+import (
+	"context"
+	"fmt"
+	"log/slog"
+	"net/http"
+	"time"
+
+	"sneak.berlin/go/netwatch/internal/config"
+	"sneak.berlin/go/netwatch/internal/globals"
+	"sneak.berlin/go/netwatch/internal/handlers"
+	"sneak.berlin/go/netwatch/internal/logger"
+	"sneak.berlin/go/netwatch/internal/middleware"
+
+	"github.com/getsentry/sentry-go"
+	"github.com/go-chi/chi/v5"
+	"go.uber.org/fx"
+)
+
+// sentryFlushTimeout is how long shutdown waits for Sentry to send
+// what it still holds.
+const sentryFlushTimeout = 2 * time.Second
+
+// Params defines the dependencies for Server.
+type Params struct {
+	fx.In
+
+	Config     *config.Config
+	Globals    *globals.Globals
+	Handlers   *handlers.Handlers
+	Logger     *logger.Logger
+	Middleware *middleware.Middleware
+	Shutdowner fx.Shutdowner
+}
+
+// Server is the top-level HTTP server orchestrator.
+type Server struct {
+	h          *handlers.Handlers
+	httpServer *http.Server
+	log        *slog.Logger
+	mw         *middleware.Middleware
+	params     Params
+	router     *chi.Mux
+	shutdowner fx.Shutdowner
+}
+
+// New creates a Server and registers lifecycle hooks for
+// starting and stopping it.
+func New(
+	lc fx.Lifecycle,
+	params Params,
+) (*Server, error) {
+	s := new(Server)
+	s.params = params
+	s.mw = params.Middleware
+	s.h = params.Handlers
+	s.log = params.Logger.Get()
+	s.shutdowner = params.Shutdowner
+
+	err := s.enableSentry()
+	if err != nil {
+		return nil, err
+	}
+
+	lc.Append(fx.Hook{
+		OnStart: func(_ context.Context) error {
+			// Build the router and http.Server synchronously
+			// here, before spawning the serving goroutine, so
+			// httpServer is fully constructed by the time OnStop
+			// (or an early signal) can read it. fx guarantees
+			// OnStart returns before OnStop runs, so no
+			// synchronization or nil check is needed at shutdown.
+			s.SetupRoutes()
+			s.httpServer = s.newHTTPServer()
+
+			go s.listenAndServe()
+
+			return nil
+		},
+		OnStop: func(ctx context.Context) error {
+			return s.shutdown(ctx)
+		},
+	})
+
+	return s, nil
+}
+
+// ServeHTTP delegates to the chi router.
+func (s *Server) ServeHTTP(
+	w http.ResponseWriter,
+	r *http.Request,
+) {
+	s.router.ServeHTTP(w, r)
+}
+
+// enableSentry sets Sentry up when SENTRY_DSN is set, so that
+// SetupRoutes can report panics to it. With SENTRY_DSN empty it does
+// nothing. A DSN Sentry refuses stops the start.
+func (s *Server) enableSentry() error {
+	if s.params.Config.SentryDSN == "" {
+		return nil
+	}
+
+	err := sentry.Init(sentry.ClientOptions{
+		Dsn:     s.params.Config.SentryDSN,
+		Release: s.params.Globals.Appname + "-" + s.params.Globals.Version,
+	})
+	if err != nil {
+		return fmt.Errorf("SENTRY_DSN: %w", err)
+	}
+
+	s.log.Info("sentry error reporting activated")
+
+	return nil
+}
+
+// shutdown gracefully stops the HTTP server within the
+// deadline of the context fx provides for OnStop, then gives
+// Sentry, if set up, time to send what it still holds.
+func (s *Server) shutdown(ctx context.Context) error {
+	err := s.httpServer.Shutdown(ctx)
+
+	if s.params.Config.SentryDSN != "" {
+		sentry.Flush(sentryFlushTimeout)
+	}
+
+	if err != nil {
+		s.log.Error("server clean shutdown failed", "error", err)
+
+		return err
+	}
+
+	s.log.Info("server stopped")
+
+	return nil
+}