From ad019f6b090a508403039d5158ed405146a851e2 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Mon, 22 Jun 2026 14:45:12 +1000 Subject: [PATCH] normalise control characters in generated manifest scalars --- enroll/puppet.py | 46 ++++++++++++++++++++ tests/test_manifest_puppet.py | 82 +++++++++++++++++++++++++++++++++++ 2 files changed, 128 insertions(+) diff --git a/enroll/puppet.py b/enroll/puppet.py index 1b78789..e034cea 100644 --- a/enroll/puppet.py +++ b/enroll/puppet.py @@ -291,8 +291,54 @@ def _puppet_name(raw: str, *, fallback: str = "role") -> str: return s +# Control characters (C0 range plus DEL) that should never appear raw inside a +# generated Puppet manifest scalar. They cannot occur in values harvested from a +# live host (e.g. /etc/passwd GECOS is newline-delimited), so their presence +# indicates a hand-edited or tampered harvest. Emitting them verbatim is valid +# Puppet but produces multi-line / control-laden manifests; normalise them into +# explicit escapes instead. +_PP_CONTROL_CHARS = frozenset(chr(c) for c in range(0x20)) | {"\x7f"} + +# Puppet double-quoted recognised single-character escapes. +_PP_DQ_ESCAPES = { + "\n": "\\n", + "\t": "\\t", + "\r": "\\r", + "\\": "\\\\", + '"': '\\"', + "$": "\\$", +} + + +def _pp_quote_double(s: str) -> str: + """Render a Puppet double-quoted string with control characters escaped. + + Only used as a fallback when a value contains raw control characters, so the + common case stays single-quoted and byte-identical to historical output. + """ + + out = [] + for ch in s: + esc = _PP_DQ_ESCAPES.get(ch) + if esc is not None: + out.append(esc) + elif ch in _PP_CONTROL_CHARS: + # Puppet supports \uXXXX style escapes inside double-quoted strings. + out.append(f"\\u{{{ord(ch):04x}}}") + else: + out.append(ch) + return '"' + "".join(out) + '"' + + def _pp_quote(value: Any) -> str: s = str(value) + # Puppet single-quoted strings only honour \\ and \' escapes; everything + # else (including a literal newline) is taken verbatim. That is safe but lets + # a tampered harvest splatter raw control characters across the manifest. + # When any are present, fall back to a double-quoted string where they can be + # neutralised into explicit escapes. + if any(ch in _PP_CONTROL_CHARS for ch in s): + return _pp_quote_double(s) s = s.replace("\\", "\\\\").replace("'", "\\'") return f"'{s}'" diff --git a/tests/test_manifest_puppet.py b/tests/test_manifest_puppet.py index e4c4792..c54f6c7 100644 --- a/tests/test_manifest_puppet.py +++ b/tests/test_manifest_puppet.py @@ -1326,3 +1326,85 @@ def test_manifest_puppet_fqdn_writes_erb_template_values_to_node_hiera( ) assert "Any $main_key = undef," in init_pp assert "content => template($attrs['template'])" in init_pp + + +def test_pp_quote_common_case_is_single_quoted_and_stable(): + """Values without control characters keep the historical single-quoted form.""" + from enroll.puppet import _pp_quote + + assert _pp_quote("Alice Example") == "'Alice Example'" + assert _pp_quote("0644") == "'0644'" + assert _pp_quote("/etc/nginx/nginx.conf") == "'/etc/nginx/nginx.conf'" + # Single quote and backslash keep their single-quoted escaping. + assert _pp_quote("a'b") == "'a\\'b'" + assert _pp_quote("back\\slash") == "'back\\\\slash'" + + +def test_pp_quote_neutralises_raw_control_characters(): + """A tampered harvest cannot splatter raw control characters into a manifest. + + GECOS and similar scalars are newline-delimited on a live host, so control + characters only appear via a hand-edited/tampered state.json. When present, + _pp_quote switches to a double-quoted Puppet string and escapes them rather + than emitting them verbatim. + """ + from enroll.puppet import _pp_quote + + rendered = _pp_quote("a\ntouch /tmp/pwned") + assert rendered == '"a\\ntouch /tmp/pwned"' + # No raw C0/DEL byte survives into the rendered scalar. + for value in ("a\nb", "x\r\ny", "a\tb", "a\x00b", "a\x7fb"): + out = _pp_quote(value) + assert not any(ch in out for ch in [chr(c) for c in range(0x20)] + ["\x7f"]) + + +def test_pp_quote_double_fallback_cannot_introduce_interpolation(): + """The double-quoted fallback must not enable Puppet interpolation/breakout.""" + from enroll.puppet import _pp_quote + + # $ would interpolate in a double-quoted Puppet string; it must be escaped. + out = _pp_quote("a\n${::osfamily}") + assert "\\${::osfamily}" in out + assert "${::osfamily}" not in out.replace("\\${::osfamily}", "") + # A double quote cannot terminate the string early. + out2 = _pp_quote('a\n"; notify{x:} ') + assert out2.startswith('"') and out2.endswith('"') + assert '\\"' in out2 + + +def test_manifest_puppet_user_gecos_with_newline_is_single_line(tmp_path: Path): + """End-to-end: a newline in a user's gecos yields a single-line comment.""" + bundle = tmp_path / "bundle" + out = tmp_path / "puppet" + state = { + "roles": { + "users": { + "role_name": "users", + "users": [ + { + "name": "eviluser", + "uid": 1001, + "primary_group": "evil", + "supplementary_groups": [], + "home": "/home/eviluser", + "shell": "/bin/bash", + "gecos": "Real Name\ntouch /tmp/pwned", + } + ], + "managed_files": [], + "managed_dirs": [], + "excluded": [], + "notes": [], + } + } + } + _write_state(bundle, state) + manifest.manifest(str(bundle), str(out), target="puppet") + + init_pp = (out / "modules" / "users" / "manifests" / "init.pp").read_text( + encoding="utf-8" + ) + # The comment attribute must be on one line with the newline escaped. + assert 'comment => "Real Name\\ntouch /tmp/pwned"' in init_pp + # And there must be no line that is just the injected command. + assert "\ntouch /tmp/pwned\n" not in init_pp