From f6d2ae377d23136a353b426107a5ea58dfb1f71e Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Tue, 9 Dec 2025 15:37:20 -0800 Subject: [PATCH] Fix --webroot-path action (#10509) Fixes #10506. When --webroot-path was specified multiple times, Certbot was erroring with `DNSName SAN compared to non-SAN`. That's because, in the _WebrootPathAction that builds `namespace.webroot_path`, we were passing `domain` (type `san.DNSName`) as the keys. The other code that modifies or accesses `namespace.webroot_path` expects the keys to be of type `str`. In particular `webroot.Authenticator._set_webroots` does: ```python for achall in achalls: self.conf("map").setdefault(achall.domain, webroot_path) ``` Where `achall.domain` is a `str`. Two existing unittests would have caught this: `test_multiwebroot` and `test_webroot_map_partial_without_perform`. However, they faked out the parsing of the `--domains` flag, and that faked out code was not updated in #10468. Since this bug is caused by an interaction between the types produced by the `--domains` flag and those produced by the `--webroot-path` flag, the tests failed to catch the problem. I've updated the tests and confirmed that they fail before the fix is applied. --- certbot/src/certbot/_internal/plugins/webroot.py | 2 +- certbot/src/certbot/_internal/tests/plugins/webroot_test.py | 4 ++-- newsfragments/10509.fixed | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 newsfragments/10509.fixed diff --git a/certbot/src/certbot/_internal/plugins/webroot.py b/certbot/src/certbot/_internal/plugins/webroot.py index e77a72383..5a3d3d8fd 100644 --- a/certbot/src/certbot/_internal/plugins/webroot.py +++ b/certbot/src/certbot/_internal/plugins/webroot.py @@ -323,7 +323,7 @@ class _WebrootPathAction(argparse.Action): # domains before setting the new webroot path prev_webroot = namespace.webroot_path[-1] for domain in namespace.domains: - namespace.webroot_map.setdefault(domain, prev_webroot) + namespace.webroot_map.setdefault(domain.dns_name, prev_webroot) elif namespace.domains: self._domain_before_webroot = True diff --git a/certbot/src/certbot/_internal/tests/plugins/webroot_test.py b/certbot/src/certbot/_internal/tests/plugins/webroot_test.py index b8d1eab5e..06c620a08 100644 --- a/certbot/src/certbot/_internal/tests/plugins/webroot_test.py +++ b/certbot/src/certbot/_internal/tests/plugins/webroot_test.py @@ -20,6 +20,7 @@ from certbot import errors from certbot.compat import filesystem from certbot.compat import os from certbot.display import util as display_util +from certbot._internal import cli from certbot.tests import acme_util from certbot.tests import util as test_util @@ -308,7 +309,6 @@ class AuthenticatorTest(unittest.TestCase): assert not os.path.exists(self.validation_path) assert os.path.exists(self.root_challenge_path) - class WebrootActionTest(unittest.TestCase): """Tests for webroot argparse actions.""" @@ -322,7 +322,7 @@ class WebrootActionTest(unittest.TestCase): self.path = tempfile.mkdtemp() self.parser = argparse.ArgumentParser() self.parser.add_argument("-d", "--domains", - action="append", default=[]) + action=cli._DomainsAction, default=[]) Authenticator.inject_parser_options(self.parser, "webroot") def test_webroot_map_action(self): diff --git a/newsfragments/10509.fixed b/newsfragments/10509.fixed new file mode 100644 index 000000000..4842fe710 --- /dev/null +++ b/newsfragments/10509.fixed @@ -0,0 +1 @@ +Fixed a regression that caused certbot to crash if multiple --webroot-path values were set on the command line.