search

clojure-lsp / clojure-lsp

Clojure & ClojureScript Language Server (LSP) implementation
https://clojure-lsp.io
MIT License
1.17k stars 153 forks source link

Add ability to safely analyze without executing project.clj #1747

Open technomancy opened 10 months ago

technomancy commented 10 months ago

Is your feature request related to a problem? Please describe. It should be possible to safely open an untrusted file without running code that's part of the project.

Imagine you are looking at a codebase, and you're not sure if you can trust the code. You open up one of its files in your editor to read it and see if it's safe or not. Since your editor has clojure-lsp configured, this starts up analysis. On the bright side, clojure-lsp does not execute any of the project code in the src/ directory. However, it does run lein classpath, which executes project.clj. If the project is in fact unsafe, your machine has now been compromised.

Describe the solution you'd like When clojure-lsp detects a Leiningen project, it should try running lein static-classpath first instead of lein classpath since the former is safe to run in any project. Since this task is relatively new, it should probably fall back to lein classpath if the former fails.

https://codeberg.org/leiningen/leiningen/commit/62175459ac3cde9faf0f812ec202ef25a8e9cb11

I'm open to suggestions for improvements to the implementation of static-classpath.

Additional context It should be possible to configure it so it never performs the unsafe fallback. It should probably also be possible to configure it so it always prefers lein classpath since there are projects that need plugins in order to calculate the classpath.

However, I don't have a clear idea of how such configuration should be accomplished. If the configuration is read from the project repo itself, it would defeat the purpose, since an attacker could just set it to unsafe mode as part of the untrusted repository.

ericdallo commented 9 months ago

Thanks for the report @technomancy, clojure-lsp has a general waterfall settings checking from project to home dir, but maybe we could check only the home one for this one somehow. Also we don't have ATM a way to fallback for classpath lookup commands, we would need to support that, unless we think in a way the fallback is not needed.

technomancy commented 9 months ago

Honestly I don't know that I have enough context to say what the right thing to do here is. Thinking about this more, there are legitimate and safe reasons to want to override the classpath command on a per-project basis; maybe clojure-lsp guesses wrong and you want the project to set an override to a different command, but the other command is still safe.

I've never used deps.edn or any of the other classpath calculation tools. Are they safe to run on untrusted projects? (Disabling *read-eval* etc?) I see that there is also a :java-command config setting which could trivially be overridden by an untrusted project to run arbitrary code.

It's clear to me that my original suggestion of "avoid lein classpath and use lein static-classpath instead" is insufficient for the purposes of achieving "opening an untrusted file shouldn't pwn your system"; it's a small part of the puzzle, but there's a lot more to it. From my perspective, I want to make sure that static-classpath is available for people who want to do the work of making the tooling safe, but I don't think I can be the one to do that work, and I don't know how much of a priority it is for the maintainers of clojure-lsp.