Changing load/dump in source files

.buildkite/pipeline-resource-definitions/scripts/fix-location-collection.ts

@@ -36,13 +36,15 @@ async function main() {
 
   const preamble = locationFileLines.slice(0, 1);
 
+  // eslint-disable-next-line @kbn/eslint/no_unsafe_js_yaml
   const locationObj = jsYaml.load(
     locationFileLines.slice(1).join('\n')
   ) as BackstageLocationResource;
   locationObj.spec.targets = pipelines.map(
     (fileName) => `${resourceDefinitionsBaseUrl}/${fileName}`
   );
 
+  // eslint-disable-next-line @kbn/eslint/no_unsafe_js_yaml
   const locationYaml = jsYaml.dump(locationObj, { lineWidth: 400 });
 
   fs.writeFileSync(locationFile, `${preamble.join('\n')}\n${locationYaml}`);

.buildkite/pipeline-utils/agent_images.ts

@@ -6,6 +6,7 @@
  * Side Public License, v 1.
  */
 
+// eslint-disable-next-line @kbn/eslint/no_unsafe_js_yaml
 import { dump } from 'js-yaml';
 import { BuildkiteClient, BuildkiteCommandStep } from './buildkite';
 

.buildkite/pipeline-utils/buildkite/client.ts

@@ -8,7 +8,10 @@
 
 import axios, { AxiosInstance } from 'axios';
 import { execSync, ExecSyncOptions } from 'child_process';
+
+// eslint-disable-next-line @kbn/eslint/no_unsafe_js_yaml
 import { dump } from 'js-yaml';
+
 import { parseLinkHeader } from './parse_link_header';
 import { Artifact } from './types/artifact';
 import { Build, BuildStatus } from './types/build';

.buildkite/pipeline-utils/ci-stats/pick_test_group_run_order.ts

@@ -10,6 +10,8 @@ import * as Fs from 'fs';
 
 import * as globby from 'globby';
 import minimatch from 'minimatch';
+
+// eslint-disable-next-line @kbn/eslint/no_unsafe_js_yaml
 import { load as loadYaml } from 'js-yaml';
 
 import { BuildkiteClient, BuildkiteStep } from '../buildkite';

packages/kbn-es/src/utils/read_roles_from_resource.ts

@@ -7,7 +7,7 @@
  */
 import fs from 'fs';
 import { extname } from 'path';
-import { load as loadYaml } from 'js-yaml';
+import { safeLoad as loadYaml } from 'js-yaml';
 
 export const readRolesFromResource = (resourcePath: string) => {
   if (!fs.existsSync(resourcePath) || extname(resourcePath) !== '.yml') {

packages/kbn-eslint-config/.eslintrc.js

@@ -308,6 +308,7 @@ module.exports = {
     '@kbn/eslint/no_constructor_args_in_property_initializers': 'error',
     '@kbn/eslint/no_this_in_property_initializers': 'error',
     '@kbn/eslint/no_unsafe_console': 'error',
+    '@kbn/eslint/no_unsafe_js_yaml': 'error',
     '@kbn/imports/no_unresolvable_imports': 'error',
     '@kbn/imports/uniform_imports': 'error',
     '@kbn/imports/no_unused_imports': 'error',

packages/kbn-eslint-plugin-eslint/index.js

@@ -18,5 +18,6 @@ module.exports = {
     no_constructor_args_in_property_initializers: require('./rules/no_constructor_args_in_property_initializers'),
     no_this_in_property_initializers: require('./rules/no_this_in_property_initializers'),
     no_unsafe_console: require('./rules/no_unsafe_console'),
+    no_unsafe_js_yaml: require('./rules/no_unsafe_js_yaml'),
   },
 };

packages/kbn-eslint-plugin-eslint/rules/no_unsafe_js_yaml.js

@@ -0,0 +1,90 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+module.exports = {
+  meta: {
+    fixable: 'code',
+    schema: [],
+  },
+  create(context) {
+    const sourceCode = context.getSourceCode();
+    const jsYamlIdentifiers = new Set();
+    const isUnsafeMethod = (node) => node.name === 'load' || node.name === 'dump';
+
+    return {
+      ImportDeclaration(node) {
+        if (node.source.value === 'js-yaml') {
+          node.specifiers.forEach((specifier) => {
+            jsYamlIdentifiers.add(specifier.local.name);
+
+            if (specifier.imported && isUnsafeMethod(specifier.imported)) {
+              context.report({
+                node: specifier,
+                message:
+                  'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+                fix(fixer) {
+                  const replacement =
+                    specifier.imported.name === 'load'
+                      ? fixer.replaceText(specifier.imported, 'safeLoad')
+                      : fixer.replaceText(specifier.imported, 'safeDump');
+                  return replacement;
+                },
+              });
+            }
+          });
+        }
+      },
+      CallExpression(node) {
+        const callee = node.callee;
+
+        if (isUnsafeMethod(callee)) {
+          const scope = sourceCode.getScope(node);
+          const variable = scope.variables.find((v) => v.name === callee.name);
+
+          if (variable && variable.defs.length) {
+            const [def] = variable.defs;
+
+            if (def?.parent?.source?.value === 'js-yaml') {
+              context.report({
+                node: callee,
+                message:
+                  'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+                fix(fixer) {
+                  const replacement =
+                    callee.name === 'load'
+                      ? fixer.replaceText(callee, 'safeLoad')
+                      : fixer.replaceText(callee, 'safeDump');
+                  return replacement;
+                },
+              });
+            }
+          }
+        }
+
+        if (
+          callee.type === 'MemberExpression' &&
+          isUnsafeMethod(callee.property) &&
+          jsYamlIdentifiers.has(callee.object.name)
+        ) {
+          context.report({
+            node: callee.property,
+            message:
+              'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+            fix(fixer) {
+              const replacement =
+                callee.property.name === 'load'
+                  ? fixer.replaceText(callee.property, 'safeLoad')
+                  : fixer.replaceText(callee.property, 'safeDump');
+              return replacement;
+            },
+          });
+        }
+      },
+    };
+  },
+};

packages/kbn-eslint-plugin-eslint/rules/no_unsafe_js_yaml.test.js

@@ -0,0 +1,104 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+const { RuleTester } = require('eslint');
+const rule = require('./no_unsafe_js_yaml');
+
+const ruleTester = new RuleTester({
+  parser: require.resolve('@typescript-eslint/parser'),
+  parserOptions: {
+    sourceType: 'module',
+    ecmaVersion: 2018,
+  },
+});
+
+ruleTester.run('no_unsafe_js_yaml', rule, {
+  valid: [
+    "import { safeLoad } from 'js-yaml'; const data = safeLoad(yamlString);",
+    "import { safeDump } from 'js-yaml'; const yaml = safeDump(data);",
+    "import * as yaml from 'js-yaml'; const data = yaml.safeLoad(yamlString);",
+    "import yaml from 'js-yaml'; yaml.safeLoad('yamlString');",
+  ],
+  invalid: [
+    {
+      code: "import { load } from 'js-yaml'; const data = load(yamlString);",
+      errors: [
+        {
+          message:
+            'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+          line: 1,
+          column: 10,
+          endLine: 1,
+          endColumn: 14,
+        },
+        {
+          message:
+            'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+          line: 1,
+          column: 46,
+          endLine: 1,
+          endColumn: 50,
+        },
+      ],
+      output: "import { safeLoad } from 'js-yaml'; const data = safeLoad(yamlString);",
+    },
+    {
+      code: "import { dump } from 'js-yaml'; const yaml = dump(data);",
+      errors: [
+        {
+          message:
+            'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+          line: 1,
+          column: 10,
+          endLine: 1,
+          endColumn: 14,
+        },
+        {
+          message:
+            'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+          line: 1,
+          column: 46,
+          endLine: 1,
+          endColumn: 50,
+        },
+      ],
+      output: "import { safeDump } from 'js-yaml'; const yaml = safeDump(data);",
+    },
+    {
+      code: "import * as yaml from 'js-yaml'; const data = yaml.load(yamlString);",
+      errors: [
+        {
+          message:
+            'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+        },
+      ],
+      output: "import * as yaml from 'js-yaml'; const data = yaml.safeLoad(yamlString);",
+    },
+    {
+      code: "import yaml from 'js-yaml'; yaml.load('someYAMLContent')",
+      errors: [
+        {
+          message:
+            'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+        },
+      ],
+      output: "import yaml from 'js-yaml'; yaml.safeLoad('someYAMLContent')",
+    },
+    {
+      code: "import yaml, { safeDump } from 'js-yaml'; safeDump(data); yaml.load('someYAMLContent');",
+      errors: [
+        {
+          message:
+            'Use `safeLoad` instead of `load` and `safeDump` instead of `dump` from `js-yaml`.',
+        },
+      ],
+      output:
+        "import yaml, { safeDump } from 'js-yaml'; safeDump(data); yaml.safeLoad('someYAMLContent');",
+    },
+  ],
+});

packages/kbn-openapi-bundler/src/utils/extract_by_json_pointer.ts

@@ -7,7 +7,7 @@
  */
 
 import chalk from 'chalk';
-import { dump } from 'js-yaml';
+import { safeDump } from 'js-yaml';
 import { isPlainObjectType } from './is_plain_object_type';
 
 /**
@@ -32,7 +32,9 @@ export function extractByJsonPointer(document: unknown, pointer: string): unknow
       throw new Error(
         `JSON Pointer ${chalk.bold(pointer)} resolution failure. Expected ${chalk.magenta(
           path.join('/')
-        )} to be a plain object but it has type "${typeof target}" in \n\n${dump(document)}`
+        )} to be a plain object but it has type "${typeof target}" in \n\n${safeDump(document, {
+          skipInvalid: true,
+        })}`
       );
     }
 
@@ -66,7 +68,7 @@ export function extractObjectByJsonPointer(
     throw new Error(
       `JSON Pointer resolution failure. Expected ${chalk.magenta(
         pointer
-      )} to be a plain object in \n\n${dump(document)}`
+      )} to be a plain object in \n\n${safeDump(document, { skipInvalid: true })}`
     );
   }
 

packages/kbn-openapi-bundler/src/utils/read_document.ts

@@ -8,7 +8,7 @@
 
 import fs from 'fs/promises';
 import { basename, extname } from 'path';
-import { load } from 'js-yaml';
+import { safeLoad } from 'js-yaml';
 import chalk from 'chalk';
 import { logger } from '../logger';
 import { isPlainObjectType } from './is_plain_object_type';
@@ -45,7 +45,7 @@ async function readYamlFile(filePath: string): Promise<Record<string, unknown>>
   // Typing load's result to Record<string, unknown> is optimistic as we can't be sure
   // there is object inside a yaml file. We don't have this validation layer so far
   // but using JSON Schemas here should mitigate this problem.
-  return load(await fs.readFile(filePath, { encoding: 'utf8' }));
+  return safeLoad(await fs.readFile(filePath, { encoding: 'utf8' }));
 }
 
 async function readJsonFile(filePath: string): Promise<Record<string, unknown>> {

packages/kbn-openapi-bundler/src/utils/write_yaml_document.ts

@@ -7,7 +7,7 @@
  */
 
 import fs from 'fs/promises';
-import { dump } from 'js-yaml';
+import { safeDump } from 'js-yaml';
 import { dirname } from 'path';
 
 export async function writeYamlDocument(filePath: string, document: unknown): Promise<void> {
@@ -23,21 +23,16 @@ export async function writeYamlDocument(filePath: string, document: unknown): Pr
 
 function stringifyToYaml(document: unknown): string {
   try {
-    // We don't want to have `undefined` values serialized into YAML.
-    // `JSON.stringify()` simply skips `undefined` values while js-yaml v 3.14 DOES NOT.
-    // js-yaml >= v4 has it fixed so `dump()`'s behavior is consistent with  `JSON.stringify()`.
-    // Until js-yaml is updated to v4 use the hack with JSON serialization/deserialization.
-    const clearedDocument = JSON.parse(JSON.stringify(document));
-
     // Disable YAML Anchors https://yaml.org/spec/1.2.2/#3222-anchors-and-aliases
     // It makes YAML much more human readable
-    return dump(clearedDocument, {
+    return safeDump(document, {
       noRefs: true,
       sortKeys: sortYamlKeys,
+      skipInvalid: true, // Skip invalid types like `undefined`
     });
   } catch (e) {
     // Try to stringify with YAML Anchors enabled
-    return dump(document, { noRefs: false, sortKeys: sortYamlKeys });
+    return safeDump(document, { noRefs: false, sortKeys: sortYamlKeys, skipInvalid: true });
   }
 }
 

packages/kbn-openapi-bundler/tests/bundler/bundle_specs.ts

@@ -16,7 +16,7 @@ import {
   unlinkSync,
   writeFileSync,
 } from 'fs';
-import { dump, load } from 'js-yaml';
+import { safeDump, safeLoad } from 'js-yaml';
 import { OpenAPIV3 } from 'openapi-types';
 import { bundle, BundlerConfig } from '../../src/openapi_bundler';
 
@@ -56,7 +56,10 @@ function dumpSpecs(folderPath: string, oasSpecs: Record<string, OpenAPIV3.Docume
   mkdirSync(folderPath, { recursive: true });
 
   for (const [fileName, oasSpec] of Object.entries(oasSpecs)) {
-    writeFileSync(join(folderPath, `${fileName}.schema.yaml`), dump(oasSpec));
+    writeFileSync(
+      join(folderPath, `${fileName}.schema.yaml`),
+      safeDump(oasSpec, { skipInvalid: true }) // Skip invalid types like `undefined`
+    );
   }
 }
 
@@ -66,7 +69,7 @@ export function readBundledSpecs(folderPath: string): Record<string, OpenAPIV3.D
   for (const fileName of readdirSync(folderPath)) {
     const yaml = readFileSync(join(folderPath, fileName), { encoding: 'utf8' });
 
-    bundledSpecs[fileName] = load(yaml);
+    bundledSpecs[fileName] = safeLoad(yaml);
   }
 
   return bundledSpecs;

packages/kbn-openapi-bundler/tests/bundler/bundle_specs_with_multiple_modifications.test.ts

@@ -7,7 +7,7 @@
  */
 
 import { readFileSync } from 'fs';
-import { load } from 'js-yaml';
+import { safeLoad } from 'js-yaml';
 import { join } from 'path';
 import { bundleFolder, readBundledSpecs } from './bundle_specs';
 
@@ -25,7 +25,7 @@ describe('OpenAPI Bundler - specs with multiple modifications', () => {
 
     const [bundledSpec] = Object.values(readBundledSpecs(outputFolderPath));
 
-    const expected = load(
+    const expected = safeLoad(
       readFileSync(join(folderToBundlePath, 'expected.yaml'), { encoding: 'utf8' })
     );