Ensure markdown README gets sanitised

enroll/ansible.py

@@ -8,7 +8,13 @@ from dataclasses import dataclass
 from pathlib import Path
 from typing import Any, Dict, List, Optional, Set, Tuple
 
-from .cm import CMModule, markdown_list, snapshot_excluded_lines, snapshot_note_lines
+from .cm import (
+    CMModule,
+    markdown_list,
+    sanitize_markdown_text,
+    snapshot_excluded_lines,
+    snapshot_note_lines,
+)
 from .jinjaturtle import (
     jinjify_managed_files as _jinjify_managed_files,
     resolve_jinjaturtle_mode,
@@ -824,7 +830,7 @@ def _render_readme(
     fqdn: Optional[str] = None,
 ) -> str:
     host = state.get("host", {}) if isinstance(state.get("host"), dict) else {}
-    hostname = host.get("hostname") or "unknown"
+    hostname = sanitize_markdown_text(host.get("hostname") or "unknown") or "unknown"
     roles = state.get("roles", {}) if isinstance(state.get("roles"), dict) else {}
     excluded = snapshot_excluded_lines(roles)
     notes = snapshot_note_lines(roles)

enroll/cm.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import re
 import shlex
 from dataclasses import dataclass, field
 from pathlib import Path
@@ -713,8 +714,53 @@ def role_order_key(role: str) -> tuple[int, str]:
     return (priority.get(role, 50), role)
 
 
+# Control characters (excluding ordinary tab) that must never reach generated
+# documentation. A raw newline/carriage return in a harvested value would let it
+# break out of a Markdown list item or code span and inject new document
+# structure (a fake heading, a misleading link/command block); other C0/C1
+# control bytes can smuggle terminal escape sequences when the README is printed.
+_MARKDOWN_CONTROL_RE = re.compile(r"[\x00-\x08\x0b-\x1f\x7f-\x9f]")
+
+
+def sanitize_markdown_text(value: Any) -> str:
+    """Neutralise harvested text before it is spliced into generated Markdown.
+
+    Generated docs (the Ansible ``README.md``) embed harvested, attacker-
+    influenceable values such as the host name and captured file paths. These
+    are not executed by Ansible, but a value containing a newline, carriage
+    return, backtick, or control byte could otherwise break out of its
+    surrounding list item / code span and inject misleading Markdown structure
+    (a forged heading, a deceptive ``[link](...)``/command block) or a terminal
+    escape sequence when the file is viewed. This collapses any whitespace run
+    (including newlines and tabs) to a single space, drops other control bytes,
+    and replaces backticks with a similar-looking single quote so a value can
+    never escape an inline code span. It is deliberately lossy: the README is a
+    human-readable summary, and faithful representation of hostile bytes there
+    is not a goal.
+    """
+
+    text = str(value)
+    # Collapse any run of whitespace (newlines, CR, tabs, spaces) to one space so
+    # a harvested value stays on a single Markdown line / inside one code span.
+    text = re.sub(r"\s+", " ", text)
+    # Drop remaining control characters that survived the whitespace collapse.
+    text = _MARKDOWN_CONTROL_RE.sub("", text)
+    # A backtick would close an inline code span and let following characters be
+    # interpreted as Markdown; swap it for a visually-similar acute accent.
+    text = text.replace("`", "\u00b4")
+    return text.strip()
+
+
 def markdown_list(items: Iterable[Any], *, empty: str = "None.") -> str:
-    values = [str(item) for item in items if str(item)]
+    """Render already-composed Markdown list lines.
+
+    Callers that embed harvested values (``snapshot_note_lines``,
+    ``snapshot_excluded_lines``, ``path_reason_lines``) sanitise those values
+    with :func:`sanitize_markdown_text` before composing each line, so this
+    helper only joins lines it is given. It still drops empty entries.
+    """
+
+    values = [str(item) for item in items if str(item).strip()]
     return "\n".join(f"- {item}" for item in values) or f"- {empty}"
 
 
@@ -723,10 +769,10 @@ def path_reason_lines(
 ) -> List[str]:
     lines: List[str] = []
     for item in items or []:
-        path = str(item.get(source_key) or "")
+        path = sanitize_markdown_text(item.get(source_key) or "")
         if not path:
             continue
-        reason = str(item.get("reason") or "")
+        reason = sanitize_markdown_text(item.get("reason") or "")
         lines.append(f"{path} ({reason})" if reason else path)
     return lines
 
@@ -744,17 +790,20 @@ def iter_role_snapshots(roles: Mapping[str, Any]) -> Iterator[Mapping[str, Any]]
 def snapshot_note_lines(roles: Mapping[str, Any]) -> List[str]:
     notes: List[str] = []
     for snap in iter_role_snapshots(roles):
-        source = str(
+        source = sanitize_markdown_text(
             snap.get("role_name") or snap.get("unit") or snap.get("package") or "role"
         )
-        notes.extend(f"`{source}`: {note}" for note in snap.get("notes", []) or [])
+        notes.extend(
+            f"`{source}`: {sanitize_markdown_text(note)}"
+            for note in snap.get("notes", []) or []
+        )
     return notes
 
 
 def snapshot_excluded_lines(roles: Mapping[str, Any]) -> List[str]:
     excluded: List[str] = []
     for snap in iter_role_snapshots(roles):
-        source = str(
+        source = sanitize_markdown_text(
             snap.get("role_name") or snap.get("unit") or snap.get("package") or "role"
         )
         for line in path_reason_lines(snap.get("excluded", []) or []):

tests/test_markdown_sanitize.py

@@ -0,0 +1,87 @@
+"""Security regression: harvested values embedded in the generated Ansible
+``README.md`` must be neutralised so an attacker-influenceable host name, file
+path, or note cannot inject Markdown structure (a forged heading, deceptive
+link/command block) or a terminal escape sequence into the documentation.
+
+Harvested values are not executed by Ansible, but the README is the
+human-readable summary an operator reads before applying a manifest, so a value
+that breaks out of its surrounding list item / inline code span and forges
+document structure is misleading and must be prevented.
+"""
+
+from __future__ import annotations
+
+from enroll.cm import (
+    markdown_list,
+    sanitize_markdown_text,
+    snapshot_excluded_lines,
+    snapshot_note_lines,
+)
+
+
+def test_sanitize_collapses_newlines_and_strips_controls():
+    # A newline would let a value break out of its Markdown line; a backtick
+    # would close an inline code span; ESC/BEL are terminal escape vectors.
+    payload = "host`\n## Injected\n[x](http://evil)\n`y\x1b[31mRED\x07\ttab"
+    out = sanitize_markdown_text(payload)
+
+    assert "\n" not in out and "\r" not in out
+    assert "`" not in out  # backticks replaced so a code span cannot be closed
+    assert "\x1b" not in out and "\x07" not in out  # control bytes stripped
+    # The visible text is preserved on a single line (heading marker is now inert
+    # because it no longer starts a line).
+    assert "## Injected" in out
+    assert "http://evil" in out
+
+
+def test_sanitize_is_idempotent_and_handles_non_str():
+    once = sanitize_markdown_text("a\nb")
+    assert sanitize_markdown_text(once) == once
+    assert sanitize_markdown_text(1234) == "1234"
+    assert sanitize_markdown_text(None) == "None"
+
+
+def test_markdown_list_does_not_gain_extra_lines_from_payload():
+    # Even if a pre-composed line still contained a newline, markdown_list must
+    # not emit more bullets than it was given.
+    out = markdown_list(["clean line", "two\nlines should be one bullet"])
+    # Two input items -> at most two leading "- " bullets.
+    assert out.count("\n- ") <= 1
+
+
+def test_snapshot_note_lines_neutralises_injected_note():
+    roles = {
+        "etc_custom": {
+            "role_name": "etc_custom",
+            "notes": ["benign`\n## PWNED\n- fake bullet\x1b[5m"],
+        }
+    }
+    lines = snapshot_note_lines(roles)
+    assert len(lines) == 1
+    line = lines[0]
+    # The role name code span is intact; the payload is folded onto one line
+    # with backticks and control bytes removed.
+    assert line.startswith("`etc_custom`: ")
+    assert "\n" not in line
+    assert "\x1b" not in line
+    # Only the (structural) backticks around the role name remain -- the
+    # payload's backtick was neutralised.
+    assert line.count("`") == 2
+
+
+def test_snapshot_excluded_lines_neutralises_injected_path():
+    roles = {
+        "etc_custom": {
+            "role_name": "etc_custom",
+            "excluded": [
+                {"path": "/etc/`\n## fake-heading\nx", "reason": "denied_path"}
+            ],
+        }
+    }
+    lines = snapshot_excluded_lines(roles)
+    assert len(lines) == 1
+    line = lines[0]
+    assert "\n" not in line
+    assert line.startswith("`etc_custom`: ")
+    assert line.count("`") == 2  # only the structural role-name span
+    assert "(denied_path)" in line