[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.