Skip to content

fix: widen sanitizeAIOutput to cover HTML and @mentions (Bug #29)#49

Open
avrabe wants to merge 1 commit intomainfrom
fix/sanitize-ai-output
Open

fix: widen sanitizeAIOutput to cover HTML and @mentions (Bug #29)#49
avrabe wants to merge 1 commit intomainfrom
fix/sanitize-ai-output

Conversation

@avrabe
Copy link
Copy Markdown
Contributor

@avrabe avrabe commented May 1, 2026

Summary

  • Widens sanitizeAIOutput from a single workflow-command regex to a four-stage filter: workflow commands, HTML angle-bracket escaping, and @mention de-fanging via U+200B.
  • Closes Bug fix: rivet binary path resolution + drop fragile .issues guard #29 from docs/agent-fleet/bugs.md (flagged by wave-1 LLM + Security agents).
  • Source uses the escape (not the literal char) so no-irregular-whitespace passes and the de-fang is visible to future maintainers.

Why this matters

AI output is embedded directly in PR comments. The previous sanitiser only handled ::workflow-command:: injection; it left HTML tags and @org/team mentions intact, which GitHub will happily render as markup or convert into mass notifications.

Test plan

  • npm test → 837 pass (3 new tests added)
  • npm run lint → clean
  • Live-fire on a real PR after merge — confirm <script> renders as text and @octocat doesn't ping

Risk

Low. Defence-in-depth on free-text fields. The strict-JSON contract + computeVerdict already prevent the model from steering the verdict; this widens what's neutralised in summary/claim text.

🤖 Generated with Claude Code

Why: wave-1 LLM and Security agents flagged the previous one-pattern
sanitiser (workflow commands only) as too narrow. AI model output is
embedded directly in PR comments — an attacker controlling the diff
content could inject `<img onerror>`, `<script>`, or mass `@team`
mentions that GitHub renders as HTML/notifications.

What:
  - Escape `<` and `>` to HTML entities so injected tags render as text.
  - De-fang `@username` and `@org/team` patterns by inserting U+200B
    (zero-width space) after the @ sigil. The @ remains visible to
    humans but GitHub's mention parser no longer matches.
  - Source uses `​` escape (not the literal char) to satisfy
    `no-irregular-whitespace` and so the intent is visible to maintainers.
  - Email-like text (foo@example.com) is preserved — not a GitHub
    username pattern, so the @-alphanumeric regex doesn't match it.

Test plan:
  - 3 new tests in __tests__/integration/ai-review.test.js cover the
    three new behaviours (HTML escape, mention de-fang, email preserved).
  - npm test → 837 pass (was 834).
  - npm run lint → clean.

Risk: low — defence-in-depth on AI output. The strict-JSON contract +
verdict-from-findings already prevent the model from controlling the
verdict; this widens what gets neutralised in free-text fields too.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant