Repository navigation
Strip any embedded framework without a valid dynamic-library executable - #8245
Conversation
Build 20260716043221 was still rejected with ITMS-90208 after the ar-archive-only strip: Xcode's export-time distribution processing can remove a static SPM binaryTarget's executable and leave an invalid framework shell (Info.plist, no binary) in Frameworks/, which the previous check's [[ -f ]] guard skipped. The keep policy is now a whitelist: an embedded framework stays only if its executable (per its own CFBundleExecutable) exists and is a dynamically linked Mach-O; static archives, stripped shells, and any other blob are removed, gated on the app executable not referencing the framework in its dynamic load commands. The pre-strip Frameworks/ state and each framework's file(1) kind are logged so any future ASC rejection comes with ground truth. verify_ipa_framework_minimum_os_versions enforces the same whitelist on the final IPA for the automatic-signing path. Verified locally against all three states (static archive from the real iroh-ffi 1.0.2-cmux.2 artifact, shell without binary, real dylib). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe upload script now resolves embedded framework executables from ChangesiOS framework handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR closes a gap in the static-framework strip guard for the manual-resign TestFlight path: Xcode's export-time distribution processing can fully remove a static SPM binaryTarget's executable and leave an invalid framework shell (Info.plist with no binary), which the previous
Confidence Score: 4/5Safe to merge; the whitelist-based strip and verifier correctly handle all three invalid-framework cases (static ar, missing binary, non-dylib Mach-O) and the otool -L safety gate prevents stripping a framework the app executable genuinely links against. The logic of the fix is sound: replacing the blacklist with a whitelist closes the shell-framework gap that slipped past the previous guard. The strip loop and the verifier now use the same string ("dynamically linked shared library") and the same PlistBuddy-driven binary-path resolution. The only notable roughness is the double ios/scripts/upload-testflight.sh — specifically the verifier function around lines 168–172 where Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Embedded .framework found] --> B[Read CFBundleExecutable from Info.plist]
B --> C{Binary file exists?}
C -- No --> D[embedded_fw_kind = <executable missing>]
C -- Yes --> E[file -b binary → embedded_fw_kind]
D --> F{Is dynamically linked shared library?}
E --> F
F -- Yes --> G[vtool -show-build → log build info]
G --> H[Keep framework, continue]
F -- No --> I{otool -L app_exe references framework?}
I -- Yes --> J[ERROR: refusing to strip — exit 1]
I -- No --> K[Strip: rm -rf embedded_fw]
K --> L[After loop: rmdir Frameworks/ if empty]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[Embedded .framework found] --> B[Read CFBundleExecutable from Info.plist]
B --> C{Binary file exists?}
C -- No --> D[embedded_fw_kind = <executable missing>]
C -- Yes --> E[file -b binary → embedded_fw_kind]
D --> F{Is dynamically linked shared library?}
E --> F
F -- Yes --> G[vtool -show-build → log build info]
G --> H[Keep framework, continue]
F -- No --> I{otool -L app_exe references framework?}
I -- Yes --> J[ERROR: refusing to strip — exit 1]
I -- No --> K[Strip: rm -rf embedded_fw]
K --> L[After loop: rmdir Frameworks/ if empty]
Reviews (1): Last reviewed commit: "Strip any embedded framework without a v..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if ! file -b "$framework_binary" | grep -q 'dynamically linked shared library'; then | ||
| echo "error: $framework_name is embedded in the app bundle but its executable is not a dynamic library ($(file -b "$framework_binary")); ASC rejects this (ITMS-90208). Strip it from Frameworks/ (static code is already linked into the app executable)." >&2 | ||
| rm -rf "$workdir" | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
file -b invoked twice in the error path — once to drive the grep check and again inside the error string. If the framework binary is a large or slow-to-probe file this doubles the I/O, but more importantly the two calls happen at slightly different times and could in theory produce different output (e.g. on a modified-during-scan temp directory). Capturing the output once in a variable — as the strip loop already does with embedded_fw_kind — keeps the verifier consistent with the pattern established 30 lines above and avoids the double stat.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Follow-up to #8235. Build 20260716043221 (run https://github.com/manaflow-ai/cmux/actions/runs/29471458156, which contained the ar-archive-only strip) was still rejected with ITMS-90208: Xcode's export-time processing can remove a static SPM binaryTarget's executable and leave an invalid framework shell (Info.plist, no binary) in
Frameworks/, which the[[ -f ]]guard skipped.The keep policy is now a whitelist: an embedded framework stays only if its executable exists and is a dynamically linked Mach-O. Static archives, stripped shells, and anything else are removed (gated on the app executable not referencing the framework in its load commands), the pre-strip
Frameworks/state is logged for ground truth, and the final-IPA verifier enforces the same whitelist for the automatic-signing path. Verified locally against the real iroh-ffi1.0.2-cmux.2static artifact, a binary-less shell, and a real dylib.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Prevent App Store Connect ITMS-90208 by only keeping embedded frameworks that have a valid, dynamically linked executable; everything else is stripped with clear logging. The same whitelist is enforced during IPA verification to catch issues in both manual and automatic signing paths.
CFBundleExecutableexists and is a Mach-O dynamic library; strip static archives, binary-less shells, and other non-dylib blobs.Frameworks/contents and each framework’sfiletype; remove an emptyFrameworks/directory afterward.otool -Land fail if it links an invalid framework we would otherwise strip.Written for commit 834874d. Summary will update on new commits.
Summary by CodeRabbit