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

Tests fail after installing this addon because of ember-cli #199

Closed
boris-petrov opened this issue Apr 20, 2021 · 6 comments · Fixed by #201
Closed

Tests fail after installing this addon because of ember-cli #199

boris-petrov opened this issue Apr 20, 2021 · 6 comments · Fixed by #201
Labels

Comments

@boris-petrov
Copy link

ember-cli adds a script tag (see here) when running tests. This fails the tests unless one adds unsafe-inline to script-src. What are we supposed to do about that? I guess that should be fixed in ember-cli?

@jelhan
Copy link
Collaborator

jelhan commented Apr 20, 2021

What version of ember-cli-content-security-policy are you using? The addon should whitelist that script tag automatically. A fix for Ember CLI >=3.25.1 was just released a few days ago in v2.0.0-3. Please note that it is a prerelease. There hasn't been a stable release of v2 yet. But in my experience it is stable enough to be used in production.

It is very likely that v1 has the same issue. There wasn't a patch release for v1 and it is not planned. Before doing that I would prefer to finally ship a stable version of v2.

@jelhan jelhan added the bug label Apr 20, 2021
@boris-petrov
Copy link
Author

@jelhan - thanks for the support!

I should have been more explicit, sorry about that. I'm talking about the latest prerelase version of ember-cli-content-security-policy - v2.0.0-3 and latest stable ember-cli - 3.26.1.

It's actually trivial to reproduce:

ember new ember-cli-content-security-policy-bug
cd ember-cli-content-security-policy-bug
ember install ember-cli-content-security-policy
ember test -s

And check the HTML - there is the script tag without a nonce. It is strange that the code checks for Ember.assert but in the generated code from ember-cli there is no such thing?

@jelhan
Copy link
Collaborator

jelhan commented Apr 21, 2021

It seems as if the code, which we are trying to rewrite, has changed a lot in the last Ember CLI releases.

This is how it looks like for different versions of Ember CLI:

@snewcomer added support for Ember CLI >= 3.25.1 <= 3.26.0 in #197. I guess we need a similar hot fix for Ember CLI >= 3.26.0. @boris-petrov do you have time to work on such a hotfix?

A long-term strategy, which would not require hard-coding a regular expression for Ember CLI and would also support similar use cases from other addons is discussed in #149.

I'm a little bit concerned that the tests are not failing if run with Ember CLI 3.26.1. There is a test, which should verify that tests are passing for a newly created Ember application: https://github.com/rwjblue/ember-cli-content-security-policy/blob/93924fa54dd472de89ea0b0f5d1f126c40758c9a/node-tests/e2e/test-support-test.js#L69-L75 Will have a look what is going on there.

@snewcomer
Copy link
Contributor

I can try fixing in #198 if that is ok with you! Or if Boris already has a fix in place, that is fine with me to.

@boris-petrov
Copy link
Author

@snewcomer - go ahead, I haven't worked on this.

@jelhan - a new Ember app with all the latest versions does pass the tests, yes. I think it's because for some reason the handler for a CSP violation is not registered - even if I put failTests: true. I didn't dig into why - as looking in the HTML one could see the script tag from ember-cli has no nonce. But yes, if you have tests for that, they should be made to fail.

@rwjblue
Copy link
Member

rwjblue commented Apr 21, 2021

Shoot, sorry for breaking this again @jelhan 🤦

bendemboski added a commit to bendemboski/ember-file-upload that referenced this issue May 2, 2021
Update `ember-cli-content-security-policy` to the latest for [email protected] support, and pin ember-cli to 3.25.1 pending a fix for adopted-ember-addons/ember-cli-content-security-policy#199
bendemboski added a commit to bendemboski/ember-file-upload that referenced this issue May 2, 2021
Update ember-cli-content-security-policy to the latest for [email protected] support, and pin ember-cli to 3.25.1 pending a fix for adopted-ember-addons/ember-cli-content-security-policy#199
bendemboski added a commit to bendemboski/ember-file-upload that referenced this issue May 2, 2021
* update ember-cli-content-security-policy to the latest for [email protected] support
* pin ember-cli to 3.25.1 pending a fix for adopted-ember-addons/ember-cli-content-security-policy#199
* disable the use of `eval()` in embroider scenarios to prevent violating the unsave-eval rule
bendemboski added a commit to bendemboski/ember-file-upload that referenced this issue May 2, 2021
* update ember-cli-content-security-policy to the latest for [email protected] support
* pin ember-cli to 3.25.1 pending a fix for adopted-ember-addons/ember-cli-content-security-policy#199
* disable the use of `eval()` in embroider scenarios to prevent violating the unsave-eval rule
bendemboski added a commit to bendemboski/ember-file-upload that referenced this issue May 2, 2021
* update ember-cli-content-security-policy to the latest for [email protected] support
* pin ember-cli to 3.25.1 pending a fix for adopted-ember-addons/ember-cli-content-security-policy#199
* disable the use of `eval()` in embroider scenarios to prevent violating the unsave-eval rule
bendemboski added a commit to bendemboski/ember-file-upload that referenced this issue May 31, 2021
* update ember-cli-content-security-policy to the latest for [email protected] support
* pin ember-cli to 3.25.1 pending a fix for adopted-ember-addons/ember-cli-content-security-policy#199
* disable the use of `eval()` in embroider scenarios to prevent violating the unsave-eval rule
bendemboski added a commit to bendemboski/ember-file-upload that referenced this issue May 31, 2021
* update ember-cli-content-security-policy to the latest for [email protected] support
* pin ember-cli to 3.25.1 pending a fix for adopted-ember-addons/ember-cli-content-security-policy#199
* disable the use of `eval()` in embroider scenarios to prevent violating the unsave-eval rule
jelhan pushed a commit to adopted-ember-addons/ember-file-upload that referenced this issue May 31, 2021
* v3.5.1...v3.26.0

* Fix linting

auto-fix a bunch of stuff (a lot of prettier-related changes), manually fix up a few easy-to-fix things, and add linting overrides for the remainder of the failures, leaving that as tech debt for later

* Update addon docs addons

* Embroider compatibility

These are just fixes to tests to pass under embroder:

* update our dynamic imports to be embroider-compatible
* something about rendering ember-cli-addon-docs components under embroider is causing problems, so put the UI that we need for our upload test into a component and test it in a rendering test
* remove the trim polyfill and uuid branching behavior -- these aren't directly embroider issues, but the way we tested them was failing with the optimized embroider config, and the behaviors they were polyfilling have been implemented in major browser for a long time now

* Fix CSP

* update ember-cli-content-security-policy to the latest for [email protected] support
* pin ember-cli to 3.25.1 pending a fix for adopted-ember-addons/ember-cli-content-security-policy#199
* disable the use of `eval()` in embroider scenarios to prevent violating the unsave-eval rule

* Remove resolutions

Now that we've upgraded a bunch of dependencies, we no longer need to pin these
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants