diff --git a/.changes/v1.15/BUG FIXES-37601.yaml b/.changes/v1.15/BUG FIXES-37601.yaml new file mode 100644 index 000000000000..658f9e2cd06e --- /dev/null +++ b/.changes/v1.15/BUG FIXES-37601.yaml @@ -0,0 +1,5 @@ +kind: BUG FIXES +body: 'backend/s3: Fixed incorrect parsing of `AWS_USE_FIPS_ENDPOINT` and `AWS_USE_DUALSTACK_ENDPOINT` environment variables where any non-empty value incorrectly enabled the feature instead of properly parsing boolean values' +time: 2025-11-25T13:30:00.000000Z +custom: + Issue: "37601" diff --git a/internal/backend/remote-state/s3/backend.go b/internal/backend/remote-state/s3/backend.go index be1af3f6fd19..b74fc8f89bbe 100644 --- a/internal/backend/remote-state/s3/backend.go +++ b/internal/backend/remote-state/s3/backend.go @@ -1331,14 +1331,21 @@ func boolAttrOk(obj cty.Value, name string) (bool, bool) { } } -// boolAttrDefaultEnvVarOk checks for a configured bool argument or a non-empty -// value in any of the provided environment variables. If any of the environment -// variables are non-empty, to boolean is considered true. +// boolAttrDefaultEnvVarOk checks for a configured bool argument or a boolean +// value ("true" or "false", case-insensitive) in any of the provided environment +// variables. Invalid values are treated as unset. func boolAttrDefaultEnvVarOk(obj cty.Value, name string, envvars ...string) (bool, bool) { if val := obj.GetAttr(name); val.IsNull() { for _, envvar := range envvars { if v := os.Getenv(envvar); v != "" { - return true, true + if strings.EqualFold(v, "true") { + return true, true + } + if strings.EqualFold(v, "false") { + return false, true + } + // Invalid boolean value, treat as unset + return false, false } } return false, false diff --git a/internal/backend/remote-state/s3/backend_test.go b/internal/backend/remote-state/s3/backend_test.go index ace878f21933..2d654c0623c8 100644 --- a/internal/backend/remote-state/s3/backend_test.go +++ b/internal/backend/remote-state/s3/backend_test.go @@ -3963,3 +3963,125 @@ func objectLockPreCheck(t *testing.T) { t.Skip("s3 backend tests using object lock enabled buckets require setting TF_S3_OBJECT_LOCK_TEST") } } + +// TestBoolAttrDefaultEnvVarOk tests the boolAttrDefaultEnvVarOk helper function +// which reads boolean values from environment variables. This is a regression +// test for https://github.com/hashicorp/terraform/issues/37601 where setting +// AWS_USE_FIPS_ENDPOINT=false incorrectly enabled FIPS endpoints. +func TestBoolAttrDefaultEnvVarOk(t *testing.T) { + testCases := map[string]struct { + attrValue cty.Value + envValue string + wantBool bool + wantOk bool + }{ + "attr set true": { + attrValue: cty.BoolVal(true), + envValue: "", + wantBool: true, + wantOk: true, + }, + "attr set false": { + attrValue: cty.BoolVal(false), + envValue: "", + wantBool: false, + wantOk: true, + }, + "attr null, env true": { + attrValue: cty.NullVal(cty.Bool), + envValue: "true", + wantBool: true, + wantOk: true, + }, + "attr null, env TRUE": { + attrValue: cty.NullVal(cty.Bool), + envValue: "TRUE", + wantBool: true, + wantOk: true, + }, + "attr null, env True": { + attrValue: cty.NullVal(cty.Bool), + envValue: "True", + wantBool: true, + wantOk: true, + }, + "attr null, env false": { + attrValue: cty.NullVal(cty.Bool), + envValue: "false", + wantBool: false, + wantOk: true, + }, + "attr null, env FALSE": { + attrValue: cty.NullVal(cty.Bool), + envValue: "FALSE", + wantBool: false, + wantOk: true, + }, + "attr null, env False": { + attrValue: cty.NullVal(cty.Bool), + envValue: "False", + wantBool: false, + wantOk: true, + }, + "attr null, env empty": { + attrValue: cty.NullVal(cty.Bool), + envValue: "", + wantBool: false, + wantOk: false, + }, + "attr null, env invalid": { + attrValue: cty.NullVal(cty.Bool), + envValue: "invalid", + wantBool: false, + wantOk: false, + }, + "attr null, env yes": { + attrValue: cty.NullVal(cty.Bool), + envValue: "yes", + wantBool: false, + wantOk: false, + }, + "attr null, env 1": { + attrValue: cty.NullVal(cty.Bool), + envValue: "1", + wantBool: false, + wantOk: false, + }, + "attr null, env 0": { + attrValue: cty.NullVal(cty.Bool), + envValue: "0", + wantBool: false, + wantOk: false, + }, + "attr takes precedence over env": { + attrValue: cty.BoolVal(false), + envValue: "true", + wantBool: false, + wantOk: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + servicemocks.StashEnv(t) + + const testEnvVar = "TF_TEST_BOOL_ENV_VAR" + if tc.envValue != "" { + os.Setenv(testEnvVar, tc.envValue) + } + + obj := cty.ObjectVal(map[string]cty.Value{ + "test_attr": tc.attrValue, + }) + + gotBool, gotOk := boolAttrDefaultEnvVarOk(obj, "test_attr", testEnvVar) + + if gotBool != tc.wantBool { + t.Errorf("got bool %v, want %v", gotBool, tc.wantBool) + } + if gotOk != tc.wantOk { + t.Errorf("got ok %v, want %v", gotOk, tc.wantOk) + } + }) + } +}