Skip to content

Commit

Permalink
Prevent EPERM error hit when trying to open installer.exe (#1855)
Browse files Browse the repository at this point in the history 
* Make Command Executor return an object instead of string

This prevents us from having to run commands twice to get the status and stdout.

Caution must be taken to make sure that the assumptions here in what is returned as stdout vs stderr are correct.

Furthermore, when looking at the code, I saw something that looked just plain incorrect, so I will need to investigate that by adding the TODO before merge

* Fix Execution Type Parsing

* fix linting issue

* Fix line parsing logic

* Fix tests

* Run icacls to set execute permissions

* move to sha256

* remove unnecessary code

* Remove todo that should go in another PR

* Remove redundant trim

* rename class CommandProcessorOutput since it didnt match CommandExecutor

* Rename class file

* Report json object for failed permissions.

* Fix test to use correct folder

* fix lint

* Fix installer file deletion

* respond to linter
  • Loading branch information
@nagilson
nagilson authored Jul 3, 2024
1 parent 557ffc6 commit 8cabdcf
Show file tree
Hide file tree
Showing 20 changed files with 314 additions and 311 deletions.
11 changes: 6 additions & 5 deletions vscode-dotnet-runtime-extension/src/test/functional/DotnetCoreAcquisitionExtension.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
} from 'vscode-dotnet-runtime-library';
import * as extension from '../../extension';
import { warn } from 'console';
import { json } from 'stream/consumers';
/* tslint:disable:no-any */
/* tslint:disable:no-unsafe-finally */

Expand Down Expand Up @@ -149,9 +150,9 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
let dotnetPaths: string[] = [];
for (const version of versions) {
const result = await vscode.commands.executeCommand<IDotnetAcquireResult>('dotnet.acquire', { version, requestingExtensionId, mode: installMode });
assert.exists(result);
assert.exists(result!.dotnetPath);
assert.include(result!.dotnetPath, version);
assert.exists(result, 'acquire command returned a result/success');
assert.exists(result!.dotnetPath, 'the result has a path');
assert.include(result!.dotnetPath, version, 'the path includes the version');
if (result!.dotnetPath) {
dotnetPaths = dotnetPaths.concat(result!.dotnetPath);
}
Expand Down Expand Up @@ -262,8 +263,8 @@ suite('DotnetCoreAcquisitionExtension End to End', function()
const uninstallCompletedEvent = MockTelemetryReporter.telemetryEvents.find((event: ITelemetryEvent) => event.eventName === 'DotnetUninstallAllCompleted');
assert.exists(uninstallCompletedEvent, 'Uninstall All success is reported in telemetry');
// Check that no errors were reported
const errors = MockTelemetryReporter.telemetryEvents.filter((event: ITelemetryEvent) => event.eventName.includes('Error'));
assert.isEmpty(errors, 'No error events were reported in telemetry reporting');
const errors = MockTelemetryReporter.telemetryEvents.filter((event: ITelemetryEvent) => event.eventName.includes('Error') && event.eventName !== 'CommandExecutionStdError');
assert.isEmpty(errors, `No error events were reported in telemetry reporting. ${JSON.stringify(errors)}`);
}).timeout(standardTimeoutTime);

test('Telemetry Sent on Error', async () => {
Expand Down
16 changes: 8 additions & 8 deletions vscode-dotnet-runtime-library/src/Acquisition/DotnetInstall.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export type DotnetInstallOrStr = DotnetInstall | string;
* That is not the case at the moment, but it is a possibility.
* Think carefully between using this and IsEquivalentInstallation
*/
export function IsEquivalentInstallationFile(a: DotnetInstall, b: DotnetInstall): boolean {
export function IsEquivalentInstallationFile(a: DotnetInstall, b: DotnetInstall): boolean
{
return a.version === b.version && a.architecture === b.architecture &&
a.isGlobal === b.isGlobal && a.installMode === b.installMode;
}
Expand All @@ -42,18 +43,16 @@ export function IsEquivalentInstallationFile(a: DotnetInstall, b: DotnetInstall)
* (e.g. auto updating the '8.0' install.)
* Think carefully between using this and IsEquivalentInstallationFile. There is no difference between the two *yet*
*/
export function IsEquivalentInstallation(a: DotnetInstall, b: DotnetInstall): boolean {
export function IsEquivalentInstallation(a: DotnetInstall, b: DotnetInstall): boolean
{
return a.installKey === b.installKey;
}

/**
* @returns A string set representing the installation of either a .NET runtime or .NET SDK.
*/
export function InstallToStrings(key: DotnetInstall | null) {
if (!key) {
return { installKey: '', version: '', architecture: '', isGlobal: '', installMode: '' };
}

export function InstallToStrings(key: DotnetInstall)
{
return {
installKey: key.installKey,
version: key.version,
Expand All @@ -63,7 +62,8 @@ export function InstallToStrings(key: DotnetInstall | null) {
};
}

export function looksLikeRuntimeVersion(version: string): boolean {
export function looksLikeRuntimeVersion(version: string): boolean
{
const band: string | undefined = version.split('.')?.at(2);
return !band || band.length <= 2; // assumption : there exists no runtime version at this point over 99 sub versions
}
Expand Down
46 changes: 17 additions & 29 deletions vscode-dotnet-runtime-library/src/Acquisition/GenericDistroSDKProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,47 +23,39 @@ export class GenericDistroSDKProvider extends IDistroDotnetSDKProvider
let commands = this.myDistroCommands(this.installCommandKey);
const sdkPackage = await this.myDotnetVersionPackageName(fullySpecifiedVersion, installType);

const oldReturnStatusSetting = this.commandRunner.returnStatus;
this.commandRunner.returnStatus = true;
commands = CommandExecutor.replaceSubstringsInCommands(commands, this.missingPackageNameKey, sdkPackage);
const updateCommandsResult = (await this.commandRunner.executeMultipleCommands(commands.slice(0, -1), undefined))[0];
const installCommandResult = await this.commandRunner.execute(commands.slice(-1)[0]);
const installCommandResult = (await this.commandRunner.execute(commands.slice(-1)[0])).status;

this.commandRunner.returnStatus = oldReturnStatusSetting;
return installCommandResult;
}

public async getInstalledGlobalDotnetPathIfExists(installType : DotnetInstallMode) : Promise<string | null>
{
const commandResult = await this.commandRunner.executeMultipleCommands(this.myDistroCommands(this.currentInstallPathCommandKey));

const oldReturnStatusSetting = this.commandRunner.returnStatus;
this.commandRunner.returnStatus = true;
const commandSignal = await this.commandRunner.executeMultipleCommands(this.myDistroCommands(this.currentInstallPathCommandKey));
this.commandRunner.returnStatus = oldReturnStatusSetting;

if(commandSignal[0] !== '0') // no dotnet error can be returned, dont want to try to parse this as a path
if(commandResult[0].status !== '0') // no dotnet error can be returned, dont want to try to parse this as a path
{
return null;
}

if(commandResult[0])
if(commandResult[0].stdout)
{
commandResult[0] = commandResult[0].trim();
commandResult[0].stdout = commandResult[0].stdout.trim();
}

if(commandResult[0] && this.resolvePathAsSymlink)
{
let symLinkReadCommand = this.myDistroCommands(this.readSymbolicLinkCommandKey);
symLinkReadCommand = CommandExecutor.replaceSubstringsInCommands(symLinkReadCommand, this.missingPathKey, commandResult[0]);
const resolvedPath = (await this.commandRunner.executeMultipleCommands(symLinkReadCommand))[0];
symLinkReadCommand = CommandExecutor.replaceSubstringsInCommands(symLinkReadCommand, this.missingPathKey, commandResult[0].stdout);
const resolvedPath = (await this.commandRunner.executeMultipleCommands(symLinkReadCommand))[0].stdout;
if(resolvedPath)
{
return path.dirname(resolvedPath.trim());
}
}

return commandResult[0] ?? null;
return commandResult[0].stdout ?? null;
}

public async dotnetPackageExistsOnSystem(fullySpecifiedDotnetVersion : string, installType : DotnetInstallMode) : Promise<boolean>
Expand All @@ -74,7 +66,7 @@ export class GenericDistroSDKProvider extends IDistroDotnetSDKProvider
const commandResult = (await this.commandRunner.executeMultipleCommands(command))[0];

const noPackageResult = 'no packages found';
return commandResult[0].toLowerCase().includes(noPackageResult);
return commandResult.stdout.toLowerCase().includes(noPackageResult);
}

public getExpectedDotnetDistroFeedInstallationDirectory(): string
Expand All @@ -89,15 +81,11 @@ export class GenericDistroSDKProvider extends IDistroDotnetSDKProvider

public async upgradeDotnet(versionToUpgrade : string, installType : DotnetInstallMode): Promise<string>
{
const oldReturnStatusSetting = this.commandRunner.returnStatus;
this.commandRunner.returnStatus = true;

let command = this.myDistroCommands(this.updateCommandKey);
const sdkPackage = await this.myDotnetVersionPackageName(versionToUpgrade, installType);
command = CommandExecutor.replaceSubstringsInCommands(command, this.missingPackageNameKey, sdkPackage);
const commandResult = (await this.commandRunner.executeMultipleCommands(command))[0];
const commandResult = (await this.commandRunner.executeMultipleCommands(command))[0].status;

this.commandRunner.returnStatus = oldReturnStatusSetting;
return commandResult[0];
}

Expand All @@ -108,22 +96,22 @@ export class GenericDistroSDKProvider extends IDistroDotnetSDKProvider
command = CommandExecutor.replaceSubstringsInCommands(command, this.missingPackageNameKey, sdkPackage);
const commandResult = (await this.commandRunner.executeMultipleCommands(command))[0];

return commandResult[0];
return commandResult.stdout;
}

public async getInstalledDotnetSDKVersions(): Promise<string[]>
{
const command = this.myDistroCommands(this.installedSDKVersionsCommandKey);
const commandResult = (await this.commandRunner.executeMultipleCommands(command))[0];

const outputLines : string[] = commandResult.split('\n');
const outputLines : string[] = commandResult.stdout.split('\n');
const versions : string[] = [];

for(const line of outputLines)
{
const splitLine = line.split(/\s+/);
// list sdk lines shows in the form: version [path], so the version is the 2nd item
if(splitLine.length === 2)
if(splitLine.length === 2 && splitLine[0] !== '')
{
versions.push(splitLine[0]);
}
Expand All @@ -136,7 +124,7 @@ export class GenericDistroSDKProvider extends IDistroDotnetSDKProvider
const command = this.myDistroCommands(this.installedRuntimeVersionsCommandKey);
const commandResult = (await this.commandRunner.executeMultipleCommands(command))[0];

const outputLines : string[] = commandResult.split('\n');
const outputLines : string[] = commandResult.stdout.split('\n');
const versions : string[] = [];

for(const line of outputLines)
Expand All @@ -157,15 +145,15 @@ export class GenericDistroSDKProvider extends IDistroDotnetSDKProvider

// we need to run this command in the root directory otherwise local dotnets on the path may interfere
const rootDir = path.parse(__dirname).root;
let commandResult = (await this.commandRunner.executeMultipleCommands(command, { cwd: path.resolve(rootDir), shell: true }))[0];
const commandResult = (await this.commandRunner.executeMultipleCommands(command, { cwd: path.resolve(rootDir), shell: true }))[0];

commandResult = commandResult.replace('\n', '');
if(!this.versionResolver.isValidLongFormVersionFormat(commandResult))
commandResult.stdout = commandResult.stdout.replace('\n', '');
if(!this.versionResolver.isValidLongFormVersionFormat(commandResult.stdout))
{
return null;
}
{
return commandResult;
return commandResult.stdout;
}
}

Expand Down
12 changes: 6 additions & 6 deletions vscode-dotnet-runtime-library/src/Acquisition/IDistroDotnetSDKProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,14 +210,14 @@ Please install the .NET SDK manually: https://dotnet.microsoft.com/download`),
let command = this.myDistroCommands(this.searchCommandKey);
command = CommandExecutor.replaceSubstringsInCommands(command, this.missingPackageNameKey, packageName);

const packageIsAvailableResult = (await this.commandRunner.executeMultipleCommands(command))[0].trim();
const oldReturnStatusSetting = this.commandRunner.returnStatus;
const packageIsAvailableResult = (await this.commandRunner.executeMultipleCommands(command))[0];
packageIsAvailableResult.stdout = packageIsAvailableResult.stdout.trim();
packageIsAvailableResult.stderr = packageIsAvailableResult.stderr.trim();
packageIsAvailableResult.status = packageIsAvailableResult.status.trim();

this.commandRunner.returnStatus = true;
const packageAvailableExitCode = (await this.commandRunner.executeMultipleCommands(command))[0].trim();
this.commandRunner.returnStatus = oldReturnStatusSetting;
const packageExists = this.isPackageFoundInSearch(`${packageIsAvailableResult.stdout}${packageIsAvailableResult.stderr}`,
packageIsAvailableResult.status);

const packageExists = this.isPackageFoundInSearch(packageIsAvailableResult, packageAvailableExitCode);
if(packageExists)
{
thisVersionPackage.packages.push(packageName);
Expand Down
5 changes: 3 additions & 2 deletions vscode-dotnet-runtime-library/src/Acquisition/IGlobalInstaller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* ------------------------------------------------------------------------------------------ */

import path = require('path');
import crypto = require('crypto')
import { IAcquisitionWorkerContext } from './IAcquisitionWorkerContext';
import { IUtilityContext } from '../Utils/IUtilityContext';
import { DotnetInstall } from './DotnetInstall';
Expand All @@ -29,8 +30,8 @@ export abstract class IGlobalInstaller {
*
* @returns The folder where global sdk installers will be downloaded onto the disk.
*/
public static getDownloadedInstallFilesFolder() : string
public static getDownloadedInstallFilesFolder(uniqueInstallerId : string) : string
{
return path.join(__dirname, 'installers');
return path.join(__dirname, 'installers', crypto.createHash('sha256').update(uniqueInstallerId).digest('hex'));
}
}
2 changes: 1 addition & 1 deletion vscode-dotnet-runtime-library/src/Acquisition/LinuxVersionResolver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ If you would like to contribute to the list of supported distros, please visit:

try
{
const stdOut = commandResult.toString().split('\n');
const stdOut = commandResult.stdout.toString().split('\n');
// We need to remove the quotes from the KEY="VALUE"\n pairs returned by the command stdout, and then turn it into a dictionary. We can't use replaceAll for older browsers.
// Replace only replaces one quote, so we remove the 2nd one later.
const stdOutWithQuotesRemoved = stdOut.map( x => x.replace('"', ''));
Expand Down
53 changes: 35 additions & 18 deletions vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,19 +141,29 @@ We cannot verify .NET is safe to download at this time. Please try again later.`
*/
private async downloadInstaller(installerUrl : string) : Promise<string>
{
const ourInstallerDownloadFolder = IGlobalInstaller.getDownloadedInstallFilesFolder();
const ourInstallerDownloadFolder = IGlobalInstaller.getDownloadedInstallFilesFolder(installerUrl);
this.file.wipeDirectory(ourInstallerDownloadFolder, this.acquisitionContext.eventStream);
const installerPath = path.join(ourInstallerDownloadFolder, `${installerUrl.split('/').slice(-1)}`);

const installerDir = path.dirname(installerPath);
if (!fs.existsSync(installerDir)){
fs.mkdirSync(installerDir);
fs.mkdirSync(installerDir, {recursive: true});
}

await this.webWorker.downloadFile(installerUrl, installerPath);
try
{
fs.chmodSync(installerPath, 0o744);

if(os.platform() === 'win32') // Windows does not have chmod +x ability with nodejs.
{
const commandRes = await this.commandRunner.execute(CommandExecutor.makeCommand('icacls', ['/grant', `"${installerPath}"`, `"%username%":F`, '/t', '/c']), null, false);
if(commandRes.stderr !== '')
{
const error = new EventBasedError('FailedToSetInstallerPermissions', `Failed to set icacls permissions on the installer file ${installerPath}. ${commandRes.stderr}`);
this.acquisitionContext.eventStream.post(new SuppressedAcquisitionError(error, error.message));
}
}
}
catch(error : any)
{
Expand Down Expand Up @@ -222,7 +232,6 @@ We cannot verify .NET is safe to download at this time. Please try again later.`
*/
public async executeInstall(installerPath : string) : Promise<string>
{
this.commandRunner.returnStatus = true;
if(os.platform() === 'darwin')
{
// For Mac:
Expand Down Expand Up @@ -254,8 +263,7 @@ Please correct your PATH variable or make sure the 'open' utility is installed s
);
this.acquisitionContext.eventStream.post(new NetInstallerEndExecutionEvent(`The OS X .NET Installer has closed.`));

this.commandRunner.returnStatus = false;
return commandResult;
return commandResult.status;
}
else
{
Expand All @@ -267,13 +275,25 @@ Please correct your PATH variable or make sure the 'open' utility is installed s
}

this.acquisitionContext.eventStream.post(new NetInstallerBeginExecutionEvent(`The Windows .NET Installer has been launched.`));
const commandResult = await this.commandRunner.execute(
CommandExecutor.makeCommand(command, commandOptions)
);
this.acquisitionContext.eventStream.post(new NetInstallerEndExecutionEvent(`The Windows .NET Installer has closed.`));

this.commandRunner.returnStatus = false;
return commandResult;
try
{
const commandResult = await this.commandRunner.execute(CommandExecutor.makeCommand(command, commandOptions));
this.acquisitionContext.eventStream.post(new NetInstallerEndExecutionEvent(`The Windows .NET Installer has closed.`));
return commandResult.status;
}
catch(error : any)
{
if(error?.message?.includes('EPERM'))
{
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}"`])))}`;
}
else if(error?.message?.includes('ENOENT'))
{
error.message = `The .NET Installation files were not found. Please try again. ${error.message}`;
}
throw error;
}
}
}

Expand Down Expand Up @@ -355,14 +375,11 @@ Please correct your PATH variable or make sure the 'open' utility is installed s
const command = CommandExecutor.makeCommand(registryQueryCommand, [`query`, `${query}`, `\/reg:32`]);

let installRecordKeysOfXBit = '';
const oldReturnStatusSetting = this.commandRunner.returnStatus;
this.commandRunner.returnStatus = true;
const registryLookupStatusCode = await this.commandRunner.execute(command);
this.commandRunner.returnStatus = oldReturnStatusSetting;
const registryLookup = (await this.commandRunner.execute(command));

if(registryLookupStatusCode === '0')
if(registryLookup.status === '0')
{
installRecordKeysOfXBit = await this.commandRunner.execute(command);
installRecordKeysOfXBit = registryLookup.stdout;
}

const installedSdks = this.extractVersionsOutOfRegistryKeyStrings(installRecordKeysOfXBit);
Expand Down
Loading

0 comments on commit 8cabdcf

Please sign in to comment.