Skip to content

Commit a7b489f

Browse files
committed
fix(diag): redact embedder-chosen segment names from crash reports
A corruption report may be shipped to crash telemetry, and the segment `name` carried by `CorruptionDetail::SegmentMissing` is application data the embedder supplied (a tenant id, a user's collection name), not something diagnosis needs — the `segment_id` already identifies the file. Render it as a redacted byte length instead of the raw string.
1 parent abd5c8a commit a7b489f

1 file changed

Lines changed: 32 additions & 3 deletions

File tree

src/diag.rs

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,12 @@ mod imp {
188188
/// discipline `PageReadVerifyFailure` already uses, so one bug landed
189189
/// once still collapses to one report under fuzzing.
190190
grouping_key: String,
191-
/// Full `Debug` rendering of the detail, for a report that is
192-
/// diagnosable on its own.
191+
/// `Debug` rendering of the detail, for a report that is diagnosable
192+
/// on its own — with embedder-chosen text redacted. A report may be
193+
/// shipped to crash telemetry, and a segment name is application data
194+
/// (a tenant id, a user's collection name) that diagnosis never needs:
195+
/// the identity that locates the file is the `segment_id`, which is
196+
/// kept. See [`redacted_debug`].
193197
detail_debug: String,
194198
}
195199

@@ -205,6 +209,31 @@ mod imp {
205209
}
206210
}
207211

212+
/// Render a [`CorruptionDetail`] for a report, replacing any
213+
/// embedder-chosen string with its length.
214+
///
215+
/// Every field in the taxonomy is either a `&'static str` this crate
216+
/// wrote, a numeric id, or a raw identity — none of which is application
217+
/// data — except the segment `name` carried by
218+
/// [`CorruptionDetail::SegmentMissing`], which is whatever the embedder
219+
/// passed to `link_segment`. That one is replaced here rather than
220+
/// suppressed at the call site, so a future variant that adds a name gets
221+
/// the same treatment by extending this one function.
222+
fn redacted_debug(detail: &CorruptionDetail) -> String {
223+
match detail {
224+
CorruptionDetail::SegmentMissing {
225+
realm_id,
226+
name,
227+
segment_id,
228+
} => format!(
229+
"SegmentMissing {{ realm_id: {realm_id:?}, name: <redacted, {} bytes>, \
230+
segment_id: {segment_id:?} }}",
231+
name.len()
232+
),
233+
other => format!("{other:?}"),
234+
}
235+
}
236+
208237
/// Classify a [`CorruptionDetail`] into a stable domain kind and a
209238
/// grouping key that names the structural fields of the failure but never
210239
/// the instance-specific ones (page ids, segment ids), so many
@@ -332,7 +361,7 @@ mod imp {
332361
let ctx = CorruptionConstructed {
333362
kind,
334363
grouping_key,
335-
detail_debug: format!("{detail:?}"),
364+
detail_debug: redacted_debug(detail),
336365
};
337366
let _ = faultbox::Capture::new(
338367
faultbox::EventKind::Corruption,

0 commit comments

Comments
 (0)