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

AS3: Run npm audit #5161

Closed
glasser opened this issue May 3, 2021 · 1 comment
Closed

AS3: Run npm audit #5161

glasser opened this issue May 3, 2021 · 1 comment

Comments

@glasser
Copy link
Member

glasser commented May 3, 2021

No description provided.

glasser added a commit that referenced this issue May 5, 2021
Our tests use the deprecated `apollo-fetch` package. This depends on an old
version of the `cross-fetch` polyfill package which itself depends on a version
of `node-fetch` with a minor CVE. This makes `npm audit` noisy, which is sad.

There's no real vulnerability from `node-fetch` here since it's only used in
tests, but it would be nice to quiet `npm audit`. Plus, in #5165 we grudgingly
added `"dom"` to the `lib` in `tsconfig.test.base.json` just to support
`apollo-fetch`.

Updating `cross-fetch` in `apollo-fetch` is not enticing because it does pull in
major version bumps of dependent packages; I wouldn't want to release a new
version of a dead project that makes bad changes in some obscure contexts.

But `apollo-fetch` is a pretty simple package. So instead, I just inlined
`apollo-fetch` into `apollo-server-integration-testsuite`, deleted a bunch of
features we aren't using like batch support, and made it use the
`apollo-server-env` `fetch` rather than a global polyfill.

The only use of `apollo-fetch` that didn't already depend on
`apollo-server-integration-testsuite` was `apollo-server`, so add that
devDependency.

Fixes #5161.
glasser added a commit that referenced this issue May 5, 2021
Our tests use the deprecated `apollo-fetch` package. This depends on an old
version of the `cross-fetch` polyfill package which itself depends on a version
of `node-fetch` with a minor CVE. This makes `npm audit` noisy, which is sad.

There's no real vulnerability from `node-fetch` here since it's only used in
tests, but it would be nice to quiet `npm audit`. Plus, in #5165 we grudgingly
added `"dom"` to the `lib` in `tsconfig.test.base.json` just to support
`apollo-fetch`.

Updating `cross-fetch` in `apollo-fetch` is not enticing because it does pull in
major version bumps of dependent packages; I wouldn't want to release a new
version of a dead project that makes bad changes in some obscure contexts.

But `apollo-fetch` is a pretty simple package. So instead, I just inlined
`apollo-fetch` into `apollo-server-integration-testsuite`, deleted a bunch of
features we aren't using like batch support, and made it use the
`apollo-server-env` `fetch` rather than a global polyfill.

The only use of `apollo-fetch` that didn't already depend on
`apollo-server-integration-testsuite` was `apollo-server`, so add that
devDependency.

Fixes #5161.
@glasser
Copy link
Member Author

glasser commented May 5, 2021

Fixed on release-3.0

@glasser glasser closed this as completed May 5, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant