From 641aba68b1c2e57a912e5edf95178cd29fd66b4b Mon Sep 17 00:00:00 2001
From: Felix Lechner <felix.lechner@lease-up.com>
Date: Thu, 30 May 2019 15:02:15 -0700
Subject: [PATCH] Ignore editor backups when running hooks. (#7109)

* Ignore editor backups when running hooks.

When processing hooks, certbot also runs editor backups even though
such files are outdated, clearly warranted correction and may quite
possibly be defective.

That behavior could lead to unexpected breakage, and perhaps even pose
security risks---for example, if a previous script was careless with
file permissions. As an aggravating factor, the backup runs after the
corrected version and could unintentionally override a fix the user
thought was properly implemented.

This commit causes editor backup files ending in tilde (~) to be
excluded when running hooks.

Additional information can be found here:

https://github.com/certbot/certbot/issues/7107
https://community.letsencrypt.org/t/editor-backup-files-executed-as-renewal-hooks/94750

* Add unit test for hook scripts with filenames ending in tilde.

* Provide changelog entry for not running hook scripts ending in tilde.

* Add Felix Lechner to the list of contributors.
---
 AUTHORS.md                 | 1 +
 CHANGELOG.md               | 2 ++
 certbot/hooks.py           | 5 +++--
 certbot/tests/hook_test.py | 6 ++++++
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/AUTHORS.md b/AUTHORS.md
index 6ee739bc0..0e8d88a4d 100644
--- a/AUTHORS.md
+++ b/AUTHORS.md
@@ -75,6 +75,7 @@ Authors
 * [Fabian](https://github.com/faerbit)
 * [Faidon Liambotis](https://github.com/paravoid)
 * [Fan Jiang](https://github.com/tcz001)
+* [Felix Lechner](https://github.com/lechner)
 * [Felix Schwarz](https://github.com/FelixSchwarz)
 * [Felix Yan](https://github.com/felixonmars)
 * [Filip Ochnik](https://github.com/filipochnik)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8669d34a5..83e62c792 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 * Renewal parameter `webroot_path` is always saved, avoiding some regressions
   when `webroot` authenticator plugin is invoked with no challenge to perform.
+* Scripts in Certbot hook directories are no longer executed when their
+  filenames end in a tilde.
 
 Despite us having broken lockstep, we are continuing to release new versions of
 all Certbot components during releases for the time being, however, the only
diff --git a/certbot/hooks.py b/certbot/hooks.py
index 7de846ae4..34e06e0a3 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -266,5 +266,6 @@ def list_hooks(dir_path):
     :rtype: sorted list of absolute paths to executables in dir_path
 
     """
-    paths = (os.path.join(dir_path, f) for f in os.listdir(dir_path))
-    return sorted(path for path in paths if util.is_exe(path))
+    allpaths = (os.path.join(dir_path, f) for f in os.listdir(dir_path))
+    hooks = [path for path in allpaths if util.is_exe(path) and not path.endswith('~')]
+    return sorted(hooks)
diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py
index 2a3742aa2..2ed7d4229 100644
--- a/certbot/tests/hook_test.py
+++ b/certbot/tests/hook_test.py
@@ -480,6 +480,12 @@ class ListHooksTest(util.TempDirTestCase):
 
         self.assertEqual(self._call(self.tempdir), [name])
 
+    def test_ignore_tilde(self):
+        name = os.path.join(self.tempdir, "foo~")
+        create_hook(name)
+
+        self.assertEqual(self._call(self.tempdir), [])
+
 
 def create_hook(file_path):
     """Creates an executable file at the specified path.