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

Proposal to provide package safety check #96

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
55 changes: 55 additions & 0 deletions accepted/0000-static-safety-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
- Start Date: 2018-08-15
- RFC PR: (leave this empty)
- Yarn Issue: (leave this empty)

# Summary

Static safety-check for packages

# Motivation

The recent eslint-scope security incident has came as a surprise for many, if not all developers, as very few of us would expect a widely-used package on npm would be `hacked` in such a way.

Although measures must and have be taken in the publishing process both for eslint and npm team, human error and negligence are in its nature unavoidable; Thereof, for the developers to trust and embrace open source packages, actions must also be taken from the package manager's side, which developer interact everyday to download their packages, to provide an extra safety-net from malicious attackers.

The most concerning malware are the ones that steals data, thereof, we need to let developers know if the package they install have the ability to access the internet or not, and it should be up to the developers to decide if it is desired.

# Detailed design

The design is pretty straightforward:

Any network access would end up in requiring certain node modules or using certain browser api, therefore, it should not be difficult to do static analyses of those packages and provide basic safety check as for whether those packages requires those modules and/or calls those globals.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is in fact impossible to statically determine anything like that with certainty in JavaScript or in node.

Copy link
Author

@viztor viztor Aug 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. The purpose here isn't to build an anti-virus software, but rather to check what we can detect. The reason I recommend static check at this stage is because the implementation cost would be relatively low, consider other measures taken by Chrome or macOS - Sandbox and such.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure the implementation cost would be that low. Also consider that such a measure would be pointless unless followed by the ecosystem. And the ecosystem would only follow it if it actually worked. So imo, any solution will have to be designed to be the silver bullet and solve all the use cases from the very beginning.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, any "check" against malice is either 100% effective, or it's useless.


For packages that requires other packages, if any of their required package has `network capacity`, it should be marked as to have `network capacity`.

User need to manually confirm when they are adding a package with network capacity.

If the yarn.lock file indicates the package has no `network capacity` and yarn found that the currently pulled one does, it should throw an error.

In an CI/CD environment, the confirmation can be skipped if a lockfile is provided.

The result of such analysis can and should be cached on server. // it can and probably should be signed by the server of the package hash, analysis result and version.

When a publisher tries to publish a package with `network capacity` which previously doesn't, they will be asked to re-authenticate with their credentials, or provide a 2FA code. // not sure if yarn can do this. maybe only npm?
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would have to be on the registry side, and would be impossible to detect or prevent without runtime cooperation from both node and the OS.


# How We Teach This

As this RFC improves existing API and extra warnings, the change should mostly be unobstrusive.

# Drawbacks

It may over-complicate the yarn package.

Those packages that uses network interface but only provides wrapper function are still considered package that has `network-capacity` and user would still be warned.

# Alternatives

The other designs might be to develop or use a separate package to perform static safety check and use that package within yarn to provide such functionality.

Or such measure could be and/or taken from the registry's side by providing an extra field in its response and relevant hash and signature from trusted server.

# Unresolved questions

Should we add extra endpoint to allow manual checking of existing access?

Perhaps we should let the user decide if they want this feature through config?