Security prompt annoyance

[rjra100]

Afraid some of our users are annoyed by the security prompt added in version 0.17.3 ("The CMake Test Explorer extension detected the following CTest path: "<blah>". Do you allow execution of this file?"). It's probably fine in a stable environment, but we do all our development in Docker containers in fairly transient workspaces, and every time we build a new one, we get another prompt to allow ctest to run. Our users are also reporting that the prompt doesn't actually show up until after the window is reloaded; prior to this, the test explorer just doesn't work.

I'm not entirely clear what the vulnerability was (guess a repo could somehow insert its own ctest executable into the path and have it run using the extension host's permissions?). But would a viable alternative be to provide a configuration setting "cmakeExplorer.ctestPath", allowing the user (or remote) settings to specify the path? That way we can set up our standard container environment to specify where we expect ctest to be. If that's left unset (default), the current logic, including the prompt, could continue to apply.

[fredericbonnet]

Hi! Sorry for the inconvenience, this is an unfortunate side effect of the security fix. I didn't want to disclose it before the new version was widely available, but you have the right intuition about the vulnerability: it allows an attacker to distribute a repository that will execute an arbitrary command at workspace load time by providing its own ctest path.

The solution you propose wouldn't solve the security issue, as it would still allow the attacker to provide a settings file with a malicious path.

This sounds like an unsolvable issue, but fortunately VS Code provides a Workspace Trust framework that disables extensions by default on untrusted workspaces. I'll take a look and push a fix soon.

In the meantime can you tell me a bit more about your setup: how do you solve the Workspace Trust issue with your transient workspaces? Can you distribute global settings?

[rjra100]

To be honest, we basically don't worry about it - all our workspaces are for developing in company-internal repos, and we trust our own devs not to do anything nefarious. So I think we just told everyone to disable security.workspace.trust.enabled (and I imagine half our devs ignored that and just click "yes, trust this workspace" every time they open a new one!).

Most of our development is done with Docker and the Dev Containers extension: we have a setup script which creates our dev docker images; you run that on a workspace; it looks at the repo(s) in it and determines what packages it references and builds a Docker image containing the necessary stuff. It also adds .devcontainer config files telling the Dev Containers extension what image to use, extensions to install in the container, and so on. It does put a remote settings file into the Docker image ($HOME/.vscode-server/Machine/settings.json), so we have a location to put standard settings for the containers, though of course this only works for settings that can be applied locally to a container. We don't have a particularly convenient way to distribute global settings, though. Global settings would be the wrong place to specify where to find ctest anyway, given that it could vary depending on what container or machine you're connected to: for example, in a Docker container we'd expect to use something like /usr/bin/ctest, while we might also use Remote-SSH to connect to a remote server, in which case our dev dependencies would usually be installed in a refroot (<some directory>/usr/bin/ctest).

If it's unsolvable, we can live with it, but it'd be nice if there was a better way. It's definitely possible to have settings that can only be set at global/user level and can't be overridden at all; not sure whether it's possible to have settings which can be set at machine/remote level but not overridden by a workspace-level .vscode/settings.json.

[rjra100]

To follow on a bit, there seem to be two problems; one an annoyance we can live with if necessary, while the other seems more like a bug:

• The popup "allow ctest to run" window - sounds like this is intended, albeit a mild annoyance.
• When a container is created or rebuilt, no CMake configure has yet been carried out and there is no CMakeCache.txt file in the output directory. This frequently (but somehow not always?) seems to break the extension completely: the CMake Explorer Log output pane says "Error: CMake cache file <build dir>/CMakeCache.txt does not exist"; we usually get a popup notification saying the same thing as well. This is fair enough, but it often persists even after a configure & build has been carried out - after clicking the refresh test lists button, it still claims CMakeCache is missing. We have to do "Developer: Reload Window" or equivalent (close & reopen the window) before it'll wake up and attempt to refresh the tests list (at which point it then asks permission to run ctest, if it's a new container).

Possibly that should be a separate issue?

[fredericbonnet]

@rjra100 indeed they are two different issues. I've pushed a new version that reverts the change and explicitly rely on the standard workspace trust instead.

The second point is the same as #74 I think. BTW the only reason I can see for this error to persist after a build is if the cmakeExplorer.buildDir does not point to the right location, as the extension checks for the file each time the test list is refreshed.