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

Zed downloads NodeJS binary and npm packages from Internet without user’s consent #12589

Open
1 task done
jirutka opened this issue Jun 2, 2024 · 55 comments
Open
1 task done
Labels
bug [core label] network Network connectivity issues, protocols and services support security & privacy Data privacy issue, security vulnerabilities, etc

Comments

@jirutka
Copy link

jirutka commented Jun 2, 2024

Check for existing issues

  • Completed

Describe the bug / provide steps to reproduce it

I noticed that Zed automatically downloads the NodeJS binary from https://nodejs.org without asking or even informing the user about it. Right after starting it and opening a file, without doing anything else. And there’s no option to disable it.

This is completely unacceptable!

Not just for security reasons but also from a usability point of view. I’m currently connected via metered LTE, and Zed has just eaten up 14 MiB of my plan. Moreover, I already have node installed and on PATH. Also, the downloaded binary is somehow corrupted and it wouldn’t work on my system anyway because it’s built against glibc (that’s how I noticed it in the first place).

And to make matters worse, if it did work, it would start installing arbitrary packages from npmjs.com via npm and running their scripts. This represents a huge attack vector.

This approach is completely unacceptable for anyone who’s concerned about cybersecurity and for virtually all companies, at least in the EU, because of cybersecurity laws, related certifications and audits.

EDIT: Now I found that it downloads (here) even some proprietary binary from https://supermaven.com, i.e. unaudited and unauditable code, without any verification (except TLS)! At least this is not downloaded by default… I hope…

EDIT2: Zed also automatically downloads and executes prebuilt language servers for C#, Clojure, Deno, Elixir, Gleam, GLSL, Lua, Terraform, Toml and Zig. It automatically resolves the latest version available on GitHub and downloads it, again, without any verification.

Environment

N/A

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

No response

Somehow related issues

@jirutka jirutka added admin read Pending admin review bug [core label] triage Maintainer needs to classify the issue labels Jun 2, 2024
@asesh
Copy link

asesh commented Jun 3, 2024

I agree with you, it can cause severe security issues. It will also download unsigned executable files and run them w/o any consent or permission. That's why I have language server disabled. You can disable this feature though: https://zed.dev/docs/configuring-zed

@vielmetti
Copy link

vielmetti commented Jun 3, 2024

Some of this is previously discussed in #7054 , specifically #7054 (comment) in which @SomeoneToIgnore writes

"We do not have plans to abandon this approach since there's so much code written to support various frontend tools already, that rewriting those in Rust will take an eternity, so not sure what is actionable here, hence closing."

@jirutka
Copy link
Author

jirutka commented Jun 3, 2024

That's why I have language server disabled. You can disable this feature though: https://zed.dev/docs/configuring-zed

Language servers are very useful. I don’t see an option to disable the automatic fetching of dependencies from the internet without disabling the language servers completely.

@jirutka
Copy link
Author

jirutka commented Jun 3, 2024

"We do not have plans to abandon this approach since there's so much code written to support various frontend tools already, that rewriting those in Rust will take an eternity, so not sure what is actionable here, hence closing."

This is a false dichotomy. Nobody is asking to rewrite everything in Rust and bundle it into the application (on the contrary, I’d ask the opposite). Node.js is a normal system dependency that should be installed by the system’s package manager, or manually (if there’s no package manager in place). And that’s what I did for the Alpine Linux package, but I had to patch the Zed sources to do it.

@someone13574
Copy link
Contributor

someone13574 commented Jun 4, 2024

Maybe a popup, which is enabled by default, could ask for permission to download some-binary from some-link? It wouldn't mitigate the fact that its downloading a binary, but it would at least give the user an option to see whats getting downloaded beforehand and see that its from somewhere official. Then in settings there could be three options for download_permission or something:

  • ask: The default setting. Asks the user for permission to download a file.
  • always: The current behavior. Just download without asking.
  • never: Never download any files. May break extensions and isn't recommended (but available for users who don't have a glibc system, really value security, or any other reason).

@JosephTLyons JosephTLyons added network Network connectivity issues, protocols and services support security & privacy Data privacy issue, security vulnerabilities, etc and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Jun 5, 2024
@asesh
Copy link

asesh commented Jun 5, 2024

That's why I have language server disabled. You can disable this feature though: https://zed.dev/docs/configuring-zed

Language servers are very useful. I don’t see an option to disable the automatic fetching of dependencies from the internet without disabling the language servers completely.

They are useful but I would rather keep my machine safe than ran arbitrary executables and scripts. That's why I have it disabled. This is a really weird and insecure feature of Zed!!

@someone13574
Copy link
Contributor

Ideally you would be able to turn off auto-download but still be able to use an lsp if it’s already on the users system.

@crabdancing
Copy link

crabdancing commented Jun 16, 2024

Yeah, this issue really turned me off using Zed until it's fixed. :/

It tried to run them and then hilariously couldn't because I'm on a NixOS system:

[2024-06-16T12:55:07-06:00 ERROR project] failed to start language server "json-language-server": failed to execute npm info subcommand:
stdout: ""
stderr: "Could not start dynamically linked executable: /home/ada/.local/share/zed/node/node-v18.15.0-linux-x64/bin/node\nNixOS cannot run dynamically linked executables intended for generic\nlinux environments out of the box. For more information, see:\nhttps://nix.dev/permalink/stub-ld\n"

Frankly, not having the ability to configure it to just use what's in my environment (which I have already audited), or specific paths that I give it, is a dealbreaker. I much prefer Helix's simpler approach to this of just telling you what it has access to and what it doesn't, and letting you override the things it's trying to use.

@vinicius507
Copy link

Yeah, this issue really turned me off using Zed until it's fixed. :/

It tried to run them and then hilariously couldn't because I'm on a NixOS system:

[2024-06-16T12:55:07-06:00 ERROR project] failed to start language server "json-language-server": failed to execute npm info subcommand:
stdout: ""
stderr: "Could not start dynamically linked executable: /home/ada/.local/share/zed/node/node-v18.15.0-linux-x64/bin/node\nNixOS cannot run dynamically linked executables intended for generic\nlinux environments out of the box. For more information, see:\nhttps://nix.dev/permalink/stub-ld\n"

Frankly, not having the ability to configure it to just use what's in my environment (which I have already audited), or specific paths that I give it, is a dealbreaker. I much prefer Helix's simpler approach to this of just telling you what it has access to and what it doesn't, and letting you override the things it's trying to use.

Yes, on NixOS and I had to wrap Zed in a FHS environment so I could use the LSPs. Not the best experience imo.

@imbev
Copy link

imbev commented Jul 7, 2024

I am also concerned about this. Either I disable LSP or I allow the automatic download of arbitrary binaries from GitHub. This is a dealbreaker, and prevents Zed's use in security conscious environments.

@sgriff96
Copy link

sgriff96 commented Jul 7, 2024

I don't think anyone is asking for the team to re-write every binary in Rust. We just want to be asked to install the language server or not. If you click no, then you don't install all the necessary things for that language.

@imbev
Copy link

imbev commented Jul 7, 2024

I don't think anyone is asking for the team to re-write every binary in Rust. We just want to be asked to install the language server or not. If you click no, then you don't install all the necessary things for that language.

The current behavior prompts on a per-language basis, but #12589 (comment) would be a significant improvement.

@someone13574
Copy link
Contributor

I don't think anyone is asking for the team to re-write every binary in Rust. We just want to be asked to install the language server or not. If you click no, then you don't install all the necessary things for that language.

This has been turned down already: #12703 (comment). As said in the comment it is likely that the extension API will be changed if we don't want to manually make sure every extension does the desired behavior (respect a "no downloads" setting, searching path, gracefully handling when downloads aren't allowed, etc.)

@shinebayar-g
Copy link

shinebayar-g commented Jul 8, 2024

@nukeop

This comment was marked as off-topic.

@xpe
Copy link

xpe commented Jul 8, 2024

Apologies for the pedantic note: a Language Server Protocol (LSP) isn't insecure, but a Language Server (LS) can be. Many of the comments above include "lsp" but they really mean "language server".

@xpe
Copy link

xpe commented Jul 8, 2024

Just to step back for a moment. Surely we can all recognize that the "opt-in" versus "opt-out" debate is filled with landmines? So let's keep this in mind. For someone to "breeze past" the long-line of wreckage associated with this debate and proclaim an easy answer is naive at best.

Ok, now that I've said that, let's try to frame the key questions carefully. Very often, solving a problem well demands this. If this was an in-person synchronous process, I would say this:

Let's not try to offer solutions (whether they "easy" or "obvious" or "work for 99% of users" or whatever) until the contours of the problem are well understood.

The above advice gets really hard in this context. Everybody has their own ideas. But if you are reading this, stop for a few minutes and try to write out the essence of the problem. Try to state the concerns of all parties even better than they have so far. Don't try to offer solutions just yet.

P.S. Anchoring on how VS Code does it should be considered abject failure of thinking and creativity. Here is a chance to do better. So let's lay out the full contours of the problem before jumping to some half-baked compromise that is suboptimal.

@Calandracas606
Copy link

I am going to put forward a hypothesis that this is a really really niche use case and 99.99% of users just want to open a JS/TS file and have it work. And the way OP phrased it ("it's COMPLETELY UNACCEPTABLE") is just nasty, entitled, and tries to make it much more of an issue than it has any right to be.

OP appears to be someone who is attempting to package Zed for Alpine.

From a package maintainers perspective, it is completely unacceptable, a total deal-breaker for this to be the default behaviour.

People are relying on maintainers having vetted the software in their repositories. Many users will only install software from their distribution's repositories, because they trust that the maintainers have done their due-diligence.

Ultimately, its the maintainers for distributions that need to be convinced in order for Zed to be adopted by linux users.

@Calandracas606
Copy link

As an additional point which should be brought up again, is that downloading binaries like this simply will not work on distributions using musl as the libc, and there should be an alternate method of providing dependencies for those users.

@peterlandry
Copy link

peterlandry commented Jul 8, 2024

@sambonbonne fair enough, reasonable point. I understand there's a lot of nuance to think about, but I don't see why it would stand in the way of a behavior where on the first need to grab something, zed asks, and records the answer. It's a good start while more in-depth solutions are discussed (rather than punting on the whole issue).

@alerque
Copy link

alerque commented Jul 8, 2024

There is quite a bit of missing the forest for the trees here, and some false dichotomies.

  • Nobody is freaking out about 14 MB. The issue is when and how anything at all is downloaded and the current lack of transparency and that sensitive action. This behind the scenes action makes the editor completely unusable on some platforms (e.g. Nix) and introduces anti-patterns and conflicts for others (many/most distro packaging). For others just workplace operational rules preclude the use of any tooling that acts like this.

  • There is not a binary choice between software being "friendly to end users" and "secure". You can have your cake and eat it too—it just takes intention and a little more developer focus. The main thing that I think should be added by developers is not a user facing popup or preference (where opt-in vs. opt-out is an issue, as is noise & friction), but compile-time options (most useful to packagers, but also usable by workplaces or end users). If Zed offered compile time configuration this could be fixed to everybody's satisfaction. Distros could ship a pre-configured package with all the dependencies already provided so that the user experience is just "install and run" (appeasing both the distros and their end users expectations) while also not having an app that downloads (or attempts to download) binaries behind the users back. End users with strong preferences could also compile it to suite their requirements. Meanwhile if Zed wants to ship a binary package themselves that defaults to silently downloading tooling at runtime they still can.

    The necessary compile time options would be a master "don't try downloading anything silently" option (that dovetails nicely with the existing "don't try to self-update" which distros already use via export ZED_UPDATE_EXPLANATION="...") and then specific options to set paths to existing tooling that could be used directly, e.g. USE_SYSTEM_NODEJS USE_SYSTEM_PRETTIER.

@michealp-coder
Copy link

michealp-coder commented Jul 8, 2024

@mrnugget

This isn't just a language server concern.

1-year ago, this security concern was also documented here.

Where the VSCode theme importer is silently attempting to install a Home Brew package and execute it.

(I hope people's voicing concern adds this to the priority pile. And also want to say thank you so much for creating Zed ... and acknowledge it's currently free, pre 1.0 and some users can have unrealistic demands on creators)

@mrnugget
Copy link
Member

mrnugget commented Jul 8, 2024

This isn't just a language server concern.

1-year ago, this security concern was also documented here.

Where the VSCode theme importer is silently attempting to install a Home Brew package and execute it.

I wasn't around a year ago, but as far as I can tell: that extension runs in VS Code and wasn't published nor written by the Zed team. The theme importer it uses under the hood was written by the Zed team and everybody can read through its source here: https://github.com/zed-industries/zed/tree/main/crates/theme_importer That theme_importer was not published to homebrew by Zed either. (Also, @michealp-coder, you were in that thread in which it was explained how that theme_importer was created: #7111 (comment))

Since Zed doesn't advertise nor officially recommend that VS Code extension, I don't think there's a lot we can do about it (should we actively advertise against it? not sure)

@jarcane

This comment was marked as off-topic.

@mrnugget

This comment was marked as off-topic.

@Sinofine

This comment was marked as off-topic.

@tv42
Copy link

tv42 commented Jul 8, 2024

For anyone else like me who wants to put in a technical stopgap kludge to prevent Zed from downloading things that pretty much aren't going to work on my NixOS anyway, putting this in your settings.json breaks at least some of that:

  "server_url": "https://disable-zed-downloads.invalid",

@revskill10
Copy link

Allow an environment variable like NODE_PATH should skip the unessessary Node download.

@kurucu

This comment was marked as off-topic.

@nukeop

This comment was marked as off-topic.

@SichangHe

This comment was marked as off-topic.

@xpe

This comment was marked as off-topic.

@GoldsteinE

This comment was marked as off-topic.

@xpe

This comment was marked as off-topic.

@BobobUnicorn

This comment was marked as off-topic.

@nukeop

This comment was marked as off-topic.

@BobobUnicorn

This comment was marked as off-topic.

@mikayla-maki
Copy link
Contributor

mikayla-maki commented Jul 9, 2024

Since 28 separate people get a ping on every message here, I'm going to lock this issue for now. We are currently working on this, follow along here: #14034

@zed-industries zed-industries locked as off-topic and limited conversation to collaborators Jul 9, 2024
@mikayla-maki
Copy link
Contributor

Status update: We are still working on this! The major blocker is that extensions have not been setup to interact with setting. However, we also need to change this API to support our upcoming remote development feature. So we're going to roll both of these breaking changes into a larger extension update, coming this November or December :)

@github-actions github-actions bot added admin read Pending admin review triage Maintainer needs to classify the issue labels Nov 5, 2024
@notpeter notpeter removed triage Maintainer needs to classify the issue admin read Pending admin review labels Nov 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug [core label] network Network connectivity issues, protocols and services support security & privacy Data privacy issue, security vulnerabilities, etc
Projects
None yet