Skip to content

fix(release): Sparkle appcast fails closed when the signing key is not the app's SUPublicEDKey - #27732

Merged
teamleaderleo merged 7 commits into
feat-cmux-nextfrom
lane/fix-release-b-2906
Oct 31, 2026
Merged

teamleaderleo merged 7 commits into
feat-cmux-nextfrom
lane/fix-release-b-2906

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes an internal report: the Sparkle appcast fallback signed the feed with a key the app does not trust.

What was wrong

scripts/sparkle_generate_appcast.sh ran sign_update -p --ed-key-file and injected the signature whenever generate_appcast left the feed unsigned. generate_appcast leaves it unsigned exactly when the private key's public key is not the app's SUPublicEDKey, so a wrong key produced a feed that looked signed and passed the script's only check (a grep for sparkle:edSignature), while every installed app rejects the update. The fallback also added a second length attribute to an enclosure that already had one, which makes the feed unparsable, and used stat -f%z (macOS only).

Fix

  • Before anything is generated (and before delta prebuilding), derive the public key of SPARKLE_PRIVATE_KEY (scripts/ci/dev_app_linux.py sparkle-public-key through openssl, falling back to scripts/derive_sparkle_public_key.swift where openssl is LibreSSL), mount the DMG copy read-only and read the app's SUPublicEDKey. Fail closed on a mismatch, a missing SUPublicEDKey, or a key that cannot be derived.
  • Fallback: verify the injected signature with sign_update --verify and replace the enclosure's length with the DMG's exact size instead of adding a duplicate.
  • Final check: parse the feed and require the DMG's single enclosure to carry an edSignature and a length equal to the DMG's byte size. A failing feed is removed from the output path.

Callers checked (all run on macOS, pass the DMG whose app the key must match, and need no change): release.yml, nightly.yml, auto-resume-nightly-notarization.yml (4 calls), repair-v0-64-25-helper-rpaths.yml, scripts/build-sign-upload.sh, scripts/ci/nightly-steps/recover-generate-appcast.sh, scripts/ci/generate-nightly-deltas.sh. No environment, secret or workflow permission changes.

Red/green

bash tests/test_sparkle_generate_appcast_no_deltas.sh (runs on Linux with stub generate_appcast, sign_update and hdiutil; the keys are the RFC 8032 Ed25519 test vectors). New cases: mismatched key (signed and unsigned generator), app without SUPublicEDKey, a non-Ed25519 private key, a wrong enclosure length, a fallback signature that fails --verify, and the matching-key fallback (one length, the DMG's size, signature verified).

  • Base (test commit d996772): FAIL: mismatch: missing 'does not match the app's SUPublicEDKey' (the fallback signed the feed).
  • Head: all three PASS lines.

Observed / Not run

Observed: the test above red at the test commit and green at the head on Linux bash 5.2; prepush-gate.py reports no new violations. Not run: a real macOS run of hdiutil attach and the real Sparkle 2.8.1 sign_update --verify (the fallback path only runs when generate_appcast did not sign, which the up-front key check now makes rare).

Risk

High tier (release/signing). A release, nightly or repair run whose private key does not match the built app now fails at appcast generation instead of publishing a feed nobody can install. Matching keys (the normal case, since every caller embeds the key derived from the same private key) behave as before, plus one read-only DMG mount and a key derivation (openssl, or one swift script run on hosts with LibreSSL).

Changelog

none

Agent session: agent session

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes the Sparkle appcast script signing the feed with a key the shipped app does not trust, which published updates every installed app would reject.

The script now derives the public key of SPARKLE_PRIVATE_KEY, mounts the DMG read-only, and compares it to the app's SUPublicEDKey before generating anything. It fails closed on a mismatch, a missing SUPublicEDKey, or an underivable key. The sign_update fallback now verifies the injected signature and replaces the enclosure's length instead of adding a duplicate attribute, which made the feed unparsable. A final check parses the feed and requires the DMG's enclosure to be signed with the DMG's exact byte size, removing failing feeds from the output path.

The nightly deltas test fixture now mounts this build's DMG through a fake hdiutil to serve the key check and writes each archive's real enclosure length. Callers need no changes; matching keys (the normal case) behave as before, plus one read-only DMG mount and a key derivation.

Written for commit d88b047. Summary will update on new commits.

View guided diff Turn on auto-fix

teamleaderleo and others added 2 commits October 30, 2026 00:20
Red on base: with the app's SUPublicEDKey different from the private key's
public key, generate_appcast leaves the feed unsigned and the sign_update
fallback signs it anyway.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-401d85755f96aead5ecd
…trust

generate_appcast leaves the feed unsigned when the private key's public key
is not the app's SUPublicEDKey, and the sign_update fallback then signed it
anyway: a feed that passed the edSignature grep but every installed app
rejects. Before anything is generated, derive the private key's public key
and compare it with SUPublicEDKey of the app inside the DMG; fail on a
mismatch, a missing key or a key that cannot be derived. The fallback now
verifies its signature with sign_update --verify and replaces the enclosure's
length instead of adding a duplicate attribute, and the final check parses the
feed and requires the DMG's enclosure to be signed with the DMG's exact size.

Red/green: bash tests/test_sparkle_generate_appcast_no_deltas.sh
(red at the previous commit, green here).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-401d85755f96aead5ecd
@github-actions github-actions Bot added the risk:high Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 85c56d1: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr27732 -f ref=lane/fix-release-b-2906 -f platform=macos -f backend_mode=local

web and size checks: green

Shipped in:

@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026

Copy link
Copy Markdown

Found 2 test failures on Blacksmith runners:

Failures

Test View Logs
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)/
test_the_repository_highlights_validate (main.DigestTests.test_the_repository_highl
ights_validate)
View Logs
the delta script failed/the delta script failed View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Independent review (codex gpt-6-astra, reasoning high, fast, read-only) of 760ac10..660cb41:

No P1/P2 findings. Sparkle 2.8.1's sign_update --verify invocation is correct, key derivation supports both key formats, and URL/name handling matches all supplied callers.

  • P3: Detach failures are swallowed and app_mount is cleared regardless (scripts/sparkle_generate_appcast.sh:132-133), so a busy image may remain mounted and cleanup cannot retry.

Validation: bash -n, ShellCheck, and direct comparison with Sparkle 2.8.1 sources.

Review (codex gpt-6-astra): SHIP at 660cb41

sparkle_generate_appcast.sh now mounts the new DMG to compare the signing
key with the app's SUPublicEDKey and checks the enclosure length against
the DMG size, so the fixture serves this build's app (with the fake swift's
public key) through a fake hdiutil and writes each archive's real size.
Earlier builds still never mount.

bash tests/test_generate_nightly_deltas.sh: red at 660cb41 (hdiutil:
command not found), green here.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-401d85755f96aead5ecd
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high, next-review): SHIP at 8aedb93

No blocking findings.

cmux-bot[bot]
cmux-bot Bot previously approved these changes Oct 30, 2026

@cmux-bot cmux-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

cmux-bot approves 8aedb93: an independent review passed this exact head (next-review receipt by teamleaderleo). A push needs a new review; a later BLOCK at this head dismisses this approval.

@cmux-bot
cmux-bot Bot dismissed their stale review October 30, 2026 06:26

A newer verdict blocks 85c56d1 (no independent SHIP at the head).

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6.1-sol high fast, review-router; started by the backlog lander): SHIP at 85c56d1

SHIP

  1. P3 — scripts/sparkle_generate_appcast.sh:132: If both normal and forced hdiutil detach fail, the error is ignored and app_mount is cleared, preventing the exit trap from retrying. A busy DMG can remain mounted and accumulate on the CI runner.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Independent review (gpt-6.1-sol high) at b947a5f: SHIP.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Independent review (gpt-6.1-sol high) at 767df57: SHIP.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 30, 2026 23:22 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 30, 2026 23:23 Inactive
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (next-review patch-id; carried): SHIP at d88b047

The independent SHIP at 767df57 carries to this head: the PR's own change (git diff $(git merge-base feat-cmux-next HEAD) HEAD) has the same git patch-id --verbatim (a80f16decb5aefeb) at both heads, and no merge between them was resolved by hand. Nothing new to review. A newer BLOCK, or any change to the PR's own diff, ends the carry.

@teamleaderleo
teamleaderleo merged commit c0fd6da into feat-cmux-next Oct 31, 2026
149 of 154 checks passed
@teamleaderleo
teamleaderleo deleted the lane/fix-release-b-2906 branch October 31, 2026 00:42
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (independent Codex gpt-6.1-sol, high, via the review router; full diff from merge base 3f7ad5a to this head): SHIP at d88b047.

No P1/P2 findings.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — d88b047b Deployed Oct 30, 2026 by cmux-bot[bot] via wait for same-key tree publisher #85664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:high Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant