Browse Source

Custom command options for MongoDB hook #837

Gautam Aggarwal 2 months ago
parent
commit
d651813601

+ 18 - 0
borgmatic/config/schema.yaml

@@ -1726,6 +1726,24 @@ 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

+ 10 - 4
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:

+ 129 - 6
tests/unit/hooks/data_source/test_mongodb.py

@@ -24,6 +24,9 @@ def test_use_streaming_false_for_no_databases():
 
 
 def test_dump_data_sources_runs_mongodump_for_each_database():
+    flexmock(module.borgmatic.hooks.command).should_receive('Before_after_hooks').and_return(
+        flexmock()
+    )
     databases = [{'name': 'foo'}, {'name': 'bar'}]
     processes = [flexmock(), flexmock()]
     flexmock(module).should_receive('make_dump_path').and_return('')
@@ -53,6 +56,9 @@ def test_dump_data_sources_runs_mongodump_for_each_database():
 
 
 def test_dump_data_sources_with_dry_run_skips_mongodump():
+    flexmock(module.borgmatic.hooks.command).should_receive('Before_after_hooks').and_return(
+        flexmock()
+    )
     databases = [{'name': 'foo'}, {'name': 'bar'}]
     flexmock(module).should_receive('make_dump_path').and_return('')
     flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return(
@@ -75,6 +81,9 @@ def test_dump_data_sources_with_dry_run_skips_mongodump():
 
 
 def test_dump_data_sources_runs_mongodump_with_hostname_and_port():
+    flexmock(module.borgmatic.hooks.command).should_receive('Before_after_hooks').and_return(
+        flexmock()
+    )
     databases = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}]
     process = flexmock()
     flexmock(module).should_receive('make_dump_path').and_return('')
@@ -111,9 +120,12 @@ def test_dump_data_sources_runs_mongodump_with_hostname_and_port():
 
 
 def test_dump_data_sources_runs_mongodump_with_username_and_password():
+    flexmock(module.borgmatic.hooks.command).should_receive('Before_after_hooks').and_return(
+        flexmock()
+    )
     databases = [
         {
-            'name': 'foo',
+            'name': 'foo',  # Ensure this matches the expected format in the related functions
             'username': 'mongo',
             'password': 'trustsome1',
             'authentication_database': 'admin',
@@ -162,6 +174,9 @@ def test_dump_data_sources_runs_mongodump_with_username_and_password():
 
 
 def test_dump_data_sources_runs_mongodump_with_directory_format():
+    flexmock(module.borgmatic.hooks.command).should_receive('Before_after_hooks').and_return(
+        flexmock()
+    )
     databases = [{'name': 'foo', 'format': 'directory'}]
     flexmock(module).should_receive('make_dump_path').and_return('')
     flexmock(module.dump).should_receive('make_data_source_dump_filename').and_return(
@@ -189,6 +204,9 @@ def test_dump_data_sources_runs_mongodump_with_directory_format():
 
 
 def test_dump_data_sources_runs_mongodump_with_options():
+    flexmock(module.borgmatic.hooks.command).should_receive('Before_after_hooks').and_return(
+        flexmock()
+    )
     databases = [{'name': 'foo', 'options': '--stuff=such'}]
     process = flexmock()
     flexmock(module).should_receive('make_dump_path').and_return('')
@@ -222,6 +240,9 @@ def test_dump_data_sources_runs_mongodump_with_options():
 
 
 def test_dump_data_sources_runs_mongodumpall_for_all_databases():
+    flexmock(module.borgmatic.hooks.command).should_receive('Before_after_hooks').and_return(
+        flexmock()
+    )
     databases = [{'name': 'all'}]
     process = flexmock()
     flexmock(module).should_receive('make_dump_path').and_return('')
@@ -275,7 +296,7 @@ def test_build_dump_command_with_username_injection_attack_gets_escaped():
 
 def test_restore_data_source_dump_runs_mongorestore():
     hook_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}]
-    extract_process = flexmock(stdout=flexmock())
+    extract_process = flexmock(stdout=flexmock(read=lambda: b""))
 
     flexmock(module).should_receive('make_dump_path')
     flexmock(module.dump).should_receive('make_data_source_dump_filename')
@@ -290,9 +311,9 @@ def test_restore_data_source_dump_runs_mongorestore():
     ).once()
 
     module.restore_data_source_dump(
-        hook_config,
-        {},
-        data_source={'name': 'foo'},
+        hook_config=hook_config,
+        config={},
+        data_source=hook_config[0],
         dry_run=False,
         extract_process=extract_process,
         connection_params={
@@ -309,7 +330,7 @@ def test_restore_data_source_dump_runs_mongorestore_with_hostname_and_port():
     hook_config = [
         {'name': 'foo', 'hostname': 'database.example.org', 'port': 5433, 'schemas': None}
     ]
-    extract_process = flexmock(stdout=flexmock())
+    extract_process = flexmock(stdout=flexmock(read=lambda: b""))
 
     flexmock(module).should_receive('make_dump_path')
     flexmock(module.dump).should_receive('make_data_source_dump_filename')
@@ -681,3 +702,105 @@ 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'
+
+    from borgmatic.hooks.data_source.mongodb import build_dump_command, build_restore_command  # Import the functions
+
+    command = 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',
+    )
+
+
+ 
+