Conversation
release.sh has known how to notarize since it was written, but the workflow pinned PLONK_SIGN_IDENTITY to "Plonk Signing", so the branch could never be reached from CI. This wires the other path up and leaves it dormant: set APPLE_DEVELOPER_ID_P12 and the release switches over on its own, unset it and the run is byte-for-byte the one that happens today. Notarization matters here for more than the Gatekeeper warning. TCC keys Accessibility and Screen Recording to the designated requirement, and the current one pins a self-signed root by its own SHA-1. Regenerate that certificate, or let it lapse, and every installed copy loses both permissions at once with nothing to say why. A Developer ID pins Apple's root and a team ID, which do not move. - notarytool credentials come from an App Store Connect key in CI, since store-credentials cannot be answered on a runner. The keychain-profile path a laptop uses is untouched, and release.sh takes whichever it was handed. - The image is packed by hdiutil and nothing else, after stapling, so the bundle a user drags to Applications carries its own ticket. It is signed and notarized in its own right, because the quarantine bit lands on the .dmg. - Both artifacts get a provenance attestation; only the zip did before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two review passes over the previous commit. Everything here came out of them. - The requirement check now runs before notarization, not after. Apple is the slow and finite half of the script, and the first Developer ID release is precisely the build that fails this check — it should not spend a submission to find that out. Nothing about the check changed, only when it happens. - The workflow header no longer claims there is "nothing to remember on release day". There is exactly one thing, and it now says what it is and where the build will stop. - Release notes and the step summary name each artifact next to its own digest, and carry an attestation command for both. Publishing the zip's sha256 under a .dmg is worse than publishing none: someone checks it, sees a mismatch, and concludes the download was tampered with. - make-dmg.sh no longer prints a digest. It was computed before release.sh staples the image, which rewrites it, so the log carried two hashes for one file and the first matched nothing. - make-dmg.sh refuses when the app's signing authority is not the identity the image would be signed with. Each signature verifies on its own, so nothing downstream notices a Developer ID app wrapped in a self-signed image — and a run by hand does that by default. - The image is now mounted and the bundle inside it verified, deep and strict and against the recorded requirement, the same round trip the zip already got. That an image mounts says nothing about whether what a user drags out of it will run. - The publish step iterates a positional list rather than splitting a string, so the loops do not depend on the shell's word-splitting rules. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
Ran the review twice over
The rest: a digest printed before stapling rewrote the file, no guard against Both passes confirmed the extra |
make-dmg.sh was 52% comment, the highest in the repo, on a script of forty lines. The style is the house one and most of it earns its place, but some of it was explaining the product rather than the code. - Dropped the header paragraph arguing why a .dmg is worth shipping at all. That belongs in the pull request, not above the script that builds it. - Dropped the second explanation of ditto-versus-cp, six lines after the first. - One line, not three, for why no digest is printed here. - The HFS+ comment claimed an APFS image would not mount on older macOS, which is not the reason and reads as though nobody was sure. The real one is that stating the filesystem keeps the artifact from changing shape the day the runner's macOS does. The workflow header had grown to 44 lines, half of it saying twice over what release.sh already says once about API keys and the requirement switch. Ratios now sit where the rest of the repo does: 38%, 35% and 33% against build-mcpb.sh at 45% and lint.sh at 34%. Comments explaining a real trap — ditto and extended attributes, --identifier and the filename truncation, the two-signature guard — are untouched. No code changed. lint, shellcheck, YAML parse and a full make-dmg.sh run all still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Why
scripts/release.shhas been able to notarize since it was written. It nevercould from CI: the workflow set
PLONK_SIGN_IDENTITY: Plonk Signing, whichforces the self-signed branch every time. This wires the other branch up.
The Gatekeeper warning is the visible half. The half that costs more is
invisible: TCC keys Accessibility and Screen Recording to the bundle's
designated requirement, and ours currently pins a self-signed root by its own
SHA-1.
If that certificate is ever regenerated, lost, or lapses, every installed copy
drops both permissions with nothing on screen to explain it, and no copy can
update to anything again. A Developer ID pins Apple's root plus a team ID —
neither of which moves when a certificate is rotated.
Dormant until the secrets exist
Nothing changes until
APPLE_DEVELOPER_ID_P12is set. Without it the run is theone that happens today: self-signed, no notarization, zip only, same
attestation. With it, the workflow switches paths by itself — no flag, nothing
to remember on release day.
APPLE_DEVELOPER_ID_P12APPLE_DEVELOPER_ID_PASSWORDAPPLE_NOTARY_KEY.p8, base64APPLE_NOTARY_KEY_IDAPPLE_NOTARY_ISSUERAn API key rather than an Apple ID and app-specific password:
notarytool store-credentialscannot be answered on a runner, and a key is revocable on itsown without touching the account.
What it does
add-trusted-cert; a Developer ID does not, since inventing trust for acertificate that already has a real chain would be a lie about its origin.
a Developer ID carries an account and team that nothing here knows in advance.
notary()inrelease.shpicks the API key when it is there and the keychainprofile otherwise, so the laptop path is untouched.
scripts/make-dmg.shpacks the image withhdiutiland nothing else. Theusual
create-dmgwrapper drives Finder over AppleScript for icon placement,which needs a logged-in session a runner does not have — and it would be the
only build dependency in a repo that has none.
its own ticket, and is then notarized and stapled itself, because the
quarantine bit lands on the
.dmgand that is what Gatekeeper reads first.RUNNER_TEMP,chmod 600, and removed in analways()step so a failed build does not leave it behind.Verified locally
make-dmg.shwas run against the current self-signed build — the whole pathexcept the two Apple round trips, which need an account that does not exist yet:
Plonk.appand anApplicationssymlinkcodesign --verify --deep --strictpasses on the app inside, and it stillsatisfies its designated requirement —
dittokeeps the signature thatcp -Rwould have strippedscripts/release-requirementlint.shclean,shellcheck -s shclean, workflow YAML parses, all tenappsteps ordered as intended.
Found and fixed on the way: left to itself
codesignderives a disk image'sidentifier from the filename and stops at the first dot, so
Plonk-0.2.3.dmgsigned as
Plonk-0— a different identifier on every minor bump. Set todev.plonk.dmgexplicitly.Before this can be merged and used
This is why it is a draft.
on anything.
scripts/release-requirementhas to be updated in the same commit thatadds the secrets.
release.shwill refuse the build otherwise, by design andwith a clear message. The new value is not knowable until the Team ID exists.
designated requirement, so copies signed with
Plonk Signingwill not acceptthe update and have to be reinstalled by hand, with both permissions granted
again — exactly what 0.0.5 did.
SECURITY.mdalready makes the argument fordoing this early: the bill scales with how many people are running it, and
the latest release currently has 2 downloads.
README.mdsays "signed, but notnotarized", and
SECURITY.mdexplains there is no paid account. Bothbecome false the moment this turns on, and AGENTS.md requires they be fixed
alongside. Not done here, because doing it now would make them false in the
other direction.
CHANGELOG.mddeliberately untouched. Nothing user-visible changes whilethe secrets are absent; the entry belongs to the release that flips it on.
Unrelated, but found while reading
The bundle signs with
--options runtimeand no entitlements at all(
SECURITY.mddocuments this and gives a command to check). Under the hardenedruntime, microphone access normally needs
com.apple.security.device.audio-input. Push-to-talk may be relying onsomething that will not hold on a clean machine. Left out of this PR on purpose
— it changes runtime behaviour and would make the
SECURITY.mdclaim false, soit deserves its own change with a real test behind it, not a drive-by.