-
Notifications
You must be signed in to change notification settings - Fork 23
fix/macos-packaged-readonly-startup #2122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
lost-particles
wants to merge
3
commits into
PostHog:main
Choose a base branch
from
lost-particles:fix/macos-packaged-readonly-startup
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+367
−1
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
202 changes: 202 additions & 0 deletions
202
apps/code/src/main/utils/macos-packaged-install-guard.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,202 @@ | ||
| import { describe, expect, it, vi } from "vitest"; | ||
| import { | ||
| type DarwinMountEntry, | ||
| isMacosAppTranslocationPath, | ||
| isMacosPackagedUnsafeBundleLocation, | ||
| isMacosPathOnReadOnlyNonRootMountFromTable, | ||
| parseDarwinMountTable, | ||
| type ReadDarwinMountTable, | ||
| } from "./macos-packaged-install-guard"; | ||
|
|
||
| describe("isMacosAppTranslocationPath", () => { | ||
| it.each([ | ||
| { | ||
| case: "appPath is translocated", | ||
| appPath: | ||
| "/private/var/folders/yf/xx/AppTranslocation/C6283C3C-9D6E-4D81-A7D5-8BA2567ED486/d/PostHog Code.app/Contents/Resources/app.asar", | ||
| exePath: "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| expected: true, | ||
| }, | ||
| { | ||
| case: "exePath is translocated", | ||
| appPath: "/Applications/PostHog Code.app/Contents/Resources/app.asar", | ||
| exePath: | ||
| "/private/var/folders/yf/xx/AppTranslocation/C6283C3C/d/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| expected: true, | ||
| }, | ||
| { | ||
| case: "neither path is translocated (/Applications)", | ||
| appPath: "/Applications/PostHog Code.app/Contents/Resources/app.asar", | ||
| exePath: "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| expected: false, | ||
| }, | ||
| { | ||
| case: "neither path is translocated (/Users)", | ||
| appPath: "/Users/dev/PostHog Code.app/Contents/Resources/app.asar", | ||
| exePath: "/Users/dev/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| expected: false, | ||
| }, | ||
| ])("$case → $expected", ({ appPath, exePath, expected }) => { | ||
| expect(isMacosAppTranslocationPath(appPath, exePath)).toBe(expected); | ||
| }); | ||
| }); | ||
|
|
||
| describe("parseDarwinMountTable", () => { | ||
| it.each<{ | ||
| case: string; | ||
| input: string; | ||
| expected: DarwinMountEntry[]; | ||
| }>([ | ||
| { | ||
| case: "standard macOS mount lines", | ||
| input: `/dev/disk3s1s1 on / (apfs, sealed, local, read-only, journaled) | ||
| /dev/disk7s1 on /Volumes/My Dmg (apfs, local, read-only, journaled) | ||
| /dev/disk5s1 on /Volumes/Writable (apfs, local, journaled) | ||
| `, | ||
| expected: [ | ||
| { | ||
| mountPoint: "/", | ||
| options: "apfs, sealed, local, read-only, journaled", | ||
| }, | ||
| { | ||
| mountPoint: "/Volumes/My Dmg", | ||
| options: "apfs, local, read-only, journaled", | ||
| }, | ||
| { mountPoint: "/Volumes/Writable", options: "apfs, local, journaled" }, | ||
| ], | ||
| }, | ||
| { | ||
| case: "mount point name contains ' (' — anchors to trailing options", | ||
| input: | ||
| "/dev/disk9s1 on /Volumes/My Backup (2) (apfs, local, read-only, journaled)\n", | ||
| expected: [ | ||
| { | ||
| mountPoint: "/Volumes/My Backup (2)", | ||
| options: "apfs, local, read-only, journaled", | ||
| }, | ||
| ], | ||
| }, | ||
| ])("parses: $case", ({ input, expected }) => { | ||
| expect(parseDarwinMountTable(input)).toEqual(expected); | ||
| }); | ||
| }); | ||
|
|
||
| describe("isMacosPathOnReadOnlyNonRootMountFromTable", () => { | ||
| const baseTable = `/dev/disk3s1s1 on / (apfs, sealed, local, read-only, journaled) | ||
| /dev/disk7s1 on /Volumes/ReadOnlyVol (apfs, local, read-only, journaled) | ||
| /dev/disk5s1 on /Volumes/Writable (apfs, local, journaled) | ||
| `; | ||
| const nestedTable = `/dev/x on / (apfs, read-only) | ||
| /dev/y on /Volumes/RW (apfs, local, journaled) | ||
| /dev/z on /Volumes/RW/nested (apfs, local, read-only) | ||
| `; | ||
|
|
||
| it.each([ | ||
| { | ||
| case: "path under read-only / is ignored (Users)", | ||
| table: baseTable, | ||
| path: "/Users/me/app", | ||
| expected: false, | ||
| }, | ||
| { | ||
| case: "path under read-only / is ignored (Applications)", | ||
| table: baseTable, | ||
| path: "/Applications/Foo.app", | ||
| expected: false, | ||
| }, | ||
| { | ||
| case: "read-only non-root volume", | ||
| table: baseTable, | ||
| path: "/Volumes/ReadOnlyVol/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| expected: true, | ||
| }, | ||
| { | ||
| case: "writable non-root volume", | ||
| table: baseTable, | ||
| path: "/Volumes/Writable/out/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| expected: false, | ||
| }, | ||
| { | ||
| case: "nested read-only mount wins over writable parent", | ||
| table: nestedTable, | ||
| path: "/Volumes/RW/nested/app", | ||
| expected: true, | ||
| }, | ||
| { | ||
| case: "writable parent wins when no deeper match", | ||
| table: nestedTable, | ||
| path: "/Volumes/RW/other/app", | ||
| expected: false, | ||
| }, | ||
| ])("$case → $expected", ({ table, path, expected }) => { | ||
| expect(isMacosPathOnReadOnlyNonRootMountFromTable(path, table)).toBe( | ||
| expected, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("isMacosPackagedUnsafeBundleLocation", () => { | ||
| const writableMountTable = `/dev/disk3s1s1 on / (apfs, sealed, local, read-only, journaled) | ||
| /dev/disk5s1 on /Volumes/build (apfs, local, journaled) | ||
| /dev/disk6s1 on /Applications (apfs, local, journaled) | ||
| `; | ||
| const readOnlyMountTable = `/dev/disk3s1s1 on / (apfs, sealed, local, read-only, journaled) | ||
| /dev/disk7s1 on /Volumes/ReadOnlyVol (apfs, local, read-only, journaled) | ||
| `; | ||
|
|
||
| it.each<{ | ||
| case: string; | ||
| appPath: string; | ||
| exePath: string; | ||
| readMountTable: ReadDarwinMountTable; | ||
| expected: boolean; | ||
| }>([ | ||
| { | ||
| case: "translocated bundle", | ||
| appPath: | ||
| "/private/var/.../AppTranslocation/UUID/d/PostHog Code.app/Contents/Resources/app.asar", | ||
| exePath: "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| readMountTable: () => writableMountTable, | ||
| expected: true, | ||
| }, | ||
| { | ||
| case: "ordinary non-translocated path on a writable mount", | ||
| appPath: | ||
| "/Volumes/build/out/PostHog Code.app/Contents/Resources/app.asar", | ||
| exePath: | ||
| "/Volumes/build/out/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| readMountTable: () => writableMountTable, | ||
| expected: false, | ||
| }, | ||
| { | ||
| case: "bundle on a read-only non-root volume", | ||
| appPath: | ||
| "/Volumes/ReadOnlyVol/PostHog Code.app/Contents/Resources/app.asar", | ||
| exePath: | ||
| "/Volumes/ReadOnlyVol/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| readMountTable: () => readOnlyMountTable, | ||
| expected: true, | ||
| }, | ||
| { | ||
| case: "mount table cannot be read (degrade to non-blocking)", | ||
| appPath: "/Applications/PostHog Code.app/Contents/Resources/app.asar", | ||
| exePath: "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| readMountTable: () => null, | ||
| expected: false, | ||
| }, | ||
| ])("$case → $expected", ({ appPath, exePath, readMountTable, expected }) => { | ||
| expect( | ||
| isMacosPackagedUnsafeBundleLocation(appPath, exePath, readMountTable), | ||
| ).toBe(expected); | ||
| }); | ||
|
|
||
| it("short-circuits on translocation without reading the mount table", () => { | ||
| const readMountTable = vi.fn(() => writableMountTable); | ||
| isMacosPackagedUnsafeBundleLocation( | ||
| "/private/var/.../AppTranslocation/UUID/d/PostHog Code.app/Contents/Resources/app.asar", | ||
| "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code", | ||
| readMountTable, | ||
| ); | ||
| expect(readMountTable).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
138 changes: 138 additions & 0 deletions
138
apps/code/src/main/utils/macos-packaged-install-guard.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| import { execFileSync } from "node:child_process"; | ||
| import path from "node:path"; | ||
|
|
||
| const APP_TRANSLOCATION_SEGMENT = "AppTranslocation"; | ||
| const MOUNT_READ_TIMEOUT_MS = 3000; | ||
|
|
||
| export type DarwinMountEntry = { | ||
| mountPoint: string; | ||
| options: string; | ||
| }; | ||
|
|
||
| /** | ||
| * Reads the Darwin mount table. Returns `null` when the table cannot be | ||
| * obtained (e.g. `/sbin/mount` is missing, times out, or exits non-zero). | ||
| */ | ||
| export type ReadDarwinMountTable = () => string | null; | ||
|
|
||
| /** Parse `/sbin/mount` lines: `<device> on <mountPoint> (<opts>)` */ | ||
| export function parseDarwinMountTable(output: string): DarwinMountEntry[] { | ||
| const entries: DarwinMountEntry[] = []; | ||
| for (const line of output.split("\n")) { | ||
| const onMarker = line.indexOf(" on "); | ||
| if (onMarker === -1) continue; | ||
| const afterOn = line.slice(onMarker + 4); | ||
| // `lastIndexOf` anchors to the trailing options block, so mount points | ||
| // whose display names contain " (" (e.g. "/Volumes/My Backup (2)") still | ||
| // parse correctly. The `line.endsWith(")")` check guarantees those parens | ||
| // really are the options. | ||
| const openParen = afterOn.lastIndexOf(" ("); | ||
| if (openParen === -1 || !line.endsWith(")")) continue; | ||
| const mountPoint = afterOn.slice(0, openParen); | ||
| const options = afterOn.slice(openParen + 2, -1); | ||
| entries.push({ mountPoint, options }); | ||
| } | ||
| return entries; | ||
| } | ||
|
|
||
| function mountOptionsImplyReadOnly(options: string): boolean { | ||
| return options.toLowerCase().includes("read-only"); | ||
| } | ||
|
|
||
| function longestMatchingMount( | ||
| resolvedPath: string, | ||
| entries: DarwinMountEntry[], | ||
| ): DarwinMountEntry | null { | ||
| let best: DarwinMountEntry | null = null; | ||
| for (const e of entries) { | ||
| const mp = e.mountPoint; | ||
| // For `/` we'd otherwise build `//` which no real path starts with, so the | ||
| // root mount would silently drop out of the comparison and the | ||
| // `best.mountPoint === "/"` guard below would be unreachable. | ||
| const under = | ||
| resolvedPath === mp || | ||
| resolvedPath.startsWith(mp === "/" ? "/" : `${mp}/`); | ||
| if (!under) continue; | ||
| if (!best || mp.length > best.mountPoint.length) { | ||
| best = e; | ||
| } | ||
| } | ||
| return best; | ||
| } | ||
|
|
||
| /** | ||
| * True when `resolvedAbsolutePath` sits on a **non-root** mount that `mount(8)` | ||
| * reports as read-only (e.g. many DMGs, some external volumes). | ||
| * | ||
| * Ignores read-only `/` — on sealed macOS the system volume is read-only while | ||
| * normal apps under /Applications or /Users still work. | ||
| */ | ||
| export function isMacosPathOnReadOnlyNonRootMountFromTable( | ||
| resolvedAbsolutePath: string, | ||
| mountTable: string, | ||
| ): boolean { | ||
| const normalized = path.resolve(resolvedAbsolutePath); | ||
| const entries = parseDarwinMountTable(mountTable); | ||
| const best = longestMatchingMount(normalized, entries); | ||
| if (!best || best.mountPoint === "/") { | ||
| return false; | ||
| } | ||
| return mountOptionsImplyReadOnly(best.options); | ||
| } | ||
|
|
||
| /** | ||
| * Reads `/sbin/mount` synchronously. A short timeout keeps a hung NFS/SMB | ||
| * share from freezing app startup — the exact failure mode this guard exists | ||
| * to prevent. Returns `null` on any failure so callers can degrade to "don't | ||
| * block". | ||
| */ | ||
| function readDarwinMountTableSync(): string | null { | ||
| try { | ||
| return execFileSync("/sbin/mount", { | ||
| encoding: "utf8", | ||
| maxBuffer: 10 * 1024 * 1024, | ||
| timeout: MOUNT_READ_TIMEOUT_MS, | ||
| }); | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * True when either path is under macOS App Translocation (read-only runtime). | ||
| * Caller should gate on packaged darwin before using this to block startup. | ||
| */ | ||
| export function isMacosAppTranslocationPath( | ||
| appPath: string, | ||
| exePath: string, | ||
| ): boolean { | ||
| return ( | ||
| appPath.includes(APP_TRANSLOCATION_SEGMENT) || | ||
| exePath.includes(APP_TRANSLOCATION_SEGMENT) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Packaged macOS: translocated bundle path, or binary on a non-root read-only | ||
| * mount (see mount(8)). | ||
| * | ||
| * `readMountTable` is injectable so tests can drive the mount-table branch | ||
| * deterministically instead of relying on the host's real `/sbin/mount`. | ||
| */ | ||
| export function isMacosPackagedUnsafeBundleLocation( | ||
| appPath: string, | ||
| exePath: string, | ||
| readMountTable: ReadDarwinMountTable = readDarwinMountTableSync, | ||
| ): boolean { | ||
| if (isMacosAppTranslocationPath(appPath, exePath)) { | ||
| return true; | ||
| } | ||
| const table = readMountTable(); | ||
| if (table === null) { | ||
| return false; | ||
| } | ||
| return isMacosPathOnReadOnlyNonRootMountFromTable( | ||
| path.resolve(exePath), | ||
| table, | ||
| ); | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please parametrize these tests <3