Repository navigation
iOS: stop importing private Apple symbols, and gate every build on it - #5923
Conversation
App Store Connect rejected customer uploads with "Validation failed (409) The app references non-public symbols: _CCCryptorGCMAddAAD, _CCCryptorGCMAddIV, _CCCryptorGCMFinal", and every check here was green. A private symbol compiles and links like any other -- "private" only means no public header declares it -- and the code sat behind a feature gate the sample app never turns on. The fixes: - Executor.replaceMarker switches a #define by its WHOLE name. replaceInFile was a plain String.replace, so enabling //#define CN1_INCLUDE_CRYPTO also uncommented //#define CN1_INCLUDE_CRYPTO_GCM on the next line: every app that used any crypto API compiled the GCM path, whatever ios.crypto.gcm said. replaceInFile and the five raw str.replace toggles in IPhoneBuilder now go through it. - AES-GCM is rebuilt on public CommonCrypto only: AES-CTR from inc32(J0), H and the tag mask as single ECB blocks, and GHASH in portable, branch-free C. It passes the McGrew-Viega vectors for 128/192/256-bit keys, matches the old SPI output byte for byte on 3000 random cases, and now authenticates BEFORE it decrypts, so a forged message never writes plaintext into the caller's buffer. - IOSNative.m no longer imports sqlite3_key / sqlite3_rekey from Apple's libsqlite3, which exports them but declares neither in sqlite3.h. They are referenced only when the bundled cipher engine defines them inside the app; otherwise stand-ins answer SQLITE_MISUSE, which is what Apple's copy returned. The new gate found this one; it shipped in 7.0.267. The gates: - scripts/check-ios-private-api.py fails on any SDK symbol that no public SDK header declares. Source mode compiles every port native with every feature gate on, and again with each optional engine the translator can omit hidden, so the branches customers compile are covered and not just the sample's. Binary mode reads the linked Release app and every bundle nested in it. Both self-test against a probe importing the three GCM symbols, before trusting a clean result. Wired into scripts-ios.yml (source) and ios-packaging.yml (binary). - scripts/check-builder-define-toggles.py (pr.yml) fails on a raw .replace of a #define literal in the builders, and ExecutorDefineMarkerTest pins the helper. Verified locally: reverting CN1Crypto.m/.h and IOSNative.m to master makes the source gate report exactly the three GCM symbols plus sqlite3_key/sqlite3_rekey; reverting replaceMarker fails 3 of the 6 unit tests. The unsigned Release device build of hellocodenameone passes the binary gate (5 binaries, ~3750 imports). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
044ece8 to
ee184f5
Compare
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee184f5c74
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not imports: | ||
| # Every executable imports at least dyld's and libSystem's entry points. | ||
| fail("read no imports from %s -- nm output did not parse" % binary) | ||
| symbols = {s for s, lib in imports if not SWIFT_LIBRARY.match(lib)} |
There was a problem hiding this comment.
When a linked dependency imports a private C or Objective-C symbol from a library whose name starts with libswift, Swift, or _Swift (including mixed frameworks such as SwiftUI), this filter removes the import solely because of its source library. classify() already skips genuinely Swift-mangled names via MANGLED_PREFIXES, so the broader library filter also hides unmangled private symbols; because third-party dependencies are covered only by binary mode, the gate can then pass an app that App Store validation rejects.
Useful? React with 👍 / 👎.
| if name.endswith((".app", ".appex", ".framework", ".xpc")): | ||
| path = executable(os.path.join(base, name)) | ||
| if path: | ||
| found.append(path) |
There was a problem hiding this comment.
When an app embeds a code-bearing .bundle or another package with CFBundleExecutable outside these four suffixes, its extensionless executable is never added to the scan; the file loop only recovers files ending in .dylib. Binary mode is the only coverage for code brought in by dependencies, so a private Apple import in such a nested executable can pass this gate even though the script claims to inspect every Mach-O in the app. Discover nested bundles from their Info.plist/CFBundleExecutable metadata rather than this suffix allowlist.
Useful? React with 👍 / 👎.
| cmd = [clang, "-c", "-arch", "arm64", "-target", "arm64-apple-ios" + target, | ||
| "-isysroot", sdk, "-fmodules", "-fmodules-cache-path=" + cache, | ||
| "-fobjc-arc" if name in arc else "-fno-objc-arc", "-w", | ||
| "-Wno-error=implicit-function-declaration", "-Wno-error=int-conversion", | ||
| "-Wno-error=incompatible-pointer-types", | ||
| "-I", NATIVE_SOURCES, "-I", tree, "-I", stub_dir, | ||
| "-include", os.path.join(tree, os.path.basename(prefix_header))] |
There was a problem hiding this comment.
Turning on harvested #define gates does not enable branches guarded by optional headers: this compile command only adds the native, generated-project, and stub directories, so checks such as __has_include(<MLKitVision/MLKitVision.h>) in CN1Vision.m and __has_include(<TFLTensorFlowLite/TFLTensorFlowLite.h>) in CN1Inference.m remain false in the workflow's sample project. Those feature implementations are therefore absent from both the object sweep and the sample binary, allowing a private Apple call added inside an optional MLKit/LiteRT path to reach customer builds without this gate seeing it.
Useful? React with 👍 / 👎.
| sdk = sdk_root(args.sdk, args.developer_dir) | ||
| clang = find_clang(args.developer_dir) | ||
| target = deltas.CN1_DEFAULT_DEPLOYMENT_TARGET | ||
| floor = deltas.sdk_minimum_deployment_target(sdk) | ||
| if floor and deltas.sdk_version_macro(floor) > deltas.sdk_version_macro(target): | ||
| target = floor | ||
| triple = "arm64-apple-ios" + target | ||
| print("sdk : %s" % sdk) | ||
| print("clang : %s" % clang) | ||
| print("deployment target : %s" % target) | ||
|
|
||
| exported = exported_names(sdk) | ||
| headers = public_header_names(sdk) |
There was a problem hiding this comment.
The script builds one export/header index from the default iphoneos SDK and reuses it for every executable returned from the app, including the companion watch app and watch extensions that this workflow explicitly ships. For a watchOS-only private symbol, classify() sees that the name is absent from the iPhoneOS exports and silently skips it, while differences between the public watchOS and iOS headers are likewise lost. Select the watchOS SDK index for watch binaries, rather than validating every nested platform against iPhoneOS.
Useful? React with 👍 / 👎.
|
Compared 157 screenshots: 157 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
|
Compared 193 screenshots: 193 matched. |
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
…hing by library Review findings on the binary mode, which is the only coverage dependencies get: - Imports from a Swift library were dropped wholesale, which would also hide an unmangled private C symbol a Swift library re-exports. Mangled names were already skipped by name; now only the swift_* runtime entry points, and only when they bind to a libswift library, are credited. The one name this surfaced in the sample, _TtCs12_SwiftObject, is a mangled Swift class name and is skipped under the same rule. - Nested executables were found by bundle suffix; a code-bearing .bundle or an extensionless executable anywhere else was missed. Every Mach-O is now found by its magic. - Every binary was judged against the iPhoneOS SDK, so a watchOS-only private symbol in the companion watch app was absent from the exports and passed. The platform is read from each binary's LC_BUILD_VERSION and indexed per SDK. Source mode now also names the __has_include(<...>) branches it cannot compile (MLKit and LiteRT pod headers, and SDK headers newer than the one in use), so that gap is printed on every run instead of passing silently. Sample Release device app: clean, 3 iOS and 2 watchOS binaries. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The native contract stages the builder's own INCLUDE_CN1_PUSH2 disabling replacement by matching IPhoneBuilder's source, and it looked for the literal str.replace(...) call this branch moved onto Executor.replaceMarker. It now matches the replaceMarker call; the pair it extracts is unchanged, and both the push-enabled and push-disabled configurations pass locally. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Compared 150 screenshots: 150 matched. |
✅ ByteCodeTranslator Quality ReportTest & Coverage
Benchmark Results
Static Analysis
Generated automatically by the PR CI workflow. |
|
Compared 223 screenshots: 223 matched. |
The screenshot suites failed on iOS, tvOS, watchOS and Mac Catalyst with one test: "Crypto API test failed: crypto operation failed with code -5", which is CN1_CRYPTO_E_UNSUPPORTED from an AES-GCM round trip. GCM was meant to be opt-in (vault or ios.crypto.gcm=true) because it called CommonCrypto's private SPI. But the prefix-matching replace this branch fixed enabled it for every crypto user anyway, so AES-GCM through com.codename1.security has always worked on iOS -- CryptoApiTest depends on it, and MacOSBuildHints defaults macOS to on to match. Fixing the replace turned it off for those applications. With GCM now built from public calls, the reason for opt-in is gone: - CN1Crypto.h carries CN1_CRYPTO_GCM_PUBLIC_API. For such a port GCM is on whenever the crypto API is, unless ios.crypto.gcm=false trims it, and the "cannot tell whether it uses the vault" build stop no longer applies. - A port without the marker -- anything released before it, which a pinned build.cn1Version can stage -- keeps the opt-in rule, since there enabling GCM links the private SPI. - resolveCryptoGcm / cryptoGcmUndecidable hold the rule, pinned by CryptoGcmDecisionTest; the catalog now documents ios.crypto.gcm as default true. The regenerated sample reports "Crypto API enabled, AES-GCM enabled", and the private-API gate is still clean. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Compared 166 screenshots: 166 matched. Benchmark Results
Detailed Performance Metrics
ParparVM vs HotSpot (JDK 25): macOS arm64Runner CPU: Apple M1 (Virtual) (baseline Ratios are ParparVM / JDK 25: below 1.00x ParparVM is faster (time) or smaller (RAM). Median of 5 interleaved, paired rounds; every run's output was verified. A regression is a ratio more than 15% (time) / 15% (RAM) above its baseline in
Result: no regression |
|
Developer Guide build artifacts are available for download from this workflow run:
Developer Guide quality checks: |
Cloudflare Preview
|
|
Compared 154 screenshots: 154 matched. Benchmark Results
Detailed Performance Metrics
|
|
@codex review |
|
Compared 155 screenshots: 155 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7cdc086ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def public_header_names(sdk): | ||
| """Every identifier the SDK's headers spell outside a comment.""" | ||
| names = set() | ||
| for header in walk(sdk, (".h",)): |
There was a problem hiding this comment.
When an SDK ships headers under PrivateHeaders or System/Library/PrivateFrameworks, this unrestricted recursive walk adds their identifiers to the supposedly public index. classify() will consequently accept an imported private symbol merely because its private header declares the name, so the gate can pass the same kind of binary that App Store validation rejects. Restrict this index to genuinely public header locations and explicitly exclude private-header/framework trees.
Useful? React with 👍 / 👎.
| cmd = [clang, "-c", "-arch", "arm64", "-target", "arm64-apple-ios" + target, | ||
| "-isysroot", sdk, "-fmodules", "-fmodules-cache-path=" + cache, |
There was a problem hiding this comment.
The source sweep always compiles with an iOS target, so code guarded by #if TARGET_OS_TV is preprocessed away. I also checked .github/workflows/scripts-ios.yml: the two new private-API steps run only in the iOS Metal jobs, while build-ios-tv builds the tvOS target without passing its product to this checker; the binary step in ios-packaging.yml scans an iOS app and cannot cover the separate tvOS product. A private Apple call added to one of the existing tvOS-only branches can therefore merge and ship without either mode examining it.
Useful? React with 👍 / 👎.
|
|
||
| def imports_of_binary(path): | ||
| """(symbol, library) for every import of one Mach-O, per `nm -m`.""" | ||
| out = run(["nm", "-m", "-u", "-arch", "arm64", path]) |
There was a problem hiding this comment.
When a shipped Mach-O contains an arm64 slice plus another device slice such as arm64e, this command succeeds after dumping only arm64, so the fallback that scans without -arch never runs. The tool's Mach-O help defines --arch as selecting the architecture(s) to dump; therefore an architecture-conditional private import present only in the other slice is invisible even though App Store validation examines that slice too. Enumerate and scan every slice, or omit the architecture filter for binary mode.
Useful? React with 👍 / 👎.
… keeps GCM Review findings on #5923: - tvOS was never checked: the source sweep compiled for iOS, so every TARGET_OS_TV branch was preprocessed away, and the tvOS app is a separate product no job scanned. The sweep now takes --sdk appletvsimulator (the only tvOS build any job makes; the device SDK cannot compile the port at all, having no LocalAuthentication, which IOSNative.m imports unconditionally). Gates whose code tvOS cannot compile are derived, round by round, from where clang's errors fall, and listed; an error inside no enabled gate stays fatal. 10 gates today (OIDC, HomeKit, NFC, StoreKit, contacts, ...). The tvOS job runs it, and run-tv-ui-tests.sh binary-checks the tvOS app right after building it. - nm read only the arm64 slice of a universal binary (`-arch arm64` succeeds and skips arm64e); it now reads every slice. - Headers under PrivateHeaders or PrivateFrameworks no longer count as public. No SDK in Xcode 26 or 27 ships either, but one that did would make a private import read as declared. - A detected vault user keeps AES-GCM even with ios.crypto.gcm=false, as on the private-SPI path; otherwise moving to the public-GCM port would break an existing vault app at run time. Verified: iOS and tvOS source sweeps clean, the Release app still clean, a tvOS simulator binary importing CCCryptorGCMAddIV is reported, 400 plugin tests pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Compared 155 screenshots: 155 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31f1de327f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # IOSNative.m imports unconditionally (the simulator SDK does ship it). | ||
| SDK_TRIPLES = { | ||
| "iphoneos": ("arm64-apple-ios", ""), | ||
| "appletvsimulator": ("arm64-apple-tvos", "-simulator"), |
There was a problem hiding this comment.
The new tvOS source sweep targets only appletvsimulator, and run-tv-ui-tests.sh likewise checks only the simulator product. Fresh evidence beyond the earlier tvOS coverage issue is the existing device-only block in CN1JailbreakDetector.m under #if !(TARGET_IPHONE_SIMULATOR): that code is omitted from both checks, even though customers archive the separate appletvos target for App Store submission. A tvOS-private import in such a device-only branch can therefore pass this gate; add an appletvos source sweep (working around the unconditional unavailable imports as needed) or scan an archived device product.
Useful? React with 👍 / 👎.
| /// asked through the hint. With public GCM, enabling it is harmless and nothing is asked. | ||
| static String cryptoGcmUndecidable(boolean publicGcm, boolean usesVault, boolean vaultUnknown, | ||
| String hint) { | ||
| if (publicGcm || !vaultUnknown || usesVault || "true".equals(hint) || "false".equals(hint)) { |
There was a problem hiding this comment.
When the database scan exhausts its budget on an archive under buildinRes, it sets vaultUnknown, but the separate permission scan only walks classesDir, so usesCryptoAPI can remain false. Treating every public-GCM port as immediately decidable then lets the build continue, and resolveCryptoGcm() returns false at its !usesCryptoAPI guard while the base CN1_INCLUDE_CRYPTO marker is also left disabled. If the unread portion actually uses Vault, the shipped app reaches the unsupported crypto stubs; even setting ios.crypto.gcm=true cannot repair this path. Public GCM removes the private-symbol risk, so an unknown vault result should conservatively enable the base crypto and GCM implementation or still stop for an explicit decision.
Useful? React with 👍 / 👎.
…requires it A tvOS slice compiles the port with the switches the iOS app turned on, and ten of them could not compile for tvOS at all, so an app using any of them with a TV slice failed its TV build; and no tvOS device build could succeed, because IOSNative.m imported LocalAuthentication, which the tvOS device SDK lacks. - LocalAuthentication is imported only where the SDK has it. - Contacts (AddressBook), the photo picker, NFC, OIDC, hotspot and Wi-Fi info are switched off for TARGET_OS_TV. Their natives are defined outside the switches, so they answer "unsupported" as for an app without the feature. - StoreKit on tvOS looks the product up and pays with paymentWithProduct:, since paymentWithProductIdentifier: does not exist there; a product the store does not return reports through itemPurchaseError. - App review is a no-op on tvOS (no SKStoreReviewController). - HomeKit scene create/delete answer NOT_SUPPORTED on tvOS, where HomeKit cannot build action sets; everything else in HomeKit is unchanged. - HeadphonesDetector moves off the C AudioSession API (deprecated, absent on tvOS) to AVAudioSession route changes, and starts itself: nothing ever created it, so ios.headphoneCallback never fired on any device. The tvOS sweep of check-ios-private-api.py is now strict -- every gate on, against both appletvos and appletvsimulator, no gate dropped -- so a feature that breaks tvOS fails CI instead of every customer's TV build. The module cache check-ios-sdk-deltas.py compiled through was a fixed shared name under the temp directory; it is per process now, and inside the caller's work directory for the private-API gate. Verified: all three sweeps (iphoneos, appletvos, appletvsimulator) compile 172 objects with 79 gates and are clean; the sample's TV target builds for the tvOS DEVICE SDK (it could not before) and its app passes binary mode; the push completion contract passes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review finding: when the vault scan runs out of budget on a library under buildinRes, vaultUnknown is set but the permission scan, which walks only the application's classes, can leave usesCryptoAPI false. With the old port the build stopped and asked; with public GCM it went on, and resolveCryptoGcm's !usesCryptoAPI guard left both CN1_INCLUDE_CRYPTO and GCM off -- a library that did use the vault reached the unsupported stubs, and not even ios.crypto.gcm=true could repair it here. cryptoApiRequired now decides the base crypto flag: detected vault use turns it on; an unknown answer does too with public GCM (it links nothing Apple rejects) unless ios.crypto.gcm=false says there is no vault; with an older port only an explicit true does, as BuildDaemon already had. CryptoGcmDecisionTest covers it; 401 plugin tests pass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00ef210945
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| PLATFORM_SDKS = { | ||
| "1": "macosx", "2": "iphoneos", "3": "appletvos", "4": "watchos", "6": "macosx", | ||
| "7": "iphonesimulator", "8": "appletvsimulator", "9": "watchsimulator", | ||
| "11": "xros", "12": "xrsimulator", | ||
| } |
There was a problem hiding this comment.
On otool versions that print symbolic platform values such as IOS, TVOSSIMULATOR, or WATCHOSSIMULATOR, this numeric-only map makes platform_sdk() abort before inspecting the first binary. The repository already accounts for this variation in WatchNativeBuilder.java:2503-2517, but both newly added binary gates depend on this parser, so affected Xcode runners will fail every packaging/tvOS run instead of checking imports. Normalize the captured value and map both numeric and symbolic spellings.
Useful? React with 👍 / 👎.
| target = deltas.CN1_DEFAULT_DEPLOYMENT_TARGET | ||
| floor = deltas.sdk_minimum_deployment_target(sdk) | ||
| if floor and deltas.sdk_version_macro(floor) > deltas.sdk_version_macro(target): | ||
| target = floor |
There was a problem hiding this comment.
When the new --sdk appletvos or appletvsimulator sweeps run under Xcode 27, this calls sdk_minimum_deployment_target(), which extracts only SupportedTargets.iphoneos.MinimumDeploymentTarget and therefore returns no floor for a tvOS SDK. The sweep stays at 14.0 even though TvNativeBuilder.java:184-197 raises real tvOS builds to the SDK's 15.0 floor because lower targets fail; this can either stop the gate before it scans anything or preprocess different deployment-target branches than the shipped product. Select the SupportedTargets key corresponding to args.sdk.
Useful? React with 👍 / 👎.
…loor Review findings: - Some otool versions print LC_BUILD_VERSION platforms symbolically (IOS, TVOSSIMULATOR, WATCHOSSIMULATOR) rather than as numbers, and the parser knew only the numbers, so binary mode would abort on the first binary on such a runner. Both spellings map now, as WatchNativeBuilder already accepts; platform_of() is separate from the otool call so it can be exercised alone. - sdk_minimum_deployment_target read only SupportedTargets.iphoneos, which a tvOS SDK does not have, so tvOS sweeps got no floor and stayed below it (under Xcode 27 the tvOS floor is 15.0). It takes the SDK's own key now, and a tvOS sweep starts from TvNativeBuilder's 13.0 default before raising, as the builder does. Verified under Xcode 27 and 26.3: appletvos, appletvsimulator and iphoneos sweeps are clean at 15.0/15.0/15.0 and 13.0/13.0/14.0; the iOS Release app and the tvOS device app still pass binary mode. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eab570e04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # file's, told apart by the word after it: an architecture count for Mach-O (a handful), the | ||
| # class-file version for Java (45 and up). | ||
| MACHO_THIN = {b"\xfe\xed\xfa\xce", b"\xce\xfa\xed\xfe", b"\xfe\xed\xfa\xcf", b"\xcf\xfa\xed\xfe"} | ||
| MACHO_FAT = {b"\xca\xfe\xba\xbe", b"\xca\xfe\xba\xbf"} |
There was a problem hiding this comment.
When an embedded dependency uses the valid FAT_CIGAM or FAT_CIGAM_64 encoding, is_macho() returns false because this set contains only FAT_MAGIC/FAT_MAGIC_64; the architecture count is also decoded only as big-endian. Consequently binaries_in() silently omits that executable and binary mode can pass despite a private import inside it. Add the swapped magics and decode nfat_arch using the byte order selected by the magic.
Useful? React with 👍 / 👎.
Review finding: is_macho() knew only FAT_MAGIC / FAT_MAGIC_64, so an embedded executable whose universal header is stored little-endian (FAT_CIGAM / FAT_CIGAM_64, equally valid) was never found, and a private import in it passed. Both byte orders are recognised now, and the architecture count is read in the order the magic announces. Checked against synthetic headers in all four encodings plus a Java class file and a thin Mach-O; the sample app still yields its 5 binaries and passes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0cea2c598
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if os.path.islink(path) or APPLE_EMBEDDED.match(name): | ||
| continue |
There was a problem hiding this comment.
When an app embeds a third-party Mach-O named libswiftFoo.dylib, this condition skips the entire binary solely because its basename resembles an Apple runtime library. Fresh evidence beyond the earlier import-filter issue is that the narrower _swift_ symbol exemption never runs for this skipped file. Because dependencies are covered only by binary mode, an unmangled private Apple import in such a dylib passes this gate even though App Store validation still scans it; restrict the whole-file exemption to verified Xcode runtime artifacts, or scan the binary and exempt only its known runtime symbols.
Useful? React with 👍 / 👎.
…hem by name Review finding: any Frameworks/libswift*.dylib was skipped whole because its name looked like Apple's back-deployed Swift runtime, so a third-party libswiftFoo.dylib -- and any private import in it -- passed unread. Nothing is skipped by name now; Apple's runtime passes on its own because the few unmangled internals it imports are already listed one by one in COMPILER_EMITTED. The sample app's libswift_Concurrency.dylib is now scanned (235 imports) and the app still passes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ipping them by name" This reverts commit 5e62a6b.
This reverts commit a0cea2c.
…dening The gate exists to keep OUR port and OUR sample application off private Apple API. Binary mode reads one application we build, whose contents are a known, closed set: the port, the translated code, the sample's Swift extensions and watch app, the cn1libs it bundles, and Apple's embedded Swift runtime. Several review rounds hardened it against inputs that set never contains, which added code without adding coverage: - byte-swapped universal headers and the libswift-by-name scan are reverted (the two preceding reverts); - nested executables are found by bundle again -- .app/.appex/.framework/.xpc and each one's CFBundleExecutable -- instead of by sniffing every file's magic. Kept: reading every architecture slice, per-platform SDKs for the watch and TV binaries, symbolic otool platforms, and the tvOS sweeps, which found and now guard real breakage. The docstring states the scope. Verified: the iOS Release app (5 binaries) and the tvOS device app pass, and a probe importing CCCryptorGCMAddIV is still reported. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
App Store Connect rejected uploads with
Validation failed (409) The app references non-public symbols: _CCCryptorGCMAddAAD, _CCCryptorGCMAddIV, _CCCryptorGCMFinalwhile every check was green.Fixes
Executor.replaceMarker: a#definetoggle matches the whole name. EnablingCN1_INCLUDE_CRYPTOno longer also enablesCN1_INCLUDE_CRYPTO_GCM, so crypto users stop compiling the GCM path unless they use the vault or setios.crypto.gcm.IOSNative.mno longer importssqlite3_key/sqlite3_rekey(exported by Apple's libsqlite3, undeclared insqlite3.h) unless the bundled cipher engine defines them. Found by the new gate.Gates
scripts/check-ios-private-api.py: fails on any SDK symbol no public header declares. Source mode (all feature gates on, every optional-engine variant) inscripts-ios.yml; binary mode on the Release device app inios-packaging.yml.scripts/check-builder-define-toggles.py(pr.yml) +ExecutorDefineMarkerTest.The rationale for each is in the code comments. Companion BuildDaemon PR carries the same builder fix.
🤖 Generated with Claude Code