Fail the macOS build if any bundled library links outside the bundle#10135
Open
dpage wants to merge 1 commit into
Open
Fail the macOS build if any bundled library links outside the bundle#10135dpage wants to merge 1 commit into
dpage wants to merge 1 commit into
Conversation
The macOS appbundle is meant to be self-contained: every binary should reference only OS libraries (/usr/lib, /System) or bundle-relative paths (@loader_path/@rpath/@executable_path). An absolute install-name pointing at a build-host location such as Homebrew's /usr/local/opt/openssl@3 or the Jenkins workspace will resolve on the build machine but is absent (or, for a differently-signed dylib, rejected by hardened-runtime library validation) on an end user's Mac, so the app dies on startup. This is exactly how 9.16 broke on Intel (issue pgadmin-org#10123): with no Intel cryptography wheel published upstream, pip compiled it from source and its openssl-sys build linked the build host's Homebrew OpenSSL into _rust.abi3.so, which _fixup_imports deliberately skips, so the dangling reference shipped unnoticed. Add a _verify_bundle_linkage step that runs after the bundle is fully assembled and relocated, and before code-signing, walking every .so/.dylib and failing the build if anything links to a build-host prefix. This turns the whole class of problem from a silent runtime failure on user machines into a hard build failure on the builder.
WalkthroughAdds a ChangesBundle Linkage Verification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/mac/build-functions.sh`:
- Around line 498-508: The Mach-O dependency scan in the bundle check is too
narrow because the loop only inspects files matched by the .so and .dylib
suffixes, so shipped executables and framework binaries can be missed. Update
the file discovery in the build-functions.sh scan to target all Mach-O files
under BUNDLE_DIR, then keep the existing otool -L, grep, and error reporting
logic in the same loop so any host-linked binary is caught regardless of
extension.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e48e57f7-c8e9-440b-bd04-fe6b4372a21a
📒 Files selected for processing (2)
pkg/mac/build-functions.shpkg/mac/build.sh
Comment on lines
+498
to
+508
| while IFS= read -r f; do | ||
| # otool prints the filename header then one line per dependency; drop | ||
| # the header, take the install-name column, and keep only build-host | ||
| # paths. grep exits non-zero (no match) for a clean binary, which is | ||
| # the common case, so fall through to the next file. | ||
| deps=$(otool -L "${f}" 2>/dev/null | tail -n +2 | awk '{print $1}' | \ | ||
| grep -E "^(${PREFIXES})") || continue | ||
| echo "ERROR: ${f} links against build-host libraries:" >&2 | ||
| echo "${deps}" | sed 's/^/ /' >&2 | ||
| found="yes" | ||
| done < <(find "${BUNDLE_DIR}" \( -name '*.so' -o -name '*.dylib' \) -type f) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the verifier is suffix-only and that the macOS packaging logic includes framework payloads.
sed -n '460,520p' pkg/mac/build-functions.sh
rg -n -C2 'Python\.framework|\.framework|Contents/MacOS' pkg/macRepository: pgadmin-org/pgadmin4
Length of output: 8552
🏁 Script executed:
#!/bin/bash
sed -n '460,520p' pkg/mac/build-functions.sh
sed -n '160,220p' pkg/mac/build-functions.sh
sed -n '360,450p' pkg/mac/build-functions.shRepository: pgadmin-org/pgadmin4
Length of output: 10457
🏁 Script executed:
#!/bin/bash
# List Mach-O files in the shipped bundle locations that are not .so/.dylib
# and inspect whether the packaging code already rewrites their imports.
rg -n -C2 'find .*Mach-O 64-bit|otool -L|_fixup_imports|_verify_bundle_linkage' pkg/mac/build-functions.shRepository: pgadmin-org/pgadmin4
Length of output: 2201
Scan all Mach-O binaries, not just .so/.dylib files.
This loop misses shipped executables and framework binaries (for example Contents/MacOS/* and Python.framework payloads), so a bundle can still contain host-linked Mach-O files without tripping this check. Broaden the scan to all Mach-O files, not just suffix-matched libraries.
🧰 Tools
🪛 Shellcheck (0.11.0)
[style] 506-506: See if you can use ${variable//search/replace} instead.
(SC2001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/mac/build-functions.sh` around lines 498 - 508, The Mach-O dependency
scan in the bundle check is too narrow because the loop only inspects files
matched by the .so and .dylib suffixes, so shipped executables and framework
binaries can be missed. Update the file discovery in the build-functions.sh scan
to target all Mach-O files under BUNDLE_DIR, then keep the existing otool -L,
grep, and error reporting logic in the same loop so any host-linked binary is
caught regardless of extension.
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.
Belt-and-braces guard for the macOS appbundle build.
Background
9.16 failed to start on Intel Macs (#10123). The
cryptographypackage no longer publishes a prebuilt wheel for Intel macOS, so pip compiled it from source on the builder, and itsopenssl-sysbuild linked the build host's Homebrew OpenSSL (/usr/local/opt/openssl@3/...) into_rust.abi3.sorather than the OpenSSL we ship inside the bundle._fixup_importsdeliberately skips_rust.abi3.so, so that dangling reference shipped unnoticed and the app died on startup on every machine that lacked (or could not validate) that external dylib.The actual fix lives in the build configuration (force
cryptographyto link the bundled OpenSSL). This PR is the separate safety net so the same class of problem cannot silently reach a release again.Change
Adds a
_verify_bundle_linkagestep, run after the bundle is fully assembled and relocated and before code-signing, that walks every.so/.dylibin the bundle and fails the build if any install-name points at a build-host prefix (/usr/local,/opt/homebrew,/opt/local, or$SLAVE_HOMEon the CI builders). Legitimate OS libraries (/usr/lib,/System) and bundle-relative references (@loader_path/@rpath/@executable_path) pass untouched.A correct, self-contained bundle passes silently; a left-behind external reference turns into a hard build failure on the builder instead of a runtime failure on a user's Mac.
Testing
macOS build scripts only; no runtime code paths affected.
bash -nclean on both files, and the detection pipeline was checked against the realotool -Loutput from the #10123 reports (flags the twoopenssl@3lines, ignores the@rpathself-id and the/usr/libsystem libs).Summary by CodeRabbit