Compare commits

2 Commits
Author SHA1 Message Date
sneak 45d541526f Align DeliveryResult tags and make the index table test-local
check / check (push) Successful in 2m52s
Re-align the json tag column in DeliveryResult after the wider
DeliveryID gorm tag, and move the test's index table out of package
scope into the test function.

Model: opus-5-5
2026-09-28 10:32:00 +00:00
sneak 06f14315fa Index the event-tier columns the sweeps, event log and retention scan (closes #314)
The per-webhook tables declared no secondary indexes, so the recovery
and sweep queries (by delivery status, every minute), the event log
(deliveries by event, results by delivery) and retention (events by
age) each scanned a whole table. Add indexes through GORM model tags so
AutoMigrate creates them on a fresh and on an existing per-webhook
database. events.created_at is indexed by overriding the embedded
BaseModel field on Event alone, leaving the other tables' created_at
unindexed. A test drops the indexes from an opened database, reopens it,
and asserts the open recreated them. The README Data Model section lists
the indexes.

Model: opus-4-8
2026-09-28 10:31:20 +00:00
11 changed files with 95 additions and 218 deletions
+31 -36
View File
@@ -44,9 +44,9 @@ make bootstrap
# Run all checks (test, lint, format check)
make check
# Run the server from the clone. DATA_DIR defaults to
# /var/lib/webhooker in every environment, so set it (in .env or the
# shell) to a writable directory.
# Run in development mode. DATA_DIR defaults to /var/lib/webhooker in
# every environment, so set it (in .env or the shell) to a writable
# directory when running from a clone.
DATA_DIR=./data make dev
# Build Docker image
@@ -92,8 +92,7 @@ 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: `prod`; `dev` must be set explicitly). The setting
controls exactly one behavior:
or `prod` (default: `dev`). The setting controls exactly one behavior:
| Behavior | `dev` | `prod` |
| -------- | ----------------------- | ---------------- |
@@ -135,7 +134,7 @@ TTY detection, and security headers are always applied.
| Variable | Description | Default |
| ----------------------- | ----------------------------------- | -------- |
| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod` | `prod` |
| `WEBHOOKER_ENVIRONMENT` | `dev` or `prod` | `dev` |
| `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 +399,10 @@ 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,
because behind a proxy every client shares one bucket in `dev` and
`prod` alike. The warning is informational when nothing proxies to the
at startup whenever `TRUSTED_PROXIES` is empty, in every environment —
not only when `WEBHOOKER_ENVIRONMENT=prod`, because that variable
defaults to `dev` and an operator who never set it is precisely the
one at 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.
@@ -638,6 +638,7 @@ decision:
docker run -d \
-p 127.0.0.1:8080:8080 \
-v /path/to/data:/var/lib/webhooker \
-e WEBHOOKER_ENVIRONMENT=prod \
-e BIND_ADDRESS=0.0.0.0 \
webhooker:latest
```
@@ -811,18 +812,17 @@ reports.
serves the admin login form and the unauthenticated receiver with
no TLS at all, and the proxy in front of it changes nothing about
that.
2. **Make sure the environment is not `dev` (leave
`WEBHOOKER_ENVIRONMENT` unset or set it to `prod`), and make sure
the proxy sends `X-Forwarded-Proto`.** These are two requirements,
not one. The environment setting decides CORS and nothing else:
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
`dev` answers every origin with `Access-Control-Allow-Origin: *`
(without credentials), which a server-rendered production
deployment has no use for, and `prod` — the default — disables it.
Cookie `Secure` and the strict Origin/Referer mode are **not** tied
to it — they are decided per request from the transport, which
behind a proxy means the `X-Forwarded-Proto` header. The block below
sets it; without it every request is read as plaintext and cookies
ship without `Secure`. See [Configuration](#configuration).
deployment has no use for. Cookie `Secure` and the strict
Origin/Referer mode are **not** tied to it — they are decided per
request from the transport, which behind a proxy means the
`X-Forwarded-Proto` header. The block below sets it; without it
every request is read as plaintext and cookies ship 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
@@ -914,6 +914,7 @@ sent — `$scheme` above does.
With that block, webhooker's environment is:
```sh
WEBHOOKER_ENVIRONMENT=prod
BIND_ADDRESS=127.0.0.1 # the default; stated here to be explicit
TRUSTED_PROXIES=127.0.0.1
```
@@ -1761,25 +1762,19 @@ retries) is individually logged for full observability.
#### Event-tier indexes
These indexes on the per-webhook event databases are declared in the model
tags, so `AutoMigrate` creates them on a fresh and on an existing database:
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:
| Table | Columns | Serves |
| ------------------ | --------------------------- | ------ |
| `deliveries` | `status`, `deleted_at` | Startup recovery, the retry and pending sweeps every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status |
| `deliveries` | `event_id`, `deleted_at` | The event log, which loads each event's deliveries, and retention, which selects and deletes the deliveries of expired events |
| `delivery_results` | `delivery_id`, `deleted_at` | The event log, which loads the attempts of a page's deliveries, and retention, which deletes the attempts of expired events |
| `events` | `deleted_at`, `created_at` | Retention, which selects expired events by age |
| `events` | `created_at` | Retention's delete of the expired events themselves |
| 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 |
GORM's soft delete adds `deleted_at IS NULL` to these queries; retention's
deletes leave it out, but their lookups of expired rows keep it. SQLite keeps
no statistics on these tables, and without them it rates the `deleted_at`
index, which every live row matches, above an index on a column matched
against several values or compared with `<`. So every index but the last also
covers `deleted_at`. It comes second, so that retention's deletes can use the
index without it, except in `events`, where `created_at` is compared with `<`
and SQLite narrows by a `<` only on the last column it uses.
The `events.resubmitted_from_id` column is also indexed, to resolve the
resubmit relationship both ways in the event log.
#### Common Fields
+7 -7
View File
@@ -585,14 +585,12 @@ func resolveMetricsAuth() (string, string, error) {
)
}
// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to prod
// when it is unset so a deployment that forgets the variable is not
// silently permissive; dev must be set explicitly. It rejects
// unrecognised values.
// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
// dev, and rejects unrecognised values.
func resolveEnvironment() (string, error) {
environment := os.Getenv("WEBHOOKER_ENVIRONMENT")
if environment == "" {
environment = EnvironmentProd
environment = EnvironmentDev
}
if environment != EnvironmentDev &&
@@ -774,8 +772,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:
// behind a proxy every client shares one bucket in dev and prod alike.
// The warning is deliberately not gated on WEBHOOKER_ENVIRONMENT. That
// variable defaults to dev, so gating on it would silence the warning
// for exactly the operator who forgot to configure the deployment —
// the case it exists to catch.
//
// The default of trusting nobody is deliberate — trusting forwarded
// headers from arbitrary peers lets any client choose its own bucket —
+11 -6
View File
@@ -44,9 +44,9 @@ func TestEnvironmentConfig(t *testing.T) {
isProd bool
}{
{
name: "default is prod",
isDev: false,
isProd: true,
name: "default is dev",
isDev: true,
isProd: false,
},
{
name: "explicit dev",
@@ -848,9 +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, in any
// environment, because behind a proxy every client shares one bucket
// in dev and prod alike. It stays quiet once proxies are named.
// counting. It must fire whenever TRUSTED_PROXIES is empty,
// in any environment: WEBHOOKER_ENVIRONMENT defaults to dev, so gating
// on it would silence the warning for exactly the operator who never
// configured the deployment. It stays quiet once proxies are named.
func TestSharedRateLimitBucketWarning(t *testing.T) {
tests := []struct {
name string
@@ -870,6 +871,10 @@ func TestSharedRateLimitBucketWarning(t *testing.T) {
expectWarning: false,
},
{
// The default environment. An internet-exposed
// deployment whose operator never set
// WEBHOOKER_ENVIRONMENT lands here and has exactly
// the exposure the warning announces.
name: "dev without trusted proxies warns",
environment: config.EnvironmentDev,
expectWarning: true,
+26 -120
View File
@@ -2,33 +2,37 @@ package database_test
import (
"context"
"fmt"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"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
}
// TestWebhookDBManager_OpenAddsEventTierIndexes verifies that opening a
// per-webhook database that predates these indexes creates them. It
// per-webhook database that predates these indexes creates them, so the
// queries that read by those columns 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()
indexes := []struct {
model any
name string
}{
{&database.Delivery{}, "idx_deliveries_status"},
{&database.Delivery{}, "idx_deliveries_event_id"},
{&database.DeliveryResult{}, "idx_delivery_results_delivery_id"},
{&database.Event{}, "idx_events_deleted_at_created_at"},
{&database.Event{}, "idx_events_created_at"},
// 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).
eventTierIndexes := []indexedColumn{
{&database.Delivery{}, "Status"},
{&database.Delivery{}, "EventID"},
{&database.DeliveryResult{}, "DeliveryID"},
{&database.Event{}, "CreatedAt"},
}
mgr, lc := setupTestWebhookDBManager(t)
@@ -43,14 +47,14 @@ func TestWebhookDBManager_OpenAddsEventTierIndexes(t *testing.T) {
require.NoError(t, err)
// A fresh database has them.
for _, ix := range indexes {
require.True(t, db.Migrator().HasIndex(ix.model, ix.name))
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 indexes {
require.NoError(t, db.Migrator().DropIndex(ix.model, ix.name))
require.False(t, db.Migrator().HasIndex(ix.model, ix.name))
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
@@ -60,107 +64,9 @@ func TestWebhookDBManager_OpenAddsEventTierIndexes(t *testing.T) {
db, err = mgr.GetDB(webhookID)
require.NoError(t, err)
for _, ix := range indexes {
assert.True(t, db.Migrator().HasIndex(ix.model, ix.name),
"opening the existing database should create %s", ix.name)
}
}
// TestEventTierQueriesUseTheirIndexes verifies that the statements the
// indexes are for use them. GORM builds each statement in a dry run as
// the code named above it does, soft-delete condition included, and
// SQLite, which keeps no statistics on these tables, must plan to seek
// on each index listed by the columns in parentheses.
func TestEventTierQueriesUseTheirIndexes(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)) }()
db, err := mgr.GetDB(uuid.New().String())
require.NoError(t, err)
dry := db.Session(&gorm.Session{DryRun: true})
ids := []string{
uuid.New().String(), uuid.New().String(), uuid.New().String(),
}
cutoff := time.Now()
var (
deliveries []database.Delivery
results []database.DeliveryResult
depths []struct{ Depth int }
)
byStatus := "idx_deliveries_status (status=? AND deleted_at=?)"
byEvent := "idx_deliveries_event_id (event_id=? AND deleted_at=?)"
byAge := "idx_events_deleted_at_created_at (deleted_at=? AND created_at<?)"
// The delivery engine: recovery and the retry sweep, the sweep for
// stranded pending deliveries, and the queue depth count.
assertPlanUses(t, db, dry.Where(
"status = ?", database.DeliveryStatusRetrying,
).Find(&deliveries), byStatus)
assertPlanUses(t, db, dry.Where(
"status = ? AND updated_at < ?",
database.DeliveryStatusPending, cutoff,
).Limit(500).Find(&deliveries), byStatus)
assertPlanUses(t, db, dry.Model(&database.Delivery{}).
Select("target_id", "status", "count(*) as depth").
Where("status IN ?", []database.DeliveryStatus{
database.DeliveryStatusPending,
database.DeliveryStatusRetrying,
}).Group("target_id, status").Find(&depths), byStatus)
// The event log: each event's deliveries, then their attempts
// (loadEventsWithDeliveries, loadDeliveryResults).
assertPlanUses(t, db, dry.Where("event_id = ?", ids[0]).
Find(&deliveries), byEvent)
assertPlanUses(t, db, dry.Where("delivery_id IN ?", ids).
Order("attempt_num ASC").Find(&results),
"idx_delivery_results_delivery_id (delivery_id=? AND deleted_at=?)")
// Retention's three deletes (reapExpired), whose subqueries are built
// afresh for each statement as it builds them.
expiredEventIDs := func() *gorm.DB {
return dry.Model(&database.Event{}).Select("id").
Where("created_at < ?", cutoff)
}
assertPlanUses(t, db, dry.Unscoped().Where(
"delivery_id IN (?)", dry.Model(&database.Delivery{}).
Select("id").Where("event_id IN (?)", expiredEventIDs()),
).Delete(&database.DeliveryResult{}),
"idx_delivery_results_delivery_id (delivery_id=?)", byEvent, byAge)
assertPlanUses(t, db, dry.Unscoped().Where(
"event_id IN (?)", expiredEventIDs(),
).Delete(&database.Delivery{}),
"idx_deliveries_event_id (event_id=?)", byAge)
assertPlanUses(t, db, dry.Unscoped().Where(
"created_at < ?", cutoff,
).Delete(&database.Event{}), "idx_events_created_at (created_at<?)")
}
// assertPlanUses asserts that SQLite's plan for a statement GORM built
// in a dry run, run with the same SQL and arguments GORM would send,
// names each of the given indexes.
func assertPlanUses(
t *testing.T, db, built *gorm.DB, indexes ...string,
) {
t.Helper()
var plan []struct{ Detail string }
require.NoError(t, db.Raw(
"EXPLAIN QUERY PLAN "+built.Statement.SQL.String(),
built.Statement.Vars...,
).Scan(&plan).Error)
for _, index := range indexes {
assert.Contains(t, fmt.Sprint(plan), index,
built.Statement.SQL.String())
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)
}
}
+3 -12
View File
@@ -1,7 +1,5 @@
package database
import "gorm.io/gorm"
// DeliveryStatus represents the status of a delivery
type DeliveryStatus string
@@ -31,19 +29,12 @@ func (s DeliveryStatus) Terminal() bool {
}
// Delivery represents a delivery attempt for an event to a target
//
//nolint:lll // a struct tag cannot wrap
type Delivery struct {
BaseModel
EventID string `gorm:"type:uuid;not null;index:idx_deliveries_event_id,priority:1" json:"eventId"`
TargetID string `gorm:"type:uuid;not null" json:"targetId"`
Status DeliveryStatus `gorm:"not null;default:'pending';index:idx_deliveries_status,priority:1" json:"status"`
// DeletedAt repeats the BaseModel field only to be the second column
// of the event_id and status indexes, for the reason DeliveryResult
// gives.
DeletedAt gorm.DeletedAt `gorm:"index:idx_deliveries_event_id,priority:2;index:idx_deliveries_status,priority:2" json:"deletedAt,omitzero"`
EventID string `gorm:"type:uuid;not null;index" json:"eventId"`
TargetID string `gorm:"type:uuid;not null" json:"targetId"`
Status DeliveryStatus `gorm:"not null;default:'pending';index" json:"status"`
// Relations
Event Event `json:"event,omitzero"`
+3 -14
View File
@@ -1,25 +1,14 @@
package database
import "gorm.io/gorm"
// DeliveryResult represents the result of a delivery attempt
//
//nolint:lll // a struct tag cannot wrap
type DeliveryResult struct {
BaseModel
// DeliveryID and DeletedAt make up one index, in that order.
// DeletedAt repeats the BaseModel field only to join it: GORM adds
// "deleted_at IS NULL" to almost every query, and where a column is
// matched against several values SQLite otherwise reads through the
// deleted_at index, which every live row matches.
DeliveryID string `gorm:"type:uuid;not null;index:idx_delivery_results_delivery_id,priority:1" json:"deliveryId"`
DeletedAt gorm.DeletedAt `gorm:"index:idx_delivery_results_delivery_id,priority:2" json:"deletedAt,omitzero"`
AttemptNum int `gorm:"not null" json:"attemptNum"`
DeliveryID string `gorm:"type:uuid;not null;index" json:"deliveryId"`
AttemptNum int `gorm:"not null" json:"attemptNum"`
Success bool `json:"success"`
StatusCode int `json:"statusCode,omitempty"`
ResponseBody string `gorm:"type:text" json:"responseBody,omitempty"`
ResponseBody string `gorm:"type:text" json:"responseBody,omitempty"`
Error string `json:"error,omitempty"`
Duration int64 `json:"durationMs"` // Duration in milliseconds
+6 -16
View File
@@ -1,26 +1,16 @@
package database
import (
"time"
"gorm.io/gorm"
)
import "time"
// Event represents a captured webhook event
//
//nolint:lll // a struct tag cannot wrap
type Event struct {
BaseModel
// CreatedAt and DeletedAt repeat the BaseModel fields only to index
// them for retention, which finds events by age. Its lookups carry
// GORM's "deleted_at IS NULL" (see DeliveryResult) and compare
// created_at with <, so their index has deleted_at first: SQLite
// narrows by a < only on the last column it uses. Its final delete
// has no deleted_at condition and uses the index on created_at
// alone. The other tables keep the unindexed BaseModel created_at.
CreatedAt time.Time `gorm:"index;index:idx_events_deleted_at_created_at,priority:2" json:"createdAt"`
DeletedAt gorm.DeletedAt `gorm:"index:idx_events_deleted_at_created_at,priority:1" json:"deletedAt,omitzero"`
// 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"`
+3 -2
View File
@@ -380,8 +380,9 @@ func csrfTookStrictPath(
// TestCSRF_ForwardedProtoSpellingsTakeStrictPath runs the header
// spellings a real proxy emits through the middleware. The environment
// is set to dev -- the permissive setting -- to pin that the routing is
// a per-request transport decision and owes nothing to configuration.
// is dev -- the DEFAULT when WEBHOOKER_ENVIRONMENT is unset -- to pin
// that the routing is a per-request transport decision and owes
// nothing to configuration.
func TestCSRF_ForwardedProtoSpellingsTakeStrictPath(t *testing.T) {
t.Parallel()
+1 -1
View File
@@ -5,7 +5,7 @@
// several packages, by hand, and the answers disagreed. The session
// cookie's Secure attribute was decided at startup from the configured
// environment while the CSRF cookie's was decided per-request, so a
// deployment behind a TLS proxy in the dev environment emitted one
// deployment behind a TLS proxy in the default environment emitted one
// Secure cookie and one non-Secure cookie on the same response.
// Everything kept working, which is exactly why nobody noticed.
//
+2 -2
View File
@@ -146,8 +146,8 @@ func newStore(key []byte) *sessions.CookieStore {
//
// This is decided per-request, not once at startup. Deciding it at
// startup from the configured environment is what this replaces, and
// it got the DEFAULT posture wrong: "dev" was then the environment when
// WEBHOOKER_ENVIRONMENT was unset, so a deployment terminating TLS at a
// it got the DEFAULT posture wrong: "dev" is the environment when
// WEBHOOKER_ENVIRONMENT is unset, so a deployment terminating TLS at a
// proxy without also setting the environment emitted the
// authentication cookie with no Secure attribute -- silently, and on
// the same response as a CSRF cookie that did have one.
+2 -2
View File
@@ -990,8 +990,8 @@ func sessionCookieFrom(
// TestSave_SecureFollowsRequestTransport is the regression test for
// the defect this replaces: Secure was fixed at startup from the
// configured environment, and "dev" was then the environment when
// WEBHOOKER_ENVIRONMENT was unset. A deployment behind a TLS proxy in
// configured environment, and "dev" is the environment when
// WEBHOOKER_ENVIRONMENT is unset. A deployment behind a TLS proxy in
// that DEFAULT posture shipped the authentication cookie with no
// Secure attribute and said nothing about it.
//