-
Notifications
You must be signed in to change notification settings - Fork 918
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
Set the form-action
directive in the report-only CSP
#15554
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15554 +/- ##
=======================================
Coverage 78.95% 78.96%
=======================================
Files 158 158
Lines 8293 8294 +1
=======================================
+ Hits 6548 6549 +1
Misses 1745 1745 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
form-action
direction in the report-only CSPform-action
directive in the report-only CSP
@@ -127,6 +127,7 @@ | |||
CONTENT_SECURITY_POLICY_REPORT_ONLY["DIRECTIVES"]["object-src"] = [csp.constants.NONE] | |||
CONTENT_SECURITY_POLICY_REPORT_ONLY["DIRECTIVES"]["frame-ancestors"] = [csp.constants.NONE] | |||
CONTENT_SECURITY_POLICY_REPORT_ONLY["DIRECTIVES"]["style-src"].remove(csp.constants.UNSAFE_INLINE) | |||
CONTENT_SECURITY_POLICY_REPORT_ONLY["DIRECTIVES"]["form-action"] = [csp.constants.SELF, BASKET_URL, FXA_ENDPOINT] |
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: FXA_ENDPOINT has a trailing slash and BASKET_URL does not. Is that gonna cause any wrinkles or is CSP smart enough to strip trailing slashes?
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 noticing that. The browser, from what I've read, doesn't really differentiate, so this should be fine. But I'm a fan of consistency so perhaps I'll add some trailing slash stripping.
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.
NB: The slash has a function in CSP wrt to being a path component (i.e. an implicit wildcard after it) or not.
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.
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.
Looking at the CSP path-part-match
rules more closely...
- With
https://example.com/
in our CSP, a form submit tohttps://example.com/
matches. - With
https://example.com
in our CSP (no path), a form submit tohttps://example.com/
matches, since when split on'/'
, the path is empty string.
And vice-versa for the form submit to the non-slash URL.
Focusing on the BASKET_URL
, however, reading the path-part-match
rules @janbrasna mentioned, we would want the trailing slash for the basket URL. Without the trailing slash it is in "exact match" mode and a form submit to an actual path would fail. In the comment above I was just looking at the FXA_ENDPOINT
in our code where we don't use any path.
So the slash is important, just not necessarily if we're submitting to ""
vs /
.
@janbrasna - does that "match" your understanding of it?
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.
Yup… and I was only looking at FXA_ENDPOINT and CJMS_AFFILIATE_ENDPOINT for another rule, which is either a wildcard, or a particular endpoint only.
OTOH for Basket the easy way might be setting the form action CSP rule as f"{BASKET_URL}/news/"
to match the existing code? (And make it even more specific? Or are there even more endpoints that I'm not aware 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.
I'm working on adding some more basket APIs under /api/
. So I'd probably keep that one at the root level for now. Then again, the ones under /api/
are more for javascript, not form submits.
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.
r+wc
Summary
This PR introduces the
form-action
directive to the report-only Content Security Policy (CSP) header. The goal is to test and evaluate its compatibility before eventually applying it to the enforced CSP header. Theform-action
directive restricts where forms on the site can send their data upon submission, adding an additional layer of security to prevent potential vulnerabilities.Why
form-action
is Important for SecurityThe
form-action
directive is a key security measure designed to mitigate attacks that exploit form submissions, such as:By specifying trusted domains or paths for form submissions,
form-action
ensures that forms behave only as intended and cannot be abused for unauthorized data capture.Next Steps
form-action
.References
MDN Documentation on
form-action
I used an AI to write some of this code.
Issue / Bugzilla link
#15553 (from #11943)
Testing
form-action
directive in the report-only CSP header locally.DEBUG=False
- CSP headers aren't added while in DEBUG modeCSP_RO_REPORT_URI=/csp-violation
- the report-only header will only be added if this is set.