From 833767c240a88961cb547cd005cc57a51f70d01e Mon Sep 17 00:00:00 2001
From: Nick Lynch <nlynch@amazon.com>
Date: Thu, 7 Oct 2021 13:34:51 +0100
Subject: [PATCH] fix(cloudfront): EdgeFunctions cannot be created when IDs
 contain spaces (#16845)

The EdgeFunction uses a SSM string parameter under the hood to pass the Function
ARN between the different regions. The name of the parameter is derived from the
node path; this path may contain characters (e.g., spaces) that are invalid as
SSM parameter names.

Two fixes here: introduce new validation for SSM parameter names, and sanitize
the path prior to passing to SSM.

fixes #16832


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
---
 .../lib/experimental/edge-function.ts         |  4 ++-
 .../test/experimental/edge-function.test.ts   | 10 ++++++
 packages/@aws-cdk/aws-ssm/lib/parameter.ts    | 18 +++++++----
 .../@aws-cdk/aws-ssm/test/parameter.test.ts   | 32 +++++++++++++++++--
 4 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/packages/@aws-cdk/aws-cloudfront/lib/experimental/edge-function.ts b/packages/@aws-cdk/aws-cloudfront/lib/experimental/edge-function.ts
index a45bc0ce6db97..4a3974c2af632 100644
--- a/packages/@aws-cdk/aws-cloudfront/lib/experimental/edge-function.ts
+++ b/packages/@aws-cdk/aws-cloudfront/lib/experimental/edge-function.ts
@@ -148,7 +148,9 @@ export class EdgeFunction extends Resource implements lambda.IVersion {
     if (Token.isUnresolved(this.env.region)) {
       throw new Error('stacks which use EdgeFunctions must have an explicitly set region');
     }
-    const parameterName = `${parameterNamePrefix}/${this.env.region}/${this.node.path}`;
+    // SSM parameter names must only contain letters, numbers, ., _, -, or /.
+    const sanitizedPath = this.node.path.replace(/[^\/\w.-]/g, '_');
+    const parameterName = `${parameterNamePrefix}/${this.env.region}/${sanitizedPath}`;
     const functionStack = this.edgeStack(props.stackId);
 
     const edgeFunction = new lambda.Function(functionStack, id, props);
diff --git a/packages/@aws-cdk/aws-cloudfront/test/experimental/edge-function.test.ts b/packages/@aws-cdk/aws-cloudfront/test/experimental/edge-function.test.ts
index 4a22798eb48a1..55d6f3689103c 100644
--- a/packages/@aws-cdk/aws-cloudfront/test/experimental/edge-function.test.ts
+++ b/packages/@aws-cdk/aws-cloudfront/test/experimental/edge-function.test.ts
@@ -298,6 +298,16 @@ test('cross-region stack supports new-style synthesis with assets', () => {
   expect(() => app.synth()).not.toThrow();
 });
 
+test('SSM parameter name is sanitized to remove disallowed characters', () => {
+  new cloudfront.experimental.EdgeFunction(stack, 'My Bad#Fn$Name-With.Bonus', defaultEdgeFunctionProps());
+
+  const fnStack = getFnStack();
+
+  expect(fnStack).toHaveResourceLike('AWS::SSM::Parameter', {
+    Name: '/cdk/EdgeFunctionArn/testregion/Stack/My_Bad_Fn_Name-With.Bonus',
+  });
+});
+
 function defaultEdgeFunctionProps(stackId?: string) {
   return {
     code: lambda.Code.fromInline('foo'),
diff --git a/packages/@aws-cdk/aws-ssm/lib/parameter.ts b/packages/@aws-cdk/aws-ssm/lib/parameter.ts
index a33759d7cf014..cbec2073cbe1f 100644
--- a/packages/@aws-cdk/aws-ssm/lib/parameter.ts
+++ b/packages/@aws-cdk/aws-ssm/lib/parameter.ts
@@ -430,9 +430,7 @@ export class StringParameter extends ParameterBase implements IStringParameter {
       _assertValidValue(props.stringValue, props.allowedPattern);
     }
 
-    if (this.physicalName.length > 2048) {
-      throw new Error('Name cannot be longer than 2048 characters.');
-    }
+    validateParameterName(this.physicalName);
 
     if (props.description && props.description?.length > 1024) {
       throw new Error('Description cannot be longer than 1024 characters.');
@@ -497,9 +495,7 @@ export class StringListParameter extends ParameterBase implements IStringListPar
       props.stringListValue.forEach(str => _assertValidValue(str, props.allowedPattern!));
     }
 
-    if (this.physicalName.length > 2048) {
-      throw new Error('Name cannot be longer than 2048 characters.');
-    }
+    validateParameterName(this.physicalName);
 
     if (props.description && props.description?.length > 1024) {
       throw new Error('Description cannot be longer than 1024 characters.');
@@ -546,3 +542,13 @@ function _assertValidValue(value: string, allowedPattern: string): void {
 function makeIdentityForImportedValue(parameterName: string) {
   return `SsmParameterValue:${parameterName}:C96584B6-F00A-464E-AD19-53AFF4B05118`;
 }
+
+function validateParameterName(parameterName: string) {
+  if (Token.isUnresolved(parameterName)) { return; }
+  if (parameterName.length > 2048) {
+    throw new Error('name cannot be longer than 2048 characters.');
+  }
+  if (!parameterName.match(/^[\/\w.-]+$/)) {
+    throw new Error(`name must only contain letters, numbers, and the following 4 symbols .-_/; got ${parameterName}`);
+  }
+}
diff --git a/packages/@aws-cdk/aws-ssm/test/parameter.test.ts b/packages/@aws-cdk/aws-ssm/test/parameter.test.ts
index 2e40e4626cbd3..17b2af748d9ef 100644
--- a/packages/@aws-cdk/aws-ssm/test/parameter.test.ts
+++ b/packages/@aws-cdk/aws-ssm/test/parameter.test.ts
@@ -141,7 +141,21 @@ test('String SSM Parameter throws on long names', () => {
       Nam eget dui. Etiam rhoncus. Maecenas tempus, tellus eget condimentum rhoncus, sem quam semper libero, sit amet adipiscing \
       sem neque sed ipsum.',
     });
-  }).toThrow(/Name cannot be longer than 2048 characters./);
+  }).toThrow(/name cannot be longer than 2048 characters./);
+});
+
+test.each([
+  '/parameter/with spaces',
+  'charactersOtherThan^allowed',
+  'trying;this',
+])('String SSM Parameter throws on invalid name %s', (parameterName) => {
+  // GIVEN
+  const stack = new cdk.Stack();
+
+  // THEN
+  expect(() => {
+    new ssm.StringParameter(stack, 'Parameter', { stringValue: 'Foo', parameterName });
+  }).toThrow(/name must only contain letters, numbers, and the following 4 symbols.*/);
 });
 
 test('StringList SSM Parameter throws on long descriptions', () => {
@@ -194,7 +208,21 @@ test('StringList SSM Parameter throws on long names', () => {
       Nam eget dui. Etiam rhoncus. Maecenas tempus, tellus eget condimentum rhoncus, sem quam semper libero, sit amet adipiscing \
       sem neque sed ipsum.',
     });
-  }).toThrow(/Name cannot be longer than 2048 characters./);
+  }).toThrow(/name cannot be longer than 2048 characters./);
+});
+
+test.each([
+  '/parameter/with spaces',
+  'charactersOtherThan^allowed',
+  'trying;this',
+])('StringList SSM Parameter throws on invalid name %s', (parameterName) => {
+  // GIVEN
+  const stack = new cdk.Stack();
+
+  // THEN
+  expect(() => {
+    new ssm.StringListParameter(stack, 'Parameter', { stringListValue: ['Foo'], parameterName });
+  }).toThrow(/name must only contain letters, numbers, and the following 4 symbols.*/);
 });
 
 test('StringList SSM Parameter values cannot contain commas', () => {