Repository navigation
Conversation
electron-builder renames Electron's executables but never relinks them, so every binary in a packaged Maka.app keeps the LC_UUID burned in by Electron's own build — identical across every Electron 43 app. macOS 15+ local-network privacy (NECP) identifies a program by its main executable's LC_UUID, so Maka shares a privacy identity with all of them, leaks cross-app network consent, and never gets an entry of its own in system settings. Add an afterPack hook (it runs before signing, so the signature covers the rewritten bytes) that replaces the 16-byte LC_UUID payload of the main and every helper executable with UUIDv5 (SHA-1, RFC 4122) over the app id, the executable file name and the slice's CPU type. Derivation is deterministic: repackaging yields the same identity and the rewrite is idempotent. The file length and every other byte stay untouched; Windows and Linux are unaffected. Fixes apache#6026 Generated-by: GLM-5.3-Flash (ZCode)
|
First-round CI red is mine, and it is fixed in |
…act lane The CI test planner asserts that every test file named in `check:release` reaches a lane that runs that gate; `scripts/macos-lc-uuid.test.mjs` matched no release-contract path, so the planner failed the job. Add the suite to RELEASE_CONTRACT_FILES like its sibling packaged-app suites. Generated-by: GLM-5.3-Flash (ZCode)
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Reviewed all six changed files at 0fa32d6d08870d8ba41a839b306273bcde8ef0af against main dc4515cd6e7637e8267d59cd4f71ad53f403f9e7. I found two P2 issues.
The motivation has independent support: Apple documents that local-network privacy uses the main executable UUID and that sharing it between apps can cause incorrect behavior. TN3179 This establishes the UUID requirement; I did not reproduce the macOS privacy prompt or settings behavior on this host.
- Native build identity and its symbol correspondence are lost. The replacement depends only on app ID, executable name and CPU type, so an Electron upgrade cannot produce a different UUID for a different native image. Through the actual configured hook, official Electron 43.4.1 and 43.5.0 arm64 main executables, renamed to
Maka, changed from4c4c4470-5555-3144-a124-518576dd11e3and4c4c44b3-5555-3144-a163-485ef1d5d71frespectively to the samea16a76f5-7ae2-54c5-9b4b-0032949b1d91. All four helper names also collapsed across those versions. The official 43.5.0 Breakpad symbol IDs matched all five original executables and matched none after rewriting. There is no corresponding symbol rewrite or UUID mapping here. Keep a native-build discriminator and preserve a way to obtain matching symbols for the rewritten identities. Apple explains why distinct native images need distinct build UUIDs and why symbol lookup depends on them in TN3178. - The new test file fails the formatting gate. Exact-head CI run 39454504056 fails in
Check formatting, namingscripts/macos-lc-uuid.test.mjs. The local full format check reproduces this sole error across 2,504 files. The CI-planner registration commit fixes the earlier missing-suite registration, but the current head is still failing. Format the new test and rerun CI.
What passed:
- 51 original tests covering UUID rewriting, Desktop release targets and CI planning; fresh dependencies/patching; the normal clean root test build; all five Desktop type checks; and the normal renderer build.
- The actual configured hook processed 14 files: ten official thin executables from the two Electron releases, plus four FAT32/FAT64 combinations of official 43.5.0 x64/arm64 main and helper executables. An independent parser verified that only the UUID payloads changed, lengths and executable modes were retained, a second invocation was byte-identical, and Windows/Linux calls left a nonexistent output directory untouched. Bypassing the hook makes this check fail. Python's UUIDv5 independently matched the five rewritten arm64 identities.
- Inspection of the installed electron-builder implementation confirms that both ordinary and universal packaging invoke this hook before signing. Targeted lint, the ASF header check,
git diff --check, and the unchanged protocol epoch 225 passed. The trial merge with current main is clean.
I did not run or sign a macOS package, execute these binaries, notarize an artifact, or symbolicate a live crash. The symbol finding is an observed identifier mismatch with the official symbol files; it does not claim that every native crash frame is affected. The Electron Framework library itself is outside this hook's executable rewrite. Linux packaging and the Windows/macOS Host-owner jobs are green, but they do not establish macOS package or privacy behavior.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Second, independent review, written at a816f92d before reading the review above and posted at 0fa32d6d. The only change since is the one-line RELEASE_CONTRACT_FILES registration in scripts/ci-test-plan.mjs, which fixes the planner failure noted below; the formatting failure on the current head is covered in the review above. On the fixed-UUID point (item 4 below), this review originally rated it P3. The review above reproduced it on official Electron 43.4.1/43.5.0 binaries and cites Apple TN3178, so it is tracked there as P2; treat item 4 as superseded by that finding.
Reviewed range: dc4515cd..a816f92d (one commit, 5 files, +397/-5). This is a first review, so there are no earlier findings to carry forward. Since then the branch has moved to 0fa32d6d, which adds only a one-line entry to RELEASE_CONTRACT_FILES in scripts/ci-test-plan.mjs. The range-diff shows a816f92 itself is unchanged. No protocol files are touched, so there is no epoch impact.
Verdict: The approach and the parser look correct. The only blocking-grade problem was a CI failure on a816f92, and the follow-up commit fixes it. The remaining findings are P3.
What I checked
- Mach-O parsing. Fat magic is read little-endian: 0xbebafeca / 0xbfbafeca correspond to FAT_MAGIC / FAT_MAGIC_64 on disk. The slice offset is read big-endian at arch+8 for both
fat_arch(20 bytes) andfat_arch_64(32 bytes). Thin slices must be MH_MAGIC_64 (little-endian), and anything else throws.ncmdsis read at +16, load commands start at +32, and acmdsizebelow 8 is rejected. Out-of-range reads throw a RangeError from Buffer, so a truncated or corrupt file fails loudly rather than being misparsed. 32-bit and big-endian thin Mach-O are rejected; Electron never ships them. - Real binaries. I ran the module against the fat x86_64+arm64
fsevents.nodefrom the fsevents@2.3.3 tarball. It found both LC_UUIDs, changed exactly 32 bytes (the two 16-byte payloads) and gave each slice a distinct UUID. A Go-built thin arm64 binary (esbuild) has no LC_UUID, and the module correctly reports zero locations. - Which files are rewritten. Only regular files under
*.app/Contents/MacOSare touched, for the main app and every nested*.apphelper. Symlinks are skipped because Dirent does not follow them, soVersions/Currentis not walked twice.Electron Framework, the dylibs,chrome_crashpad_handler, ShipIt and native addons keep their original UUIDs. That is enough for NECP, which keys on the main executable; network traffic goes through the Maka Helper utility process, which is rewritten. - Ordering with code signing. In app-builder-lib 26.17.0 (
platformPackager.doPack) the order isemitAfterPack(this hook), then sanity check, then fuses, thendoSignAfterPack. The rewrite therefore breaks Electron's upstream ad-hoc/linker signature, and osx-sign then re-signs with--force, hardened runtime and entitlements.mac.forceCodeSigning: truemeans no macOS packaging path can skip signing and ship a binary with a broken signature. The-c.mac.identityoverride inpackage-macos-autoupdate-next.mjsstill signs. Notarization only sees the final, consistently signed bundle. - UUID derivation. The new UUID is UUIDv5 of (appId, executable basename, cputype). The test vector matches Python's
uuid5. The result is deterministic, idempotent, and distinct per helper and per slice. Release and nightly sharecom.maka.desktop, so they share UUIDs, which matches their shared bundle id. - Failure behavior. Every failure path throws, so the build fails rather than silently skipping. This covers: no
.appfound, an unsupported magic, a corruptcmdsize, an executable with no LC_UUID, and an ENOENT from a.appwithoutContents/MacOS. Non-darwin builds return before touching anything. - Mergeability and CI. GitHub reports the PR as MERGEABLE, with merge state BLOCKED pending checks and review.
Findings
-
P2 (PR-caused CI failure at a816f92; fixed at 0fa32d6). In CI run 39453659232 on a816f92, the
testjob fails in "Test CI planner" withscripts/macos-lc-uuid.test.mjs reaches no releaseContract. The cause is that the suite was added tocheck:releasebut not toRELEASE_CONTRACT_FILES. Commit 0fa32d6 adds exactly that entry, and the checks on the new head were still pending when I looked. -
P3: the buffer-level tests pass even if the rewrite does nothing.
rewrittenLike(test lines 90-100) writes the expected UUID intorewrittenbefore it callsrewriteBufferUuids. I replaced the.copy(buffer, offset)inrewriteBufferUuidswith a no-op. With that change the thin, fat and idempotence tests still pass, and only the afterPack directory test (thin only) fails. The effect is that the fat-binary rewrite has no test that would catch a regression. Copy the expected bytes intoexpectedonly, and add a FAT_MAGIC_64 fixture. -
P3: the rewrite has not been exercised on a real packaged Maka.app. No PR workflow packages macOS; the Linux and Windows release checks return early from this hook. The first real run will therefore be a release or nightly build. It would help to package locally and confirm three things:
dwarfdump --uuidshows the new UUIDs onMakaand eachMaka Helper*;codesign --verify --deep --strictpasses;spctl -a -vvpasses.
A further option is an assertion in
verify-packaged-app/verify-macos-dmgthat the main executable's UUID equalsdeterministicExecutableUuid(...). -
P3: one UUID is reused across every Maka version. This is what keeps the NECP identity stable across updates, which is presumably what you want. The cost is that different binaries now share a UUID. Electron's published dSYMs no longer match the main and helper executables, and crash-report tooling cannot tell versions apart by UUID. The impact is small because those executables are thin stubs and the code that matters stays in
Electron Framework, which is not rewritten. I found nocrashReporterusage inapps/desktop/src. The trade-off is worth a sentence in the module comment. -
P3: the module comment claims more than the code does, and the walk has some loose edges.
- The header comment says "every Mach-O in a packaged macOS app bundle", but only
Contents/MacOSexecutables are rewritten. findAppBundleExecutableswalks the whole tree, includingContents/Resources. Any third-party*.appthat ships there would also be rewritten, and a.appdirectory withoutContents/MacOSfails the build with ENOENT. Nothing in the current closure ships one.- The LC_UUID
cmdsizeis not checked to be 24, andncmdsis not checked againstsizeofcmds. Both only matter for corrupt input.
- The header comment says "every Mach-O in a packaged macOS app bundle", but only
|
Second red also mine, fixed in |
biome format over `scripts/macos-lc-uuid.test.mjs`: three assertions the formatter would reflow. `npm run format:check` is clean on the full tree. Generated-by: GLM-5.3-Flash (ZCode)
|
Heads-up on the third red, which is not from this branch: the |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Reviewed 77eeeea14df3fa890e6effdd5e49aa1608151bf2, including the complete increment from 0fa32d6d08870d8ba41a839b306273bcde8ef0af. This commit only reformats three assertions in scripts/macos-lc-uuid.test.mjs; the production hook and UUID algorithm are unchanged. The formatting P2 is fixed. The native-build UUID P2 remains. I found no additional P0–P2 issue in this increment.
At scripts/macos-lc-uuid.mjs:92, the replacement still depends only on app ID, executable name and CPU type. Rerunning the actual configured hook on 14 Mach-O files reproduced the previous results exactly: official Electron 43.4.1 and 43.5.0 arm64 main executables have different original UUIDs but both become a16a76f5-7ae2-54c5-9b4b-0032949b1d91; the four helper names also collide across those releases. The official 43.5.0 Breakpad symbol IDs match 5/5 original executables and 0/5 rewritten executables. There is still no corresponding symbol rewrite or UUID mapping. Preserve a native-build discriminator and matching symbol identity. Apple TN3178 explains the build-UUID and symbol correspondence requirement. The Electron Framework library is outside this hook, so this does not claim every native crash frame is affected.
Validation passed: all 51 original tests for UUID rewriting, release targets and CI planning; the full format check across 2,504 files; fresh dependencies and patches; the normal clean root test build; all five Desktop type checks; the normal renderer build; git diff --check; and protocol epoch 225. The 14-file hook probe covers ten official thin executables and four FAT32/FAT64 combinations, and verifies UUID-only changes, preserved size/mode, idempotence and Windows/Linux no-op behavior. The trial merge with main dc4515cd6e7637e8267d59cd4f71ad53f403f9e7 is clean.
Exact-head test CI 39514754250 succeeds. Audit CI 39514754248 fails on advisories for @modelcontextprotocol/client, @modelcontextprotocol/server, follow-redirects and hono. The lockfile, workspace dependency manifests and audit implementation match current main; main pins the same affected versions. I did not run a separate main audit. This is not a new failure caused by the assertion-formatting increment, and the overall checks are not all green.
I did not execute a macOS app, reproduce the privacy prompt, build/sign/notarize a macOS package, or symbolicate a live crash. The remaining P2 is supported by actual executable and official symbol identifier comparisons.
|
Following up on my Oct 20 heads-up (the third red): instead of waiting for a separate dependency-bump PR, I merged Verification (local, macOS): |
Review round on 77eeeea: the replacement UUID took only the app id, executable name and CPU type, so every Electron dist rewritten to the same app advertised one identical build UUID and crash reports could no longer pick the right native image (Apple TN3178). The slice's ORIGINAL UUID now feeds the derivation as the native-build discriminator — two Electron builds stay distinguishable, the same dist stays stable across repackaging — and the afterPack hook writes lc-uuid-map.json next to the bundle mapping every original UUID to its replacement so the published Electron dSYMs still symbolicate the rewritten executables. The header comment now states the actual scope (regular files under Contents/MacOS; frameworks, dylibs and helpers like crashpad keep Electron's UUIDs) and the dSYM trade-off. The suite no longer pre-writes the expected bytes into the rewritten buffer (a no-op rewrite used to pass the thin/fat/idempotence assertions — verified red with a no-op probe), and the idempotence test is honest about its contract: the original UUID feeds the hash, so the stable identity is guaranteed per pristine Electron dist, not across rewrite-of-rewrite. Generated-by: GLM-5.3-Flash (ZCode)
|
All four findings are addressed in
One honest contract change: with the original UUID inside the hash, a rewritten buffer fed through the rewrite again derives a different (still stable) identity, so the old "idempotent" test is now "repackaging the same Electron dist yields the same identity" — which is what the hook's real flow guarantees (every build starts from the pristine dist). Verification: |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.
Reviewed 465ec36, separating the main merge from the two-file UUID/test fix. The previous native-build UUID collision P2 is fixed. The original slice UUID now participates in derivation, and the hook records original/replacement pairs. I found no new P0–P2 issue in this increment.
I ran the actual configured afterPack hook on ten retained official Electron 43.4.1/43.5.0 arm64 executables (SHA-256 rechecked) and four FAT32/FAT64 combinations. All five main/helper names now remain distinct across the two releases. Only UUID bytes change; file size and executable mode remain intact. Repackaging pristine copies is deterministic, and Windows/Linux are no-ops. Restoring only the old derivation makes the cross-version separation assertion fail.
For all five 43.5.0 executables, the recorded original identifiers match the official Breakpad symbol headers. The rewritten identifiers themselves still match none of those unmodified symbol files: a mapping is diagnostic data, not automatic symbolication. Apple TN3178 describes the build UUID's role in matching native images and debug symbols.
P3 — retain the map for release diagnostics. The hook writes lc-uuid-map.json beside the bundle, but current Nightly staging copies only declared payloads and the update feed; it does not preserve this file. Add a durable diagnostic-artifact path if this map is the intended recovery mechanism. Also update the description's old three-input/idempotence wording: the implementation now uses the original UUID and guarantees repeatability from pristine distributions, not repeated hashing of already rewritten files.
All 55 focused UUID/release-target/CI-planner/Nightly-stage tests, formatting, ASF headers, whitespace and the protocol guard passed; epoch 227 is unchanged. The static merge with current main is clean. Exact-head test, audit, Linux/Windows package and Host-owner platform checks pass. I did not repeat the full workspace build/typecheck at this script-only increment, package/sign/notarize a macOS app, observe local-network privacy behavior, or symbolicate a live crash. The macOS Host-owner job does not validate those package behaviors.
github-actions
Bot
added
effort/L
|
Responding to the two loose ends in your 10-22 review:
|
Generated-by: GLM-5.3-Flash (ZCode)
|
Both red lanes on the merge trace to one upstream issue, not to this branch: |
|
The upstream sync merge resolved the 43ae3cf restores the pin to 7.30.1, matching upstream/main (eeeae51, #6236) byte for byte. The lockfile already resolved 7.30.1 and is byte-identical to the one at the merge's upstream parent, so no lock changes were needed; |
The upstream sync merge eecb655 resolved the package.json conflict by keeping the stale branch-side pin, regressing the @jackwener/opencli@1.8.8 undici override from 7.30.1 back to 7.29.1, while package-lock.json kept the upstream 7.30.1 resolution. That left package.json and the lockfile inconsistent (npm ci would fail). Restore the pin to 7.30.1, matching upstream/main (eeeae51, apache#6236) byte for byte. The lockfile already resolves 7.30.1 and is byte-identical to the one at the merge's upstream parent, so no lock changes are needed; npm install confirms the tree is in sync. Generated-by: GLM-5.3-Flash (ZCode)
|
Heads-up on CI: the |
Summary
Packaged macOS apps kept Electron's own Mach-O
LC_UUIDon every executable: electron-builder renames but never relinks, so all Electron 43 apps shared one identity (e.g.4C4C4470-5555-3144-A124-518576DD11E3). macOS 15+ local-network privacy (NECP) keys programs by the main executable's UUID, so Maka collided with every other Electron app — cross-app network-consent bleed and no settings entry of its own.This adds an
afterPackhook (runs before code signing) that rewrites the 16-byte UUID payload of the main and every helper executable with UUIDv5 overappId/executableName/cpuTypeplus the slice's original UUID — the native-build discriminator (Apple TN3178), so two Electron builds stay distinguishable and repackaging the same dist is stable. It also writeslc-uuid-map.jsonnext to the bundle mapping every original UUID to its replacement for crash diagnostics; changes no other byte, and never runs for Windows/Linux.Verification
node --test scripts/macos-lc-uuid.test.mjsnode --test scripts/desktop-release-targets.test.mjsnpm run format:checknpm run check:asf-headersapps/desktopnpm run typecheck/bin/lscopy) +dwarfdump --uuiduuid.uuid5cross-checkAI use
Implemented by GLM-5.3-Flash (ZCode); human-reviewed.
Checklist
Fixes #6026