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

fix(security): update vulnerable packages, include React 17 in peerDeps #28545

Merged
merged 2 commits into from
Jan 15, 2021

Conversation

hoobdeebla
Copy link
Contributor

@hoobdeebla hoobdeebla commented Dec 9, 2020

This PR aims to reduce the amount of warnings users see from their package manager on first install.

  1. Updates React peerDependency of Gatsby and plugins to include React 17
  2. Updates the following packages to remove vulnerabilities and peerDependency warnings: axios, babel-plugin-add-module-exports, contentful-management, debug, documentation, gatsby-plugin-webfonts, hicat, lodash-webpack-plugin, node-fetch, probe-image-size, rewire, subfont, verdaccio, webpack-dev-server, yargs, yurnalist. Major changes are as follows:
Package Change Breaking Changes
babel-plugin-add-module-exports ^0.3.3 -> ^1.0.4 actually work with Babel 7 (does not affect usage here)
contentful-management ^5.26.3 -> ^7.5.1 none that affect usage in gatsby-recipes
documentation ^12.3.0 -> ^13.1.0 require Node 10
hicat ^0.7.0 -> ^0.8.0 none (I made the changes myself)
node-fetch ^1.7.3 -> ^2.6.1 none that affect usage in gatsby-source-graphql
probe-image-size ^5.0.0 -> ^6.0.0 require Node 10, switch from request to needle - none that affect usage in gatsby packages
rewire ^4.0.1 -> ^5.0.0 require Node 8 (does not affect usage in gatsby-plugin-offline)
subfont ^4.2.2 -> ^5.2.5 require Node 10 (does not affect usage in gatsby-plugin-subfont)
yargs ^10.1.2 -> ^15.4.1 none that affect usage at the root of the monorepo
yurnalist ^1.1.2 -> ^2.1.0 improve silent mode, update dependencies - none that affect usage in gatsby-cli

Fixes #28781
Fixes #28858

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 9, 2020
@hoobdeebla hoobdeebla changed the title fix: remove some install warnings fix: remove some first-install warnings Dec 9, 2020
@hoobdeebla
Copy link
Contributor Author

No idea why the bootstrap run keeps failing 🤔 everything bootstraps locally. Even the Windows run (which includes its own bootstrap step) is passing on CI.

Each run fails on the gatsby-plugin-image build step. Here is the output from the most recent run:

lerna ERR! yarn run prepare stderr:
(node:6686) ExperimentalWarning: The fs.promises API is experimental
(node:6722) ExperimentalWarning: The fs.promises API is experimental
(node:6708) ExperimentalWarning: The fs.promises API is experimental
(node:6707) ExperimentalWarning: The fs.promises API is experimental
(node:6730) ExperimentalWarning: The fs.promises API is experimental
internal/child_process.js:358
    throw errnoException(err, 'spawn');
    ^

Error: spawn ENOMEM
    at ChildProcess.spawn (internal/child_process.js:358:11)
    at spawn (child_process.js:533:9)
    at Object.fork (child_process.js:108:10)
    at ChildProcessWorker.initialize (/home/circleci/project/node_modules/jest-worker/build/workers/ChildProcessWorker.js:137:44)
    at ChildProcessWorker.onExit (/home/circleci/project/node_modules/jest-worker/build/workers/ChildProcessWorker.js:263:12)
    at ChildProcess.emit (events.js:182:13)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:240:12)
error Command failed with exit code 1.
ERROR: "build:browser" exited with 1.
error Command failed with exit code 1.
error Command failed with exit code 1.

lerna ERR! yarn run prepare exited 1 in 'gatsby-plugin-image'

The two errors it generates are: 1) fs.promises is an experimental API; 2) there is a memory leak somewhere during the microbundle build. Neither of these errors occur when I bootstrap locally on Node 10.13.0 or 12.20.0. Whether the first, the second, or both errors throw on each CI run seems to be random.

@LekoArts LekoArts added type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 9, 2020
@hoobdeebla hoobdeebla force-pushed the updates/fewer-install-warnings branch 4 times, most recently from 4016e75 to 72014f1 Compare December 15, 2020 00:36
@hoobdeebla
Copy link
Contributor Author

Updated the PR to include an upstream security fix for hicat and a yarn.lock security patch for ini.

I am still receiving the aforementioned errors on CircleCI but not locally. Also, it seems like ALL current PRs are erroring on windows_unit_tests runs because of a connection problem. I would appreciate if a team member could look into these issues.

kailan
kailan previously approved these changes Dec 15, 2020
@hoobdeebla hoobdeebla force-pushed the updates/fewer-install-warnings branch 8 times, most recently from 2d92121 to 6cbf5bf Compare December 22, 2020 01:20
@hoobdeebla
Copy link
Contributor Author

Still getting the bootstrap errors about every 9 out of 10 CI runs (but never locally). Occasionally it passes without erroring, but then it doesn't pass some e2e tests, which I have been unable to run because I develop in WSL.

@hoobdeebla hoobdeebla force-pushed the updates/fewer-install-warnings branch 2 times, most recently from 54e04ef to cd68be6 Compare December 23, 2020 02:41
@hoobdeebla hoobdeebla marked this pull request as draft December 23, 2020 02:44
@hoobdeebla
Copy link
Contributor Author

I guess the issue was because I removed react and react-dom from the peerDeps of gatsby-plugin-image. Again, I only saw this issue on CI and never had a build fail locally with either of the two errors CircleCI gave.

@hoobdeebla hoobdeebla marked this pull request as ready for review December 23, 2020 05:12
JackHowa pushed a commit to JackHowa/gatsby that referenced this pull request Jan 5, 2021
Closes gatsbyjs#28781. Based on the work of gatsbyjs#28545 but more specific to hicat
@LekoArts
Copy link
Contributor

LekoArts commented Jan 5, 2021

Can you please revert Removes React peerDependency from Gatsby dependents since it will be met by Gatsby itself as those are necessary for module resolutions? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: maintenance An issue or pull request describing a change that isn't a bug, feature or documentation change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

npm audit shows 2 high severity vulnerabilities Security vulnerability reported in highlight.js
5 participants