From af159e8a2b60d4f4043f6556a4ac5cbba299fd66 Mon Sep 17 00:00:00 2001 From: Enriquefft Date: Fri, 24 Apr 2026 18:58:34 -0500 Subject: [PATCH] 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); } }