Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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 python.analysis.nodeExecutable in .vscode/settings.json and with ${workspaceFolder} templating #5761

Closed
Tom-Newton opened this issue Apr 14, 2024 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@Tom-Newton
Copy link

python.analysis.nodeExecutable was recently added and it seems to be a big help. Thank you for adding it. I would like to be able to configure it in our repo so that all my colleagues get the benefit without each of them needing to set it up individually.

To do this it needs to be configurable in the .vscode/settings.json of the project. Currently it seems python.analysis.nodeExecutable only works in the user's settings.json. Additionally it would be useful to support https://code.visualstudio.com/docs/editor/variables-reference. Personally I'm specifically interested in ${workspaceFolder}, which appears to be supported in other pylance configs such as python.analysis.extraPaths.

@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label Apr 14, 2024
@debonte
Copy link
Contributor

debonte commented Apr 14, 2024

I believe that this is intentionally not supported for security reasons. We want users to opt into this feature explicitly to ensure that a malicious node.exe isn't used without their knowledge.

Reassigning to @rchiodo.

Maybe we could somehow require the user to approve the setting if it's in the workspace settings.json, and not use that setting value until that happens? Something along the lines of "do you trust the owners of this repo?" Which I think may be same thinking/language behind trusted workspaces?

@debonte debonte assigned rchiodo and unassigned bschnurr Apr 14, 2024
@debonte debonte added enhancement New feature or request and removed needs repro Issue has not been reproduced yet labels Apr 14, 2024
@Tom-Newton
Copy link
Author

Interesting. I hadn't thought of the security concern, but I am wondering if this presents a greater security concern than configuring python.defaultInterpreterPath on a project level (which is already supported by pylance).

@rchiodo
Copy link
Contributor

rchiodo commented Apr 16, 2024

Yeah this was intentional. The node executable (if allowed in a workspace settings.json) could theoretically run a workspace relative node that could compromise a machine. The 'trusted' workspace setting is supposed to handle this situation, but we didn't want to add another location (like defaultInterpreterPath) that users would have to double check in a folder before opening it.

Using a custom node is really supposed to be a workaround for memory issues though. It might be better if we just figured out what is causing memory problems for you @Tom-Newton.

@Tom-Newton
Copy link
Author

Tom-Newton commented Apr 18, 2024

Thanks for explaining. Personally I would find this useful but if you don't think its a good idea I can understand.

It might be better if we just figured out what is causing memory problems

That would be great but I think it would be a bit of a rabbit hole and I can't share the repo in question. Probably its caused by a large number of pip dependencies. We also use bazel so our python.autoComplete.extraPaths and python.analysis.extraPaths contain about 1000 entries each of our pip dependencies (#5364 would help here).

@debonte
Copy link
Contributor

debonte commented Apr 18, 2024

python.autoComplete.extraPaths

I hadn't heard of this setting before. FYI, it appears to only be honored when using Jedi instead of Pylance. https://github.com/microsoft/vscode-python/blob/01e5cbdbe836e51447a887f5afb350293a5ec11e/src/client/activation/jedi/analysisOptions.ts#L44

@microsoft microsoft locked and limited conversation to collaborators Oct 4, 2024
@rchiodo rchiodo converted this issue into discussion #6527 Oct 4, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants