fix(security): truncate session IDs in all log calls in session.go#2731
fix(security): truncate session IDs in all log calls in session.go#2731
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/39b98ec9-5f8c-4342-a78e-808dd1801832 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses a security issue in the server session handling by redacting session IDs (raw API keys per spec) in log output, aligning session.go with the rest of the server package’s safe-logging pattern.
Changes:
- Imported
internal/authininternal/server/session.go. - Wrapped
sessionIDarguments inlog.Printf/logSession.Printfcalls withauth.TruncateSessionID().
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logUnified.Printf("Created session directory: %s", sessionDir) | ||
| log.Printf("Created payload directory for session: %s", sessionID) | ||
| log.Printf("Created payload directory for session: %s", auth.TruncateSessionID(sessionID)) |
There was a problem hiding this comment.
sessionDir includes the raw sessionID (API key), so logUnified.Printf("Created session directory: %s", sessionDir) still logs the full secret via the path. Consider logging payloadDir separately and the session via auth.TruncateSessionID(sessionID) (and apply the same redaction to the "Session directory already exists" log in this function) so no log line can reconstruct the full session ID.
session.gowas logging full session IDs (which are the raw API keys per spec §7.1) vialog.PrintfandlogSession.Printf, violating the spec's "no plaintext API key logging" requirement. Every other file in the server package already usesauth.TruncateSessionID()for this purpose.Changes
internal/server/session.go: Addedinternal/authimport and wrapped allsessionIDarguments in log calls withauth.TruncateSessionID()— covering both standardlog.Printfcalls andlogSession.Printfdebug logger calls.auth.TruncateSessionIDreturns only the first 8 characters, consistent with the pattern inhttp_helpers.goandsdk_logging.go.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-build725754765/b334/launcher.test /tmp/go-build725754765/b334/launcher.test -test.testlogfile=/tmp/go-build725754765/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s 4072�� om/!burnt!sushi/toml@v1.6.0/deco-c=4 om/!burnt!sushi/toml@v1.6.0/depr-nolocalimports x_amd64/vet(dns block)invalid-host-that-does-not-exist-12345.com/tmp/go-build725754765/b319/config.test /tmp/go-build725754765/b319/config.test -test.testlogfile=/tmp/go-build725754765/b319/testlog.txt -test.paniconexit0 -test.timeout=10m0s 64/s�� ternal/engine/wazevo/ssa/basic_block.go ternal/engine/wazevo/ssa/basic_block_sort.go x_amd64/compile(dns block)nonexistent.local/tmp/go-build725754765/b334/launcher.test /tmp/go-build725754765/b334/launcher.test -test.testlogfile=/tmp/go-build725754765/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s 4072�� om/!burnt!sushi/toml@v1.6.0/deco-c=4 om/!burnt!sushi/toml@v1.6.0/depr-nolocalimports x_amd64/vet(dns block)slow.example.com/tmp/go-build725754765/b334/launcher.test /tmp/go-build725754765/b334/launcher.test -test.testlogfile=/tmp/go-build725754765/b334/testlog.txt -test.paniconexit0 -test.timeout=10m0s 4072�� om/!burnt!sushi/toml@v1.6.0/deco-c=4 om/!burnt!sushi/toml@v1.6.0/depr-nolocalimports x_amd64/vet(dns block)this-host-does-not-exist-12345.com/tmp/go-build725754765/b343/mcp.test /tmp/go-build725754765/b343/mcp.test -test.testlogfile=/tmp/go-build725754765/b343/testlog.txt -test.paniconexit0 -test.timeout=10m0s -o /mcp/connection.go /mcp/errors.go x_amd64/vet -p strings lcache/go/1.25.8/x64=/_/GOROOT x_amd64/vet -I g_.a 4072538/b151/ x_amd64/vet -o ernal/middleware--version lcache/go/1.25.8/x64=/_/GOROOT x_amd64/vet(dns block)If you need me to access, download, or install something from one of these locations, you can either:
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.