Default WEBHOOKER_ENVIRONMENT to prod (closes #307)
check / check (push) Successful in 3m14s
check / check (push) Successful in 3m14s
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)
This commit was merged in pull request #322.
This commit is contained in:
@@ -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 —
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
@@ -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.
|
||||
//
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
//
|
||||
|
||||
Reference in New Issue
Block a user