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

Explore ideas to avoid installing uWebSockets.js from github #5581

Closed
belgattitude opened this issue Jun 16, 2023 · 6 comments
Closed

Explore ideas to avoid installing uWebSockets.js from github #5581

belgattitude opened this issue Jun 16, 2023 · 6 comments

Comments

@belgattitude
Copy link
Contributor

belgattitude commented Jun 16, 2023

Is your feature request related to a problem? Please describe.

In recent version I started to have issues with the install of @graphql-mesh/cli related to https://github.com/uNetworking/uWebSockets.js/

"dependencies": {
    "uWebSockets.js": "uNetworking/uWebSockets.js#v20.30.0",
  },

https://github.com/Urigo/graphql-mesh/blob/09196ea90bb8a5d52eb2e3a38c23cef30b6f6b24/packages/cli/package.json#LL75C24-L75C59

It generally works but not in every situations

image

And really I don't feel it trustable at all (even to the point to avoid graphql-mesh totally)

See: belgattitude/compare-package-managers#20

Describe the solution you'd like

Avoid making installs less reliable or risky... if uWebsockets was packaged as napi (napi-rs... with supportedArchitectures) would be less a problem.

Or make it optional so there's a way to avoid it at install. It also adds 10s-30s per install + size on lambdas (cold starts)

Describe alternatives you've considered

I'd love to have a way to prevent install of that kind of deps. Node ecosystem is already complicated (caching, package managers)

Additional context

@ardatan
Copy link
Owner

ardatan commented Jun 16, 2023

This is the recommended way of installing that package by its maintainer;
https://github.com/uNetworking/uWebSockets.js/
I don't see any reasons to see it not trustable.
If you want to avoid installing any dependencies, you can follow the recipes here for lambdas and use a bundler to get a single executable.
https://the-guild.dev/graphql/mesh/docs/getting-started/deploy-mesh-gateway

@belgattitude
Copy link
Contributor Author

belgattitude commented Jun 16, 2023

I don't see any reasons to see it not trustable

Security requirements varies. Difficult to audit all kinds of installs... node gyp, download on postinstalls... really depends where you use it and company rules

Ideally I'd suggest to take care of not bringing too much diversity in deps installs. Could be interesting for users to opt in specifically for websocket features and in that case consider the risk of adding native binaries from a git repo... security but also reproducibility, ie ensure platform support (musl, aarch64...)

Not an expert though and i know it isn't directly related to the mesh. Just sharing that I'd prefer to be able to opt out (an optional peer dep for example)

@ardatan
Copy link
Owner

ardatan commented Jun 16, 2023

Security requirements varies. Difficult to audit all kinds of installs... node gyp, download on postinstalls... really depends where you use it and company rules

This is the first time I hear that to be honest :) Node-gyp is part of Node.js environment and Mesh CLI doesn't aim other environments. For other environments, we recommend other deployment methods.

Could be interesting for users to opt in specifically for websocket features

uWebSockets.js is not "websockets". It is a HTTP server implementation.

Still I don't see any security problems here. You are free to fork and replace it however you want since it is an Open Source project.

@HIMISOCOOL
Copy link

Id also love to use graphql-mesh but cannot as you cannot install from github at my company. It would be great if it was a peer dep or somthing like that

@wiktor-obrebski
Copy link

This is the recommended way of installing that package by its maintainer;
https://github.com/uNetworking/uWebSockets.js/
I don't see any reasons to see it not trustable.

Still I don't see any security problems here. You are free to fork and replace it however you want since it is an Open Source project.

@ardatan The maintainer can, at any time, delete an old git tag and replace it with a new one that has the same name but entirely different code, potentially introducing malicious changes. While you might trust the current maintainer, there's no guarantee about who will take up that role in the future. This issue doesn't exist with packages installed via npm.
While the risk can be somewhat mitigated through the use of a package-lock.json file, this isn't an ideal solution. Not everyone employs this method, and IMHO, it introduces an unnecessary risk for your users..

This also leads me to question the extent of the project's focus on adhering to security best practices.

I've been exploring the possibility of integrating GraphQL Mesh into a major commercial project. However, I have reservations about this aspect, which currently makes it difficult for me to fully endorse its use.

@bigman73
Copy link

bigman73 commented Apr 17, 2024

Using graphql mesh is constantly breaking the lock file

#10 0.512  WARN  Broken lockfile: no entry for 'uWebSockets.js@https://codeload.github.com/uNetworking/uWebSockets.js/tar.gz/1977b5039938ad863d42fc4958d48c17e5a1fa06' in pnpm-lock.yaml
#10 0.513  ERR_PNPM_LOCKFILE_MISSING_DEPENDENCY  The lockfile is broken! Resolution step will be performed to fix it.

Using uWebSockets in such a way is not just a security issue but a real functional problem concerning the build process.

Please address the concern.

One solution is for the maintainer to publish the package, or for this project to publish the package and have a stable copy which also solves the security concerns mentioned above by other comments.

This was referenced Apr 30, 2024
This was referenced May 7, 2024
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

No branches or pull requests

5 participants