Przeglądaj źródła

fix: use SQLite upsert to eliminate race condition on domain uniqueness constraint

Previously the CreateCertificate handler checked domain existence and then
inserted separately, creating a race window where concurrent requests could
both pass the check and trigger 'UNIQUE constraint failed' on INSERT.

Now uses SQLite's native INSERT ... ON CONFLICT (upsert) which atomically
handles the uniqueness constraint at the database level, eliminating the
race condition entirely.
cnbugs 1 tydzień temu
rodzic
commit
d0e738e1ef
1 zmienionych plików z 32 dodań i 58 usunięć
  1. 32 58
      backend/handlers/cert.go

+ 32 - 58
backend/handlers/cert.go

@@ -75,67 +75,41 @@ func (h *CertHandler) CreateCertificate(c *gin.Context) {
 	// Trim spaces from domain
 	// Trim spaces from domain
 	req.Domain = strings.TrimSpace(req.Domain)
 	req.Domain = strings.TrimSpace(req.Domain)
 
 
-	// Check if domain already exists
-	var existing config.Certificate
-	if err := config.DB.Where("domain = ?", req.Domain).First(&existing).Error; err == nil {
-		// Domain exists, check if it's a failed/expired certificate that can be retried
-		if existing.Status == "error" || existing.Status == "expired" || existing.Status == "pending" {
-			// Update existing record and retry issuance
-			existing.Email = req.Email
-			existing.Provider = req.Provider
-			existing.ChallengeType = req.ChallengeType
-			existing.DNSProvider = req.DNSProvider
-			existing.DNSConfig = req.DNSConfig
-			existing.Status = "pending"
-			existing.ErrorMessage = ""
-			if req.AutoRenew != nil {
-				existing.AutoRenew = *req.AutoRenew
-			}
-			if req.RenewDays != nil {
-				existing.RenewDays = *req.RenewDays
-			}
-			if err := config.DB.Save(&existing).Error; err != nil {
-				c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
-				return
-			}
-			// Start issuance in background
-			go func() {
-				if err := services.GetACMECertificate(&existing, h.Cfg); err != nil {
-					existing.Status = "error"
-					existing.ErrorMessage = err.Error()
-				} else {
-					existing.Status = "active"
-				}
-				config.DB.Save(&existing)
-			}()
-			c.JSON(http.StatusAccepted, gin.H{"message": "certificate re-issuance started", "certificate": existing})
-			return
-		}
-		c.JSON(http.StatusConflict, gin.H{"error": "domain already exists with status: " + existing.Status})
+	// Use SQLite upsert (INSERT ... ON CONFLICT) to atomically handle domain uniqueness
+	// This avoids the race condition between checking existence and inserting
+	result := config.DB.Exec(`
+		INSERT INTO certificates (domain, email, provider, challenge_type, dns_provider, dns_config, status, auto_renew, renew_days, created_at, updated_at)
+		VALUES (?, ?, ?, ?, ?, ?, 'pending', ?, ?, datetime('now'), datetime('now'))
+		ON CONFLICT(domain) DO UPDATE SET
+			email = excluded.email,
+			provider = excluded.provider,
+			challenge_type = excluded.challenge_type,
+			dns_provider = excluded.dns_provider,
+			dns_config = excluded.dns_config,
+			status = CASE 
+				WHEN status IN ('error', 'expired', 'pending') THEN 'pending'
+				ELSE status
+			END,
+			error_message = CASE 
+				WHEN status IN ('error', 'expired', 'pending') THEN ''
+				ELSE error_message
+			END,
+			updated_at = datetime('now')
+		WHERE excluded.domain = certificates.domain
+	`, req.Domain, req.Email, req.Provider, req.ChallengeType, req.DNSProvider, req.DNSConfig, req.AutoRenew != nil && *req.AutoRenew, req.RenewDays)
+
+	if result.Error != nil {
+		c.JSON(http.StatusInternalServerError, gin.H{"error": result.Error.Error()})
 		return
 		return
 	}
 	}
 
 
-	cert := config.Certificate{
-		Domain:        req.Domain,
-		Email:         req.Email,
-		Provider:      req.Provider,
-		ChallengeType: req.ChallengeType,
-		DNSProvider:   req.DNSProvider,
-		DNSConfig:     req.DNSConfig,
-		Status:        "pending",
-		AutoRenew:     true,
-		RenewDays:     30,
-	}
-
-	if req.AutoRenew != nil {
-		cert.AutoRenew = *req.AutoRenew
-	}
-	if req.RenewDays != nil {
-		cert.RenewDays = *req.RenewDays
-	}
+	// Reload to get the current state
+	var cert config.Certificate
+	config.DB.Where("domain = ?", req.Domain).First(&cert)
 
 
-	if err := config.DB.Create(&cert).Error; err != nil {
-		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
+	// If domain already existed and was active, return conflict
+	if cert.Status != "pending" {
+		c.JSON(http.StatusConflict, gin.H{"error": "domain already exists with status: " + cert.Status})
 		return
 		return
 	}
 	}
 
 
@@ -150,7 +124,7 @@ func (h *CertHandler) CreateCertificate(c *gin.Context) {
 		config.DB.Save(&cert)
 		config.DB.Save(&cert)
 	}()
 	}()
 
 
-	c.JSON(http.StatusAccepted, cert)
+	c.JSON(http.StatusAccepted, gin.H{"message": "certificate issuance started", "certificate": cert})
 }
 }
 
 
 // RenewCertificate manually renews a certificate
 // RenewCertificate manually renews a certificate