Skip to content

Commit

Permalink
fix: Switch from js-yaml to yaml (#1092)
Browse files Browse the repository at this point in the history
Implementations that produce `YAML-1.2` in compatible mode still can end
up producing YAML that will be icnorrectly parsed by CloudFormation. For
example, literal strings containing only numbers and starting with a `0`
do not get quoted, which results in a `YAML-1.1` parser interpreting
those as a number. This can lead to invalid principals being generated
where the account ID form is used (for example, in Lambda permissions
objects).

Switching to `yaml@1.0.0` addresses this as it can be configured to emit
`YAML-1.1` explicitly.
  • Loading branch information
RomainMuller authored and rix0rrr committed Nov 6, 2018
1 parent ae03ddb commit 0b132b5
Show file tree
Hide file tree
Showing 7 changed files with 53 additions and 20 deletions.
6 changes: 2 additions & 4 deletions packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,9 @@ import child_process = require('child_process');
import fs = require('fs-extra');
import os = require('os');
import path = require('path');
import YAML = require('yaml');
import { isStackConstructor, parseApplet } from '../lib/applet-helpers';

// tslint:disable-next-line:no-var-requires
const YAML = require('js-yaml');

main().catch(e => {
// tslint:disable-next-line:no-console
console.error(e);
Expand All @@ -26,7 +24,7 @@ async function main() {
}

// read applet(s) properties from the provided file
const fileContents = YAML.safeLoad(await fs.readFile(appletFile, { encoding: 'utf-8' }));
const fileContents = YAML.parse(await fs.readFile(appletFile, { encoding: 'utf-8' }), { schema: 'yaml-1.1' });
if (typeof fileContents !== 'object') {
throw new Error(`${appletFile}: should contain a YAML object`);
}
Expand Down
9 changes: 4 additions & 5 deletions packages/@aws-cdk/applet-js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,16 @@
},
"license": "Apache-2.0",
"devDependencies": {
"@types/yamljs": "^0.2.0",
"@types/fs-extra": "^5.0.4",
"@types/yaml": "^1.0.0",
"cdk-build-tools": "^0.14.1",
"pkglint": "^0.14.1"
},
"dependencies": {
"@aws-cdk/cdk": "^0.14.1",
"@types/fs-extra": "^5.0.4",
"@types/js-yaml": "^3.11.2",
"fs-extra": "^7.0.0",
"js-yaml": "^3.12.0",
"source-map-support": "^0.5.6"
"source-map-support": "^0.5.6",
"yaml": "^1.0.0"
},
"repository": {
"url": "https://github.com/awslabs/aws-cdk.git",
Expand Down
8 changes: 3 additions & 5 deletions packages/aws-cdk/bin/cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import 'source-map-support/register';
import cxapi = require('@aws-cdk/cx-api');
import colors = require('colors/safe');
import fs = require('fs-extra');
import YAML = require('js-yaml');
import minimatch = require('minimatch');
import util = require('util');
import YAML = require('yaml');
import yargs = require('yargs');
import cdkUtil = require('../lib/util');

Expand Down Expand Up @@ -610,7 +610,7 @@ async function initCommandLine() {
/* Attempt to parse YAML, fall back to JSON. */
function parseTemplate(text: string): any {
try {
return YAML.safeLoad(text);
return YAML.parse(text, { schema: 'yaml-1.1' });
} catch (e) {
return JSON.parse(text);
}
Expand Down Expand Up @@ -684,9 +684,7 @@ async function initCommandLine() {
const indentWidth = 2;
return JSON.stringify(object, noFiltering, indentWidth);
} else {
const inlineJsonAfterDepth = 16;
const indentWidth = 4;
return YAML.safeDump(object, { indent: indentWidth, flowLevel: inlineJsonAfterDepth });
return YAML.stringify(object, { schema: 'yaml-1.1' });
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import cxapi = require('@aws-cdk/cx-api');
import aws = require('aws-sdk');
import colors = require('colors/safe');
import YAML = require('js-yaml');
import uuid = require('uuid');
import YAML = require('yaml');
import { prepareAssets } from '../assets';
import { debug, error, print } from '../logging';
import { Mode } from './aws-auth/credentials';
Expand Down Expand Up @@ -113,7 +113,7 @@ async function getStackOutputs(cfn: aws.CloudFormation, stackName: string): Prom
* @param toolkitInfo information about the toolkit stack
*/
async function makeBodyParameter(stack: cxapi.SynthesizedStack, toolkitInfo?: ToolkitInfo): Promise<TemplateBodyParameter> {
const templateJson = YAML.safeDump(stack.template, { indent: 4, flowLevel: 16 });
const templateJson = YAML.stringify(stack.template, { schema: 'yaml-1.1' });
if (toolkitInfo) {
const s3KeyPrefix = `cdk/${stack.name}/`;
const s3KeySuffix = '.yml';
Expand Down
9 changes: 7 additions & 2 deletions packages/aws-cdk/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions packages/aws-cdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@
"devDependencies": {
"@types/archiver": "^2.1.2",
"@types/fs-extra": "^4.0.8",
"@types/js-yaml": "^3.11.2",
"@types/minimatch": "^3.0.3",
"@types/mockery": "^1.4.29",
"@types/request": "^2.47.1",
"@types/semver": "^5.5.0",
"@types/uuid": "^3.4.3",
"@types/yaml": "^1.0.0",
"@types/yargs": "^8.0.3",
"cdk-build-tools": "^0.14.1",
"mockery": "^2.1.0",
Expand All @@ -54,14 +54,14 @@
"colors": "^1.2.1",
"decamelize": "^2.0.0",
"fs-extra": "^4.0.2",
"js-yaml": "^3.12.0",
"json-diff": "^0.3.1",
"minimatch": ">=3.0",
"promptly": "^0.2.0",
"proxy-agent": "^3.0.1",
"request": "^2.83.0",
"semver": "^5.5.0",
"source-map-support": "^0.5.6",
"yaml": "^1.0.0",
"yargs": "^9.0.1"
},
"repository": {
Expand Down
33 changes: 33 additions & 0 deletions packages/aws-cdk/test/test.yaml.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { Test } from 'nodeunit';
import YAML = require('yaml');

export = {
'validate that our YAML quotes the word "ON"'(test: Test) {
// tslint:disable-next-line:no-console
const output = YAML.stringify({
notABoolean: "ON"
}, { schema: 'yaml-1.1' });

test.equals(output.trim(), 'notABoolean: "ON"');

test.done();
},

'validate that our YAML correctly quotes strings with a leading zero'(test: Test) {
const output = YAML.stringify({
leadingZero: "0123456789"
} , { schema: 'yaml-1.1' });

test.equals(output.trim(), 'leadingZero: "0123456789"');

test.done();
},

'validate that our YAML correctly parses strings with a leading zero'(test: Test) {
const output = YAML.parse('leadingZero: "0123456789"', { schema: 'yaml-1.1' });

test.deepEqual(output, { leadingZero: '0123456789' });

test.done();
},
};

0 comments on commit 0b132b5

Please sign in to comment.