-
Notifications
You must be signed in to change notification settings - Fork 244
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
Add in SecureHeaders, move config over to that #1102
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1102 +/- ##
==========================================
- Coverage 96.54% 96.53% -0.01%
==========================================
Files 50 50
Lines 1041 1038 -3
==========================================
- Hits 1005 1002 -3
Misses 36 36
Continue to review full report at Codecov.
|
I like this a lot. After an evening of self clowning I think it might be a good idea to roll with a CSP like this and progressively work on getting inline javascript out of the app as much as possible as a longer term goal. Either way we should merge this because it's definitely a value add -- I'll poke at it tomorrow and push if it passes muster, which I bet it does because our test suite is good $$$$ |
<%= favicon_link_tag 'favicon.ico' %> | ||
<%= javascript_include_tag 'application' %> | ||
<%= stylesheet_link_tag :application, media: 'all' %> | ||
<%= favicon_link_tag %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this, I'd been meaning to do it for awhile 💯
I bumped the issue refs -- I think there's a little additional CSP work we want to do at this juncture: a) get any and all inline js out of the app where possible and b) follow that up by adjusting the CSP so it makes things choke rather than |
I rule and have completed some work on Case Manager that's ready for review!
An alternative to #1096 , adds in Twitter's SecureHeaders and moves config over to that, still a bit of investigation on hash stuff needed.
This pull request makes the following changes:
It relates to the following issue #s: