Skip to content

fix(replay): mask FLAG_SECURE windows and discard frames with unplaceable masked views - #910

Merged
turnipdabeets merged 12 commits into
mainfrom
fix/replay-flag-secure
Oct 30, 2026
Merged

turnipdabeets merged 12 commits into
mainfrom
fix/replay-flag-secure

Conversation

@turnipdabeets

@turnipdabeets turnipdabeets commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Privacy gaps in session replay's screenshot mode (opt-in on 3.x):

  • FLAG_SECURE windows were recorded. Apps set FLAG_SECURE to keep a screen out of screenshots, but replay copied it anyway. These windows are now recorded as one fully masked (black) frame, and their pixels are never recorded or uploaded. The flag is checked when capture starts and again after the copy, so a window that turns secure mid-copy is masked too.
  • Taps on FLAG_SECURE windows were recorded. Touch coordinates (e.g. on a PIN pad) were still uploaded over the black frame. They're now dropped in screenshot mode. Flutter's Android touches come from this native interceptor, so posthog-flutter gets this through the native SDK; its frames are handled in fix(android): mask session replay frames on FLAG_SECURE windows posthog-flutter#685.
  • Animating views with sensitive content could be recorded unmasked. In the default (non-verified) capture, a view mid-animation or in a transient state (e.g. text selection) was skipped by the mask walk, so it went out unmasked. The frame is now discarded when such a view holds anything we'd mask. Animations with nothing to mask keep their frames.

Before this, the SDK's autocapture taps and interaction snapshots already skipped FLAG_SECURE windows, but replay did not. Wireframe mode (the 3.x default) still records secure windows after this PR; that's fixed in the stacked #912.

Intentional behaviour changes (patch):

  • Screens with FLAG_SECURE now show as a solid black frame in recordings, without taps.
  • Default screenshot capture now drops frames while a view below the root is in a transient state on a screen with something to mask (with default text masking, that's any text). Before, that whole subtree was recorded unmasked. Verified capture already discarded in this case. These discards now log their own reason.

The transient-state fix is the 3.x port of 37df5a2 from #908, and this change reaches the 4.0 branches when main is merged into them.

Follow-up: #913 replaces the duplicated mask rules (findMaskableWidgets / hasMaskableContent) with one shared classifier.

Deferred (not in this PR):

  • Mask the nearest stable parent instead of dropping the frame.
  • Cache the black frame instead of encoding it on each capture.
  • sdk-specs proposal for an OS screen-capture-protection requirement.

💚 How did you test it?

  • New tests, each confirmed to fail without its fix:
    • a secure window is recorded fully masked without copying its pixels (all 16 scale/colour/verification settings): captures a white frame, sets FLAG_SECURE, and checks the next frame is all black with no new PixelCopy, so stale pixels in the reused bitmap can't leak.
    • a window that turns secure during the copy is recorded fully masked (16 settings).
    • a touch on a secure window is not recorded in screenshot mode: touches are recorded before the flag is set and dropped after, and the app still receives them.
    • legacy capture discards a frame with a masked view in transient state: plain text inside a nested layout.
    • legacy mask walk is not poisoned by an animating view with nothing to mask (control).
  • ./gradlew spotlessCheck apiCheck :posthog-android:testDebugUnitTest passes.
  • Not tested on a device.

📝 Checklist

  • All commits are signed (see Commit signing)
  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

  • Claude Code. The maintainer chose to mask the whole FLAG_SECURE window rather than skip the frame, so recordings show that something private was there instead of freezing on the previous screen. Dropping taps and re-checking the flag after the copy follow from that decision.
  • A pre-PR review loop and an independent review/triage ran; their test, docs and logging fixes are included, and the larger design items are listed under Deferred above.

🤖 Generated with Claude Code

Also splits the legacy discard log so a transient-state discard says why, documents the FLAG_SECURE behaviour on the screenshot option, and points the duplicated mask rules at each other.

@posthog posthog Bot 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.

PostHog Review · Standard · reviewed e23f2a8: 1 Should fix.

See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.

@ioannisj ioannisj 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.

🤖 An AI agent wrote this on behalf of @ioannisj.

The branch conflicts with main now that #906 landed, GitHub shows it as CONFLICTING and a test merge hits posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt. One thing to watch in the resolution: the secure branch in toScreenshotWireframe sizes its frame with scaledScreenshotDimension(sourceWidth, screenshotScale), and #906 changed that computation on the normal path, so the black frame wants the same dp scaling or it comes out a different size than the frames around it. #912 and #913 stack on this branch, so they'll need a rebase once this moves too.

Sizes the secure-window frame with the same dp-scaled target as #906's normal path and masks before the downscale.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

@ioannisj ioannisj 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.

🤖 An AI agent wrote this on behalf of @ioannisj.

The merge looks right to me, the secure frame picks up the dp scaling and nothing from main got lost. One thing it did open: hasMaskableContent didn't pick up the insideNoCapture flag that #914 added to the walk, so an animating untagged child of an unclipped ph-no-capture container can still ship unmasked in legacy capture. Details on the thread, I have a failing test for it. Approving to unblock, happy for that one to land here or in #913.

turnipdabeets and others added 3 commits October 30, 2026 11:29
… the discard check (#913)

* refactor(replay): share one mask classifier between the mask walk and the discard check

* test(replay): cover Compose and hidden children in the mask classifier parity test

Drops the refactor changeset: an internal refactor has no user-facing changelog entry.

---------

Co-authored-by: Claude Opus 5.5 <[email protected]>
* fix(replay): record FLAG_SECURE windows as a black wireframe and drop their touches in all modes

* fix(replay): mask the wireframe when a window turns secure during the walk

* test(replay): cover the wireframe secure check before the walk
@turnipdabeets
turnipdabeets enabled auto-merge (squash) October 30, 2026 15:37
@turnipdabeets
turnipdabeets merged commit c09b501 into main Oct 30, 2026
16 checks passed
@turnipdabeets
turnipdabeets deleted the fix/replay-flag-secure branch October 30, 2026 15:55
turnipdabeets added a commit to PostHog/posthog-js that referenced this pull request Oct 30, 2026
Carries the FLAG_SECURE and animating-view replay fixes from PostHog/posthog-android#910.
Thegreatsura pushed a commit to Thegreatsura/posthog-js that referenced this pull request Oct 31, 2026
)

* chore(react-native-plugin): bump posthog-android to 3.74.0

* fix(react-native-plugin): leave native screenshot scale default for non-finite values

* docs(react-native): document the new Android screenshotScale default

* test(react-native-plugin): pin screenshotScale 1 to native resolution

* chore(react-native-plugin): bump posthog-android to 3.74.2

Carries the FLAG_SECURE and animating-view replay fixes from PostHog/posthog-android#910.

---------

Co-authored-by: Anna Garcia <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants