fix(observability): SQL leaked to Sentry a second way — EF Core error events

[mrviduus]

Summary

Found by the Sentry integration's own first day in production, not by a test.

PR #445 found and fixed EF Core breadcrumbs carrying Executed DbCommand … SELECT …. That fix was real but closed one channel, not the class. EF Core also logs a failed command at Error level, and Sentry's ILogger integration turns any Error into an event — so the first live capture of a PUT /me/progress failure arrived with the statement inline in the event message:

INSERT INTO reading_progresses (id, chapter_id, edition_id, locator,
  max_chapter_number, percent, site_id, updated_at, user_id) VALUES (@p0, …)

Two channels, one shared wrong assumption: that the SQL lived in the structured data bag. It never did — EF interpolates the statement into the human-readable text on both paths.

Scope of the exposure

• No parameter values left the process. EnableSensitiveDataLogging is off, so EF renders @p0 / '?', and Npgsql writes "Detail redacted as it may contain sensitive data" on the inner PostgresException.
• What did leave: statement shape and schema (table + column names).
• No book text, no reader identity.

Still a promise #445 explicitly made and broke.

Changes

SentryScrubber.Scrub drops any event whose Logger starts with Microsoft.EntityFrameworkCore, plus a message probe for Executed DbCommand / Failed executing DbCommand so a re-categorised logger can't reopen it (IsDatabaseCommandEvent).

Dropping loses no signal — that's what makes it right rather than a redaction exercise. The same failure is already reported by ExceptionMiddleware as a DbUpdateException with a full stack trace, SQLSTATE 23505 and the violated constraint name. Everything needed to debug; none of the SQL.

Tests

• Scrub_EfCoreCommandEvent_ReturnsNull — the exact shape that leaked
• Scrub_DbCommandInMessageWithoutEfLogger_ReturnsNull — logger re-categorisation can't reopen it
• Scrub_DbUpdateExceptionFromMiddleware_IsKept — locks that the signal survives
• 1284 unit tests green, build + dotnet format --verify-no-changes clean.

Rollback plan

git revert. No migration, no config, no schema. Unsetting SENTRY_DSN disables the whole path regardless.

Notes

The generalisable lesson: a scrubber written against one egress path will be bypassed by the next one. #445 rejected Sentry's OpenTelemetry exporter precisely because it bypasses BeforeSend — then shipped with two log-pipeline channels doing the same thing from the inside. Unit tests were green through both, because they asserted the scrubber's behaviour on the input shape we imagined rather than what the SDK actually assembles. Only reading a real captured event found either.

Follow-up (separate PR): the underlying 23505 on ix_reading_progresses_user_id_site_id_edition_id — an upsert race in UserDataEndpoints.UpsertProgress losing readers' positions — which is the genuine production bug this event was reporting.

🤖 Generated with Claude Code