Fix interaction between environment variable interpolation in constants and shell escaping (#860).

Reviewed-on: #859

NEWS

@@ -1,3 +1,13 @@
+1.8.12.dev0
+ * #860: Fix interaction between environment variable interpolation in constants and shell escaping.
+
+1.8.11
+ * #815: Add optional Healthchecks auto-provisioning via "create_slug" option.
+ * #851: Fix lack of file extraction when using "extract --strip-components all" on a path with a
+   leading slash.
+ * #854: Fix a traceback when the "data" consistency check is used.
+ * #857: Fix a traceback with "check --only spot" when the "spot" check is unconfigured.
+
 1.8.10
  * #656 (beta): Add a "spot" consistency check that compares file counts and contents between your
    source files and the latest archive, ensuring they fall within configured tolerances. This can

borgmatic/actions/check.py

@@ -480,7 +480,13 @@ def spot_check(
     '''
     log_label = f'{repository.get("label", repository["path"])}'
     logger.debug(f'{log_label}: Running spot check')
-    spot_check_config = next(check for check in config['checks'] if check['name'] == 'spot')
+
+    try:
+        spot_check_config = next(
+            check for check in config.get('checks', ()) if check.get('name') == 'spot'
+        )
+    except StopIteration:
+        raise ValueError('Cannot run spot check because it is unconfigured')
 
     if spot_check_config['data_tolerance_percentage'] > spot_check_config['data_sample_percentage']:
         raise ValueError(

borgmatic/borg/check.py

@@ -52,8 +52,8 @@ def make_archive_filter_flags(local_borg_version, config, checks, check_argument
 
 def make_check_flags(checks, archive_filter_flags):
     '''
-    Given a parsed sequence of checks and a sequence of flags to filter archives, transform the
-    checks into tuple of command-line check flags.
+    Given a parsed checks set and a sequence of flags to filter archives,
+    transform the checks into tuple of command-line check flags.
 
     For example, given parsed checks of:
 
@@ -68,13 +68,13 @@ def make_check_flags(checks, archive_filter_flags):
     '''
     if 'data' in checks:
         data_flags = ('--verify-data',)
-        checks += ('archives',)
+        checks.update({'archives'})
     else:
         data_flags = ()
 
     common_flags = (archive_filter_flags if 'archives' in checks else ()) + data_flags
 
-    if {'repository', 'archives'}.issubset(set(checks)):
+    if {'repository', 'archives'}.issubset(checks):
         return common_flags
 
     return (

borgmatic/borg/extract.py

@@ -104,8 +104,13 @@ def extract_archive(
         if not paths:
             raise ValueError('The --strip-components flag with "all" requires at least one --path')
 
-        # Calculate the maximum number of leading path components of the given paths.
-        strip_components = max(0, *(len(path.split(os.path.sep)) - 1 for path in paths))
+        # Calculate the maximum number of leading path components of the given paths. "if piece"
+        # ignores empty path components, e.g. those resulting from a leading slash. And the "- 1"
+        # is so this doesn't count the final path component, e.g. the filename itself.
+        strip_components = max(
+            0,
+            *(len(tuple(piece for piece in path.split(os.path.sep) if piece)) - 1 for path in paths)
+        )
 
     full_command = (
         (local_path, 'extract')

borgmatic/config/constants.py

@@ -50,12 +50,15 @@ def apply_constants(value, constants, shell_escape=False):
             value[index] = apply_constants(list_value, constants, shell_escape)
     elif isinstance(value, dict):
         for option_name, option_value in value.items():
-            shell_escape = (
-                shell_escape
-                or option_name.startswith('before_')
-                or option_name.startswith('after_')
-                or option_name == 'on_error'
+            value[option_name] = apply_constants(
+                option_value,
+                constants,
+                shell_escape=(
+                    shell_escape
+                    or option_name.startswith('before_')
+                    or option_name.startswith('after_')
+                    or option_name == 'on_error'
+                ),
             )
-            value[option_name] = apply_constants(option_value, constants, shell_escape)
 
     return value

borgmatic/config/schema.yaml

@@ -269,7 +269,8 @@ properties:
     compression:
         type: string
         description: |
-            Type of compression to use when creating archives. See
+            Type of compression to use when creating archives. (Compression
+            level can be added separated with a comma, like "zstd,7".) See
             http://borgbackup.readthedocs.io/en/stable/usage/create.html for
             details. Defaults to "lz4".
         example: lz4
@@ -1662,6 +1663,14 @@ properties:
                     states.
                 example:
                     - finish
+            create_slug:
+                type: boolean
+                description: |
+                    Create the check if it does not exist. Only works with
+                    the slug URL scheme (https://hc-ping.com/<ping-key>/<slug>
+                    as opposed to https://hc-ping.com/<uuid>).
+                    Defaults to false.
+                example: true
         description: |
             Configuration for a monitoring integration with Healthchecks. Create
             an account at https://healthchecks.io (or self-host Healthchecks) if

borgmatic/hooks/healthchecks.py

@@ -1,4 +1,5 @@
 import logging
+import re
 
 import requests
 
@@ -59,10 +60,20 @@ def ping_monitor(hook_config, config, config_filename, state, monitoring_log_lev
         )
         return
 
+    ping_url_is_uuid = re.search(r'\w{8}-\w{4}-\w{4}-\w{4}-\w{12}$', ping_url)
+
     healthchecks_state = MONITOR_STATE_TO_HEALTHCHECKS.get(state)
     if healthchecks_state:
         ping_url = f'{ping_url}/{healthchecks_state}'
 
+    if hook_config.get('create_slug'):
+        if ping_url_is_uuid:
+            logger.warning(
+                f'{config_filename}: Healthchecks UUIDs do not support auto provisionning; ignoring'
+            )
+        else:
+            ping_url = f'{ping_url}?create=1'
+
     logger.info(f'{config_filename}: Pinging Healthchecks {state.name.lower()}{dry_run_label}')
     logger.debug(f'{config_filename}: Using Healthchecks ping URL {ping_url}')
 

docs/how-to/backup-your-databases.md

@@ -437,19 +437,28 @@ borgmatic's own configuration file. So include your configuration file in
 backups to avoid getting caught without a way to restore a database.
 3. borgmatic does not currently support backing up or restoring multiple
 databases that share the exact same name on different hosts.
-4. Because database hooks implicitly enable the `read_special` configuration,
-any special files are excluded from backups (named pipes, block devices,
-character devices, and sockets) to prevent hanging. Try a command like `find
-/your/source/path -type b -or -type c -or -type p -or -type s` to find such
-files. Common directories to exclude are `/dev` and `/run`, but that may not
-be exhaustive. <span class="minilink minilink-addedin">New in version
-1.7.3</span> When database hooks are enabled, borgmatic automatically excludes
-special files (and symlinks to special files) that may cause Borg to hang, so
-generally you no longer need to manually exclude them. There are potential
-edge cases though in which applications on your system create new special files
-*after* borgmatic constructs its exclude list, resulting in Borg hangs. If that
-occurs, you can resort to the manual excludes described above. And to opt out
-of the auto-exclude feature entirely, explicitly set `read_special` to true.
+4. When database hooks are enabled, borgmatic instructs Borg to consume
+special files (via `--read-special`) to support database dump
+streaming—regardless of the value of your `read_special` configuration option.
+And because this can cause Borg to hang, borgmatic also automatically excludes
+special files (and symlinks to them) that Borg may get stuck on. Even so,
+there are still potential edge cases in which applications on your system
+create new special files *after* borgmatic constructs its exclude list,
+resulting in Borg hangs. If that occurs, you can resort to manually excluding
+those files. And if you explicitly set the `read-special` option to `true`,
+borgmatic will opt you out of the auto-exclude feature entirely, but will
+still instruct Borg to consume special files—you will just be on your own to
+exclude them. <span class="minilink minilink-addedin">Prior to version
+1.7.3</span>Special files were not auto-excluded, and you were responsible for
+excluding them yourself. Common directories to exclude are `/dev` and `/run`,
+but that may not be exhaustive.
+5. Database hooks also implicitly enable the `one_file_system` option, which
+means Borg won't cross filesystem boundaries when looking for files to backup.
+This is especially important when running borgmatic in a container, as
+container volumes are mounted as separate filesystems. One work-around is to
+explicitly add each mounted volume you'd like to backup to
+`source_directories` instead of relying on Borg to include them implicitly via
+a parent directory.
 
 
 ### Manual restoration

docs/how-to/deal-with-very-large-backups.md

@@ -121,7 +121,7 @@ incorrect excludes, inadvertent deletes, files changed by malware, etc.
 
 However, because an exhaustive comparison of all source files against the
 latest archive might be too slow, the spot check supports *sampling* a
-percentage of your source files for the comparison, ensuring it falls within
+percentage of your source files for the comparison, ensuring they fall within
 configured tolerances.
 
 Here's how it works. Start by installing the `xxhash` OS package if you don't
@@ -149,12 +149,11 @@ fail.)
 The `data_sample_percentage` is the percentage of total files in the source
 directories to randomly sample and compare to their corresponding files in the
 latest backup archive. A higher value allows a more accurate check—and a
-slower one. The comparison is performed by hashing the selected files in each
-of the source paths and counting hashes that don't match the latest archive.
-For instance, if you have 1,000 source files and your sample percentage is 1%,
-then only 10 source files will be compared against the latest archive. These
-sampled files are selected randomly each time, so in effect the spot check is
-probabilistic.
+slower one. The comparison is performed by hashing the selected source files
+and counting hashes that don't match the latest archive. For instance, if you
+have 1,000 source files and your sample percentage is 1%, then only 10 source
+files will be compared against the latest archive. These sampled files are
+selected randomly each time, so in effect the spot check is probabilistic.
 
 The `data_tolerance_percentage` is the percentage of total files in the source
 directories that can fail a spot check data comparison without failing the
@@ -175,6 +174,11 @@ want the spot check to fail the next time it's run? Run `borgmatic create` to
 create a new backup, thereby allowing the next spot check to run against an
 archive that contains your recent changes.
 
+Because the spot check only looks at the most recent archive, you may not want
+to run it immediately after a `create` action (borgmatic's default behavior).
+Instead, it may make more sense to run the spot check on a separate schedule
+from `create`.
+
 As long as the spot check feature is in beta, it may be subject to breaking
 changes. But feel free to use it in production if you're okay with that
 caveat, and please [provide any

docs/how-to/monitor-your-backups.md

@@ -435,11 +435,16 @@ apprise:
           label: gotify
         - url: mastodons://access_key@hostname/@user
           label: mastodon
+    states:
+        - start
+        - finish
+        - fail
 ```
 
 With this configuration, borgmatic pings each of the configured Apprise
 services when a backup begins, ends, or errors, but only when any of the
-`prune`, `compact`, `create`, or `check` actions are run.
+`prune`, `compact`, `create`, or `check` actions are run. (By default, if
+`states` is not specified, Apprise services are only pinged on error.)
 
 You can optionally customize the contents of the default messages sent to
 these services:

setup.py

@@ -1,6 +1,6 @@
 from setuptools import find_packages, setup
 
-VERSION = '1.8.10'
+VERSION = '1.8.12.dev0'
 
 
 setup(

tests/unit/actions/test_check.py

@@ -769,6 +769,36 @@ def test_compare_spot_check_hashes_considers_non_existent_path_as_not_matching()
     ) == ('/bar',)
 
 
+def test_spot_check_without_spot_configuration_errors():
+    with pytest.raises(ValueError):
+        module.spot_check(
+            repository={'path': 'repo'},
+            config={
+                'checks': [
+                    {
+                        'name': 'archives',
+                    },
+                ]
+            },
+            local_borg_version=flexmock(),
+            global_arguments=flexmock(),
+            local_path=flexmock(),
+            remote_path=flexmock(),
+        )
+
+
+def test_spot_check_without_any_configuration_errors():
+    with pytest.raises(ValueError):
+        module.spot_check(
+            repository={'path': 'repo'},
+            config={},
+            local_borg_version=flexmock(),
+            global_arguments=flexmock(),
+            local_path=flexmock(),
+            remote_path=flexmock(),
+        )
+
+
 def test_spot_check_data_tolerance_percenatge_greater_than_data_sample_percentage_errors():
     with pytest.raises(ValueError):
         module.spot_check(

tests/unit/borg/test_check.py

@@ -223,25 +223,25 @@ def test_make_archive_filter_flags_with_default_checks_and_prefix_includes_match
 
 
 def test_make_check_flags_with_repository_check_returns_flag():
-    flags = module.make_check_flags(('repository',), ())
+    flags = module.make_check_flags({'repository'}, ())
 
     assert flags == ('--repository-only',)
 
 
 def test_make_check_flags_with_archives_check_returns_flag():
-    flags = module.make_check_flags(('archives',), ())
+    flags = module.make_check_flags({'archives'}, ())
 
     assert flags == ('--archives-only',)
 
 
 def test_make_check_flags_with_archives_check_and_archive_filter_flags_includes_those_flags():
-    flags = module.make_check_flags(('archives',), ('--match-archives', 'sh:foo-*'))
+    flags = module.make_check_flags({'archives'}, ('--match-archives', 'sh:foo-*'))
 
     assert flags == ('--archives-only', '--match-archives', 'sh:foo-*')
 
 
 def test_make_check_flags_without_archives_check_and_with_archive_filter_flags_includes_those_flags():
-    flags = module.make_check_flags(('repository',), ('--match-archives', 'sh:foo-*'))
+    flags = module.make_check_flags({'repository'}, ('--match-archives', 'sh:foo-*'))
 
     assert flags == ('--repository-only',)
 
@@ -250,7 +250,7 @@ def test_make_check_flags_with_data_check_returns_flag_and_implies_archives():
     flexmock(module.feature).should_receive('available').and_return(True)
     flexmock(module.flags).should_receive('make_match_archives_flags').and_return(())
 
-    flags = module.make_check_flags(('data',), ())
+    flags = module.make_check_flags({'data'}, ())
 
     assert flags == (
         '--archives-only',
@@ -262,7 +262,7 @@ def test_make_check_flags_with_extract_omits_extract_flag():
     flexmock(module.feature).should_receive('available').and_return(True)
     flexmock(module.flags).should_receive('make_match_archives_flags').and_return(())
 
-    flags = module.make_check_flags(('extract',), ())
+    flags = module.make_check_flags({'extract'}, ())
 
     assert flags == ()
 
@@ -272,10 +272,10 @@ def test_make_check_flags_with_repository_and_data_checks_does_not_return_reposi
     flexmock(module.flags).should_receive('make_match_archives_flags').and_return(())
 
     flags = module.make_check_flags(
-        (
+        {
             'repository',
             'data',
-        ),
+        },
         (),
     )
 

tests/unit/borg/test_extract.py

@@ -507,6 +507,39 @@ def test_extract_archive_calls_borg_with_strip_components_calculated_from_all():
     )
 
 
+def test_extract_archive_calls_borg_with_strip_components_calculated_from_all_with_leading_slash():
+    flexmock(module.os.path).should_receive('abspath').and_return('repo')
+    insert_execute_command_mock(
+        (
+            'borg',
+            'extract',
+            '--strip-components',
+            '2',
+            'repo::archive',
+            '/foo/bar/baz.txt',
+            '/foo/bar.txt',
+        )
+    )
+    flexmock(module.feature).should_receive('available').and_return(True)
+    flexmock(module.flags).should_receive('make_repository_archive_flags').and_return(
+        ('repo::archive',)
+    )
+    flexmock(module.borgmatic.config.validate).should_receive(
+        'normalize_repository_path'
+    ).and_return('repo')
+
+    module.extract_archive(
+        dry_run=False,
+        repository='repo',
+        archive='archive',
+        paths=['/foo/bar/baz.txt', '/foo/bar.txt'],
+        config={},
+        local_borg_version='1.2.3',
+        global_arguments=flexmock(log_json=False),
+        strip_components='all',
+    )
+
+
 def test_extract_archive_with_strip_components_all_and_no_paths_raises():
     flexmock(module.os.path).should_receive('abspath').and_return('repo')
     flexmock(module.feature).should_receive('available').and_return(True)

tests/unit/config/test_constants.py

@@ -50,6 +50,16 @@ def test_apply_constants_with_empty_constants_passes_through_value():
         ({'before_backup': '{inject}'}, {'before_backup': "'echo hi; naughty-command'"}),
         ({'after_backup': '{inject}'}, {'after_backup': "'echo hi; naughty-command'"}),
         ({'on_error': '{inject}'}, {'on_error': "'echo hi; naughty-command'"}),
+        (
+            {
+                'before_backup': '{env_pass}',
+                'postgresql_databases': [{'name': 'users', 'password': '{env_pass}'}],
+            },
+            {
+                'before_backup': "'${PASS}'",
+                'postgresql_databases': [{'name': 'users', 'password': '${PASS}'}],
+            },
+        ),
         (3, 3),
         (True, True),
         (False, False),
@@ -63,6 +73,7 @@ def test_apply_constants_makes_string_substitutions(value, expected_value):
         'int': 3,
         'bool': True,
         'inject': 'echo hi; naughty-command',
+        'env_pass': '${PASS}',
     }
 
     assert module.apply_constants(value, constants) == expected_value

tests/unit/hooks/test_healthchecks.py

@@ -264,6 +264,72 @@ def test_ping_monitor_hits_ping_url_when_states_matching():
     )
 
 
+def test_ping_monitor_adds_create_query_parameter_when_create_slug_true():
+    flexmock(module.borgmatic.hooks.logs).should_receive('Forgetful_buffering_handler').never()
+    hook_config = {'ping_url': 'https://example.com', 'create_slug': True}
+    flexmock(module.requests).should_receive('post').with_args(
+        'https://example.com/start?create=1', data=''.encode('utf-8'), verify=True
+    ).and_return(flexmock(ok=True))
+
+    module.ping_monitor(
+        hook_config,
+        {},
+        'config.yaml',
+        state=module.monitor.State.START,
+        monitoring_log_level=1,
+        dry_run=False,
+    )
+
+
+def test_ping_monitor_does_not_add_create_query_parameter_when_create_slug_false():
+    flexmock(module.borgmatic.hooks.logs).should_receive('Forgetful_buffering_handler').never()
+    hook_config = {'ping_url': 'https://example.com', 'create_slug': False}
+    flexmock(module.requests).should_receive('post').with_args(
+        'https://example.com/start', data=''.encode('utf-8'), verify=True
+    ).and_return(flexmock(ok=True))
+
+    module.ping_monitor(
+        hook_config,
+        {},
+        'config.yaml',
+        state=module.monitor.State.START,
+        monitoring_log_level=1,
+        dry_run=False,
+    )
+
+
+def test_ping_monitor_does_not_add_create_query_parameter_when_ping_url_is_uuid():
+    hook_config = {'ping_url': 'b3611b24-df9c-4d36-9203-fa292820bf2a', 'create_slug': True}
+    flexmock(module.requests).should_receive('post').with_args(
+        f"https://hc-ping.com/{hook_config['ping_url']}",
+        data=''.encode('utf-8'),
+        verify=True,
+    ).and_return(flexmock(ok=True))
+
+    module.ping_monitor(
+        hook_config,
+        {},
+        'config.yaml',
+        state=module.monitor.State.FINISH,
+        monitoring_log_level=1,
+        dry_run=False,
+    )
+
+
+def test_ping_monitor_issues_warning_when_ping_url_is_uuid_and_create_slug_true():
+    hook_config = {'ping_url': 'b3611b24-df9c-4d36-9203-fa292820bf2a', 'create_slug': True}
+    flexmock(module.logger).should_receive('warning').once()
+
+    module.ping_monitor(
+        hook_config,
+        {},
+        'config.yaml',
+        state=module.monitor.State.FINISH,
+        monitoring_log_level=1,
+        dry_run=False,
+    )
+
+
 def test_ping_monitor_with_connection_error_logs_warning():
     flexmock(module.borgmatic.hooks.logs).should_receive('Forgetful_buffering_handler').never()
     hook_config = {'ping_url': 'https://example.com'}