From a906c26b5d864cd2a7678ab202f21d045f7e8b1f Mon Sep 17 00:00:00 2001 From: Dmitrii Iurco Date: Sat, 30 May 2026 15:01:15 -0400 Subject: [PATCH] fix: resolve Caddy env vars at write time to prevent parse errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit acme_ca and the pic_ngo DNS credentials ({$PIC_NGO_DDNS_TOKEN}, {$PIC_NGO_DDNS_API}) were written as Caddy env-var placeholders, but the Caddy container does not inherit the API container's environment, so the substitutions always failed — Caddy saw bare directive names with no arguments and rejected the Caddyfile. - _global_acme_block: only emit the acme_ca directive when ACME_CA_URL is actually set; omitting it makes Caddy default to Let's Encrypt production. - _caddyfile_pic_ngo: embed the DDNS_TOTP_SECRET and DDNS_URL values directly into the Caddyfile at write time rather than relying on Caddy env expansion. Co-Authored-By: Claude Sonnet 4.6 --- api/caddy_manager.py | 19 +++++++++++++++---- tests/test_caddy_manager.py | 33 +++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/api/caddy_manager.py b/api/caddy_manager.py index 839cea4..143918c 100644 --- a/api/caddy_manager.py +++ b/api/caddy_manager.py @@ -158,8 +158,12 @@ class CaddyManager(BaseServiceManager): lines.append(" admin 0.0.0.0:2019") if email: lines.append(f" email {email}") - # Always allow tests to override the ACME directory via env var. - lines.append(" acme_ca {$ACME_CA_URL}") + # Only write acme_ca when a URL is configured — an empty ACME_CA_URL + # causes Caddy to reject the Caddyfile with "wrong argument count". + # When absent, Caddy defaults to Let's Encrypt production. + acme_ca_url = os.environ.get('ACME_CA_URL', '').strip() + if acme_ca_url: + lines.append(f" acme_ca {acme_ca_url}") lines.append("}") return "\n".join(lines) @@ -272,14 +276,21 @@ class CaddyManager(BaseServiceManager): body.append(core_routes) inner = "\n".join(body) email = f"admin@{domain}" + + # Resolve credentials at write time — Caddy runs in its own container + # and does not inherit the API's environment variables, so we embed the + # actual values instead of {$VAR} placeholders. + ddns_token = (os.environ.get('DDNS_TOTP_SECRET') or '').strip() + ddns_api = (os.environ.get('DDNS_URL') or 'https://ddns.pic.ngo/api/v1').strip() + return ( f"{self._global_acme_block(email)}\n" "\n" f"*.{domain}, {domain} {{\n" " tls {\n" " dns pic_ngo {\n" - " token {$PIC_NGO_DDNS_TOKEN}\n" - " api_base_url {$PIC_NGO_DDNS_API}\n" + f" token {ddns_token}\n" + f" api_base_url {ddns_api}\n" " }\n" " }\n" f"{inner}\n" diff --git a/tests/test_caddy_manager.py b/tests/test_caddy_manager.py index a476859..a7e2d72 100644 --- a/tests/test_caddy_manager.py +++ b/tests/test_caddy_manager.py @@ -60,15 +60,35 @@ class TestGenerateCaddyfilePicNgo(unittest.TestCase): def test_pic_ngo_has_dns_plugin_and_wildcard(self): mgr = _mgr() identity = {'cell_name': 'alpha', 'domain_mode': 'pic_ngo'} - out = mgr.generate_caddyfile(identity, []) + import os + with unittest.mock.patch.dict(os.environ, { + 'DDNS_TOTP_SECRET': 'TESTSECRET123', + 'DDNS_URL': 'https://ddns.pic.ngo/api/v1', + }): + out = mgr.generate_caddyfile(identity, []) self.assertIn('dns pic_ngo', out) self.assertIn('*.alpha.pic.ngo', out) self.assertIn('alpha.pic.ngo', out) - self.assertIn('{$PIC_NGO_DDNS_TOKEN}', out) - self.assertIn('{$PIC_NGO_DDNS_API}', out) + # Credentials are resolved at write time and embedded — no {$VAR} placeholders + self.assertIn('token TESTSECRET123', out) + self.assertIn('api_base_url https://ddns.pic.ngo/api/v1', out) + self.assertNotIn('{$PIC_NGO_DDNS_TOKEN}', out) + self.assertNotIn('{$PIC_NGO_DDNS_API}', out) self.assertIn('email admin@alpha.pic.ngo', out) - # ACME staging hook - self.assertIn('acme_ca {$ACME_CA_URL}', out) + # acme_ca is omitted when ACME_CA_URL is not set + self.assertNotIn('acme_ca', out) + + def test_pic_ngo_acme_ca_included_when_env_set(self): + mgr = _mgr() + identity = {'cell_name': 'alpha', 'domain_mode': 'pic_ngo'} + import os + with unittest.mock.patch.dict(os.environ, { + 'DDNS_TOTP_SECRET': 'TESTSECRET123', + 'DDNS_URL': 'https://ddns.pic.ngo/api/v1', + 'ACME_CA_URL': 'https://acme-staging-v02.api.letsencrypt.org/directory', + }): + out = mgr.generate_caddyfile(identity, []) + self.assertIn('acme_ca https://acme-staging-v02.api.letsencrypt.org/directory', out) def test_pic_ngo_has_api_route_without_registry(self): mgr = _mgr() @@ -94,7 +114,8 @@ class TestGenerateCaddyfileCloudflare(unittest.TestCase): self.assertIn('dns cloudflare {$CF_API_TOKEN}', out) self.assertIn('*.example.com', out) self.assertIn('email {$ACME_EMAIL}', out) - self.assertIn('acme_ca {$ACME_CA_URL}', out) + # acme_ca is omitted when ACME_CA_URL is not set in the environment + self.assertNotIn('acme_ca', out) def test_caddyfile_cloudflare_uses_domain_name(self): """Caddyfile must use domain_name for TLS host, not any 'custom_domain' key."""