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 another settings scope for safe per-project configuration #103100

Closed
hyangah opened this issue Jul 22, 2020 · 8 comments
Closed

Support another settings scope for safe per-project configuration #103100

hyangah opened this issue Jul 22, 2020 · 8 comments
Assignees
Labels
under-discussion Issue is under discussion for relevance, priority, approach

Comments

@hyangah
Copy link

hyangah commented Jul 22, 2020

As discussed briefly in the "Settings and security" document, some settings are dangerous
to be set through the workspace settings - i.e. an attacker can influence an extension to run
arbitrary binaries by storing a workspace settings file formulated in a specific way and
convincing users to browse the repo with vscode.

Security concerns around the workspace settings were raised for various extensions
multiple times (e.g. vscode-python, etc).

One option to reduce such risks is to disallow workspace settings for those critical ones.
E.g. using the machine scope, or implementing a custom way to read only global settings.
However, this completely prevents from project-specific settings, which is not desirable.
(e.g. I want to use a different version of a tool for a different project)

Another option is to use VSCode storage (e.g. move of python's pythonPath).
However, this comes with its own cost - each extension has to come up with its own
UI/UX solution so that users can easily browse and set those settings.
This is not ideal for consistent user experience across extensions and VS code itself.

Neither option is satisfactory. Can you consider another mode of settings that allows
per-project settings, but does not store the state as a file in the repo? The settings should
be still configurable through VS Code's awesome, user-friendly settings UI.

@sandy081
Copy link
Member

/duplicate

This looks similar to local workspace settings - #40233

@karrtikr
Copy link
Contributor

Local workspace settings are still inside the workspace which is a problem security wise, as user can clone a malicious repo containing that setting.

I think the ask is to provide a place outside of the workspace.

@sandy081
Copy link
Member

Local workspace settings are still inside the workspace

It was just one of the proposals. There are also other proposals to have local workspace settings outside workspace.

@hyangah
Copy link
Author

hyangah commented Jul 23, 2020

The thread got so long that it's hard to locate what's the current status of the bug. I supposed this is the one currently under discussion & implementation.

#40233 (comment)

Glad to see the proposal look quite similar to one of the ideas brought up in our team discussion, and see the configuration get stored outside workspace. Maybe worth mention the security consideration explicitly in the proposal?

A couple of details I want the proposal to add:

Either

  1. restrict some settings to be "only" outside workspace, or
  2. offer an easy read API to exclude the settings stored inside workspace

Re 1) Something similar to the machine scope. But some users will dislike this - Many users involved in the discussion of #40233 seem to be in favor of sharing various settings with project collaborators or checking them in in the repo.

Re 2) The current get (or getConfiguration?) will do all the complex merging automatically. If there are workspace*Valuebut users didn't set up resource*Value, the workspace*Value loaded from the cloned repository will be returned. I assume most users will start without the resource*Value settings.

Extension authors may call inspect and implement their own way of merging the different levels of configuration. That's what I was going to do if this feature request wouldn't be accepted. But that's tedious and probably prone to errors.
I wish get could accept a set of ConfigurationTarget to consider so I could exclude the values from the repository by default (unless users explicitly opt-in for the unsafe mode.

@hyangah
Copy link
Author

hyangah commented Jul 23, 2020

Thinking further - I wonder if vscode can prevent loading settings from the workspace in the default mode, and ask users to review the settings from the workspace and explicitly opt in to apply them.

Sharing the settings through the files checked in a remote repo comes with many goodies but also opens many doors for attackers.

@sandy081
Copy link
Member

CCing @dbaeumer @kieferrm as they are discussing about the same.

Reopening for discussion

@sandy081 sandy081 reopened this Jul 23, 2020
@sandy081 sandy081 assigned kieferrm and unassigned roblourens and sandy081 Jul 23, 2020
@sandy081 sandy081 added under-discussion Issue is under discussion for relevance, priority, approach and removed *duplicate Issue identified as a duplicate of another issue(s) labels Jul 23, 2020
@hyangah
Copy link
Author

hyangah commented Mar 6, 2021

I recently learned about #106488
The direction the issue 106488 is taking is slightly different from what i hoped -
I wanted to continue enabling some features using global configurations when operating on
untrusted workspaces/folders. But, still trusted workspaces concept will address the main
issue (arbitrary code execution due to configuration from untrusted workspace), I am happy
to close this issue in favor of it.

@hyangah hyangah closed this as completed Mar 6, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Apr 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

7 participants
@roblourens @chrisdias @kieferrm @hyangah @sandy081 @karrtikr and others