-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Snyk] Security upgrade electron from 22.2.0 to 22.3.3 #16071
Conversation
The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JS-ELECTRON-3356652 - https://snyk.io/vuln/SNYK-JS-ELECTRON-3356653 - https://snyk.io/vuln/SNYK-JS-ELECTRON-3361725 - https://snyk.io/vuln/SNYK-JS-ELECTRON-3361731
Oh yes, we actually do. We are actually writing down guidelines on how to review these PRs. But for now, since we are updating electron here, it would be ideal if we can have general testing of the app and make sure nothing broke due to the package bump. |
Release Notes 22.2.0 -> 22.3.3Features
Fixes
Other changes
It seems there are no major refactors or changes that can affect or break the app. |
@@ -135,7 +135,7 @@ | |||
"css-loader": "^6.7.2", | |||
"diff-so-fancy": "^1.3.0", | |||
"dotenv": "^16.0.3", | |||
"electron": "22.2.0", | |||
"electron": "^22.3.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pecanoro I noticed that package-lock.json
Synk generated is different from ours after npm i
"electron": "^22.3.3", | |
"electron": "22.3.3", |
Should we commit this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, as long as everything is still working 😄 It is not letting me commit the suggestion, so I will push to the PR.
Reviewer Checklist
Screenshots/VideosWebweb.movDesktopdesktop.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All apps are working as normal. No console errors.
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by https://github.com/pecanoro in version: 1.2.88-0 🚀
|
🚀 Deployed to staging by https://github.com/pecanoro in version: 1.2.88-0 🚀
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 1.2.88-2 🚀
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 1.2.88-2 🚀
|
Details
Vulnerabilities that will be fixed
With an upgrade:
SNYK-JS-ELECTRON-3356652
SNYK-JS-ELECTRON-3356653
SNYK-JS-ELECTRON-3361725
SNYK-JS-ELECTRON-3361731
Check the changes in this PR to ensure they won't cause issues with your project.
Fixed Issues
$ #16072
PROPOSAL: https://github.com/Expensify/App/issues/16072_ISSUE(COMMENT)
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android