ember-auto-import icon indicating copy to clipboard operation
ember-auto-import copied to clipboard

CSP failure in consumers of ember-auto-import

Open sandstrom opened this issue 5 years ago • 8 comments

Scenario

An app [consuming app] uses an addon that has ember-auto-import as a dependency, and that rely on ember-auto-import to work. The consuming app does not use ember-auto-import.

Problem

If a consuming app doesn't allow eval (via CSP) and the addon doesn't explicitly tell ember-auto-import to forbid eval, the compiled output will contain eval statements which will cause failure in the consuming app.

The error in the console will be something like:

Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script

and then:

Could not find module some-npm-package imported from 'name-of-addon/path/to/import/statement'

Possible solution

What's the performance cost? May I suggest that we drop eval altogether? Or at least always drop it when running within the context of an addon (vs an app).

@ef4 Let me know what you think!

sandstrom avatar Apr 08 '20 16:04 sandstrom

The consuming app does not use ember-auto-import.

Even when the consuming app doesn't use ember-auto-import, the app is allowed to include configuration (including forbidEval) in order to control how its addons use ember-auto-import.

This got fixed in https://github.com/ef4/ember-auto-import/pull/213

ef4 avatar Apr 08 '20 22:04 ef4

@ef4 It's good that we can configure the ember-auto-import even though it's not included in our app directly. So it's solved for us now! Thanks! 🏅

But I still think this is an issue we should try to adress. It's not great that when one of our addons switch to ember-auto-import our app breaks + it breaks in a fairly non-obvious way (it took me a while to track it down).

How about dropping eval when running within the context of an addon? They should more or less always be generated only once, right? Because during app development you aren't changing any of the code imported from addons. So they are always cached after the first compile.

Or perhaps we could switch to cheap-source-map or inline-source-map for addons?

https://webpack.js.org/configuration/devtool/

Happy Easter! 🥚

sandstrom avatar Apr 09 '20 09:04 sandstrom

Picking a different default when the app itself doesn't use ember-auto-import is a reasonable idea.

ef4 avatar Apr 09 '20 15:04 ef4

@ef4 Yeah, that would be great I think!

sandstrom avatar Apr 09 '20 15:04 sandstrom

@ef4 I've made a stab at it here: https://github.com/ef4/ember-auto-import/issues/275

No idea if that's a good route to go down. Feel free to discard everything or make any changes you'd like to that PR. I've given you access to the branch, so edit at will.

Also not sure how one would check if the root package is using this addon or not! I guessed an empty options object may be an indicator, but I'm not sure about that.

sandstrom avatar Apr 15 '20 08:04 sandstrom

Picking a different default when the app itself doesn't use ember-auto-import is a reasonable idea.

Maybe the default could also depend on the presence of ember-cli-content-security-policy? There might be cases in which the app depends on the ember-cli-content-security-policy addon but it's disabled for local development. There might be even cases in which the app uses it but includes 'unsafe-eval' in script-src. But both scenarios aren't the common ones.

jelhan avatar Jun 09 '20 15:06 jelhan

Add the following config to ember-cli-build.js has worked for us.

let app = new EmberApp(defaults, {
    autoImport: {
      forbidEval: true
    },

Actually after doing so, it will result in issue like, https://github.com/ef4/ember-auto-import/issues/265.

vendor.js contains multiple map source, like

//# sourceMappingURL=chunk.vendors~app.a9aaaf3b8e7d5be91d4e.js.map//# sourceMappingURL=vendor.map

This results not able to debug third party libraries, like ember-light-table

adong avatar Aug 12 '20 18:08 adong

If the alternative was cheap and worked well, we could definitely change the default here to make it work better for more apps. The problem is that it would probably be a step backward for many apps, because the eval-based sourcemapping is faster and interacts better with ember-cli's other sourcemapping. So it's a tradeoff.

My general takeaway after an awful lot of time down the build systems rabbit hole is that it's better to put responsibility on apps to configure global aspects of their build that are necessarily global, like the forbidEval setting. Trying to do it automatically from addon code ends up being harder to maintain and evolve across an ecosystem.

The fact that an addon can "sneak" ember-auto-import into your app without the app author managing anything about ember-auto-import is nice, but it comes at a cost, and in practice I see that use case declining and moving toward deprecation. App authors really need to be aware of ember-auto-import and control it, because

  • all your addons are going to need to agree on a major version of ember-auto-import (this doesn't need to be very onerous, because our own changes are all highly compatible, it's mostly just if they're using webpack features directly that we can't control). We've never done a breaking major, but we're going to pretty soon to release with webpack 5. If an addon changes the major version it needs, that will need be considered part of that addon's public API guarantee, so that addon will need to do its own semver major.
  • if an addon uses dynamic import, you can end up with lazy chunks in your build output, and you need to be aware of them when deploying your app or it will break in production. You may also need to configure the public asset URL for those lazy chunks, depending on your deployment situation.
  • ember-auto-import is going to be how you consume embroider v2-formatted addons in non-embroider builds, so in practice you're going to need it for more and more. ember-auto-import is effectively a polyfill for embroider, and embroider itself will be available with an official flag in ember-cli 3.27 and become the default for new apps one or two minor releases after that.

In summary, apps need to move toward directly using and configuring ember-auto-import.

ef4 avatar Mar 23 '21 16:03 ef4