Web Hook Rules check http headers in case sensitive manner

[amanda11]

SUMMARY

The case used for the header name in trigger.headers[] in a web-hook rule is treated in a case sensitive manner. HTTP headers are case insensitive so the case of the name in the headers should not e relevant.

STACKSTORM VERSION

3.2.0

OS, environment, install method

Seen on one-line install and HA

Steps to reproduce the problem

See #4995 for initial case.

1. Configure webhookrule with trigger.headers['X-GitHub-Event']
2. Send in header via curl of X-GitHub-Event to webhook
3. Rule doesn't match
4. Change rule to be trigger.headers['X-Github-Event'] - rule matches

Expected Results

As http headers are case insensitive then it should not matter what case is used in the rule. Therefore no matter what case header is or case of rule then they should match.

Actual Results

Only matched when rule defined as X-Github-Event

[amanda11]

@armab Issue raised as discussed. I can take a look at a PR fix for this.

[nmaludy]

Someone brought up this problem in slack today, so i started looking into it.

The hard part that i see is around backwards compatability. If someone has already written a rule with a criteria:

trigger.headers['X-Github-Token']:
        type: "eq"
        pattern : "customvalue"

We need to provide this same key to them. The trigger instance itself is just a JSON blob and we used jsonpath_rw to render the key's expression into a value.

• https://github.com/StackStorm/st2/blob/master/st2reactor/st2reactor/rules/filter.py#L140
• https://github.com/StackStorm/st2/blob/master/st2reactor/st2reactor/rules/filter.py#L86
• https://github.com/StackStorm/st2/blob/master/st2reactor/st2reactor/rules/filter.py#L31
• https://github.com/StackStorm/st2/blob/master/st2common/st2common/util/payload.py#L34-L39
• https://github.com/StackStorm/st2/blob/master/st2common/st2common/util/payload.py#L17

Idea 1
One thought i had on how to fix or workaround this and still provide backwards compatability:

In the webhook trigger instances we could return another parameter that is the exact same headers dictionary but with all of the keys set to lowercase, maybe call it something like headers_lower and then your code would look like:

trigger.headers_lower['x-github-token']:
        type: "eq"
        pattern : "customvalue"

This extra field would need to be inserted here (maybe other places too, but this is the first one i found):

• https://github.com/StackStorm/st2/blob/master/st2api/st2api/controllers/v1/webhooks.py#L161

Idea 2
My other thought would be to use some other expression language (Jinja, etc) to allow some more complex expressions so we can manipulate the header keys. Maybe providing a custom filter dict_lower_keys that takes a dictionary as input and returns a dictionary with all of the keys lowercased as output, example:

"{{ (trigger.headers | dict_lower_keys)['x-github-token'] }}":
        type: "eq"
        pattern : "customvalue"

I think this would take a decent amount of work though in order to detect if the expression is Jinja or not and then render it properly. Might be useful in some other scenarios though?

We would need to make this change here:

• https://github.com/StackStorm/st2/blob/master/st2reactor/st2reactor/rules/filter.py#L140

[arm4b]

Looking at the #5038 (Thanks @Rand01ph for opening a PR)

In this situation:

trigger.headers['X-Github-Token']:
        type: "eq"
        pattern : "customvalue"

@nmaludy what if we convert to lower both trigger.headers and pattern ?
This way they'll be lowercase in any way after transformation, no matter how user represents them in the rule.

It will be fully backward compatible and transparent to the user. Any drawbacks?

[nmaludy]

@armab I don't think this will work, because we don't want the pattern to be lowercase because this is trying to match the value inside the header.

[arm4b]

@nmaludy Yeah, my bad. I was too fast on that.

So just a header name/key then?
Can we lower-case the actual header key and its trigger.headers['X-Github-Token']: during the transformation eg. on a key lookup we lower-case it. Same for payload?

This means no matter what case user writes in trigger.headers, eg. trigger.headers['X-Github-Token']:, trigger.headers['x-github-Token']:, trigger.headers['x-github-token']:, - it'll be transformed into the lower-case format. This would be a special case for headers only.

This looks more like an Idea 2 from your write-up, but more changes hidden under the st2 hood.

[arm4b]

@nmaludy In other words, to avoid incompatibility issue you mentioned we can transform the trigger.headers to lowercase upon content registration when the rule metadata is imported into the DB.
And so trigger.headers['X-Github-Token']: written in yaml rule becomes trigger.headers['x-github-token']: when registered.

Hope that clarifies the idea now. WDYT?

[cognifloyd]

#5038 is ready to merge with the trigger.headers_lower solution.

[cognifloyd]

If we're going to do any special handling for trigger.headers, I would recommend we exchange the dictionary for a requests.structures.CaseInsensitveDictionary: https://docs.python-requests.org/en/master/_modules/requests/structures/

But, that depends on someone actually doing the work. The triggers.headers_lower PR is available now.