Skip to content
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

chore: update NPM dependencies #685

Merged
merged 5 commits into from
Nov 30, 2023
Merged

chore: update NPM dependencies #685

merged 5 commits into from
Nov 30, 2023

Conversation

jamaljsr
Copy link
Member

I've updated some of the NPM packages to resolve known security vulnerabilities in dependencies.

Since this project uses create-react-app, not all of the dependencies are used in the production build. Many of them, especially the dependencies of react-scripts, are only used in the development environment or during CI builds. Vulnerabilities found in these dev dependencies are less of a threat because 99% of the time, the vulnerability cannot be exploited via the build output (html/js) that runs in the browser.

Here are more detailed explanations of the situation:
facebook/create-react-app#11174
https://overreacted.io/npm-audit-broken-by-design/

To ensure that we are addressing any true security concerns, we should be sure to keep any user facing code listed in the dependencies list and all build-time dependencies in devDependencies. Then we can use the command yarn audit --groups dependencies to easily list all packages that we are using with known vulnerabilities.

Here's the output of this command before the changes in this PR:

$ yarn audit --groups dependencies
yarn audit v1.22.21
...
86 vulnerabilities found - Packages audited: 1490
Severity: 56 Moderate | 27 High | 3 Critical

Here's the output of this command after the changes in this PR. The warnings are due to us now forcing updates to our dependency's dependencies.

$ yarn audit --groups dependencies
yarn audit v1.22.21
warning Resolution field "[email protected]" is incompatible with requested version "jackspeak@^2.3.5"
warning Resolution field "[email protected]" is incompatible with requested version "strip-ansi@^7.0.1"
warning Resolution field "[email protected]" is incompatible with requested version "strip-ansi@^7.0.0"
warning Resolution field "[email protected]" is incompatible with requested version "strip-ansi@^7.0.1"
0 vulnerabilities found - Packages audited: 262

Copy link
Contributor

@itsrachelfish itsrachelfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK, working locally

image

app/.storybook/main.js Show resolved Hide resolved
app/package.json Show resolved Hide resolved
Copy link
Contributor

@ViktorTigerstrom ViktorTigerstrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK LGTM after rebase!

the dependencies of react-scripts are not used in production, so it
makes sense to move them to dev dependencies to minimize the false
positives when running `yarn audit --groups dependencies`. After making
this change, I needed to add some module resolutions in order to keep
storybook and jest working.
@jamaljsr
Copy link
Member Author

Thanks for the reviews. Just rebased on master. I'll merge this once all of the Github checks have succeeded.

@jamaljsr jamaljsr merged commit 1fc8cb6 into master Nov 30, 2023
12 checks passed
@jamaljsr jamaljsr deleted the update-deps branch November 30, 2023 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants