From b9a88c433092baa4ecdc3b4f8651ca950c2c872f Mon Sep 17 00:00:00 2001 From: fuomag9 <1580624+fuomag9@users.noreply.github.com> Date: Fri, 3 Apr 2026 12:34:18 +0200 Subject: [PATCH] fix: remove ACME cert scanning to eliminate caddy-data permission issue (#88) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Caddy's certmagic creates storage dirs with hardcoded 0700 permissions, making the web container's supplementary group membership ineffective. Rather than working around this with ACLs or chmod hacks, remove the feature entirely — it was cosmetic (issuer/expiry display) for certs that Caddy auto-manages anyway. Also bump access list dropdown timeout from 5s to 10s to fix flaky E2E test. Co-Authored-By: Claude Opus 4.6 (1M context) --- .env.example | 4 - README.md | 1 - .../certificates/CertificatesClient.tsx | 1 - .../certificates/components/AcmeTab.tsx | 16 +--- app/(dashboard)/certificates/page.tsx | 33 ++----- docker-compose.yml | 1 - src/lib/acme-certs.ts | 95 ------------------- tests/helpers/proxy-api.ts | 2 +- 8 files changed, 9 insertions(+), 144 deletions(-) delete mode 100644 src/lib/acme-certs.ts diff --git a/.env.example b/.env.example index 5a60e8dd..98381560 100644 --- a/.env.example +++ b/.env.example @@ -101,10 +101,6 @@ OAUTH_ALLOW_AUTO_LINKING=false # Auto-link OAuth to accounts without pas # Certificate storage directory (usually no need to change) # CERTS_DIRECTORY=./data/certs -# Caddy certificate directory for ACME metadata scanning in the Certificates page -# (Only needed for custom/non-standard deployments) -# CADDY_CERTS_DIR=/caddy-data/caddy/certificates - # Login rate limiting (optional, for custom rate limit settings) # LOGIN_MAX_ATTEMPTS=5 # LOGIN_WINDOW_MS=300000 diff --git a/README.md b/README.md index c07cf606..3aab0c2c 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,6 @@ Data persists in Docker volumes (caddy-manager-data, caddy-data, caddy-config, c | `CADDY_API_URL` | Caddy Admin API endpoint | `http://caddy:2019` (prod)
`http://localhost:2019` (dev) | No | | `DATABASE_URL` | SQLite database URL | `file:/app/data/caddy-proxy-manager.db` | No | | `CERTS_DIRECTORY` | Certificate storage directory | `./data/certs` | No | -| `CADDY_CERTS_DIR` | Caddy cert storage path used for ACME metadata scanning (non-default deployments) | `/caddy-data/caddy/certificates` | No | | `LOGIN_MAX_ATTEMPTS` | Max login attempts before rate limit | `5` | No | | `LOGIN_WINDOW_MS` | Rate limit window in milliseconds | `300000` (5 min) | No | | `LOGIN_BLOCK_MS` | Rate limit block duration in milliseconds | `900000` (15 min) | No | diff --git a/app/(dashboard)/certificates/CertificatesClient.tsx b/app/(dashboard)/certificates/CertificatesClient.tsx index a78185e8..1c88950b 100644 --- a/app/(dashboard)/certificates/CertificatesClient.tsx +++ b/app/(dashboard)/certificates/CertificatesClient.tsx @@ -44,7 +44,6 @@ export default function CertificatesClient({ const [statusFilter, setStatusFilter] = useState(null); const allStatuses: (CertExpiryStatus | null)[] = [ - ...acmeHosts.map((h) => h.certExpiryStatus), ...importedCerts.map((c) => c.expiryStatus), ]; const { expired, expiringSoon, healthy } = countExpiry(allStatuses); diff --git a/app/(dashboard)/certificates/components/AcmeTab.tsx b/app/(dashboard)/certificates/components/AcmeTab.tsx index 0d86f09e..1b6606ac 100644 --- a/app/(dashboard)/certificates/components/AcmeTab.tsx +++ b/app/(dashboard)/certificates/components/AcmeTab.tsx @@ -5,7 +5,6 @@ import { Card, CardContent } from "@/components/ui/card"; import { DataTable } from "@/components/ui/DataTable"; import { StatusChip } from "@/components/ui/StatusChip"; import type { AcmeHost } from "../page"; -import { RelativeTime } from "./RelativeTime"; type Props = { acmeHosts: AcmeHost[]; @@ -40,18 +39,6 @@ const columns = [ ), }, - { - id: "issuer", - label: "Issuer", - render: (r: AcmeHost) => ( - {r.certIssuer ?? "—"} - ), - }, - { - id: "expiry", - label: "Expiry", - render: (r: AcmeHost) => , - }, { id: "status", label: "Status", @@ -71,7 +58,6 @@ function acmeMobileCard(r: AcmeHost) { {r.domains[0]}{r.domains.length > 1 ? ` +${r.domains.length - 1}` : ""}

-
@@ -81,7 +67,7 @@ function acmeMobileCard(r: AcmeHost) { export function AcmeTab({ acmeHosts, acmePagination, search, statusFilter }: Props) { const filtered = acmeHosts.filter((h) => { - if (statusFilter && h.certExpiryStatus !== statusFilter) return false; + if (statusFilter) return false; // ACME hosts have no expiry status if (search) { const q = search.toLowerCase(); return ( diff --git a/app/(dashboard)/certificates/page.tsx b/app/(dashboard)/certificates/page.tsx index b7a892a7..1d20c2cf 100644 --- a/app/(dashboard)/certificates/page.tsx +++ b/app/(dashboard)/certificates/page.tsx @@ -4,7 +4,6 @@ import { proxyHosts, certificates } from '@/src/lib/db/schema'; import { isNull, isNotNull, count } from 'drizzle-orm'; import { requireAdmin } from '@/src/lib/auth'; import CertificatesClient from './CertificatesClient'; -import { scanAcmeCerts } from '@/src/lib/acme-certs'; import { listCaCertificates, type CaCertificate } from '@/src/lib/models/ca-certificates'; import { listIssuedClientCertificates, type IssuedClientCertificate } from '@/src/lib/models/issued-client-certificates'; @@ -23,10 +22,6 @@ export type AcmeHost = { domains: string[]; ssl_forced: boolean; enabled: boolean; - certValidTo: string | null; - certValidFrom: string | null; - certIssuer: string | null; - certExpiryStatus: CertExpiryStatus | null; }; export type ImportedCertView = { @@ -86,8 +81,6 @@ export default async function CertificatesPage({ searchParams }: PageProps) { const { page: pageParam } = await searchParams; const page = Math.max(1, parseInt(pageParam ?? "1", 10) || 1); const offset = (page - 1) * PER_PAGE; - const acmeCertMap = scanAcmeCerts(); - const [caCerts, issuedClientCerts] = await Promise.all([ listCaCertificates(), listIssuedClientCertificates() @@ -124,25 +117,13 @@ export default async function CertificatesPage({ searchParams }: PageProps) { .where(isNotNull(proxyHosts.certificateId)), ]); - const acmeHosts: AcmeHost[] = acmeRows.map(r => { - const domains = JSON.parse(r.domains) as string[]; - let certInfo = null; - for (const domain of domains) { - const info = acmeCertMap.get(domain.toLowerCase()); - if (info) { certInfo = info; break; } - } - return { - id: r.id, - name: r.name, - domains, - ssl_forced: r.sslForced, - enabled: r.enabled, - certValidTo: certInfo?.validTo ?? null, - certValidFrom: certInfo?.validFrom ?? null, - certIssuer: certInfo?.issuer ?? null, - certExpiryStatus: certInfo?.validTo ? getExpiryStatus(certInfo.validTo) : null, - }; - }); + const acmeHosts: AcmeHost[] = acmeRows.map(r => ({ + id: r.id, + name: r.name, + domains: JSON.parse(r.domains) as string[], + ssl_forced: r.sslForced, + enabled: r.enabled, + })); const usageMap = new Map(); for (const u of usageRows) { diff --git a/docker-compose.yml b/docker-compose.yml index d8bcc4f8..ff0b14b9 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -58,7 +58,6 @@ services: - caddy-manager-data:/app/data - geoip-data:/usr/share/GeoIP:ro,z - caddy-logs:/logs:ro - - caddy-data:/caddy-data:ro depends_on: caddy: condition: service_healthy diff --git a/src/lib/acme-certs.ts b/src/lib/acme-certs.ts deleted file mode 100644 index 670566af..00000000 --- a/src/lib/acme-certs.ts +++ /dev/null @@ -1,95 +0,0 @@ -import { readdirSync, readFileSync, statSync } from 'node:fs'; -import { join } from 'node:path'; -import { X509Certificate } from 'node:crypto'; - -export type AcmeCertInfo = { - validTo: string; - validFrom: string; - issuer: string; - domains: string[]; -}; - -/** - * Walks Caddy's certificate storage directory and parses every .crt file. - * Returns a map from lowercase domain → cert info (most recent cert wins for - * a given domain if multiple exist). - * - * Caddy stores certs under: - * /acme-v02.api.letsencrypt.org-directory//.crt - * /acme.zerossl.com-v2-DV90//.crt - * ...etc - * - * The directory is mounted at /caddy-data in the web container, so: - * CADDY_CERTS_DIR defaults to /caddy-data/caddy/certificates - */ -const CADDY_CERTS_DIR = - process.env.CADDY_CERTS_DIR ?? '/caddy-data/caddy/certificates'; - -function walkCrtFiles(dir: string): string[] { - const results: string[] = []; - let entries: string[]; - try { - entries = readdirSync(dir); - } catch { - return results; // directory doesn't exist yet (e.g. no certs issued) - } - for (const entry of entries) { - const full = join(dir, entry); - try { - const stat = statSync(full); - if (stat.isDirectory()) { - results.push(...walkCrtFiles(full)); - } else if (entry.endsWith('.crt')) { - results.push(full); - } - } catch { - // skip unreadable entries - } - } - return results; -} - -export function scanAcmeCerts(): Map { - const map = new Map(); - const crtFiles = walkCrtFiles(CADDY_CERTS_DIR); - - for (const file of crtFiles) { - try { - const pem = readFileSync(file, 'utf-8'); - const cert = new X509Certificate(pem); - - const sanDomains = - cert.subjectAltName - ?.split(',') - .map(s => s.trim()) - .filter(s => s.startsWith('DNS:')) - .map(s => s.slice(4).toLowerCase()) ?? []; - - const issuerLine = cert.issuer ?? ''; - const issuer = ( - issuerLine.match(/O=([^\n,]+)/)?.[1] ?? - issuerLine.match(/CN=([^\n,]+)/)?.[1] ?? - issuerLine - ).trim(); - - const info: AcmeCertInfo = { - validTo: new Date(cert.validTo).toISOString(), - validFrom: new Date(cert.validFrom).toISOString(), - issuer, - domains: sanDomains, - }; - - for (const domain of sanDomains) { - // Keep the cert with the latest validTo for each domain - const existing = map.get(domain); - if (!existing || new Date(info.validTo).getTime() > new Date(existing.validTo).getTime()) { - map.set(domain, info); - } - } - } catch { - // skip unreadable / malformed certs - } - } - - return map; -} diff --git a/tests/helpers/proxy-api.ts b/tests/helpers/proxy-api.ts index 53caf3c6..c8d98999 100644 --- a/tests/helpers/proxy-api.ts +++ b/tests/helpers/proxy-api.ts @@ -90,7 +90,7 @@ export async function createProxyHost(page: Page, config: ProxyHostConfig): Prom await accessListTrigger.scrollIntoViewIfNeeded(); await accessListTrigger.click(); const option = page.getByRole('option', { name: config.accessListName }); - await expect(option).toBeVisible({ timeout: 5_000 }); + await expect(option).toBeVisible({ timeout: 10_000 }); await option.click(); }