From 279ab195c67f3bec08279774d5e72d99d3f2c843 Mon Sep 17 00:00:00 2001
From: Rico Huijbers <huijbers@amazon.com>
Date: Thu, 21 Mar 2019 10:01:48 +0100
Subject: [PATCH] fix(secretsmanager/ssm): verify presence of parameter name

Throw an error if Secrets or SSM Parameter are referenced with an empty
name. This adds clear messaging around an otherwise obscure
CloudFormation error.
---
 .../aws-secretsmanager/lib/secret-string.ts        |  6 ++++++
 .../aws-secretsmanager/test/test.secret-string.ts  | 14 ++++++++++++++
 .../@aws-cdk/aws-ssm/lib/parameter-store-string.ts | 12 ++++++++++++
 .../aws-ssm/test/test.parameter-store-string.ts    | 14 ++++++++++++++
 4 files changed, 46 insertions(+)

diff --git a/packages/@aws-cdk/aws-secretsmanager/lib/secret-string.ts b/packages/@aws-cdk/aws-secretsmanager/lib/secret-string.ts
index 1ece79d6c3bc2..06ce2097e28ef 100644
--- a/packages/@aws-cdk/aws-secretsmanager/lib/secret-string.ts
+++ b/packages/@aws-cdk/aws-secretsmanager/lib/secret-string.ts
@@ -39,6 +39,12 @@ export class SecretString extends cdk.DynamicReference {
       service: cdk.DynamicReferenceService.SecretsManager,
       referenceKey: '',
     });
+
+    // If we don't validate this here it will lead to a very unclear
+    // error message in CloudFormation, so better do it.
+    if (!props.secretId) {
+      throw new Error('SecretString: secretId cannot be empty');
+    }
   }
 
   /**
diff --git a/packages/@aws-cdk/aws-secretsmanager/test/test.secret-string.ts b/packages/@aws-cdk/aws-secretsmanager/test/test.secret-string.ts
index 51f2677b41a20..acb2087bb74aa 100644
--- a/packages/@aws-cdk/aws-secretsmanager/test/test.secret-string.ts
+++ b/packages/@aws-cdk/aws-secretsmanager/test/test.secret-string.ts
@@ -32,4 +32,18 @@ export = {
 
     test.done();
   },
+
+  'empty secretId will throw'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+
+    // WHEN
+    test.throws(() => {
+      new secretsmanager.SecretString(stack, 'Ref', {
+        secretId: '',
+      });
+    }, /secretId cannot be empty/);
+
+    test.done();
+  },
 };
diff --git a/packages/@aws-cdk/aws-ssm/lib/parameter-store-string.ts b/packages/@aws-cdk/aws-ssm/lib/parameter-store-string.ts
index d945cef0cc2d2..c3115dcf0a0c9 100644
--- a/packages/@aws-cdk/aws-ssm/lib/parameter-store-string.ts
+++ b/packages/@aws-cdk/aws-ssm/lib/parameter-store-string.ts
@@ -28,6 +28,12 @@ export class ParameterStoreString extends cdk.Construct {
   constructor(scope: cdk.Construct, id: string, props: ParameterStoreStringProps) {
     super(scope, id);
 
+    // If we don't validate this here it will lead to a very unclear
+    // error message in CloudFormation, so better do it.
+    if (!props.parameterName) {
+      throw new Error('ParameterStoreString: parameterName cannot be empty');
+    }
+
     // We use a different inner construct depend on whether we want the latest
     // or a specific version.
     //
@@ -80,5 +86,11 @@ export class ParameterStoreSecureString extends cdk.DynamicReference {
       service: cdk.DynamicReferenceService.SsmSecure,
       referenceKey: `${props.parameterName}:${props.version}`,
     });
+
+    // If we don't validate this here it will lead to a very unclear
+    // error message in CloudFormation, so better do it.
+    if (!props.parameterName) {
+      throw new Error('ParameterStoreSecureString: parameterName cannot be empty');
+    }
   }
 }
diff --git a/packages/@aws-cdk/aws-ssm/test/test.parameter-store-string.ts b/packages/@aws-cdk/aws-ssm/test/test.parameter-store-string.ts
index 7d2d5e1a9c9c8..fd226f7ad8b81 100644
--- a/packages/@aws-cdk/aws-ssm/test/test.parameter-store-string.ts
+++ b/packages/@aws-cdk/aws-ssm/test/test.parameter-store-string.ts
@@ -59,4 +59,18 @@ export = {
 
     test.done();
   },
+
+  'empty parameterName will throw'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+
+    // WHEN
+    test.throws(() => {
+      new ssm.ParameterStoreString(stack, 'Ref', {
+        parameterName: '',
+      });
+    }, /parameterName cannot be empty/);
+
+    test.done();
+  },
 };
\ No newline at end of file