Set the form-action directive in the report-only CSP

[robhudson]

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. The form-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 Security

The form-action directive is a key security measure designed to mitigate attacks that exploit form submissions, such as:

• Phishing and data exfiltration attacks: Prevents malicious actors from redirecting form submissions to unauthorized or external servers.
• Clickjacking: Works in tandem with other CSP directives to ensure that embedded forms are protected.
• Cross-site scripting (XSS) risks: Reduces the attack surface by limiting submission endpoints, even if an attacker manages to inject a form.

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

1. Deploy the change and monitor CSP violation reports for any issues related to form-action.
2. Refine the directive based on findings from the report-only stage.
3. Once confident, apply the directive to the enforced CSP header to fully benefit from its security enhancements.

References

• MDN Documentation on form-action

• I used an AI to write some of this code.

Issue / Bugzilla link

#15553 (from #11943)

Testing

• Verify the addition of the form-action directive in the report-only CSP header locally.
  • To do this we need to set a couple local env vars:
    • DEBUG=False - CSP headers aren't added while in DEBUG mode
    • CSP_RO_REPORT_URI=/csp-violation - the report-only header will only be added if this is set.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.82%. Comparing base (7175260) to head (d1cbe85).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #15554   +/-   ##
=======================================
  Coverage   79.81%   79.82%           
=======================================
  Files         160      160           
  Lines        8493     8494    +1     
=======================================
+ Hits         6779     6780    +1     
  Misses       1714     1714           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[stevejalim]

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?

[robhudson]

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.

[janbrasna]

NB: The slash has a function in CSP wrt to being a path component (i.e. an implicit wildcard after it) or not.

[janbrasna]

ref: https://w3c.github.io/webappsec-csp/#path-part-match
see: https://bugzilla.mozilla.org/show_bug.cgi?id=741019#c25

[robhudson]

Looking at the CSP path-part-match rules more closely...

1. With https://example.com/ in our CSP, a form submit to https://example.com/ matches.
2. With https://example.com in our CSP (no path), a form submit to https://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?

[robhudson]

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.

[robhudson]

Updated the basket URL to {BASKET_URL}/news/. This change ensures that CSP's path-part matching rules allow any endpoint under this URL (e.g., {BASKET_URL}/news/subscribe/) to be valid as a form action, instead of requiring an exact match.

[janbrasna]

Looking at this in context of #15787 — even if that's a JSON-only API, it still formally uses form's action; so thinking about how important It would be to support submitting that form (in nonJS, or in case the JS loading fails etc.; to still allow users to recover(?)…)

[robhudson]

@alexgibson Any thoughts on the progressive enhancement Jan mentions above?

Summary:

• Setting form-action allows the form to submit to these endpoints. Currently, if the PR in 15787 lands, that changes the form action endpoint to one that isn't in this PR. That wasn't intentional but does bring up the point.
• The form-action doesn't affect javascript submitting the form to an endpoint -- connect-src does and all of basket is already supported there.

My assumption is we do appreciate progressive enhancement although I think we depend on JS for newsletter management. And I would add the new API URL to form-action here if #15787 lands first.

[alexgibson]

@robhudson the current version of the newsletter recovery form does work with JS disabled, however it's not really a great user experience, and the newsletter management form itself (which this form leads directly to) already relies on JS to function. So progressive enhancement here is not really a huge concern imho.

Perhaps we should add a message for users with JS disabled to #15787, similar to how we do for the management center? (I'll take a look at that PR next too).

[stevejalim]

r+wc

[stevejalim]

@robhudson Is this still viable to merge, please, or does it need more work?