[Alerts] unable to use a context variable with an embedded newline in a webhook action

[pmuellr]

Working with a customer, and though I don't know for sure this is their problem, feels like it probably is.

They are using a webhook action, with a JSON mustache template, that works in one case, but when they add an additional context variable to one of the property values in the JSON, they are getting a 500 response from the service they are calling.

My guess is the variable contains a " character. If so, it would end up rendering invalid JSON.

Eg,

template: {"foo": "{{context.bar}}"}

if context.bar evaluates to the string xyz, the result of rendering the template will be {"foo": "xyz"}, which is fine.

if context.bar evaluates to the string x"z, the result of rendering the template will be {"foo": "x"z"}, which is no longer valid JSON, and won't be usable in any webhook expecting JSON.

I don't know of any work-around for this ATM, nor am I sure how we can make fixes to make this work, given the way the mustache templating currently works with alert params.

[elasticmachine]

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

[pmuellr]

Turns out it wasn't a " char, but a new line char \n - same problem, yields invalid JSON as you can't use multi-line strings in JSON.

[pmuellr]

To be able to handle this, we're going to need to do different escaping for different types of content - at least Slack markdown, "regular" markdown (for email), JSON for the index action and webhook, and perhaps others for webhook (XML?).

There is a way to customize the "escaper" in the mustache module we're using, shown here:

  // Export the escaping function so that the user may override it.
  // See https://github.com/janl/mustache.js/issues/244
  mustache.escape = escapeHtml;

The referenced issue contains some discussion, none terribly useful for us ATM, other than indicating this facility exists.

It appears that mustache doesn't do any context switches (async, callbacks, etc), so in theory we can change the escaper before calling into mustache, then change it back when we're done, without affecting any other callers of the module. Kinda awful, but should work.

We'll need to build out the escapers we need, and I think we'll need to allow webhook action to indicate WHICH escaper to use; we could try to guess via a provided content-type header, but I'd guess that's not used all the time. Feels like the "escaper" to use for webhook is going to have to be a config property.

[pmuellr]

I should note that this assumes customers that customers aren't relying on other "structure" in their context variables. Eg, in theory you could do this with a JSON mustache template string, or similar, with the idea being that context.additionalProperties could be set to the string"bar": "foo":

{
  "foo": "bar",
  {{context.additionalProperties}}
}

I would be very surprised if there was an existing context variable that could be used in that situation today, but hopefully you get the gist.

The intention would be that you can only use context variables where "plain text strings" (or numbers, or booleans) could be used, no embedded markup/structuring .

[pmuellr]

Another thought is to use a different templating system - depending on if it was wildly different than mustache, we might need to allow the customer to choose which one to use, which would get complicated. There are some templating systems that allow you to filter the value with additional built-in functions, I think typically using a | char as a separator. Eg, you could imagine

{{context.someVariable | trimString | escapeForJSON}}

where trimString and escapeForJSON are functions we provide, that do what you would expect, so the resulting value would be trimmed and then JSON-escaped before rendered into the template.

[pmuellr]

I took a look at handlebars as a replacement for mustache, but two things:

• there are existing security concerns using it
• it's waaaay more complicated, and I don't really want to put more complication in front of users; it's really designed for developers, not as a end-user templating story

[pmuellr]

I started looking into a story where we can allow actions to determine how to escape strings in their parameters. Probably this would evolve to the point where an action type would need to declare an entrypoint to "expand it's templates", and then can expand whatever params they want (and NOT expand params they DON'T want - something we don't support today!), and indicate how to do escaping of the content.

Initial bits in a branch here: master...pmuellr:actions/per-type-mustache-escaping

[pmuellr]

In addition to the story outlined in #79371 (comment), which I think we should certainly do long-term, wondering if there's a simpler fix for short-term, since this is a pretty serious issue. Mainly affects JSON-based actions - webhook and index.