Fix missing hash verification - #49
Conversation
55be8cc to
5529153
Compare
There was a problem hiding this comment.
Pull request overview
Adds integrity verification for unsigned full ZIP updates on Android and iOS.
Changes:
- Runs folder-hash verification unconditionally for ZIP updates.
- Generates real hashes in the E2E mock server.
- Aligns rollback expectations with native behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
android/.../CodePushUpdateManager.java |
Enforces Android ZIP content verification. |
ios/CodePush/CodePushPackage.m |
Enforces iOS ZIP content verification. |
test/test.ts |
Computes mock hashes and updates rollback expectations. |
code-push-plugin-testing-framework/script/serverUtil.js |
Associates archives with computed hashes. |
code-push-plugin-testing-framework/typings/code-push-plugin-testing-framework.d.ts |
Declares the new hash API. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| ); | ||
| } | ||
|
|
||
| CodePushUpdateUtils.verifyFolderHash(newUpdateFolderPath, newUpdateHash); |
There was a problem hiding this comment.
Acceptable, the non-ZIP update codepath is considered legacy, we don't recommend generating plain JS bundle updates (fortunately, only around 1% of our users do). It's not just the integrity check that's missing from this codepath, but also code signing.
| private updateMockPackageHash(bundleFolder: string, isDiff: boolean, archivePath: string): string { | ||
| if (!isDiff) { | ||
| ServerUtil.setPackageHashForPath(archivePath, computeUpdateContentsHash(bundleFolder)); | ||
| } |
There was a problem hiding this comment.
Acknowledged in PR description, see decisions section.
| ); | ||
| } | ||
|
|
||
| if (!isSignatureVerificationEnabled && isSignatureAppearedInBundle) { |
There was a problem hiding this comment.
How likely it is in the wild? Can you release an app without signing it to the store (if not, that app should in practice always have a public key)?
There was a problem hiding this comment.
This isn't about code signing the native app itself, it's just the CodePush OTA upload. And CodePush updates are not code-signed by default, we don't enforce this. Other products also don't enfoce code signing, in fact, Expo limits this feature to their highest paid tiers.
There was a problem hiding this comment.
What will provide the stringPublicKey used for the isSignatureVerificationEnabled?
There was a problem hiding this comment.
Coming from here:
Essentially, this is coming from the app's strings.xml
There was a problem hiding this comment.
And the public key in strings.xml and Info.plist is optional or mandatory?
| * pointing exports.updatePackagePath at this archive gets the matching package_hash instead | ||
| * of the one filled in by default. | ||
| */ | ||
| function setPackageHashForPath(archivePath, packageHash) { |
There was a problem hiding this comment.
Can 2 tests use the same archivePath and different packageHash? If so running those tests migth step en each others to by overriding the hash for the path in the map.
| "Please ensure that public key is properly configured within your application."); | ||
| } | ||
|
|
||
| if (
a1ace37 to
016f033
Compare
Why
Android and iOS skipped hash verification entirely for a full (non-diff), unsigned update - so a corrupted or tampered download could install with no integrity check. Diff updates and signed updates are not affected.
What
CodePushUpdateManager.java) and iOS (CodePushPackage.m): hash verification now always runs, regardless of diff/full or signing status.localPackage.install.revert.dorevertto match actual native rollback behavior (rejected on the first revert, not the second). It only passed before because the mock's random hash bug made a re-served identical package look like a different update on retry.Decisions
updateMockPackageHash()/applyKnownPackageHash().