From dde720eb4416e93e8df0293244e7ae784b1b5bcd Mon Sep 17 00:00:00 2001 From: root Date: Mon, 3 Aug 2026 11:50:47 +0300 Subject: [PATCH] fix(worker): correct inverted WORKER_COMPOSE_ENABLED parsing parseBoolFalseDefault returned false only for explicit falsy literals (including the empty string) and true otherwise, but the ComposeEnabled call site negated it. The double error cancelled for an unset variable (empty -> false -> !false -> true) but inverted every explicit value: WORKER_COMPOSE_ENABLED=true disabled the subsystem while =false enabled it. Rename the helper to parseBoolTrueDefault, drop the empty string from the falsy set so unset stays on, and drop the negation. Add a table-driven regression test pinning unset/true/1/yes -> on and false/0/no/off (any case, trimmed) -> off. --- internal/webapp/server.go | 18 +++++++++-------- internal/webapp/server_test.go | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/internal/webapp/server.go b/internal/webapp/server.go index 4508639..0557cfa 100644 --- a/internal/webapp/server.go +++ b/internal/webapp/server.go @@ -185,18 +185,20 @@ func ConfigFromEnv(env map[string]string, defaultDataDir string) (Config, error) cfg.StorePath = v } cfg.ReleaseURL = strings.TrimSpace(env[envReleaseURL]) - // Compose discovery defaults to on; only an explicit false disables it. - cfg.ComposeEnabled = !parseBoolFalseDefault(env[envComposeEnabled]) + // Compose discovery defaults to on; only an explicit falsy literal + // disables it. An unset env var (empty) keeps the feature on. + cfg.ComposeEnabled = parseBoolTrueDefault(env[envComposeEnabled]) return cfg, nil } -// parseBoolFalseDefault returns true for any value that is not an -// explicit falsy literal. Used by ComposeEnabled so that the default -// (unset env var) keeps the feature on, unlike parseBool where the -// default is false. -func parseBoolFalseDefault(v string) bool { +// parseBoolTrueDefault reports true for any value that is not an +// explicit falsy literal. The empty string (unset env var) is true, +// so a feature using this parser is on unless the operator opts out. +// This mirrors parseBool's literal set but inverts the default, which +// is what ComposeEnabled needs. +func parseBoolTrueDefault(v string) bool { switch strings.ToLower(strings.TrimSpace(v)) { - case "false", "0", "no", "off", "": + case "false", "0", "no", "off": return false } return true diff --git a/internal/webapp/server_test.go b/internal/webapp/server_test.go index b282db1..75f3f7b 100644 --- a/internal/webapp/server_test.go +++ b/internal/webapp/server_test.go @@ -88,6 +88,41 @@ func TestConfigFromEnvBasicAuthRejectsXOR(t *testing.T) { assert.Error(t, err, "XOR (password only) must be rejected") } +// TestConfigFromEnvComposeEnabled pins the true-default parsing of +// WORKER_COMPOSE_ENABLED: the feature is on when unset and only an +// explicit falsy literal turns it off. Regression guard for the +// inverted-negation bug that made "true" disable the subsystem. +func TestConfigFromEnvComposeEnabled(t *testing.T) { + cases := []struct { + name string + env string + want bool + }{ + {"unset empty", "", true}, + {"explicit true", "true", true}, + {"explicit one", "1", true}, + {"explicit yes", "yes", true}, + {"random non-falsy", "on", true}, + {"explicit false", "false", false}, + {"explicit zero", "0", false}, + {"explicit no", "no", false}, + {"explicit off", "off", false}, + {"falsy with case", "FALSE", false}, + {"falsy with whitespace", " off ", false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + env := map[string]string{} + if c.env != "" { + env[envComposeEnabled] = c.env + } + cfg, err := ConfigFromEnv(env, t.TempDir()) + require.NoError(t, err) + assert.Equal(t, c.want, cfg.ComposeEnabled) + }) + } +} + // TestConfigFromEnvReleaseURL pins the env-driven WORKER_RELEASE_URL // plumbing. The handler reads cfg.ReleaseURL when the page renders, // so the value must survive ConfigFromEnv exactly.