Files
Charon/docs/plans/ssl_card_pending_fix.md
GitHub Actions 9ad3afbd22 Fix Rate Limiting Issues
- Updated Definition of Done report with detailed checks and results for backend and frontend tests.
- Documented issues related to race conditions and test failures in QA reports.
- Improved security scan notes and code cleanup status in QA reports.
- Added summaries for rate limit integration test fixes, including root causes and resolutions.
- Introduced new debug and integration scripts for rate limit testing.
- Updated security documentation to reflect changes in configuration and troubleshooting steps.
- Enhanced troubleshooting guides for CrowdSec and Go language server (gopls) errors.
- Improved frontend and scripts README files for clarity and usage instructions.
2025-12-12 19:21:44 +00:00

19 KiB

Bug Fix: SSL Card Shows "Waiting for 14 Certs" When All Certificates Are Loaded

Status: Ready for Implementation Created: December 12, 2025 Priority: High (User-facing display bug)


Issue Summary

The SSL card on the Dashboard incorrectly shows "waiting for 14 certs" (or similar) when all SSL certificates have actually been provisioned and are available. This is a logic bug in the "pending" certificate detection.


Root Cause Analysis

The Current Logic

In frontend/src/components/CertificateStatusCard.tsx:

// Pending = hosts with ssl_forced and enabled but no certificate_id
const sslHosts = hosts.filter(h => h.ssl_forced && h.enabled)
const hostsWithCerts = sslHosts.filter(h => h.certificate_id != null)
const pendingCount = sslHosts.length - hostsWithCerts.length

Why This Logic Is Wrong

The fundamental misunderstanding: The code assumes certificate_id on a ProxyHost indicates whether that host has a valid SSL certificate. This is incorrect for ACME (Let's Encrypt/ZeroSSL) managed certificates.

How Caddy/Charon actually works:

  1. ACME certificates (Let's Encrypt, ZeroSSL): Caddy automatically provisions and manages these certificates. The certificate_id field on ProxyHost is NOT used for ACME certs - it's only for custom uploaded certificates.

  2. Custom certificates: Only when a user uploads a custom certificate is it stored in ssl_certificates table and linked via certificate_id on the proxy host.

  3. Backend evidence - From backend/internal/api/routes/routes.go:

    // Let's Encrypt certs are auto-managed by Caddy and should not be assigned via certificate_id
    
  4. Certificate service - From backend/internal/services/certificate_service.go: The certificate service scans data/certificates/ directory for .crt files and syncs them to the database. These certificates are discovered by domain name, not by any host relationship.

The Actual Bug

When a user has:

  • 14 proxy hosts with ssl_forced: true
  • All 14 hosts have ACME-managed certificates (Let's Encrypt/ZeroSSL)
  • None of these hosts have certificate_id set (because ACME certs don't use this field)

The current logic sees 14 SSL hosts with no certificate_id → reports "14 hosts awaiting certificate"

Data Flow Diagram

ProxyHost                              SSLCertificate (DB)
┌─────────────────────┐                ┌──────────────────────┐
│ uuid: "abc123"      │                │ domain: "example.com"│
│ domain_names:       │   NO LINK      │ provider: "letsencrypt"
│   "example.com"     │ ══════════════ │ status: "valid"      │
│ ssl_forced: true    │  (for ACME)    │ expires_at: ...      │
│ certificate_id: null│                └──────────────────────┘
└─────────────────────┘
          ↓
   Current logic sees certificate_id: null
          ↓
   Incorrectly reports as "pending"

Correct Solution

New Logic: Match by Domain Name

Instead of checking certificate_id, we should check if a certificate exists for the host's domain(s):

// A host is "pending" if:
// 1. ssl_forced is true AND enabled is true
// 2. AND there's no certificate with a matching domain

// Build a set of all domains that have certificates
const certifiedDomains = new Set<string>()
certificates.forEach(cert => {
  // Certificate domains can be comma-separated
  cert.domain.split(',').forEach(d => {
    certifiedDomains.add(d.trim().toLowerCase())
  })
})

// Check each SSL host
const sslHosts = hosts.filter(h => h.ssl_forced && h.enabled)
const pendingHosts = sslHosts.filter(host => {
  // Check if ANY of the host's domains have a certificate
  const hostDomains = host.domain_names.split(',').map(d => d.trim().toLowerCase())
  return !hostDomains.some(domain => certifiedDomains.has(domain))
})

const pendingCount = pendingHosts.length

Files to Modify

Frontend Changes

File Change
frontend/src/components/CertificateStatusCard.tsx Update pending detection logic to match by domain
frontend/src/components/tests/CertificateStatusCard.test.tsx Update tests for new domain-matching logic

No Backend Changes Required

The backend already provides:

  • ProxyHost.domain_names - comma-separated list of domains
  • Certificate.domain - the domain(s) covered by the certificate

Implementation Details

Updated CertificateStatusCard.tsx

import { useMemo } from 'react'
import { Link } from 'react-router-dom'
import { Loader2 } from 'lucide-react'
import type { Certificate } from '../api/certificates'
import type { ProxyHost } from '../api/proxyHosts'

interface CertificateStatusCardProps {
  certificates: Certificate[]
  hosts: ProxyHost[]
}

export default function CertificateStatusCard({ certificates, hosts }: CertificateStatusCardProps) {
  const validCount = certificates.filter(c => c.status === 'valid').length
  const expiringCount = certificates.filter(c => c.status === 'expiring').length
  const untrustedCount = certificates.filter(c => c.status === 'untrusted').length

  // Build a set of all domains that have certificates (case-insensitive)
  const certifiedDomains = useMemo(() => {
    const domains = new Set<string>()
    certificates.forEach(cert => {
      // Certificate domain field can be comma-separated
      cert.domain.split(',').forEach(d => {
        const trimmed = d.trim().toLowerCase()
        if (trimmed) domains.add(trimmed)
      })
    })
    return domains
  }, [certificates])

  // Calculate pending hosts: SSL-enabled hosts without any domain covered by a certificate
  const { pendingCount, totalSSLHosts, hostsWithCerts } = useMemo(() => {
    const sslHosts = hosts.filter(h => h.ssl_forced && h.enabled)

    let withCerts = 0
    sslHosts.forEach(host => {
      // Check if any of the host's domains have a certificate
      const hostDomains = host.domain_names.split(',').map(d => d.trim().toLowerCase())
      if (hostDomains.some(domain => certifiedDomains.has(domain))) {
        withCerts++
      }
    })

    return {
      pendingCount: sslHosts.length - withCerts,
      totalSSLHosts: sslHosts.length,
      hostsWithCerts: withCerts,
    }
  }, [hosts, certifiedDomains])

  const hasProvisioning = pendingCount > 0
  const progressPercent = totalSSLHosts > 0
    ? Math.round((hostsWithCerts / totalSSLHosts) * 100)
    : 100

  return (
    <Link
      to="/certificates"
      className="bg-dark-card p-6 rounded-lg border border-gray-800 hover:border-gray-700 transition-colors block"
    >
      <div className="text-sm text-gray-400 mb-2">SSL Certificates</div>
      <div className="text-3xl font-bold text-white mb-1">{certificates.length}</div>

      {/* Status breakdown */}
      <div className="flex flex-wrap gap-x-3 gap-y-1 mt-2 text-xs">
        <span className="text-green-400">{validCount} valid</span>
        {expiringCount > 0 && <span className="text-yellow-400">{expiringCount} expiring</span>}
        {untrustedCount > 0 && <span className="text-orange-400">{untrustedCount} staging</span>}
      </div>

      {/* Pending indicator */}
      {hasProvisioning && (
        <div className="mt-3 pt-3 border-t border-gray-700">
          <div className="flex items-center gap-2 text-blue-400 text-xs">
            <Loader2 className="h-3 w-3 animate-spin" />
            <span>{pendingCount} host{pendingCount !== 1 ? 's' : ''} awaiting certificate</span>
          </div>
          <div className="mt-2 h-1.5 bg-gray-700 rounded-full overflow-hidden">
            <div
              className="h-full bg-blue-500 transition-all duration-500 rounded-full"
              style={{ width: `${progressPercent}%` }}
            />
          </div>
          <div className="text-xs text-gray-500 mt-1">{progressPercent}% provisioned</div>
        </div>
      )}
    </Link>
  )
}

Updated Test Cases

Key Test Changes

The existing tests rely on certificate_id for determining "pending" status. We need to update these to use domain-based matching.

File: frontend/src/components/__tests__/CertificateStatusCard.test.tsx

Tests to Keep (Unchanged)

  • shows total certificate count
  • shows valid certificate count
  • shows expiring count when certificates are expiring
  • hides expiring count when no certificates are expiring
  • shows staging count for untrusted certificates
  • hides staging count when no untrusted certificates
  • links to certificates page
  • handles empty certificates array
  • shows spinning loader icon when pending

Tests to Update

Remove tests that check certificate_id directly:

  • shows pending indicator when hosts lack certificates - needs domain matching
  • shows plural for multiple pending hosts - needs domain matching
  • hides pending indicator when all hosts have certificates - needs domain matching
  • ignores disabled hosts when calculating pending - update to use domain
  • ignores hosts without SSL when calculating pending - update to use domain
  • calculates progress percentage correctly - update to use domain

Add new domain-based tests:

const mockCertWithDomain = (domain: string, status: 'valid' | 'expiring' | 'expired' | 'untrusted' = 'valid'): Certificate => ({
  id: Math.random(),
  name: domain,
  domain: domain,
  issuer: "Let's Encrypt",
  expires_at: new Date(Date.now() + 90 * 24 * 60 * 60 * 1000).toISOString(),
  status,
  provider: 'letsencrypt',
})

describe('CertificateStatusCard - Domain Matching', () => {
  it('does not show pending when host domain matches certificate domain', () => {
    const certs: Certificate[] = [mockCertWithDomain('example.com')]
    const hosts: ProxyHost[] = [
      { ...mockHost, domain_names: 'example.com', ssl_forced: true, certificate_id: null, enabled: true }
    ]
    renderWithRouter(<CertificateStatusCard certificates={certs} hosts={hosts} />)

    // Should NOT show "awaiting certificate" since domain matches
    expect(screen.queryByText(/awaiting certificate/)).not.toBeInTheDocument()
  })

  it('shows pending when host domain has no matching certificate', () => {
    const certs: Certificate[] = [mockCertWithDomain('other.com')]
    const hosts: ProxyHost[] = [
      { ...mockHost, domain_names: 'example.com', ssl_forced: true, certificate_id: null, enabled: true }
    ]
    renderWithRouter(<CertificateStatusCard certificates={certs} hosts={hosts} />)

    expect(screen.getByText('1 host awaiting certificate')).toBeInTheDocument()
  })

  it('handles case-insensitive domain matching', () => {
    const certs: Certificate[] = [mockCertWithDomain('EXAMPLE.COM')]
    const hosts: ProxyHost[] = [
      { ...mockHost, domain_names: 'example.com', ssl_forced: true, certificate_id: null, enabled: true }
    ]
    renderWithRouter(<CertificateStatusCard certificates={certs} hosts={hosts} />)

    expect(screen.queryByText(/awaiting certificate/)).not.toBeInTheDocument()
  })

  it('handles multi-domain hosts with partial certificate coverage', () => {
    // Host has two domains, but only one has a certificate - should be "covered"
    const certs: Certificate[] = [mockCertWithDomain('example.com')]
    const hosts: ProxyHost[] = [
      { ...mockHost, domain_names: 'example.com, www.example.com', ssl_forced: true, certificate_id: null, enabled: true }
    ]
    renderWithRouter(<CertificateStatusCard certificates={certs} hosts={hosts} />)

    // Host should be considered "covered" if any domain has a cert
    expect(screen.queryByText(/awaiting certificate/)).not.toBeInTheDocument()
  })

  it('handles comma-separated certificate domains', () => {
    const certs: Certificate[] = [{
      ...mockCertWithDomain('example.com'),
      domain: 'example.com, www.example.com'
    }]
    const hosts: ProxyHost[] = [
      { ...mockHost, domain_names: 'www.example.com', ssl_forced: true, certificate_id: null, enabled: true }
    ]
    renderWithRouter(<CertificateStatusCard certificates={certs} hosts={hosts} />)

    expect(screen.queryByText(/awaiting certificate/)).not.toBeInTheDocument()
  })

  it('ignores disabled hosts even without certificate', () => {
    const certs: Certificate[] = []
    const hosts: ProxyHost[] = [
      { ...mockHost, domain_names: 'example.com', ssl_forced: true, certificate_id: null, enabled: false }
    ]
    renderWithRouter(<CertificateStatusCard certificates={certs} hosts={hosts} />)

    expect(screen.queryByText(/awaiting certificate/)).not.toBeInTheDocument()
  })

  it('ignores hosts without SSL forced', () => {
    const certs: Certificate[] = []
    const hosts: ProxyHost[] = [
      { ...mockHost, domain_names: 'example.com', ssl_forced: false, certificate_id: null, enabled: true }
    ]
    renderWithRouter(<CertificateStatusCard certificates={certs} hosts={hosts} />)

    expect(screen.queryByText(/awaiting certificate/)).not.toBeInTheDocument()
  })

  it('calculates progress percentage with domain matching', () => {
    const certs: Certificate[] = [
      mockCertWithDomain('a.example.com'),
      mockCertWithDomain('b.example.com'),
    ]
    const hosts: ProxyHost[] = [
      { ...mockHost, uuid: 'h1', domain_names: 'a.example.com', ssl_forced: true, certificate_id: null, enabled: true },
      { ...mockHost, uuid: 'h2', domain_names: 'b.example.com', ssl_forced: true, certificate_id: null, enabled: true },
      { ...mockHost, uuid: 'h3', domain_names: 'c.example.com', ssl_forced: true, certificate_id: null, enabled: true },
      { ...mockHost, uuid: 'h4', domain_names: 'd.example.com', ssl_forced: true, certificate_id: null, enabled: true },
    ]
    renderWithRouter(<CertificateStatusCard certificates={certs} hosts={hosts} />)

    // 2 out of 4 hosts have matching certs = 50%
    expect(screen.getByText('50% provisioned')).toBeInTheDocument()
    expect(screen.getByText('2 hosts awaiting certificate')).toBeInTheDocument()
  })

  it('shows all pending when no certificates exist', () => {
    const certs: Certificate[] = []
    const hosts: ProxyHost[] = [
      { ...mockHost, uuid: 'h1', domain_names: 'a.example.com', ssl_forced: true, certificate_id: null, enabled: true },
      { ...mockHost, uuid: 'h2', domain_names: 'b.example.com', ssl_forced: true, certificate_id: null, enabled: true },
    ]
    renderWithRouter(<CertificateStatusCard certificates={certs} hosts={hosts} />)

    expect(screen.getByText('2 hosts awaiting certificate')).toBeInTheDocument()
    expect(screen.getByText('0% provisioned')).toBeInTheDocument()
  })
})

Edge Cases to Handle

Scenario Expected Behavior
Multiple domains per host Covered if any domain has a certificate
Case sensitivity Case-insensitive matching (Example.COM = example.com)
Whitespace in domain list Trim whitespace from both sides
Custom certificates Hosts with certificate_id set should still be considered "covered"
Disabled hosts Ignored (not counted as pending)
Non-SSL hosts (ssl_forced: false) Ignored (not counted as pending)
Empty domain list Ignored
Wildcard certs Currently not specially handled (future enhancement)

Testing Plan

Unit Tests (Automated)

  1. Host with matching certificate domain → not pending
  2. Host with no matching certificate → pending
  3. Host with multiple domains, one matched → not pending
  4. Case-insensitive domain matching
  5. Disabled hosts ignored
  6. Non-SSL hosts ignored
  7. Progress percentage calculation with domain matching
  8. Empty certificates list → all SSL hosts pending
  9. Empty hosts list → no pending indicator

Manual Testing

  1. Import a Caddyfile with multiple hosts
  2. Wait for ACME certificates to be provisioned (1-5 minutes)
  3. Verify dashboard SSL card:
    • Shows correct number of certificates
    • Shows "X valid" count
    • Does NOT show "waiting for X certs" when all domains have certs
  4. Verify the pending indicator appears only when:
    • A new host is added and certificate hasn't been provisioned yet
    • Truly no certificate exists for that domain

Dashboard Polling Behavior

The Dashboard already has conditional polling logic:

// Dashboard.tsx
const hasPendingCerts = useMemo(() => {
  const sslHosts = hosts.filter(h => h.ssl_forced && h.enabled)
  return sslHosts.some(h => !h.certificate_id)  // <-- This also needs updating
}, [hosts])

const { certificates } = useCertificates({
  refetchInterval: hasPendingCerts ? 15000 : false,
})

Also update Dashboard.tsx to use domain-based checking for the polling trigger:

const hasPendingCerts = useMemo(() => {
  // Build set of certified domains
  const certifiedDomains = new Set<string>()
  certificates.forEach(cert => {
    cert.domain.split(',').forEach(d => {
      certifiedDomains.add(d.trim().toLowerCase())
    })
  })

  // Check if any SSL host lacks a certificate
  const sslHosts = hosts.filter(h => h.ssl_forced && h.enabled)
  return sslHosts.some(host => {
    const hostDomains = host.domain_names.split(',').map(d => d.trim().toLowerCase())
    return !hostDomains.some(domain => certifiedDomains.has(domain))
  })
}, [hosts, certificates])

Rollback Plan

If issues arise, the fallback approach keeps backward compatibility:

// Fallback: host is "pending" only if:
// 1. No certificate_id AND
// 2. No certificate with matching domain
const pendingHosts = sslHosts.filter(host => {
  if (host.certificate_id != null) return false  // Has custom cert
  const hostDomains = host.domain_names.split(',').map(d => d.trim().toLowerCase())
  return !hostDomains.some(domain => certifiedDomains.has(domain))
})

Acceptance Criteria

  • SSL card does not show "waiting for X certs" when all certificates are provisioned
  • SSL card correctly shows pending count only for hosts without any matching certificate
  • Domain matching is case-insensitive
  • Multi-domain hosts are handled correctly
  • Existing custom certificate workflow still works (hosts with certificate_id)
  • Dashboard polling stops correctly once all domains have certificates
  • All unit tests pass
  • No regression in certificate status display

Summary

Aspect Current (Bug) Fixed
Check method certificate_id != null Domain name matching
ACME certs Always "pending" Correctly detected as covered
Custom certs Works Still works
Multi-domain N/A Covered if any domain matches
Case sensitivity N/A Case-insensitive
Polling Never stops Stops when all covered

Root Cause: Using certificate_id (custom cert reference) instead of domain matching for ACME certificates.

Fix: Match host domains against certificate domains to determine coverage.

Effort: ~1 hour implementation + testing