-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Isolate (sandbox) language servers #12358
Comments
Is there any prior art in this space? |
The extreme solution would basically be containers (docker, lxc, etc). I don't know how they work on other platforms but on Linux it AFAIK boils down to some cgroup2 trickery and chroot'ing into a virtual directory hierarchy set up with bind-mounts that looks like a linux system with only the specified things present. |
I was thinking more of other editors that have sandboxed execution of language servers. |
Implementing good sandboxes from scratch can be surprisingly tricky. For example if you forget to create a new network namespace (for example because the language server needs internet access to download dependencies from the internet and you don't want to use a bridge between the host and container network namespace like netavark), you just got yourself a way to escape the sandbox by sending arbitrary keypresses to the X server over a unix socket in the "abstract namespace" which is attached to the network namespace rather than the mount namespace like regular unix sockets. Also the exact extent to which you can sandbox a language server depends on the language server in question. For example rust-analyzer needs to be able to run builds with cargo, which in turn requires read-only access to the system toolchain and all source code as well as full write access to the target dir, Cargo.lock and the caches in |
I'd argue that that isn't a feature that should be built directly into a language server, and instead handled by the whatever is (planned for) managing the extension.
some language servers also check I checked the docs for extensions but didn't see anything relevant. If extension sandboxing is something zed is interest in supporting, one way to support this would be to have some kind of allowlist in the extension toml:
|
Regarding linux:
From my POV, Landlock and Bubblewrap seems to be the best available options at sandboxing launched extensions processes |
Landlock currently doesn't support restricting unix sockets in the abstract namespace, so without separately creating a new network namespace using eg Bubblewrap or restricting all network access sycalls using seccomp, it isn't really useful for sandboxing on desktop systems. |
Looks like there's a "rust-native" implementation based on seccomp+namespaces+cgroup resource limits with a fairly nice API: https://crates.io/crates/hakoniwa Bubblewrap would be the more "mainstream" option. AFAIK it is also used by steam, among others, but I did not find existing rust bindings for it with a quick search. |
As long as the final solution do not make Zed rely on user namespaces, it's fine. |
is this what you mean by user namespaces? Looks like they are optional. |
Without user namespaces or a setuid helper it is not possible to create new mount or network namespaces without root permission. This means that the only sandboxing you can do is with seccomp. This is rather fragile for two reasons however: You can't selectively allow specific paths to be read or written. Only allow or deny all filesystem access. This is fine when the program to be sandboxed applies the seccomp rules after dynamic linking and other kinds of initialization that need filesystem access and the program genuinely doesn't need any filesystem access at runtime or all files it needs access to are opened by the program before applying the seccomp rules. This doesn't work when you want to sandbox an untrusted application rather than a trusted program that may happen to have a bug reachable only after the initialization phase. The second reason is that seccomp works at the level of individual syscalls. You either have to allow all syscalls except for specific ones (which will allow a sandbox escape when updating the kernel) or deny all syscalls except for specific ones. When denying all except specified ones any change to the to be sandboxed program may cause the sandboxed process to crash. Any libc update can start using new syscalls and the same applies for other libraries and the program itself. |
Since VSCode is based on electron it can take advantage of the SUID helper |
Relying on bwrap would still be an option. It can take advantage of user namespaces, but if those are not available it can run as SUID helper. A lot of people have it installed already as it is required by flatpak. And for the remaining users you can ship a copy and try to use user namespaces. For the small subset of users who have neither bwrap not user namespaces opting for no sandbox would be better than opting for no sandbox for anyone. |
Check for existing issues
Describe the feature
Related to #12354. Language servers downloaded by Zed have full access to everything on the machine. This is problematic from a security perspective (e.g. as seen recently the github release could be tampered with, causing people to download and execute malicious code). To mitigate the possible impact from this, it would be useful to sandbox language servers somehow, e.g. restrict them to only access files that belong to the work tree (and the language's respective system/standard library modules).
If applicable, add mockups / screenshots to help present your vision of the feature
No response
The text was updated successfully, but these errors were encountered: