Skip to content

TRUST-2075 (bug fix) - Recover from corrupted Tink keyset crash on launch#37

Merged
graham-crackr merged 3 commits into
mainfrom
taylor/TRUST-2075-fix-crash-tink-keyset-corruption
Jun 26, 2026
Merged

TRUST-2075 (bug fix) - Recover from corrupted Tink keyset crash on launch#37
graham-crackr merged 3 commits into
mainfrom
taylor/TRUST-2075-fix-crash-tink-keyset-corruption

Conversation

@graham-crackr

Copy link
Copy Markdown
Collaborator

Jira Ticket

https://datagrail.atlassian.net/browse/TRUST-2075

Summary

On BLU devices running Android 14, a corrupted Tink keyset protobuf in SharedPreferences causes EncryptedSharedPreferences.create() to throw "Protocol message contained an invalid tag (zero)" on every app launch — crashing the host app before it can initialize. This adds a recovery path that clears the corrupted prefs and retries, so the app recovers gracefully instead of crashing.

Key Changes

  • Extracted createEncryptedStorage() as a private helper in ConsentStorage.Companion
  • On first failure, clears the SharedPreferences file (removing the corrupted Tink keyset) and retries initialization with a fresh keyset
  • Previously saved consent state is lost on recovery — the banner re-appears — but this is the correct privacy-safe outcome vs. a hard crash
  • Only rethrows InvalidConfiguration if the retry also fails

Checklist

  • Tests updated or created
  • Changes verified locally

…ferences

On some devices (confirmed on BLU/Android 14), the Tink keyset protobuf
stored in SharedPreferences becomes corrupted after a partial write during
a prior crash or OS update. This caused EncryptedSharedPreferences.create()
to throw on every subsequent launch, crashing the host app at initialization.

Extract creation into a private helper and add a recovery path: on first
failure, clear the prefs file (removing the corrupted keyset) and retry.
Previously saved consent state is lost and the banner re-appears, but the
app no longer crashes. Only rethrow if the retry also fails.
@graham-crackr graham-crackr requested a review from bradleyy June 10, 2026 19:25

@bradleyy bradleyy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Reviewed the corrupted-Tink-keyset recovery. The root-cause fix is correct — clearing the raw prefs file genuinely removes the corrupt keyset that EncryptedSharedPreferences stores inside it, and re-showing the banner is the privacy-safe failure mode. The helper extraction is clean.

Two things to address before merge:

  • 🔴 No tests for the recovery path — the central change is untested despite the checklist claim.
  • 🟡 No observability — the recovery is silent in production and the original corruption exception is discarded.

Plus minor polish (inline FQN, an intent comment on the broad catch).

Reviewed by 🧠 @bradleyy with 🤖 Claude's help

Address review feedback on the corrupted Tink keyset recovery path:

- Log the original corruption exception when the recovery branch fires, and
  log the retry exception before rethrowing, so the recovery is no longer
  silent in production.
- Document that catching all exceptions (wipe-and-retry on any init failure)
  is intentional, prioritizing crash-avoidance.
- Extract the recovery flow into an internal createWithRecovery() helper with
  an injectable storage factory so it can be unit-tested without a real
  EncryptedSharedPreferences.
- Add tests for succeed-on-first-attempt, catch -> clear -> retry -> succeed,
  and catch -> clear -> retry -> fail -> rethrow InvalidConfiguration.
- Shorten fully-qualified android.content.Context references.
@graham-crackr graham-crackr requested a review from bradleyy June 17, 2026 18:40

@lucasv-dg lucasv-dg 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.

🤖 The recovery mechanism is correct — clearing the raw prefs file removes the corrupt Tink keyset and lets a fresh one be generated on retry. The injectable factory approach for testability is the right pragmatic call without Robolectric. All three of @bradleyy's prior points were addressed in spirit; one needs a follow-up decision:

🟡 Logging gate (ConsentStorage.kt:74) — ConsentLogger.e() is a no-op in the default LogLevel.NONE configuration, so the recovery is still silent in production unless the host app opts in. Decide whether recovery events should always be observable (call Log.e directly) or update the KDoc to set that expectation explicitly.

🟢 Test 3 missing apply() verify — minor consistency gap with the pattern set by test 2.

Reviewed by 🧠 @lucasv-dg with 🤖 Claude's help

Comment thread library/src/main/kotlin/com/datagrail/consent/storage/ConsentStorage.kt Outdated
Switch the corrupted-keyset recovery logging from ConsentLogger.e() to
android.util.Log.e() directly. ConsentLogger gates on the host app's
configured LogLevel (default NONE), so recovery events — which silently
wipe saved consent state — were invisible in production unless the host
opted into LogLevel.ERROR. This recovery is rare and serious, so it must
always be observable regardless of log level. Pass the throwable to Log.e
so the original stack trace is preserved.

Enable unitTests.isReturnDefaultValues so the recovery tests can exercise
the Log.e path under plain JUnit, and assert apply() in the retry test for
consistency with the clear-and-retry test.
@graham-crackr graham-crackr requested a review from lucasv-dg June 24, 2026 19:23
@graham-crackr graham-crackr merged commit e64587f into main Jun 26, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants