Doc updates

README.md

@@ -285,6 +285,8 @@ By default, `enroll` does **not** assume how you handle secrets in Ansible. It w
 
 Safe-mode content scanning is intentionally conservative. It treats common assignment-style credential keys as sensitive, including names such as `password`, `client_secret`, `secret_key`, `auth_token`, `api_key`, `aws_access_key_id`, `aws_secret_access_key`, `azure_client_secret`, `GOOGLE_APPLICATION_CREDENTIALS`, and service-account key names.
 
+**IMPORTANT**: Enroll ignores comments in files! If you have commented out *real secrets*, there's still a risk that Enroll could capture that data even without `--dangerous`. If you are in doubt, play it safe: use `--sops` and/or encrypt the output at rest in a way that makes sense to you.
+
 Automatic harvesting of per-user shell dotfiles is also disabled by default, even when those files differ from `/etc/skel`, because `.bashrc`, `.profile`, `.bash_aliases`, and similar files commonly contain exported tokens, credentials, or aliases/functions with embedded secrets. Use `--dangerous` for automatic shell-dotfile capture, or use targeted `--include-path` patterns for narrower safe-mode review.
 
 If you wish to opt in to collecting everything, use `--dangerous` mode, but be aware of what it means:

SECURITY.md

@@ -49,6 +49,7 @@ The following are generally out of scope and should not be reported as Enroll vu
 * A user configuring a webhook, email target, SSH proxy command, SOPS binary, package manager, or configuration-management tool that they do not trust.
 * A compromised system where an attacker already controls root-owned files, root’s shell, root’s configuration, or the privileged tools Enroll invokes.
 * Reports that amount to “if root runs this tool with malicious options, root can make the system do dangerous things.”
+* Enroll harvesting a file that has a *commented out* secret even with `--dangerous` disabled (it ignores comments so as to not be totally useless when it comes to harvesting config files). It is still the responsibility of the user to use `--sops` or appropriate at-rest encryption if in the slightest doubt about what might get harvested.
 
 Enroll is a tool for administrators, not a sandbox for hostile local users. It cannot make unsafe local trust decisions safe if the operator’s own execution environment is already attacker-controlled.
 

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}'"
 

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