-
Notifications
You must be signed in to change notification settings - Fork 190
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
Opt in #46
Comments
While it's true that The action being executed is always abundantly clear, because it matches what the user ran (ie it will always run a install with Yarn on "yarn install"), and the code can be audited ahead-of-time since the version is pinned within the package.json, and releases come from the official repositories for the supported package manager (and if we implement #10, it would even be enforced via signatures). If anything, the behavior should be more predictable than it currently is, not less.
That's the case in any JS project, and is the entire point of packages: you store the behaviour of your application within an isolated and deterministic environment, shielded as much as possible from your global binaries. Interacting with these projects (whether it's by installing them, linting them, building them, or running them) then becomes a tacit aknowledgement of trust (which perhaps is a problem in itself, but one that is more related to the Node permission model than Corepack itself). Also note that in a world where Corepack would be the default, you would still have the option of running
I'm not sure I understand which network vulnerabilities you have in mind (unless you mean we should enforce https?) or what curl has to do with it, but domains can already be hijacked. In fact, Corepack actually makes this safer by giving us the option to implement release signing via multiple parties: if Node provided the public keys and each individual project the releases (#10), any attack would require to compromise the systems for both Node and the relevant package manager systems. |
I should have been more clear: the action I was referencing wasn't about the package manager's behavior, but executing the package manager itself, or whatever proxy script Corepack uses to bootstrap the package manager. If this isn't opt-in, and it's built into Node, people running It's likely low risk, but...
That's based on the assumption that the user installing/running dependencies knows that they are, allowing them to audit and shield how they see fit. Automatically pulling in another dependency, silently, which then installs those other dependencies, breaks that explicit trust both for the package manager itself and for all the packages it installs. It also prevents control over how that installation occurs. Which is not something users normally worry about with
Of course, but I would have to know to even do that. Which I would, if I try to run
Network vulnerabilities (non-exhaustive):
I mentioned curl because that's the default PNPM installation instruction. If you're not using that, probably great? And it seems obvious in hindsight you probably are not. But that raises the question to me: Both
Saved this for last, because I wanted to take the opportunity to make clarifications. But I do think #10 would go a long way towards easing my concerns about automatic usage in Node. But I'd still like to be given the choice, and I don't think there's a good justification not to. Again with |
I totally agree with @eyelidlessness and it really annoys me that you pushed corepack even into previous LTS version! I maintain the nodejs package in Alpine Linux and right now I’m wondering what to do about it, whether to remove |
Is there another place where the binaries would be available in the users' PATH (at least the current one) without requiring bashrc schenanigans? |
@jirutka, I suspect you are already aware of this, but for the sake of others who happen upon this thread, even Node 18.0.0 ships with Corepack disabled by default. |
Hi 👋 I happened on this just meandering around the Node GH. I think this would be an excellent thing to provide with Node, but I’m concerned about some of what’s proposed.
Background
Based on my reading of README/issues clarifying progress, the current behavior is:
$PATH
My understanding is that this will be be bundled with Node, eliminating step 1. While convenient, that makes the following steps troubling.
Risks
Users are executing code they didn’t take any affirmative steps to install. This is a clear security issue, but it’s (subtly) more too.
In terms of security risk, it’s similar to
npx
prior to NPM 7:curl … | …
The subtly more bit: false negatives. Users or orgs might have a policy of aliasing these package manager commands to harden or restrict them for their own usage. If an environment isn’t properly provisioned, this would undo those restrictions rather than failing as expected.
Solutions/prior art
NPM 7, thankfully, introduces a prompt when using
npx
to ask if the user wants to download and execute commands that aren’t already available on$PATH
. This isn’t a panacea, but it’s a meaningful improvement over silently downloading and executing third party code.The same technique could be used here. The downside is it’s a minor inconvenience, but the risks it would help to alleviate are huge.
Considerations
The text was updated successfully, but these errors were encountered: