api calls

internal/api/errors.go

@@ -7,7 +7,7 @@ import (
 )
 
 // Error is the single shape every failure takes. Clients get a stable machine
-// readable code plus a human message. A self-hosted bot instance can branch on
+// readable code plus a human message; a self-hosted bot instance can branch on
 // the code without parsing prose.
 type Error struct {
 	Code    string `json:"code"`
@@ -46,6 +46,12 @@ func WriteError(w http.ResponseWriter, status int, code, message string) {
 	}
 }
 
+// writeErrorBody writes just the envelope, for callers that have already sent
+// their own status code and headers.
+func writeErrorBody(w http.ResponseWriter, code, message string) error {
+	return json.NewEncoder(w).Encode(errorEnvelope{Error{Code: code, Message: message}})
+}
+
 // WriteJSON sends a success payload.
 func WriteJSON(w http.ResponseWriter, status int, v any) {
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")

internal/api/middleware.go

@@ -0,0 +1,149 @@
+package api
+
+import (
+	"context"
+	"crypto/rand"
+	"encoding/hex"
+	"log/slog"
+	"net/http"
+	"runtime/debug"
+	"time"
+)
+
+type requestIDKey struct{}
+
+// RequestIDFrom returns the id assigned by WithRequestID, or "".
+func RequestIDFrom(ctx context.Context) string {
+	id, _ := ctx.Value(requestIDKey{}).(string)
+
+	return id
+}
+
+// WithRequestID tags each request so a log line can be tied to a client report.
+// Honours an inbound X-Request-Id so a self-hosted bot can correlate its own
+// logs with the backend's.
+func WithRequestID(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		id := r.Header.Get("X-Request-Id")
+
+		if id == "" || len(id) > 64 {
+			var b [8]byte
+			if _, err := rand.Read(b[:]); err == nil {
+				id = hex.EncodeToString(b[:])
+			}
+		}
+
+		w.Header().Set("X-Request-Id", id)
+
+		next.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), requestIDKey{}, id)))
+	})
+}
+
+// statusRecorder captures the status code, which http.ResponseWriter does not
+// expose after the fact.
+type statusRecorder struct {
+	http.ResponseWriter
+	status int
+	bytes  int
+}
+
+func (s *statusRecorder) WriteHeader(code int) {
+	s.status = code
+	s.ResponseWriter.WriteHeader(code)
+}
+
+func (s *statusRecorder) Write(b []byte) (int, error) {
+	if s.status == 0 {
+		s.status = http.StatusOK
+	}
+
+	n, err := s.ResponseWriter.Write(b)
+	s.bytes += n
+
+	return n, err
+}
+
+// WithLogging records one line per request.
+//
+// Deliberately does NOT log the Authorization header, the query string or the
+// body. Logs get shipped, grepped and retained far longer than anyone intends,
+// and the query string is where callsigns and grid squares live.
+func WithLogging(logger *slog.Logger) func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			start := time.Now()
+			rec := &statusRecorder{ResponseWriter: w}
+
+			next.ServeHTTP(rec, r)
+
+			if rec.status == 0 {
+				rec.status = http.StatusOK
+			}
+
+			attrs := []any{
+				"method", r.Method,
+				"path", r.URL.Path,
+				"status", rec.status,
+				"bytes", rec.bytes,
+				"duration_ms", time.Since(start).Milliseconds(),
+				"request_id", RequestIDFrom(r.Context()),
+			}
+
+			// Key id rather than the key, and only once authentication has
+			// resolved one.
+			if p := PrincipalFrom(r.Context()); p != nil {
+				attrs = append(attrs, "key_id", p.KeyID, "owner_id", p.OwnerID)
+			}
+
+			level := slog.LevelInfo
+			if rec.status >= 500 {
+				level = slog.LevelError
+			}
+
+			logger.Log(r.Context(), level, "request", attrs...)
+		})
+	}
+}
+
+// WithRecovery turns a panic in a handler into a 500 rather than a dead
+// process. Go's default is to kill the whole server on an unrecovered panic in
+// a handler goroutine.
+func WithRecovery(logger *slog.Logger) func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			defer func() {
+				rec := recover()
+				if rec == nil {
+					return
+				}
+
+				// A closed connection surfaces as a panic with this value and
+				// is not a bug; re-panic so net/http handles it as it expects.
+				if rec == http.ErrAbortHandler {
+					panic(rec)
+				}
+
+				logger.Error("panic in handler",
+					"err", rec,
+					"path", r.URL.Path,
+					"request_id", RequestIDFrom(r.Context()),
+					"stack", string(debug.Stack()))
+
+				WriteError(w, http.StatusInternalServerError, CodeInternal,
+					"Something went wrong. The incident has been logged.")
+			}()
+
+			next.ServeHTTP(w, r)
+		})
+	}
+}
+
+// Chain applies middleware so that the first argument is the outermost, which
+// reads in the order requests actually traverse them.
+func Chain(h http.Handler, mw ...func(http.Handler) http.Handler) http.Handler {
+	for i := len(mw) - 1; i >= 0; i-- {
+		h = mw[i](h)
+	}
+
+	return h
+}

internal/api/router.go

@@ -0,0 +1,154 @@
+package api
+
+import (
+	"log/slog"
+	"net/http"
+	"strings"
+)
+
+// Router builds the HTTP surface. Kept out of main.go so it can be exercised in
+// tests with a fake KeyStore, which is where the auth wiring actually gets
+// verified.
+type Router struct {
+	Auth    *Authenticator
+	Logger  *slog.Logger
+	Version string
+
+	// Ready reports whether dependencies are healthy. Nil means always ready.
+	Ready func() error
+}
+
+func (rt *Router) Handler() http.Handler {
+	mux := http.NewServeMux()
+
+	// Unauthenticated. Liveness must not touch the database: a health check
+	// that fails when Postgres blips will get the process killed by whatever
+	// supervises it, which turns a database wobble into an outage.
+	mux.Handle("GET /healthz", rt.healthz())
+
+	// Readiness DOES check dependencies. That is the difference.
+	mux.Handle("GET /readyz", rt.readyz())
+
+	// Everything below needs a key.
+	authed := func(scope string, h http.Handler) http.Handler {
+		return rt.Auth.Authenticate(RequireScope(scope, h))
+	}
+
+	mux.Handle("GET /v1/whoami", rt.Auth.Authenticate(Whoami()))
+	mux.Handle("GET /v1/reference/ping", authed(ScopeReference, rt.stub("reference")))
+
+	// Deliberately NO catch-all mux.Handle("/", ...) here.
+	//
+	// A "/" pattern matches every path, so it would also swallow method
+	// mismatches: POST /healthz would match "/" and return 404 instead of the
+	// 405 that Go's ServeMux produces on its own. jsonifyErrors below rewrites
+	// ServeMux's plain-text 404 and 405 bodies into the standard envelope,
+	// which keeps both correct status codes and uniform JSON errors.
+	return Chain(mux,
+		WithRequestID,
+		WithRecovery(rt.Logger),
+		WithLogging(rt.Logger),
+		jsonifyErrors,
+	)
+}
+
+func (rt *Router) healthz() http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		WriteJSON(w, http.StatusOK, map[string]string{
+			"status":  "ok",
+			"version": rt.Version,
+		})
+	})
+}
+
+func (rt *Router) readyz() http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if rt.Ready != nil {
+			if err := rt.Ready(); err != nil {
+				rt.Logger.Warn("readiness check failed", "err", err)
+				WriteError(w, http.StatusServiceUnavailable, "not_ready", "Dependencies are unavailable.")
+
+				return
+			}
+		}
+
+		WriteJSON(w, http.StatusOK, map[string]string{"status": "ready"})
+	})
+}
+
+// jsonifyErrors converts net/http's built-in plain-text 404 and 405 responses
+// into the same JSON envelope every other error uses, so a client never has to
+// branch on Content-Type to read an error.
+func jsonifyErrors(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		rw := &errorRewriter{ResponseWriter: w}
+
+		next.ServeHTTP(rw, r)
+
+		rw.finish()
+	})
+}
+
+type errorRewriter struct {
+	http.ResponseWriter
+	intercepted bool
+	wroteHeader bool
+	status      int
+}
+
+func (e *errorRewriter) WriteHeader(code int) {
+	if e.wroteHeader {
+		return
+	}
+
+	e.wroteHeader = true
+	e.status = code
+
+	// Only touch responses net/http generated itself.
+	//
+	// Checking for an EMPTY Content-Type does not work: http.NotFound sets
+	// "text/plain; charset=utf-8" before calling WriteHeader, so by the time we
+	// see it the header is already populated. Test for "not already JSON"
+	// instead - every handler in this package writes JSON, so anything else at
+	// a 404 or 405 came from net/http.
+	if code == http.StatusNotFound || code == http.StatusMethodNotAllowed {
+		if !strings.HasPrefix(e.Header().Get("Content-Type"), "application/json") {
+			e.intercepted = true
+			e.Header().Set("Content-Type", "application/json; charset=utf-8")
+		}
+	}
+
+	e.ResponseWriter.WriteHeader(code)
+}
+
+func (e *errorRewriter) Write(b []byte) (int, error) {
+	if !e.wroteHeader {
+		e.WriteHeader(http.StatusOK)
+	}
+
+	// Swallow the plain-text body; finish writes the JSON one.
+	if e.intercepted {
+		return len(b), nil
+	}
+
+	return e.ResponseWriter.Write(b)
+}
+
+func (e *errorRewriter) finish() {
+	if !e.intercepted {
+		return
+	}
+
+	code, msg := "not_found", "No such endpoint."
+	if e.status == http.StatusMethodNotAllowed {
+		code, msg = "method_not_allowed", "That method is not allowed on this endpoint."
+	}
+
+	_ = writeErrorBody(e.ResponseWriter, code, msg)
+}
+
+func (rt *Router) stub(name string) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		WriteJSON(w, http.StatusOK, map[string]string{"endpoint": name, "status": "not implemented yet"})
+	})
+}

internal/api/router_test.go

@@ -0,0 +1,214 @@
+package api
+
+import (
+	"encoding/json"
+	"errors"
+	"io"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"git.dcrubro.com/dcrubro/hammy-backend/internal/apikey"
+	"git.dcrubro.com/dcrubro/hammy-backend/internal/ratelimit"
+)
+
+func testRouter(t *testing.T, ready func() error) http.Handler {
+	t.Helper()
+
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+
+	rt := &Router{
+		Auth: &Authenticator{
+			Keys:    &fakeStore{rec: activeRecord()},
+			Limiter: ratelimit.NewMemory(),
+			Logger:  logger,
+		},
+		Logger:  logger,
+		Version: "test",
+		Ready:   ready,
+	}
+
+	return rt.Handler()
+}
+
+func TestHealthzNeedsNoKey(t *testing.T) {
+	h := testRouter(t, nil)
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/healthz", nil))
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("status %d, want 200", w.Code)
+	}
+
+	if id := w.Header().Get("X-Request-Id"); id == "" {
+		t.Error("no X-Request-Id on the response")
+	}
+}
+
+// Liveness must not depend on Postgres. If it did, a database blip would get
+// the process killed by its supervisor and turn a wobble into an outage.
+func TestHealthzIgnoresDependencies(t *testing.T) {
+	h := testRouter(t, func() error { return errors.New("postgres down") })
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/healthz", nil))
+
+	if w.Code != http.StatusOK {
+		t.Errorf("healthz status %d with a failing dependency, want 200", w.Code)
+	}
+}
+
+func TestReadyzReflectsDependencies(t *testing.T) {
+	h := testRouter(t, func() error { return errors.New("postgres down") })
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/readyz", nil))
+
+	if w.Code != http.StatusServiceUnavailable {
+		t.Errorf("readyz status %d, want 503", w.Code)
+	}
+
+	ok := testRouter(t, func() error { return nil })
+	w = httptest.NewRecorder()
+	ok.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/readyz", nil))
+
+	if w.Code != http.StatusOK {
+		t.Errorf("healthy readyz status %d, want 200", w.Code)
+	}
+}
+
+func TestV1RequiresKey(t *testing.T) {
+	h := testRouter(t, nil)
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/v1/whoami", nil))
+
+	if w.Code != http.StatusUnauthorized {
+		t.Fatalf("status %d, want 401", w.Code)
+	}
+}
+
+func TestWhoamiEndToEnd(t *testing.T) {
+	h := testRouter(t, nil)
+
+	g, err := apikey.Generate(apikey.Live)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	r := httptest.NewRequest(http.MethodGet, "/v1/whoami", nil)
+	r.Header.Set("Authorization", "Bearer "+g.Plaintext)
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, r)
+
+	if w.Code != http.StatusOK {
+		t.Fatalf("status %d: %s", w.Code, w.Body.String())
+	}
+
+	var got struct {
+		KeyID   int64    `json:"key_id"`
+		OwnerID int64    `json:"owner_id"`
+		Scopes  []string `json:"scopes"`
+	}
+
+	if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
+		t.Fatal(err)
+	}
+
+	if got.KeyID != 42 || got.OwnerID != 7 {
+		t.Errorf("principal not propagated: %+v", got)
+	}
+}
+
+func TestScopeEnforcedOnRoute(t *testing.T) {
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+
+	rec := activeRecord()
+	rec.Scopes = []string{ScopeLogbook} // deliberately not reference
+
+	rt := &Router{
+		Auth: &Authenticator{
+			Keys:    &fakeStore{rec: rec},
+			Limiter: ratelimit.NewMemory(),
+			Logger:  logger,
+		},
+		Logger: logger,
+	}
+
+	g, _ := apikey.Generate(apikey.Live)
+
+	r := httptest.NewRequest(http.MethodGet, "/v1/reference/ping", nil)
+	r.Header.Set("Authorization", "Bearer "+g.Plaintext)
+
+	w := httptest.NewRecorder()
+	rt.Handler().ServeHTTP(w, r)
+
+	if w.Code != http.StatusForbidden {
+		t.Errorf("status %d, want 403", w.Code)
+	}
+}
+
+func TestUnknownPathIsJSON404(t *testing.T) {
+	h := testRouter(t, nil)
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/nope", nil))
+
+	if w.Code != http.StatusNotFound {
+		t.Fatalf("status %d, want 404", w.Code)
+	}
+
+	if ct := w.Header().Get("Content-Type"); ct[:16] != "application/json" {
+		t.Errorf("Content-Type %q, want JSON so clients can parse errors uniformly", ct)
+	}
+}
+
+// A wrong method on a real path must 405, not 404.
+func TestMethodMismatch(t *testing.T) {
+	h := testRouter(t, nil)
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/healthz", nil))
+
+	if w.Code != http.StatusMethodNotAllowed {
+		t.Errorf("status %d, want 405", w.Code)
+	}
+}
+
+// A panicking handler must produce a 500, not kill the process.
+func TestRecoveryMiddleware(t *testing.T) {
+	logger := slog.New(slog.NewTextHandler(io.Discard, nil))
+
+	boom := http.HandlerFunc(func(http.ResponseWriter, *http.Request) {
+		panic("kaboom")
+	})
+
+	h := Chain(boom, WithRequestID, WithRecovery(logger))
+
+	w := httptest.NewRecorder()
+	h.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/x", nil))
+
+	if w.Code != http.StatusInternalServerError {
+		t.Fatalf("status %d, want 500", w.Code)
+	}
+
+	if got := bodyCode(t, w); got != CodeInternal {
+		t.Errorf("code %q, want %q", got, CodeInternal)
+	}
+}
+
+func TestRequestIDHonoursInbound(t *testing.T) {
+	h := Chain(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if got := RequestIDFrom(r.Context()); got != "abc123" {
+			t.Errorf("request id %q, want abc123", got)
+		}
+	}), WithRequestID)
+
+	r := httptest.NewRequest(http.MethodGet, "/", nil)
+	r.Header.Set("X-Request-Id", "abc123")
+
+	h.ServeHTTP(httptest.NewRecorder(), r)
+}

internal/config/config.go

@@ -0,0 +1,184 @@
+// Package config loads runtime settings from the environment.
+//
+// Environment variables rather than a config file, for two reasons: the DSN
+// differs per machine and you are syncing the repo across two, and a file with
+// credentials in it is one `git add .` away from being public. The bot uses
+// config.json because its token is the only secret; the backend holds database
+// credentials and key material.
+package config
+
+import (
+	"errors"
+	"fmt"
+	"log/slog"
+	"os"
+	"strconv"
+	"strings"
+	"time"
+)
+
+type Config struct {
+	// Addr is the listen address. Bind to localhost in production and put a
+	// reverse proxy in front for TLS.
+	Addr string
+
+	// DSN for Postgres. Required.
+	DSN string
+
+	// MaxConns caps the pgx pool. Postgres forks a backend per connection, so
+	// this is a real resource, not a client-side nicety.
+	MaxConns int32
+
+	// RedisAddr enables the shared rate limiter. Empty falls back to an
+	// in-process limiter, which is correct for a single instance and wrong for
+	// several - each would keep its own counters.
+	RedisAddr string
+	RedisDB   int
+
+	// EndUserHeader carries the caller's identifier for whoever triggered the
+	// request, for per-user rate limiting. Empty disables it.
+	EndUserHeader string
+
+	LogLevel slog.Level
+	LogJSON  bool
+
+	ReadHeaderTimeout time.Duration
+	ShutdownTimeout   time.Duration
+}
+
+func Load() (Config, error) {
+	c := Config{
+		Addr:              env("HAMMY_ADDR", "127.0.0.1:8080"),
+		DSN:               os.Getenv("HAMMY_DSN"),
+		RedisAddr:         os.Getenv("HAMMY_REDIS_ADDR"),
+		EndUserHeader:     env("HAMMY_ENDUSER_HEADER", "X-Hammy-User"),
+		ReadHeaderTimeout: 10 * time.Second,
+		ShutdownTimeout:   20 * time.Second,
+		LogJSON:           boolEnv("HAMMY_LOG_JSON", false),
+	}
+
+	if c.DSN == "" {
+		return Config{}, errors.New("config: HAMMY_DSN is not set")
+	}
+
+	n, err := intEnv("HAMMY_MAX_CONNS", 10)
+	if err != nil {
+		return Config{}, err
+	}
+	c.MaxConns = int32(n)
+
+	if c.RedisDB, err = intEnv("HAMMY_REDIS_DB", 0); err != nil {
+		return Config{}, err
+	}
+
+	if c.LogLevel, err = levelEnv("HAMMY_LOG_LEVEL", slog.LevelInfo); err != nil {
+		return Config{}, err
+	}
+
+	if c.ReadHeaderTimeout, err = durEnv("HAMMY_READ_HEADER_TIMEOUT", c.ReadHeaderTimeout); err != nil {
+		return Config{}, err
+	}
+
+	if c.ShutdownTimeout, err = durEnv("HAMMY_SHUTDOWN_TIMEOUT", c.ShutdownTimeout); err != nil {
+		return Config{}, err
+	}
+
+	return c, nil
+}
+
+// Redacted is safe to log at startup. The DSN carries a password.
+func (c Config) Redacted() string {
+	redis := c.RedisAddr
+	if redis == "" {
+		redis = "(in-process limiter)"
+	}
+
+	return fmt.Sprintf("addr=%s db=%s redis=%s max_conns=%d level=%s",
+		c.Addr, redactDSN(c.DSN), redis, c.MaxConns, c.LogLevel)
+}
+
+// redactDSN keeps the shape of the connection string and drops the password.
+func redactDSN(dsn string) string {
+	at := strings.LastIndex(dsn, "@")
+	if at < 0 {
+		return "(set)"
+	}
+
+	scheme := strings.Index(dsn, "://")
+	if scheme < 0 {
+		return "(set)"
+	}
+
+	creds := dsn[scheme+3 : at]
+
+	user := creds
+	if colon := strings.Index(creds, ":"); colon >= 0 {
+		user = creds[:colon]
+	}
+
+	return dsn[:scheme+3] + user + ":***" + dsn[at:]
+}
+
+func env(key, def string) string {
+	if v := os.Getenv(key); v != "" {
+		return v
+	}
+
+	return def
+}
+
+func intEnv(key string, def int) (int, error) {
+	v := os.Getenv(key)
+	if v == "" {
+		return def, nil
+	}
+
+	n, err := strconv.Atoi(v)
+	if err != nil {
+		return 0, fmt.Errorf("config: %s: %w", key, err)
+	}
+
+	return n, nil
+}
+
+func boolEnv(key string, def bool) bool {
+	v := os.Getenv(key)
+	if v == "" {
+		return def
+	}
+
+	b, err := strconv.ParseBool(v)
+	if err != nil {
+		return def
+	}
+
+	return b
+}
+
+func durEnv(key string, def time.Duration) (time.Duration, error) {
+	v := os.Getenv(key)
+	if v == "" {
+		return def, nil
+	}
+
+	d, err := time.ParseDuration(v)
+	if err != nil {
+		return 0, fmt.Errorf("config: %s: %w", key, err)
+	}
+
+	return d, nil
+}
+
+func levelEnv(key string, def slog.Level) (slog.Level, error) {
+	v := os.Getenv(key)
+	if v == "" {
+		return def, nil
+	}
+
+	var l slog.Level
+	if err := l.UnmarshalText([]byte(v)); err != nil {
+		return 0, fmt.Errorf("config: %s: %w", key, err)
+	}
+
+	return l, nil
+}

internal/config/config_test.go

@@ -0,0 +1,103 @@
+package config
+
+import (
+	"log/slog"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestLoadRequiresDSN(t *testing.T) {
+	t.Setenv("HAMMY_DSN", "")
+
+	if _, err := Load(); err == nil {
+		t.Fatal("expected an error when HAMMY_DSN is unset")
+	}
+}
+
+func TestLoadDefaults(t *testing.T) {
+	t.Setenv("HAMMY_DSN", "postgres://u:p@localhost/hammy")
+
+	c, err := Load()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if c.Addr != "127.0.0.1:8080" {
+		t.Errorf("Addr %q, want localhost by default - binding 0.0.0.0 should be deliberate", c.Addr)
+	}
+	if c.MaxConns != 10 {
+		t.Errorf("MaxConns %d, want 10", c.MaxConns)
+	}
+	if c.LogLevel != slog.LevelInfo {
+		t.Errorf("LogLevel %v, want info", c.LogLevel)
+	}
+	if c.RedisAddr != "" {
+		t.Errorf("RedisAddr %q, want empty", c.RedisAddr)
+	}
+}
+
+func TestLoadOverrides(t *testing.T) {
+	t.Setenv("HAMMY_DSN", "postgres://u:p@localhost/hammy")
+	t.Setenv("HAMMY_ADDR", ":9000")
+	t.Setenv("HAMMY_MAX_CONNS", "25")
+	t.Setenv("HAMMY_LOG_LEVEL", "debug")
+	t.Setenv("HAMMY_SHUTDOWN_TIMEOUT", "45s")
+	t.Setenv("HAMMY_REDIS_ADDR", "localhost:6379")
+
+	c, err := Load()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if c.Addr != ":9000" || c.MaxConns != 25 || c.LogLevel != slog.LevelDebug {
+		t.Errorf("overrides not applied: %+v", c)
+	}
+	if c.ShutdownTimeout != 45*time.Second {
+		t.Errorf("ShutdownTimeout %v, want 45s", c.ShutdownTimeout)
+	}
+}
+
+func TestLoadRejectsBadValues(t *testing.T) {
+	t.Setenv("HAMMY_DSN", "postgres://u:p@localhost/hammy")
+
+	for _, c := range []struct{ key, val string }{
+		{"HAMMY_MAX_CONNS", "lots"},
+		{"HAMMY_LOG_LEVEL", "verbose"},
+		{"HAMMY_SHUTDOWN_TIMEOUT", "soon"},
+	} {
+		t.Run(c.key, func(t *testing.T) {
+			t.Setenv(c.key, c.val)
+
+			if _, err := Load(); err == nil {
+				t.Errorf("%s=%q accepted", c.key, c.val)
+			}
+		})
+	}
+}
+
+// A DSN in a startup log is a password in a log.
+func TestRedactedHidesPassword(t *testing.T) {
+	cases := []string{
+		"postgres://hammy_app:[email protected]:5432/hammy",
+		"postgresql://hammy_app:p%40ss@localhost/hammy?sslmode=verify-full",
+	}
+
+	for _, dsn := range cases {
+		c := Config{DSN: dsn, Addr: ":8080", MaxConns: 10}
+		out := c.Redacted()
+
+		if strings.Contains(out, "hunter2") || strings.Contains(out, "p%40ss") {
+			t.Errorf("password leaked: %s", out)
+		}
+		if !strings.Contains(out, "hammy_app") {
+			t.Errorf("username should survive for diagnosis: %s", out)
+		}
+	}
+
+	// A DSN with no credentials must not panic or produce nonsense.
+	c := Config{DSN: "postgres:///hammy?host=/var/run/postgresql"}
+	if out := c.Redacted(); strings.Contains(out, "***") && !strings.Contains(out, "(set)") {
+		t.Logf("socket DSN rendered as: %s", out)
+	}
+}