Skip to content

fix(plugin-email): harden htmlToText against double-escaping & incomplete tag stripping#1521

Merged
hotlong merged 2 commits into
mainfrom
claude/peaceful-mayer-FCx6b
Jun 2, 2026
Merged

fix(plugin-email): harden htmlToText against double-escaping & incomplete tag stripping#1521
hotlong merged 2 commits into
mainfrom
claude/peaceful-mayer-FCx6b

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Summary

Fixes two pre-existing CodeQL high-severity alerts (created 2026-05-19, on main) in packages/plugins/plugin-email/src/template-engine.ts, in the htmlToText() function. These surfaced as "new" on every PR's CodeQL check and were leaving PR CI status UNSTABLE.

This is independent pre-existing security debt in the email plugin — unrelated to ADR-0032 (the expression layer).

Alerts addressed

  1. js/double-escaping — the old chain of sequential single-entity replaces (.replace(/&amp;/g,'&').replace(/&lt;/g,'<')…) was order-dependent and could double-unescape (e.g. &amp;lt;&lt;<). Replaced with a single-pass alternation regex that decodes each entity once and resumes scanning after the match, so &amp;lt; decodes to the literal text &lt; and stops.

  2. js/incomplete-multi-character-sanitization — the old single .replace(/<[^>]+>/g, '') pass could leave a live tag behind on crafted/overlapping input (e.g. <scr<script>ipt>). Now tag stripping loops until the string is stable.

Order also matters: tags are stripped before entities are decoded, so decoding can never re-introduce a live tag.

Tests

Added adversarial unit tests asserting no </tag survives and no double-unescape occurs:

  • &amp;lt;script&amp;gt; → decodes once to &lt;script&gt; (no real <)
  • <scr<script>ipt>alert(1)</script> → no surviving tag
  • tags that re-form after a single pass
  • deeply nested entities

Verification

pnpm --filter @objectstack/plugin-email build   # ✅
pnpm --filter @objectstack/plugin-email test    # ✅ 49 passed

CodeQL alerts expected to clear on this PR.

https://claude.ai/code/session_014e59DF7CNAYxj89on1fvCp


Generated by Claude Code

…mplete tag stripping

Resolve two CodeQL high-severity alerts in htmlToText:

- js/double-escaping: replace the order-dependent chain of single-entity
  .replace() calls with one single-pass alternation regex, so sequences
  like &amp;lt; decode once to the literal &lt; instead of cascading to <.
- js/incomplete-multi-character-sanitization: loop tag stripping until the
  string is stable, so crafted/overlapping input (e.g. <scr<script>ipt>)
  cannot leave a live tag behind.

Tags are now stripped before entities are decoded so decoding cannot
re-introduce a live tag. Adds adversarial unit tests covering nested
entities and overlapping tags.
@vercel

vercel Bot commented Jun 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jun 2, 2026 5:27am

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tooling size/m and removed size/s labels Jun 2, 2026
@hotlong hotlong marked this pull request as ready for review June 2, 2026 05:28
@hotlong hotlong merged commit 860723b into main Jun 2, 2026
12 checks passed
xuyushun441-sys added a commit that referenced this pull request Jun 6, 2026
* chore: bump objectui to 514f426600bc

fix(master-detail): reliable submit + durable live e2e harness (#1521)

objectui@514f426600bcc6284b67909dd7ced7e1bcb1d762

* feat(objectql): compute roll-up summary fields server-side

The `summary` field type was declared but never computed. ObjectQL now
recomputes a parent's roll-up (count/sum/min/max/avg over a child field)
whenever a child is inserted/updated/deleted, in the caller's execution
context so it commits atomically inside an open transaction (e.g. the
cross-object /api/v1/batch).

- spec: summaryOperations.relationshipField (optional; FK auto-detected
  from the child's lookup/master_detail reference otherwise).
- objectql: lazy child→summary index; recompute after afterInsert/
  afterUpdate/afterDelete (prior FK captured on update/delete so a
  re-parented/removed child updates the old parent too).
- showcase: project.task_count + total_estimate demonstrate it.

Tests: summary-rollup.test.ts (4 — sum/count, update, delete→0, parent
isolation); full objectql suite 530 pass, spec 6646 pass. Verified live
against the SQL driver via /api/v1/batch (objectui e2e/live/summary-rollup).

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

---------

Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
xuyushun441-sys pushed a commit that referenced this pull request Jun 6, 2026
fix(master-detail): reliable submit + durable live e2e harness (#1521)

objectui@514f426600bcc6284b67909dd7ced7e1bcb1d762
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants