test(ec2): add security related unit tests (#5778)

## Problem
We want unit tests to ensure certain security measures are maintained in
the code. Properties tested in this PR include:
1. create inline policy with minimal actions necessary to perform
desired action
2. prompt user to confirm attached role and allow cancellation. 
3. test run the ssh agent before using it for remote connections. 

## Solution
For each test we do the following:

1. assert that the actions we are adding to the policy remains as the
core 6.
2. use `getTestWindow()` to ensure user's are able to see policies
added, and cancel the process.
3. spy on `tryRun` to ensure it is used before returning the ssh path. 

As part of (3), we refactor the pathfinding code to be contained in its
own class. This makes the code easier to test since we can now use
`sinon.spy` and `sinon.stub`. It also allows us to move some of the
shared logic into one place.

---

<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

---------

Co-authored-by: Justin M. Keyes <[email protected]>

Author: Hweinstock

packages/core/src/awsService/ec2/sshKeyPair.ts

@@ -45,11 +45,11 @@ export class SshKeyPair {
 
     public static async generateSshKeyPair(keyPath: string): Promise<void> {
         const keyGenerated = await SshKeyPair.tryKeyTypes(keyPath, ['ed25519', 'rsa'])
-        await SshKeyPair.assertGenerated(keyPath, keyGenerated)
         // Should already be the case, but just in case we assert permissions.
         // skip on Windows since it only allows write permission to be changed.
         if (!globals.isWeb && os.platform() !== 'win32') {
             await fs.chmod(keyPath, 0o600)
+            await SshKeyPair.assertGenerated(keyPath, keyGenerated)
         }
     }
     /**

packages/core/src/codewhisperer/commands/basicCommands.ts

@@ -41,8 +41,9 @@ import { ToolkitError, getTelemetryReason, getTelemetryReasonDesc } from '../../
 import { isRemoteWorkspace } from '../../shared/vscode/env'
 import { isBuilderIdConnection } from '../../auth/connection'
 import globals from '../../shared/extensionGlobals'
-import { getVscodeCliPath, tryRun } from '../../shared/utilities/pathFind'
+import { getVscodeCliPath } from '../../shared/utilities/pathFind'
 import { setContext } from '../../shared/vscode/setContext'
+import { tryRun } from '../../shared/utilities/pathFind'
 
 const MessageTimeOut = 5_000
 

packages/core/src/shared/remoteSession.ts

@@ -30,7 +30,7 @@ export interface MissingTool {
     readonly reason?: string
 }
 
-const minimumSsmActions = [
+export const minimumSsmActions = [
     'ssmmessages:CreateControlChannel',
     'ssmmessages:CreateDataChannel',
     'ssmmessages:OpenControlChannel',

packages/core/src/shared/utilities/pathFind.ts

@@ -115,8 +115,8 @@ export async function findTypescriptCompiler(): Promise<string | undefined> {
  * Gets the configured `ssh` path, or falls back to "ssh" (not absolute),
  * or tries common locations, or returns undefined.
  */
-export async function findSshPath(): Promise<string | undefined> {
-    if (sshPath !== undefined) {
+export async function findSshPath(useCache: boolean = true): Promise<string | undefined> {
+    if (useCache && sshPath !== undefined) {
         return sshPath
     }
 
@@ -133,7 +133,7 @@ export async function findSshPath(): Promise<string | undefined> {
             continue
         }
         if (await tryRun(p, ['-G', 'x'], 'noresult' /* "ssh -G" prints quasi-sensitive info. */)) {
-            sshPath = p
+            sshPath = useCache ? p : sshPath
             return p
         }
     }

packages/core/src/test/shared/remoteSession.test.ts

@@ -0,0 +1,32 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import assert from 'assert'
+import { minimumSsmActions, promptToAddInlinePolicy } from '../../shared/remoteSession'
+import { IamClient } from '../../shared/clients/iamClient'
+import { getTestWindow } from './vscode/window'
+import { cancel } from '../../shared'
+
+describe('minimumSsmActions', function () {
+    it('should contain minimal actions needed for ssm connection', function () {
+        assert.deepStrictEqual(minimumSsmActions, [
+            'ssmmessages:CreateControlChannel',
+            'ssmmessages:CreateDataChannel',
+            'ssmmessages:OpenControlChannel',
+            'ssmmessages:OpenDataChannel',
+            'ssm:DescribeAssociation',
+            'ssm:ListAssociations',
+            'ssm:UpdateInstanceInformation',
+        ])
+    })
+
+    it('prompts the user for confirmation before adding policies and allow cancels', async function () {
+        getTestWindow().onDidShowMessage((message) => {
+            assert.ok(message.message.includes('add'), 'should prompt to add policies')
+            getTestWindow().getFirstMessage().selectItem(cancel)
+        })
+        const added = await promptToAddInlinePolicy({} as IamClient, 'roleArnTest')
+        assert.ok(!added, 'should not add policies by default')
+    })
+})

packages/core/src/test/shared/utilities/pathFind.test.ts

@@ -7,16 +7,15 @@ import assert from 'assert'
 import * as vscode from 'vscode'
 import * as os from 'os'
 import * as path from 'path'
-
 import * as testutil from '../../testUtil'
-import { findTypescriptCompiler, getVscodeCliPath } from '../../../shared/utilities/pathFind'
 import { fs } from '../../../shared'
+import { findSshPath, findTypescriptCompiler, getVscodeCliPath, tryRun } from '../../../shared/utilities/pathFind'
+import { isCI, isWin } from '../../../shared/vscode/env'
 
 describe('pathFind', function () {
     it('findTypescriptCompiler()', async function () {
-        const iswin = process.platform === 'win32'
         const workspace = vscode.workspace.workspaceFolders![0]
-        const tscNodemodules = path.join(workspace.uri.fsPath, `foo/bar/node_modules/.bin/tsc${iswin ? '.cmd' : ''}`)
+        const tscNodemodules = path.join(workspace.uri.fsPath, `foo/bar/node_modules/.bin/tsc${isWin() ? '.cmd' : ''}`)
         await fs.delete(tscNodemodules, { force: true })
 
         // The test workspace normally doesn't have node_modules so this will
@@ -42,4 +41,90 @@ describe('pathFind', function () {
         const regex = /bin[\\\/](code|code-insiders)$/
         assert.ok(regex.test(vscPath), `expected regex ${regex} to match: "${vscPath}"`)
     })
+
+    describe('findSshPath', function () {
+        let previousPath: string | undefined
+
+        beforeEach(function () {
+            previousPath = process.env.PATH
+        })
+
+        afterEach(function () {
+            process.env.PATH = previousPath
+        })
+
+        it('first tries ssh in $PATH (Non-Windows)', async function () {
+            // skip on windows because ssh in path will never work with .exe extension.
+            if (isWin()) {
+                return
+            }
+            const workspace = await testutil.createTestWorkspaceFolder()
+            const fakeSshPath = path.join(workspace.uri.fsPath, `ssh`)
+
+            process.env.PATH = workspace.uri.fsPath
+            const firstResult = await findSshPath(false)
+
+            await testutil.createExecutableFile(fakeSshPath, '')
+
+            const secondResult = await findSshPath(false)
+
+            assert.notStrictEqual(firstResult, secondResult)
+            assert.strictEqual(secondResult, 'ssh')
+        })
+
+        it('only returns valid executable ssh path (Non-Windows)', async function () {
+            if (isWin()) {
+                return
+            }
+            // On non-windows, we can overwrite path and create our own executable to find.
+            const workspace = await testutil.createTestWorkspaceFolder()
+            const fakeSshPath = path.join(workspace.uri.fsPath, `ssh`)
+
+            process.env.PATH = workspace.uri.fsPath
+
+            await testutil.createExecutableFile(fakeSshPath, '')
+
+            const ssh = await findSshPath(false)
+            assert.ok(ssh)
+            const result = await tryRun(ssh, [], 'yes')
+            assert.ok(result)
+        })
+
+        it('caches result from previous runs (Non-Windows)', async function () {
+            if (isWin()) {
+                return
+            }
+            // On non-windows, we can overwrite path and create our own executable to find.
+            const workspace = await testutil.createTestWorkspaceFolder()
+            // We move the ssh to a temp directory temporarily to test if cache works.
+            const fakeSshPath = path.join(workspace.uri.fsPath, `ssh`)
+
+            process.env.PATH = workspace.uri.fsPath
+
+            await testutil.createExecutableFile(fakeSshPath, '')
+
+            const ssh1 = (await findSshPath(true))!
+
+            await fs.delete(fakeSshPath)
+
+            const ssh2 = await findSshPath(true)
+
+            assert.strictEqual(ssh1, ssh2)
+        })
+
+        it('finds valid executable path (Windows CI)', async function () {
+            // Don't want to be messing with System32 on peoples local machines.
+            if (!isWin() || !isCI()) {
+                return
+            }
+            const expectedPathInCI = 'C:/Windows/System32/OpenSSH/ssh.exe'
+
+            if (!(await fs.exists(expectedPathInCI))) {
+                await testutil.createExecutableFile(expectedPathInCI, '')
+            }
+            const ssh = (await findSshPath(true))!
+            const result = await tryRun(ssh, ['-G', 'x'], 'noresult')
+            assert.ok(result)
+        })
+    })
 })

packages/core/src/test/shared/utilities/testUtils.test.ts

@@ -0,0 +1,39 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import assert from 'assert'
+import { createExecutableFile, createTestWorkspaceFolder, copyEnv } from '../../testUtil'
+import path from 'path'
+import { fs } from '../../../shared'
+import { isWin } from '../../../shared/vscode/env'
+import { tryRun } from '../../../shared/utilities/pathFind'
+
+describe('copyEnv', function () {
+    it('modifies the node environment variables (Non-Windows)', function () {
+        // PATH returns undefined on Windows.
+        if (isWin()) {
+            this.skip()
+        }
+
+        const originalPath = copyEnv().PATH
+        const fakePath = 'fakePath'
+        process.env.PATH = fakePath
+        assert.strictEqual(copyEnv().PATH, fakePath)
+
+        process.env.PATH = originalPath
+        assert.strictEqual(copyEnv().PATH, originalPath)
+    })
+})
+
+describe('createExecutableFile', function () {
+    it('creates a file that can be executed', async function () {
+        const tempDir = await createTestWorkspaceFolder()
+        const filePath = path.join(tempDir.uri.fsPath, `exec${isWin() ? '.cmd' : ''}`)
+        await createExecutableFile(filePath, '')
+
+        const result = await tryRun(filePath, [], 'yes')
+        assert.ok(result)
+        await fs.delete(tempDir.uri, { force: true, recursive: true })
+    })
+})

packages/core/src/test/testUtil.ts

@@ -630,3 +630,7 @@ export function tryRegister(command: DeclaredCommand<() => Promise<any>>) {
 export function getFetchStubWithResponse(response: Partial<Response>) {
     return stub(request, 'fetch').returns({ response: new Promise((res, _) => res(response)) } as any)
 }
+
+export function copyEnv(): NodeJS.ProcessEnv {
+    return { ...process.env }
+}