feat(#653): add strict CSP directives for security hardening - #904
feat(#653): add strict CSP directives for security hardening#904Shan7Usmani wants to merge 4 commits into
Conversation
…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.
🚀 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 #653. 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! |
📝 WalkthroughWalkthroughThe PR centralizes HTML escaping, applies sanitization across dashboard-rendered values, expands utility tests, strengthens the extension-page CSP, and replaces troubleshooting documentation with scenario-based guidance. ChangesSecurity hardening and documentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/dashboard.ts (1)
1075-1102: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep raw transcript values separate from HTML-escaped display values.
sanitizeDataAttr(speaker/timeStr/text)escapes the&introduced byescapeHtml; after HTML parsing,datasetstill contains entity text (for example,&rather than&), so copied transcript content is corrupted. The same sequencing produces&as the avatar initial for speakers beginning with an escapable character.
src/dashboard.ts#L1075-L1102: retain raw speaker/time/text values; derive initials andisAudiofrom raw speaker, pass raw values tosanitizeDataAttr, and useescapeHtmlonly for rendered text.src/utils/domHelpers.test.ts#L50-L70: update the regression test to derive initials from the raw speaker and escape only the final rendered initial.🤖 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 `@src/dashboard.ts` around lines 1075 - 1102, Separate raw transcript values from escaped display values in createTranscriptEntryHTML: derive initials and isAudio from raw speaker, pass raw speaker/time/text to sanitizeDataAttr, and apply escapeHtml only when rendering visible HTML. Update src/utils/domHelpers.test.ts lines 50-70 to derive initials from the raw speaker and escape only the final rendered initial; both sites require changes.
🤖 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 `@README.md`:
- Line 1019: Update the “Background Script Errors” troubleshooting instruction
in README.md to direct users to click the service worker link under “Inspect
views” for Late-Meet, instead of referencing background.ts/background.js
filenames. Keep the existing chrome://extensions and Developer mode navigation
intact.
---
Outside diff comments:
In `@src/dashboard.ts`:
- Around line 1075-1102: Separate raw transcript values from escaped display
values in createTranscriptEntryHTML: derive initials and isAudio from raw
speaker, pass raw speaker/time/text to sanitizeDataAttr, and apply escapeHtml
only when rendering visible HTML. Update src/utils/domHelpers.test.ts lines
50-70 to derive initials from the raw speaker and escape only the final rendered
initial; both sites require changes.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fd78f68f-9a1b-4066-8a9f-1eaabe11cc6a
📒 Files selected for processing (6)
README.mdsrc/dashboard.tssrc/manifest.jsonsrc/utils/domHelpers.test.tssrc/utils/sanitize.test.tssrc/utils/sanitize.ts
| If an error persists or you run into a transcript drop mid-meeting, check the hidden developer logs to find the exact error code: | ||
|
|
||
| - **Popup Errors:** Right-click anywhere inside the extension popup window and select **Inspect**. Look at the _Console_ tab. | ||
| - **Background Script Errors:** Go to `chrome://extensions/`, enable **Developer mode** (top right toggle), find **Late-Meet**, and click on the `background.ts` / `background.js` link next to "Inspect views". |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "README snippet:"
sed -n '1015,1022p' README.md || true
echo
echo "Manifest/package hints:"
for f in manifest.json manifest.json5 manifest.v3.json package.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,180p' "$f"
fi
done
echo
echo "Background-related files tracked:"
git ls-files | grep -Ei '\b(background|service_worker|sw)\.(ts|js|tsx|jsx)$|manifest\.(json|json5|v3\.json)$' || trueRepository: shouri123/Late-Meet
Length of output: 3219
🌐 Web query:
Chrome extensions Manifest V3 debug background service worker Inspect views service worker inactive
💡 Result:
In Chrome Manifest V3, the "service worker (inactive)" status is expected behavior rather than an error [1][2]. Chrome automatically terminates extension service workers after approximately 30 seconds of inactivity to conserve system resources [3][4]. The worker will automatically restart when a registered event (such as an extension API call, browser action, or alarm) occurs [2][5]. To debug or inspect your background service worker, use the following methods: 1. Standard Inspection: Go to chrome://extensions, enable "Developer mode," and click the blue "service worker" link under your extension's "Inspect views." This opens a DevTools window that keeps the service worker active for the duration of the session [1][2][6]. 2. Lifecycle Management: Within the DevTools window for the service worker, navigate to the Application panel and select "Service Workers" from the sidebar [7][2][6]. Here, you can manually "Start" or "Stop" the worker to simulate its lifecycle and test how your extension handles termination [2][8][9]. 3. Advanced Monitoring: For low-level details, including registration scope, status, and error logs, you can visit chrome://serviceworker-internals/ and filter for your extension's ID [10][2][11]. 4. Tips for Effective Debugging: - Persistence: If you need to retain logs across service worker restarts, check the "Preserve log" box in the DevTools Console tab [2]. - Testing Termination: To ensure your extension is resilient to termination (i.e., it does not rely on global variables that disappear when the worker stops), remember to close the DevTools window after debugging, as the presence of an active inspector prevents the worker from going idle [1][6][5]. - Debugging without DevTools: If you need to debug without keeping the worker permanently active, you can open DevTools for another context (like a popup or options page) and access the service worker through the Application tab there, which allows you to switch between contexts without forcing the background script to stay alive [7][8].
Citations:
- 1: https://developer.chrome.com/docs/extensions/get-started/tutorial/service-worker-events
- 2: https://www.codegenes.net/blog/how-to-debug-chrome-extension-service-worker-for-manifest-v3/
- 3: https://developer.chrome.com/docs/extensions/develop/concepts/service-workers/lifecycle.md.txt
- 4: https://developer.chrome.com/docs/extensions/develop/concepts/service-workers/lifecycle
- 5: https://www.crxlytics.com/blog/chrome-extension-service-worker-keeps-stopping
- 6: https://developer.chrome.com/docs/extensions/get-started/tutorial/debug
- 7: https://stackoverflow.com/questions/71267201/debugging-and-performance-profiling-manifestv3-extension-service-worker
- 8: https://stackoverflow.com/questions/73609724/force-extension-become-inactive
- 9: https://developer.chrome.com/docs/devtools/progressive-web-apps
- 10: https://stackoverflow.com/questions/63024113/how-to-debug-chrome-extension-service-worker-for-manifest-v3
- 11: https://stackoverflow.com/questions/71724980/chrome-extension-always-show-service-worker-inactive-after-browser-restart-if
Refer to the service worker link, not the background filename.
Chrome MV3 extensions are inspected via the service worker link under “Inspect views” on chrome://extensions/; the filename (src/background.ts or background.js) is not the visible link users need.
Proposed wording
- - **Background Script Errors:** Go to `chrome://extensions/`, enable **Developer mode** (top right toggle), find **Late-Meet**, and click on the `background.ts` / `background.js` link next to "Inspect views".
+ - **Background Script Errors:** Go to `chrome://extensions/`, enable **Developer mode** (top right toggle), find **Late-Meet**, and click the **service worker** link next to "Inspect views".📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - **Background Script Errors:** Go to `chrome://extensions/`, enable **Developer mode** (top right toggle), find **Late-Meet**, and click on the `background.ts` / `background.js` link next to "Inspect views". | |
| - **Background Script Errors:** Go to `chrome://extensions/`, enable **Developer mode** (top right toggle), find **Late-Meet**, and click the **service worker** link next to "Inspect views". |
🤖 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 1019, Update the “Background Script Errors”
troubleshooting instruction in README.md to direct users to click the service
worker link under “Inspect views” for Late-Meet, instead of referencing
background.ts/background.js filenames. Keep the existing chrome://extensions and
Developer mode navigation intact.
|
Hi @shouri123! Friendly ping — this PR along with #905, #906, #907, #908 were opened 3 days ago with all checks green. Would love to get these reviewed before they go stale like the previous batch. Happy to address any feedback. Thanks! |
2 participants
Description
Adds missing CSP directives to the MV3 manifest to block unauthorized external scripts, base tag injection, and form-based exfiltration vectors.
Changes
Testing
Fixes #653
Summary by CodeRabbit
Security
Documentation