From 876a760a91c6a264338d0ac70bbdf63985fc1747 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 12 Dec 2016 16:17:08 -0800
Subject: [PATCH 01/18] Begin implementing pre / post-hook preservation

---
 certbot/hooks.py   | 6 +++---
 certbot/main.py    | 4 ++--
 certbot/renewal.py | 3 ++-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 37afee9b0..5c14881e9 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -43,11 +43,11 @@ def pre_hook(config):
 
 pre_hook.already = False
 
-def post_hook(config, final=False):
+def post_hook(config, renew_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.
+    we're called with renew_final=True before actually doing anything.
     """
     if config.post_hook:
         if not pre_hook.already:
@@ -55,7 +55,7 @@ def post_hook(config, final=False):
             if config.verb != "renew":
                 logger.warning("Sanity failure in renewal hooks")
             return
-        if final or config.verb != "renew":
+        if renew_final or config.verb != "renew":
             logger.info("Running post-hook command: %s", config.post_hook)
             _run_hook(config.post_hook)
 
diff --git a/certbot/main.py b/certbot/main.py
index 2baab9670..687d872d2 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -108,7 +108,7 @@ def _auth_from_available(le_client, config, domains=None, certname=None, lineage
             if lineage is False:
                 raise errors.Error("Certificate could not be obtained")
     finally:
-        hooks.post_hook(config, final=False)
+        hooks.post_hook(config, renew_final=False)
 
     if not config.dry_run and not config.verb == "renew":
         _report_new_cert(config, lineage.cert, lineage.fullchain)
@@ -634,7 +634,7 @@ def renew(config, unused_plugins):
     try:
         renewal.handle_renewal_request(config)
     finally:
-        hooks.post_hook(config, final=True)
+        hooks.post_hook(config, renew_final=True)
 
 
 def setup_log_file_handler(config, logfile, fmt):
diff --git a/certbot/renewal.py b/certbot/renewal.py
index a057a63a9..9c847a9be 100644
--- a/certbot/renewal.py
+++ b/certbot/renewal.py
@@ -30,7 +30,8 @@ logger = logging.getLogger(__name__)
 # the renewal configuration process loses this information.
 STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
                     "server", "account", "authenticator", "installer",
-                    "standalone_supported_challenges", "renew_hook"]
+                    "standalone_supported_challenges", "renew_hook",
+                    "pre_hook", "post_hook"]
 INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
 
 

From 0bea6c73506ed495cf793e5d2ed3c2676bb380f7 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 15 Dec 2016 13:09:00 -0800
Subject: [PATCH 02/18] Log when we run renew hooks (why weren't we doing this
 already?)

---
 certbot/hooks.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 5c14881e9..0eec93ab8 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -21,6 +21,7 @@ def _prog(shell_cmd):
     cmd = _which(shell_cmd)
     return os.path.basename(cmd) if cmd else None
 
+
 def validate_hook(shell_cmd, hook_name):
     """Check that a command provided as a hook is plausibly executable.
 
@@ -65,6 +66,7 @@ def renew_hook(config, domains, lineage_path):
         if not config.dry_run:
             os.environ["RENEWED_DOMAINS"] = " ".join(domains)
             os.environ["RENEWED_LINEAGE"] = lineage_path
+            logger.info("Running renew-hook command: %s", config.renew_hook)
             _run_hook(config.renew_hook)
         else:
             logger.warning("Dry run: skipping renewal hook command: %s", config.renew_hook)

From 6f9abde89418f851b01a68c2c8359ff9f256e68b Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 15 Dec 2016 16:32:31 -0800
Subject: [PATCH 03/18] Support intricate combinations of pre-hooks in
 different lineages

---
 certbot/hooks.py           | 34 ++++++++++++++++++++++------------
 certbot/tests/hook_test.py | 25 +++++++++++++++----------
 2 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 0eec93ab8..1783a37bd 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -37,12 +37,14 @@ def validate_hook(shell_cmd, hook_name):
 
 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
+    cmd = config.pre_hook
+    if cmd and cmd not in pre_hook.already:
+        logger.info("Running pre-hook command: %s", cmd)
+        _run_hook(cmd)
+        pre_hook.already[cmd] = True
+
+pre_hook.already = {}
 
-pre_hook.already = False
 
 def post_hook(config, renew_final=False):
     """Run post hook if defined.
@@ -50,16 +52,24 @@ def post_hook(config, renew_final=False):
     If the verb is renew, we might have more certs to renew, so we wait until
     we're called with renew_final=True before actually doing anything.
     """
-    if config.post_hook:
-        if not pre_hook.already:
-            logger.info("No renewals attempted, so not running post-hook")
-            if config.verb != "renew":
-                logger.warning("Sanity failure in renewal hooks")
-            return
-        if renew_final or config.verb != "renew":
+
+    if config.verb == "renew":
+        if not renew_final:
+            if config.post_hook:
+                post_hook.eventually[config.post_hook] = True
+        else:
+            for cmd in post_hook.eventually:
+                logger.info("Running post-hook command: %s", cmd)
+                _run_hook(cmd)
+            if len(post_hook.eventually) == 0:
+                logger.info("No renewals attempted, so not running post-hook")
+    else: # certonly / run
+        if config.post_hook:
             logger.info("Running post-hook command: %s", config.post_hook)
             _run_hook(config.post_hook)
 
+post_hook.eventually = {}
+
 def renew_hook(config, domains, lineage_path):
     "Run post-renewal hook if defined."
     if config.renew_hook:
diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py
index be7fb852d..3c55b675c 100644
--- a/certbot/tests/hook_test.py
+++ b/certbot/tests/hook_test.py
@@ -46,34 +46,39 @@ class HookTest(unittest.TestCase):
         mockwhich.return_value = None
         self.assertEqual(hooks._prog("funky"), None)
 
-    def _test_a_hook(self, config, hook_function, calls_expected):
+    def _test_a_hook(self, config, hook_function, calls_expected, **kwargs):
         with mock.patch('certbot.hooks.logger') as mock_logger:
             mock_logger.warning = mock.MagicMock()
             with mock.patch('certbot.hooks._run_hook') as mock_run_hook:
-                hook_function(config)
-                hook_function(config)
+                hook_function(config, **kwargs)
+                hook_function(config, **kwargs)
                 self.assertEqual(mock_run_hook.call_count, calls_expected)
             return mock_logger.warning
 
     def test_pre_hook(self):
-        hooks.pre_hook.already = False
+        hooks.pre_hook.already = {}
         config = mock.MagicMock(pre_hook="true")
         self._test_a_hook(config, hooks.pre_hook, 1)
+        self._test_a_hook(config, hooks.pre_hook, 0)
+        config = mock.MagicMock(pre_hook="more_true")
+        self._test_a_hook(config, hooks.pre_hook, 1)
+        self._test_a_hook(config, hooks.pre_hook, 0)
         config = mock.MagicMock(pre_hook="")
         self._test_a_hook(config, hooks.pre_hook, 0)
 
     def test_post_hook(self):
-        hooks.pre_hook.already = False
-        # if pre-hook isn't called, post-hook shouldn't be
         config = mock.MagicMock(post_hook="true", verb="splonk")
-        self._test_a_hook(config, hooks.post_hook, 0)
-
-        config = mock.MagicMock(post_hook="true", verb="splonk")
-        self._test_a_hook(config, hooks.pre_hook, 1)
         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)
+        self._test_a_hook(config, hooks.post_hook, 2, renew_final=True)
+        self._test_a_hook(config, hooks.post_hook, 0)
+        self._test_a_hook(config, hooks.post_hook, 2, renew_final=True)
+
+        config = mock.MagicMock(post_hook="more_true", verb="renew")
+        self._test_a_hook(config, hooks.post_hook, 0)
+        self._test_a_hook(config, hooks.post_hook, 4, renew_final=True)
 
     def test_renew_hook(self):
         with mock.patch.dict('os.environ', {}):

From 0c2dc60484983012bb5fca8d9b853765ab94a8ff Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 15 Dec 2016 17:17:51 -0800
Subject: [PATCH 04/18] Integration tests for hooks

---
 certbot/hooks.py             |  2 ++
 tests/boulder-integration.sh | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 1783a37bd..a5e09b585 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -42,6 +42,8 @@ def pre_hook(config):
         logger.info("Running pre-hook command: %s", cmd)
         _run_hook(cmd)
         pre_hook.already[cmd] = True
+    elif cmd:
+        logger.info("Pre-hook command already run, skipping: %s", cmd)
 
 pre_hook.already = {}
 
diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh
index a70f13f8e..f0e6d993e 100755
--- a/tests/boulder-integration.sh
+++ b/tests/boulder-integration.sh
@@ -37,14 +37,41 @@ common() {
 # unrequested challenge to prevent regressions in #3601.
 python -m SimpleHTTPServer $http_01_port &
 python_server_pid=$!
-common --domains le1.wtf --preferred-challenges tls-sni-01 auth
+
+export HOOK_TEST="/tmp/hook$$"
+common --domains le1.wtf --preferred-challenges tls-sni-01 auth \
+       --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \
+       --post-hook 'echo wtf.post >> "$HOOK_TEST"'\
+       --renew-hook 'echo renew >> "$HOOK_TEST"'
 kill $python_server_pid
 python -m SimpleHTTPServer $tls_sni_01_port &
 python_server_pid=$!
-common --domains le2.wtf --preferred-challenges http-01 run
+common --domains le2.wtf --preferred-challenges http-01 run \
+       --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \
+       --post-hook 'echo wtf.post >> "$HOOK_TEST"'\
+       --renew-hook 'echo renew >> "$HOOK_TEST"'
 kill $python_server_pid
 
-common -a manual -d le.wtf auth --rsa-key-size 4096
+common -a manual -d le.wtf auth --rsa-key-size 4096 \
+       --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \
+       --post-hook 'echo wtf2.post >> "$HOOK_TEST"'
+
+CheckHooks() {
+    EXPECTED="/tmp/expected$$"
+    echo "wtf.pre" > "$EXPECTED"
+    echo "wtf2.pre" >> "$EXPECTED"
+    echo "renew" >> "$EXPECTED"
+    echo "renew" >> "$EXPECTED"
+    echo "wtf.post" > "$EXPECTED"
+    echo "wtf2.post" >> "$EXPECTED"
+    if cmp --quiet "$EXPECTED" "$HOOK_TEST" ; then
+        echo Hooks did not run as expected\; got
+        cat "$HOOK_TEST"
+        echo Expected
+        cat "$EXPECTED"
+    fi
+    [ -f "$HOOK_TEST" ] && rm -f "$HOOK_TEST"
+}
 
 export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \
        OPENSSL_CNF=examples/openssl.cnf
@@ -73,8 +100,11 @@ common_no_force_renew renew
 CheckCertCount 1
 
 # --renew-by-default is used, so renewal should occur
+[ -f "$HOOK_TEST" ] && rm -f "$HOOK_TEST"
 common renew
 CheckCertCount 2
+CheckHooks
+
 
 # This will renew because the expiry is less than 10 years from now
 sed -i "4arenew_before_expiry = 4 years" "$root/conf/renewal/le.wtf.conf"

From 33de782fba94ae555058a0a51ba85de9d3fead2f Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 15 Dec 2016 17:21:20 -0800
Subject: [PATCH 05/18] Make post hooks run in deterministic order

---
 certbot/hooks.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index a5e09b585..0db4d508d 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -58,7 +58,7 @@ def post_hook(config, renew_final=False):
     if config.verb == "renew":
         if not renew_final:
             if config.post_hook:
-                post_hook.eventually[config.post_hook] = True
+                post_hook.eventually.append(config.post_hook)
         else:
             for cmd in post_hook.eventually:
                 logger.info("Running post-hook command: %s", cmd)
@@ -70,7 +70,7 @@ def post_hook(config, renew_final=False):
             logger.info("Running post-hook command: %s", config.post_hook)
             _run_hook(config.post_hook)
 
-post_hook.eventually = {}
+post_hook.eventually = []
 
 def renew_hook(config, domains, lineage_path):
     "Run post-renewal hook if defined."

From de77dd74adcd3205a83664fc31efdacffceb91b9 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 15 Dec 2016 17:31:32 -0800
Subject: [PATCH 06/18] Unbreak repeated insertion logic

---
 certbot/hooks.py | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 0db4d508d..6aece9bde 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -55,10 +55,11 @@ def post_hook(config, renew_final=False):
     we're called with renew_final=True before actually doing anything.
     """
 
+    cmd = config.post_hook
     if config.verb == "renew":
         if not renew_final:
-            if config.post_hook:
-                post_hook.eventually.append(config.post_hook)
+            if cmd and cmd not in post_hook.eventually:
+                post_hook.eventually.append(cmd)
         else:
             for cmd in post_hook.eventually:
                 logger.info("Running post-hook command: %s", cmd)
@@ -66,9 +67,9 @@ def post_hook(config, renew_final=False):
             if len(post_hook.eventually) == 0:
                 logger.info("No renewals attempted, so not running post-hook")
     else: # certonly / run
-        if config.post_hook:
-            logger.info("Running post-hook command: %s", config.post_hook)
-            _run_hook(config.post_hook)
+        if cmd:
+            logger.info("Running post-hook command: %s", cmd)
+            _run_hook(cmd)
 
 post_hook.eventually = []
 

From 99482e80476bac26dea677f4a215f7ac9b29a985 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 15 Dec 2016 17:41:34 -0800
Subject: [PATCH 07/18] Document subtle new semantics

---
 certbot/cli.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/certbot/cli.py b/certbot/cli.py
index 356a03764..bdf3f248d 100644
--- a/certbot/cli.py
+++ b/certbot/cli.py
@@ -876,13 +876,16 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         " 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.")
+        " obtained/renewed. When renewing several certificates that have"
+        " identical pre-hooks, only the first will be executed.")
     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. This is only"
-        " run if an attempt was made to obtain/renew a certificate.")
+        " run if an attempt was made to obtain/renew a certificate. If"
+        " multiple renewed certificates have identical post-hooks, only"
+        " one will be run.")
     helpful.add(
         "renew", "--renew-hook",
         help="Command to be run in a shell once for each successfully renewed"

From 64daefea6b5d1cce9ad1828668f23ceef17591de Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 17 Dec 2016 14:51:34 -0800
Subject: [PATCH 08/18] util.exe_exists and hooks._which were almost identical;
 merge them

---
 certbot/hooks.py           | 24 ++---------------------
 certbot/tests/hook_test.py | 13 +------------
 certbot/tests/util_test.py | 39 ++++++++++++--------------------------
 certbot/util.py            | 34 ++++++++++++++++++++-------------
 4 files changed, 36 insertions(+), 74 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 6aece9bde..4662c4515 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -7,6 +7,7 @@ import os
 from subprocess import Popen, PIPE
 
 from certbot import errors
+from certbot import util
 
 logger = logging.getLogger(__name__)
 
@@ -18,7 +19,7 @@ def validate_hooks(config):
 
 def _prog(shell_cmd):
     """Extract the program run by a shell command"""
-    cmd = _which(shell_cmd)
+    cmd = util.which(shell_cmd)
     return os.path.basename(cmd) if cmd else None
 
 
@@ -108,24 +109,3 @@ def execute(shell_cmd):
         logger.error('Error output from %s:\n%s', _prog(shell_cmd), err)
     return (err, out)
 
-
-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
-
-    fpath, _fname = os.path.split(program)
-    if fpath:
-        if _is_exe(program):
-            return program
-    else:
-        for path in os.environ["PATH"].split(os.pathsep):
-            exe_file = os.path.join(path, program)
-            if _is_exe(exe_file):
-                return exe_file
-
-    return None
diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py
index 3c55b675c..95fede230 100644
--- a/certbot/tests/hook_test.py
+++ b/certbot/tests/hook_test.py
@@ -27,19 +27,8 @@ class HookTest(unittest.TestCase):
         config = mock.MagicMock(pre_hook="explodinator", post_hook="", renew_hook="")
         self.assertRaises(errors.HookCommandNotFound, hooks.validate_hooks, config)
 
-    @mock.patch('certbot.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('certbot.hooks._which')
+    @mock.patch('certbot.hooks.util.which')
     def test_prog(self, mockwhich):
         mockwhich.return_value = "/very/very/funky"
         self.assertEqual(hooks._prog("funky"), "funky")
diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py
index cd02d835d..866f71e3d 100644
--- a/certbot/tests/util_test.py
+++ b/certbot/tests/util_test.py
@@ -45,34 +45,19 @@ class RunScriptTest(unittest.TestCase):
         self.assertRaises(errors.SubprocessError, self._call, ["test"])
 
 
-class ExeExistsTest(unittest.TestCase):
-    """Tests for certbot.util.exe_exists."""
+class WhichTest(unittest.TestCase):
+    @mock.patch('certbot.util._is_exe')
+    def test_which(self, mock_is_exe):
+        from certbot.util import which
+        mock_is_exe.return_value = True
+        self.assertEqual(which("/path/to/something"), "/path/to/something")
 
-    @classmethod
-    def _call(cls, exe):
-        from certbot.util import exe_exists
-        return exe_exists(exe)
-
-    @mock.patch("certbot.util.os.path.isfile")
-    @mock.patch("certbot.util.os.access")
-    def test_full_path(self, mock_access, mock_isfile):
-        mock_access.return_value = True
-        mock_isfile.return_value = True
-        self.assertTrue(self._call("/path/to/exe"))
-
-    @mock.patch("certbot.util.os.path.isfile")
-    @mock.patch("certbot.util.os.access")
-    def test_on_path(self, mock_access, mock_isfile):
-        mock_access.return_value = True
-        mock_isfile.return_value = True
-        self.assertTrue(self._call("exe"))
-
-    @mock.patch("certbot.util.os.path.isfile")
-    @mock.patch("certbot.util.os.access")
-    def test_not_found(self, mock_access, mock_isfile):
-        mock_access.return_value = False
-        mock_isfile.return_value = True
-        self.assertFalse(self._call("exe"))
+        with mock.patch.dict('os.environ', {"PATH": "/floop:/fleep"}):
+            mock_is_exe.return_value = True
+            self.assertEqual(which("pingify"), "/floop/pingify")
+            mock_is_exe.return_value = False
+            self.assertEqual(which("pingify"), None)
+        self.assertEqual(which("/path/to/something"), None)
 
 
 class MakeOrVerifyDirTest(unittest.TestCase):
diff --git a/certbot/util.py b/certbot/util.py
index cbcfa3314..c09d307d2 100644
--- a/certbot/util.py
+++ b/certbot/util.py
@@ -66,28 +66,36 @@ def run_script(params):
     return stdout, stderr
 
 
-def exe_exists(exe):
+def _is_exe(fpath):
+    return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
+
+def which(program):
     """Determine whether path/name refers to an executable.
 
-    :param str exe: Executable path or name
+    :param str program: Executable path or name
 
-    :returns: If exe is a valid executable
-    :rtype: bool
+    :returns: Path to executable if it exists, or None
+    :rtype: str or None
 
     """
-    def is_exe(path):
-        """Determine if path is an exe."""
-        return os.path.isfile(path) and os.access(path, os.X_OK)
+    # Borrowed from:
+    # https://stackoverflow.com/questions/377017/test-if-executable-exists-in-python
+    # XXX May need more porting to handle .exe extensions on Windows
 
-    path, _ = os.path.split(exe)
-    if path:
-        return is_exe(exe)
+    fpath, _fname = os.path.split(program)
+    if fpath:
+        if _is_exe(program):
+            return program
     else:
         for path in os.environ["PATH"].split(os.pathsep):
-            if is_exe(os.path.join(path, exe)):
-                return True
+            exe_file = os.path.join(path, program)
+            if _is_exe(exe_file):
+                return exe_file
 
-    return False
+    return None
+
+# Either name makes sense, and we have a lot of call sites!
+exe_exists = which
 
 
 def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False):

From 186a8c888f8159a249cb89ff21d6c33c2f393c2a Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sat, 17 Dec 2016 14:59:51 -0800
Subject: [PATCH 09/18] Path surgery makes sense for hooks that may be called
 from cron

---
 certbot/hooks.py           |  6 ++++++
 certbot/plugins/util.py    | 12 ++++++------
 certbot/tests/hook_test.py |  5 ++++-
 3 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 4662c4515..9a2d8b659 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -9,6 +9,8 @@ from subprocess import Popen, PIPE
 from certbot import errors
 from certbot import util
 
+from certbot.plugins.util import path_surgery
+
 logger = logging.getLogger(__name__)
 
 def validate_hooks(config):
@@ -20,6 +22,10 @@ def validate_hooks(config):
 def _prog(shell_cmd):
     """Extract the program run by a shell command"""
     cmd = util.which(shell_cmd)
+    if not cmd:
+        path_surgery(shell_cmd)
+        cmd = util.which(shell_cmd)
+
     return os.path.basename(cmd) if cmd else None
 
 
diff --git a/certbot/plugins/util.py b/certbot/plugins/util.py
index 8f6a62a7f..ceedbd39c 100644
--- a/certbot/plugins/util.py
+++ b/certbot/plugins/util.py
@@ -30,12 +30,12 @@ RENEWER_EXTRA_MSG = (
     " needing to stop and start your webserver.")
 
 
-def path_surgery(restart_cmd):
-    """Attempt to perform PATH surgery to find restart_cmd
+def path_surgery(cmd):
+    """Attempt to perform PATH surgery to find cmd
 
     Mitigates https://github.com/certbot/certbot/issues/1833
 
-    :param str restart_cmd: the command that is being searched for in the PATH
+    :param str cmd: the command that is being searched for in the PATH
 
     :returns: True if the operation succeeded, False otherwise
     """
@@ -49,14 +49,14 @@ def path_surgery(restart_cmd):
 
     if any(added):
         logger.debug("Can't find %s, attempting PATH mitigation by adding %s",
-                     restart_cmd, os.pathsep.join(added))
+                     cmd, os.pathsep.join(added))
         os.environ["PATH"] = path
 
-    if util.exe_exists(restart_cmd):
+    if util.exe_exists(cmd):
         return True
     else:
         expanded = " expanded" if any(added) else ""
-        logger.warning("Failed to find %s in%s PATH: %s", restart_cmd,
+        logger.warning("Failed to find %s in%s PATH: %s", cmd,
                        expanded, path)
         return False
 
diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py
index 95fede230..dc1f44787 100644
--- a/certbot/tests/hook_test.py
+++ b/certbot/tests/hook_test.py
@@ -29,11 +29,14 @@ class HookTest(unittest.TestCase):
 
 
     @mock.patch('certbot.hooks.util.which')
-    def test_prog(self, mockwhich):
+    @mock.patch('certbot.hooks.path_surgery')
+    def test_prog(self, mock_ps, mockwhich):
         mockwhich.return_value = "/very/very/funky"
         self.assertEqual(hooks._prog("funky"), "funky")
+        self.assertEqual(mock_ps.call_count, 0)
         mockwhich.return_value = None
         self.assertEqual(hooks._prog("funky"), None)
+        self.assertEqual(mock_ps.call_count, 1)
 
     def _test_a_hook(self, config, hook_function, calls_expected, **kwargs):
         with mock.patch('certbot.hooks.logger') as mock_logger:

From 6a67ce5567c92aa70886d88614e68e3e113ad84e Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 19 Dec 2016 16:56:21 -0800
Subject: [PATCH 10/18] Ensure that path_surgery doesn't happen until we have a
 logger

---
 certbot/cli.py  |  4 ----
 certbot/main.py | 21 +++++++++++++++------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/certbot/cli.py b/certbot/cli.py
index bdf3f248d..9c861dad6 100644
--- a/certbot/cli.py
+++ b/certbot/cli.py
@@ -18,7 +18,6 @@ import certbot
 from certbot import constants
 from certbot import crypto_util
 from certbot import errors
-from certbot import hooks
 from certbot import interfaces
 from certbot import util
 
@@ -394,9 +393,6 @@ class HelpfulArgumentParser(object):
         if parsed_args.must_staple:
             parsed_args.staple = True
 
-        if parsed_args.validate_hooks:
-            hooks.validate_hooks(parsed_args)
-
         return parsed_args
 
     def set_test_server(self, parsed_args):
diff --git a/certbot/main.py b/certbot/main.py
index 687d872d2..2fc810c40 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -782,6 +782,20 @@ def set_displayer(config):
         displayer = display_util.FileDisplay(sys.stdout)
     zope.component.provideUtility(displayer)
 
+def _post_logging_setup(config, plugins, cli_args):
+    "Perform any setup or configuration tasks that require a logger."
+
+    # This needs logging, but would otherwise be in HelpfulArgumentParser
+    if config.validate_hooks:
+        hooks.validate_hooks(config)
+
+    cli.possible_deprecation_warning(config)
+
+    logger.debug("certbot version: %s", certbot.__version__)
+    # do not log `config`, as it contains sensitive data (e.g. revoke --key)!
+    logger.debug("Arguments: %r", cli_args)
+    logger.debug("Discovered plugins: %r", plugins)
+
 
 def main(cli_args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
@@ -799,12 +813,7 @@ def main(cli_args=sys.argv[1:]):
     # logger ..." TODO: this should be done before plugins discovery
     setup_logging(config)
 
-    cli.possible_deprecation_warning(config)
-
-    logger.debug("certbot version: %s", certbot.__version__)
-    # do not log `config`, as it contains sensitive data (e.g. revoke --key)!
-    logger.debug("Arguments: %r", cli_args)
-    logger.debug("Discovered plugins: %r", plugins)
+    _post_logging_setup(config, plugins, cli_args)
 
     sys.excepthook = functools.partial(_handle_exception, config=config)
 

From ac17f98b0c4926612e51bac753dd23b70f8c7414 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 21 Dec 2016 23:02:27 -0800
Subject: [PATCH 11/18] Refactor post_hook storage during "renew"

---
 certbot/hooks.py           | 38 ++++++++++++++++++++------------------
 certbot/main.py            |  4 ++--
 certbot/tests/hook_test.py | 22 ++++++++++++++++------
 3 files changed, 38 insertions(+), 26 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 9a2d8b659..951d039b9 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -9,7 +9,7 @@ from subprocess import Popen, PIPE
 from certbot import errors
 from certbot import util
 
-from certbot.plugins.util import path_surgery
+from certbot.plugins import util as plug_util
 
 logger = logging.getLogger(__name__)
 
@@ -23,7 +23,7 @@ def _prog(shell_cmd):
     """Extract the program run by a shell command"""
     cmd = util.which(shell_cmd)
     if not cmd:
-        path_surgery(shell_cmd)
+        plug_util.path_surgery(shell_cmd)
         cmd = util.which(shell_cmd)
 
     return os.path.basename(cmd) if cmd else None
@@ -55,33 +55,35 @@ def pre_hook(config):
 pre_hook.already = {}
 
 
-def post_hook(config, renew_final=False):
+def post_hook(config):
     """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 renew_final=True before actually doing anything.
+    run_saved_post_hooks() is called.
     """
 
     cmd = config.post_hook
+    # In the "renew" case, we save these up to run at the end
     if config.verb == "renew":
-        if not renew_final:
-            if cmd and cmd not in post_hook.eventually:
-                post_hook.eventually.append(cmd)
-        else:
-            for cmd in post_hook.eventually:
-                logger.info("Running post-hook command: %s", cmd)
-                _run_hook(cmd)
-            if len(post_hook.eventually) == 0:
-                logger.info("No renewals attempted, so not running post-hook")
-    else: # certonly / run
-        if cmd:
-            logger.info("Running post-hook command: %s", cmd)
-            _run_hook(cmd)
+        if cmd and cmd not in post_hook.eventually:
+            post_hook.eventually.append(cmd)
+    # certonly / run
+    elif cmd:
+        logger.info("Running post-hook command: %s", cmd)
+        _run_hook(cmd)
 
 post_hook.eventually = []
 
+def run_saved_post_hooks():
+    """Run any post hooks that were saved up in the course of the 'renew' verb"""
+    for cmd in post_hook.eventually:
+        logger.info("Running post-hook command: %s", cmd)
+        _run_hook(cmd)
+    if len(post_hook.eventually) == 0:
+        logger.info("No renewals attempted, so not running post-hook")
+
 def renew_hook(config, domains, lineage_path):
-    "Run post-renewal hook if defined."
+    """Run post-renewal hook if defined."""
     if config.renew_hook:
         if not config.dry_run:
             os.environ["RENEWED_DOMAINS"] = " ".join(domains)
diff --git a/certbot/main.py b/certbot/main.py
index 2fc810c40..b48172677 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -108,7 +108,7 @@ def _auth_from_available(le_client, config, domains=None, certname=None, lineage
             if lineage is False:
                 raise errors.Error("Certificate could not be obtained")
     finally:
-        hooks.post_hook(config, renew_final=False)
+        hooks.post_hook(config)
 
     if not config.dry_run and not config.verb == "renew":
         _report_new_cert(config, lineage.cert, lineage.fullchain)
@@ -634,7 +634,7 @@ def renew(config, unused_plugins):
     try:
         renewal.handle_renewal_request(config)
     finally:
-        hooks.post_hook(config, renew_final=True)
+        hooks.run_saved_post_hooks()
 
 
 def setup_log_file_handler(config, logfile, fmt):
diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py
index dc1f44787..f799ed74f 100644
--- a/certbot/tests/hook_test.py
+++ b/certbot/tests/hook_test.py
@@ -27,9 +27,8 @@ class HookTest(unittest.TestCase):
         config = mock.MagicMock(pre_hook="explodinator", post_hook="", renew_hook="")
         self.assertRaises(errors.HookCommandNotFound, hooks.validate_hooks, config)
 
-
     @mock.patch('certbot.hooks.util.which')
-    @mock.patch('certbot.hooks.path_surgery')
+    @mock.patch('certbot.hooks.plug_util.path_surgery')
     def test_prog(self, mock_ps, mockwhich):
         mockwhich.return_value = "/very/very/funky"
         self.assertEqual(hooks._prog("funky"), "funky")
@@ -58,19 +57,30 @@ class HookTest(unittest.TestCase):
         config = mock.MagicMock(pre_hook="")
         self._test_a_hook(config, hooks.pre_hook, 0)
 
-    def test_post_hook(self):
+    def _test_renew_post_hooks(self, expected_count):
+        with mock.patch('certbot.hooks.logger.info') as mock_info:
+            with mock.patch('certbot.hooks._run_hook') as mock_run:
+                hooks.run_saved_post_hooks()
+                self.assertEqual(mock_run.call_count, expected_count)
+                if expected_count:
+                    self.assertEqual(mock_info.call_count, expected_count)
+                else:
+                    self.assertEqual(mock_info.call_count, 1)
+
+    def test_post_hooks(self):
         config = mock.MagicMock(post_hook="true", verb="splonk")
         self._test_a_hook(config, hooks.post_hook, 2)
+        self._test_renew_post_hooks(0)
 
         config = mock.MagicMock(post_hook="true", verb="renew")
         self._test_a_hook(config, hooks.post_hook, 0)
-        self._test_a_hook(config, hooks.post_hook, 2, renew_final=True)
+        self._test_renew_post_hooks(1)
         self._test_a_hook(config, hooks.post_hook, 0)
-        self._test_a_hook(config, hooks.post_hook, 2, renew_final=True)
+        self._test_renew_post_hooks(1)
 
         config = mock.MagicMock(post_hook="more_true", verb="renew")
         self._test_a_hook(config, hooks.post_hook, 0)
-        self._test_a_hook(config, hooks.post_hook, 4, renew_final=True)
+        self._test_renew_post_hooks(2)
 
     def test_renew_hook(self):
         with mock.patch.dict('os.environ', {}):

From d08cf89ad2738ed2d89b0bba9723584d3404c901 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Wed, 4 Jan 2017 12:54:51 -0800
Subject: [PATCH 12/18] Remove all calls to which

---
 certbot/hooks.py           | 21 ++++++++++++++-------
 certbot/tests/hook_test.py |  8 ++++----
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 6757c5a72..ae190a930 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -20,13 +20,19 @@ def validate_hooks(config):
     validate_hook(config.renew_hook, "renew")
 
 def _prog(shell_cmd):
-    """Extract the program run by a shell command"""
-    cmd = util.which(shell_cmd)
-    if not cmd:
-        plug_util.path_surgery(shell_cmd)
-        cmd = util.which(shell_cmd)
+    """Extract the program run by a shell command.
 
-    return os.path.basename(cmd) if cmd else None
+    :param str shell_cmd: command to be executed
+
+    :returns: basename of command or None if the command isn't found
+    :rtype: str or None
+
+    """
+    if not util.exe_exists(shell_cmd):
+        plug_util.path_surgery(shell_cmd)
+        if not util.exe_exists(shell_cmd):
+            return None
+    return os.path.basename(shell_cmd)
 
 
 def validate_hook(shell_cmd, hook_name):
@@ -117,6 +123,7 @@ def execute(shell_cmd):
         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)
+        base_cmd = os.path.basename(shell_cmd.split(None, 1)[0])
+        logger.error('Error output from %s:\n%s', base_cmd, err)
     return (err, out)
 
diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py
index f799ed74f..a8f49745b 100644
--- a/certbot/tests/hook_test.py
+++ b/certbot/tests/hook_test.py
@@ -27,13 +27,13 @@ class HookTest(unittest.TestCase):
         config = mock.MagicMock(pre_hook="explodinator", post_hook="", renew_hook="")
         self.assertRaises(errors.HookCommandNotFound, hooks.validate_hooks, config)
 
-    @mock.patch('certbot.hooks.util.which')
+    @mock.patch('certbot.hooks.util.exe_exists')
     @mock.patch('certbot.hooks.plug_util.path_surgery')
-    def test_prog(self, mock_ps, mockwhich):
-        mockwhich.return_value = "/very/very/funky"
+    def test_prog(self, mock_ps, mock_exe_exists):
+        mock_exe_exists.return_value = True
         self.assertEqual(hooks._prog("funky"), "funky")
         self.assertEqual(mock_ps.call_count, 0)
-        mockwhich.return_value = None
+        mock_exe_exists.return_value = False
         self.assertEqual(hooks._prog("funky"), None)
         self.assertEqual(mock_ps.call_count, 1)
 

From b748f1afe429a4a0ef03783281975b4377fb973f Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Wed, 4 Jan 2017 12:55:55 -0800
Subject: [PATCH 13/18] Undo changes to certbot.util

---
 certbot/tests/util_test.py | 39 ++++++++++++++++++++++++++------------
 certbot/util.py            | 34 +++++++++++++--------------------
 2 files changed, 40 insertions(+), 33 deletions(-)

diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py
index 866f71e3d..cd02d835d 100644
--- a/certbot/tests/util_test.py
+++ b/certbot/tests/util_test.py
@@ -45,19 +45,34 @@ class RunScriptTest(unittest.TestCase):
         self.assertRaises(errors.SubprocessError, self._call, ["test"])
 
 
-class WhichTest(unittest.TestCase):
-    @mock.patch('certbot.util._is_exe')
-    def test_which(self, mock_is_exe):
-        from certbot.util import which
-        mock_is_exe.return_value = True
-        self.assertEqual(which("/path/to/something"), "/path/to/something")
+class ExeExistsTest(unittest.TestCase):
+    """Tests for certbot.util.exe_exists."""
 
-        with mock.patch.dict('os.environ', {"PATH": "/floop:/fleep"}):
-            mock_is_exe.return_value = True
-            self.assertEqual(which("pingify"), "/floop/pingify")
-            mock_is_exe.return_value = False
-            self.assertEqual(which("pingify"), None)
-        self.assertEqual(which("/path/to/something"), None)
+    @classmethod
+    def _call(cls, exe):
+        from certbot.util import exe_exists
+        return exe_exists(exe)
+
+    @mock.patch("certbot.util.os.path.isfile")
+    @mock.patch("certbot.util.os.access")
+    def test_full_path(self, mock_access, mock_isfile):
+        mock_access.return_value = True
+        mock_isfile.return_value = True
+        self.assertTrue(self._call("/path/to/exe"))
+
+    @mock.patch("certbot.util.os.path.isfile")
+    @mock.patch("certbot.util.os.access")
+    def test_on_path(self, mock_access, mock_isfile):
+        mock_access.return_value = True
+        mock_isfile.return_value = True
+        self.assertTrue(self._call("exe"))
+
+    @mock.patch("certbot.util.os.path.isfile")
+    @mock.patch("certbot.util.os.access")
+    def test_not_found(self, mock_access, mock_isfile):
+        mock_access.return_value = False
+        mock_isfile.return_value = True
+        self.assertFalse(self._call("exe"))
 
 
 class MakeOrVerifyDirTest(unittest.TestCase):
diff --git a/certbot/util.py b/certbot/util.py
index 733b3e501..cc0a74bd2 100644
--- a/certbot/util.py
+++ b/certbot/util.py
@@ -68,36 +68,28 @@ def run_script(params, log=logger.error):
     return stdout, stderr
 
 
-def _is_exe(fpath):
-    return os.path.isfile(fpath) and os.access(fpath, os.X_OK)
-
-def which(program):
+def exe_exists(exe):
     """Determine whether path/name refers to an executable.
 
-    :param str program: Executable path or name
+    :param str exe: Executable path or name
 
-    :returns: Path to executable if it exists, or None
-    :rtype: str or None
+    :returns: If exe is a valid executable
+    :rtype: bool
 
     """
-    # 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(path):
+        """Determine if path is an exe."""
+        return os.path.isfile(path) and os.access(path, os.X_OK)
 
-    fpath, _fname = os.path.split(program)
-    if fpath:
-        if _is_exe(program):
-            return program
+    path, _ = os.path.split(exe)
+    if path:
+        return is_exe(exe)
     else:
         for path in os.environ["PATH"].split(os.pathsep):
-            exe_file = os.path.join(path, program)
-            if _is_exe(exe_file):
-                return exe_file
+            if is_exe(os.path.join(path, exe)):
+                return True
 
-    return None
-
-# Either name makes sense, and we have a lot of call sites!
-exe_exists = which
+    return False
 
 
 def make_or_verify_dir(directory, mode=0o755, uid=0, strict=False):

From 458c7c8131cb2b846fd0850ca17b4672076cfce6 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Wed, 4 Jan 2017 12:57:54 -0800
Subject: [PATCH 14/18] Use a set for pre_hook.already

---
 certbot/hooks.py           | 4 ++--
 certbot/tests/hook_test.py | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index ae190a930..a8099c4a8 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -54,11 +54,11 @@ def pre_hook(config):
     if cmd and cmd not in pre_hook.already:
         logger.info("Running pre-hook command: %s", cmd)
         _run_hook(cmd)
-        pre_hook.already[cmd] = True
+        pre_hook.already.add(cmd)
     elif cmd:
         logger.info("Pre-hook command already run, skipping: %s", cmd)
 
-pre_hook.already = {}
+pre_hook.already = set()
 
 
 def post_hook(config):
diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py
index a8f49745b..3165a67fc 100644
--- a/certbot/tests/hook_test.py
+++ b/certbot/tests/hook_test.py
@@ -47,7 +47,7 @@ class HookTest(unittest.TestCase):
             return mock_logger.warning
 
     def test_pre_hook(self):
-        hooks.pre_hook.already = {}
+        hooks.pre_hook.already = set()
         config = mock.MagicMock(pre_hook="true")
         self._test_a_hook(config, hooks.pre_hook, 1)
         self._test_a_hook(config, hooks.pre_hook, 0)

From dae9eee7d4397c167beee0e9d06307d42fb7ae31 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Wed, 4 Jan 2017 12:58:59 -0800
Subject: [PATCH 15/18] bool(len([])) == bool([])

---
 certbot/hooks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index a8099c4a8..ce4f16262 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -85,7 +85,7 @@ def run_saved_post_hooks():
     for cmd in post_hook.eventually:
         logger.info("Running post-hook command: %s", cmd)
         _run_hook(cmd)
-    if len(post_hook.eventually) == 0:
+    if not post_hook.eventually:
         logger.info("No renewals attempted, so not running post-hook")
 
 def renew_hook(config, domains, lineage_path):

From beb5db805b6d66344672dc484f7f503683bf677d Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Wed, 4 Jan 2017 12:59:43 -0800
Subject: [PATCH 16/18] Fix docstring

---
 certbot/main.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/certbot/main.py b/certbot/main.py
index 7e2a999a4..91b860dbb 100644
--- a/certbot/main.py
+++ b/certbot/main.py
@@ -805,7 +805,7 @@ def set_displayer(config):
     zope.component.provideUtility(displayer)
 
 def _post_logging_setup(config, plugins, cli_args):
-    "Perform any setup or configuration tasks that require a logger."
+    """Perform any setup or configuration tasks that require a logger."""
 
     # This needs logging, but would otherwise be in HelpfulArgumentParser
     if config.validate_hooks:

From 7fb4e6627c4d8be956628f88c0f49f0f981a24dd Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Wed, 4 Jan 2017 13:12:50 -0800
Subject: [PATCH 17/18] HOOK_TEST++

---
 tests/boulder-integration.sh | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh
index 38448bbf6..55d3d2fe4 100755
--- a/tests/boulder-integration.sh
+++ b/tests/boulder-integration.sh
@@ -33,21 +33,30 @@ common() {
         "$@"
 }
 
+export HOOK_TEST="/tmp/hook$$"
 CheckHooks() {
     EXPECTED="/tmp/expected$$"
-    echo "wtf.pre" > "$EXPECTED"
-    echo "wtf2.pre" >> "$EXPECTED"
-    echo "renew" >> "$EXPECTED"
-    echo "renew" >> "$EXPECTED"
-    echo "wtf.post" > "$EXPECTED"
-    echo "wtf2.post" >> "$EXPECTED"
+    if [ $(head -n1 $HOOK_TEST) = "wtf.pre" ]; then
+        echo "wtf2.pre" >> "$EXPECTED"
+        echo "renew" >> "$EXPECTED"
+        echo "renew" >> "$EXPECTED"
+        echo "wtf.post" >> "$EXPECTED"
+        echo "wtf2.post" >> "$EXPECTED"
+    else
+        echo "wtf.pre" >> "$EXPECTED"
+        echo "renew" >> "$EXPECTED"
+        echo "renew" >> "$EXPECTED"
+        echo "wtf2.post" >> "$EXPECTED"
+        echo "wtf.post" >> "$EXPECTED"
+    fi
+
     if cmp --quiet "$EXPECTED" "$HOOK_TEST" ; then
         echo Hooks did not run as expected\; got
         cat "$HOOK_TEST"
         echo Expected
         cat "$EXPECTED"
     fi
-    [ -f "$HOOK_TEST" ] && rm -f "$HOOK_TEST"
+    rm "$HOOK_TEST"
 }
 
 # We start a server listening on the port for the
@@ -55,7 +64,6 @@ CheckHooks() {
 python -m SimpleHTTPServer $http_01_port &
 python_server_pid=$!
 
-export HOOK_TEST="/tmp/hook$$"
 common --domains le1.wtf --preferred-challenges tls-sni-01 auth \
        --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \
        --post-hook 'echo wtf.post >> "$HOOK_TEST"'\
@@ -70,8 +78,8 @@ common --domains le2.wtf --preferred-challenges http-01 run \
 kill $python_server_pid
 
 common certonly -a manual -d le.wtf --rsa-key-size 4096 \
-    --manual-auth-hook ./tests/manual-http-auth.sh \
-    --manual-cleanup-hook ./tests/manual-http-cleanup.sh
+    --manual-auth-hook 'echo wtf2.auth >> "$HOOK_TEST" && ./tests/manual-http-auth.sh' \
+    --manual-cleanup-hook 'echo wtf2.cleanup >> "$HOOK_TEST" && ./tests/manual-http-cleanup.sh' \
     --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \
     --post-hook 'echo wtf2.post >> "$HOOK_TEST"'
 

From 05afb545220c21d589cc93b5b6e6d0c6e544b984 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Wed, 4 Jan 2017 13:39:46 -0800
Subject: [PATCH 18/18] Fix and cleanup CheckHooks

---
 tests/boulder-integration.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh
index 55d3d2fe4..fe2f223ef 100755
--- a/tests/boulder-integration.sh
+++ b/tests/boulder-integration.sh
@@ -35,17 +35,18 @@ common() {
 
 export HOOK_TEST="/tmp/hook$$"
 CheckHooks() {
+    COMMON="wtf2.auth\nwtf2.cleanup\nrenew\nrenew"
     EXPECTED="/tmp/expected$$"
     if [ $(head -n1 $HOOK_TEST) = "wtf.pre" ]; then
+        echo "wtf.pre" > "$EXPECTED"
         echo "wtf2.pre" >> "$EXPECTED"
-        echo "renew" >> "$EXPECTED"
-        echo "renew" >> "$EXPECTED"
+        echo $COMMON >> "$EXPECTED"
         echo "wtf.post" >> "$EXPECTED"
         echo "wtf2.post" >> "$EXPECTED"
     else
+        echo "wtf2.pre" > "$EXPECTED"
         echo "wtf.pre" >> "$EXPECTED"
-        echo "renew" >> "$EXPECTED"
-        echo "renew" >> "$EXPECTED"
+        echo $COMMON >> "$EXPECTED"
         echo "wtf2.post" >> "$EXPECTED"
         echo "wtf.post" >> "$EXPECTED"
     fi