Skip to content

Commit

Permalink
fix(cli): format of tags in cdk.json is not validated (aws#21050)
Browse files Browse the repository at this point in the history
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 aws#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*
  • Loading branch information
SydneyUni-Jim authored and daschaa committed Jul 9, 2022
1 parent d804573 commit 5a74a84
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 2 deletions.
5 changes: 3 additions & 2 deletions packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']),
Expand Down Expand Up @@ -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,
Expand Down
20 changes: 20 additions & 0 deletions packages/aws-cdk/lib/util/tags.ts
Original file line number Diff line number Diff line change
@@ -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');
}
}
31 changes: 31 additions & 0 deletions packages/aws-cdk/test/util/tags.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});

0 comments on commit 5a74a84

Please sign in to comment.