From c73a0c8edfcdae8ee8a461d43c6324de1a16e595 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 30 Mar 2026 16:30:09 +0200 Subject: [PATCH 1/4] fix: prevent session replay screenshot PII leak on PixelCopy timeout - 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 --- .changeset/warm-pixels-glow.md | 5 +++++ .../posthog/android/replay/PostHogReplayIntegration.kt | 10 +++++----- 2 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 .changeset/warm-pixels-glow.md diff --git a/.changeset/warm-pixels-glow.md b/.changeset/warm-pixels-glow.md new file mode 100644 index 00000000..32ef9a10 --- /dev/null +++ b/.changeset/warm-pixels-glow.md @@ -0,0 +1,5 @@ +--- +'posthog-android': patch +--- + +Fix session replay screenshot PII leak on PixelCopy timeout and improve masking loop early exit diff --git a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt index ccd3169c..9bf08f35 100644 --- a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt @@ -982,13 +982,13 @@ public class PostHogReplayIntegration( return@request } - maskableWidgets.forEach { + for (rect in maskableWidgets) { if (isOnDrawnCalled) { config.logger.log("Session Replay screenshot discarded due to screen changes.") success = false - return@forEach + break } - canvas.drawRoundRect(RectF(it), 10f, 10f, paint) + canvas.drawRoundRect(RectF(rect), 10f, 10f, paint) } } else { config.logger.log("Session Replay screenshot discarded due to screen changes.") @@ -1021,9 +1021,9 @@ public class PostHogReplayIntegration( try { // await for 1s max - latch.await(1000, TimeUnit.MILLISECONDS) + val completed = latch.await(1000, TimeUnit.MILLISECONDS) - if (success) { + if (completed && success) { base64 = bitmap.webpBase64() } } catch (e: Throwable) { From 2ef4bd346e909b69c0909339153ddc9a8e08a70f Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 30 Mar 2026 16:48:06 +0200 Subject: [PATCH 2/4] fix: move mask rect collection to main thread and add fallback rects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../replay/PostHogReplayIntegration.kt | 304 ++++++++++++------ 1 file changed, 205 insertions(+), 99 deletions(-) diff --git a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt index 9bf08f35..890c1eff 100644 --- a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt @@ -23,6 +23,7 @@ import android.graphics.drawable.VectorDrawable import android.os.Build import android.os.Handler import android.os.HandlerThread +import android.os.Looper import android.text.InputType import android.util.TypedValue import android.view.Gravity @@ -615,6 +616,31 @@ public class PostHogReplayIntegration( } } + /** + * Returns the global visible rect for masking purposes. + * Falls back to location + dimensions when the view is in a transient state + * (animation, layout pass) to ensure we never silently skip masking a view + * that PixelCopy already captured on screen. + */ + private fun View.globalVisibleRectForMasking(): Rect? { + globalVisibleRect()?.let { return it } + + // Fallback: compute from location + dimensions. + // Less accurate (ignores clipping by parents) but ensures we don't miss masks. + return try { + val location = IntArray(2) + getLocationOnScreen(location) + if (width > 0 && height > 0) { + Rect(location[0], location[1], location[0] + width, location[1] + height) + } else { + null + } + } catch (e: Throwable) { + config.logger.log("Session Replay globalVisibleRectForMasking fallback failed: $e.") + null + } + } + /** * Fast visibility check that reuses a scratch Rect instead of allocating. * Only checks whether the view has a non-empty visible rect, does not return the rect. @@ -627,6 +653,40 @@ public class PostHogReplayIntegration( } } + /** + * Lenient visibility check for masking purposes. + * Unlike [isVisible], this does NOT require [isViewStateStableForMatrixOperations] + * because PixelCopy may have already captured the view on screen even if the view + * is in a transient state (animation, layout pass). For masking, we err on the side + * of treating uncertain views as visible to avoid leaking PII. + */ + private fun View.isLikelyVisibleForMasking(): Boolean { + try { + if (width <= 0 || height <= 0) return false + + if (isAttachedToWindow) { + if (windowVisibility != View.VISIBLE) return false + + var current: Any? = this + while (current is View) { + val view = current + val transitionAlpha = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) view.transitionAlpha else 1f + if (view.alpha <= 0 || transitionAlpha <= 0 || view.visibility != View.VISIBLE) { + return false + } + current = view.parent + } + // Don't check hasGlobalVisibleRect — it fails for views in transient state + return true + } + } catch (e: Throwable) { + config.logger.log("Session Replay isLikelyVisibleForMasking failed: $e.") + // err on side of masking + return true + } + return false + } + /** * Returns the global visible rect of just the text content area within a TextView. * For EditText, Button, and CompoundButton subclasses (CheckBox, RadioButton, Switch), @@ -662,6 +722,14 @@ public class PostHogReplayIntegration( } } + /** + * Like [getTextAreaGlobalVisibleRect] but with a fallback for masking purposes. + * Ensures we never silently skip masking a text view due to transient state. + */ + private fun TextView.getTextAreaGlobalVisibleRectForMasking(): Rect? { + return getTextAreaGlobalVisibleRect() ?: globalVisibleRectForMasking() + } + private fun View.isViewStateStableForMatrixOperations(): Boolean { return try { isAttachedToWindow && @@ -736,7 +804,7 @@ public class PostHogReplayIntegration( } view.isNoCapture() -> { - view.globalVisibleRect()?.let { + view.globalVisibleRectForMasking()?.let { maskableWidgets.add(it) } } @@ -758,7 +826,7 @@ public class PostHogReplayIntegration( if (maskIt) { // For EditText, mask only the text area (excluding padding and compound drawables) // For regular TextView, mask the full view - view.getTextAreaGlobalVisibleRect()?.let { + view.getTextAreaGlobalVisibleRectForMasking()?.let { maskableWidgets.add(it) } } @@ -766,7 +834,7 @@ public class PostHogReplayIntegration( view is Spinner -> { if (view.shouldMaskSpinner()) { - view.globalVisibleRect()?.let { + view.globalVisibleRectForMasking()?.let { maskableWidgets.add(it) } } @@ -774,7 +842,7 @@ public class PostHogReplayIntegration( view is ImageView -> { if (view.shouldMaskImage()) { - view.globalVisibleRect()?.let { + view.globalVisibleRectForMasking()?.let { maskableWidgets.add(it) } } @@ -782,7 +850,7 @@ public class PostHogReplayIntegration( view is WebView -> { if (view.isAnyInputSensitive()) { - view.globalVisibleRect()?.let { + view.globalVisibleRectForMasking()?.let { maskableWidgets.add(it) } } @@ -802,7 +870,9 @@ public class PostHogReplayIntegration( val viewChild = view.getChildAt(i) ?: continue - if (!viewChild.isVisible()) { + // Use lenient visibility for masking: don't skip views just because they're + // in a transient state (animation, layout). PixelCopy already captured them. + if (!viewChild.isLikelyVisibleForMasking()) { continue } @@ -816,62 +886,81 @@ public class PostHogReplayIntegration( return true } - private fun findMaskableComposeWidgets( + /** + * Core compose widget detection logic. Must be called on the main thread. + */ + private fun doFindMaskableComposeWidgets( view: View, maskableWidgets: MutableList, ) { - val latch = CountDownLatch(1) - - // compose requires the handler to be on the main thread - // see https://github.com/PostHog/posthog-android/issues/203 - mainHandler.handler.post { - try { - val semanticsOwner = - (view as? RootForTest)?.semanticsOwner ?: run { - config.logger.log("View is not a RootForTest: $view") - return@post + try { + val semanticsOwner = + (view as? RootForTest)?.semanticsOwner ?: run { + config.logger.log("View is not a RootForTest: $view") + return + } + val semanticsNodes = semanticsOwner.getAllSemanticsNodes(true) + + semanticsNodes.forEach { node -> + val hasText = node.config.contains(SemanticsProperties.Text) + val hasEditableText = node.config.contains(SemanticsProperties.EditableText) + val hasPassword = node.config.contains(SemanticsProperties.Password) + val hasImage = node.config.contains(SemanticsProperties.ContentDescription) + + // isEnabled=false means the modifier has no effect, as if it was never applied + // Check the node itself and its ancestors for mask/unmask modifiers + val isMaskEnabled = node.hasActiveModifier(PostHogReplayMask) + val isUnmaskEnabled = node.hasActiveModifier(PostHogReplayUnmask) + + when { + // postHogUnmask has precedence over everything, skip masking + isUnmaskEnabled -> { + // do not mask this node } - val semanticsNodes = semanticsOwner.getAllSemanticsNodes(true) - - semanticsNodes.forEach { node -> - val hasText = node.config.contains(SemanticsProperties.Text) - val hasEditableText = node.config.contains(SemanticsProperties.EditableText) - val hasPassword = node.config.contains(SemanticsProperties.Password) - val hasImage = node.config.contains(SemanticsProperties.ContentDescription) - - // isEnabled=false means the modifier has no effect, as if it was never applied - // Check the node itself and its ancestors for mask/unmask modifiers - val isMaskEnabled = node.hasActiveModifier(PostHogReplayMask) - val isUnmaskEnabled = node.hasActiveModifier(PostHogReplayUnmask) - - when { - // postHogUnmask has precedence over everything, skip masking - isUnmaskEnabled -> { - // do not mask this node - } - // postHogMask forces masking - isMaskEnabled -> { - maskableWidgets.add(node.boundsInWindow.toRect()) - } + // postHogMask forces masking + isMaskEnabled -> { + maskableWidgets.add(node.boundsInWindow.toRect()) + } - // no active modifier, apply default config rules - else -> { - when { - (hasText || hasEditableText) && (config.sessionReplayConfig.maskAllTextInputs || hasPassword) -> { - maskableWidgets.add(node.boundsInWindow.toRect()) - } + // no active modifier, apply default config rules + else -> { + when { + (hasText || hasEditableText) && (config.sessionReplayConfig.maskAllTextInputs || hasPassword) -> { + maskableWidgets.add(node.boundsInWindow.toRect()) + } - hasImage && config.sessionReplayConfig.maskAllImages -> { - maskableWidgets.add(node.boundsInWindow.toRect()) - } + hasImage && config.sessionReplayConfig.maskAllImages -> { + maskableWidgets.add(node.boundsInWindow.toRect()) } } } } - } catch (e: Throwable) { - // swallow possible errors due to compose versioning, etc - config.logger.log("Session Replay findMaskableComposeWidgets (main thread) failed: $e") + } + } catch (e: Throwable) { + // swallow possible errors due to compose versioning, etc + config.logger.log("Session Replay findMaskableComposeWidgets (main thread) failed: $e") + } + } + + private fun findMaskableComposeWidgets( + view: View, + maskableWidgets: MutableList, + ) { + // If already on main thread (e.g. when findMaskableWidgets is called from main thread), + // run inline to avoid deadlock from posting to ourselves and waiting. + if (Looper.myLooper() == Looper.getMainLooper()) { + doFindMaskableComposeWidgets(view, maskableWidgets) + return + } + + val latch = CountDownLatch(1) + + // compose requires the handler to be on the main thread + // see https://github.com/PostHog/posthog-android/issues/203 + mainHandler.handler.post { + try { + doFindMaskableComposeWidgets(view, maskableWidgets) } finally { latch.countDown() } @@ -943,85 +1032,102 @@ public class PostHogReplayIntegration( val height = view.height.densityValue(screenDensity) var base64: String? = null + // Step 1: Collect maskable rects on the main thread. + // This ensures thread-safe access to View properties (text, visibility, layout, etc.) + // and avoids silent rect drops from isViewStateStableForMatrixOperations() returning + // false on a background thread during animations or layout passes. + val maskableWidgets = mutableListOf() + val rectsLatch = CountDownLatch(1) + var rectsSuccess = true + + mainHandler.handler.post { + try { + if (!findMaskableWidgets(view, maskableWidgets)) { + rectsSuccess = false + } + } catch (e: Throwable) { + config.logger.log("Session Replay findMaskableWidgets failed: $e.") + rectsSuccess = false + } finally { + // Reset isOnDrawnCalled on the main thread right after collecting rects. + // Since we're on the main thread, no onDraw can fire between this reset + // and the end of this post. Any subsequent onDraw (after we return) will + // set isOnDrawnCalled = true, which we check after PixelCopy to detect + // screen changes between rect collection and pixel capture. + isOnDrawnCalled = false + rectsLatch.countDown() + } + } + + val rectsCompleted = + try { + rectsLatch.await(1000, TimeUnit.MILLISECONDS) + } catch (e: Throwable) { + config.logger.log("Session Replay findMaskableWidgets timed out: $e.") + false + } + + if (!rectsCompleted || !rectsSuccess) { + return null + } + + // Step 2: Capture the screen with PixelCopy, then apply pre-computed masks. val bitmap = Bitmap.createBitmap(view.width, view.height, Bitmap.Config.ARGB_8888) - val latch = CountDownLatch(1) + val pixelCopyLatch = CountDownLatch(1) var success = true val handler = ensurePixelCopyHandler() - - // Track whether the PixelCopy callback has finished to avoid recycling the bitmap - // while the callback is still using it (e.g. if latch.await times out). - // We use the latch itself as the synchronization mechanism (await happens-before countDown) var callbackCompleted = false try { - // reset the isOnDrawnCalled since we are about to take a screenshot - isOnDrawnCalled = false - PixelCopy.request(window, bitmap, { copyResult -> try { if (copyResult != PixelCopy.SUCCESS) { config.logger.log("Session Replay PixelCopy failed: $copyResult.") success = false - } else { - if (!isOnDrawnCalled) { - val maskableWidgets = mutableListOf() - - if (findMaskableWidgets(view, maskableWidgets)) { - if (!bitmap.isValid()) { - config.logger.log("Session Replay Bitmap is invalid.") + } else if (isOnDrawnCalled) { + // Screen changed between rect collection and pixel capture. + // The pre-computed rects may not match the captured pixels. + config.logger.log("Session Replay screenshot discarded due to screen changes.") + success = false + } else if (maskableWidgets.isNotEmpty()) { + // Apply pre-computed masks to the captured bitmap. + if (!bitmap.isValid()) { + config.logger.log("Session Replay Bitmap is invalid.") + success = false + } else { + val canvas = + try { + Canvas(bitmap) + } catch (e: Throwable) { + config.logger.log("Session Replay Canvas creation failed: $e.") success = false - return@request + null } - - val canvas = - try { - Canvas(bitmap) - } catch (e: Throwable) { - config.logger.log("Session Replay Canvas creation failed: $e.") - success = false - return@request - } - + canvas?.let { for (rect in maskableWidgets) { - if (isOnDrawnCalled) { - config.logger.log("Session Replay screenshot discarded due to screen changes.") - success = false - break - } - canvas.drawRoundRect(RectF(rect), 10f, 10f, paint) + it.drawRoundRect(RectF(rect), 10f, 10f, paint) } - } else { - config.logger.log("Session Replay screenshot discarded due to screen changes.") - success = false } - } else { - config.logger.log("Session Replay screenshot discarded due to screen changes.") - // if isOnDrawnCalled is true, it means that the view has already been drawn - // again, so we don't need to draw the maskable widgets otherwise - // they might be out of sync (leaking possible PII) - success = false } } } catch (e: Throwable) { - config.logger.log("Session Replay PixelCopy failed: $e.") + config.logger.log("Session Replay PixelCopy callback failed: $e.") success = false } finally { - // reset the isOnDrawnCalled since we've taken the screenshot isOnDrawnCalled = false callbackCompleted = true - latch.countDown() + pixelCopyLatch.countDown() } }, handler) } catch (e: Throwable) { - config.logger.log("Session Replay PixelCopy failed: $e.") + config.logger.log("Session Replay PixelCopy request failed: $e.") success = false callbackCompleted = true - latch.countDown() + pixelCopyLatch.countDown() } try { - // await for 1s max - val completed = latch.await(1000, TimeUnit.MILLISECONDS) + val completed = pixelCopyLatch.await(1000, TimeUnit.MILLISECONDS) if (completed && success) { base64 = bitmap.webpBase64() From 86a0a6ff90ab0efe07eede74b2cb7541858cc288 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 30 Mar 2026 17:32:35 +0200 Subject: [PATCH 3/4] fix: move isOnDrawnCalled reset to right before PixelCopy.request 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. --- .../posthog/android/replay/PostHogReplayIntegration.kt | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt index 890c1eff..fdfadd4a 100644 --- a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt @@ -1049,12 +1049,6 @@ public class PostHogReplayIntegration( config.logger.log("Session Replay findMaskableWidgets failed: $e.") rectsSuccess = false } finally { - // Reset isOnDrawnCalled on the main thread right after collecting rects. - // Since we're on the main thread, no onDraw can fire between this reset - // and the end of this post. Any subsequent onDraw (after we return) will - // set isOnDrawnCalled = true, which we check after PixelCopy to detect - // screen changes between rect collection and pixel capture. - isOnDrawnCalled = false rectsLatch.countDown() } } @@ -1079,6 +1073,10 @@ public class PostHogReplayIntegration( var callbackCompleted = false try { + // Reset right before capture to keep the detection window tight (~1 vsync). + // Only draws that happen during the actual PixelCopy capture will be detected. + isOnDrawnCalled = false + PixelCopy.request(window, bitmap, { copyResult -> try { if (copyResult != PixelCopy.SUCCESS) { From a9b929602a312acaf6afc8ff16bbe0f59b0fe468 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 30 Mar 2026 17:50:13 +0200 Subject: [PATCH 4/4] fix: remove isOnDrawnCalled checks from findMaskableWidgets 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. --- .../replay/PostHogReplayIntegration.kt | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt index fdfadd4a..2c14f51c 100644 --- a/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt +++ b/posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt @@ -777,16 +777,21 @@ public class PostHogReplayIntegration( return this.isTextInputSensitive(ancestorUnmasked) || passwordInputTypes.contains(inputType - 1) } + /** + * Traverses the view tree and collects rects of views that need masking. + * Must be called on the main thread to ensure thread-safe access to View properties + * and a stable view tree (no concurrent modifications). + */ private fun findMaskableWidgets( view: View, maskableWidgets: MutableList, visitedViews: MutableSet = mutableSetOf(), - ): Boolean { + ) { val viewId = System.identityHashCode(view) // Check for cycles to prevent stack overflow if (viewId in visitedViews) { - return true + return } visitedViews.add(viewId) @@ -863,11 +868,6 @@ public class PostHogReplayIntegration( if (walkChildren && view is ViewGroup && view.childCount > 0) { for (i in 0 until view.childCount) { - if (isOnDrawnCalled) { - config.logger.log("Session Replay screenshot discarded due to screen changes.") - return false - } - val viewChild = view.getChildAt(i) ?: continue // Use lenient visibility for masking: don't skip views just because they're @@ -876,14 +876,9 @@ public class PostHogReplayIntegration( continue } - if (!findMaskableWidgets(viewChild, maskableWidgets, visitedViews)) { - // do not continue if the screen has changed - return false - } + findMaskableWidgets(viewChild, maskableWidgets, visitedViews) } } - - return true } /** @@ -1042,9 +1037,7 @@ public class PostHogReplayIntegration( mainHandler.handler.post { try { - if (!findMaskableWidgets(view, maskableWidgets)) { - rectsSuccess = false - } + findMaskableWidgets(view, maskableWidgets) } catch (e: Throwable) { config.logger.log("Session Replay findMaskableWidgets failed: $e.") rectsSuccess = false