Revert to truncating ALL environment variable values for security

Security-first approach: ALL environment variable values in Docker args are
now truncated to prevent ANY secrets from being exposed in logs.

Rationale:
- Trying to selectively identify secrets is less secure
- Detection logic can miss secrets or evolve over time
- Better to sacrifice some debugging convenience than risk secret exposure
- Truncation shows first 4 chars (e.g., ghs_...) for basic debugging

This approach follows the principle of "secure by default" rather than
"convenient but potentially insecure".

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

Author: Copilot

internal/logger/sanitize/sanitize.go

@@ -130,8 +130,9 @@ func SanitizeJSON(payloadBytes []byte) json.RawMessage {
 
 // SanitizeArgs returns a sanitized version of command arguments for safe logging.
 // It specifically handles Docker-style environment variable arguments (-e VAR=VALUE)
-// by selectively truncating values that look like secrets while leaving non-sensitive
-// values unchanged for debugging purposes.
+// by truncating ALL values to prevent exposing sensitive data like API tokens.
+// This approach prioritizes security over debugging convenience - we truncate all
+// environment variable values rather than trying to selectively identify secrets.
 // Other arguments are passed through unchanged.
 func SanitizeArgs(args []string) []string {
 	if len(args) == 0 {
@@ -148,16 +149,8 @@ func SanitizeArgs(args []string) []string {
 			// Split on first = to get VAR and VALUE
 			parts := strings.SplitN(arg, "=", 2)
 			if len(parts) == 2 {
-				varName := parts[0]
-				value := parts[1]
-				
-				// Only truncate if the value looks like a secret
-				if looksLikeSecret(varName, value) {
-					sanitized[i] = varName + "=" + TruncateSecret(value)
-				} else {
-					// Leave non-sensitive values unchanged for debugging
-					sanitized[i] = arg
-				}
+				// Always truncate the value part for security
+				sanitized[i] = parts[0] + "=" + TruncateSecret(parts[1])
 			} else {
 				sanitized[i] = arg
 			}
@@ -169,74 +162,3 @@ func SanitizeArgs(args []string) []string {
 	return sanitized
 }
 
-// looksLikeSecret determines if an environment variable value appears to be sensitive
-// based on the variable name and value patterns.
-func looksLikeSecret(varName, value string) bool {
-	// Empty values are not secrets
-	if value == "" {
-		return false
-	}
-	
-	// Check variable name for common secret indicators
-	varNameLower := strings.ToLower(varName)
-	secretNamePatterns := []string{
-		"token", "secret", "key", "password", "passwd", "pwd",
-		"credential", "auth", "api_key", "apikey", "access",
-	}
-	for _, pattern := range secretNamePatterns {
-		if strings.Contains(varNameLower, pattern) {
-			return true
-		}
-	}
-	
-	// Check if value matches secret patterns (long strings, tokens, etc.)
-	// Short values like "1", "true", "false" are unlikely to be secrets
-	if len(value) <= 4 {
-		return false
-	}
-	
-	// Check common non-sensitive values
-	nonSensitiveValues := []string{
-		"true", "false", "yes", "no", "on", "off",
-		"debug", "info", "warn", "error",
-		"dumb", "xterm", "ansi",
-	}
-	valueLower := strings.ToLower(value)
-	for _, nonSensitive := range nonSensitiveValues {
-		if valueLower == nonSensitive {
-			return false
-		}
-	}
-	
-	// Check if value looks like a token/key (GitHub PAT, JWT, API keys, etc.)
-	for _, pattern := range SecretPatterns {
-		if pattern.MatchString(value) {
-			return true
-		}
-	}
-	
-	// If value is longer than 16 chars and contains alphanumeric, treat as potential secret
-	if len(value) > 16 && containsAlphanumeric(value) {
-		return true
-	}
-	
-	return false
-}
-
-// containsAlphanumeric checks if a string contains both letters and numbers
-func containsAlphanumeric(s string) bool {
-	hasLetter := false
-	hasDigit := false
-	for _, c := range s {
-		if (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') {
-			hasLetter = true
-		}
-		if c >= '0' && c <= '9' {
-			hasDigit = true
-		}
-		if hasLetter && hasDigit {
-			return true
-		}
-	}
-	return false
-}

internal/logger/sanitize/sanitize_test.go

@@ -535,9 +535,9 @@ func TestSanitizeArgs(t *testing.T) {
 			},
 			expected: []string{
 				"run",
-				"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...",            // Secret detected by pattern
-				"-e", "API_KEY=sk_t...",                                 // Secret detected by name and pattern
-				"-e", "SHORT=abc",                                        // Non-sensitive: short value
+				"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...",
+				"-e", "API_KEY=sk_t...",
+				"-e", "SHORT=...",
 				"--rm",
 				"-i",
 				"image:latest",
@@ -560,11 +560,11 @@ func TestSanitizeArgs(t *testing.T) {
 				"run",
 				"--rm",
 				"-i",
-				"-e", "NO_COLOR=1",                                      // Non-sensitive: unchanged
-				"-e", "TERM=dumb",                                       // Non-sensitive: unchanged
-				"-e", "PYTHONUNBUFFERED=1",                              // Non-sensitive: unchanged
-				"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...",            // Secret: truncated
-				"-e", "GITHUB_READ_ONLY=1",                              // Non-sensitive: unchanged
+				"-e", "NO_COLOR=...",
+				"-e", "TERM=...",
+				"-e", "PYTHONUNBUFFERED=...",
+				"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...",
+				"-e", "GITHUB_READ_ONLY=...",
 				"ghcr.io/github/github-mcp-server:v0.29.0",
 			},
 		},
@@ -586,7 +586,7 @@ func TestSanitizeArgs(t *testing.T) {
 			},
 			expected: []string{
 				"run",
-				"-e", "LOG_FORMAT=json=pretty",  // Non-sensitive: short value, no secret indicators
+				"-e", "LOG_FORMAT=json...",
 			},
 		},
 		{
@@ -607,49 +607,9 @@ func TestSanitizeArgs(t *testing.T) {
 			expected: []string{
 				"run",
 				"--name", "test-container",
-				"-e", "API_KEY=secr...",        // Secret: detected by name "API_KEY"
+				"-e", "API_KEY=secr...",
 				"--network", "host",
-				"-e", "TOKEN=myto...",          // Secret: detected by name "TOKEN"
-				"image:latest",
-			},
-		},
-		{
-			name: "non-sensitive configuration values",
-			input: []string{
-				"run",
-				"-e", "DEBUG=true",
-				"-e", "LOG_LEVEL=info",
-				"-e", "ENABLE_FEATURE=yes",
-				"-e", "PORT=8080",
-				"image:latest",
-			},
-			expected: []string{
-				"run",
-				"-e", "DEBUG=true",              // Non-sensitive: common config value
-				"-e", "LOG_LEVEL=info",          // Non-sensitive: common config value
-				"-e", "ENABLE_FEATURE=yes",      // Non-sensitive: common config value
-				"-e", "PORT=8080",               // Non-sensitive: port number
-				"image:latest",
-			},
-		},
-		{
-			name: "mixed sensitive and non-sensitive values",
-			input: []string{
-				"run",
-				"-e", "NO_COLOR=1",
-				"-e", "GITHUB_TOKEN=ghp_abcdefghijklmnopqrstuvwxyz1234567890",
-				"-e", "TERM=xterm",
-				"-e", "DATABASE_PASSWORD=my_secret_db_pass_123",
-				"-e", "TIMEOUT=30",
-				"image:latest",
-			},
-			expected: []string{
-				"run",
-				"-e", "NO_COLOR=1",                                      // Non-sensitive
-				"-e", "GITHUB_TOKEN=ghp_...",                            // Secret: matches pattern
-				"-e", "TERM=xterm",                                      // Non-sensitive
-				"-e", "DATABASE_PASSWORD=my_s...",                       // Secret: detected by name
-				"-e", "TIMEOUT=30",                                      // Non-sensitive
+				"-e", "TOKEN=myto...",
 				"image:latest",
 			},
 		},
@@ -663,51 +623,6 @@ func TestSanitizeArgs(t *testing.T) {
 	}
 }
 
-func TestLooksLikeSecret(t *testing.T) {
-	tests := []struct {
-		name     string
-		varName  string
-		value    string
-		expected bool
-	}{
-		// Secret detection by name
-		{name: "token in name", varName: "GITHUB_TOKEN", value: "abc123", expected: true},
-		{name: "secret in name", varName: "API_SECRET", value: "xyz789", expected: true},
-		{name: "key in name", varName: "DATABASE_KEY", value: "test123", expected: true},
-		{name: "password in name", varName: "DB_PASSWORD", value: "pass123", expected: true},
-		{name: "apikey in name", varName: "SERVICE_APIKEY", value: "key123", expected: true},
-		
-		// Non-sensitive names
-		{name: "color setting", varName: "NO_COLOR", value: "1", expected: false},
-		{name: "term setting", varName: "TERM", value: "dumb", expected: false},
-		{name: "debug flag", varName: "DEBUG", value: "true", expected: false},
-		{name: "port number", varName: "PORT", value: "8080", expected: false},
-		
-		// Secret detection by value pattern
-		{name: "github pat", varName: "VAR", value: "ghp_1234567890123456789012345678901234567890", expected: true},
-		{name: "long hex string", varName: "VAR", value: "abcdef1234567890abcdef1234567890abcdef12", expected: true},
-		{name: "jwt token", varName: "VAR", value: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.payload.signature", expected: true},
-		
-		// Short non-sensitive values
-		{name: "single digit", varName: "VAR", value: "1", expected: false},
-		{name: "boolean true", varName: "VAR", value: "true", expected: false},
-		{name: "boolean false", varName: "VAR", value: "false", expected: false},
-		{name: "yes value", varName: "VAR", value: "yes", expected: false},
-		
-		// Edge cases
-		{name: "empty value", varName: "VAR", value: "", expected: false},
-		{name: "long non-alphanumeric", varName: "VAR", value: "====================", expected: false},
-		{name: "long config path", varName: "CONFIG_PATH", value: "/usr/local/etc/app/config.json", expected: false},
-	}
-	
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			result := looksLikeSecret(tt.varName, tt.value)
-			assert.Equal(t, tt.expected, result, "looksLikeSecret(%q, %q) should return %v", tt.varName, tt.value, tt.expected)
-		})
-	}
-}
-
 func TestSanitizeArgsDoesNotLeakSecrets(t *testing.T) {
 	// Test that actual secrets are not present in sanitized output
 	secretToken := "ghp_verysecrettokenthatshouldbehidden1234567890"