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

Bump isomorphic-unfetch in @appsignal/core #621

Closed
MarkZsombor opened this issue Apr 15, 2024 · 6 comments · Fixed by #622
Closed

Bump isomorphic-unfetch in @appsignal/core #621

MarkZsombor opened this issue Apr 15, 2024 · 6 comments · Fixed by #622
Assignees

Comments

@MarkZsombor
Copy link

The version of isomorphic-unfetch (v3.1.0) used in the current version of @appsignal/core (v1.1.20) is using a library node-fetch v2.6.1 which has a known security venerability. GHSA-r683-j2x4-v87g

Is it possible to bump isomorphic-unfetch to v4 which has the patched version of node-fetch ?

@unflxw unflxw self-assigned this Apr 22, 2024
@unflxw
Copy link
Contributor

unflxw commented Apr 22, 2024

Thanks for letting us know @MarkZsombor! I'll take a look.

@unflxw
Copy link
Contributor

unflxw commented Apr 22, 2024

I have bumped up the isomorphic-unfetch dependency to v4.0.0 in order to remove warnings about it on automated vulnerability detectors. You can upgrade to @appsignal/[email protected], or upgrade any packages which depend on it, which will remove the warning.


However, I would like to clarify that there is no vulnerability to be exploited in how AppSignal uses isomorphic-unfetch.

First of all, note that neither isomorphic-unfetch nor node-fetch are used in the AppSignal for JavaScript (front-end) integration at all.

The isomorphic-unfetch library is only used by our CLI installer (so, not at run-time, and only server-side) and the only data transmitted through it is the AppSignal API key, which is validated by the CLI installer by performing a request to our servers.

Furthermore, the request that is performed does not use headers to authenticate, therefore the vulnerability where headers are passed in cross-domain requests is not relevant.

Finally, since the request is made over HTTPS, there is no reasonable attack model to exploit this vulnerability and steal the AppSignal API key -- an attacker who can do an HTTPS man-in-the-middle to make AppSignal return a redirect that node-fetch would follow, could simply intercept the API key within the request.

@unflxw unflxw closed this as completed Apr 22, 2024
@fekle
Copy link

fekle commented Apr 22, 2024

Hi there, unfortunately the switch to isomorphic-unfetch > 4 now causes all my builds with esbuild to fail:

✘ [ERROR] Could not resolve "unfetch"

    node_modules/.pnpm/[email protected]/node_modules/isomorphic-unfetch/index.mjs:8:18:
      8 │         return import("unfetch").then((m) => r(m)(url, opts));
        ╵                       ~~~~~~~~~

  The module "./index.mjs" was not found on the file system:

    node_modules/.pnpm/[email protected]/node_modules/unfetch/package.json:18:16:
      18 │       "import": "./index.mjs",
         ╵                 ~~~~~~~~~~~~~

  You can mark the path "unfetch" as external to exclude it from the bundle, which will remove this
  error and leave the unresolved path in the bundle. You can also add ".catch()" here to handle this
  failure at run-time instead of bundle-time.

✘ [ERROR] Could not resolve "node:http"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/index.js:9:17:
      9 │ import http from 'node:http';
        ╵                  ~~~~~~~~~~~

  The package "node:http" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:https"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/index.js:10:18:
      10 │ import https from 'node:https';
         ╵                   ~~~~~~~~~~~~

  The package "node:https" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:zlib"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/index.js:11:17:
      11 │ import zlib from 'node:zlib';
         ╵                  ~~~~~~~~~~~

  The package "node:zlib" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:stream"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/index.js:12:52:
      12 │ import Stream, {PassThrough, pipeline as pump} from 'node:stream';
         ╵                                                     ~~~~~~~~~~~~~

  The package "node:stream" wasn't found on the file system but is built into node. Are you trying
  to bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:buffer"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/index.js:13:21:
      13 │ import {Buffer} from 'node:buffer';
         ╵                      ~~~~~~~~~~~~~

  The package "node:buffer" wasn't found on the file system but is built into node. Are you trying
  to bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:util"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/headers.js:7:20:
      7 │ import {types} from 'node:util';
        ╵                     ~~~~~~~~~~~

  The package "node:util" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:url"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/request.js:9:34:
      9 │ import {format as formatUrl} from 'node:url';
        ╵                                   ~~~~~~~~~~

  The package "node:url" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:fs"

    node_modules/.pnpm/[email protected]/node_modules/fetch-blob/from.js:1:59:
      1 │ import { statSync, createReadStream, promises as fs } from 'node:fs'
        ╵                                                            ~~~~~~~~~

  The package "node:fs" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:stream"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/body.js:8:34:
      8 │ import Stream, {PassThrough} from 'node:stream';
        ╵                                   ~~~~~~~~~~~~~

  The package "node:stream" wasn't found on the file system but is built into node. Are you trying
  to bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:net"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/utils/referrer.js:1:19:
      1 │ import {isIP} from 'node:net';
        ╵                    ~~~~~~~~~~

  The package "node:net" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:http"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/headers.js:8:17:
      8 │ import http from 'node:http';
        ╵                  ~~~~~~~~~~~

  The package "node:http" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:util"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/request.js:10:24:
      10 │ import {deprecate} from 'node:util';
         ╵                         ~~~~~~~~~~~

  The package "node:util" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:path"

    node_modules/.pnpm/[email protected]/node_modules/fetch-blob/from.js:2:25:
      2 │ import { basename } from 'node:path'
        ╵                          ~~~~~~~~~~~

  The package "node:path" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:util"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/body.js:9:42:
      9 │ import {types, deprecate, promisify} from 'node:util';
        ╵                                           ~~~~~~~~~~~

  The package "node:util" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

✘ [ERROR] Could not resolve "node:buffer"

    node_modules/.pnpm/[email protected]/node_modules/node-fetch/src/body.js:10:21:
      10 │ import {Buffer} from 'node:buffer';
         ╵                      ~~~~~~~~~~~~~

  The package "node:buffer" wasn't found on the file system but is built into node. Are you trying
  to bundle for node? You can use "platform: 'node'" to do that, which will remove this error.

Looks like v4 is now node or ESM-only, at least with esbuild. Setting the esbuild target to node is not an option, as I'm using this in a frontend application.

Over at unfetch there is an open issue with a similar problem and webpack: developit/unfetch#176 but not much activity.

Unfortunately I haven't had time to look into this further, just wanted to give a heads up in case anyone else runs into this problem. :)

Maybe the easiest solution would be to downgrade isomorphic fetch again while figuring this out, especially given that the vulnerability does not affect appsignal?

@unflxw
Copy link
Contributor

unflxw commented Apr 23, 2024

Hi @fekle, I will take a look at this and let you know.

@unflxw
Copy link
Contributor

unflxw commented Apr 23, 2024

Hi @fekle,

First of all, thank you for reporting this issue. I have not been able to reproduce the exact issue you're encountering, but I have been able to reproduce this similar issue: developit/unfetch#164. I was able to fix it by using version 4.0.0 instead of 4.0.2, which forces the usage of unfetch@^4 instead of unfetch@5.

However, a more fundamental point is that isomorphic-unfetch shouldn't be a dependency in @appsignal/core at all. It's only used by the @appsignal/cli package, and as such it should be a dependency there instead.

We have now released @appsignal/[email protected] (as well as version bumps of other packages that depend on it) which does not depend on isomorphic-unfetch at all -- the dependency has been moved to @appsignal/cli instead, which should never be bundled. Upgrading should fix the issue you're encountering -- please let us know if it doesn't.

/cc @menno

@fekle
Copy link

fekle commented Apr 23, 2024

Hi @unflxw, thank you very much!

Upgrading to 1.3.30 has indeed resolved the issue.
Moving the dependency to the cli looks like a good solution, iwas indeed wondering why this library contains a fetch polyfill for node ;-)

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 a pull request may close this issue.

3 participants