From 8e6a387b2f8502a9a1c861d5bd1c5c7b0d3ada54 Mon Sep 17 00:00:00 2001 From: Jim Nicholls Date: Fri, 8 Jul 2022 19:10:51 +1000 Subject: [PATCH] fix(cli): format of tags in cdk.json is not validated (#21050) If tags is present in cdk.json, validate that it is an array of objects, and each object has a Tag string and a Value string. If tags is not structurally valid `cdk bootstrap` and `cdk deploy` fail with an error. `tags must be an array of { Tag: string, Value: string } objects` There is no attempt to validate the strings of each Tag and Value beyond that they are strings. closes #20854 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk/lib/cli.ts | 5 ++-- packages/aws-cdk/lib/util/tags.ts | 20 ++++++++++++++++ packages/aws-cdk/test/util/tags.test.ts | 31 +++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 packages/aws-cdk/lib/util/tags.ts create mode 100644 packages/aws-cdk/test/util/tags.test.ts diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 4533125708409..5aaedcc7e5759 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -23,6 +23,7 @@ import { data, debug, error, print, setLogLevel, setCI } from '../lib/logging'; import { displayNotices, refreshNotices } from '../lib/notices'; import { Command, Configuration, Settings } from '../lib/settings'; import * as version from '../lib/version'; +import { validateTags } from './util/tags'; // https://github.com/yargs/yargs/issues/1929 // https://github.com/evanw/esbuild/issues/1492 @@ -434,7 +435,7 @@ async function initCommandLine() { force: argv.force, toolkitStackName: toolkitStackName, execute: args.execute, - tags: configuration.settings.get(['tags']), + tags: validateTags(configuration.settings.get(['tags'])), terminationProtection: args.terminationProtection, parameters: { bucketName: configuration.settings.get(['toolkitBucket', 'bucketName']), @@ -464,7 +465,7 @@ async function initCommandLine() { notificationArns: args.notificationArns, requireApproval: configuration.settings.get(['requireApproval']), reuseAssets: args['build-exclude'], - tags: configuration.settings.get(['tags']), + tags: validateTags(configuration.settings.get(['tags'])), execute: args.execute, changeSetName: args.changeSetName, force: args.force, diff --git a/packages/aws-cdk/lib/util/tags.ts b/packages/aws-cdk/lib/util/tags.ts new file mode 100644 index 0000000000000..75d3277aff00f --- /dev/null +++ b/packages/aws-cdk/lib/util/tags.ts @@ -0,0 +1,20 @@ +import { Tag } from '../cdk-toolkit'; + +/** + * Throws an error if tags is neither undefined nor an array of Tags, + * as defined in cdk-toolkit.ts. + * + * It does not attempt to validate the tags themselves. It only validates + * that the objects in the array conform to the Tags interface. + */ +export function validateTags(tags: any): Tag[] | undefined { + const valid = tags === undefined || ( + Array.isArray(tags) && + tags.every(t => typeof(t.Tag) === 'string' && typeof(t.Value) === 'string') + ); + if (valid) { + return tags; + } else { + throw new Error('tags must be an array of { Tag: string, Value: string } objects'); + } +} diff --git a/packages/aws-cdk/test/util/tags.test.ts b/packages/aws-cdk/test/util/tags.test.ts new file mode 100644 index 0000000000000..9789eb449300d --- /dev/null +++ b/packages/aws-cdk/test/util/tags.test.ts @@ -0,0 +1,31 @@ +import { validateTags } from '../../lib/util/tags'; + +test('validateTags does not throw when given undefined', () => { + expect(validateTags(undefined)).toBeUndefined(); +}); + +test('validateTags does not throw when given an empty array', () => { + const tags: any = []; + expect(validateTags(tags)).toBe(tags); +}); + +test('validateTags does not throw when given array of Tag objects', () => { + const tags: any = [{ Tag: 'a', Value: 'b' }]; + expect(validateTags(tags)).toBe(tags); +}); + +test('validateTags throws when given an object', () => { + expect(() => validateTags({ a: 'b' })).toThrow('tags must be'); +}); + +test('validateTags throws when given an array of non-Tag objects', () => { + expect(() => validateTags([{ a: 'b' }])).toThrow('tags must be'); +}); + +test('validateTags throws when Tag is not a string', () => { + expect(() => validateTags([{ Tag: null, Value: 'b' }])).toThrow(); +}); + +test('validateTags throws when Value is not a string', () => { + expect(() => validateTags([{ Tag: 'a', Value: null }])).toThrow(); +});