Fix AWS SDK debug logging by making it configurable (issue #2270) (#2290)

* fix: make AWS SDK debug logging configurable (issue #2270) This PR fixes issue #2270 where AWS SDK debug logs expose sensitive credentials in helmfile output, by adding flexible, configurable AWS SDK logging with secure defaults. Problem: -------- Despite PR #2288's fix, AWS SDK debug logs still appeared in helmfile output, exposing sensitive information: - AWS tokens and authorization headers - Request/response bodies containing credentials - Secret metadata from vals providers Root Cause: ----------- 1. PR #2288 only suppressed vals' own logging via LogOutput: io.Discard 2. AWS SDK v2 uses separate logging (AWS_SDK_GO_LOG_LEVEL, WithClientLogMode) 3. Vals library defaulted to verbose logging (aws.LogRetries | aws.LogRequest) 4. No programmatic way to control AWS SDK logging Solution: --------- Two-part fix in conjunction with vals PR #893: 1. Vals library enhancement (helmfile/vals#893): - Added Options.AWSLogLevel field for programmatic control - Changed default from verbose to secure (no logging) - Added preset levels: off, minimal, standard, verbose - Maintains AWS_SDK_GO_LOG_LEVEL precedence 2. Helmfile changes (this PR): - Added HELMFILE_AWS_SDK_LOG_LEVEL environment variable - Enhanced vals configuration to use new AWSLogLevel field - Added conditional AWS SDK log suppression in remote.go (3 locations) - Comprehensive unit tests (15 test cases) Configuration: -------------- Preset levels via HELMFILE_AWS_SDK_LOG_LEVEL: - "off" (default) - No logging, secure, prevents credential leakage - "minimal" - Log retries only - "standard" - Log retries + requests (previous default behavior) - "verbose" - Log everything (requests, responses, bodies, signing) - Custom - Comma-separated values (e.g., "request,response") Priority order: 1. AWS_SDK_GO_LOG_LEVEL env var (highest) 2. HELMFILE_AWS_SDK_LOG_LEVEL env var 3. Secure default ("off") Testing: -------- Added comprehensive unit tests: - pkg/plugins/vals_test.go: 9 test cases * TestAWSSDKLogLevelConfiguration - all preset levels * TestEnvironmentVariableReading - env var parsing - pkg/remote/remote_test.go: 6 test cases * TestAWSSDKLogLevelInit - init() logic All tests passing: - pkg/plugins: PASS (3/3 test suites) - pkg/remote: PASS (all test suites) - golangci-lint: 0 issues Files changed: 7 files, 271 insertions(+), 31 deletions(-) Security: --------- Before: Credentials exposed by default (aws.LogRetries | aws.LogRequest) After: Credentials protected by default (no logging unless explicitly enabled) Follows security principles: - Secure by default - Principle of least privilege - Explicit opt-in for sensitive logging - Defense in depth Dependency: ----------- Depends on: helmfile/vals#893 Currently using: aditmeno/vals@a97336ce2b (via go.mod replace) After vals PR merges: Update to official release Fixes: #2270 Related: #2288, #2289, helmfile/vals#893 Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * chore: update vals to use parameter-based AWS log level configuration Updated vals dependency to commit 06d7cd29 which implements clean parameter-based AWS SDK logging configuration instead of using global state mutation. Changes in vals implementation: - AWS log level passed through function parameters to each provider - No os.Setenv() - no environment mutation - No package-level global variables - No sync/atomic dependency needed - Thread-safe by design - each provider instance has its own log level This maintains the same functionality as before but with a cleaner implementation that avoids global state mutation. Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * deps: update vals to upstream v0.42.6 Update from vals fork (aditmeno/vals) to official release v0.42.6. Remove replace directive now that vals PR #893 has been merged upstream. This brings in the AWS SDK log level configuration improvements: - SetDefaultLogLevel() package-level function - Options.AWSLogLevel field support - Secure default (no logging) - Preset log levels (off, minimal, standard, verbose) Also updates related dependencies: - Azure SDK and auth libraries - AWS SDK config and credentials - OAuth2 library Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> --------- Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
2026-10-18 14:36:51 +02:00 · 2025-11-24 18:27:04 +08:00
parent 570ee3a8bb
commit 83b4a8ffc7
7 changed files with 323 additions and 33 deletions
@@ -2,9 +2,13 @@ package plugins

 import (
 	"io"
+	"os"
+	"strings"
 	"sync"

 	"github.com/helmfile/vals"
+
+	"github.com/helmfile/helmfile/pkg/envvar"
 )

 const (
@@ -18,13 +22,40 @@ var once sync.Once
 func ValsInstance() (*vals.Runtime, error) {
 	var err error
 	once.Do(func() {
-		// Set LogOutput to io.Discard to suppress debug logs from AWS SDK and other providers
-		// This prevents sensitive information (tokens, auth headers) from being logged to stdout
-		// See issue #2270
-		instance, err = vals.New(vals.Options{
+		// Configure AWS SDK logging via HELMFILE_AWS_SDK_LOG_LEVEL environment variable
+		// Default: "off" to prevent sensitive information (tokens, auth headers) from being exposed
+		// See issue #2270 and vals PR helmfile/vals#893
+		//
+		// Valid values:
+		// - "off" (default): No AWS SDK logging - secure, prevents credential leakage
+		// - "minimal": Log retries only - minimal debugging info
+		// - "standard": Log retries + requests - moderate debugging (previous default)
+		// - "verbose": Log everything - full debugging (requests, responses, bodies, signing)
+		// - Custom: Comma-separated values like "request,response"
+		//
+		// Note: AWS_SDK_GO_LOG_LEVEL environment variable always takes precedence over this setting
+		logLevel := strings.TrimSpace(os.Getenv(envvar.AWSSDKLogLevel))
+
+		opts := vals.Options{
 			CacheSize: valsCacheSize,
-			LogOutput: io.Discard,
-		})
+		}
+
+		// Default to "off" for security if not specified
+		if logLevel == "" {
+			logLevel = "off"
+		}
+
+		// Set AWS SDK log level for vals library
+		opts.AWSLogLevel = logLevel
+
+		// Also suppress vals' own internal logging unless user wants verbose output
+		// This prevents vals' log messages (separate from AWS SDK logs) from exposing credentials
+		if logLevel == "off" {
+			opts.LogOutput = io.Discard
+		}
+		// For other levels, allow vals to log to default output for debugging
+
+		instance, err = vals.New(opts)
 	})

 	return instance, err
@@ -1,6 +1,15 @@
 package plugins

-import "testing"
+import (
+	"io"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/helmfile/vals"
+
+	"github.com/helmfile/helmfile/pkg/envvar"
+)

 func TestValsInstance(t *testing.T) {
 	i, err := ValsInstance()
@@ -15,3 +24,143 @@ func TestValsInstance(t *testing.T) {
 		t.Error("Instances should be equal")
 	}
 }
+
+// TestAWSSDKLogLevelConfiguration tests the AWS SDK log level configuration logic
+func TestAWSSDKLogLevelConfiguration(t *testing.T) {
+	tests := []struct {
+		name              string
+		envValue          string
+		expectedLogLevel  string
+		expectedLogOutput bool // true if LogOutput should be io.Discard
+	}{
+		{
+			name:              "no env var defaults to off",
+			envValue:          "",
+			expectedLogLevel:  "off",
+			expectedLogOutput: true, // LogOutput should be io.Discard
+		},
+		{
+			name:              "explicit off",
+			envValue:          "off",
+			expectedLogLevel:  "off",
+			expectedLogOutput: true,
+		},
+		{
+			name:              "minimal logging",
+			envValue:          "minimal",
+			expectedLogLevel:  "minimal",
+			expectedLogOutput: false, // LogOutput should NOT be io.Discard
+		},
+		{
+			name:              "standard logging",
+			envValue:          "standard",
+			expectedLogLevel:  "standard",
+			expectedLogOutput: false,
+		},
+		{
+			name:              "verbose logging",
+			envValue:          "verbose",
+			expectedLogLevel:  "verbose",
+			expectedLogOutput: false,
+		},
+		{
+			name:              "custom logging",
+			envValue:          "request,response",
+			expectedLogLevel:  "request,response",
+			expectedLogOutput: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Note: This test verifies the configuration logic, not the actual vals.New() call
+			// since ValsInstance() uses sync.Once and can only be initialized once per test run.
+
+			// Simulate the logic from ValsInstance()
+			var logLevel string
+			if tt.envValue != "" {
+				logLevel = strings.TrimSpace(tt.envValue)
+			}
+
+			// Default to "off" for security if not specified
+			if logLevel == "" {
+				logLevel = "off"
+			}
+
+			// Verify expected log level
+			if logLevel != tt.expectedLogLevel {
+				t.Errorf("Expected log level %q, got %q", tt.expectedLogLevel, logLevel)
+			}
+
+			// Verify LogOutput configuration logic
+			opts := vals.Options{
+				CacheSize: valsCacheSize,
+			}
+			opts.AWSLogLevel = logLevel
+
+			// Verify LogOutput is set to io.Discard only when level is "off"
+			if tt.expectedLogOutput {
+				opts.LogOutput = io.Discard
+				if opts.LogOutput != io.Discard {
+					t.Error("Expected LogOutput to be io.Discard for 'off' level")
+				}
+			}
+		})
+	}
+}
+
+// TestEnvironmentVariableReading verifies that the HELMFILE_AWS_SDK_LOG_LEVEL env var is read correctly
+func TestEnvironmentVariableReading(t *testing.T) {
+	tests := []struct {
+		name          string
+		envValue      string
+		expectedValue string
+	}{
+		{
+			name:          "empty defaults to off",
+			envValue:      "",
+			expectedValue: "off",
+		},
+		{
+			name:          "whitespace trimmed",
+			envValue:      "  minimal  ",
+			expectedValue: "minimal",
+		},
+		{
+			name:          "standard value preserved",
+			envValue:      "standard",
+			expectedValue: "standard",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Save and restore env var
+			original := os.Getenv(envvar.AWSSDKLogLevel)
+			defer func() {
+				if original == "" {
+					os.Unsetenv(envvar.AWSSDKLogLevel)
+				} else {
+					os.Setenv(envvar.AWSSDKLogLevel, original)
+				}
+			}()
+
+			// Set test env var
+			if tt.envValue == "" {
+				os.Unsetenv(envvar.AWSSDKLogLevel)
+			} else {
+				os.Setenv(envvar.AWSSDKLogLevel, tt.envValue)
+			}
+
+			// Read and process like ValsInstance() does
+			logLevel := strings.TrimSpace(os.Getenv(envvar.AWSSDKLogLevel))
+			if logLevel == "" {
+				logLevel = "off"
+			}
+
+			if logLevel != tt.expectedValue {
+				t.Errorf("Expected %q, got %q", tt.expectedValue, logLevel)
+			}
+		})
+	}
+}