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

content_security_policy - EvalError: Refused to evaluate a string #4535

Closed
goatonabicycle opened this issue Dec 18, 2020 · 17 comments · Fixed by #4577
Closed

content_security_policy - EvalError: Refused to evaluate a string #4535

goatonabicycle opened this issue Dec 18, 2020 · 17 comments · Fixed by #4577
Labels
status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer

Comments

@goatonabicycle
Copy link

The problem: I've had to add the following line to a chrome extension manifest file in order for the latest version of Mocha to work: "content_security_policy": "script-src 'self' 'unsafe-eval'; object-src 'self';",

Without this line, mocha scripts fails to run with the following error:

mocha.js:13165 Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'self' blob: filesystem:".

    at Function (<anonymous>)
    at mocha.js:13165
    at createCommonjsModule (mocha.js:16)
    at mocha.js:12456
    at mocha.js:4
    at mocha.js:5

The mentioned workaround was found here: https://stackoverflow.com/a/48047578 but ideally we don't want to meddle with the content_security_policy

The error seems related to the following piece of code in mocha.js :

	  // In sloppy mode, unbound `this` refers to the global object, fallback to
	  // Function constructor if we're in global strict mode. That is sadly a form
	  // of indirect eval which violates Content Security Policy.
	  function () {
	    return this;
	  }() || Function("return this")());

This looks like a regression in the newer versions of Mocha as this issue doesn't occur after downgrading to Mocha 8.1.3 .

What I've tried:
Experimented with different devtool settings including "cheap-module-source-map" and false

How I'm running Mocha: Slightly differently because of testing a chrome extension with a specific page. But the core idea works in other projects and shouldn't be related to the described error.

mocha.setup("bdd");
mocha.timeout(8000);
// set up all the relevant tests
mocha.run()

How I'm working around this at the moment: Downgraded to Mocha 8.1.3

@outsideris outsideris added the type: bug a defect, confirmed by a maintainer label Jan 4, 2021
@outsideris
Copy link
Member

I don't know the root cause, but we introduced it in 12db9db after I tracked it via git bisect.

@snoack
Copy link
Contributor

snoack commented Feb 17, 2021

This issue keeps us for a while from updating to any version of Mocha newer than 8.1.3, and I assume that applies to everyone who uses Mocha to test a browser extension. Right now, we could as a workaround relax the content-security-policy in the extension's manifest.json. But we don't want to run our tests in an environment that is unnecessarily different from the production environment. Even more importantly, with manifest v3, this will no longer be an option. Consequently, if this issue is not being addressed, it means that Mocha is no longer a solution for testing browser extensions, and anyone using it for this purpose should start looking for alternatives.

@juergba
Copy link
Member

juergba commented Feb 17, 2021

@snoack what about a PR from your side?

@juergba juergba added the status: accepting prs Mocha can use your help with this one! label Feb 17, 2021
@goatonabicycle
Copy link
Author

Hello.

So, this issue was investigated with an eye on seeing how to help out or at least get some direction.

Like @outsideris mentioned, a git bisect shows that commit 12db9db does introduce the code block mentioned above (searching for "sloppy" in the build output to quickly find it)

It seems like that block of code gets indirectly introduced by regenerator-runtime. Which seems to tie in with the global setup stuff mentioned in this release: https://github.com/mochajs/mocha/releases/tag/v8.2.0

Relevant to this discussion is also this post which seems to very closely mirror this exact issue we're experiencing.

@boneskull Sorry for the random ping but would you perhaps be able to add some insight into this?

Thank you so much!

@snoack
Copy link
Contributor

snoack commented Feb 17, 2021

As @goatonabicycle concluded above, the offender here is regenerator-runtime. Babel automatically includes regenerator-runtime into the generated code when transpiling any generator functions or async functions. Since 12db9db is the first commit that made use of async/await this change caused the regression here.

If it's acceptable to drop support for Internet Explorer 11 (which is the only browser listed in .browserslistrc that doesn't have native generator support), all it would take to fix this bug is telling Babel to just not transpile generator functions:

diff --git a/rollup.config.js b/rollup.config.js
index 4b4f3494..5fae0247 100644
--- a/rollup.config.js
+++ b/rollup.config.js
@@ -43,5 +43,6 @@ const config = {
               version: 3,
               proposals: false
-            }
+            },
+            exclude: ["transform-regenerator"]
           }
         ]

Note that async functions will still work on browser that lack native support as long as they support generators.

@juergba, what do you think?

@juergba
Copy link
Member

juergba commented Feb 18, 2021

@goatonabicycle @snoack thanks for your support. I'm willing to solve this issue, but you have to take the lead.
I lack the knowledge and will have to ask questions.

Unfortunately we haven't dropped the support of IE11. But we started using modern Javascript and transpiling since Mocha v8.1.0.
So I'm surprised that v8.1.3 is working as expected.
Whatever we will change, Mocha has to keep supporting lovely IE11.

If it's acceptable to drop support for Internet Explorer 11 [...]

I'm afraid no.

Since 12db9db is the first commit that made use of async/await this change caused the regression here.

This commit landed in v8.2.0, and would explain why v8.1.3 is still working.
I will have to check wether we haven't used async/await before.

Do you have any other suggestions, please? @outsideris ?

@snoack
Copy link
Contributor

snoack commented Feb 18, 2021

The obvious solution if supporting Internet Explorer is a requirement would be to refactor the code to again not use async functions.

Or if you prefer to keep using async, you could provide two builds, one using the suggested Rollup/Babel configuration that will work without unsafe eval, but not on IE, and another build with the current configuration that will work on IE but not without unsafe eval.

@juergba
Copy link
Member

juergba commented Feb 18, 2021

The obvious solution if supporting Internet Explorer is a requirement [...]

No, no way!

facebook still breaking things:

Things work in the browser when using Mocha 7.0.1, but not Mocha 8.1.0. The breaking change is in Mocha 8.x.x.

This PR states, that v8.1.3 is working, above link says v8.1.0 isn't working. I'm starting to doubt that the root cause is the use of async/await.

This sloppy mode guilty code snippet: is this a Babel or a Rollup result?

@snoack
Copy link
Contributor

snoack commented Feb 18, 2021

This PR states, that v8.1.3 is working, above link says v8.1.0 isn't working.

Indeed, this doesn't quite match up. But I just double checked, and this regression was introduced in 8.2.0. 8.1.0 and 8.1.3 work fine and don't include the offending code from regenerator-runtime. I can only assume that this it a typo in the blog post. Maybe @dfkaye who wrote that blog post can confirm?

I'm starting to doubt that the root cause is the use of async/await.

If in doubt just remove any async & await statements from lib/mocha.js and lib/runner.js (don't bother to keep the code functional as long as it can still be parsed). Then build the bundle before and after doing so. You will notice two important differences:

  1. The offending Function("return this") is gone if async/await isn't used.
  2. In stats.html you can see that regenerator-runtime is imported by lib/mocha.js and lib/runner.js which happen to be the only modules using async functions. After removing any async code, regenerator-runtime is no longer bundled at all.

This sloppy mode guilty code snippet: is this a Babel or a Rollup result?

Babel, or rather regenerator-runtime which is bundled by Babel if transforming any generator functions (and async functions since they are internally transformed to generators first).

@juergba
Copy link
Member

juergba commented Feb 18, 2021

We all are sick of IE11, but decided to keep supporting it.
In case we want to build a second bundle for modern non-IE11 browsers:

  • we don't have to transpile, just bundle, right? We are using ES2018.
  • can you support me in configuring this bundling process?
  • where do I put this new bundle? In our npm package? What about its growing size?
  • does shipping the huge mocha.js.map make any sense?

@dfkaye
Copy link

dfkaye commented Feb 18, 2021

@snoack - First, thanks for pinging me - that blog post morphed a couple times as I worked through the problems - sorry for the confusion.

Clarifications in a nutshell:

  • I think the problem I had with 8.1 is actually a setup issue I wrote about in the 2 December 2020 note.
  • 8.2.0 is the version I was using that resulted in no-eval problems.
  • 8.3.0 has been working fine.
  • And finally, big caveat, I don't bundle or transpile for my blog - which means I can't verify whether 8.3 works on IE 11.

Let me know if you need more info or further clarification.

@snoack
Copy link
Contributor

snoack commented Feb 18, 2021

In case we want to build a second bundle for modern non-IE11 browsers

There seems to be another option. If we update regenerator-runtime to the latest version, it uses a different approach to set a variable in the global scope:

try {
  regeneratorRuntime = runtime;
} catch (accidentalStrictMode) {
  // This module should not be running in strict mode, so the above
  // assignment should always work unless something is misconfigured. Just
  // in case runtime.js accidentally runs in strict mode, we can escape
  // strict mode using a global Function call. This could conceivably fail
  // if a Content Security Policy forbids using Function, but in that case
  // the proper solution is to fix the accidental strict mode problem. If
  // you've misconfigured your bundler to force strict mode and applied a
  // CSP to forbid Function, and you're not willing to fix either of those
  // problems, please detail your unique predicament in a GitHub issue.
  Function("r", "regeneratorRuntime = r")(runtime);
}

This will still fail in strict mode without unsafe eval. But now we can just add var regeneratorRuntime; to the top of the generated code (with Rollup's output.intro setting). That way regeneratorRuntime = runtime is no longer an implicit definition of a global variable (which fails in strict mode), but it becomes just a reassignment of an existing variable which never fails, and so the code in the catch block (which requires unsafe eval) is never reached.

This will give us a bundle that will work both on Internet Explorer 11, and in environments that don't allow unsafe eval.

@juergba
Copy link
Member

juergba commented Feb 19, 2021

Thanks @dfkaye for your feedback.
@snoack your proposition sounds very promising, I will check your PR 👍 .

@juergba juergba linked a pull request Feb 19, 2021 that will close this issue
@juergba
Copy link
Member

juergba commented Feb 20, 2021

8.3.0 has been working fine.

Above statement of @dfkaye is worrying me. Are we hunting ghosts?
@snoack does v8.3.0 work without any patch?

And finally, big caveat, I don't bundle or transpile for my blog - which means I can't verify whether 8.3 works on IE 11.

The bundled file is part of Mocha's npm package.

@snoack
Copy link
Contributor

snoack commented Feb 20, 2021

Mocha 8.3.0 does not work without unsafe eval. Feel free to see for yourself. Here is a simple test case. Just put this code into an HTML file and open it in Chrome or Firefox:

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Security-Policy" content="script-src https: 'unsafe-inline'">
    <link rel="stylesheet" href="https://unpkg.com/mocha@8.3.0/mocha.css" />
  </head>
  <body>
    <div id="mocha"></div>
    <script src="https://unpkg.com/mocha@8.3.0/mocha.js"></script>
    <script>
      mocha.setup('bdd');
      it('works', () => {});
      mocha.run();
    </script>
  </body>
</html>

You will see a blank page, and find an EvalError logged in the console.

@snoack
Copy link
Contributor

snoack commented Mar 5, 2021

Is there an ETA for a release with this fix?

@juergba
Copy link
Member

juergba commented Mar 5, 2021

This weekend, probably.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants