Skip to content

Commit d21b73e

Browse files
Copilotlpcox
andcommitted
Improve sanitization to selectively truncate only secret values
Enhanced SanitizeArgs to use smart detection that: - Detects secrets by variable name (token, secret, key, password, etc.) - Detects secrets by value patterns (GitHub PATs, JWT, API keys, etc.) - Leaves non-sensitive config values unchanged (NO_COLOR=1, TERM=dumb, etc.) - Only truncates values that actually look like secrets This provides better debugging visibility while still protecting sensitive data. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
1 parent 2dffe0a commit d21b73e

2 files changed

Lines changed: 182 additions & 16 deletions

File tree

internal/logger/sanitize/sanitize.go

Lines changed: 84 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ func SanitizeJSON(payloadBytes []byte) json.RawMessage {
130130

131131
// SanitizeArgs returns a sanitized version of command arguments for safe logging.
132132
// It specifically handles Docker-style environment variable arguments (-e VAR=VALUE)
133-
// by truncating the values to prevent exposing sensitive data like API tokens.
133+
// by selectively truncating values that look like secrets while leaving non-sensitive
134+
// values unchanged for debugging purposes.
134135
// Other arguments are passed through unchanged.
135136
func SanitizeArgs(args []string) []string {
136137
if len(args) == 0 {
@@ -147,8 +148,16 @@ func SanitizeArgs(args []string) []string {
147148
// Split on first = to get VAR and VALUE
148149
parts := strings.SplitN(arg, "=", 2)
149150
if len(parts) == 2 {
150-
// Truncate the value part
151-
sanitized[i] = parts[0] + "=" + TruncateSecret(parts[1])
151+
varName := parts[0]
152+
value := parts[1]
153+
154+
// Only truncate if the value looks like a secret
155+
if looksLikeSecret(varName, value) {
156+
sanitized[i] = varName + "=" + TruncateSecret(value)
157+
} else {
158+
// Leave non-sensitive values unchanged for debugging
159+
sanitized[i] = arg
160+
}
152161
} else {
153162
sanitized[i] = arg
154163
}
@@ -159,3 +168,75 @@ func SanitizeArgs(args []string) []string {
159168
}
160169
return sanitized
161170
}
171+
172+
// looksLikeSecret determines if an environment variable value appears to be sensitive
173+
// based on the variable name and value patterns.
174+
func looksLikeSecret(varName, value string) bool {
175+
// Empty values are not secrets
176+
if value == "" {
177+
return false
178+
}
179+
180+
// Check variable name for common secret indicators
181+
varNameLower := strings.ToLower(varName)
182+
secretNamePatterns := []string{
183+
"token", "secret", "key", "password", "passwd", "pwd",
184+
"credential", "auth", "api_key", "apikey", "access",
185+
}
186+
for _, pattern := range secretNamePatterns {
187+
if strings.Contains(varNameLower, pattern) {
188+
return true
189+
}
190+
}
191+
192+
// Check if value matches secret patterns (long strings, tokens, etc.)
193+
// Short values like "1", "true", "false" are unlikely to be secrets
194+
if len(value) <= 4 {
195+
return false
196+
}
197+
198+
// Check common non-sensitive values
199+
nonSensitiveValues := []string{
200+
"true", "false", "yes", "no", "on", "off",
201+
"debug", "info", "warn", "error",
202+
"dumb", "xterm", "ansi",
203+
}
204+
valueLower := strings.ToLower(value)
205+
for _, nonSensitive := range nonSensitiveValues {
206+
if valueLower == nonSensitive {
207+
return false
208+
}
209+
}
210+
211+
// Check if value looks like a token/key (GitHub PAT, JWT, API keys, etc.)
212+
for _, pattern := range SecretPatterns {
213+
if pattern.MatchString(value) {
214+
return true
215+
}
216+
}
217+
218+
// If value is longer than 16 chars and contains alphanumeric, treat as potential secret
219+
if len(value) > 16 && containsAlphanumeric(value) {
220+
return true
221+
}
222+
223+
return false
224+
}
225+
226+
// containsAlphanumeric checks if a string contains both letters and numbers
227+
func containsAlphanumeric(s string) bool {
228+
hasLetter := false
229+
hasDigit := false
230+
for _, c := range s {
231+
if (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') {
232+
hasLetter = true
233+
}
234+
if c >= '0' && c <= '9' {
235+
hasDigit = true
236+
}
237+
if hasLetter && hasDigit {
238+
return true
239+
}
240+
}
241+
return false
242+
}

internal/logger/sanitize/sanitize_test.go

Lines changed: 98 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -535,9 +535,9 @@ func TestSanitizeArgs(t *testing.T) {
535535
},
536536
expected: []string{
537537
"run",
538-
"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...",
539-
"-e", "API_KEY=sk_t...",
540-
"-e", "SHORT=...",
538+
"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...", // Secret detected by pattern
539+
"-e", "API_KEY=sk_t...", // Secret detected by name and pattern
540+
"-e", "SHORT=abc", // Non-sensitive: short value
541541
"--rm",
542542
"-i",
543543
"image:latest",
@@ -560,11 +560,11 @@ func TestSanitizeArgs(t *testing.T) {
560560
"run",
561561
"--rm",
562562
"-i",
563-
"-e", "NO_COLOR=...",
564-
"-e", "TERM=...",
565-
"-e", "PYTHONUNBUFFERED=...",
566-
"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...",
567-
"-e", "GITHUB_READ_ONLY=...",
563+
"-e", "NO_COLOR=1", // Non-sensitive: unchanged
564+
"-e", "TERM=dumb", // Non-sensitive: unchanged
565+
"-e", "PYTHONUNBUFFERED=1", // Non-sensitive: unchanged
566+
"-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_...", // Secret: truncated
567+
"-e", "GITHUB_READ_ONLY=1", // Non-sensitive: unchanged
568568
"ghcr.io/github/github-mcp-server:v0.29.0",
569569
},
570570
},
@@ -579,14 +579,14 @@ func TestSanitizeArgs(t *testing.T) {
579579
expected: []string{"run", "-e", "EMPTY=", "image:latest"},
580580
},
581581
{
582-
name: "env var with equals in value",
582+
name: "env var with equals in value (non-sensitive config)",
583583
input: []string{
584584
"run",
585-
"-e", "CONFIG=key=value=extra",
585+
"-e", "LOG_FORMAT=json=pretty",
586586
},
587587
expected: []string{
588588
"run",
589-
"-e", "CONFIG=key=...",
589+
"-e", "LOG_FORMAT=json=pretty", // Non-sensitive: short value, no secret indicators
590590
},
591591
},
592592
{
@@ -607,9 +607,49 @@ func TestSanitizeArgs(t *testing.T) {
607607
expected: []string{
608608
"run",
609609
"--name", "test-container",
610-
"-e", "API_KEY=secr...",
610+
"-e", "API_KEY=secr...", // Secret: detected by name "API_KEY"
611611
"--network", "host",
612-
"-e", "TOKEN=myto...",
612+
"-e", "TOKEN=myto...", // Secret: detected by name "TOKEN"
613+
"image:latest",
614+
},
615+
},
616+
{
617+
name: "non-sensitive configuration values",
618+
input: []string{
619+
"run",
620+
"-e", "DEBUG=true",
621+
"-e", "LOG_LEVEL=info",
622+
"-e", "ENABLE_FEATURE=yes",
623+
"-e", "PORT=8080",
624+
"image:latest",
625+
},
626+
expected: []string{
627+
"run",
628+
"-e", "DEBUG=true", // Non-sensitive: common config value
629+
"-e", "LOG_LEVEL=info", // Non-sensitive: common config value
630+
"-e", "ENABLE_FEATURE=yes", // Non-sensitive: common config value
631+
"-e", "PORT=8080", // Non-sensitive: port number
632+
"image:latest",
633+
},
634+
},
635+
{
636+
name: "mixed sensitive and non-sensitive values",
637+
input: []string{
638+
"run",
639+
"-e", "NO_COLOR=1",
640+
"-e", "GITHUB_TOKEN=ghp_abcdefghijklmnopqrstuvwxyz1234567890",
641+
"-e", "TERM=xterm",
642+
"-e", "DATABASE_PASSWORD=my_secret_db_pass_123",
643+
"-e", "TIMEOUT=30",
644+
"image:latest",
645+
},
646+
expected: []string{
647+
"run",
648+
"-e", "NO_COLOR=1", // Non-sensitive
649+
"-e", "GITHUB_TOKEN=ghp_...", // Secret: matches pattern
650+
"-e", "TERM=xterm", // Non-sensitive
651+
"-e", "DATABASE_PASSWORD=my_s...", // Secret: detected by name
652+
"-e", "TIMEOUT=30", // Non-sensitive
613653
"image:latest",
614654
},
615655
},
@@ -623,6 +663,51 @@ func TestSanitizeArgs(t *testing.T) {
623663
}
624664
}
625665

666+
func TestLooksLikeSecret(t *testing.T) {
667+
tests := []struct {
668+
name string
669+
varName string
670+
value string
671+
expected bool
672+
}{
673+
// Secret detection by name
674+
{name: "token in name", varName: "GITHUB_TOKEN", value: "abc123", expected: true},
675+
{name: "secret in name", varName: "API_SECRET", value: "xyz789", expected: true},
676+
{name: "key in name", varName: "DATABASE_KEY", value: "test123", expected: true},
677+
{name: "password in name", varName: "DB_PASSWORD", value: "pass123", expected: true},
678+
{name: "apikey in name", varName: "SERVICE_APIKEY", value: "key123", expected: true},
679+
680+
// Non-sensitive names
681+
{name: "color setting", varName: "NO_COLOR", value: "1", expected: false},
682+
{name: "term setting", varName: "TERM", value: "dumb", expected: false},
683+
{name: "debug flag", varName: "DEBUG", value: "true", expected: false},
684+
{name: "port number", varName: "PORT", value: "8080", expected: false},
685+
686+
// Secret detection by value pattern
687+
{name: "github pat", varName: "VAR", value: "ghp_1234567890123456789012345678901234567890", expected: true},
688+
{name: "long hex string", varName: "VAR", value: "abcdef1234567890abcdef1234567890abcdef12", expected: true},
689+
{name: "jwt token", varName: "VAR", value: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.payload.signature", expected: true},
690+
691+
// Short non-sensitive values
692+
{name: "single digit", varName: "VAR", value: "1", expected: false},
693+
{name: "boolean true", varName: "VAR", value: "true", expected: false},
694+
{name: "boolean false", varName: "VAR", value: "false", expected: false},
695+
{name: "yes value", varName: "VAR", value: "yes", expected: false},
696+
697+
// Edge cases
698+
{name: "empty value", varName: "VAR", value: "", expected: false},
699+
{name: "long non-alphanumeric", varName: "VAR", value: "====================", expected: false},
700+
{name: "long config path", varName: "CONFIG_PATH", value: "/usr/local/etc/app/config.json", expected: false},
701+
}
702+
703+
for _, tt := range tests {
704+
t.Run(tt.name, func(t *testing.T) {
705+
result := looksLikeSecret(tt.varName, tt.value)
706+
assert.Equal(t, tt.expected, result, "looksLikeSecret(%q, %q) should return %v", tt.varName, tt.value, tt.expected)
707+
})
708+
}
709+
}
710+
626711
func TestSanitizeArgsDoesNotLeakSecrets(t *testing.T) {
627712
// Test that actual secrets are not present in sanitized output
628713
secretToken := "ghp_verysecrettokenthatshouldbehidden1234567890"

0 commit comments

Comments
 (0)