Turn off shell by default

vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts

@@ -138,7 +138,7 @@ suite('DotnetCoreAcquisitionExtension End to End', function ()
             context.architecture = arch;
         }
         const result = await vscode.commands.executeCommand<IDotnetAcquireResult>('dotnet.acquire', context);
-        assert.exists(result, 'Command results a result');
+        assert.exists(result, 'Acquire Command results a result');
         assert.exists(result!.dotnetPath, 'The return type of the local runtime install command has a .dotnetPath property');
         assert.isTrue(fs.existsSync(result!.dotnetPath), 'The returned path of .net does exist');
         assert.include(result!.dotnetPath, '.dotnet', '.dotnet is in the path of the local runtime install');

vscode-dotnet-runtime-library/src/Acquisition/DotnetConditionValidator.ts

@@ -107,7 +107,7 @@ export class DotnetConditionValidator implements IDotnetConditionValidator
             return '';
         }
 
-        const infoCommand = CommandExecutor.makeCommand(`"${hostPath}"`, ['--info']);
+        const infoCommand = CommandExecutor.makeCommand(`${hostPath}`, ['--info']);
         const envWithForceEnglish = process.env;
         envWithForceEnglish.DOTNET_CLI_UI_LANGUAGE = 'en-US';
         // System may not have english installed, but CDK already calls this without issue -- the .NET SDK language invocation is also wrapped by a runtime library and natively includes english assets
@@ -143,7 +143,7 @@ Please set the PATH to a dotnet host that matches the architecture ${requirement
             return [];
         }
 
-        const findSDKsCommand = CommandExecutor.makeCommand(`"${existingPath}"`, ['--list-sdks', '--arch', requestedArchitecture]);
+        const findSDKsCommand = CommandExecutor.makeCommand(`${existingPath}`, ['--list-sdks', '--arch', requestedArchitecture]);
 
         const sdkInfo = await (this.executor!).execute(findSDKsCommand, { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false).then(async (result) =>
         {
@@ -182,7 +182,7 @@ Please set the PATH to a dotnet host that matches the architecture ${requirement
         // We don't check that the version is 10.0 or later after 2026 when .NET 11 starts rolling out, as It will be slower to check all of the numbers in the output for versions >= 10.
         const hostMaySupportArchFlag = listDotnetInstallsStdout.includes("10.0") || listDotnetInstallsStdout.includes("11.0") || Date.now() >= new Date('2026-03-01').getTime();
         // Use runtimes instead of sdks, as sdks will always have a runtime, and runtime search can be cached across both mode calls.
-        const findInvalidCommand = CommandExecutor.makeCommand(`"${dotnetExecutablePath}"`, ['--list-runtimes', '--arch', 'invalid-arch']);
+        const findInvalidCommand = CommandExecutor.makeCommand(`${dotnetExecutablePath}`, ['--list-runtimes', '--arch', 'invalid-arch']);
         const hostSupportsArchFlag = hostMaySupportArchFlag ? (await (this.executor!).execute(findInvalidCommand, { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false)).status !== '0' : false;
         return hostSupportsArchFlag;
     }
@@ -325,7 +325,7 @@ Please set the PATH to a dotnet host that matches the architecture ${requirement
 
     private getRuntimesCommand(existingPath: string, requestedArchitecture: string): CommandExecutorCommand
     {
-        return CommandExecutor.makeCommand(`"${existingPath}"`, ['--list-runtimes', '--arch', requestedArchitecture]);
+        return CommandExecutor.makeCommand(`${existingPath}`, ['--list-runtimes', '--arch', requestedArchitecture]);
     }
 
     public async getRuntimes(existingPath: string, requestedArchitecture: string | null, knownArchitecture: boolean): Promise<IDotnetListInfo[]>

vscode-dotnet-runtime-library/src/Acquisition/DotnetCoreAcquisitionWorker.ts

@@ -35,8 +35,7 @@ import
     DotnetWSLSecurityError,
     EventBasedError,
     EventCancellationError,
-    SuppressedAcquisitionError,
-    UtilizingExistingInstallPromise
+    SuppressedAcquisitionError
 } from '../EventStream/EventStreamEvents';
 import * as versionUtils from './VersionUtilities';
 
@@ -360,7 +359,7 @@ To keep your .NET version up to date, please reconnect to the internet at your s
     private async sdkIsFound(context: IAcquisitionWorkerContext, version: string): Promise<boolean>
     {
         const executor = new CommandExecutor(context, this.utilityContext);
-        const listSDKsCommand = CommandExecutor.makeCommand('dotnet', ['--list-sdks', '--arch']);
+        const listSDKsCommand = CommandExecutor.makeCommand('dotnet', ['--list-sdks', '--arch', context.acquisitionContext.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture()], false);
         const result = await executor.execute(listSDKsCommand, { dotnetInstallToolCacheTtlMs: DOTNET_INFORMATION_CACHE_DURATION_MS }, false);
 
         if (result.status !== '0')

vscode-dotnet-runtime-library/src/Acquisition/DotnetPathFinder.ts

@@ -411,7 +411,7 @@ export class DotnetPathFinder implements IDotnetPathFinder
                 // /usr/local/bin/dotnet becomes /snap/dotnet-sdk/current/dotnet in reality, may have different behavior in shells.
                 if (os.platform() === 'win32')
                 {
-                    LocalMemoryCacheSingleton.getInstance().aliasCommandAsAnotherCommandRoot(`"${truePath}"`, `"${tentativePath}"`, this.workerContext.eventStream);
+                    LocalMemoryCacheSingleton.getInstance().aliasCommandAsAnotherCommandRoot(`${truePath}`, `${tentativePath}`, this.workerContext.eventStream);
                 }
             }
             else

vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts

@@ -262,7 +262,7 @@ This report should be made at https://github.com/dotnet/vscode-dotnet-runtime/is
         {
             if (os.platform() === 'win32') // Windows does not have chmod +x ability with nodejs.
             {
-                const permissionsCommand = CommandExecutor.makeCommand('icacls', [`"${installerPath}"`, '/grant:r', `"%username%":F`, '/t', '/c']);
+                const permissionsCommand = CommandExecutor.makeCommand('icacls', [`${installerPath}`, '/grant:r', `%username%:F`, '/t', '/c']);
                 const commandRes = await this.commandRunner.execute(permissionsCommand, {}, false);
                 if (commandRes.stderr !== '')
                 {
@@ -334,7 +334,7 @@ Please try again, or download the .NET Installer file yourself. You may also rep
             else if (error?.message?.includes('EPERM'))
             {
                 this.acquisitionContext.eventStream.post(new DotnetFileIntegrityFailureEvent(`The file ${installerFile} did not have the correct permissions scope to be assessed.
-Permissions: ${JSON.stringify(await this.commandRunner.execute(CommandExecutor.makeCommand('icacls', [`"${installerFile}"`]), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false))}`));
+Permissions: ${JSON.stringify(await this.commandRunner.execute(CommandExecutor.makeCommand('icacls', [`${installerFile}`]), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false))}`));
             }
             return ask ? this.userChoosesToContinueWithInvalidHash() : false;
         }
@@ -436,7 +436,7 @@ Please correct your PATH variable or make sure the 'open' utility is installed s
             }
             else if (workingCommand.commandRoot === 'command')
             {
-                workingCommand = CommandExecutor.makeCommand(`open`, [`-W`, `"${path.resolve(installerPath)}"`]);
+                workingCommand = CommandExecutor.makeCommand(`open`, [`-W`, `${path.resolve(installerPath)}`]);
             }
 
             this.acquisitionContext.eventStream.post(new NetInstallerBeginExecutionEvent(`The OS X .NET Installer has been launched.`));
@@ -450,15 +450,15 @@ Please correct your PATH variable or make sure the 'open' utility is installed s
         }
         else
         {
-            const command = `"${path.resolve(installerPath)}"`;
+            const command = `${path.resolve(installerPath)}`;
             let commandOptions: string[] = [];
             if (await this.file.isElevated(this.acquisitionContext, this.utilityContext))
             {
-                commandOptions = [`/quiet`, `/install`, `/norestart`];
+                commandOptions = [`/install`, `/quiet`, `/norestart`];
             }
             else
             {
-                commandOptions = [`/passive`, `/install`, `/norestart`]
+                commandOptions = [`/install`, `/passive`, `/norestart`]
             }
 
             this.acquisitionContext.eventStream.post(new NetInstallerBeginExecutionEvent(`The Windows .NET Installer has been launched.`));
@@ -478,7 +478,7 @@ Please correct your PATH variable or make sure the 'open' utility is installed s
                     // Remove this when https://github.com/typescript-eslint/typescript-eslint/issues/2728 is done
                     // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
                     error.message = `The installer does not have permission to execute. Please try running as an administrator. ${error?.message}.
-Permissions: ${JSON.stringify(await this.commandRunner.execute(CommandExecutor.makeCommand('icacls', [`"${installerPath}"`]), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false))}`;
+Permissions: ${JSON.stringify(await this.commandRunner.execute(CommandExecutor.makeCommand('icacls', [`${installerPath}`]), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false))}`;
                 }
                 // Remove this when https://github.com/typescript-eslint/typescript-eslint/issues/2728 is done
                 // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access

vscode-dotnet-runtime-library/src/Utils/CommandExecutor.ts

@@ -6,7 +6,6 @@
 import * as proc from 'child_process';
 import * as fs from 'fs';
 import * as os from 'os';
-import { promisify } from 'util';
 import open = require('open');
 import path = require('path');
 
@@ -440,7 +439,11 @@ ${stderr}`));
             options.cwd ??= path.resolve(__dirname);
 
             // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-            options.shell ??= true;
+            options.shell ??= os.platform() === 'win32' ? false : true;
+            // ^ Windows systemcalls (node.js process library uses process_wrap which uses process.cc which makes system calls)
+            // Windows seems to resolve the PATH by default in a system call. Unix system calls do not.
+            // We could further improve Unix performance in the future by re-implementing our own PATH resolution.
+            // And turning SHELL off by default on Unix as well.
 
             // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
             options.encoding = 'utf8';
@@ -455,7 +458,7 @@ ${stderr}`));
         else
         {
             options = {
-                cwd: path.resolve(__dirname), shell: true, encoding: 'utf8', env:
+                cwd: path.resolve(__dirname), shell: os.platform() === 'win32' ? false : true, encoding: 'utf8', env:
                     { ...process.env, DOTNET_CLI_UI_LANGUAGE: 'en-US', DOTNET_NOLOGO: 'true' }
             };
         }
@@ -515,39 +518,79 @@ ${stderr}`));
             }
 
             const commandStartTime = process.hrtime.bigint();
-            const commandResult: CommandExecutorResult = await promisify(proc.exec)(fullCommandString, options).then(
-                fulfilled =>
+            const commandResult: CommandExecutorResult = await this.asyncSpawn(command, options, terminalFailure);
+            this.logCommandResult(commandResult, fullCommandString, commandStartTime, command.commandRoot);
+
+            if (useCache)
+            {
+                LocalMemoryCacheSingleton.getInstance().putCommand({ command, options }, commandResult, this.context);
+            }
+            return commandResult;
+        }
+    }
+
+    private asyncSpawn(commandToExecute: CommandExecutorCommand, options: any, terminalFailure: boolean): Promise<CommandExecutorResult>
+    {
+        return new Promise((resolve, reject) =>
+        {
+            const child = proc.spawn(commandToExecute.commandRoot, commandToExecute.commandParts, {
+                ...options,
+                stdio: ['ignore', 'pipe', 'pipe'], // Ignore stdin (we won't send any input), Capture stdout and stderr
+            });
+
+            let stdout = '';
+            let stderr = '';
+
+            child.stdout.on('data', (data: any) =>
+            {
+                stdout += data.toString();
+            });
+
+            child.stderr.on('data', (data: any) =>
+            {
+                stderr += data.toString();
+            });
+
+            child.on('close', (code: any) =>
+            {
+                if (code === 0)
+                {
+                    return resolve({ stdout, stderr, status: '0' });
+                }
+                else
                 {
-                    // If any status besides 0 is returned, an error is thrown by nodejs
-                    return { stdout: fulfilled.stdout?.toString() ?? '', stderr: fulfilled.stderr?.toString() ?? '', status: '0' };
-                },
-                rejected => // Rejected object: error type with stderr : Buffer, stdout : Buffer ... with .code (number) or .signal (string)}
-                { // see https://nodejs.org/api/child_process.html#child_processexeccommand-options-callback
-                    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-                    const result = { stdout: rejected?.stdout?.toString() ?? '', stderr: rejected?.stderr?.toString() ?? '', status: rejected?.code?.toString() ?? rejected?.signal?.toString() ?? '' };
+                    const result = { stdout, stderr, status: code?.toString() ?? '1' };
                     if (terminalFailure)
                     {
-                        this.logCommandResult(result, fullCommandString, commandStartTime, command.commandRoot);
-                        throw rejected ?? new Error(`Spawning ${fullCommandString} failed with an unspecified error.`); // according to nodejs spec, this should never be possible
+                        this.logCommandResult(result, CommandExecutor.prettifyCommandExecutorCommand(commandToExecute, false), process.hrtime.bigint(), commandToExecute.commandRoot);
+                        return reject(new CommandExecutionNonZeroExitFailure(new EventBasedError('CommandExecutionNonZeroExitFailure', ''), null));
                     }
                     else
                     {
                         // signal is a string or obj, code is a number
-                        return result;
+                        return resolve(result);
                     }
                 }
-            );
+            }); // We don't need to handle exit, close is when all exits have been called. (stderr, stdout)
 
-            this.logCommandResult(commandResult, fullCommandString, commandStartTime, command.commandRoot);
-
-            if (useCache)
+            child.on('error', (error) =>
             {
-                LocalMemoryCacheSingleton.getInstance().putCommand({ command, options }, commandResult, this.context);
-            }
-            return commandResult;
-        }
+                const result = { stdout, stderr, status: error.name?.toString() ?? '' };
+                if (terminalFailure)
+                {
+                    this.logCommandResult(result, CommandExecutor.prettifyCommandExecutorCommand(commandToExecute, false), process.hrtime.bigint(), commandToExecute.commandRoot);
+                    return reject(new CommandExecutionNonZeroExitFailure(new EventBasedError('CommandExecutionNonZeroExitFailure', ''), null));
+                }
+                else
+                {
+                    // signal is a string or obj, code is a number
+                    return resolve(result);
+                }
+            });
+        });
     }
 
+
     private logCommandResult(commandResult: CommandExecutorResult, fullCommandStringForTelemetryOnly: string, commandStartTime: bigint, commandRoot: string)
     {
         const durationMs = (Number(process.hrtime.bigint() - commandStartTime) / 1000000).toFixed(2);
@@ -646,10 +689,10 @@ Please report this at https://github.com/dotnet/vscode-dotnet-runtime/issues.`),
         if (os.platform() === 'win32')
         {
             const setShellVariable = CommandExecutor.makeCommand(`set`, [`${variable}=${value}`]);
-            const setSystemVariable = CommandExecutor.makeCommand(`setx`, [`${variable}`, `"${value}"`]);
+            const setSystemVariable = CommandExecutor.makeCommand(`setx`, [`${variable}`, `${value}`]);
             try
             {
-                const shellEditResponse = (await this.execute(setShellVariable, null, false)).status;
+                const shellEditResponse = (await this.execute(setShellVariable, { shell: true }, false)).status;
                 environmentEditExitCode += Number(shellEditResponse[0]);
                 const systemEditResponse = (await this.execute(setSystemVariable, null, false)).status
                 environmentEditExitCode += Number(systemEditResponse[0]);

vscode-dotnet-runtime-library/src/Utils/FileUtilities.ts

@@ -318,7 +318,7 @@ export class FileUtilities extends IFileUtilities
         {
             try
             {
-                const commandResult = await executor.execute(CommandExecutor.makeCommand('id', ['-u']), { dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false);
+                const commandResult = await executor.execute(CommandExecutor.makeCommand('id', ['-u']), { shell: true, dotnetInstallToolCacheTtlMs: SYSTEM_INFORMATION_CACHE_DURATION_MS }, false);
                 return commandResult.status === '0';
             }
             catch (error: any)

vscode-dotnet-runtime-library/src/Utils/TypescriptUtilities.ts

@@ -49,7 +49,7 @@ export async function isRunningUnderWSL(acquisitionContext: IAcquisitionWorkerCo
 
     const command = CommandExecutor.makeCommand('grep', ['-i', 'Microsoft', '/proc/version']);
     executor ??= new CommandExecutor(acquisitionContext, utilityContext);
-    const commandResult = await executor.execute(command, {}, false);
+    const commandResult = await executor.execute(command, { shell: true }, false);
 
     if (!commandResult || !commandResult.stdout)
     {

vscode-dotnet-runtime-library/src/test/unit/WinMacGlobalInstaller.test.ts

@@ -112,17 +112,19 @@ suite('Windows & Mac Global Installer Tests', function ()
         {
             assert.isTrue(mockExecutor.attemptedCommand.startsWith('open'), `It ran the right mac command, open. Command found: ${mockExecutor.attemptedCommand}`);
             assert.isTrue(mockExecutor.attemptedCommand.includes('-W'), 'It used the -W flag');
-            assert.isTrue(mockExecutor.attemptedCommand.includes('"'), 'It put the installer in quotes for username with space in it');
+            const executablePath = mockExecutor.attemptedCommand.split('-W')[1].trim();
+            assert.isTrue(fs.existsSync(executablePath), 'It used a full path of an executable that exists');
         }
         else if (os.platform() === 'win32')
         {
-            const returnedPath = mockExecutor.attemptedCommand.split(' ')[0].slice(1, -1);
+            const returnedPath = mockExecutor.attemptedCommand.split('/install')[0].trim();
             assert.isTrue(fs.existsSync(returnedPath), `It ran a command to an executable that exists: ${returnedPath}`);
-            assert.isTrue(mockExecutor.attemptedCommand.includes('"'), 'It put the installer in quotes for username with space in it');
+            assert.isTrue(mockExecutor.attemptedCommand.includes('.exe'), 'It has the .exe in the path of the installer, and did not skip / split incorrectly via a path space');
             if (await new FileUtilities().isElevated(mockSdkContext, utilContext))
             {
                 assert.include(mockExecutor.attemptedCommand, ' /quiet /install /norestart', 'It ran under the hood if it had privileges already');
             }
+
         }
         mockSdkContext
         // Rerun install to clean it up.