From 4d79f582df8ed756e79688d1371a05efa3a4d5f4 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 18 Jan 2024 10:39:40 -0800 Subject: [PATCH] Fix a traceback when providing an invalid "--override" value for a list option (#814). --- NEWS | 1 + borgmatic/config/override.py | 5 ++++ docs/how-to/make-per-application-backups.md | 26 ++++++++++++--------- tests/unit/config/test_override.py | 7 ++++++ 4 files changed, 28 insertions(+), 11 deletions(-) diff --git a/NEWS b/NEWS index d97daa27..bb5dc78c 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,7 @@ bootstrap" action. Previously, only top-level configuration files were stored. * #810: SECURITY: Prevent shell injection attacks within the PostgreSQL hook, the MongoDB hook, the SQLite hook, the "borgmatic borg" action, and command hook variable/constant interpolation. + * #814: Fix a traceback when providing an invalid "--override" value for a list option. 1.8.6 * #767: Add an "--ssh-command" flag to the "config bootstrap" action for setting a custom SSH diff --git a/borgmatic/config/override.py b/borgmatic/config/override.py index 22365ec2..d1ab1277 100644 --- a/borgmatic/config/override.py +++ b/borgmatic/config/override.py @@ -13,6 +13,11 @@ def set_values(config, keys, value): first_key = keys[0] if len(keys) == 1: + if isinstance(config, list): + raise ValueError( + 'When overriding a list option, the value must use list syntax (e.g., "[foo, bar]" or "[{key: value}]" as appropriate)' + ) + config[first_key] = value return diff --git a/docs/how-to/make-per-application-backups.md b/docs/how-to/make-per-application-backups.md index 6e6489c1..091c7dfd 100644 --- a/docs/how-to/make-per-application-backups.md +++ b/docs/how-to/make-per-application-backups.md @@ -495,21 +495,29 @@ borgmatic create --override parent_option.option1=value1 --override parent_optio forget to specify the section that an option is in. That looks like a prefix on the option name, e.g. `location.repositories`. -Note that each value is parsed as an actual YAML string, so you can even set -list values by using brackets. For instance: +Note that each value is parsed as an actual YAML string, so you can set list +values by using brackets. For instance: ```bash borgmatic create --override repositories=[test1.borg,test2.borg] ``` -Or even a single list element: +Or a single list element: ```bash borgmatic create --override repositories=[/root/test.borg] ``` -If your override value contains special YAML characters like colons, then -you'll need quotes for it to parse correctly: +Or a single list element that is a key/value pair: + +```bash +borgmatic create --override repositories="[{path: test.borg, label: test}]" +``` + +If your override value contains characters like colons or spaces, then you'll +need to use quotes for it to parse correctly. + +Another example: ```bash borgmatic create --override repositories="['user@server:test.borg']" @@ -518,16 +526,12 @@ borgmatic create --override repositories="['user@server:test.borg']" There is not currently a way to override a single element of a list without replacing the whole list. -Note that if you override an option of the list type (like -`location.repositories`), you do need to use the `[ ]` list syntax. See the -[configuration +Using the `[ ]` list syntax is required when overriding an option of the list +type (like `location.repositories`). See the [configuration reference](https://torsion.org/borgmatic/docs/reference/configuration/) for which options are list types. (YAML list values look like `- this` with an indentation and a leading dash.) -Be sure to quote your overrides if they contain spaces or other characters -that your shell may interpret. - An alternate to command-line overrides is passing in your values via [environment variables](https://torsion.org/borgmatic/docs/how-to/provide-your-passwords/). diff --git a/tests/unit/config/test_override.py b/tests/unit/config/test_override.py index f0c506a9..ef6d0f64 100644 --- a/tests/unit/config/test_override.py +++ b/tests/unit/config/test_override.py @@ -44,6 +44,13 @@ def test_set_values_with_multiple_keys_updates_hierarchy(): assert config == {'option': {'key': 'value', 'other': 'other_value'}} +def test_set_values_with_key_when_list_index_expected_errors(): + config = {'option': ['foo', 'bar', 'baz']} + + with pytest.raises(ValueError): + module.set_values(config, keys=('option', 'key'), value='value') + + @pytest.mark.parametrize( 'schema,option_keys,expected_type', (