diff --git a/changes/914.changed b/changes/914.changed new file mode 100644 index 00000000..f6ba3b34 --- /dev/null +++ b/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. diff --git a/changes/914.fixed b/changes/914.fixed new file mode 100644 index 00000000..9b11aa10 --- /dev/null +++ b/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. diff --git a/netutils/config/clean.py b/netutils/config/clean.py index d45263a2..ba336555 100644 --- a/netutils/config/clean.py +++ b/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...)`) 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...)`) 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) diff --git a/tests/unit/test_config_clean.py b/tests/unit/test_config_clean.py index b7d33d91..611b7734 100644 --- a/tests/unit/test_config_clean.py +++ b/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\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\S+) privilege 15 secret 9 (?P\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"}] 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 "}, + ] + assert clean.sanitize_config_jinja(config, filters) == "secret " + + +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"}] + assert clean.sanitize_config_jinja("enable secret 5 supersecret", filters) == "enable secret 5 " + + +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", "render_jinja": True}] + assert clean.sanitize_config_jinja(config, filters) == "enable secret 5 " + + +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 %}{% else %}{% endif %}", + "render_jinja": True, + } + ] + assert clean.sanitize_config_jinja(config, filters) == "enable secret 5 \nenable secret 5 " + + +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