New remember me features

[plpmd]

Summary

Adds three new capabilities to the Remember Me component (remember-me.ts) for more flexible opt-in and clear-autofill rendering. Also resolves all blocking and minor issues raised in the code review.

Changes

• rightSide positioning: Adds a rightSide value to fieldOptInSelectorTargetLocation and fieldClearSelectorTargetLocation. The target element is moved into a new flex <div> wrapper (.rememberme-right-side-wrapper) and the element is appended alongside it on the right. The target's original margin-top and margin-bottom are read before the DOM move (once inside a flex container, margin collapsing no longer applies and computed values can change) and transferred to the wrapper; the target's own vertical margins are then zeroed to prevent double-spacing inside the flex row.
• Responsive rightSide: Enables flex-wrap on the wrapper so the appended element drops below the target when both don't fit on one line (e.g. mobile). Alignment is computed via requestAnimationFrame (deferred so layout is settled before offsetTop is read) and recomputed on every wrapper resize via ResizeObserver (with a feature-detect fallback for older browsers): margin-left is set to the target's padding-left when wrapped below, and reset to 0 when side-by-side.
• $username token and clickable {...} segment: The clear-autofill label replaces $username with the remembered supporter.firstName. When the label contains a {...} segment, the first such segment renders as the clickable clear link while surrounding text stays plain; any additional {...} pairs remain as literal text (braces included). Falls back to the legacy fully-clickable element when no braces are present.

Security fixes (from code review)

• Stored XSS — supporter.firstName (user-controlled, decoded back to plaintext) was interpolated into innerHTML unescaped. Added escapeHtml() and applied it before any innerHTML assignment. Verified fix covers the URL-prefill attack vector described in the review.
• String.replace pattern injection — first names containing $&, $', or $` were interpreted as replacement patterns. Switched all $username substitutions to the function form of .replace() so values are always inserted literally.

UX / correctness fixes (from code review)

• label-tooltip on outer <span> — in hasInnerLink mode the outer <span> now carries a neutral clear-autofill-data-wrapper class; label-tooltip and cursor: pointer are applied only to the inner <a> so surrounding plain text does not look clickable.
• Tooltip token leak — added getClearLabelPlainText() which resolves $username and strips {...} markers; the opt-in tooltip now receives this plain-text version so braces and the raw token never appear literally.

dist

Rebuilt packages/scripts/dist/ to match all src changes above.

Docs

reference-materials/docs/rememberme.md updated to accurately reflect the margin-transfer behaviour for rightSide, first-match-only {...} linking (with a clarifying note that additional pairs stay as literal text), and tooltip plain-text rendering.

[fernanDOTdo]

Review: New remember me features

Verdict: Request changes — two blocking issues (one is an XSS), both quick fixes. The layout work itself is solid and the legacy path is preserved.

Verified: ✅ CI green (CodeQL + Analyze) · ✅ tsc --noEmit passes · ✅ legacy behavior unchanged (default (clear autofill) label takes the exact same code path as before) · ❌ dist/ out of sync with src/ (see #2)

---

🔴 Blocking

1. Stored XSS: user-controlled first name goes into innerHTML unescaped

buildClearLabelMarkup() substitutes this.fieldData["supporter.firstName"] — raw user input, decoded back to plaintext in updateFieldData() — into the label, which insertClearRememberMeLink() then assigns via innerHTML (src/remember-me.ts:227).

Verified empirically:

firstName: "<img src=x onerror=alert(1)>"
label:     "Hi {Not $username?}"
→ html:     Hi <a>Not <img src=x onerror=alert(1)>?</a>   ← executes on next visit

Why this matters beyond self-XSS:

• Persistent — the payload is stored in the cookie / remote-iframe store and re-executes on every subsequent visit.
• Plantable via link — EN pages support URL-prefill of supporter fields, so a crafted link can prefill the malicious first name; the victim just needs to submit once with Remember Me checked.
• Integrity risk on a donation form — injected script runs in the page context and can silently alter the submission (amount, frequency, destination) before the donor clicks give.
• encryptData does not mitigate — decryption is client-side, so the plaintext is back in the DOM either way.

Suggested fix (~3 lines, also resolves #3):

const escapeHtml = (s: string) =>
  s.replace(/[&<>"']/g, (c) =>
    ({"&":"&amp;","<":"&lt;",">":"&gt;",'"':"&quot;","'":"&#39;"}[c] as string));
// ...
? this.fieldClearLabel.replace(/\$username/g, () => escapeHtml(username))

2. dist/ is stale — the final "fix: improve regex" commit was never rebuilt

The committed dist/remember-me.js still ships the old \s*\$username regex, while src has the improved [^\S\r\n]?\$username from 0587f3b8. A fresh tsc build produces exactly a 1-line diff against the committed dist (that line). Since the package entry point is dist, the regex fix would never reach consumers. Please rebuild dist/ before merging.

🟡 Should fix

3. Replacement-pattern injection in String.replace

src/remember-me.ts:274 passes username as a string replacement, so first names containing $&, $', or $` are interpreted as replacement patterns. Verified: a first name of $& renders as the literal text $username; $' renders Hi !!. The function form — .replace(/\$username/g, () => escapeHtml(username)) — fixes this and the XSS in one shot.

📝 Minor notes

• One-shot wrap measurement — alignment is computed once at render (offsetTop comparison at src/remember-me.ts:346); no resize/orientation handling. Resizing across the wrap threshold after render leaves stale margins. It's documented as computed once, but a ResizeObserver on the wrapper would be cheap insurance.
• DOM restructuring side effects — moving the target into a new flex wrapper can break theme CSS that relied on the target's original position: > child selectors from the old parent, +/~ sibling combinators. Full-width fields will also compress as flex items. Worth a warning sentence in the docs.
• .label-tooltip class stays on the outer <span> in hasInnerLink mode — theme CSS targeting .label-tooltip (cursor, underline, weight) will make the whole sentence look clickable even though only the braced part is.
• Only the first {...} becomes a link — {a} and {b} → the second stays literal with braces visible; {} yields an invisible empty anchor. Docs imply a general mechanism; a clarifying sentence would help.
• Token leaks into the opt-in tooltip — fieldClearLabel is passed raw into ENGrid.t("rememberMe.tooltip") (src/remember-me.ts:362), so braces and $username show literally in the tooltip. Cosmetic.

✅ What's good

• Legacy path byte-identical when the label has no braces/token — new features are strictly opt-in
• Sensible null-fallback on the click target (?? clearRememberMeField); wrapper reuse via class check makes placeOnRightSide idempotent
• Location options typed as plain string — no .d.ts consumer breakage
• Docs are thorough and accurate, including a worked example

---

Summary: escape the username + use the function-replace form (one line), rebuild dist/, and this is good to go. Happy to re-review — should be a fast turnaround.

[fernanDOTdo]

Re-review

Verdict: Approve — everything from the request-changes round is addressed, and the fixes go beyond what I asked for. Verified empirically and by rebuild.

Blocking issues from last round — resolved ✅

1. XSS — fixed

escapeHtml() added and the $username substitution now uses the function-replacement form (src/remember-me.ts:290-292). Tested against the original payloads:

"Hi {Not $username?}" + "<img src=x onerror=alert(1)>"
→ "Hi <a ...>Not &lt;img src=x onerror=alert(1)&gt;?</a>"   ← escaped, inert

"Hi $username" + "<script>alert(1)</script>"
→ "Hi &lt;script&gt;alert(1)&lt;/script&gt;"              ← escaped, legacy path

I also checked the remaining username sink: the opt-in tooltip (getClearLabelPlainText) inserts the username unescaped, but that's safe — ENGrid.t interpolates via function replacement (engrid.ts:362) and the tippy() call doesn't set allowHTML, so the content renders as text.

2. Stale dist/ — fixed

Full tsc rebuild produces zero diff against the committed dist. The regex improvement now actually ships.

3. Replacement-pattern injection — fixed

Same function-replacement change covers it: names of $& and $' now render literally ($&, $').

Minor notes from last round — also addressed ✅

• label-tooltip styling — the class (and cursor: pointer) moved onto the inner <a>; the outer span gets clear-autofill-data-wrapper. Surrounding text no longer looks clickable.
• Multi-{...} semantics — regex is now \{([^}]+)\} (first non-empty segment wins, {} stays literal), and the docs state this explicitly.
• Tooltip token leak — getClearLabelPlainText() resolves $username and strips braces; verified: "Welcome {Not $username?}" → "Welcome Not John?".
• One-shot wrap measurement — alignment now runs in requestAnimationFrame (fixing the stale-offsetTop read) and re-runs via ResizeObserver where supported.
• DOM-restructuring warning — docs now include a callout about the wrapper move breaking >/+/~ selectors and compressing full-width fields.
• Layout robustness — nice touch beyond the review: computed styles are read before the DOM move, the target's margin-top/-bottom are transferred to the wrapper (since flex doesn't collapse margins), and the target's own are zeroed to avoid double-spacing.

Verification

• ✅ tsc --noEmit clean; fresh build = zero diff vs committed dist
• ✅ CI green (CodeQL + Analyze)
• ✅ All label edge cases re-tested: escaped payloads, $-sequences, empty/multiple braces, no-name fallback, tooltip text

One tiny nit (non-blocking)

Each placeOnRightSide() call creates a new ResizeObserver on the wrapper without disconnecting a previous one — at most two exist in practice (opt-in and clear link), and observing a detached element is harmless, but holding the observer on the instance would be marginally tidier.

Nice turnaround — merging is fine from my side.