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

chore(csp): Filter out unsupported report types #3295

Merged
merged 4 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions relay-server/src/endpoints/security_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@ impl SecurityReportParams {

if let Ok(items) = variant {
for item in items {
let report_item =
Self::create_security_item(&query, Bytes::from(item.to_owned().to_string()));
envelope.add_item(report_item);
let data = Bytes::from(item.to_owned().to_string());
if is_supported_type(&data) {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of parsing the data here, couldn't we filter out the unsupported type in the envelope processor (after or during the parsing step)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I first thought to do it in processor, but then , I've got an idea to discard those items as earlier as possible and later we won't do extra work. But yeah, I will move this back to processor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in a0e3a4a

let report_item = Self::create_security_item(&query, data);
envelope.add_item(report_item);
}
}
} else {
let report_item = Self::create_security_item(&query, body);
Expand All @@ -72,6 +74,21 @@ impl SecurityReportParams {
}
}

/// Check if the report type is supported.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Check if the report type is supported.
/// Checks if the report type is supported.

///
/// Note: use only for the reports sent through the Reporting Api.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Note: use only for the reports sent through the Reporting Api.
/// Note: use only for the reports sent through the Reporting API.

fn is_supported_type(data: &[u8]) -> bool {
#[derive(Deserialize)]
#[serde(rename_all = "kebab-case")]
#[serde(tag = "type")]
enum SecurityReportVariant {
CspViolation,
}

let helper: Option<SecurityReportVariant> = serde_json::from_slice(data).ok();
helper.is_some()
}

fn is_security_mime(mime: Mime) -> bool {
let ty = mime.type_().as_str();
let subty = mime.subtype().as_str();
Expand Down
66 changes: 65 additions & 1 deletion tests/integration/test_security_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,76 @@ def test_csp_violation_reports_with_processing(
events.append(event)
event, _ = events_consumer.get_event()
events.append(event)
events_consumer.assert_empty()

events_consumer.assert_empty()
assert len(events), 2
assert any(d for d in events if d["csp"]["disposition"] == "enforce")
assert any(d for d in events if d["csp"]["disposition"] == "report")


def test_deprication_reports_with_processing(
mini_sentry,
relay_with_processing,
events_consumer,
):
# UA parsing introduces higher latency in debug mode
events_consumer = events_consumer(timeout=10)

proj_id = 42
relay = relay_with_processing()
mini_sentry.add_full_project_config(proj_id)

reports = [
{
"age": 0,
"body": {
"blockedURL": "https://example.com/tst/media/7_del.png",
"disposition": "report",
"documentURL": "https://example.com/tst/test_frame.php?ID=229&hash=da964209653e467d337313e51876e27d",
"effectiveDirective": "img-src",
"lineNumber": 9,
"originalPolicy": "default-src 'none'; report-to endpoint-csp;",
"referrer": "https://example.com/test229/",
"sourceFile": "https://example.com/tst/test_frame.php?ID=229&hash=da964209653e467d337313e51876e27d",
"statusCode": 0,
},
"type": "csp-violation",
"url": "https://example.com/tst/test_frame.php?ID=229&hash=da964209653e467d337313e51876e27d",
"user_agent": "Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.88 Safari/537.36",
},
{
"type": "deprecation",
"age": 10,
"url": "https://example.com/",
"user_agent": "BarBrowser/98.0 (Mozilla/5.0 compatiblish)",
"body": {
"id": "websql",
"anticipatedRemoval": "1/1/2020",
"message": "WebSQL is deprecated and will be removed in Chrome 97 around January 2020",
"sourceFile": "https://example.com/index.js",
"lineNumber": 1234,
"columnNumber": 42,
},
},
]

relay.send_security_report(
project_id=proj_id,
content_type="application/reports+json; charset=utf-8",
payload=reports,
release="01d5c3165d9fbc5c8bdcf9550a1d6793a80fc02b",
environment="production",
)

events = []
event, _ = events_consumer.get_event()
events.append(event)

events_consumer.assert_empty()
assert len(events), 1
assert any(d for d in events if d["csp"]["disposition"] == "report")


@pytest.mark.parametrize(
"test_case",
[
Expand Down
Loading