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

Setting a terminal by workspace settings is broken! User settings are not sufficient! #19758

Closed
RobIsHere opened this issue Feb 2, 2017 · 12 comments
Assignees
Labels
feature-request Request for new features or functionality terminal Integrated terminal issues
Milestone

Comments

@RobIsHere
Copy link

  • VSCode Version: 1.9.0
  • OS Version: Windows 10

Steps to Reproduce:

  1. Upgrade to VSCode 1.9.0
  2. Open Terminal

VSCode complains that my custom terminal settings in the workspace configuration are wrong after the update. It does not respect these anymore.

I need per project/workspace configuration because i use remote ssh terminals to work on the dev server. The dev server is different in every workspace.

This was perfectly working:

    "terminal.integrated.shell.windows": "ssh.exe",
    "terminal.integrated.shellArgs.windows": [
        "myuser@myserver.com",
        "-t",
        "cd /data/project; bash --login"
    ],

Please tell me how i can downgrade to the working version until this is fixed.

@AdrianPell
Copy link

Similar thing - I use Bash in some workspaces, Cmd in others - hence user settings alone isn't sufficient.

@RobIsHere
Copy link
Author

I could easily downgrade by uninstalling and installing the previous version via chocolately:
https://chocolatey.org/packages/VisualStudioCode/1.8.1

@Tyriar Tyriar added the terminal Integrated terminal issues label Feb 3, 2017
@Tyriar
Copy link
Member

Tyriar commented Feb 3, 2017

@kieferrm thoughts?

@Tyriar Tyriar added the info-needed Issue requires more information from poster label Feb 3, 2017
@Matticusau
Copy link
Member

Matticusau commented Feb 3, 2017

I agree, we need a way to specify these sort of settings per workspace. I work across different projects where while the bulk use PowerShell some others use SQL Command line and Bash etc

Another point to this is I collaborate across teams with these projects. I dont really want to have to explain to them how to edit their User settings each time a new member helps out on a project. It should just ship the workspace setting with the code base. Isn't that the intent here? I mean I understand this could be seen as a security risk, but no more than running any terminal / command line code? If I was starting out on an existing project after I get the code I would check what workspace settings are set. Maybe that could be a solution, the first time someone opens a project up display the workspace settings to them for review?

@Matticusau
Copy link
Member

I've been reading the release notes and there seems to be language specific settings now (or maybe I'm just catching up). This might help in some circumstance but not in all cases like mentioned in other comments

@Tyriar
Copy link
Member

Tyriar commented Feb 3, 2017

Maybe we are being too strict, this use case does seem particularly useful. How about we take another approach to the problem while still protecting users from running malicious executables?

Consider the following scenario:

  1. Download a git repo with terminal.integrated.shell.windows set to "foo" and terminal.integrated.shellArgs.windows set to "bar"
  2. Open the git repo in VS Code
  3. Press ctrl+`, at this moment the following things happen;
    • the terminal opens with the user setting for terminal.integrated.shell.windows
    • a message box appears explaining that the workspace sets terminal.integrated.shell.windows and asks if the user wants to allow it or ignore it
    • a message box appears explaining that the workspace sets terminal.integrated.shellArgs.windows and asks if the user wants to allow it or ignore it

For example:

image

The actions on the message box could be [Yes] and [Ignore] (or something more verbose). When [Yes] if selected, the option would be saved in local storage and not bug the user again. When [Ignore] is selected, the message box will go away from the rest of the session, displaying again the next time the workspace is loaded.

Something like this would prevent anything from running without an explicit opt in from the user.

@AdrianPell
Copy link

I understand the concern with targeting malicious executables in git repo. Hadn't thought about that before.
I do like the proposal, though. As long as the action is remembered across sessions, that would seem to address most needs. How about adding a [Remove] action as well that would remove the setting from the workspace settings ... might be useful for somebody who doesn't know where those settings are?

@gulshan
Copy link

gulshan commented Feb 16, 2017

I think the shell name and shell executable path can be separate settings. And executable path settings will not be allowed in project/workspace settings. The names can be- "BASH", "CMD", "Powershell", "SQL", "Custom" etc. Then a project will be able to set the name of the intended shell but not the path.

@RobIsHere
Copy link
Author

The usable thing is the whole use case, not just a part of it:
Open SSH AND Open working directory

The settings as in 1.8.1 are flexible and this way good for every situation.
The proposed solution by @Tyriar is good, because flexibility is preserved.

@kieferrm
Copy link
Member

@Tyriar your proposed solution makes sense that is applicable to all platforms.

@kieferrm kieferrm added this to the March 2017 milestone Feb 17, 2017
@kieferrm kieferrm mentioned this issue Mar 3, 2017
58 tasks
@Tyriar Tyriar added feature-request Request for new features or functionality and removed info-needed Issue requires more information from poster labels Mar 23, 2017
@Tyriar
Copy link
Member

Tyriar commented Mar 23, 2017

This is what you should see in tomorrow's Insiders build:

image

@Tyriar Tyriar closed this as completed in 521a1f9 Mar 23, 2017
@RobIsHere
Copy link
Author

Thank you!

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality terminal Integrated terminal issues
Projects
None yet
Development

No branches or pull requests

6 participants