Sanitize sensitive data in all log outputs#878
Conversation
- Fix env passthrough logging to use TruncateSecret instead of custom truncation - Sanitize session IDs in transport.go HTTP connection logs - Sanitize request/response bodies in http_helpers.go and transport.go - Sanitize tool call arguments and results in unified.go - Sanitize stderr output from backend MCP servers in connection.go - All changes use existing sanitize.SanitizeString() and sanitize.TruncateSecret() - Tests pass, build succeeds Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a critical security issue where sensitive authentication tokens, API keys, and environment variables were being logged in plain text across the codebase. The changes introduce consistent sanitization of log outputs using the existing sanitize package infrastructure.
Changes:
- Applied
sanitize.TruncateSecret()to truncate environment variables and session IDs (showing first 4 chars + "...") - Applied
sanitize.SanitizeString()to sanitize HTTP request/response bodies, tool call arguments/results, and backend stderr output - Added import statements for the sanitize package in files that needed it
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/launcher/log_helpers.go | Changed environment variable logging to use sanitize.TruncateSecret() instead of custom 10-char truncation |
| internal/server/transport.go | Added sanitization for session IDs and HTTP response bodies in connection logs |
| internal/server/http_helpers.go | Added sanitization for HTTP request bodies before logging |
| internal/server/unified.go | Added sanitization for tool call arguments, results, and session initialization data |
| internal/mcp/connection.go | Added sanitization for backend stderr output to catch sensitive data from MCP server processes |
Comments suppressed due to low confidence (2)
internal/server/unified.go:347
- The session ID is being logged without sanitization. Session IDs can be API keys or authentication tokens and should be truncated using
sanitize.TruncateSecret()before logging to prevent credential exposure in log files. This is inconsistent with the session ID logging in transport.go line 91 which correctly usessanitize.TruncateSecret(sessionID).
logger.LogInfo("client", "MCP tool call response, session=%s, tool=%s, result=%s", sessionID, toolNameCopy, sanitizedResult)
internal/server/unified.go:447
- The session ID is being logged without sanitization. Session IDs can be API keys or authentication tokens and should be truncated using
sanitize.TruncateSecret()before logging to prevent credential exposure in log files. This is inconsistent with the session ID logging in transport.go line 91 which correctly usessanitize.TruncateSecret(sessionID).
logger.LogInfo("client", "MCP session initialization complete, session=%s, result=%s", sessionID, sanitizedResult)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| argsJSON, _ := json.Marshal(toolArgs) | ||
| logger.LogInfo("client", "MCP tool call request, session=%s, tool=%s, args=%s", sessionID, toolNameCopy, string(argsJSON)) | ||
| sanitizedArgs := sanitize.SanitizeString(string(argsJSON)) | ||
| logger.LogInfo("client", "MCP tool call request, session=%s, tool=%s, args=%s", sessionID, toolNameCopy, sanitizedArgs) |
There was a problem hiding this comment.
The session ID is being logged without sanitization. Session IDs can be API keys or authentication tokens and should be truncated using sanitize.TruncateSecret() before logging to prevent credential exposure in log files. This is inconsistent with the session ID logging in transport.go line 91 which correctly uses sanitize.TruncateSecret(sessionID).
This issue also appears in the following locations of the same file:
- line 347
- line 447
| logger.LogDebug("client", "MCP client request body, backend=%s, body=%s", backendID, sanitizedBody) | ||
| } else { | ||
| logger.LogDebug("client", "MCP request body, session=%s, body=%s", sessionID, string(bodyBytes)) | ||
| logger.LogDebug("client", "MCP request body, session=%s, body=%s", sessionID, sanitizedBody) |
There was a problem hiding this comment.
The session ID is being logged without sanitization. Session IDs can be API keys or authentication tokens and should be truncated using sanitize.TruncateSecret() before logging to prevent credential exposure in log files. This is inconsistent with the session ID logging in transport.go line 91 which correctly uses sanitize.TruncateSecret(sessionID).
Log outputs were exposing sensitive authentication tokens, API keys, and environment variables in plain text. Docker launch commands logged full environment variable values, and HTTP request/response bodies, tool arguments, and backend stderr were logged without redaction.
Changes
Environment variable logging (
internal/launcher/log_helpers.go): Usesanitize.TruncateSecret()(first 4 chars + "...") instead of custom 10-char truncation for environment variable passthrough loggingSession ID logging (
internal/server/transport.go): Truncate full session IDs in HTTP connection logs to prevent API key exposureRequest/response bodies (
internal/server/http_helpers.go,internal/server/transport.go): Applysanitize.SanitizeString()to sanitize HTTP payloads before loggingTool call data (
internal/server/unified.go): Sanitize both tool arguments and results to prevent tokens in tool parameters from being loggedBackend stderr (
internal/mcp/connection.go): Sanitize stderr output from MCP server processes to catch sensitive data printed by backendsAll sanitization uses existing infrastructure:
sanitize.SanitizeString()- Regex-based detection/redaction (GitHub PATs, Bearer tokens, JWT, API keys, passwords)sanitize.TruncateSecret()- Shows first 4 characters + "..."Example:
Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
example.com/tmp/go-build2148812847/b275/launcher.test /tmp/go-build2148812847/b275/launcher.test -test.testlogfile=/tmp/go-build2148812847/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go HEAD 64/pkg/tool/linu-o(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build2148812847/b260/config.test /tmp/go-build2148812847/b260/config.test -test.testlogfile=/tmp/go-build2148812847/b260/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go HEAD 64/pkg/tool/linu-lang=go1.25(dns block)nonexistent.local/tmp/go-build2148812847/b275/launcher.test /tmp/go-build2148812847/b275/launcher.test -test.testlogfile=/tmp/go-build2148812847/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go HEAD 64/pkg/tool/linu-o(dns block)slow.example.com/tmp/go-build2148812847/b275/launcher.test /tmp/go-build2148812847/b275/launcher.test -test.testlogfile=/tmp/go-build2148812847/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true go HEAD 64/pkg/tool/linu-o(dns block)this-host-does-not-exist-12345.com/tmp/go-build2148812847/b284/mcp.test /tmp/go-build2148812847/b284/mcp.test -test.testlogfile=/tmp/go-build2148812847/b284/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -c=4 -nolocalimports -importcfg /tmp/go-build2148812847/b253/importcfg -pack /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/internal/sys/sys.go 64/pkg/tool/linu/tmp/go-build2148812847/b097/vet.cfg rev-�� go 2aPTQz7W- /home/REDACTED/.local/bin/git --abbrev-ref HEAD /usr/local/bin/g/tmp/go-build2148812847/b256/_pkg_.a git(dns block)If you need me to access, download, or install something from one of these locations, you can either: