diff --git a/electron/ipc/project/manager.test.ts b/electron/ipc/project/manager.test.ts index 2fc02df1..4e9d90d6 100644 --- a/electron/ipc/project/manager.test.ts +++ b/electron/ipc/project/manager.test.ts @@ -114,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")}`; diff --git a/electron/ipc/project/manager.ts b/electron/ipc/project/manager.ts index c40b6cac..8d13e832 100644 --- a/electron/ipc/project/manager.ts +++ b/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,14 +54,39 @@ export function isAllowedLocalReadPath(candidatePath: string) { const allowedPrefixes = [RECORDINGS_DIR, USER_DATA_PATH, getAssetRootPath(), app.getPath("temp")]; const normalizedCandidatePath = normalizePath(candidatePath); + // 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). 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. - return ( + // 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) ); } diff --git a/electron/ipc/register/assets.ts b/electron/ipc/register/assets.ts index cf35bff4..50025d17 100644 --- a/electron/ipc/register/assets.ts +++ b/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' } } diff --git a/scripts/build-whisper-runtime.mjs b/scripts/build-whisper-runtime.mjs index b1ebd8ce..193e5bc2 100644 --- a/scripts/build-whisper-runtime.mjs +++ b/scripts/build-whisper-runtime.mjs @@ -365,12 +365,20 @@ async function main() { const cmake = findCmake(); if (!cmake) { - // Mirror build-windows-capture: if every target already has a staged - // runtime, postinstall is a no-op. This keeps `npm ci` working for - // contributors who do not have CMake installed and only need to run the - // app or tests against the bundled binaries. + // 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))); - if (skipChecks.every(Boolean)) { + const allTargetsStaged = skipChecks.every(Boolean); + + if (allTargetsStaged) { console.log( "[build-whisper-runtime] CMake not found; using bundled whisper runtime artifacts.", ); @@ -381,12 +389,20 @@ async function main() { .filter((_target, index) => !skipChecks[index]) .map((target) => target.archTag) .join(", "); - 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`.", + + 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 stage the whisper runtime for: ${missing}. ` + + "Install CMake and retry, or set WHISPER_RUNTIME_ALLOW_MISSING=1 to build without auto-caption support.", ); - return; } const sourceDir = await ensureSourceTree();