[Detail Bug] MultiProvider ignores empty values from higher‑priority providers #1103
Description
Detail Bug Report
Summary
- Context:
MultiProvideris used throughout the codebase to provide a cascading lookup of environment variables across multiple sources (env files, OS environment, Docker secrets, keychain, etc.) with a defined priority order. - Bug:
MultiProvider.Get()cannot distinguish between "variable not found" and "variable explicitly set to empty string", treating both cases as "not found" and cascading to the next provider. - Actual vs. expected: When a higher-priority provider explicitly sets a variable to empty string,
MultiProviderignores it and returns a value from a lower-priority provider; it should return the empty string. - Impact: Users cannot explicitly override/clear environment variables using higher-priority sources (like env files), as the system will incorrectly fall back to lower-priority sources, potentially exposing secrets or using unwanted default values.
Code with bug
// pkg/environment/multi.go
func (p *MultiProvider) Get(ctx context.Context, name string) string {
for _, provider := range p.providers {
value := provider.Get(ctx, name)
if value != "" { // <-- BUG 🔴 Treats empty string as "not found"
return value
}
}
return ""
}Failing test
package environment
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
)
// TestMultiProviderEmptyValue tests that an empty string value from a higher
// priority provider should be returned, not skipped in favor of a non-empty
// value from a lower priority provider.
func TestMultiProviderEmptyValue(t *testing.T) {
// Setup: Create two providers
// - First provider returns empty string (e.g., env var explicitly set to "")
// - Second provider returns "fallback" (e.g., a default value)
firstProvider := NewEnvListProvider([]string{"MY_VAR="}) // Empty value
secondProvider := NewEnvListProvider([]string{"MY_VAR=fallback"})
// Create multi provider with first having higher priority
provider := NewMultiProvider(firstProvider, secondProvider)
// Act: Get the value
value := provider.Get(context.Background(), "MY_VAR")
// Assert: Should return empty string from first provider, not "fallback" from second
// Because the first provider found the variable (even though it's empty)
assert.Equal(t, "", value, "Should return empty string from higher priority provider, not skip to next provider")
}Test output:
=== RUN TestMultiProviderEmptyValue
multi_empty_test.go:29:
Error Trace:
Error: Not equal:
expected: ""
actual : "fallback"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-
+fallback
Test: TestMultiProviderEmptyValue
Messages: Should return empty string from higher priority provider, not skip to next provider
--- FAIL: TestMultiProviderEmptyValue (0.00s)
FAIL
Recommended fix
- Change the
Providerinterface to return(string, bool)to distinguish "found" from "not found", following theos.LookupEnvpattern, and updateMultiProviderto honor the boolean:
// pkg/environment/provider.go
type Provider interface {
// Get retrieves the value of an environment variable by name.
// Returns (value, true) if found (value may be empty)
// Returns ("", false) if not found
Get(ctx context.Context, name string) (string, bool)
}
// pkg/environment/multi.go
func (p *MultiProvider) Get(ctx context.Context, name string) (string, bool) {
for _, provider := range p.providers {
value, found := provider.Get(ctx, name)
if found {
return value, true
}
}
return "", false
}