fix(compliance): stop false redisCache deletion audit + drop full-event PII log - #247
Open
pallakartheekreddy wants to merge 1 commit into
Open
fix(compliance): stop false redisCache deletion audit + drop full-event PII log#247pallakartheekreddy wants to merge 1 commit into
pallakartheekreddy wants to merge 1 commit into
Conversation
…nt PII log
UserDeletionCleanupFunction seeded the deletion audit with redisCache:false then
unconditionally set redisCache -> true right after dataCache.del(key). DataCache.del
is a no-op that only logs when redis.enabled=false (the default, BaseJobConfig:28),
so the emitted AUDIT telemetry claimed the user's Redis cache was purged when it was
not -- a false Right-to-Erasure compliance trail.
- Set redisCache from config.redisEnabled so the audit reflects whether the cache
delete could actually run (true only when Redis is enabled).
- Remove `logger.info(s"${event}")` which dumped the entire deletion event
(userId and other PII) to logs at INFO.
Scoped, safe slice of D1/D3. The full D1 purge (ES user index, user_consent /
user_declarations / user_feed / cert-registry deletes, per-store verify-then-flag,
ML-PII delegation) plus the DataCache.del Unit->Long verification are deferred: they
need new store wiring, DBA delete grants, are irreversible, and per the design must
ship behind per-store feature flags with non-prod validation and DB backups.
Verified: jobs-core + user-deletion-cleanup compile (main + test Scala) under JDK 11.
Implements the safe portion of D1/D3 from implementation-designs/data-pipeline.md.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Safe, self-contained slice of the D1 (CRITICAL/compliance) and D3 (PII-in-logs) findings in
implementation-designs/data-pipeline.md, inUserDeletionCleanupFunction.The bug (false compliance audit)
The deletion audit map seeded
redisCache -> false, then setredisCache -> trueunconditionally right afterdataCache.del(key). ButDataCache.delis a no-op that only logs whenredis.enabled=false— and that's the default (BaseJobConfig.scala:28). So the emitted AUDIT telemetry claimed the user's Redis cache was purged when it was not — a false Right-to-Erasure (DPDP/GDPR Art.17) compliance trail.Separately,
logger.info(s"${event}")dumped the entire deletion event (userId + other PII) to logs at INFO.The fix
redisCache -> config.redisEnabled— the flag istrueonly when Redis is actually enabled (i.e. thedelcould run), so the audit is truthful.Two-line behavioral change, no new stores/wiring, fully reversible.
Verification
jobs-core+user-deletion-cleanupcompile (main + test Scala) under JDK 11 (mvn -pl user-org-jobs/user-deletion-cleanup -am test-compile→ BUILD SUCCESS).Scope / deferred
The full D1 purge — ES
userindex,user_consent/user_declarations/user_feed/ cert-registry deletes, per-store verify-then-flag, ML-PII delegation — plus the jobs-coreDataCache.delUnit→Longverification are intentionally not here: they need net-new store wiring, DBA delete grants, are irreversible, and per the design must ship behind per-store feature flags with non-prod validation and DB backups. This PR makes the audit honest now; the expanded purge is coordinated L-effort work.