diff --git a/enroll/validate.py b/enroll/validate.py index f500b55..97c2afe 100644 --- a/enroll/validate.py +++ b/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), - "warnings": list(self.warnings), + "errors": [sanitize_report_text(e) for e in self.errors], + "warnings": [sanitize_report_text(w) for w in self.warnings], } def to_text(self) -> str: - lines: List[str] = [] - if not self.errors and not 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)") + errors = [sanitize_report_text(e) for e in self.errors] + warnings = [sanitize_report_text(w) for w in self.warnings] - 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" diff --git a/tests/test_validate_sanitize.py b/tests/test_validate_sanitize.py new file mode 100644 index 0000000..e9353c9 --- /dev/null +++ b/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