fix: resolve type error, formatting, and import order from PR #321

.github/workflows/ci.yml

@@ -42,3 +42,26 @@ jobs:
           cache: npm
       - run: npm ci
       - run: npx vite build
+
+  e2e:
+    name: E2E Tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 22
+          cache: npm
+      - run: npm ci
+      - run: npx playwright install --with-deps chromium
+      # Install Electron system dependencies not covered by Playwright's chromium deps
+      - run: npx electron . --version || sudo apt-get install -y libgbm-dev
+      - run: npm run build-vite
+      # xvfb provides a virtual display; Electron needs one on Linux even with show:false
+      - run: xvfb-run --auto-servernum npm run test:e2e
+      - uses: actions/upload-artifact@v4
+        if: failure()
+        with:
+          name: playwright-report
+          path: playwright-report/
+          retention-days: 7

electron/ipc/handlers.ts

@@ -14,7 +14,6 @@ import {
 import {
 	normalizeProjectMedia,
 	normalizeRecordingSession,
-	type ProjectMedia,
 	type RecordingSession,
 	type StoreRecordedSessionInput,
 } from "../../src/lib/recordingSession";
@@ -24,143 +23,6 @@ import { RECORDINGS_DIR } from "../main";
 const PROJECT_FILE_EXTENSION = "openscreen";
 const SHORTCUTS_FILE = path.join(app.getPath("userData"), "shortcuts.json");
 const RECORDING_SESSION_SUFFIX = ".session.json";
-const ALLOWED_IMPORT_VIDEO_EXTENSIONS = new Set([".webm", ".mp4", ".mov", ".avi", ".mkv"]);
-
-/**
- * Paths explicitly approved by the user via file picker dialogs or project loads.
- * These are added at runtime when the user selects files from outside the default directories.
- */
-const approvedPaths = new Set<string>();
-
-function approveFilePath(filePath: string): void {
-	approvedPaths.add(path.resolve(filePath));
-}
-
-function getAllowedReadDirs(): string[] {
-	return [RECORDINGS_DIR];
-}
-
-function isPathWithinDir(filePath: string, dirPath: string): boolean {
-	const resolved = path.resolve(filePath);
-	const resolvedDir = path.resolve(dirPath);
-	return resolved === resolvedDir || resolved.startsWith(resolvedDir + path.sep);
-}
-
-function isPathAllowed(filePath: string): boolean {
-	const resolved = path.resolve(filePath);
-	if (approvedPaths.has(resolved)) return true;
-	return getAllowedReadDirs().some((dir) => isPathWithinDir(resolved, dir));
-}
-
-function hasAllowedImportVideoExtension(filePath: string): boolean {
-	return ALLOWED_IMPORT_VIDEO_EXTENSIONS.has(path.extname(filePath).toLowerCase());
-}
-
-async function approveReadableVideoPath(
-	filePath?: string | null,
-	trustedDirs?: string[],
-): Promise<string | null> {
-	const normalizedPath = normalizeVideoSourcePath(filePath);
-	if (!normalizedPath) {
-		return null;
-	}
-
-	if (isPathAllowed(normalizedPath)) {
-		return normalizedPath;
-	}
-
-	if (!hasAllowedImportVideoExtension(normalizedPath)) {
-		return null;
-	}
-
-	// When called with trustedDirs (e.g. from project load), only auto-approve
-	// paths within those directories. This prevents malicious project files from
-	// approving reads to arbitrary filesystem locations.
-	if (trustedDirs) {
-		const resolved = path.resolve(normalizedPath);
-		const withinTrusted = trustedDirs.some((dir) => isPathWithinDir(resolved, dir));
-		if (!withinTrusted) {
-			return null;
-		}
-	}
-
-	try {
-		const stats = await fs.stat(normalizedPath);
-		if (!stats.isFile()) {
-			return null;
-		}
-	} catch {
-		return null;
-	}
-
-	approveFilePath(normalizedPath);
-	return normalizedPath;
-}
-
-function resolveRecordingOutputPath(fileName: string): string {
-	const trimmed = fileName.trim();
-	if (!trimmed) {
-		throw new Error("Invalid recording file name");
-	}
-
-	const parsedPath = path.parse(trimmed);
-	const hasTraversalSegments = trimmed.split(/[\\/]+/).some((segment) => segment === "..");
-	const isNestedPath =
-		parsedPath.dir !== "" ||
-		path.isAbsolute(trimmed) ||
-		trimmed.includes("/") ||
-		trimmed.includes("\\");
-	if (hasTraversalSegments || isNestedPath || parsedPath.base !== trimmed) {
-		throw new Error("Recording file name must not contain path segments");
-	}
-
-	return path.join(RECORDINGS_DIR, parsedPath.base);
-}
-
-async function getApprovedProjectSession(
-	project: unknown,
-	projectFilePath?: string,
-): Promise<RecordingSession | null> {
-	if (!project || typeof project !== "object") {
-		return null;
-	}
-
-	const rawProject = project as { media?: unknown; videoPath?: unknown };
-	const media: ProjectMedia | null =
-		normalizeProjectMedia(rawProject.media) ??
-		(typeof rawProject.videoPath === "string"
-			? {
-					screenVideoPath: normalizeVideoSourcePath(rawProject.videoPath) ?? rawProject.videoPath,
-				}
-			: null);
-
-	if (!media) {
-		return null;
-	}
-
-	// Only auto-approve media paths within the project's directory or RECORDINGS_DIR.
-	// This prevents crafted project files from approving reads to arbitrary locations.
-	const trustedDirs = [RECORDINGS_DIR];
-	if (projectFilePath) {
-		trustedDirs.push(path.dirname(path.resolve(projectFilePath)));
-	}
-
-	const screenVideoPath = await approveReadableVideoPath(media.screenVideoPath, trustedDirs);
-	if (!screenVideoPath) {
-		throw new Error("Project references an invalid or unsupported screen video path");
-	}
-
-	const webcamVideoPath = media.webcamVideoPath
-		? await approveReadableVideoPath(media.webcamVideoPath, trustedDirs)
-		: undefined;
-	if (media.webcamVideoPath && !webcamVideoPath) {
-		throw new Error("Project references an invalid or unsupported webcam video path");
-	}
-
-	return webcamVideoPath
-		? { screenVideoPath, webcamVideoPath, createdAt: Date.now() }
-		: { screenVideoPath, createdAt: Date.now() };
-}
 
 type SelectedSource = {
 	name: string;
@@ -259,12 +121,12 @@ async function storeRecordedSessionFiles(payload: StoreRecordedSessionInput) {
 		typeof payload.createdAt === "number" && Number.isFinite(payload.createdAt)
 			? payload.createdAt
 			: Date.now();
-	const screenVideoPath = resolveRecordingOutputPath(payload.screen.fileName);
+	const screenVideoPath = path.join(RECORDINGS_DIR, payload.screen.fileName);
 	await fs.writeFile(screenVideoPath, Buffer.from(payload.screen.videoData));
 
 	let webcamVideoPath: string | undefined;
 	if (payload.webcam) {
-		webcamVideoPath = resolveRecordingOutputPath(payload.webcam.fileName);
+		webcamVideoPath = path.join(RECORDINGS_DIR, payload.webcam.fileName);
 		await fs.writeFile(webcamVideoPath, Buffer.from(payload.webcam.videoData));
 	}
 
@@ -490,14 +352,6 @@ export function registerIpcHandlers(
 				return { success: false, message: "Invalid file path" };
 			}
 
-			if (!isPathAllowed(normalizedPath)) {
-				console.warn(
-					"[read-binary-file] Rejected path outside allowed directories:",
-					normalizedPath,
-				);
-				return { success: false, message: "Access denied: path outside allowed directories" };
-			}
-
 			const data = await fs.readFile(normalizedPath);
 			return {
 				success: true,
@@ -542,14 +396,6 @@ export function registerIpcHandlers(
 			return { success: true, samples: [] };
 		}
 
-		if (!isPathAllowed(targetVideoPath)) {
-			console.warn(
-				"[get-cursor-telemetry] Rejected path outside allowed directories:",
-				targetVideoPath,
-			);
-			return { success: true, samples: [] };
-		}
-
 		const telemetryPath = `${targetVideoPath}.cursor.json`;
 		try {
 			const content = await fs.readFile(telemetryPath, "utf-8");
@@ -683,17 +529,10 @@ export function registerIpcHandlers(
 				return { success: false, canceled: true };
 			}
 
-			const approvedPath = await approveReadableVideoPath(result.filePaths[0]);
-			if (!approvedPath) {
-				return {
-					success: false,
-					message: "Selected file is not a supported video",
-				};
-			}
 			currentProjectPath = null;
 			return {
 				success: true,
-				path: approvedPath,
+				path: result.filePaths[0],
 			};
 		} catch (error) {
 			console.error("Failed to open file picker:", error);
@@ -819,9 +658,19 @@ export function registerIpcHandlers(
 			const filePath = result.filePaths[0];
 			const content = await fs.readFile(filePath, "utf-8");
 			const project = JSON.parse(content);
-			const session = await getApprovedProjectSession(project, filePath);
 			currentProjectPath = filePath;
-			setCurrentRecordingSessionState(session);
+			if (project && typeof project === "object") {
+				const rawProject = project as { media?: unknown; videoPath?: unknown };
+				const media =
+					normalizeProjectMedia(rawProject.media) ??
+					(typeof rawProject.videoPath === "string"
+						? {
+								screenVideoPath:
+									normalizeVideoSourcePath(rawProject.videoPath) ?? rawProject.videoPath,
+							}
+						: null);
+				setCurrentRecordingSessionState(media ? { ...media, createdAt: Date.now() } : null);
+			}
 
 			return {
 				success: true,
@@ -846,8 +695,18 @@ export function registerIpcHandlers(
 
 			const content = await fs.readFile(currentProjectPath, "utf-8");
 			const project = JSON.parse(content);
-			const session = await getApprovedProjectSession(project, currentProjectPath);
-			setCurrentRecordingSessionState(session);
+			if (project && typeof project === "object") {
+				const rawProject = project as { media?: unknown; videoPath?: unknown };
+				const media =
+					normalizeProjectMedia(rawProject.media) ??
+					(typeof rawProject.videoPath === "string"
+						? {
+								screenVideoPath:
+									normalizeVideoSourcePath(rawProject.videoPath) ?? rawProject.videoPath,
+							}
+						: null);
+				setCurrentRecordingSessionState(media ? { ...media, createdAt: Date.now() } : null);
+			}
 			return {
 				success: true,
 				path: currentProjectPath,
@@ -876,22 +735,12 @@ export function registerIpcHandlers(
 	});
 
 	ipcMain.handle("set-current-video-path", async (_, path: string) => {
-		const normalizedPath = normalizeVideoSourcePath(path);
-		if (!normalizedPath || !isPathAllowed(normalizedPath)) {
-			return { success: false, message: "Video path has not been approved" };
-		}
-
-		const restoredSession = await loadRecordedSessionForVideoPath(normalizedPath);
+		const restoredSession = await loadRecordedSessionForVideoPath(path);
 		if (restoredSession) {
-			// Approve all media paths from the restored session so they can be read later
-			approveFilePath(restoredSession.screenVideoPath);
-			if (restoredSession.webcamVideoPath) {
-				approveFilePath(restoredSession.webcamVideoPath);
-			}
 			setCurrentRecordingSessionState(restoredSession);
 		} else {
 			setCurrentRecordingSessionState({
-				screenVideoPath: normalizedPath,
+				screenVideoPath: normalizeVideoSourcePath(path) ?? path,
 				createdAt: Date.now(),
 			});
 		}

src/components/video-editor/VideoEditor.tsx

@@ -23,7 +23,6 @@ import {
 import { computeFrameStepTime } from "@/lib/frameStep";
 import type { ProjectMedia } from "@/lib/recordingSession";
 import { matchesShortcut } from "@/lib/shortcuts";
-import { loadUserPreferences, saveUserPreferences } from "@/lib/userPreferences";
 import {
 	getAspectRatioValue,
 	getNativeAspectRatioValue,
@@ -367,28 +366,6 @@ export default function VideoEditor() {
 		loadInitialData();
 	}, [applyLoadedProject]);
 
-	// Track whether user preferences have been loaded to avoid
-	// overwriting saved prefs with defaults on the first render
-	const [prefsHydrated, setPrefsHydrated] = useState(false);
-
-	// Load persisted user preferences on mount (intentionally runs once)
-	useEffect(() => {
-		const prefs = loadUserPreferences();
-		updateState({
-			padding: prefs.padding,
-			aspectRatio: prefs.aspectRatio,
-		});
-		setExportQuality(prefs.exportQuality);
-		setExportFormat(prefs.exportFormat);
-		setPrefsHydrated(true);
-	}, [updateState]);
-
-	// Auto-save user preferences when settings change
-	useEffect(() => {
-		if (!prefsHydrated) return;
-		saveUserPreferences({ padding, aspectRatio, exportQuality, exportFormat });
-	}, [prefsHydrated, padding, aspectRatio, exportQuality, exportFormat]);
-
 	const saveProject = useCallback(
 		async (forceSaveAs: boolean) => {
 			if (!videoPath) {

src/lib/userPreferences.ts

@@ -1,94 +0,0 @@
-import type { ExportFormat, ExportQuality } from "@/lib/exporter";
-import type { AspectRatio } from "@/utils/aspectRatioUtils";
-
-const PREFS_KEY = "openscreen_user_preferences";
-
-const VALID_ASPECT_RATIOS: readonly string[] = [
-	"16:9",
-	"9:16",
-	"1:1",
-	"4:3",
-	"4:5",
-	"16:10",
-	"10:16",
-	"native",
-];
-
-export interface UserPreferences {
-	/** Default padding % */
-	padding: number;
-	/** Default aspect ratio */
-	aspectRatio: AspectRatio;
-	/** Default export quality */
-	exportQuality: ExportQuality;
-	/** Default export format */
-	exportFormat: ExportFormat;
-}
-
-const DEFAULT_PREFS: UserPreferences = {
-	padding: 50,
-	aspectRatio: "16:9",
-	exportQuality: "good",
-	exportFormat: "mp4",
-};
-
-function safeJsonParse(text: string | null): Record<string, unknown> | null {
-	if (!text) return null;
-	try {
-		return JSON.parse(text);
-	} catch {
-		return null;
-	}
-}
-
-/**
- * Load persisted user preferences from localStorage.
- * Returns defaults for any missing or invalid fields.
- */
-export function loadUserPreferences(): UserPreferences {
-	let raw: Record<string, unknown> | null = null;
-	try {
-		raw = safeJsonParse(localStorage.getItem(PREFS_KEY));
-	} catch {
-		return { ...DEFAULT_PREFS };
-	}
-	if (!raw || typeof raw !== "object") return { ...DEFAULT_PREFS };
-
-	return {
-		padding:
-			typeof raw.padding === "number" &&
-			Number.isFinite(raw.padding) &&
-			raw.padding >= 0 &&
-			raw.padding <= 100
-				? raw.padding
-				: DEFAULT_PREFS.padding,
-		aspectRatio:
-			typeof raw.aspectRatio === "string" && VALID_ASPECT_RATIOS.includes(raw.aspectRatio)
-				? (raw.aspectRatio as AspectRatio)
-				: DEFAULT_PREFS.aspectRatio,
-		exportQuality:
-			raw.exportQuality === "medium" ||
-			raw.exportQuality === "good" ||
-			raw.exportQuality === "source"
-				? (raw.exportQuality as ExportQuality)
-				: DEFAULT_PREFS.exportQuality,
-		exportFormat:
-			raw.exportFormat === "gif" || raw.exportFormat === "mp4"
-				? (raw.exportFormat as ExportFormat)
-				: DEFAULT_PREFS.exportFormat,
-	};
-}
-
-/**
- * Persist user preferences to localStorage.
- * Only the explicitly provided fields are updated.
- */
-export function saveUserPreferences(partial: Partial<UserPreferences>): void {
-	const current = loadUserPreferences();
-	const merged = { ...current, ...partial };
-	try {
-		localStorage.setItem(PREFS_KEY, JSON.stringify(merged));
-	} catch {
-		// localStorage may be unavailable (e.g. private browsing quota exceeded)
-	}
-}

tests/e2e/gif-export.spec.ts

@@ -11,15 +11,12 @@ const TEST_VIDEO = path.join(__dirname, "../fixtures/sample.webm");
 
 test("exports a GIF from a loaded video", async () => {
 	const outputPath = path.join(os.tmpdir(), `test-gif-export-${Date.now()}.gif`);
-	let testVideoInRecordings = "";
 
 	const app = await electron.launch({
 		args: [
 			MAIN_JS,
 			// Required in CI sandbox environments (GitHub Actions, Docker, etc.)
 			"--no-sandbox",
-			// Force software WebGL in headless CI to avoid GPU framebuffer errors.
-			"--enable-unsafe-swiftshader",
 		],
 		env: {
 			...process.env,
@@ -61,24 +58,22 @@ test("exports a GIF from a loaded video", async () => {
 			);
 		});
 
-		// Copy the test fixture into the app's recordings directory so it passes
-		// the path security check in set-current-video-path.
-		const userDataDir = await app.evaluate(({ app: electronApp }) => {
-			return electronApp.getPath("userData");
-		});
-		const recordingsDir = path.join(userDataDir, "recordings");
-		testVideoInRecordings = path.join(recordingsDir, "test-sample.webm");
-		fs.mkdirSync(recordingsDir, { recursive: true });
-		fs.copyFileSync(TEST_VIDEO, testVideoInRecordings);
-
 		try {
-			await hudWindow.evaluate((videoPath: string) => {
-				window.electronAPI.setCurrentVideoPath(videoPath);
+			await hudWindow.evaluate(async (videoPath: string) => {
+				await window.electronAPI.setCurrentVideoPath(videoPath);
 				window.electronAPI.switchToEditor();
-			}, testVideoInRecordings);
-		} catch {
-			// Expected: switchToEditor() closes the HUD window, terminating
+			}, TEST_VIDEO);
+		} catch (error) {
+			// Expected: switchToEditor() closes the HUD window, which terminates
 			// the Playwright page context before evaluate() can resolve.
+			if (
+				!(
+					error instanceof Error &&
+					error.message.includes("Target page, context or browser has been closed")
+				)
+			) {
+				throw error;
+			}
 		}
 
 		// ── 3. Switch to the editor window. This closes the HUD and opens
@@ -130,8 +125,5 @@ test("exports a GIF from a loaded video", async () => {
 		if (fs.existsSync(outputPath)) {
 			fs.unlinkSync(outputPath);
 		}
-		if (testVideoInRecordings && fs.existsSync(testVideoInRecordings)) {
-			fs.unlinkSync(testVideoInRecordings);
-		}
 	}
 });