dotansimha / graphql-code-generator

A tool for generating code based on a GraphQL schema and GraphQL operations (query/mutation/subscription), with flexible support for custom plugins.
https://the-guild.dev/graphql/codegen/
MIT License
10.83k stars 1.33k forks source link

Version 1.8.0 introduces dependency to mem@1.1.0 #2718

Closed cdupuis closed 5 years ago

cdupuis commented 5 years ago

Version 1.8.0 seems to introduce a transitive dependency to mem@1.1.0 which raises security vulnerability alerts, eg at https://snyk.io/vuln/npm:mem:20180117

The dependency tree looks as follows:

@atomist/automation-client@1.8.0 /Users/cdupuis/Development/atomist/automation-client-ts
└─┬ @graphql-codegen/typescript@1.8.0
  └─┬ @graphql-codegen/visitor-plugin-common@1.8.0
    └─┬ relay-compiler@6.0.0
      └─┬ yargs@9.0.1
        └─┬ os-locale@2.1.0
          └── mem@1.1.0 

Is there any chance this dependency can be updated to mitigate this alert?

(Updated to include a proper link to a security advisory)

jeroenvisser101 commented 5 years ago

Related: https://github.com/yargs/yargs/pull/1270

jeroenvisser101 commented 5 years ago

This was introduced in https://github.com/dotansimha/graphql-code-generator/pull/2540

dotansimha commented 5 years ago

This was introduced by adding relay-compiler in order to allow selection set optimizations. I'm thinking about using the optimizers as-is, without the entire relay-compiler library. @n1ru4l what do you think?

n1ru4l commented 5 years ago

How would mem be a security issue? GraphQL Code Generator is a Development only Tool 🤷‍♂️.

@dotansimha what world ne the benefit of doing this?

dotansimha commented 5 years ago

@n1ru4l you are right, it's a dev tool, but it's still causes a warning to show, and that's the only reason I would like to fix that ;) I guess you are right, it might not worth the benefit or maintaing it in our codebase.

n1ru4l commented 5 years ago

I think this should be fixed in relay and then we can update the relay-compiler package.

I can understand that the warning can be annoying, but unfortunately snyk and co should do a better job when dealing with dev dependencies vs production dependencies 😅.

dotansimha commented 5 years ago

@n1ru4l you are right. Closing this issue. When relay-compiler will update it, we'll update as well. If you wish to workaround that, you can use Yarn resolutions field.

jeroenvisser101 commented 5 years ago

As an aside, the code that uses yargs is the binary version of relay-compiler and was never used by us to begin with, but indeed, this is for relay-compiler to fix. I didn't find any open issues there though, nobody seems to have mentioned this before, at least not in their current monorepo.