[NoQA] Bump expo 54.0.10 → 54.0.22 (Snyk security fix)#83666
Conversation
Addresses Snyk-identified vulnerabilities in expo by bumping to 54.0.22 without the unrelated major version bumps (jest 30, glob 12, etc.) that were included in the original Snyk PR #83596. - Update expo 54.0.10 → 54.0.22 in package.json - Rename expo patch file for new version - Update patches/expo/details.md reference - Regenerate package-lock.json Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com>
The version bump in package.json was missing the corresponding Podfile.lock update, causing the verifyPodfile CI check to fail. Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com>
|
Fixed the failing Regarding the |
…ility expo@54.0.22's iOS Swift code references ExpoReactNativeFactoryProtocol, which is defined in expo-modules-core@3.0.24 but not in 3.0.18. This was causing the iOS HybridApp build to fail with Swift compilation errors. Changes: - Bump expo-modules-core from 3.0.18 to 3.0.24 in package.json - Update package-lock.json accordingly - Rename patch file to match new version - Update ios/Podfile.lock version reference Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com>
|
Fixed the iOS HybridApp build failure. The root cause was a missing
Changes pushed:
Regarding the |
minipass 7.1.3 changed its license from ISC to BlueOak-1.0.0, which is not in the Snyk approved license list. The expo 54.0.22 upgrade pulls in glob@13 which requires minipass ^7.1.3, triggering the license change. Pinning minipass to 7.1.2 (ISC) via npm overrides resolves the Snyk license compliance failure. Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com>
|
Fixed the Root cause: The expo 54.0.22 upgrade pulls in Fix: Added Note: If Snyk still flags the license after the re-scan, it may be flagging one of the other new BlueOak-1.0.0 transitive dependencies (e.g., nested |
The global minipass v7.1.2 override was forcing all nested minipass instances to v7, but @sentry/bundler-plugin-core's glob dependency requires minipass v4.x which provides a default export. The override caused a SyntaxError in Storybook builds and a spellcheck failure. The expo bump is the actual security fix; the minipass override is unnecessary. Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com>
|
Fixed the failing spellcheck and Storybook tests checks by removing the Root cause: The global The spellcheck also failed because Fix: Removed the unnecessary Note: The |
glob@13.0.6 (from expo 54.0.22) requires minipass@^7.1.3, but the lockfile had minipass@7.1.2 pinned which does not satisfy the constraint, causing npm ci to fail on all builds. Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com>
|
Fixed the failing build checks by updating Root cause: The expo 54.0.22 bump brought in What was fixed:
Remaining: |
|
The only failing check is Root cause: The expo bump (54.0.10 → 54.0.22) transitively bumped What's needed: A Snyk admin needs to approve the All other checks are either passing or still running. |
|
@marcaaron Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@MelvinBot is this still needed? expo-modules-core+3.0.24+001+disableViewRecycling.patch |
|
Yes, this patch is still needed after the expo bump to 54.0.22. What the patch does: It adds Why it's still needed: The Expo team did fix this upstream in expo/expo#31841, but that fix only modifies Version alignment: With this expo bump, |
|
Cool, then it's good for a review. |
|
Seems the license/snyk job is failing? Not sure what that is doing tbh. Did anyone test this? |
marcaaron
left a comment
marcaaron
left a comment
There was a problem hiding this comment.
Code itself looks fine. Not sure if we are wanting to test this before QA gets their hands on it or how we would do this. I'd guess a lot of things could potentially be impacted by Expo version bump that are not immediately obvious.
| + | ||
| #pragma mark - Events | ||
|
|
There was a problem hiding this comment.
Did we verify that the file being patched didn't materially change? Also why are there whitespace changes?
|
If this requires C+ testing, I can help. |
The expo 54.0.22 upgrade transitively bumped lightningcss from 1.30.1 to 1.31.1, which introduced a new lightningcss-android-arm64 optional binary not yet approved in Snyk's license policy. Adding an npm override to pin lightningcss at 1.30.1 removes the unapproved package while keeping the expo upgrade intact. Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com>
|
Fixed the Root cause: The expo 54.0.22 upgrade transitively bumped Fix: Added The other failing check ( |
Co-authored-by: Michał Jasikowski <mjasikowski@users.noreply.github.com>
|
Fixed the failing The other failing check ( |
|
All checks are now passing. The previously failing checks ( |
|
@situchan I'll leave it to @mjasikowski - but thinking maybe at a minimum we could look at the diff for this version bump and try to identify if any of our expo packages are affected. @MelvinBot Is that something you can do? If so, leave a comment here with a brief audit of what core features could possibly be affected by this version bump and suggestions on what to test. |
Expo 54.0.10 → 54.0.22 — Impact AuditPackages That Changed Version
Unchanged direct expo dependencies: Notable Upstream Changes (No Breaking Changes Found)🔒 Security / Crash Fixes:
🐛 Bug Fixes: Feature-to-Risk MappingSince only
Suggested Test PlanPriority 1 — Directly affected (test on both iOS and Android):
Priority 2 — Indirectly affected (test on both platforms):
Priority 3 — Build/tooling (validate once):
Low priority — unlikely to be affected:
|
mjasikowski
changed the title
|
@marcaaron I don't think it needs C+ testing; it's a bugfix version bump, I highly doubt it will cause any problems - and if, they will be immediately visible during QA |
|
@mollfpr can you paste the reviewer checklist before we merge? |
Reviewer Checklist
|
|
🚧 @mollfpr has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/mollfpr in version: 9.3.32-0 🚀
|
|
🚀 Deployed to production by https://github.com/blimpich in version: 9.3.32-3 🚀
|
6 participants
Explanation of Change
This is a focused security fix extracted from Snyk PR #83596, which bundled several breaking major version bumps alongside the actual vulnerability fix.
This PR only bumps
expofrom54.0.10to54.0.22to address Snyk-identified vulnerabilities. The original Snyk PR also included major version bumps forjest(29→30),glob(10→12),@sentry/webpack-plugin(4→5), andreact-native(0.81→0.84) — all of which caused CI failures and are not included here.Changes:
expo: 54.0.10 → 54.0.22 inpackage.jsonexpo+54.0.22+001+fix-missing-blob-variable-error.patch)patches/expo/details.mdto reference the renamed patch filepackage-lock.jsonNote:
ios/Podfile.lockmay need updating vianpx pod-installon macOS to reflect the new expo version.Fixed Issues
$
PROPOSAL:
Tests
npm ciand verify it completes without errorsOffline tests
N/A — dependency version bump only, no behavioral changes.
QA Steps
[No QA] — This is a patch version bump of expo (54.0.10 → 54.0.22) to address security vulnerabilities. No functional changes.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
N/A — dependency version bump only
Android: mWeb Chrome
N/A — dependency version bump only
iOS: Native
N/A — dependency version bump only
iOS: mWeb Safari
N/A — dependency version bump only
MacOS: Chrome / Safari
N/A — dependency version bump only