Skip to content

Commit

Permalink
fix(cloudfront): EdgeFunctions cannot be created when IDs contain spa…
Browse files Browse the repository at this point in the history
…ces (#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*
  • Loading branch information
njlynch committed Oct 11, 2021
1 parent 7826d5c commit 833767c
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
18 changes: 12 additions & 6 deletions packages/@aws-cdk/aws-ssm/lib/parameter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.');
Expand Down Expand Up @@ -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.');
Expand Down Expand Up @@ -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}`);
}
}
32 changes: 30 additions & 2 deletions packages/@aws-cdk/aws-ssm/test/parameter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit 833767c

Please sign in to comment.