From 2d5da7d014588bec49c7e5d145880aaf138ec311 Mon Sep 17 00:00:00 2001 From: RajeshKumar11 Date: Tue, 24 Feb 2026 21:41:02 +0530 Subject: [PATCH] envconfig: return defaultValue on invalid boolean env var BoolWithDefault was returning hardcoded true when strconv.ParseBool failed to parse the env var value, instead of returning defaultValue. This silently enabled boolean feature flags (e.g. OLLAMA_FLASH_ATTENTION) when users set common-but-invalid values like "yes", "on", or "enabled". Also adds a slog.Warn so users are notified about the misconfigured value, consistent with how the Uint helper handles invalid input. Fixes #14389 --- envconfig/config.go | 3 ++- envconfig/config_test.go | 46 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/envconfig/config.go b/envconfig/config.go index ec36d451b..69394fe12 100644 --- a/envconfig/config.go +++ b/envconfig/config.go @@ -156,7 +156,8 @@ func BoolWithDefault(k string) func(defaultValue bool) bool { if s := Var(k); s != "" { b, err := strconv.ParseBool(s) if err != nil { - return true + slog.Warn("invalid boolean value, using default", "key", k, "value", s, "default", defaultValue) + return defaultValue } return b diff --git a/envconfig/config_test.go b/envconfig/config_test.go index 9242e66f5..859d6aa3b 100644 --- a/envconfig/config_test.go +++ b/envconfig/config_test.go @@ -1,6 +1,7 @@ package envconfig import ( + "fmt" "log/slog" "math" "os" @@ -157,9 +158,12 @@ func TestBool(t *testing.T) { "false": false, "1": true, "0": false, - // invalid values - "random": true, - "something": true, + // invalid values fall back to default (false for Bool) + "random": false, + "something": false, + "yes": false, + "on": false, + "enabled": false, } for k, v := range cases { @@ -172,6 +176,42 @@ func TestBool(t *testing.T) { } } +func TestBoolWithDefault(t *testing.T) { + cases := []struct { + value string + defaultValue bool + expect bool + }{ + // valid values override the default + {"true", false, true}, + {"false", true, false}, + {"1", false, true}, + {"0", true, false}, + // empty value returns the default unchanged + {"", true, true}, + {"", false, false}, + // invalid values fall back to the default, not hardcoded true + {"yes", false, false}, + {"yes", true, true}, + {"on", false, false}, + {"on", true, true}, + {"enabled", false, false}, + {"enabled", true, true}, + {"random", false, false}, + {"random", true, true}, + } + + for _, tt := range cases { + name := fmt.Sprintf("value=%q default=%v", tt.value, tt.defaultValue) + t.Run(name, func(t *testing.T) { + t.Setenv("OLLAMA_BOOL_WITH_DEFAULT", tt.value) + if b := BoolWithDefault("OLLAMA_BOOL_WITH_DEFAULT")(tt.defaultValue); b != tt.expect { + t.Errorf("%s: expected %v, got %v", name, tt.expect, b) + } + }) + } +} + func TestUint(t *testing.T) { cases := map[string]uint{ "0": 0,