Add report to header config #30
Conversation
src/app/index.ts
Outdated
| app.use( | ||
| helmet({ | ||
| // override default contentSecurityPolicy directive like script-src to include cloudflare cdn and github static content | ||
| // might consider turning this off to allow individual front-ends set Content-Security-Policy on meta tags themselves if list grows long | ||
| // <meta http-equiv="Content-Security-Policy" content="default-src 'self' 'unsafe-inline'; script-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com;" > | ||
| contentSecurityPolicy: { | ||
| directives: EXPRESS_CONTENT_SECURITY_POLICY_CONFIG, | ||
| reportOnly: !!reportOnly, |
There was a problem hiding this comment.
There is a 'danger' to using this header IMO. If it's accidentally set to true it negates all the benefits of csp (Content-Security-Policy-Report-Only header is set instead of Content-Security-Policy). At the very worst both csp headers should co-exist (i.e Content-Security-Policy and Content-Security-Policy-Report-Only). I would vote to have this directive removed (defaults to false) or to set both as described here helmetjs/helmet#351 (comment)
There was a problem hiding this comment.
cool, I will remove the reportOnly for now, I think we can use the more general express_reponse_headers to append other headers including the report-only variant
.env.sample
Outdated
| EXPRESS_CONTENT_SECURITY_POLICY_CONFIG=`{"default-src":["'self'"]}` | ||
| EXPRESS_CONTENT_SECURITY_POLICY_CONFIG=`{"default-src":["'self'"], report-only: true}` | ||
| # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/report-to. `Map<string, stringifiedJson>`. | ||
| EXPRESS_RESPONSE_HEADERS='{"Report-To":"{ \"group\": \"csp-endpoint\", \"max_age\": 10886400, \"endpoints\": [{ \"url\": \"https://example.com/endpoint\" }] }"}' |
There was a problem hiding this comment.
add an example second response header to show how to pass multiple headers
There was a problem hiding this comment.
also is the Report-To key case sensitive?
There was a problem hiding this comment.
no, Header fields are not case sensitive
src/app/index.ts
Outdated
| const customHeaders = Object.entries(EXPRESS_RESPONSE_HEADERS); | ||
| if (customHeaders.length > 0) { | ||
| customHeaders.forEach(([key, value]) => { | ||
| if (typeof value === 'string') { |
There was a problem hiding this comment.
maybe we also want to check if value !== ""?
peterMuriuki
force-pushed
the
report-to-header
branch
from
75f5243 to
1aacb9b
Compare
* Add flag to enable/disable CQL to JSON strict mode validation * Upgrade dependencies * Release version bump to 2.3.1
Add report to header config