Path surgery makes sense for hooks that may be called from cron

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
 
 

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
 

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: