fix(minidump): Always validate minidump and extract only limited bytes - #6282
fix(minidump): Always validate minidump and extract only limited bytes#6282tobias-wilfert wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e970594. Configure here.
| DiscardAttachmentType::Minidump, | ||
| )), | ||
| None => BadStoreRequest::InvalidBody(std::io::Error::other(e)), | ||
| })?; |
There was a problem hiding this comment.
Missing reject on early failures
Medium Severity
In raw_minidump_to_item, failures from to_bytes size limiting and from decode_and_validate_stream return without rejecting the Managed item. Drop then emits Internal for attachment quantities instead of ItemTooLarge or InvalidMinidump. Nearby upload_stream_checked already rejects with the correct outcome before returning.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e970594. Configure here.
| } else { | ||
| let minidump_data = request.extract().await?; | ||
| let minidump_data = | ||
| axum::body::to_bytes(request.into_body(), state.config().max_attachment_size()) |
There was a problem hiding this comment.
Shouldn't the middleware/configured limit on the route already enforce this, or was that removed due to the streaming appraoch?


These are some issues that came up in previous PRs (related to the large attachments/minidump work). Specifically this adds a check to streamed minidumps to ensure that they are 'valid' (since this was deemed a cheap check). Furthermore this also fixes a logic-path for buffered minidumps where size limits were not correctly applied.
Fix INGEST-983