Sanitise the 'validate' output error messages too just in case

enroll/validate.py

@@ -13,6 +13,7 @@ import jsonschema
 from .diff import BundleRef, _bundle_from_input
 from .manifest_safety import ArtifactSafetyError, safe_artifact_file
 from .state import load_state
+from .cm import sanitize_report_text
 
 
 @dataclass
@@ -25,30 +26,41 @@ class ValidationResult:
         return not self.errors
 
     def to_dict(self) -> Dict[str, Any]:
+        # Error/warning messages embed harvested, attacker-influenceable values
+        # (a managed_file src_rel, a role name, artifact paths). A raw newline in
+        # such a value could forge an extra message line in the text output, and
+        # a control byte could smuggle a terminal escape sequence when the result
+        # is printed or piped. Neutralise every message centrally here (and in
+        # to_text) so all current and future append sites are covered by one
+        # chokepoint, mirroring how the rest of Enroll sanitises harvested values
+        # before they reach a report/README/explain surface.
         return {
             "ok": self.ok,
-            "errors": list(self.errors),
+            "errors": [sanitize_report_text(e) for e in self.errors],
-            "warnings": list(self.warnings),
+            "warnings": [sanitize_report_text(w) for w in self.warnings],
         }
 
     def to_text(self) -> str:
-        lines: List[str] = []
+        errors = [sanitize_report_text(e) for e in self.errors]
-        if not self.errors and not self.warnings:
+        warnings = [sanitize_report_text(w) for w in self.warnings]
-            lines.append("OK: harvest bundle validated")
-        elif not self.errors and self.warnings:
-            lines.append(f"WARN: {len(self.warnings)} warning(s)")
-        else:
-            lines.append(f"ERROR: {len(self.errors)} validation error(s)")
 
-        if self.errors:
+        lines: List[str] = []
+        if not errors and not warnings:
+            lines.append("OK: harvest bundle validated")
+        elif not errors and warnings:
+            lines.append(f"WARN: {len(warnings)} warning(s)")
+        else:
+            lines.append(f"ERROR: {len(errors)} validation error(s)")
+
+        if errors:
             lines.append("")
             lines.append("Errors:")
-            for e in self.errors:
+            for e in errors:
                 lines.append(f"- {e}")
-        if self.warnings:
+        if warnings:
             lines.append("")
             lines.append("Warnings:")
-            for w in self.warnings:
+            for w in warnings:
                 lines.append(f"- {w}")
         return "\n".join(lines) + "\n"
 

tests/test_validate_sanitize.py

@@ -0,0 +1,62 @@
+"""Security regression (audit finding): ValidationResult.to_text()/to_dict()
+render error and warning messages that embed harvested, attacker-influenceable
+values (a managed_file src_rel, a role name, artifact paths). A src_rel with an
+embedded newline passes the schema (pattern ``^[^/].*``) and could otherwise
+forge an extra message line, and a control byte could smuggle a terminal escape
+sequence when the validation result is printed or piped.
+
+The pass/fail decision (``ok``) is unaffected by sanitisation; only the rendered
+output is neutralised.
+"""
+
+from __future__ import annotations
+
+import json
+
+from enroll.validate import ValidationResult
+
+
+def _malicious_result():
+    return ValidationResult(
+        errors=[
+            "missing artifact for role etc_custom: "
+            "artifacts/etc_custom/foo\n## FORGED ERROR\x1b[31m\nOK: fake pass",
+        ],
+        warnings=["unreferenced artifact: bar\x07\nFORGED WARNING LINE"],
+    )
+
+
+def test_to_text_neutralises_injection():
+    txt = _malicious_result().to_text()
+
+    for line in txt.splitlines():
+        stripped = line.lstrip("- ").lstrip()
+        assert not stripped.startswith("## FORGED ERROR")
+        assert not stripped.startswith("FORGED WARNING LINE")
+        # A forged "OK:" line must not be injectable (there ARE errors here).
+        assert stripped != "OK: fake pass"
+    assert "\x1b" not in txt
+    assert "\x07" not in txt
+
+
+def test_to_dict_strips_control_bytes():
+    d = _malicious_result().to_dict()
+    blob = json.dumps(d)
+    assert "\x1b" not in blob
+    assert "\x07" not in blob
+    # Sanitisation must not flip the verdict.
+    assert d["ok"] is False
+
+
+def test_benign_messages_are_preserved():
+    vr = ValidationResult(errors=["missing state.json at /tmp/x"], warnings=[])
+    txt = vr.to_text()
+    assert "missing state.json at /tmp/x" in txt
+    assert vr.to_dict()["errors"] == ["missing state.json at /tmp/x"]
+    assert vr.ok is False
+
+
+def test_ok_result_unchanged():
+    vr = ValidationResult(errors=[], warnings=[])
+    assert "OK: harvest bundle validated" in vr.to_text()
+    assert vr.to_dict()["ok"] is True