Skip to content

Commit

Permalink
Allow action types to perform their own mustache variable escaping in…
Browse files Browse the repository at this point in the history
… parameter templates (elastic#83919)

resolves elastic#79371
resolves elastic#62928

In this PR, we allow action types to determine how to escape the
variables used in their parameters, when rendered as mustache
templates.  Prior to this, action parameters were recursively
rendered as mustache templates using the default mustache
templating, by the alerts library.  The default mustache
templating used html escaping.

Action types opt-in to the new capability via a new optional
method in the action type, `renderParameterTemplates()`.  If not
provided, the previous recursive rendering is done, but now with
no escaping at all.

For elastic#62928, changed the mustache template rendering to be
replaced with the error message, if an error occurred,
so at least you can now see that an error occurred.  Useful
to diagnose problems with invalid mustache templates.
  • Loading branch information
pmuellr committed Dec 15, 2020
1 parent 9b0ec30 commit 20a7cc9
Show file tree
Hide file tree
Showing 22 changed files with 862 additions and 37 deletions.
33 changes: 33 additions & 0 deletions x-pack/plugins/actions/server/builtin_action_types/email.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,4 +396,37 @@ describe('execute()', () => {
}
`);
});

test('renders parameter templates as expected', async () => {
expect(actionType.renderParameterTemplates).toBeTruthy();
const paramsWithTemplates = {
to: [],
cc: ['{{rogue}}'],
bcc: ['jim', '{{rogue}}', 'bob'],
subject: '{{rogue}}',
message: '{{rogue}}',
};
const variables = {
rogue: '*bold*',
};
const params = actionType.renderParameterTemplates!(paramsWithTemplates, variables);
// Yes, this is tested in the snapshot below, but it's double-escaped there,
// so easier to see here that the escaping is correct.
expect(params.message).toBe('\\*bold\\*');
expect(params).toMatchInlineSnapshot(`
Object {
"bcc": Array [
"jim",
"*bold*",
"bob",
],
"cc": Array [
"*bold*",
],
"message": "\\\\*bold\\\\*",
"subject": "*bold*",
"to": Array [],
}
`);
});
});
14 changes: 14 additions & 0 deletions x-pack/plugins/actions/server/builtin_action_types/email.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { portSchema } from './lib/schemas';
import { Logger } from '../../../../../src/core/server';
import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from '../types';
import { ActionsConfigurationUtilities } from '../actions_config';
import { renderMustacheString, renderMustacheObject } from '../lib/mustache_renderer';

export type EmailActionType = ActionType<
ActionTypeConfigType,
Expand Down Expand Up @@ -140,10 +141,23 @@ export function getActionType(params: GetActionTypeParams): EmailActionType {
secrets: SecretsSchema,
params: ParamsSchema,
},
renderParameterTemplates,
executor: curry(executor)({ logger }),
};
}

function renderParameterTemplates(
params: ActionParamsType,
variables: Record<string, unknown>
): ActionParamsType {
return {
// most of the params need no escaping
...renderMustacheObject(params, variables),
// message however, needs to escaped as markdown
message: renderMustacheString(params.message, variables, 'markdown'),
};
}

// action executor

async function executor(
Expand Down
12 changes: 12 additions & 0 deletions x-pack/plugins/actions/server/builtin_action_types/slack.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,4 +213,16 @@ describe('execute()', () => {
'IncomingWebhook was called with proxyUrl https://someproxyhost'
);
});

test('renders parameter templates as expected', async () => {
expect(actionType.renderParameterTemplates).toBeTruthy();
const paramsWithTemplates = {
message: '{{rogue}}',
};
const variables = {
rogue: '*bold*',
};
const params = actionType.renderParameterTemplates!(paramsWithTemplates, variables);
expect(params.message).toBe('`*bold*`');
});
});
11 changes: 11 additions & 0 deletions x-pack/plugins/actions/server/builtin_action_types/slack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { pipe } from 'fp-ts/lib/pipeable';
import { map, getOrElse } from 'fp-ts/lib/Option';
import { Logger } from '../../../../../src/core/server';
import { getRetryAfterIntervalFromHeaders } from './lib/http_rersponse_retry_header';
import { renderMustacheString } from '../lib/mustache_renderer';

import {
ActionType,
Expand Down Expand Up @@ -73,10 +74,20 @@ export function getActionType({
}),
params: ParamsSchema,
},
renderParameterTemplates,
executor,
};
}

function renderParameterTemplates(
params: ActionParamsType,
variables: Record<string, unknown>
): ActionParamsType {
return {
message: renderMustacheString(params.message, variables, 'slack'),
};
}

function valdiateActionTypeConfig(
configurationUtilities: ActionsConfigurationUtilities,
secretsObject: ActionTypeSecretsType
Expand Down
24 changes: 24 additions & 0 deletions x-pack/plugins/actions/server/builtin_action_types/webhook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,4 +373,28 @@ describe('execute()', () => {
}
`);
});

test('renders parameter templates as expected', async () => {
const rogue = `double-quote:"; line-break->\n`;

expect(actionType.renderParameterTemplates).toBeTruthy();
const paramsWithTemplates = {
body: '{"x": "{{rogue}}"}',
};
const variables = {
rogue,
};
const params = actionType.renderParameterTemplates!(paramsWithTemplates, variables);

// eslint-disable-next-line @typescript-eslint/no-explicit-any
let paramsObject: any;
try {
paramsObject = JSON.parse(`${params.body}`);
} catch (err) {
expect(err).toBe(null); // kinda weird, but test should fail if it can't parse
}

expect(paramsObject.x).toBe(rogue);
expect(params.body).toBe(`{"x": "double-quote:\\"; line-break->\\n"}`);
});
});
12 changes: 12 additions & 0 deletions x-pack/plugins/actions/server/builtin_action_types/webhook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { ActionType, ActionTypeExecutorOptions, ActionTypeExecutorResult } from
import { ActionsConfigurationUtilities } from '../actions_config';
import { Logger } from '../../../../../src/core/server';
import { request } from './lib/axios_utils';
import { renderMustacheString } from '../lib/mustache_renderer';

// config definition
export enum WebhookMethods {
Expand Down Expand Up @@ -91,10 +92,21 @@ export function getActionType({
secrets: SecretsSchema,
params: ParamsSchema,
},
renderParameterTemplates,
executor: curry(executor)({ logger }),
};
}

function renderParameterTemplates(
params: ActionParamsType,
variables: Record<string, unknown>
): ActionParamsType {
if (!params.body) return params;
return {
body: renderMustacheString(params.body, variables, 'json'),
};
}

function validateActionTypeConfig(
configurationUtilities: ActionsConfigurationUtilities,
configObject: ActionTypeConfigType
Expand Down
183 changes: 183 additions & 0 deletions x-pack/plugins/actions/server/lib/mustache_renderer.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { renderMustacheString, renderMustacheObject, Escape } from './mustache_renderer';

const variables = {
a: 1,
b: '2',
c: false,
d: null,
e: undefined,
f: {
g: 3,
h: null,
},
i: [42, 43, 44],
lt: '<',
gt: '>',
amp: '&',
nl: '\n',
dq: '"',
bt: '`',
bs: '\\',
st: '*',
ul: '_',
st_lt: '*<',
};

describe('mustache_renderer', () => {
describe('renderMustacheString()', () => {
for (const escapeVal of ['none', 'slack', 'markdown', 'json']) {
const escape = escapeVal as Escape;

it(`handles basic templating that does not need escaping for ${escape}`, () => {
expect(renderMustacheString('', variables, escape)).toBe('');
expect(renderMustacheString('{{a}}', variables, escape)).toBe('1');
expect(renderMustacheString('{{b}}', variables, escape)).toBe('2');
expect(renderMustacheString('{{c}}', variables, escape)).toBe('false');
expect(renderMustacheString('{{d}}', variables, escape)).toBe('');
expect(renderMustacheString('{{e}}', variables, escape)).toBe('');
if (escape === 'markdown') {
expect(renderMustacheString('{{f}}', variables, escape)).toBe('\\[object Object\\]');
} else {
expect(renderMustacheString('{{f}}', variables, escape)).toBe('[object Object]');
}
expect(renderMustacheString('{{f.g}}', variables, escape)).toBe('3');
expect(renderMustacheString('{{f.h}}', variables, escape)).toBe('');
expect(renderMustacheString('{{i}}', variables, escape)).toBe('42,43,44');
});
}

it('handles escape:none with commonly escaped strings', () => {
expect(renderMustacheString('{{lt}}', variables, 'none')).toBe(variables.lt);
expect(renderMustacheString('{{gt}}', variables, 'none')).toBe(variables.gt);
expect(renderMustacheString('{{amp}}', variables, 'none')).toBe(variables.amp);
expect(renderMustacheString('{{nl}}', variables, 'none')).toBe(variables.nl);
expect(renderMustacheString('{{dq}}', variables, 'none')).toBe(variables.dq);
expect(renderMustacheString('{{bt}}', variables, 'none')).toBe(variables.bt);
expect(renderMustacheString('{{bs}}', variables, 'none')).toBe(variables.bs);
expect(renderMustacheString('{{st}}', variables, 'none')).toBe(variables.st);
expect(renderMustacheString('{{ul}}', variables, 'none')).toBe(variables.ul);
});

it('handles escape:markdown with commonly escaped strings', () => {
expect(renderMustacheString('{{lt}}', variables, 'markdown')).toBe(variables.lt);
expect(renderMustacheString('{{gt}}', variables, 'markdown')).toBe(variables.gt);
expect(renderMustacheString('{{amp}}', variables, 'markdown')).toBe(variables.amp);
expect(renderMustacheString('{{nl}}', variables, 'markdown')).toBe(variables.nl);
expect(renderMustacheString('{{dq}}', variables, 'markdown')).toBe(variables.dq);
expect(renderMustacheString('{{bt}}', variables, 'markdown')).toBe('\\' + variables.bt);
expect(renderMustacheString('{{bs}}', variables, 'markdown')).toBe('\\' + variables.bs);
expect(renderMustacheString('{{st}}', variables, 'markdown')).toBe('\\' + variables.st);
expect(renderMustacheString('{{ul}}', variables, 'markdown')).toBe('\\' + variables.ul);
});

it('handles triple escapes', () => {
expect(renderMustacheString('{{{bt}}}', variables, 'markdown')).toBe(variables.bt);
expect(renderMustacheString('{{{bs}}}', variables, 'markdown')).toBe(variables.bs);
expect(renderMustacheString('{{{st}}}', variables, 'markdown')).toBe(variables.st);
expect(renderMustacheString('{{{ul}}}', variables, 'markdown')).toBe(variables.ul);
});

it('handles escape:slack with commonly escaped strings', () => {
expect(renderMustacheString('{{lt}}', variables, 'slack')).toBe('&lt;');
expect(renderMustacheString('{{gt}}', variables, 'slack')).toBe('&gt;');
expect(renderMustacheString('{{amp}}', variables, 'slack')).toBe('&amp;');
expect(renderMustacheString('{{nl}}', variables, 'slack')).toBe(variables.nl);
expect(renderMustacheString('{{dq}}', variables, 'slack')).toBe(variables.dq);
expect(renderMustacheString('{{bt}}', variables, 'slack')).toBe(`'`);
expect(renderMustacheString('{{bs}}', variables, 'slack')).toBe(variables.bs);
expect(renderMustacheString('{{st}}', variables, 'slack')).toBe('`*`');
expect(renderMustacheString('{{ul}}', variables, 'slack')).toBe('`_`');
// html escapes not needed when using backtic escaping
expect(renderMustacheString('{{st_lt}}', variables, 'slack')).toBe('`*<`');
});

it('handles escape:json with commonly escaped strings', () => {
expect(renderMustacheString('{{lt}}', variables, 'json')).toBe(variables.lt);
expect(renderMustacheString('{{gt}}', variables, 'json')).toBe(variables.gt);
expect(renderMustacheString('{{amp}}', variables, 'json')).toBe(variables.amp);
expect(renderMustacheString('{{nl}}', variables, 'json')).toBe('\\n');
expect(renderMustacheString('{{dq}}', variables, 'json')).toBe('\\"');
expect(renderMustacheString('{{bt}}', variables, 'json')).toBe(variables.bt);
expect(renderMustacheString('{{bs}}', variables, 'json')).toBe('\\\\');
expect(renderMustacheString('{{st}}', variables, 'json')).toBe(variables.st);
expect(renderMustacheString('{{ul}}', variables, 'json')).toBe(variables.ul);
});

it('handles errors', () => {
expect(renderMustacheString('{{a}', variables, 'none')).toMatchInlineSnapshot(
`"error rendering mustache template \\"{{a}\\": Unclosed tag at 4"`
);
});
});

const object = {
literal: 0,
literals: {
a: 1,
b: '2',
c: true,
d: null,
e: undefined,
eval: '{{lt}}{{b}}{{gt}}',
},
list: ['{{a}}', '{{bt}}{{st}}{{bt}}'],
object: {
a: ['{{a}}', '{{bt}}{{st}}{{bt}}'],
},
};

describe('renderMustacheObject()', () => {
it('handles deep objects', () => {
expect(renderMustacheObject(object, variables)).toMatchInlineSnapshot(`
Object {
"list": Array [
"1",
"\`*\`",
],
"literal": 0,
"literals": Object {
"a": 1,
"b": "2",
"c": true,
"d": null,
"e": undefined,
"eval": "<2>",
},
"object": Object {
"a": Array [
"1",
"\`*\`",
],
},
}
`);
});

it('handles primitive objects', () => {
expect(renderMustacheObject(undefined, variables)).toMatchInlineSnapshot(`undefined`);
expect(renderMustacheObject(null, variables)).toMatchInlineSnapshot(`null`);
expect(renderMustacheObject(0, variables)).toMatchInlineSnapshot(`0`);
expect(renderMustacheObject(true, variables)).toMatchInlineSnapshot(`true`);
expect(renderMustacheObject('{{a}}', variables)).toMatchInlineSnapshot(`"1"`);
expect(renderMustacheObject(['{{a}}'], variables)).toMatchInlineSnapshot(`
Array [
"1",
]
`);
});

it('handles errors', () => {
expect(renderMustacheObject({ a: '{{a}' }, variables)).toMatchInlineSnapshot(`
Object {
"a": "error rendering mustache template \\"{{a}\\": Unclosed tag at 4",
}
`);
});
});
});
Loading

0 comments on commit 20a7cc9

Please sign in to comment.