grafana / pyroscope-nodejs

Pyroscope NodeJS integration
Apache License 2.0
26 stars 18 forks source link

fix: Bump axios version #40

Closed bryanhuhta closed 5 months ago

bryanhuhta commented 6 months ago

Fixes CVE-2023-45857. I also removed the "compatible with" operator to avoid blindly accepting more upstream changes that might break us.

bryanhuhta commented 6 months ago

After more research, there are two problems with simply bumping the axios version to address this CVE:

  1. v1 axios changes its module export from CJS modules to ESM
  2. This library may not even be impacted by this CVE

For 1, upgrading to axios v1 causes our Jest test to break, which can be fixed, but this signals we may be breaking users of the library. We should proceed with caution here.

For 2, CVE-2023-45857 has reproduction steps which include:

const instance = axios.create({
  withCredentials: true,
});

This library doesn't use the withCredentials: true option and thereby may not actually be susceptible to this vulnerability.

simonswine commented 5 months ago

Went with #56 #57 instead