Skip to content

refactor(release): one notarization path, one scratch-directory helper, one plugin build - #2365

Merged
datlechin merged 1 commit into
mainfrom
ci/release-script-duplication
Aug 21, 2026
Merged

refactor(release): one notarization path, one scratch-directory helper, one plugin build#2365
datlechin merged 1 commit into
mainfrom
ci/release-script-duplication

Conversation

@datlechin

Copy link
Copy Markdown
Member

Three duplications in the release scripts. The notarization one had leaked real rigor.

Notarization existed three times, at three rigor levels

staple stapler validate Gatekeeper (spctl) notary log on failure
build-plugin.sh yes yes yes no
create-dmg.sh yes yes no no
build-release.sh yes no no yes

The app, the artifact users actually download, got the weakest checks. The DMG and the plugins got the best diagnostics withheld: when a submission failed, only the app path fetched the notary log that says why.

scripts/lib/notarize.sh is one implementation at the strictest level all three reached between them. All three callers now get every column.

The two post-staple checks are not redundant, which is worth stating because it looks like they are. spctl asks Gatekeeper, and Gatekeeper will fetch the ticket from Apple over the network, so an artifact that was notarized but never successfully stapled still passes. Measured on this machine:

Ghostty.app       -t exec  -> accepted source=Notarized Developer ID
Ghostty.app       stapler  -> Ghostty.app does not have a ticket stapled to it.
Google Chrome.app stapler  -> The validate action worked!

Ghostty is notarized, has no stapled ticket, and spctl accepts it anyway. Only stapler validate proves the ticket is in the artifact, which is what a Mac with no network needs. build-release.sh stapled and never validated, so a staple that silently produced no ticket shipped.

The assessment type is a parameter: exec for the app, open for the DMG and the plugin bundles, since a user opens those rather than launching them.

Also dropped: --apple-id and --team-id on the plugin submission. The keychain profile the apple-signing action stores already carries both plus the password, which is why the app and DMG paths have always worked with --keychain-profile alone. build-plugin.sh additionally required an APPLE_ID env var and failed without it, so the workflow passed a secret that changed nothing.

Verified

I have no Developer ID certificate locally, so the submission itself cannot run here. Two things I could verify, and did:

The spctl and stapler invocations, against genuinely notarized apps (above), and against a non-notarized bundle to confirm the check fails — an ad-hoc signed bundle returns rejected and the grep does not match.

Every branch of the function, with stubbed xcrun/spctl:

bundle, all good           exit=0 want=0 ok | notarized, stapled and accepted
dmg file, -t open          exit=0 want=0 ok | notarized, stapled and accepted
missing path               exit=1 want=1 ok | nothing to notarize
submit exits nonzero       exit=1 want=1 ok | FAKE NOTARY LOG BODY
submit ok but Invalid      exit=1 want=1 ok | FAKE NOTARY LOG BODY
staple fails               exit=1 want=1 ok | stapling failed
validate fails             exit=1 want=1 ok | did not validate
gatekeeper rejects         exit=1 want=1 ok | Gatekeeper still rejects

"submit ok but Invalid" is why the exit-status check alone is not enough: notarytool has shipped versions that exit 0 on a rejected submission, and only build-release.sh was checking the status line. And the argument order in output="$(...)" || submit_status=$? is load-bearing; written the other way round submit_status=$? reads the assignment's status, which is always 0, and the log fetch becomes unreachable. That was already fixed once in build-release.sh and the comment moves with the code.

The plugin workflow built each architecture from scratch

./scripts/build-plugin.sh "$TARGET" arm64  "$VERSION"
./scripts/build-plugin.sh "$TARGET" x86_64 "$VERSION"

The script has always accepted both. Each invocation regenerates both Xcode projects and runs rm -rf build/DerivedData, so the second call threw away everything the first had compiled. Measured here on SQLiteDriverPlugin, one of the smaller ones:

wiped between architectures (CI today)   arm64 41s  x86_64 24s   total 66s
one shared DerivedData ('both')          arm64 24s  x86_64  3s   total 27s

The second architecture costs 24s or 3s depending only on this. Same two zips, same names, same SHA-256 sidecars: both was already the script's default and takes the same code path.

cleanup in five scripts

The roadmap said ten; five is the real count. The other five already use the terse trap 'rm -rf "$WORK"' EXIT and need no function. The five had an identical BUILD_DIR="$(mktemp -d)", a three-line cleanup() and the same trap, plus an identical NCPU=$(sysctl -n hw.ncpu). All five already source lib/macos.sh, so make_build_dir and NCPU go there.

build-freetds.sh keeps its own trap: it removes a tarball cache as well, so it is not the same shape.

Verified

build-libssh2.sh arm64 run end to end after the change. The rebuilt library against the published one:

built     7bcf99d3c1da78d8  arm64  291 text symbols
published ba616a85e4de9800  arm64  291 text symbols
exported symbol sets identical: YES
deployment target: macOS 14.0 (the script's own check)

The archive hash differs because a static archive embeds timestamps; the arch, symbol count and full exported symbol set match. Libs/ was then restored with download-libs.sh --force and re-verified against the checksums committed in git.

The trap fires on a clean exit and on a failure:

ok:   removed
fail: removed

ThirdPartyLicenseInventoryTests scans scripts/build-*.sh and scripts/lib/*.sh for version pins, and I broke it once with a change of this shape. The pin set is byte-identical before and after: 10 pins, no diff.

shellcheck -x --severity=warning and bash -n clean on every touched script; actionlint clean.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin
datlechin merged commit 2c93cf3 into main Aug 21, 2026
5 of 7 checks passed
@datlechin
datlechin deleted the ci/release-script-duplication branch August 21, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant