diff --git a/NEWS b/NEWS index 13785b52..e0013d24 100644 --- a/NEWS +++ b/NEWS @@ -1,5 +1,7 @@ 1.9.9.dev0 * #635: Log the repository path or label on every relevant log message, not just some logs. + * #961: Add a passcommand hook so borgmatic can collect the encryption passphrase once and pass it + to Borg multiple times. * #981: Fix a "spot" check file count delta error. * #982: Fix for borgmatic "exclude_patterns" and "exclude_from" recursing into excluded subdirectories. diff --git a/borgmatic/borg/environment.py b/borgmatic/borg/environment.py index 5dc353e5..9244f162 100644 --- a/borgmatic/borg/environment.py +++ b/borgmatic/borg/environment.py @@ -1,5 +1,7 @@ import os +import borgmatic.hooks.dispatch + OPTION_TO_ENVIRONMENT_VARIABLE = { 'borg_base_directory': 'BORG_BASE_DIR', 'borg_config_directory': 'BORG_CONFIG_DIR', @@ -7,7 +9,6 @@ OPTION_TO_ENVIRONMENT_VARIABLE = { 'borg_files_cache_ttl': 'BORG_FILES_CACHE_TTL', 'borg_security_directory': 'BORG_SECURITY_DIR', 'borg_keys_directory': 'BORG_KEYS_DIR', - 'encryption_passcommand': 'BORG_PASSCOMMAND', 'encryption_passphrase': 'BORG_PASSPHRASE', 'ssh_command': 'BORG_RSH', 'temporary_directory': 'TMPDIR', @@ -36,6 +37,24 @@ def make_environment(config): if value: environment[environment_variable_name] = str(value) + passphrase = borgmatic.hooks.dispatch.call_hook( + function_name='load_credential', + config=config, + hook_name='passcommand', + credential_name='encryption_passphrase', + ) + + # If the passcommand produced a passphrase, send it to Borg via an anonymous pipe. + if passphrase: + read_file_descriptor, write_file_descriptor = os.pipe() + os.write(write_file_descriptor, passphrase.encode('utf-8')) + os.close(write_file_descriptor) + + # This, plus subprocess.Popen(..., close_fds=False) in execute.py, is necessary for the Borg + # child process to inherit the file descriptor. + os.set_inheritable(read_file_descriptor, True) + environment['BORG_PASSPHRASE_FD'] = str(read_file_descriptor) + for ( option_name, environment_variable_name, diff --git a/borgmatic/execute.py b/borgmatic/execute.py index 67a6aac3..8d850496 100644 --- a/borgmatic/execute.py +++ b/borgmatic/execute.py @@ -307,6 +307,8 @@ def execute_command( shell=shell, env=environment, cwd=working_directory, + # Necessary for the passcommand credential hook to work. + close_fds=not bool((extra_environment or {}).get('BORG_PASSPHRASE_FD')), ) if not run_to_completion: return process @@ -354,6 +356,8 @@ def execute_command_and_capture_output( shell=shell, env=environment, cwd=working_directory, + # Necessary for the passcommand credential hook to work. + close_fds=not bool((extra_environment or {}).get('BORG_PASSPHRASE_FD')), ) except subprocess.CalledProcessError as error: if ( @@ -414,6 +418,8 @@ def execute_command_with_processes( shell=shell, env=environment, cwd=working_directory, + # Necessary for the passcommand credential hook to work. + close_fds=not bool((extra_environment or {}).get('BORG_PASSPHRASE_FD')), ) except (subprocess.CalledProcessError, OSError): # Something has gone wrong. So vent each process' output buffer to prevent it from hanging. diff --git a/borgmatic/hooks/credential/__init__.py b/borgmatic/hooks/credential/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/borgmatic/hooks/credential/passcommand.py b/borgmatic/hooks/credential/passcommand.py new file mode 100644 index 00000000..d84dacc9 --- /dev/null +++ b/borgmatic/hooks/credential/passcommand.py @@ -0,0 +1,59 @@ +import functools +import logging +import shlex + +import borgmatic.config.paths +import borgmatic.execute + +logger = logging.getLogger(__name__) + + +@functools.cache +def run_passcommand(passcommand, passphrase_configured, working_directory): + ''' + Run the given passcommand using the given working directory and return the passphrase produced + by the command. But bail first if a passphrase is already configured; this mimics Borg's + behavior. + + Cache the results so that the passcommand only needs to run—and potentially prompt the user—once + per borgmatic invocation. + ''' + if passcommand and passphrase_configured: + logger.warning( + 'Ignoring the "encryption_passcommand" option because "encryption_passphrase" is set' + ) + return None + + return borgmatic.execute.execute_command_and_capture_output( + shlex.split(passcommand), + working_directory=working_directory, + ) + + +def load_credential(hook_config, config, credential_name): + ''' + Given the hook configuration dict, the configuration dict, and a credential name to load, call + the configured passcommand to produce and return an encryption passphrase. In effect, we're + doing an end-run around Borg by invoking its passcommand ourselves. This allows us to pass the + resulting passphrase to multiple different Borg invocations without the user having to be + prompted multiple times. + + If no passcommand is configured, then return None. + + The credential name must be "encryption_passphrase"; that's the only supported credential with + this particular hook. + ''' + if credential_name != 'encryption_passphrase': + raise ValueError( + f'Credential name "{credential_name}" is not supported for the passphrase credential hook' + ) + + passcommand = config.get('encryption_passcommand') + + if not passcommand: + return None + + passphrase = config.get('encryption_passphrase') + working_directory = borgmatic.config.paths.get_working_directory(config) + + return run_passcommand(passcommand, bool(passphrase), working_directory) diff --git a/borgmatic/hooks/dispatch.py b/borgmatic/hooks/dispatch.py index d806fa68..26b0832c 100644 --- a/borgmatic/hooks/dispatch.py +++ b/borgmatic/hooks/dispatch.py @@ -3,6 +3,7 @@ import importlib import logging import pkgutil +import borgmatic.hooks.credential import borgmatic.hooks.data_source import borgmatic.hooks.monitoring @@ -10,6 +11,7 @@ logger = logging.getLogger(__name__) class Hook_type(enum.Enum): + CREDENTIAL = 'credential' DATA_SOURCE = 'data_source' MONITORING = 'monitoring' @@ -40,7 +42,11 @@ def call_hook(function_name, config, hook_name, *args, **kwargs): module_name = hook_name.split('_databases')[0] # Probe for a data source or monitoring hook module corresponding to the hook name. - for parent_module in (borgmatic.hooks.data_source, borgmatic.hooks.monitoring): + for parent_module in ( + borgmatic.hooks.credential, + borgmatic.hooks.data_source, + borgmatic.hooks.monitoring, + ): if module_name not in get_submodule_names(parent_module): continue @@ -62,8 +68,8 @@ def call_hook(function_name, config, hook_name, *args, **kwargs): def call_hooks(function_name, config, hook_type, *args, **kwargs): ''' Given a configuration dict, call the requested function of the Python module corresponding to - each hook of the given hook type (either "data_source" or "monitoring"). Supply each call with - the configuration for that hook, and any given args and kwargs. + each hook of the given hook type ("credential", "data_source", or "monitoring"). Supply each + call with the configuration for that hook, and any given args and kwargs. Collect any return values into a dict from module name to return value. Note that the module name is the name of the hook module itself, which might be different from the hook configuration @@ -88,9 +94,9 @@ def call_hooks(function_name, config, hook_type, *args, **kwargs): def call_hooks_even_if_unconfigured(function_name, config, hook_type, *args, **kwargs): ''' Given a configuration dict, call the requested function of the Python module corresponding to - each hook of the given hook type (either "data_source" or "monitoring"). Supply each call with - the configuration for that hook and any given args and kwargs. Collect any return values into a - dict from hook name to return value. + each hook of the given hook type ("credential", "data_source", or "monitoring"). Supply each + call with the configuration for that hook and any given args and kwargs. Collect any return + values into a dict from hook name to return value. Raise AttributeError if the function name is not found in the module. Raise anything else that a called function raises. An error stops calls to subsequent functions. diff --git a/docs/how-to/provide-your-passwords.md b/docs/how-to/provide-your-passwords.md index 57803864..60aa33b7 100644 --- a/docs/how-to/provide-your-passwords.md +++ b/docs/how-to/provide-your-passwords.md @@ -24,11 +24,30 @@ or security reasons, read on. borgmatic supports calling another application such as a password manager to obtain the Borg passphrase to a repository. -For example, to ask the *Pass* password manager to provide the passphrase: +For example, to ask the [Pass](https://www.passwordstore.org/) password manager +to provide the passphrase: + ```yaml -encryption_passcommand: pass path/to/borg-repokey +encryption_passcommand: pass path/to/borg-passphrase ``` +Another example for [KeePassXC](https://keepassxc.org/): + +```yaml +encryption_passcommand: keepassxc-cli show --show-protected --attributes Password credentials.kdbx borg_passphrase +``` + +... where `borg_passphrase` is the title of the KeePassXC entry containing your +Borg encryption passphrase in its `Password` field. + +New in version 1.9.9 Instead of +letting Borg run the passcommand—potentially multiple times since borgmatic runs +Borg multiple times—borgmatic now runs the passcommand itself and passes the +resulting passprhase securely to Borg via an anonymous pipe. This means you +should only ever get prompted for your password manager's passphrase at most +once per borgmatic run. + + ### Using systemd service credentials Borgmatic supports using encrypted [credentials](https://systemd.io/CREDENTIALS/). diff --git a/docs/reference/source-code.md b/docs/reference/source-code.md index 5eab1a90..e15482e6 100644 --- a/docs/reference/source-code.md +++ b/docs/reference/source-code.md @@ -18,6 +18,7 @@ you get started. Starting at the top level, we have: * [commands](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/main/borgmatic/commands): Looking to add a new flag or action? Start here. This contains borgmatic's entry point, argument parsing, and shell completion. * [config](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/main/borgmatic/config): Code responsible for loading, normalizing, and validating borgmatic's configuration. Interested in adding a new configuration option? Check out `schema.yaml` here. * [hooks](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/main/borgmatic/hooks): Looking to add a new database, filesystem, or monitoring integration? Start here. + * [credential](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/main/borgmatic/hooks/credential): Credential hooks—for loading passphrases and secrets from external providers. * [data_source](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/main/borgmatic/hooks/data_source): Database and filesystem hooks—anything that produces data or files to go into a backup archive. * [monitoring](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/main/borgmatic/hooks/monitoring): Monitoring hooks—integrations with third-party or self-hosted monitoring services. * [docs](https://projects.torsion.org/borgmatic-collective/borgmatic/src/branch/main/docs): How-to and reference documentation, including the document you're reading now. diff --git a/tests/end-to-end/hooks/credential/__init__.py b/tests/end-to-end/hooks/credential/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/end-to-end/hooks/credential/test_passcommand.py b/tests/end-to-end/hooks/credential/test_passcommand.py new file mode 100644 index 00000000..e6c1c810 --- /dev/null +++ b/tests/end-to-end/hooks/credential/test_passcommand.py @@ -0,0 +1,65 @@ +import json +import os +import shutil +import subprocess +import sys +import tempfile + +import pytest + + +def generate_configuration(config_path, repository_path): + ''' + Generate borgmatic configuration into a file at the config path, and update the defaults so as + to work for testing, including updating the source directories, injecting the given repository + path, and tacking on an encryption passcommand. + ''' + subprocess.check_call(f'borgmatic config generate --destination {config_path}'.split(' ')) + config = ( + open(config_path) + .read() + .replace('ssh://user@backupserver/./sourcehostname.borg', repository_path) + .replace('- path: /mnt/backup', '') + .replace('label: local', '') + .replace('- /home/user/path with spaces', '') + .replace('- /home', f'- {config_path}') + .replace('- /etc', '') + .replace('- /var/log/syslog*', '') + + '\nencryption_passcommand: "echo test"' + ) + config_file = open(config_path, 'w') + config_file.write(config) + config_file.close() + + +def test_borgmatic_command(): + # Create a Borg repository. + temporary_directory = tempfile.mkdtemp() + repository_path = os.path.join(temporary_directory, 'test.borg') + extract_path = os.path.join(temporary_directory, 'extract') + + original_working_directory = os.getcwd() + os.mkdir(extract_path) + os.chdir(extract_path) + + try: + config_path = os.path.join(temporary_directory, 'test.yaml') + generate_configuration(config_path, repository_path) + + subprocess.check_call( + f'borgmatic -v 2 --config {config_path} repo-create --encryption repokey'.split(' ') + ) + + # Run borgmatic to generate a backup archive, and then list it to make sure it exists. + subprocess.check_call(f'borgmatic -v 2 --config {config_path}'.split(' ')) + output = subprocess.check_output( + f'borgmatic --config {config_path} list --json'.split(' ') + ).decode(sys.stdout.encoding) + parsed_output = json.loads(output) + + assert len(parsed_output) == 1 + assert len(parsed_output[0]['archives']) == 1 + archive_name = parsed_output[0]['archives'][0]['archive'] + finally: + os.chdir(original_working_directory) + shutil.rmtree(temporary_directory) diff --git a/tests/unit/borg/test_environment.py b/tests/unit/borg/test_environment.py index 30b45746..33b4417e 100644 --- a/tests/unit/borg/test_environment.py +++ b/tests/unit/borg/test_environment.py @@ -3,13 +3,22 @@ from flexmock import flexmock from borgmatic.borg import environment as module -def test_make_environment_with_passcommand_should_set_environment(): +def test_make_environment_with_passcommand_should_call_it_and_set_passphrase_file_descriptor_in_environment(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return('passphrase') + flexmock(module.os).should_receive('pipe').and_return((3, 4)) + flexmock(module.os).should_receive('write') + flexmock(module.os).should_receive('close') + flexmock(module.os).should_receive('set_inheritable') + environment = module.make_environment({'encryption_passcommand': 'command'}) - assert environment.get('BORG_PASSCOMMAND') == 'command' + assert not environment.get('BORG_PASSCOMMAND') + assert environment.get('BORG_PASSPHRASE_FD') == '3' def test_make_environment_with_passphrase_should_set_environment(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None) + flexmock(module.os).should_receive('pipe').never() flexmock(module.os.environ).should_receive('get').and_return(None) environment = module.make_environment({'encryption_passphrase': 'pass'}) @@ -17,6 +26,8 @@ def test_make_environment_with_passphrase_should_set_environment(): def test_make_environment_with_ssh_command_should_set_environment(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None) + flexmock(module.os).should_receive('pipe').never() flexmock(module.os.environ).should_receive('get').and_return(None) environment = module.make_environment({'ssh_command': 'ssh -C'}) @@ -24,6 +35,8 @@ def test_make_environment_with_ssh_command_should_set_environment(): def test_make_environment_without_configuration_sets_certain_environment_variables(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None) + flexmock(module.os).should_receive('pipe').never() flexmock(module.os.environ).should_receive('get').and_return(None) environment = module.make_environment({}) @@ -36,6 +49,8 @@ def test_make_environment_without_configuration_sets_certain_environment_variabl def test_make_environment_without_configuration_does_not_set_certain_environment_variables_if_already_set(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None) + flexmock(module.os).should_receive('pipe').never() flexmock(module.os.environ).should_receive('get').with_args( 'BORG_RELOCATED_REPO_ACCESS_IS_OK' ).and_return('yup') @@ -48,6 +63,8 @@ def test_make_environment_without_configuration_does_not_set_certain_environment def test_make_environment_with_relocated_repo_access_true_should_set_environment_yes(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None) + flexmock(module.os).should_receive('pipe').never() flexmock(module.os.environ).should_receive('get').and_return(None) environment = module.make_environment({'relocated_repo_access_is_ok': True}) @@ -55,6 +72,8 @@ def test_make_environment_with_relocated_repo_access_true_should_set_environment def test_make_environment_with_relocated_repo_access_false_should_set_environment_no(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None) + flexmock(module.os).should_receive('pipe').never() flexmock(module.os.environ).should_receive('get').and_return(None) environment = module.make_environment({'relocated_repo_access_is_ok': False}) @@ -62,6 +81,8 @@ def test_make_environment_with_relocated_repo_access_false_should_set_environmen def test_make_environment_check_i_know_what_i_am_doing_true_should_set_environment_YES(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None) + flexmock(module.os).should_receive('pipe').never() flexmock(module.os.environ).should_receive('get').and_return(None) environment = module.make_environment({'check_i_know_what_i_am_doing': True}) @@ -69,6 +90,8 @@ def test_make_environment_check_i_know_what_i_am_doing_true_should_set_environme def test_make_environment_check_i_know_what_i_am_doing_false_should_set_environment_NO(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None) + flexmock(module.os).should_receive('pipe').never() flexmock(module.os.environ).should_receive('get').and_return(None) environment = module.make_environment({'check_i_know_what_i_am_doing': False}) @@ -76,6 +99,8 @@ def test_make_environment_check_i_know_what_i_am_doing_false_should_set_environm def test_make_environment_with_integer_variable_value(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(None) + flexmock(module.os).should_receive('pipe').never() flexmock(module.os.environ).should_receive('get').and_return(None) environment = module.make_environment({'borg_files_cache_ttl': 40}) assert environment.get('BORG_FILES_CACHE_TTL') == '40' diff --git a/tests/unit/hooks/credential/__init__.py b/tests/unit/hooks/credential/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/unit/hooks/credential/test_passcommand.py b/tests/unit/hooks/credential/test_passcommand.py new file mode 100644 index 00000000..be67b368 --- /dev/null +++ b/tests/unit/hooks/credential/test_passcommand.py @@ -0,0 +1,65 @@ +import pytest +from flexmock import flexmock + +from borgmatic.hooks.credential import passcommand as module + + +def test_run_passcommand_with_passphrase_configured_bails(): + flexmock(module.borgmatic.execute).should_receive('execute_command_and_capture_output').never() + + assert ( + module.run_passcommand('passcommand', passphrase_configured=True, working_directory=None) + is None + ) + + +def test_run_passcommand_without_passphrase_configured_executes_passcommand(): + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output' + ).and_return('passphrase').once() + + assert ( + module.run_passcommand('passcommand', passphrase_configured=False, working_directory=None) + == 'passphrase' + ) + + +def test_load_credential_with_unknown_credential_name_errors(): + with pytest.raises(ValueError): + module.load_credential(hook_config={}, config={}, credential_name='wtf') + + +def test_load_credential_with_configured_passcommand_runs_it(): + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( + '/working' + ) + flexmock(module).should_receive('run_passcommand').with_args( + 'command', False, '/working' + ).and_return('passphrase').once() + + assert ( + module.load_credential( + hook_config={}, + config={'encryption_passcommand': 'command'}, + credential_name='encryption_passphrase', + ) + == 'passphrase' + ) + + +def test_load_credential_with_configured_passphrase_and_passcommand_detects_passphrase(): + flexmock(module.borgmatic.config.paths).should_receive('get_working_directory').and_return( + '/working' + ) + flexmock(module).should_receive('run_passcommand').with_args( + 'command', True, '/working' + ).and_return(None).once() + + assert ( + module.load_credential( + hook_config={}, + config={'encryption_passphrase': 'passphrase', 'encryption_passcommand': 'command'}, + credential_name='encryption_passphrase', + ) + is None + ) diff --git a/tests/unit/hooks/test_dispatch.py b/tests/unit/hooks/test_dispatch.py index 80f8c046..8f3f8ecf 100644 --- a/tests/unit/hooks/test_dispatch.py +++ b/tests/unit/hooks/test_dispatch.py @@ -17,6 +17,9 @@ def test_call_hook_invokes_module_function_with_arguments_and_returns_value(): config = {'super_hook': flexmock(), 'other_hook': flexmock()} expected_return_value = flexmock() test_module = sys.modules[__name__] + flexmock(module).should_receive('get_submodule_names').with_args( + module.borgmatic.hooks.credential + ).and_return(['other_hook']) flexmock(module).should_receive('get_submodule_names').with_args( module.borgmatic.hooks.data_source ).and_return(['other_hook']) @@ -39,6 +42,9 @@ def test_call_hook_probes_config_with_databases_suffix(): config = {'super_hook_databases': flexmock(), 'other_hook': flexmock()} expected_return_value = flexmock() test_module = sys.modules[__name__] + flexmock(module).should_receive('get_submodule_names').with_args( + module.borgmatic.hooks.credential + ).and_return(['other_hook']) flexmock(module).should_receive('get_submodule_names').with_args( module.borgmatic.hooks.data_source ).and_return(['other_hook']) @@ -61,6 +67,9 @@ def test_call_hook_strips_databases_suffix_from_hook_name(): config = {'super_hook_databases': flexmock(), 'other_hook_databases': flexmock()} expected_return_value = flexmock() test_module = sys.modules[__name__] + flexmock(module).should_receive('get_submodule_names').with_args( + module.borgmatic.hooks.credential + ).and_return(['other_hook']) flexmock(module).should_receive('get_submodule_names').with_args( module.borgmatic.hooks.data_source ).and_return(['other_hook']) @@ -83,6 +92,9 @@ def test_call_hook_without_hook_config_invokes_module_function_with_arguments_an config = {'other_hook': flexmock()} expected_return_value = flexmock() test_module = sys.modules[__name__] + flexmock(module).should_receive('get_submodule_names').with_args( + module.borgmatic.hooks.credential + ).and_return(['other_hook']) flexmock(module).should_receive('get_submodule_names').with_args( module.borgmatic.hooks.data_source ).and_return(['other_hook']) @@ -104,6 +116,9 @@ def test_call_hook_without_hook_config_invokes_module_function_with_arguments_an def test_call_hook_without_corresponding_module_raises(): config = {'super_hook': flexmock(), 'other_hook': flexmock()} test_module = sys.modules[__name__] + flexmock(module).should_receive('get_submodule_names').with_args( + module.borgmatic.hooks.credential + ).and_return(['other_hook']) flexmock(module).should_receive('get_submodule_names').with_args( module.borgmatic.hooks.data_source ).and_return(['other_hook']) @@ -121,6 +136,9 @@ def test_call_hook_without_corresponding_module_raises(): def test_call_hook_skips_non_hook_modules(): config = {'not_a_hook': flexmock(), 'other_hook': flexmock()} + flexmock(module).should_receive('get_submodule_names').with_args( + module.borgmatic.hooks.credential + ).and_return(['other_hook']) flexmock(module).should_receive('get_submodule_names').with_args( module.borgmatic.hooks.data_source ).and_return(['other_hook']) diff --git a/tests/unit/test_execute.py b/tests/unit/test_execute.py index 7741526f..8de49912 100644 --- a/tests/unit/test_execute.py +++ b/tests/unit/test_execute.py @@ -185,6 +185,7 @@ def test_execute_command_calls_full_command(): shell=False, env=None, cwd=None, + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -207,6 +208,7 @@ def test_execute_command_calls_full_command_with_output_file(): shell=False, env=None, cwd=None, + close_fds=True, ).and_return(flexmock(stderr=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -221,7 +223,14 @@ def test_execute_command_calls_full_command_without_capturing_output(): flexmock(module).should_receive('log_command') flexmock(module.os, environ={'a': 'b'}) flexmock(module.subprocess).should_receive('Popen').with_args( - full_command, stdin=None, stdout=None, stderr=None, shell=False, env=None, cwd=None + full_command, + stdin=None, + stdout=None, + stderr=None, + shell=False, + env=None, + cwd=None, + close_fds=True, ).and_return(flexmock(wait=lambda: 0)).once() flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.SUCCESS) flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) @@ -245,6 +254,7 @@ def test_execute_command_calls_full_command_with_input_file(): shell=False, env=None, cwd=None, + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -266,6 +276,7 @@ def test_execute_command_calls_full_command_with_shell(): shell=True, env=None, cwd=None, + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -287,6 +298,7 @@ def test_execute_command_calls_full_command_with_extra_environment(): shell=False, env={'a': 'b', 'c': 'd'}, cwd=None, + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -308,6 +320,7 @@ def test_execute_command_calls_full_command_with_working_directory(): shell=False, env=None, cwd='/working', + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -317,6 +330,28 @@ def test_execute_command_calls_full_command_with_working_directory(): assert output is None +def test_execute_command_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open(): + full_command = ['foo', 'bar'] + flexmock(module).should_receive('log_command') + flexmock(module.os, environ={'a': 'b'}) + flexmock(module.subprocess).should_receive('Popen').with_args( + full_command, + stdin=None, + stdout=module.subprocess.PIPE, + stderr=module.subprocess.STDOUT, + shell=False, + env={'a': 'b', 'BORG_PASSPHRASE_FD': '4'}, + cwd=None, + close_fds=False, + ).and_return(flexmock(stdout=None)).once() + flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) + flexmock(module).should_receive('log_outputs') + + output = module.execute_command(full_command, extra_environment={'BORG_PASSPHRASE_FD': '4'}) + + assert output is None + + def test_execute_command_without_run_to_completion_returns_process(): full_command = ['foo', 'bar'] process = flexmock() @@ -330,6 +365,7 @@ def test_execute_command_without_run_to_completion_returns_process(): shell=False, env=None, cwd=None, + close_fds=True, ).and_return(process).once() flexmock(module.borgmatic.logger).should_receive('Log_prefix').and_return(flexmock()) flexmock(module).should_receive('log_outputs') @@ -343,7 +379,12 @@ def test_execute_command_and_capture_output_returns_stdout(): flexmock(module).should_receive('log_command') flexmock(module.os, environ={'a': 'b'}) flexmock(module.subprocess).should_receive('check_output').with_args( - full_command, stderr=None, shell=False, env=None, cwd=None + full_command, + stderr=None, + shell=False, + env=None, + cwd=None, + close_fds=True, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output(full_command) @@ -357,7 +398,12 @@ def test_execute_command_and_capture_output_with_capture_stderr_returns_stderr() flexmock(module).should_receive('log_command') flexmock(module.os, environ={'a': 'b'}) flexmock(module.subprocess).should_receive('check_output').with_args( - full_command, stderr=module.subprocess.STDOUT, shell=False, env=None, cwd=None + full_command, + stderr=module.subprocess.STDOUT, + shell=False, + env=None, + cwd=None, + close_fds=True, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output(full_command, capture_stderr=True) @@ -372,7 +418,12 @@ def test_execute_command_and_capture_output_returns_output_when_process_error_is flexmock(module).should_receive('log_command') flexmock(module.os, environ={'a': 'b'}) flexmock(module.subprocess).should_receive('check_output').with_args( - full_command, stderr=None, shell=False, env=None, cwd=None + full_command, + stderr=None, + shell=False, + env=None, + cwd=None, + close_fds=True, ).and_raise(subprocess.CalledProcessError(1, full_command, err_output)).once() flexmock(module).should_receive('interpret_exit_code').and_return( module.Exit_status.SUCCESS @@ -389,7 +440,12 @@ def test_execute_command_and_capture_output_raises_when_command_errors(): flexmock(module).should_receive('log_command') flexmock(module.os, environ={'a': 'b'}) flexmock(module.subprocess).should_receive('check_output').with_args( - full_command, stderr=None, shell=False, env=None, cwd=None + full_command, + stderr=None, + shell=False, + env=None, + cwd=None, + close_fds=True, ).and_raise(subprocess.CalledProcessError(2, full_command, expected_output)).once() flexmock(module).should_receive('interpret_exit_code').and_return( module.Exit_status.ERROR @@ -405,7 +461,12 @@ def test_execute_command_and_capture_output_returns_output_with_shell(): flexmock(module).should_receive('log_command') flexmock(module.os, environ={'a': 'b'}) flexmock(module.subprocess).should_receive('check_output').with_args( - 'foo bar', stderr=None, shell=True, env=None, cwd=None + 'foo bar', + stderr=None, + shell=True, + env=None, + cwd=None, + close_fds=True, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output(full_command, shell=True) @@ -424,6 +485,7 @@ def test_execute_command_and_capture_output_returns_output_with_extra_environmen shell=False, env={'a': 'b', 'c': 'd'}, cwd=None, + close_fds=True, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output( @@ -439,7 +501,12 @@ def test_execute_command_and_capture_output_returns_output_with_working_director flexmock(module).should_receive('log_command') flexmock(module.os, environ={'a': 'b'}) flexmock(module.subprocess).should_receive('check_output').with_args( - full_command, stderr=None, shell=False, env=None, cwd='/working' + full_command, + stderr=None, + shell=False, + env=None, + cwd='/working', + close_fds=True, ).and_return(flexmock(decode=lambda: expected_output)).once() output = module.execute_command_and_capture_output( @@ -449,6 +516,29 @@ def test_execute_command_and_capture_output_returns_output_with_working_director assert output == expected_output +def test_execute_command_and_capture_output_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open(): + full_command = ['foo', 'bar'] + expected_output = '[]' + flexmock(module).should_receive('log_command') + flexmock(module.os, environ={'a': 'b'}) + flexmock(module.subprocess).should_receive('check_output').with_args( + full_command, + stderr=None, + shell=False, + env={'a': 'b', 'BORG_PASSPHRASE_FD': '4'}, + cwd=None, + close_fds=False, + ).and_return(flexmock(decode=lambda: expected_output)).once() + + output = module.execute_command_and_capture_output( + full_command, + shell=False, + extra_environment={'BORG_PASSPHRASE_FD': '4'}, + ) + + assert output == expected_output + + def test_execute_command_with_processes_calls_full_command(): full_command = ['foo', 'bar'] processes = (flexmock(),) @@ -462,6 +552,7 @@ def test_execute_command_with_processes_calls_full_command(): shell=False, env=None, cwd=None, + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module).should_receive('log_outputs') @@ -484,6 +575,7 @@ def test_execute_command_with_processes_returns_output_with_output_log_level_non shell=False, env=None, cwd=None, + close_fds=True, ).and_return(process).once() flexmock(module).should_receive('log_outputs').and_return({process: 'out'}) @@ -506,6 +598,7 @@ def test_execute_command_with_processes_calls_full_command_with_output_file(): shell=False, env=None, cwd=None, + close_fds=True, ).and_return(flexmock(stderr=None)).once() flexmock(module).should_receive('log_outputs') @@ -520,7 +613,14 @@ def test_execute_command_with_processes_calls_full_command_without_capturing_out flexmock(module).should_receive('log_command') flexmock(module.os, environ={'a': 'b'}) flexmock(module.subprocess).should_receive('Popen').with_args( - full_command, stdin=None, stdout=None, stderr=None, shell=False, env=None, cwd=None + full_command, + stdin=None, + stdout=None, + stderr=None, + shell=False, + env=None, + cwd=None, + close_fds=True, ).and_return(flexmock(wait=lambda: 0)).once() flexmock(module).should_receive('interpret_exit_code').and_return(module.Exit_status.SUCCESS) flexmock(module).should_receive('log_outputs') @@ -546,6 +646,7 @@ def test_execute_command_with_processes_calls_full_command_with_input_file(): shell=False, env=None, cwd=None, + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module).should_receive('log_outputs') @@ -567,6 +668,7 @@ def test_execute_command_with_processes_calls_full_command_with_shell(): shell=True, env=None, cwd=None, + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module).should_receive('log_outputs') @@ -588,6 +690,7 @@ def test_execute_command_with_processes_calls_full_command_with_extra_environmen shell=False, env={'a': 'b', 'c': 'd'}, cwd=None, + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module).should_receive('log_outputs') @@ -611,6 +714,7 @@ def test_execute_command_with_processes_calls_full_command_with_working_director shell=False, env=None, cwd='/working', + close_fds=True, ).and_return(flexmock(stdout=None)).once() flexmock(module).should_receive('log_outputs') @@ -621,6 +725,32 @@ def test_execute_command_with_processes_calls_full_command_with_working_director assert output is None +def test_execute_command_with_processes_with_BORG_PASSPHRASE_FD_leaves_file_descriptors_open(): + full_command = ['foo', 'bar'] + processes = (flexmock(),) + flexmock(module).should_receive('log_command') + flexmock(module.os, environ={'a': 'b'}) + flexmock(module.subprocess).should_receive('Popen').with_args( + full_command, + stdin=None, + stdout=module.subprocess.PIPE, + stderr=module.subprocess.STDOUT, + shell=False, + env={'a': 'b', 'BORG_PASSPHRASE_FD': '4'}, + cwd=None, + close_fds=False, + ).and_return(flexmock(stdout=None)).once() + flexmock(module).should_receive('log_outputs') + + output = module.execute_command_with_processes( + full_command, + processes, + extra_environment={'BORG_PASSPHRASE_FD': '4'}, + ) + + assert output is None + + def test_execute_command_with_processes_kills_processes_on_error(): full_command = ['foo', 'bar'] flexmock(module).should_receive('log_command') @@ -637,6 +767,7 @@ def test_execute_command_with_processes_kills_processes_on_error(): shell=False, env=None, cwd=None, + close_fds=True, ).and_raise(subprocess.CalledProcessError(1, full_command, 'error')).once() flexmock(module).should_receive('log_outputs').never()