Fix XSS in FormHandler.js showMessages method

[SteveKinzey]

Summary

• Adds escapeHtml() method to safely escape HTML metacharacters
• Escapes heading, message (string), and message (object iteration) values before inserting into DOM in showMessages()
• Prevents potential cross-site scripting via form notice content

Details

src/public/FormHandler.js inserts heading and message values directly into HTML without escaping. If these contain HTML metacharacters (e.g. <script> tags), they are interpreted as HTML — a DOM-based XSS vulnerability.

Identified by GitHub CodeQL (rule: DOM text reinterpreted as HTML, severity: high).

Fixes #35

🤖 Generated with Claude Code

[greptile-apps]

PR author is not in the allowed authors list.

[coderabbitai]

📝 Walkthrough

Walkthrough

A security fix was implemented in FormHandler.js to prevent cross-site scripting (XSS) vulnerabilities. A new escapeHtml() helper method was added to the PayFormHandler class that escapes HTML metacharacters by creating a temporary DOM element and extracting its innerHTML. The showMessages method was updated to apply this escaping to heading text, message strings, and list items before inserting them into the DOM, replacing direct string concatenation with escaped content.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing an XSS vulnerability in the FormHandler.js showMessages method.
Description check	✅ Passed	The description is well-related to the changeset, detailing the XSS vulnerability, the escapeHtml method added, where it's applied, and the CodeQL reference.
Linked Issues check	✅ Passed	The code changes fully address issue #35: escapeHtml() method is added and applied to heading and message values in showMessages() to prevent XSS via DOM.
Out of Scope Changes check	✅ Passed	All changes are scoped to the stated objective of fixing the XSS vulnerability in FormHandler.js showMessages method; no out-of-scope modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/public/FormHandler.js (1)

170-193: ⚠️ Potential issue | 🟡 Minor

Potential XSS in recordCouponMessage – consider addressing in a follow-up.

While this PR correctly fixes XSS in showMessages, the recordCouponMessage method at line 193 appends message directly to the DOM without escaping. The message parameter can contain user-controlled content like coupon.code (line 152).

Consider applying the same escapeHtml treatment here for comprehensive XSS protection.

🛡️ Suggested fix for recordCouponMessage

-        $responseDiv.append(errorHtml.append(cross, message));
+        $responseDiv.append(errorHtml.append(cross, this.escapeHtml(message)));

Note: Line 152's template literal would also need escaping:

-                let message = `${coupon.code} <span>-${couponAmount}</span>`;
+                let message = `${this.escapeHtml(coupon.code)} <span>-${couponAmount}</span>`;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/public/FormHandler.js` around lines 170 - 193, The recordCouponMessage
function appends unescaped message content into the DOM, allowing XSS; like
showMessages, call the existing escapeHtml utility on message before inserting
(and on any coupon.code template usage in the recordCouponMessage context) and
append the escaped string instead of raw message; ensure the click handler still
references this.appliedCoupons and updates .__wpf_all_applied_coupons and
calculatePayments unchanged.

🧹 Nitpick comments (1)

src/public/FormHandler.js (1)

739-743: Effective XSS mitigation using the DOM text-node pattern.

This is a well-established technique for escaping HTML metacharacters. One minor robustness improvement: guard against non-string inputs to avoid unexpected coercion (e.g., null → "null").

🛡️ Optional: Add defensive type coercion

 escapeHtml(str) {
+    if (str == null) return '';
     const div = document.createElement('div');
-    div.appendChild(document.createTextNode(str));
+    div.appendChild(document.createTextNode(String(str)));
     return div.innerHTML;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/public/FormHandler.js` around lines 739 - 743, The escapeHtml function
should guard against non-string inputs to avoid unintended coercion; update
escapeHtml to first handle null/undefined by returning an empty string, then
ensure the value passed to document.createTextNode is an explicit string (e.g.,
coerce other primitives using String(x)), and keep using the DOM text-node
pattern (escapeHtml) with document.createTextNode and div.innerHTML to produce
the escaped result.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/public/FormHandler.js`:
- Around line 170-193: The recordCouponMessage function appends unescaped
message content into the DOM, allowing XSS; like showMessages, call the existing
escapeHtml utility on message before inserting (and on any coupon.code template
usage in the recordCouponMessage context) and append the escaped string instead
of raw message; ensure the click handler still references this.appliedCoupons
and updates .__wpf_all_applied_coupons and calculatePayments unchanged.

---

Nitpick comments:
In `@src/public/FormHandler.js`:
- Around line 739-743: The escapeHtml function should guard against non-string
inputs to avoid unintended coercion; update escapeHtml to first handle
null/undefined by returning an empty string, then ensure the value passed to
document.createTextNode is an explicit string (e.g., coerce other primitives
using String(x)), and keep using the DOM text-node pattern (escapeHtml) with
document.createTextNode and div.innerHTML to produce the escaped result.