From d6d66de251105b25f4d60d9e3468e056eaacf400 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 13 Jun 2019 17:05:26 -0700 Subject: [PATCH] Set umask used when executing hooks via "umask" option in borgmatic hooks section (#189). --- NEWS | 1 + borgmatic/commands/borgmatic.py | 16 +++++++++++--- borgmatic/config/schema.yaml | 4 ++++ borgmatic/hook.py | 37 ++++++++++++++++++++++++--------- tests/unit/test_hook.py | 20 +++++++++++++----- 5 files changed, 60 insertions(+), 18 deletions(-) diff --git a/NEWS b/NEWS index 2d8c20c..7f16e29 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ customize the log level. See the documentation for more information: https://torsion.org/borgmatic/docs/how-to/inspect-your-backups/ * #178: Look for .yml configuration file extension in addition to .yaml. + * #189: Set umask used when executing hooks via "umask" option in borgmatic hooks section. * Remove Python cache files before each Tox run. * Add #borgmatic Freenode IRC channel to documentation. * Add Borg/borgmatic hosting providers section to documentation. diff --git a/borgmatic/commands/borgmatic.py b/borgmatic/commands/borgmatic.py index 568c435..0b90b75 100644 --- a/borgmatic/commands/borgmatic.py +++ b/borgmatic/commands/borgmatic.py @@ -281,7 +281,11 @@ def run_configuration(config_filename, config, args): # pragma: no cover if args.create: hook.execute_hook( - hooks.get('before_backup'), config_filename, 'pre-backup', args.dry_run + hooks.get('before_backup'), + hooks.get('umask'), + config_filename, + 'pre-backup', + args.dry_run, ) for repository_path in location['repositories']: @@ -298,10 +302,16 @@ def run_configuration(config_filename, config, args): # pragma: no cover if args.create: hook.execute_hook( - hooks.get('after_backup'), config_filename, 'post-backup', args.dry_run + hooks.get('after_backup'), + hooks.get('umask'), + config_filename, + 'post-backup', + args.dry_run, ) except (OSError, CalledProcessError): - hook.execute_hook(hooks.get('on_error'), config_filename, 'on-error', args.dry_run) + hook.execute_hook( + hooks.get('on_error'), hooks.get('umask'), config_filename, 'on-error', args.dry_run + ) raise diff --git a/borgmatic/config/schema.yaml b/borgmatic/config/schema.yaml index e8e56a1..0aca248 100644 --- a/borgmatic/config/schema.yaml +++ b/borgmatic/config/schema.yaml @@ -326,3 +326,7 @@ map: desc: List of one or more shell commands or scripts to execute in case an exception has occurred. example: - echo "Error while creating a backup." + umask: + type: scalar + desc: Umask used when executing hooks. Defaults to the umask that borgmatic is run with. + example: 0077 diff --git a/borgmatic/hook.py b/borgmatic/hook.py index f50767d..899f630 100644 --- a/borgmatic/hook.py +++ b/borgmatic/hook.py @@ -1,4 +1,5 @@ import logging +import os from borgmatic import execute from borgmatic.logger import get_logger @@ -6,10 +7,13 @@ from borgmatic.logger import get_logger logger = get_logger(__name__) -def execute_hook(commands, config_filename, description, dry_run): +def execute_hook(commands, umask, config_filename, description, dry_run): ''' - Given a list of hook commands to execute, a config filename, a hook description, and whether - this is a dry run, run the given commands. Or, don't run them if this is a dry run. + Given a list of hook commands to execute, a umask to execute with (or None), a config filename, + a hook description, and whether this is a dry run, run the given commands. Or, don't run them + if this is a dry run. + + Raise ValueError if the umask cannot be parsed. ''' if not commands: logger.debug('{}: No commands to run for {} hook'.format(config_filename, description)) @@ -28,10 +32,23 @@ def execute_hook(commands, config_filename, description, dry_run): ) ) - for command in commands: - if not dry_run: - execute.execute_command( - [command], - output_log_level=logging.ERROR if description == 'on-error' else logging.WARNING, - shell=True, - ) + if umask: + parsed_umask = int(str(umask), 8) + logger.debug('{}: Set hook umask to {}'.format(config_filename, oct(parsed_umask))) + original_umask = os.umask(parsed_umask) + else: + original_umask = None + + try: + for command in commands: + if not dry_run: + execute.execute_command( + [command], + output_log_level=logging.ERROR + if description == 'on-error' + else logging.WARNING, + shell=True, + ) + finally: + if original_umask: + os.umask(original_umask) diff --git a/tests/unit/test_hook.py b/tests/unit/test_hook.py index b57301a..135e07e 100644 --- a/tests/unit/test_hook.py +++ b/tests/unit/test_hook.py @@ -10,7 +10,7 @@ def test_execute_hook_invokes_each_command(): [':'], output_log_level=logging.WARNING, shell=True ).once() - module.execute_hook([':'], 'config.yaml', 'pre-backup', dry_run=False) + module.execute_hook([':'], None, 'config.yaml', 'pre-backup', dry_run=False) def test_execute_hook_with_multiple_commands_invokes_each_command(): @@ -21,17 +21,27 @@ def test_execute_hook_with_multiple_commands_invokes_each_command(): ['true'], output_log_level=logging.WARNING, shell=True ).once() - module.execute_hook([':', 'true'], 'config.yaml', 'pre-backup', dry_run=False) + module.execute_hook([':', 'true'], None, 'config.yaml', 'pre-backup', dry_run=False) + + +def test_execute_hook_with_umask_sets_that_umask(): + flexmock(module.os).should_receive('umask').with_args(0o77).and_return(0o22).once() + flexmock(module.os).should_receive('umask').with_args(0o22).once() + flexmock(module.execute).should_receive('execute_command').with_args( + [':'], output_log_level=logging.WARNING, shell=True + ) + + module.execute_hook([':'], 77, 'config.yaml', 'pre-backup', dry_run=False) def test_execute_hook_with_dry_run_skips_commands(): flexmock(module.execute).should_receive('execute_command').never() - module.execute_hook([':', 'true'], 'config.yaml', 'pre-backup', dry_run=True) + module.execute_hook([':', 'true'], None, 'config.yaml', 'pre-backup', dry_run=True) def test_execute_hook_with_empty_commands_does_not_raise(): - module.execute_hook([], 'config.yaml', 'post-backup', dry_run=False) + module.execute_hook([], None, 'config.yaml', 'post-backup', dry_run=False) def test_execute_hook_on_error_logs_as_error(): @@ -39,4 +49,4 @@ def test_execute_hook_on_error_logs_as_error(): [':'], output_log_level=logging.ERROR, shell=True ).once() - module.execute_hook([':'], 'config.yaml', 'on-error', dry_run=False) + module.execute_hook([':'], None, 'config.yaml', 'on-error', dry_run=False)