Skip to content

fix: wrap rich-text fields in <p> so TM renders special characters correctly - #356

Open
SavioBS629 wants to merge 4 commits into
browserstack:mainfrom
SavioBS629:fix/pmaa-185-richtext-gt-encoding
Open

fix: wrap rich-text fields in <p> so TM renders special characters correctly#356
SavioBS629 wants to merge 4 commits into
browserstack:mainfrom
SavioBS629:fix/pmaa-185-richtext-gt-encoding

Conversation

@SavioBS629

Copy link
Copy Markdown
Collaborator

Summary

Fixes PMAA-185 (FD 1709260, originally TM-26634): test cases created/updated via MCP displayed > as the literal text &gt; in the TM UI.

Root cause

TM's v2 API entity-encodes <, > and & in step/result, preconditions and description on write, and the TM UI renders those fields as rich text only when the value is HTML-wrapped — bare text is displayed literally. The MCP server sent bare text, so a plain > surfaced to users as &gt;. Confirmed with the TM team: external API content must be wrapped in <p> tags (their supported contract).

Fix

New rich-text.ts helper wraps a value in <p>…</p> when it is not already HTML and contains an encodable character (<, >, &). Applied to test_case_steps (step + result), preconditions and description in createTestCase (v1 and v2 paths) and updateTestCase. Payloads without encodable characters, and values that are already HTML, are sent unchanged.

Verification

  • Empirically verified against production TM (sandbox PR-148108):
    • bare Settings > Users → stored Settings &gt; Users → UI shows literal &gt; (bug, repro TC-5315740 / TC-5352932 via plain curl, no MCP involved)
    • <p>Settings > Users</p> → stored <p>Settings &gt; Users</p> → UI renders Settings > Users correctly (TC-5358753, confirmed by TM team screenshot)
    • quotes are not entity-encoded, so they don't trigger wrapping
  • New unit tests for the helper; full suite passes (236 tests), tsc and eslint clean.

🤖 Generated with Claude Code

…al chars

TM's v2 API entity-encodes <, > and & in step/result, preconditions and
description on write, and the TM UI renders those fields as rich text only
when the value is HTML-wrapped — bare text is displayed literally, so a
plain ">" surfaced to users as "&gt;" (PMAA-185, FD 1709260, TM-26634).

Per the TM team, external API content must be wrapped in <p> tags. Wrap
those fields on createTestCase and updateTestCase when the value is not
already HTML and contains an encodable character; all other payloads are
sent unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SavioBS629 SavioBS629 changed the title fix(testmanagement): wrap rich-text fields in <p> so TM renders special characters correctly fix: wrap rich-text fields in <p> so TM renders special characters correctly Jul 29, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@SavioBS629 SavioBS629 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Claude Code Review (automated) — 3 inline finding(s). Full report in the PR comment below. Verdict: Failed - see PR comment.

if (LOOKS_LIKE_HTML.test(text) || !HAS_ENCODABLE_CHARS.test(text)) {
return text;
}
return `<p>${text}</p>`;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[High] Wrapping bare text without escaping lets TM strip tag-like tokens (data loss)

Once wrapped in <p>, TM's sanitizer treats the value as HTML and strips unknown tag-like tokens entirely: Press <Enter> to submit the form is stored as Press to submit the form; List<String>, <username>, <input> vanish the same way (verified live: TC-5368849 in sandbox PR-148108). Pre-PR these inputs were entity-encoded and lossless.

Suggestion: escape the bare text before wrapping — `<p>${text.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")}</p>`. Verified live that this round-trips byte-identically and renders correctly (TC-5368865).

Reviewer: review (built-in fallback)

@@ -0,0 +1,20 @@
// TM UI renders these fields as rich text only when HTML-wrapped;
const LOOKS_LIKE_HTML = /^\s*<[a-z][^>]*>/i;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Medium] LOOKS_LIKE_HTML misclassifies leading tag-like tokens as HTML

"<Enter> key submits the form" matches /^\s*<[a-z][^>]*>/i, so it is passed through bare and TM's sanitizer strips <Enter> — same data loss via a different path.

Suggestion: only treat input as HTML when it starts with a known rich-text tag (p|br|ul|ol|li|b|i|em|strong|u|a|span|div|table); otherwise escape-and-wrap.

Reviewer: review (built-in fallback)

@@ -0,0 +1,20 @@
// TM UI renders these fields as rich text only when HTML-wrapped;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[Low] Truncated comment with trailing whitespace

The comment ends mid-sentence ("…only when HTML-wrapped; ").

Suggestion: complete the sentence: bare text displays TM's entity-encoding literally in the UI, hence the wrap.

Reviewer: review (built-in fallback)

@SavioBS629

Copy link
Copy Markdown
Collaborator Author

Claude Code PR Review

PR: #356Head: cc3a498Reviewers: review (built-in fallback; no in-scope project reviewers discovered)

Summary

Wraps TM rich-text fields (step/result, preconditions, description) in <p> tags on createTestCase/updateTestCase so the TM UI renders >, <, & correctly instead of showing entity-encoded text literally (PMAA-185 / FD 1709260).

Review Table

Priority Category Check Status Notes
High Security No hardcoded secrets or credentials Pass Pure string helper; no secrets
High Security Authentication/authorization checks present N/A No auth surface touched
High Security Input validation and sanitization Pass Inputs zod-validated upstream; TM sanitizes server-side
High Security No IDOR — resource ownership validated N/A
High Security No SQL injection (parameterized queries) N/A
High Correctness Logic is correct, handles edge cases Fail Tag-like tokens (<Enter>, List<String>) are stripped by TM after wrapping — verified live (TC-5368849): content silently lost
High Correctness Error handling is explicit, no swallowed exceptions Pass Helper is pure; existing error paths unchanged
High Correctness No race conditions or concurrency issues N/A
Medium Testing New code has corresponding tests Pass 9 focused unit tests for the helper
Medium Testing Error paths and edge cases tested Fail Missing the tag-like-token case (Press <Enter>), which is exactly the case that breaks
Medium Testing Existing tests still pass (no regressions) Pass Full suite green (236 tests)
Medium Performance No N+1 queries or unbounded data fetching N/A
Medium Performance Long-running tasks use background jobs N/A
Medium Quality Follows existing codebase patterns Pass Sibling util next to its two consumers
Medium Quality Changes are focused (single concern) Pass One concern, 4 files
Low Quality Meaningful names, no dead code Pass
Low Quality Comments explain why, not what Fail rich-text.ts:1 comment is truncated mid-sentence ("…HTML-wrapped; ") with trailing whitespace
Low Quality No unnecessary dependencies added Pass Zero new deps

Findings

  • File: src/tools/testmanagement-utils/rich-text.ts:9

  • Severity: High

  • Reviewer: review (built-in fallback)

  • Issue: Bare text is wrapped in <p> without entity-escaping its content first. Once wrapped, TM's sanitizer treats the value as HTML and strips tag-like tokens entirely: Press <Enter> to submit the form is stored as Press to submit the form; List<String>, <username>, <input> vanish the same way (verified live: TC-5368849 in PR-148108). Pre-PR these inputs were entity-encoded and displayed (ugly but lossless); post-PR the content is silently destroyed.

  • Suggestion: Escape &, <, > in the bare text before wrapping: `<p>${text.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;")}</p>`. Verified live that this round-trips byte-identically and renders correctly (TC-5368865).

  • File: src/tools/testmanagement-utils/rich-text.ts:2

  • Severity: Medium

  • Reviewer: review (built-in fallback)

  • Issue: LOOKS_LIKE_HTML = /^\s*<[a-z][^>]*>/i classifies any leading tag-like token as HTML, so "<Enter> key submits the form" is passed through bare and TM's sanitizer strips <Enter> — same data loss via a different path. The heuristic can't distinguish real HTML from literal angle-bracket tokens.

  • Suggestion: Only treat input as pre-formatted HTML when it starts with a known rich-text tag TM supports (e.g. p|br|ul|ol|li|b|i|em|strong|u|a|span|div|table); otherwise escape-and-wrap. Combined with the fix above, every non-HTML input round-trips losslessly.

  • File: src/tools/testmanagement-utils/rich-text.ts:1

  • Severity: Low

  • Reviewer: review (built-in fallback)

  • Issue: Comment is truncated mid-sentence ("…only when HTML-wrapped; ") and carries trailing whitespace.

  • Suggestion: Complete the sentence: bare text displays TM's entity-encoding literally in the UI, hence the wrap.


Verdict: FAIL — one High data-loss finding; fix is a two-line escape before wrapping, verified against the live TM API.

…own editor

TM's sanitizer (Sanitize.fragment with RTE_ALLOWED_CONTENT) strips unknown
tag-like tokens inside HTML values, so wrapping unescaped text lost content
like "<Enter>" or "List<String>". TM's own Lexical editor and import paths
escape text then wrap in <p>; do the same, and pass through only values that
start with a tag from TM's allowed-elements list.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@SavioBS629

Copy link
Copy Markdown
Collaborator Author

Claude Code PR Review

Continues the previous review — changes since cc3a498 (delta).

PR: #356Head: aca16d4Reviewers: review (built-in fallback; no in-scope project reviewers discovered)

Summary

Delta adds entity-escaping before the <p> wrap and replaces the generic HTML heuristic with TM's actual allowed-elements list (RTE_ALLOWED_CONTENT, teststack constants.rb), mirroring what TM's own Lexical editor and import paths do on save. Tests updated to the escaped expectations, including the tag-like-token cases.

Review Table

Priority Category Check Status Notes
High Security No hardcoded secrets or credentials Pass
High Security Authentication/authorization checks present N/A
High Security Input validation and sanitization Pass Escaping now matches TM's server-side sanitizer expectations
High Security No IDOR — resource ownership validated N/A
High Security No SQL injection (parameterized queries) N/A
High Correctness Logic is correct, handles edge cases Pass Tag-like tokens round-trip losslessly — verified live (TC-5369884): Press <Enter> and List<String> preserved byte-for-byte
High Correctness Error handling is explicit, no swallowed exceptions Pass Helper remains pure
High Correctness No race conditions or concurrency issues N/A
Medium Testing New code has corresponding tests Pass 10 unit tests incl. tag-like tokens, allowed-tag pass-through, unicode, multiline
Medium Testing Error paths and edge cases tested Pass The previously missing <Enter>/List<String> cases are now covered
Medium Testing Existing tests still pass (no regressions) Pass Suite green
Medium Performance No N+1 queries or unbounded data fetching N/A
Medium Performance Long-running tasks use background jobs N/A
Medium Quality Follows existing codebase patterns Pass
Medium Quality Changes are focused (single concern) Pass
Low Quality Meaningful names, no dead code Pass TM_ALLOWED_TAG names the source of truth
Low Quality Comments explain why, not what Pass Comment now complete; explains the strip/encode behavior being compensated for
Low Quality No unnecessary dependencies added Pass

Findings

  • src/tools/testmanagement-utils/rich-text.ts:9 High — Wrapping bare text without escaping lets TM strip tag-like tokens (data loss) ResolvedescapeHtml (&,<,>) now runs before wrapping; verified live that Press <Enter> to submit the form round-trips byte-identically (TC-5369884).
  • src/tools/testmanagement-utils/rich-text.ts:2 Medium — LOOKS_LIKE_HTML misclassifies leading tag-like tokens as HTML Resolved — replaced with TM_ALLOWED_TAG, the exact element list from TM's RTE_ALLOWED_CONTENT; <Enter> key… is now escaped-and-wrapped, only genuine TM-supported HTML passes through.
  • src/tools/testmanagement-utils/rich-text.ts:1 Low — Truncated comment with trailing whitespace Resolved — comment completed.

Note (informational, not a finding): bare text is now treated strictly as literal text — a client that pre-encodes &gt; in plain text will see the literal string &gt; rendered, which matches how TM's own editor treats typed text (WYSIWYG). Clients that want rich-text semantics send HTML starting with a TM-allowed tag.


Verdict: PASS — all three prior findings resolved and verified against the live TM API; no new findings in the delta.

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.

2 participants