From ffe22f980584487d5250bd7dee51b985da5586b3 Mon Sep 17 00:00:00 2001 From: Jonathan Visser Date: Mon, 30 Dec 2024 07:23:41 +0100 Subject: [PATCH 1/5] Add permission error when user cannot write to /etc/nginx folder --- nginx_config_reloader/__init__.py | 6 +++++- nginx_config_reloader/utils.py | 5 +++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/nginx_config_reloader/__init__.py b/nginx_config_reloader/__init__.py index fe93697..0e5389b 100644 --- a/nginx_config_reloader/__init__.py +++ b/nginx_config_reloader/__init__.py @@ -36,7 +36,7 @@ UNPRIVILEGED_UID, WATCH_IGNORE_FILES, ) -from nginx_config_reloader.utils import directory_is_unmounted +from nginx_config_reloader.utils import directory_is_unmounted, can_write_to_main_config_dir logger = logging.getLogger(__name__) dbus_loop: Optional[EventLoop] = None @@ -196,6 +196,10 @@ def _apply(self): logger.debug("Applying new config") if self.check_no_forbidden_config_directives_are_present(): return False + + if not can_write_to_main_config_dir(): + self.logger.error("No write permissions to main nginx config directory, please check your permissions.") + return False if not self.no_magento_config: try: diff --git a/nginx_config_reloader/utils.py b/nginx_config_reloader/utils.py index 42dcbea..428848b 100644 --- a/nginx_config_reloader/utils.py +++ b/nginx_config_reloader/utils.py @@ -1,6 +1,8 @@ import json import subprocess +import os +from nginx_config_reloader.settings import MAIN_CONFIG_DIR def directory_is_unmounted(path): output = subprocess.check_output( @@ -12,3 +14,6 @@ def directory_is_unmounted(path): if unit["description"] == path: return unit["active"] != "active" or unit["sub"] != "mounted" return False + +def can_write_to_main_config_dir(): + return os.access(MAIN_CONFIG_DIR, os.W_OK) From 0c814cb23c1825f8cb030e6752028042ffa88792 Mon Sep 17 00:00:00 2001 From: Jonathan Visser Date: Mon, 30 Dec 2024 12:38:43 +0100 Subject: [PATCH 2/5] Fake main directory in tests, add unit test for checking permissions --- nginx_config_reloader/__init__.py | 8 ++++++-- nginx_config_reloader/utils.py | 5 ----- tests/test_nginx_config_reloader.py | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/nginx_config_reloader/__init__.py b/nginx_config_reloader/__init__.py index 0e5389b..06197a2 100644 --- a/nginx_config_reloader/__init__.py +++ b/nginx_config_reloader/__init__.py @@ -24,6 +24,7 @@ from nginx_config_reloader.settings import ( BACKUP_CONFIG_DIR, CUSTOM_CONFIG_DIR, + MAIN_CONFIG_DIR, DIR_TO_WATCH, ERROR_FILE, FORBIDDEN_CONFIG_REGEX, @@ -36,7 +37,7 @@ UNPRIVILEGED_UID, WATCH_IGNORE_FILES, ) -from nginx_config_reloader.utils import directory_is_unmounted, can_write_to_main_config_dir +from nginx_config_reloader.utils import directory_is_unmounted logger = logging.getLogger(__name__) dbus_loop: Optional[EventLoop] = None @@ -133,6 +134,9 @@ def install_magento_config(self): # Move temporary symlink to actual location, overwriting existing link or file os.rename(MAGENTO_CONF_NEW, MAGENTO_CONF) + def check_can_write_to_main_config_dir(self): + return os.access(MAIN_CONFIG_DIR, os.W_OK) + def check_no_forbidden_config_directives_are_present(self): """ Loop over the :FORBIDDEN_CONFIG_REGEX: to check if nginx config directory contains forbidden configuration @@ -197,7 +201,7 @@ def _apply(self): if self.check_no_forbidden_config_directives_are_present(): return False - if not can_write_to_main_config_dir(): + if not self.check_can_write_to_main_config_dir(): self.logger.error("No write permissions to main nginx config directory, please check your permissions.") return False diff --git a/nginx_config_reloader/utils.py b/nginx_config_reloader/utils.py index 428848b..c8d163f 100644 --- a/nginx_config_reloader/utils.py +++ b/nginx_config_reloader/utils.py @@ -2,8 +2,6 @@ import subprocess import os -from nginx_config_reloader.settings import MAIN_CONFIG_DIR - def directory_is_unmounted(path): output = subprocess.check_output( ["systemctl", "list-units", "-t", "mount", "--all", "-o", "json"], @@ -14,6 +12,3 @@ def directory_is_unmounted(path): if unit["description"] == path: return unit["active"] != "active" or unit["sub"] != "mounted" return False - -def can_write_to_main_config_dir(): - return os.access(MAIN_CONFIG_DIR, os.W_OK) diff --git a/tests/test_nginx_config_reloader.py b/tests/test_nginx_config_reloader.py index 2076f8b..ff05b1c 100644 --- a/tests/test_nginx_config_reloader.py +++ b/tests/test_nginx_config_reloader.py @@ -26,10 +26,12 @@ def setUp(self): self.source = mkdtemp() self.dest = mkdtemp() self.backup = mkdtemp() + self.main = mkdtemp() _, self.mag_conf = mkstemp(text=True) _, self.mag1_conf = mkstemp(text=True) _, self.mag2_conf = mkstemp(text=True) + nginx_config_reloader.MAIN_CONFIG_DIR = self.main nginx_config_reloader.DIR_TO_WATCH = self.source nginx_config_reloader.CUSTOM_CONFIG_DIR = self.dest nginx_config_reloader.BACKUP_CONFIG_DIR = self.backup @@ -49,6 +51,7 @@ def tearDown(self): shutil.rmtree(self.source, ignore_errors=True) shutil.rmtree(self.dest, ignore_errors=True) shutil.rmtree(self.backup, ignore_errors=True) + shutil.rmtree(self.main, ignore_errors=True) for f in [self.mag_conf, self.mag1_conf, self.mag2_conf]: try: os.unlink(f) @@ -646,6 +649,22 @@ def test_permissions_are_masked_for_file_in_subdir(self): & stat.S_IXOTH ) + def test_no_permission_to_main_config_dir(self): + os.chmod(self.main, 0o400) # Read-only + + tm = self._get_nginx_config_reloader_instance() + try: + with self.assertLogs('root', level='ERROR') as cm: + result = tm.apply_new_config() + self.assertFalse(result) + self.assertTrue( + any("No write permissions to main nginx config directory" in message for message in cm.output), + "Expected error message not found in logs." + ) + finally: + # Restore permissions after test + os.chmod(self.main, 0o700) + def _get_nginx_config_reloader_instance( self, no_magento_config=False, From 97357f1135b0d64cb2f0e0999e2e59ad91f53c9a Mon Sep 17 00:00:00 2001 From: Jonathan Visser Date: Mon, 30 Dec 2024 13:36:47 +0100 Subject: [PATCH 3/5] Remove unneeded import --- nginx_config_reloader/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nginx_config_reloader/utils.py b/nginx_config_reloader/utils.py index c8d163f..42dcbea 100644 --- a/nginx_config_reloader/utils.py +++ b/nginx_config_reloader/utils.py @@ -1,6 +1,6 @@ import json import subprocess -import os + def directory_is_unmounted(path): output = subprocess.check_output( From 71c883a507a833021a8119dc5ac92b6a5bdcad09 Mon Sep 17 00:00:00 2001 From: Jonathan Visser Date: Thu, 16 Jan 2025 18:55:41 +0100 Subject: [PATCH 4/5] Change unit test to test the check_can_write_to_main_config_dir function --- tests/test_nginx_config_reloader.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/test_nginx_config_reloader.py b/tests/test_nginx_config_reloader.py index ff05b1c..1bfcc5b 100644 --- a/tests/test_nginx_config_reloader.py +++ b/tests/test_nginx_config_reloader.py @@ -654,13 +654,8 @@ def test_no_permission_to_main_config_dir(self): tm = self._get_nginx_config_reloader_instance() try: - with self.assertLogs('root', level='ERROR') as cm: - result = tm.apply_new_config() - self.assertFalse(result) - self.assertTrue( - any("No write permissions to main nginx config directory" in message for message in cm.output), - "Expected error message not found in logs." - ) + result = tm.check_can_write_to_main_config_dir() + self.assertFalse(result) finally: # Restore permissions after test os.chmod(self.main, 0o700) From 7e4f1caad1921d34c3540fdaa60a944bfca985ad Mon Sep 17 00:00:00 2001 From: Jonathan Visser Date: Thu, 16 Jan 2025 18:58:45 +0100 Subject: [PATCH 5/5] Fix linting issues --- nginx_config_reloader/__init__.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nginx_config_reloader/__init__.py b/nginx_config_reloader/__init__.py index 06197a2..6b98d79 100644 --- a/nginx_config_reloader/__init__.py +++ b/nginx_config_reloader/__init__.py @@ -24,13 +24,13 @@ from nginx_config_reloader.settings import ( BACKUP_CONFIG_DIR, CUSTOM_CONFIG_DIR, - MAIN_CONFIG_DIR, DIR_TO_WATCH, ERROR_FILE, FORBIDDEN_CONFIG_REGEX, MAGENTO1_CONF, MAGENTO2_CONF, MAGENTO_CONF, + MAIN_CONFIG_DIR, NGINX, NGINX_PID_FILE, UNPRIVILEGED_GID, @@ -200,9 +200,11 @@ def _apply(self): logger.debug("Applying new config") if self.check_no_forbidden_config_directives_are_present(): return False - + if not self.check_can_write_to_main_config_dir(): - self.logger.error("No write permissions to main nginx config directory, please check your permissions.") + self.logger.error( + "No write permissions to main nginx config directory, please check your permissions." + ) return False if not self.no_magento_config: