From 8ba444990b4af6eec4b6b2b26482b107d7ff1229 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Sat, 13 Dec 2025 21:54:14 +0530 Subject: [PATCH] backups: Set proper permissions for backups-data directory Fixes: #2554 - Update permissions on the backups-data directory so that files are only accessible by root users. - Ensure that the directory is created by the 'backups' app and not by each of the apps that take the backup. Tests: - Run functional tests for miniflux, dynamicdns, wordpress, zoph, and nextlcoud. There was an unrelated functional test case failure in nextcloud. - On a fresh installation, apply patch. Service is restarted. The directory is created with proper permissions and ownership. - On a fresh installation, without the patch. Backup the dynamicdns app. The directory is created with incorrect permissions. Apply the patch. Service is restarted. Proper permissions are set on the directory. - On a setup with incorrect permissions, re-run backups app's setup. The permissions are updated correctly. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- plinth/db/postgres.py | 1 - plinth/modules/backups/__init__.py | 2 +- plinth/modules/backups/privileged.py | 14 +++++++++++++- plinth/modules/nextcloud/privileged.py | 2 -- plinth/modules/wordpress/privileged.py | 1 - plinth/modules/zoph/privileged.py | 2 -- 6 files changed, 14 insertions(+), 8 deletions(-) diff --git a/plinth/db/postgres.py b/plinth/db/postgres.py index 1ddf618d6..2da14d950 100644 --- a/plinth/db/postgres.py +++ b/plinth/db/postgres.py @@ -93,7 +93,6 @@ def dump_database(backup_file: str | pathlib.Path, database_name: str): file if it exists. """ backup_path = pathlib.Path(backup_file) - backup_path.parent.mkdir(parents=True, exist_ok=True) with action_utils.service_ensure_running('postgresql'): with open(backup_path, 'w', encoding='utf-8') as file_handle: _run_as([ diff --git a/plinth/modules/backups/__init__.py b/plinth/modules/backups/__init__.py index 66a231110..bd2d617bc 100644 --- a/plinth/modules/backups/__init__.py +++ b/plinth/modules/backups/__init__.py @@ -33,7 +33,7 @@ class BackupsApp(app_module.App): app_id = 'backups' - _version = 3 + _version = 4 can_be_disabled = False diff --git a/plinth/modules/backups/privileged.py b/plinth/modules/backups/privileged.py index a2276ead4..78d49223b 100644 --- a/plinth/modules/backups/privileged.py +++ b/plinth/modules/backups/privileged.py @@ -211,6 +211,8 @@ def is_mounted(mount_point: str) -> bool: @privileged def setup(path: str): """Create repository if it does not already exist.""" + _create_backup_data_directory() + try: _run(['borg', 'info', path], check=True) except subprocess.CalledProcessError: @@ -221,6 +223,17 @@ def setup(path: str): _init_repository(path, encryption='none') +def _create_backup_data_directory(): + """Create the backups-data with proper permissions.""" + old_umask = os.umask(0o077) + try: + BACKUPS_DATA_PATH.mkdir(exist_ok=True) + BACKUPS_DATA_PATH.chmod(0o700) + shutil.chown(BACKUPS_DATA_PATH, 'root', 'root') + finally: + os.umask(old_umask) + + def _init_repository(path: str, encryption: str, encryption_passphrase: str | None = None): """Initialize a local or remote borg repository.""" @@ -464,7 +477,6 @@ def _assert_app_id(app_id): def dump_settings(app_id: str, settings: dict[str, int | float | bool | str]): """Dump an app's settings to a JSON file.""" _assert_app_id(app_id) - BACKUPS_DATA_PATH.mkdir(exist_ok=True) settings_path = BACKUPS_DATA_PATH / f'{app_id}-settings.json' settings_path.write_text(json.dumps(settings)) diff --git a/plinth/modules/nextcloud/privileged.py b/plinth/modules/nextcloud/privileged.py index 4a373f203..1bfdd6bb5 100644 --- a/plinth/modules/nextcloud/privileged.py +++ b/plinth/modules/nextcloud/privileged.py @@ -355,8 +355,6 @@ def _maintenance_mode(): @privileged def dump_database(): """Dump database to file.""" - DB_BACKUP_FILE.parent.mkdir(parents=True, exist_ok=True) - with _maintenance_mode(): with DB_BACKUP_FILE.open('w', encoding='utf-8') as file_handle: action_utils.run([ diff --git a/plinth/modules/wordpress/privileged.py b/plinth/modules/wordpress/privileged.py index 23fd7b994..dbbd34255 100644 --- a/plinth/modules/wordpress/privileged.py +++ b/plinth/modules/wordpress/privileged.py @@ -142,7 +142,6 @@ def is_public() -> bool: @privileged def dump_database(): """Dump database to file.""" - _db_backup_file.parent.mkdir(parents=True, exist_ok=True) with action_utils.service_ensure_running('mysql'): with _db_backup_file.open('w', encoding='utf-8') as file_handle: action_utils.run([ diff --git a/plinth/modules/zoph/privileged.py b/plinth/modules/zoph/privileged.py index b63e7f960..4198d7b18 100644 --- a/plinth/modules/zoph/privileged.py +++ b/plinth/modules/zoph/privileged.py @@ -2,7 +2,6 @@ """Configuration helper for Zoph server.""" import configparser -import os import pathlib import re import subprocess @@ -159,7 +158,6 @@ def dump_database(): """ with action_utils.service_ensure_running('mysql'): db_name = _get_db_config()['db_name'] - os.makedirs(os.path.dirname(DB_BACKUP_FILE), exist_ok=True) with open(DB_BACKUP_FILE, 'w', encoding='utf-8') as db_backup_file: action_utils.run(['mysqldump', db_name], stdout=db_backup_file, check=True)