Skip to content

Add client-side GDPR right-to-erasure for PII encryption keys#3449

Merged
woksin merged 6 commits into
mainfrom
feat/client-pii-key-erasure
Jul 3, 2026
Merged

Add client-side GDPR right-to-erasure for PII encryption keys#3449
woksin merged 6 commits into
mainfrom
feat/client-pii-key-erasure

Conversation

@woksin

@woksin woksin commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Enables GDPR right-to-erasure (crypto-shredding) from a Chronicle client application. Deleting a person's PII encryption key makes every PII value encrypted under that subject permanently unreadable — everywhere it appears. Previously the client IPIIManager only threw NotImplementedException, so there was no way to trigger erasure from application or ops code.

Added

  • Chronicle clients can delete a subject's PII encryption key to satisfy GDPR right-to-erasure (crypto-shredding), via IPIIManager.DeleteEncryptionKeyFor and IEventStore.PII. IPIIManager can also be injected directly.

Changed

  • Reading a PII value whose encryption key has been deleted now returns an empty value instead of throwing, so read models and queries for an erased subject keep working.

Removed

  • Removed the non-functional CreateAndRegisterKeyFor from IPIIManager; it only ever threw NotImplementedException, and encryption keys are already registered automatically on first use.

woksin and others added 5 commits July 3, 2026 13:15
Expose GDPR crypto-shredding through the kernel so a subject's PII
encryption key can be deleted, making all PII encrypted under it
permanently unreadable.

- Add a DeleteEncryptionKey operation (and request) to the ICompliance
  gRPC service contract.
- Implement it in ComplianceService by resolving the kernel IPIIManager
  grain via IGrainFactory (mirroring the Recommendations service).
- Remove the non-functional CreateAndRegisterKeyFor from the IPIIManager
  grain; the append path already auto-registers keys on first use, and
  the operation would add a new key revision that silently orphaned any
  PII already encrypted under the prior revision.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reading a PII value after its encryption key was deleted previously
threw MissingEncryptionKey, which crashed every read model and query
touching an erased subject. Guard the release path with HasFor and
surface the value as empty instead, so consumers of an erased subject
keep working. Decryption of values whose key is still present is
unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The client IPIIManager previously threw NotImplementedException, so an
application had no way to trigger GDPR right-to-erasure. Implement it to
proxy over the Chronicle client connection to the compliance service.

- PIIManager.DeleteEncryptionKeyFor forms the encryption key identifier
  from the subject the PII was encrypted under and calls the kernel via
  the ICompliance gRPC contract.
- Expose it as IEventStore.PII and register IPIIManager in the client
  service collections so applications can inject it directly.
- Remove the non-functional CreateAndRegisterKeyFor from the client
  IPIIManager to match the kernel contract.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Unit specs assert the client PIIManager calls the compliance contract
  with the correctly-formed identifier for the subject.
- Integration spec appends a PII event under a subject, reads it back
  decrypted, deletes the key through IEventStore.PII, and asserts the
  PII property is empty (not throwing) while non-PII content survives.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a rule to the corpus that specs (and code) should reference a
concept's NotSet sentinel rather than a raw string.Empty/Guid.Empty
that implicitly converts, and apply it to the PII compliance handler
specs (EventStoreName.NotSet / EventStoreNamespaceName.NotSet).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@woksin woksin added the minor label Jul 3, 2026
@woksin

woksin commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@einari could you have a look when you get a chance? 🙏

This wires up the client-side trigger for GDPR crypto-shredding — IPIIManager.DeleteEncryptionKeyFor / IEventStore.PII — which until now only threw NotImplementedException. It proxies over the existing ICompliance gRPC contract to the kernel IPIIManager grain (no new transport; mirrors the Recommendations service).

Two things I'd especially value your take on:

  1. Erased-read behavior. Reads of PII after key deletion used to throw MissingEncryptionKey, which crashes any read model/query for an erased subject. I made PIICompliancePropertyValueHandler.Release return empty on a missing key (default agreed as "good for now"). Known limitation: it can't yet tell a legitimately erased key from a wrong-subject/never-provisioned bug — both surface as empty. The proper fix (deferred) is a configurable ChronicleOptions.Compliance display policy [Empty | Placeholder | Throw] above the crypto handler, plus a delete-time tombstone so genuine bugs still throw loudly.

  2. Semver. I removed the non-functional CreateAndRegisterKeyFor from the public IPIIManager (it only threw). I labeled this minor since it was never usable, but a strict reading of "removed a public interface member" is major — your call.

Not merging — leaving it for your review.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Cratis Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 786dc3a Previous: 9c41e1c Ratio
Cratis.Chronicle.Benchmarks.AppendManyBenchmark.AppendManyEvents(EventCount: 100) 171652856.56666666 ns (± 31208453.233121686) 68128824.7 ns (± 7671149.609764319) 2.52
Cratis.Chronicle.Benchmarks.AppendManyBenchmark.AppendManyEvents(EventCount: 1000) 1164955576.3333333 ns (± 59627787.49573026) 587439487.4 ns (± 115163188.51672608) 1.98

This comment was automatically generated by workflow using github-action-benchmark.

CC: @einari

Releasing a PII value and ensuring a key on append both did HasFor
followed by GetFor — two storage round-trips per key on a cold cache,
on the read/decrypt hot path. Introduce a single-query TryGetFor that
returns null when no key exists, implement it across every backend, and
have GetFor delegate to it (throwing MissingEncryptionKey only when a
caller demands the key). The PII handler now resolves the key in one
call for both release and key-creation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@woksin

woksin commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up: addressed the decrypt-path performance concern from above.

Release (and the append-time EnsureKeyFor) used to do HasFor then GetFor — two encryption-key-store round-trips per key on a cold cache, right on the read/decrypt hot path (and the downstream "many people's PII in one query" pattern is exactly the cold-per-subject case). Added a single-query TryGetFor to IEncryptionKeyStorage that returns null when the key is absent; GetFor now delegates to it (?? throw MissingEncryptionKey). The handler resolves the key in one call — no pre-check, no exception-as-control-flow.

Implemented across all seven backends (Mongo, SQL, InMemory, Cache, Composite, HashiCorp Vault, Azure Key Vault). Verified green: handler specs, cache-store specs, Mongo encryption-key specs, and the end-to-end erasure + PII round-trip integration.

One thing to weigh for the semver call: this adds a member to the public IEncryptionKeyStorage SPI, so a third-party storage provider would need to implement TryGetFor. That's the second interface-surface change in this PR (alongside the CreateAndRegisterKeyFor removal) — both point toward major if we're strict about the public contract. Happy to make TryGetFor a default interface method (non-breaking) instead if you'd prefer to keep it minor; I went with an explicit member for clean mocking in specs. Your call.

@woksin

woksin commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Context on the removed CreateAndRegisterKeyFor, in case it's useful for the semver call — removing it loses nothing.

As shaped (CreateAndRegisterKeyFor(identifier) — generates a key internally, takes none as input), it had no use case that the existing behavior doesn't already cover, and it was a footgun for the one thing it looked useful for:

  • Normal lifecycle — already covered: EnsureKeyFor auto-generates and saves a key the first time PII is encrypted for a subject. Pre-creating a key before any PII exists just makes a key protecting nothing.
  • Re-provision after erasure — also covered by auto-create (the next append for the subject makes a fresh key); and the old data staying unreadable is the correct, permanent-erasure behavior.
  • Key rotation — where it seems useful, it's actually dangerous today: SaveFor with no revision writes a new revision, but the encrypt/decrypt path always uses the latest revision and never pins one, so creating a new key for an existing subject would silently orphan all PII already encrypted under the prior revision.

If explicit key management is ever wanted, it should come back deliberately shaped, not as this stub:

  1. BYOK / customer-managed keysRegisterKeyFor(identifier, key) that accepts caller/HSM-supplied key material (the old signature couldn't — it only generated).
  2. Rotation — genuinely valuable, but it first needs the decrypt path to become revision-aware (each stored value records which revision encrypted it), otherwise rotation corrupts existing data.

The method was scaffolded for symmetry with Delete; symmetry isn't a use case. The Delete half is what right-to-erasure actually needs — which is what this PR keeps.

@woksin woksin added patch and removed minor labels Jul 3, 2026
@woksin woksin merged commit 3c09a40 into main Jul 3, 2026
101 of 102 checks passed
@woksin woksin deleted the feat/client-pii-key-erasure branch July 3, 2026 22:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant