From 6cc93c4eb93db2bbffa778cafc3a01b69f1e7d5d Mon Sep 17 00:00:00 2001
From: Dan Helfman <witten@torsion.org>
Date: Fri, 3 Nov 2023 21:16:04 -0700
Subject: [PATCH] Fix environment variable interpolation within configured
 repository paths (#782).

---
 NEWS                                      |  1 +
 borgmatic/config/environment.py           | 34 +++++++++++++----------
 borgmatic/config/validate.py              |  4 ++-
 tests/integration/config/test_validate.py |  8 ++++--
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/NEWS b/NEWS
index 012584d..149bb12 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,7 @@
    overriding the existing "archive_name_format" and "match_archives" options in configuration.
  * #779: Only parse "--override" values as complex data types when they're for options of those
    types.
+ * #782: Fix environment variable interpolation within configured repository paths.
 
 1.8.4
  * #715: Add a monitoring hook for sending backup status to a variety of monitoring services via the
diff --git a/borgmatic/config/environment.py b/borgmatic/config/environment.py
index a2857bb..fd59584 100644
--- a/borgmatic/config/environment.py
+++ b/borgmatic/config/environment.py
@@ -1,21 +1,22 @@
 import os
 import re
 
-_VARIABLE_PATTERN = re.compile(
+VARIABLE_PATTERN = re.compile(
     r'(?P<escape>\\)?(?P<variable>\$\{(?P<name>[A-Za-z0-9_]+)((:?-)(?P<default>[^}]+))?\})'
 )
 
 
-def _resolve_string(matcher):
+def resolve_string(matcher):
     '''
-    Get the value from environment given a matcher containing a name and an optional default value.
-    If the variable is not defined in environment and no default value is provided, an Error is raised.
+    Given a matcher containing a name and an optional default value, get the value from environment.
+
+    Raise ValueError if the variable is not defined in environment and no default value is provided.
     '''
     if matcher.group('escape') is not None:
-        # in case of escaped envvar, unescape it
+        # In the case of an escaped environment variable, unescape it.
         return matcher.group('variable')
 
-    # resolve the env var
+    # Resolve the environment variable.
     name, default = matcher.group('name'), matcher.group('default')
     out = os.getenv(name, default=default)
 
@@ -27,19 +28,24 @@ def _resolve_string(matcher):
 
 def resolve_env_variables(item):
     '''
-    Resolves variables like or ${FOO} from given configuration with values from process environment
-    Supported formats:
-     - ${FOO} will return FOO env variable
-     - ${FOO-bar} or ${FOO:-bar} will return FOO env variable if it exists, else "bar"
+    Resolves variables like or ${FOO} from given configuration with values from process environment.
 
-    If any variable is missing in environment and no default value is provided, an Error is raised.
+    Supported formats:
+
+     * ${FOO} will return FOO env variable
+     * ${FOO-bar} or ${FOO:-bar} will return FOO env variable if it exists, else "bar"
+
+    Raise if any variable is missing in environment and no default value is provided.
     '''
     if isinstance(item, str):
-        return _VARIABLE_PATTERN.sub(_resolve_string, item)
+        return VARIABLE_PATTERN.sub(resolve_string, item)
+
     if isinstance(item, list):
-        for i, subitem in enumerate(item):
-            item[i] = resolve_env_variables(subitem)
+        for index, subitem in enumerate(item):
+            item[index] = resolve_env_variables(subitem)
+
     if isinstance(item, dict):
         for key, value in item.items():
             item[key] = resolve_env_variables(value)
+
     return item
diff --git a/borgmatic/config/validate.py b/borgmatic/config/validate.py
index 8024bc6..c93022f 100644
--- a/borgmatic/config/validate.py
+++ b/borgmatic/config/validate.py
@@ -110,10 +110,12 @@ def parse_configuration(config_filename, schema_filename, overrides=None, resolv
         raise Validation_error(config_filename, (str(error),))
 
     override.apply_overrides(config, schema, overrides)
-    logs = normalize.normalize(config_filename, config)
+
     if resolve_env:
         environment.resolve_env_variables(config)
 
+    logs = normalize.normalize(config_filename, config)
+
     try:
         validator = jsonschema.Draft7Validator(schema)
     except AttributeError:  # pragma: no cover
diff --git a/tests/integration/config/test_validate.py b/tests/integration/config/test_validate.py
index 545c9d5..92be96d 100644
--- a/tests/integration/config/test_validate.py
+++ b/tests/integration/config/test_validate.py
@@ -1,4 +1,5 @@
 import io
+import os
 import string
 import sys
 
@@ -244,7 +245,7 @@ def test_parse_configuration_applies_overrides():
     assert logs == []
 
 
-def test_parse_configuration_applies_normalization():
+def test_parse_configuration_applies_normalization_after_environment_variable_interpolation():
     mock_config_and_schema(
         '''
         location:
@@ -252,17 +253,18 @@ def test_parse_configuration_applies_normalization():
                 - /home
 
             repositories:
-                - path: hostname.borg
+                - ${NO_EXIST:-user@hostname:repo}
 
             exclude_if_present: .nobackup
         '''
     )
+    flexmock(os).should_receive('getenv').replace_with(lambda variable_name, default: default)
 
     config, logs = module.parse_configuration('/tmp/config.yaml', '/tmp/schema.yaml')
 
     assert config == {
         'source_directories': ['/home'],
-        'repositories': [{'path': 'hostname.borg'}],
+        'repositories': [{'path': 'ssh://user@hostname/./repo'}],
         'exclude_if_present': ['.nobackup'],
     }
     assert logs