Summary
execAsync builds a single shell string by joining the command and its arguments with spaces and no quoting, so any path containing a space is split into separate words by the shell. Every call goes through a binary path resolved from the workspace or an interpreter location, so a workspace under a directory with a space in its name breaks.
vscode/extension/src/utilities/exec.ts:
function execAsyncCore(command: string, args: string[], options = {}) {
return new Promise<ExecResult>((resolve, reject) => {
const child = exec(
`${command} ${args.join(' ')}`, // <- no quoting
options,
...
exec runs its argument through a shell, so the quoting has to be done here.
Reproduction
Any workspace whose path contains a space. Mine is /Users/mac/Documents/BrainStorm Projects/..., which produces:
/bin/sh: /Users/mac/Documents/BrainStorm: No such file or directory
and an exit code of 127 surfaced as a failed command. I hit it in the e2e suite, where diagnostics.spec.ts fails with Received: 127; the same two tests pass in a checkout whose path has no space. It is not specific to the tests though — the same call path is used at runtime.
Affected callers
Everything that shells out, since all of them pass a resolved binary path:
src/auth/auth.ts — six calls, including tcloud auth login / logout and the background login server
src/utilities/sqlmesh/sqlmesh.ts — is_sqlmesh_installed, install_sqlmesh, --version, and the interpreter check at line 441
Suggested fix
Use execFile rather than exec, and pass args as an array. That removes the shell from the path entirely, so no quoting is needed and argument values cannot be reinterpreted as shell syntax. The function already takes (command, args[]), so the signature does not change — only execAsyncCore's body and the fullCmd string used for tracing.
If a shell is wanted for some caller, the alternative is quoting each part, but execFile looks like the better fit given none of the current callers rely on shell features.
Notes
This is distinct from #5608, which is about the shell dialect used by sqlmesh: Print Environment. This one is about quoting, and affects every shell equally.
I am happy to open a PR for this if it is useful — just did not want to fold it into an unrelated change.
Summary
execAsyncbuilds a single shell string by joining the command and its arguments with spaces and no quoting, so any path containing a space is split into separate words by the shell. Every call goes through a binary path resolved from the workspace or an interpreter location, so a workspace under a directory with a space in its name breaks.vscode/extension/src/utilities/exec.ts:execruns its argument through a shell, so the quoting has to be done here.Reproduction
Any workspace whose path contains a space. Mine is
/Users/mac/Documents/BrainStorm Projects/..., which produces:and an exit code of 127 surfaced as a failed command. I hit it in the e2e suite, where
diagnostics.spec.tsfails withReceived: 127; the same two tests pass in a checkout whose path has no space. It is not specific to the tests though — the same call path is used at runtime.Affected callers
Everything that shells out, since all of them pass a resolved binary path:
src/auth/auth.ts— six calls, includingtcloud auth login/logoutand the background login serversrc/utilities/sqlmesh/sqlmesh.ts—is_sqlmesh_installed,install_sqlmesh,--version, and the interpreter check at line 441Suggested fix
Use
execFilerather thanexec, and passargsas an array. That removes the shell from the path entirely, so no quoting is needed and argument values cannot be reinterpreted as shell syntax. The function already takes(command, args[]), so the signature does not change — onlyexecAsyncCore's body and thefullCmdstring used for tracing.If a shell is wanted for some caller, the alternative is quoting each part, but
execFilelooks like the better fit given none of the current callers rely on shell features.Notes
This is distinct from #5608, which is about the shell dialect used by
sqlmesh: Print Environment. This one is about quoting, and affects every shell equally.I am happy to open a PR for this if it is useful — just did not want to fold it into an unrelated change.