Validate SAM CLI on detect (#1465)

bryceitoc9 · web-flow · commit e7df1c639798 · 2021-01-19T16:07:26.000-08:00
SAM CLI now validates detected executables. This prevents non-SAM executables that match the SAM CLI's signature from accidentally getting picked up if it comes first in the user's PATH.
diff --git a/src/shared/sam/cli/samCliBuild.ts b/src/shared/sam/cli/samCliBuild.ts
@@ -6,7 +6,6 @@
 import { fileExists } from '../../filesystemUtilities'
 import { getLogger, Logger } from '../../logger'
 import { logAndThrowIfUnexpectedExitCode, SamCliProcessInvoker } from './samCliInvokerUtils'
-import { DefaultSamCliProcessInvoker } from './samCliInvoker'
 import { pushIf } from '../../utilities/collectionUtils'
 import { localize } from '../../utilities/vsCodeUtils'
 
@@ -75,7 +74,6 @@ export class SamCliBuildInvocation {
         private readonly args: SamCliBuildInvocationArguments,
         private readonly context: { file: FileFunctions } = { file: getDefaultFileFunctions() }
     ) {
-        this.args.invoker = this.args.invoker ?? new DefaultSamCliProcessInvoker()
         this.args.useContainer = !!this.args.useContainer
         this.args.skipPullImage = !!this.args.skipPullImage
     }
diff --git a/src/shared/sam/cli/samCliConfiguration.ts b/src/shared/sam/cli/samCliConfiguration.ts
@@ -5,7 +5,6 @@
 
 import * as vscode from 'vscode'
 import { SettingsConfiguration } from '../../settingsConfiguration'
-import { SamCliLocationProvider } from './samCliLocator'
 
 export interface SamCliConfiguration {
     /** Gets the current SAM CLI location from the VSCode settings store. */
@@ -34,9 +33,12 @@ export interface SamCliConfiguration {
 export class DefaultSamCliConfiguration implements SamCliConfiguration {
     public static readonly CONFIGURATION_KEY_SAMCLI_LOCATION: string = 'samcli.location'
     private readonly _configuration: SettingsConfiguration
-    private readonly _samCliLocationProvider: SamCliLocationProvider
+    private readonly _samCliLocationProvider: { getLocation(): Promise<string | undefined> }
 
-    public constructor(configuration: SettingsConfiguration, samCliLocationProvider: SamCliLocationProvider) {
+    public constructor(
+        configuration: SettingsConfiguration,
+        samCliLocationProvider: { getLocation(): Promise<string | undefined> }
+    ) {
         this._configuration = configuration
         this._samCliLocationProvider = samCliLocationProvider
     }
diff --git a/src/shared/sam/cli/samCliContext.ts b/src/shared/sam/cli/samCliContext.ts
@@ -5,7 +5,7 @@
 
 import { SettingsConfiguration } from '../../settingsConfiguration'
 import { DefaultSamCliConfiguration } from './samCliConfiguration'
-import { DefaultSamCliProcessInvoker, SamCliProcessInvokerContext } from './samCliInvoker'
+import { DefaultSamCliProcessInvoker } from './samCliInvoker'
 import { SamCliProcessInvoker } from './samCliInvokerUtils'
 import { DefaultSamCliLocationProvider } from './samCliLocator'
 import { throwAndNotifyIfInvalid } from './samCliValidationUtils'
@@ -58,13 +58,9 @@ function makeSamCliContext(): SamCliContext {
         settingsConfiguration,
         new DefaultSamCliLocationProvider()
     )
+    const invoker = new DefaultSamCliProcessInvoker({ preloadedConfig: samCliConfiguration })
 
-    const invokerContext: SamCliProcessInvokerContext = {
-        cliConfig: samCliConfiguration,
-    }
-    const invoker = new DefaultSamCliProcessInvoker(invokerContext)
-
-    const validatorContext = new DefaultSamCliValidatorContext(samCliConfiguration, invoker)
+    const validatorContext = new DefaultSamCliValidatorContext(samCliConfiguration)
     const validator = new DefaultSamCliValidator(validatorContext)
 
     const context: SamCliContext = {
diff --git a/src/shared/sam/cli/samCliInfo.ts b/src/shared/sam/cli/samCliInfo.ts
@@ -4,6 +4,7 @@
  */
 
 import { getLogger, Logger } from '../../logger'
+import { SamCliConfiguration } from './samCliConfiguration'
 import { DefaultSamCliProcessInvoker } from './samCliInvoker'
 import { logAndThrowIfUnexpectedExitCode, SamCliProcessInvoker } from './samCliInvokerUtils'
 
@@ -15,7 +16,29 @@ export interface SamCliInfoResponse {
 }
 
 export class SamCliInfoInvocation {
-    public constructor(private readonly invoker: SamCliProcessInvoker = new DefaultSamCliProcessInvoker()) {}
+    private readonly invoker: SamCliProcessInvoker
+    public constructor(params: {
+        invoker?: SamCliProcessInvoker
+        preloadedConfig?: SamCliConfiguration
+        locationProvider?: { getLocation(): Promise<string | undefined> }
+    }) {
+        if (
+            (params.invoker && params.preloadedConfig) ||
+            (params.invoker && params.locationProvider) ||
+            (params.preloadedConfig && params.locationProvider)
+        ) {
+            throw new Error('Invalid constructor args for SamCliInfoInvocation')
+        }
+        if (params.invoker) {
+            this.invoker = params.invoker
+        } else if (params.preloadedConfig) {
+            this.invoker = new DefaultSamCliProcessInvoker({ preloadedConfig: params.preloadedConfig })
+        } else if (params.locationProvider) {
+            this.invoker = new DefaultSamCliProcessInvoker({ locationProvider: params.locationProvider })
+        } else {
+            throw new Error('Invalid constructor args for SamCliInfoInvocation')
+        }
+    }
 
     public async execute(): Promise<SamCliInfoResponse> {
         const childProcessResult = await this.invoker.invoke({ arguments: ['--info'] })
diff --git a/src/shared/sam/cli/samCliInvoker.ts b/src/shared/sam/cli/samCliInvoker.ts
@@ -3,50 +3,46 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import { extensionSettingsPrefix } from '../../constants'
 import { getLogger } from '../../logger'
-import { DefaultSettingsConfiguration } from '../../settingsConfiguration'
 import { ChildProcess, ChildProcessResult } from '../../utilities/childProcess'
-import { DefaultSamCliConfiguration, SamCliConfiguration } from './samCliConfiguration'
 import {
     makeRequiredSamCliProcessInvokeOptions,
     SamCliProcessInvokeOptions,
     SamCliProcessInvoker,
 } from './samCliInvokerUtils'
-import { DefaultSamCliLocationProvider } from './samCliLocator'
 
 import * as nls from 'vscode-nls'
+import { DefaultSamCliConfiguration, SamCliConfiguration } from './samCliConfiguration'
+import { DefaultSettingsConfiguration } from '../../settingsConfiguration'
+import { extensionSettingsPrefix } from '../../constants'
 const localize = nls.loadMessageBundle()
 
-export interface SamCliProcessInvokerContext {
-    cliConfig: SamCliConfiguration
-}
-
-export class DefaultSamCliProcessInvokerContext implements SamCliProcessInvokerContext {
-    public cliConfig: SamCliConfiguration = new DefaultSamCliConfiguration(
-        new DefaultSettingsConfiguration(extensionSettingsPrefix),
-        new DefaultSamCliLocationProvider()
-    )
-}
-
-export function resolveSamCliProcessInvokerContext(
-    params: Partial<SamCliProcessInvokerContext> = {}
-): SamCliProcessInvokerContext {
-    const defaults = new DefaultSamCliProcessInvokerContext()
-
-    return {
-        cliConfig: params.cliConfig || defaults.cliConfig,
-    }
-}
-
 /**
  * Yet another `sam` CLI wrapper.
  *
  * TODO: Merge this with `DefaultSamLocalInvokeCommand`.
  */
 export class DefaultSamCliProcessInvoker implements SamCliProcessInvoker {
     private childProcess?: ChildProcess
-    public constructor(private readonly context: SamCliProcessInvokerContext = resolveSamCliProcessInvokerContext()) {}
+    private readonly context: SamCliConfiguration
+    public constructor(params: {
+        preloadedConfig?: SamCliConfiguration
+        locationProvider?: { getLocation(): Promise<string | undefined> }
+    }) {
+        if (params.preloadedConfig && params.locationProvider) {
+            throw new Error('Invalid constructor args for DefaultSamCliProcessInvoker')
+        }
+        if (params.preloadedConfig) {
+            this.context = params.preloadedConfig
+        } else if (params.locationProvider) {
+            this.context = new DefaultSamCliConfiguration(
+                new DefaultSettingsConfiguration(extensionSettingsPrefix),
+                params.locationProvider
+            )
+        } else {
+            throw new Error('Invalid constructor args for DefaultSamCliProcessInvoker')
+        }
+    }
 
     public stop(): void {
         if (!this.childProcess) {
@@ -59,7 +55,7 @@ export class DefaultSamCliProcessInvoker implements SamCliProcessInvoker {
         const invokeOptions = makeRequiredSamCliProcessInvokeOptions(options)
         const logger = getLogger()
 
-        const sam = await this.context.cliConfig.getOrDetectSamCli()
+        const sam = await this.context.getOrDetectSamCli()
         if (!sam.path) {
             logger.warn('SAM CLI not found and not configured')
         } else if (sam.autoDetected) {
diff --git a/src/shared/sam/cli/samCliLocalInvoke.ts b/src/shared/sam/cli/samCliLocalInvoke.ts
@@ -10,9 +10,9 @@ import { fileExists } from '../../filesystemUtilities'
 import { getLogger, Logger } from '../../logger'
 import { ChildProcess } from '../../utilities/childProcess'
 import { Timeout } from '../../utilities/timeoutUtils'
-import { DefaultSamCliProcessInvokerContext, SamCliProcessInvokerContext } from './samCliInvoker'
 import { removeAnsi } from '../../utilities/textUtilities'
 import { ext } from '../../extensionGlobals'
+import { DefaultSamCliProcessInvokerContext, SamCliProcessInvokerContext } from './samCliProcessInvokerContext'
 
 const localize = nls.loadMessageBundle()
 
diff --git a/src/shared/sam/cli/samCliLocator.ts b/src/shared/sam/cli/samCliLocator.ts
@@ -7,6 +7,8 @@ import * as path from 'path'
 import { EnvironmentVariables } from '../../environmentVariables'
 import * as filesystemUtilities from '../../filesystemUtilities'
 import { getLogger, Logger } from '../../logger'
+import { SamCliInfoInvocation } from './samCliInfo'
+import { DefaultSamCliValidator, SamCliValidatorContext, SamCliVersionValidation } from './samCliValidator'
 
 export interface SamCliLocationProvider {
     getLocation(): Promise<string | undefined>
@@ -69,8 +71,29 @@ abstract class BaseSamCliLocator {
 
         for (const fullPath of fullPaths) {
             this.logger.verbose(`Searching for SAM CLI in: ${fullPath}`)
+            const context: SamCliValidatorContext = {
+                samCliLocation: async () => fullPath,
+                getSamCliExecutableId: async () => 'bogus',
+                getSamCliInfo: async () => {
+                    const samCliInfo = new SamCliInfoInvocation({
+                        locationProvider: { getLocation: async () => fullPath },
+                    })
+
+                    return await samCliInfo.execute()
+                },
+            }
+            const validator = new DefaultSamCliValidator(context)
             if (await filesystemUtilities.fileExists(fullPath)) {
-                return fullPath
+                try {
+                    const validationResult = await validator.getVersionValidatorResult()
+                    if (validationResult.validation === SamCliVersionValidation.Valid) {
+                        return fullPath
+                    }
+                    this.logger.info(`Found invalid SAM executable (${validationResult.validation}): ${fullPath}`)
+                } catch (e) {
+                    const err = e as Error
+                    this.logger.error(err)
+                }
             }
         }
 
diff --git a/src/shared/sam/cli/samCliProcessInvokerContext.ts b/src/shared/sam/cli/samCliProcessInvokerContext.ts
@@ -0,0 +1,20 @@
+/*!
+ * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { extensionSettingsPrefix } from '../../constants'
+import { DefaultSettingsConfiguration } from '../../settingsConfiguration'
+import { DefaultSamCliConfiguration, SamCliConfiguration } from './samCliConfiguration'
+import { DefaultSamCliLocationProvider } from './samCliLocator'
+
+export interface SamCliProcessInvokerContext {
+    cliConfig: SamCliConfiguration
+}
+
+export class DefaultSamCliProcessInvokerContext implements SamCliProcessInvokerContext {
+    public cliConfig: SamCliConfiguration = new DefaultSamCliConfiguration(
+        new DefaultSettingsConfiguration(extensionSettingsPrefix),
+        new DefaultSamCliLocationProvider()
+    )
+}
diff --git a/src/shared/sam/cli/samCliValidator.ts b/src/shared/sam/cli/samCliValidator.ts
@@ -7,7 +7,6 @@ import { stat } from 'fs-extra'
 import * as semver from 'semver'
 import { SamCliConfiguration } from './samCliConfiguration'
 import { SamCliInfoInvocation, SamCliInfoResponse } from './samCliInfo'
-import { SamCliProcessInvoker } from './samCliInvokerUtils'
 
 export const MINIMUM_SAM_CLI_VERSION_INCLUSIVE = '0.47.0'
 export const MINIMUM_SAM_CLI_VERSION_INCLUSIVE_FOR_IMAGE_SUPPORT = '1.13.0'
@@ -129,10 +128,7 @@ export class DefaultSamCliValidator implements SamCliValidator {
 }
 
 export class DefaultSamCliValidatorContext implements SamCliValidatorContext {
-    public constructor(
-        private readonly samCliConfiguration: SamCliConfiguration,
-        private readonly invoker: SamCliProcessInvoker
-    ) {}
+    public constructor(private readonly samCliConfiguration: SamCliConfiguration) {}
 
     public async samCliLocation(): Promise<string> {
         return (await this.samCliConfiguration.getOrDetectSamCli()).path
@@ -151,7 +147,7 @@ export class DefaultSamCliValidatorContext implements SamCliValidatorContext {
     }
 
     public async getSamCliInfo(): Promise<SamCliInfoResponse> {
-        const samCliInfo = new SamCliInfoInvocation(this.invoker)
+        const samCliInfo = new SamCliInfoInvocation({ preloadedConfig: this.samCliConfiguration })
 
         return await samCliInfo.execute()
     }
diff --git a/src/shared/sam/localLambdaRunner.ts b/src/shared/sam/localLambdaRunner.ts
@@ -13,7 +13,7 @@ import { getTemplate, getTemplateResource, isImageLambdaConfig } from '../../lam
 import { getFamily, RuntimeFamily } from '../../lambda/models/samLambdaRuntime'
 import { ExtContext } from '../extensions'
 import { getLogger } from '../logger'
-import { SettingsConfiguration } from '../settingsConfiguration'
+import { DefaultSettingsConfiguration, SettingsConfiguration } from '../settingsConfiguration'
 import * as telemetry from '../telemetry/telemetry'
 import { SamTemplateGenerator } from '../templates/sam/samTemplateGenerator'
 import * as pathutil from '../utilities/pathUtils'
@@ -25,10 +25,14 @@ import { SamCliLocalInvokeInvocation, SamCliLocalInvokeInvocationArguments } fro
 import { SamLaunchRequestArgs } from './debugger/awsSamDebugger'
 import { asEnvironmentVariables } from '../../credentials/credentialsUtilities'
 import { buildSamCliStartApiArguments } from './cli/samCliStartApi'
-import { DefaultSamCliProcessInvoker, DefaultSamCliProcessInvokerContext } from './cli/samCliInvoker'
+import { DefaultSamCliProcessInvoker } from './cli/samCliInvoker'
 import { APIGatewayProperties } from './debugger/awsSamDebugConfiguration.gen'
 import { ChildProcess } from '../utilities/childProcess'
 import { ext } from '../extensionGlobals'
+import { DefaultSamCliProcessInvokerContext } from './cli/samCliProcessInvokerContext'
+import { DefaultSamCliConfiguration } from './cli/samCliConfiguration'
+import { extensionSettingsPrefix } from '../constants'
+import { DefaultSamCliLocationProvider } from './cli/samCliLocator'
 
 const localize = nls.loadMessageBundle()
 
@@ -165,7 +169,12 @@ export async function invokeLambdaFunction(
         )
     }
 
-    const processInvoker = new DefaultSamCliProcessInvoker()
+    const processInvoker = new DefaultSamCliProcessInvoker({
+        preloadedConfig: new DefaultSamCliConfiguration(
+            new DefaultSettingsConfiguration(extensionSettingsPrefix),
+            new DefaultSamCliLocationProvider()
+        ),
+    })
 
     getLogger('channel').info(localize('AWS.output.building.sam.application', 'Building SAM application...'))
     const samBuildOutputFolder = path.join(config.baseBuildDir!, 'output')
diff --git a/src/test/shared/sam/cli/samCliInfo.test.ts b/src/test/shared/sam/cli/samCliInfo.test.ts
diff --git a/src/test/shared/sam/cli/samCliInvoker.test.ts b/src/test/shared/sam/cli/samCliInvoker.test.ts
