fix: verify and retry the notary staple instead of assuming it worked - #98
Open
yepzdk wants to merge 2 commits into
Open
fix: verify and retry the notary staple instead of assuming it worked#98yepzdk wants to merge 2 commits into
yepzdk wants to merge 2 commits into
Conversation
`--notarize` could print its full success sequence while leaving the app unstapled. Apple distributes the notary ticket asynchronously — observed accepted at 13:10:03Z, first successful staple at 13:12:14Z — and webwrap stapled once, immediately after `notarytool --wait` returned, with `quiet: true` discarding stapler's explanation and nothing checking the result. A missing stapled ticket is exactly the failure the signing was meant to prevent: Gatekeeper falls back to fetching it online, so the app is refused on an offline or restricted machine. The user ships it believing it's fine, and `spctl` on the signing machine agrees with them — it reports "accepted" even with no ticket, because assessments are cached per developer. Stapling now runs up to six attempts over ~1 minute and every attempt is verified with `stapler validate` rather than trusted on exit status: staple can exit 0 without attaching a ticket, which is how this went unnoticed. On exhaustion it throws a message saying the app IS notarized but NOT stapled, with the copy-pasteable manual fix, and leaves the bundle in place. Retries are unconditional rather than classifying transient vs permanent failures, which would mean matching Apple's undocumented message strings — guessing wrong there reintroduces this bug. The schedule and the failure message are pure statics with unit tests, per the repo's split; the stapling itself stays hand-verified. Closes #97 Co-authored-by: Claude <noreply@anthropic.com>
…l its output Two diagnostics fixes from review of #98. The retry line said "Ticket not available yet", but the loop retries on ANY failed verification — a malformed bundle fails validate with exit 66 for reasons unrelated to Apple's distribution, and would have been told six times over 67s that the ticket wasn't ready. Now "Staple not verified yet", with the failure message hedging the same way and pointing at the captured output for the real answer. The two commands' text was also concatenated unlabelled, and since runCapturingAll merges stdout and stderr there was no way to tell which command said what. Now labelled with each command's exit status, which makes the actual #97 failure legible: staple reports "The staple and validate action worked!" with exit 0 while validate reports "does not have a ticket stapled to it" with exit 65. Labelling is a pure static so the formatting is tested, including that silent commands are dropped rather than left as bare headers. Co-authored-by: Claude <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
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.
webwrap update --sign … --notarize …printed its complete success sequence —Notarized. Stapling ticket…,✓ Updated— while leaving the app unstapled. Found while signing WebReader for distribution to another Mac.Notarization itself had genuinely succeeded (
notarytool history→status: Accepted). Stapling by hand ~2 minutes later worked first try: Apple accepted at 13:10:03Z, and the secure timestamp on the successful manual staple was 13:12:14Z. Apple distributes the ticket asynchronously, so a staple attempted the instantnotarytool --waitreturns often can't find it yet.Why it mattered
A stapled ticket is what lets Gatekeeper approve the app offline. Without one the recipient's Mac must fetch it from Apple at first launch and refuses the app if it can't — precisely the audience the signing was for.
The trap was worse than a plain silent failure: on the machine that signed the app,
spctl -a -vvvreportsaccepted — source=Notarized Developer IDeven with no ticket, because assessments are cached per developer. So the obvious way to double-check agreed with the bug.xcrun stapler validateis the only truthful check, and nothing ran it.Root cause
AppBuilder.swift:489-490— one attempt, no verification, diagnostics discarded:rundoes throw on a non-zero exit, so the observed silent success meansstaplerexited 0 without attaching a ticket. The code couldn't tell the difference — andquiet: truesent both streams tonullDevice, so whatever stapler said about why was unrecoverable.The fix
stapler validate, never trust the staple's exit status. This is the fix — trusting exit 0 leaves the bug reproducible.spctl, and gives the manual fix with the path quoted (app names contain spaces).Retries are unconditional rather than classifying transient vs permanent failures: that would mean matching Apple's undocumented, unversioned message strings, and guessing wrong reintroduces this exact bug. The whole budget is a minute; a permanent failure just reports the honest error a minute later.
No
Processinjection seam was added — the thing worth testing (the schedule, the message) is pure and extractable without one, and a seam for a single call site would be a large diff for a test that asserts the implementation back at itself.Verification
swift test— 368 tests, 0 failures (11 new: schedule shape/budget/monotonicity, and the message's wording, quoting, attempt count, and captured-output handling).Measured the loop's real semantics with
stapler validateagainst both bundle states:validateSo the happy path returns on attempt one with no sleeping — no perceptible slowdown — and exit 65 is exactly the failure the old code ignored. Also confirmed a failed run leaves the bundle in place, still Developer ID signed with the hardened runtime.
Not verified end-to-end: a fresh real notarization. Submissions to
appstoreconnect.apple.com/notary/v2/asptime out from this network (~61s, reproducibly), whileapple.comreturns 200 in 0.4s andnotarytool historyworks — so something here blocks the submission endpoint specifically. That's environmental, not this change, but it means the full sign→notarize→staple round-trip should get one manual run before this is relied on.Spotted but deferred
run(quiet: true)discards stderr forcodesigntoo, so a signing failure surfaces only as/usr/bin/codesign exited with status 1— losing "no identity found" and similar. Same defect class, but it shares a helper with the icon pipeline (11 call sites), so it's a wider blast radius and belongs in its own change rather than riding along in a notarization fix.sips/iconutilare deliberately different: their failures are caught and degrade to the default icon with a visible warning.Closes #97
🤖 Generated with Claude Code