Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fleet] Escape YAML string values if necessary #91418

Merged
merged 5 commits into from
Feb 16, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions x-pack/plugins/fleet/server/services/epm/agent/agent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { safeDump, safeLoad } from 'js-yaml';
import { compileTemplate } from './agent';

describe('compileTemplate', () => {
Expand Down Expand Up @@ -179,4 +180,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);
});
});
26 changes: 18 additions & 8 deletions x-pack/plugins/fleet/server/services/epm/agent/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)) || value.match(yamlSpecialCharsRegex)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also do yamlSpecialCharsRegex.test(value) since we only want a boolean result; not the matched item. It's also faster. Both are likely Fast Enough ™️ but this seems like a good place to use the more efficient option.

return `"${value}"`;
}
return value;
};

function buildTemplateVariables(variables: PackagePolicyConfigRecord, templateStr: string) {
Expand Down Expand Up @@ -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;
Expand Down