Skip to content

Parse report-to CSP reports - #3131

Open
labkey-adam wants to merge 2 commits into
developfrom
fb_report_to
Open

Parse report-to CSP reports#3131
labkey-adam wants to merge 2 commits into
developfrom
fb_report_to

Conversation

@labkey-adam

Copy link
Copy Markdown
Contributor

Rationale

Test framework should be able to parse report-to CSP reports, especially since that's all the product sends now. https://github.com/LabKey/internal-issues/issues/900

Related Pull Requests

@labkey-adam
labkey-adam requested a review from labkey-tchad July 28, 2026 21:33
@labkey-adam

labkey-adam commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@labkey-tchad let me know if there's a good way to test this. Seems like it should have been failing since we added support for report-to.

@labkey-tchad

Copy link
Copy Markdown
Member

@labkey-tchad let me know if there's a good way to test this. Seems like it should have been failing since we added support for report-to.

The tests are fine because our standard CSP still includes report-uri for backwards compatibility.
Firefox doesn't support report-to and Chrome requires HTTPS for it.
Few of our tests run with HTTPS on TeamCity and none run on Chrome so the tests don't need to support the new report format just yet. I'd be open to making the CspReport constructor understand both formats to future proof it though.

@labkey-adam

Copy link
Copy Markdown
Contributor Author

@labkey-tchad let me know if there's a good way to test this. Seems like it should have been failing since we added support for report-to.

The tests are fine because our standard CSP still includes report-uri for backwards compatibility. Firefox doesn't support report-to and Chrome requires HTTPS for it. Few of our tests run with HTTPS on TeamCity and none run on Chrome so the tests don't need to support the new report format just yet. I'd be open to making the CspReport constructor understand both formats to future proof it though.

Current Firefox versions now support report-to, so the related PR eliminates report-uri and unconditionally adds report-to. With that change, the tests should only see report-to-style JSON going forward. See the related issue (which requested this change), and update it if you foresee a problem.

@labkey-adam

Copy link
Copy Markdown
Contributor Author

Note that Claude is somewhat concerned about how well this helper will work with report-to. The report-uri reports were sent synchronously (or nearly so), but report-to reports are queued and sent asynchronously.

It's also worried about throwing due to reports that don't contain "body" or "documentUrl," but that seems unlikely.

@labkey-tchad

Copy link
Copy Markdown
Member

Current Firefox versions now support report-to, so the related PR eliminates report-uri and unconditionally adds report-to. With that change, the tests should only see report-to-style JSON going forward. See the related issue (which requested this change), and update it if you foresee a problem.

We would have to switch all of our suites on TeamCity to use HTTPS and run on the latest Firefox version.
Luckily, the new ESR just dropped, which includes the 'report-to' support. I'm working on a PR to update the TeamCity agents.
We already have some TeamCity suites running with HTTPS. It shouldn't be too much trouble to make the change universal. I'm not positive that our agent build is actually installing the certificate so that it's recognized OS-wide. I know we install it so that Java recognizes it but we might just be using various self-signed-cert flags to make other things work.

That being said, I don't understand the rush to remove report-uri. Firefox has only supported it since March, and the ESR release the supports it has been out for about a week. Sure, that accounts for a fraction of a fraction of browser share but I don't see the harm in retaining it.
I'm also not sure how many folks are still running their dev servers with HTTP; I bet it's a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants