feat(pii): crypto-shred erasure on delete (spec 19 stage B)#515
Conversation
…delity guard (spec 19, stage B)
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Part 2 of 4 for #502 (spec 19). Builds on the merged PR A envelope (#514). Covers ACs 7–15 and Section E (32–36): deletion always crypto-shreds — there is no separate erase operation. Retention/prune (PR C) and doctor key-invariant checks (PR D) follow.
Shared shred flow (AC 7/8/14)
store.AppendUserDeletionWithShredappends theUserDeletedevent and destroys the user's DEK in one transaction — all-or-nothing (a failed DEK delete rolls the event back; no half-erased state, AC 14), then fires post-commit listeners exactly likeAppendEvent. OCC-retry safe (the DEK delete is idempotent, AC 13). All threeUserDeletedemit sites route through it so the shred is identical everywhere (AC 8): APIDeleteUser, the SCIM last-link delete, and the idp orphaned-passwordless-user auto-delete.Projector redaction (AC 7/11)
SoftDeleteUserProjectionnow overwrites all seven PII columns (email, display/given/family name, preferred_username, picture, linux_username) withcrypto.RedactionSentinel([redacted]— never null, never ciphertext) in the same statement as the soft-delete. Live delete and rebuild share this one path, so a rebuild reproduces a deleted user as the sentinel while live users reproduce 1:1 (the full-fidelity fixture now seeds an erased user, so the byte-identical round-trip proves it).Provisioning isolation (Section E, 32–36)
SyncUserSystemActionsgains an explicit fail-closed delete-state check at the generation choke point — not the incidentalis_deleted = FALSEfilter, so a futureGetchange can't silently re-enable provisioning for an erased user — plus a graceful NotFound skip.AffectedFromEvent(UserDeleted) == SyncOpNonepinned.RegisterEventListener, never aRegisterRebuildApplyapplier; guard test seeds a provisioned user, rebuilds, asserts no new system-action rows.Tests (real Postgres)
DEK shredded on both API and SCIM delete (AC 8); SCIM
active=falsedisable does not shred; absent target → NotFound, no oracle (AC 13); re-add same email after erase mints a fresh DEK and the old ciphertext stays unreadable (AC 15); all seven PII columns redacted and reproduced as the sentinel on rebuild (AC 7/11); the three E-guards; shred flow atomicity + idempotency at the store level.By construction (noted, not separately tested): AC 12 — a deleted user's
actor_idon unrelated events replays fine becauseactor_idis never a sealed field (pseudonymization, the accepted residual-risk posture); AC 14 tx rollback —WithTxis atomic and there is no DB fault seam to inject.Verification
gofmt/vet/staticcheck clean; full
go test ./... -count=1: 34 packages, 0 failures. Redaction red-checked (columns keep plaintext when the sentinel param is neutralized — the fixture's own precondition asserts the pre-state). Localcoderabbit review: no findings.