Skip to content

Commit

Permalink
[ResponseOps] Hyperlinks in Slack messages are broken when there is "…
Browse files Browse the repository at this point in the history
…_" or "*" in the URL (elastic#170067)

Resolves elastic#165673

## Summary
`escapeSlack` function in the mustasche_renderer breakes the hyperlinks
by wrapping the URL with backticks.
So the below markdown template does not work.

`<{{context.alertDetailsUrl}}|View alert details>`

This PR removes the code that adds backticks. 
With this change action variables with text wrapped in asterisks
\*text\* will render as **text** or wrapped in underscores \_text\_ will
render as _text_ .


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios


### To verify

- Create a slack connector
- Create a rule that uses a slack connector and use the above markdown
template
- Verify that hyperlink is working properly in slack
  • Loading branch information
doakalexi authored Oct 30, 2023
1 parent e17988c commit 8fe512b
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 5 deletions.
2 changes: 2 additions & 0 deletions x-pack/plugins/actions/server/lib/mustache_renderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const variables = {
ul: '_',
st_lt: '*<',
vl: '|',
link: 'https://te_st.com/',
};

describe('mustache_renderer', () => {
Expand Down Expand Up @@ -111,6 +112,7 @@ describe('mustache_renderer', () => {
expect(renderMustacheString('{{ul}}', variables, 'slack')).toBe('`_`');
// html escapes not needed when using backtic escaping
expect(renderMustacheString('{{st_lt}}', variables, 'slack')).toBe('`*<`');
expect(renderMustacheString('{{link}}', variables, 'slack')).toBe('https://te_st.com/');
});

it('handles escape:json with commonly escaped strings', () => {
Expand Down
10 changes: 7 additions & 3 deletions x-pack/plugins/actions/server/lib/mustache_renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,14 @@ function escapeSlack(value: unknown): string {
if (value == null) return '';

const valueString = `${value}`;
// if the value contains * or _, escape the whole thing with back tics
// if the value contains * or _ and is not a url, escape the whole thing with back tics
if (valueString.includes('_') || valueString.includes('*')) {
// replace unescapable back tics with single quote
return '`' + valueString.replace(/`/g, `'`) + '`';
try {
new URL(valueString);
} catch (e) {
// replace unescapable back tics with single quote
return '`' + valueString.replace(/`/g, `'`) + '`';
}
}

// otherwise, do "standard" escaping
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const EscapableStrings = {
escapableHtml: '<&>',
escapableDoubleQuote: '"double quote"',
escapableLineFeed: 'line\x0afeed',
escapableLink: 'https://te_st.com/',
};

export const DeepContextVariables = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon
// from x-pack/test/alerting_api_integration/common/plugins/alerts/server/alert_types.ts,
// const EscapableStrings
const template =
'{{context.escapableBacktic}} -- {{context.escapableBold}} -- {{context.escapableBackticBold}} -- {{context.escapableHtml}}';
'{{context.escapableBacktic}} -- {{context.escapableBold}} -- {{context.escapableBackticBold}} -- {{context.escapableHtml}} -- {{context.escapableLink}}';

const rule = await createRule({
id: slackConnector.id,
Expand All @@ -95,7 +95,9 @@ export default function executionStatusAlertTests({ getService }: FtrProviderCon
});

const body = await retry.try(async () => waitForActionBody(slackSimulatorURL, rule.id));
expect(body).to.be("back'tic -- `*bold*` -- `'*bold*'` -- &lt;&amp;&gt;");
expect(body).to.be(
"back'tic -- `*bold*` -- `'*bold*'` -- &lt;&amp;&gt; -- https://te_st.com/"
);
});

it('should handle context variable object expansion', async () => {
Expand Down

0 comments on commit 8fe512b

Please sign in to comment.