Merge pull request #207 from githubnext/copilot/refactor-secret-sanitization-logic

Author: pelikhan

internal/logger/jsonl_logger.go

@@ -7,6 +7,8 @@ import (
 	"path/filepath"
 	"sync"
 	"time"
+
+	"github.com/githubnext/gh-aw-mcpg/internal/logger/sanitize"
 )
 
 // JSONLLogger manages logging RPC messages to a JSONL file (one JSON object per line)
@@ -88,35 +90,10 @@ func (jl *JSONLLogger) Close() error {
 
 // sanitizePayload sanitizes a payload by applying regex patterns to the entire string
 // It takes raw bytes, applies regex sanitization in one pass, and returns sanitized bytes
+// This function is deprecated and will be removed in a future version.
+// Use sanitize.SanitizeJSON() directly instead.
 func sanitizePayload(payloadBytes []byte) json.RawMessage {
-	// Apply regex sanitization to the entire string in one pass
-	sanitized := sanitizeSecrets(string(payloadBytes))
-
-	// Validate that the result is valid JSON for RawMessage
-	// If not valid, wrap it in a JSON object
-	if !json.Valid([]byte(sanitized)) {
-		// Create a valid JSON object with the invalid content as a string
-		wrapped := map[string]string{
-			"_error": "invalid JSON",
-			"_raw":   sanitized,
-		}
-		wrappedBytes, _ := json.Marshal(wrapped)
-		return json.RawMessage(wrappedBytes)
-	}
-
-	// Marshal and unmarshal to ensure single-line JSON (removes newlines/whitespace)
-	var tmp interface{}
-	if err := json.Unmarshal([]byte(sanitized), &tmp); err != nil {
-		// Should not happen since we validated above, but handle gracefully
-		wrapped := map[string]string{
-			"_error": "failed to parse JSON",
-			"_raw":   sanitized,
-		}
-		wrappedBytes, _ := json.Marshal(wrapped)
-		return json.RawMessage(wrappedBytes)
-	}
-	compactBytes, _ := json.Marshal(tmp)
-	return json.RawMessage(compactBytes)
+	return sanitize.SanitizeJSON(payloadBytes)
 }
 
 // LogMessage logs an RPC message to the JSONL file

internal/logger/markdown_logger.go

@@ -4,9 +4,10 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 	"sync"
+
+	"github.com/githubnext/gh-aw-mcpg/internal/logger/sanitize"
 )
 
 // MarkdownLogger manages logging to a markdown file for GitHub workflow previews
@@ -22,20 +23,6 @@ type MarkdownLogger struct {
 var (
 	globalMarkdownLogger *MarkdownLogger
 	globalMarkdownMu     sync.RWMutex
-	// Patterns for detecting potential secrets (simple heuristics)
-	secretPatterns = []*regexp.Regexp{
-		regexp.MustCompile(`(?i)(token|key|secret|password|auth)[=:]\s*[^\s]{8,}`),
-		regexp.MustCompile(`ghp_[a-zA-Z0-9]{36,}`),                                  // GitHub PATs
-		regexp.MustCompile(`github_pat_[a-zA-Z0-9]{22}_[a-zA-Z0-9]{59}`),            // GitHub fine-grained PATs
-		regexp.MustCompile(`(?i)bearer\s+[a-zA-Z0-9\-._~+/]+=*`),                    // Bearer tokens
-		regexp.MustCompile(`(?i)authorization:\s*[a-zA-Z0-9\-._~+/]+=*`),            // Auth headers
-		regexp.MustCompile(`[a-f0-9]{32,}`),                                         // Long hex strings (API keys)
-		regexp.MustCompile(`(?i)(apikey|api_key|access_key)[=:]\s*[^\s]{8,}`),       // API keys
-		regexp.MustCompile(`(?i)(client_secret|client_id)[=:]\s*[^\s]{8,}`),         // OAuth secrets
-		regexp.MustCompile(`[a-zA-Z0-9_-]{20,}\.eyJ[a-zA-Z0-9_-]+\.[a-zA-Z0-9_-]+`), // JWT tokens
-		// JSON-specific patterns for field:value pairs
-		regexp.MustCompile(`(?i)"(token|password|passwd|pwd|apikey|api_key|api-key|secret|client_secret|api_secret|authorization|auth|key|private_key|public_key|credentials|credential|access_token|refresh_token|bearer_token)"\s*:\s*"[^"]{1,}"`),
-	}
 )
 
 // InitMarkdownLogger initializes the global markdown logger
@@ -114,22 +101,10 @@ func (ml *MarkdownLogger) Close() error {
 }
 
 // sanitizeSecrets replaces potential secrets with [REDACTED]
+// This function is deprecated and will be removed in a future version.
+// Use sanitize.SanitizeString() directly instead.
 func sanitizeSecrets(message string) string {
-	result := message
-	for _, pattern := range secretPatterns {
-		result = pattern.ReplaceAllStringFunc(result, func(match string) string {
-			// Keep the prefix (key name) but redact the value
-			if strings.Contains(match, "=") || strings.Contains(match, ":") {
-				parts := regexp.MustCompile(`[=:]\s*`).Split(match, 2)
-				if len(parts) == 2 {
-					return parts[0] + "=[REDACTED]"
-				}
-			}
-			// For tokens without key=value format, redact entirely
-			return "[REDACTED]"
-		})
-	}
-	return result
+	return sanitize.SanitizeString(message)
 }
 
 // getEmojiForLevel returns the appropriate emoji for the log level

internal/logger/rpc_logger.go

@@ -4,6 +4,8 @@ import (
 	"encoding/json"
 	"fmt"
 	"strings"
+
+	"github.com/githubnext/gh-aw-mcpg/internal/logger/sanitize"
 )
 
 // RPCMessageType represents the direction of an RPC message
@@ -47,7 +49,7 @@ type RPCMessageInfo struct {
 // truncateAndSanitize truncates the payload to max length and sanitizes secrets
 func truncateAndSanitize(payload string, maxLength int) string {
 	// First sanitize secrets
-	sanitized := sanitizeSecrets(payload)
+	sanitized := sanitize.SanitizeString(payload)
 
 	// Then truncate if needed
 	if len(sanitized) > maxLength {

internal/logger/sanitize/sanitize.go

@@ -0,0 +1,74 @@
+package sanitize
+
+import (
+	"encoding/json"
+	"regexp"
+	"strings"
+)
+
+// SecretPatterns contains regex patterns for detecting potential secrets
+var SecretPatterns = []*regexp.Regexp{
+	regexp.MustCompile(`(?i)(token|key|secret|password|auth)[=:]\s*[^\s]{8,}`),
+	regexp.MustCompile(`ghp_[a-zA-Z0-9]{36,}`),                                  // GitHub PATs
+	regexp.MustCompile(`github_pat_[a-zA-Z0-9]{22}_[a-zA-Z0-9]{59}`),            // GitHub fine-grained PATs
+	regexp.MustCompile(`(?i)bearer\s+[a-zA-Z0-9\-._~+/]+=*`),                    // Bearer tokens
+	regexp.MustCompile(`(?i)authorization:\s*[a-zA-Z0-9\-._~+/]+=*`),            // Auth headers
+	regexp.MustCompile(`[a-f0-9]{32,}`),                                         // Long hex strings (API keys)
+	regexp.MustCompile(`(?i)(apikey|api_key|access_key)[=:]\s*[^\s]{8,}`),       // API keys
+	regexp.MustCompile(`(?i)(client_secret|client_id)[=:]\s*[^\s]{8,}`),         // OAuth secrets
+	regexp.MustCompile(`[a-zA-Z0-9_-]{20,}\.eyJ[a-zA-Z0-9_-]+\.[a-zA-Z0-9_-]+`), // JWT tokens
+	// JSON-specific patterns for field:value pairs
+	regexp.MustCompile(`(?i)"(token|password|passwd|pwd|apikey|api_key|api-key|secret|client_secret|api_secret|authorization|auth|key|private_key|public_key|credentials|credential|access_token|refresh_token|bearer_token)"\s*:\s*"[^"]{1,}"`),
+}
+
+// SanitizeString replaces potential secrets in a string with [REDACTED]
+func SanitizeString(message string) string {
+	result := message
+	for _, pattern := range SecretPatterns {
+		result = pattern.ReplaceAllStringFunc(result, func(match string) string {
+			// Keep the prefix (key name) but redact the value
+			if strings.Contains(match, "=") || strings.Contains(match, ":") {
+				parts := regexp.MustCompile(`[=:]\s*`).Split(match, 2)
+				if len(parts) == 2 {
+					return parts[0] + "=[REDACTED]"
+				}
+			}
+			// For tokens without key=value format, redact entirely
+			return "[REDACTED]"
+		})
+	}
+	return result
+}
+
+// SanitizeJSON sanitizes a JSON payload by applying regex patterns to the entire string
+// It takes raw bytes, applies regex sanitization in one pass, and returns sanitized bytes
+func SanitizeJSON(payloadBytes []byte) json.RawMessage {
+	// Apply regex sanitization to the entire string in one pass
+	sanitized := SanitizeString(string(payloadBytes))
+
+	// Validate that the result is valid JSON for RawMessage
+	// If not valid, wrap it in a JSON object
+	if !json.Valid([]byte(sanitized)) {
+		// Create a valid JSON object with the invalid content as a string
+		wrapped := map[string]string{
+			"_error": "invalid JSON",
+			"_raw":   sanitized,
+		}
+		wrappedBytes, _ := json.Marshal(wrapped)
+		return json.RawMessage(wrappedBytes)
+	}
+
+	// Marshal and unmarshal to ensure single-line JSON (removes newlines/whitespace)
+	var tmp interface{}
+	if err := json.Unmarshal([]byte(sanitized), &tmp); err != nil {
+		// Should not happen since we validated above, but handle gracefully
+		wrapped := map[string]string{
+			"_error": "failed to parse JSON",
+			"_raw":   sanitized,
+		}
+		wrappedBytes, _ := json.Marshal(wrapped)
+		return json.RawMessage(wrappedBytes)
+	}
+	compactBytes, _ := json.Marshal(tmp)
+	return json.RawMessage(compactBytes)
+}