huanld/openscreen
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases 2 Wiki Activity

Merge pull request #352 from siddharthvaddem/sid/fix-read-handler-security

Browse Source 
fix(security): prevent path traversal in IPC file read handlers
This commit is contained in:
Sid
2026-04-05 17:04:27 -07:00
committed by GitHub
parent 2ce4dc49b3 d4c50c9a5e
commit e571ecbf8d
3 changed files with 201 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/ci.yml
-23
View File
@@ -42,26 +42,3 @@ 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
+180 -29
View File
@@ -14,6 +14,7 @@ import {
import {
normalizeProjectMedia,
normalizeRecordingSession,
type ProjectMedia,
type RecordingSession,
type StoreRecordedSessionInput,
} from "../../src/lib/recordingSession";
@@ -23,6 +24,143 @@ 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;
@@ -121,12 +259,12 @@ async function storeRecordedSessionFiles(payload: StoreRecordedSessionInput) {
typeof payload.createdAt === "number" && Number.isFinite(payload.createdAt)
? payload.createdAt
: Date.now();
const screenVideoPath = path.join(RECORDINGS_DIR, payload.screen.fileName);
const screenVideoPath = resolveRecordingOutputPath(payload.screen.fileName);
await fs.writeFile(screenVideoPath, Buffer.from(payload.screen.videoData));
let webcamVideoPath: string | undefined;
if (payload.webcam) {
webcamVideoPath = path.join(RECORDINGS_DIR, payload.webcam.fileName);
webcamVideoPath = resolveRecordingOutputPath(payload.webcam.fileName);
await fs.writeFile(webcamVideoPath, Buffer.from(payload.webcam.videoData));
}
@@ -352,6 +490,14 @@ 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,
@@ -396,6 +542,14 @@ 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");
@@ -529,10 +683,17 @@ 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: result.filePaths[0],
path: approvedPath,
};
} catch (error) {
console.error("Failed to open file picker:", error);
@@ -658,19 +819,9 @@ 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;
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);
}
setCurrentRecordingSessionState(session);
return {
success: true,
@@ -695,18 +846,8 @@ export function registerIpcHandlers(
const content = await fs.readFile(currentProjectPath, "utf-8");
const project = JSON.parse(content);
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);
}
const session = await getApprovedProjectSession(project, currentProjectPath);
setCurrentRecordingSessionState(session);
return {
success: true,
path: currentProjectPath,
@@ -735,12 +876,22 @@ export function registerIpcHandlers(
});
ipcMain.handle("set-current-video-path", async (_, path: string) => {
const restoredSession = await loadRecordedSessionForVideoPath(path);
const normalizedPath = normalizeVideoSourcePath(path);
if (!normalizedPath || !isPathAllowed(normalizedPath)) {
return { success: false, message: "Video path has not been approved" };
}
const restoredSession = await loadRecordedSessionForVideoPath(normalizedPath);
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: normalizeVideoSourcePath(path) ?? path,
screenVideoPath: normalizedPath,
createdAt: Date.now(),
});
}
tests/e2e/gif-export.spec.ts
+21 -13
View File
@@ -11,12 +11,15 @@ 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,
@@ -58,22 +61,24 @@ 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(async (videoPath: string) => {
await window.electronAPI.setCurrentVideoPath(videoPath);
await hudWindow.evaluate((videoPath: string) => {
window.electronAPI.setCurrentVideoPath(videoPath);
window.electronAPI.switchToEditor();
}, TEST_VIDEO);
} catch (error) {
// Expected: switchToEditor() closes the HUD window, which terminates
}, testVideoInRecordings);
} catch {
// Expected: switchToEditor() closes the HUD window, terminating
// 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
@@ -125,5 +130,8 @@ test("exports a GIF from a loaded video", async () => {
if (fs.existsSync(outputPath)) {
fs.unlinkSync(outputPath);
}
if (testVideoInRecordings && fs.existsSync(testVideoInRecordings)) {
fs.unlinkSync(testVideoInRecordings);
}
}
});