From 237f131367db343e5b8cce0826d54a6f788dae95 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 28 Sep 2026 12:47:31 +0200 Subject: [PATCH] Default WEBHOOKER_ENVIRONMENT to prod (closes #307) An unset WEBHOOKER_ENVIRONMENT now means prod, not dev. The only thing dev still changes is CORS, which then answers every origin with Access-Control-Allow-Origin: *, so an operator who forgets the variable is no longer silently permissive; dev must be set explicitly. Cookie Secure and CSRF strictness follow each request's transport and are unaffected. The README, comments and tests no longer describe dev as the default: the deployment checklist asks only that the environment is not dev, the Docker and nginx examples drop the now-redundant setting, and the TRUSTED_PROXIES warning gives its real reason for firing in every environment. Model: opus-4-8 (implementation); opus-5-5 (rework) --- README.md | 39 ++++++++++++++++---------------- internal/config/config.go | 14 ++++++------ internal/config/config_test.go | 17 +++++--------- internal/middleware/csrf_test.go | 5 ++-- internal/reqtls/reqtls.go | 2 +- internal/session/session.go | 4 ++-- internal/session/session_test.go | 4 ++-- 7 files changed, 39 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 85275c4..eb5ee27 100644 --- a/README.md +++ b/README.md @@ -44,9 +44,9 @@ make bootstrap # Run all checks (test, lint, format check) make check -# 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. +# 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. DATA_DIR=./data make dev # Build Docker image @@ -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` | @@ -399,10 +400,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 -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 +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 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,7 +638,6 @@ 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 ``` @@ -812,17 +811,18 @@ 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. **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 +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: `dev` answers every origin with `Access-Control-Allow-Origin: *` (without credentials), which a server-rendered production - 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). + 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). 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,7 +914,6 @@ 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 ``` diff --git a/internal/config/config.go b/internal/config/config.go index 6a7a628..69c5210 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -585,12 +585,14 @@ func resolveMetricsAuth() (string, string, error) { ) } -// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to -// dev, and rejects unrecognised values. +// 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. func resolveEnvironment() (string, error) { environment := os.Getenv("WEBHOOKER_ENVIRONMENT") if environment == "" { - environment = EnvironmentDev + environment = EnvironmentProd } if environment != EnvironmentDev && @@ -772,10 +774,8 @@ 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 -// 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 warning is deliberately not gated on WEBHOOKER_ENVIRONMENT: +// behind a proxy every client shares one bucket in dev and prod alike. // // The default of trusting nobody is deliberate — trusting forwarded // headers from arbitrary peers lets any client choose its own bucket — diff --git a/internal/config/config_test.go b/internal/config/config_test.go index f38f7fd..0add1b2 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -44,9 +44,9 @@ func TestEnvironmentConfig(t *testing.T) { isProd bool }{ { - name: "default is dev", - isDev: true, - isProd: false, + name: "default is prod", + isDev: false, + isProd: true, }, { name: "explicit dev", @@ -848,10 +848,9 @@ 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: 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. +// 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. func TestSharedRateLimitBucketWarning(t *testing.T) { tests := []struct { name string @@ -871,10 +870,6 @@ 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, diff --git a/internal/middleware/csrf_test.go b/internal/middleware/csrf_test.go index 4273964..f1660ed 100644 --- a/internal/middleware/csrf_test.go +++ b/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 -// that the routing is a per-request transport decision and owes -// nothing to configuration. +// is set to dev -- the permissive setting -- to pin that the routing is +// a per-request transport decision and owes nothing to configuration. func TestCSRF_ForwardedProtoSpellingsTakeStrictPath(t *testing.T) { t.Parallel() diff --git a/internal/reqtls/reqtls.go b/internal/reqtls/reqtls.go index f1704a4..b9e28b5 100644 --- a/internal/reqtls/reqtls.go +++ b/internal/reqtls/reqtls.go @@ -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 default environment emitted one +// deployment behind a TLS proxy in the dev environment emitted one // Secure cookie and one non-Secure cookie on the same response. // Everything kept working, which is exactly why nobody noticed. // diff --git a/internal/session/session.go b/internal/session/session.go index 901568e..97b6664 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -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" is the environment when -// WEBHOOKER_ENVIRONMENT is unset, so a deployment terminating TLS at a +// it got the DEFAULT posture wrong: "dev" was then the environment when +// WEBHOOKER_ENVIRONMENT was 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. diff --git a/internal/session/session_test.go b/internal/session/session_test.go index 962df24..a52ae96 100644 --- a/internal/session/session_test.go +++ b/internal/session/session_test.go @@ -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" is the environment when -// WEBHOOKER_ENVIRONMENT is unset. A deployment behind a TLS proxy in +// configured environment, and "dev" was then the environment when +// WEBHOOKER_ENVIRONMENT was unset. A deployment behind a TLS proxy in // that DEFAULT posture shipped the authentication cookie with no // Secure attribute and said nothing about it. //