Always error and exit when the borgmatic runtime directory overlaps with the configured excludes (#1122).

Reviewed-on: https://projects.torsion.org/borgmatic-collective/borgmatic/pulls/1144

NEWS

@@ -2,6 +2,8 @@
  * #1114: Document systemd configuration changes for the ZFS filesystem hook.
  * #1118: Fix a bug in which Borg hangs during database backup when different filesystems are in
    use.
+ * #1122: To prevent the user from inadvertently excluding the "bootstrap" action's manifest, always
+   error and exit when the borgmatic runtime directory overlaps with the configured excludes.
  * #1125: Clarify documentation about ZFS, Btrfs, and LVM snapshotting when a separate
    filesystem is mounted in the source directory. (Spoiler: The separate filesystem doesn't get
    included in the snapshot.)

borgmatic/borg/create.py

@@ -45,28 +45,26 @@ def any_parent_directories(path, candidate_parents):
     return False
 
 
-def collect_special_file_paths(
+def validate_planned_backup_paths(
     dry_run,
     create_command,
     config,
+    patterns,
     local_path,
     working_directory,
     borgmatic_runtime_directory,
 ):
     '''
     Given a dry-run flag, a Borg create command as a tuple, a configuration dict, a local Borg path,
-    a working directory, and the borgmatic runtime directory, collect the paths for any special
-    files (character devices, block devices, and named pipes / FIFOs) that Borg would encounter
-    during a create. These are all paths that could cause Borg to hang if its --read-special flag is
-    used.
-
-    Skip looking for special files in the given borgmatic runtime directory, as borgmatic creates
-    its own special files there for database dumps and we don't want those omitted.
-
-    Additionally, if the borgmatic runtime directory is not contained somewhere in the files Borg
-    plans to backup, that means the user must have excluded the runtime directory (e.g. via
-    "exclude_patterns" or similar). Therefore, raise, because this means Borg won't be able to
-    consume any database dumps and therefore borgmatic will hang when it tries to do so.
+    a working directory, and the borgmatic runtime directory, perform a "borg create --dry-run" to
+    determine whether Borg's planned paths to include in a backup look good. Specifically, if the
+    given runtime directory exists, validate that it will be included in a backup and hasn't been
+    excluded.
+
+    Raise ValueError if the runtime directory has been excluded via "exclude_patterns" or similar,
+    because any features that rely on the runtime directory getting backed up will break.  For
+    instance, without the runtime directory, Borg can't consume any database dumps and borgmatic may
+    hang waiting for them to be consumed.
     '''
     # Omit "--exclude-nodump" from the Borg dry run command, because that flag causes Borg to open
     # files including any named pipe we've created. And omit "--filter" because that can break the
@@ -94,26 +92,32 @@ def collect_special_file_paths(
         if path_line and path_line.startswith(('- ', '+ '))
     )
 
-    # These are the subset of those files that contain the borgmatic runtime directory.
-    paths_containing_runtime_directory = {}
-
-    if os.path.exists(borgmatic_runtime_directory):
-        paths_containing_runtime_directory = {
-            path for path in paths if any_parent_directories(path, (borgmatic_runtime_directory,))
-        }
+    # These are the subset of output paths contained within the borgmatic runtime directory.
+    paths_inside_runtime_directory = {
+        path for path in paths if any_parent_directories(path, (borgmatic_runtime_directory,))
+    }
+
+    # If the runtime directory isn't present in the source patterns, then we shouldn't expect it to
+    # be in the paths output from the Borg dry run.
+    runtime_directory_present_in_patterns = any(
+        pattern
+        for pattern in patterns
+        if any_parent_directories(pattern.path, (borgmatic_runtime_directory,))
+        if pattern.type == borgmatic.borg.pattern.Pattern_type.ROOT
+    )
 
-        # If no paths to backup contain the runtime directory, it must've been excluded.
-        if not paths_containing_runtime_directory and not dry_run:
-            raise ValueError(
-                f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes or patterns with excludes. Please ensure the runtime directory is not excluded.',
-            )
+    # If no paths to backup are inside the runtime directory, it must've been excluded.
+    if (
+        not paths_inside_runtime_directory
+        and runtime_directory_present_in_patterns
+        and not dry_run
+        and os.path.exists(borgmatic_runtime_directory)
+    ):
+        raise ValueError(
+            f'The runtime directory {os.path.normpath(borgmatic_runtime_directory)} overlaps with the configured excludes or patterns with excludes. Please ensure the runtime directory is not excluded.',
+        )
 
-    return tuple(
-        path
-        for path in paths
-        if special_file(path, working_directory)
-        if path not in paths_containing_runtime_directory
-    )
+    return tuple(path for path in paths if path not in paths_inside_runtime_directory)
 
 
 MAX_SPECIAL_FILE_PATHS_LENGTH = 1000
@@ -228,6 +232,18 @@ def make_base_create_command(
         archive_name_format,
         local_borg_version,
     )
+    working_directory = borgmatic.config.paths.get_working_directory(config)
+
+    logger.debug('Checking file paths Borg plans to include')
+    planned_backup_paths = validate_planned_backup_paths(
+        dry_run,
+        create_flags + create_positional_arguments,
+        config,
+        patterns,
+        local_path,
+        working_directory,
+        borgmatic_runtime_directory=borgmatic_runtime_directory,
+    )
 
     # If database hooks are enabled (as indicated by streaming processes), exclude files that might
     # cause Borg to hang. But skip this if the user has explicitly set the "read_special" to True.
@@ -235,16 +251,9 @@ def make_base_create_command(
         logger.warning(
             'Ignoring configured "read_special" value of false, as true is needed for database hooks.',
         )
-        working_directory = borgmatic.config.paths.get_working_directory(config)
 
-        logger.debug('Collecting special file paths')
-        special_file_paths = collect_special_file_paths(
-            dry_run,
-            create_flags + create_positional_arguments,
-            config,
-            local_path,
-            working_directory,
-            borgmatic_runtime_directory=borgmatic_runtime_directory,
+        special_file_paths = tuple(
+            path for path in planned_backup_paths if special_file(path, working_directory)
         )
 
         if special_file_paths:

tests/end-to-end/test_borgmatic.py

@@ -85,7 +85,7 @@ def test_borgmatic_command(generate_configuration):
         )
 
         # Run borgmatic to generate a backup archive, and then list it to make sure it exists.
-        subprocess.check_call(f'borgmatic --config {config_path}'.split(' '))
+        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)

tests/unit/borg/test_create.py

@@ -59,7 +59,7 @@ def test_any_parent_directories_treats_unrelated_paths_as_non_match():
     module.any_parent_directories('/foo/bar.txt', ('/usr', '/etc'))
 
 
-def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_list():
+def test_validate_planned_backup_paths_parses_borg_dry_run_file_list():
     flexmock(module.flags).should_receive('omit_flag').replace_with(
         lambda arguments, flag: arguments,
     )
@@ -70,21 +70,25 @@ def test_collect_special_file_paths_parses_special_files_from_borg_dry_run_file_
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         'Processing files ...\n- /foo\n+ /bar\n- /baz',
     )
-    flexmock(module).should_receive('special_file').and_return(True)
     flexmock(module.os.path).should_receive('exists').and_return(False)
-    flexmock(module).should_receive('any_parent_directories').never()
+    flexmock(module).should_receive('any_parent_directories').and_return(False)
 
-    assert module.collect_special_file_paths(
+    assert module.validate_planned_backup_paths(
         dry_run=False,
         create_command=('borg', 'create'),
         config={},
+        patterns=(
+            module.borgmatic.borg.pattern.Pattern('/foo'),
+            module.borgmatic.borg.pattern.Pattern('/bar'),
+            module.borgmatic.borg.pattern.Pattern('/baz'),
+        ),
         local_path=None,
         working_directory=None,
         borgmatic_runtime_directory='/run/borgmatic',
     ) == ('/foo', '/bar', '/baz')
 
 
-def test_collect_special_file_paths_skips_borgmatic_runtime_directory():
+def test_validate_planned_backup_paths_skips_borgmatic_runtime_directory():
     flexmock(module.flags).should_receive('omit_flag').replace_with(
         lambda arguments, flag: arguments,
     )
@@ -95,32 +99,29 @@ def test_collect_special_file_paths_skips_borgmatic_runtime_directory():
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         '+ /foo\n- /run/borgmatic/bar\n- /baz',
     )
-    flexmock(module).should_receive('special_file').and_return(True)
     flexmock(module.os.path).should_receive('exists').and_return(True)
-    flexmock(module).should_receive('any_parent_directories').with_args(
-        '/foo',
-        ('/run/borgmatic',),
-    ).and_return(False)
-    flexmock(module).should_receive('any_parent_directories').with_args(
-        '/run/borgmatic/bar',
-        ('/run/borgmatic',),
-    ).and_return(True)
-    flexmock(module).should_receive('any_parent_directories').with_args(
-        '/baz',
-        ('/run/borgmatic',),
-    ).and_return(False)
+    flexmock(module).should_receive('any_parent_directories').replace_with(
+        lambda path, _: path == '/run/borgmatic/bar'
+    )
 
-    assert module.collect_special_file_paths(
+    assert module.validate_planned_backup_paths(
         dry_run=False,
         create_command=('borg', 'create'),
         config={},
+        patterns=(
+            module.borgmatic.borg.pattern.Pattern('/foo'),
+            module.borgmatic.borg.pattern.Pattern(
+                '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.ROOT
+            ),
+            module.borgmatic.borg.pattern.Pattern('/baz'),
+        ),
         local_path=None,
         working_directory=None,
         borgmatic_runtime_directory='/run/borgmatic',
     ) == ('/foo', '/baz')
 
 
-def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors():
+def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_missing_from_paths_output_errors():
     flexmock(module.flags).should_receive('omit_flag').replace_with(
         lambda arguments, flag: arguments,
     )
@@ -131,22 +132,30 @@ def test_collect_special_file_paths_with_borgmatic_runtime_directory_missing_fro
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
         '+ /foo\n- /bar\n- /baz',
     )
-    flexmock(module).should_receive('special_file').and_return(True)
     flexmock(module.os.path).should_receive('exists').and_return(True)
-    flexmock(module).should_receive('any_parent_directories').and_return(False)
+    flexmock(module).should_receive('any_parent_directories').replace_with(
+        lambda path, _: path == '/run/borgmatic/bar'
+    )
 
     with pytest.raises(ValueError):
-        module.collect_special_file_paths(
+        module.validate_planned_backup_paths(
             dry_run=False,
             create_command=('borg', 'create'),
             config={},
+            patterns=(
+                module.borgmatic.borg.pattern.Pattern('/foo'),
+                module.borgmatic.borg.pattern.Pattern(
+                    '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.ROOT
+                ),
+                module.borgmatic.borg.pattern.Pattern('/baz'),
+            ),
             local_path=None,
             working_directory=None,
             borgmatic_runtime_directory='/run/borgmatic',
         )
 
 
-def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory_missing_from_paths_output_does_not_raise():
+def test_validate_planned_backup_paths_with_borgmatic_runtime_directory_missing_from_patterns_does_not_raise():
     flexmock(module.flags).should_receive('omit_flag').replace_with(
         lambda arguments, flag: arguments,
     )
@@ -155,23 +164,28 @@ def test_collect_special_file_paths_with_dry_run_and_borgmatic_runtime_directory
     )
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
-        '+ /foo\n- /bar\n- /baz',
+        '+ /foo\n- /run/borgmatic/bar\n- /baz',
     )
-    flexmock(module).should_receive('special_file').and_return(True)
     flexmock(module.os.path).should_receive('exists').and_return(True)
-    flexmock(module).should_receive('any_parent_directories').and_return(False)
+    flexmock(module).should_receive('any_parent_directories').replace_with(
+        lambda path, _: path == '/run/borgmatic/bar'
+    )
 
-    assert module.collect_special_file_paths(
-        dry_run=True,
+    assert module.validate_planned_backup_paths(
+        dry_run=False,
         create_command=('borg', 'create'),
         config={},
+        patterns=(
+            module.borgmatic.borg.pattern.Pattern('/foo'),
+            module.borgmatic.borg.pattern.Pattern('/baz'),
+        ),
         local_path=None,
         working_directory=None,
         borgmatic_runtime_directory='/run/borgmatic',
-    ) == ('/foo', '/bar', '/baz')
+    ) == ('/foo', '/baz')
 
 
-def test_collect_special_file_paths_excludes_non_special_files():
+def test_validate_planned_backup_paths_with_dry_run_and_borgmatic_runtime_directory_missing_from_paths_output_does_not_raise():
     flexmock(module.flags).should_receive('omit_flag').replace_with(
         lambda arguments, flag: arguments,
     )
@@ -180,22 +194,26 @@ def test_collect_special_file_paths_excludes_non_special_files():
     )
     flexmock(module.environment).should_receive('make_environment').and_return(None)
     flexmock(module).should_receive('execute_command_and_capture_output').and_return(
-        '+ /foo\n+ /bar\n+ /baz',
-    )
-    flexmock(module).should_receive('special_file').and_return(True).and_return(False).and_return(
-        True,
+        '+ /foo\n- /run/borgmatic/bar\n- /baz',
     )
-    flexmock(module.os.path).should_receive('exists').and_return(False)
-    flexmock(module).should_receive('any_parent_directories').never()
+    flexmock(module.os.path).should_receive('exists').and_return(True)
+    flexmock(module).should_receive('any_parent_directories').and_return(False)
 
-    assert module.collect_special_file_paths(
-        dry_run=False,
+    assert module.validate_planned_backup_paths(
+        dry_run=True,
         create_command=('borg', 'create'),
         config={},
+        patterns=(
+            module.borgmatic.borg.pattern.Pattern('/foo'),
+            module.borgmatic.borg.pattern.Pattern(
+                '/run/borgmatic/bar', module.borgmatic.borg.pattern.Pattern_type.ROOT
+            ),
+            module.borgmatic.borg.pattern.Pattern('/baz'),
+        ),
         local_path=None,
         working_directory=None,
         borgmatic_runtime_directory='/run/borgmatic',
-    ) == ('/foo', '/baz')
+    ) == ('/foo', '/run/borgmatic/bar', '/baz')
 
 
 DEFAULT_ARCHIVE_NAME = '{hostname}-{now:%Y-%m-%dT%H:%M:%S.%f}'
@@ -211,6 +229,7 @@ def test_make_base_create_produces_borg_command():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -246,6 +265,7 @@ def test_make_base_create_command_includes_patterns_file_in_borg_command():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -278,6 +298,7 @@ def test_make_base_create_command_with_store_config_false_omits_config_files():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -346,6 +367,7 @@ def test_make_base_create_command_includes_configuration_option_as_command_flag(
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -378,6 +400,7 @@ def test_make_base_create_command_includes_dry_run_in_borg_command():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=True,
@@ -410,6 +433,7 @@ def test_make_base_create_command_includes_comment_in_borg_command():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -443,6 +467,7 @@ def test_make_base_create_command_includes_local_path_in_borg_command():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -475,6 +500,7 @@ def test_make_base_create_command_includes_remote_path_in_borg_command():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -507,6 +533,7 @@ def test_make_base_create_command_includes_log_json_in_borg_command():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -539,6 +566,7 @@ def test_make_base_create_command_includes_list_flags_in_borg_command():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -578,7 +606,18 @@ def test_make_base_create_command_with_stream_processes_ignores_read_special_fal
     )
     flexmock(module.logger).should_receive('warning').twice()
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('collect_special_file_paths').and_return(('/dev/null',)).once()
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(
+        (
+            '/non/special',
+            '/dev/null',
+        )
+    )
+    flexmock(module).should_receive('special_file').with_args(
+        '/non/special', working_directory=None
+    ).and_return(False)
+    flexmock(module).should_receive('special_file').with_args(
+        '/dev/null', working_directory=None
+    ).and_return(True)
     flexmock(module.borgmatic.borg.pattern).should_receive('write_patterns_file').with_args(
         (
             Pattern(
@@ -630,7 +669,18 @@ def test_make_base_create_command_without_patterns_and_with_stream_processes_ign
     )
     flexmock(module.logger).should_receive('warning').twice()
     flexmock(module.environment).should_receive('make_environment')
-    flexmock(module).should_receive('collect_special_file_paths').and_return(('/dev/null',)).once()
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(
+        (
+            '/non/special',
+            '/dev/null',
+        )
+    )
+    flexmock(module).should_receive('special_file').with_args(
+        '/non/special', working_directory=None
+    ).and_return(False)
+    flexmock(module).should_receive('special_file').with_args(
+        '/dev/null', working_directory=None
+    ).and_return(True)
     flexmock(module.borgmatic.borg.pattern).should_receive('write_patterns_file').with_args(
         (
             Pattern(
@@ -678,7 +728,7 @@ def test_make_base_create_command_with_stream_processes_and_read_special_true_sk
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
     flexmock(module.logger).should_receive('warning').never()
-    flexmock(module).should_receive('collect_special_file_paths').never()
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -712,6 +762,7 @@ def test_make_base_create_command_includes_archive_name_format_in_borg_command()
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         ('repo::ARCHIVE_NAME',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -744,6 +795,7 @@ def test_make_base_create_command_includes_default_archive_name_format_in_borg_c
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         ('repo::{hostname}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -775,6 +827,7 @@ def test_make_base_create_command_includes_archive_name_format_with_placeholders
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (repository_archive_pattern,),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -807,6 +860,7 @@ def test_make_base_create_command_includes_repository_and_archive_name_format_wi
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (repository_archive_pattern,),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -835,6 +889,7 @@ def test_make_base_create_command_includes_archive_suffix_in_borg_command():
         DEFAULT_ARCHIVE_NAME,
     )
     flexmock(module.borgmatic.borg.flags).should_receive('make_exclude_flags').and_return(())
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,
@@ -867,6 +922,7 @@ def test_make_base_create_command_includes_extra_borg_options_in_borg_command():
     flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
         (f'repo::{DEFAULT_ARCHIVE_NAME}',),
     )
+    flexmock(module).should_receive('validate_planned_backup_paths').and_return(())
 
     (create_flags, create_positional_arguments, pattern_file) = module.make_base_create_command(
         dry_run=False,