fix(desktop): WASM not actually being used by xterm fix#383
Conversation
WalkthroughThis change updates the Content Security Policy (CSP) directives in the desktop renderer HTML file. The script-src is expanded to permit WebAssembly unsafe evaluation for xterm ImageAddon, while img-src is refined to allow avatar images from Vercel blob storage while correcting the data URI syntax. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/renderer/index.html(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-12T05:45:09.686Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-12T05:45:09.686Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx,js,jsx} apps/desktop/src/lib/**/*.{ts,tsx,js,jsx} apps/desktop/src/shared/**/*.{ts,tsx,js,tsx} : Never import Node.js modules in renderer process or shared code, only in main process
Applied to files:
apps/desktop/src/renderer/index.html
🔇 Additional comments (1)
apps/desktop/src/renderer/index.html (1)
18-18: The'wasm-unsafe-eval'addition is correct and documented.xterm ImageAddon provides inline image output supporting SIXEL and iTerm's inline image protocol (IIP), which uses WebAssembly. 'wasm-unsafe-eval' controls WebAssembly execution, and if not specified in the script-src directive, WebAssembly is blocked from loading and executing on the page. The inline HTML comment already explains this purpose.
The 'wasm-unsafe-eval' source expression is more specific than 'unsafe-eval', which permits both compilation and instantiation of WebAssembly and the use of the eval operation in JavaScript. This is the standard approach recommended by W3C and browser vendors for enabling WASM support while maintaining CSP security.
No further action needed on the CSP directive itself. The change is justified, necessary, and properly documented.
|
|
||
| - default-src 'self': Only allow resources from same origin | ||
| - script-src 'self' https://*.posthog.com: Allow scripts from same origin + PostHog | ||
| - script-src 'self' 'wasm-unsafe-eval' https://*.posthog.com: Allow scripts from same origin + WebAssembly (for xterm ImageAddon) + PostHog |
There was a problem hiding this comment.
Remove incorrect claim that xterm ImageAddon requires WebAssembly.
The comment states that 'wasm-unsafe-eval' is needed for xterm ImageAddon, but the addon processes images entirely in JavaScript and does not require WebAssembly. The encoder is documented as "not yet wasm optimized," confirming WASM is not currently used. Update the comment to reflect that this CSP directive is not needed for the ImageAddon specifically.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/index.html around line 12, the inline comment
incorrectly claims that 'wasm-unsafe-eval' is required for xterm ImageAddon;
update the comment to remove or reword that claim and state that the ImageAddon
processes images in JavaScript and does not require WebAssembly, and if
'wasm-unsafe-eval' remains in the CSP, clarify it's for other unrelated WASM
uses (or remove it if not needed elsewhere).
| - style-src 'self' 'unsafe-inline': Allow styles from same origin + inline (needed for CSS-in-JS) | ||
| - connect-src 'self' ws: wss: https://*.posthog.com: Allow WebSocket connections for HMR + PostHog analytics | ||
| - img-src 'self' data:: Allow images from same origin + data URIs | ||
| - img-src 'self' data: https://*.public.blob.vercel-storage.com: Allow images from same origin + data URIs + Vercel blob storage (avatars) |
There was a problem hiding this comment.
Good fix for data URI syntax, but verify Vercel blob storage scope.
The correction from data:: to data: fixes invalid CSP syntax for data URIs—this is a proper bug fix.
However, adding https://*.public.blob.vercel-storage.com for avatar images appears unrelated to the WASM fix described in the PR title. Consider splitting unrelated changes into separate PRs for clearer change tracking.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.