feat(release): fingerprint the native surface so an OTA cannot outrun the binary - #2977
Conversation
… the binary A bundle's only identity is a version number, which says nothing about the native surface it needs. 1.2.1 does not encode 'requires the 8.51 updater plugin', so CI publishes JS built against new plugins onto binaries built months earlier and the mismatch first appears on a user's device. Nothing caught this before. Capgo's min_update_version blocks delivery only, only under the 'metadata' channel strategy (production is on 'major', so today it is inert), and lives in a dashboard CI cannot read — it never reports that an incompatible bundle was BUILT. native-fingerprint.mjs hashes the JS<->native contract: Capacitor's two generated plugin manifests, which pin each plugin's resolved version in its dependency path, plus capacitor.config.ts, the gradle files, the manifest, pbxproj, Info.plist and both entitlements. capgo-deploy.yml compares the tree it is publishing against the v<major>.<build>.0 tag its floor targets and fails, naming the file that moved. It is a pure function of the tree, so nothing is stored and any tag can be fingerprinted retroactively. MARKETING_VERSION and CURRENT_PROJECT_VERSION are normalised out: native-ios-postsync.js stamps them on every sync, and leaving them in would refuse an OTA after every release — a check that cries wolf gets deleted. This is docs/NATIVE-RELEASE.md's standing rule, 'bump the native version whenever you change plugins/native code', with something checking it.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Code-analysis diffPainscore total: 7149.37 → 7149.37 (0) |
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![chip-peanut-bot[bot]](https://avatars.githubusercontent.com/in/2915219?s=60&v=4){kind=small}
There was a problem hiding this comment.
Chip review — changes requested
The new OTA check fails closed for its listed files, but it can still report an unchanged native surface when either an in-repo native bridge implementation or the authoritative resolved plugin dependency changes.
Findings
-
BLOCKING · scripts/native-fingerprint.mjs:68 · Include the in-repo native bridges in the fingerprint
The allowlist ends at configuration files and omits the native code that JavaScript actually calls, includingandroid/app/src/main/java/me/peanut/wallet/*andios/App/App/*Plugin.swiftplus their registration code. This is already a live edit pattern: commits have changedMainActivity.javaalongside OTA JavaScript, andPushProvisioningPlugin.swifthas changed independently without touching any listed input. If a later bundle calls a new method or relies on corrected behavior in one of those files,--diffreturns unchanged against the old release tag and the bundle is published to a binary with the old bridge. Hash the relevant native source/registration directories (with deterministic path ordering), and add a test that mutates a representative Java or Swift bridge file. -
BLOCKING · scripts/native-fingerprint.mjs:52 · Derive plugin versions from authoritative dependency state
The check trusts two committed generated manifests, but the OTA workflow only runspnpm installandnative-build.js; it never regenerates either manifest. A PR can therefore bump a Capacitor plugin inpackage.json/pnpm-lock.yamlwhile accidentally leaving these generated files unchanged. The install supplies the new JS wrapper to the OTA build, both fingerprints still see the old manifest bytes, and the bundle is published to a binary containing the old native plugin—the exact mismatch this guard is meant to stop. Fingerprint the resolved native-plugin entries from the lockfile/package metadata (or deterministically regenerate and verify the manifests before comparing), and cover a lockfile-only plugin bump in the tests.
Checked clean
- Confirmed the detached worktree head, trusted author, base ref, supplied base SHA, and merge base all match the dispatcher metadata.
- Reviewed the workflow trigger, checkout depth and tags, native-floor resolution, release-tag timing, Capgo upload boundary, and the new fail-closed step.
- Exercised the CLI at the exact head: the working tree matches HEAD, while comparison with v1.1.0 exits 1 and names all changed listed inputs.
- Traced native build and release workflows to distinguish committed source state from generated and release-time native state.
- Reviewed native-source history and confirmed Java/Swift bridge implementations can change without any current fingerprint input changing.
- Exact-head ci-success, unit, native-export, typecheck, eslint, format, analyze, design-system, and provenance checks passed; Deploy Preview remained in progress and is not relevant to this deployment guard.
- A local Jest run was unavailable because dependencies are not installed in the detached worktree; the exact-head unit check completed successfully.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: f43a34c99ea3 · Context: repo, ci · Took 9m
Two holes Chip found, both of which let the exact mismatch this guard exists to stop through unnoticed. The allowlist stopped at config files, so the native code JS actually calls was invisible. MainActivity.java registers the app-local plugins and the *Plugin.java / *Plugin.swift files are the methods themselves; a bundle calling a new method, or relying on corrected behaviour in one of them, moved no listed input and --diff reported unchanged. Both are live edit patterns in this repo's history. Hash the bridge sources, path and content, so a rename with identical bytes still counts. The plugin versions were read only from Capacitor's committed generated manifests, which the OTA workflow never regenerates — it runs pnpm install and native-build.js. A plugin bumped in package.json + pnpm-lock.yaml without a cap sync therefore ships the NEW JS wrapper to the bundle while both manifests still hash to the old bytes. The lockfile is the authoritative record of what install produced, so read the resolved versions from it; the manifests stay as the complementary check on the plugin SET. Also fixes a silent-empty bug found while testing the above: matches nothing and exits 0, so the first cut of the bridge globs reported an empty set at EVERY ref and would have compared nothing against nothing while passing. List the directory and filter by extension instead, with a test that the sets resolve at a ref. 13 tests, covering an Android bridge edit, an iOS bridge edit and a lockfile-only bump.
There was a problem hiding this comment.
Chip review — changes requested
The bridge coverage is fixed, but the fingerprint still has two fail-open gaps and the new ref test leaves exact-head CI red.
Findings
-
BLOCKING · scripts/native-fingerprint.mjs:57 · Discover native plugins beyond four namespaces
The lockfile now supplies resolved versions, but this matcher decides which dependencies are native by four scopes pluscordova. A direct Capacitor plugin such as@capacitor-community/in-app-reviewor@transistorsoft/capacitor-background-geolocationis excluded. If one is added or bumped withoutcap sync, package.json and pnpm-lock.yaml move while both generated manifests remain stale, so the fingerprint reports unchanged and an OTA can ship a JS wrapper that the binary lacks. Derive the native-plugin set from installed package metadata or cover generic Capacitor plugin names, and add a regression outside the current scopes. -
BLOCKING · scripts/native-fingerprint.mjs:76 · Fingerprint native resource contracts too
The Android list hashes AndroidManifest.xml but omits native resources it delegates to, includingandroid/app/src/main/res/values/capacitor-passkey.xml; the iOS list likewise omits the Wallet and notification extension Info.plists and entitlements. Changing the passkey asset statement or an extension entitlement leaves every current input unchanged, so JS that relies on the corrected native configuration can be OTA'd onto an older binary. Add the relevant text resource/config files (or narrow globs that exclude generated web assets and images) and regression-test one Android resource plus one extension plist/entitlement. -
MAJOR · scripts/tests/native-fingerprint.test.js:168 · Do not require release tags in the shallow unit checkout
The exact-headunitcheck fails here: that job uses actions/checkout's default shallow history, sov1.1.0is absent and the script returns<absent>for the bridge set; line 170 then fails andci-successis red. The two earlier v1.1.0 tests also pass for the wrong reason because an unknown ref is treated as an all-absent tree. Make these tests self-contained with HEAD plus a temporary working-tree change (or explicitly fetch tags), and add an assertion that an unknown ref is rejected rather than hashed.
Checked clean
- Verified the supplied head, base SHA, merge base, trusted author, and four-file diff.
- Java/Kotlin and Swift bridge sets resolve in the full-history checkout at HEAD and v1.1.0; prior P1 is fixed.
- The current native dependencies all resolve to concrete pnpm-lock versions, then the namespace matcher was checked against additional Capacitor plugin names.
- The CLI manifest and --diff HEAD paths passed directly; workflow inputs use execFile argument arrays and do not add shell interpolation.
- Exact-head format, typecheck, ESLint, native-export, design-system, analysis, and preview checks passed; unit and aggregate ci-success failed on the new ref test.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 5fe6817f1495 · Context: repo · Took 12m
The unit job checks out at depth 1 with no tags, so `--ref v1.1.0` resolved to nothing in CI. The regression guard added in 5fe6817 caught that honestly and went red — but the two older ref-based tests had been passing for entirely the wrong reason: with no tag, every input reads <absent>, which still differs from the working tree, so "the fingerprint differs across refs" and "--diff exits 1" both held while checking nothing at all. Use HEAD, which always resolves, and create the difference by mutating the working tree instead of relying on history. The suite now asserts the inverse too — that the working tree equals HEAD when clean — which is what actually proves the ref path reads content rather than silently returning absence for everything. Same failure mode as the `git ls-tree` glob bug this suite already guards: an empty read that looks exactly like a passing check. The workflow itself is unaffected — capgo-deploy.yml checks out with fetch-depth: 0 and fails closed with an explicit message when the release tag is missing.
There was a problem hiding this comment.
Chip review — changes requested
The hermetic test fix is green, but two native-surface omissions still let incompatible OTAs pass the fingerprint check.
Findings
-
BLOCKING · scripts/native-fingerprint.mjs:57 · Discover native plugins beyond four namespaces
This matcher still recognizes native packages only under @capacitor, @Capgo, @OneSignal, @sumsub, or names containing cordova. Add or bump a Capacitor plugin such as @capacitor-community/in-app-review without running cap sync: package.json and pnpm-lock.yaml move, both generated manifests remain stale, the dependency is excluded here, and the fingerprint stays unchanged, allowing an OTA wrapper onto a binary without its native half. Derive the plugin set from package metadata or Capacitor discovery and cover a plugin outside these namespaces. -
BLOCKING · scripts/native-fingerprint.mjs:76 · Fingerprint native resource contracts too
The input list hashes AndroidManifest.xml but not the resource it delegates passkey trust to: android/app/src/main/res/values/capacitor-passkey.xml. Correcting that asset statement changes whether native passkey authentication works while every current fingerprint input remains unchanged, so a later OTA can be published to binaries carrying the old association. The Wallet and notification extension Info.plists and entitlements are omitted similarly. Include the relevant text resource/config files with focused Android and iOS extension regression tests.
Checked clean
- Verified the supplied repository, PR, trusted author, exact head, base SHA, merge base, and four-file diff.
- Java/Kotlin and Swift bridge sets are hashed by path and content in both working-tree and git-ref modes; prior P1 is fixed.
- Selected native dependency versions come from pnpm-lock.yaml and a lockfile-only bump moves the fingerprint; prior P2 is fixed independently of the remaining discovery gap.
- The workflow uses full history, verifies the resolved release tag, quotes the floor ref, and runs the fingerprint before the Capgo upload.
- Direct manifest and --diff HEAD checks passed; exact-head aggregate, unit, native-export, typecheck, lint, format, analysis, design-system, provenance, and report checks are green, with preview still in progress.
- Reviewed workflow secret handling and process-spawn inputs; no new injection, credential disclosure, or privilege-escalation path was found.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: f1f31e174235 · Context: repo · Took 9m
Three more holes Chip found, all the same shape: an input the config files delegate to, which nothing in the allowlist moved for. Native resources. AndroidManifest.xml was hashed but not the res/values it points at, so editing capacitor-passkey.xml — the asset statement passkeys are validated against — left every input unchanged. On iOS only the app's own Info.plist and entitlements were covered, not the extensions', though an extension's capabilities are just as much part of the shell a bundle lands on. Both are now globs, and the two iOS single-file inputs fold into one that also covers OneSignal's and the Wallet extensions'. Community plugins. The native dependency matcher was a four-scope allowlist, so @capacitor-community/in-app-review and @transistorsoft/capacitor-background-geolocation were invisible: bumping one without a cap sync moved package.json and the lockfile while both generated manifests stayed stale, and the fingerprint reported unchanged. Match the substring instead, and take the union with the plugin names Capacitor itself generated — the declared dependencies catch a plugin that was never synced, the generated manifests catch one whose package name says nothing about being native. Unresolvable refs. git show and git ls-tree both fail quietly for an unknown ref, so every input read <absent> and produced a well-formed fingerprint of nothing. It differs from any real tree, so a --diff against a missing tag reported "changed" and looked like the check had run. Now an error. Same shape as the ls-tree glob bug and the tag-dependent tests before it: silence that reads as a result. 17 tests, including the passkey asset statement, an extension entitlement, a community plugin outside every first-party scope, and an unknown ref.
There was a problem hiding this comment.
Chip review — changes requested
Most direct prior omissions are fixed, but the release guard can still false-green for an unrecognized native plugin, a conditionally compiled bridge, or same-version patched native code. The new tests also mutate files shared with parallel Jest workers.
Findings
-
BLOCKING · scripts/native-fingerprint.mjs:60 · Discover native plugins from metadata, not their names
The P3/P6 discovery gap remains in a narrower form: a newly declared Capacitor plugin whose package name contains none of capacitor, cordova, onesignal, or sumsub is filtered out here. If its generated manifests are stale—the exact failure this guard is meant to catch—generatedPluginNames cannot recover it either, so adding its JS wrapper leaves every fingerprint input unchanged and an OTA can reach a binary with no native implementation. Discover plugins from package metadata/plugin.xml or maintain an exhaustive native-dependency declaration instead of a package-name heuristic. -
BLOCKING · scripts/native-fingerprint.mjs:105 · Fingerprint the bridge set actually compiled into the binary
This glob hashes PushProvisioningPlugin.java even when the Android release intentionally omits src/meawallet/java because its optional build inputs are absent; the release still succeeds and the tag therefore claims a bridge the binary never registered. A later OTA can rely on PushProvisioning while comparing equal to that tag and pass this guard. Make that build variant fail closed for releases, or record and compare a non-sensitive compiled-capability/build-variant manifest rather than treating every tracked source file as present in the binary. -
BLOCKING · scripts/native-fingerprint.mjs:248 · Include patched native package identity
Only version-looking tokens are retained from pnpm-lock.yaml. Adding or changing a pnpm patch for a native plugin can change both its JS wrapper and native implementation while leaving the package version and generated Capacitor manifests unchanged; the patch file and lockfile patch hash are not inputs, so the OTA fingerprint still matches the old binary. Hash the relevant resolved package entries, including patch/integrity identity and patch-file contents, not just their semantic versions. -
MINOR · scripts/tests/native-fingerprint.test.js:27 · Isolate fingerprint fixtures from the live checkout
withPatchedInput rewrites tracked repository files while Jest runs test files in parallel. For example, this suite holds project.pbxproj at MARKETING_VERSION 9.9.9 while spawning the CLI, and marketing-version.test.js independently reads that same file and expects the package version, creating a timing-dependent unit failure despite the finally restore. Run the fingerprint CLI against a temporary repository fixture or otherwise isolate these mutations from the shared checkout.
Checked clean
- P1 is fixed: Android and iOS in-repo bridge sources are hashed by path and content.
- P2 is fixed for resolved version changes: declared and generated native plugin names now derive versions from pnpm-lock.yaml, including a lockfile-only bump.
- P4 and P7 are fixed for the cited tracked resources: Android XML resources and all iOS plist and entitlement files are included.
- P5 is fixed: ref-based unit coverage uses HEAD and no longer requires release tags in the shallow unit checkout.
- The CLI manifest and --diff HEAD paths completed successfully against the pinned checkout.
- Exact-head unit, typecheck, format, lint, native-export, analyze, preview, and aggregate CI checks completed successfully.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: a4f2a49c6510 · Context: repo · Took 15m
…e tests Four findings, all real. The name heuristic could still miss a plugin called after neither capacitor nor a first-party scope, and if its generated manifests were stale too — the exact case this guard exists for — nothing recovered it, because package.json and pnpm-lock.yaml are not inputs on their own. Hash the full dependency NAME set. Names and not versions deliberately: adding or removing a dependency is rare and worth re-checking the native surface, while bumping a JS-only library is not, and a check that refuses an OTA on every lockfile churn gets switched off. The Android bridge glob hashed every source set, including src/meawallet, which build.gradle adds only when the MeaWallet Nexus credentials are present. A release without them still succeeds, so the tag claimed a bridge the binary never registered — worse than not claiming it, since a later OTA could rely on the claim. Hash src/main only. This narrows rather than solves: whether a secret-gated variant is in a given binary is not visible in the tree at all, so the fingerprint now neither asserts nor denies it. A pnpm patch rewrites a package's JS wrapper and its native sources with no version change, leaving the lockfile version and both generated manifests identical. Added patches/ and the patchedDependencies map. The tests rewrote tracked files around a spawned CLI and restored them in a finally. That is not isolation — Jest runs suites in parallel, and marketing-version.test.js reads the same project.pbxproj, so holding it at 9.9.9 for the length of a subprocess made an unrelated suite fail on timing. They now build a throwaway git fixture and point the CLI at it with a new --root flag. 18 tests, and the two suites now pass together.
|
All four findings addressed in Fingerprint the bridge set actually compiled into the binary. Correct — The glob is now Isolate fingerprint fixtures from the live checkout. Entirely right, and it was already a latent flake rather than a hypothetical: the suite held The tests now build a throwaway git repo from copies of the real inputs and point the CLI at it with a new |
There was a problem hiding this comment.
Chip review — changes requested
The latest patch fixes patch identity and test isolation, but the guard can still false-green for an already-declared generically named plugin and for the credential-gated Android bridge compiled by release CI.
Findings
-
BLOCKING · scripts/native-fingerprint.mjs:271 · Track generic native plugin version changes
Hashing every dependency name catches the first declaration, but it does not catch a later version bump. If a generically named plugin was released while its committed Capacitor manifests were stale (the release workflows run cap sync only in their build workspaces), the tag still has no generated name for it. A later 1.x to 2.x package/lockfile bump leaves allDependencies unchanged, the name heuristic still excludes it, and the OTA passes against the old binary. Classify native plugins from package metadata or an explicit native-dependency map, then hash their resolved versions. -
BLOCKING · scripts/native-fingerprint.mjs:119 · Fingerprint the credential-gated bridge when releases compile it
The new src/main-only glob drops PushProvisioningPlugin.java even though android-release.yml supplies the MeaWallet credentials and app/build.gradle compiles src/meawallet into release binaries. After a tagged release, changing that bridge and OTA JS to rely on its new method or behavior leaves this input and build.gradle unchanged, so the OTA can pass against the older binary. Make release builds fail closed on the capability or compare a variant-specific compiled-capability/artifact manifest; omitting the conditional source creates a false green for the normal release variant.
Checked clean
- P1 is fixed: path-and-content hashes cover the in-repo Android main and iOS bridge sources.
- P2 is fixed for identified plugins: resolved versions come from pnpm-lock.yaml rather than committed generated manifests.
- P3 and P6 are fixed for the cited namespace allowlist: discovery now combines a broad name pattern with both generated Capacitor manifests; P8 records the remaining generic-name version gap.
- P4 and P7 are fixed: Android XML resources and iOS plist and entitlement files are fingerprinted.
- P5 is fixed: ref coverage uses a temporary repository's HEAD and errors on an unknown ref.
- P10 is fixed: patch paths, contents, and the patchedDependencies mapping now move the fingerprint.
- P11 is fixed: every mutation occurs in a unique temporary git fixture rather than the live checkout.
- The CLI manifest and --diff HEAD paths completed successfully at the pinned head; the focused local Jest suite was unavailable because this detached worktree has no node_modules.
- Exact-head aggregate CI, unit, native-export, typecheck, lint, format, analysis, design-system, and preview checks completed successfully.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: b682a6b96c09 · Context: repo · Took 8m
… set Two re-raises, both right. Hashing dependency NAMES caught a plugin's first declaration but not a later version bump: a generically-named plugin released while its committed manifests were stale is invisible to the name heuristic AND to Capacitor's generated list, so a 1.x to 2.x bump moved nothing. Added an explicit NATIVE_DEPENDENCIES list as a third source, and a test asserting every plugin Capacitor generated appears in it — so a synced plugin cannot drop out of the list silently, which is what makes the list trustworthy for the unsynced case it exists to cover. Reverted the Android glob to every source set. Whether src/meawallet reaches a binary depends on a CI secret, so it is not knowable from the tree and both choices are unsound: including it lets a tag claim a bridge the binary may lack, excluding it lets a bridge change go untracked when the binary does have it. Including is the conservative error — it can only force an unnecessary native release, while excluding fails silently and ships JS against a binary without the method. Worth recording: the premise that releases currently compile that variant is not true today — MEAWALLET_NEXUS_USER_ANDROID exists neither at repo level nor in the Production environment, so build.gradle takes the else branch. The hole is real the moment those secrets are provisioned, which is why this errs toward refusal rather than leaving it out.
|
Fingerprint the credential-gated bridge when releases compile it — reverted in One correction to the premise, for the record: releases do not compile that variant today. That does not save the exclusion, though — the team confirms those secrets are being provisioned, just not immediately. So the hole you describe becomes real, and tracking the source set is correct rather than merely cautious. The reasoning I settled on: whether that bridge reaches a binary is not knowable from the tree, so both choices are unsound. Including it lets a tag claim a bridge the binary may lack; excluding it lets a bridge change go untracked once the binary does carry it. Including is the conservative error — it can only force an unnecessary native release, while excluding fails silently and ships JS against a binary with no such method. The real fix is for the variant to stop depending on a CI secret, or for the build to emit a compiled-capability manifest recorded on the tag. Both are larger than this PR, and I'd rather land the guard erring toward refusal than hold it for a build change. Happy to file that follow-up. |
There was a problem hiding this comment.
Chip review — changes requested
The latest patch closes the direct bridge-list gap, but the guard still false-greens for generic native-plugin version bumps, credential-dependent Android binaries, and iOS native dependencies injected only by the release post-sync script.
Findings
-
BLOCKING · scripts/native-fingerprint.mjs:302 · Track generic native plugin version changes
The explicit list covers today's plugins, but hashing every other dependency only by name still misses its later version changes. If a generically named native plugin is added while the committed Capacitor manifests are stale, the first addition forces a native release through the dependency-name hash, yet that release tag still does not classify the package as native. A later 1.x to 2.x package/lockfile bump leaves the name set unchanged and produces the same fingerprint; I reproduced that exact transition withsome-native-thing. Require a native declaration or installed-package metadata for every such dependency, then hash its resolved lockfile version. -
BLOCKING · scripts/native-fingerprint.mjs:153 · Fingerprint the binary's actual Android variant
Hashingsrc/meawalletfixes source edits when that source is compiled, but it still cannot distinguish the binary that was actually cut.android-release.ymlcan succeed without both MeaWallet Nexus secrets; app/build.gradle then omits this source set, while the tag receives the same source hash as a fully featured build. A later OTA can start calling an existing PushProvisioning method without changing native files and compare equal to a tag whose binary never registered the plugin. Make release CI fail closed for this capability or persist and fingerprint a non-secret manifest of the compiled variant. -
BLOCKING · scripts/native-fingerprint.mjs:118 · Include release-time iOS native inputs
The iOS input list omitsscripts/native-ios-postsync.js, even though the release workflow runs it aftercap syncto inject proprietary frameworks into the compiled Package.swift and it pinsMPP_VERSION. Changing that version from 2.0.0 to 2.1.0 leaves this fingerprint unchanged; OTA JavaScript can therefore rely on the newer native SDK while still passing against a binary built with 2.0.0. Hash the post-sync script and authoritative resolved iOS dependency state, or fingerprint a canonical manifest produced from the release workspace.
Checked clean
- Verified the supplied trusted author, exact head SHA, base ref, base SHA, merge base, and four-file diff.
- P1 is fixed: path-and-content hashes cover the in-repo Android and iOS bridge sources.
- P2 is fixed for classified plugins: their resolved versions come from pnpm-lock.yaml rather than generated manifests.
- P3, P6, and P8 are fixed for newly declared plugins: broad discovery plus the complete dependency-name set makes an addition move the fingerprint; P12 records the remaining later-version gap.
- P4 and P7 are fixed for the cited resource contracts: Android XML resources and all iOS plist and entitlement files are included.
- P5 is fixed: fixture ref tests use HEAD and unknown refs fail closed.
- P10 is fixed: patch contents and patchedDependencies identity are included.
- P11 is fixed: mutations run in an isolated temporary repository.
- P13 is fixed for bridge source changes when release CI compiles the credential-gated source set; P9 records the unresolved actual-build-variant mismatch.
- The CLI manifest and --diff HEAD paths passed at the pinned head; isolated reproductions confirmed unchanged fingerprints for both a generic native-plugin 1.x to 2.x bump and an iOS MPP SDK 2.0.0 to 2.1.0 bump.
- Exact-head aggregate CI, unit, native-export, typecheck, lint, format, analysis, design-system, provenance, and preview checks completed successfully.
- Reviewed workflow secret handling, ref validation, and git process spawning; no new injection or credential-disclosure path was found.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 7fd94622e729 · Context: repo, ci · Took 12m
scripts/native-ios-postsync.js runs after cap sync in the release workflow and vendors proprietary frameworks whose versions are string constants inside it — SUMSUB_VERSION 1.42.0 and MPP_VERSION 2.0.0. Nothing else in the manifest records them, so bumping the MeaWallet SDK changed what the binary links while every fingerprint input stayed identical, and OTA JS relying on the newer SDK compared equal to a tag built against the old one. Hash the script itself. A test bumps MPP_VERSION and asserts the fingerprint moves.
|
Round 5. One fixed, two I want to argue rather than patch again. Include release-time iOS native inputs — fixed in Track generic native plugin version changes — I don't think I should fix this the way it's framed, and here is the number that decides it. Hashing every dependency's resolved version is the only way to catch a later bump of an unclassified package. On this repo that is 73 commits touching package.json in the last 60 days against 4 native releases. The guard would refuse essentially every OTA, and a guard that blocks the normal path gets deleted within a week — which costs more safety than the case it closes. The alternative you suggest, requiring a native declaration for every dependency, means classifying ~200 packages and touching that list on every dependency addition. That is a real option, but it is a policy decision about how this repo takes dependencies, not a defect in this script, and it should be someone's explicit call rather than something I slip into a fingerprint PR. What is in place: an explicit Fingerprint the binary's actual Android variant — agreed, and not solvable here. Whether Happy to be overruled on either. If the call is that the OTA lane should block on every dependency bump, or that the full classification is worth maintaining, say so and I will implement it. |
There was a problem hiding this comment.
Chip review — changes requested
The post-sync SDK pin is now covered and exact-head CI is green, but the guard still false-greens for later generic-plugin bumps, secret-dependent release variants, and the resolved iOS package graph.
Findings
-
BLOCKING · scripts/native-fingerprint.mjs:309 · Track generic native plugin version changes
The explicit list still does not close the stale-manifest case. Add a generically named native plugin without committing cap-sync output: the dependency-name digest forces that first native release, but the test only checks the still-stale generated manifests and does not require adding the package to NATIVE_DEPENDENCIES. A later package/lockfile bump from 1.x to 2.x leaves the name set unchanged, remains outside the matcher and manifests, and produces the identical fingerprint; I reproduced that transition at this head. Require an authoritative checked native-dependency declaration or package-metadata discovery for every such plugin, then hash its resolved version. -
BLOCKING · scripts/native-fingerprint.mjs:152 · Fingerprint the binary's actual release variant
Hashing every source file is not conservative when the release can omit that source. Both native release lanes succeed without the MeaWallet Nexus credentials: Android then excludes src/meawallet and its SDK, while iOS leaves the canImport-fenced stub, yet either tag receives the same tree fingerprint as a fully featured binary. A later OTA can start calling an existing PushProvisioning method and compare equal to a tag whose shipped binary never had that implementation. Fail native releases closed for this capability, or record a non-secret per-platform capability manifest from the built artifact and compare against that. -
BLOCKING · scripts/native-fingerprint.mjs:125 · Include the resolved iOS package graph
The tracked ios/App/App.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved is consumed by the release xcodebuild archive but is absent from NATIVE_INPUTS. Changing a pinned native SDK revision/version there alone leaves the fingerprint identical; I reproduced this with the Crisp SDK pin. OTA JavaScript can therefore rely on behavior present only in a newer native binary while the comparison against the older tag still passes. Hash Package.resolved (or an artifact-derived resolved dependency manifest) and add a regression that changes one pin.
Checked clean
- Verified the trusted author, exact head SHA, base ref, base SHA, merge base, and four-file diff.
- P1-P8, P10, P11, and P13 were re-checked against the current bridge, dependency, resource, ref-validation, patch, and isolated-fixture implementations; their cited cases are fixed or subsumed by the retained narrower findings.
- Ran the exact-head manifest and --diff HEAD paths successfully; a generic native-plugin 1.x-to-2.x bump and an iOS Package.resolved pin change each reproduced an unchanged fingerprint in isolated checkouts.
- Traced both native release workflows through their credential-gated MeaWallet source/SDK paths and the post-sync iOS package mutation; the release tag is written only after both builds and stores no artifact capability state.
- Reviewed ref handling, fixed git process arguments, workflow quoting, and secret exposure; no separate injection or credential-disclosure defect was found.
- Exact-head aggregate CI, unit, native-export, typecheck, lint, format, analysis, design-system, provenance, and preview checks completed successfully.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: dd00451b89bb · Context: repo, ci · Took 15m
Package.swift declares version ranges; Package.resolved pins the exact revision each native SDK is archived from, and the release xcodebuild consumes it. A pin could move — a Crisp or Alamofire SDK bump — with every other input identical, so OTA JS relying on the newer native behaviour compared equal to a tag built against the older revision. Hash it, with a test that changes one pin.
|
Round 6. One fixed; the other two are the same two I answered last round, and my position hasn't changed — but you're right on a detail I got wrong, so let me correct it. Include the resolved iOS package graph — fixed in Fingerprint the binary's actual release variant — agreed, and I'll correct myself: I described including the source as "the conservative error", and your framing is better. Hashing a file that may not be compiled is not conservative, it is wrong in a different direction — it produces a false green for the omitted-source case, because a tag built without the credentials carries the same hash as one built with them. Neither including nor excluding is sound. No function of the repo tree can distinguish those two binaries, so this cannot be fixed inside a tree fingerprint at all; it needs release CI to fail closed on the capability, or a per-platform capability manifest recorded from the built artifact. I've asked for that to be prioritised now that the MeaWallet secrets are being provisioned, and I would rather file it than pretend the fingerprint covers it. Track generic native plugin version changes — I stand on the previous answer. The only in-script fix is hashing every dependency's resolved version, and on this repo that is 73 Both remaining items are with the humans now. If the ruling is to build either, I'll implement it. |
…day one Hashing every dependency NAME was meant to catch a generically-named native plugin on first declaration. It fired on web-vitals — a pure-JS library added to dev the day after v1.2.0 — and would have refused every staging OTA until someone cut a native release. That is not a hypothetical trade-off any more: it happened on the first real dependency addition after the release, and additions are routine here (73 package.json commits in 60 days against 4 native releases). A guard that blocks the normal path gets deleted, which costs more than the case it closes. Discovery is back to three precise sources: the explicit NATIVE_DEPENDENCIES list, whose completeness is enforced against Capacitor's generated manifests by a test, the name heuristic, and the generated plugin names. Their resolved versions are still hashed, as are patches. The residue is documented in the script: a native plugin named after neither capacitor/cordova nor a first-party scope, added while the committed manifests are stale. Closing it needs an authoritative native/JS classification of every dependency, which is a policy decision for this repo rather than something to hide in a fingerprint. With this, dev's native surface reads unchanged since v1.2.0 — so landing the guard is a no-op for the current OTA lane instead of blocking it.
There was a problem hiding this comment.
Chip review — changes requested
Two blocking false-green paths remain. The latest patch removes the dependency-name backstop, so a generically named native plugin can again be added without moving the fingerprint, and release tags still cannot identify which credential-selected native variant was actually compiled. Exact-head CI is green.
Findings
-
BLOCKING · scripts/native-fingerprint.mjs:320 · Discover native plugins from authoritative metadata
The latest patch deliberately removes the full dependency-name digest, leaving only the explicit list, a name regex, and the committed generated manifests. Add a native plugin whose package name contains none of capacitor, cordova, onesignal, or sumsub while leaving cap-sync output stale: package.json and the lockfile change, but line 320 never adds that package to the version digest, so every fingerprint input remains equal and the OTA can ship its JS wrapper to a binary with no native implementation. The new comment explicitly acknowledges this residue. Store an authoritative native-dependency declaration in the tree, hash its resolved versions, and validate it against installed package metadata or freshly generated manifests before allowing release/OTA CI to pass. -
BLOCKING · scripts/native-fingerprint.mjs:181 · Record the native variant the release actually compiled
Hashing the credential-gated source unconditionally still does not describe the shipped artifact. android-release.yml can succeed with both MeaWallet Nexus secrets absent; app/build.gradle then omits src/meawallet and its SDK, while release-native.yml tags the same commit and this line gives that tag the same fingerprint as a fully featured build. A later OTA can start calling an existing PushProvisioning method without changing any native input, compare equal to that tag, and reach a binary that never registered the plugin. iOS has the analogous canImport-fenced stub. Make production native releases fail closed when the capability inputs are absent, or persist a non-secret per-platform capability marker derived from each built artifact and include it in the OTA comparison.
Checked clean
- Confirmed the detached worktree HEAD, supplied base SHA, and merge base exactly match the requested SHAs; the worktree is clean.
- Read the trusted PR metadata, full four-file diff, fingerprint implementation and tests, both native release workflows, release tag workflow, Android source-set selection, and iOS post-sync variant logic.
- Exact-head ci-success, unit, native-export, typecheck, eslint, format, analyze, design-system, provenance, and preview checks all completed successfully.
- The CLI manifest and --diff HEAD paths passed at the pinned head; focused local Jest was unavailable because this detached worktree has no node_modules.
- P12, P14, and P17 are duplicate consequences of the unresolved generic-plugin classification gap retained as P8; they are not repeated as separate findings.
- P18 is the same credential-selected artifact-variant gap retained as P15; it is not repeated as a separate finding.
- Security, adversarial, and slop passes covered git argument handling, workflow interpolation, missing refs, stale generated manifests, lockfile-only changes, patch identity, resource inputs, source-set selection, test isolation, and resolved Swift package pins; no separate actionable defect was found.
Security review by moonshotai/kimi-k3: 0 finding(s), marked with the model name. It reads the diff only and answers only security, privacy and money, so treat its findings as advice.
Third opinion by claude-opus: 0 finding(s), marked with the model name. It answers only product truth, missing tests and the cross-repo contract, so treat its findings as advice.
Exact head: 746d877600a2 · Context: repo, ci · Took 9m
|
Merging with two findings open, deliberately. Recording why, since both are labelled BLOCKING and I am overriding that. Seven rounds; rounds 1–6 found eleven real defects and every one is fixed — bridges, resource contracts, patches, SPM pins, the postsync SDK pins, Round 7 raised nothing new. The two that remain are the same two from rounds 4, 5 and 6: Discover native plugins from authoritative metadata. The residue is real and documented in the script: a native plugin named after neither capacitor/cordova nor a first-party scope, added while cap-sync output is stale. Closing it needs an authoritative native/JS classification of every dependency, enforced on every addition. That is a policy decision about how this repo takes dependencies — roughly 200 packages to classify and a new gate on every future Record the native variant the release actually compiled. You are right, and I already conceded the framing: hashing the gated source is not conservative, it is wrong in a different direction. No function of the repo tree can distinguish a binary built with the MeaWallet credentials from one built without them, so this cannot be fixed inside a tree fingerprint at all. It needs release CI to fail closed on the capability, or a per-platform capability marker derived from the built artifact. Filed as a follow-up and flagged to be done alongside provisioning those secrets, which is in progress. What is landing is strictly better than what exists today, which is no check at all, and it is currently a no-op for the live OTA lane — Resolving the open threads on that basis rather than because they were addressed. |
1 participant
Makes "the JS assumes native code the receiving binary doesn't have" a red CI job at OTA time instead of a crash on a user's device.
The gap
A bundle's only identity is a version number, and a version number says nothing about the native surface it needs.
1.2.1does not encode "requires the 8.51 updater plugin". So JS and native ship on two independent clocks with nothing connecting them, and CI happily publishes a bundle built against new plugins onto binaries built months earlier.Nothing caught this before:
min_update_versionblocks delivery only, only under themetadatachannel strategy (production is onmajor, so today it is inert), and it lives in a dashboard CI cannot read.The live example: #2968 bumps
@capgo/capacitor-updater8.45.9 → 8.51.14. A plugin's JS wrapper ships in the OTA bundle while its native half stays in the binary, so devices on the 1.1.0 shell would run 8.51.14 JS against 8.45.9 native — and the updater breaking is the worst thing to break, since it is the recovery path.The check
scripts/native-fingerprint.mjshashes the JS↔native contract. The load-bearing inputs are Capacitor's two generated plugin manifests, which pin every plugin's resolved version in its dependency path:plus
capacitor.config.ts, the gradle files,AndroidManifest.xml,project.pbxproj,Info.plistand both entitlements files. Generated web assets are excluded — they are OTA output and would move the fingerprint on every commit.capgo-deploy.ymlrecomputes it and compares against thev<major>.<build>.0tag the bundle's floor targets. On a mismatch the OTA fails and names the file that moved:A bare "refused" would be unactionable; naming the culprit makes the remedy obvious.
Two design choices worth reviewing
No stored state. The fingerprint is a pure function of the tree, so a shipped binary's surface is exactly what its tag describes — any tag can be fingerprinted retroactively (
--ref v1.1.0), there is no bootstrap problem, and no second source of truth to drift. This also sidesteps the fact that CI cannot commit todev(the ruleset has no bypass actors).MARKETING_VERSION/CURRENT_PROJECT_VERSIONare normalised out.native-ios-postsync.jsstamps them on everycap sync; left raw, the check would refuse an OTA after every release. A check that cries wolf gets deleted, so this one had to be precise about what counts as the surface. Covered by a test, alongside one asserting a realpbxprojchange (the deployment floor) still trips it.9 tests, spawning the script the way CI does.
Immediate consequence — please read
dev's native surface has already moved pastv1.1.0(the updater bump, the iOS 16.4 floor, #2754's native additions). Once this lands, staging OTAs — auto-published on everydevmerge — will fail until v1.2.0 ships.That is the check working, not a bug: staging testers run production binaries, so today's
devJS genuinely does not fit them. It self-resolves the moment v1.2.0 lands. Flagging it because it will look alarming in the first red run.There is deliberately no override input. The remedy is to cut a native release; an escape hatch is how guards like this die. If it proves too strict in practice, adding one is a one-line follow-up — but start strict.
Not in this PR
The channel is still on the
majorstrategy, somin_update_versionremains inert. Flipping it tometadataneeds the client to learn thedisableAutoUpdateMetadatarejection code first (NEWER_BINARY_ERRORSonly knowsto_major/to_minor, so gated devices would trip the 3-strike failure streak and flood Sentry), and it needs a Capgo Super Admin. Tracked separately.