[Snyk] Fix for 2 vulnerabilities#97055
Conversation
The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JS-BRACEEXPANSION-18313044 - https://snyk.io/vuln/SNYK-JS-JSYAML-18313070
|
This release includes major version upgrades for critical development tools including ESLint, Jest, and Glob, each introducing significant breaking changes. Action is required to ensure compatibility, particularly for ESLint's new configuration system and Jest's updated API. Top 3 Most Impactful Upgrades
|
The following vulnerabilities are fixed with an upgrade: - https://snyk.io/vuln/SNYK-JS-BRACEEXPANSION-18313044 - https://snyk.io/vuln/SNYK-JS-JSYAML-18313070
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e317e6cd1
ℹ️ 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".
| "diff-so-fancy": "^1.3.0", | ||
| "dotenv": "^16.0.3", | ||
| "eslint": "^9.36.0", | ||
| "eslint": "^10.0.0", |
There was a problem hiding this comment.
Regenerate the dependency lockfile
These four version changes leave package-lock.json unchanged: its root entries still request ESLint 9.36, glob 10.4, Jest 29.7, and jest-cli 29.7 and resolve those old majors. I checked the install step in .github/actions/composite/setupNode/action.yml, which runs npm ci; npm's ci documentation explicitly states that it exits with an error when lockfile dependencies do not match package.json. Consequently, clean CI installs fail, while cached installs continue using the old dependency tree and do not apply the advertised vulnerability fixes.
Useful? React with 👍 / 👎.
|
|
I can't push an automated fix for this one, and it's worth explaining why rather than pushing something unverified. The root cause is already detailed in my earlier comment: Snyk bumped A correct fix here cannot be a package.json-only edit: it requires regenerating Recommended path (needs a human with a working
Given the breaking scope and that the lockfile wasn't updated, closing this Snyk PR in favor of that targeted |
|
Re-checked on this retry — the situation is unchanged, so I won't repeat the full write-up. Root cause and recommended fix are already covered in my root-cause analysis and follow-up above: Snyk's I still can't push a verified fix from here: this needs |
Explanation of Change
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
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