From 0e773d02ef6b94053c433334806d7d24f270862a Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Fri, 16 Oct 2026 01:51:19 -0700 Subject: [PATCH] fix(ingress): accept dash-leading challenges; redact secrets in certbot logs - acme-dns-alias-hook.sh: the ACME validation value is base64url and starts with "-" about once in 64; argparse then treats it as an option and the delegated auth hook fails. Pass it as --content=. - certman.py: certbot output is printed on failure. lexicon's Namecheap client sends ApiKey in the query string, so a network error can put the key in the container log. Mask DNS provider credentials before printing. Signed-off-by: Kevin Wang --- .../scripts/acme-dns-alias-hook.sh | 2 +- .../dstack-ingress/scripts/certman.py | 28 ++++++++++++++----- .../scripts/tests/test_certman.py | 7 +++++ 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/custom-domain/dstack-ingress/scripts/acme-dns-alias-hook.sh b/custom-domain/dstack-ingress/scripts/acme-dns-alias-hook.sh index e73659a8..f3b0f45d 100755 --- a/custom-domain/dstack-ingress/scripts/acme-dns-alias-hook.sh +++ b/custom-domain/dstack-ingress/scripts/acme-dns-alias-hook.sh @@ -37,7 +37,7 @@ case "$action" in echo "acme-dns-alias-hook: writing challenge TXT to delegated record $record" # A stale challenge value that cannot be deleted is harmless: the CA # accepts the record set as long as one value matches. - dnsman.py set_txt --tolerate-stale --domain "$record" --content "$CERTBOT_VALIDATION" + dnsman.py set_txt --tolerate-stale --domain "$record" --content="$CERTBOT_VALIDATION" # certbot asks Let's Encrypt to validate immediately after this hook # returns, so wait for the record to propagate before returning. # diff --git a/custom-domain/dstack-ingress/scripts/certman.py b/custom-domain/dstack-ingress/scripts/certman.py index 3b5165dd..a459b9fd 100644 --- a/custom-domain/dstack-ingress/scripts/certman.py +++ b/custom-domain/dstack-ingress/scripts/certman.py @@ -22,6 +22,20 @@ LETSENCRYPT_DIR = "/etc/letsencrypt" +# DNS credentials certbot plugins may echo back, e.g. lexicon's Namecheap +# client puts ApiKey in the request URL that ends up in error messages. +SECRET_ENV_VARS = ("CLOUDFLARE_API_TOKEN", "LINODE_API_TOKEN", "NAMECHEAP_API_KEY") + + +def redact(text: str) -> str: + """Mask DNS provider credentials in certbot output before it is logged.""" + for name in SECRET_ENV_VARS: + secret = os.environ.get(name, "").strip() + if secret: + text = text.replace(secret, "***") + return text + + def renewal_due(cert: x509.Certificate, now: datetime) -> bool: """certbot's renewal thresholds, for certificates it does not manage. @@ -374,9 +388,9 @@ def obtain_certificate(self, domain: str, email: str, replace: bool = False) -> print(f"Rate limited by Let's Encrypt") if error_output: - print(f"stderr: {error_output}") + print(f"stderr: {redact(error_output)}") if stdout_output: - print(f"stdout: {stdout_output}") + print(f"stdout: {redact(stdout_output)}") return False @@ -385,7 +399,7 @@ def obtain_certificate(self, domain: str, email: str, replace: bool = False) -> file=sys.stderr) return False except Exception as e: - print(f"Error running certbot: {e}", file=sys.stderr) + print(f"Error running certbot: {redact(str(e))}", file=sys.stderr) return False finally: if key is not None: @@ -491,9 +505,9 @@ def renew_certificate(self, domain: str, email: str = "") -> Tuple[bool, bool]: print(f"DNS validation failed during renewal") if error_output: - print(f"stderr: {error_output}") + print(f"stderr: {redact(error_output)}") if stdout_output: - print(f"stdout: {stdout_output}") + print(f"stdout: {redact(stdout_output)}") return False, False @@ -506,7 +520,7 @@ def renew_certificate(self, domain: str, email: str = "") -> Tuple[bool, bool]: ) return False, False except Exception as e: - print(f"Error running certbot: {e}", file=sys.stderr) + print(f"Error running certbot: {redact(str(e))}", file=sys.stderr) return False, False @classmethod @@ -720,7 +734,7 @@ def register_account(self, email: str) -> bool: print("ACME account registration timed out", file=sys.stderr) return False if result.returncode != 0 or not self.acme_account_exists(): - print(f"ACME account registration failed: {result.stderr.strip()}", file=sys.stderr) + print(f"ACME account registration failed: {redact(result.stderr.strip())}", file=sys.stderr) return False print(f"✓ ACME account registered with {acme_server()}") return True diff --git a/custom-domain/dstack-ingress/scripts/tests/test_certman.py b/custom-domain/dstack-ingress/scripts/tests/test_certman.py index e46c627f..e2725c11 100644 --- a/custom-domain/dstack-ingress/scripts/tests/test_certman.py +++ b/custom-domain/dstack-ingress/scripts/tests/test_certman.py @@ -465,6 +465,13 @@ def test_no_provider_ends_up_below_the_previous_fixed_cap(self): ) +class RedactTest(unittest.TestCase): + @mock.patch.dict(os.environ, {"NAMECHEAP_API_KEY": "0123abcd", "LINODE_API_TOKEN": ""}) + def test_masks_configured_secrets(self): + out = certman.redact("url: /xml.response?ApiUser=u&ApiKey=0123abcd&Command=x") + self.assertEqual(out, "url: /xml.response?ApiUser=u&ApiKey=***&Command=x") + + class NoDuplicateDefinitionsTest(unittest.TestCase): """Python shadows a repeated class or method silently; unittest counts the later one and the total still goes up, so a duplicated block looks like