From 92b825907316fa115d13852fd2751ec0282e29a7 Mon Sep 17 00:00:00 2001 From: Vikash Agrawal Date: Tue, 30 Apr 2024 19:21:18 -0700 Subject: [PATCH] fix: validate path for getHomeDirectory --- .../auth/credentials/sharedCredentialsFile.ts | 15 +++++++++--- packages/core/src/shared/systemUtilities.ts | 13 ++++++----- .../core/src/shared/utilities/pathUtils.ts | 23 +++++++++++++++++++ .../credentials/sharedCredentials.test.ts | 4 ++++ .../credentials/userCredentialsUtils.test.ts | 12 ++++++++++ .../src/test/shared/systemUtilities.test.ts | 15 ++++++++---- 6 files changed, 69 insertions(+), 13 deletions(-) diff --git a/packages/core/src/auth/credentials/sharedCredentialsFile.ts b/packages/core/src/auth/credentials/sharedCredentialsFile.ts index 051a03ff2c..11d39bfb1e 100644 --- a/packages/core/src/auth/credentials/sharedCredentialsFile.ts +++ b/packages/core/src/auth/credentials/sharedCredentialsFile.ts @@ -5,18 +5,27 @@ * This module focuses on the i/o of the credentials/config files */ -import { join } from 'path' +import { join, resolve } from 'path' import { EnvironmentVariables } from '../../shared/environmentVariables' import { SystemUtilities } from '../../shared/systemUtilities' +import { isValidPath } from '../../shared/utilities/pathUtils' export function getCredentialsFilename(): string { const env = process.env as EnvironmentVariables - return env.AWS_SHARED_CREDENTIALS_FILE || join(SystemUtilities.getHomeDirectory(), '.aws', 'credentials') + if (env.AWS_SHARED_CREDENTIALS_FILE && isValidPath(resolve(env.AWS_SHARED_CREDENTIALS_FILE))) { + return resolve(env.AWS_SHARED_CREDENTIALS_FILE) + } + + return join(SystemUtilities.getHomeDirectory(), '.aws/credentials') } export function getConfigFilename(): string { const env = process.env as EnvironmentVariables - return env.AWS_CONFIG_FILE || join(SystemUtilities.getHomeDirectory(), '.aws', 'config') + if (env.AWS_CONFIG_FILE && isValidPath(resolve(env.AWS_CONFIG_FILE))) { + return resolve(env.AWS_CONFIG_FILE) + } + + return join(SystemUtilities.getHomeDirectory(), '.aws/config') } diff --git a/packages/core/src/shared/systemUtilities.ts b/packages/core/src/shared/systemUtilities.ts index 9ce48a2904..232fb1f46e 100644 --- a/packages/core/src/shared/systemUtilities.ts +++ b/packages/core/src/shared/systemUtilities.ts @@ -16,6 +16,7 @@ import { isCloud9 } from './extensionUtilities' import { Settings } from './settings' import { PermissionsError, PermissionsTriplet, isFileNotFoundError, isNoPermissionsError } from './errors' import globals, { isWeb } from './extensionGlobals' +import { isValidPath } from './utilities/pathUtils' export function createPermissionsErrorHandler( uri: vscode.Uri, @@ -60,16 +61,16 @@ export class SystemUtilities { const env = process.env as EnvironmentVariables - if (env.HOME !== undefined) { - return env.HOME + if (env.HOME && isValidPath(path.resolve(env.HOME))) { + return path.resolve(env.HOME) } - if (env.USERPROFILE !== undefined) { - return env.USERPROFILE + if (env.USERPROFILE && isValidPath(path.resolve(env.USERPROFILE))) { + return path.resolve(env.USERPROFILE) } - if (env.HOMEPATH !== undefined) { + if (env.HOMEPATH && isValidPath(path.resolve(env.HOMEPATH))) { const homeDrive: string = env.HOMEDRIVE || 'C:' - return path.join(homeDrive, env.HOMEPATH) + return path.join(homeDrive, path.resolve(env.HOMEPATH)) } return os.homedir() diff --git a/packages/core/src/shared/utilities/pathUtils.ts b/packages/core/src/shared/utilities/pathUtils.ts index c1193c64e9..4768344761 100644 --- a/packages/core/src/shared/utilities/pathUtils.ts +++ b/packages/core/src/shared/utilities/pathUtils.ts @@ -5,6 +5,8 @@ import * as os from 'os' import * as _path from 'path' +import { getLogger } from '../logger' +import { isWeb } from '../extensionGlobals' export const driveLetterRegex = /^[a-zA-Z]\:/ @@ -126,3 +128,24 @@ export function getDriveLetter(path: string): string { return fullpath.substring(0, 1) } + +/** + * Checks if the provided path is valid and exists. + * @todo: Migrate fs.existsSync to fsCommon.exists + * + * @param {string} [path] - The path to be checked. If not provided, defaults to an empty string. + * @returns {boolean} Returns true if the path is valid and exists, otherwise returns false. + */ +export function isValidPath(path?: string): boolean { + if (isWeb()) { + return !!path + } + + const fs = require('fs') + + if (path && path.length !== 0 && fs.existsSync(path)) { + return true + } + getLogger().error('Invalid path %s', path) + return false +} diff --git a/packages/core/src/test/credentials/sharedCredentials.test.ts b/packages/core/src/test/credentials/sharedCredentials.test.ts index 32cab48841..9049f14fe8 100644 --- a/packages/core/src/test/credentials/sharedCredentials.test.ts +++ b/packages/core/src/test/credentials/sharedCredentials.test.ts @@ -6,6 +6,8 @@ import assert from 'assert' import * as path from 'path' import * as fs from 'fs-extra' +import * as sinon from 'sinon' +import * as pathUtils from '../../shared/utilities/pathUtils' import { EnvironmentVariables } from '../../shared/environmentVariables' import { makeTemporaryToolkitFolder } from '../../shared/filesystemUtilities' import { getCredentialsFilename, getConfigFilename } from '../../auth/credentials/sharedCredentialsFile' @@ -17,6 +19,7 @@ describe('sharedCredentials', function () { // Make a temp folder for all these tests // Stick some temp credentials files in there to load from tempFolder = await makeTemporaryToolkitFolder() + sinon.stub(pathUtils, 'isValidPath').returns(true) }) afterEach(async function () { @@ -27,6 +30,7 @@ describe('sharedCredentials', function () { after(async function () { await fs.remove(tempFolder) + sinon.restore() }) describe('getCredentialsFilename', function () { diff --git a/packages/core/src/test/shared/credentials/userCredentialsUtils.test.ts b/packages/core/src/test/shared/credentials/userCredentialsUtils.test.ts index 42b004afd3..5ea9ee2833 100644 --- a/packages/core/src/test/shared/credentials/userCredentialsUtils.test.ts +++ b/packages/core/src/test/shared/credentials/userCredentialsUtils.test.ts @@ -19,17 +19,28 @@ import { import { UserCredentialsUtils } from '../../../shared/credentials/userCredentialsUtils' import { EnvironmentVariables } from '../../../shared/environmentVariables' import { makeTemporaryToolkitFolder } from '../../../shared/filesystemUtilities' +import * as pathUtils from '../../../shared/utilities/pathUtils' +import { getConfigFilename, getCredentialsFilename } from '../../../auth/credentials/sharedCredentialsFile' describe('UserCredentialsUtils', function () { let tempFolder: string + const defaultConfigFileName = getConfigFilename() + const defaultCredentialsFilename = getCredentialsFilename() before(async function () { // Make a temp folder for all these tests // Stick some temp credentials files in there to load from tempFolder = await makeTemporaryToolkitFolder() + sinon.stub(pathUtils, 'isValidPath').returns(true) }) afterEach(async function () { + if (fs.existsSync(defaultConfigFileName)) { + await fs.rm(defaultConfigFileName) + } + if (fs.existsSync(defaultCredentialsFilename)) { + await fs.rm(defaultCredentialsFilename) + } sinon.restore() }) @@ -113,6 +124,7 @@ describe('UserCredentialsUtils', function () { profileName: profileName, secretKey: 'ABC', } + createCredentialsFile(credentialsFilename, [profileName]) await UserCredentialsUtils.generateCredentialsFile(creds) const sharedConfigFiles = await loadSharedConfigFiles({ credentials: Uri.file(credentialsFilename) }) diff --git a/packages/core/src/test/shared/systemUtilities.test.ts b/packages/core/src/test/shared/systemUtilities.test.ts index a06ce2b370..5621c25916 100644 --- a/packages/core/src/test/shared/systemUtilities.test.ts +++ b/packages/core/src/test/shared/systemUtilities.test.ts @@ -8,8 +8,10 @@ import * as vscode from 'vscode' import * as fs from 'fs-extra' import * as os from 'os' import * as path from 'path' +import * as sinon from 'sinon' import * as utils from 'util' +import * as pathUtils from '../../shared/utilities/pathUtils' import { EnvironmentVariables } from '../../shared/environmentVariables' import { makeTemporaryToolkitFolder } from '../../shared/filesystemUtilities' import { SystemUtilities } from '../../shared/systemUtilities' @@ -23,10 +25,12 @@ describe('SystemUtilities', function () { // Make a temp folder for all these tests // Stick some temp credentials files in there to load from tempFolder = await makeTemporaryToolkitFolder() + sinon.stub(pathUtils, 'isValidPath').returns(true) }) after(async function () { await fs.remove(tempFolder) + sinon.restore() }) describe('getHomeDirectory', function () { @@ -34,7 +38,7 @@ describe('SystemUtilities', function () { const env = process.env as EnvironmentVariables env.HOME = 'c:\\qwerty' - assert.strictEqual(SystemUtilities.getHomeDirectory(), 'c:\\qwerty') + assert.strictEqual(SystemUtilities.getHomeDirectory(), path.resolve('c:\\qwerty')) }) it('gets USERPROFILE if set and HOME is not set', async function () { @@ -42,7 +46,7 @@ describe('SystemUtilities', function () { delete env.HOME env.USERPROFILE = 'c:\\profiles\\qwerty' - assert.strictEqual(SystemUtilities.getHomeDirectory(), 'c:\\profiles\\qwerty') + assert.strictEqual(SystemUtilities.getHomeDirectory(), path.resolve('c:\\profiles\\qwerty')) }) it('gets HOMEPATH if set and HOME and USERPROFILE are not set', async function () { @@ -54,7 +58,7 @@ describe('SystemUtilities', function () { env.HOMEPATH = `${path.sep}users${path.sep}homepath` assert.strictEqual( SystemUtilities.getHomeDirectory().toLowerCase(), - `c:${path.sep}users${path.sep}homepath` + path.resolve(`c:${path.sep}users${path.sep}homepath`) ) }) @@ -65,7 +69,10 @@ describe('SystemUtilities', function () { delete env.USERPROFILE env.HOMEPATH = `${path.sep}users${path.sep}homepath` env.HOMEDRIVE = 'x:' - assert.strictEqual(SystemUtilities.getHomeDirectory(), `x:${path.sep}users${path.sep}homepath`) + assert.strictEqual( + SystemUtilities.getHomeDirectory(), + path.resolve(`x:${path.sep}users${path.sep}homepath`) + ) }) it('falls back on os.homedir if no environment variables are set', async function () {