fix: prevent session replay screenshot PII leak on PixelCopy timeout#471
Draft
marandaneto wants to merge 4 commits intomainfrom
Draft
fix: prevent session replay screenshot PII leak on PixelCopy timeout#471marandaneto wants to merge 4 commits intomainfrom
marandaneto wants to merge 4 commits intomainfrom
Conversation
- Check latch.await() return value so unmasked bitmaps are never encoded when the callback times out - Replace forEach+return@forEach with for+break so the masking loop exits immediately on screen changes
marandaneto
commented
Mar 30, 2026
| } | ||
|
|
||
| maskableWidgets.forEach { | ||
| for (rect in maskableWidgets) { |
Member
Author
There was a problem hiding this comment.
not a bug but wasting cycles
marandaneto
commented
Mar 30, 2026
| try { | ||
| // await for 1s max | ||
| latch.await(1000, TimeUnit.MILLISECONDS) | ||
| val completed = latch.await(1000, TimeUnit.MILLISECONDS) |
Member
Author
There was a problem hiding this comment.
the PixelCopy callback can take longer than the 1-second latch timeout
very unlikely but doing the change anyway
Member
Author
|
wondering if it helps with #450 |
Root cause of #450: findMaskableWidgets ran on the PixelCopy callback thread, causing three classes of PII leaks: 1. Thread safety: View properties (text, visibility, layout) were read from a background thread — undefined behavior on Android that can return stale/inconsistent values. 2. Silent rect drops: isViewStateStableForMatrixOperations() returns false during animations/layout passes, causing globalVisibleRect() to return null. The mask rect was silently not added — the view IS visible on screen (PixelCopy captured it) but no mask is applied. 3. Subtree skipping: isVisible() relies on hasGlobalVisibleRect() which also fails during transient state. Entire view subtrees were skipped even though PixelCopy captured them. Changes: - Move findMaskableWidgets to main thread (via mainHandler.post) before PixelCopy capture. This ensures thread-safe property access and views are in a stable state during traversal. - Add globalVisibleRectForMasking() that falls back to getLocationOnScreen + width/height when the strict stability check fails. - Add isLikelyVisibleForMasking() that checks alpha/visibility without requiring hasGlobalVisibleRect(), preventing subtree skipping. - Add getTextAreaGlobalVisibleRectForMasking() with same fallback. - Fix findMaskableComposeWidgets to detect when already on main thread and run inline, preventing deadlock. - Reset isOnDrawnCalled on main thread after rect collection to detect screen changes between rect collection and pixel capture.
auto-merge was automatically disabled
March 30, 2026 14:49
Pull request was converted to draft
Resetting the flag on the main thread after rect collection created a wide gap (thread switch + bitmap allocation) during which normal draws (cursor blinks, animations) would always set it to true, causing nearly every screenshot to be discarded. Move the reset to right before PixelCopy.request() on the executor thread, restoring the original ~1 vsync detection window.
Now that findMaskableWidgets runs on the main thread, these checks are harmful: isOnDrawnCalled is still true from the draw that triggered the snapshot, causing the method to always return false and bail early. On the main thread, onDraw cannot fire during traversal (we're blocking the looper), so the checks are unnecessary. Screen-change detection is handled by the isOnDrawnCalled check in the PixelCopy callback. Also changed return type to Unit since the method no longer signals early abort.
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.
💡 Motivation and Context
Fixes #450 — Session replay screenshots intermittently sent with literally nothing masked, leaking PII.
Root cause analysis
findMaskableWidgetsran on the PixelCopy callback thread (background), causing three classes of failures:Thread safety: View properties (
text,hint,visibility,alpha,childCount,getGlobalVisibleRect, etc.) were read from a background thread. On Android, the UI toolkit is single-threaded — reading from another thread is undefined behavior that can return stale or inconsistent values.Silent mask rect drops:
isViewStateStableForMatrixOperations()returnsfalseduring animations, layout passes, or transient state (common in React Native bridge updates). This causedglobalVisibleRect()to returnnull, and the mask rect was silently not added — no log, no error. The view IS visible on screen (PixelCopy captured it) but no mask is applied. When ALL maskable views happen to be in transient state, the entire screenshot goes out unmasked.Subtree skipping:
isVisible()relies onhasGlobalVisibleRect()which also fails during transient state. Entire view subtrees were skipped in the traversal even though PixelCopy captured them.Latch timeout PII leak: If the PixelCopy callback took >1s (e.g. due to Compose semantics traversal),
latch.await()timed out butsuccesswas stilltrue, encoding an unmasked bitmap.Approach
Instead of traversing the view tree on a background thread AFTER PixelCopy, we now:
isOnDrawnCalledright beforePixelCopy.request()to keep the detection window tight (~1 vsync)isOnDrawnCalled— if the screen changed during capture, discard💚 How did you test it?
./gradlew :posthog-android:compileReleaseKotlin)make testJava) — all pass📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file