Skip to content
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

FF130 CSP report-to directive #35331

Merged
merged 13 commits into from
Aug 23, 2024
Merged

Conversation

hamishwillee
Copy link
Collaborator

@hamishwillee hamishwillee commented Aug 6, 2024

FF130 supports the CPS report-to directive in https://bugzilla.mozilla.org/show_bug.cgi?id=1391243 behind a pref. It also supports

It also supports lots of reporting API stuff too, which was not noted.

The summary of the status quo for CSP and reporting is:

  • CSP reports are now supposed to be sent using the Reporting API according to the spec, and FF now supports that API behind a pref. Safari and Chrome already do this.
  • The reporting API uses Reporting-Endpoints to define a set of named endpoint targets. You can then use a report-to directive in "some" headers to select one of these endpoints for sending reports to.
    • The header you use depends on the type you want to report. For CSP that's the CSP or CSP reporting header.
    • The reports are then serialised as JSON and POST to the endpoint. The data that is sent is in a body property and is a serialised version of a specific report item CSPViolationReportBody (the type is specific to the type of report)
    • There is a header Report-To which is deprecated - this is available on Chrome only and does similar job as Reporting-Endpoints.
  • There is a deprecated directive for CSP report-uri that takes a URL target and sends a slightly different report JSON with a slightly different content mime type. This is what FF uses.
  • Right now you're supposed to use both because FF doesn't support the new form yet.

The docs were quite mixed up so it was unclear how this worked. What I have done is pushed the documentation of the old reporting format into report-url directive. The new docs are mostly about the reporting API, though I do often mention the deprecated directive. I show sample reports in the new and old formats, but I point to CSPViolationReportBody for definition of the properties rather than repeating everywhere.

The docs are quite mixed up. Still seeking confirmations on some things in https://bugzilla.mozilla.org/show_bug.cgi?id=1391243#c11
Fortunately there is a bit of a guide in https://developer.chrome.com/docs/capabilities/web-apis/reporting-api

Related docs work can be tracked in #35279

@github-actions github-actions bot added the Content:HTTP HTTP docs label Aug 6, 2024
@github-actions github-actions bot added the size/m [PR only] 51-500 LoC changed label Aug 6, 2024
Copy link
Contributor

github-actions bot commented Aug 6, 2024

Preview URLs (14 pages)
Flaws (13)

Note! 12 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/API/CSPViolationReportBody
Title: CSPViolationReportBody
Flaw count: 12

  • macros:
    • /en-US/docs/Web/API/CSPViolationReportBody/blockedURL does not exist
    • /en-US/docs/Web/API/CSPViolationReportBody/columnNumber does not exist
    • /en-US/docs/Web/API/CSPViolationReportBody/disposition does not exist
    • /en-US/docs/Web/API/CSPViolationReportBody/documentURL does not exist
    • /en-US/docs/Web/API/CSPViolationReportBody/effectiveDirective does not exist
    • and 7 more flaws omitted

URL: /en-US/docs/Web/HTTP/Headers/Reporting-Endpoints
Title: Reporting-Endpoints
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: http.headers.Reporting-Endpoints
External URLs (2)

URL: /en-US/docs/Web/HTTP/Headers/Content-Security-Policy/report-uri
Title: CSP: report-uri


URL: /en-US/docs/Mozilla/Firefox/Releases/108
Title: Firefox 108 for developers

(comment last updated: 2024-08-23 07:52:42)

@github-actions github-actions bot added the merge conflicts 🚧 [PR only] label Aug 12, 2024
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

Comment on lines 12 to 13
> [!NOTE]
> This interface is similar, but not identical to, the [JSON objects](/en-US/docs/Web/HTTP/CSP#violation_report_syntax) sent back to the [`report-uri`](/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/report-uri) or [`report-to`](/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/report-to) policy directive of the {{HTTPHeader("Content-Security-Policy")}} header.
Copy link
Collaborator Author

@hamishwillee hamishwillee Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, this may have been true at some point, but not any more. From testing I can see that the object is returned as the body of the report in the ReportObserver, or as a serialized JSON version in the body property of the report.

So essentially one is an object and one is JSON, but the property names and content of stuff that is/can be serialized is the same. I have captured that.

The CSP docs will have to be updated because they still document the "similar" format for reports, where there are some casing differences in the property names.

Copy link
Collaborator Author

@hamishwillee hamishwillee Aug 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wrong, but so was this.

  • The Reporting API, uses the CSP report-to directive
  • CSP reports are a slightly different format, and are sent when you specify the endpoint using report-uri

@github-actions github-actions bot added Content:Security Security docs Content:Firefox Content in the Mozilla/Firefox subtree size/l [PR only] 501-1000 LoC changed and removed size/m [PR only] 51-500 LoC changed labels Aug 20, 2024
@hamishwillee hamishwillee marked this pull request as ready for review August 20, 2024 08:08
@hamishwillee hamishwillee requested review from a team as code owners August 20, 2024 08:08
@hamishwillee hamishwillee requested review from Elchi3 and chrisdavidmills and removed request for a team August 20, 2024 08:08
@hamishwillee
Copy link
Collaborator Author

This is not "100%" finished, but the guts of it are - the remainder of the work that I know about can be done as a post process if this doesn't get reviewed in the next few days.
That work, specifically, is adding missing property docs for things like CSPViolationReportBody.

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work @hamishwillee. I've made several comments, but it is mostly language stuff. The intent behind the work makes a lot of sense, and it is a vast improvement.

files/en-us/web/api/cspviolationreportbody/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/cspviolationreportbody/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/cspviolationreportbody/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/cspviolationreportbody/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/cspviolationreportbody/index.md Outdated Show resolved Hide resolved
files/en-us/web/http/headers/report-to/index.md Outdated Show resolved Hide resolved
files/en-us/web/http/headers/report-to/index.md Outdated Show resolved Hide resolved
files/en-us/web/http/headers/report-to/index.md Outdated Show resolved Hide resolved
files/en-us/web/http/headers/reporting-endpoints/index.md Outdated Show resolved Hide resolved
@pepelsbey pepelsbey removed their request for review August 21, 2024 11:05
@sideshowbarker sideshowbarker removed their request for review August 23, 2024 04:54
@hamishwillee
Copy link
Collaborator Author

@chrisdavidmills Thank you for that mammoth review.

I accepted nearly everything, and fixed the specific open questions. Ready for another final look.

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hamishwillee I'm satisfied, you're satisfied, we're all satisfied!

And it's Friday. Let's get this merged.

@chrisdavidmills chrisdavidmills merged commit 6b4c6ac into mdn:main Aug 23, 2024
8 checks passed
@hamishwillee hamishwillee deleted the ff130_csp_report-to branch August 25, 2024 04:17
@hamishwillee
Copy link
Collaborator Author

Thanks @chrisdavidmills !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Firefox Content in the Mozilla/Firefox subtree Content:HTTP HTTP docs Content:Security Security docs Content:WebAPI Web API docs size/l [PR only] 501-1000 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants