Skip to content

Fix incomplete multi-character HTML sanitization in AI tool result previews - #4260

Merged
wojtekn merged 1 commit into
trunkfrom
fix-incomplete-html-sanitization
Jul 23, 2026
Merged

Fix incomplete multi-character HTML sanitization in AI tool result previews#4260
wojtekn merged 1 commit into
trunkfrom
fix-incomplete-html-sanitization

Conversation

@wojtekn

@wojtekn wojtekn commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Related issues

How AI was used in this PR

Claude Code investigated the CodeQL alert, applied the fix, added a regression test, and ran lint / typecheck / the affected test suite. I reviewed the change.

Proposed Changes

When rendering previews of WordPress.com API tool results in the AI chat, Studio strips HTML tags out of rendered fields (post titles, etc.) so the summary shows plain text. That stripping used a single-pass regex replacement, which is unsafe: for a crafted value such as <scr<script>ipt>, removing the inner match leaves the outer fragments to recombine back into a live <script> tag. A malicious or compromised API response could therefore smuggle a tag through the sanitizer.

The tag-stripping now runs iteratively until the output stops changing, so no tag can reappear after removal. This is display-only sanitization for the tool-result summary line; behavior for normal (non-adversarial) content is unchanged.

Testing Instructions

  • npm test -- packages/common/ai/tests/tools.test.ts — passes, including a new test that feeds a recombining <scr<script>ipt>… payload through getToolResultPreview and asserts no <script (nor any <) survives in the summary.
  • npm run typecheck — passes.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
…eviews

A single-pass tag-stripping regex could let a crafted rendered field
recombine into a live tag after removal. Strip tags iteratively until
the output is stable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@wpmobilebot

Copy link
Copy Markdown
Collaborator

📊 Performance Test Results

Comparing f2ecde1 vs trunk

app-size

Metric trunk f2ecde1 Diff Change
App Size (Mac) 1355.58 MB 1355.58 MB +0.00 MB ⚪ 0.0%

site-editor

Metric trunk f2ecde1 Diff Change
load 1052 ms 1077 ms +25 ms ⚪ 0.0%

site-startup

Metric trunk f2ecde1 Diff Change
siteCreation 7028 ms 7015 ms 13 ms ⚪ 0.0%
siteStartup 2347 ms 2357 ms +10 ms ⚪ 0.0%

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

@wojtekn
wojtekn requested review from a team and youknowriad July 21, 2026 07:00
if ( value && typeof value === 'object' && 'rendered' in value ) {
const rendered = ( value as { rendered?: unknown } ).rendered;
return typeof rendered === 'string' ? rendered.replace( /<[^>]*>/g, '' ) : '';
return typeof rendered === 'string' ? stripHtmlTags( rendered ) : '';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's probably a @wordpress/ package that offers a utility like that no? html-entities maybe?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it makes sense to reuse the existing library. However, html-entities does completely different thing as it decodes HTML entities. There is also remove-invalid-html but it operates on DOM nodes. We could use a library like https://www.npmjs.com/package/sanitize-html, but it seems to be overkill in this case.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, thanks for looking.

@wojtekn
wojtekn merged commit 25c893b into trunk Jul 23, 2026
20 checks passed
@wojtekn
wojtekn deleted the fix-incomplete-html-sanitization branch July 23, 2026 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants