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

Support announcing to the server whether the user trusts the project #1915

Open
nthykier opened this issue Apr 6, 2024 · 1 comment
Open
Labels
feature-request Request for new features or functionality initialization
Milestone

Comments

@nthykier
Copy link

nthykier commented Apr 6, 2024

I am working on a language server, where part of the diagnostics would be delegated to separate tools. Some of these tools will execute code from the project as a part of doing their analysis of a parse-time Turing complete language. This is fine, you wrote the code yourself. It is less than ideal, if you downloaded it off the internet to review the content.

In my case, my consumers are often both reviewing some projects while being the maintainer of other projects. I want them to have the best experience on their own projects without providing a "backdoor" into their system when they are reviewing other people's work. A considerable portion of my language server does not require code execution and it would still be a valuable aid with reviews of potentially unsafe code provided the few unsafe features are disabled.

Related, at least IntelliJ/PyCharm (the JetBrains IDE) has a notion of "Trusted paths" for projects and will ask you whether you trust the project when you open it for the first time. This suggests to me that the concept of a trusted project is general problem beyond my own language server and also part of why I am bring it up to be included in the specification.

I could have the language track trusted projects itself, but that creates a UX problem: You would have announce the trust in the editor and then once for each language server used that might execute code. It is state/configuration maintenance I feel belong in the editor to ensure that there is one source of truth.

For me, a single boolean during the initialize method would be sufficient. Maybe with a setTrusted notification or request from the client in case the user changes trust level (usually from untrusted to trusted) after the server was started. When the project is not trusted, the language server should not run tools/analysis that might compromise the system if it came from an untrusted source. On the other hand, when the project is trusted, tools executing content from the project can be done safely.

Based on the initialize request supporting "progress", I suspect a client would not be able to rely on 3.17 servers to not execute unsafe code. Therefore, I am less convinced that it would make sense to have a boolean in the server capabilities. This also implies that clients will need a curated list of language servers that are safe to run on an untrusted project if they wish to support language servers on untrusted projects.

Another alternative to this proposal is an explicit mention of what editors/clients and language servers should expect from each other on this topic. I could see that as a subsection to the "Implementation Considerations".

@nthykier
Copy link
Author

I am mostly concerned by "Completion", "Hover docs", "Diagnostics" and similar features that seem like they would be safe when you are just reviewing the code. Completion being relevant for a reviewer if someone said "I have this diagnostic that I cannot fix" and sent it your way to aid them (which could happen in a PR from a first time contributor). I feel it is not reasonable for a developer to mentally go "oh wait, this might be untrusted. I should disable my language server while I review this PR".

On the other hand, I would not mind allowing servers to emit "run tests" code lenses for an untrusted project (assuming here that the server does not need to run code to have the code lens presented to the user). The code lens are manually triggered/activated and can have names that strongly implies "This will execute code" (like "Build project", "Run tests", "Start app"). Though, there is also an argument for "untrusted means untrusted. Do not let people shoot themselves in the foot", which is also an acceptable stance for me. This could come in multiple forms (editor not requesting code lenses on untrusted projects or warning the user when triggering a code lens might be unsafe when the project is untrusted)

Anyway, it was just to clarify the nuance of where my concerns with trusted/untrusted lies.

@dbaeumer dbaeumer added this to the Backlog milestone Aug 19, 2024
@dbaeumer dbaeumer added feature-request Request for new features or functionality initialization labels Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality initialization
Projects
None yet
Development

No branches or pull requests

2 participants