Repository navigation
Conversation
Squirrel.Mac now verifies the optional `sha256` (lowercase hex) and `size` (bytes) of the update ZIP when they are present in `updateTo`. Always write them for new entries; older Electron versions ignore the extra keys. Also fix the `macUpdateReleaseNotes` TSDoc, which referred to a nonexistent `squirrelMacManifestBaseUrl` option. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TwQrz6fWPnhLV1spvJA8Sj
Add a `macUpdateDelta` option that, when making for darwin on a macOS host with `macUpdateManifestBaseUrl` set, creates a Sparkle BinaryDelta patch from the manifest's current release to the new one and offers it as `updateTo.delta` in RELEASES.json. Squirrel.Mac (Squirrel/Squirrel.Mac#328, electron/electron#52820) downloads it instead of the ZIP when the running app's CFBundleVersion matches `from_version`, and falls back to the ZIP on any failure. Older Electron versions ignore the key. The maker downloads the previous ZIP (verifying its recorded sha256 and size), extracts it with ditto, reads CFBundleVersion from its Info.plist for `from_version`, refuses to continue if either app has group- or other-writable files (Squirrel.Mac strips those on install, so the delta would only work once), then runs `BinaryDelta create --version 4 --compression lzma` and self-checks the patch with `BinaryDelta apply` and `codesign --verify --deep --strict`. The `.delta` file is written next to the ZIP and returned as an artifact so publishers upload it beside the ZIP. BinaryDelta comes from `binaryDeltaPath`, or else from the Sparkle 2.9.5 release archive (the version Squirrel.Mac pins), verified against a pinned SHA-256 and cached in ~/Library/Caches/electron-forge. Failures warn and publish without a delta unless `strict` is set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TwQrz6fWPnhLV1spvJA8Sj
Describe `macUpdateDelta` in the ZIP maker docs: how the delta is made, the RELEASES.json `delta` entry, how to strip group/other write permissions before signing with a `packageAfterCopy` hook, and the limitations. Show the new `sha256`/`size` fields in the example manifest, and fix that example's code block title (it is RELEASES.json, not forge.config.js) and ZIP file name. Link to the new section from the S3 auto-update guide. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TwQrz6fWPnhLV1spvJA8Sj
erickzhao
changed the title
The delta tests run the real group/other-writable preflight against temporary app bundles. Windows has no POSIX permission bits (Node reports writable files as 0o666 and chmod cannot clear them), so the preflight rejected every bundle and the tests failed. Delta creation only runs on macOS hosts, so skip these tests on Windows; Linux and macOS still cover them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TwQrz6fWPnhLV1spvJA8Sj
Instead of skipping the macUpdateDelta tests on Windows hosts, report the file modes the group/other-writable preflight sees from the test (0o755/0o644, or 0o664 for paths marked group-writable) rather than the host file system. Windows has no POSIX permission bits and reports every writable file as 0o666, which made the preflight reject every test bundle. The tests now pass with Windows-like modes on disk. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TwQrz6fWPnhLV1spvJA8Sj
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Beyond the confirmed inline finding, I also checked two other candidate issues and ruled them out: fetching the previous release's URL from the remote RELEASES.json in makeMacDelta isn't a new trust boundary, since that manifest already controls the real update URLs/checksums Squirrel.Mac itself trusts; and the catch block's fs.rm(deltaPath, { force: true }) on a failed retry only removes a locally re-buildable build artifact (not the already-published delta), so it isn't a real data-loss bug on its own.
Extended reasoning...
The change adds a new mac-delta.ts module that downloads and shells out to Sparkle's BinaryDelta and codesign, verifies checksums of downloaded content, and wires this into MakerZIP.make()/makeMacDelta; it touches external-binary invocation, remote downloads, and filesystem permission checks, which is meaningfully security- and correctness-sensitive surface. A confirmed race condition in the shared BinaryDelta cache under concurrent multi-arch make() is already being posted inline, which alone means a human should look; I additionally traced the remote-URL trust boundary and the delta-file cleanup-on-failure path from the diff and found both bounded/low-severity rather than independently blocking.
|
I think we should publish the |
`forge make` runs the makers for each architecture concurrently in one process, so with an empty cache several MakerZIP instances could download Sparkle at once and copy BinaryDelta into the same `BinaryDelta.<pid>.partial` file. Interleaved writes could corrupt it, and the corrupt binary would then stay cached. Share one in-flight download per cache path within the process (forgotten if it fails, so a later make retries), and give each partial file a unique name so separate processes cannot collide either. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TwQrz6fWPnhLV1spvJA8Sj
There was a problem hiding this comment.
I reviewed this PR again and found no new bugs. The concurrent-download race I flagged in the earlier version looks fixed by e83dca4 (memoizes one in-flight download promise per cache path, and copies to a UUID-named partial file before renaming, so two same-process makers or two hosts can no longer clobber each other's copy). Given this still touches security-sensitive surface — downloading and executing an external binary (Sparkle's BinaryDelta) and verifying code signatures — a human look remains worthwhile before merging.
- Verified the race fix directly in
mac-delta.ts:getBinaryDelta()now shares one promise percachedPathviabinaryDeltaDownloads, anddownloadBinaryDelta()writes to${cachedPath}.${randomUUID()}.partialinstead of a pid-based name. - Checked download integrity:
downloadFile/digestFilevalidate SHA-256 and size before the Sparkle archive or previous release ZIP is trusted. - Checked delta validation:
createBinaryDeltaapplies the new delta back to a copy of the old app and runscodesign --verifybefore the delta is published.
Extended reasoning...
This is a second pass on a PR adding binary-delta support for Squirrel.Mac auto-updates in packages/maker/zip (mac-delta.ts, Config.ts, MakerZIP.ts, plus ~400 lines of new tests). It touches security-relevant surface: downloading and executing an external binary (Sparkle's BinaryDelta) fetched over the network, and verifying code signatures on the resulting patched app. A prior run of this reviewer flagged a real concurrency bug (concurrent same-process makers could corrupt the cached BinaryDelta binary); commit e83dca4 fixes it with a per-cache-path in-flight-download map and a UUID-named partial file, which I re-verified by reading the current code. No new bugs were found this run, but given the size, security-sensitive surface, and that a third-party comment (MarshallOfSound, content not visible to this reviewer) landed in the same window as the fix, a human should still take a final look.
Sure, that makes sense. I can do it as a new |
Add @electron-forge/binary-delta, which exports the path to the BinaryDelta executable from Sparkle 2.9.5 (the Sparkle release that Squirrel.Mac pins). The package only installs on macOS (`os: darwin`). Its prepack script downloads the Sparkle release archive, checks it against a pinned SHA-256, and copies bin/BinaryDelta and Sparkle's LICENSE into the package, so the binary is fetched once at publish time and never on users' machines, and never lives in git. The Verdaccio test setup publishes every package locally and never makes deltas, so it skips the download with ELECTRON_FORGE_SKIP_BINARY_DELTA_FETCH. Also generate TypeScript project references for optionalDependencies, so that makers can depend on the package optionally. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TwQrz6fWPnhLV1spvJA8Sj
Instead of downloading Sparkle from GitHub releases and caching BinaryDelta in ~/Library/Caches at build time, depend optionally on @electron-forge/binary-delta, which npm installs on macOS. This drops the runtime download, the cache and the concurrent-download handling. `binaryDeltaPath` still overrides it. If the package or its binary is missing, the delta fails with a clear message (and the release ships without it unless `strict` is set). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TwQrz6fWPnhLV1spvJA8Sj
|
@MarshallOfSound done in ea239b6 and f3c50cb:
Per CONTRIBUTING, a Forger will need to publish the first version of Generated by Claude Code |
There was a problem hiding this comment.
I reviewed this PR and didn't find any bugs. The concurrent-download race I flagged in an earlier pass on this PR no longer applies: BinaryDelta is now shipped as a prebuilt binary in the new @ electron-forge/binary-delta package (fetched once at publish time, verified against a pinned SHA-256) rather than downloaded at build time, so getBinaryDelta() just resolves an already-installed path with no shared mutable download/rename path to race on. Because this is still a large feature touching code-signing verification, binary patching, and a new package's publish pipeline, a human look would still be worthwhile.
- Checked
mac-delta.ts'sgetBinaryDelta/createBinaryDelta: no runtime download, delta creation is followed by an apply-and-codesign self-check. - Checked
fetch-binary-delta.ts: Sparkle archive is downloaded and SHA-256-verified before extracting theBinaryDeltabinary at pack time. - Checked
MakerZIP.ts's delta flow: previous-ZIP checksum verification, group/other-writable file checks, and strict vs. non-strict failure handling.
Extended reasoning...
This PR adds macOS binary-delta ZIP update support: a new @ electron-forge/binary-delta package that ships Sparkle's BinaryDelta binary (fetched and SHA-256-verified at publish time), and a mac-delta.ts module in maker-zip that downloads/verifies the previous release, checks file permissions, creates the delta, and validates it via apply-and-codesign. Security-sensitive surface: external binary download with checksum verification and code-signature verification, no injection/auth exposure. Since my last review the design was changed (in response to a race condition I flagged and a maintainer comment) from a runtime download with an in-process cache to shipping a prebuilt binary via the package registry, which structurally removes the concurrency race; no other findings were reported this round. Given the size and security-adjacent nature of the change, human review is still worthwhile even though no bugs were found.
Summary
Adds Squirrel.Mac binary delta updates to
@electron-forge/maker-zip, and a new macOS-only package,@electron-forge/binary-delta, that ships Sparkle'sBinaryDeltatool.When
macUpdateDeltais enabled, the ZIP maker builds a delta from the release thatRELEASES.jsoncurrently lists to the one being made. It then records that delta in the manifest so Squirrel.Mac can download a small patch instead of the full ZIP.Changes
@electron-forge/maker-zipupdateToentry inRELEASES.jsonnow carries the ZIP'ssha256(lowercase hex) andsize.macUpdateDelta: boolean | { binaryDeltaPath?, strict? }— when enabled on a darwin target:sha256/size.ditto.go-wwhen it installs an update, so a delta built from writable files wouldn't apply.CFBundleVersionwithplutilto use asfrom_version.BinaryDelta create --version 4 --compression lzma. Squirrel.Mac applies formats 3 and 4 with any compression except bzip2.BinaryDelta applyonto the previous app, thencodesign --verify --deep --stricton the patched result.<name>-<prev>-to-<version>.deltanext to the ZIP and addsupdateTo.delta = { from_version, url, sha256, size }. The.deltafile is returned as a make artifact, so publishers upload it.strict: truethe make throws instead.BinaryDeltacomes from:binaryDeltaPathif set; otherwise@electron-forge/binary-delta, which is an optional dependency. If that package isn't installed or its binary isn't executable, the error explains why.@electron-forge/binary-delta(new,os: ["darwin"])binaryDeltaPathandSPARKLE_VERSION(2.9.5).prepackscript downloadsSparkle-2.9.5.tar.xzand checks it against a pinned SHA-256. It then extracts onlybin/BinaryDelta(chmod 755) and Sparkle's license, which ship in the tarball. Users download nothing at install time.ELECTRON_FORGE_SKIP_BINARY_DELTA_FETCHskips the download. The Verdaccio test publish sets it, since those tests never make deltas.Repo tooling
tools/gen-tsconfigs.tsnow includesoptionalDependenciesin project references.yarn.config.cjsallows the new optional dependency.knip.jsongains a workspace entry for the new package.Docs
docs/config/makers/zip.md:RELEASES.jsonexample (title, ZIP name,sha256/size).packageAfterCopyhook that makes the bundle non-group/other-writable before signing, and limitations. Only one delta is created per release, from the immediately previous version, and a macOS host is required.docs/config/publishers/s3.mdxlinks to the delta section.Notes for reviewers
@electron-forge/binary-deltais a new package name, so someone with publish rights must publish it to npm by hand the first time.2.9.5(commit79bc9e8).ditto,plutil,BinaryDeltaandcodesign. To check it by hand:macUpdateDelta: true.deltaentry and that the self-check passed..deltaand relaunches as the second version.Testing
packages/maker/zip/spec/MakerZip.spec.tscovers:sha256/sizein the manifest, and the delta entry's contents.BinaryDeltacreate/apply andcodesignarguments.from_versionread from the plist.BinaryDeltafailure, with and withoutstrict.BinaryDelta, and a missing packaged binary.File modes are controlled through an
lstatspy, so the tests pass on Windows as well.packages/utils/binary-delta/spec/fetch-binary-delta.spec.tscovers the binary path, a SHA-256 mismatch and a download HTTP error.I have read the contribution documentation for this project.
I agree to follow the code of conduct that this project follows, as appropriate.
The changes are appropriately documented (if applicable).
The changes have sufficient test coverage (if applicable).
The testsuite passes successfully on my local machine (if applicable).
🤖 Generated with Claude Code
https://claude.ai/code/session_01TwQrz6fWPnhLV1spvJA8Sj