README.md

@@ -71,8 +71,18 @@ make clean      # Remove bin/
 ### Configuration
 
 All configuration is via environment variables. For local development,
-you can place variables in a `.env` file in the project root (loaded
+you can place variables in a `.env` file in the process working
-automatically via `godotenv/autoload`).
+directory, read once at startup before anything else looks at the
+environment.
+
+The file is optional and having none is the normal case for a
+deployment. A file that is there but cannot be parsed aborts startup
+with a message naming it, because a single malformed line makes none
+of the file apply: every variable in it silently reverts to its
+default, which is exactly the failure [Invalid values abort
+startup](#invalid-values-abort-startup) exists to prevent, for all of
+them at once. A variable already present in the real environment wins
+over the file's value for the same name.
 
 The environment is selected by setting `WEBHOOKER_ENVIRONMENT` to `dev`
 or `prod` (default: `dev`). The setting controls exactly one behavior:
@@ -125,7 +135,7 @@ TTY detection, and security headers are always applied.
 | `MAINTENANCE_MODE`      | Report `maintenanceMode: true` in the healthcheck JSON. It does not change how any request is served — no maintenance page exists | `false`  |
 | `METRICS_USERNAME`      | Basic auth username for `/metrics`. Must be set together with `METRICS_PASSWORD`; one without the other fails startup | `""`     |
 | `METRICS_PASSWORD`      | Basic auth password for `/metrics`. Must be set together with `METRICS_USERNAME`; one without the other fails startup | `""`     |
-| `SENTRY_DSN`            | Sentry error reporting DSN          | `""`     |
+| `SENTRY_DSN`            | Sentry error reporting DSN. Unset leaves error reporting off; a value the Sentry SDK cannot parse fails startup rather than serving with reporting silently off | `""`     |
 | `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
 | `SESSION_IDLE_TIMEOUT`  | Idle session timeout (Go duration)  | `24h`    |
 | `RECEIVER_RATE_LIMIT`   | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
@@ -464,10 +474,20 @@ startup), every entry in `TRUSTED_PROXIES` and
 `ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address, and
 `BIND_ADDRESS` must be an IP address literal — `localhost`,
 `127.0.0.1:8080` and `10.0.0.0/8` are each rejected rather than
-resolved, split, or narrowed to something they do not say.
+resolved, split, or narrowed to something they do not say — and
+`SENTRY_DSN` must parse as a Sentry DSN.
 `SESSION_IDLE_TIMEOUT` is the exception: a
 non-positive value there means idle expiry is disabled, not invalid.
 
+`SENTRY_DSN` is checked with the Sentry SDK's own parser, the same call
+the SDK makes on the DSN it is later handed, so what configuration
+accepts is exactly what will initialise. A typo in it is the one
+configuration mistake nothing downstream can ever notice — the variable
+is still set, so every later signal reports error reporting as on while
+no report is being sent — which is why it aborts rather than starting
+with reporting off. Leaving it unset is not a mistake and not affected:
+error reporting is simply off and startup is normal.
+
 Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
 spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
 `true`, `True`, `0`, `f`, `F`, `FALSE`, `false`, `False` — and nothing

cmd/webhooker/dotenv_test.go

@@ -0,0 +1,107 @@
+package main
+
+import (
+	"bytes"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/config"
+)
+
+// dotEnvKey is a throwaway variable name these tests write and read,
+// so they cannot disturb real configuration.
+const dotEnvKey = "WEBHOOKER_TEST_DISPATCH_VALUE"
+
+// writeDotEnvInWorkingDir puts contents in a .env file in a fresh
+// temporary directory and moves the process there.
+//
+// The callers are deliberately not parallel and must stay that way:
+// t.Chdir moves the whole process. Go releases parallel tests only
+// after every sequential test in the package has finished, so nothing
+// else runs while these do.
+func writeDotEnvInWorkingDir(t *testing.T, contents string) {
+	t.Helper()
+
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(
+		filepath.Join(dir, config.DotEnvPath),
+		[]byte(contents), 0o600,
+	))
+	t.Chdir(dir)
+}
+
+// TestDispatch_MalformedDotEnvRefuses pins the second half of the
+// defect. godotenv applies nothing at all when a file will not parse,
+// so one mistyped line used to revert every variable in it to its
+// default and start the server anyway, with no log line naming the
+// file. The refusal has to arrive before any subcommand runs, which
+// is why `help` — the one subcommand that touches nothing — is still
+// refused here.
+//
+//nolint:paralleltest // t.Chdir moves the whole process.
+func TestDispatch_MalformedDotEnvRefuses(t *testing.T) {
+	writeDotEnvInWorkingDir(t, "PORT 19615\n")
+
+	var stdout, stderr bytes.Buffer
+
+	code := dispatch(
+		[]string{helpCommand}, strings.NewReader(""), &stdout, &stderr,
+	)
+
+	require.Equal(t, 1, code, "a broken .env must exit non-zero")
+	assert.Contains(
+		t, stderr.String(), config.DotEnvPath,
+		"the refusal must name the file",
+	)
+	assert.Empty(
+		t, stdout.String(),
+		"the subcommand must not have run",
+	)
+}
+
+// TestDispatch_LoadsDotEnvBeforeSubcommands pins the ordering the
+// godotenv/autoload import used to provide for free. It ran in an
+// init(), so .env was in the environment before anything read it —
+// including config.DataDir, which both the DATA_DIR lock and resetpw
+// call outside the fx graph. Loading any later would let a .env that
+// sets DATA_DIR lock one directory while the config opened databases
+// in another.
+func TestDispatch_LoadsDotEnvBeforeSubcommands(t *testing.T) {
+	t.Setenv(dotEnvKey, "placeholder")
+	require.NoError(t, os.Unsetenv(dotEnvKey))
+
+	writeDotEnvInWorkingDir(t, dotEnvKey+"=from-dot-env\n")
+
+	var stdout, stderr bytes.Buffer
+
+	code := dispatch(
+		[]string{helpCommand}, strings.NewReader(""), &stdout, &stderr,
+	)
+
+	require.Equal(t, 0, code)
+	assert.Equal(
+		t, "from-dot-env", os.Getenv(dotEnvKey),
+		"the file must be applied before the subcommand runs",
+	)
+}
+
+// TestDispatch_MissingDotEnvIsFine pins the case most deployments are
+// in: no .env at all, which must stay a normal start.
+//
+//nolint:paralleltest // t.Chdir moves the whole process.
+func TestDispatch_MissingDotEnvIsFine(t *testing.T) {
+	t.Chdir(t.TempDir())
+
+	var stdout, stderr bytes.Buffer
+
+	code := dispatch(
+		[]string{helpCommand}, strings.NewReader(""), &stdout, &stderr,
+	)
+
+	require.Equal(t, 0, code)
+	assert.Empty(t, stderr.String())
+}

cmd/webhooker/main.go

@@ -54,6 +54,11 @@ const stopTimeout = 5 * time.Second
 // caller can tell "called wrong" from "declined".
 const exitUsage = 2
 
+// helpCommand is the subcommand that prints usage. The flag spellings
+// beside it in the switch are aliases; this is the name the usage text
+// documents and the one tests invoke.
+const helpCommand = "help"
+
 // Build-time variables set via -ldflags.
 //
 //nolint:gochecknoglobals // Build-time variables injected by the linker.
@@ -75,11 +80,27 @@ func main() {
 // every existing deployment invoke; that path is unchanged, including
 // where the DATA_DIR lock is taken relative to building the fx graph
 // and how fx propagates a non-zero exit itself.
+//
+// The optional .env file is read here, before any subcommand and so
+// before anything reads the environment — config.DataDir, which both
+// the DATA_DIR lock and resetpw call outside the fx graph, above all.
+// It used to be read from an init() in internal/config, which put it
+// earlier still but threw the error away: a single malformed line
+// applied none of the file and said nothing about it. A file that is
+// not there stays fine, since .env is optional and most deployments
+// do not have one.
 func dispatch(
 	args []string,
 	stdin io.Reader,
 	stdout, stderr io.Writer,
 ) int {
+	err := config.LoadDotEnv()
+	if err != nil {
+		_, _ = fmt.Fprintf(stderr, "%s: %v\n", appname, err)
+
+		return 1
+	}
+
 	if len(args) == 0 {
 		return run(stderr)
 	}
@@ -87,7 +108,7 @@ func dispatch(
 	switch args[0] {
 	case resetpw.Name:
 		return resetpw.Run(args[1:], stdin, stdout, stderr)
-	case "help", "-h", "-help", "--help":
+	case helpCommand, "-h", "-help", "--help":
 		usage(stdout)
 
 		return 0

cmd/webhooker/main_test.go

@@ -121,7 +121,7 @@ func TestDispatch_Help(t *testing.T) {
 	var stdout, stderr bytes.Buffer
 
 	code := dispatch(
-		[]string{"help"}, strings.NewReader(""), &stdout, &stderr,
+		[]string{helpCommand}, strings.NewReader(""), &stdout, &stderr,
 	)
 
 	require.Equal(t, 0, code)

internal/config/config.go

@@ -4,6 +4,7 @@ package config
 import (
 	"errors"
 	"fmt"
+	"io/fs"
 	"log/slog"
 	"net/netip"
 	"os"
@@ -11,13 +12,11 @@ import (
 	"strings"
 	"time"
 
+	"github.com/getsentry/sentry-go"
+	"github.com/joho/godotenv"
 	"go.uber.org/fx"
 	"sneak.berlin/go/webhooker/internal/globals"
 	"sneak.berlin/go/webhooker/internal/logger"
-
-	// Populates the environment from a ./.env file automatically for
-	// development configuration. Kept in one place only (here).
-	_ "github.com/joho/godotenv/autoload"
 )
 
 const (
@@ -84,6 +83,12 @@ const (
 	// IPv6 prefix spends on the ::ffff:0:0/96 wrapper, so a /104
 	// covers the same addresses as an IPv4 /8.
 	mappedV4Offset = 96
+
+	// DotEnvPath is the optional file of KEY=value lines read into the
+	// environment at startup, relative to the process working
+	// directory. Exported so that documentation and tests name the
+	// same path the loader opens.
+	DotEnvPath = ".env"
 )
 
 // ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
@@ -107,6 +112,15 @@ var ErrInvalidCIDR = errors.New("invalid CIDR")
 // something that is not an IP address literal.
 var ErrInvalidBindAddress = errors.New("invalid bind address")
 
+// ErrInvalidSentryDSN is returned when SENTRY_DSN is set to something
+// the Sentry SDK cannot parse as a DSN.
+var ErrInvalidSentryDSN = errors.New("invalid Sentry DSN")
+
+// ErrDotEnvUnreadable is returned when the optional .env file exists
+// but cannot be read or parsed. A file that is not there is not an
+// error; a file that is there and broken is.
+var ErrDotEnvUnreadable = errors.New("unreadable .env file")
+
 // ErrIncompleteMetricsAuth is returned when exactly one of
 // METRICS_USERNAME and METRICS_PASSWORD carries a value. Neither
 // fallback is acceptable: serving /metrics on the username alone
@@ -212,12 +226,62 @@ func (c *Config) MetricsAuthEnabled() bool {
 	return c.MetricsUsername != "" && c.MetricsPassword != ""
 }
 
+// SentryEnabled reports whether error reporting is shipped to Sentry.
+// It is the only answer to that question in the codebase: the SDK
+// initialisation, the sentryhttp middleware registration and the
+// startup log's sentryEnabled field all read this one method, so the
+// log cannot report reporting as on while nothing is sending.
+//
+// A non-empty DSN is enough because loadFromEnv already parsed it with
+// the SDK's own parser and refused to build a Config around one the
+// SDK would reject, and because initialising the SDK with a DSN that
+// parsed and failed anyway aborts the process rather than leaving this
+// true and the client absent.
+func (c *Config) SentryEnabled() bool {
+	return c.SentryDSN != ""
+}
+
 // envString returns the value of the named environment variable,
 // or an empty string if not set.
 func envString(key string) string {
 	return os.Getenv(key)
 }
 
+// LoadDotEnv reads DotEnvPath into the environment when that file is
+// present, and reports a file that is present but broken.
+//
+// It has to run before anything reads the environment, so that every
+// reader agrees on what the environment holds — the DATA_DIR lock
+// taken before the fx graph exists as much as loadFromEnv itself. A
+// variable already set in the real environment wins: godotenv never
+// overwrites one.
+//
+// A missing file is not an error. It is a development convenience and
+// most deployments set the environment directly.
+//
+// Any other failure is. godotenv parses the whole file before setting
+// anything, so a single malformed line applies none of it: every
+// variable in the file silently reverts to its default, which defeats
+// the fail-loud guarantee for all of them at once.
+func LoadDotEnv() error {
+	return loadDotEnvFile(DotEnvPath)
+}
+
+// loadDotEnvFile is LoadDotEnv over a named file, so tests can point
+// at a temporary one instead of the process working directory.
+func loadDotEnvFile(path string) error {
+	err := godotenv.Load(path)
+	if err == nil || errors.Is(err, fs.ErrNotExist) {
+		return nil
+	}
+
+	return fmt.Errorf(
+		"%w: %s: %w; nothing in it was applied, so fix the file or "+
+			"remove it",
+		ErrDotEnvUnreadable, path, err,
+	)
+}
+
 // DataDir resolves DATA_DIR, applying DefaultDataDir when it is unset
 // or empty. It is exported so that entry points which must act on the
 // data directory before the fx graph exists — taking the exclusive
@@ -462,6 +526,41 @@ func envBindAddress(key, defaultValue string) (string, error) {
 	return addr.String(), nil
 }
 
+// envSentryDSN returns the value of the named environment variable
+// checked as a Sentry DSN. An unset (or empty, or whitespace-only)
+// value yields "", which means error reporting stays off — the common
+// case, and a normal start.
+//
+// A set value is parsed with sentry.NewDsn, which is the call
+// sentry.Init makes on the DSN it is handed, so what passes here is
+// exactly what the SDK will accept later and the two cannot disagree.
+// Reproducing the check by hand instead would cost this package its
+// dependency on the SDK — already a module dependency, already linked
+// into the binary — in exchange for a second definition of "valid DSN"
+// free to drift from the one that decides.
+//
+// A set value that does not parse is a hard error naming the key, so
+// startup fails loudly. Losing error reporting is the failure this
+// variable exists to prevent, and a typo in a DSN is silent forever:
+// nothing later in the process can notice that reports are going
+// nowhere. The bad value is quoted because it is a URL to a public
+// endpoint carrying a public key, not a secret.
+func envSentryDSN(key string) (string, error) {
+	v := strings.TrimSpace(os.Getenv(key))
+	if v == "" {
+		return "", nil
+	}
+
+	_, err := sentry.NewDsn(v)
+	if err != nil {
+		return "", fmt.Errorf(
+			"%w: %s: %q: %w", ErrInvalidSentryDSN, key, v, err,
+		)
+	}
+
+	return v, nil
+}
+
 // resolveMetricsAuth reads the /metrics basic-auth credentials and
 // rejects a half-set pair, naming both variables either way. The
 // error carries neither value: the password is a secret.
@@ -594,6 +693,11 @@ func loadFromEnv() (*Config, error) {
 		return nil, err
 	}
 
+	sentryDSN, err := envSentryDSN("SENTRY_DSN")
+	if err != nil {
+		return nil, err
+	}
+
 	return &Config{
 		DataDir:                DataDir(),
 		Debug:                  debug,
@@ -603,7 +707,7 @@ func loadFromEnv() (*Config, error) {
 		MetricsPassword:        metricsPassword,
 		Port:                   port,
 		BindAddress:            bindAddress,
-		SentryDSN:              envString("SENTRY_DSN"),
+		SentryDSN:              sentryDSN,
 		RetentionSweepInterval: retentionSweepInterval,
 		SessionIdleTimeout:     sessionIdleTimeout,
 		ReceiverRateLimit:      receiverRateLimit,
@@ -738,7 +842,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
 		"receiverRateLimit", s.ReceiverRateLimit,
 		"trustedProxies", len(s.TrustedProxies),
 		"allowedEgressCIDRs", len(s.AllowedEgressCIDRs),
-		"hasSentryDSN", s.SentryDSN != "",
+		"sentryEnabled", s.SentryEnabled(),
 		"hasMetricsAuth", s.MetricsAuthEnabled(),
 	)
 

internal/config/dotenv_test.go

@@ -0,0 +1,158 @@
+package config_test
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/config"
+)
+
+// dotEnvKey is a throwaway variable name the .env tests write and
+// read, so they cannot disturb real configuration.
+const dotEnvKey = "WEBHOOKER_TEST_DOTENV_VALUE"
+
+// malformedDotEnv is a file godotenv cannot parse. The first line is
+// the realistic typo — a space where the `=` belongs — and the rest
+// make sure nothing downstream treats the file as salvageable line by
+// line.
+const malformedDotEnv = "PORT 19615\n" +
+	"this is not = valid ! syntax\n" +
+	"\"unclosed\n"
+
+// unsetDotEnvKey makes dotEnvKey genuinely absent for the duration of
+// the test and restores it afterwards. t.Setenv registers the restore;
+// the Unsetenv that follows is what the test actually needs, because a
+// variable set to the empty string is still present in os.Environ and
+// godotenv would refuse to overwrite it.
+func unsetDotEnvKey(t *testing.T) {
+	t.Helper()
+	t.Setenv(dotEnvKey, "placeholder")
+	require.NoError(t, os.Unsetenv(dotEnvKey))
+}
+
+// writeDotEnv writes contents to a .env file in a fresh temporary
+// directory and returns its path.
+func writeDotEnv(t *testing.T, contents string) string {
+	t.Helper()
+
+	path := filepath.Join(t.TempDir(), config.DotEnvPath)
+	require.NoError(t, os.WriteFile(path, []byte(contents), 0o600))
+
+	return path
+}
+
+// TestLoadDotEnv_MissingFileIsFine pins the case most deployments are
+// in. The file is optional: it is a development convenience, and a
+// deployment that configures the environment directly must start
+// normally rather than be refused for a file it was never meant to
+// have.
+//
+//nolint:paralleltest // unsetDotEnvKey uses t.Setenv.
+func TestLoadDotEnv_MissingFileIsFine(t *testing.T) {
+	unsetDotEnvKey(t)
+
+	absent := filepath.Join(t.TempDir(), config.DotEnvPath)
+	require.NoError(t, config.LoadDotEnvFileForTest(absent))
+
+	_, present := os.LookupEnv(dotEnvKey)
+	assert.False(t, present, "nothing may be set from an absent file")
+}
+
+// TestLoadDotEnv_AppliesValues pins that a well-formed file still
+// reaches the environment, which is the whole reason the file is read
+// at all.
+//
+//nolint:paralleltest // unsetDotEnvKey uses t.Setenv.
+func TestLoadDotEnv_AppliesValues(t *testing.T) {
+	unsetDotEnvKey(t)
+
+	path := writeDotEnv(t, "# a comment\n"+dotEnvKey+"=from-dot-env\n")
+
+	require.NoError(t, config.LoadDotEnvFileForTest(path))
+	assert.Equal(t, "from-dot-env", os.Getenv(dotEnvKey))
+}
+
+// TestLoadDotEnv_RealEnvironmentWins pins that the file cannot
+// override a variable the process was actually started with. A
+// deployment that sets DATA_DIR in its unit file must not have it
+// silently replaced by a stale .env left in the working directory.
+func TestLoadDotEnv_RealEnvironmentWins(t *testing.T) {
+	t.Setenv(dotEnvKey, "from-environment")
+
+	path := writeDotEnv(t, dotEnvKey+"=from-dot-env\n")
+
+	require.NoError(t, config.LoadDotEnvFileForTest(path))
+	assert.Equal(t, "from-environment", os.Getenv(dotEnvKey))
+}
+
+// TestLoadDotEnv_MalformedFileAborts is the defect this fixes. One bad
+// line makes godotenv apply none of the file, so every variable in it
+// reverts to its default; the process used to start that way with no
+// log line naming the file at all.
+//
+//nolint:paralleltest // unsetDotEnvKey uses t.Setenv.
+func TestLoadDotEnv_MalformedFileAborts(t *testing.T) {
+	unsetDotEnvKey(t)
+
+	path := writeDotEnv(
+		t, malformedDotEnv+dotEnvKey+"=from-dot-env\n",
+	)
+
+	err := config.LoadDotEnvFileForTest(path)
+
+	require.Error(t, err)
+	require.ErrorIs(t, err, config.ErrDotEnvUnreadable)
+	assert.Contains(
+		t, err.Error(), config.DotEnvPath,
+		"the failure must name the file it could not read",
+	)
+
+	_, present := os.LookupEnv(dotEnvKey)
+	assert.False(
+		t, present,
+		"a rejected file must apply nothing, not part of itself",
+	)
+}
+
+// TestLoadDotEnv_UnreadableFileAborts pins that only absence is
+// tolerated. A .env that exists but cannot be read is a file the
+// operator meant to be applied, so it fails like a malformed one
+// rather than being treated as though it were not there.
+func TestLoadDotEnv_UnreadableFileAborts(t *testing.T) {
+	t.Parallel()
+
+	// A directory in the file's place: open succeeds and the read
+	// fails, which no umask or root-ness can turn back into success
+	// the way a chmod could.
+	path := filepath.Join(t.TempDir(), config.DotEnvPath)
+	require.NoError(t, os.Mkdir(path, 0o750))
+
+	err := config.LoadDotEnvFileForTest(path)
+
+	require.Error(t, err)
+	require.ErrorIs(t, err, config.ErrDotEnvUnreadable)
+}
+
+// TestLoadDotEnv_ReadsTheWorkingDirectory pins the path LoadDotEnv
+// itself opens, which the tests above bypass. It is relative to the
+// process working directory, as it was under godotenv/autoload and as
+// the README documents.
+//
+//nolint:paralleltest // t.Chdir moves the whole process.
+func TestLoadDotEnv_ReadsTheWorkingDirectory(t *testing.T) {
+	unsetDotEnvKey(t)
+
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(
+		filepath.Join(dir, config.DotEnvPath),
+		[]byte(dotEnvKey+"=from-working-directory\n"),
+		0o600,
+	))
+	t.Chdir(dir)
+
+	require.NoError(t, config.LoadDotEnv())
+	assert.Equal(t, "from-working-directory", os.Getenv(dotEnvKey))
+}

internal/config/env_test.go

@@ -518,8 +518,20 @@ type badEnvValueCase struct {
 }
 
 // badEnvValueCases is the config.New table, kept out of the test body
-// so the test itself stays readable.
+// so the test itself stays readable. It is assembled from per-variable
+// groups because one literal covering every variable outgrew the
+// function-length budget.
 func badEnvValueCases() []badEnvValueCase {
+	cases := listenerEnvValueCases()
+	cases = append(cases, flagEnvValueCases()...)
+	cases = append(cases, sentryEnvValueCases()...)
+
+	return cases
+}
+
+// listenerEnvValueCases covers the two variables that describe the
+// HTTP listener.
+func listenerEnvValueCases() []badEnvValueCase {
 	return []badEnvValueCase{
 		{
 			name:  "valid PORT is used",
@@ -542,27 +554,6 @@ func badEnvValueCases() []badEnvValueCase {
 			value:       "70000",
 			expectError: true,
 		},
-		{
-			name:  "valid DEBUG is used",
-			key:   envKeyDebug,
-			value: "true",
-			check: func(t *testing.T, cfg *config.Config) {
-				t.Helper()
-				assert.True(t, cfg.Debug)
-			},
-		},
-		{
-			name:        "unparseable DEBUG aborts startup",
-			key:         envKeyDebug,
-			value:       "ture",
-			expectError: true,
-		},
-		{
-			name:        "unparseable MAINTENANCE_MODE aborts startup",
-			key:         envKeyMaintenanceMode,
-			value:       "sometimes",
-			expectError: true,
-		},
 		{
 			name:  "valid BIND_ADDRESS is used",
 			key:   envKeyBindAddress,
@@ -595,6 +586,69 @@ func badEnvValueCases() []badEnvValueCase {
 	}
 }
 
+// flagEnvValueCases covers the boolean variables.
+func flagEnvValueCases() []badEnvValueCase {
+	return []badEnvValueCase{
+		{
+			name:  "valid DEBUG is used",
+			key:   envKeyDebug,
+			value: "true",
+			check: func(t *testing.T, cfg *config.Config) {
+				t.Helper()
+				assert.True(t, cfg.Debug)
+			},
+		},
+		{
+			name:        "unparseable DEBUG aborts startup",
+			key:         envKeyDebug,
+			value:       "ture",
+			expectError: true,
+		},
+		{
+			name:        "unparseable MAINTENANCE_MODE aborts startup",
+			key:         envKeyMaintenanceMode,
+			value:       "sometimes",
+			expectError: true,
+		},
+	}
+}
+
+// sentryEnvValueCases covers SENTRY_DSN. The three rejected values are
+// the ones measured on the defect: each initialised the SDK with an
+// error and left the process serving with error reporting off.
+func sentryEnvValueCases() []badEnvValueCase {
+	return []badEnvValueCase{
+		{
+			name:  "valid SENTRY_DSN is used",
+			key:   envKeySentryDSN,
+			value: validSentryDSN,
+			check: func(t *testing.T, cfg *config.Config) {
+				t.Helper()
+				assert.Equal(t, validSentryDSN, cfg.SentryDSN)
+				assert.True(t, cfg.SentryEnabled())
+			},
+		},
+		{
+			name:        "unparseable SENTRY_DSN aborts startup",
+			key:         envKeySentryDSN,
+			value:       "not-a-dsn",
+			expectError: true,
+		},
+		{
+			name:        "SENTRY_DSN that is not a URL aborts startup",
+			key:         envKeySentryDSN,
+			value:       "%%%",
+			expectError: true,
+		},
+		{
+			name:        "keyless SENTRY_DSN aborts startup",
+			key:         envKeySentryDSN,
+			value:       "https://example.invalid/1",
+			expectError: true,
+		},
+	}
+}
+
 // TestNewUsesDefaultsWhenUnset proves the fail-loud behaviour did not
 // break the legitimate unset case: absent variables still get their
 // documented defaults.
@@ -603,7 +657,7 @@ func TestNewUsesDefaultsWhenUnset(t *testing.T) {
 
 	for _, key := range []string{
 		envKeyPort, envKeyDebug, envKeyMaintenanceMode,
-		envKeyBindAddress,
+		envKeyBindAddress, envKeySentryDSN,
 	} {
 		require.NoError(t, os.Unsetenv(key))
 	}
@@ -626,4 +680,9 @@ func TestNewUsesDefaultsWhenUnset(t *testing.T) {
 		t, config.DefaultBindAddressForTest, cfg.BindAddress,
 	)
 	assert.Equal(t, bindAddressDefault, cfg.BindAddress)
+
+	// An absent SENTRY_DSN is the common case and must stay a normal
+	// start with error reporting off, not a refusal.
+	assert.Empty(t, cfg.SentryDSN)
+	assert.False(t, cfg.SentryEnabled())
 }

internal/config/export_test.go

@@ -51,6 +51,18 @@ func EnvPortForTest(key string, defaultValue int) (int, error) {
 	return envPort(key, defaultValue)
 }
 
+// EnvSentryDSNForTest exposes envSentryDSN.
+func EnvSentryDSNForTest(key string) (string, error) {
+	return envSentryDSN(key)
+}
+
+// LoadDotEnvFileForTest exposes the loader LoadDotEnv runs, over a
+// caller-named file rather than the process working directory, so
+// each .env state can be covered without moving the test process.
+func LoadDotEnvFileForTest(path string) error {
+	return loadDotEnvFile(path)
+}
+
 // EnvBindAddressForTest exposes envBindAddress.
 func EnvBindAddressForTest(key, defaultValue string) (string, error) {
 	return envBindAddress(key, defaultValue)

internal/config/sentry_test.go

@@ -0,0 +1,141 @@
+package config_test
+
+import (
+	"os"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/webhooker/internal/config"
+)
+
+// envKeySentryDSN is the variable envSentryDSN reads in production.
+const envKeySentryDSN = "SENTRY_DSN"
+
+// validSentryDSN is a syntactically complete DSN. The host is under
+// .invalid (RFC 2606), so nothing a test builds around it can reach a
+// real Sentry installation.
+const validSentryDSN = "https://abc123@sentry.invalid/42"
+
+// envSentryDSNCase is one row of the envSentryDSN table.
+type envSentryDSNCase struct {
+	name        string
+	set         bool
+	value       string
+	expectError bool
+	expected    string
+}
+
+// envSentryDSNCases is the envSentryDSN table. The three invalid
+// values are the ones measured on the defect: each initialised the SDK
+// with an error and left the process serving with reporting off.
+func envSentryDSNCases() []envSentryDSNCase {
+	return []envSentryDSNCase{
+		{
+			name:     "unset means reporting off",
+			expected: "",
+		},
+		{
+			name:     "empty means reporting off",
+			set:      true,
+			value:    "",
+			expected: "",
+		},
+		{
+			name:     "whitespace means reporting off",
+			set:      true,
+			value:    "   ",
+			expected: "",
+		},
+		{
+			name:     "a valid DSN is kept",
+			set:      true,
+			value:    validSentryDSN,
+			expected: validSentryDSN,
+		},
+		{
+			name:     "surrounding whitespace is trimmed",
+			set:      true,
+			value:    "  " + validSentryDSN + "\t",
+			expected: validSentryDSN,
+		},
+		{
+			name:        "a value that is not a URL is rejected",
+			set:         true,
+			value:       "not-a-dsn",
+			expectError: true,
+		},
+		{
+			name:        "an unparseable URL is rejected",
+			set:         true,
+			value:       "%%%",
+			expectError: true,
+		},
+		{
+			name:        "a DSN without a public key is rejected",
+			set:         true,
+			value:       "https://example.invalid/1",
+			expectError: true,
+		},
+		{
+			name:        "a DSN without a project id is rejected",
+			set:         true,
+			value:       "https://abc123@sentry.invalid/",
+			expectError: true,
+		},
+		{
+			name:        "a non-HTTP scheme is rejected",
+			set:         true,
+			value:       "ftp://abc123@sentry.invalid/42",
+			expectError: true,
+		},
+	}
+}
+
+// TestEnvSentryDSN covers the helper directly. What it pins beyond the
+// value is the failure shape: a set-but-unparseable DSN names the
+// variable and the value, exactly as the other fail-loud helpers do,
+// so an operator reads the fix off the message.
+func TestEnvSentryDSN(t *testing.T) {
+	for _, tt := range envSentryDSNCases() {
+		t.Run(tt.name, func(t *testing.T) {
+			// Cannot use t.Parallel() here because t.Setenv
+			// is incompatible with parallel subtests.
+			if tt.set {
+				t.Setenv(envKeySentryDSN, tt.value)
+			} else {
+				require.NoError(t, os.Unsetenv(envKeySentryDSN))
+			}
+
+			got, err := config.EnvSentryDSNForTest(envKeySentryDSN)
+
+			if tt.expectError {
+				require.Error(t, err)
+				require.ErrorIs(t, err, config.ErrInvalidSentryDSN)
+				assert.Contains(t, err.Error(), envKeySentryDSN)
+				assert.Contains(t, err.Error(), tt.value)
+				assert.Empty(t, got)
+
+				return
+			}
+
+			require.NoError(t, err)
+			assert.Equal(t, tt.expected, got)
+		})
+	}
+}
+
+// TestSentryEnabled_TracksTheDSN pins that the one method answering
+// "is anything being reported" agrees with the DSN in every state. The
+// startup log, the SDK initialisation and the sentryhttp middleware
+// all read it, so a log field cannot report reporting as on while
+// nothing is sending.
+func TestSentryEnabled_TracksTheDSN(t *testing.T) {
+	t.Parallel()
+
+	assert.False(t, (&config.Config{}).SentryEnabled())
+	assert.True(
+		t,
+		(&config.Config{SentryDSN: validSentryDSN}).SentryEnabled(),
+	)
+}

internal/server/http.go

@@ -70,7 +70,7 @@ func (s *Server) serveUntilShutdown() {
 	err := s.httpServer.ListenAndServe()
 	if err != nil && !errors.Is(err, http.ErrServerClosed) {
 		s.log.Error("listen error", "error", err)
-		s.shutdownOnListenFailure()
+		s.shutdownWithFailure()
 	}
 }
 

internal/server/listen_failure_test.go

@@ -93,7 +93,7 @@ func requireListenFailureExit(t *testing.T, env *testEnv) {
 	select {
 	case sig := <-app.Wait():
 		require.Equal(
-			t, server.ListenFailureExitCode, sig.ExitCode,
+			t, server.StartupFailureExitCode, sig.ExitCode,
 			"listen failure must exit non-zero",
 		)
 	case <-time.After(listenFailureDeadline):

internal/server/sentry_failure_test.go

@@ -0,0 +1,99 @@
+package server_test
+
+import (
+	"context"
+	"net"
+	"strconv"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"go.uber.org/fx"
+	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/globals"
+	"sneak.berlin/go/webhooker/internal/server"
+)
+
+// TestSentryInitFailure_ShutsDownTheApp pins that error reporting
+// which is configured and cannot be started ends the application
+// instead of serving without it.
+//
+// The measured defect logged `sentry init failure` and kept running,
+// so the deployment served traffic with reporting off while every
+// other signal — SENTRY_DSN still set, the startup summary's own
+// field — said it was on. Nothing later in the process can notice
+// that reports are going nowhere, which is why this exits rather than
+// degrades.
+//
+// The DSN is placed on a hand-built Config, which is the only way to
+// reach this branch at all: loadFromEnv now parses SENTRY_DSN with
+// sentry.NewDsn, the same call sentry.Init makes, so a DSN that
+// survives configuration cannot fail initialisation in the SDK
+// version this pins. The branch stays because that is a property of
+// the SDK's current implementation rather than of its contract.
+func TestSentryInitFailure_ShutsDownTheApp(t *testing.T) {
+	t.Parallel()
+
+	port := freePort(t)
+
+	env := newTestEnvWithConfig(t, &config.Config{
+		DataDir:     t.TempDir(),
+		Environment: config.EnvironmentDev,
+		BindAddress: loopbackV4,
+		Port:        port,
+		SentryDSN:   "not-a-dsn",
+	})
+
+	app := fx.New(
+		fx.NopLogger,
+		fx.Supply(env.log, env.cfg, env.mw, env.hnd),
+		fx.Provide(globals.New, server.New),
+		fx.Invoke(func(*server.Server) {}),
+	)
+
+	startCtx, cancelStart := context.WithTimeout(
+		context.Background(), lifecycleTimeout,
+	)
+	defer cancelStart()
+
+	require.NoError(t, app.Start(startCtx))
+
+	select {
+	case sig := <-app.Wait():
+		require.Equal(
+			t, server.StartupFailureExitCode, sig.ExitCode,
+			"a sentry failure must exit non-zero",
+		)
+	case <-time.After(listenFailureDeadline):
+		t.Fatal("a sentry failure left the app running")
+	}
+
+	// The stop sequence still has to complete: the failure must reach
+	// shutdown through fx rather than around it.
+	stopCtx, cancelStop := context.WithTimeout(
+		context.Background(), lifecycleTimeout,
+	)
+	defer cancelStop()
+
+	require.NoError(t, app.Stop(stopCtx))
+
+	// And it must give up before it listens. A process that bound the
+	// port and then exited would have accepted requests it could not
+	// report on, which is the state under test in miniature.
+	requireBindable(t, port)
+}
+
+// requireBindable asserts that the port is free, which it is only if
+// the server under test never claimed it.
+func requireBindable(t *testing.T, port int) {
+	t.Helper()
+
+	var listenCfg net.ListenConfig
+
+	listener, err := listenCfg.Listen(
+		t.Context(), "tcp",
+		net.JoinHostPort(loopbackV4, strconv.Itoa(port)),
+	)
+	require.NoError(t, err, "the server bound a port it then gave up")
+	require.NoError(t, listener.Close())
+}

internal/server/server.go

@@ -51,12 +51,13 @@ const (
 	minSentryFlush = 250 * time.Millisecond
 )
 
-// ListenFailureExitCode is the status the process exits with when the
+// StartupFailureExitCode is the status the process exits with when
-// HTTP listener cannot be established, or dies for a reason other
+// the serving goroutine gives up: the HTTP listener cannot be
-// than a requested shutdown. It must stay non-zero: systemd
+// established or dies for a reason other than a requested shutdown, or
-// `Restart=on-failure` and Docker's restart policies key off it, and a
+// error reporting is configured and cannot be started. It must stay
-// zero exit would read as a deliberate stop.
+// non-zero: systemd `Restart=on-failure` and Docker's restart policies
-const ListenFailureExitCode = 1
+// key off it, and a zero exit would read as a deliberate stop.
+const StartupFailureExitCode = 1
 
 // SentryFlushBudget reports how long the Sentry flush may run when
 // remaining is the time left on the fx stop context after the HTTP
@@ -135,11 +136,25 @@ func New(lc fx.Lifecycle, params ServerParams) (*Server, error) {
 }
 
 // Run configures Sentry and starts serving HTTP requests.
+//
+// A Sentry failure ends the application instead of listening. It runs
+// before the listener rather than after it so that the process never
+// binds a port it is about to give up.
 func (s *Server) Run() {
 	s.configure()
 
 	// logging before sentry, because sentry logs
-	s.enableSentry()
+	err := s.enableSentry()
+	if err != nil {
+		s.log.Error(
+			"SENTRY_DSN is set but error reporting could not be "+
+				"started; refusing to serve with it off",
+			"error", err,
+		)
+		s.shutdownWithFailure()
+
+		return
+	}
 
 	s.serve()
 }
@@ -150,11 +165,23 @@ func (s *Server) MaintenanceMode() bool {
 	return s.params.Config.MaintenanceMode
 }
 
-func (s *Server) enableSentry() {
+// enableSentry initialises the Sentry SDK when error reporting is
+// configured, and reports the failure when it is configured and cannot
+// be initialised. A DSN that is not set is not a failure: reporting
+// stays off and the server starts normally.
+//
+// There is no fallback to running with reporting off. An operator who
+// set SENTRY_DSN asked for failures to be visible, and serving traffic
+// with reporting quietly off is the one state nothing can ever tell
+// them about — the DSN is still set, so every later signal says it is
+// on. Config already refused a DSN the SDK cannot parse, which is what
+// a typo produces, so reaching this branch means the SDK refused
+// something that parsed: not a condition to guess at either.
+func (s *Server) enableSentry() error {
 	s.sentryEnabled.Store(false)
 
-	if s.params.Config.SentryDSN == "" {
+	if !s.params.Config.SentryEnabled() {
-		return
+		return nil
 	}
 
 	err := sentry.Init(sentryClientOptions(
@@ -166,19 +193,19 @@ func (s *Server) enableSentry() {
 		),
 	))
 	if err != nil {
-		s.log.Error("sentry init failure", "error", err)
+		return fmt.Errorf("initialising sentry: %w", err)
-		// Don't use fatal since we still want the service to run
-		return
 	}
 
 	s.log.Info("sentry error reporting activated")
 	s.sentryEnabled.Store(true)
+
+	return nil
 }
 
 // serve installs the signal watcher, starts the listener and blocks
 // until the server's context is cancelled. The process exit status is
 // fx's to decide — from a signal, or from the code
-// shutdownOnListenFailure hands the Shutdowner — so this reports
+// shutdownWithFailure hands the Shutdowner — so this reports
 // nothing back to its caller.
 func (s *Server) serve() {
 	ctx, cancelFunc := context.WithCancel(context.Background())
@@ -208,20 +235,24 @@ func (s *Server) serve() {
 	// Do not call cleanShutdown() here to avoid double invocation.
 }
 
-// shutdownOnListenFailure ends the application after the HTTP
+// shutdownWithFailure ends the application non-zero from the serving
-// listener failed. The fx OnStart hook returns as soon as the serving
+// goroutine. It is how anything on that goroutine fails fatally: the
-// goroutine is spawned, so nothing downstream of it ever learns that
+// fx OnStart hook returns as soon as the goroutine is spawned, so
-// the listen failed: fx reports RUNNING and the process sits alive
+// nothing downstream of it ever learns that the goroutine gave up. fx
-// with nothing bound, which is invisible to systemd and Docker
+// reports RUNNING and the process sits alive having done neither what
-// restart policies. Asking the Shutdowner to stop the app with a
+// it was asked nor anything visible instead, which systemd and
-// non-zero code is what turns that into a visible failure.
+// Docker restart policies cannot see. Asking the Shutdowner to stop
+// the app with a non-zero code is what turns that into a visible
+// failure, and it is the whole of "fatal" here — no panic, no
+// os.Exit, and every stop hook still runs.
 //
-// The context cancel that follows only unwinds serve()'s own wait.
+// The context cancel that follows only unwinds serve()'s own wait,
-// The shutdown itself runs through fx's normal stop sequence, so the
+// and is skipped before serve has installed one. The shutdown itself
-// clean-shutdown drain in cleanShutdown is reached unchanged.
+// runs through fx's normal stop sequence, so the clean-shutdown drain
-func (s *Server) shutdownOnListenFailure() {
+// in cleanShutdown is reached unchanged.
+func (s *Server) shutdownWithFailure() {
 	err := s.params.Shutdowner.Shutdown(
-		fx.ExitCode(ListenFailureExitCode),
+		fx.ExitCode(StartupFailureExitCode),
 	)
 	if err != nil {
 		s.log.Error("shutdown request failed", "error", err)