diff --git a/README.md b/README.md index 679bc12d3..e0b38d8ef 100644 --- a/README.md +++ b/README.md @@ -65,7 +65,7 @@ Every platform has a recommended route below. On Windows that is the Microsoft S ### System requirements - **Windows**: version 1903+ (build 18362) with Intel 8th Gen / AMD Ryzen 2000 series or newer minimum; Windows 11 with Intel 12th Gen / Ryzen 4000 series or newer recommended -- **macOS**: 12.3 (Monterey) or later — required by ScreenCaptureKit for native capture +- **macOS**: 13 (Ventura) or later — required by ScreenCaptureKit for capture - **Linux**: `xdg-desktop-portal` and PipeWire for native capture and system audio; recording still works without them through the browser-capture fallback, with fewer capabilities (see [Platform differences](#platform-differences)) - **RAM**: 8 GB minimum, 16 GB recommended @@ -171,10 +171,10 @@ You may need to grant screen recording permissions depending on your desktop env Everything in the editor and export is the same on macOS, Windows, and Linux: zooms, backgrounds, motion blur, crop/trim/speed, blur regions, annotations, auto-captions, AI editing, projects, export, and all languages. All three now record through a native capture pipeline; the remaining differences are narrower than they used to be: - **Native recording**: macOS (ScreenCaptureKit), Windows (Windows Graphics Capture), and Linux (PipeWire via the ScreenCast portal) all record through a native pipeline for higher quality and clean window-level capture. On Linux the browser pipeline stays as an automatic fallback if the helper isn't available. -- **Custom cursors**: on macOS and Windows the real cursor is captured with shape, type, and clicks. Linux captures position and cursor shape through the portal, so cursor themes and the editable cursor overlay work there too — but the portal reports no mouse button events, so **click effects remain macOS and Windows only**. +- **Custom cursors**: on macOS and Windows the real cursor is captured with shape, type, and clicks. Linux captures position and cursor shape through the portal, so cursor themes and the editable cursor overlay work there too. Click effects work on Linux as well, but not through the portal — Wayland exposes no portal for mouse buttons, so the capture helper reads the left button from evdev, which needs your user in the `input` group. Without that, recording is unaffected and every cursor sample is simply a move. - **Webcam**: Windows muxes the webcam natively into the recording; macOS and Linux record it alongside as a separate file. It works as a picture-in-picture overlay on all three. - **System audio** support varies by OS: - - **macOS**: requires macOS 13+. On macOS 14.2+ you'll be prompted to grant audio capture permission. macOS 12 and below can't capture system audio (mic still works). + - **macOS**: works on every supported version. On macOS 14.2+ you'll be prompted to grant audio capture permission. - **Windows**: works out of the box. - **Linux**: needs PipeWire (default on Ubuntu 22.04+, Fedora 34+). Older PulseAudio-only setups may not capture system audio (mic should still work). diff --git a/electron-builder.json5 b/electron-builder.json5 index 998d38ae3..66ee93af4 100644 --- a/electron-builder.json5 +++ b/electron-builder.json5 @@ -101,6 +101,17 @@ ], "mac": { + // Declared, not merely documented. Electron 41's own LSMinimumSystemVersion is 12.0 + // and the .app inherits it verbatim when this key is absent — so before this line the + // bundle advertised macOS 12 while its native payload was built for 13, and a + // Monterey user got as far as the record button before anything went wrong (#515). + // LaunchServices now refuses to open the app below 13 instead, which is the honest + // signal. + // + // macOS 13 because ScreenCaptureKit capture requires it: ScreenCaptureRecorder is + // `@available(macOS 13.0, *)`. Keep in step with README.md, website/docs/ + // installation.md, and electron/native/screencapturekit/Package.swift. + "minimumSystemVersion": "13.0", "notarize": false, "hardenedRuntime": true, "entitlements": "macos.entitlements", diff --git a/electron/electron-env.d.ts b/electron/electron-env.d.ts index 4eb288e14..e140a4e37 100644 --- a/electron/electron-env.d.ts +++ b/electron/electron-env.d.ts @@ -81,7 +81,10 @@ interface Window { requestNativeMacCursorAccess: () => Promise<{ success: boolean; granted: boolean; - status: string; + // "not-determined" is the only genuine denial; the rest mean the helper + // never got to ask. See macNativeCursorRecordingSession.ts. + status: "granted" | "not-determined" | "missing-helper" | "error" | "exited" | "timeout"; + accessibilityTrusted: boolean; error?: string; }>; assetBaseUrl: string; diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 958ef6d96..aa2014670 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -69,7 +69,10 @@ import { LinuxNativeCaptureSession, } from "../native-bridge/capture/linuxNativeCaptureSession"; import { createCursorRecordingSession } from "../native-bridge/cursor/recording/factory"; -import { requestMacCursorAccessibilityAccess } from "../native-bridge/cursor/recording/macNativeCursorRecordingSession"; +import { + isMacCursorHelperUnavailable, + requestMacCursorAccessibilityAccess, +} from "../native-bridge/cursor/recording/macNativeCursorRecordingSession"; import { findPipeWireCursorHelperPath } from "../native-bridge/cursor/recording/pipeWireCursorRecordingSession"; import type { CursorRecordingSession } from "../native-bridge/cursor/recording/session"; import { toHelperRect } from "../native-bridge/helperCoordinates"; @@ -1913,14 +1916,27 @@ export function registerIpcHandlers( ipcMain.handle("request-native-mac-cursor-access", async () => { const access = await requestMacCursorAccessibilityAccess(); - // When the editable cursor can't get Accessibility trust, pop a native dialog - // that deep-links to the Accessibility pane (mirrors the Screen Recording flow). + // Pop the native Accessibility dialog ONLY for a genuine denial — the helper ran, + // asked, and was told no. Every other !granted status means the helper never got + // to ask (absent from the build, killed by the loader, crashed, hung), and telling + // the user to grant a permission they may well already hold is what made #515 + // impossible to escape. Those degrade silently instead; the recorder falls back to + // position-only cursor telemetry and the countdown still runs. if (process.platform === "darwin" && !access.granted) { + if (isMacCursorHelperUnavailable(access.status)) { + console.warn( + `[cursor-macos] editable cursor unavailable (status=${access.status}${ + access.error ? `, error=${access.error}` : "" + }); the app ${ + access.accessibilityTrusted ? "does" : "does not" + } hold Accessibility trust. Recording continues with position-only cursor telemetry.`, + ); + return access; + } + const mainWin = getMainWindow(); const detail = - access.status === "missing-helper" - ? "The cursor helper couldn't be found in this build, so the editable cursor can't be enabled. Rebuild the native helper (npm run build:native:mac) or switch the HUD cursor mode to system." - : "Allow OpenScreen under System Settings → Privacy & Security → Accessibility, then press record again to start the countdown."; + "Allow OpenScreen under System Settings → Privacy & Security → Accessibility, then press record again to start the countdown."; const messageOptions = { type: "warning", buttons: ["Open Accessibility Settings", "Cancel"], diff --git a/electron/native-bridge/cursor/recording/macNativeCursorAccess.test.ts b/electron/native-bridge/cursor/recording/macNativeCursorAccess.test.ts new file mode 100644 index 000000000..a8a7dfd8c --- /dev/null +++ b/electron/native-bridge/cursor/recording/macNativeCursorAccess.test.ts @@ -0,0 +1,205 @@ +import { EventEmitter } from "node:events"; +import { PassThrough } from "node:stream"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +/** + * The cast on `actual` is written out in the factory rather than shared in a + * helper: `vi.mock` calls are HOISTED above every top-level statement, so a + * module-scope helper is still in its temporal dead zone when the factory runs. + */ +type WithDefault = { default?: Record }; + +vi.mock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + const spawn = vi.fn(); + return { ...actual, spawn, default: { ...((actual as WithDefault).default ?? {}), spawn } }; +}); + +const mocks = vi.hoisted(() => ({ + isTrustedAccessibilityClient: vi.fn(() => true), + // Shared rather than two separate vi.fn()s so a test can make every candidate path + // unreadable and reach the missing-helper branch. + accessSync: vi.fn(), +})); + +vi.mock("node:fs", async (importOriginal) => { + const actual = await importOriginal(); + // No helper binary exists in a test checkout; by default pretend the first candidate + // path is executable so path resolution is not what is under test. + return { + ...actual, + accessSync: mocks.accessSync, + default: { ...((actual as WithDefault).default ?? {}), accessSync: mocks.accessSync }, + }; +}); + +vi.mock("electron", () => ({ + systemPreferences: { isTrustedAccessibilityClient: mocks.isTrustedAccessibilityClient }, + screen: { + getCursorScreenPoint: () => ({ x: 0, y: 0 }), + getDisplayNearestPoint: () => ({ scaleFactor: 2 }), + }, +})); + +import { spawn } from "node:child_process"; +import { + isMacCursorHelperUnavailable, + requestMacCursorAccessibilityAccess, +} from "./macNativeCursorRecordingSession"; + +/** Minimal stand-in for the cursor helper: stdio pipes plus kill bookkeeping. */ +class FakeHelper extends EventEmitter { + stdout = new PassThrough(); + stderr = new PassThrough(); + killed = false; + + kill() { + this.killed = true; + return true; + } + + /** Feeds one NDJSON line, the way the real helper emits them. */ + emitEvent(event: Record) { + this.stdout.write(`${JSON.stringify(event)}\n`); + } +} + +const spawnMock = vi.mocked(spawn); +let helper: FakeHelper; +let originalPlatform: PropertyDescriptor | undefined; + +beforeEach(() => { + originalPlatform = Object.getOwnPropertyDescriptor(process, "platform"); + Object.defineProperty(process, "platform", { value: "darwin", configurable: true }); + helper = new FakeHelper(); + spawnMock.mockReset(); + spawnMock.mockReturnValue(helper as unknown as ReturnType); + mocks.isTrustedAccessibilityClient.mockReset(); + mocks.isTrustedAccessibilityClient.mockReturnValue(true); + mocks.accessSync.mockReset(); + const silence = () => { + // The access probe logs every helper diagnostic; keep the test output readable. + }; + vi.spyOn(console, "warn").mockImplementation(silence); + vi.spyOn(console, "error").mockImplementation(silence); +}); + +afterEach(() => { + if (originalPlatform) { + Object.defineProperty(process, "platform", originalPlatform); + } + vi.restoreAllMocks(); +}); + +/** Lets the spawn listeners attach before the fake helper speaks. */ +async function settle(pending: Promise, act: () => void): Promise { + await Promise.resolve(); + act(); + return pending; +} + +describe("requestMacCursorAccessibilityAccess", () => { + it("grants when the helper reports Accessibility trust", async () => { + const access = await settle(requestMacCursorAccessibilityAccess(), () => + helper.emitEvent({ type: "ready", timestampMs: 1, accessibilityTrusted: true }), + ); + + expect(access).toMatchObject({ success: true, granted: true, status: "granted" }); + }); + + it("reports a genuine denial when the helper ran and was told no", async () => { + const access = await settle(requestMacCursorAccessibilityAccess(), () => + helper.emitEvent({ type: "ready", timestampMs: 1, accessibilityTrusted: false }), + ); + + expect(access).toMatchObject({ granted: false, status: "not-determined" }); + // The ONLY status that should ever raise the "grant Accessibility" dialog. + expect(isMacCursorHelperUnavailable(access.status)).toBe(false); + }); + + /** + * The regression test for #515. On macOS 12 the helper was stamped with a macOS 13 + * deployment target, so it died in the loader before printing its `ready` line — and + * the app answered by telling the user to grant a permission they already held. + * A helper that never got to ask must never be reported as a denial. + */ + it("does not call a helper that died before ready a denied permission", async () => { + const access = await settle(requestMacCursorAccessibilityAccess(), () => + helper.emit("exit", null, "SIGABRT"), + ); + + expect(access.granted).toBe(false); + expect(access.status).toBe("exited"); + // The app itself IS trusted — proof this is a broken build, not a missing grant. + expect(access.accessibilityTrusted).toBe(true); + expect(isMacCursorHelperUnavailable(access.status)).toBe(true); + }); + + it("distinguishes a helper that could not be spawned at all", async () => { + const access = await settle(requestMacCursorAccessibilityAccess(), () => + helper.emit("error", new Error("spawn ENOENT")), + ); + + expect(access).toMatchObject({ granted: false, status: "error" }); + expect(isMacCursorHelperUnavailable(access.status)).toBe(true); + }); + + it("distinguishes a helper that hung without ever answering", async () => { + vi.useFakeTimers(); + try { + const pending = requestMacCursorAccessibilityAccess(); + await Promise.resolve(); + await vi.advanceTimersByTimeAsync(5_000); + const access = await pending; + + expect(access).toMatchObject({ granted: false, status: "timeout" }); + expect(isMacCursorHelperUnavailable(access.status)).toBe(true); + } finally { + vi.useRealTimers(); + } + }); + + /** + * The other half of #515's conflation, and the branch whose dialog used to tell the + * user to run a build script. No helper on disk is not a permission problem either. + */ + it("reports an absent helper as unavailable, not as a denial", async () => { + mocks.accessSync.mockImplementation(() => { + throw new Error("ENOENT"); + }); + mocks.isTrustedAccessibilityClient.mockReturnValue(false); + + const access = await requestMacCursorAccessibilityAccess(); + + expect(access).toMatchObject({ success: true, granted: false, status: "missing-helper" }); + expect(access.accessibilityTrusted).toBe(false); + expect(isMacCursorHelperUnavailable(access.status)).toBe(true); + // Nothing was spawned: there was nothing to spawn. + expect(spawnMock).not.toHaveBeenCalled(); + }); + + /** + * The probe must not raise the macOS Accessibility prompt. It runs before the helper + * is even located, so on every unavailable branch it would be asking for a grant that + * is not what is missing. + */ + it("reads Accessibility trust without prompting", async () => { + await settle(requestMacCursorAccessibilityAccess(), () => + helper.emitEvent({ type: "ready", timestampMs: 1, accessibilityTrusted: true }), + ); + + expect(mocks.isTrustedAccessibilityClient).toHaveBeenCalledWith(false); + expect(mocks.isTrustedAccessibilityClient).not.toHaveBeenCalledWith(true); + }); + + it("keeps the app's own trust separate from the helper's fate", async () => { + mocks.isTrustedAccessibilityClient.mockReturnValue(false); + + const access = await settle(requestMacCursorAccessibilityAccess(), () => + helper.emit("exit", 1, null), + ); + + expect(access.accessibilityTrusted).toBe(false); + expect(access.status).toBe("exited"); + }); +}); diff --git a/electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.ts b/electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.ts index e274b681f..a8d916a59 100644 --- a/electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.ts +++ b/electron/native-bridge/cursor/recording/macNativeCursorRecordingSession.ts @@ -80,98 +80,142 @@ export function findMacCursorHelperPath() { return null; } -export async function requestMacCursorAccessibilityAccess() { +/** + * Why `granted: false` is not the same question as "did the user deny Accessibility". + * + * `not-determined` is the ONLY genuine denial: the helper ran, asked, and was told no. + * The other four mean the helper never got to ask — it is absent from the build, the + * loader killed it, it crashed, or it hung. Reporting those as a denial is what made + * #515 unfixable from the user's side: on macOS 12 the helper died in dyld, and the app + * answered by telling the user to grant a permission they had already granted. + */ +export type MacCursorAccessStatus = + | "granted" + | "not-determined" + | "missing-helper" + | "error" + | "exited" + | "timeout"; + +export interface MacCursorAccessResult { + success: boolean; + granted: boolean; + status: MacCursorAccessStatus; + /** + * Whether *the app* holds Accessibility trust, read from the main process rather + * than from the helper. This is what separates the two failure modes: a helper that + * could not run while this is `true` is a broken build, not a missing grant. + */ + accessibilityTrusted: boolean; + error?: string; +} + +/** True when the helper never got far enough to answer the permission question. */ +export function isMacCursorHelperUnavailable(status: MacCursorAccessStatus) { + return ( + status === "missing-helper" || status === "error" || status === "exited" || status === "timeout" + ); +} + +export async function requestMacCursorAccessibilityAccess(): Promise { if (process.platform !== "darwin") { - return { success: true, granted: true, status: "granted" }; + return { success: true, granted: true, status: "granted", accessibilityTrusted: true }; } + // The return value is the signal, not a side effect: it says whether OpenScreen.app + // itself is trusted, independently of whether the child helper can be launched. + // + // `false`, so this is a silent read. Prompting here would ask for Accessibility + // BEFORE discovering whether the helper can run at all — and in every branch below + // where it cannot (missing-helper, error, exited, timeout) the grant is not what is + // missing, so the prompt is exactly the noise this function now exists to stop. + // + // Nothing is lost on the one path that does ask the user for the grant: reaching + // `not-determined` means the helper RAN, and it calls AXIsProcessTrustedWithOptions + // with kAXTrustedCheckOptionPrompt itself on every start + // (OpenScreenMacOSCursorHelper/main.swift), which is what puts OpenScreen in the + // Accessibility list for the user to tick. + let accessibilityTrusted = false; try { - systemPreferences.isTrustedAccessibilityClient(true); + accessibilityTrusted = systemPreferences.isTrustedAccessibilityClient(false); } catch { - // Continue with helper probing; it can trigger the same macOS prompt. + // Continue with helper probing; the helper performs the same check itself. } const helperPath = findMacCursorHelperPath(); if (!helperPath) { - return { success: true, granted: false, status: "missing-helper" }; + return { success: true, granted: false, status: "missing-helper", accessibilityTrusted }; } - return new Promise<{ success: boolean; granted: boolean; status: string; error?: string }>( - (resolve) => { - const child = spawn(helperPath, [JSON.stringify({ sampleIntervalMs: 250 })], { - stdio: ["ignore", "pipe", "pipe"], + return new Promise((resolve) => { + const child = spawn(helperPath, [JSON.stringify({ sampleIntervalMs: 250 })], { + stdio: ["ignore", "pipe", "pipe"], + }); + let settled = false; + let lineBuffer = ""; + const finish = (result: Omit) => { + if (settled) { + return; + } + settled = true; + clearTimeout(timer); + if (!child.killed) { + child.kill("SIGTERM"); + } + resolve({ ...result, accessibilityTrusted }); + }; + const timer = setTimeout(() => { + finish({ + success: false, + granted: false, + status: "timeout", + error: "Timed out waiting for macOS cursor helper", }); - let settled = false; - let lineBuffer = ""; - const finish = (result: { - success: boolean; - granted: boolean; - status: string; - error?: string; - }) => { - if (settled) { - return; - } - settled = true; - clearTimeout(timer); - if (!child.killed) { - child.kill("SIGTERM"); - } - resolve(result); - }; - const timer = setTimeout(() => { - finish({ - success: false, - granted: false, - status: "timeout", - error: "Timed out waiting for macOS cursor helper", - }); - }, READY_TIMEOUT_MS); + }, READY_TIMEOUT_MS); - child.stdout.setEncoding("utf8"); - child.stdout.on("data", (chunk: string) => { - lineBuffer += chunk; - const lines = lineBuffer.split(/\r?\n/); - lineBuffer = lines.pop() ?? ""; - for (const line of lines) { - const trimmed = line.trim(); - if (!trimmed) { - continue; - } - try { - const event = JSON.parse(trimmed) as MacCursorEvent; - if (event.type === "ready") { - finish({ - success: true, - granted: event.accessibilityTrusted === true, - status: event.accessibilityTrusted === true ? "granted" : "not-determined", - }); - return; - } - } catch { - // Ignore non-JSON helper output. + child.stdout.setEncoding("utf8"); + child.stdout.on("data", (chunk: string) => { + lineBuffer += chunk; + const lines = lineBuffer.split(/\r?\n/); + lineBuffer = lines.pop() ?? ""; + for (const line of lines) { + const trimmed = line.trim(); + if (!trimmed) { + continue; + } + try { + const event = JSON.parse(trimmed) as MacCursorEvent; + if (event.type === "ready") { + finish({ + success: true, + granted: event.accessibilityTrusted === true, + status: event.accessibilityTrusted === true ? "granted" : "not-determined", + }); + return; } + } catch { + // Ignore non-JSON helper output. } - }); + } + }); - child.once("error", (error) => { - finish({ - success: false, - granted: false, - status: "error", - error: error.message, - }); + child.once("error", (error) => { + finish({ + success: false, + granted: false, + status: "error", + error: error.message, }); - child.once("exit", (code, signal) => { - finish({ - success: false, - granted: false, - status: "exited", - error: `macOS cursor helper exited before ready (code=${code}, signal=${signal})`, - }); + }); + child.once("exit", (code, signal) => { + finish({ + success: false, + granted: false, + status: "exited", + error: `macOS cursor helper exited before ready (code=${code}, signal=${signal})`, }); - }, - ); + }); + }); } function normalizeCursorType(value: unknown): NativeCursorType | null { @@ -204,6 +248,10 @@ export class MacNativeCursorRecordingSession implements CursorRecordingSession { this.previousLeftButtonDown = false; this.consecutiveOutsideSamples = 0; + // `true` here, unlike the silent read in requestMacCursorAccessibilityAccess: the + // return value is discarded, so prompting IS the point. Recording is starting and + // the helper is about to spawn, so this is the moment the grant can still change + // what the take records. try { systemPreferences.isTrustedAccessibilityClient(true); } catch { diff --git a/electron/native/screencapturekit/Package.swift b/electron/native/screencapturekit/Package.swift index b865f8ae6..e478693b1 100644 --- a/electron/native/screencapturekit/Package.swift +++ b/electron/native/screencapturekit/Package.swift @@ -4,6 +4,24 @@ import PackageDescription let package = Package( name: "OpenScreenScreenCaptureKitHelper", + // macOS 13 is DELIBERATE, and it is the same number the app declares in + // electron-builder.json5 (`mac.minimumSystemVersion`) and promises in the README. + // Those three must move together; scripts/check-macos-deployment-target.test.mjs + // asserts this one never rises above what the app declares. + // + // It has to be at least 13 regardless: ScreenCaptureRecorder is + // `@available(macOS 13.0, *)` and its main() hard-guards `#available(macOS 13.0, *)`, + // because SCStream's usable surface starts there. + // + // What this block is NOT allowed to become is higher than the declared floor, which is + // how #515 happened. The floor was set here when ScreenCaptureKit was the only target; + // openscreen-macos-cursor-helper was added later and inherited it, because SwiftPM has + // no per-target override. The app then advertised macOS 12 while shipping a 13-only + // helper, and the damage was not the version number: at a deployment target >= 13 the + // linker resolves the Swift Foundation overlay symbols against Foundation.framework and + // drops /usr/lib/swift/libswiftFoundation.dylib from the load commands, so on macOS 12 + // the helper died in dyld before it could speak — which the app reported to the user as + // a denied Accessibility grant. platforms: [ .macOS(.v13) ], diff --git a/scripts/before-pack.cjs b/scripts/before-pack.cjs index 70a2d6ccb..d9701c654 100644 --- a/scripts/before-pack.cjs +++ b/scripts/before-pack.cjs @@ -419,13 +419,19 @@ function checkWinNativePayload() { } function checkMacNativePayload(context) { + const dir = path.join(ROOT, "electron", "native", "bin", `darwin-${archTagFor(context)}`); checkNativePayload({ - dir: path.join(ROOT, "electron", "native", "bin", `darwin-${archTagFor(context)}`), + dir, required: MAC_REQUIRED, osLabel: "macOS", bundleNoun: "the .app", emptyDirFix: `${FIX_MAC}\n\nThe STT helper and the capture helper are separate builds — see\ntechnical-documentation/engineering/build-and-packaging.md.`, }); + + // "Complete" is not the same property as "runnable on the macOS we claim". This file + // exists because a payload can be whole and still broken; a floor above the supported + // one is the second way that happens. See checkMacOsVersionFloor(). + checkMacOsVersionFloor(dir); } function checkLinuxNativePayload(context) { @@ -504,6 +510,18 @@ function checkLinuxNativePayload(context) { */ const MAX_SYMBOL_VERSION = { GLIBC: "2.35", GLIBCXX: "3.4.30", CXXABI: "1.3.13" }; +/** + * The oldest macOS anything in the payload may demand — the macOS twin of + * MAX_SYMBOL_VERSION above, and the same class of bug on a different libc. + * + * Must equal `mac.minimumSystemVersion` in electron-builder.json5, which is what the .app + * tells LaunchServices; before-pack.test.mjs asserts exactly that, so the two cannot drift + * apart quietly. Not read from the config at runtime because this hook must keep working + * if that file is ever restructured — a guard that throws while parsing is a guard that + * gets deleted. + */ +const MAC_MIN_OS_FLOOR = "13.0"; + /** * The one supported way past the ceiling, for the one case it does not fit: a developer * on a distro newer than the floor, building a package for their own machine. @@ -718,7 +736,13 @@ function resolveSymbolCeiling() { // are the only things standing between this escape hatch and a published package that // starts on nobody's machine but the builder's, and they are reachable from a test // without a payload to scan — so they are tested rather than trusted. -exports.__testing = { resolveSymbolCeiling, MAX_SYMBOL_VERSION }; +exports.__testing = { + resolveSymbolCeiling, + MAX_SYMBOL_VERSION, + machoMinOs, + checkMacOsVersionFloor, + MAC_MIN_OS_FLOOR, +}; /** Every ELF under `dir`, recursively — the helper's ffmpeg sits in a subdirectory. */ function elfFilesUnder(dir) { @@ -820,6 +844,158 @@ function checkLinuxSymbolVersionFloor(dir) { ); } +/** + * The macOS minimum-OS a Mach-O declares, as "12.0", or null if it declares none. + * + * Reads LC_BUILD_VERSION (and LC_VERSION_MIN_MACOSX, which is what anything built + * against an older SDK carries) straight out of the file. Parsed here rather than + * shelled out to `vtool -show-build` for the same reason neededSymbolVersions() does not + * use readelf and importedDlls() does not use dumpbin — but with an extra one on top: + * this hook runs for the Windows and Linux packs too, and vtool exists on neither, so a + * subprocess would have to be skipped on exactly the hosts where skipping is silent. + * Parsing makes the guard host-independent instead of conditionally absent. + * + * Universal binaries are walked slice by slice and the HIGHEST floor wins: an x86_64 half + * built on a newer machine strands Intel users just as thoroughly as a thin binary would. + */ +function machoMinOs(file) { + const b = fs.readFileSync(file); + const FAT_MAGIC = 0xcafebabe; + const FAT_MAGIC_64 = 0xcafebabf; + const MH_MAGIC_64 = 0xfeedfacf; + const MH_MAGIC_32 = 0xfeedface; + const LC_VERSION_MIN_MACOSX = 0x24; + const LC_BUILD_VERSION = 0x32; + const PLATFORM_MACOS = 1; + + /** X.Y.Z packed as nibbles: 0x000c0000 is 12.0.0. */ + const decode = (packed) => `${packed >>> 16}.${(packed >> 8) & 0xff}.${packed & 0xff}`; + + const sliceMinOs = (start) => { + const magic = b.readUInt32LE(start); + if (magic !== MH_MAGIC_64 && magic !== MH_MAGIC_32) return null; + const ncmds = b.readUInt32LE(start + 16); + // 32 bytes of mach_header_64 (28 + 4 bytes of `reserved`); 28 for the 32-bit one. + let off = start + (magic === MH_MAGIC_64 ? 32 : 28); + for (let i = 0; i < ncmds; i++) { + if (off + 8 > b.length) return null; + const cmd = b.readUInt32LE(off); + const cmdsize = b.readUInt32LE(off + 4); + if (cmdsize < 8) return null; + if (cmd === LC_BUILD_VERSION && b.readUInt32LE(off + 8) === PLATFORM_MACOS) { + return decode(b.readUInt32LE(off + 12)); + } + if (cmd === LC_VERSION_MIN_MACOSX) { + return decode(b.readUInt32LE(off + 8)); + } + off += cmdsize; + } + return null; + }; + + const fat = b.readUInt32BE(0); + if (fat === FAT_MAGIC || fat === FAT_MAGIC_64) { + const wide = fat === FAT_MAGIC_64; + const nfat = b.readUInt32BE(4); + let best = null; + for (let i = 0; i < nfat; i++) { + const entry = 8 + i * (wide ? 32 : 20); + const offset = wide ? Number(b.readBigUInt64BE(entry + 8)) : b.readUInt32BE(entry + 8); + const found = sliceMinOs(offset); + if (found && (!best || compareVersions(found, best) > 0)) best = found; + } + return best; + } + + return sliceMinOs(0); +} + +/** Every Mach-O under `dir`, recursively. Symlinks are skipped — see elfFilesUnder(). */ +function machoFilesUnder(dir) { + const found = []; + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + found.push(...machoFilesUnder(full)); + continue; + } + if (!entry.isFile()) continue; + // By magic, not by extension: the helpers and whisper-stt-server have none, and + // the ggml/whisper dylibs come as chains of symlinks onto one real file. + const magic = Buffer.alloc(4); + const fd = fs.openSync(full, "r"); + try { + fs.readSync(fd, magic, 0, 4, 0); + } finally { + fs.closeSync(fd); + } + const le = magic.readUInt32LE(0); + const be = magic.readUInt32BE(0); + if (le === 0xfeedfacf || le === 0xfeedface || be === 0xcafebabe || be === 0xcafebabf) { + found.push(full); + } + } + return found; +} + +/** Nothing we ship may demand a newer macOS than MAC_MIN_OS_FLOOR. */ +function checkMacOsVersionFloor(dir) { + const scanned = machoFilesUnder(dir).map((file) => ({ + name: path.relative(dir, file), + minOs: machoMinOs(file), + })); + + // Same assertion the Linux floor makes, for the same reason: a guard that quietly + // stops looking reports "clean" for the rest of the project's life. Every binary we + // ship is built with a deployment target, so reading none from any of them means the + // parser broke rather than that the payload is unusually clean. + if (scanned.length > 0 && !scanned.some((entry) => entry.minOs)) { + throw new Error( + `Refusing to package: read no macOS deployment target from any of the ${scanned.length} ` + + `Mach-O files in ${path.relative(ROOT, dir)}.\n\n` + + "Every one of them carries LC_BUILD_VERSION, so this is a bug in machoMinOs()\n" + + "(scripts/before-pack.cjs), not an unusually clean payload. Fix the parser — leaving\n" + + "it is how a build that cannot start on the supported macOS gets shipped again.", + ); + } + + const offenders = scanned.filter( + (entry) => entry.minOs && compareVersions(entry.minOs, MAC_MIN_OS_FLOOR) > 0, + ); + if (offenders.length === 0) { + return; + } + + throw new Error( + `Refusing to package binaries that demand a newer macOS than the ${MAC_MIN_OS_FLOOR} floor\n` + + "the app claims to support.\n\n" + + ` looked in: ${path.relative(ROOT, dir)}\n\n` + + `${offenders.map((o) => ` - ${o.name} is built for macOS ${o.minOs} (floor ${MAC_MIN_OS_FLOOR})`).join("\n")}\n\n` + + "Almost certainly nothing asked for this: clang and CMake default the deployment\n" + + "target to the BUILD MACHINE's SDK, so this usually means a build script forgot to\n" + + "pin one and the floor followed whatever image compiled it. CI's macos-latest moves\n" + + "on its own, so the same source can ship a different floor month to month.\n\n" + + "The number itself is not what breaks: dyld does NOT refuse a binary whose minos\n" + + "exceeds the running OS. The damage is done at link time — the deployment target\n" + + "decides which symbols the linker resolves against the OS instead of emitting\n" + + "locally, so a too-high floor leaves strong references to symbols the target macOS\n" + + "has never had, and the binary dies in dyld with 'Symbol not found'. That is issue\n" + + "#515: a helper built for 13 stranded every macOS 12 user, and the app reported it\n" + + "as a denied Accessibility permission.\n\n" + + "Pin the deployment target in whichever script built the file:\n\n" + + // Derived, not spelled out: this line said `.v12` for a while after the floor + // moved to 13, i.e. the guard's own remediation advice contradicted the floor + // it was enforcing. + ` Swift platforms: [.macOS(.v${MAC_MIN_OS_FLOOR.split(".")[0]})] electron/native/screencapturekit/Package.swift\n` + + " CMake -DCMAKE_OSX_DEPLOYMENT_TARGET scripts/build-whisper-stt.sh\n" + + " clang -mmacosx-version-min scripts/fetch-ffmpeg-macos.mjs\n" + + " rustc MACOSX_DEPLOYMENT_TARGET scripts/build-macos-compositor-addon.mjs\n\n" + + "To see it yourself:\n\n" + + " vtool -show-build \n\n" + + `Raising MAC_MIN_OS_FLOOR drops a macOS version the README claims to support.`, + ); +} + /** Newest mtime under `target` (file or directory), or 0 if it does not exist. */ function newestMtimeMs(target) { let stat; diff --git a/scripts/before-pack.test.mjs b/scripts/before-pack.test.mjs index 905283510..bc204c737 100644 --- a/scripts/before-pack.test.mjs +++ b/scripts/before-pack.test.mjs @@ -88,3 +88,203 @@ describe("symbol-version ceiling", () => { }); }); }); + +// --------------------------------------------------------------------------- +// macOS deployment floor (issue #515) +// --------------------------------------------------------------------------- +// +// Mach-O headers are synthesised here rather than compiled with clang, so this runs on +// the Linux and Windows CI legs too. That is the same reason the guard parses the file +// itself instead of shelling out to `vtool`: the check has to be present everywhere the +// hook is, not conditionally absent on the hosts where nobody would notice. +// +// The parser is separately cross-checked against the real thing — on a machine with a +// staged macOS payload, every Mach-O in it agreed with `vtool -show-build` (44/44). + +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { declaredAppVersionFrom } from "./macos-floor.mjs"; + +/** Packs X.Y.Z the way LC_BUILD_VERSION does: one byte per component, X in the top half. */ +function packVersion(text) { + const [x = 0, y = 0, z = 0] = text.split(".").map(Number); + return ((x & 0xffff) << 16) | ((y & 0xff) << 8) | (z & 0xff); +} + +/** A 64-bit Mach-O carrying exactly one load command: LC_BUILD_VERSION for macOS. */ +function thinMachO(minOs) { + const header = Buffer.alloc(32); + header.writeUInt32LE(0xfeedfacf, 0); // MH_MAGIC_64 + header.writeUInt32LE(1, 16); // ncmds + const lc = Buffer.alloc(24); + lc.writeUInt32LE(0x32, 0); // LC_BUILD_VERSION + lc.writeUInt32LE(24, 4); // cmdsize + lc.writeUInt32LE(1, 8); // PLATFORM_MACOS + lc.writeUInt32LE(packVersion(minOs), 12); + return Buffer.concat([header, lc]); +} + +/** The older spelling, which anything built against an older SDK carries instead. */ +function thinMachOVersionMin(minOs) { + const header = Buffer.alloc(32); + header.writeUInt32LE(0xfeedfacf, 0); + header.writeUInt32LE(1, 16); + const lc = Buffer.alloc(16); + lc.writeUInt32LE(0x24, 0); // LC_VERSION_MIN_MACOSX + lc.writeUInt32LE(16, 4); + lc.writeUInt32LE(packVersion(minOs), 8); + return Buffer.concat([header, lc]); +} + +/** A universal binary whose slices disagree — the highest floor is the one that counts. */ +function fatMachO(minOsPerSlice) { + const headerSize = 8 + minOsPerSlice.length * 20; + const head = Buffer.alloc(headerSize); + head.writeUInt32BE(0xcafebabe, 0); + head.writeUInt32BE(minOsPerSlice.length, 4); + const slices = minOsPerSlice.map(thinMachO); + let offset = headerSize; + slices.forEach((slice, i) => { + const entry = 8 + i * 20; + head.writeUInt32BE(offset, entry + 8); // offset + head.writeUInt32BE(slice.length, entry + 12); // size + offset += slice.length; + }); + return Buffer.concat([head, ...slices]); +} + +function withPayload(files, body) { + const dir = mkdtempSync(path.join(tmpdir(), "openscreen-minos-")); + try { + for (const [name, bytes] of Object.entries(files)) { + writeFileSync(path.join(dir, name), bytes); + } + return body(dir); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +const testing = () => require(BEFORE_PACK).__testing; + +describe("machoMinOs", () => { + it("reads LC_BUILD_VERSION", () => { + withPayload({ helper: thinMachO("12.0") }, (dir) => { + expect(testing().machoMinOs(path.join(dir, "helper"))).toBe("12.0.0"); + }); + }); + + it("reads the older LC_VERSION_MIN_MACOSX spelling", () => { + withPayload({ helper: thinMachOVersionMin("11.3") }, (dir) => { + expect(testing().machoMinOs(path.join(dir, "helper"))).toBe("11.3.0"); + }); + }); + + it("takes the HIGHEST floor across a universal binary's slices", () => { + // An arm64 half built correctly does not rescue an x86_64 half that was not: + // Intel users are stranded just as thoroughly. + withPayload({ fat: fatMachO(["12.0", "26.0"]) }, (dir) => { + expect(testing().machoMinOs(path.join(dir, "fat"))).toBe("26.0.0"); + }); + }); + + it("returns null for a Mach-O that declares no deployment target", () => { + const header = Buffer.alloc(32); + header.writeUInt32LE(0xfeedfacf, 0); + withPayload({ bare: header }, (dir) => { + expect(testing().machoMinOs(path.join(dir, "bare"))).toBeNull(); + }); + }); +}); + +describe("checkMacOsVersionFloor", () => { + /** + * Fixtures are derived from the floor rather than written as literals. An earlier + * revision hardcoded the then-current floor as "the offending version", and raising + * the floor silently turned the offender into a compliant binary — the guard's own + * tests stopped testing it. The exact versions were never the point; being on the + * wrong side of the floor is. + */ + const floorMajor = Number(testing().MAC_MIN_OS_FLOOR.split(".")[0]); + const above = (bump = 1) => `${floorMajor + bump}.0`; + const below = () => `${floorMajor - 1}.0`; + + it("passes a payload built at or below the floor", () => { + withPayload({ a: thinMachO(testing().MAC_MIN_OS_FLOOR), b: thinMachO(below()) }, (dir) => { + expect(() => testing().checkMacOsVersionFloor(dir)).not.toThrow(); + }); + }); + + /** + * The regression test for #515: the helper that stranded Monterey was built for 13, + * and nothing in the pipeline looked. The message has to carry enough for whoever + * hits it to understand the consequence rather than just raise the constant. + */ + it("refuses a binary built above the floor, and says which and why", () => { + withPayload({ "openscreen-macos-cursor-helper": thinMachO(above()) }, (dir) => { + let message = ""; + try { + testing().checkMacOsVersionFloor(dir); + } catch (err) { + message = err.message; + } + expect(message).toContain("openscreen-macos-cursor-helper"); + expect(message).toContain(`macOS ${above()}.0`); + expect(message).toContain(testing().MAC_MIN_OS_FLOOR); + expect(message).toContain("#515"); + // The mechanism, so nobody "fixes" it by assuming dyld gates on the number. + expect(message).toContain("Symbol not found"); + }); + }); + + it("reports every offender, not just the first", () => { + withPayload( + { + ok: thinMachO(testing().MAC_MIN_OS_FLOOR), + bad1: thinMachO(above(1)), + bad2: thinMachO(above(2)), + }, + (dir) => { + expect(() => testing().checkMacOsVersionFloor(dir)).toThrow(/bad1[\s\S]*bad2/); + }, + ); + }); + + it("shouts if it parsed nothing, rather than reporting a clean payload", () => { + const header = Buffer.alloc(32); + header.writeUInt32LE(0xfeedfacf, 0); + withPayload({ bare: header }, (dir) => { + expect(() => testing().checkMacOsVersionFloor(dir)).toThrow(/bug in machoMinOs/); + }); + }); + + it("says nothing about a directory with no Mach-O in it", () => { + // Non-macOS packs reach this only if the tree exists; an empty one is not an error + // here — checkNativePayload already owns "the payload is incomplete". + withPayload({ "notes.txt": Buffer.from("hello") }, (dir) => { + expect(() => testing().checkMacOsVersionFloor(dir)).not.toThrow(); + }); + }); +}); + +describe("MAC_MIN_OS_FLOOR", () => { + it("matches the floor the .app declares to LaunchServices", () => { + // Shared parser rather than a regex of its own: electron-builder.json5 is heavily + // commented, its comments name this very key, and a private copy here is how the + // two guards drift into one hardened and one not (see scripts/macos-floor.mjs). + const declared = declaredAppVersionFrom( + readFileSync(path.join(path.dirname(BEFORE_PACK), "..", "electron-builder.json5"), "utf8"), + ); + expect( + declared, + 'no "minimumSystemVersion" in the mac block of electron-builder.json5 — without ' + + "it the .app inherits Electron's own floor, which is what let #515 ship", + ).not.toBeNull(); + + const { MAC_MIN_OS_FLOOR } = testing(); + const norm = (v) => v.split(".").concat(["0", "0"]).slice(0, 2).join("."); + // Equal, not merely <=: a pack-time guard looser than the app's own declaration + // would wave through exactly the binaries LaunchServices then refuses to run. + expect(norm(MAC_MIN_OS_FLOOR)).toBe(norm(declared)); + }); +}); diff --git a/scripts/build-whisper-stt.sh b/scripts/build-whisper-stt.sh index dc4728141..74f5be046 100644 --- a/scripts/build-whisper-stt.sh +++ b/scripts/build-whisper-stt.sh @@ -73,6 +73,11 @@ os_arch_tag() { readonly OS_ARCH="$(os_arch_tag)" readonly OUT_DIR="${OUT_ROOT}/${OS_ARCH}" +# Kept beside the other build constants so it is greppable next to the ffmpeg one in +# scripts/fetch-ffmpeg-macos.mjs; the two must agree, and both must match +# `mac.minimumSystemVersion` in electron-builder.json5. +readonly MACOS_DEPLOYMENT_TARGET="13.0" + # Determine the default backend flag for this host. backend_flag_for_host() { case "${OS_ARCH}" in @@ -305,6 +310,21 @@ BUILD_FLAGS=() if [[ -n "${DEFAULT_FLAG}" ]]; then BUILD_FLAGS+=("${DEFAULT_FLAG}") fi +# Pin the macOS floor the app actually ships against (`mac.minimumSystemVersion` in +# electron-builder.json5). Without it CMake +# defaults the deployment target to the BUILD MACHINE's SDK, so whisper-stt-server and +# the libwhisper/libggml*/libparakeet dylibs inherit whatever macOS built them — +# measured 26.0 on the shipped v1.10.0 payload, and ~15.x from CI's `macos-latest`, +# a floor that moves on its own whenever GitHub rolls that image. +# +# This is the macOS twin of the ubuntu-22.04 pin in build-whisper-stt.yml: same defect +# (a shipped binary's floor decided by the runner rather than by the project), different +# libc. Note it is NOT a loader version gate — dyld does not refuse a binary whose minos +# exceeds the running OS. Setting it is what makes the linker enforce macOS 12 symbol +# availability, which is what actually fails at load time. See issue #515. +if [[ "${OS_ARCH}" == darwin-* ]]; then + BUILD_FLAGS+=("-DCMAKE_OSX_DEPLOYMENT_TARGET=${MACOS_DEPLOYMENT_TARGET}") +fi # See the comment in build_variant() re: bash 3.2 + `set -u` + empty arrays # (macOS x64/CPU has no DEFAULT_FLAG, so BUILD_FLAGS is genuinely empty here). build_variant "default" ${BUILD_FLAGS[@]+"${BUILD_FLAGS[@]}"} diff --git a/scripts/check-macos-deployment-target.test.mjs b/scripts/check-macos-deployment-target.test.mjs new file mode 100644 index 000000000..d405fa241 --- /dev/null +++ b/scripts/check-macos-deployment-target.test.mjs @@ -0,0 +1,146 @@ +// Guards the macOS deployment floor of the native Swift helpers (issue #515). +// +// The floor is declared in THREE places that must agree: `mac.minimumSystemVersion` in +// electron-builder.json5 (what the .app tells LaunchServices), the README's system +// requirements (what we promise), and the `platforms:` block in Package.swift (what the +// helpers are actually built for). This file ties the third to the first. +// +// The direction matters. Package.swift may not declare a floor HIGHER than the app +// advertises — that is exactly #515: the floor here was set to 13 when ScreenCaptureKit +// was the only target, openscreen-macos-cursor-helper was added later and inherited it +// because SwiftPM has no per-target override, and the bundle went on advertising macOS 12 +// (Electron's own LSMinimumSystemVersion, inherited because the key was unset). +// +// The damage was not the version number. At a deployment target >= 13 the linker resolves +// the Swift Foundation overlay symbols against Foundation.framework and drops +// /usr/lib/swift/libswiftFoundation.dylib from the load commands; on macOS 12 those +// symbols live only in that dylib, so the helper died in the loader before it could speak +// — and the app reported that as a denied Accessibility grant. +// +// A text assertion rather than a build: this has to fail on Linux and Windows CI too, +// where no Swift toolchain exists. + +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +import { declaredAppFloorFrom } from "./macos-floor.mjs"; + +const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const PACKAGE_SWIFT = path.join(ROOT, "electron", "native", "screencapturekit", "Package.swift"); +const BUILDER_CONFIG = path.join(ROOT, "electron-builder.json5"); + +function declaredAppFloor() { + return declaredAppFloorFrom(readFileSync(BUILDER_CONFIG, "utf8")); +} + +/** + * Reads the major version out of the `platforms:` block, accepting both spellings + * SwiftPM allows — `.macOS(.v12)` and `.macOS("12.3")`. + */ +function declaredMacOsFloor(source) { + // Scoped to the platforms block, with comments stripped from it, rather than matched + // across the whole manifest. That block is preceded by a long comment discussing these + // very version numbers, so a file-wide match is one careless edit away from reading the + // prose instead of the declaration — and reporting a floor the build does not use is + // the one failure this guard must not have. + const block = source.match(/\bplatforms\s*:\s*\[([\s\S]*?)\]/)?.[1]; + if (!block) { + return null; + } + const declarations = block.replace(/\/\/[^\n]*/g, ""); + + const enumMatch = declarations.match(/\.macOS\(\s*\.v(\d+)(?:_\d+)?\s*\)/); + if (enumMatch) { + return Number(enumMatch[1]); + } + + const stringMatch = declarations.match(/\.macOS\(\s*"(\d+)(?:\.\d+)*"\s*\)/); + return stringMatch ? Number(stringMatch[1]) : null; +} + +describe("macOS native helper deployment target", () => { + const source = readFileSync(PACKAGE_SWIFT, "utf8"); + + it("declares a floor no higher than the app itself advertises", () => { + const floor = declaredMacOsFloor(source); + const appFloor = declaredAppFloor(); + + expect(floor, `no .macOS(...) platform found in ${PACKAGE_SWIFT}`).not.toBeNull(); + expect( + appFloor, + 'no "minimumSystemVersion" found in electron-builder.json5 — without it the .app ' + + "inherits Electron's own floor, which is what let #515 ship", + ).not.toBeNull(); + expect( + floor, + `Package.swift builds the native helpers for macOS ${floor}, above the ${appFloor} ` + + "the .app advertises to LaunchServices. This block is package-wide and also " + + "governs openscreen-macos-cursor-helper, which needs nothing newer than 10.15. " + + "Every user between the two versions gets a helper that dies in the loader, " + + "reported as a denied Accessibility grant. See issue #515.", + ).toBeLessThanOrEqual(appFloor); + }); + + it("parses both spellings SwiftPM accepts", () => { + expect(declaredMacOsFloor("platforms: [ .macOS(.v12) ]")).toBe(12); + expect(declaredMacOsFloor("platforms: [ .macOS(.v10_15) ]")).toBe(10); + expect(declaredMacOsFloor('platforms: [ .macOS("12.3") ]')).toBe(12); + expect(declaredMacOsFloor("platforms: [ .iOS(.v16) ]")).toBeNull(); + }); + + it("reads the mac block's floor, not the first match in the file", () => { + // Both failure shapes the real config invites: a comment discussing the key + // (electron-builder.json5 carries a long one directly above it), and another + // platform block that could grow the same key later. + const decoyComment = [ + '// was "minimumSystemVersion": "12.0" before #515', + '"mac": {', + '\t"minimumSystemVersion": "13.0",', + "}", + ].join("\n"); + expect(declaredAppFloorFrom(decoyComment)).toBe(13); + + const decoySibling = [ + '"win": {', + '\t"minimumSystemVersion": "99.0",', + "},", + '"mac": {', + '\t"minimumSystemVersion": "13.0",', + "}", + ].join("\n"); + expect(declaredAppFloorFrom(decoySibling)).toBe(13); + + // A URL's `//` must survive the comment strip, or the mac block is lost with it. + const withUrl = [ + '"publish": [{ "url": "https://example.invalid/feed" }],', + '"mac": {', + '\t"minimumSystemVersion": "13.0",', + "}", + ].join("\n"); + expect(declaredAppFloorFrom(withUrl)).toBe(13); + + expect(declaredAppFloorFrom('"win": { "minimumSystemVersion": "13.0" }')).toBeNull(); + }); + + it("reads the declaration, not prose that happens to mention a version", () => { + // The real manifest carries exactly this shape: a comment about the floor sitting + // directly above the floor. Matching file-wide would report 12 while the build used + // 13 — a guard that passes for the very bug it exists to catch. + const decoyAbove = [ + "// It was .macOS(.v12) until this changed; see issue #515.", + "platforms: [", + "\t.macOS(.v13)", + "],", + ].join("\n"); + expect(declaredMacOsFloor(decoyAbove)).toBe(13); + + const decoyInside = ["platforms: [", "\t// was .macOS(.v12)", "\t.macOS(.v13)", "],"].join( + "\n", + ); + expect(declaredMacOsFloor(decoyInside)).toBe(13); + + expect(declaredMacOsFloor("// .macOS(.v12) with no platforms block at all")).toBeNull(); + }); +}); diff --git a/scripts/fetch-ffmpeg-macos.mjs b/scripts/fetch-ffmpeg-macos.mjs index d611aa627..48d92669b 100644 --- a/scripts/fetch-ffmpeg-macos.mjs +++ b/scripts/fetch-ffmpeg-macos.mjs @@ -29,6 +29,20 @@ const ROOT = path.join(__dirname, ".."); const CRATES_DIR = path.join(ROOT, "crates"); /** Pinned release. The directory name is what build.rs looks for. */ +// The macOS floor the app ships against — keep in step with `mac.minimumSystemVersion` +// in electron-builder.json5, which is what the .app tells LaunchServices. +// +// Without it, clang defaults the deployment target +// to the BUILD MACHINE's SDK, so the vendored dylibs inherit whatever macOS built them — +// measured 26.0 on a local build and ~15.x from CI's `macos-latest`, a floor that moves on +// its own every time GitHub rolls that image. That is the same class of leak the configure +// comment below guards against for Homebrew packages, and it is the one it missed. +// +// Note this is NOT a loader version gate: dyld does not refuse a dylib whose minos exceeds +// the running OS (verified). Setting it is what makes the LINKER enforce macOS 12 symbol +// availability, which is the thing that actually breaks at load time. See issue #515. +const MACOS_DEPLOYMENT_TARGET = "13.0"; + const VERSION = "8.1.2"; const TARBALL_SHA256 = "464beb5e7bf0c311e68b45ae2f04e9cc2af88851abb4082231742a74d97b524c"; const DEST = path.join(CRATES_DIR, "thirdparty", `ffmpeg-n${VERSION}-macos64-lgpl-shared`); @@ -202,6 +216,10 @@ run( "--disable-x86asm", `--arch=${process.arch === "arm64" ? "arm64" : "x86_64"}`, "--cc=clang", + // Both, not just cflags: the deployment target has to reach the link step too, or + // the dylibs are stamped with the build machine's floor however they were compiled. + `--extra-cflags=-mmacosx-version-min=${MACOS_DEPLOYMENT_TARGET}`, + `--extra-ldflags=-mmacosx-version-min=${MACOS_DEPLOYMENT_TARGET}`, ], { cwd: src }, ); diff --git a/scripts/macos-floor.mjs b/scripts/macos-floor.mjs new file mode 100644 index 000000000..347685bc6 --- /dev/null +++ b/scripts/macos-floor.mjs @@ -0,0 +1,80 @@ +// Reads the macOS floor the app declares to LaunchServices out of electron-builder.json5. +// +// Its own module because the number is asserted from two different guards — the +// Package.swift floor check and before-pack's pack-time payload check — and a second copy +// of the parser is precisely how one of them ends up hardened and the other not. That +// already happened once: declaredMacOsFloor() was scoped to its declaration block after +// review, while the function written directly beside it still took the first match in the +// whole file. +// +// Hand-rolled rather than a JSON5 parse to stay dependency-free and runnable on every CI +// platform. + +/** + * Drops `//` comments, ignoring any that appear inside a string — electron-builder.json5 + * is heavily commented, and its comments discuss the very keys parsed below. + * + * String-aware rather than a plain `s.replace(/\/\/.*$/gm, "")` because the config also + * carries URLs, whose `//` a naive strip would eat. + */ +function stripJson5Comments(source) { + let out = ""; + let inString = false; + for (let i = 0; i < source.length; i++) { + const ch = source[i]; + if (inString) { + out += ch; + if (ch === "\\") { + out += source[++i] ?? ""; + } else if (ch === '"') { + inString = false; + } + continue; + } + if (ch === '"') { + inString = true; + out += ch; + continue; + } + if (ch === "/" && source[i + 1] === "/") { + while (i < source.length && source[i] !== "\n") i++; + out += "\n"; + continue; + } + out += ch; + } + return out; +} + +/** The body of a top-level `"": { ... }` object, brace-matched. */ +function objectBody(source, key) { + const opener = new RegExp(`"${key}"\\s*:\\s*{`).exec(source); + if (!opener) { + return null; + } + let depth = 0; + for (let i = opener.index + opener[0].length - 1; i < source.length; i++) { + if (source[i] === "{") depth++; + else if (source[i] === "}" && --depth === 0) { + return source.slice(opener.index + opener[0].length, i); + } + } + return null; +} + +/** The declared macOS major floor, or null if the `mac` block does not carry one. */ +export function declaredAppFloorFrom(source) { + const mac = objectBody(stripJson5Comments(source), "mac"); + if (!mac) { + return null; + } + const match = mac.match(/"minimumSystemVersion"\s*:\s*"(\d+)(?:\.\d+)*"/); + return match ? Number(match[1]) : null; +} + +/** The full declared version string (e.g. "13.0"), for callers comparing exactly. */ +export function declaredAppVersionFrom(source) { + const mac = objectBody(stripJson5Comments(source), "mac"); + const match = mac?.match(/"minimumSystemVersion"\s*:\s*"([\d.]+)"/); + return match ? match[1] : null; +} diff --git a/src/hooks/useScreenRecorder.ts b/src/hooks/useScreenRecorder.ts index 14eec5ab4..fcab5452b 100644 --- a/src/hooks/useScreenRecorder.ts +++ b/src/hooks/useScreenRecorder.ts @@ -48,6 +48,31 @@ const RECORDING_FILE_PREFIX = "recording-"; const VIDEO_FILE_EXTENSION = ".webm"; const WEBCAM_FILE_SUFFIX = "-webcam"; +/** + * The cursor mode a BROWSER-pipeline take can actually honour, which is not always the + * one the user picked. + * + * Only win32 reaches that pipeline through `getDisplayMedia`, the sole browser API here + * that can exclude the system cursor (`cursor: "never"`). Everywhere else the + * desktop-capture stream bakes the real cursor into the pixels, so keeping + * "editable-overlay" would start cursor telemetry and have the editor composite a + * SECOND, synthetic cursor on top of it. + * + * This only bites when a platform falls back to browser capture with the editable cursor + * selected — on macOS 12 that is now the normal path (#515), and on Linux it is the + * no-PipeWire path, where the same latent defect lives. + * + * One function rather than the expression inlined twice: the mode reported to the main + * process at start and the mode persisted at finalize have to agree, and they are ~1200 + * lines apart. + */ +function effectiveBrowserCursorMode( + platform: string, + requested: CursorCaptureMode, +): CursorCaptureMode { + return platform === "win32" ? requested : "system"; +} + const AUDIO_BITRATE_VOICE = 128_000; const AUDIO_BITRATE_SYSTEM = 192_000; @@ -550,7 +575,16 @@ export function useScreenRecorder(): UseScreenRecorderReturn { ? { videoData: webcamVideoData, fileName: webcamFileName } : undefined, createdAt: activeRecordingId, - cursorCaptureMode, + // What this take actually did, not what was requested. Only the browser + // pipeline reaches this finalizer (stopRecording returns earlier for all + // three native paths), and off win32 it cannot exclude the system cursor + // — so the mode reported to the main process was forced to "system" and + // the stored metadata has to agree. It is user-visible: `openscreen + // project show` prints it. + cursorCaptureMode: effectiveBrowserCursorMode( + window.electronAPI.getPlatform(), + cursorCaptureMode, + ), durationMs: duration, }); @@ -1546,13 +1580,26 @@ export function useScreenRecorder(): UseScreenRecorderReturn { try { const platform = window.electronAPI.getPlatform(); if (platform === "darwin" && cursorCaptureMode === "editable-overlay") { - // The main process shows a native dialog that deep-links to the - // Accessibility settings pane when access is missing, so we just stop - // here and let the user grant it and press record again. + // Stop before the countdown ONLY when the user genuinely denied + // Accessibility — the main process is showing them a dialog that + // deep-links to the settings pane, so pressing record again after + // granting it will work. + // + // When the helper simply could not run (missing from the build, killed + // by the loader, crashed, hung) there is nothing for the user to grant, + // and blocking here is what left macOS 12 unable to record at all + // (#515). Recording degrades on its own: the session falls back to + // position-only cursor telemetry and the editor draws the cursor from + // its bundled sprites, so only the pointer/text shape hints are lost. const access = await window.electronAPI.requestNativeMacCursorAccess(); - if (!access.granted) { + if (!access.granted && access.status === "not-determined") { return; } + if (!access.granted) { + console.warn( + `Editable cursor unavailable (${access.status}); recording with position-only cursor telemetry.`, + ); + } } } catch (error) { console.warn("Failed to preflight macOS cursor accessibility before countdown:", error); @@ -1654,6 +1701,9 @@ export function useScreenRecorder(): UseScreenRecorderReturn { countdownRunToken?: number, preparedRecordingId?: number | null, ) => { + const platform = window.electronAPI.getPlatform(); + const browserCursorCaptureMode = effectiveBrowserCursorMode(platform, cursorCaptureMode); + try { if (!isCountdownRunActive(countdownRunToken)) { teardownMedia(); @@ -1688,8 +1738,6 @@ export function useScreenRecorder(): UseScreenRecorderReturn { // `getUserMedia` calls is the dominant source of the mic-vs-video lag at the // start of the recording (issue #57). const screenCapture = (async (): Promise => { - const platform = window.electronAPI.getPlatform(); - if (platform === "win32") { // getDisplayMedia + setDisplayMediaRequestHandler (main.ts) supplies the // pre-selected source. Editable cursor mode excludes the system cursor so @@ -1920,7 +1968,7 @@ export function useScreenRecorder(): UseScreenRecorderReturn { setRecording(true); setPaused(false); setElapsedSeconds(0); - window.electronAPI?.setRecordingState(true, recordingId.current, cursorCaptureMode); + window.electronAPI?.setRecordingState(true, recordingId.current, browserCursorCaptureMode); const activeScreenRecorder = screenRecorder.current; const activeWebcamRecorder = webcamRecorder.current; diff --git a/website/docs/installation.md b/website/docs/installation.md index 5d2c7545f..528769017 100644 --- a/website/docs/installation.md +++ b/website/docs/installation.md @@ -23,7 +23,7 @@ Download the latest installer for your platform from the [download page](/downlo | | Minimum | Recommended | |---|---|---| | **Windows** | Windows 10 version 1903 (build 18362) or later, Intel 8th Gen / AMD Ryzen 2000 series or newer | Windows 11, Intel 12th Gen / AMD Ryzen 4000 series or newer | -| **macOS** | macOS 12.3 (Monterey) — required by ScreenCaptureKit for native capture | macOS 14 or later | +| **macOS** | macOS 13 (Ventura) — required by ScreenCaptureKit for capture | macOS 14 or later | | **Linux** | `xdg-desktop-portal` and PipeWire for native capture and system audio (default on Ubuntu 22.04+, Fedora 34+) — recording still works without them through the [browser-capture fallback](#platform-differences), with fewer capabilities. Recording mouse clicks on Wayland additionally needs your user in the `input` group — see [Mouse clicks on Wayland](#mouse-clicks-on-wayland) | Same, kept up to date | | **RAM** | 8 GB | 16 GB | @@ -131,10 +131,10 @@ The editing tools are the same everywhere — zooms, backgrounds, crop/trim/spee | | macOS | Windows | Linux | |---|---|---|---| -| Capture pipeline | Native (ScreenCaptureKit) | Native (Windows Graphics Capture) | Browser pipeline | +| Capture pipeline | Native (ScreenCaptureKit) | Native (Windows Graphics Capture) | Native (PipeWire via the ScreenCast portal); browser fallback without the helper, losing hardware encode and cursor telemetry | | Custom cursor themes / click effects | ✅ | ✅ | ✅ on Wayland — click capture needs the `input` group ([details](#mouse-clicks-on-wayland)) | | Webcam | Native capture | Native capture | Browser capture (still works as PiP) | -| System audio | macOS 13+; permission prompt on 14.2+; not available on macOS 12 and below | Works out of the box | Needs PipeWire (default on Ubuntu 22.04+, Fedora 34+) | +| System audio | Works out of the box; permission prompt on macOS 14.2+ | Works out of the box | Needs PipeWire (default on Ubuntu 22.04+, Fedora 34+) | | MP4 export | ✅ | ✅ | ✅ (software encode) | | GIF export | ✅ | ✅ | ✅ | | On-device transcription | Metal (Apple Silicon) / CPU | Vulkan / CPU | Vulkan / CPU |