From d145f800415545f444193727455c063595f35c76 Mon Sep 17 00:00:00 2001 From: Enriquefft Date: Fri, 24 Apr 2026 17:59:21 -0500 Subject: [PATCH 1/8] fix: wallpaper backgrounds black in exported video (#376) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three independent defects plus one SSOT violation caused reported symptom of image wallpapers rendering solid black in exported MP4/GIF while appearing correctly in the editor preview. Bug A — Dev-mode IPC handler returned /public/assets/, but wallpapers live at public/wallpapers/. No assets/ subdirectory exists in source. Bug B — FrameRenderer.setupBackground bypassed getAssetPath and did window.location.origin + wallpaper, producing file:///wallpapers/*.jpg 404s in packaged Electron. Bug C — setupBackground silently caught any background-load error and filled black. Masked Bug B from the export pipeline; why the bug shipped. Smell D — Asset layout asymmetric: public/wallpapers/ (dev) vs resources/assets/wallpapers/ (packaged). assets/ subdirectory had no other consumers. Fixes: - Unify asset layout. electron-builder extraResources now copies to resources/wallpapers/ (no assets/). Main handler returns / packaged and /public/ unpackaged. Same convention in both modes: /wallpapers/x.jpg maps to /wallpapers/x.jpg. Nix package.nix mirror updated. - New src/lib/wallpaper.ts module owns the wallpaper contract: DEFAULT_WALLPAPER, classifyWallpaper (color/gradient/image), and resolveImageWallpaperUrl (pure URL resolver, wraps getAssetPath). BackgroundLoadError typed error for short-circuit detection. - FrameRenderer.setupBackground uses the new helpers. Silent black fallback removed; rethrows as BackgroundLoadError. Export pipeline (VideoExporter + GifExporter) short-circuits encoder-retry loop on BackgroundLoadError. VideoEditor catch site dispatches to translated exportBackgroundLoadFailed toast. - VideoPlayback editor preview consolidated onto the same helpers. Three default-wallpaper path literals (useEditorHistory, projectPersistence, VideoPlayback) collapsed onto DEFAULT_WALLPAPER. - i18n: new errors.exportBackgroundLoadFailed key added to all seven locales (en, zh-CN, zh-TW, es, fr, tr, ko-KR). - Tests: 20 unit tests for wallpaper module (classifyWallpaper + resolveImageWallpaperUrl branches + BackgroundLoadError). videoExporter.browser.test.ts and gifExporter.browser.test.ts extended with image-wallpaper happy path and BackgroundLoadError failure path. Migration note: packaged users upgrading in place may retain an empty resources/assets/ directory from the prior layout. Unreferenced at runtime; cosmetic only. DMG/AppImage fresh installs get the new layout directly. --- electron-builder.json5 | 2 +- electron/ipc/handlers.ts | 15 +- nix/package.nix | 8 +- src/components/video-editor/VideoEditor.tsx | 14 +- src/components/video-editor/VideoPlayback.tsx | 51 +---- .../video-editor/projectPersistence.ts | 3 +- src/hooks/useEditorHistory.ts | 3 +- src/i18n/locales/en/editor.json | 1 + src/i18n/locales/es/editor.json | 1 + src/i18n/locales/fr/editor.json | 1 + src/i18n/locales/ko-KR/editor.json | 1 + src/i18n/locales/tr/editor.json | 1 + src/i18n/locales/zh-CN/editor.json | 1 + src/i18n/locales/zh-TW/editor.json | 1 + src/lib/exporter/frameRenderer.ts | 179 ++++++++---------- src/lib/exporter/gifExporter.browser.test.ts | 41 ++++ src/lib/exporter/gifExporter.ts | 4 + .../exporter/videoExporter.browser.test.ts | 39 ++++ src/lib/exporter/videoExporter.ts | 6 + src/lib/wallpaper.test.ts | 167 ++++++++++++++++ src/lib/wallpaper.ts | 43 +++++ 21 files changed, 418 insertions(+), 164 deletions(-) create mode 100644 src/lib/wallpaper.test.ts create mode 100644 src/lib/wallpaper.ts diff --git a/electron-builder.json5 b/electron-builder.json5 index 18498df..441eda4 100644 --- a/electron-builder.json5 +++ b/electron-builder.json5 @@ -23,7 +23,7 @@ "extraResources": [ { "from": "public/wallpapers", - "to": "assets/wallpapers" + "to": "wallpapers" } ], diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 261d93f..6aec971 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -801,15 +801,16 @@ export function registerIpcHandlers( } }); - // Return base path for assets so renderer can resolve file:// paths in production + // Return base path for assets so renderer can resolve file:// paths in production. + // Packaged: electron-builder extraResources copies public/wallpapers -> resources/wallpapers. + // Unpackaged: wallpapers live at /public/wallpapers. + // Single convention: "/wallpapers/x.jpg" resolves in both modes. ipcMain.handle("get-asset-base-path", () => { try { - if (app.isPackaged) { - const assetPath = path.join(process.resourcesPath, "assets"); - return pathToFileURL(`${assetPath}${path.sep}`).toString(); - } - const assetPath = path.join(app.getAppPath(), "public", "assets"); - return pathToFileURL(`${assetPath}${path.sep}`).toString(); + const baseDir = app.isPackaged + ? process.resourcesPath + : path.join(app.getAppPath(), "public"); + return pathToFileURL(`${baseDir}${path.sep}`).toString(); } catch (err) { console.error("Failed to resolve asset base path:", err); return null; diff --git a/nix/package.nix b/nix/package.nix index 195043f..13a8658 100644 --- a/nix/package.nix +++ b/nix/package.nix @@ -68,10 +68,10 @@ buildNpmPackage { cp -r node_modules "$out/lib/openscreen/" # Asset resolution: when app.isPackaged is false, the main process resolves - # assets at /public/assets/. Mirror the electron-builder - # extraResources layout so wallpapers load correctly. - mkdir -p "$out/lib/openscreen/public/assets" - cp -r public/wallpapers "$out/lib/openscreen/public/assets/wallpapers" + # assets at /public/. Place wallpapers at that root to match the + # packaged layout (electron-builder extraResources -> resources/wallpapers). + mkdir -p "$out/lib/openscreen/public" + cp -r public/wallpapers "$out/lib/openscreen/public/wallpapers" # Wrap system electron with the app directory mkdir -p "$out/bin" diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index 6d21d13..3694d9e 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -32,6 +32,7 @@ import { computeFrameStepTime } from "@/lib/frameStep"; import type { ProjectMedia } from "@/lib/recordingSession"; import { matchesShortcut } from "@/lib/shortcuts"; import { loadUserPreferences, saveUserPreferences } from "@/lib/userPreferences"; +import { BackgroundLoadError } from "@/lib/wallpaper"; import { getAspectRatioValue, getNativeAspectRatioValue, @@ -1566,9 +1567,15 @@ export default function VideoEditor() { } } catch (error) { console.error("Export error:", error); - const errorMessage = error instanceof Error ? error.message : "Unknown error"; - setExportError(errorMessage); - toast.error(`Export failed: ${errorMessage}`); + if (error instanceof BackgroundLoadError) { + const message = t("errors.exportBackgroundLoadFailed", { url: error.url }); + setExportError(message); + toast.error(message); + } else { + const errorMessage = error instanceof Error ? error.message : "Unknown error"; + setExportError(errorMessage); + toast.error(t("errors.exportFailedWithError", { error: errorMessage })); + } } finally { setIsExporting(false); exporterRef.current = null; @@ -1601,6 +1608,7 @@ export default function VideoEditor() { exportQuality, handleExportSaved, cursorTelemetry, + t, ], ); diff --git a/src/components/video-editor/VideoPlayback.tsx b/src/components/video-editor/VideoPlayback.tsx index b798641..db69310 100644 --- a/src/components/video-editor/VideoPlayback.tsx +++ b/src/components/video-editor/VideoPlayback.tsx @@ -18,7 +18,6 @@ import { useRef, useState, } from "react"; -import { getAssetPath } from "@/lib/assetPath"; import { getWebcamLayoutCssBoxShadow, type Size, @@ -26,6 +25,7 @@ import { type WebcamLayoutPreset, type WebcamSizePreset, } from "@/lib/compositeLayout"; +import { classifyWallpaper, DEFAULT_WALLPAPER, resolveImageWallpaperUrl } from "@/lib/wallpaper"; import { getCssClipPath } from "@/lib/webcamMaskShapes"; import { type AspectRatio, @@ -1179,49 +1179,14 @@ const VideoPlayback = forwardRef( useEffect(() => { let mounted = true; (async () => { - try { - if (!wallpaper) { - const def = await getAssetPath("wallpapers/wallpaper1.jpg"); - if (mounted) setResolvedWallpaper(def); - return; - } - - if ( - wallpaper.startsWith("#") || - wallpaper.startsWith("linear-gradient") || - wallpaper.startsWith("radial-gradient") - ) { - if (mounted) setResolvedWallpaper(wallpaper); - return; - } - - // If it's a data URL (custom uploaded image), use as-is - if (wallpaper.startsWith("data:")) { - if (mounted) setResolvedWallpaper(wallpaper); - return; - } - - // If it's an absolute web/http or file path, use as-is - if ( - wallpaper.startsWith("http") || - wallpaper.startsWith("file://") || - wallpaper.startsWith("/") - ) { - // If it's an absolute server path (starts with '/'), resolve via getAssetPath as well - if (wallpaper.startsWith("/")) { - const rel = wallpaper.replace(/^\//, ""); - const p = await getAssetPath(rel); - if (mounted) setResolvedWallpaper(p); - return; - } - if (mounted) setResolvedWallpaper(wallpaper); - return; - } - const p = await getAssetPath(wallpaper.replace(/^\//, "")); - if (mounted) setResolvedWallpaper(p); - } catch (_err) { - if (mounted) setResolvedWallpaper(wallpaper || "/wallpapers/wallpaper1.jpg"); + const source = wallpaper || DEFAULT_WALLPAPER; + const classified = classifyWallpaper(source); + if (classified.kind !== "image") { + if (mounted) setResolvedWallpaper(classified.value); + return; } + const resolved = await resolveImageWallpaperUrl(classified.path); + if (mounted) setResolvedWallpaper(resolved); })(); return () => { mounted = false; diff --git a/src/components/video-editor/projectPersistence.ts b/src/components/video-editor/projectPersistence.ts index c085e0d..f9640e4 100644 --- a/src/components/video-editor/projectPersistence.ts +++ b/src/components/video-editor/projectPersistence.ts @@ -2,6 +2,7 @@ import { normalizeBlurColor, normalizeBlurType } from "@/lib/blurEffects"; import type { ExportFormat, ExportQuality, GifFrameRate, GifSizePreset } from "@/lib/exporter"; import type { ProjectMedia } from "@/lib/recordingSession"; import { normalizeProjectMedia } from "@/lib/recordingSession"; +import { DEFAULT_WALLPAPER } from "@/lib/wallpaper"; import { ASPECT_RATIOS, type AspectRatio, isPortraitAspectRatio } from "@/utils/aspectRatioUtils"; import { type AnnotationRegion, @@ -425,7 +426,7 @@ export function normalizeProjectEditor(editor: Partial): Pro const cropHeight = clamp(rawCropHeight, 0.01, 1 - cropY); return { - wallpaper: typeof editor.wallpaper === "string" ? editor.wallpaper : WALLPAPER_PATHS[0], + wallpaper: typeof editor.wallpaper === "string" ? editor.wallpaper : DEFAULT_WALLPAPER, shadowIntensity: typeof editor.shadowIntensity === "number" ? editor.shadowIntensity : 0, showBlur: typeof editor.showBlur === "boolean" ? editor.showBlur : false, motionBlurAmount: isFiniteNumber(editor.motionBlurAmount) diff --git a/src/hooks/useEditorHistory.ts b/src/hooks/useEditorHistory.ts index cc19222..bd410da 100644 --- a/src/hooks/useEditorHistory.ts +++ b/src/hooks/useEditorHistory.ts @@ -17,6 +17,7 @@ import { DEFAULT_WEBCAM_POSITION, DEFAULT_WEBCAM_SIZE_PRESET, } from "@/components/video-editor/types"; +import { DEFAULT_WALLPAPER } from "@/lib/wallpaper"; import type { AspectRatio } from "@/utils/aspectRatioUtils"; // Undoable state — selection IDs are intentionally excluded (undoing a @@ -46,7 +47,7 @@ export const INITIAL_EDITOR_STATE: EditorState = { speedRegions: [], annotationRegions: [], cropRegion: DEFAULT_CROP_REGION, - wallpaper: "/wallpapers/wallpaper1.jpg", + wallpaper: DEFAULT_WALLPAPER, shadowIntensity: 0, showBlur: false, motionBlurAmount: 0, diff --git a/src/i18n/locales/en/editor.json b/src/i18n/locales/en/editor.json index a171b16..254272d 100644 --- a/src/i18n/locales/en/editor.json +++ b/src/i18n/locales/en/editor.json @@ -14,6 +14,7 @@ "failedToSaveVideo": "Failed to save video", "exportFailed": "Export failed", "exportFailedWithError": "Export failed: {{error}}", + "exportBackgroundLoadFailed": "Export failed: could not load background image ({{url}})", "failedToSaveExport": "Failed to save export", "failedToSaveExportedVideo": "Failed to save exported video", "failedToRevealInFolder": "Error revealing in folder: {{error}}" diff --git a/src/i18n/locales/es/editor.json b/src/i18n/locales/es/editor.json index 7956b75..c71368a 100644 --- a/src/i18n/locales/es/editor.json +++ b/src/i18n/locales/es/editor.json @@ -8,6 +8,7 @@ "failedToSaveVideo": "Error al guardar el video", "exportFailed": "La exportación falló", "exportFailedWithError": "La exportación falló: {{error}}", + "exportBackgroundLoadFailed": "La exportación falló: no se pudo cargar la imagen de fondo ({{url}})", "failedToSaveExport": "Error al guardar la exportación", "failedToSaveExportedVideo": "Error al guardar el video exportado", "failedToRevealInFolder": "Error al mostrar en la carpeta: {{error}}" diff --git a/src/i18n/locales/fr/editor.json b/src/i18n/locales/fr/editor.json index 03596bd..ae48fdf 100644 --- a/src/i18n/locales/fr/editor.json +++ b/src/i18n/locales/fr/editor.json @@ -14,6 +14,7 @@ "failedToSaveVideo": "Échec de l'enregistrement de la vidéo", "exportFailed": "L'export a échoué", "exportFailedWithError": "L'export a échoué : {{error}}", + "exportBackgroundLoadFailed": "L'export a échoué : impossible de charger l'image d'arrière-plan ({{url}})", "failedToSaveExport": "Échec de l'enregistrement de l'export", "failedToSaveExportedVideo": "Échec de l'enregistrement de la vidéo exportée", "failedToRevealInFolder": "Erreur lors de l'affichage dans le dossier : {{error}}" diff --git a/src/i18n/locales/ko-KR/editor.json b/src/i18n/locales/ko-KR/editor.json index 4db7d1f..eed0261 100644 --- a/src/i18n/locales/ko-KR/editor.json +++ b/src/i18n/locales/ko-KR/editor.json @@ -14,6 +14,7 @@ "failedToSaveVideo": "비디오 저장에 실패했습니다", "exportFailed": "내보내기에 실패했습니다", "exportFailedWithError": "내보내기 실패: {{error}}", + "exportBackgroundLoadFailed": "내보내기 실패: 배경 이미지를 불러올 수 없습니다 ({{url}})", "failedToSaveExport": "내보낸 파일 저장에 실패했습니다", "failedToSaveExportedVideo": "내보낸 비디오 저장에 실패했습니다", "failedToRevealInFolder": "폴더에서 파일 표시 오류: {{error}}" diff --git a/src/i18n/locales/tr/editor.json b/src/i18n/locales/tr/editor.json index dfa4cb1..c34d64b 100644 --- a/src/i18n/locales/tr/editor.json +++ b/src/i18n/locales/tr/editor.json @@ -8,6 +8,7 @@ "failedToSaveVideo": "Video kaydedilemedi", "exportFailed": "Dışa aktarım başarısız oldu", "exportFailedWithError": "Dışa aktarım başarısız: {{error}}", + "exportBackgroundLoadFailed": "Dışa aktarım başarısız: arka plan görüntüsü yüklenemedi ({{url}})", "failedToSaveExport": "Dışa aktarım kaydedilemedi", "failedToSaveExportedVideo": "Dışa aktarılan video kaydedilemedi", "failedToRevealInFolder": "Klasörde gösterme hatası: {{error}}" diff --git a/src/i18n/locales/zh-CN/editor.json b/src/i18n/locales/zh-CN/editor.json index 1980354..f2eba2e 100644 --- a/src/i18n/locales/zh-CN/editor.json +++ b/src/i18n/locales/zh-CN/editor.json @@ -14,6 +14,7 @@ "failedToSaveVideo": "保存视频失败", "exportFailed": "导出失败", "exportFailedWithError": "导出失败：{{error}}", + "exportBackgroundLoadFailed": "导出失败：无法加载背景图片（{{url}}）", "failedToSaveExport": "保存导出文件失败", "failedToSaveExportedVideo": "保存导出的视频失败", "failedToRevealInFolder": "在文件夹中显示时出错：{{error}}" diff --git a/src/i18n/locales/zh-TW/editor.json b/src/i18n/locales/zh-TW/editor.json index 73a3f4e..ee502fb 100644 --- a/src/i18n/locales/zh-TW/editor.json +++ b/src/i18n/locales/zh-TW/editor.json @@ -14,6 +14,7 @@ "failedToSaveVideo": "儲存影片失敗", "exportFailed": "匯出失敗", "exportFailedWithError": "匯出失敗：{{error}}", + "exportBackgroundLoadFailed": "匯出失敗：無法載入背景圖片（{{url}}）", "failedToSaveExport": "儲存匯出檔案失敗", "failedToSaveExportedVideo": "儲存匯出的影片失敗", "failedToRevealInFolder": "在資料夾中顯示時出錯：{{error}}" diff --git a/src/lib/exporter/frameRenderer.ts b/src/lib/exporter/frameRenderer.ts index 9b1cf6d..e7362eb 100644 --- a/src/lib/exporter/frameRenderer.ts +++ b/src/lib/exporter/frameRenderer.ts @@ -45,6 +45,7 @@ import { type Size, type StyledRenderRect, } from "@/lib/compositeLayout"; +import { BackgroundLoadError, classifyWallpaper, resolveImageWallpaperUrl } from "@/lib/wallpaper"; import { drawCanvasClipPath } from "@/lib/webcamMaskShapes"; import { renderAnnotations } from "./annotationRenderer"; import { @@ -231,123 +232,93 @@ export class FrameRenderer { private async setupBackground(): Promise { const wallpaper = this.config.wallpaper; - // Create background canvas for separate rendering (not affected by zoom) const bgCanvas = document.createElement("canvas"); bgCanvas.width = this.config.width; bgCanvas.height = this.config.height; const bgCtx = bgCanvas.getContext("2d")!; - try { - // Render background based on type - if ( - wallpaper.startsWith("file://") || - wallpaper.startsWith("data:") || - wallpaper.startsWith("/") || - wallpaper.startsWith("http") - ) { - // Image background - const img = new Image(); - // Don't set crossOrigin for same-origin images to avoid CORS taint - // Only set it for cross-origin URLs - let imageUrl: string; - if (wallpaper.startsWith("http")) { - imageUrl = wallpaper; - if (!imageUrl.startsWith(window.location.origin)) { - img.crossOrigin = "anonymous"; - } - } else if (wallpaper.startsWith("file://") || wallpaper.startsWith("data:")) { - imageUrl = wallpaper; - } else { - imageUrl = window.location.origin + wallpaper; - } + const classified = classifyWallpaper(wallpaper); + if (classified.kind === "color") { + bgCtx.fillStyle = classified.value; + bgCtx.fillRect(0, 0, this.config.width, this.config.height); + } else if (classified.kind === "gradient") { + const parsedGradient = parseCssGradient(classified.value); + if (!parsedGradient) { + throw new BackgroundLoadError(classified.value); + } + const gradient = + parsedGradient.type === "linear" + ? (() => { + const points = getLinearGradientPoints( + resolveLinearGradientAngle(parsedGradient.descriptor), + this.config.width, + this.config.height, + ); + return bgCtx.createLinearGradient(points.x0, points.y0, points.x1, points.y1); + })() + : (() => { + const shape = getRadialGradientShape( + parsedGradient.descriptor, + this.config.width, + this.config.height, + ); + return bgCtx.createRadialGradient( + shape.cx, + shape.cy, + 0, + shape.cx, + shape.cy, + shape.radius, + ); + })(); + + parsedGradient.stops.forEach((stop) => { + gradient.addColorStop(stop.offset, stop.color); + }); + + bgCtx.fillStyle = gradient; + bgCtx.fillRect(0, 0, this.config.width, this.config.height); + } else { + const imageUrl = await resolveImageWallpaperUrl(classified.path); + const img = new Image(); + if (imageUrl.startsWith("http") && !imageUrl.startsWith(window.location.origin)) { + img.crossOrigin = "anonymous"; + } + + try { await new Promise((resolve, reject) => { img.onload = () => resolve(); - img.onerror = (err) => { - console.error("[FrameRenderer] Failed to load background image:", imageUrl, err); - reject(new Error(`Failed to load background image: ${imageUrl}`)); - }; + img.onerror = (err) => reject(err); img.src = imageUrl; }); - - // Draw the image using cover and center positioning - const imgAspect = img.width / img.height; - const canvasAspect = this.config.width / this.config.height; - - let drawWidth, drawHeight, drawX, drawY; - - if (imgAspect > canvasAspect) { - drawHeight = this.config.height; - drawWidth = drawHeight * imgAspect; - drawX = (this.config.width - drawWidth) / 2; - drawY = 0; - } else { - drawWidth = this.config.width; - drawHeight = drawWidth / imgAspect; - drawX = 0; - drawY = (this.config.height - drawHeight) / 2; - } - - bgCtx.drawImage(img, drawX, drawY, drawWidth, drawHeight); - } else if (wallpaper.startsWith("#")) { - bgCtx.fillStyle = wallpaper; - bgCtx.fillRect(0, 0, this.config.width, this.config.height); - } else if ( - wallpaper.startsWith("linear-gradient") || - wallpaper.startsWith("radial-gradient") - ) { - const parsedGradient = parseCssGradient(wallpaper); - if (parsedGradient) { - const gradient = - parsedGradient.type === "linear" - ? (() => { - const points = getLinearGradientPoints( - resolveLinearGradientAngle(parsedGradient.descriptor), - this.config.width, - this.config.height, - ); - - return bgCtx.createLinearGradient(points.x0, points.y0, points.x1, points.y1); - })() - : (() => { - const shape = getRadialGradientShape( - parsedGradient.descriptor, - this.config.width, - this.config.height, - ); - - return bgCtx.createRadialGradient( - shape.cx, - shape.cy, - 0, - shape.cx, - shape.cy, - shape.radius, - ); - })(); - - parsedGradient.stops.forEach((stop) => { - gradient.addColorStop(stop.offset, stop.color); - }); - - bgCtx.fillStyle = gradient; - bgCtx.fillRect(0, 0, this.config.width, this.config.height); - } else { - console.warn("[FrameRenderer] Could not parse gradient, using black fallback"); - bgCtx.fillStyle = "#000000"; - bgCtx.fillRect(0, 0, this.config.width, this.config.height); - } - } else { - bgCtx.fillStyle = wallpaper; - bgCtx.fillRect(0, 0, this.config.width, this.config.height); + } catch (err) { + throw new BackgroundLoadError(imageUrl, err); } - } catch (error) { - console.error("[FrameRenderer] Error setting up background, using fallback:", error); - bgCtx.fillStyle = "#000000"; - bgCtx.fillRect(0, 0, this.config.width, this.config.height); + + const imgAspect = img.width / img.height; + const canvasAspect = this.config.width / this.config.height; + + let drawWidth: number; + let drawHeight: number; + let drawX: number; + let drawY: number; + + if (imgAspect > canvasAspect) { + drawHeight = this.config.height; + drawWidth = drawHeight * imgAspect; + drawX = (this.config.width - drawWidth) / 2; + drawY = 0; + } else { + drawWidth = this.config.width; + drawHeight = drawWidth / imgAspect; + drawX = 0; + drawY = (this.config.height - drawHeight) / 2; + } + + bgCtx.drawImage(img, drawX, drawY, drawWidth, drawHeight); } - // Store the background canvas for compositing this.backgroundSprite = bgCanvas; } diff --git a/src/lib/exporter/gifExporter.browser.test.ts b/src/lib/exporter/gifExporter.browser.test.ts index db9b144..5a69468 100644 --- a/src/lib/exporter/gifExporter.browser.test.ts +++ b/src/lib/exporter/gifExporter.browser.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import sampleVideoUrl from "../../../tests/fixtures/sample.webm?url"; +import { BackgroundLoadError } from "../wallpaper"; import { GifExporter } from "./gifExporter"; import type { ExportProgress } from "./types"; @@ -40,4 +41,44 @@ describe("GifExporter (real browser)", () => { expect(finalizing.length).toBeGreaterThan(0); expect(finalizing.at(-1)!.percentage).toBe(100); }); + + it("exports successfully with an image wallpaper (served by Vite dev server)", async () => { + const exporter = new GifExporter({ + videoUrl: sampleVideoUrl, + width: 320, + height: 180, + frameRate: 15, + loop: true, + sizePreset: "medium", + wallpaper: "/wallpapers/wallpaper1.jpg", + zoomRegions: [], + showShadow: false, + shadowIntensity: 0, + showBlur: false, + cropRegion: { x: 0, y: 0, width: 1, height: 1 }, + }); + + const result = await exporter.export(); + expect(result.success, result.error).toBe(true); + expect(result.blob!.size).toBeGreaterThan(1024); + }); + + it("throws BackgroundLoadError when wallpaper fails to load (no silent black fallback)", async () => { + const exporter = new GifExporter({ + videoUrl: sampleVideoUrl, + width: 320, + height: 180, + frameRate: 15, + loop: true, + sizePreset: "medium", + wallpaper: "/wallpapers/does-not-exist.jpg", + zoomRegions: [], + showShadow: false, + shadowIntensity: 0, + showBlur: false, + cropRegion: { x: 0, y: 0, width: 1, height: 1 }, + }); + + await expect(exporter.export()).rejects.toBeInstanceOf(BackgroundLoadError); + }); }); diff --git a/src/lib/exporter/gifExporter.ts b/src/lib/exporter/gifExporter.ts index 46ac6a0..f073d6b 100644 --- a/src/lib/exporter/gifExporter.ts +++ b/src/lib/exporter/gifExporter.ts @@ -8,6 +8,7 @@ import type { WebcamSizePreset, ZoomRegion, } from "@/components/video-editor/types"; +import { BackgroundLoadError } from "@/lib/wallpaper"; import { getPlatform } from "@/utils/platformUtils"; import { AsyncVideoFrameQueue } from "./asyncVideoFrameQueue"; import { FrameRenderer } from "./frameRenderer"; @@ -326,6 +327,9 @@ export class GifExporter { return { success: true, blob }; } catch (error) { + if (error instanceof BackgroundLoadError) { + throw error; + } console.error("GIF Export error:", error); return { success: false, diff --git a/src/lib/exporter/videoExporter.browser.test.ts b/src/lib/exporter/videoExporter.browser.test.ts index ec2b0f6..ae8c7bc 100644 --- a/src/lib/exporter/videoExporter.browser.test.ts +++ b/src/lib/exporter/videoExporter.browser.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import sampleVideoUrl from "../../../tests/fixtures/sample.webm?url"; +import { BackgroundLoadError } from "../wallpaper"; import type { ExportProgress } from "./types"; import { VideoExporter } from "./videoExporter"; @@ -40,4 +41,42 @@ describe("VideoExporter (real browser)", () => { expect(finalizing.length).toBeGreaterThan(0); expect(finalizing.at(-1)!.percentage).toBe(100); }); + + it("exports successfully with an image wallpaper (served by Vite dev server)", async () => { + const exporter = new VideoExporter({ + videoUrl: sampleVideoUrl, + width: 320, + height: 180, + frameRate: 15, + bitrate: 1_000_000, + wallpaper: "/wallpapers/wallpaper1.jpg", + zoomRegions: [], + showShadow: false, + shadowIntensity: 0, + showBlur: false, + cropRegion: { x: 0, y: 0, width: 1, height: 1 }, + }); + + const result = await exporter.export(); + expect(result.success, result.error).toBe(true); + expect(result.blob!.size).toBeGreaterThan(1024); + }); + + it("throws BackgroundLoadError when wallpaper fails to load (no silent black fallback)", async () => { + const exporter = new VideoExporter({ + videoUrl: sampleVideoUrl, + width: 320, + height: 180, + frameRate: 15, + bitrate: 1_000_000, + wallpaper: "/wallpapers/does-not-exist.jpg", + zoomRegions: [], + showShadow: false, + shadowIntensity: 0, + showBlur: false, + cropRegion: { x: 0, y: 0, width: 1, height: 1 }, + }); + + await expect(exporter.export()).rejects.toBeInstanceOf(BackgroundLoadError); + }); }); diff --git a/src/lib/exporter/videoExporter.ts b/src/lib/exporter/videoExporter.ts index 44c1b88..0ea1ec6 100644 --- a/src/lib/exporter/videoExporter.ts +++ b/src/lib/exporter/videoExporter.ts @@ -7,6 +7,7 @@ import type { WebcamSizePreset, ZoomRegion, } from "@/components/video-editor/types"; +import { BackgroundLoadError } from "@/lib/wallpaper"; import { getPlatform } from "@/utils/platformUtils"; import { AsyncVideoFrameQueue } from "./asyncVideoFrameQueue"; import { AudioProcessor } from "./audioEncoder"; @@ -82,6 +83,11 @@ export class VideoExporter { return { success: false, error: "Export cancelled" }; } + if (normalizedError instanceof BackgroundLoadError) { + this.cleanup(); + throw normalizedError; + } + if (encoderPreferences.length > 1) { console.warn( `[VideoExporter] ${encoderPreference} export attempt failed:`, diff --git a/src/lib/wallpaper.test.ts b/src/lib/wallpaper.test.ts new file mode 100644 index 0000000..dbb5e3c --- /dev/null +++ b/src/lib/wallpaper.test.ts @@ -0,0 +1,167 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + BackgroundLoadError, + classifyWallpaper, + DEFAULT_WALLPAPER, + resolveImageWallpaperUrl, +} from "./wallpaper"; + +describe("classifyWallpaper", () => { + it("classifies hex color", () => { + expect(classifyWallpaper("#1a1a2e")).toEqual({ kind: "color", value: "#1a1a2e" }); + }); + + it("classifies linear gradient", () => { + const value = "linear-gradient(90deg, red, blue)"; + expect(classifyWallpaper(value)).toEqual({ kind: "gradient", value }); + }); + + it("classifies radial gradient", () => { + const value = "radial-gradient(circle, red, blue)"; + expect(classifyWallpaper(value)).toEqual({ kind: "gradient", value }); + }); + + it("classifies leading-slash image path", () => { + expect(classifyWallpaper("/wallpapers/wallpaper1.jpg")).toEqual({ + kind: "image", + path: "/wallpapers/wallpaper1.jpg", + }); + }); + + it("classifies http URL as image", () => { + expect(classifyWallpaper("https://example.com/bg.jpg")).toEqual({ + kind: "image", + path: "https://example.com/bg.jpg", + }); + }); + + it("classifies file:// URL as image", () => { + expect(classifyWallpaper("file:///tmp/bg.jpg")).toEqual({ + kind: "image", + path: "file:///tmp/bg.jpg", + }); + }); + + it("classifies data URI as image", () => { + expect(classifyWallpaper("data:image/png;base64,AAA")).toEqual({ + kind: "image", + path: "data:image/png;base64,AAA", + }); + }); + + it("DEFAULT_WALLPAPER classifies as image", () => { + expect(classifyWallpaper(DEFAULT_WALLPAPER)).toEqual({ + kind: "image", + path: DEFAULT_WALLPAPER, + }); + }); +}); + +describe("resolveImageWallpaperUrl", () => { + beforeEach(() => { + vi.stubGlobal("window", { + ...globalThis.window, + location: { protocol: "http:" }, + electronAPI: undefined, + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("passes through http URL unchanged", async () => { + expect(await resolveImageWallpaperUrl("http://example.com/bg.jpg")).toBe( + "http://example.com/bg.jpg", + ); + }); + + it("passes through https URL unchanged", async () => { + expect(await resolveImageWallpaperUrl("https://example.com/bg.jpg")).toBe( + "https://example.com/bg.jpg", + ); + }); + + it("passes through file:// URL unchanged", async () => { + expect(await resolveImageWallpaperUrl("file:///tmp/bg.jpg")).toBe("file:///tmp/bg.jpg"); + }); + + it("passes through data URI unchanged", async () => { + const uri = "data:image/png;base64,AAAA"; + expect(await resolveImageWallpaperUrl(uri)).toBe(uri); + }); + + it("resolves leading-slash path via http dev server fallback", async () => { + expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( + "/wallpapers/wallpaper1.jpg", + ); + }); + + it("resolves bare relative path via http dev server fallback", async () => { + expect(await resolveImageWallpaperUrl("wallpapers/wallpaper1.jpg")).toBe( + "/wallpapers/wallpaper1.jpg", + ); + }); + + it("encodes path segments with special characters", async () => { + expect(await resolveImageWallpaperUrl("/wallpapers/my image.jpg")).toBe( + "/wallpapers/my%20image.jpg", + ); + }); + + it("resolves via electronAPI when not http protocol", async () => { + vi.stubGlobal("window", { + ...globalThis.window, + location: { protocol: "file:" }, + electronAPI: { + getAssetBasePath: vi.fn().mockResolvedValue("file:///opt/app/public/"), + }, + }); + expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( + "file:///opt/app/public/wallpapers/wallpaper1.jpg", + ); + }); + + it("electronAPI branch appends trailing slash to base if missing", async () => { + vi.stubGlobal("window", { + ...globalThis.window, + location: { protocol: "file:" }, + electronAPI: { + getAssetBasePath: vi.fn().mockResolvedValue("file:///opt/app/public"), + }, + }); + expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( + "file:///opt/app/public/wallpapers/wallpaper1.jpg", + ); + }); + + it("falls back to leading-slash relative when electronAPI returns null", async () => { + vi.stubGlobal("window", { + ...globalThis.window, + location: { protocol: "file:" }, + electronAPI: { + getAssetBasePath: vi.fn().mockResolvedValue(null), + }, + }); + expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( + "/wallpapers/wallpaper1.jpg", + ); + }); +}); + +describe("BackgroundLoadError", () => { + it("carries the failing URL and is instanceof Error", () => { + const err = new BackgroundLoadError("file:///missing.jpg"); + expect(err).toBeInstanceOf(Error); + expect(err).toBeInstanceOf(BackgroundLoadError); + expect(err.url).toBe("file:///missing.jpg"); + expect(err.name).toBe("BackgroundLoadError"); + expect(err.message).toContain("file:///missing.jpg"); + }); + + it("preserves cause when provided", () => { + const cause = new Error("inner"); + const err = new BackgroundLoadError("file:///missing.jpg", cause); + expect(err.cause).toBe(cause); + }); +}); diff --git a/src/lib/wallpaper.ts b/src/lib/wallpaper.ts new file mode 100644 index 0000000..f949177 --- /dev/null +++ b/src/lib/wallpaper.ts @@ -0,0 +1,43 @@ +import { getAssetPath } from "@/lib/assetPath"; + +export const DEFAULT_WALLPAPER = "/wallpapers/wallpaper1.jpg"; + +export type WallpaperClassification = + | { kind: "color"; value: string } + | { kind: "gradient"; value: string } + | { kind: "image"; path: string }; + +export function classifyWallpaper(value: string): WallpaperClassification { + if (value.startsWith("#")) { + return { kind: "color", value }; + } + if (value.startsWith("linear-gradient") || value.startsWith("radial-gradient")) { + return { kind: "gradient", value }; + } + return { kind: "image", path: value }; +} + +export async function resolveImageWallpaperUrl(imagePath: string): Promise { + if ( + imagePath.startsWith("http://") || + imagePath.startsWith("https://") || + imagePath.startsWith("file://") || + imagePath.startsWith("data:") + ) { + return imagePath; + } + const relative = imagePath.replace(/^\/+/, ""); + return getAssetPath(relative); +} + +export class BackgroundLoadError extends Error { + readonly url: string; + readonly cause?: unknown; + + constructor(url: string, cause?: unknown) { + super(`Failed to load background image: ${url}`); + this.name = "BackgroundLoadError"; + this.url = url; + this.cause = cause; + } +} From adf3855ac89d188c8b94ffc9c7dd6f4779460abd Mon Sep 17 00:00:00 2001 From: Enriquefft Date: Fri, 24 Apr 2026 18:16:57 -0500 Subject: [PATCH 2/8] harden wallpaper resolver against traversal, PII, and SSOT drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review surfaced four defects and four drive-bys. All applied: B1 (security, MEDIUM) — Path traversal via encodeRelativeAssetPath. encodeURIComponent passed "." and ".." through unchanged; percent-encoded "%2e%2e" got decoded by the URL constructor. Either form escaped the asset root: new URL("../../etc/passwd", "file:///opt/Openscreen/resources/") → file:///opt/etc/passwd. Reject both at src/lib/assetPath.ts via a new UnsafeAssetPathError thrown when a decoded segment equals "." or "..". B2 (correctness) — classifyWallpaper returned { kind: "image" } for conic-gradient(...), rgb(...), hsl(...), oklch(...), empty string, and named colors like "red". Old frameRenderer's bare fillStyle = value handled these; new code would throw BackgroundLoadError with misleading message. Classification now anchors on regexes, accepts all CSS color functions and all three gradient types, treats unknown strings as fallthrough color (old behavior), and normalizes "" to "#000000". B3 (SSOT) — DEFAULT_WALLPAPER, projectPersistence.WALLPAPER_PATHS, and SettingsPanel.WALLPAPER_RELATIVE independently hardcoded the same /wallpapers/wallpaperN.jpg pattern. Three drift sites collapse into one: WALLPAPER_PATHS lives in src/lib/wallpaper.ts, DEFAULT_WALLPAPER derives from WALLPAPER_PATHS[0], projectPersistence re-exports from the canonical module, SettingsPanel imports it directly. B4 (privacy) — BackgroundLoadError.message and the translated toast surfaced full file paths like file:///home//…/wallpaper.jpg — leaks the user's home directory in copy-pasted bug reports. Added a displayUrl getter that returns just the basename (or "data:…" for data URIs), wired into the toast. Full URL remains in console.error and error.url for debugging. N1 — resolveImageWallpaperUrl now rejects image paths that don't live under /wallpapers/ (throws BackgroundLoadError). Narrows the blast radius of the returned / base so the renderer can only request files within the wallpapers directory, regardless of what the project JSON claims. N2 — videoExporter retry loop no longer calls cleanup() twice in the BackgroundLoadError branch; the finally handles it. N3 — Browser tests assert BackgroundLoadError.url contains the failing path. Guards the {{url}} i18n interpolation contract. N4 — VideoPlayback wallpaper resolve effect now catches resolver throws (UnsafeAssetPathError, BackgroundLoadError from /wallpapers/ prefix enforcement). Prevents the new strict-rejection logic from silently leaving the preview without a background. Tests: 35 unit tests pass (up from 20); new coverage for all color functions, all gradient types, empty string, named color fallback, whitespace trimming, /wallpapers/ prefix enforcement, traversal rejection, percent-encoded traversal rejection, displayUrl basename and data-URI abbreviation. --- src/components/video-editor/SettingsPanel.tsx | 84 ++++++------ src/components/video-editor/VideoEditor.tsx | 2 +- src/components/video-editor/VideoPlayback.tsx | 9 +- .../video-editor/projectPersistence.ts | 8 +- src/lib/assetPath.ts | 23 +++- src/lib/exporter/gifExporter.browser.test.ts | 6 +- .../exporter/videoExporter.browser.test.ts | 6 +- src/lib/exporter/videoExporter.ts | 1 - src/lib/wallpaper.test.ts | 128 ++++++++++++++---- src/lib/wallpaper.ts | 62 +++++++-- 10 files changed, 235 insertions(+), 94 deletions(-) diff --git a/src/components/video-editor/SettingsPanel.tsx b/src/components/video-editor/SettingsPanel.tsx index 4fb4193..ec5ad0d 100644 --- a/src/components/video-editor/SettingsPanel.tsx +++ b/src/components/video-editor/SettingsPanel.tsx @@ -34,11 +34,11 @@ import { Slider } from "@/components/ui/slider"; import { Switch } from "@/components/ui/switch"; import { Tabs, TabsContent, TabsList, TabsTrigger } from "@/components/ui/tabs"; import { useScopedT } from "@/contexts/I18nContext"; -import { getAssetPath } from "@/lib/assetPath"; import { WEBCAM_LAYOUT_PRESETS } from "@/lib/compositeLayout"; import type { ExportFormat, ExportQuality, GifFrameRate, GifSizePreset } from "@/lib/exporter"; import { GIF_FRAME_RATES, GIF_SIZE_PRESETS } from "@/lib/exporter"; import { cn } from "@/lib/utils"; +import { resolveImageWallpaperUrl, WALLPAPER_PATHS } from "@/lib/wallpaper"; import { type AspectRatio, isPortraitAspectRatio } from "@/utils/aspectRatioUtils"; import { getTestId } from "@/utils/getTestId"; import { AnnotationSettingsPanel } from "./AnnotationSettingsPanel"; @@ -123,11 +123,6 @@ function CustomSpeedInput({ ); } -const WALLPAPER_COUNT = 18; -const WALLPAPER_RELATIVE = Array.from( - { length: WALLPAPER_COUNT }, - (_, i) => `wallpapers/wallpaper${i + 1}.jpg`, -); const GRADIENTS = [ "linear-gradient( 111.6deg, rgba(114,167,232,1) 9.4%, rgba(253,129,82,1) 43.9%, rgba(253,129,82,1) 54.8%, rgba(249,202,86,1) 86.3% )", "linear-gradient(120deg, #d4fc79 0%, #96e6a1 100%)", @@ -334,10 +329,10 @@ export function SettingsPanel({ let mounted = true; (async () => { try { - const resolved = await Promise.all(WALLPAPER_RELATIVE.map((p) => getAssetPath(p))); + const resolved = await Promise.all(WALLPAPER_PATHS.map((p) => resolveImageWallpaperUrl(p))); if (mounted) setWallpaperPaths(resolved); } catch (_err) { - if (mounted) setWallpaperPaths(WALLPAPER_RELATIVE.map((p) => `/${p}`)); + if (mounted) setWallpaperPaths([...WALLPAPER_PATHS]); } })(); return () => { @@ -526,7 +521,7 @@ export function SettingsPanel({ setCustomImages((prev) => prev.filter((img) => img !== imageUrl)); // If the removed image was selected, clear selection if (selected === imageUrl) { - onWallpaperChange(wallpaperPaths[0] || WALLPAPER_RELATIVE[0]); + onWallpaperChange(wallpaperPaths[0] || WALLPAPER_PATHS[0]); } }; @@ -1146,42 +1141,41 @@ export function SettingsPanel({ ); })} - {(wallpaperPaths.length > 0 - ? wallpaperPaths - : WALLPAPER_RELATIVE.map((p) => `/${p}`) - ).map((path) => { - const isSelected = (() => { - if (!selected) return false; - if (selected === path) return true; - try { - const clean = (s: string) => - s.replace(/^file:\/\//, "").replace(/^\//, ""); - if (clean(selected).endsWith(clean(path))) return true; - if (clean(path).endsWith(clean(selected))) return true; - } catch { - // Best-effort comparison; fallback to strict match. - } - return false; - })(); - return ( -
onWallpaperChange(path)} - role="button" - /> - ); - })} + {(wallpaperPaths.length > 0 ? wallpaperPaths : WALLPAPER_PATHS).map( + (path) => { + const isSelected = (() => { + if (!selected) return false; + if (selected === path) return true; + try { + const clean = (s: string) => + s.replace(/^file:\/\//, "").replace(/^\//, ""); + if (clean(selected).endsWith(clean(path))) return true; + if (clean(path).endsWith(clean(selected))) return true; + } catch { + // Best-effort comparison; fallback to strict match. + } + return false; + })(); + return ( +
onWallpaperChange(path)} + role="button" + /> + ); + }, + )}
diff --git a/src/components/video-editor/VideoEditor.tsx b/src/components/video-editor/VideoEditor.tsx index 3694d9e..0a03bf1 100644 --- a/src/components/video-editor/VideoEditor.tsx +++ b/src/components/video-editor/VideoEditor.tsx @@ -1568,7 +1568,7 @@ export default function VideoEditor() { } catch (error) { console.error("Export error:", error); if (error instanceof BackgroundLoadError) { - const message = t("errors.exportBackgroundLoadFailed", { url: error.url }); + const message = t("errors.exportBackgroundLoadFailed", { url: error.displayUrl }); setExportError(message); toast.error(message); } else { diff --git a/src/components/video-editor/VideoPlayback.tsx b/src/components/video-editor/VideoPlayback.tsx index db69310..d356012 100644 --- a/src/components/video-editor/VideoPlayback.tsx +++ b/src/components/video-editor/VideoPlayback.tsx @@ -1185,8 +1185,13 @@ const VideoPlayback = forwardRef( if (mounted) setResolvedWallpaper(classified.value); return; } - const resolved = await resolveImageWallpaperUrl(classified.path); - if (mounted) setResolvedWallpaper(resolved); + try { + const resolved = await resolveImageWallpaperUrl(classified.path); + if (mounted) setResolvedWallpaper(resolved); + } catch (err) { + console.warn("[VideoPlayback] wallpaper resolve failed:", err); + if (mounted) setResolvedWallpaper(null); + } })(); return () => { mounted = false; diff --git a/src/components/video-editor/projectPersistence.ts b/src/components/video-editor/projectPersistence.ts index f9640e4..6d8f689 100644 --- a/src/components/video-editor/projectPersistence.ts +++ b/src/components/video-editor/projectPersistence.ts @@ -2,7 +2,7 @@ import { normalizeBlurColor, normalizeBlurType } from "@/lib/blurEffects"; import type { ExportFormat, ExportQuality, GifFrameRate, GifSizePreset } from "@/lib/exporter"; import type { ProjectMedia } from "@/lib/recordingSession"; import { normalizeProjectMedia } from "@/lib/recordingSession"; -import { DEFAULT_WALLPAPER } from "@/lib/wallpaper"; +import { DEFAULT_WALLPAPER, WALLPAPER_PATHS } from "@/lib/wallpaper"; import { ASPECT_RATIOS, type AspectRatio, isPortraitAspectRatio } from "@/utils/aspectRatioUtils"; import { type AnnotationRegion, @@ -38,13 +38,9 @@ import { type ZoomRegion, } from "./types"; -const WALLPAPER_COUNT = 18; const VALID_BLUR_SHAPES = new Set(["rectangle", "oval", "freehand"] as const); -export const WALLPAPER_PATHS = Array.from( - { length: WALLPAPER_COUNT }, - (_, i) => `/wallpapers/wallpaper${i + 1}.jpg`, -); +export { WALLPAPER_PATHS }; export const PROJECT_VERSION = 2; diff --git a/src/lib/assetPath.ts b/src/lib/assetPath.ts index 8188de5..7ba1015 100644 --- a/src/lib/assetPath.ts +++ b/src/lib/assetPath.ts @@ -1,9 +1,22 @@ +export class UnsafeAssetPathError extends Error { + constructor(segment: string) { + super(`Unsafe asset path segment: ${segment}`); + this.name = "UnsafeAssetPathError"; + } +} + function encodeRelativeAssetPath(relativePath: string): string { return relativePath .replace(/^\/+/, "") .split("/") .filter(Boolean) - .map((part) => encodeURIComponent(part)) + .map((part) => { + const decoded = decodeURIComponent(part); + if (decoded === "." || decoded === "..") { + throw new UnsafeAssetPathError(decoded); + } + return encodeURIComponent(decoded); + }) .join("/"); } @@ -16,7 +29,6 @@ export async function getAssetPath(relativePath: string): Promise { try { if (typeof window !== "undefined") { - // If running in a dev server (http/https), prefer the web-served path if ( window.location && window.location.protocol && @@ -32,11 +44,12 @@ export async function getAssetPath(relativePath: string): Promise { } } } - } catch { - // ignore and use fallback + } catch (err) { + if (err instanceof UnsafeAssetPathError) { + throw err; + } } - // Fallback for web/dev server: public/wallpapers are served at '/wallpapers/...' return `/${encodedRelativePath}`; } diff --git a/src/lib/exporter/gifExporter.browser.test.ts b/src/lib/exporter/gifExporter.browser.test.ts index 5a69468..1d96076 100644 --- a/src/lib/exporter/gifExporter.browser.test.ts +++ b/src/lib/exporter/gifExporter.browser.test.ts @@ -79,6 +79,10 @@ describe("GifExporter (real browser)", () => { cropRegion: { x: 0, y: 0, width: 1, height: 1 }, }); - await expect(exporter.export()).rejects.toBeInstanceOf(BackgroundLoadError); + const rejection = exporter.export(); + await expect(rejection).rejects.toBeInstanceOf(BackgroundLoadError); + await expect(rejection).rejects.toMatchObject({ + url: expect.stringContaining("does-not-exist"), + }); }); }); diff --git a/src/lib/exporter/videoExporter.browser.test.ts b/src/lib/exporter/videoExporter.browser.test.ts index ae8c7bc..cca896f 100644 --- a/src/lib/exporter/videoExporter.browser.test.ts +++ b/src/lib/exporter/videoExporter.browser.test.ts @@ -77,6 +77,10 @@ describe("VideoExporter (real browser)", () => { cropRegion: { x: 0, y: 0, width: 1, height: 1 }, }); - await expect(exporter.export()).rejects.toBeInstanceOf(BackgroundLoadError); + const rejection = exporter.export(); + await expect(rejection).rejects.toBeInstanceOf(BackgroundLoadError); + await expect(rejection).rejects.toMatchObject({ + url: expect.stringContaining("does-not-exist"), + }); }); }); diff --git a/src/lib/exporter/videoExporter.ts b/src/lib/exporter/videoExporter.ts index 0ea1ec6..cc8b7cf 100644 --- a/src/lib/exporter/videoExporter.ts +++ b/src/lib/exporter/videoExporter.ts @@ -84,7 +84,6 @@ export class VideoExporter { } if (normalizedError instanceof BackgroundLoadError) { - this.cleanup(); throw normalizedError; } diff --git a/src/lib/wallpaper.test.ts b/src/lib/wallpaper.test.ts index dbb5e3c..f4fe08e 100644 --- a/src/lib/wallpaper.test.ts +++ b/src/lib/wallpaper.test.ts @@ -1,54 +1,109 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { UnsafeAssetPathError } from "./assetPath"; import { BackgroundLoadError, classifyWallpaper, DEFAULT_WALLPAPER, resolveImageWallpaperUrl, + WALLPAPER_COUNT, + WALLPAPER_PATHS, } from "./wallpaper"; +describe("WALLPAPER_PATHS", () => { + it("contains WALLPAPER_COUNT entries", () => { + expect(WALLPAPER_PATHS).toHaveLength(WALLPAPER_COUNT); + }); + + it("DEFAULT_WALLPAPER is WALLPAPER_PATHS[0]", () => { + expect(DEFAULT_WALLPAPER).toBe(WALLPAPER_PATHS[0]); + }); +}); + describe("classifyWallpaper", () => { - it("classifies hex color", () => { + it("hex color", () => { expect(classifyWallpaper("#1a1a2e")).toEqual({ kind: "color", value: "#1a1a2e" }); }); - it("classifies linear gradient", () => { - const value = "linear-gradient(90deg, red, blue)"; - expect(classifyWallpaper(value)).toEqual({ kind: "gradient", value }); + it("rgb() color", () => { + expect(classifyWallpaper("rgb(1, 2, 3)")).toEqual({ kind: "color", value: "rgb(1, 2, 3)" }); }); - it("classifies radial gradient", () => { - const value = "radial-gradient(circle, red, blue)"; - expect(classifyWallpaper(value)).toEqual({ kind: "gradient", value }); + it("rgba() color", () => { + expect(classifyWallpaper("rgba(1, 2, 3, 0.5)")).toEqual({ + kind: "color", + value: "rgba(1, 2, 3, 0.5)", + }); }); - it("classifies leading-slash image path", () => { + it("hsl() color", () => { + expect(classifyWallpaper("hsl(180, 50%, 50%)")).toEqual({ + kind: "color", + value: "hsl(180, 50%, 50%)", + }); + }); + + it("oklch() color", () => { + expect(classifyWallpaper("oklch(50% 0.1 180)")).toEqual({ + kind: "color", + value: "oklch(50% 0.1 180)", + }); + }); + + it("linear gradient", () => { + const v = "linear-gradient(90deg, red, blue)"; + expect(classifyWallpaper(v)).toEqual({ kind: "gradient", value: v }); + }); + + it("radial gradient", () => { + const v = "radial-gradient(circle, red, blue)"; + expect(classifyWallpaper(v)).toEqual({ kind: "gradient", value: v }); + }); + + it("conic gradient", () => { + const v = "conic-gradient(red, blue)"; + expect(classifyWallpaper(v)).toEqual({ kind: "gradient", value: v }); + }); + + it("leading-slash image path", () => { expect(classifyWallpaper("/wallpapers/wallpaper1.jpg")).toEqual({ kind: "image", path: "/wallpapers/wallpaper1.jpg", }); }); - it("classifies http URL as image", () => { + it("http URL as image", () => { expect(classifyWallpaper("https://example.com/bg.jpg")).toEqual({ kind: "image", path: "https://example.com/bg.jpg", }); }); - it("classifies file:// URL as image", () => { + it("file:// URL as image", () => { expect(classifyWallpaper("file:///tmp/bg.jpg")).toEqual({ kind: "image", path: "file:///tmp/bg.jpg", }); }); - it("classifies data URI as image", () => { + it("data URI as image", () => { expect(classifyWallpaper("data:image/png;base64,AAA")).toEqual({ kind: "image", path: "data:image/png;base64,AAA", }); }); + it("named color falls back to color", () => { + expect(classifyWallpaper("red")).toEqual({ kind: "color", value: "red" }); + }); + + it("empty string falls back to black", () => { + expect(classifyWallpaper("")).toEqual({ kind: "color", value: "#000000" }); + }); + + it("trims whitespace", () => { + expect(classifyWallpaper(" #abcdef ")).toEqual({ kind: "color", value: "#abcdef" }); + }); + it("DEFAULT_WALLPAPER classifies as image", () => { expect(classifyWallpaper(DEFAULT_WALLPAPER)).toEqual({ kind: "image", @@ -70,46 +125,64 @@ describe("resolveImageWallpaperUrl", () => { vi.unstubAllGlobals(); }); - it("passes through http URL unchanged", async () => { + it("passes through http URL", async () => { expect(await resolveImageWallpaperUrl("http://example.com/bg.jpg")).toBe( "http://example.com/bg.jpg", ); }); - it("passes through https URL unchanged", async () => { + it("passes through https URL", async () => { expect(await resolveImageWallpaperUrl("https://example.com/bg.jpg")).toBe( "https://example.com/bg.jpg", ); }); - it("passes through file:// URL unchanged", async () => { + it("passes through file:// URL", async () => { expect(await resolveImageWallpaperUrl("file:///tmp/bg.jpg")).toBe("file:///tmp/bg.jpg"); }); - it("passes through data URI unchanged", async () => { + it("passes through data URI", async () => { const uri = "data:image/png;base64,AAAA"; expect(await resolveImageWallpaperUrl(uri)).toBe(uri); }); - it("resolves leading-slash path via http dev server fallback", async () => { + it("resolves leading-slash wallpaper path via http fallback", async () => { expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( "/wallpapers/wallpaper1.jpg", ); }); - it("resolves bare relative path via http dev server fallback", async () => { + it("resolves bare relative wallpaper path", async () => { expect(await resolveImageWallpaperUrl("wallpapers/wallpaper1.jpg")).toBe( "/wallpapers/wallpaper1.jpg", ); }); - it("encodes path segments with special characters", async () => { + it("encodes special characters in path segments", async () => { expect(await resolveImageWallpaperUrl("/wallpapers/my image.jpg")).toBe( "/wallpapers/my%20image.jpg", ); }); - it("resolves via electronAPI when not http protocol", async () => { + it("rejects image paths outside /wallpapers/", async () => { + await expect(resolveImageWallpaperUrl("/etc/passwd")).rejects.toBeInstanceOf( + BackgroundLoadError, + ); + }); + + it("rejects traversal attempts", async () => { + await expect(resolveImageWallpaperUrl("/wallpapers/../etc/passwd")).rejects.toBeInstanceOf( + UnsafeAssetPathError, + ); + }); + + it("rejects percent-encoded traversal", async () => { + await expect(resolveImageWallpaperUrl("/wallpapers/%2e%2e/app.asar")).rejects.toBeInstanceOf( + UnsafeAssetPathError, + ); + }); + + it("resolves via electronAPI when not http", async () => { vi.stubGlobal("window", { ...globalThis.window, location: { protocol: "file:" }, @@ -122,7 +195,7 @@ describe("resolveImageWallpaperUrl", () => { ); }); - it("electronAPI branch appends trailing slash to base if missing", async () => { + it("electronAPI branch appends trailing slash if missing", async () => { vi.stubGlobal("window", { ...globalThis.window, location: { protocol: "file:" }, @@ -151,12 +224,21 @@ describe("resolveImageWallpaperUrl", () => { describe("BackgroundLoadError", () => { it("carries the failing URL and is instanceof Error", () => { - const err = new BackgroundLoadError("file:///missing.jpg"); + const err = new BackgroundLoadError("/home/user/secret/wallpaper.jpg"); expect(err).toBeInstanceOf(Error); expect(err).toBeInstanceOf(BackgroundLoadError); - expect(err.url).toBe("file:///missing.jpg"); + expect(err.url).toBe("/home/user/secret/wallpaper.jpg"); expect(err.name).toBe("BackgroundLoadError"); - expect(err.message).toContain("file:///missing.jpg"); + }); + + it("displayUrl hides parent directories to avoid leaking PII", () => { + const err = new BackgroundLoadError("file:///home/enrique/projects/openscreen/wallpaper1.jpg"); + expect(err.displayUrl).toBe("wallpaper1.jpg"); + }); + + it("displayUrl abbreviates data URIs", () => { + const err = new BackgroundLoadError("data:image/png;base64,AAA"); + expect(err.displayUrl).toBe("data:…"); }); it("preserves cause when provided", () => { diff --git a/src/lib/wallpaper.ts b/src/lib/wallpaper.ts index f949177..449d2e2 100644 --- a/src/lib/wallpaper.ts +++ b/src/lib/wallpaper.ts @@ -1,22 +1,42 @@ import { getAssetPath } from "@/lib/assetPath"; -export const DEFAULT_WALLPAPER = "/wallpapers/wallpaper1.jpg"; +export const WALLPAPER_COUNT = 18; + +export const WALLPAPER_PATHS: readonly string[] = Array.from( + { length: WALLPAPER_COUNT }, + (_, i) => `/wallpapers/wallpaper${i + 1}.jpg`, +); + +export const DEFAULT_WALLPAPER = WALLPAPER_PATHS[0]; export type WallpaperClassification = | { kind: "color"; value: string } | { kind: "gradient"; value: string } | { kind: "image"; path: string }; +const GRADIENT_RE = /^(linear|radial|conic)-gradient\(/; +const COLOR_FUNC_RE = /^(rgb|rgba|hsl|hsla|hwb|lab|lch|oklab|oklch|color)\(/; +const IMAGE_URL_RE = /^(\/|https?:\/\/|file:\/\/|data:)/; + export function classifyWallpaper(value: string): WallpaperClassification { - if (value.startsWith("#")) { - return { kind: "color", value }; + const trimmed = value.trim(); + if (trimmed === "") { + return { kind: "color", value: "#000000" }; } - if (value.startsWith("linear-gradient") || value.startsWith("radial-gradient")) { - return { kind: "gradient", value }; + if (trimmed.startsWith("#") || COLOR_FUNC_RE.test(trimmed)) { + return { kind: "color", value: trimmed }; } - return { kind: "image", path: value }; + if (GRADIENT_RE.test(trimmed)) { + return { kind: "gradient", value: trimmed }; + } + if (IMAGE_URL_RE.test(trimmed)) { + return { kind: "image", path: trimmed }; + } + return { kind: "color", value: trimmed }; } +const ALLOWED_IMAGE_PREFIX = "/wallpapers/"; + export async function resolveImageWallpaperUrl(imagePath: string): Promise { if ( imagePath.startsWith("http://") || @@ -26,8 +46,14 @@ export async function resolveImageWallpaperUrl(imagePath: string): Promise Date: Fri, 24 Apr 2026 18:22:27 -0500 Subject: [PATCH 3/8] avoid 404s on first swatch render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SettingsPanel fell back to rendering WALLPAPER_PATHS (raw /wallpapers/*.jpg strings) during the brief window before the resolveImageWallpaperUrl effect populated wallpaperPaths. In packaged Electron the browser resolved those against a file:// origin, producing 18 ERR_FILE_NOT_FOUND requests per load / reload. The second render replaced them with correct URLs, so swatches appeared — but the wasted requests showed up in devtools and churned the network panel. Drop the fallback; render nothing until the effect completes. The resolution is effectively instant and avoids the empty-origin round trip. --- src/components/video-editor/SettingsPanel.tsx | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/src/components/video-editor/SettingsPanel.tsx b/src/components/video-editor/SettingsPanel.tsx index ec5ad0d..4ebdf33 100644 --- a/src/components/video-editor/SettingsPanel.tsx +++ b/src/components/video-editor/SettingsPanel.tsx @@ -1141,41 +1141,39 @@ export function SettingsPanel({ ); })} - {(wallpaperPaths.length > 0 ? wallpaperPaths : WALLPAPER_PATHS).map( - (path) => { - const isSelected = (() => { - if (!selected) return false; - if (selected === path) return true; - try { - const clean = (s: string) => - s.replace(/^file:\/\//, "").replace(/^\//, ""); - if (clean(selected).endsWith(clean(path))) return true; - if (clean(path).endsWith(clean(selected))) return true; - } catch { - // Best-effort comparison; fallback to strict match. - } - return false; - })(); - return ( -
onWallpaperChange(path)} - role="button" - /> - ); - }, - )} + {wallpaperPaths.map((path) => { + const isSelected = (() => { + if (!selected) return false; + if (selected === path) return true; + try { + const clean = (s: string) => + s.replace(/^file:\/\//, "").replace(/^\//, ""); + if (clean(selected).endsWith(clean(path))) return true; + if (clean(path).endsWith(clean(selected))) return true; + } catch { + // Best-effort comparison; fallback to strict match. + } + return false; + })(); + return ( +
onWallpaperChange(path)} + role="button" + /> + ); + })}
From 702b7330744f1a4f9b66eae95a2d597841381720 Mon Sep 17 00:00:00 2001 From: Enriquefft Date: Fri, 24 Apr 2026 18:33:03 -0500 Subject: [PATCH 4/8] resolve asset base path synchronously from preload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every consumer of /wallpapers/*.jpg — SettingsPanel, VideoPlayback, frameRenderer — was doing async IPC round trips, useEffect dances, and Promise.all for a value that is a build-time constant per process. Each consumer showed briefly-empty or briefly-404ing state on first paint until the handler's reply resolved. The asset base URL depends only on process.defaultApp and process.resourcesPath / __dirname — all available in preload at context-bridge time. Compute once there, expose as a sync string. - preload.ts resolves baseDir (process.resourcesPath packaged, /public unpackaged) and emits assetBaseUrl synchronously. - get-asset-base-path IPC handler + main-process branching deleted. - getAssetPath() is now sync. Returns string, not Promise. Throws AssetBaseUnavailableError (new) when electronAPI.assetBaseUrl is missing — catastrophic preload failure, not silent 404. - resolveImageWallpaperUrl() sync; same sync throw semantics. - SettingsPanel: Promise.all + useState + useEffect collapse to one useMemo. First paint has real URLs, no 18× ERR_FILE_NOT_FOUND, no flicker. - VideoPlayback: wallpaper-resolve useEffect collapses to useMemo. - frameRenderer.setupBackground: drops the await. - electronAPI type decls updated in both .d.ts files. - 35 unit tests updated to reflect sync signature + new AssetBaseUnavailableError contract. Silent-fallback behavior from getAssetPath (returning /relative when electronAPI failed) is gone. Renderers now surface preload failures instead of rendering 404s. --- electron/electron-env.d.ts | 2 +- electron/ipc/handlers.ts | 18 +---- electron/preload.ts | 18 ++++- src/components/video-editor/SettingsPanel.tsx | 19 +---- src/components/video-editor/VideoPlayback.tsx | 34 +++------ src/lib/assetPath.ts | 44 +++++------ src/lib/exporter/frameRenderer.ts | 2 +- src/lib/wallpaper.test.ts | 74 ++++++++----------- src/lib/wallpaper.ts | 2 +- src/vite-env.d.ts | 2 +- 10 files changed, 83 insertions(+), 132 deletions(-) diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index e20cf7f..dff8029 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -37,7 +37,7 @@ interface Window { status: string; error?: string; }>; - getAssetBasePath: () => Promise; + assetBaseUrl: string; storeRecordedVideo: ( videoData: ArrayBuffer, fileName: string, diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 6aec971..2b72241 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1,6 +1,6 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { fileURLToPath, pathToFileURL } from "node:url"; +import { fileURLToPath } from "node:url"; import { app, BrowserWindow, @@ -801,22 +801,6 @@ export function registerIpcHandlers( } }); - // Return base path for assets so renderer can resolve file:// paths in production. - // Packaged: electron-builder extraResources copies public/wallpapers -> resources/wallpapers. - // Unpackaged: wallpapers live at /public/wallpapers. - // Single convention: "/wallpapers/x.jpg" resolves in both modes. - ipcMain.handle("get-asset-base-path", () => { - try { - const baseDir = app.isPackaged - ? process.resourcesPath - : path.join(app.getAppPath(), "public"); - return pathToFileURL(`${baseDir}${path.sep}`).toString(); - } catch (err) { - console.error("Failed to resolve asset base path:", err); - return null; - } - }); - /** * Handles saving an exported video file. * Shows a save dialog, normalizes the file path for the current OS, diff --git a/electron/preload.ts b/electron/preload.ts index 6aa066f..ec5181c 100644 --- a/electron/preload.ts +++ b/electron/preload.ts @@ -1,17 +1,27 @@ +import path from "node:path"; +import { pathToFileURL } from "node:url"; import { contextBridge, ipcRenderer } from "electron"; import type { RecordingSession, StoreRecordedSessionInput } from "../src/lib/recordingSession"; +// Asset base URL is a build-time constant per process; resolve once here so +// the renderer can consume it synchronously. Packaged: electron-builder +// extraResources copies public/wallpapers -> resources/wallpapers (see +// electron-builder.json5). Unpackaged: wallpapers live at /public/, +// and __dirname in dist-electron resolves to /dist-electron/. +const isPackagedProcess = !process.defaultApp; +const assetBaseDir = isPackagedProcess + ? process.resourcesPath + : path.join(__dirname, "..", "public"); +const assetBaseUrl = pathToFileURL(`${assetBaseDir}${path.sep}`).toString(); + contextBridge.exposeInMainWorld("electronAPI", { + assetBaseUrl, hudOverlayHide: () => { ipcRenderer.send("hud-overlay-hide"); }, hudOverlayClose: () => { ipcRenderer.send("hud-overlay-close"); }, - getAssetBasePath: async () => { - // ask main process for the correct base path (production vs dev) - return await ipcRenderer.invoke("get-asset-base-path"); - }, getSources: async (opts: Electron.SourcesOptions) => { return await ipcRenderer.invoke("get-sources", opts); }, diff --git a/src/components/video-editor/SettingsPanel.tsx b/src/components/video-editor/SettingsPanel.tsx index 4ebdf33..91840bc 100644 --- a/src/components/video-editor/SettingsPanel.tsx +++ b/src/components/video-editor/SettingsPanel.tsx @@ -14,7 +14,7 @@ import { Upload, X, } from "lucide-react"; -import { useCallback, useEffect, useRef, useState } from "react"; +import { useCallback, useMemo, useRef, useState } from "react"; import { toast } from "sonner"; import { Accordion, @@ -321,24 +321,9 @@ export function SettingsPanel({ onWebcamSizePresetCommit, }: SettingsPanelProps) { const t = useScopedT("settings"); - const [wallpaperPaths, setWallpaperPaths] = useState([]); + const wallpaperPaths = useMemo(() => WALLPAPER_PATHS.map(resolveImageWallpaperUrl), []); const [customImages, setCustomImages] = useState([]); const fileInputRef = useRef(null); - - useEffect(() => { - let mounted = true; - (async () => { - try { - const resolved = await Promise.all(WALLPAPER_PATHS.map((p) => resolveImageWallpaperUrl(p))); - if (mounted) setWallpaperPaths(resolved); - } catch (_err) { - if (mounted) setWallpaperPaths([...WALLPAPER_PATHS]); - } - })(); - return () => { - mounted = false; - }; - }, []); const colorPalette = [ "#FF0000", "#FFD700", diff --git a/src/components/video-editor/VideoPlayback.tsx b/src/components/video-editor/VideoPlayback.tsx index d356012..89a0a6c 100644 --- a/src/components/video-editor/VideoPlayback.tsx +++ b/src/components/video-editor/VideoPlayback.tsx @@ -1108,7 +1108,17 @@ const VideoPlayback = forwardRef( videoReadyRafRef.current = requestAnimationFrame(waitForRenderableFrame); }; - const [resolvedWallpaper, setResolvedWallpaper] = useState(null); + const resolvedWallpaper = useMemo(() => { + const source = wallpaper || DEFAULT_WALLPAPER; + const classified = classifyWallpaper(source); + if (classified.kind !== "image") return classified.value; + try { + return resolveImageWallpaperUrl(classified.path); + } catch (err) { + console.warn("[VideoPlayback] wallpaper resolve failed:", err); + return null; + } + }, [wallpaper]); const webcamCssBoxShadow = useMemo( () => getWebcamLayoutCssBoxShadow(webcamLayoutPreset), [webcamLayoutPreset], @@ -1176,28 +1186,6 @@ const VideoPlayback = forwardRef( webcamVideo.currentTime = 0; }, [webcamVideoPath]); - useEffect(() => { - let mounted = true; - (async () => { - const source = wallpaper || DEFAULT_WALLPAPER; - const classified = classifyWallpaper(source); - if (classified.kind !== "image") { - if (mounted) setResolvedWallpaper(classified.value); - return; - } - try { - const resolved = await resolveImageWallpaperUrl(classified.path); - if (mounted) setResolvedWallpaper(resolved); - } catch (err) { - console.warn("[VideoPlayback] wallpaper resolve failed:", err); - if (mounted) setResolvedWallpaper(null); - } - })(); - return () => { - mounted = false; - }; - }, [wallpaper]); - useEffect(() => { return () => { if (videoReadyRafRef.current) { diff --git a/src/lib/assetPath.ts b/src/lib/assetPath.ts index 7ba1015..edba758 100644 --- a/src/lib/assetPath.ts +++ b/src/lib/assetPath.ts @@ -5,6 +5,13 @@ export class UnsafeAssetPathError extends Error { } } +export class AssetBaseUnavailableError extends Error { + constructor() { + super("electronAPI.assetBaseUrl is not available; preload did not load correctly"); + this.name = "AssetBaseUnavailableError"; + } +} + function encodeRelativeAssetPath(relativePath: string): string { return relativePath .replace(/^\/+/, "") @@ -24,33 +31,22 @@ function ensureTrailingSlash(value: string): string { return value.endsWith("/") ? value : `${value}/`; } -export async function getAssetPath(relativePath: string): Promise { - const encodedRelativePath = encodeRelativeAssetPath(relativePath); +export function getAssetPath(relativePath: string): string { + const encoded = encodeRelativeAssetPath(relativePath); - try { - if (typeof window !== "undefined") { - if ( - window.location && - window.location.protocol && - window.location.protocol.startsWith("http") - ) { - return `/${encodedRelativePath}`; - } - - if (window.electronAPI && typeof window.electronAPI.getAssetBasePath === "function") { - const base = await window.electronAPI.getAssetBasePath(); - if (base) { - return new URL(encodedRelativePath, ensureTrailingSlash(base)).toString(); - } - } - } - } catch (err) { - if (err instanceof UnsafeAssetPathError) { - throw err; - } + if (typeof window === "undefined") { + return `/${encoded}`; } - return `/${encodedRelativePath}`; + if (window.location?.protocol?.startsWith("http")) { + return `/${encoded}`; + } + + const base = window.electronAPI?.assetBaseUrl; + if (!base) { + throw new AssetBaseUnavailableError(); + } + return new URL(encoded, ensureTrailingSlash(base)).toString(); } export default getAssetPath; diff --git a/src/lib/exporter/frameRenderer.ts b/src/lib/exporter/frameRenderer.ts index e7362eb..20a2b2d 100644 --- a/src/lib/exporter/frameRenderer.ts +++ b/src/lib/exporter/frameRenderer.ts @@ -280,7 +280,7 @@ export class FrameRenderer { bgCtx.fillStyle = gradient; bgCtx.fillRect(0, 0, this.config.width, this.config.height); } else { - const imageUrl = await resolveImageWallpaperUrl(classified.path); + const imageUrl = resolveImageWallpaperUrl(classified.path); const img = new Image(); if (imageUrl.startsWith("http") && !imageUrl.startsWith(window.location.origin)) { img.crossOrigin = "anonymous"; diff --git a/src/lib/wallpaper.test.ts b/src/lib/wallpaper.test.ts index f4fe08e..f1cb80b 100644 --- a/src/lib/wallpaper.test.ts +++ b/src/lib/wallpaper.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { UnsafeAssetPathError } from "./assetPath"; +import { AssetBaseUnavailableError, UnsafeAssetPathError } from "./assetPath"; import { BackgroundLoadError, classifyWallpaper, @@ -125,99 +125,87 @@ describe("resolveImageWallpaperUrl", () => { vi.unstubAllGlobals(); }); - it("passes through http URL", async () => { - expect(await resolveImageWallpaperUrl("http://example.com/bg.jpg")).toBe( - "http://example.com/bg.jpg", - ); + it("passes through http URL", () => { + expect(resolveImageWallpaperUrl("http://example.com/bg.jpg")).toBe("http://example.com/bg.jpg"); }); - it("passes through https URL", async () => { - expect(await resolveImageWallpaperUrl("https://example.com/bg.jpg")).toBe( + it("passes through https URL", () => { + expect(resolveImageWallpaperUrl("https://example.com/bg.jpg")).toBe( "https://example.com/bg.jpg", ); }); - it("passes through file:// URL", async () => { - expect(await resolveImageWallpaperUrl("file:///tmp/bg.jpg")).toBe("file:///tmp/bg.jpg"); + it("passes through file:// URL", () => { + expect(resolveImageWallpaperUrl("file:///tmp/bg.jpg")).toBe("file:///tmp/bg.jpg"); }); - it("passes through data URI", async () => { + it("passes through data URI", () => { const uri = "data:image/png;base64,AAAA"; - expect(await resolveImageWallpaperUrl(uri)).toBe(uri); + expect(resolveImageWallpaperUrl(uri)).toBe(uri); }); - it("resolves leading-slash wallpaper path via http fallback", async () => { - expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( + it("resolves leading-slash wallpaper path via http fallback", () => { + expect(resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( "/wallpapers/wallpaper1.jpg", ); }); - it("resolves bare relative wallpaper path", async () => { - expect(await resolveImageWallpaperUrl("wallpapers/wallpaper1.jpg")).toBe( + it("resolves bare relative wallpaper path", () => { + expect(resolveImageWallpaperUrl("wallpapers/wallpaper1.jpg")).toBe( "/wallpapers/wallpaper1.jpg", ); }); - it("encodes special characters in path segments", async () => { - expect(await resolveImageWallpaperUrl("/wallpapers/my image.jpg")).toBe( - "/wallpapers/my%20image.jpg", - ); + it("encodes special characters in path segments", () => { + expect(resolveImageWallpaperUrl("/wallpapers/my image.jpg")).toBe("/wallpapers/my%20image.jpg"); }); - it("rejects image paths outside /wallpapers/", async () => { - await expect(resolveImageWallpaperUrl("/etc/passwd")).rejects.toBeInstanceOf( - BackgroundLoadError, - ); + it("rejects image paths outside /wallpapers/", () => { + expect(() => resolveImageWallpaperUrl("/etc/passwd")).toThrow(BackgroundLoadError); }); - it("rejects traversal attempts", async () => { - await expect(resolveImageWallpaperUrl("/wallpapers/../etc/passwd")).rejects.toBeInstanceOf( + it("rejects traversal attempts", () => { + expect(() => resolveImageWallpaperUrl("/wallpapers/../etc/passwd")).toThrow( UnsafeAssetPathError, ); }); - it("rejects percent-encoded traversal", async () => { - await expect(resolveImageWallpaperUrl("/wallpapers/%2e%2e/app.asar")).rejects.toBeInstanceOf( + it("rejects percent-encoded traversal", () => { + expect(() => resolveImageWallpaperUrl("/wallpapers/%2e%2e/app.asar")).toThrow( UnsafeAssetPathError, ); }); - it("resolves via electronAPI when not http", async () => { + it("resolves via electronAPI.assetBaseUrl when not http", () => { vi.stubGlobal("window", { ...globalThis.window, location: { protocol: "file:" }, - electronAPI: { - getAssetBasePath: vi.fn().mockResolvedValue("file:///opt/app/public/"), - }, + electronAPI: { assetBaseUrl: "file:///opt/app/public/" }, }); - expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( + expect(resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( "file:///opt/app/public/wallpapers/wallpaper1.jpg", ); }); - it("electronAPI branch appends trailing slash if missing", async () => { + it("appends trailing slash to assetBaseUrl if missing", () => { vi.stubGlobal("window", { ...globalThis.window, location: { protocol: "file:" }, - electronAPI: { - getAssetBasePath: vi.fn().mockResolvedValue("file:///opt/app/public"), - }, + electronAPI: { assetBaseUrl: "file:///opt/app/public" }, }); - expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( + expect(resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( "file:///opt/app/public/wallpapers/wallpaper1.jpg", ); }); - it("falls back to leading-slash relative when electronAPI returns null", async () => { + it("throws loudly when assetBaseUrl is empty (no silent fallback)", () => { vi.stubGlobal("window", { ...globalThis.window, location: { protocol: "file:" }, - electronAPI: { - getAssetBasePath: vi.fn().mockResolvedValue(null), - }, + electronAPI: { assetBaseUrl: "" }, }); - expect(await resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toBe( - "/wallpapers/wallpaper1.jpg", + expect(() => resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toThrow( + AssetBaseUnavailableError, ); }); }); diff --git a/src/lib/wallpaper.ts b/src/lib/wallpaper.ts index 449d2e2..c86ce43 100644 --- a/src/lib/wallpaper.ts +++ b/src/lib/wallpaper.ts @@ -37,7 +37,7 @@ export function classifyWallpaper(value: string): WallpaperClassification { const ALLOWED_IMAGE_PREFIX = "/wallpapers/"; -export async function resolveImageWallpaperUrl(imagePath: string): Promise { +export function resolveImageWallpaperUrl(imagePath: string): string { if ( imagePath.startsWith("http://") || imagePath.startsWith("https://") || diff --git a/src/vite-env.d.ts b/src/vite-env.d.ts index d76ee15..bdcb537 100644 --- a/src/vite-env.d.ts +++ b/src/vite-env.d.ts @@ -55,7 +55,7 @@ interface Window { message?: string; error?: string; }>; - getAssetBasePath: () => Promise; + assetBaseUrl: string; setRecordingState: (recording: boolean) => Promise; getCursorTelemetry: (videoPath?: string) => Promise<{ success: boolean; From f2ff7fb21cf38b018135f82da9f96692d645ae5c Mon Sep 17 00:00:00 2001 From: Enriquefft Date: Fri, 24 Apr 2026 18:55:04 -0500 Subject: [PATCH 5/8] address review audit: persist canonical wallpaper, dedupe types, tighten edge cases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R1 — Persisted wallpaper is now always the canonical /wallpapers/wallpaperN.jpg form, never the resolved file:// URL. Swatch clicks pass WALLPAPER_PATHS[i] (the relative path) to onWallpaperChange; the resolved URL stays in wallpaperPreviewUrls for rendering only. This prevents machine-specific paths from being written into project JSON and avoids break-on-upgrade / break-on-share regressions. Legacy projects carrying resolved file:// URLs are rewritten by a new normalizer in normalizeProjectEditor: file://…(/assets)?/wallpapers/wallpaperN.jpg → /wallpapers/wallpaperN.jpg. R2 — resolveImageWallpaperUrl now catches anything getAssetPath throws (UnsafeAssetPathError, AssetBaseUnavailableError) and rewraps as BackgroundLoadError with the original as cause. Callers (videoExporter retry loop, gifExporter catch, VideoEditor toast) only need one instanceof check and users always see the translated errors.exportBackgroundLoadFailed toast. R3 — src/vite-env.d.ts no longer duplicates Window.electronAPI. The interface had drifted — renderer declaration was missing readBinaryFile, getPlatform, revealInFolder, getShortcuts, saveShortcuts, hudOverlay*, countdown overlay methods that electron-env.d.ts already declares. Removed the duplicate and kept the triple-slash reference so the authoritative declaration is the one in electron/electron-env.d.ts. N1 — GRADIENT_RE accepts optional "repeating-" prefix so repeating-linear/radial/conic-gradient values classify as gradients instead of falling through to color. N2 — displayBasename returns "(unknown)" sentinel for URLs without a meaningful basename (file:///, bare /) instead of leaking the original string. N3 — electron-builder.json5 extraResources block gets an inline comment pointing at preload.ts:assetBaseDir so the bidirectional coupling is discoverable from either file. Tests: 54 unit tests pass (up from 35). New coverage for repeating gradients, displayBasename sentinels, BackgroundLoadError cause wrapping, legacy file:// wallpaper normalization (5 cases). --- electron-builder.json5 | 2 + src/components/video-editor/SettingsPanel.tsx | 30 ++--- .../video-editor/projectPersistence.test.ts | 40 ++++++ .../video-editor/projectPersistence.ts | 16 ++- src/lib/wallpaper.test.ts | 56 ++++++-- src/lib/wallpaper.ts | 12 +- src/vite-env.d.ts | 124 ------------------ 7 files changed, 120 insertions(+), 160 deletions(-) diff --git a/electron-builder.json5 b/electron-builder.json5 index 441eda4..ca053ef 100644 --- a/electron-builder.json5 +++ b/electron-builder.json5 @@ -20,6 +20,8 @@ "!CONTRIBUTING.md", "!LICENSE" ], + // Asset layout contract: "wallpapers/" under resourcesPath must align with + // assetBaseDir in electron/preload.ts (packaged branch). "extraResources": [ { "from": "public/wallpapers", diff --git a/src/components/video-editor/SettingsPanel.tsx b/src/components/video-editor/SettingsPanel.tsx index 91840bc..78d6bb4 100644 --- a/src/components/video-editor/SettingsPanel.tsx +++ b/src/components/video-editor/SettingsPanel.tsx @@ -321,7 +321,10 @@ export function SettingsPanel({ onWebcamSizePresetCommit, }: SettingsPanelProps) { const t = useScopedT("settings"); - const wallpaperPaths = useMemo(() => WALLPAPER_PATHS.map(resolveImageWallpaperUrl), []); + // Resolved URLs are for DOM rendering only (backgroundImage). The canonical + // `/wallpapers/wallpaperN.jpg` form in WALLPAPER_PATHS is what gets persisted + // on click — never the machine-specific file:// URL. + const wallpaperPreviewUrls = useMemo(() => WALLPAPER_PATHS.map(resolveImageWallpaperUrl), []); const [customImages, setCustomImages] = useState([]); const fileInputRef = useRef(null); const colorPalette = [ @@ -506,7 +509,7 @@ export function SettingsPanel({ setCustomImages((prev) => prev.filter((img) => img !== imageUrl)); // If the removed image was selected, clear selection if (selected === imageUrl) { - onWallpaperChange(wallpaperPaths[0] || WALLPAPER_PATHS[0]); + onWallpaperChange(WALLPAPER_PATHS[0]); } }; @@ -1126,23 +1129,12 @@ export function SettingsPanel({ ); })} - {wallpaperPaths.map((path) => { - const isSelected = (() => { - if (!selected) return false; - if (selected === path) return true; - try { - const clean = (s: string) => - s.replace(/^file:\/\//, "").replace(/^\//, ""); - if (clean(selected).endsWith(clean(path))) return true; - if (clean(path).endsWith(clean(selected))) return true; - } catch { - // Best-effort comparison; fallback to strict match. - } - return false; - })(); + {WALLPAPER_PATHS.map((canonicalPath, i) => { + const previewUrl = wallpaperPreviewUrls[i] ?? canonicalPath; + const isSelected = selected === canonicalPath; return (
onWallpaperChange(path)} + onClick={() => onWallpaperChange(canonicalPath)} role="button" /> ); diff --git a/src/components/video-editor/projectPersistence.test.ts b/src/components/video-editor/projectPersistence.test.ts index 14dc240..d816f48 100644 --- a/src/components/video-editor/projectPersistence.test.ts +++ b/src/components/video-editor/projectPersistence.test.ts @@ -197,3 +197,43 @@ it("detects unsaved changes from differing snapshots", () => { expect(hasProjectUnsavedChanges("same", "same")).toBe(false); expect(hasProjectUnsavedChanges("current", "baseline")).toBe(true); }); + +describe("wallpaper legacy normalization", () => { + it("rewrites resolved file:// resources paths from pre-fix projects", () => { + const normalized = normalizeProjectEditor({ + wallpaper: "file:///opt/Openscreen/resources/assets/wallpapers/wallpaper5.jpg", + }); + expect(normalized.wallpaper).toBe("/wallpapers/wallpaper5.jpg"); + }); + + it("rewrites resolved file:// paths under the new resources/wallpapers layout", () => { + const normalized = normalizeProjectEditor({ + wallpaper: "file:///opt/Openscreen/resources/wallpapers/wallpaper3.jpg", + }); + expect(normalized.wallpaper).toBe("/wallpapers/wallpaper3.jpg"); + }); + + it("rewrites unpackaged dev paths (public/wallpapers/…)", () => { + const normalized = normalizeProjectEditor({ + wallpaper: "file:///home/user/project/public/wallpapers/wallpaper1.jpg", + }); + expect(normalized.wallpaper).toBe("/wallpapers/wallpaper1.jpg"); + }); + + it("leaves canonical relative paths untouched", () => { + const normalized = normalizeProjectEditor({ wallpaper: "/wallpapers/wallpaper2.jpg" }); + expect(normalized.wallpaper).toBe("/wallpapers/wallpaper2.jpg"); + }); + + it("leaves data URIs untouched", () => { + const dataUri = "data:image/png;base64,AAA"; + expect(normalizeProjectEditor({ wallpaper: dataUri }).wallpaper).toBe(dataUri); + }); + + it("leaves colors and gradients untouched", () => { + expect(normalizeProjectEditor({ wallpaper: "#1a1a2e" }).wallpaper).toBe("#1a1a2e"); + expect( + normalizeProjectEditor({ wallpaper: "linear-gradient(90deg, red, blue)" }).wallpaper, + ).toBe("linear-gradient(90deg, red, blue)"); + }); +}); diff --git a/src/components/video-editor/projectPersistence.ts b/src/components/video-editor/projectPersistence.ts index 6d8f689..c0def97 100644 --- a/src/components/video-editor/projectPersistence.ts +++ b/src/components/video-editor/projectPersistence.ts @@ -40,6 +40,17 @@ import { const VALID_BLUR_SHAPES = new Set(["rectangle", "oval", "freehand"] as const); +// Pre-fix projects could persist resolved file:// URLs (machine-specific) instead +// of the canonical `/wallpapers/wallpaperN.jpg` form. Rewrite those on load so +// they resolve against the current install's resources directory. +const LEGACY_FILE_WALLPAPER_RE = /^file:\/\/.*?\/(?:assets\/)?wallpapers\/(wallpaper\d+\.jpg)$/i; + +function normalizeWallpaperValue(value: string): string { + const match = LEGACY_FILE_WALLPAPER_RE.exec(value); + if (!match) return value; + return `/wallpapers/${match[1]}`; +} + export { WALLPAPER_PATHS }; export const PROJECT_VERSION = 2; @@ -422,7 +433,10 @@ export function normalizeProjectEditor(editor: Partial): Pro const cropHeight = clamp(rawCropHeight, 0.01, 1 - cropY); return { - wallpaper: typeof editor.wallpaper === "string" ? editor.wallpaper : DEFAULT_WALLPAPER, + wallpaper: + typeof editor.wallpaper === "string" + ? normalizeWallpaperValue(editor.wallpaper) + : DEFAULT_WALLPAPER, shadowIntensity: typeof editor.shadowIntensity === "number" ? editor.shadowIntensity : 0, showBlur: typeof editor.showBlur === "boolean" ? editor.showBlur : false, motionBlurAmount: isFiniteNumber(editor.motionBlurAmount) diff --git a/src/lib/wallpaper.test.ts b/src/lib/wallpaper.test.ts index f1cb80b..6e1b74a 100644 --- a/src/lib/wallpaper.test.ts +++ b/src/lib/wallpaper.test.ts @@ -64,6 +64,16 @@ describe("classifyWallpaper", () => { expect(classifyWallpaper(v)).toEqual({ kind: "gradient", value: v }); }); + it("repeating-linear gradient", () => { + const v = "repeating-linear-gradient(45deg, red 0 10px, blue 10px 20px)"; + expect(classifyWallpaper(v)).toEqual({ kind: "gradient", value: v }); + }); + + it("repeating-radial gradient", () => { + const v = "repeating-radial-gradient(circle, red, blue 20px)"; + expect(classifyWallpaper(v)).toEqual({ kind: "gradient", value: v }); + }); + it("leading-slash image path", () => { expect(classifyWallpaper("/wallpapers/wallpaper1.jpg")).toEqual({ kind: "image", @@ -164,16 +174,24 @@ describe("resolveImageWallpaperUrl", () => { expect(() => resolveImageWallpaperUrl("/etc/passwd")).toThrow(BackgroundLoadError); }); - it("rejects traversal attempts", () => { - expect(() => resolveImageWallpaperUrl("/wallpapers/../etc/passwd")).toThrow( - UnsafeAssetPathError, - ); + it("wraps traversal attempts in BackgroundLoadError (preserves UnsafeAssetPathError as cause)", () => { + try { + resolveImageWallpaperUrl("/wallpapers/../etc/passwd"); + expect.fail("should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(BackgroundLoadError); + expect((err as BackgroundLoadError).cause).toBeInstanceOf(UnsafeAssetPathError); + } }); - it("rejects percent-encoded traversal", () => { - expect(() => resolveImageWallpaperUrl("/wallpapers/%2e%2e/app.asar")).toThrow( - UnsafeAssetPathError, - ); + it("wraps percent-encoded traversal in BackgroundLoadError", () => { + try { + resolveImageWallpaperUrl("/wallpapers/%2e%2e/app.asar"); + expect.fail("should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(BackgroundLoadError); + expect((err as BackgroundLoadError).cause).toBeInstanceOf(UnsafeAssetPathError); + } }); it("resolves via electronAPI.assetBaseUrl when not http", () => { @@ -198,15 +216,19 @@ describe("resolveImageWallpaperUrl", () => { ); }); - it("throws loudly when assetBaseUrl is empty (no silent fallback)", () => { + it("wraps AssetBaseUnavailableError in BackgroundLoadError when assetBaseUrl is empty", () => { vi.stubGlobal("window", { ...globalThis.window, location: { protocol: "file:" }, electronAPI: { assetBaseUrl: "" }, }); - expect(() => resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg")).toThrow( - AssetBaseUnavailableError, - ); + try { + resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg"); + expect.fail("should have thrown"); + } catch (err) { + expect(err).toBeInstanceOf(BackgroundLoadError); + expect((err as BackgroundLoadError).cause).toBeInstanceOf(AssetBaseUnavailableError); + } }); }); @@ -229,6 +251,16 @@ describe("BackgroundLoadError", () => { expect(err.displayUrl).toBe("data:…"); }); + it("displayUrl returns sentinel for empty-basename URLs", () => { + const err = new BackgroundLoadError("file:///"); + expect(err.displayUrl).toBe("(unknown)"); + }); + + it("displayUrl returns sentinel for unparseable bare slash", () => { + const err = new BackgroundLoadError("/"); + expect(err.displayUrl).toBe("(unknown)"); + }); + it("preserves cause when provided", () => { const cause = new Error("inner"); const err = new BackgroundLoadError("file:///missing.jpg", cause); diff --git a/src/lib/wallpaper.ts b/src/lib/wallpaper.ts index c86ce43..34869d7 100644 --- a/src/lib/wallpaper.ts +++ b/src/lib/wallpaper.ts @@ -14,7 +14,7 @@ export type WallpaperClassification = | { kind: "gradient"; value: string } | { kind: "image"; path: string }; -const GRADIENT_RE = /^(linear|radial|conic)-gradient\(/; +const GRADIENT_RE = /^(repeating-)?(linear|radial|conic)-gradient\(/; const COLOR_FUNC_RE = /^(rgb|rgba|hsl|hsla|hwb|lab|lch|oklab|oklch|color)\(/; const IMAGE_URL_RE = /^(\/|https?:\/\/|file:\/\/|data:)/; @@ -53,7 +53,11 @@ export function resolveImageWallpaperUrl(imagePath: string): string { new Error(`Image wallpaper path must live under ${ALLOWED_IMAGE_PREFIX}`), ); } - return getAssetPath(withLeadingSlash.slice(1)); + try { + return getAssetPath(withLeadingSlash.slice(1)); + } catch (cause) { + throw new BackgroundLoadError(imagePath, cause); + } } export class BackgroundLoadError extends Error { @@ -79,9 +83,9 @@ function displayBasename(url: string): string { try { const parsed = new URL(url); const last = parsed.pathname.split("/").filter(Boolean).pop(); - return last ? decodeURIComponent(last) : url; + return last ? decodeURIComponent(last) : "(unknown)"; } catch { const last = url.split("/").filter(Boolean).pop(); - return last ?? url; + return last ?? "(unknown)"; } } diff --git a/src/vite-env.d.ts b/src/vite-env.d.ts index bdcb537..b7a0735 100644 --- a/src/vite-env.d.ts +++ b/src/vite-env.d.ts @@ -1,126 +1,2 @@ /// /// - -interface ProcessedDesktopSource { - id: string; - name: string; - display_id: string; - thumbnail: string | null; - appIcon: string | null; -} - -interface CursorTelemetryPoint { - timeMs: number; - cx: number; - cy: number; -} - -interface Window { - electronAPI: { - getSources: (opts: Electron.SourcesOptions) => Promise; - switchToEditor: () => Promise; - switchToHud: () => Promise; - startNewRecording: () => Promise<{ success: boolean; error?: string }>; - openSourceSelector: () => Promise; - selectSource: (source: ProcessedDesktopSource) => Promise; - getSelectedSource: () => Promise; - requestCameraAccess: () => Promise<{ - success: boolean; - granted: boolean; - status: string; - error?: string; - }>; - storeRecordedVideo: ( - videoData: ArrayBuffer, - fileName: string, - ) => Promise<{ - success: boolean; - path?: string; - session?: import("./lib/recordingSession").RecordingSession; - message?: string; - error?: string; - }>; - storeRecordedSession: ( - payload: import("./lib/recordingSession").StoreRecordedSessionInput, - ) => Promise<{ - success: boolean; - path?: string; - session?: import("./lib/recordingSession").RecordingSession; - message?: string; - error?: string; - }>; - getRecordedVideoPath: () => Promise<{ - success: boolean; - path?: string; - message?: string; - error?: string; - }>; - assetBaseUrl: string; - setRecordingState: (recording: boolean) => Promise; - getCursorTelemetry: (videoPath?: string) => Promise<{ - success: boolean; - samples: CursorTelemetryPoint[]; - message?: string; - error?: string; - }>; - onStopRecordingFromTray: (callback: () => void) => () => void; - openExternalUrl: (url: string) => Promise<{ success: boolean; error?: string }>; - saveExportedVideo: ( - videoData: ArrayBuffer, - fileName: string, - ) => Promise<{ - success: boolean; - path?: string; - message?: string; - canceled?: boolean; - }>; - openVideoFilePicker: () => Promise<{ success: boolean; path?: string; canceled?: boolean }>; - setCurrentVideoPath: (path: string) => Promise<{ success: boolean }>; - setCurrentRecordingSession: ( - session: import("./lib/recordingSession").RecordingSession | null, - ) => Promise<{ - success: boolean; - session?: import("./lib/recordingSession").RecordingSession; - }>; - getCurrentVideoPath: () => Promise<{ success: boolean; path?: string }>; - getCurrentRecordingSession: () => Promise<{ - success: boolean; - session?: import("./lib/recordingSession").RecordingSession; - }>; - clearCurrentVideoPath: () => Promise<{ success: boolean }>; - saveProjectFile: ( - projectData: unknown, - suggestedName?: string, - existingProjectPath?: string, - ) => Promise<{ - success: boolean; - path?: string; - message?: string; - canceled?: boolean; - error?: string; - }>; - loadProjectFile: () => Promise<{ - success: boolean; - path?: string; - project?: unknown; - message?: string; - canceled?: boolean; - error?: string; - }>; - loadCurrentProjectFile: () => Promise<{ - success: boolean; - path?: string; - project?: unknown; - message?: string; - canceled?: boolean; - error?: string; - }>; - onMenuLoadProject: (callback: () => void) => () => void; - onMenuSaveProject: (callback: () => void) => () => void; - onMenuSaveProjectAs: (callback: () => void) => () => void; - setMicrophoneExpanded: (expanded: boolean) => void; - setHasUnsavedChanges: (hasChanges: boolean) => void; - onRequestSaveBeforeClose: (callback: () => Promise | boolean) => () => void; - setLocale: (locale: string) => Promise; - }; -} From af159e8a2b60d4f4043f6556a4ac5cbba299fd66 Mon Sep 17 00:00:00 2001 From: Enriquefft Date: Fri, 24 Apr 2026 18:58:34 -0500 Subject: [PATCH 6/8] tighten legacy normalizer and guard against BackgroundLoadError double-wrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer audit found two real risks in the prior amendment: 1. LEGACY_FILE_WALLPAPER_RE was too permissive. Any file:// URL containing /wallpapers/wallpaperN.jpg would match — including a user's own file at /home/me/wallpapers/wallpaper1.jpg that happened to share the name pattern. Silent data-loss potential: user's photo replaced with a bundled asset. In-app upload flow uses data: URIs today so it can't actually produce such a value, but the regex should be tight on intent. Now requires a known install-layout segment: resources/[assets/]wallpapers/ (packaged) or public/wallpapers/ (dev). 2. No upper bound on \d+. A corrupted or future-schema project with wallpaper99.jpg was silently rewritten to /wallpapers/wallpaper99.jpg which 404s. Now validates against WALLPAPER_PATHS; out-of-set bundled-looking values fall back to DEFAULT_WALLPAPER. Also applied R2.2 defensive guard: resolveImageWallpaperUrl's catch block now checks instanceof BackgroundLoadError and rethrows unchanged instead of wrapping a second time. Current getAssetPath cannot throw BackgroundLoadError so this is a future-proof against refactors. Tests: 56 pass (up from 54). Added coverage for "user file outside install dir stays untouched" and "bundled-looking but out-of-set falls back to default". --- .../video-editor/projectPersistence.test.ts | 18 +++++++++++++++--- .../video-editor/projectPersistence.ts | 15 ++++++++++----- src/lib/wallpaper.ts | 1 + 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/components/video-editor/projectPersistence.test.ts b/src/components/video-editor/projectPersistence.test.ts index d816f48..0659cb7 100644 --- a/src/components/video-editor/projectPersistence.test.ts +++ b/src/components/video-editor/projectPersistence.test.ts @@ -199,21 +199,21 @@ it("detects unsaved changes from differing snapshots", () => { }); describe("wallpaper legacy normalization", () => { - it("rewrites resolved file:// resources paths from pre-fix projects", () => { + it("rewrites pre-fix packaged paths (resources/assets/wallpapers/…)", () => { const normalized = normalizeProjectEditor({ wallpaper: "file:///opt/Openscreen/resources/assets/wallpapers/wallpaper5.jpg", }); expect(normalized.wallpaper).toBe("/wallpapers/wallpaper5.jpg"); }); - it("rewrites resolved file:// paths under the new resources/wallpapers layout", () => { + it("rewrites new packaged layout (resources/wallpapers/…)", () => { const normalized = normalizeProjectEditor({ wallpaper: "file:///opt/Openscreen/resources/wallpapers/wallpaper3.jpg", }); expect(normalized.wallpaper).toBe("/wallpapers/wallpaper3.jpg"); }); - it("rewrites unpackaged dev paths (public/wallpapers/…)", () => { + it("rewrites unpackaged dev layout (public/wallpapers/…)", () => { const normalized = normalizeProjectEditor({ wallpaper: "file:///home/user/project/public/wallpapers/wallpaper1.jpg", }); @@ -236,4 +236,16 @@ describe("wallpaper legacy normalization", () => { normalizeProjectEditor({ wallpaper: "linear-gradient(90deg, red, blue)" }).wallpaper, ).toBe("linear-gradient(90deg, red, blue)"); }); + + it("does NOT rewrite user files outside the known install layout", () => { + const userPath = "file:///home/user/Pictures/wallpapers/wallpaper1.jpg"; + expect(normalizeProjectEditor({ wallpaper: userPath }).wallpaper).toBe(userPath); + }); + + it("falls back to default for bundled paths outside WALLPAPER_PATHS", () => { + const normalized = normalizeProjectEditor({ + wallpaper: "file:///opt/Openscreen/resources/wallpapers/wallpaper99.jpg", + }); + expect(normalized.wallpaper).toBe("/wallpapers/wallpaper1.jpg"); + }); }); diff --git a/src/components/video-editor/projectPersistence.ts b/src/components/video-editor/projectPersistence.ts index c0def97..7c963d7 100644 --- a/src/components/video-editor/projectPersistence.ts +++ b/src/components/video-editor/projectPersistence.ts @@ -40,15 +40,20 @@ import { const VALID_BLUR_SHAPES = new Set(["rectangle", "oval", "freehand"] as const); -// Pre-fix projects could persist resolved file:// URLs (machine-specific) instead -// of the canonical `/wallpapers/wallpaperN.jpg` form. Rewrite those on load so -// they resolve against the current install's resources directory. -const LEGACY_FILE_WALLPAPER_RE = /^file:\/\/.*?\/(?:assets\/)?wallpapers\/(wallpaper\d+\.jpg)$/i; +// Pre-fix projects could persist resolved file:// URLs (machine-specific) for +// bundled wallpapers. Rewrite only paths that match a known install layout +// (resources/[assets/]wallpapers for packaged, public/wallpapers for dev) so +// a legitimate user file that happens to live in a folder named "wallpapers" +// elsewhere is never silently replaced. +const LEGACY_FILE_WALLPAPER_RE = + /^file:\/\/.*?\/(?:resources\/(?:assets\/)?|public\/)wallpapers\/(wallpaper\d+\.jpg)$/i; +const CANONICAL_WALLPAPERS = new Set(WALLPAPER_PATHS); function normalizeWallpaperValue(value: string): string { const match = LEGACY_FILE_WALLPAPER_RE.exec(value); if (!match) return value; - return `/wallpapers/${match[1]}`; + const canonical = `/wallpapers/${match[1]}`; + return CANONICAL_WALLPAPERS.has(canonical) ? canonical : DEFAULT_WALLPAPER; } export { WALLPAPER_PATHS }; diff --git a/src/lib/wallpaper.ts b/src/lib/wallpaper.ts index 34869d7..7361df2 100644 --- a/src/lib/wallpaper.ts +++ b/src/lib/wallpaper.ts @@ -56,6 +56,7 @@ export function resolveImageWallpaperUrl(imagePath: string): string { try { return getAssetPath(withLeadingSlash.slice(1)); } catch (cause) { + if (cause instanceof BackgroundLoadError) throw cause; throw new BackgroundLoadError(imagePath, cause); } } From 373319808e99dc9e2bf9c226691aeebba851dbb2 Mon Sep 17 00:00:00 2001 From: Enriquefft Date: Fri, 24 Apr 2026 21:58:59 -0500 Subject: [PATCH 7/8] cover Windows drive-letter file URLs in legacy wallpaper normalizer test --- src/components/video-editor/projectPersistence.test.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/components/video-editor/projectPersistence.test.ts b/src/components/video-editor/projectPersistence.test.ts index 0659cb7..8a17b9e 100644 --- a/src/components/video-editor/projectPersistence.test.ts +++ b/src/components/video-editor/projectPersistence.test.ts @@ -220,6 +220,13 @@ describe("wallpaper legacy normalization", () => { expect(normalized.wallpaper).toBe("/wallpapers/wallpaper1.jpg"); }); + it("rewrites Windows-style file URLs with drive letter", () => { + const normalized = normalizeProjectEditor({ + wallpaper: "file:///C:/Users/me/openscreen/resources/wallpapers/wallpaper2.jpg", + }); + expect(normalized.wallpaper).toBe("/wallpapers/wallpaper2.jpg"); + }); + it("leaves canonical relative paths untouched", () => { const normalized = normalizeProjectEditor({ wallpaper: "/wallpapers/wallpaper2.jpg" }); expect(normalized.wallpaper).toBe("/wallpapers/wallpaper2.jpg"); From e06e40dbc2614f402d1034f66cae585c92d5681a Mon Sep 17 00:00:00 2001 From: Enriquefft Date: Fri, 24 Apr 2026 22:34:00 -0500 Subject: [PATCH 8/8] clean review nits: typed prefix sentinel, instanceof narrowing, drop dead re-export - Replace anonymous Error in resolveImageWallpaperUrl with typed UnsafeImagePrefixError, mirroring UnsafeAssetPathError so cause chains stay discriminable. - Replace `(err as BackgroundLoadError).cause` casts in wallpaper tests with instanceof narrowing (no `as` per project rules). - Remove unused `WALLPAPER_PATHS` re-export from projectPersistence; consumers import directly from @/lib/wallpaper (SSOT). --- .../video-editor/projectPersistence.ts | 2 -- src/lib/wallpaper.test.ts | 23 ++++++++++++------- src/lib/wallpaper.ts | 12 ++++++---- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/components/video-editor/projectPersistence.ts b/src/components/video-editor/projectPersistence.ts index 7c963d7..7259c1e 100644 --- a/src/components/video-editor/projectPersistence.ts +++ b/src/components/video-editor/projectPersistence.ts @@ -56,8 +56,6 @@ function normalizeWallpaperValue(value: string): string { return CANONICAL_WALLPAPERS.has(canonical) ? canonical : DEFAULT_WALLPAPER; } -export { WALLPAPER_PATHS }; - export const PROJECT_VERSION = 2; export interface ProjectEditorState { diff --git a/src/lib/wallpaper.test.ts b/src/lib/wallpaper.test.ts index 6e1b74a..02596aa 100644 --- a/src/lib/wallpaper.test.ts +++ b/src/lib/wallpaper.test.ts @@ -5,6 +5,7 @@ import { classifyWallpaper, DEFAULT_WALLPAPER, resolveImageWallpaperUrl, + UnsafeImagePrefixError, WALLPAPER_COUNT, WALLPAPER_PATHS, } from "./wallpaper"; @@ -170,8 +171,14 @@ describe("resolveImageWallpaperUrl", () => { expect(resolveImageWallpaperUrl("/wallpapers/my image.jpg")).toBe("/wallpapers/my%20image.jpg"); }); - it("rejects image paths outside /wallpapers/", () => { - expect(() => resolveImageWallpaperUrl("/etc/passwd")).toThrow(BackgroundLoadError); + it("rejects image paths outside /wallpapers/ with UnsafeImagePrefixError as cause", () => { + try { + resolveImageWallpaperUrl("/etc/passwd"); + expect.fail("should have thrown"); + } catch (err) { + if (!(err instanceof BackgroundLoadError)) throw err; + expect(err.cause).toBeInstanceOf(UnsafeImagePrefixError); + } }); it("wraps traversal attempts in BackgroundLoadError (preserves UnsafeAssetPathError as cause)", () => { @@ -179,8 +186,8 @@ describe("resolveImageWallpaperUrl", () => { resolveImageWallpaperUrl("/wallpapers/../etc/passwd"); expect.fail("should have thrown"); } catch (err) { - expect(err).toBeInstanceOf(BackgroundLoadError); - expect((err as BackgroundLoadError).cause).toBeInstanceOf(UnsafeAssetPathError); + if (!(err instanceof BackgroundLoadError)) throw err; + expect(err.cause).toBeInstanceOf(UnsafeAssetPathError); } }); @@ -189,8 +196,8 @@ describe("resolveImageWallpaperUrl", () => { resolveImageWallpaperUrl("/wallpapers/%2e%2e/app.asar"); expect.fail("should have thrown"); } catch (err) { - expect(err).toBeInstanceOf(BackgroundLoadError); - expect((err as BackgroundLoadError).cause).toBeInstanceOf(UnsafeAssetPathError); + if (!(err instanceof BackgroundLoadError)) throw err; + expect(err.cause).toBeInstanceOf(UnsafeAssetPathError); } }); @@ -226,8 +233,8 @@ describe("resolveImageWallpaperUrl", () => { resolveImageWallpaperUrl("/wallpapers/wallpaper1.jpg"); expect.fail("should have thrown"); } catch (err) { - expect(err).toBeInstanceOf(BackgroundLoadError); - expect((err as BackgroundLoadError).cause).toBeInstanceOf(AssetBaseUnavailableError); + if (!(err instanceof BackgroundLoadError)) throw err; + expect(err.cause).toBeInstanceOf(AssetBaseUnavailableError); } }); }); diff --git a/src/lib/wallpaper.ts b/src/lib/wallpaper.ts index 7361df2..6974a04 100644 --- a/src/lib/wallpaper.ts +++ b/src/lib/wallpaper.ts @@ -37,6 +37,13 @@ export function classifyWallpaper(value: string): WallpaperClassification { const ALLOWED_IMAGE_PREFIX = "/wallpapers/"; +export class UnsafeImagePrefixError extends Error { + constructor(prefix: string) { + super(`Image wallpaper path must live under ${prefix}`); + this.name = "UnsafeImagePrefixError"; + } +} + export function resolveImageWallpaperUrl(imagePath: string): string { if ( imagePath.startsWith("http://") || @@ -48,10 +55,7 @@ export function resolveImageWallpaperUrl(imagePath: string): string { } const withLeadingSlash = imagePath.startsWith("/") ? imagePath : `/${imagePath}`; if (!withLeadingSlash.startsWith(ALLOWED_IMAGE_PREFIX)) { - throw new BackgroundLoadError( - imagePath, - new Error(`Image wallpaper path must live under ${ALLOWED_IMAGE_PREFIX}`), - ); + throw new BackgroundLoadError(imagePath, new UnsafeImagePrefixError(ALLOWED_IMAGE_PREFIX)); } try { return getAssetPath(withLeadingSlash.slice(1));