-
Notifications
You must be signed in to change notification settings - Fork 16
Sanitize sensitive data in all log outputs #878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import ( | |
| "github.com/github/gh-aw-mcpg/internal/guard" | ||
| "github.com/github/gh-aw-mcpg/internal/launcher" | ||
| "github.com/github/gh-aw-mcpg/internal/logger" | ||
| "github.com/github/gh-aw-mcpg/internal/logger/sanitize" | ||
| "github.com/github/gh-aw-mcpg/internal/mcp" | ||
| "github.com/github/gh-aw-mcpg/internal/middleware" | ||
| "github.com/github/gh-aw-mcpg/internal/sys" | ||
|
|
@@ -326,7 +327,8 @@ func (us *UnifiedServer) registerToolsFromBackend(serverID string) error { | |
| // Log the MCP tool call request | ||
| sessionID := us.getSessionID(ctx) | ||
| 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) | ||
|
||
|
|
||
| // Check session is initialized | ||
| if err := us.requireSession(ctx); err != nil { | ||
|
|
@@ -341,7 +343,8 @@ func (us *UnifiedServer) registerToolsFromBackend(serverID string) error { | |
| logger.LogError("client", "MCP tool call error, session=%s, tool=%s, error=%v", sessionID, toolNameCopy, err) | ||
| } else { | ||
| resultJSON, _ := json.Marshal(data) | ||
| logger.LogInfo("client", "MCP tool call response, session=%s, tool=%s, result=%s", sessionID, toolNameCopy, string(resultJSON)) | ||
| sanitizedResult := sanitize.SanitizeString(string(resultJSON)) | ||
| logger.LogInfo("client", "MCP tool call response, session=%s, tool=%s, result=%s", sessionID, toolNameCopy, sanitizedResult) | ||
| } | ||
|
|
||
| return result, data, err | ||
|
|
@@ -440,7 +443,8 @@ func (us *UnifiedServer) registerSysTools() error { | |
| } | ||
|
|
||
| resultJSON, _ := json.Marshal(result) | ||
| logger.LogInfo("client", "MCP session initialization complete, session=%s, result=%s", sessionID, string(resultJSON)) | ||
| sanitizedResult := sanitize.SanitizeString(string(resultJSON)) | ||
| logger.LogInfo("client", "MCP session initialization complete, session=%s, result=%s", sessionID, sanitizedResult) | ||
| return nil, result, nil | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).