fix: CSP worker-src blocking editor video/camera preview - #9
Merged
Conversation
No worker-src was declared, so Chromium fell back to script-src for the waveform/decode workers the editor spins up from blob: URLs - and script-src doesn't allow blob:, so worker creation was blocked outright with a CSP violation. That block was severe enough to abort the surrounding media-loading sequence, so after a recording the editor's preview showed no video and no camera feed at all - this was a regression from the original webSecurity/CSP hardening, not a problem with the recording pipeline itself. Note: the console also showed connect-src violations for sidecar audio files (file:// fetches for possible .system.wav/.mic.m4a tracks). I initially widened connect-src to allow file: broadly, but reverted that - RECORDINGS_DIR is already an allowed prefix for the proper getLocalMediaUrl()/media-server path (electron/ipc/project/manager.ts isAllowedLocalReadPath), and the sidecar-probing code already retries per-candidate on any failure (src/components/video-editor/timeline/hooks/useTimelineAudioPeaks.ts), so those file:// fetches failing is harmless - they're a last-resort fallback for when the media server itself is unavailable, not the normal path. Broadening connect-src to file: renderer-wide would have let any compromised renderer code (a bad extension, XSS) read arbitrary local files directly via fetch(), bypassing every path allowlist fixed in the original security review, to fix a symptom that the worker-src fix alone already resolves. Verified by loading a known-good recording directly in a debug build: with only this change, every CSP violation is gone and the main video load no longer aborts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
worker-srcwas declared in the CSP, so Chromium fell back toscript-srcfor the waveform/decode workers the editor creates fromblob:URLs.script-srcdoesn't allowblob:, so worker creation threw a CSP violation, severe enough to abort the surrounding media-loading sequence entirely.worker-src 'self' blob: <allowed origins>directive.A security note on scope
The console also showed
connect-srcviolations for sidecar audio files (fetch('file://.../recording.system.wav')etc.). I initially widenedconnect-srcto allowfile:broadly to silence those too, but reverted that after checkingRECORDINGS_DIRis already an allowed prefix for the propergetLocalMediaUrl()media-server path, and the sidecar-probing code (useTimelineAudioPeaks.ts) already retries per-candidate on any failure - so those specificfile://fallback fetches failing is harmless. Broadeningconnect-srctofile:renderer-wide would have let any compromised renderer code (a bad extension, XSS) read arbitrary local files directly viafetch(), bypassing every path allowlist fixed in the original security review - to fix a symptom theworker-srcfix alone already resolves.Test plan
tsc --noEmitcleannpm test- 94/94 files, 817/817 tests passingnet::ERR_ABORTED)