-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Adding "style-src 'unsafe-inline' 'self'" to default CSP rules #41305
Changes from 8 commits
cfd4d24
d33e926
f845e88
af2627e
c9db527
2cc6f7c
c0d3fe1
397b5e1
c80b6b0
3db12ee
8dcb305
de62534
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,8 +32,16 @@ export default function ({ getService }) { | |
it('csp header does not allow all inline scripts', async () => { | ||
const response = await supertest.get('/app/kibana'); | ||
|
||
expect(response.headers['content-security-policy']).to.contain('script-src'); | ||
expect(response.headers['content-security-policy']).not.to.contain('unsafe-inline'); | ||
const header = response.headers['content-security-policy']; | ||
const parsed = new Map(header.split(';').map(rule => { | ||
const parts = rule.trim().split(' '); | ||
const key = parts.splice(0, 1)[0]; | ||
return [key, parts]; | ||
})); | ||
|
||
const scriptSrc = parsed.get('script-src'); | ||
expect(scriptSrc).to.be.an(Array); | ||
expect(scriptSrc).not.to.contain('unsafe-inline'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: I'm a bit concerned about check like this. Our parsing logic in this test may not work as we expect at some point (even with packages like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. I initially was trying to keep this test similar to how it was originally written, but you've made a convincing argument. Changes will be forth-coming. |
||
}); | ||
}); | ||
} |
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.
question: is there any reason we don't want to use
nonce
for inline styles as well?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.
Using the
nonce
has caused quite a few issues, as discussed here and I'd like to find a way to move towardself
even forscript-src
. I could see the argument being made for using thenonce
until we switchscript-src
toself
, but it's quite a bit more work which we'd rather quickly get rid of.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.
Okay, I see, thanks for the explanation. Yeah, let's see how it goes then.