From 5943dc7c3d33101b25b16ffc7b2d8fdfe90de1d4 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 25 Mar 2016 20:37:12 -0700
Subject: [PATCH 01/16] Start implementing some renewal hook flags

Also some refactoring:
  - split renewal out of _auth_from_domains into renewal.renew_cert
  - split main._csr_obtain_cert out of main.obtain_cert
---
 letsencrypt/cli.py            |  18 ++++-
 letsencrypt/errors.py         |   4 ++
 letsencrypt/hooks.py          |  96 +++++++++++++++++++++++++
 letsencrypt/main.py           | 129 +++++++++++++++-------------------
 letsencrypt/renewal.py        |  47 +++++++++++++
 letsencrypt/tests/cli_test.py |   4 +-
 6 files changed, 222 insertions(+), 76 deletions(-)
 create mode 100644 letsencrypt/hooks.py

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 256e0c801..20fe60f23 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -692,7 +692,23 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         " used to create obtain or most recently successfully renew each"
         " certificate lineage. You can try it with `--dry-run` first. For"
         " more fine-grained control, you can renew individual lineages with"
-        " the `certonly` subcommand.")
+        " the `certonly` subcommand. Hooks are available to run commands "
+        " before and after renewal; see XXX for more information on these.")
+
+    helpful.add(
+        "renew", "--pre-hook",
+        help="Command to be run in a shell before obtaining any certificates. Intended"
+        " primarily for renewal, where it can be used to temporarily shut down a"
+        " webserver that might conflict with the standalone plugin. This will "
+        " only be called if a certificate is actually to be obtained/renewed. ")
+    helpful.add(
+        "renew", "--post-hook",
+        help="Command to be run in a shell after attempting to obtain/renew "
+        " certificates. Can be used to deploy renewed certificates, or to restart"
+        " any servers that were stopped by --pre-hook.")
+    helpful.add(
+        "renew", "--renew-hook",
+        help="Command to be run in a shell once for each renewed certificate")
 
     helpful.add_deprecated_argument("--agree-dev-preview", 0)
 
diff --git a/letsencrypt/errors.py b/letsencrypt/errors.py
index b2b078f6a..532a3a545 100644
--- a/letsencrypt/errors.py
+++ b/letsencrypt/errors.py
@@ -25,6 +25,10 @@ class CertStorageError(Error):
     """Generic `.CertStorage` error."""
 
 
+class HookCommandNotFound(Error):
+    """Failed to find a hook command in the PATH."""
+
+
 # Auth Handler Errors
 class AuthorizationError(Error):
     """Authorization error."""
diff --git a/letsencrypt/hooks.py b/letsencrypt/hooks.py
new file mode 100644
index 000000000..289dc0333
--- /dev/null
+++ b/letsencrypt/hooks.py
@@ -0,0 +1,96 @@
+"""Facilities for implementing hooks that call shell commands."""
+from __future__ import print_function
+
+import logging
+import os
+
+from subprocess import Popen, PIPE
+
+from letsencrypt import errors
+
+logger = logging.getLogger(__name__)
+
+def validate_hooks(config):
+    """Check hook commands are executable."""
+    _validate_hook(config.pre_hook)
+    _validate_hook(config.post_hook)
+    _validate_hook(config.renew_hook)
+
+def _prog(shell_cmd):
+    """Extract the program run by a shell command"""
+    cmd = _which(shell_cmd)
+    return os.path.basename(cmd) if cmd else None
+
+def _validate_hook(shell_cmd):
+    """Check that a command provided as a hook is plausibly executable.
+
+    :raises .errors.HookCommandNotFound: if the command is not found
+    """
+    cmd = shell_cmd.partition(" ")[0]
+    if not _prog(cmd):
+        path = os.environ["PATH"]
+        msg = "Unable to find command {0} in the PATH.\n(PATH is {1})".format(
+            cmd, path)
+        raise errors.HookCommandNotFound(msg)
+
+    return True
+
+def pre_hook(config):
+    "Run pre-hook if it's defined and hasn't been run."
+    if config.pre_hook and not pre_hook.already:
+        logger.info("Running pre-hook command: %s", config.pre_hook)
+        _run_hook(config.pre_hook)
+        pre_hook.already = True
+
+pre_hook.already = False
+
+def post_hook(config, final=False):
+    """Run post hook if defined.
+
+    If the verb is renew, we might have more certs to renew, so we wait until
+    we're called with final=True before actually doing anything.
+    """
+    if config.post_hook:
+        if final or config.verb != "renew":
+            logger.info("Running post-hook command: %s", config.post_hook)
+            _run_hook(config.post_hook)
+
+def renew_hook(config, domains, lineage_path):
+    "Run post-renewal hook if defined."
+    if config.renew_hook:
+        os.environ["RENEWED_DOMAINS"] = " ".join(domains)
+        os.environ["RENEWED_LINEAGE"] = lineage_path
+        _run_hook(config.renew_hook)
+
+def _run_hook(shell_cmd):
+    """Run a hook command.
+
+    :returns: stderr if there was any"""
+
+    cmd = Popen(shell_cmd, shell=True, stdout=PIPE, stderr=PIPE)
+    _out, err = cmd.communicate()
+    if cmd.returncode != 0:
+        logger.error('Hook command "%s" returned error code %d', shell_cmd, cmd.returncode)
+    if err:
+        logger.error('Error output from %s:\n%s', _prog(shell_cmd), err)
+
+def _which(program):
+    """Test if program is in the path."""
+    # Borrowed from:
+    # https://stackoverflow.com/questions/377017/test-if-executable-exists-in-python
+    # XXX May need more porting to handle .exe extensions on Windows
+    def _is_exe(fpath):
+        return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
+
+    fpath, _fname = os.path.split(program)
+    if fpath:
+        if _is_exe(program):
+            return program
+    else:
+        for path in os.environ["PATH"].split(os.pathsep):
+            path = path.strip('"')
+            exe_file = os.path.join(path, program)
+            if _is_exe(exe_file):
+                return exe_file
+
+    return None
diff --git a/letsencrypt/main.py b/letsencrypt/main.py
index 0afccc85e..0df26c5be 100644
--- a/letsencrypt/main.py
+++ b/letsencrypt/main.py
@@ -8,7 +8,6 @@ import sys
 import time
 import traceback
 
-import OpenSSL
 import zope.component
 
 from acme import jose
@@ -23,6 +22,7 @@ from letsencrypt import colored_logging
 from letsencrypt import configuration
 from letsencrypt import constants
 from letsencrypt import errors
+from letsencrypt import hooks
 from letsencrypt import interfaces
 from letsencrypt import le_util
 from letsencrypt import log
@@ -52,28 +52,6 @@ def _suggest_donation_if_appropriate(config, action):
     reporter_util.add_message(msg, reporter_util.LOW_PRIORITY)
 
 
-def _avoid_invalidating_lineage(config, lineage, original_server):
-    "Do not renew a valid cert with one from a staging server!"
-    def _is_staging(srv):
-        return srv == constants.STAGING_URI or "staging" in srv
-
-    # Some lineages may have begun with --staging, but then had production certs
-    # added to them
-    latest_cert = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM,
-                                                  open(lineage.cert).read())
-    # all our test certs are from happy hacker fake CA, though maybe one day
-    # we should test more methodically
-    now_valid = "fake" not in repr(latest_cert.get_issuer()).lower()
-
-    if _is_staging(config.server):
-        if not _is_staging(original_server) or now_valid:
-            if not config.break_my_certs:
-                names = ", ".join(lineage.names())
-                raise errors.Error(
-                    "You've asked to renew/replace a seemingly valid certificate with "
-                    "a test certificate (domains: {0}). We will not do that "
-                    "unless you use the --break-my-certs flag!".format(names))
-
 
 def _report_successful_dry_run(config):
     reporter_util = zope.component.getUtility(interfaces.IReporter)
@@ -82,6 +60,7 @@ def _report_successful_dry_run(config):
                                   reporter_util.HIGH_PRIORITY, on_crash=False)
 
 
+
 def _auth_from_domains(le_client, config, domains, lineage=None):
     """Authenticate and enroll certificate."""
     # Note: This can raise errors... caught above us though. This is now
@@ -105,31 +84,18 @@ def _auth_from_domains(le_client, config, domains, lineage=None):
         # The lineage already exists; allow the caller to try installing
         # it without getting a new certificate at all.
         return lineage, "reinstall"
-    elif action == "renew":
-        original_server = lineage.configuration["renewalparams"]["server"]
-        _avoid_invalidating_lineage(config, lineage, original_server)
-        # TODO: schoen wishes to reuse key - discussion
-        # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574
-        new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains)
-        # TODO: Check whether it worked! <- or make sure errors are thrown (jdk)
-        if config.dry_run:
-            logger.info("Dry run: skipping updating lineage at %s",
-                        os.path.dirname(lineage.cert))
-        else:
-            lineage.save_successor(
-                lineage.latest_common_version(), OpenSSL.crypto.dump_certificate(
-                    OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped),
-                new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain),
-                configuration.RenewerConfiguration(config.namespace))
-            lineage.update_all_links_to(lineage.latest_common_version())
-        # TODO: Check return value of save_successor
-        # TODO: Also update lineage renewal config with any relevant
-        #       configuration values from this attempt? <- Absolutely (jdkasten)
-    elif action == "newcert":
-        # TREAT AS NEW REQUEST
-        lineage = le_client.obtain_and_enroll_certificate(domains)
-        if lineage is False:
-            raise errors.Error("Certificate could not be obtained")
+
+    hooks.pre_hook(config)
+    try:
+        if action == "renew":
+            renewal.renew_cert(config, domains, le_client, lineage)
+        elif action == "newcert":
+            # TREAT AS NEW REQUEST
+            lineage = le_client.obtain_and_enroll_certificate(domains)
+            if lineage is False:
+                raise errors.Error("Certificate could not be obtained")
+    finally:
+        hooks.post_hook(config)
 
     if not config.dry_run and not config.verb == "renew":
         _report_new_cert(lineage.cert, lineage.fullchain)
@@ -142,7 +108,8 @@ def _handle_subset_cert_request(config, domains, cert):
 
     :param storage.RenewableCert cert:
 
-    :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal
+    :returns: Tuple of (stringa action, cert_or_None) as per _treat_as_renewal
+              action can be: "newcert" | "renew" | "reinstall"
     :rtype: tuple
 
     """
@@ -183,7 +150,8 @@ def _handle_identical_cert_request(config, cert):
 
     :param storage.RenewableCert cert:
 
-    :returns: Tuple of (string, cert_or_None) as per _treat_as_renewal
+    :returns: Tuple of (string action, cert_or_None) as per _treat_as_renewal
+              action can be: "newcert" | "renew" | "reinstall"
     :rtype: tuple
 
     """
@@ -507,41 +475,53 @@ def run(config, plugins):  # pylint: disable=too-many-branches,too-many-locals
     _suggest_donation_if_appropriate(config, action)
 
 
+def _csr_obtain_cert(config, le_client):
+    """Obtain a cert using a user-supplied CSR
+
+    This works differently in the CSR case (for now) because we don't
+    have the privkey, and therefore can't construct the files for a lineage.
+    So we just save the cert & chain to disk :/
+    """
+    csr, typ = config.actual_csr
+    certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ)
+    if config.dry_run:
+        logger.info(
+            "Dry run: skipping saving certificate to %s", config.cert_path)
+    else:
+        cert_path, _, cert_fullchain = le_client.save_certificate(
+            certr, chain, config.cert_path, config.chain_path, config.fullchain_path)
+        _report_new_cert(cert_path, cert_fullchain)
+
+
 def obtain_cert(config, plugins, lineage=None):
-    """Implements "certonly": authenticate & obtain cert, but do not install it."""
-    # pylint: disable=too-many-locals
+    """Authenticate & obtain cert, but do not install it.
+
+    This implements the 'certonly' subcommand, and is also called from within the
+    'renew' command."""
+
+    # SETUP: Select plugins and construct a client instance
     try:
         # installers are used in auth mode to determine domain names
-        installer, authenticator = plug_sel.choose_configurator_plugins(config, plugins, "certonly")
+        installer, auth = plug_sel.choose_configurator_plugins(config, plugins, "certonly")
     except errors.PluginSelectionError as e:
         logger.info("Could not choose appropriate plugin: %s", e)
         raise
+    le_client = _init_le_client(config, auth, installer)
 
-    # TODO: Handle errors from _init_le_client?
-    le_client = _init_le_client(config, authenticator, installer)
-
-    action = "newcert"
-    # This is a special case; cert and chain are simply saved
-    if config.csr is not None:
-        assert lineage is None, "Did not expect a CSR with a RenewableCert"
-        csr, typ = config.actual_csr
-        certr, chain = le_client.obtain_certificate_from_csr(config.domains, csr, typ)
-        if config.dry_run:
-            logger.info(
-                "Dry run: skipping saving certificate to %s", config.cert_path)
-        else:
-            cert_path, _, cert_fullchain = le_client.save_certificate(
-                certr, chain, config.cert_path, config.chain_path, config.fullchain_path)
-            _report_new_cert(cert_path, cert_fullchain)
-    else:
+    # SHOWTIME: Possibly obtain/renew a cert, and set action to renew | newcert | reinstall
+    if config.csr is None: # the common case
         domains = _find_domains(config, installer)
         _, action = _auth_from_domains(le_client, config, domains, lineage)
+    else:
+        assert lineage is None, "Did not expect a CSR with a RenewableCert"
+        _csr_obtain_cert(config, le_client)
+        action = "newcert"
 
+    # POSTPRODUCTION: Cleanup, deployment & reporting
     if config.dry_run:
         _report_successful_dry_run(config)
     elif config.verb == "renew":
         if installer is None:
-            # Tell the user that the server was not restarted.
             print("new certificate deployed without reload, fullchain is",
                   lineage.fullchain)
         else:
@@ -553,10 +533,13 @@ def obtain_cert(config, plugins, lineage=None):
                   config.installer, "server; fullchain is", lineage.fullchain)
     _suggest_donation_if_appropriate(config, action)
 
+
 def renew(config, unused_plugins):
     """Renew previously-obtained certificates."""
-    renewal.renew_all_lineages(config)
-
+    try:
+        renewal.renew_all_lineages(config)
+    finally:
+        hooks.post_hook(config, final=True)
 
 
 def setup_log_file_handler(config, logfile, fmt):
diff --git a/letsencrypt/renewal.py b/letsencrypt/renewal.py
index 27546bec9..7a9b18867 100644
--- a/letsencrypt/renewal.py
+++ b/letsencrypt/renewal.py
@@ -9,8 +9,13 @@ import traceback
 import six
 import zope.component
 
+import OpenSSL
+
 from letsencrypt import configuration
 from letsencrypt import cli
+from letsencrypt import constants
+
+from letsencrypt import crypto_util
 from letsencrypt import errors
 from letsencrypt import storage
 from letsencrypt.plugins import disco as plugins_disco
@@ -199,6 +204,48 @@ def should_renew(config, lineage):
     return False
 
 
+def _avoid_invalidating_lineage(config, lineage, original_server):
+    "Do not renew a valid cert with one from a staging server!"
+    def _is_staging(srv):
+        return srv == constants.STAGING_URI or "staging" in srv
+
+    # Some lineages may have begun with --staging, but then had production certs
+    # added to them
+    latest_cert = OpenSSL.crypto.load_certificate(
+        OpenSSL.crypto.FILETYPE_PEM, open(lineage.cert).read())
+    # all our test certs are from happy hacker fake CA, though maybe one day
+    # we should test more methodically
+    now_valid = "fake" not in repr(latest_cert.get_issuer()).lower()
+
+    if _is_staging(config.server):
+        if not _is_staging(original_server) or now_valid:
+            if not config.break_my_certs:
+                names = ", ".join(lineage.names())
+                raise errors.Error(
+                    "You've asked to renew/replace a seemingly valid certificate with "
+                    "a test certificate (domains: {0}). We will not do that "
+                    "unless you use the --break-my-certs flag!".format(names))
+
+
+def renew_cert(config, domains, le_client, lineage):
+    "Renew a certificate lineage."
+    original_server = lineage.configuration["renewalparams"]["server"]
+    _avoid_invalidating_lineage(config, lineage, original_server)
+    new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains)
+    if config.dry_run:
+        logger.info("Dry run: skipping updating lineage at %s",
+                    os.path.dirname(lineage.cert))
+    else:
+        prior_version = lineage.latest_common_version()
+        new_cert = OpenSSL.crypto.dump_certificate(
+            OpenSSL.crypto.FILETYPE_PEM, new_certr.body.wrapped)
+        new_chain = crypto_util.dump_pyopenssl_chain(new_chain)
+        renewal_conf = configuration.RenewerConfiguration(config.namespace)
+        lineage.save_successor(prior_version, new_cert, new_key.pem, new_chain, renewal_conf)
+        lineage.update_all_links_to(lineage.latest_common_version())
+    # TODO: Check return value of save_successor
+
+
 def _renew_describe_results(config, renew_successes, renew_failures,
                             renew_skipped, parse_failures):
     def _status(msgs, category):
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 04b5a2f3c..de8b0c7e8 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -579,11 +579,11 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                     mock_init.return_value = mock_client
                     get_utility_path = 'letsencrypt.main.zope.component.getUtility'
                     with mock.patch(get_utility_path) as mock_get_utility:
-                        with mock.patch('letsencrypt.main.OpenSSL') as mock_ssl:
+                        with mock.patch('letsencrypt.main.renewal.OpenSSL') as mock_ssl:
                             mock_latest = mock.MagicMock()
                             mock_latest.get_issuer.return_value = "Fake fake"
                             mock_ssl.crypto.load_certificate.return_value = mock_latest
-                            with mock.patch('letsencrypt.main.crypto_util'):
+                            with mock.patch('letsencrypt.main.renewal.crypto_util'):
                                 if not args:
                                     args = ['-d', 'isnot.org', '-a', 'standalone', 'certonly']
                                 if extra_args:

From ad5a08f5fc6e3cc6f4d9b73cc83e4e1e8292b071 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Mar 2016 17:58:53 -0700
Subject: [PATCH 02/16] Actually run the renew hook

---
 letsencrypt/hooks.py   | 2 +-
 letsencrypt/renewal.py | 1 +
 letsencrypt/storage.py | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/hooks.py b/letsencrypt/hooks.py
index 289dc0333..b7fd8b4e9 100644
--- a/letsencrypt/hooks.py
+++ b/letsencrypt/hooks.py
@@ -67,7 +67,7 @@ def _run_hook(shell_cmd):
 
     :returns: stderr if there was any"""
 
-    cmd = Popen(shell_cmd, shell=True, stdout=PIPE, stderr=PIPE)
+    cmd = Popen(shell_cmd, shell=True, stdout=PIPE, stderr=PIPE, stdin=PIPE)
     _out, err = cmd.communicate()
     if cmd.returncode != 0:
         logger.error('Hook command "%s" returned error code %d', shell_cmd, cmd.returncode)
diff --git a/letsencrypt/renewal.py b/letsencrypt/renewal.py
index 7a9b18867..b42b62caa 100644
--- a/letsencrypt/renewal.py
+++ b/letsencrypt/renewal.py
@@ -243,6 +243,7 @@ def renew_cert(config, domains, le_client, lineage):
         renewal_conf = configuration.RenewerConfiguration(config.namespace)
         lineage.save_successor(prior_version, new_cert, new_key.pem, new_chain, renewal_conf)
         lineage.update_all_links_to(lineage.latest_common_version())
+        hooks.renew_hook(config, domains, lineage.live_dir)
     # TODO: Check return value of save_successor
 
 
diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py
index 59daa1a0d..89f7d0f70 100644
--- a/letsencrypt/storage.py
+++ b/letsencrypt/storage.py
@@ -252,6 +252,7 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
         self.privkey = self.configuration["privkey"]
         self.chain = self.configuration["chain"]
         self.fullchain = self.configuration["fullchain"]
+        self.live_dir = os.path.dirname(self.cert)
 
         self._fix_symlinks()
         self._check_symlinks()

From 87aa1bd14012229050ec062ce38c9b81ec4ff4ff Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Mar 2016 18:15:50 -0700
Subject: [PATCH 03/16] lint

---
 letsencrypt/hooks.py   | 9 ++++++---
 letsencrypt/renewal.py | 1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/hooks.py b/letsencrypt/hooks.py
index b7fd8b4e9..f3561628c 100644
--- a/letsencrypt/hooks.py
+++ b/letsencrypt/hooks.py
@@ -58,9 +58,12 @@ def post_hook(config, final=False):
 def renew_hook(config, domains, lineage_path):
     "Run post-renewal hook if defined."
     if config.renew_hook:
-        os.environ["RENEWED_DOMAINS"] = " ".join(domains)
-        os.environ["RENEWED_LINEAGE"] = lineage_path
-        _run_hook(config.renew_hook)
+        if not config.dry_run:
+            os.environ["RENEWED_DOMAINS"] = " ".join(domains)
+            os.environ["RENEWED_LINEAGE"] = lineage_path
+            _run_hook(config.renew_hook)
+        else:
+            print("Dry run: skipping renewal hook command: {0}".format(config.renew_hook))
 
 def _run_hook(shell_cmd):
     """Run a hook command.
diff --git a/letsencrypt/renewal.py b/letsencrypt/renewal.py
index b42b62caa..3b0dd3ea0 100644
--- a/letsencrypt/renewal.py
+++ b/letsencrypt/renewal.py
@@ -17,6 +17,7 @@ from letsencrypt import constants
 
 from letsencrypt import crypto_util
 from letsencrypt import errors
+from letsencrypt import hooks
 from letsencrypt import storage
 from letsencrypt.plugins import disco as plugins_disco
 

From 3265660478e15c11b8eb3e82000707907d2a13db Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Mar 2016 18:33:57 -0700
Subject: [PATCH 04/16] Dry run testable

---
 letsencrypt/cli.py     | 5 ++++-
 letsencrypt/renewal.py | 3 ++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 20fe60f23..41c6ff186 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -708,7 +708,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         " any servers that were stopped by --pre-hook.")
     helpful.add(
         "renew", "--renew-hook",
-        help="Command to be run in a shell once for each renewed certificate")
+        help="Command to be run in a shell once for each renewed certificate."
+        "For this command, the shell variable $RENEWED_LINEAGE will point to the"
+        "config live subdirectory containing the new certs and keys; the shell variable "
+        "$RENEWED_DOMAINS will conatain a space-delimited list of renewed cert domains")
 
     helpful.add_deprecated_argument("--agree-dev-preview", 0)
 
diff --git a/letsencrypt/renewal.py b/letsencrypt/renewal.py
index 3b0dd3ea0..8a172098b 100644
--- a/letsencrypt/renewal.py
+++ b/letsencrypt/renewal.py
@@ -244,7 +244,8 @@ def renew_cert(config, domains, le_client, lineage):
         renewal_conf = configuration.RenewerConfiguration(config.namespace)
         lineage.save_successor(prior_version, new_cert, new_key.pem, new_chain, renewal_conf)
         lineage.update_all_links_to(lineage.latest_common_version())
-        hooks.renew_hook(config, domains, lineage.live_dir)
+
+    hooks.renew_hook(config, domains, lineage.live_dir)
     # TODO: Check return value of save_successor
 
 

From 8b8319355df4cf57760d93d8461270e6ab6fa53e Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Mar 2016 18:45:14 -0700
Subject: [PATCH 05/16] Actually validate hooks

---
 letsencrypt/cli.py   | 3 +++
 letsencrypt/hooks.py | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 41c6ff186..cc25e3d9e 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -18,6 +18,7 @@ import letsencrypt
 from letsencrypt import constants
 from letsencrypt import crypto_util
 from letsencrypt import errors
+from letsencrypt import hooks
 from letsencrypt import interfaces
 from letsencrypt import le_util
 
@@ -306,6 +307,8 @@ class HelpfulArgumentParser(object):
         if self.detect_defaults:  # plumbing
             parsed_args.store_false_vars = self.store_false_vars
 
+        hooks.validate_hooks(parsed_args)
+
         return parsed_args
 
     def handle_csr(self, parsed_args):
diff --git a/letsencrypt/hooks.py b/letsencrypt/hooks.py
index f3561628c..b734a89d3 100644
--- a/letsencrypt/hooks.py
+++ b/letsencrypt/hooks.py
@@ -27,7 +27,7 @@ def _validate_hook(shell_cmd):
     :raises .errors.HookCommandNotFound: if the command is not found
     """
     cmd = shell_cmd.partition(" ")[0]
-    if not _prog(cmd):
+    if shell_cmd and not _prog(cmd):
         path = os.environ["PATH"]
         msg = "Unable to find command {0} in the PATH.\n(PATH is {1})".format(
             cmd, path)

From ffefac466a3da6c1c3d1f94b1c98775fdbeb45b5 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 29 Mar 2016 18:46:51 -0700
Subject: [PATCH 06/16] Make hook errors more helpful

---
 letsencrypt/hooks.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/letsencrypt/hooks.py b/letsencrypt/hooks.py
index b734a89d3..4a3fabf37 100644
--- a/letsencrypt/hooks.py
+++ b/letsencrypt/hooks.py
@@ -12,16 +12,16 @@ logger = logging.getLogger(__name__)
 
 def validate_hooks(config):
     """Check hook commands are executable."""
-    _validate_hook(config.pre_hook)
-    _validate_hook(config.post_hook)
-    _validate_hook(config.renew_hook)
+    _validate_hook(config.pre_hook, "pre")
+    _validate_hook(config.post_hook, "post")
+    _validate_hook(config.renew_hook, "renew")
 
 def _prog(shell_cmd):
     """Extract the program run by a shell command"""
     cmd = _which(shell_cmd)
     return os.path.basename(cmd) if cmd else None
 
-def _validate_hook(shell_cmd):
+def _validate_hook(shell_cmd, hook_name):
     """Check that a command provided as a hook is plausibly executable.
 
     :raises .errors.HookCommandNotFound: if the command is not found
@@ -29,8 +29,8 @@ def _validate_hook(shell_cmd):
     cmd = shell_cmd.partition(" ")[0]
     if shell_cmd and not _prog(cmd):
         path = os.environ["PATH"]
-        msg = "Unable to find command {0} in the PATH.\n(PATH is {1})".format(
-            cmd, path)
+        msg = "Unable to find {2}-hook command {0} in the PATH.\n(PATH is {1})".format(
+            cmd, path, hook_name)
         raise errors.HookCommandNotFound(msg)
 
     return True

From c11af67f2a575f30a3d88949fb3c9572d7e33768 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Mar 2016 13:15:54 -0700
Subject: [PATCH 07/16] Some unit tests for hooks.py

---
 letsencrypt/hooks.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/hooks.py b/letsencrypt/hooks.py
index 4a3fabf37..f6450022f 100644
--- a/letsencrypt/hooks.py
+++ b/letsencrypt/hooks.py
@@ -77,13 +77,14 @@ def _run_hook(shell_cmd):
     if err:
         logger.error('Error output from %s:\n%s', _prog(shell_cmd), err)
 
+def _is_exe(fpath):
+    return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
+
 def _which(program):
     """Test if program is in the path."""
     # Borrowed from:
     # https://stackoverflow.com/questions/377017/test-if-executable-exists-in-python
     # XXX May need more porting to handle .exe extensions on Windows
-    def _is_exe(fpath):
-        return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
 
     fpath, _fname = os.path.split(program)
     if fpath:

From c98bdd6988ac50c995b37256b077fb8aacec357d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Mar 2016 13:33:55 -0700
Subject: [PATCH 08/16] Don't try to test empty hooks

---
 letsencrypt/hooks.py | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/letsencrypt/hooks.py b/letsencrypt/hooks.py
index f6450022f..ed491ab72 100644
--- a/letsencrypt/hooks.py
+++ b/letsencrypt/hooks.py
@@ -26,14 +26,13 @@ def _validate_hook(shell_cmd, hook_name):
 
     :raises .errors.HookCommandNotFound: if the command is not found
     """
-    cmd = shell_cmd.partition(" ")[0]
-    if shell_cmd and not _prog(cmd):
-        path = os.environ["PATH"]
-        msg = "Unable to find {2}-hook command {0} in the PATH.\n(PATH is {1})".format(
-            cmd, path, hook_name)
-        raise errors.HookCommandNotFound(msg)
-
-    return True
+    if shell_cmd:
+        cmd = shell_cmd.partition(" ")[0]
+        if not _prog(cmd):
+            path = os.environ["PATH"]
+            msg = "Unable to find {2}-hook command {0} in the PATH.\n(PATH is {1})".format(
+                cmd, path, hook_name)
+            raise errors.HookCommandNotFound(msg)
 
 def pre_hook(config):
     "Run pre-hook if it's defined and hasn't been run."

From 509600dec146fee99e2fa6a7f33de3cfefdb3b3e Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Mar 2016 13:56:10 -0700
Subject: [PATCH 09/16] Address review comments

---
 letsencrypt/cli.py  | 2 +-
 letsencrypt/main.py | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index cc25e3d9e..1091a804b 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -711,7 +711,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         " any servers that were stopped by --pre-hook.")
     helpful.add(
         "renew", "--renew-hook",
-        help="Command to be run in a shell once for each renewed certificate."
+        help="Command to be run in a shell once for each successfully renewed certificate."
         "For this command, the shell variable $RENEWED_LINEAGE will point to the"
         "config live subdirectory containing the new certs and keys; the shell variable "
         "$RENEWED_DOMAINS will conatain a space-delimited list of renewed cert domains")
diff --git a/letsencrypt/main.py b/letsencrypt/main.py
index 0df26c5be..d2962ba87 100644
--- a/letsencrypt/main.py
+++ b/letsencrypt/main.py
@@ -108,7 +108,7 @@ def _handle_subset_cert_request(config, domains, cert):
 
     :param storage.RenewableCert cert:
 
-    :returns: Tuple of (stringa action, cert_or_None) as per _treat_as_renewal
+    :returns: Tuple of (str action, cert_or_None) as per _treat_as_renewal
               action can be: "newcert" | "renew" | "reinstall"
     :rtype: tuple
 
@@ -150,7 +150,7 @@ def _handle_identical_cert_request(config, cert):
 
     :param storage.RenewableCert cert:
 
-    :returns: Tuple of (string action, cert_or_None) as per _treat_as_renewal
+    :returns: Tuple of (str action, cert_or_None) as per _treat_as_renewal
               action can be: "newcert" | "renew" | "reinstall"
     :rtype: tuple
 

From af1b13684659ddeaad9f95a91db32aa7a09d3137 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Mar 2016 13:58:26 -0700
Subject: [PATCH 10/16] Ooops, actually add hook_test.py to the repo!

---
 letsencrypt/tests/hook_test.py | 106 +++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 letsencrypt/tests/hook_test.py

diff --git a/letsencrypt/tests/hook_test.py b/letsencrypt/tests/hook_test.py
new file mode 100644
index 000000000..c491cd58b
--- /dev/null
+++ b/letsencrypt/tests/hook_test.py
@@ -0,0 +1,106 @@
+"""Tests for hooks.py"""
+
+import os
+import unittest
+
+import mock
+
+from letsencrypt import errors
+from letsencrypt import hooks
+
+from letsencrypt.tests import test_util
+
+class HookTest(unittest.TestCase):
+    def setUp(self):
+        pass
+
+    def tearDown(self):
+        pass
+
+    @mock.patch('letsencrypt.hooks._prog')
+    def test_validate_hooks(self, mock_prog):
+        config = mock.MagicMock(pre_hook="", post_hook="ls -lR", renew_hook="uptime")
+        hooks.validate_hooks(config)
+        self.assertEqual(mock_prog.call_count, 2)
+        self.assertEqual(mock_prog.call_args_list[1][0][0], 'uptime')
+        self.assertEqual(mock_prog.call_args_list[0][0][0], 'ls')
+        mock_prog.return_value = None
+        config = mock.MagicMock(pre_hook="explodinator", post_hook="", renew_hook="")
+        self.assertRaises(errors.HookCommandNotFound, hooks.validate_hooks, config)
+
+    @mock.patch('letsencrypt.hooks._is_exe')
+    def test_which(self, mock_is_exe):
+        mock_is_exe.return_value = True 
+        self.assertEqual(hooks._which("/path/to/something"), "/path/to/something")
+
+        with mock.patch.dict('os.environ', {"PATH": "/floop:/fleep"}):
+            mock_is_exe.return_value = True 
+            self.assertEqual(hooks._which("pingify"), "/floop/pingify")
+            mock_is_exe.return_value = False
+            self.assertEqual(hooks._which("pingify"), None)
+        self.assertEqual(hooks._which("/path/to/something"), None)
+
+    @mock.patch('letsencrypt.hooks._which')
+    def test_prog(self, mockwhich):
+        mockwhich.return_value = "/very/very/funky" 
+        self.assertEqual(hooks._prog("funky"), "funky")
+        mockwhich.return_value = None
+        self.assertEqual(hooks._prog("funky"), None)
+
+    def _test_a_hook(self, config, hook_function, calls_expected):
+        with mock.patch('letsencrypt.hooks.logger'):
+            with mock.patch('letsencrypt.hooks._run_hook') as mock_run_hook:
+                hook_function(config)
+                hook_function(config)
+                self.assertEqual(mock_run_hook.call_count, calls_expected)
+    
+    def test_pre_hook(self):
+        config = mock.MagicMock(pre_hook="true")
+        self._test_a_hook(config, hooks.pre_hook, 1)
+        config = mock.MagicMock(pre_hook="")
+        self._test_a_hook(config, hooks.pre_hook, 0)
+
+    def test_post_hook(self):
+        config = mock.MagicMock(post_hook="true", verb="splonk")
+        self._test_a_hook(config, hooks.post_hook, 2)
+        config = mock.MagicMock(post_hook="true", verb="renew")
+        self._test_a_hook(config, hooks.post_hook, 0)
+
+    def test_renew_hook(self):
+        with mock.patch.dict('os.environ', {}):
+            domains = ["a", "b"]
+            lineage = "thing"
+            rhook = lambda x: hooks.renew_hook(x, domains, lineage)
+
+            config = mock.MagicMock(renew_hook="true", dry_run=False)
+            self._test_a_hook(config, rhook, 2)
+            self.assertEqual(os.environ["RENEWED_DOMAINS"], "a b")
+            self.assertEqual(os.environ["RENEWED_LINEAGE"], "thing")
+
+            with mock.patch("letsencrypt.hooks.print") as mock_print:
+                config = mock.MagicMock(renew_hook="true", dry_run=True)
+                self._test_a_hook(config, rhook, 0)
+                self.assertEqual(mock_print.call_count, 2)
+
+    @mock.patch('letsencrypt.hooks.logger.error')
+    @mock.patch('letsencrypt.hooks.Popen')
+    def test_run_hook(self, mock_popen, mock_error):
+        
+        mock_cmd = mock.MagicMock()
+        mock_cmd.returncode = 1
+        mock_cmd.communicate.return_value = ("", "")
+        mock_popen.return_value = mock_cmd
+        hooks._run_hook("ls")
+        self.assertEqual(mock_error.call_count, 1)
+
+
+        
+
+        
+
+
+if __name__ == '__main__':
+    unittest.main()  # pragma: no cover
+
+
+        

From db5c4f9f91be7b5e18f40415dd334a619ae77c69 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Mar 2016 13:59:12 -0700
Subject: [PATCH 11/16] Fix whitespace

---
 letsencrypt/tests/hook_test.py | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/letsencrypt/tests/hook_test.py b/letsencrypt/tests/hook_test.py
index c491cd58b..671ea3fc8 100644
--- a/letsencrypt/tests/hook_test.py
+++ b/letsencrypt/tests/hook_test.py
@@ -85,18 +85,15 @@ class HookTest(unittest.TestCase):
     @mock.patch('letsencrypt.hooks.logger.error')
     @mock.patch('letsencrypt.hooks.Popen')
     def test_run_hook(self, mock_popen, mock_error):
-        
         mock_cmd = mock.MagicMock()
         mock_cmd.returncode = 1
         mock_cmd.communicate.return_value = ("", "")
         mock_popen.return_value = mock_cmd
         hooks._run_hook("ls")
         self.assertEqual(mock_error.call_count, 1)
-
-
-        
-
-        
+        mock_cmd.communicate.return_value = ("", "thing")
+        hooks._run_hook("ls")
+        self.assertEqual(mock_error.call_count, 2)
 
 
 if __name__ == '__main__':

From 285f3b5536bf2a2af6a42d09c74eaaf95faa3a32 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Mar 2016 15:38:38 -0700
Subject: [PATCH 12/16] lint

---
 letsencrypt/tests/hook_test.py    | 14 +++++---------
 letsencrypt/tests/storage_test.py |  1 +
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/letsencrypt/tests/hook_test.py b/letsencrypt/tests/hook_test.py
index 671ea3fc8..ffd97aa47 100644
--- a/letsencrypt/tests/hook_test.py
+++ b/letsencrypt/tests/hook_test.py
@@ -1,4 +1,5 @@
 """Tests for hooks.py"""
+# pylint: disable=protected-access
 
 import os
 import unittest
@@ -8,8 +9,6 @@ import mock
 from letsencrypt import errors
 from letsencrypt import hooks
 
-from letsencrypt.tests import test_util
-
 class HookTest(unittest.TestCase):
     def setUp(self):
         pass
@@ -30,11 +29,11 @@ class HookTest(unittest.TestCase):
 
     @mock.patch('letsencrypt.hooks._is_exe')
     def test_which(self, mock_is_exe):
-        mock_is_exe.return_value = True 
+        mock_is_exe.return_value = True
         self.assertEqual(hooks._which("/path/to/something"), "/path/to/something")
 
         with mock.patch.dict('os.environ', {"PATH": "/floop:/fleep"}):
-            mock_is_exe.return_value = True 
+            mock_is_exe.return_value = True
             self.assertEqual(hooks._which("pingify"), "/floop/pingify")
             mock_is_exe.return_value = False
             self.assertEqual(hooks._which("pingify"), None)
@@ -42,7 +41,7 @@ class HookTest(unittest.TestCase):
 
     @mock.patch('letsencrypt.hooks._which')
     def test_prog(self, mockwhich):
-        mockwhich.return_value = "/very/very/funky" 
+        mockwhich.return_value = "/very/very/funky"
         self.assertEqual(hooks._prog("funky"), "funky")
         mockwhich.return_value = None
         self.assertEqual(hooks._prog("funky"), None)
@@ -53,7 +52,7 @@ class HookTest(unittest.TestCase):
                 hook_function(config)
                 hook_function(config)
                 self.assertEqual(mock_run_hook.call_count, calls_expected)
-    
+
     def test_pre_hook(self):
         config = mock.MagicMock(pre_hook="true")
         self._test_a_hook(config, hooks.pre_hook, 1)
@@ -98,6 +97,3 @@ class HookTest(unittest.TestCase):
 
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover
-
-
-        
diff --git a/letsencrypt/tests/storage_test.py b/letsencrypt/tests/storage_test.py
index 49b4f0821..7e862146d 100644
--- a/letsencrypt/tests/storage_test.py
+++ b/letsencrypt/tests/storage_test.py
@@ -1,4 +1,5 @@
 """Tests for letsencrypt.storage."""
+# pylint disable=protected-access
 import datetime
 import os
 import shutil

From 526aa7d5ae783f8fa9743ea2b9359c1033ad20a3 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Mar 2016 17:41:59 -0700
Subject: [PATCH 13/16] Fix _run_hook test

---
 letsencrypt/tests/hook_test.py | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/letsencrypt/tests/hook_test.py b/letsencrypt/tests/hook_test.py
index ffd97aa47..45025b054 100644
--- a/letsencrypt/tests/hook_test.py
+++ b/letsencrypt/tests/hook_test.py
@@ -81,18 +81,19 @@ class HookTest(unittest.TestCase):
                 self._test_a_hook(config, rhook, 0)
                 self.assertEqual(mock_print.call_count, 2)
 
-    @mock.patch('letsencrypt.hooks.logger.error')
     @mock.patch('letsencrypt.hooks.Popen')
-    def test_run_hook(self, mock_popen, mock_error):
-        mock_cmd = mock.MagicMock()
-        mock_cmd.returncode = 1
-        mock_cmd.communicate.return_value = ("", "")
-        mock_popen.return_value = mock_cmd
-        hooks._run_hook("ls")
-        self.assertEqual(mock_error.call_count, 1)
-        mock_cmd.communicate.return_value = ("", "thing")
-        hooks._run_hook("ls")
-        self.assertEqual(mock_error.call_count, 2)
+    def test_run_hook(self, mock_popen):
+        with mock.patch('letsencrypt.hooks.logger.error') as mock_error:
+            mock_cmd = mock.MagicMock()
+            mock_cmd.returncode = 1
+            mock_cmd.communicate.return_value = ("", "")
+            mock_popen.return_value = mock_cmd
+            hooks._run_hook("ls")
+            self.assertEqual(mock_error.call_count, 1)
+        with mock.patch('letsencrypt.hooks.logger.error') as mock_error:
+            mock_cmd.communicate.return_value = ("", "thing")
+            hooks._run_hook("ls")
+            self.assertEqual(mock_error.call_count, 2)
 
 
 if __name__ == '__main__':

From 8f8d80e7d1f1a35b7db1c36b81c14eed907740a9 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 30 Mar 2016 17:50:22 -0700
Subject: [PATCH 14/16] Don't strip " from PATH entries

There shouldn't be any, but if they're there perhaps they're there for a
reason.
---
 letsencrypt/hooks.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/letsencrypt/hooks.py b/letsencrypt/hooks.py
index ed491ab72..6a0997708 100644
--- a/letsencrypt/hooks.py
+++ b/letsencrypt/hooks.py
@@ -91,7 +91,6 @@ def _which(program):
             return program
     else:
         for path in os.environ["PATH"].split(os.pathsep):
-            path = path.strip('"')
             exe_file = os.path.join(path, program)
             if _is_exe(exe_file):
                 return exe_file

From 37c070597cb83ad7f0fd378b5b786d856d707a43 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 31 Mar 2016 09:57:57 -0700
Subject: [PATCH 15/16] Fix tests on py26

the print() function does not appear to be mockable there
---
 letsencrypt/tests/hook_test.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/tests/hook_test.py b/letsencrypt/tests/hook_test.py
index 45025b054..6506eea2c 100644
--- a/letsencrypt/tests/hook_test.py
+++ b/letsencrypt/tests/hook_test.py
@@ -3,6 +3,7 @@
 
 import os
 import unittest
+import sys
 
 import mock
 
@@ -76,10 +77,14 @@ class HookTest(unittest.TestCase):
             self.assertEqual(os.environ["RENEWED_DOMAINS"], "a b")
             self.assertEqual(os.environ["RENEWED_LINEAGE"], "thing")
 
-            with mock.patch("letsencrypt.hooks.print") as mock_print:
-                config = mock.MagicMock(renew_hook="true", dry_run=True)
+            config = mock.MagicMock(renew_hook="true", dry_run=True)
+            if sys.version_info < (2, 7):
+                # the print() function is not mockable in py26
                 self._test_a_hook(config, rhook, 0)
-                self.assertEqual(mock_print.call_count, 2)
+            else:
+                with mock.patch("letsencrypt.hooks.print") as mock_print:
+                    self._test_a_hook(config, rhook, 0)
+                    self.assertEqual(mock_print.call_count, 2)
 
     @mock.patch('letsencrypt.hooks.Popen')
     def test_run_hook(self, mock_popen):

From 18e05cc28424d63312e62988bc922bac099458f0 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 31 Mar 2016 12:40:11 -0700
Subject: [PATCH 16/16] Document which hooks are run with --dry-run

---
 letsencrypt/cli.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 1091a804b..199b98311 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -558,7 +558,14 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         None, "--dry-run", action="store_true", dest="dry_run",
         help="Perform a test run of the client, obtaining test (invalid) certs"
              " but not saving them to disk. This can currently only be used"
-             " with the 'certonly' subcommand.")
+             " with the 'certonly' and 'renew' subcommands. \nNote: Although --dry-run"
+             " tries to avoid making any persistent changes on a system, it "
+             " is not completely side-effect free: if used with webserver authenticator plugins"
+             " like apache and nginx, it makes and then reverts temporary config changes"
+             " in order to obtain test certs, and reloads webservers to deploy and then"
+             " roll back those changes.  It also calls --pre-hook and --post-hook commands"
+             " if they are defined because they may be necessary to accurately simulate"
+             " renewal. --renew-hook commands are not called.")
     helpful.add(
         None, "--register-unsafely-without-email", action="store_true",
         help="Specifying this flag enables registering an account with no "