fix: Auto-renew certificates by default, with honest renew reporting and proxy tuning

config.example.yml

@@ -38,7 +38,7 @@ certbot:
     webroot_path: ""
     container_webroot_path: ""
     dns_provider: ""
-    auto_renewal_enabled: false
+    auto_renewal_enabled: true
     renewal_threshold_days: 30
     renewal_check_interval: 12h0m0s
 logging:

internal/api/server.go

@@ -791,7 +791,10 @@ func (s *Server) Start() error {
 		}
 	}()
 
-	if s.config.Certbot.Enabled && s.config.Certbot.AutoRenewalEnabled {
+	// The renewer runs whenever certbot is enabled; whether each certificate renews
+	// is decided per certificate (with the global setting only as the default), so a
+	// certificate marked for auto-renew is never vetoed by the global flag.
+	if s.config.Certbot.Enabled {
 		s.certRenewer = ssl.NewRenewer(
 			s.proxyOrchestrator.SSLManager(),
 			s.config.Certbot.RenewalThresholdDays,
@@ -4726,15 +4729,21 @@ func (s *Server) getCertificate(c *gin.Context) {
 
 func (s *Server) renewCertificate(c *gin.Context) {
 	domain := c.Param("domain")
+	force := c.Query("force") == "true"
 
-	result, err := s.proxyOrchestrator.RenewCertificate(domain)
+	result, err := s.proxyOrchestrator.RenewCertificate(domain, force)
 	if err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
 	}
 
+	message := "Certificate renewed"
+	if !result.Renewed {
+		message = "Certificate is not yet due for renewal"
+	}
+
 	c.JSON(http.StatusOK, gin.H{
-		"message": "Certificate renewed",
+		"message": message,
 		"domain":  domain,
 		"result":  result,
 	})

internal/proxy/orchestrator.go

@@ -266,12 +266,12 @@ func (o *Orchestrator) RenewCertificates() (*ssl.RenewalResult, error) {
 	return result, nil
 }
 
-func (o *Orchestrator) RenewCertificate(domain string) (*ssl.RenewalResult, error) {
+func (o *Orchestrator) RenewCertificate(domain string, force bool) (*ssl.RenewalResult, error) {
 	if err := o.ssl.ValidateDomain(domain); err != nil {
 		return nil, err
 	}
 
-	result, err := o.ssl.RenewCertificate(domain)
+	result, err := o.ssl.RenewCertificate(domain, force)
 	if err != nil {
 		return nil, err
 	}
@@ -293,7 +293,7 @@ func (o *Orchestrator) RenewCertificatesForDomains(domains []string) *ssl.MultiC
 		if !o.ssl.CertificateExists(domain) {
 			continue
 		}
-		if _, err := o.ssl.RenewCertificate(domain); err != nil {
+		if _, err := o.ssl.RenewCertificate(domain, false); err != nil {
 			result.Results = append(result.Results, &ssl.CertificateResult{
 				Domain:  domain,
 				Success: false,

internal/ssl/autorenew.go

@@ -93,7 +93,7 @@ func (r *Renewer) Run() {
 		if !cert.AutoRenew {
 			continue
 		}
-		if _, err := r.manager.RenewCertificate(cert.Domain); err != nil {
+		if _, err := r.manager.RenewCertificate(cert.Domain, false); err != nil {
 			log.Printf("auto-renew: failed to renew %s (days_left=%d): %v", cert.Domain, cert.DaysLeft, err)
 			continue
 		}

internal/ssl/manager.go

@@ -176,28 +176,49 @@ func (m *Manager) RenewCertificates() (*RenewalResult, error) {
 	}, nil
 }
 
-func (m *Manager) RenewCertificate(domain string) (*RenewalResult, error) {
+func (m *Manager) RenewCertificate(domain string, force bool) (*RenewalResult, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
 
 	if !m.certificateExistsLocked(domain) {
 		return nil, fmt.Errorf("certificate for domain %q not found", domain)
 	}
 
-	output, err := m.executeCertbot([]string{
-		"renew",
-		"--non-interactive",
-		"--cert-name", domain,
-	})
+	args := []string{"renew", "--non-interactive", "--cert-name", domain}
+	if force {
+		args = append(args, "--force-renewal")
+	}
+	output, err := m.executeCertbot(args)
 	if err != nil {
 		return nil, fmt.Errorf("renewal failed for %s: %s - %w", domain, string(output), err)
 	}
 
-	return &RenewalResult{
-		Success:        true,
-		Message:        string(output),
-		RenewedDomains: []string{domain},
-	}, nil
+	// Without --force-renewal certbot skips a not-yet-due cert and still exits 0,
+	// so distinguish an actual reissue from a no-op instead of always claiming success.
+	renewed := force || certbotDidRenew(string(output))
+	result := &RenewalResult{
+		Success: true,
+		Renewed: renewed,
+		Message: string(output),
+	}
+	if renewed {
+		result.RenewedDomains = []string{domain}
+	}
+	return result, nil
+}
+
+// certbotDidRenew reports whether a `certbot renew` run actually reissued a cert,
+// by looking for the phrases certbot prints when it skips a not-yet-due lineage.
+func certbotDidRenew(output string) bool {
+	lower := strings.ToLower(output)
+	switch {
+	case strings.Contains(lower, "no renewals were attempted"),
+		strings.Contains(lower, "not yet due for renewal"),
+		strings.Contains(lower, "no certificates are due for renewal"):
+		return false
+	default:
+		return true
+	}
 }
 
 func (m *Manager) certificateExistsLocked(domain string) bool {
@@ -206,6 +227,14 @@ func (m *Manager) certificateExistsLocked(domain string) bool {
 	return err == nil
 }
 
+// Per-certificate auto-renew markers. An explicit marker overrides the global
+// default in either direction; a certificate with neither marker follows the
+// global default. Only one marker is ever present at a time.
+const (
+	autoRenewEnabledMarker  = ".flatrun-auto-renew-enabled"
+	autoRenewDisabledMarker = ".flatrun-auto-renew-disabled"
+)
+
 func (m *Manager) SetAutoRenew(domain string, enabled bool) error {
 	m.mu.Lock()
 	defer m.mu.Unlock()
@@ -214,25 +243,34 @@ func (m *Manager) SetAutoRenew(domain string, enabled bool) error {
 		return fmt.Errorf("certificate for domain %q not found", domain)
 	}
 
-	marker := filepath.Join(m.certsPath, domain, ".flatrun-auto-renew-disabled")
+	dir := filepath.Join(m.certsPath, domain)
+	write := filepath.Join(dir, autoRenewDisabledMarker)
+	remove := filepath.Join(dir, autoRenewEnabledMarker)
 	if enabled {
-		if err := os.Remove(marker); err != nil && !os.IsNotExist(err) {
-			return fmt.Errorf("failed to enable auto-renew: %w", err)
-		}
-		return nil
+		write, remove = remove, write
 	}
 
-	f, err := os.Create(marker)
+	if err := os.Remove(remove); err != nil && !os.IsNotExist(err) {
+		return fmt.Errorf("failed to set auto-renew: %w", err)
+	}
+	f, err := os.Create(write)
 	if err != nil {
-		return fmt.Errorf("failed to disable auto-renew: %w", err)
+		return fmt.Errorf("failed to set auto-renew: %w", err)
 	}
 	return f.Close()
 }
 
+// isAutoRenewEnabled reports whether a certificate should auto-renew. An explicit
+// per-certificate marker wins over the global default in both directions.
 func (m *Manager) isAutoRenewEnabled(domain string) bool {
-	marker := filepath.Join(m.certsPath, domain, ".flatrun-auto-renew-disabled")
-	_, err := os.Stat(marker)
-	return os.IsNotExist(err)
+	dir := filepath.Join(m.certsPath, domain)
+	if _, err := os.Stat(filepath.Join(dir, autoRenewDisabledMarker)); err == nil {
+		return false
+	}
+	if _, err := os.Stat(filepath.Join(dir, autoRenewEnabledMarker)); err == nil {
+		return true
+	}
+	return m.config.AutoRenewalEnabled == nil || *m.config.AutoRenewalEnabled
 }
 
 func (m *Manager) RevokeCertificate(domain string) error {
@@ -423,6 +461,7 @@ type CertificateResult struct {
 
 type RenewalResult struct {
 	Success        bool     `json:"success"`
+	Renewed        bool     `json:"renewed"`
 	Message        string   `json:"message"`
 	RenewedDomains []string `json:"renewed_domains,omitempty"`
 }

internal/ssl/manager_test.go

@@ -12,9 +12,10 @@ import (
 )
 
 type mockExecutor struct {
-	mu   sync.Mutex
+	mu    sync.Mutex
 	calls []executorCall
 	err   error
+	out   []byte
 }
 
 type executorCall struct {
@@ -29,6 +30,9 @@ func (e *mockExecutor) Execute(cfg *config.ServiceExecConfig, args []string) ([]
 	if e.err != nil {
 		return nil, e.err
 	}
+	if e.out != nil {
+		return e.out, nil
+	}
 	return []byte("ok"), nil
 }
 

internal/ssl/renewal_test.go

@@ -71,48 +71,64 @@ func newTestManager(t *testing.T) (*Manager, *mockExecutor, string) {
 	return m, mock, certsDir
 }
 
-func TestRenewCertificate_PassesCertName(t *testing.T) {
+func hasArg(args []string, want string) bool {
+	for _, a := range args {
+		if a == want {
+			return true
+		}
+	}
+	return false
+}
+
+func TestRenewCertificate_ForceAddsForceRenewal(t *testing.T) {
 	m, mock, certsDir := newTestManager(t)
 	writeTestCert(t, certsDir, "example.com", time.Now().Add(10*24*time.Hour))
 
-	result, err := m.RenewCertificate("example.com")
+	result, err := m.RenewCertificate("example.com", true)
 	if err != nil {
 		t.Fatalf("RenewCertificate: %v", err)
 	}
-	if !result.Success {
-		t.Error("expected Success=true")
+	if !result.Success || !result.Renewed {
+		t.Errorf("expected Success and Renewed true, got %+v", result)
 	}
 	if len(result.RenewedDomains) != 1 || result.RenewedDomains[0] != "example.com" {
 		t.Errorf("RenewedDomains = %v, want [example.com]", result.RenewedDomains)
 	}
 
-	if len(mock.calls) != 1 {
-		t.Fatalf("expected 1 executor call, got %d", len(mock.calls))
+	args := mock.calls[0].args
+	if !hasArg(args, "renew") || !hasArg(args, "--cert-name") || !hasArg(args, "--force-renewal") {
+		t.Errorf("expected force renewal args, got: %v", args)
 	}
+}
 
-	args := mock.calls[0].args
-	var hasRenew, hasNonInteractive, hasCertName bool
-	for i, arg := range args {
-		switch arg {
-		case "renew":
-			hasRenew = true
-		case "--non-interactive":
-			hasNonInteractive = true
-		case "--cert-name":
-			if i+1 < len(args) && args[i+1] == "example.com" {
-				hasCertName = true
-			}
-		}
+func TestRenewCertificate_NoForceSkipsForceRenewalAndReportsNotDue(t *testing.T) {
+	m, mock, certsDir := newTestManager(t)
+	mock.out = []byte("Cert not yet due for renewal\nNo renewals were attempted.")
+	writeTestCert(t, certsDir, "example.com", time.Now().Add(60*24*time.Hour))
+
+	result, err := m.RenewCertificate("example.com", false)
+	if err != nil {
+		t.Fatalf("RenewCertificate: %v", err)
+	}
+	if !result.Success {
+		t.Error("expected Success true")
+	}
+	if result.Renewed {
+		t.Error("expected Renewed false when certbot reports the cert is not yet due")
 	}
-	if !hasRenew || !hasNonInteractive || !hasCertName {
-		t.Errorf("unexpected certbot args: %v", args)
+	if len(result.RenewedDomains) != 0 {
+		t.Errorf("expected no RenewedDomains, got %v", result.RenewedDomains)
+	}
+
+	if hasArg(mock.calls[0].args, "--force-renewal") {
+		t.Errorf("did not expect --force-renewal without force, got: %v", mock.calls[0].args)
 	}
 }
 
 func TestRenewCertificate_ErrorsWhenMissing(t *testing.T) {
 	m, mock, _ := newTestManager(t)
 
-	_, err := m.RenewCertificate("missing.example.com")
+	_, err := m.RenewCertificate("missing.example.com", false)
 	if err == nil {
 		t.Fatal("expected error for missing certificate")
 	}
@@ -166,6 +182,35 @@ func TestSetAutoRenew_ErrorsWhenCertMissing(t *testing.T) {
 	}
 }
 
+func TestAutoRenew_PerCertOverridesGlobalDefault(t *testing.T) {
+	m, _, certsDir := newTestManager(t)
+	writeTestCert(t, certsDir, "untouched.example.com", time.Now().Add(10*24*time.Hour))
+	writeTestCert(t, certsDir, "forced-on.example.com", time.Now().Add(10*24*time.Hour))
+
+	off := false
+	m.config.AutoRenewalEnabled = &off
+
+	if m.isAutoRenewEnabled("untouched.example.com") {
+		t.Error("untouched certificate should follow the global default (off)")
+	}
+
+	if err := m.SetAutoRenew("forced-on.example.com", true); err != nil {
+		t.Fatalf("SetAutoRenew(true): %v", err)
+	}
+	if !m.isAutoRenewEnabled("forced-on.example.com") {
+		t.Error("explicitly enabled certificate must renew despite the global default being off")
+	}
+
+	on := true
+	m.config.AutoRenewalEnabled = &on
+	if err := m.SetAutoRenew("untouched.example.com", false); err != nil {
+		t.Fatalf("SetAutoRenew(false): %v", err)
+	}
+	if m.isAutoRenewEnabled("untouched.example.com") {
+		t.Error("explicitly disabled certificate must not renew despite the global default being on")
+	}
+}
+
 func TestGetExpiringCertificates_FiltersByThreshold(t *testing.T) {
 	m, _, certsDir := newTestManager(t)
 	writeTestCert(t, certsDir, "fresh.example.com", time.Now().Add(90*24*time.Hour))

pkg/config/config.go

@@ -103,7 +103,7 @@ type CertbotConfig struct {
 	WebrootPath          string        `yaml:"webroot_path" json:"webroot_path"`
 	ContainerWebrootPath string        `yaml:"container_webroot_path" json:"container_webroot_path"`
 	DNSProvider          string        `yaml:"dns_provider" json:"dns_provider"`
-	AutoRenewalEnabled   bool          `yaml:"auto_renewal_enabled" json:"auto_renewal_enabled"`
+	AutoRenewalEnabled   *bool         `yaml:"auto_renewal_enabled" json:"auto_renewal_enabled"`
 	RenewalThresholdDays int           `yaml:"renewal_threshold_days" json:"renewal_threshold_days"`
 	RenewalCheckInterval time.Duration `yaml:"renewal_check_interval" json:"renewal_check_interval"`
 }
@@ -345,6 +345,12 @@ func setDefaults(cfg *Config) {
 	if cfg.Certbot.RenewalCheckInterval == 0 {
 		cfg.Certbot.RenewalCheckInterval = 12 * time.Hour
 	}
+	// Auto-renewal defaults on: an unset flag should renew certificates before they
+	// expire, not leave them to lapse. An explicit false is still honored.
+	if cfg.Certbot.AutoRenewalEnabled == nil {
+		enabled := true
+		cfg.Certbot.AutoRenewalEnabled = &enabled
+	}
 	// Security defaults
 	if cfg.Security.ScanInterval == 0 {
 		cfg.Security.ScanInterval = 30 * time.Second

pkg/config/registry_test.go

@@ -152,6 +152,21 @@ func TestFilesShowHiddenDefaultsTrueAndAcceptsFalse(t *testing.T) {
 	}
 }
 
+func TestAutoRenewalDefaultsOnAndAcceptsFalse(t *testing.T) {
+	cfg := &Config{}
+	setDefaults(cfg)
+	if cfg.Certbot.AutoRenewalEnabled == nil || !*cfg.Certbot.AutoRenewalEnabled {
+		t.Errorf("auto_renewal_enabled default = %v, want true", cfg.Certbot.AutoRenewalEnabled)
+	}
+
+	off := false
+	cfg.Certbot.AutoRenewalEnabled = &off
+	setDefaults(cfg)
+	if cfg.Certbot.AutoRenewalEnabled == nil || *cfg.Certbot.AutoRenewalEnabled {
+		t.Errorf("explicit false reset to %v, want false", cfg.Certbot.AutoRenewalEnabled)
+	}
+}
+
 func entryKeys(entries []Entry) []string {
 	out := make([]string, 0, len(entries))
 	for _, e := range entries {