Repository navigation
Downgrade pdfjs-dist to 5.5.207 to address security advisory - #103023
Conversation
|
@eVoloshchak 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] |
|
The failing check is unrelated to this PR, so I didn't push a fix. Analysis: Evidence:
The test needs a fix on |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 361f463ac5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
I re-checked, and nothing has changed. The failure is still unrelated to this PR, so I didn't push a fix.
When a fix lands on |
|
The failing check is still unrelated to this PR, so I didn't push a fix.
When a fix for that test lands on |
|
The failing check is unrelated to this PR, so I didn't push a fix. Analysis: In Jest Unit Tests / test (job 2), the OS killed the Jest worker ( Evidence: This PR only changes Next step: Rerun the failed job. It should pass. The |
|
@Pujan92 @robertjchen One of you needs to 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] |
|
🚧 robertjchen 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/robertjchen in version: 9.5.6-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.5.6-6 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
Downgrades
pdfjs-distfrom 5.6.205 to 5.5.207 and pins it exactly, to fix GHSA-hq66-cqwq-w95j (high: arbitrary JS execution on opening a malicious PDF). The bug was introduced in 5.6.83, so 5.5.207 predates it. There is no patched 5.x.Why downgrade instead of upgrading to 6.x:
react-pdf@11(viareact-fast-pdf), which makes Suspense the default and raises the minimum browsers to Chrome 125 / Safari 18. Other projects are pinning below the affected range for the same reason, e.g. modelcontextprotocol/ext-apps#789.react-fast-pdfandwebpack#80160 set it to drop our worker patch. The bump to 5.6.205 was an unintended side effect of the lockfile refresh in Introduce a Bottom Tab Navigator for Top-Level Navigation #85234, not a fix we needed.The
react-pdf@11/ pdf.js 6 upgrade will be a separate follow-up.Usage: transitive via
react-fast-pdf→react-pdf. Renders PDF attachments/receipts on web/desktop (PDFView,ReceiptPDFOverlay,PDFThumbnail,LocalPDFReceiptPreview). Native usesreact-native-pdf(unaffected).Fixed Issues
$ https://github.com/Expensify/Expensify/issues/688714
PROPOSAL:
Tests
Offline tests
N/A, no network behavior change.
QA Steps
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari