Merge pull request #419 from crbender/fix/local-read-allowlist-and-postinstall-fallback

fix: close local read allowlist bypass and make whisper-runtime postinstall non-fatal

electron/ipc/project/manager.test.ts

@@ -47,13 +47,17 @@ describe("local media path policy", () => {
 		}
 	});
 
-	it("allows existing exported media files outside the session directories", async () => {
+	it("rejects existing media files outside allowed directories until they are approved", async () => {
 		const downloadsPath = path.join(tempRoot, "Downloads");
 		const exportPath = path.join(downloadsPath, "export-test.mp4");
 		await fs.mkdir(downloadsPath, { recursive: true });
 		await fs.writeFile(exportPath, "test-video");
 
-		const { isAllowedLocalMediaPath } = await import("./manager");
+		const { isAllowedLocalMediaPath, rememberApprovedLocalReadPath } = await import("./manager");
+
+		await expect(isAllowedLocalMediaPath(exportPath)).resolves.toBe(false);
+
+		await rememberApprovedLocalReadPath(exportPath);
 
 		await expect(isAllowedLocalMediaPath(exportPath)).resolves.toBe(true);
 	});
@@ -74,17 +78,25 @@ describe("local media path policy", () => {
 		await expect(isAllowedLocalMediaPath(pendingExportPath)).resolves.toBe(true);
 	});
 
-	it("approves media-server access for existing external files resolved through the URL policy", async () => {
+	it("approves media-server access for approved external files resolved through the URL policy", async () => {
 		const downloadsPath = path.join(tempRoot, "Downloads");
 		const videoPath = path.join(downloadsPath, "external-video.mp4");
 		await fs.mkdir(downloadsPath, { recursive: true });
 		await fs.writeFile(videoPath, "test-video");
 		const resolvedVideoPath = await fs.realpath(videoPath);
 
-		const { resolveApprovedLocalMediaPath } = await import("./manager");
+		const { resolveApprovedLocalMediaPath, rememberApprovedLocalReadPath } = await import(
+			"./manager"
+		);
 		const { isAllowedMediaPath } = await import("../../mediaServer");
 
+		// Unapproved external paths are rejected before they ever reach the media server.
 		expect(isAllowedMediaPath(videoPath)).toBe(false);
+		await expect(resolveApprovedLocalMediaPath(videoPath)).resolves.toBeNull();
+
+		// Once the user opts in (via dialog/export/etc.) the path is approved.
+		await rememberApprovedLocalReadPath(videoPath);
+
 		await expect(resolveApprovedLocalMediaPath(videoPath)).resolves.toBe(resolvedVideoPath);
 		expect(isAllowedMediaPath(videoPath)).toBe(true);
 	});
@@ -102,6 +114,29 @@ describe("local media path policy", () => {
 		expect(isAllowedMediaPath(textPath)).toBe(false);
 	});
 
+	it("rejects symlinks under allowed prefixes that point outside the allowlist", async () => {
+		const outsideTarget = path.join(tempRoot, "outside-secret.mp4");
+		const symlinkInsideUserData = path.join(userDataPath, "shortcut-to-secret.mp4");
+		await fs.writeFile(outsideTarget, "secret-bytes");
+
+		try {
+			await fs.symlink(outsideTarget, symlinkInsideUserData);
+		} catch (error) {
+			// Windows requires Developer Mode or admin to create file symlinks. If
+			// we can't create one, the bypass we're guarding against also can't be
+			// crafted on this machine, so skipping is safe.
+			if ((error as NodeJS.ErrnoException).code === "EPERM") {
+				return;
+			}
+			throw error;
+		}
+
+		const { isAllowedLocalMediaPath, resolveApprovedLocalMediaPath } = await import("./manager");
+
+		await expect(isAllowedLocalMediaPath(symlinkInsideUserData)).resolves.toBe(false);
+		await expect(resolveApprovedLocalMediaPath(symlinkInsideUserData)).resolves.toBeNull();
+	});
+
 	it("preserves an existing project thumbnail when no replacement is provided", async () => {
 		const projectPath = path.join(tempRoot, "Projects", "demo.recordly");
 		const thumbnailDataUrl = `data:image/png;base64,${Buffer.from("png-thumbnail").toString("base64")}`;

electron/ipc/project/manager.ts

@@ -1,5 +1,5 @@
 import { constants as fsConstants } from "node:fs";
-import { existsSync } from "node:fs";
+import { existsSync, realpathSync } from "node:fs";
 import fs from "node:fs/promises";
 import path from "node:path";
 import { app } from "electron";
@@ -54,10 +54,39 @@ export function isAllowedLocalReadPath(candidatePath: string) {
 	const allowedPrefixes = [RECORDINGS_DIR, USER_DATA_PATH, getAssetRootPath(), app.getPath("temp")];
 	const normalizedCandidatePath = normalizePath(candidatePath);
 
-	return (
-		existsSync(normalizedCandidatePath) ||
+	// Canonicalize so a symlink placed under an allowed prefix can't smuggle in a
+	// target that lives outside it. realpathSync throws when the path doesn't
+	// exist yet (e.g. a pending export approved before the file is written) — in
+	// that case fall back to the lexical path, which can only succeed via the
+	// approvedLocalReadPaths check below since no symlink target exists yet.
+	let canonicalCandidatePath = normalizedCandidatePath;
+	try {
+		canonicalCandidatePath = normalizePath(realpathSync(normalizedCandidatePath));
+	} catch {
+		// File may not exist yet; keep the lexical path.
+	}
+
+	// Security: only allow paths under app-managed directories or paths the user
+	// has explicitly opted into (recording session sources, files chosen via
+	// dialog, app-produced exports). The lexical path must satisfy the policy
+	// AND the canonical (real) path must satisfy it too, so a symlink under an
+	// allowed prefix that points outside the allowlist is rejected. Previously
+	// this returned true for any existing file, which made the allowlist a no-op
+	// for read-local-file and the local media URL handler.
+	const lexicalAllowed =
 		allowedPrefixes.some((prefix) => isPathInsideDirectory(normalizedCandidatePath, prefix)) ||
-		approvedLocalReadPaths.has(normalizedCandidatePath)
+		approvedLocalReadPaths.has(normalizedCandidatePath);
+	if (!lexicalAllowed) {
+		return false;
+	}
+
+	if (canonicalCandidatePath === normalizedCandidatePath) {
+		return true;
+	}
+
+	return (
+		allowedPrefixes.some((prefix) => isPathInsideDirectory(canonicalCandidatePath, prefix)) ||
+		approvedLocalReadPaths.has(canonicalCandidatePath)
 	);
 }
 

electron/ipc/register/assets.ts

@@ -19,9 +19,10 @@ export function registerAssetHandlers() {
   ipcMain.handle('generate-wallpaper-thumbnail', async (_, filePath: string) => {
     try {
       const resolved = normalizePath(filePath)
-      const realResolved = await fs.realpath(resolved).catch(() => resolved)
-
-      if (!isAllowedLocalReadPath(resolved) && !isAllowedLocalReadPath(realResolved)) {
+      // isAllowedLocalReadPath now canonicalizes via realpath internally and
+      // requires both the lexical and real paths to satisfy the policy, so a
+      // single check covers symlinks under allowed prefixes.
+      if (!isAllowedLocalReadPath(resolved)) {
         return { success: false, error: 'Access denied' }
       }
 
@@ -106,8 +107,7 @@ export function registerAssetHandlers() {
   ipcMain.handle('read-local-file', async (_, filePath: string) => {
     try {
       const resolved = normalizePath(filePath)
-      const realResolved = await fs.realpath(resolved).catch(() => resolved)
-      if (!isAllowedLocalReadPath(resolved) && !isAllowedLocalReadPath(realResolved)) {
+      if (!isAllowedLocalReadPath(resolved)) {
         console.warn(`[read-local-file] Blocked read outside allowed directories: ${resolved}`)
         return { success: false, error: 'Access denied: path outside allowed directories' }
       }

scripts/build-whisper-runtime.mjs

@@ -361,15 +361,51 @@ async function stageRuntimeArtifacts(target, candidateDir, runtimeEntries) {
 }
 
 async function main() {
+	const targets = getTargetConfigs();
 	const cmake = findCmake();
+
 	if (!cmake) {
+		// Soft-fail only when this script runs as part of `npm install`/`npm ci`,
+		// in CI, or when the developer explicitly opted in. Direct invocations
+		// (e.g. via `npm run build`, `build:win`, `build:mac`, `build:linux`)
+		// must still fail loudly so we never ship a release build that is
+		// missing the whisper runtime and silently ships broken auto-captions.
+		const isPostinstall = process.env.npm_lifecycle_event === "postinstall";
+		const isCI = process.env.CI === "true";
+		const allowMissing = process.env.WHISPER_RUNTIME_ALLOW_MISSING === "1";
+		const softFailAllowed = isPostinstall || isCI || allowMissing;
+
+		const skipChecks = await Promise.all(targets.map((target) => shouldSkipBuild(target)));
+		const allTargetsStaged = skipChecks.every(Boolean);
+
+		if (allTargetsStaged) {
+			console.log(
+				"[build-whisper-runtime] CMake not found; using bundled whisper runtime artifacts.",
+			);
+			return;
+		}
+
+		const missing = targets
+			.filter((_target, index) => !skipChecks[index])
+			.map((target) => target.archTag)
+			.join(", ");
+
+		if (softFailAllowed) {
+			console.warn(
+				`[build-whisper-runtime] CMake not found and no bundled runtime is staged for: ${missing}. ` +
+					"Auto-caption features that rely on whisper.cpp will be unavailable until you install CMake " +
+					"and rerun `npm run build:whisper-runtime`.",
+			);
+			return;
+		}
+
 		throw new Error(
-			"[build-whisper-runtime] CMake is required to build the bundled Whisper runtime.",
+			`[build-whisper-runtime] CMake is required to stage the whisper runtime for: ${missing}. ` +
+				"Install CMake and retry, or set WHISPER_RUNTIME_ALLOW_MISSING=1 to build without auto-caption support.",
 		);
 	}
 
 	const sourceDir = await ensureSourceTree();
-	const targets = getTargetConfigs();
 
 	console.log(
 		`[build-whisper-runtime] Target architectures for ${process.platform}: ${targets.map((target) => target.archTag).join(", ")}`,