diff --git a/README.md b/README.md index 319989c..c48aa4c 100644 --- a/README.md +++ b/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 -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 +the warning does not depend on `WEBHOOKER_ENVIRONMENT`, because an +operator who never configured the deployment 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. @@ -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 - `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). + 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, 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 diff --git a/internal/config/config.go b/internal/config/config.go index 6a7a628..0c09163 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,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 -// 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: an +// operator who never configured the deployment is exactly the case it +// exists to catch, so the exposure it announces is independent of the +// environment setting. // // 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..4c8850e 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,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: 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: the warning does not depend on WEBHOOKER_ENVIRONMENT, +// since an operator who never configured the deployment is exactly the +// 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 - // deployment whose operator never set - // WEBHOOKER_ENVIRONMENT lands here and has exactly - // the exposure the warning announces. + // An internet-exposed deployment whose operator set + // WEBHOOKER_ENVIRONMENT=dev 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()