-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
feat(cloudfront): reportOnly
flag on ResponseHeadersContentSecurityPolicy
#29031
Closed
Closed
Changes from all commits
Commits
Show all changes
73 commits
Select commit
Hold shift + click to select a range
f47fe50
fix-29006 reportOnly flag on ResponseHeadersContentSecurityPolicy
dillonstreator c9fee16
update readme to show reportOnly field in comprehensive ResponseHeade…
dillonstreator 1c790fd
reorder fields
dillonstreator 14c8284
lint fix
dillonstreator 84d7c91
add @default to jsdoc for optional reportOnly field
dillonstreator 3a55e7e
Merge branch 'main' into fix-29006
paulhcsun 0ea3894
Merge branch 'main' into fix-29006
dillonstreator 552354d
Merge branch 'main' into fix-29006
dillonstreator e75b4e2
Merge branch 'main' into fix-29006
dillonstreator fa4b6c4
Merge branch 'main' into fix-29006
dillonstreator 8d9e0d6
Merge branch 'main' into fix-29006
dillonstreator 47f8fac
Merge branch 'main' into fix-29006
dillonstreator 51cb257
Merge branch 'main' into fix-29006
dillonstreator 0aa1f57
Merge branch 'main' into fix-29006
dillonstreator 7118566
Merge branch 'main' into fix-29006
dillonstreator 5ec8407
Merge branch 'main' into fix-29006
dillonstreator e210249
Merge branch 'main' into fix-29006
dillonstreator 7e124bc
Merge branch 'main' into fix-29006
dillonstreator 4931911
Merge branch 'main' into fix-29006
dillonstreator 15f17e3
Merge branch 'main' into fix-29006
dillonstreator 7611b2e
Merge branch 'main' into fix-29006
dillonstreator d94ee1f
Merge branch 'main' into fix-29006
dillonstreator afc4396
Merge branch 'main' into fix-29006
dillonstreator 0509e04
Merge branch 'main' into fix-29006
dillonstreator 30b4705
Merge branch 'main' into fix-29006
dillonstreator 98cfd74
Merge branch 'main' into fix-29006
dillonstreator e19a082
Merge branch 'main' into fix-29006
dillonstreator 348136a
Merge branch 'main' into fix-29006
dillonstreator 166dabe
Merge branch 'main' into fix-29006
dillonstreator 3ef1f43
Merge branch 'main' into fix-29006
dillonstreator c6573a2
Merge branch 'main' into fix-29006
dillonstreator d28adcd
Merge branch 'main' into fix-29006
dillonstreator 8438a60
Merge branch 'main' into fix-29006
dillonstreator d56cc40
Merge branch 'main' into fix-29006
dillonstreator f58e951
Merge branch 'main' into fix-29006
dillonstreator 2be701a
Merge branch 'main' into fix-29006
dillonstreator 24b97c3
Merge branch 'main' into fix-29006
dillonstreator 3103a30
Merge branch 'main' into fix-29006
dillonstreator 14d53f9
Merge branch 'main' into fix-29006
dillonstreator b58598e
Merge branch 'main' into fix-29006
dillonstreator 97c48bc
Merge branch 'main' into fix-29006
dillonstreator fd200a3
Merge branch 'main' into fix-29006
dillonstreator 324152a
Merge branch 'main' into fix-29006
dillonstreator 6727c67
Merge branch 'main' into fix-29006
dillonstreator e33fa2c
Merge branch 'main' into fix-29006
dillonstreator 1d3f440
Merge branch 'main' into fix-29006
dillonstreator 63517b5
Merge branch 'main' into fix-29006
dillonstreator e0494ba
Merge branch 'main' into fix-29006
dillonstreator 50d9da7
Merge branch 'main' into fix-29006
dillonstreator 8afc414
Merge branch 'main' into fix-29006
dillonstreator ecff3b9
securityHeadersBehavior.contentSecurityPolicy takes precedence with `…
dillonstreator d9287c6
Merge branch 'main' into fix-29006
dillonstreator 3d68fd0
lint
dillonstreator e6980b2
Merge branch 'main' into fix-29006
dillonstreator 3a2c2be
Merge branch 'main' into fix-29006
dillonstreator 54d9b06
Merge branch 'main' into fix-29006
dillonstreator d0b9c16
Merge branch 'main' into fix-29006
dillonstreator baa9828
Merge branch 'main' into fix-29006
dillonstreator 8a89345
Merge branch 'main' into fix-29006
dillonstreator 3334206
Merge branch 'main' into fix-29006
dillonstreator dc8da84
Merge branch 'main' into fix-29006
dillonstreator b317afc
Merge branch 'main' into fix-29006
dillonstreator ce93883
Merge branch 'main' into fix-29006
dillonstreator 1b509da
Merge branch 'main' into fix-29006
dillonstreator 0c7a4d6
Merge branch 'main' into fix-29006
dillonstreator d7b62b5
Merge branch 'main' into fix-29006
dillonstreator 8d2d529
Merge branch 'main' into fix-29006
dillonstreator 937abd9
Merge branch 'main' into fix-29006
dillonstreator 54398cb
Merge branch 'main' into fix-29006
dillonstreator 1b4c42c
Merge branch 'main' into fix-29006
dillonstreator facb205
Merge branch 'main' into fix-29006
dillonstreator 1719796
Merge branch 'main' into fix-29006
dillonstreator daa3037
Merge branch 'main' into fix-29006
dillonstreator File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -180,4 +180,57 @@ describe('ResponseHeadersPolicy', () => { | |
}, | ||
}); | ||
}); | ||
|
||
test('it respects CSP `reportOnly` flag by mapping to custom header', () => { | ||
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. This is only testing a single logic path. We need to test each of the paths added by this PR. 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. This test hits both new paths introduced by this PR
|
||
new ResponseHeadersPolicy(stack, 'ResponseHeadersPolicy', { | ||
securityHeadersBehavior: { | ||
contentSecurityPolicy: { contentSecurityPolicy: 'default-src https:;', reportOnly: true, override: true }, | ||
}, | ||
}); | ||
|
||
Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::ResponseHeadersPolicy', { | ||
ResponseHeadersPolicyConfig: { | ||
CustomHeadersConfig: { | ||
Items: [ | ||
{ | ||
Header: 'Content-Security-Policy-Report-Only', | ||
Value: 'default-src https:;', | ||
Override: true, | ||
}, | ||
], | ||
}, | ||
}, | ||
}); | ||
}); | ||
|
||
test('securityHeadersBehavior.contentSecurityPolicy takes precedence with `reportOnly`', () => { | ||
new ResponseHeadersPolicy(stack, 'ResponseHeadersPolicy', { | ||
customHeadersBehavior: { | ||
customHeaders: [ | ||
{ | ||
header: 'Content-Security-Policy-Report-Only', | ||
value: 'default-src self;', | ||
override: true, | ||
}, | ||
], | ||
}, | ||
securityHeadersBehavior: { | ||
contentSecurityPolicy: { contentSecurityPolicy: 'default-src https:;', reportOnly: true, override: true }, | ||
}, | ||
}); | ||
|
||
Template.fromStack(stack).hasResourceProperties('AWS::CloudFront::ResponseHeadersPolicy', { | ||
ResponseHeadersPolicyConfig: { | ||
CustomHeadersConfig: { | ||
Items: [ | ||
{ | ||
Header: 'Content-Security-Policy-Report-Only', | ||
Value: 'default-src https:;', | ||
Override: true, | ||
}, | ||
], | ||
}, | ||
}, | ||
}); | ||
}); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why are you making this an empty array only if
reportOnly
is set?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.
The
Content-Security-Policy-Report-Only
header is only respected as a custom header configuration. The following line is pushing the report only csp header into thiscustomHeaders
array. If thecustomHeadersBehavior
object is undefined (which it's allowed to be), we wont have anything to push into.In the non-report-only configuration case, we don't need to worry about touching the
customHeaders
array.