-
Notifications
You must be signed in to change notification settings - Fork 446
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
Adjust the setting's scope to enhance security #1535
Adjust the setting's scope to enhance security #1535
Conversation
Signed-off-by: Jinbo Wang <[email protected]>
Below are the commonly used scope definition:
Sort them by granularity:
application VS machine: Regarding the gradle arguments, it could define script path, should be machine specific. So prefer to use |
machine VS machine-overridable: |
@@ -155,7 +155,7 @@ | |||
"type": "string", | |||
"default": null, | |||
"description": "Arguments to pass to Gradle.", | |||
"scope": "window" | |||
"scope": "machine" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that means all projects will share the same gradle arguments. Seems wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
YES. Regarding security, putting it at workspace scope is vulnerable.
And in Intellij IDEA, this parameter is even not exposed to user. Did you have some specific user scenarios that strongly require it overridable at workspace or project level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@snjeza ^^?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it is a missing feature in Intellij IDEA. Eclipse allows the property to be set at the project level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cons for each option are listed below in short, we can compare the serverity and tradeoff, and then determine which to use...
application
: blocks remote scenarios, as no way to configure in remote environments like WSL/docker container/etc.machine-overridable
/window
/resource
: security issue for anyone who simply clones and opens an opensource repo with malicious settings. (because it's stored in <project_root>/.vscode/settings.json)machine
: cannot be set per project, have to share one for all projects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually most of Gradle command line arguments can be configured by gradle.properties, that can be saved at the project level. This means the user still has alternative to customize the build environment at the project level. The main purpose here is to disable Gradle init script at project level because project scope settings can be saved in git and distribute to others. Both IJ and VS Code are auto import, that will auto load init scripts silently. That's probably the reason why IJ doesn't expose it for import/open.
Eclipse is not auto import, the user need click the wizard step by step to finish the import. It's the user's choice to execute the operation. That's the difference from the security perspective. Also, the import wizard of Eclipse turns off the project's level settings by default, you need manually turn it on.
merged as 4718da6 (also changes |
Signed-off-by: Jinbo Wang [email protected]
The settings that allows you to specify an executable to perform some operation on startup should only be defined in the user settings, and not at workspace scope.