Tweak settings to prevent accidental execution of code from within a workspace

[brettcannon]

We want to prevent the chance of someone opening a fresh workspace on a repository to explore the code and then accidentally triggering the execution of code from within that repository. This is possible if you specify certain settings in a certain way, e.g.:

• python.pythonPath
• python.linting.pylintPath
• python.formatting.blackPath
• python.testing.pytestPath

The expected solution to all of this is:

• Store a workspace's Python path internally and require the user to select a path/virtual environment manually if it exists within the workspace (Select a Python Interpreter without modifying the workspace's settings.json #2125)
• Make tool paths be user-only, so that installation into an environment still works, but can be overridden only system-wide

By requiring a user action to set the Python path to e.g. an virtual environment within the workspace it forces the user to choose to trust that interpreter. And by only running paths to tools as specified at the user level then it doesn't allow a repository to override that location.

[brettcannon]

One thing to figure out is how to handle python -m unittest and if someone specifies a unittest.py in the workspace (this also includes having to worry about .pth). We might need to get sys.path from the selected Python interpreter, do a search for the relevant tools that would get picked up by -m, and not use it if it lands in the workspace or at least warn about it before using it.

[obambrough]

As a team we use a workspace relative path in python.pythonPath so that we can open any of our workspaces and run with the correct Python interpreter for that project, without any further configuration by any team member. That's one of the primary benefits of VS Code over Intellij, how simple it has been to share our configuration. It seems like you'll be taking away this very convenient feature.

[brettcannon]

@obambrough we expect to continue to automatically detect virtual environments. We would also store what environment you choose in VS Code's internal storage so it's a one-time setting. Also realize some people hate that we always set "python.pythonPath" as it isn't OS-agnostic. So this is a can't-make-everyone-happy-all-the-time situations.

[DonJayamanne]

@brettcannon I thought the proposed change of storing settings in internal storage was an optional feature (either opt in or opt out), giving users the ability to continue using python.pythonPath. I.e. @obambrough would be able to continue with python.pythonPath.

[brettcannon]

@DonJayamanne it's not fully spec'ed yet so I would prefer to not get into a discussion right now about this until that has occurred.

[kynan]

FWIW, not setting python.pythonPath in .vscode/settings.json breaks other extensions that rely on that setting, like lextudio.restructuredtext.

[luabud]

@kynan we already have an API that allows for extensions to retrieve the value of the selected environment, but we're making some final updates on it: #12596

[williamcol3]

I mentioned this in #12313, but I figured I would also bring it up here for discussion. Regardless of the possible merits or demerits of the proposed changes, there is a real, irrefutable cost to enacting the proposed changes.

With regard to this specific issue, I believe getting user certification of to-be-used paths the first time they are encountered by the extension (per workspace) would be easier to implement and lower cost to both the project and users. Is there any reason such a solution would not be sufficient in addressing this issue?

[luabud]

With regard to this specific issue, I believe getting user certification of to-be-used paths the first time they are encountered by the extension (per workspace) would be easier to implement and lower cost to both the project and users. Is there any reason such a solution would not be sufficient in addressing this issue?

The problem is that the current behaviour of the extension is to store the path of the selected interpreter for a workspace in the settings.json file, so asking for permission to use that setting is weird when the default experience is to add/modify that setting on interpreter change. One key point is the extension doesn't know if, for example, the setting was changed by the person by manually typing the path or if it was changed due to a git pull operation.

However, as mentioned in #12313 (comment), we may have a solution to keep the python.pythonPath setting in an opt-in way while addressing security concerns.

[zackees]

I'm hitting this issue. I have a project I use on MacOS / Ubuntu and trying to get windows to work with it leads to an automatic change in settings.json file to use the windows path.

Here's my diff:

This is the setting:
"python.pythonPath": "${workspaceFolder}/prod_env/bin/python",

Which get's changed to
"python.pythonPath": "prod_env\\bin\\python.exe",

..when I select the python interpreter to enable debugging.

I want to propose a fix that should be simple and avoid the complicated architecture change.

Proposal 1: Still have the path change as it's currently doing, but add a check which only changes the path if and only if the absolute windows path is not equivalent to the absolute windows interpretation of the path which is currently set.

In python this would look like:

if (os.path.abspath(toWindowsPath(curr_path)) != os.path.abspath(toWindowsPath(new_path)):
  # set path.

Proposal 2: Add an optional windows specific python path. For example, python.pythonPath.win, which if present, would override the python.pythonPath variable. This pattern has been done before, see terminal.integrated.shell.windows

In either of these cases the bug would effectively go away for me.

[brettcannon]

@zackees that's actually a different issue as that's a settings flip more than accidental code execution. See #12313 for our long term plans to stop changing the python.pythonPath setting on your behalf.

[butzek]

like @obambrough said, the Python interpreter is required to be workspace dependent - it does not make any sense to change the interpreter to be user dependent with this experiment. A developer has multiple Python interpreters installed and the workspace defines which one is taken.

[aytekinar]

See #12313 for our long term plans to stop changing the python.pythonPath setting on your behalf.

@brettcannon, in my humble opinion, the python.pythonPath setting comes very handy, especially when using the Remote - Containers extension within a team.

In our team, for instance, we rely on "Remote - Containers" to obtain a fully-reproducible development environment among the members, and this setting comes handy (i.e., because the path is always the same according to the .devcontainer/devcontainer.json settings, pipenv hashes and other venv-related paths with respect to the root of the development container are also fully-reproducible and/or static). The problem with your A/B testing is as follows:

When some of our team members have python.pythonPath feature deprecated (even though it is not even documented, nor does your documentation have any clue about this A/B testing experiment), we get confused. We assume that we have fully-reproducible environments simply because we are using the same IDE version, same Docker version, and same host machines, as well as remote containers with fully-reproducible Docker image tags and Pipfile.lock files. Yet, we observe different behavior in our IDEs. Moreover (and, even worse), IDEs of those members taking part in the A/B testing experiment simply remove the setting from .vscode/settings.json, whereas those of the members not taking part in the experiment add this setting. As a result, we need to be very careful when git commit'ing the changes not to break the behavior of the IDEs for some of the team members.

Is there any place where I can share this comment as a feedback so that this is also taken into account when deciding on the long-term plans? Perhaps one of #2125 or #12313?

Cc: @baboune

[brettcannon]

@aytekinar we have a solution that covers your issue. We are hoping to release it in 1 to 2 releases from now.

[Kurt-von-Laven]

@brettcannon, my comment may be moot, so if there is some place I can read about what you already have planned, I would love to.

I like the way VSCode handles the same issue for typescript, and I wish there were an equivalent of "typescript.enablePromptUseWorkspaceTsdk" for python. We use pipenv together with a .venv file containing the path to an untracked directory in our repository to ensure consistency across development environments. pipenv reads the .venv, and creates its virtual environment at the specified path when pipenv install is run. Thus, all developers are guaranteed a consistent virtual environment at a consistent path. Naturally, we specify "python.pythonPath" as a workspace setting to ensure that all VSCode users use the correct version of python. I propose continuing to allow workspaces to specify a python path, but introducing a "python.enablePromptUseWorkspacePython" setting, and ignoring the workspace python path unless the user accepts it. This simplifies the per-user workload from selecting the correct python environment from a potentially lengthy list to a binary decision of whether to trust that the workspace is configured correctly and non-maliciously.

[brettcannon]

We are starting to roll out the experiment and we plan to write a better explanation in another issue, but what we are doing is:
2. you can set python.defaultInterpreterPath and we will read it, but not write it
3. We will be no longer using python.pythonPath (I can't remember if we are updating it to python.defaultInterpreterPath or still reading it during the transition)

So people who don't want their git repos accidentally ending up with a machine-specific path can avoid it while people who generate their settings will be able to still do that.

[brettcannon]

We have rolled out the changes (see the announcement). Workspace trust also gives proper protections from not accidentally running code in an untrusted code base.

[aytekinar]

Thank you, @brettcannon, and the team for handling this in a nice and clean way. I have already been using the new python.defaultInterpreterPath setting, and I am very happy about it. Cheers!