Skip to content

fix(ai-review): neutralize public markdown output - #902

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-ai-review-markdown-injection-vulnerability
Jun 19, 2026
Merged

fix(ai-review): neutralize public markdown output#902
JSONbored merged 1 commit into
mainfrom
codex/fix-ai-review-markdown-injection-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The AI review path published LLM-derived text into trusted bot comments while only rejecting forbidden product/private terms, leaving markdown, mentions, links, and control characters intact and enabling public-comment injection/spoofing.
  • Neutralize AI-generated output before it reaches the public panel so attacker-controlled prompt injection cannot cause preserved markdown (links, images, mentions, lists) to appear in a trusted automation surface.

Description

  • Add a neutralizePublicMarkdown helper that flattens control characters/extra whitespace, defangs mentions and :// sequences, and escapes common Markdown metacharacters.
  • Apply the neutralizer inside toPublicSafe after the existing sanitizePublicComment check so only allowed content is published and any remaining Markdown/mentions/URLs are neutralized.
  • Update the PR panel text in buildPublicPrIntelligenceComment to document that notes are markdown-neutralized upstream while keeping the final angle-bracket escaping guard.
  • Add/adjust unit coverage in test/unit/ai-review.test.ts to assert neutralization of links, image syntax, mentions, and newline/list injection.

Testing

  • Ran npm test -- --run test/unit/ai-review.test.ts and all unit tests passed (31/31).
  • Ran npm run typecheck (tsc --noEmit) and type checking succeeded.

Codex Task

@ghost

ghost commented Jun 18, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #902 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 18, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@ghost

ghost commented Jun 18, 2026

Copy link
Copy Markdown

gittensory · advisory review

Reviewed 3 changed file(s) — two independent AI reviewers.

Changed files: src/services/ai-review.ts, src/signals/engine.ts, test/unit/ai-review.test.ts

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR introduces a markdown neutralization step for public‑safe text, enhancing security by stripping control characters, breaking mentions/links, and escaping markdown syntax, and adds a corresponding unit test.

Suggestions

  • Document the purpose of neutralizePublicMarkdown in a comment near its definition for future maintainers.
  • Consider adding a test case that verifies control characters and excessive whitespace are correctly normalized.
  • Review any UI components that render toPublicSafe output to ensure the escaped markdown still displays as intended.

Worth double-checking

  • Escaping all markdown characters may affect legitimate formatting in public notes; verify that this is the desired behavior across the UI.
  • Zero‑width spaces inserted into URLs/mentions could impact downstream tooling that parses such strings.

Reviewer B · mistral-small-3.1-24b-instruct — recommends ✅ merge
This PR adds a markdown neutralization function to ensure public-facing text is safe and neutralized. The changes are well-targeted and include corresponding tests.

Suggestions

  • Consider adding comments to explain the purpose of each regex replacement in the neutralizePublicMarkdown function for better maintainability.
  • Ensure that the neutralizePublicMarkdown function handles all edge cases, such as nested or escaped special characters.

Worth double-checking

  • Verify that the neutralizePublicMarkdown function correctly handles all potential markdown injection vectors.
  • Ensure that the new function does not introduce performance bottlenecks, especially for large text inputs.

@codecov

codecov Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.71%. Comparing base (5d9d73a) to head (d9dcc3f).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #902   +/-   ##
=======================================
  Coverage   96.71%   96.71%           
=======================================
  Files         108      108           
  Lines       14641    14642    +1     
  Branches     5298     5298           
=======================================
+ Hits        14160    14161    +1     
  Misses        102      102           
  Partials      379      379           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 33d5779 into main Jun 19, 2026
20 checks passed
@JSONbored
JSONbored deleted the codex/fix-ai-review-markdown-injection-vulnerability branch June 19, 2026 05:50
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