Merge branch 'main' into config-command-line.

2025-03-25 22:39:51 -07:00 · 2025-03-25 22:39:51 -07:00 · 5a0430b9c8
commit 5a0430b9c8
parent 65b1d8e8b2 23efbb8df3
8 changed files with 236 additions and 7 deletions
--- a/2
+++ b/2
@ -7,6 +7,7 @@
   applies to the "repo-create" action, serving as a default for the "--encryption" flag. See the
   documentation for more information: https://torsion.org/borgmatic/docs/reference/configuration/
 * #345: Add a "key import" action to import a repository key from backup.
+ * #422: Add home directory expansion to file-based and KeePassXC credential hooks.
 * #790, #821: Deprecate all "before_*", "after_*" and "on_error" command hooks in favor of more
   flexible "commands:". See the documentation for more information:
   https://torsion.org/borgmatic/docs/how-to/add-preparation-and-cleanup-steps-to-backups/
@ -16,6 +17,7 @@
 * #790: BREAKING: Run all command hooks (both new and deprecated) respecting the
   "working_directory" option if configured, meaning that hook commands are run in that directory.
 * #836: Add a custom command option for the SQLite hook.
+ * #837: Add custom command options for the MongoDB hook.
 * #1010: When using Borg 2, don't pass the "--stats" flag to "borg prune".
 * #1020: Document a database use case involving a temporary database client container:
   https://torsion.org/borgmatic/docs/how-to/backup-your-databases/#containers
--- a/borgmatic/config/schema.yaml
+++ b/borgmatic/config/schema.yaml
@ -1834,6 +1834,25 @@ properties:
                        dump command, without performing any validation on them.
                        See mongorestore documentation for details.
                    example: --restoreDbUsersAndRoles
+                mongodump_command:
+                    type: string
+                    description: |
+                        Command to use instead of "mongodump". This can be used
+                        to run a specific mongodump version (e.g., one inside a
+                        running container). If you run it from within a
+                        container, make sure to mount the path in the
+                        "user_runtime_directory" option from the host into the
+                        container at the same location.  Defaults to
+                        "mongodump".
+                    example: docker exec mongodb_container mongodump
+                mongorestore_command:
+                    type: string
+                    description: |
+                        Command to run when restoring a database instead of
+                        "mongorestore". This can be used to run a specific
+                        mongorestore version (e.g., one inside a running
+                        container). Defaults to "mongorestore".
+                    example: docker exec mongodb_container mongorestore
        description: |
            List of one or more MongoDB databases to dump before creating a
            backup, run once per configuration file. The database dumps are
--- a/borgmatic/hooks/credential/file.py
+++ b/borgmatic/hooks/credential/file.py
@ -19,9 +19,11 @@ def load_credential(hook_config, config, credential_parameters):

        raise ValueError(f'Cannot load invalid credential: "{name}"')

+    expanded_credential_path = os.path.expanduser(credential_path)
+
    try:
        with open(
-            os.path.join(config.get('working_directory', ''), credential_path)
+            os.path.join(config.get('working_directory', ''), expanded_credential_path)
        ) as credential_file:
            return credential_file.read().rstrip(os.linesep)
    except (FileNotFoundError, OSError) as error:
--- a/borgmatic/hooks/credential/keepassxc.py
+++ b/borgmatic/hooks/credential/keepassxc.py
@ -24,7 +24,9 @@ def load_credential(hook_config, config, credential_parameters):
            f'Cannot load credential with invalid KeePassXC database path and attribute name: "{path_and_name}"'
        )

-    if not os.path.exists(database_path):
+    expanded_database_path = os.path.expanduser(database_path)
+
+    if not os.path.exists(expanded_database_path):
        raise ValueError(
            f'Cannot load credential because KeePassXC database path does not exist: {database_path}'
        )
@ -36,7 +38,7 @@ def load_credential(hook_config, config, credential_parameters):
            '--show-protected',
            '--attributes',
            'Password',
-            database_path,
+            expanded_database_path,
            attribute_name,
        )
    ).rstrip(os.linesep)
--- a/borgmatic/hooks/data_source/mongodb.py
+++ b/borgmatic/hooks/data_source/mongodb.py
@ -114,14 +114,17 @@ def make_password_config_file(password):

 def build_dump_command(database, config, dump_filename, dump_format):
    '''
-    Return the mongodump command from a single database configuration.
+    Return the custom mongodump_command from a single database configuration.
    '''
    all_databases = database['name'] == 'all'

    password = borgmatic.hooks.credential.parse.resolve_credential(database.get('password'), config)

+    dump_command = tuple(
+        shlex.quote(part) for part in shlex.split(database.get('mongodump_command') or 'mongodump')
+    )
    return (
-        ('mongodump',)
+        dump_command
        + (('--out', shlex.quote(dump_filename)) if dump_format == 'directory' else ())
        + (('--host', shlex.quote(database['hostname'])) if 'hostname' in database else ())
        + (('--port', shlex.quote(str(database['port']))) if 'port' in database else ())
@ -230,7 +233,7 @@ def restore_data_source_dump(

 def build_restore_command(extract_process, database, config, dump_filename, connection_params):
    '''
-    Return the mongorestore command from a single database configuration.
+    Return the custom mongorestore_command from a single database configuration.
    '''
    hostname = connection_params['hostname'] or database.get(
        'restore_hostname', database.get('hostname')
@ -251,7 +254,10 @@ def build_restore_command(extract_process, database, config, dump_filename, conn
        config,
    )

-    command = ['mongorestore']
+    command = list(
+        shlex.quote(part)
+        for part in shlex.split(database.get('mongorestore_command') or 'mongorestore')
+    )
    if extract_process:
        command.append('--archive')
    else:
--- a/tests/unit/hooks/credential/test_file.py
+++ b/tests/unit/hooks/credential/test_file.py
@ -26,6 +26,9 @@ def test_load_credential_reads_named_credential_from_file():
    credential_stream = io.StringIO('password')
    credential_stream.name = '/credentials/mycredential'
    builtins = flexmock(sys.modules['builtins'])
+    flexmock(module.os.path).should_receive('expanduser').with_args(
+        '/credentials/mycredential'
+    ).and_return('/credentials/mycredential')
    builtins.should_receive('open').with_args('/credentials/mycredential').and_return(
        credential_stream
    )
@ -42,6 +45,9 @@ def test_load_credential_reads_named_credential_from_file_using_working_director
    credential_stream = io.StringIO('password')
    credential_stream.name = '/working/credentials/mycredential'
    builtins = flexmock(sys.modules['builtins'])
+    flexmock(module.os.path).should_receive('expanduser').with_args(
+        'credentials/mycredential'
+    ).and_return('credentials/mycredential')
    builtins.should_receive('open').with_args('/working/credentials/mycredential').and_return(
        credential_stream
    )
@ -58,6 +64,9 @@ def test_load_credential_reads_named_credential_from_file_using_working_director

 def test_load_credential_with_file_not_found_error_raises():
    builtins = flexmock(sys.modules['builtins'])
+    flexmock(module.os.path).should_receive('expanduser').with_args(
+        '/credentials/mycredential'
+    ).and_return('/credentials/mycredential')
    builtins.should_receive('open').with_args('/credentials/mycredential').and_raise(
        FileNotFoundError
    )
@ -66,3 +75,22 @@ def test_load_credential_with_file_not_found_error_raises():
        module.load_credential(
            hook_config={}, config={}, credential_parameters=('/credentials/mycredential',)
        )
+
+
+def test_load_credential_reads_named_credential_from_expanded_directory():
+    credential_stream = io.StringIO('password')
+    credential_stream.name = '/root/credentials/mycredential'
+    builtins = flexmock(sys.modules['builtins'])
+    flexmock(module.os.path).should_receive('expanduser').with_args(
+        '~/credentials/mycredential'
+    ).and_return('/root/credentials/mycredential')
+    builtins.should_receive('open').with_args('/root/credentials/mycredential').and_return(
+        credential_stream
+    )
+
+    assert (
+        module.load_credential(
+            hook_config={}, config={}, credential_parameters=('~/credentials/mycredential',)
+        )
+        == 'password'
+    )
--- a/tests/unit/hooks/credential/test_keepassxc.py
+++ b/tests/unit/hooks/credential/test_keepassxc.py
@ -15,6 +15,9 @@ def test_load_credential_with_invalid_credential_parameters_raises(credential_pa


 def test_load_credential_with_missing_database_raises():
+    flexmock(module.os.path).should_receive('expanduser').with_args('database.kdbx').and_return(
+        'database.kdbx'
+    )
    flexmock(module.os.path).should_receive('exists').and_return(False)
    flexmock(module.borgmatic.execute).should_receive('execute_command_and_capture_output').never()

@ -25,6 +28,9 @@ def test_load_credential_with_missing_database_raises():


 def test_load_credential_with_present_database_fetches_password_from_keepassxc():
+    flexmock(module.os.path).should_receive('expanduser').with_args('database.kdbx').and_return(
+        'database.kdbx'
+    )
    flexmock(module.os.path).should_receive('exists').and_return(True)
    flexmock(module.borgmatic.execute).should_receive(
        'execute_command_and_capture_output'
@ -51,6 +57,9 @@ def test_load_credential_with_present_database_fetches_password_from_keepassxc()


 def test_load_credential_with_custom_keepassxc_cli_command_calls_it():
+    flexmock(module.os.path).should_receive('expanduser').with_args('database.kdbx').and_return(
+        'database.kdbx'
+    )
    config = {'keepassxc': {'keepassxc_cli_command': '/usr/local/bin/keepassxc-cli --some-option'}}
    flexmock(module.os.path).should_receive('exists').and_return(True)
    flexmock(module.borgmatic.execute).should_receive(
@ -78,3 +87,32 @@ def test_load_credential_with_custom_keepassxc_cli_command_calls_it():
        )
        == 'password'
    )
+
+
+def test_load_credential_with_expanded_directory_with_present_database_fetches_password_from_keepassxc():
+    flexmock(module.os.path).should_receive('expanduser').with_args('~/database.kdbx').and_return(
+        '/root/database.kdbx'
+    )
+    flexmock(module.os.path).should_receive('exists').and_return(True)
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).with_args(
+        (
+            'keepassxc-cli',
+            'show',
+            '--show-protected',
+            '--attributes',
+            'Password',
+            '/root/database.kdbx',
+            'mypassword',
+        )
+    ).and_return(
+        'password'
+    ).once()
+
+    assert (
+        module.load_credential(
+            hook_config={}, config={}, credential_parameters=('~/database.kdbx', 'mypassword')
+        )
+        == 'password'
+    )
--- a/tests/unit/hooks/data_source/test_mongodb.py
+++ b/tests/unit/hooks/data_source/test_mongodb.py
@ -681,3 +681,135 @@ def test_restore_data_source_dump_without_extract_process_restores_from_disk():
        },
        borgmatic_runtime_directory='/run/borgmatic',
    )
+
+
+def test_dump_data_sources_uses_custom_mongodump_command():
+    flexmock(module.borgmatic.hooks.command).should_receive('Before_after_hooks').and_return(
+        flexmock()
+    )
+    databases = [{'name': 'foo', 'mongodump_command': 'custom_mongodump'}]
+    process = flexmock()
+    flexmock(module).should_receive('make_dump_path').and_return('')
+    flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return(
+        'databases/localhost/foo'
+    )
+    flexmock(module.dump).should_receive('create_named_pipe_for_dump')
+
+    flexmock(module).should_receive('execute_command').with_args(
+        (
+            'custom_mongodump',
+            '--db',
+            'foo',
+            '--archive',
+            '>',
+            'databases/localhost/foo',
+        ),
+        shell=True,
+        run_to_completion=False,
+    ).and_return(process).once()
+
+    assert module.dump_data_sources(
+        databases,
+        {},
+        config_paths=('test.yaml',),
+        borgmatic_runtime_directory='/run/borgmatic',
+        patterns=[],
+        dry_run=False,
+    ) == [process]
+
+
+def test_build_dump_command_prevents_shell_injection():
+    database = {
+        'name': 'testdb; rm -rf /',  # Malicious input
+        'hostname': 'localhost',
+        'port': 27017,
+        'username': 'user',
+        'password': 'password',
+        'mongodump_command': 'mongodump',
+        'options': '--gzip',
+    }
+    config = {}
+    dump_filename = '/path/to/dump'
+    dump_format = 'archive'
+
+    command = module.build_dump_command(database, config, dump_filename, dump_format)
+
+    # Ensure the malicious input is properly escaped and does not execute
+    assert 'testdb; rm -rf /' not in command
+    assert any(
+        'testdb' in part for part in command
+    )  # Check if 'testdb' is in any part of the tuple
+
+
+def test_restore_data_source_dump_uses_custom_mongorestore_command():
+    hook_config = [
+        {
+            'name': 'foo',
+            'mongorestore_command': 'custom_mongorestore',
+            'schemas': None,
+            'restore_options': '--gzip',
+        }
+    ]
+    extract_process = flexmock(stdout=flexmock())
+
+    flexmock(module).should_receive('make_dump_path')
+    flexmock(module.dump).should_receive('make_data_source_dump_filename')
+    flexmock(module.borgmatic.hooks.credential.parse).should_receive(
+        'resolve_credential'
+    ).replace_with(lambda value, config: value)
+    flexmock(module).should_receive('execute_command_with_processes').with_args(
+        [
+            'custom_mongorestore',  # Should use custom command instead of default
+            '--archive',
+            '--drop',
+            '--gzip',  # Should include restore options
+        ],
+        processes=[extract_process],
+        output_log_level=logging.DEBUG,
+        input_file=extract_process.stdout,
+    ).once()
+
+    module.restore_data_source_dump(
+        hook_config,
+        {},
+        data_source=hook_config[0],
+        dry_run=False,
+        extract_process=extract_process,
+        connection_params={
+            'hostname': None,
+            'port': None,
+            'username': None,
+            'password': None,
+        },
+        borgmatic_runtime_directory='/run/borgmatic',
+    )
+
+
+def test_build_restore_command_prevents_shell_injection():
+    database = {
+        'name': 'testdb; rm -rf /',  # Malicious input
+        'restore_hostname': 'localhost',
+        'restore_port': 27017,
+        'restore_username': 'user',
+        'restore_password': 'password',
+        'mongorestore_command': 'mongorestore',
+        'restore_options': '--gzip',
+    }
+    config = {}
+    dump_filename = '/path/to/dump'
+    connection_params = {
+        'hostname': None,
+        'port': None,
+        'username': None,
+        'password': None,
+    }
+    extract_process = None
+
+    command = module.build_restore_command(
+        extract_process, database, config, dump_filename, connection_params
+    )
+
+    # print(command)
+    # Ensure the malicious input is properly escaped and does not execute
+    assert 'rm -rf /' not in command
+    assert ';' not in command