From 7f587a39a8da760cfc78a36a49d71a8ef976f2e7 Mon Sep 17 00:00:00 2001 From: webadderall <131426131+webadderall@users.noreply.github.com> Date: Sat, 18 Apr 2026 16:09:16 +1000 Subject: [PATCH] fix: address CodeRabbit review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use videoSourcePath instead of fromFileUrl(videoPath) for cursor telemetry — fixes broken path when videoPath is an HTTP media-server URL - Validate filePath against approvedLocalReadPaths in get-local-media-url IPC handler before returning a URL - Use fs.realpath() instead of path.resolve() to close symlink bypass - Fix range parser to handle suffix ranges (bytes=-500) and guard against NaN values - Add CORS headers (Access-Control-Allow-Origin) to media server responses to prevent canvas tainting when using video frames - Handle OPTIONS preflight requests - Clear stale resolvedWebcamVideoUrl before resolving new URL to prevent flash of stale content --- electron/ipc/register/project.ts | 6 ++ electron/mediaServer.ts | 70 ++++++++++++++++----- src/components/video-editor/VideoEditor.tsx | 7 ++- 3 files changed, 66 insertions(+), 17 deletions(-) diff --git a/electron/ipc/register/project.ts b/electron/ipc/register/project.ts index 96b31b56..1831247b 100644 --- a/electron/ipc/register/project.ts +++ b/electron/ipc/register/project.ts @@ -15,6 +15,7 @@ import { setCurrentVideoPath, currentRecordingSession, setCurrentRecordingSession, + approvedLocalReadPaths, } from "../state"; import { normalizeVideoSourcePath } from "../utils"; import { isPathInsideDirectory, replaceApprovedSessionLocalReadPaths } from "../project/manager"; @@ -383,6 +384,11 @@ export function registerProjectHandlers() { if (!baseUrl || !filePath) { return { success: false as const }; } + const resolved = path.resolve(filePath); + if (!approvedLocalReadPaths.has(resolved)) { + console.warn(`[get-local-media-url] Blocked unapproved path: ${resolved}`); + return { success: false as const }; + } return { success: true as const, url: buildMediaUrl(baseUrl, filePath) }; }); diff --git a/electron/mediaServer.ts b/electron/mediaServer.ts index c2c655c6..2f790a97 100644 --- a/electron/mediaServer.ts +++ b/electron/mediaServer.ts @@ -25,15 +25,18 @@ function getMediaContentType(filePath: string): string { return MEDIA_MIME_TYPES[path.extname(filePath).toLowerCase()] ?? "application/octet-stream"; } -function isAllowedMediaPath(filePath: string): boolean { +async function resolveRealPath(filePath: string): Promise { try { - const resolved = path.resolve(filePath); - return approvedLocalReadPaths.has(resolved); + return await fs.realpath(path.resolve(filePath)); } catch { - return false; + return null; } } +function isAllowedMediaPath(realPath: string): boolean { + return approvedLocalReadPaths.has(realPath); +} + async function handleMediaRequest( request: IncomingMessage, response: ServerResponse, @@ -54,9 +57,9 @@ async function handleMediaRequest( return; } - const resolvedPath = path.resolve(rawPath); - if (!isAllowedMediaPath(resolvedPath)) { - console.warn(`[media-server] Blocked access to unapproved path: ${resolvedPath}`); + const resolvedPath = await resolveRealPath(rawPath); + if (!resolvedPath || !isAllowedMediaPath(resolvedPath)) { + console.warn(`[media-server] Blocked access to unapproved path: ${rawPath}`); response.writeHead(403, { "Content-Type": "text/plain; charset=utf-8" }); response.end("Forbidden"); return; @@ -73,25 +76,63 @@ async function handleMediaRequest( const fileSize = stat.size; const rangeHeader = request.headers.range; + const corsHeaders = { + "Access-Control-Allow-Origin": "*", + "Access-Control-Allow-Credentials": "false", + "Access-Control-Expose-Headers": "Content-Range, Content-Length, Accept-Ranges", + }; + + if (request.method === "OPTIONS") { + response.writeHead(204, { + ...corsHeaders, + "Access-Control-Allow-Methods": "GET, HEAD, OPTIONS", + "Access-Control-Allow-Headers": "Range", + }); + response.end(); + return; + } + if (rangeHeader) { - const match = rangeHeader.match(/bytes=(\d+)-(\d*)/); - if (!match) { - response.writeHead(416, { "Content-Range": `bytes */${fileSize}` }); + const match = rangeHeader.match(/bytes=(\d*)-(\d*)/); + if (!match || (!match[1] && !match[2])) { + response.writeHead(416, { ...corsHeaders, "Content-Range": `bytes */${fileSize}` }); response.end(); return; } - const start = Number.parseInt(match[1], 10); - const end = match[2] ? Number.parseInt(match[2], 10) : fileSize - 1; + let start: number; + let end: number; - if (start >= fileSize || end >= fileSize || start > end) { - response.writeHead(416, { "Content-Range": `bytes */${fileSize}` }); + if (!match[1] && match[2]) { + // Suffix range: bytes=-500 + const suffixLength = Number.parseInt(match[2], 10); + if (Number.isNaN(suffixLength) || suffixLength <= 0) { + response.writeHead(416, { ...corsHeaders, "Content-Range": `bytes */${fileSize}` }); + response.end(); + return; + } + start = Math.max(0, fileSize - suffixLength); + end = fileSize - 1; + } else { + start = Number.parseInt(match[1], 10); + end = match[2] ? Number.parseInt(match[2], 10) : fileSize - 1; + } + + if (Number.isNaN(start) || Number.isNaN(end) || start > end || start >= fileSize || end >= fileSize) { + response.writeHead(416, { ...corsHeaders, "Content-Range": `bytes */${fileSize}` }); + response.end(); + return; + } + + if (fileSize === 0) { + response.writeHead(416, { ...corsHeaders, "Content-Range": `bytes */0` }); response.end(); return; } const chunkSize = end - start + 1; response.writeHead(206, { + ...corsHeaders, "Content-Range": `bytes ${start}-${end}/${fileSize}`, "Accept-Ranges": "bytes", "Content-Length": String(chunkSize), @@ -114,6 +155,7 @@ async function handleMediaRequest( }); } else { response.writeHead(200, { + ...corsHeaders, "Accept-Ranges": "bytes", "Content-Length": String(fileSize), "Content-Type": contentType, diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index 4863122c..4fef5c49 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -1806,6 +1806,7 @@ export default function VideoEditor() { setResolvedWebcamVideoUrl(null); return; } + setResolvedWebcamVideoUrl(null); void resolveVideoUrl(webcam.sourcePath).then((url) => { if (!cancelled) setResolvedWebcamVideoUrl(url); }); @@ -2232,7 +2233,7 @@ export default function VideoEditor() { let retryAttempts = 0; async function loadCursorTelemetry() { - if (!videoPath) { + if (!videoPath || !videoSourcePath) { if (mounted) { setCursorTelemetry([]); } @@ -2240,7 +2241,7 @@ export default function VideoEditor() { } try { - const result = await window.electronAPI.getCursorTelemetry(fromFileUrl(videoPath)); + const result = await window.electronAPI.getCursorTelemetry(videoSourcePath); if (mounted) { const samples = result.success ? result.samples : []; setCursorTelemetry(samples); @@ -2295,7 +2296,7 @@ export default function VideoEditor() { pendingTelemetryRetryTimeoutRef.current = null; } }; - }, [videoPath]); + }, [videoPath, videoSourcePath]); const normalizedCursorTelemetry = useMemo(() => { if (cursorTelemetry.length === 0) {