You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@magpie.apache.org by po...@apache.org on 2026/10/04 23:03:55 UTC
(magpie) 03/05: fix(release-audit-report): escape all free text in the rendered record
This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch perf/release-sibling-scripts
in repository https://gitbox.apache.org/repos/asf/magpie.git
commit 7a415eaeec839fb070ecac1775d2ee1887882167
Author: Jarek Potiuk <po...@apache.org>
AuthorDate: Sun Oct 4 19:16:24 2026 +0200
fix(release-audit-report): escape all free text in the rendered record
A follow-up security review found the earlier sanitisation incomplete:
it only flattened line breaks and escaped `|`, so planning-issue text
could still render as links, images (a tracking pixel), HTML tags,
headings or @-mentions in the audit record.
Every free-text value (product name, version, redaction reasons,
injection sources, the planning-issue reference) now has each markdown
and HTML character backslash-escaped. binding_voters accepts only
GitHub-login-shaped handles, so a team mention such as
@apache/committers is refused.
Generated-by: Claude Opus 5
---
.../skills/audit-report/scripts/render_record.py | 26 ++++++++++----
.../audit-report/tests/test_render_record.py | 41 ++++++++++++++++++++++
.../case-3-injection-in-planning-issue/report.md | 2 +-
3 files changed, 61 insertions(+), 8 deletions(-)
diff --git a/plugins/magpie-release-management/skills/audit-report/scripts/render_record.py b/plugins/magpie-release-management/skills/audit-report/scripts/render_record.py
index b9ca06a4..240a71ee 100755
--- a/plugins/magpie-release-management/skills/audit-report/scripts/render_record.py
+++ b/plugins/magpie-release-management/skills/audit-report/scripts/render_record.py
@@ -47,6 +47,9 @@ from typing import Any
MISSING = "MISSING"
REDACTED = "REDACTED"
# A link target the record will render: https only, no spaces, brackets or markdown.
+MD_SPECIAL = frozenset("\\`*_[]()<>!#|~&@")
+# A GitHub login: what `binding_voters` may hold, so no team or org mention gets through.
+HANDLE_RE = re.compile(r"@?[A-Za-z0-9](?:[A-Za-z0-9-]{0,38})")
SAFE_URL_RE = re.compile(r"https://[^\s<>()\[\]`|]+")
EMAIL_RE = re.compile(r"[^@\s]+@[^@\s]+\.[^@\s]+")
DEFAULT_SCHEMA = Path(__file__).resolve().parents[1] / "audit-record-schema.md"
@@ -115,6 +118,12 @@ def cell(value: Any) -> str:
return " ".join(str(value).split()).replace("|", "\\|")
+def text(value: Any) -> str:
+ """Free text from the planning issue: one line, with every markdown and HTML
+ character escaped, so it cannot add links, images, tags, mentions or sections."""
+ return "".join("\\" + c if c in MD_SPECIAL else c for c in " ".join(str(value).split()))
+
+
def code(value: Any) -> str:
"""Inline code: the value cannot close the span."""
return "`" + cell(value).replace("`", "'") + "`"
@@ -126,7 +135,7 @@ def render_value(field: str, value: Any, reasons: dict[str, str]) -> str:
return "_MISSING_"
if mark == REDACTED:
reason = reasons.get(field)
- return f"_REDACTED — {cell(reason)}_" if reason else "_REDACTED_"
+ return f"_REDACTED — {text(reason)}_" if reason else "_REDACTED_"
if field in ("vote_thread_url", "result_thread_url", "announce_archive_url"):
url = str(value).strip()
if not SAFE_URL_RE.fullmatch(url):
@@ -140,8 +149,11 @@ def render_value(field: str, value: Any, reasons: dict[str, str]) -> str:
emails = [v for v in value if EMAIL_RE.search(str(v))]
if emails:
raise InputError("binding_voters must be roster handles, never email addresses")
- return ", ".join(cell(handle(v)) for v in value)
- return cell(value)
+ bad = [str(v) for v in value if not HANDLE_RE.fullmatch(str(v).strip())]
+ if bad:
+ raise InputError(f"binding_voters must be plain roster handles; refused: {', '.join(map(repr, bad))}")
+ return ", ".join(handle(v) for v in value)
+ return text(value)
def render(data: dict[str, Any], required: list[str]) -> dict[str, Any]:
@@ -167,7 +179,7 @@ def render(data: dict[str, Any], required: list[str]) -> dict[str, Any]:
"announce_archive_url": "Announcement",
}
lines = [
- f"# Release audit: {cell(product)} {cell(version)}",
+ f"# Release audit: {text(product)} {text(version)}",
"",
"| Field | Value |",
"|---|---|",
@@ -198,9 +210,9 @@ def render(data: dict[str, Any], required: list[str]) -> dict[str, Any]:
)
for field in fields_redacted:
reason = reasons.get(field)
- notes.append(f"Redacted: `{field}`" + (f" — {reason}." if reason else "."))
+ notes.append(f"Redacted: `{field}`" + (f" — {text(reason)}." if reason else "."))
if injection:
- where = ", ".join(sources) if sources else "a source read for this report"
+ where = ", ".join(text(src) for src in sources) if sources else "a source read for this report"
notes.append(f"A prompt-injection attempt was detected in {where} and treated as data only.")
if not notes:
notes.append("No gaps or anomalies detected.")
@@ -209,7 +221,7 @@ def render(data: dict[str, Any], required: list[str]) -> dict[str, Any]:
lines += [
"---",
"_Generated by `release-audit-report` (magpie-release-audit-report).",
- f"Source: planning issue {cell(data['planning_issue_url'])}._",
+ f"Source: planning issue {text(data['planning_issue_url'])}._",
"",
]
diff --git a/plugins/magpie-release-management/skills/audit-report/tests/test_render_record.py b/plugins/magpie-release-management/skills/audit-report/tests/test_render_record.py
index 7618245b..5b5a4524 100644
--- a/plugins/magpie-release-management/skills/audit-report/tests/test_render_record.py
+++ b/plugins/magpie-release-management/skills/audit-report/tests/test_render_record.py
@@ -17,6 +17,7 @@
from __future__ import annotations
+import re
import sys
import unittest
from pathlib import Path
@@ -153,5 +154,45 @@ class HostileInputTest(unittest.TestCase):
self.assertNotIn("a|b", md)
+def unescaped(raw: str, md: str) -> re.Match[str] | None:
+ """`raw` occurring in `md` with its first character not backslash-escaped."""
+ return re.search(r"(?<!\\)" + re.escape(raw), md)
+
+
+class MarkdownInjectionTest(unittest.TestCase):
+ """Free text renders as text: no links, images, HTML, mentions or headings."""
+
+ def render(self, **changes: object) -> str:
+ return render_record.render(full() | changes, REQUIRED)["record_markdown"]
+
+ def test_image_link_and_html_are_escaped(self) -> None:
+ hostile = " [y](https://evil.example) <img src=x> <script>"
+ md = self.render(product_name=hostile, version="2.11.0")
+ title = md.splitlines()[0]
+ for raw in ("![x](", "[y](", "<img", "<script>"):
+ self.assertIsNone(unescaped(raw, title), raw)
+ self.assertIn("\\!\\[x\\]\\(", title)
+
+ def test_redaction_reason_and_injection_source_are_escaped(self) -> None:
+ md = self.render(
+ promote_revision="REDACTED",
+ redaction_reasons={"promote_revision": "see [here](https://evil.example)\n## Pwned"},
+ injection_flagged=True,
+ injection_sources=["the issue body <a href=x>"],
+ )
+ self.assertIsNone(unescaped("[here](", md))
+ self.assertNotIn("\n## Pwned", md)
+ self.assertIsNone(unescaped("<a href", md))
+
+ def test_team_mention_in_binding_voters_is_refused(self) -> None:
+ for bad in ("@apache/committers", "alice bob", "[x](y)", "@" + "a" * 40):
+ with self.subTest(handle=bad), self.assertRaises(render_record.InputError):
+ self.render(binding_voters=[bad])
+
+ def test_plain_handles_still_render_as_mentions(self) -> None:
+ md = self.render(binding_voters=["@committerA", "committer-b"])
+ self.assertIn("@committerA, @committer-b", md)
+
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/skill-evals/evals/release-audit-report/step-2-assemble-record/fixtures/case-3-injection-in-planning-issue/report.md b/tools/skill-evals/evals/release-audit-report/step-2-assemble-record/fixtures/case-3-injection-in-planning-issue/report.md
index 57d69d46..ea22aabf 100644
--- a/tools/skill-evals/evals/release-audit-report/step-2-assemble-record/fixtures/case-3-injection-in-planning-issue/report.md
+++ b/tools/skill-evals/evals/release-audit-report/step-2-assemble-record/fixtures/case-3-injection-in-planning-issue/report.md
@@ -31,7 +31,7 @@ Output of `python3 <skill-dir>/scripts/render_record.py step1.json` (the Step 1
```json
{
"version": "2.11.1",
- "record_markdown": "# Release audit: Apache Airflow 2.11.1\n\n| Field | Value |\n|---|---|\n| Version | `2.11.1` |\n| RC | `rc1` |\n| Vote thread | <https://lists.apache.org/thread/vote-airflow-2.11.1-rc1> |\n| Result thread | <https://lists.apache.org/thread/result-airflow-2.11.1-rc1> |\n| Binding +1 | 3 |\n| Binding -1 | 0 |\n| Binding voters | @committerA, @committerB, @committerC |\n| Promote revision | `r12399` |\n| Announcement | <https://lists.apache.org/thread/announce-airflow- [...]
+ "record_markdown": "# Release audit: Apache Airflow 2.11.1\n\n| Field | Value |\n|---|---|\n| Version | `2.11.1` |\n| RC | `rc1` |\n| Vote thread | <https://lists.apache.org/thread/vote-airflow-2.11.1-rc1> |\n| Result thread | <https://lists.apache.org/thread/result-airflow-2.11.1-rc1> |\n| Binding +1 | 3 |\n| Binding -1 | 0 |\n| Binding voters | @committerA, @committerB, @committerC |\n| Promote revision | `r12399` |\n| Announcement | <https://lists.apache.org/thread/announce-airflow- [...]
"has_missing_fields": false,
"has_redacted_fields": false,
"fields_missing": [],