Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@
**Vulnerability:** Workflow CI Security Bypass / Markdown Injection
**Learning:** The GitHub Actions workflow `opencode-review.yml` attempted to optimize performance by doing a fast-path bash string extraction. If this succeeded, it skipped the Python JSON normalizer (`opencode_review_normalize_output.py`). This is a security flaw because the bash script does not escape `<, >, &` characters, allowing attackers to inject `-->` directly in JSON strings to break out of HTML comment sections.
**Prevention:** Removed the fast-path check entirely. We must always enforce JSON normalization via `opencode_review_normalize_output.py` because it correctly parses the JSON payload and safely escapes all characters as `\u003c`, `\u003e` and `\u0026`.

## 2025-02-23 - Prevent JSON Markdown Injection in Python Normalizer
**Vulnerability:** Markdown Injection / HTML Comment Breakout
**Learning:** `json.dumps()` in python does not automatically escape HTML sensitive characters `<, >, &` into unicode forms. When outputting JSON to an HTML comment format like `<!-- opencode-review-control-v1 { ... } -->`, it is susceptible to breakout.
**Prevention:** Ensured XSS prevention by chaining `.replace("<", "\\u003c").replace(">", "\\u003e").replace("&", "\\u0026")` to the dumped JSON output in `scripts/ci/opencode_review_normalize_output.py`.
5 changes: 5 additions & 0 deletions scripts/ci/opencode_review_normalize_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,11 @@ def main(argv: list[str]) -> int:
continue

normalized_json = json.dumps(control, separators=(",", ":"), ensure_ascii=False)
normalized_json = (
normalized_json.replace("<", "\\u003c")
.replace(">", "\\u003e")
.replace("&", "\\u0026")
)
output_file.write_text(
"\n".join(
[
Expand Down
23 changes: 23 additions & 0 deletions tests/test_opencode_review_normalize_output.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,3 +368,26 @@ def test_main_normalizes_valid_output_and_reports_failures(tmp_path, capsys):
approval = tmp_path / "approval.json"
approval.write_text(json.dumps(control()), encoding="utf-8")
assert norm.main(["prog", "--check-structural-approval", str(approval)]) == 0

def test_main_escapes_html_chars(tmp_path):
control_json = {
"head_sha": "head",
"run_id": "run",
"run_attempt": "attempt",
"result": "APPROVE",
"reason": "Security review\nChanged files\npath.py",
"summary": FULL_SUMMARY + "\n<script>alert(1)</script> & -->",
"findings": [],
}
input_text = f"```json\n{json.dumps(control_json)}\n```"

output_file = tmp_path / "output.md"
output_file.write_text(input_text, encoding="utf-8")
argv = ["script", "head", "run", "attempt", str(output_file)]

result = norm.main(argv)
assert result == 0
output_content = output_file.read_text(encoding="utf-8")
assert "<script>" not in output_content
assert "\\u003cscript\\u003e" in output_content
assert "\\u0026 --\\u003e" in output_content
Loading