From b0e083eff23a035634981dd805abe8442fdf145e Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 5 Jan 2026 15:39:14 -0800 Subject: [PATCH] BREAKING: Support disabling both constant and variable interpolation by escaping with backslashes (#1213). --- NEWS | 3 + borgmatic/config/constants.py | 60 +++++++++++--- borgmatic/hooks/command.py | 82 +++++++++++++------ docs/reference/configuration/command-hooks.md | 28 ++++++- docs/reference/configuration/constants.md | 19 +++++ tests/integration/config/test_constants.py | 64 +++++++++++++++ tests/integration/hooks/test_command.py | 44 ++++++++++ tests/unit/config/test_constants.py | 55 ------------- tests/unit/hooks/test_command.py | 34 -------- 9 files changed, 262 insertions(+), 127 deletions(-) create mode 100644 tests/integration/config/test_constants.py create mode 100644 tests/integration/hooks/test_command.py diff --git a/NEWS b/NEWS index b0e0047c..10b8e1a4 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,9 @@ * #1211: Fix an error about the runtime directory getting excluded by tweaking its logic and lowering the error to a warning. * #1212: Fix an error when restoring multiple directory-format database dumps at once. + * #1213: BREAKING: Support disabling both constant and variable interpolation by escaping with + backslashes. For instance, interpret "\{name\}" as literally "{name}" instead of trying to + resolve it as a constant/variable. * #1220: Cleanup snapshots immediately after ZFS, LVM, or Btrfs hooks error—rather than waiting until the next time borgmatic runs. * #1221: Add an "unsafe_skip_path_validation_before_create" option to skip pre-backup safety checks diff --git a/borgmatic/config/constants.py b/borgmatic/config/constants.py index 806e656f..3478980d 100644 --- a/borgmatic/config/constants.py +++ b/borgmatic/config/constants.py @@ -1,4 +1,6 @@ import contextlib +import functools +import re import shlex @@ -21,7 +23,44 @@ def coerce_scalar(value): return value -def apply_constants(value, constants, shell_escape=False): +CONSTANT_PATTERN = re.compile(r'(?P\\)?\{(?P[\w]+)(?P\\)?\}') + + +def resolve_constant(match, constants, command_hook): + ''' + Given a re.Match instance of CONSTANT_PATTERN representing a matched constant name to be + interpolated, a constants dict, and whether this is for a command hook, lookup the matched + constant name within the given constants and return its value. + + If the match is escaped with backslashes, then instead of resolving the variable's value, strip + off the backslashing and return the literal value. + + If the variable name isn't found in the given constants, then return the literal value. + ''' + name = match.group('name') + + # The would-be variable is escaped, so strip off the escaping and return the result without + # resolving the name—unless this is for a command hook, in which case just return the literal + # string. That way, subsequent variable interpolation will still see the string as escaped + # instead of trying to interpolate it. + if match.group('left_escape') and match.group('right_escape'): + if command_hook: + return match.group(0) + + return '{' + name + '}' + + value = constants.get(name) + + # The matched variable is in the constants, so return its value. And if this is for a command + # hook, then shell escape the value so as to prevent shell injection attacks. + if value is not None: + return shlex.quote(str(value)) if command_hook else str(value) + + # The matched variable name isn't in the constants. Return the whole string unaltered. + return match.group(0) + + +def apply_constants(value, constants, command_hook=False): ''' Given a configuration value (bool, dict, int, list, or string) and a dict of named constants, replace any configuration string values of the form "{constant}" (or containing it) with the @@ -39,24 +78,23 @@ def apply_constants(value, constants, shell_escape=False): return value if isinstance(value, str): - for constant_name, constant_value in constants.items(): - value = value.replace( - '{' + constant_name + '}', - shlex.quote(str(constant_value)) if shell_escape else str(constant_value), - ) - # Support constants within non-string scalars by coercing the value to its appropriate type. - value = coerce_scalar(value) + value = coerce_scalar( + CONSTANT_PATTERN.sub( + functools.partial(resolve_constant, constants=constants, command_hook=command_hook), + value, + ) + ) elif isinstance(value, list): for index, list_value in enumerate(value): - value[index] = apply_constants(list_value, constants, shell_escape) + value[index] = apply_constants(list_value, constants, command_hook) elif isinstance(value, dict): for option_name, option_value in value.items(): value[option_name] = apply_constants( option_value, constants, - shell_escape=( - shell_escape + command_hook=( + command_hook or option_name.startswith(('before_', 'after_')) or option_name in {'on_error', 'run'} ), diff --git a/borgmatic/hooks/command.py b/borgmatic/hooks/command.py index 6140d0bc..a1c6014d 100644 --- a/borgmatic/hooks/command.py +++ b/borgmatic/hooks/command.py @@ -13,39 +13,71 @@ logger = logging.getLogger(__name__) SOFT_FAIL_EXIT_CODE = 75 -BORG_PLACEHOLDERS = { - '{hostname}', - '{fqdn}', - '{reverse-fqdn}', - '{now}', - '{utcnow}', - '{unixtime}', - '{user}', - '{pid}', - '{borgversion}', - '{borgmajor}', - '{borgminor}', - '{borgpatch}', +BORG_PLACEHOLDER_NAMES = { + 'hostname', + 'fqdn', + 'reverse-fqdn', + 'now', + 'utcnow', + 'unixtime', + 'user', + 'pid', + 'borgversion', + 'borgmajor', + 'borgminor', + 'borgpatch', } +VARIABLE_PATTERN = re.compile(r'(?P\\)?\{(?P[\w]+)(?P\\)?\}') + + +def resolve_variable(match, context, hook_description): + ''' + Given a re.Match instance of VARIABLE_PATTERN representing a matched variable name to be + interpolated, a context dict, and a description of the current command hook, lookup the matched + variable name within the given context and return its value. + + If the match is escaped with backslashes, then instead of resolving the variable's value, strip + off the backslashing and return the literal value. + + If the variable name isn't found in the given context (and isn't a Borg placeholder), then + warn and return the literal value. + ''' + name = match.group('name') + + # The would-be variable is escaped, so strip off the escaping and return the result without + # resolving the name. + if match.group('left_escape') and match.group('right_escape'): + return '{' + name + '}' + + value = context.get(name) + + # The matched variable is in the context, so return its value. + if value is not None: + return shlex.quote(str(value)) + + # The matched variable name isn't in the context. Warn about variables unknown to borgmatic, but + # don't warn if the variable name happens to be a Borg placeholder, as Borg should hopefully + # consume it. + if name not in BORG_PLACEHOLDER_NAMES: + logger.warning( + f'Variable "{name}" is not supported in the {hook_description} hook', + ) + + # Return the whole string unaltered. + return match.group(0) + + def interpolate_context(hook_description, command, context): ''' Given a config filename, a hook description, a single hook command, and a dict of context names/values, interpolate the values by "{name}" into the command and return the result. ''' - for name, value in context.items(): - command = command.replace(f'{{{name}}}', shlex.quote(str(value))) - - for unsupported_variable in re.findall(r'\{\w+\}', command): - # Warn about variables unknown to borgmatic, but don't warn if the variable name happens to - # be a Borg placeholder, as Borg should hopefully consume it. - if unsupported_variable not in BORG_PLACEHOLDERS: - logger.warning( - f'Variable "{unsupported_variable}" is not supported in the {hook_description} hook', - ) - - return command + return VARIABLE_PATTERN.sub( + functools.partial(resolve_variable, context=context, hook_description=hook_description), + command, + ) def make_environment(current_environment, sys_module=sys): diff --git a/docs/reference/configuration/command-hooks.md b/docs/reference/configuration/command-hooks.md index c36a6b8a..2cb90486 100644 --- a/docs/reference/configuration/command-hooks.md +++ b/docs/reference/configuration/command-hooks.md @@ -135,6 +135,12 @@ don't run in the context of a single repository. But the deprecated command hooks (`before_backup`, `on_error`, etc.) do generally support variable interpolation. +Note that you can also interpolate [arbitrary environment +variables](https://torsion.org/borgmatic/reference/configuration/environment-variables/). + + +### Shell escaping + borgmatic automatically escapes these interpolated values to prevent shell injection attacks. One implication is that you shouldn't wrap the interpolated values in your own quotes, as that will interfere with the quoting performed by @@ -158,8 +164,26 @@ commands: - send-text-message.sh {error} ``` -Note that you can also interpolate [arbitrary environment -variables](https://torsion.org/borgmatic/reference/configuration/environment-variables/). + +### Disabling variable interpolation + +New in version 2.1.0 To prevent +borgmatic from attempting variable interpolation on a specific would-be variable +name—or warning about unknown variables—you can backslash its curly brackets. +For instance: + +```yaml +commands: + - after: action + when: [prune] + run: + - record-prune.sh \{name\} +``` + +This tells borgmatic to skip variable interpolation for `{name}` and instead +pass the `{name}` literal to the command. This is handy if you've got a command +containing literal curly brackets around a name that appears to reference a +variable—but actually doesn't. ## Soft failure diff --git a/docs/reference/configuration/constants.md b/docs/reference/configuration/constants.md index a351f06d..eba2a27b 100644 --- a/docs/reference/configuration/constants.md +++ b/docs/reference/configuration/constants.md @@ -75,3 +75,22 @@ that uses that constant. An alternate to constants is passing in your values via [environment variables](https://torsion.org/borgmatic/reference/configuration/environment-variables/). + + +## Disabling constants + +New in version 2.1.0 To prevent +borgmatic from attempting constant interpolation on a specific would-be +constant name, you can backslash its curly brackets. For instance: + +```yaml +constants: + name: foo + +source_directories: + - /home/user/\{name\} +``` + +This tells borgmatic to skip constant interpolation for `{name}` and instead use +the `{name}` literal. This is handy if you've got a filename that has literal +curly brackets around a name that happens to match a constant. diff --git a/tests/integration/config/test_constants.py b/tests/integration/config/test_constants.py new file mode 100644 index 00000000..472aa8b7 --- /dev/null +++ b/tests/integration/config/test_constants.py @@ -0,0 +1,64 @@ +import pytest +from flexmock import flexmock + +from borgmatic.config import constants as module + + +def test_apply_constants_with_empty_constants_passes_through_value(): + assert module.apply_constants(value='thing', constants={}) == 'thing' + + +@pytest.mark.parametrize( + 'value,expected_value', + ( + (None, None), + ('thing', 'thing'), + ('{foo}', 'bar'), + ('abc{foo}', 'abcbar'), + ('{foo}xyz', 'barxyz'), + ('{foo}{baz}', 'barquux'), + ('{int}', '3'), + ('{bool}', 'True'), + ('{unknown}', '{unknown}'), + (['thing', 'other'], ['thing', 'other']), + (['thing', '{foo}'], ['thing', 'bar']), + (['{foo}', '{baz}'], ['bar', 'quux']), + ({'key': 'value'}, {'key': 'value'}), + ({'key': '{foo}'}, {'key': 'bar'}), + ({'key': '{inject}'}, {'key': 'echo hi; naughty-command'}), + ({'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'"}), + ({'run': '{inject}'}, {'run': "'echo hi; naughty-command'"}), + ({'something': '{inject}'}, {'something': '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), + (r'\{foo\}', '{foo}'), + (r'\{unknown\}', '{unknown}'), + ({'run': r'\{foo\}'}, {'run': r'\{foo\}'}), + ({'run': r'\{unknown\}'}, {'run': r'\{unknown\}'}), + ), +) +def test_apply_constants_makes_string_substitutions(value, expected_value): + flexmock(module).should_receive('coerce_scalar').replace_with(lambda value: value) + constants = { + 'foo': 'bar', + 'baz': 'quux', + 'int': 3, + 'bool': True, + 'inject': 'echo hi; naughty-command', + 'env_pass': '${PASS}', + } + + assert module.apply_constants(value, constants) == expected_value diff --git a/tests/integration/hooks/test_command.py b/tests/integration/hooks/test_command.py new file mode 100644 index 00000000..cbcd2e25 --- /dev/null +++ b/tests/integration/hooks/test_command.py @@ -0,0 +1,44 @@ +from flexmock import flexmock + +from borgmatic.hooks import command as module + + +def test_interpolate_context_passes_through_command_without_variable(): + assert module.interpolate_context('pre-backup', 'ls', {'foo': 'bar'}) == 'ls' + + +def test_interpolate_context_warns_and_passes_through_command_with_unknown_variable(): + command = 'ls {baz}' + flexmock(module.logger).should_receive('warning').once() + + assert module.interpolate_context('pre-backup', command, {'foo': 'bar'}) == command + + +def test_interpolate_context_does_not_warn_and_passes_through_command_with_unknown_variable_matching_borg_placeholder(): + command = 'ls {hostname}' + flexmock(module.logger).should_receive('warning').never() + + assert module.interpolate_context('pre-backup', command, {'foo': 'bar'}) == command + + +def test_interpolate_context_interpolates_variables(): + command = 'ls {foo}{baz} {baz}' + context = {'foo': 'bar', 'baz': 'quux'} + + assert module.interpolate_context('pre-backup', command, context) == 'ls barquux quux' + + +def test_interpolate_context_escapes_interpolated_variables(): + command = 'ls {foo} {inject}' + context = {'foo': 'bar', 'inject': 'hi; naughty-command'} + + assert ( + module.interpolate_context('pre-backup', command, context) == "ls bar 'hi; naughty-command'" + ) + + +def test_interpolate_context_strips_backslashed_names_and_does_not_intepolate_them(): + command = r'ls \{foo\}\{baz\} \{baz\}' + context = {'foo': 'bar', 'baz': 'quux'} + + assert module.interpolate_context('pre-backup', command, context) == 'ls {foo}{baz} {baz}' diff --git a/tests/unit/config/test_constants.py b/tests/unit/config/test_constants.py index d475902b..decd0c89 100644 --- a/tests/unit/config/test_constants.py +++ b/tests/unit/config/test_constants.py @@ -1,5 +1,4 @@ import pytest -from flexmock import flexmock from borgmatic.config import constants as module @@ -24,57 +23,3 @@ from borgmatic.config import constants as module ) def test_coerce_scalar_converts_value(value, expected_value): assert module.coerce_scalar(value) == expected_value - - -def test_apply_constants_with_empty_constants_passes_through_value(): - assert module.apply_constants(value='thing', constants={}) == 'thing' - - -@pytest.mark.parametrize( - 'value,expected_value', - ( - (None, None), - ('thing', 'thing'), - ('{foo}', 'bar'), - ('abc{foo}', 'abcbar'), - ('{foo}xyz', 'barxyz'), - ('{foo}{baz}', 'barquux'), - ('{int}', '3'), - ('{bool}', 'True'), - (['thing', 'other'], ['thing', 'other']), - (['thing', '{foo}'], ['thing', 'bar']), - (['{foo}', '{baz}'], ['bar', 'quux']), - ({'key': 'value'}, {'key': 'value'}), - ({'key': '{foo}'}, {'key': 'bar'}), - ({'key': '{inject}'}, {'key': 'echo hi; naughty-command'}), - ({'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'"}), - ({'run': '{inject}'}, {'run': "'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), - ), -) -def test_apply_constants_makes_string_substitutions(value, expected_value): - flexmock(module).should_receive('coerce_scalar').replace_with(lambda value: value) - constants = { - 'foo': 'bar', - 'baz': 'quux', - 'int': 3, - 'bool': True, - 'inject': 'echo hi; naughty-command', - 'env_pass': '${PASS}', - } - - assert module.apply_constants(value, constants) == expected_value diff --git a/tests/unit/hooks/test_command.py b/tests/unit/hooks/test_command.py index 45666f6c..07c742a1 100644 --- a/tests/unit/hooks/test_command.py +++ b/tests/unit/hooks/test_command.py @@ -7,40 +7,6 @@ from flexmock import flexmock from borgmatic.hooks import command as module -def test_interpolate_context_passes_through_command_without_variable(): - assert module.interpolate_context('pre-backup', 'ls', {'foo': 'bar'}) == 'ls' - - -def test_interpolate_context_warns_and_passes_through_command_with_unknown_variable(): - command = 'ls {baz}' - flexmock(module.logger).should_receive('warning').once() - - assert module.interpolate_context('pre-backup', command, {'foo': 'bar'}) == command - - -def test_interpolate_context_does_not_warn_and_passes_through_command_with_unknown_variable_matching_borg_placeholder(): - command = 'ls {hostname}' - flexmock(module.logger).should_receive('warning').never() - - assert module.interpolate_context('pre-backup', command, {'foo': 'bar'}) == command - - -def test_interpolate_context_interpolates_variables(): - command = 'ls {foo}{baz} {baz}' - context = {'foo': 'bar', 'baz': 'quux'} - - assert module.interpolate_context('pre-backup', command, context) == 'ls barquux quux' - - -def test_interpolate_context_escapes_interpolated_variables(): - command = 'ls {foo} {inject}' - context = {'foo': 'bar', 'inject': 'hi; naughty-command'} - - assert ( - module.interpolate_context('pre-backup', command, context) == "ls bar 'hi; naughty-command'" - ) - - def test_make_environment_without_pyinstaller_does_not_touch_environment(): assert module.make_environment({}, sys_module=flexmock()) == {}