1530607 - Stop using ajv in custom actions to avoid the need for script-src 'unsafe-eval'

Status: NEW

(Tree Management :: Treeherder, enhancement, P1)

Description

[Ed Morley [:emorley]]

The custom actions feature uses both ajv and js-yaml to process the schema:
https://github.com/mozilla/treeherder/blob/0ab0fca64681718afcdd52999d1085c46384c72e/ui/job-view/CustomJobActions.jsx#L84-L125

Unfortunately the use of ajv.compile() requires the use of CSP 'unsafe-eval', (see bug 1530602), which is insecure.

Also both ajv and js-yaml are pretty heavyweight, so would be great to remove from the bundle if possible.

Hassan, do you have any thoughts about how best to do this whilst not losing too much functionality? :-)

Comment 1

[Hassan Ali (:hassan)]

One option is to remove usage of ajv.compile and not validate the schema. I wonder if validating in the server or at least validating only when there is an error would help. This would allow us to return better error messages that would prevent issues like https://github.com/taskcluster/taskcluster-tools/issues/548. Dustin, thoughts on using ajv in the server to return better error messages?

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Ajv's use of eval is not unsafe -- it is constructing JS expressions and evaluating them into functions for speed. I agree that it's a big chunk.

This validation is not a security boundary, so skipping it does not risk compromise of Firefox, although validation of the hook's triggerPayload does. It's important to keep such security considerations in mind!

Yes, this could be done server-side, assuming by "server side" you mean in the treeherder backend. That's in Python so I don't think you'd use Ajv there.

Comment 3

[Ed Morley [:emorley]]

I agree ajv.compile is safe, but that's not the point :-)

Use of eval() means having to include 'unsafe-eval' in the Content-Security-Policy header, which turns off most of the protection that CSP can provide. ie: It's an anti-pattern discouraged in modern web apps.

Comment 4

[Ed Morley [:emorley]]

(A significant motivation for us enabling CSP beyond just the security teams wanting all Mozilla properties to do so, is that Treeherder now has Auth0 and Taskcluster tokens in localstorage, so we would like to do our best to protect them :-) )

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

So right now, we don't have any awareness of actions on the "server side" of taskcluster (that is, in the services). We could think through the implications of adding an "executeAction" API method somewhere (maybe on a dedicated service?) which would download actions.json, etc., on the user's behalf. Figuring out the permissions checks for that might be difficult!