Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions electron/ipc/recording/mac.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
} from "./diagnostics";
import { emitRecordingInterrupted } from "./events";
import { getFinalMacCompanionAudioPath } from "./macCompanionAudio";
import { isUnfinalizedMp4 } from "./mp4Integrity";
import { pruneAutoRecordings } from "./prune";

export function waitForNativeCaptureStart(process: ChildProcessWithoutNullStreams) {
Expand Down Expand Up @@ -279,6 +280,15 @@ export async function recoverNativeMacCaptureOutput() {
return null;
}

// The capture helper writes through AVAssetWriter, so a helper that died
// mid-recording leaves bytes on disk that no decoder can open. Recovering
// with such a file hands the editor an undecodable stream instead of telling
// the user the recording failed.
if (await isUnfinalizedMp4(candidatePath)) {
console.error("[mac-recover] Capture file was never finalized:", candidatePath);
return null;
}
Comment on lines +287 to +290

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Return the interrupted-recording failure to the caller.

This branch returns null, which makes stop-native-screen-recording report “No native screen recording is active” when the helper already produced an unfinalized file. The dedicated recovery IPC handler similarly reports that no recoverable output exists.

Return { success: false, message, unfinalizedPath: candidatePath } from this branch. This preserves the failed-recording state and the raw path without passing the file to muxing or the editor.

🧰 Tools
🪛 ast-grep (0.45.0)

[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import type { ChildProcessWithoutNullStreams } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').

(detect-child-process-typescript)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@electron/ipc/recording/mac.ts` around lines 287 - 290, Update the
unfinalized-file branch in the recording recovery helper around isUnfinalizedMp4
to return { success: false, message, unfinalizedPath: candidatePath } instead of
null. Preserve the existing console error and ensure this failure result
propagates to both stop-native-screen-recording and the dedicated recovery IPC
handler without muxing or editor processing.


try {
if (systemAudioPath || microphonePath) {
try {
Expand Down
123 changes: 123 additions & 0 deletions electron/ipc/recording/mp4Integrity.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { mkdtemp, rm, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import path from "node:path";
import { afterAll, beforeAll, describe, expect, it } from "vitest";

import { classifyMp4Layout, isUnfinalizedMp4, readMp4TopLevelBoxes } from "./mp4Integrity";

function box(type: string, size: number, payload = 0): Buffer {
const header = Buffer.alloc(8 + payload);
header.writeUInt32BE(size, 0);
header.write(type, 4, "latin1");
return header;
}

describe("classifyMp4Layout", () => {
it("treats a file with a moov index as finalized", () => {
expect(
classifyMp4Layout(
[
{ type: "ftyp", size: 28 },
{ type: "mdat", size: 3809329 },
{ type: "moov", size: 1634 },
],
3810991,
),
).toBe("finalized");
});

it("flags the interrupted-writer signature as unfinalized", () => {
expect(
classifyMp4Layout(
[
{ type: "ftyp", size: 28 },
{ type: "wide", size: 8 },
{ type: "mdat", size: 0 },
],
45989367,
),
).toBe("unfinalized");
});

it("keeps a moov-bearing file finalized even when mdat runs to the end", () => {
expect(
classifyMp4Layout(
[
{ type: "ftyp", size: 28 },
{ type: "moov", size: 1634 },
{ type: "mdat", size: 0 },
],
4096,
),
).toBe("finalized");
});

it("does not guess when the layout is unfamiliar", () => {
expect(classifyMp4Layout([{ type: "ftyp", size: 28 }], 28)).toBe("unknown");
expect(
classifyMp4Layout(
[
{ type: "ftyp", size: 28 },
{ type: "moof", size: 512 },
],
540,
),
).toBe("unknown");
});

it("does not classify an empty or unreadable box table", () => {
expect(classifyMp4Layout([], 1024)).toBe("unknown");
expect(classifyMp4Layout([{ type: "mdat", size: 0 }], 0)).toBe("unknown");
});
});

describe("readMp4TopLevelBoxes / isUnfinalizedMp4", () => {
let dir: string;

beforeAll(async () => {
dir = await mkdtemp(path.join(tmpdir(), "recordly-mp4-"));
});

afterAll(async () => {
await rm(dir, { recursive: true, force: true });
});

it("walks the box table of a finalized file", async () => {
const file = path.join(dir, "finalized.mp4");
await writeFile(
file,
Buffer.concat([box("ftyp", 16, 8), box("mdat", 24, 16), box("moov", 16, 8)]),
);

expect(await readMp4TopLevelBoxes(file)).toEqual([
{ type: "ftyp", size: 16 },
{ type: "mdat", size: 24 },
{ type: "moov", size: 16 },
]);
expect(await isUnfinalizedMp4(file)).toBe(false);
});

it("stops at an open-ended mdat and reports the file as unfinalized", async () => {
const file = path.join(dir, "interrupted.mp4");
await writeFile(
file,
Buffer.concat([
box("ftyp", 16, 8),
box("wide", 8),
box("mdat", 0),
Buffer.alloc(4096, 7),
]),
);

expect(await readMp4TopLevelBoxes(file)).toEqual([
{ type: "ftyp", size: 16 },
{ type: "wide", size: 8 },
{ type: "mdat", size: 0 },
]);
expect(await isUnfinalizedMp4(file)).toBe(true);
});

it("reports missing files as not-unfinalized so callers keep their fallback", async () => {
expect(await isUnfinalizedMp4(path.join(dir, "nope.mp4"))).toBe(false);
});
});
100 changes: 100 additions & 0 deletions electron/ipc/recording/mp4Integrity.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import fs from "node:fs/promises";

export interface Mp4Box {
type: string;
size: number;
}

export type Mp4Layout = "finalized" | "unfinalized" | "unknown";

/**
* An MP4 written by AVAssetWriter (the macOS capture helper) or by the Windows
* capture helper only becomes playable once the writer finalizes it: the `mdat`
* gets its real size and a `moov` index is appended. A helper that dies
* mid-recording leaves `ftyp` + an open-ended `mdat` and no `moov` — bytes on
* disk that no player can open.
*
* Classification is deliberately conservative: only the exact interrupted-writer
* signature is reported as `unfinalized`. Anything unexpected is `unknown`, so
* callers keep their existing behaviour rather than rejecting a file that might
* be perfectly fine.
*/
export function classifyMp4Layout(boxes: Mp4Box[], fileSize: number): Mp4Layout {
if (boxes.length === 0 || fileSize <= 0) {
return "unknown";
}

if (boxes.some((box) => box.type === "moov")) {
return "finalized";
}

const lastBox = boxes[boxes.length - 1];
if (lastBox.type === "mdat" && lastBox.size === 0) {
// size 0 means "this box runs to the end of the file" — the placeholder a
// writer patches on finish.
return "unfinalized";
}

return "unknown";
}

/**
* Reads the top-level box table without loading the file: each header is 8 bytes
* and points at the next one, so even a multi-gigabyte capture costs a handful of
* reads.
*/
export async function readMp4TopLevelBoxes(filePath: string, maxBoxes = 32): Promise<Mp4Box[]> {
const handle = await fs.open(filePath, "r");
try {
const { size } = await handle.stat();
const boxes: Mp4Box[] = [];
const header = Buffer.alloc(16);
let offset = 0;

while (offset < size && boxes.length < maxBoxes) {
const { bytesRead } = await handle.read(header, 0, 16, offset);
if (bytesRead < 8) {
break;
}

const declaredSize = header.readUInt32BE(0);
const type = header.toString("latin1", 4, 8);
boxes.push({ type, size: declaredSize });

let boxSize = declaredSize;
if (declaredSize === 1) {
if (bytesRead < 16) {
break;
}
boxSize = Number(header.readBigUInt64BE(8));
} else if (declaredSize === 0) {
// Runs to end of file — nothing can follow it.
break;
}

if (boxSize < 8) {
break;
}
offset += boxSize;
}

return boxes;
} finally {
await handle.close();
}
}

/**
* True only when the file is positively identified as a capture that was never
* finalized. Unreadable or unusual files return false so callers fall back to
* their previous behaviour.
*/
export async function isUnfinalizedMp4(filePath: string): Promise<boolean> {
try {
const boxes = await readMp4TopLevelBoxes(filePath);
const { size } = await fs.stat(filePath);
return classifyMp4Layout(boxes, size) === "unfinalized";
} catch {
return false;
}
}
21 changes: 20 additions & 1 deletion electron/ipc/register/recording.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
waitForNativeCaptureStart,
waitForNativeCaptureStop,
} from "../recording/mac";
import { isUnfinalizedMp4 } from "../recording/mp4Integrity";
import { resolveRecordedVideoStoragePath } from "../recording/storagePath";
import {
attachWindowsCaptureLifecycle,
Expand Down Expand Up @@ -1201,7 +1202,25 @@ export function registerRecordingHandlers(
error: String(error),
});

// Try to recover: if the target file exists on disk, finalize with it
// Try to recover: if the target file exists on disk, finalize with it.
// Existence alone is not enough — a helper that died mid-capture leaves
// an unfinalized MP4 (open `mdat`, no `moov` index) that no decoder can
// open, and passing it on shows the user a garbled editor preview
// instead of telling them the recording failed.
if (fallbackPath && (await isUnfinalizedMp4(fallbackPath))) {
console.error(
"[stop-native-screen-recording] Capture file was never finalized:",
fallbackPath,
);
return {
success: false,
message:
"The recording was interrupted before it could be finalized. The raw capture data is still on disk.",
error: String(error),
unfinalizedPath: fallbackPath,
};
}

if (fallbackPath) {
try {
await fs.access(fallbackPath);
Expand Down
Loading