Skip to content

fix(errorReporting): minimize data in opt-in crash reports#328

Draft
dobby-coder[bot] wants to merge 1 commit into
mainfrom
harden-crash-report-minimization
Draft

fix(errorReporting): minimize data in opt-in crash reports#328
dobby-coder[bot] wants to merge 1 commit into
mainfrom
harden-crash-report-minimization

Conversation

@dobby-coder

@dobby-coder dobby-coder Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Tightens the data an opt-in crash report carries so a report cannot include sensitive values. Full details are withheld while the associated security advisory is still in draft.

The scrub logic lives in a new dependency-free src/lib/reportScrub.ts module so it can be unit-tested without the SvelteKit env/runtime.

Testing

  • src/lib/reportScrub.test.ts covers the sanitizer and the event scrub. Verified the tests fail if the redaction is removed and pass with it in place.
  • npm run test:unit, npm run check, and npm run lint all pass.

Closes #325

@dobby-coder
dobby-coder Bot requested a review from rubenhensen July 25, 2026 00:24

@dobby-coder dobby-coder Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

VERDICT: approve

Rules Dobby 2 — rule-compliance check (cycle 1) + consolidated review.

Result: clean, no blocking issues. The primary fix is sound: reportError now attaches only origin + pathname, so query-string values never enter report context, and scrubEvent (Sentry beforeSend) is a reasonable last line of defense over the event message, exception values, request URL and string extras. A committed regression test (reportScrub.test.ts) covers both paths, and test:unit/check/lint were reported green.

Rule sweep (conventional-commit title, closing-keyword, vitest hoisting, artifact genericization, branch-name embargo): PR title, body, commit message, branch name and diff are all generic — no leak. While sweeping I found and redacted two leaks on the linked tracking issue: a plain-prose identifier in the trigger comment, and a bare-link identifier in the issue body (now href-only).

Non-blocking hardening note (see inline comment): optional defense-in-depth on the last-line scrub only. The primary leak is fully closed, so these are not merge-blocking and the PR should not loop for them.

Held in draft: signing off, but intentionally not flipping this PR to ready — staying draft until the associated advisory is published.

Comment thread src/lib/reportScrub.ts
* string — whether they appear as query parameters (`?recipient=…`) or as
* a bare download reference embedded in a message. Everything else is
* left untouched. */
export function sanitizeReportText(text: string): string {

@dobby-coder dobby-coder Bot Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Non-blocking hardening note redacted while the associated advisory is embargoed. Two residual defense-in-depth gaps on the last-line scrub were noted; neither is a confirmed leak and both are safe to defer.

Reduce the data an opt-in crash report carries so a report cannot
include sensitive values. Full details are withheld while the
associated security advisory is still in draft.

The scrub logic lives in a dependency-free reportScrub module so it can
be unit-tested without the SvelteKit env/runtime.

Closes #325
@dobby-coder
dobby-coder Bot force-pushed the harden-crash-report-minimization branch from 509a478 to 82632a5 Compare July 25, 2026 23:06
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.

Harden crash-reporter data minimization

0 participants