Skip to content

feat(pii): crypto-shred erasure on delete (spec 19 stage B)#515

Merged
PaulDotterer merged 3 commits into
mainfrom
feat/spec-19-crypto-shred
Jul 5, 2026
Merged

feat(pii): crypto-shred erasure on delete (spec 19 stage B)#515
PaulDotterer merged 3 commits into
mainfrom
feat/spec-19-crypto-shred

Conversation

@PaulDotterer

Copy link
Copy Markdown
Contributor

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.AppendUserDeletionWithShred appends the UserDeleted event 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 like AppendEvent. OCC-retry safe (the DEK delete is idempotent, AC 13). All three UserDeleted emit sites route through it so the shred is identical everywhere (AC 8): API DeleteUser, the SCIM last-link delete, and the idp orphaned-passwordless-user auto-delete.

Projector redaction (AC 7/11)

SoftDeleteUserProjection now overwrites all seven PII columns (email, display/given/family name, preferred_username, picture, linux_username) with crypto.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)

  • AC 32: SyncUserSystemActions gains an explicit fail-closed delete-state check at the generation choke point — not the incidental is_deleted = FALSE filter, so a future Get change can't silently re-enable provisioning for an erased user — plus a graceful NotFound skip.
  • AC 33: AffectedFromEvent(UserDeleted) == SyncOpNone pinned.
  • AC 34: a rebuild dispatches no system actions — provisioning is a live-only RegisterEventListener, never a RegisterRebuildApply applier; 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=false disable 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_id on unrelated events replays fine because actor_id is never a sealed field (pseudonymization, the accepted residual-risk posture); AC 14 tx rollback — WithTx is 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). Local coderabbit review: no findings.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@PaulDotterer, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b5b0947a-50c4-4a5e-af43-5bf94736b0c5

📥 Commits

Reviewing files that changed from the base of the PR and between 265298a and b4a57ec.

⛔ Files ignored due to path filters (1)
  • internal/store/generated/users.sql.go is excluded by !**/generated/**
📒 Files selected for processing (13)
  • internal/api/idp_handler.go
  • internal/api/shred_delete_handler_test.go
  • internal/api/shred_provisioning_isolation_test.go
  • internal/api/system_actions.go
  • internal/api/user_handler.go
  • internal/projectors/user_listener.go
  • internal/scim/users.go
  • internal/scim/users_test.go
  • internal/store/queries/users.sql
  • internal/store/rebuild_fidelity_test.go
  • internal/store/shred.go
  • internal/store/shred_redaction_test.go
  • internal/store/shred_test.go
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/spec-19-crypto-shred

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@PaulDotterer PaulDotterer merged commit 2ce1872 into main Jul 5, 2026
11 checks passed
@PaulDotterer PaulDotterer deleted the feat/spec-19-crypto-shred branch July 5, 2026 07:31
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.

1 participant