fix(#657): prevent HTML injection and XSS by sanitizing participant names#819
fix(#657): prevent HTML injection and XSS by sanitizing participant names#819Shan7Usmani wants to merge 4 commits into
Conversation
🚀 Thank You for Contributing to Late-MeetPlease ensure that:
Thank you for contributing 💙 |
github-actions
Bot
added
gssoc
|
👋 Thank you @Shan7Usmani for your contribution to Late-Meet! ✅ Verified: You are assigned to the linked issue #657. Please review any automated suggestions or code review comments that may appear below! We will review your PR as soon as possible! Please consider starring the repository ⭐ to show your support! |
📝 WalkthroughWalkthrough
ChangesXSS Hardening and escapeHtml Migration
README troubleshooting updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/dashboard.ts`:
- Around line 1099-1103: The data attributes for the copy-transcript-btn button
(data-speaker, data-time, data-message) are receiving already-escaped values
(speaker, timeStr, and text from lines 1076-1077 and 1088) and passing them
through sanitizeDataAttr(), causing double-encoding. Remove the
sanitizeDataAttr() calls and pass the raw unescaped values directly to the data
attributes, or if the raw values are not available in scope, retrieve them
before they were escaped and pass those raw values to sanitizeDataAttr()
instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0aacd107-872b-452b-a6ed-3275135935ce
📒 Files selected for processing (4)
src/dashboard.tssrc/utils/domHelpers.test.tssrc/utils/sanitize.test.tssrc/utils/sanitize.ts
…icipant names - Fix initials derivation in createTranscriptEntryHTML to use escaped speaker - Use sanitizeDataAttr for data-* attributes instead of escapeHtml - Consolidate duplicate escapeHtml: deprecate sanitize.ts version, keep domHelpers.ts canonical - Harden getEmptyStateHTML and truncatedNoticeHtml with escapeHtml for defense-in-depth - Add domHelpers.test.ts with XSS, attribute-context, and regression tests - Update sanitize.test.ts to import escapeHtml from canonical source - Remove redundant DOM mock from sanitize.test.ts Audit result: 0 active XSS vulnerabilities across all 47 innerHTML assignments.
|
@shouri123 all 4 PRs (#819, #820, #821, #822) have been rebased on latest main and all checks are passing. Could a maintainer please review and merge when you get a chance? These have been open since June 8 — would really appreciate a look. Thanks! 🙏 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
984-984: 📐 Maintainability & Code Quality | 🔵 TrivialUse consistent branding: "Late-Meet" or "Late Meet".
Line 984 uses "Late-Meet" (hyphenated), while line 1032 uses "Late Meet" (unhyphenated). Pick one form and use it throughout the README for consistency.
Also applies to: 1032-1032
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 984, The README uses inconsistent product branding with both “Late-Meet” and “Late Meet”; choose one canonical form and update the affected README text to use it consistently everywhere. Locate the introductory/setup wording in the README and the later section that references the product name, then replace the alternate spelling so the same branding appears throughout.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@README.md`:
- Line 984: The README uses inconsistent product branding with both “Late-Meet”
and “Late Meet”; choose one canonical form and update the affected README text
to use it consistently everywhere. Locate the introductory/setup wording in the
README and the later section that references the product name, then replace the
alternate spelling so the same branding appears throughout.
|
2 participants
Description
Prevent XSS vulnerabilities by ensuring all participant names are sanitized via \escapeHtml()\ before DOM insertion. Fixes a specific bug where the transcript initials derivation used raw (unescaped) speaker names, and hardens all rendering paths for defense-in-depth.
Fixes #657
Type of Change
Checklist:
Summary of Changes
Bug Fixes
Consolidation
Defense-in-Depth
Tests
XSS Audit
All 47 \innerHTML\ assignments across \dashboard.ts, \popup.ts, \onboarding.ts, \�piUsageDashboard.ts, and \storageDashboard.ts\ were reviewed. 0 active vulnerabilities found.
Summary by CodeRabbit