mocha icon indicating copy to clipboard operation
mocha copied to clipboard

feat: support running with TrustedTypes enforced

Open rictic opened this issue 5 years ago • 11 comments

Description of the Change

Trusted Types is a new Content Security Policy specification, currently implemented in browsers based on Chromium 83 or higher, which requires that data passed to APIs which may result in arbitrary code execution must go through an explicit policy. This helps to catch unintended use of dangerous APIs, and reduces the surface area for some security reviews.

I'm not sure if test infrastructure like mocha is a likely target for attack – seems like in most cases an attacker could only access test data, and it is rare for tests to handle untrusted data. However, there's value for infrastructure to be compatible with running with Trusted Types enabled, as it will allow users to write tests to ensure that the code under test can run with Trusted Types.

This change creates and applies policies for the two places in mocha that call innerHTML, and adds a temporary patch to the rollup build. With those changes in place, we can run mocha's karma tests with Trusted Types enabled (save for the one test that runs with requirejs, which relies on eval).

Alternate Designs

It would be possible to integrate dompurify into the HTML reporter, which would make it more secure. Alternatively, it may be possible to do a larger refactoring to construct the HTML output in a more structured way, rather than using string concatenation of HTML.

Why should this be in core?

This must be in core in order for these parts of core to be trusted types compatible.

Benefits

Users will be able to test their code with mocha in a browser with native Trusted Types enforcement enabled.

Possible Drawbacks

This change adds additional code, which adds weight to the mocha package for users, and maintenance burden on the mocha core devs.

Our own dependencies may inadvertently break us by doing otherwise safe operations (e.g. eval(str), Function(str), setting innerHTML, etc).

This is an enhancement (minor release).

More info and related work

  • Spec: https://w3c.github.io/webappsec-trusted-types/dist/spec/#introduction
  • Related PR adding support to karma: https://github.com/karma-runner/karma/pull/3360

rictic avatar Sep 15 '20 00:09 rictic

CLA assistant check
All committers have signed the CLA.

jsf-clabot avatar Sep 15 '20 00:09 jsf-clabot

Coverage Status

Coverage increased (+0.007%) to 93.966% when pulling 095bc9118b779f662b9cac9ebdc2b3fcf91050cb on rictic:trustedTypes into 738a57574f3eb82ba2844c8a512cafec974ed5ab on mochajs:master.

coveralls avatar Sep 15 '20 01:09 coveralls

Further, I think it'd be good to see a test which asserts the behavior works as intended.

boneskull avatar Sep 25 '20 20:09 boneskull

My main question is: if a test is written using eval(), and we ship this change, will the test now fail when run in a Trusted-Types-supporting browser? If so, this behavior must be opt-in. And if it needs to be opt-in, does this defeat the purpose?

Trusted type enforcement is opt in. Like other CSP features, you enable it with either an HTTP header or with a <meta> tag in the page's <head>.

The change should have no effect on existing tests, which won't have opted into enforcing Trusted Types.

Shipping this change will make mocha compatible with trusted types, so it will enable testing with mocha with TT enforced. See e.g. https://github.com/Polymer/lit-html/pull/1323.

So tests that use potential unsafe APIs like eval() will not be broken by this change. They'd need to take the additional step of configuring their test runtime to enforce trusted types, in which case breaking a naive use of eval() would be the point.

Further, I think it'd be good to see a test which asserts the behavior works as intended.

See the changes to karma.conf.js, which adds headers to enable Trusted Type enforcement for mocha's existing browser tests (in supporting browsers, which is currently browsers based on a recent Chromium like Brave, Chrome, and Edge).

rictic avatar Sep 26 '20 22:09 rictic

Further, I think it'd be good to see a test which asserts the behavior works as intended. See the changes to karma.conf.js, which adds headers to enable Trusted Type enforcement for mocha's existing browser tests (in supporting browsers, which is currently browsers based on a recent Chromium like Brave, Chrome, and Edge).

sorry, what I meant was that the existing tests assert compatibility, but we do not have:

  • a test asserting some sort of exception is thrown when unsafe code is run
  • a test asserting no such exception is thrown when unsafe code is run

boneskull avatar Sep 29 '20 23:09 boneskull

(AppVeyor is no longer building our PRs; I've removed it from the list of checks)

boneskull avatar Sep 29 '20 23:09 boneskull

I'm not 100% sure I follow. You'd like to see a test of native Trusted Types functionality? A test that only runs in Chromium browsers and would fail if the trusted types headers in karma.conf.js didn't result in TT being enforced?

rictic avatar Sep 30 '20 05:09 rictic

I'm not 100% sure I follow. You'd like to see a test of native Trusted Types functionality? A test that only runs in Chromium browsers and would fail if the trusted types headers in karma.conf.js didn't result in TT being enforced?

Yes, essentially:

  • a test that runs in Chromium that asserts "unsafe" code will break
  • a test that runs either in Chromium w/ the headers disabled, or somewhere else that lacks support, and asserts "unsafe" code does not break

The point is not to test the browser feature--I'm sure Chromium has plenty of tests--but rather to establish a baseline of behavior which Mocha will exhibit when encountering these configurations. We need to make sure that future changes in Mocha do not violate the assumptions of its behavior established here.

boneskull avatar Oct 02 '20 21:10 boneskull

I need to learn about TrustedTypes.

outsideris avatar Oct 03 '20 09:10 outsideris

@rictic are you able to continue on this PR?

boneskull avatar Nov 04 '20 20:11 boneskull

👋 coming back to this @rictic, are you still interested in working on this PR? As of https://github.com/mochajs/mocha/issues/5027 there are again maintainers who can review it now. No worries if you no longer have time - but if you do that'd be great!

JoshuaKGoldberg avatar Mar 02 '24 16:03 JoshuaKGoldberg