Skip to content

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

Open
turnipdabeets wants to merge 8 commits into
mainfrom
fix/replay-flag-secure
Open

turnipdabeets wants to merge 8 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.

}

// Main thread only, like the window callbacks that add and remove these interceptors.
private val touchInterceptors = WeakHashMap<Window, TouchEventInterceptor>()

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.

P2 · Synchronize the interceptor cache across lifecycle threads

When background PostHog.close() overlaps window callbacks, uninstall() calls clearViewListeners() off-main and races getOrPut/remove on this unsynchronized WeakHashMap. Lost cache entries prevent teardown from finding attached interceptors. Follow the mutable-collection rule in AGENTS.md:28: confine accesses to main or guard them with one lock.

This branch has not been deployed

No deployments
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.

1 participant