Default WEBHOOKER_ENVIRONMENT to prod

An unset WEBHOOKER_ENVIRONMENT now resolves to prod rather than dev, so
an operator who forgets the variable is not silently permissive. The
only behaviour dev still changes is the CORS middleware, which answers
every origin with Access-Control-Allow-Origin: *; that is now off
unless dev is set explicitly. Cookie Secure and CSRF strictness are
decided per request from the transport and are unaffected.

Updated resolveEnvironment and its comment, the README configuration
table and prose, and tests covering the default and the explicit dev.
Comments that justified behaviour by the old dev default (the
TRUSTED_PROXIES warning, a CSRF test) were corrected; that warning
still fires in every environment when TRUSTED_PROXIES is empty.

Model: opus-4-8

README.md

@@ -92,7 +92,8 @@ 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:
+or `prod` (default: `prod`; `dev` must be set explicitly). The setting
+controls exactly one behavior:
 
 | Behavior | `dev`                   | `prod`           |
 | -------- | ----------------------- | ---------------- |
@@ -134,7 +135,7 @@ TTY detection, and security headers are always applied.
 
 | Variable                | Description                         | Default  |
 | ----------------------- | ----------------------------------- | -------- |
-| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod`                     | `dev`    |
+| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod`                     | `prod`   |
 | `PORT`                  | HTTP listen port                    | `8080`   |
 | `BIND_ADDRESS`          | IP address the HTTP listener binds. Loopback by default, so the cleartext listener is not published on every interface. The Docker image ships `0.0.0.0` instead. See [Bind address](#bind-address) | `127.0.0.1` (image: `0.0.0.0`) |
 | `DATA_DIR`              | Directory for all SQLite databases  | `/var/lib/webhooker` |
@@ -400,9 +401,9 @@ the bucket is. See [Rate Limiting](#rate-limiting).
 The remedy is to set `TRUSTED_PROXIES` to your reverse proxy's
 address, which restores per-client buckets. webhooker logs a warning
 at startup whenever `TRUSTED_PROXIES` is empty, in every environment —
-not only when `WEBHOOKER_ENVIRONMENT=prod`, because that variable
+the warning does not depend on `WEBHOOKER_ENVIRONMENT`, because an
-defaults to `dev` and an operator who never set it is precisely the
+operator who never configured the deployment is precisely the one at
-one at risk. The warning is informational when nothing proxies to the
+risk. The warning is informational when nothing proxies to the
 process: with no proxy in front, the peer address is the client's own
 and the buckets are already per-client. See
 [Rate Limiting](#rate-limiting) for what each limit shares.
@@ -754,15 +755,15 @@ reports.
    that.
 2. **Set `WEBHOOKER_ENVIRONMENT=prod`, and make sure the proxy sends
    `X-Forwarded-Proto`.** These are two requirements, not one. The
-   environment setting decides CORS and nothing else: the default
+   environment setting decides CORS and nothing else: `dev` answers
-   `dev` answers every origin with `Access-Control-Allow-Origin: *`
+   every origin with `Access-Control-Allow-Origin: *` (without
-   (without credentials), which a server-rendered production
+   credentials), which a server-rendered production deployment has no
-   deployment has no use for. Cookie `Secure` and the strict
+   use for, and `prod` — the default — disables it. Cookie `Secure`
-   Origin/Referer mode are **not** tied to it — they are decided per
+   and the strict Origin/Referer mode are **not** tied to it — they
-   request from the transport, which behind a proxy means the
+   are decided per request from the transport, which behind a proxy
-   `X-Forwarded-Proto` header. The block below sets it; without it
+   means the `X-Forwarded-Proto` header. The block below sets it;
-   every request is read as plaintext and cookies ship without
+   without it every request is read as plaintext and cookies ship
-   `Secure`. See [Configuration](#configuration).
+   without `Secure`. See [Configuration](#configuration).
 3. **Set `TRUSTED_PROXIES` to the proxy's address.** Unset, every rate
    limiter keys on the connecting peer, which behind a proxy is the
    proxy on every request: all clients collapse into one global bucket
@@ -1700,22 +1701,6 @@ retries) is individually logged for full observability.
 
 **Relations:** Belongs to Delivery.
 
-#### Event-tier indexes
-
-Beyond the primary keys, the per-webhook event databases carry secondary
-indexes on the columns the background work reads by, each created by
-`AutoMigrate` on a fresh and on an existing database:
-
-| Column                         | Serves |
-| ------------------------------ | ------ |
-| `deliveries.status`            | The recovery and sweep queries that select deliveries by status once a minute |
-| `deliveries.event_id`          | Loading a page of the event log, which reads deliveries by event |
-| `delivery_results.delivery_id` | Loading a page of the event log, which reads results by delivery |
-| `events.created_at`            | Retention, which deletes events by age |
-
-The `events.resubmitted_from_id` column is also indexed, to resolve the
-resubmit relationship both ways in the event log.
-
 #### Common Fields
 
 Every entity except `Setting` includes these fields from `BaseModel`.

internal/config/config.go

@@ -585,12 +585,14 @@ func resolveMetricsAuth() (string, string, error) {
 	)
 }
 
-// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
+// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to prod
-// dev, and rejects unrecognised values.
+// when it is unset so a deployment that forgets the variable is not
+// silently permissive; dev must be set explicitly. It rejects
+// unrecognised values.
 func resolveEnvironment() (string, error) {
 	environment := os.Getenv("WEBHOOKER_ENVIRONMENT")
 	if environment == "" {
-		environment = EnvironmentDev
+		environment = EnvironmentProd
 	}
 
 	if environment != EnvironmentDev &&
@@ -772,10 +774,10 @@ func (c *Config) warnEgressAllowlist(log *slog.Logger) {
 // everyone else's wrong passwords, and the receiver's limits become
 // service-wide ceilings.
 //
-// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT. That
+// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT: an
-// variable defaults to dev, so gating on it would silence the warning
+// operator who never configured the deployment is exactly the case it
-// for exactly the operator who forgot to configure the deployment —
+// exists to catch, so the exposure it announces is independent of the
-// the case it exists to catch.
+// environment setting.
 //
 // The default of trusting nobody is deliberate — trusting forwarded
 // headers from arbitrary peers lets any client choose its own bucket —

internal/config/config_test.go

@@ -44,9 +44,9 @@ func TestEnvironmentConfig(t *testing.T) {
 		isProd      bool
 	}{
 		{
-			name:   "default is dev",
+			name:   "default is prod",
-			isDev:  true,
+			isDev:  false,
-			isProd: false,
+			isProd: true,
 		},
 		{
 			name:     "explicit dev",
@@ -848,10 +848,10 @@ func TestEgressAllowlistWarning(t *testing.T) {
 // tells an operator a deployment behind a reverse proxy shares one
 // rate-limit bucket between every client, which turns the receiver
 // limits into service-wide ceilings and collapses login failure
-// counting. It must fire whenever TRUSTED_PROXIES is empty,
+// counting. It must fire whenever TRUSTED_PROXIES is empty, in any
-// in any environment: WEBHOOKER_ENVIRONMENT defaults to dev, so gating
+// environment: the warning does not depend on WEBHOOKER_ENVIRONMENT,
-// on it would silence the warning for exactly the operator who never
+// since an operator who never configured the deployment is exactly the
-// configured the deployment. It stays quiet once proxies are named.
+// one it exists to catch. It stays quiet once proxies are named.
 func TestSharedRateLimitBucketWarning(t *testing.T) {
 	tests := []struct {
 		name           string
@@ -871,10 +871,9 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
 			expectWarning:  false,
 		},
 		{
-			// The default environment. An internet-exposed
+			// An internet-exposed deployment whose operator set
-			// deployment whose operator never set
+			// WEBHOOKER_ENVIRONMENT=dev has exactly the exposure
-			// WEBHOOKER_ENVIRONMENT lands here and has exactly
+			// the warning announces.
-			// the exposure the warning announces.
 			name:          "dev without trusted proxies warns",
 			environment:   config.EnvironmentDev,
 			expectWarning: true,

internal/database/database.go

@@ -17,12 +17,12 @@ import (
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/banner"
 	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/datadir"
 	"sneak.berlin/go/webhooker/internal/gormlog"
 	"sneak.berlin/go/webhooker/internal/logger"
 )
 
 const (
-	dataDirPerm       = 0750
 	randomPasswordLen = 16
 	sessionKeyLen     = 32
 )
@@ -185,7 +185,9 @@ func (d *Database) connect() error {
 // caller's decision.
 func (d *Database) connectTo(dataDir string) error {
 	// Ensure the data directory exists before opening the database.
-	err := os.MkdirAll(dataDir, dataDirPerm)
+	// datadir.DirPerm is the single source of the directory mode; this
+	// package creates the directory too, since either may run first.
+	err := os.MkdirAll(dataDir, datadir.DirPerm)
 	if err != nil {
 		return fmt.Errorf(
 			"creating data directory %s: %w",

internal/database/event_tier_indexes_test.go

@@ -1,72 +0,0 @@
-package database_test
-
-import (
-	"context"
-	"testing"
-
-	"github.com/google/uuid"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"sneak.berlin/go/webhooker/internal/database"
-)
-
-// indexedColumn names a secondary index by the model and struct field
-// GORM derives the index name from.
-type indexedColumn struct {
-	model any
-	field string
-}
-
-// eventTierIndexes are the columns the background work reads by: the
-// recovery and sweep queries (status), the event log (event_id and
-// delivery_id) and retention (created_at).
-var eventTierIndexes = []indexedColumn{
-	{&database.Delivery{}, "Status"},
-	{&database.Delivery{}, "EventID"},
-	{&database.DeliveryResult{}, "DeliveryID"},
-	{&database.Event{}, "CreatedAt"},
-}
-
-// TestWebhookDBManager_OpenAddsEventTierIndexes verifies that opening a
-// per-webhook database that predates these indexes creates them, so the
-// queries above stop scanning whole tables. It stands in for an older
-// database file by dropping the indexes AutoMigrate just created, then
-// reopening the same file.
-func TestWebhookDBManager_OpenAddsEventTierIndexes(t *testing.T) {
-	t.Parallel()
-
-	mgr, lc := setupTestWebhookDBManager(t)
-	ctx := context.Background()
-	require.NoError(t, lc.Start(ctx))
-
-	defer func() { require.NoError(t, lc.Stop(ctx)) }()
-
-	webhookID := uuid.New().String()
-
-	db, err := mgr.GetDB(webhookID)
-	require.NoError(t, err)
-
-	// A fresh database has them.
-	for _, ix := range eventTierIndexes {
-		require.True(t, db.Migrator().HasIndex(ix.model, ix.field))
-	}
-
-	// Stand in for a database file created before the indexes existed.
-	for _, ix := range eventTierIndexes {
-		require.NoError(t, db.Migrator().DropIndex(ix.model, ix.field))
-		require.False(t, db.Migrator().HasIndex(ix.model, ix.field))
-	}
-
-	// Drop the cached connection so the next open reopens the file and
-	// runs AutoMigrate against it, as a restart would.
-	require.NoError(t, mgr.CloseAll())
-
-	db, err = mgr.GetDB(webhookID)
-	require.NoError(t, err)
-
-	for _, ix := range eventTierIndexes {
-		assert.True(t, db.Migrator().HasIndex(ix.model, ix.field),
-			"opening the existing database should create the index on %s",
-			ix.field)
-	}
-}

internal/database/model_delivery.go

@@ -32,9 +32,9 @@ func (s DeliveryStatus) Terminal() bool {
 type Delivery struct {
 	BaseModel
 
-	EventID  string         `gorm:"type:uuid;not null;index"         json:"eventId"`
+	EventID  string         `gorm:"type:uuid;not null"         json:"eventId"`
 	TargetID string         `gorm:"type:uuid;not null"         json:"targetId"`
-	Status   DeliveryStatus `gorm:"not null;default:'pending';index" json:"status"`
+	Status   DeliveryStatus `gorm:"not null;default:'pending'" json:"status"`
 
 	// Relations
 	Event           Event            `json:"event,omitzero"`

internal/database/model_delivery_result.go

@@ -4,7 +4,7 @@ package database
 type DeliveryResult struct {
 	BaseModel
 
-	DeliveryID   string `gorm:"type:uuid;not null;index" json:"deliveryId"`
+	DeliveryID   string `gorm:"type:uuid;not null"   json:"deliveryId"`
 	AttemptNum   int    `gorm:"not null"             json:"attemptNum"`
 	Success      bool   `json:"success"`
 	StatusCode   int    `json:"statusCode,omitempty"`

internal/database/model_event.go

@@ -1,17 +1,9 @@
 package database
 
-import "time"
-
 // Event represents a captured webhook event
 type Event struct {
 	BaseModel
 
-	// CreatedAt overrides BaseModel.CreatedAt only to add an index:
-	// retention deletes events by age, so events.created_at is queried
-	// on every sweep. The other tables keep the unindexed BaseModel
-	// field.
-	CreatedAt time.Time `gorm:"index" json:"createdAt"`
-
 	WebhookID    string `gorm:"type:uuid;not null" json:"webhookId"`
 	EntrypointID string `gorm:"type:uuid;not null" json:"entrypointId"`
 

internal/database/webhook_db_manager.go

@@ -13,6 +13,7 @@ import (
 	"gorm.io/driver/sqlite"
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/config"
+	"sneak.berlin/go/webhooker/internal/datadir"
 	"sneak.berlin/go/webhooker/internal/gormlog"
 	"sneak.berlin/go/webhooker/internal/logger"
 )
@@ -53,8 +54,9 @@ func NewWebhookDBManager(
 		log:     params.Logger.Get(),
 	}
 
-	// Create data directory if it doesn't exist
+	// Create data directory if it doesn't exist. datadir.DirPerm is the
-	err := os.MkdirAll(m.dataDir, dataDirPerm)
+	// single source of the directory mode; either package may run first.
+	err := os.MkdirAll(m.dataDir, datadir.DirPerm)
 	if err != nil {
 		return nil, fmt.Errorf(
 			"creating data directory %s: %w",

internal/datadir/lock.go

@@ -29,9 +29,11 @@ import (
 // process that was killed with SIGKILL blocks nothing.
 const LockFileName = "webhooker.lock"
 
-// dirPerm is the mode Acquire creates DATA_DIR with. It matches what
+// DirPerm is the mode DATA_DIR is created with. It is the single
-// internal/database uses, since whichever runs first creates it.
+// source of that mode: internal/database consumes it rather than
-const dirPerm = 0o750
+// keeping its own copy, so the two packages that both create the
+// directory cannot drift into disagreeing about its permissions.
+const DirPerm = 0o750
 
 // ErrLocked reports that another live process holds the data
 // directory. Callers that need to know whether a deployment is running
@@ -64,7 +66,7 @@ func Acquire(dir string) (*Lock, error) {
 		return nil, ErrNoDir
 	}
 
-	err := os.MkdirAll(dir, dirPerm)
+	err := os.MkdirAll(dir, DirPerm)
 	if err != nil {
 		return nil, fmt.Errorf(
 			"creating data directory %s: %w", dir, err,

internal/middleware/csrf_test.go

@@ -380,9 +380,8 @@ func csrfTookStrictPath(
 
 // TestCSRF_ForwardedProtoSpellingsTakeStrictPath runs the header
 // spellings a real proxy emits through the middleware. The environment
-// is dev -- the DEFAULT when WEBHOOKER_ENVIRONMENT is unset -- to pin
+// is set to dev -- the permissive setting -- to pin that the routing is
-// that the routing is a per-request transport decision and owes
+// a per-request transport decision and owes nothing to configuration.
-// nothing to configuration.
 func TestCSRF_ForwardedProtoSpellingsTakeStrictPath(t *testing.T) {
 	t.Parallel()