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

Prevent CSP violations by not having script URLs #4062

Merged
merged 3 commits into from
Aug 12, 2019
Merged

Conversation

arikfr
Copy link
Member

@arikfr arikfr commented Aug 12, 2019

What type of PR is this? (check all applicable)

  • Bug Fix

Description

Following the discovery in #4039 I searched the code for further locations of where we use this pattern and removed them all. I also restored the lint rule to prevent future cases.

We should probably use <Button type="link" ...> in the future instead of <a> tags in such cases, but didn't want to work around the styling issues so kept things as is.

@@ -173,6 +173,7 @@ body {

&:hover, &:focus {
color: @yellow-darker;
cursor: pointer;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranbena this adds some work for #4017... Sorry, but @gabrieldutra is right: we can YOLO it, but only after V8 branch is cut.

@jezdez
Copy link
Member

jezdez commented Aug 12, 2019

Thank you @arikfr!

@arikfr arikfr merged commit 685b536 into master Aug 12, 2019
@arikfr arikfr deleted the csp-dashboard branch August 12, 2019 10:25
harveyrendell pushed a commit to pushpay/redash that referenced this pull request Nov 14, 2019
* Fix: remove inline script to avoid CSP violation

Closes getredash#4039.

* Restore eslint rule that prevents javascript href attributes.

* Remove all inline script links.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants