owtaylor / toolbox-vscode

Toolbox Visual Studio Code integration
Apache License 2.0
285 stars 24 forks source link

Tasks with -c in args get mangled (was: deprecated parameters in settings.json) #11

Open ma3yta opened 2 years ago

ma3yta commented 2 years ago

https://github.com/owtaylor/toolbox-vscode/blob/main/code.sh#L371-L381

Screenshot from 2021-10-22 21-53-46

Also, these parameters causing errors for some tasks. Example:

> Executing task: func host start <

func host start: No such file or directory
The terminal process "/usr/sbin/capsh '--caps=', '--', '-c', 'exec "$@"', '/bin/sh', '/usr/bin/fish', '-l', 'func host start'" failed to launch (exit code: 127).
owtaylor commented 2 years ago

I don't think the error is related to the deprecation. Can you give reproduction instructions for the error - is this with https://marketplace.visualstudio.com/items?itemName=ms-azuretools.vscode-azurefunctions ? It's not obvious to me why/how the terminal arguments would be manipulated to produce an error like that.

owtaylor commented 2 years ago

For some background the capsh invocation is to avoid problems because the user's UID is actually effectively root in the container and gets root like capabilities - in particular cap_dac_override, which means that it completely overrides file permissions.

The error is likely caused by some dodgy code in Visual Studio Code:

https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts#L1141

// Set shellArgs to the value from the config
// Set toAdd to ['-c']
toAdd.forEach(element => {
if (!shellArgs.some(arg => arg.toLowerCase() === element)) { 
    shellArgs.push(element);
        }
});
shellArgs.push(commandLine);

Because -c is already in shellArgs it doesn't get added, so what should have been:

/usr/sbin/capsh -caps= -- -c exec "$@"' /bin/sh /usr/bin/fish -l -c 'func host start'

ends up as:

/usr/sbin/capsh -caps= -- -c exec "$@"' /bin/sh /usr/bin/fish -l 'func host start'

I can't see any way that skipping -c could ever produce the right result. So, I think there's a Visual Studio code bug here. This part of the code was added as part of a larger commit, so it's not clear what the intent is - a pretty safe change would be to limit the the skipping to the Windows case, though I doubt it is correct in this case either.

I don't know a great workaround here - one thing we could do is set "terminal.integrated.automationShell.linux" to bash - we'd still have funky capabilities in the automated task case, which could be confusing, but the scope would be limited.

owtaylor commented 2 years ago

Filed a vscode issue - https://github.com/microsoft/vscode/issues/136459 - will need a workaround here however.