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

[alerts] fix up the docs on mustache escaping #87933

Closed
pmuellr opened this issue Jan 11, 2021 · 2 comments · Fixed by #88521
Closed

[alerts] fix up the docs on mustache escaping #87933

pmuellr opened this issue Jan 11, 2021 · 2 comments · Fixed by #88521
Assignees
Labels
needs_docs Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@pmuellr
Copy link
Member

pmuellr commented Jan 11, 2021

The current docs for mustache escaping of alert context variables in action parameter templates isn't quite right - from about 2/3 down on the page https://www.elastic.co/guide/en/kibana/7.11/defining-alerts.html :

Using the Mustache template syntax {{variable name}}, you can pass alert values at the time a condition is detected to an action. Note that using two curly braces will escape any HTML. Should you need to preserve HTML, use three curly braces ({{{). Available variables differ by alert type, and a list can be accessed using the "add variable" button.

What's not quite right about it is that we changed the way we do escaping, so that most of the time, there is no longer any escaping going on at all, as it's not really needed. We should not be referencing HTML at all, as we no longer do any HTML escaping ever.

Previously, we always escaped every context variable for every action parameter template as HTML. Because that's what the version of mustache we are using does.

With PR #83919 , this changed. We now only escape three action parameter templates - message for email and slack, and body for webhook. In all cases, the escaping we do is the escaping of the format the parameters are intended to be in; Markdown escaping for message for email, Slack markdown escaping for message in slack, and JSON escaping for body in webhook.

At this point, it's unlikely that anyone should have to use triple braces - because we're doing more precise escaping than we were before. Folks used to use triple braces so that you wouldn't see < chars turned into &lt; (or anything that would need to be escaped for HTML). With the new escaping, it should do the right thing, all the time.

There's still a potential that customers may need to use triple braces, if eg our escaping logic is not correct, or they REALLY want the data in the context parameters to be interpreted as markdown (seems a stretch, but ???; eg of this would be some context variable having the value *some text*, where the customer would want those asterisks interpreted as "Slack bold formatting"). However, using triple braces now could potentially mess up the expected formatting for the action parameters that we do escaping for. Eg, if you use triple braces in the JSON body of a webhook, and the variable in the braces contains a newline character, it won't be escaped and will end up rendering the JSON as unparseable (one of the problems that caused us to fix the escaping). So we definitely should not recommend it be used. Mentioning that it can be used seems fine, but makes me wonder if we can describe all this in an easy-to-comprehend way.

@pmuellr pmuellr added needs_docs Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Jan 11, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

@pmuellr pmuellr self-assigned this Jan 12, 2021
@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
@chadnormanimpact
Copy link

Has anyone got this working, I really want to surface user.email in my alerts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_docs Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants