feat(codebuild): prevent using Secrets in plain-text environment vari…

…ables

If you use a Secret in an environment variable of the default type
`BuildEnvironmentVariableType.PLAINTEXT`,
it will be visible in plain text in the AWS Console.
Add validation that checks for this common mistake,
along with a flag that allows you to opt out of it.

packages/@aws-cdk/aws-codebuild/lib/project.ts

@@ -7,7 +7,7 @@ import * as iam from '@aws-cdk/aws-iam';
 import * as kms from '@aws-cdk/aws-kms';
 import * as s3 from '@aws-cdk/aws-s3';
 import * as secretsmanager from '@aws-cdk/aws-secretsmanager';
-import { Aws, Duration, IResource, Lazy, Names, PhysicalName, Resource, Stack } from '@aws-cdk/core';
+import { Aws, Duration, IResource, Lazy, Names, PhysicalName, Resource, SecretValue, Stack, Tokenization } from '@aws-cdk/core';
 import { Construct } from 'constructs';
 import { IArtifacts } from './artifacts';
 import { BuildSpec } from './build-spec';
@@ -465,6 +465,17 @@ export interface CommonProjectProps {
    */
   readonly environmentVariables?: { [name: string]: BuildEnvironmentVariable };
 
+  /**
+   * Whether to check for the presence of any secrets in the environment variables of the default type, BuildEnvironmentVariableType.PLAINTEXT.
+   * Since using a secret for the value of that kind of variable would result in it being displayed in plain text in the AWS Console,
+   * the construct will throw an exception if it detects a secret was passed there.
+   * Pass this property as false if you want to skip this validation,
+   * and keep using a secret in a plain text environment variable.
+   *
+   * @default true
+   */
+  readonly checkSecretsInPlainTextEnvVariables?: boolean;
+
   /**
    * The physical, human-readable name of the CodeBuild Project.
    *
@@ -659,15 +670,39 @@ export class Project extends ProjectBase {
    * which is the representation of environment variables in CloudFormation.
    *
    * @param environmentVariables the map of string to environment variables
+   * @param validateNoPlainTextSecrets whether to throw an exception
+   *   if any of the plain text environment variables contain secrets, defaults to 'false'
    * @returns an array of {@link CfnProject.EnvironmentVariableProperty} instances
    */
-  public static serializeEnvVariables(environmentVariables: { [name: string]: BuildEnvironmentVariable }):
-  CfnProject.EnvironmentVariableProperty[] {
-    return Object.keys(environmentVariables).map(name => ({
-      name,
-      type: environmentVariables[name].type || BuildEnvironmentVariableType.PLAINTEXT,
-      value: environmentVariables[name].value,
-    }));
+  public static serializeEnvVariables(environmentVariables: { [name: string]: BuildEnvironmentVariable },
+    validateNoPlainTextSecrets: boolean = false): CfnProject.EnvironmentVariableProperty[] {
+
+    const ret = new Array<CfnProject.EnvironmentVariableProperty>();
+
+    for (const [name, envVariable] of Object.entries(environmentVariables)) {
+      const cfnEnvVariable: CfnProject.EnvironmentVariableProperty = {
+        name,
+        type: envVariable.type || BuildEnvironmentVariableType.PLAINTEXT,
+        value: envVariable.value?.toString(),
+      };
+      ret.push(cfnEnvVariable);
+
+      // validate that the plain-text environment variables don't contain any secrets in them
+      if (validateNoPlainTextSecrets && cfnEnvVariable.type === BuildEnvironmentVariableType.PLAINTEXT) {
+        const fragments = Tokenization.reverseString(cfnEnvVariable.value);
+        for (const token of fragments.tokens) {
+          if (token instanceof SecretValue) {
+            throw new Error(`Plaintext environment variable '${name}' contains a secret value! ` +
+              'This means the value of this variable will be visible in plain text in the AWS Console. ' +
+              "Please consider using CodeBuild's SecretsManager environment variables feature instead. " +
+              "If you'd like to continue with having this secret in the plaintext environment variables, " +
+              'please set the checkSecretsInPlainTextEnvVariables property to false');
+          }
+        }
+      }
+    }
+
+    return ret;
   }
 
   public readonly grantPrincipal: iam.IPrincipal;
@@ -761,7 +796,7 @@ export class Project extends ProjectBase {
       },
       artifacts: artifactsConfig.artifactsProperty,
       serviceRole: this.role.roleArn,
-      environment: this.renderEnvironment(props.environment, environmentVariables),
+      environment: this.renderEnvironment(props, environmentVariables),
       fileSystemLocations: Lazy.any({ produce: () => this.renderFileSystemLocations() }),
       // lazy, because we have a setter for it in setEncryptionKey
       // The 'alias/aws/s3' default is necessary because leaving the `encryptionKey` field
@@ -974,8 +1009,10 @@ export class Project extends ProjectBase {
   }
 
   private renderEnvironment(
-    env: BuildEnvironment = {},
+    props: ProjectProps,
     projectVars: { [name: string]: BuildEnvironmentVariable } = {}): CfnProject.EnvironmentProperty {
+
+    const env = props.environment ?? {};
     const vars: { [name: string]: BuildEnvironmentVariable } = {};
     const containerVars = env.environmentVariables || {};
 
@@ -1030,7 +1067,9 @@ export class Project extends ProjectBase {
         : undefined,
       privilegedMode: env.privileged || false,
       computeType: env.computeType || this.buildImage.defaultComputeType,
-      environmentVariables: hasEnvironmentVars ? Project.serializeEnvVariables(vars) : undefined,
+      environmentVariables: hasEnvironmentVars
+        ? Project.serializeEnvVariables(vars, props.checkSecretsInPlainTextEnvVariables ?? true)
+        : undefined,
     };
   }
 

packages/@aws-cdk/aws-codebuild/test/test.project.ts

@@ -924,5 +924,40 @@ export = {
 
       test.done();
     },
+
+    'should fail creating when using a secret value in a plaintext variable'(test: Test) {
+      // GIVEN
+      const stack = new cdk.Stack();
+
+      // THEN
+      test.throws(() => {
+        new codebuild.PipelineProject(stack, 'Project', {
+          environmentVariables: {
+            'a': {
+              value: `a_${cdk.SecretValue.secretsManager('my-secret')}_b`,
+            },
+          },
+        });
+      }, /Plaintext environment variable 'a' contains a secret value!/);
+
+      test.done();
+    },
+
+    "should allow opting out of the 'secret value in a plaintext variable' validation"(test: Test) {
+      // GIVEN
+      const stack = new cdk.Stack();
+
+      // THEN
+      new codebuild.PipelineProject(stack, 'Project', {
+        environmentVariables: {
+          'b': {
+            value: cdk.SecretValue.secretsManager('my-secret'),
+          },
+        },
+        checkSecretsInPlainTextEnvVariables: false,
+      });
+
+      test.done();
+    },
   },
 };

packages/@aws-cdk/aws-codepipeline-actions/lib/codebuild/build-action.ts

@@ -78,6 +78,17 @@ export interface CodeBuildActionProps extends codepipeline.CommonAwsActionProps
    */
   readonly environmentVariables?: { [name: string]: codebuild.BuildEnvironmentVariable };
 
+  /**
+   * Whether to check for the presence of any secrets in the environment variables of the default type, BuildEnvironmentVariableType.PLAINTEXT.
+   * Since using a secret for the value of that kind of variable would result in it being displayed in plain text in the AWS Console,
+   * the construct will throw an exception if it detects a secret was passed there.
+   * Pass this property as false if you want to skip this validation,
+   * and keep using a secret in a plain text environment variable.
+   *
+   * @default true
+   */
+  readonly checkSecretsInPlainTextEnvVariables?: boolean;
+
   /**
    * Trigger a batch build.
    *
@@ -177,7 +188,8 @@ export class CodeBuildAction extends Action {
     const configuration: any = {
       ProjectName: this.props.project.projectName,
       EnvironmentVariables: this.props.environmentVariables &&
-        cdk.Stack.of(scope).toJsonString(codebuild.Project.serializeEnvVariables(this.props.environmentVariables)),
+        cdk.Stack.of(scope).toJsonString(codebuild.Project.serializeEnvVariables(this.props.environmentVariables,
+          this.props.checkSecretsInPlainTextEnvVariables ?? true)),
     };
     if ((this.actionProperties.inputs || []).length > 1) {
       // lazy, because the Artifact name might be generated lazily

packages/@aws-cdk/aws-codepipeline-actions/test/codebuild/test.codebuild-action.ts

@@ -4,7 +4,7 @@ import * as codecommit from '@aws-cdk/aws-codecommit';
 import * as codepipeline from '@aws-cdk/aws-codepipeline';
 import * as s3 from '@aws-cdk/aws-s3';
 import * as sns from '@aws-cdk/aws-sns';
-import { App, Stack } from '@aws-cdk/core';
+import { App, SecretValue, Stack } from '@aws-cdk/core';
 import { Test } from 'nodeunit';
 import * as cpactions from '../../lib';
 
@@ -257,5 +257,83 @@ export = {
 
       test.done();
     },
+
+    'environment variables': {
+      'should fail by default when added to a Pipeline while using a secret value in a plaintext variable'(test: Test) {
+        const stack = new Stack();
+
+        const sourceOutput = new codepipeline.Artifact();
+        const pipeline = new codepipeline.Pipeline(stack, 'Pipeline', {
+          stages: [
+            {
+              stageName: 'Source',
+              actions: [new cpactions.CodeCommitSourceAction({
+                actionName: 'source',
+                repository: new codecommit.Repository(stack, 'CodeCommitRepo', {
+                  repositoryName: 'my-repo',
+                }),
+                output: sourceOutput,
+              })],
+            },
+          ],
+        });
+
+        const buildStage = pipeline.addStage({
+          stageName: 'Build',
+        });
+        const codeBuildProject = new codebuild.PipelineProject(stack, 'CodeBuild');
+        const buildAction = new cpactions.CodeBuildAction({
+          actionName: 'Build',
+          project: codeBuildProject,
+          input: sourceOutput,
+          environmentVariables: {
+            'X': {
+              value: SecretValue.secretsManager('my-secret'),
+            },
+          },
+        });
+
+        test.throws(() => {
+          buildStage.addAction(buildAction);
+        }, /Plaintext environment variable 'X' contains a secret value!/);
+        test.done();
+      },
+
+      "should allow opting out of the 'secret value in a plaintext variable' validation"(test: Test) {
+        const stack = new Stack();
+
+        const sourceOutput = new codepipeline.Artifact();
+        new codepipeline.Pipeline(stack, 'Pipeline', {
+          stages: [
+            {
+              stageName: 'Source',
+              actions: [new cpactions.CodeCommitSourceAction({
+                actionName: 'source',
+                repository: new codecommit.Repository(stack, 'CodeCommitRepo', {
+                  repositoryName: 'my-repo',
+                }),
+                output: sourceOutput,
+              })],
+            },
+            {
+              stageName: 'Build',
+              actions: [new cpactions.CodeBuildAction({
+                actionName: 'build',
+                project: new codebuild.PipelineProject(stack, 'CodeBuild'),
+                input: sourceOutput,
+                environmentVariables: {
+                  'X': {
+                    value: SecretValue.secretsManager('my-secret'),
+                  },
+                },
+                checkSecretsInPlainTextEnvVariables: false,
+              })],
+            },
+          ],
+        });
+
+        test.done();
+      },
+    },
   },
 };