Skip to content

fix(ci): drop dangling symlinks before signing the macOS bundle - #24

Merged
pacnpal merged 1 commit into
mainfrom
claude/macos-signing-injection-rvlnbq
Aug 20, 2026
Merged

pacnpal merged 1 commit into
mainfrom
claude/macos-signing-injection-rvlnbq

Conversation

@pacnpal

@pacnpal pacnpal commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Follow-up to #23. macOS release builds currently fail at the signing step — with or without signing secrets configured, so this blocks every macOS release on main today, not just signed ones.

Signing dist/floppybootcd.app with: ***
xattr: No such file: dist/floppybootcd.app/Contents/Frameworks/PySide6/glue
xattr: No such file: dist/floppybootcd.app/Contents/Frameworks/PySide6/include
xattr: No such file: dist/floppybootcd.app/Contents/Frameworks/PySide6/typesystems
xattr: No such file: dist/floppybootcd.app/Contents/Frameworks/PySide6/support
xattr: No such file: dist/floppybootcd.app/Contents/Frameworks/PySide6/scripts
Error: Process completed with exit code 1

Cause

The Prune unused Qt and PySide6 resources step deletes typesystems/, include/, glue/, support/ and scripts/ from Contents/Resources/PySide6. PyInstaller also cross-links each of those entries from Contents/Frameworks/PySide6, so pruning one side leaves the other pointing at nothing.

xattr -cr stats through a symlink — on a broken one it prints No such file and exits non-zero, which set -e turns into a failed job. The five paths in the log are exactly the five pruned directories.

Fix

Sweep broken symlinks out of the bundle before anything walks it, rather than tolerating xattr's exit code:

  • codesign can't seal a link to a missing target either, and Apple's notary service rejects them — suppressing the xattr failure would just move the error later.
  • They resolve to nothing, so deleting them costs nothing.
  • It runs on the ad-hoc path too, so bundles built without secrets stop shipping five links to nowhere.

Valid symlinks (e.g. Frameworks/PySide6/Qt) are untouched.

Testing

Reproduced against a bundle shaped like the real post-prune layout, with stubbed xattr/codesign/file/security that mimic the macOS tools (xattr stats through links and fails on broken ones):

Case Result
Pre-fix script fails with the same five paths as CI, exit 1
Post-fix, Developer ID path drops exactly the 5 broken links, keeps the valid Qt one, signs, exit 0
Post-fix, ad-hoc path (no secrets) same, exit 0
Post-fix, re-run on a clean bundle no-op, nothing reported, exit 0

Still unverified without a Mac: that the resulting bundle notarizes and launches. This unblocks the smoke test that surfaced the bug.


Generated by Claude Code

macOS release builds fail at the signing step, with or without signing
secrets configured:

    xattr: No such file: .../Contents/Frameworks/PySide6/glue
    xattr: No such file: .../Contents/Frameworks/PySide6/include
    ...
    Error: Process completed with exit code 1

The prune step deletes unused PySide6 payload (typesystems/, include/,
glue/, support/, scripts/) from Contents/Resources, but PyInstaller also
cross-links each of those entries from Contents/Frameworks. Pruning one
side leaves the other pointing at nothing, and `xattr -cr` stats through
a symlink — it reports "No such file" and exits non-zero on a broken one,
which `set -e` turns into a failed job.

Sweeping the broken links is the fix rather than tolerating xattr's exit
code: codesign cannot seal a link to a missing target either, and Apple's
notary service rejects them. They resolve to nothing, so removing them
costs nothing — and it happens on the ad-hoc path too, so the bundles
built without secrets stop shipping five links to nowhere.

Reproduced against a bundle shaped like the real post-prune layout with
stubbed macOS tooling: the pre-fix script fails with the same five paths
as CI, the fixed one drops exactly the broken links, keeps the valid
Qt one, signs, and is a no-op on re-run. Verified on both the Developer
ID and ad-hoc paths.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDuaD9Tgz6PT1yUypGzAmU
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for security reviews. Please try again later.

@coderabbitai

coderabbitai Bot commented Aug 20, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9e38fe41-75b8-428a-8990-1221cfa6d40c

📥 Commits

Reviewing files that changed from the base of the PR and between 0d7bb3d and 26340b2.

📒 Files selected for processing (1)
  • .github/signing/macos-sign.sh

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Improved macOS app signing reliability by automatically detecting and removing dangling links from the application bundle before signing.
    • Added reporting of any removed items during the signing process.

Walkthrough

The macOS signing script now removes dangling symlinks from the app bundle before clearing extended attributes and signing. It logs each removal and reports the total count.

Changes

macOS signing preparation

Layer / File(s) Summary
Pre-signing symlink cleanup
.github/signing/macos-sign.sh
The script scans the app bundle for broken symlinks, removes them with logging, counts removals, and reports the count before signing passes.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 26340

The PR removes only dangling symlinks before macOS bundle signing while preserving valid links and covering both signed and ad-hoc builds; no actionable merge-blocking risk remains after normal checks and review.

Poem

Broken links wobble in the night,
The signing script makes them right.
Each stray thread is neatly swept,
The bundle’s tasty, clean, and prepped.
Then signatures dance in light.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the macOS signing fix for dangling symlinks.
Description check ✅ Passed The description directly explains the signing failure, root cause, fix, and testing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/macos-signing-injection-rvlnbq

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pacnpal
pacnpal merged commit 2e2f655 into main Aug 20, 2026
11 checks passed
@pacnpal
pacnpal deleted the claude/macos-signing-injection-rvlnbq branch August 20, 2026 16:24
pacnpal added a commit that referenced this pull request Aug 20, 2026
* fix(ci): prune Qt's mangled nested .app bundles before signing

macOS release builds still fail at the signing step, now further in —
past the dangling symlinks fixed in #24, after ~60 binaries sign
successfully:

    .../PySide6/Assistant__dot__app/Contents/MacOS/Assistant: replacing existing signature
    .../Assistant__dot__app/Contents/MacOS/Assistant: the main executable
        or Info.plist must be a regular file (no symlinks, etc.)

PyInstaller splits the bundle across Contents/Frameworks (binaries) and
Contents/Resources (data), cross-linking each side, and relocates Qt's
nested tool bundles into Frameworks under a mangled "<name>__dot__app"
name. The directory is real but its Contents/Info.plist is a symlink
into Resources. codesign, handed a Mach-O at <X>/Contents/MacOS/<n>,
signs <X> as a bundle — and refuses outright when that bundle's
Info.plist is a symlink.

Assistant is Qt's help viewer. This app links QtCore/QtGui/QtWidgets
only, and QtHelp — the module that would drive Assistant — is already
excluded from the build, so nothing can launch it. Pruning the mangled
nested bundles fixes signing and drops dead weight from every macOS
artifact. QtWebEngineProcess is spared by name: it is the one nested
helper Qt needs at runtime, if WebEngine ever returns to the build.

macos-sign.sh now also names the file and explains this specific failure
instead of dying on a bare codesign error, so the next occurrence is
self-diagnosing.

Verified against a bundle reproducing the real Frameworks/Resources
split, with a codesign stub that refuses a symlinked Info.plist the way
Apple's does: the pre-fix script fails with CI's exact message, and with
the prune the Assistant bundle is removed from both roots, the WebEngine
helper survives and signs, and the bundle signs clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDuaD9Tgz6PT1yUypGzAmU

* fix(ci): make the WebEngine carve-out honest instead of a booby trap

Review finding, and a fair one: sparing QtWebEngineProcess__dot__app
protected nothing. That helper has the same PyInstaller-split layout and
symlinked Contents/Info.plist as Assistant, so if it were ever present
the signing pass would walk into it and fail with the identical codesign
error this change exists to fix — the carve-out only guaranteed the
failure it claimed to prevent. My own test fixture hid this by giving
the helper a real Info.plist, which is not how PyInstaller lays it out.

Not pruning it either, though: Qt needs it at runtime, so pruning trades
a loud build failure for a silently broken WebEngine at the user's end.
It cannot appear today (every WebEngine module is excluded), so the
build now stops with an explanation of what re-enabling WebEngine
actually requires — rebuilding the helper into a signable layout — which
is the one path that neither ships something broken nor fails later
inside codesign.

Tested both branches with a fixture that reproduces the real layout:
Assistant-only prunes and signs clean, and a re-enabled WebEngine helper
fails in the prune step with the explanation rather than in codesign.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDuaD9Tgz6PT1yUypGzAmU

* fix(ci): make a failed nested-bundle scan fatal instead of silent

Partly adopting a review finding. The finding's premise — that
`-maxdepth` is GNU-only and unsupported by Apple's find(1) — is not
right: it is a BSD primary, documented in macOS's own find(1) man page,
inherited from FreeBSD, and this workflow already relies on it a few
lines up. No change needed there.

The second half of the finding is correct and worth fixing, though: a
find(1) failure inside a process substitution does not propagate under
set -e. Whatever the cause, a scan that failed would sweep nothing, the
step would report success, and the build would die later inside codesign
with nothing pointing back here. Capturing via command substitution puts
find's exit status back in reach, so a failed scan stops the build where
the problem is.

Branches covered by the fixture: Assistant-only prunes and signs clean;
a WebEngine helper stops the build with the explanation and is left in
place; a bundle with no nested apps is a clean no-op; and a find that
fails now exits 1 with a diagnostic where it previously reported success
with the bundle untouched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JDuaD9Tgz6PT1yUypGzAmU

---------

Co-authored-by: Claude <noreply@anthropic.com>
pacnpal added a commit that referenced this pull request Aug 28, 2026
* release: v1.3.2 — signed and notarized macOS builds

The Developer ID signing + notarization pipeline landed in #23 (with
follow-up fixes in #24 and #25) after v1.3.1 was cut, so every published
macOS zip is still the old ad-hoc-signed build. Nothing ships it until a
`v*` tag is pushed: release.yml only publishes assets on a tag push, and
the 2026-08-20 dispatch smoke-test on this commit confirmed sign +
notarize + staple succeed on macos-arm64, macos-x86_64, and
macos-universal2.

- Bump the package version to 1.3.2. It had been left at 1.2.6 through
  the v1.3.0 and v1.3.1 tags, and it is not cosmetic: `__version__` is
  read from installed package metadata and stamped into every disc's
  boot menu (`MENU TITLE FloppyBootCD vX.Y.Z`), so discs built from
  v1.3.1 identify themselves as v1.2.6. Also surfaced by `--version`
  and the About dialog.
- Rewrite the macOS install docs now that a signed build exists to point
  at. README, the Pages site, and `.postbeep/docs.md` all told every
  macOS user to strip the quarantine flag; that step is unnecessary from
  v1.3.2 on, and `.postbeep/docs.md` (rendered on POSTBEEP.NET) still
  described the app as flatly "unsigned". Each now leads with the plain
  unzip-and-open path plus the `spctl` check, and keeps the `xattr`
  workaround scoped to v1.3.1 and older.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016NNtwTQvf8G9hEuEsHwQBH

* fix(ci): fail a tag build closed when signing credentials are missing

Review caught a real gap: the docs added in the previous commit promise
that every macOS artifact from v1.3.2 on is signed and notarized, but
release.yml could publish an ad-hoc-signed one. macos-import-cert.sh and
macos-notarize.sh both exit 0 when their secrets are absent, and the
"Locate signing tooling" step only warns when the tooling is missing —
so a release cut with a lapsed or misconfigured secret would ship a
bundle Gatekeeper blocks, under a promise that it wouldn't.

That graceful degradation is right for fork PRs and dispatch smoke-tests
and is kept. It is wrong for a release, which is the one trigger that
publishes. Introduce a single MACOS_SIGNING_REQUIRED contract, set at
the workflow level for pushed v* tags only, and honour it everywhere the
fallback lives:

- macos-import-cert.sh, macos-sign.sh, macos-notarize.sh: each turns its
  clean no-op into a hard failure with a message naming the missing
  secret.
- "Locate signing tooling" (both the matrix job and universal2): missing
  tooling is fatal on a tag build, since every signing step below is
  gated on its output and would otherwise silently skip.

A release now either carries a real Developer ID signature and a stapled
ticket, or it doesn't happen — which is what makes the docs' claim
unconditional rather than best-effort.

Also from review:

- docs/index.html: the spctl example checked the relative path, but the
  block moves the app to /Applications first, so following it in order
  made the check fail. Point it at the installed path.
- README: the legacy xattr command sat in the same copy-pasteable block
  as the current-release steps, so a v1.3.2 user working top to bottom
  still stripped quarantine. Move it into a collapsed
  "Downloading v1.3.1 or older?" section.
- .github/signing/README.md: document the new contract.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016NNtwTQvf8G9hEuEsHwQBH

---------

Co-authored-by: Claude <thewesker@gmail.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

2 participants