[Fleet] Escape YAML string values if necessary (#91418)

* Use js-yaml.safeDump() to escape string values. * Add unit test. * Explicitly check for YAML special characters. * Remove unnecessary imports. * Use RegExp.prototype.test() for speed.
elastic · Feb 16, 2021 · 58849bc · 58849bc
1 parent bb653a4
commit 58849bc
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 8 deletions.
diff --git a/x-pack/plugins/fleet/server/services/epm/agent/agent.test.ts b/x-pack/plugins/fleet/server/services/epm/agent/agent.test.ts
@@ -179,4 +179,79 @@ input: logs
       input: 'logs',
     });
   });
+
+  it('should escape string values when necessary', () => {
+    const stringTemplate = `
+my-package:
+    opencurly: {{opencurly}}
+    closecurly: {{closecurly}}
+    opensquare: {{opensquare}}
+    closesquare: {{closesquare}}
+    ampersand: {{ampersand}}
+    asterisk: {{asterisk}}
+    question: {{question}}
+    pipe: {{pipe}}
+    hyphen: {{hyphen}}
+    openangle: {{openangle}}
+    closeangle: {{closeangle}}
+    equals: {{equals}}
+    exclamation: {{exclamation}}
+    percent: {{percent}}
+    at: {{at}}
+    colon: {{colon}}
+    numeric: {{numeric}}
+    mixed: {{mixed}}`;
+
+    // List of special chars that may lead to YAML parsing errors when not quoted.
+    // See YAML specification section 5.3 Indicator characters
+    // https://yaml.org/spec/1.2/spec.html#id2772075
+    // {,},[,],&,*,?,|,-,<,>,=,!,%,@,:
+    const vars = {
+      opencurly: { value: '{', type: 'string' },
+      closecurly: { value: '}', type: 'string' },
+      opensquare: { value: '[', type: 'string' },
+      closesquare: { value: ']', type: 'string' },
+      comma: { value: ',', type: 'string' },
+      ampersand: { value: '&', type: 'string' },
+      asterisk: { value: '*', type: 'string' },
+      question: { value: '?', type: 'string' },
+      pipe: { value: '|', type: 'string' },
+      hyphen: { value: '-', type: 'string' },
+      openangle: { value: '<', type: 'string' },
+      closeangle: { value: '>', type: 'string' },
+      equals: { value: '=', type: 'string' },
+      exclamation: { value: '!', type: 'string' },
+      percent: { value: '%', type: 'string' },
+      at: { value: '@', type: 'string' },
+      colon: { value: ':', type: 'string' },
+      numeric: { value: '100', type: 'string' },
+      mixed: { value: '1s', type: 'string' },
+    };
+
+    const targetOutput = {
+      'my-package': {
+        opencurly: '{',
+        closecurly: '}',
+        opensquare: '[',
+        closesquare: ']',
+        ampersand: '&',
+        asterisk: '*',
+        question: '?',
+        pipe: '|',
+        hyphen: '-',
+        openangle: '<',
+        closeangle: '>',
+        equals: '=',
+        exclamation: '!',
+        percent: '%',
+        at: '@',
+        colon: ':',
+        numeric: '100',
+        mixed: '1s',
+      },
+    };
+
+    const output = compileTemplate(vars, stringTemplate);
+    expect(output).toEqual(targetOutput);
+  });
 });
diff --git a/x-pack/plugins/fleet/server/services/epm/agent/agent.ts b/x-pack/plugins/fleet/server/services/epm/agent/agent.ts
@@ -13,7 +13,6 @@ const handlebars = Handlebars.create();
 
 export function compileTemplate(variables: PackagePolicyConfigRecord, templateStr: string) {
   const { vars, yamlValues } = buildTemplateVariables(variables, templateStr);
-
   const template = handlebars.compile(templateStr, { noEscape: true });
   let compiledTemplate = template(vars);
   compiledTemplate = replaceRootLevelYamlVariables(yamlValues, compiledTemplate);
@@ -58,8 +57,17 @@ function replaceVariablesInYaml(yamlVariables: { [k: string]: any }, yaml: any)
   return yaml;
 }
 
-const maybeEscapeNumericString = (value: string) => {
-  return value.length && !isNaN(+value) ? `"${value}"` : value;
+const maybeEscapeString = (value: string) => {
+  // List of special chars that may lead to YAML parsing errors when not quoted.
+  // See YAML specification section 5.3 Indicator characters
+  // https://yaml.org/spec/1.2/spec.html#id2772075
+  const yamlSpecialCharsRegex = /[{}\[\],&*?|\-<>=!%@:]/;
+
+  // In addition, numeric strings need to be quoted to stay strings.
+  if ((value.length && !isNaN(+value)) || yamlSpecialCharsRegex.test(value)) {
+    return `"${value}"`;
+  }
+  return value;
 };
 
 function buildTemplateVariables(variables: PackagePolicyConfigRecord, templateStr: string) {
@@ -88,13 +96,15 @@ function buildTemplateVariables(variables: PackagePolicyConfigRecord, templateSt
       const yamlKeyPlaceholder = `##${key}##`;
       varPart[lastKeyPart] = `"${yamlKeyPlaceholder}"`;
       yamlValues[yamlKeyPlaceholder] = recordEntry.value ? safeLoad(recordEntry.value) : null;
-    } else if (recordEntry.type && recordEntry.type === 'text' && recordEntry.value?.length) {
+    } else if (
+      recordEntry.type &&
+      (recordEntry.type === 'text' || recordEntry.type === 'string') &&
+      recordEntry.value?.length
+    ) {
       if (Array.isArray(recordEntry.value)) {
-        varPart[lastKeyPart] = recordEntry.value.map((value: string) =>
-          maybeEscapeNumericString(value)
-        );
+        varPart[lastKeyPart] = recordEntry.value.map((value: string) => maybeEscapeString(value));
       } else {
-        varPart[lastKeyPart] = maybeEscapeNumericString(recordEntry.value);
+        varPart[lastKeyPart] = maybeEscapeString(recordEntry.value);
       }
     } else {
       varPart[lastKeyPart] = recordEntry.value;