fix: Switch from js-yaml to yaml (#1092)

RomainMuller · rix0rrr · commit 0b132b539374 · 2018-11-06T16:31:08.000+01:00
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.
diff --git a/packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts b/packages/@aws-cdk/applet-js/bin/cdk-applet-js.ts
@@ -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);
@@ -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`);
   }
diff --git a/packages/@aws-cdk/applet-js/package.json b/packages/@aws-cdk/applet-js/package.json
@@ -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",
diff --git a/packages/aws-cdk/bin/cdk.ts b/packages/aws-cdk/bin/cdk.ts
@@ -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');
 
@@ -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);
       }
@@ -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' });
     }
   }
 }
diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts
@@ -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';
@@ -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';
diff --git a/packages/aws-cdk/package-lock.json b/packages/aws-cdk/package-lock.json
diff --git a/packages/aws-cdk/package.json b/packages/aws-cdk/package.json
@@ -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",
@@ -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": {
diff --git a/packages/aws-cdk/test/test.yaml.ts b/packages/aws-cdk/test/test.yaml.ts
@@ -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();
+  },
+};
