-
Notifications
You must be signed in to change notification settings - Fork 278
feat(mcp): implement logging capability with error handling and secret redaction #629
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
Merged
+678
−1
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| package mcplog | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
| ) | ||
|
|
||
| // HandleK8sError sends appropriate MCP log messages based on Kubernetes API error types. | ||
| // operation should describe the operation (e.g., "pod access", "deployment deletion"). | ||
| func HandleK8sError(ctx context.Context, err error, operation string) { | ||
| if err == nil { | ||
| return | ||
| } | ||
|
|
||
| if apierrors.IsNotFound(err) { | ||
| SendMCPLog(ctx, LevelInfo, "Resource not found - it may not exist or may have been deleted") | ||
| } else if apierrors.IsForbidden(err) { | ||
| SendMCPLog(ctx, LevelError, "Permission denied - check RBAC permissions for "+operation) | ||
| } else if apierrors.IsUnauthorized(err) { | ||
| SendMCPLog(ctx, LevelError, "Authentication failed - check cluster credentials") | ||
| } else if apierrors.IsAlreadyExists(err) { | ||
| SendMCPLog(ctx, LevelWarning, "Resource already exists") | ||
| } else if apierrors.IsInvalid(err) { | ||
| SendMCPLog(ctx, LevelError, "Invalid resource specification - check resource definition") | ||
| } else if apierrors.IsBadRequest(err) { | ||
| SendMCPLog(ctx, LevelError, "Invalid request - check parameters") | ||
| } else if apierrors.IsConflict(err) { | ||
| SendMCPLog(ctx, LevelError, "Resource conflict - resource may have been modified") | ||
| } else if apierrors.IsTimeout(err) { | ||
| SendMCPLog(ctx, LevelError, "Request timeout - cluster may be slow or overloaded") | ||
| } else if apierrors.IsServerTimeout(err) { | ||
| SendMCPLog(ctx, LevelError, "Server timeout - cluster may be slow or overloaded") | ||
| } else if apierrors.IsServiceUnavailable(err) { | ||
| SendMCPLog(ctx, LevelError, "Service unavailable - cluster may be unreachable") | ||
| } else if apierrors.IsTooManyRequests(err) { | ||
| SendMCPLog(ctx, LevelWarning, "Rate limited - too many requests to the cluster") | ||
| } else { | ||
| SendMCPLog(ctx, LevelError, "Operation failed - cluster may be unreachable or experiencing issues") | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,167 @@ | ||
| package mcplog | ||
|
|
||
| import ( | ||
| "context" | ||
| "regexp" | ||
|
|
||
| "github.com/go-logr/logr" | ||
| "github.com/modelcontextprotocol/go-sdk/mcp" | ||
| "k8s.io/klog/v2" | ||
| ) | ||
|
|
||
| // ContextKey is a type for context keys to avoid collisions | ||
| type ContextKey string | ||
|
|
||
| // MCPSessionContextKey is the context key for storing MCP ServerSession | ||
| const MCPSessionContextKey = ContextKey("mcp_session") | ||
|
|
||
| // Level represents MCP log severity levels per RFC 5424 syslog specification. | ||
| // https://modelcontextprotocol.io/specification/2025-11-25/server/utilities/logging#log-levels | ||
| type Level int | ||
|
|
||
| // Log levels from least to most severe, per MCP specification. | ||
| const ( | ||
| // LevelDebug is for detailed debugging information. | ||
| LevelDebug Level = iota | ||
| // LevelInfo is for general informational messages. | ||
| LevelInfo | ||
| // LevelNotice is for normal but significant events. | ||
| LevelNotice | ||
| // LevelWarning is for warning conditions. | ||
| LevelWarning | ||
| // LevelError is for error conditions. | ||
| LevelError | ||
| // LevelCritical is for critical conditions. | ||
| LevelCritical | ||
| // LevelAlert is for conditions requiring immediate action. | ||
| LevelAlert | ||
| // LevelEmergency is for system unusable conditions. | ||
| LevelEmergency | ||
| ) | ||
|
|
||
| // levelStrings maps Level values to their MCP protocol string representation. | ||
| var levelStrings = [...]string{ | ||
| LevelDebug: "debug", | ||
| LevelInfo: "info", | ||
| LevelNotice: "notice", | ||
| LevelWarning: "warning", | ||
| LevelError: "error", | ||
| LevelCritical: "critical", | ||
| LevelAlert: "alert", | ||
| LevelEmergency: "emergency", | ||
| } | ||
|
|
||
| // String returns the MCP protocol string representation of the level. | ||
| func (l Level) String() string { | ||
| if l >= 0 && int(l) < len(levelStrings) { | ||
| return levelStrings[l] | ||
| } | ||
| return "debug" | ||
| } | ||
|
|
||
| var ( | ||
| // mcpLogger is a dedicated named logger for MCP client-facing logs | ||
| // This provides complete separation from server logs | ||
| // issue for the sdk to implement this https://github.com/modelcontextprotocol/go-sdk/issues/748 | ||
| mcpLogger logr.Logger = klog.NewKlogr().WithName("mcp") | ||
|
|
||
| sensitivePatterns = []*regexp.Regexp{ | ||
| // Generic JSON/YAML fields | ||
| regexp.MustCompile(`("password"\s*:\s*)"[^"]*"`), | ||
| regexp.MustCompile(`("token"\s*:\s*)"[^"]*"`), | ||
| regexp.MustCompile(`("secret"\s*:\s*)"[^"]*"`), | ||
| regexp.MustCompile(`("api[_-]?key"\s*:\s*)"[^"]*"`), | ||
| regexp.MustCompile(`("access[_-]?key"\s*:\s*)"[^"]*"`), | ||
| regexp.MustCompile(`("client[_-]?secret"\s*:\s*)"[^"]*"`), | ||
| regexp.MustCompile(`("private[_-]?key"\s*:\s*)"[^"]*"`), | ||
| // Authorization headers | ||
| regexp.MustCompile(`(Bearer\s+)[A-Za-z0-9\-._~+/]+=*`), | ||
| regexp.MustCompile(`(Basic\s+)[A-Za-z0-9+/]+=*`), | ||
| // AWS credentials | ||
| regexp.MustCompile(`(AKIA[0-9A-Z]{16})`), | ||
| regexp.MustCompile(`(aws_secret_access_key\s*=\s*)([A-Za-z0-9/+=]{40})`), | ||
| regexp.MustCompile(`(A3T[A-Z0-9]|AKIA|AGPA|AIDA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16}`), | ||
| // GitHub tokens | ||
| regexp.MustCompile(`(ghp_[a-zA-Z0-9]{36})`), | ||
| regexp.MustCompile(`(github_pat_[a-zA-Z0-9]{22}_[a-zA-Z0-9]{59})`), | ||
| // GitLab tokens | ||
| regexp.MustCompile(`(glpat-[a-zA-Z0-9\-_]{20})`), | ||
| // GCP | ||
| regexp.MustCompile(`(AIza[0-9A-Za-z\-_]{35})`), | ||
| // Azure | ||
| regexp.MustCompile(`(AccountKey=[A-Za-z0-9+/]{88}==)`), | ||
| // OpenAI / Anthropic | ||
| regexp.MustCompile(`(sk-proj-[a-zA-Z0-9]{48})`), | ||
| regexp.MustCompile(`(sk-ant-api03-[a-zA-Z0-9\-_]{95})`), | ||
| // JWT tokens | ||
| regexp.MustCompile(`(eyJ[a-zA-Z0-9_-]+\.eyJ[a-zA-Z0-9_-]+\.[a-zA-Z0-9_-]+)`), | ||
| // Private keys | ||
| regexp.MustCompile(`(-----BEGIN[A-Z ]+PRIVATE KEY-----)`), | ||
| regexp.MustCompile(`(-----BEGIN RSA PRIVATE KEY-----)`), | ||
| regexp.MustCompile(`(-----BEGIN EC PRIVATE KEY-----)`), | ||
| regexp.MustCompile(`(-----BEGIN OPENSSH PRIVATE KEY-----)`), | ||
| regexp.MustCompile(`(-----BEGIN PGP PRIVATE KEY BLOCK-----)`), | ||
| // Database connection strings | ||
| regexp.MustCompile(`(postgres://[^:]+:)([^@]+)(@)`), | ||
| regexp.MustCompile(`(mysql://[^:]+:)([^@]+)(@)`), | ||
| regexp.MustCompile(`(mongodb(\+srv)?://[^:]+:)([^@]+)(@)`), | ||
| } | ||
| ) | ||
|
|
||
| func sanitizeMessage(msg string) string { | ||
| // JSON/YAML field patterns (indices 0-6) - preserve field name | ||
| for i := 0; i < 7 && i < len(sensitivePatterns); i++ { | ||
| msg = sensitivePatterns[i].ReplaceAllString(msg, `$1"[REDACTED]"`) | ||
| } | ||
|
|
||
| // Authorization headers (indices 7-8) - preserve header type | ||
| for i := 7; i < 9 && i < len(sensitivePatterns); i++ { | ||
| msg = sensitivePatterns[i].ReplaceAllString(msg, `$1[REDACTED]`) | ||
| } | ||
|
|
||
| // Database connection strings (indices 25-27) - preserve URL structure | ||
| if len(sensitivePatterns) > 27 { | ||
| msg = sensitivePatterns[25].ReplaceAllString(msg, `$1[REDACTED]$3`) // PostgreSQL | ||
| msg = sensitivePatterns[26].ReplaceAllString(msg, `$1[REDACTED]$3`) // MySQL | ||
| msg = sensitivePatterns[27].ReplaceAllString(msg, `$1[REDACTED]$4`) // MongoDB | ||
| } | ||
|
|
||
| // All other patterns (AWS, GitHub, tokens, keys, etc.) - redact entire match | ||
| for i := 9; i < len(sensitivePatterns); i++ { | ||
| // Skip database patterns (already handled) | ||
| if i >= 25 && i <= 27 { | ||
| continue | ||
| } | ||
| msg = sensitivePatterns[i].ReplaceAllString(msg, `[REDACTED]`) | ||
| } | ||
|
|
||
| return msg | ||
| } | ||
|
|
||
| // SendMCPLog sends a log notification to the MCP client and server logs. | ||
| // Uses dedicated "mcp" named logger. Message is automatically sanitized. | ||
| func SendMCPLog(ctx context.Context, level Level, message string) { | ||
| switch level { | ||
| case LevelError, LevelCritical, LevelAlert, LevelEmergency: | ||
| mcpLogger.Error(nil, message) | ||
| case LevelWarning, LevelNotice: | ||
| mcpLogger.V(1).Info(message) | ||
| default: | ||
| mcpLogger.V(2).Info(message) | ||
| } | ||
|
|
||
| session, ok := ctx.Value(MCPSessionContextKey).(*mcp.ServerSession) | ||
| if !ok || session == nil { | ||
| return | ||
| } | ||
|
|
||
| message = sanitizeMessage(message) | ||
|
|
||
| if err := session.Log(ctx, &mcp.LoggingMessageParams{ | ||
| Level: mcp.LoggingLevel(level.String()), | ||
| Logger: "kubernetes-mcp-server", | ||
| Data: message, | ||
| }); err != nil { | ||
| mcpLogger.V(3).Info("failed to send log to MCP client", "error", err) | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I guess there hard coded checks seem a bit funky ? 😅
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.
yeah, I feel the same, but couldn't think of something better, will take another pass at it
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.
Would this not be one of the sanitizations that the MCP gateway would handle? (Although I get that that depends on a specific deployment topology, so maybe not the best fix for here)
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 spec states that the server should take care of this: https://modelcontextprotocol.io/specification/2025-11-25/server/utilities/logging#implementation-considerations
However, I believe this should be part of the SDK itself, in addition to our own implementation, we might want to open an issue for this.
It's my understanding that most servers will deal with the same kind of logic to remove sensitive information.
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.
created an issue modelcontextprotocol/go-sdk#748
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.
awesome, thx!