diff --git a/backend/internal/api/handlers/certificate_handler.go b/backend/internal/api/handlers/certificate_handler.go index e5a1f1dd..ed2c630d 100644 --- a/backend/internal/api/handlers/certificate_handler.go +++ b/backend/internal/api/handlers/certificate_handler.go @@ -10,6 +10,7 @@ import ( "time" "github.com/gin-gonic/gin" + "github.com/google/uuid" "gorm.io/gorm" "github.com/Wikid82/charon/backend/internal/logger" @@ -367,7 +368,7 @@ func (h *CertificateHandler) Export(c *gin.Context) { c.JSON(http.StatusNotFound, gin.H{"error": "certificate not found"}) return } - logger.Log().WithError(err).Error("failed to export certificate") + logger.Log().WithError(fmt.Errorf("%s", util.SanitizeForLog(err.Error()))).Error("failed to export certificate") c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to export certificate"}) return } @@ -423,19 +424,21 @@ func (h *CertificateHandler) Delete(c *gin.Context) { return } - // UUID path - value isn't numeric, validate it looks like a UUID - if idStr == "" || idStr == "0" || len(idStr) < 32 { + // UUID path - parse to validate format and produce a canonical, safe string + parsedUUID, parseErr := uuid.Parse(idStr) + if parseErr != nil { c.JSON(http.StatusBadRequest, gin.H{"error": "invalid id"}) return } + certUUID := parsedUUID.String() - inUse, err := h.service.IsCertificateInUseByUUID(idStr) + inUse, err := h.service.IsCertificateInUseByUUID(certUUID) if err != nil { if err == services.ErrCertNotFound { c.JSON(http.StatusNotFound, gin.H{"error": "certificate not found"}) return } - logger.Log().WithError(err).WithField("certificate_uuid", idStr).Error("failed to check certificate usage") + logger.Log().WithError(err).WithField("certificate_uuid", util.SanitizeForLog(certUUID)).Error("failed to check certificate usage") c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to check certificate usage"}) return } @@ -459,7 +462,7 @@ func (h *CertificateHandler) Delete(c *gin.Context) { } } - if err := h.service.DeleteCertificate(idStr); err != nil { + if err := h.service.DeleteCertificate(certUUID); err != nil { if err == services.ErrCertInUse { c.JSON(http.StatusConflict, gin.H{"error": "certificate is in use by one or more proxy hosts"}) return @@ -468,12 +471,12 @@ func (h *CertificateHandler) Delete(c *gin.Context) { c.JSON(http.StatusNotFound, gin.H{"error": "certificate not found"}) return } - logger.Log().WithError(err).WithField("certificate_uuid", idStr).Error("failed to delete certificate") + logger.Log().WithError(err).WithField("certificate_uuid", util.SanitizeForLog(certUUID)).Error("failed to delete certificate") c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to delete certificate"}) return } - h.sendDeleteNotification(c, idStr) + h.sendDeleteNotification(c, certUUID) c.JSON(http.StatusOK, gin.H{"message": "certificate deleted"}) } @@ -482,11 +485,15 @@ func (h *CertificateHandler) sendDeleteNotification(c *gin.Context, certRef stri return } + // Re-validate to produce a CodeQL-safe value (breaks taint from user input). + // Callers already pass validated data; this is defense-in-depth. + safeRef := sanitizeCertRef(certRef) + h.notificationMu.Lock() lastTime, exists := h.lastNotificationTime[certRef] if exists && time.Since(lastTime) < 10*time.Second { h.notificationMu.Unlock() - logger.Log().WithField("certificate_ref", certRef).Debug("notification rate limited") + logger.Log().WithField("certificate_ref", safeRef).Debug("notification rate limited") return } h.lastNotificationTime[certRef] = time.Now() @@ -495,10 +502,22 @@ func (h *CertificateHandler) sendDeleteNotification(c *gin.Context, certRef stri h.notificationService.SendExternal(c.Request.Context(), "cert", "Certificate Deleted", - fmt.Sprintf("Certificate %s deleted", certRef), + fmt.Sprintf("Certificate %s deleted", safeRef), map[string]any{ - "Ref": certRef, + "Ref": safeRef, "Action": "deleted", }, ) } + +// sanitizeCertRef re-validates a certificate reference (UUID or numeric ID) +// and returns a safe string representation. Returns a placeholder if invalid. +func sanitizeCertRef(ref string) string { + if parsed, err := uuid.Parse(ref); err == nil { + return parsed.String() + } + if n, err := strconv.ParseUint(ref, 10, 64); err == nil { + return strconv.FormatUint(n, 10) + } + return "[invalid-ref]" +} diff --git a/backend/internal/api/handlers/certificate_handler_coverage_test.go b/backend/internal/api/handlers/certificate_handler_coverage_test.go index 146fd158..30131600 100644 --- a/backend/internal/api/handlers/certificate_handler_coverage_test.go +++ b/backend/internal/api/handlers/certificate_handler_coverage_test.go @@ -36,7 +36,7 @@ func TestCertificateHandler_Delete_InvalidID(t *testing.T) { r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db, nil) h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/invalid", http.NoBody) w := httptest.NewRecorder() @@ -52,7 +52,7 @@ func TestCertificateHandler_Delete_NotFound(t *testing.T) { r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db, nil) h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/9999", http.NoBody) w := httptest.NewRecorder() @@ -74,7 +74,7 @@ func TestCertificateHandler_Delete_NoBackupService(t *testing.T) { // No backup service h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -97,7 +97,7 @@ func TestCertificateHandler_Delete_CheckUsageDBError(t *testing.T) { r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db, nil) h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -137,7 +137,7 @@ func TestCertificateHandler_Delete_ZeroID(t *testing.T) { r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db, nil) h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/0", http.NoBody) w := httptest.NewRecorder() diff --git a/backend/internal/api/handlers/certificate_handler_security_test.go b/backend/internal/api/handlers/certificate_handler_security_test.go index 08a062ad..95f77f69 100644 --- a/backend/internal/api/handlers/certificate_handler_security_test.go +++ b/backend/internal/api/handlers/certificate_handler_security_test.go @@ -32,7 +32,7 @@ func TestCertificateHandler_Delete_RequiresAuth(t *testing.T) { }) svc := services.NewCertificateService("/tmp", db, nil) h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/1", http.NoBody) w := httptest.NewRecorder() @@ -135,7 +135,7 @@ func TestCertificateHandler_Delete_DiskSpaceCheck(t *testing.T) { } h := NewCertificateHandler(svc, mockBackup, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/api/certificates/%d", cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -186,7 +186,7 @@ func TestCertificateHandler_Delete_NotificationRateLimiting(t *testing.T) { } h := NewCertificateHandler(svc, mockBackup, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) // Delete first cert req1 := httptest.NewRequest(http.MethodDelete, fmt.Sprintf("/api/certificates/%d", cert1.ID), http.NoBody) diff --git a/backend/internal/api/handlers/certificate_handler_test.go b/backend/internal/api/handlers/certificate_handler_test.go index f8b23797..fc45f0ef 100644 --- a/backend/internal/api/handlers/certificate_handler_test.go +++ b/backend/internal/api/handlers/certificate_handler_test.go @@ -41,7 +41,7 @@ func setupCertTestRouter(t *testing.T, db *gorm.DB) *gin.Engine { svc := services.NewCertificateService("/tmp", db, nil) h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) return r } @@ -123,7 +123,7 @@ func TestDeleteCertificate_CreatesBackup(t *testing.T) { } h := NewCertificateHandler(svc, mockBackupService, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -174,7 +174,7 @@ func TestDeleteCertificate_BackupFailure(t *testing.T) { } h := NewCertificateHandler(svc, mockBackupService, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -229,7 +229,7 @@ func TestDeleteCertificate_InUse_NoBackup(t *testing.T) { } h := NewCertificateHandler(svc, mockBackupService, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -557,7 +557,7 @@ func TestDeleteCertificate_InvalidID(t *testing.T) { r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db, nil) h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/invalid", http.NoBody) w := httptest.NewRecorder() @@ -582,7 +582,7 @@ func TestDeleteCertificate_ZeroID(t *testing.T) { r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db, nil) h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/0", http.NoBody) w := httptest.NewRecorder() @@ -621,7 +621,7 @@ func TestDeleteCertificate_LowDiskSpace(t *testing.T) { } h := NewCertificateHandler(svc, mockBackupService, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -672,7 +672,7 @@ func TestDeleteCertificate_DiskSpaceCheckError(t *testing.T) { } h := NewCertificateHandler(svc, mockBackupService, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -726,7 +726,7 @@ func TestDeleteCertificate_ExpiredLetsEncrypt_NotInUse(t *testing.T) { } h := NewCertificateHandler(svc, mockBS, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -784,7 +784,7 @@ func TestDeleteCertificate_ValidLetsEncrypt_NotInUse(t *testing.T) { } h := NewCertificateHandler(svc, mockBS, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -822,7 +822,7 @@ func TestDeleteCertificate_UsageCheckError(t *testing.T) { r.Use(mockAuthMiddleware()) svc := services.NewCertificateService("/tmp", db, nil) h := NewCertificateHandler(svc, nil, nil) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert.ID), http.NoBody) w := httptest.NewRecorder() @@ -867,7 +867,7 @@ func TestDeleteCertificate_NotificationRateLimit(t *testing.T) { } h := NewCertificateHandler(svc, mockBackupService, ns) - r.DELETE("/api/certificates/:id", h.Delete) + r.DELETE("/api/certificates/:uuid", h.Delete) // Delete first certificate req := httptest.NewRequest(http.MethodDelete, "/api/certificates/"+toStr(cert1.ID), http.NoBody)