[Security] workspace trust: language servers & tree sitter

[DuckDuckWhale]

As a question

Is it safe to automatically run languages servers and tree sitter on arbitrary files? Sometimes I just want to view files without executing code in any way, especially when viewing other people's code.

As a feature request

Can something like VS Code's workspace trust be implemented?

As a security vulnerability bug report

If a language server is allowed to run automatically upon the opening of a file and is allowed to execute arbitrary code (e.g. to run build scripts for Rust) when doing its job, users thinking that this is just a text editor and view arbitrary code or users who inadvertently view untrusted code by mistake have the risk of having their machines compromised.

This was CVE-2021-34529 for VS Code. The relevant release notes can be found here. I couldn't find any vulnerability reporting guidelines for this project, so here it is.

[willparsons]

I think for the most part helix is not affected by this since 'Tasks', 'Debugging' and 'Extensions' tools don't exist (currently).

As for the LSP it could be as simple as checking if the user wants to spawn an LSP then saving that setting for the workspace. The only issue is that there may be things down the line that require the 'workspace trust'.

There could be a wrapper that lets you call actions and it would determine if they were available based on the current trust setting for the workspace, this avoids the need to wrap everything in if trusted....
E.g.

action!(Action::LSP::Spawn, ...)

I haven't looked into the helix codebase that much so this is just a idea, I'm not sure how effectively it would work. What do you think?

[sgued]

As a general idea I think it could be good to not have the language server always running. An option to have off by default and a command to start it when needed would be welcome.

[crabdancing]

Any progress on this?

[the-dipsy]

As described in the issue that just mentioned this one, as of this PR the user doesn't even need to have any language servers installed to be vulnerable to the execution of arbitrary code. This has apparently already been noticed here and here. Vim has this disabled and warns about it explicitly. @the-mikedavis kindly pointed me towards setting editor.lsp.enable = false for now.

[the-dipsy]

@lazytanuki remarked that setting editor.lsp.enable = false doesn't fix this, because a project-local .helix/config.toml can override this. I was not aware that local config.tomls work too. I would therefor still urge for making project-local configs optional and disabled by default as soon as possible instead of waiting for a workspace trust solution.

[crabdancing]

This seems like a pretty simple feature to implement. Have a state directory (~/.local/share/helix) (de|)serialize with serde, and have trust remembered on a per-directory basis.

[charles-dyfis-net]

This seems like a pretty simple feature to implement. Have a state directory (~/.local/share/helix) (de|)serialize with serde, and have trust remembered on a per-directory basis.

When working in particularly sensitive contexts, I might want to review each config file state -- that is, to approve file X with hash Y in directory Z, but then be asked to re-review / reapprove should that hash change, or should an additional config file be added.

That said, even without that level of paranoia, this would be an immense improvement over where we are now.

[stevenengler]

FWIW this issue is the reason I personally don't use Helix and the reason I give to others when I recommend not to use it. The fact that you can git clone a project, open a file in Helix, and then Helix immediately runs arbitrary commands from that repository makes Helix a non-starter for me and other people who are security-conscious. Other tools like vim, VS code, git, etc have made conscious decisions not to do things like this. I do consider this a security vulnerability in Helix. For example someone could easily change the language server command to steal your ssh keys with curl (or worse).

An argument could be made not to open files in untrusted directories, but that places a heavy burden on users who then need to recursively look for malicious config files in hidden directories in every project they ever open. Afaik they can't even use Helix to check the config files since the attacker may have set a malicious TOML language server command. It's also easy to forget and does not follow the principle of least astonishment (opening a plain text file leads to arbitrary command execution).

I agree with the above comment that reading the local configuration files should be disabled by default, or should at least be off by default for the specific configuration options that can be used for arbitrary command execution.

[archseer]

You should probably disable language servers altogether then, since many will evaluate code (or evaluate/expand) macros.

[stevenengler]

You should probably disable language servers altogether then, since many will evaluate code (or evaluate/expand) macros.

I don't use language servers. But as mentioned above, this doesn't matter:

@lazytanuki remarked that setting editor.lsp.enable = false doesn't fix this, because a project-local .helix/config.toml can override this

[charles-dyfis-net]

You should probably disable language servers altogether then, since many will evaluate code (or evaluate/expand) macros.

"Many" is not "all". One can (very!) rationally choose to leverage only language servers that don't support features that would require them to trust the code being operated on (or which enable those features only given explicit configuration).

Mind, getting configuration from the same source as the code being operated on makes that precaution meaningless -- but that's why there are feature requests here.

[MatrixManAtYrService]

I feel like running some arbitrary command under the auspices of it being a language server (because it showed up in .helix/languages.toml in the workspace dir) is no different from running arbitrary plugin code for the same reason.

That's a bit separate from the idea that a language server, configured normally (not based on the CWD), might then do something problematic when it encounters workspace contents.

The former is probably both more concerning and easier to fix. We could store a list of workspace .helix/ locations in $XDG_STATE_HOME and refuse to load ones that aren't in the list, while remembering the cases where the user has explicitly asked to load one. I think direnv allow uses a similar strategy.

Whether to use the same mechanism to prevent trusted language servers from operating on untrusted content... 🤷

[noor-tg]

So will opening helix inside docker container be more secure ?
So if any malicious executed code will run only inside the container ?

[stevenengler]

So will opening helix inside docker container be more secure ?
So if any malicious executed code will run only inside the container ?

Somewhat. Container escapes and Linux privilege escalation vulnerabilities are fairly common, and typically containers shouldn't be used as security boundaries. VMs do a much better job at isolating code. But containers do probably help protect against very simple attacks.

There are also other types of malicious code that could run inside the container and still be harmful without needing to escape from the container. For example cryptocurrency miners, ddos tools, etc.

[noor-tg]

@stevenengler is there any pull request to disable usage of project local config entirely ?

[noor-tg]

even if it is not merged to main

[stevenengler]

@stevenengler is there any pull request to disable usage of project local config entirely ?

I'm not aware of any, but I haven't looked at helix in almost half a year so I don't remember much.

From a quick look, maybe the PR at #9545 could be used. It sounds like it allows you to disable the local config.