fix(errorReporting): minimize data in opt-in crash reports#328
fix(errorReporting): minimize data in opt-in crash reports#328dobby-coder[bot] wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
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.
| * 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 { |
There was a problem hiding this comment.
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
509a478 to
82632a5
Compare
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.tsmodule so it can be unit-tested without the SvelteKit env/runtime.Testing
src/lib/reportScrub.test.tscovers 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, andnpm run lintall pass.Closes #325