From 1e09bdedd8dd7fbb3839faef92517b0dcae1340e Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Thu, 22 Oct 2026 12:59:50 +0000 Subject: [PATCH 1/2] fix: reject env webhook responses with a non-2xx status getEnvValueFromHTTP returned the body of any response as the value, so an error page such as "404 page not found" became the value and replaced the cached value that LookupEnv falls back to. Treat a non-2xx status as a fetch error. --- env/env_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ env/web_env.go | 6 ++++++ 2 files changed, 55 insertions(+) diff --git a/env/env_test.go b/env/env_test.go index 71c8216..cac251b 100644 --- a/env/env_test.go +++ b/env/env_test.go @@ -278,3 +278,52 @@ func TestLookupEnvReferenceScheme(t *testing.T) { }) } } + +func TestLookupEnvRejectsErrorStatus(t *testing.T) { + var status atomic.Int32 + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + code := int(status.Load()) + w.WriteHeader(code) + if code == http.StatusOK { + w.Write([]byte("fetched-value")) + return + } + w.Write([]byte(http.StatusText(code))) + })) + t.Cleanup(ts.Close) + const key = "_TEST_ENV_STATUS" + t.Setenv(key, "env://minio:minio123@"+strings.TrimPrefix(ts.URL, "http://")) + + for _, code := range []int{ + http.StatusUnauthorized, + http.StatusForbidden, + http.StatusNotFound, + http.StatusInternalServerError, + http.StatusServiceUnavailable, + } { + t.Setenv("_"+key, "") + os.Unsetenv("_" + key) + status.Store(int32(code)) + v, _, _, err := LookupEnv(key) + if err == nil || v != "" { + t.Errorf("HTTP %d: LookupEnv = %q, %v; want an error", code, v, err) + } + if c, ok := os.LookupEnv("_" + key); ok { + t.Errorf("HTTP %d: cached %q, want no cache", code, c) + } + } + + t.Setenv("_"+key, "") + os.Unsetenv("_" + key) + status.Store(http.StatusOK) + if v, _, _, err := LookupEnv(key); err != nil || v != "fetched-value" { + t.Fatalf("HTTP 200: LookupEnv = %q, %v; want fetched-value", v, err) + } + status.Store(http.StatusServiceUnavailable) + if v, _, _, err := LookupEnv(key); err != nil || v != "fetched-value" { + t.Errorf("HTTP 503 after a fetch: LookupEnv = %q, %v; want the cached fetched-value", v, err) + } + if c := os.Getenv("_" + key); c != "fetched-value" { + t.Errorf("HTTP 503 after a fetch: cache = %q, want fetched-value", c) + } +} diff --git a/env/web_env.go b/env/web_env.go index a09eef6..36c2a70 100644 --- a/env/web_env.go +++ b/env/web_env.go @@ -151,6 +151,12 @@ func getEnvValueFromHTTP(urlStr, envKey string) (string, string, string, error) } defer resp.Body.Close() + // The body of an error response is not the value: returning it would + // also replace the cached value that LookupEnv falls back to. + if resp.StatusCode < 200 || resp.StatusCode > 299 { + return "", "", "", fmt.Errorf("env webhook returned %s", resp.Status) + } + envValueBytes, err := io.ReadAll(resp.Body) if err != nil { return "", "", "", err From d8e2d714752fbe3c13a6ffc49915291ecd934e99 Mon Sep 17 00:00:00 2001 From: Allan Roger Reid Date: Thu, 22 Oct 2026 14:58:58 +0000 Subject: [PATCH 2/2] test: pin the 2xx range and the 3xx boundary in the env webhook test The test checked only HTTP 200 as a success, so an implementation accepting 200 alone passed it. It now asserts the value and the cache for 200, 201 and 299, and an error with no cache for 300 next to the 4xx and 5xx codes. --- env/env_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/env/env_test.go b/env/env_test.go index cac251b..1ecd698 100644 --- a/env/env_test.go +++ b/env/env_test.go @@ -284,7 +284,7 @@ func TestLookupEnvRejectsErrorStatus(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { code := int(status.Load()) w.WriteHeader(code) - if code == http.StatusOK { + if code >= 200 && code <= 299 { w.Write([]byte("fetched-value")) return } @@ -294,7 +294,20 @@ func TestLookupEnvRejectsErrorStatus(t *testing.T) { const key = "_TEST_ENV_STATUS" t.Setenv(key, "env://minio:minio123@"+strings.TrimPrefix(ts.URL, "http://")) + for _, code := range []int{http.StatusOK, http.StatusCreated, 299} { + t.Setenv("_"+key, "") + os.Unsetenv("_" + key) + status.Store(int32(code)) + if v, _, _, err := LookupEnv(key); err != nil || v != "fetched-value" { + t.Errorf("HTTP %d: LookupEnv = %q, %v; want fetched-value", code, v, err) + } + if c := os.Getenv("_" + key); c != "fetched-value" { + t.Errorf("HTTP %d: cache = %q, want fetched-value", code, c) + } + } + for _, code := range []int{ + http.StatusMultipleChoices, http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound,