CSP reporting

[mattrobenolt]

• Code
• Tests

Fixes GH-729

The only part I'm super not happy about is the Http interface, since technically, this shouldn't really have one, but it's being leverage to collect tags from the User-Agent. It just looks a bit misleading in the UI since we don't know the method, or any of the other headers, and replaying doesn't make sense.

One thing I'm happy about with this is the normalization with FF into Chrome (and everyone) else styles. FF sends reports a tiny bit differently than the rest of the world, and in my tests, I've gotten them to group together and act sensible.

This is leveraging a url like: /api/<id>/csp-report/?sentry_key=fc1091a194da4309bdf4816628344602&sentry_version=5 which will have to be set in report-uri of their CSP header.

[mattrobenolt]

Want this to be Csp instead of CSP?

[mattrobenolt]

Nevermind, just did it. It fits the naming pattern of all the others, specifically like Http.

[dcramer]

Ping me in the morning to review but i agree on HTTP. IIRC JS tracebacks have the same exact issue though? We could very easily make the user agent stuff work with CSP.

[dcramer]

trailing comma is invalid // @benvinegar how do we get lint for this

[benvinegar]

It's not invalid. Only IE7 and below throw error on this.

On Monday, October 12, 2015, David Cramer notifications@github.com wrote:

In src/sentry/static/sentry/app/components/events/eventEntries.jsx
#2154 (comment):

@@ -31,7 +31,8 @@ var EventEntries = React.createClass({
exception: require("./interfaces/exception"),
request: require("./interfaces/request"),
stacktrace: require("./interfaces/stacktrace"),

• template: require("./interfaces/template")
• template: require("./interfaces/template"),
• csp: require("./interfaces/csp"),

trailing comma is invalid // @benvinegar https://github.com/benvinegar
how do we get lint for this

—
Reply to this email directly or view it on GitHub
https://github.com/getsentry/sentry/pull/2154/files#r41730286.

[mattrobenolt]

Oh, right. Yeah, @benvinegar, we talked about this before. You have a preference of no trailing commas just as convention, but a linter to enforce would be nice for those of us who don't do that instinctively. :)

[benvinegar]

Yeah, I added the rule to raven-js. I can do the same here.

[dcramer]

Regarding tests, can you setup a few fixtures for them that will have each of the browsers behavior just to make sure we dont end up in a similar place to how raven-js/tracekit was (where there were no tests for all the browser behavior)

[mattrobenolt]

IIRC JS tracebacks have the same exact issue though?

Yeah, this is fair. I don't like how it's done there either, since it's slightly misleading. 👿

We could very easily make the user agent stuff work with CSP.

I could call the plugins directly to extract the pieces, but this is fine for now imo. I'd rather let the processing pipeline work for us, instead of trying to outsmart it.

[joaomilho]

Hey, is there any way to send also tags in this endpoint?

[arthurzenika]

hi @mattrobenolt this feature is really great, started using it today.

I can't find any reference to this feature in https://docs.getsentry.com/ is this deliberate, if not, should I add an issue ?

[arthurzenika]

oh, and by the way, it seems that firefox does not report when using an "external" report-uri, filed a bug https://bugzilla.mozilla.org/show_bug.cgi?id=1259393 (just putting this here as an info for those trying to use report-uri)

[mattrobenolt]

It's intentionally not documented yet since it's a bit unclear to us yet just how CSP/HPKP reporting fits into the product. In the meantime, I have documentation here https://gist.github.com/mattrobenolt/fe01bb9e37773289ae38

Also, thanks for the Firefox issue. I was unaware. I thought that worked before, but maybe I'm crazy. :)

[mattrobenolt]

@joaomilho I missed your question, there is not. Since CSP reports don't go through a client, this becomes a bit more difficult and requires custom stuff on our side.

The overall idea is that maybe we support adding all this stuff into the querystring, so you could do something like: &sentry_tags=a,b,c&sentry_release=abc

This way we can leverage release tracking, etc, on CSP reports (which in practice is actually pretty important). But this hasn't been fully planned out yet.

[arthurzenika]

@mattrobenolt I'm getting a probable bug on CSP handling. It's a follow up of that firefox bug I started submitting to mozilla. In fact the data is sent but I get a 403. The only difference I spotted so far is that Chrome uses type 'text/html' while Firefox uses 'json'. Do you have any idea about this ?

[mattrobenolt]

iirc Firefox fails to send us data that we're requiring and what is in the spec, I believe it's the effective-directive. If we don't have this, we'll reject. We need this to provide better grouping.

Also, Chrome sends the report as application/csp-report. At least, it used to, and Firefox send the incorrect type, application/json, but I submit a patch to fix that which has been merged. Sentry accepted both of these though.

[connorshea]

@mattrobenolt we'd like to use CSP reporting to Sentry at GitLab, any chance this is closer to being usable?

[mattrobenolt]

@connorshea You can opt into using this with the Early Adopter flag on your organization settings. Things are pretty solid imo.

[connorshea]

@mattrobenolt any documentation? I can't find it so I'm not sure how to get started with CSP reporting in Sentry :)

[mattrobenolt]

You need to be running at least Sentry 8.4 to have that option, I believe.