Fixed sanitize_config_jinja to use an explicit opt-in boolean

changes/914.changed

@@ -0,0 +1 @@
+Changed `sanitize_config_jinja` to render a filter's replacement as a Jinja template only when that filter sets `render_jinja` to `True`, rather than whenever the replacement contains any Jinja expression.

changes/914.fixed

@@ -0,0 +1 @@
+Fixed `sanitize_config_jinja` emitting `re.sub` backreferences literally when they appeared outside a Jinja expression, and rewriting those inside a `{% raw %}` block.

netutils/config/clean.py

@@ -14,6 +14,18 @@
 except ImportError:
     HAS_JINJA2 = False
 
+# A `re.sub` backreference, such as \1, within a replacement template.
+_RE_BACKREF = re.compile(r"\\(\d+)")
+
+# The regions of a replacement template that are not literal text. The raw alternative is listed
+# first so that a `{% raw %}` block is matched whole instead of as a bare statement.
+_RE_JINJA_SEGMENT = re.compile(
+    r"({%-?\s*raw\s*-?%}.*?{%-?\s*endraw\s*-?%})"  # raw block
+    r"|({{.*?}})"  # expression
+    r"|({%.*?%})",  # statement
+    re.DOTALL,
+)
+
 
 def clean_config(config: str, filters: t.List[t.Dict[str, str]]) -> str:
     r"""Given a list of regex patterns, delete those lines that match.
@@ -92,27 +104,62 @@ def sanitize_config(config: str, filters: t.Optional[t.List[t.Dict[str, str]]] =
     return config
 
 
-def sanitize_config_jinja(config: str, filters: t.Optional[t.List[t.Dict[str, str]]] = None) -> str:
-    r"""Like `sanitize_config`, but renders each `replace` value as a Jinja2 template.
+def _prepare_template(replace: str) -> str:
+    r"""Rewrite the `re.sub` backreferences in a replacement template into Jinja references.
+
+    Positional groups are reached through `_re_groups`, so how a backreference is rewritten
+    depends on where it sits: inside a Jinja expression or statement it becomes a bare
+    subscript, and in literal text it becomes an expression of its own. A `{% raw %}` block
+    is emitted verbatim, so anything inside it stays literal.
+
+    Args:
+        replace: A Jinja-aware replacement template.
+
+    Returns:
+        str: The template with its backreferences rewritten, ready to render.
+    """
+    parts = []
+    position = 0
+    for segment in _RE_JINJA_SEGMENT.finditer(replace):
+        parts.append(_RE_BACKREF.sub(r"{{ _re_groups[\1] }}", replace[position : segment.start()]))
+        raw_block, expression, statement = segment.groups()
+        if raw_block is not None:
+            parts.append(raw_block)
+        else:
+            parts.append(_RE_BACKREF.sub(r"_re_groups[\1]", expression or statement))
+        position = segment.end()
+    parts.append(_RE_BACKREF.sub(r"{{ _re_groups[\1] }}", replace[position:]))
+    return "".join(parts)
+
+
+def sanitize_config_jinja(config: str, filters: t.Optional[t.List[t.Dict[str, t.Any]]] = None) -> str:
+    r"""Like `sanitize_config`, but renders opted-in `replace` values as Jinja2 templates.
 
     This allows the replacement text to transform the matched data, e.g. hashing a secret
-    with the `hash_data` filter instead of dropping it with a static placeholder. The regex
-    capture groups are exposed to the template so the original values can be transformed in
-    place. References to capture groups follow the familiar `re.sub` backreference syntax
-    (`\1`, `\2`, ...) and may be used anywhere inside a `{{ ... }}` expression. Named
-    groups (`(?P<name>...)`) are additionally available by name, so a user-defined group
-    name can never shadow a positional backreference.
+    with the `hash_data` filter instead of dropping it with a static placeholder. A filter
+    opts in by setting `render_jinja` to `True`; every other filter is substituted with plain
+    `re.sub`, so a mixed list of filters works as expected.
+
+    The regex capture groups are exposed to the template so the original values can be
+    transformed in place. References to capture groups follow the familiar `re.sub`
+    backreference syntax (`\1`, `\2`, ...) and may be used both inside and outside a
+    `{{ ... }}` expression. Named groups (`(?P<name>...)`) are additionally available by
+    name, so a user-defined group name can never shadow a positional backreference.
 
-    A `replace` value that contains no Jinja expression (`{{`) falls back to plain
-    `re.sub` string substitution, so a mixed list of filters works as expected.
+    Jinja that should reach the sanitized configuration as literal text, such as a
+    placeholder rendered later by a separate templating pass, must be wrapped in
+    `{% raw %}...{% endraw %}`. The contents of a raw block, backreferences included, are
+    passed through untouched.
 
     This function requires the optional `jinja2` dependency
-    (`pip install netutils[optionals]`).
+    (`pip install netutils[optionals]`) when at least one filter opts in.
 
     Args:
         config: A string representation of a device configuration.
-        filters: A list of dictionaries of regex patterns and Jinja-aware replacement
-            templates used to sanitize configuration. Defaults to an empty list.
+        filters: A list of dictionaries of regex patterns and replacement templates used to
+            sanitize configuration, each optionally setting `render_jinja` to `True` to render its
+            replacement as a Jinja template. Defaults to `None`, which returns the configuration
+            unchanged.
 
     Returns:
         str: Sanitized configuration.
@@ -122,8 +169,9 @@ def sanitize_config_jinja(config: str, filters: t.Optional[t.List[t.Dict[str, st
         >>> config = "username admin privilege 15 secret 9 SuperSecret"
         >>> SANITIZE_FILTERS = [
         ...     {
-        ...         "regex": r"^username (\S+) privilege 15 secret 9 (\S+)$",
-        ...         "replace": r"username {{ \1 }} privilege 15 secret 9 {{ \2 | hash_data('md5') }}",
+        ...         "regex": r"^(username \S+ privilege 15 secret 9 )(\S+)$",
+        ...         "replace": r"\1{{ \2 | hash_data('md5') }}",
+        ...         "render_jinja": True,
         ...     }
         ... ]
         >>> sanitize_config_jinja(config, SANITIZE_FILTERS)
@@ -132,8 +180,8 @@ def sanitize_config_jinja(config: str, filters: t.Optional[t.List[t.Dict[str, st
     if not filters:
         return config
 
-    # Only the Jinja path needs jinja2; if every filter is plain, behave like sanitize_config.
-    if not any("{{" in item["replace"] for item in filters):
+    # Only the Jinja path needs jinja2; if no filter opts in, behave like sanitize_config.
+    if not any(item.get("render_jinja", False) for item in filters):
         return sanitize_config(config, filters)
 
     if not HAS_JINJA2:
@@ -146,8 +194,7 @@ def sanitize_config_jinja(config: str, filters: t.Optional[t.List[t.Dict[str, st
     env.filters.update(jinja2_convenience_function())
 
     def _make_replacer(template_str: str) -> t.Callable[[t.Match[str]], str]:
-        jinja_ready = re.sub(r"\\(\d+)", r"_re_groups[\1]", template_str)
-        template = env.from_string(jinja_ready)
+        template = env.from_string(_prepare_template(template_str))
 
         def _replace(match: t.Match[str]) -> str:
             # Named groups are exposed by name; positional groups are reached via `_re_groups`,
@@ -159,7 +206,7 @@ def _replace(match: t.Match[str]) -> str:
         return _replace
 
     for item in filters:
-        if "{{" in item["replace"]:
+        if item.get("render_jinja", False):
             config = re.sub(item["regex"], _make_replacer(item["replace"]), config, flags=re.MULTILINE)
         else:
             config = re.sub(item["regex"], item["replace"], config, flags=re.MULTILINE)

tests/unit/test_config_clean.py

@@ -35,12 +35,23 @@ def test_sanitize_config(_file, get_text_data):
 MD5_BAR = "37b51d194a7513e45b56f6524f2d51f2"
 
 
+# A Golden Config postprocessing placeholder: the backup must carry this through verbatim so it
+# matches the intended config, which holds the same literal string for `render_secrets` to fill in.
+POSTPROCESSING_REPLACE = r'\1{{ secrets_group["name"] | get_secret_by_secret_group_name("password") }}'
+POSTPROCESSING_REGEX = r"^(username \S+ password 7 )\S+$"
+POSTPROCESSING_CONFIG = "username foo password 7 bar"
+POSTPROCESSING_EXPECTED = (
+    'username foo password 7 {{ secrets_group["name"] | get_secret_by_secret_group_name("password") }}'
+)
+
+
 def test_sanitize_config_jinja_hashes_capture_group():
     config = "username foo privilege 15 secret 9 bar"
     filters = [
         {
             "regex": r"^username (\S+) privilege 15 secret 9 (\S+)$",
             "replace": r"username {{ \1 | hash_data('md5') }} privilege 15 secret 9 {{ \2 | hash_data('md5') }}",
+            "render_jinja": True,
         }
     ]
     assert clean.sanitize_config_jinja(config, filters) == f"username {MD5_FOO} privilege 15 secret 9 {MD5_BAR}"
@@ -53,6 +64,7 @@ def test_sanitize_config_jinja_carries_static_group_through():
         {
             "regex": r"^username (\S+) (.+) secret 9 (\S+)$",
             "replace": r"username {{ \1 | hash_data('md5') }} {{ \2 }} secret 9 {{ \3 | hash_data('md5') }}",
+            "render_jinja": True,
         }
     ]
     assert clean.sanitize_config_jinja(config, filters) == f"username {MD5_FOO} privilege 15 secret 9 {MD5_BAR}"
@@ -65,6 +77,7 @@ def test_sanitize_config_jinja_named_group():
         {
             "regex": r"^username (?P<user>\S+) privilege 15 secret 9 (\S+)$",
             "replace": r"username {{ user }} privilege 15 secret 9 {{ \2 | hash_data('md5') }}",
+            "render_jinja": True,
         }
     ]
     assert clean.sanitize_config_jinja(config, filters) == f"username foo privilege 15 secret 9 {MD5_BAR}"
@@ -77,6 +90,7 @@ def test_sanitize_config_jinja_named_group_piped_through_hash_data():
         {
             "regex": r"^username (?P<user>\S+) privilege 15 secret 9 (?P<secret>\S+)$",
             "replace": r"username {{ user | hash_data('md5') }} privilege 15 secret 9 {{ secret | hash_data('md5') }}",
+            "render_jinja": True,
         }
     ]
     assert clean.sanitize_config_jinja(config, filters) == f"username {MD5_FOO} privilege 15 secret 9 {MD5_BAR}"
@@ -90,6 +104,7 @@ def test_sanitize_config_jinja_mixed_filters():
         {
             "regex": r"^username (\S+) privilege 15 secret 9 (\S+)$",
             "replace": r"username {{ \1 }} privilege 15 secret 9 {{ \2 | hash_data('md5') }}",
+            "render_jinja": True,
         },
     ]
     assert (
@@ -99,7 +114,7 @@ def test_sanitize_config_jinja_mixed_filters():
 
 
 def test_sanitize_config_jinja_no_jinja_matches_sanitize_config():
-    # With no Jinja in any replace, the result matches plain sanitize_config.
+    # With no filter opting in, the result matches plain sanitize_config.
     config = "enable secret 5 supersecret"
     filters = [{"regex": r"^(enable secret 5 ).+$", "replace": r"\1<removed>"}]
     assert clean.sanitize_config_jinja(config, filters) == clean.sanitize_config(config, filters)
@@ -109,3 +124,121 @@ def test_sanitize_config_jinja_empty_filters():
     config = "username foo privilege 15 secret 9 bar"
     assert clean.sanitize_config_jinja(config, None) == config
     assert clean.sanitize_config_jinja(config, []) == config
+
+
+def test_sanitize_config_jinja_unflagged_jinja_is_not_rendered():
+    # Without the `render_jinja` key, a replace containing Jinja is substituted literally. Rendering
+    # would raise, since `get_secret_by_secret_group_name` is not a netutils filter.
+    filters = [{"regex": POSTPROCESSING_REGEX, "replace": POSTPROCESSING_REPLACE}]
+    assert clean.sanitize_config_jinja(POSTPROCESSING_CONFIG, filters) == POSTPROCESSING_EXPECTED
+
+
+def test_sanitize_config_jinja_explicitly_disabled_is_not_rendered():
+    filters = [{"regex": POSTPROCESSING_REGEX, "replace": POSTPROCESSING_REPLACE, "render_jinja": False}]
+    assert clean.sanitize_config_jinja(POSTPROCESSING_CONFIG, filters) == POSTPROCESSING_EXPECTED
+
+
+def test_sanitize_config_jinja_unflagged_jinja_alongside_flagged_filter():
+    # One filter opting in does not drag an unflagged postprocessing placeholder into the renderer.
+    config = f"{POSTPROCESSING_CONFIG}\nenable secret 9 foo"
+    filters = [
+        {"regex": POSTPROCESSING_REGEX, "replace": POSTPROCESSING_REPLACE},
+        {"regex": r"^(enable secret 9 )(\S+)$", "replace": r"\1{{ \2 | hash_data('md5') }}", "render_jinja": True},
+    ]
+    assert clean.sanitize_config_jinja(config, filters) == f"{POSTPROCESSING_EXPECTED}\nenable secret 9 {MD5_FOO}"
+
+
+def test_sanitize_config_jinja_filters_are_applied_in_order():
+    config = "secret foo"
+    filters = [
+        {"regex": r"^secret (\S+)$", "replace": r"secret {{ \1 | hash_data('md5') }}", "render_jinja": True},
+        {"regex": f"^secret {MD5_FOO}$", "replace": "secret <removed>"},
+    ]
+    assert clean.sanitize_config_jinja(config, filters) == "secret <removed>"
+
+
+def test_sanitize_config_jinja_requires_jinja2_when_a_filter_opts_in(monkeypatch):
+    monkeypatch.setattr(clean, "HAS_JINJA2", False)
+    filters = [{"regex": r"^(enable secret 9 )(\S+)$", "replace": r"\1{{ \2 }}", "render_jinja": True}]
+    with pytest.raises(ImportError, match="jinja2"):
+        clean.sanitize_config_jinja("enable secret 9 foo", filters)
+
+
+def test_sanitize_config_jinja_does_not_require_jinja2_when_no_filter_opts_in(monkeypatch):
+    monkeypatch.setattr(clean, "HAS_JINJA2", False)
+    filters = [{"regex": r"^(enable secret 5 ).+$", "replace": r"\1<removed>"}]
+    assert clean.sanitize_config_jinja("enable secret 5 supersecret", filters) == "enable secret 5 <removed>"
+
+
+def test_sanitize_config_jinja_backreference_outside_expression():
+    config = "username foo privilege 15 secret 9 bar"
+    filters = [
+        {
+            "regex": r"^(username \S+ privilege 15 secret 9 )(\S+)$",
+            "replace": r"\1{{ \2 | hash_data('md5') }}",
+            "render_jinja": True,
+        }
+    ]
+    assert clean.sanitize_config_jinja(config, filters) == f"username foo privilege 15 secret 9 {MD5_BAR}"
+
+
+def test_sanitize_config_jinja_backreference_without_any_expression():
+    # An opted-in filter whose replace holds no Jinja at all still resolves its backreferences.
+    config = "enable secret 5 supersecret"
+    filters = [{"regex": r"^(enable secret 5 ).+$", "replace": r"\1<removed>", "render_jinja": True}]
+    assert clean.sanitize_config_jinja(config, filters) == "enable secret 5 <removed>"
+
+
+def test_sanitize_config_jinja_whole_match_backreference():
+    config = "enable secret 5 supersecret"
+    filters = [{"regex": r"^enable secret 5 .+$", "replace": r"! \0", "render_jinja": True}]
+    assert clean.sanitize_config_jinja(config, filters) == "! enable secret 5 supersecret"
+
+
+def test_sanitize_config_jinja_two_digit_backreference():
+    config = "a b c d e f g h i j"
+    filters = [
+        {
+            "regex": r"^" + r" ".join(r"(\S+)" for _ in range(10)) + r"$",
+            "replace": r"\10 \1",
+            "render_jinja": True,
+        }
+    ]
+    assert clean.sanitize_config_jinja(config, filters) == "j a"
+
+
+def test_sanitize_config_jinja_raw_block_passes_jinja_through():
+    # The opt-in escape hatch: wrap postprocessing Jinja in `{% raw %}` so it survives rendering.
+    raw_replace = r'\1{% raw %}{{ secrets_group["name"] | get_secret_by_secret_group_name("password") }}{% endraw %}'
+    filters = [{"regex": POSTPROCESSING_REGEX, "replace": raw_replace, "render_jinja": True}]
+    assert clean.sanitize_config_jinja(POSTPROCESSING_CONFIG, filters) == POSTPROCESSING_EXPECTED
+
+
+def test_sanitize_config_jinja_raw_block_with_whitespace_control():
+    raw_replace = r"\1{%- raw -%}{{ secret }}{%- endraw -%}"
+    filters = [{"regex": POSTPROCESSING_REGEX, "replace": raw_replace, "render_jinja": True}]
+    assert clean.sanitize_config_jinja(POSTPROCESSING_CONFIG, filters) == "username foo password 7 {{ secret }}"
+
+
+def test_sanitize_config_jinja_backreference_inside_raw_block_stays_literal():
+    filters = [{"regex": POSTPROCESSING_REGEX, "replace": r"{% raw %}\1{% endraw %}", "render_jinja": True}]
+    assert clean.sanitize_config_jinja(POSTPROCESSING_CONFIG, filters) == r"\1"
+
+
+def test_sanitize_config_jinja_backreference_inside_statement_block():
+    config = "enable secret 5 supersecret\nenable secret 5 "
+    filters = [
+        {
+            "regex": r"^(enable secret 5 )(.*)$",
+            "replace": r"\1{% if \2 %}<removed>{% else %}<empty>{% endif %}",
+            "render_jinja": True,
+        }
+    ]
+    assert clean.sanitize_config_jinja(config, filters) == "enable secret 5 <removed>\nenable secret 5 <empty>"
+
+
+def test_sanitize_config_jinja_captured_value_is_not_treated_as_jinja():
+    # Device output that happens to look like Jinja must not be rendered as part of the template.
+    config = "banner motd {{ 7 * 7 }}"
+    filters = [{"regex": r"^(banner motd )(.+)$", "replace": r"\1\2", "render_jinja": True}]
+    assert clean.sanitize_config_jinja(config, filters) == config