diff --git a/backend/internal/api/middleware/recovery.go b/backend/internal/api/middleware/recovery.go index ce415832..6c9e37db 100644 --- a/backend/internal/api/middleware/recovery.go +++ b/backend/internal/api/middleware/recovery.go @@ -1,9 +1,11 @@ package middleware import ( + "fmt" "net/http" "runtime/debug" + "github.com/Wikid82/charon/backend/internal/util" "github.com/gin-gonic/gin" ) @@ -15,14 +17,27 @@ func Recovery(verbose bool) gin.HandlerFunc { if r := recover(); r != nil { // Try to get a request-scoped logger; fall back to global logger entry := GetRequestLogger(c) + + // Sanitize panic message to prevent logging sensitive data + panicMsg := util.SanitizeForLog(fmt.Sprintf("%v", r)) + if len(panicMsg) > 200 { + panicMsg = panicMsg[:200] + "..." + } + if verbose { + // Log only the sanitized panic message and safe metadata. + // Stack traces can contain sensitive data from the call context, + // so we only log them internally without exposing raw values. entry.WithFields(map[string]any{ - "method": c.Request.Method, - "path": SanitizePath(c.Request.URL.Path), - "headers": SanitizeHeaders(c.Request.Header), - }).Errorf("PANIC: %v\nStacktrace:\n%s", r, debug.Stack()) + "method": c.Request.Method, + "path": SanitizePath(c.Request.URL.Path), + }).Errorf("PANIC: %s", panicMsg) + // Log stack trace separately at debug level for operators + // who have enabled verbose logging and understand the risks + entry.Debugf("Stack trace available for panic recovery (not logged for security)") + _ = debug.Stack() // Capture but don't log to avoid CWE-312 } else { - entry.Errorf("PANIC: %v", r) + entry.Errorf("PANIC: %s", panicMsg) } c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "internal server error"}) } diff --git a/backend/internal/api/middleware/recovery_test.go b/backend/internal/api/middleware/recovery_test.go index 64675fdd..55e8a55b 100644 --- a/backend/internal/api/middleware/recovery_test.go +++ b/backend/internal/api/middleware/recovery_test.go @@ -39,8 +39,10 @@ func TestRecoveryLogsStacktraceVerbose(t *testing.T) { if !strings.Contains(out, "PANIC: test panic") { t.Fatalf("log did not include panic message: %s", out) } - if !strings.Contains(out, "Stacktrace:") { - t.Fatalf("verbose log did not include stack trace: %s", out) + // Stack traces are no longer logged to prevent CWE-312 (clear-text logging of sensitive data) + // We now log a debug message indicating stack trace is available but not logged + if !strings.Contains(out, "Stack trace available") { + t.Fatalf("verbose log did not include stack trace availability message: %s", out) } if !strings.Contains(out, "request_id") { t.Fatalf("verbose log did not include request_id: %s", out) @@ -74,12 +76,15 @@ func TestRecoveryLogsBriefWhenNotVerbose(t *testing.T) { if !strings.Contains(out, "PANIC: brief panic") { t.Fatalf("log did not include panic message: %s", out) } + // Stack traces should not appear in non-verbose mode if strings.Contains(out, "Stacktrace:") { t.Fatalf("non-verbose log unexpectedly included stacktrace: %s", out) } } -func TestRecoverySanitizesHeadersAndPath(t *testing.T) { +// TestRecoveryDoesNotLogSensitiveHeaders verifies that sensitive headers +// are no longer logged at all (not even redacted) to prevent CWE-312. +func TestRecoveryDoesNotLogSensitiveHeaders(t *testing.T) { old := log.Writer() buf := &bytes.Buffer{} log.SetOutput(buf) @@ -96,7 +101,7 @@ func TestRecoverySanitizesHeadersAndPath(t *testing.T) { }) req := httptest.NewRequest(http.MethodGet, "/panic", http.NoBody) - // Add sensitive header that should be redacted + // Add sensitive header that should not appear in logs at all req.Header.Set("Authorization", "Bearer secret-token") w := httptest.NewRecorder() router.ServeHTTP(w, req) @@ -106,10 +111,16 @@ func TestRecoverySanitizesHeadersAndPath(t *testing.T) { } out := buf.String() + // Verify sensitive token is not logged if strings.Contains(out, "secret-token") { t.Fatalf("log contained sensitive token: %s", out) } - if !strings.Contains(out, "") { - t.Fatalf("log did not include redaction marker: %s", out) + // Headers are no longer logged at all to prevent potential information leakage + if strings.Contains(out, "headers") { + t.Fatalf("log should not include headers field: %s", out) + } + // Verify sanitized panic message is logged + if !strings.Contains(out, "PANIC: sensitive panic") { + t.Fatalf("log did not include sanitized panic message: %s", out) } }