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..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 @@ -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 && @@ -709,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) @@ -736,7 +809,7 @@ public class PostHogReplayIntegration( } view.isNoCapture() -> { - view.globalVisibleRect()?.let { + view.globalVisibleRectForMasking()?.let { maskableWidgets.add(it) } } @@ -758,7 +831,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 +839,7 @@ public class PostHogReplayIntegration( view is Spinner -> { if (view.shouldMaskSpinner()) { - view.globalVisibleRect()?.let { + view.globalVisibleRectForMasking()?.let { maskableWidgets.add(it) } } @@ -774,7 +847,7 @@ public class PostHogReplayIntegration( view is ImageView -> { if (view.shouldMaskImage()) { - view.globalVisibleRect()?.let { + view.globalVisibleRectForMasking()?.let { maskableWidgets.add(it) } } @@ -782,7 +855,7 @@ public class PostHogReplayIntegration( view is WebView -> { if (view.isAnyInputSensitive()) { - view.globalVisibleRect()?.let { + view.globalVisibleRectForMasking()?.let { maskableWidgets.add(it) } } @@ -795,83 +868,94 @@ 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 - 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 } - if (!findMaskableWidgets(viewChild, maskableWidgets, visitedViews)) { - // do not continue if the screen has changed - return false - } + findMaskableWidgets(viewChild, maskableWidgets, visitedViews) } } - - 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,18 +1027,47 @@ 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 { + findMaskableWidgets(view, maskableWidgets) + } catch (e: Throwable) { + config.logger.log("Session Replay findMaskableWidgets failed: $e.") + rectsSuccess = false + } finally { + 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 + // 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 -> @@ -962,68 +1075,52 @@ public class PostHogReplayIntegration( 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 - } - - maskableWidgets.forEach { - if (isOnDrawnCalled) { - config.logger.log("Session Replay screenshot discarded due to screen changes.") - success = false - return@forEach - } - canvas.drawRoundRect(RectF(it), 10f, 10f, paint) + canvas?.let { + for (rect in maskableWidgets) { + 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 - latch.await(1000, TimeUnit.MILLISECONDS) + val completed = pixelCopyLatch.await(1000, TimeUnit.MILLISECONDS) - if (success) { + if (completed && success) { base64 = bitmap.webpBase64() } } catch (e: Throwable) {