Skip to content

fix(replay): mask content drawn outside a view's bounds - #1000

Merged
turnipdabeets merged 1 commit into
mainfrom
fix/replay-masking-gaps-3x
Oct 30, 2026
Merged

turnipdabeets merged 1 commit into
mainfrom
fix/replay-masking-gaps-3x

Conversation

@turnipdabeets

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Backports #976 to 3.x, as @ioannisj suggested on that PR. Apps with screenshotMode on have the same leak: masks only covered each masked view's bounds, so anything a view drew past them was uploaded.

Now masked:

  • unclipped images that overflow their view (aspect-fill and natural-size content modes)
  • unclipped layer shadows, which repeat the text
  • button titles and images laid out past the button
  • subviews overflowing masked labels and text views, including ph-no-capture ones
  • image attachments in labels and text views when maskAllImages is on (symbols and asset-catalog images stay visible)

Differences from #976:

  • Wireframe mode is unchanged. 3.x still has wireframes, and they use isLabelSensitive and isTextViewSensitive to star out text. The attachment check is in the screenshot mask walk instead of those helpers, so wireframe text isn't masked because of an attachment.
  • The attachment helper is in a private extension. The 3.x class is larger, and keeping it in the class body hit SwiftLint's 1200-line type_body_length error.

As in #976, masks aren't clipped to a clipping ancestor, so an overflowing image in a clipping cell can get a mask that spills into neighbouring cells. That fails closed.

💚 How did you test it?

The tests from #976 in PostHogMaskFailClosedTest came across unchanged. Local runs on Xcode 26.4.1, iPhone 17 Pro simulator:

  • make lint: passes.
  • make buildSdk: passes. It ran before the helper moved to the extension; after the move I re-ran lint and the iOS masking suite.
  • make testOniOSSimulator: passes. It also ran before the move.
  • PostHogMaskFailClosedTest after the move: 27 tests pass.
  • make test (macOS, where this code is compiled out): 2 app-group storage tests fail (excludes new and existing project folders from backup, writes to disk in a project token folder under a group container directory). I didn't re-run these on unmodified main; fix(replay): mask content drawn outside a view's bounds #976 saw the same backup-exclusion failure on its unmodified base.
  • Not run: make maskSnapshots and make testPresentationMasks, because the pinned snapshot toolchain isn't installed here. There's no automated test that wireframe text stays the same, since toWireframe is private.

📝 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)

Built with Claude Code (Claude Opus 5.5) by applying the squash commit from #976 to main. The only conflict was a doc comment in UIView+Util.swift, where I kept main's wireframe note. The wireframe and lint differences above came up during the backport.

🤖 Generated with Claude Code

Backport of #976 to 3.x. Screenshot masks only covered each masked view's bounds, so unclipped images, layer shadows, overflowing button titles and subviews of masked text stayed visible. Image attachments in text are masked when maskAllImages is on, in the screenshot walk only so wireframe text is unchanged.
@turnipdabeets turnipdabeets self-assigned this Oct 30, 2026
@turnipdabeets
turnipdabeets requested a review from a team as a code owner October 30, 2026 16:16
@github-actions

Copy link
Copy Markdown
Contributor

posthog-ios Compliance Report

Date: 2026-10-30T16:31:03.355172+00:00
Duration: 223772ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 3323ms
Format Validation.Event Has Uuid ✅ 2843ms
Format Validation.Event Has Lib Properties ✅ 596ms
Format Validation.Distinct Id Is String ✅ 441ms
Format Validation.Token Is Present ✅ 2795ms
Format Validation.Custom Properties Preserved ✅ 2768ms
Format Validation.Event Has Timestamp ✅ 2913ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 279ms
Retry Behavior.Retries On 503 ✅ 11700ms
Retry Behavior.Does Not Retry On 400 ✅ 2430ms
Retry Behavior.Does Not Retry On 401 ✅ 4639ms
Retry Behavior.Respects Retry After Header ✅ 7741ms
Retry Behavior.Implements Backoff ✅ 21436ms
Retry Behavior.Retries On 500 ✅ 9239ms
Retry Behavior.Retries On 502 ✅ 9346ms
Retry Behavior.Retries On 504 ✅ 5421ms
Retry Behavior.Max Retries Respected ❌ 21794ms
Deduplication.Generates Unique Uuids ✅ 2986ms
Deduplication.Preserves Uuid On Retry ✅ 9318ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10458ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 7815ms
Deduplication.No Duplicate Events In Batch ✅ 2994ms
Deduplication.Different Events Have Different Uuids ✅ 3266ms
Compression.Sends Gzip When Enabled ✅ 295ms
Batch Format.Uses Proper Batch Structure ✅ 2791ms
Batch Format.Flush With No Events Sends Nothing ✅ 319ms
Batch Format.Multiple Events Batched Together ✅ 2996ms
Error Handling.Does Not Retry On 403 ✅ 4745ms
Error Handling.Does Not Retry On 413 ✅ 4820ms
Error Handling.Retries On 408 ✅ 9257ms

Failures

retry_behavior.max_retries_respected

Expected 4 requests, got 6

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 2827ms
Request Payload.Flags Request Uses V2 Query Param ✅ 2707ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 2789ms
Request Payload.Flags Request Omits Authorization Header ✅ 2803ms
Request Payload.Token In Flags Body Matches Init ✅ 2966ms
Request Payload.Groups Round Trip ✅ 2909ms
Request Payload.Groups Default To Empty Object ✅ 3370ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 3156ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 2840ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 2854ms
Request Lifecycle.No Flags Request On Init Alone ✅ 89ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 2795ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 5598ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 2841ms
Retry Behavior.Retries Flags On 502 ✅ 2838ms
Retry Behavior.Retries Flags On 504 ✅ 2896ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 3145ms

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

No actionable issues found in this 3.x screenshot-masking backport. The changes preserve wireframe behavior and the public API.

AI-assisted review.

@turnipdabeets
turnipdabeets merged commit 9b7e911 into main Oct 30, 2026
54 checks passed
@turnipdabeets
turnipdabeets deleted the fix/replay-masking-gaps-3x branch October 30, 2026 19:05
turnipdabeets added a commit that referenced this pull request Nov 10, 2026
…line-masking

Brings in #911's current head, which includes main.

Conflict resolutions:
- Portals: take #911's model (the portal and, for one shown elsewhere, its rect; the whole portal is masked unless the source is clean). The whole-portal and unsettled-source regions are tagged .rectOnly. wholePortals and hasInPlaceCopy are removed: the walk already masks the whole portal whenever a mask sits under or over its source.
- GPU capture: mirrorAndMask gains a .fallback case for #911's culled full-window cover, checked before inline masking; maskableRects/GPUMaskWalk stay for the plain walk, sharing the cover check.
- postHogMask reporters keep #911's region(of:) (collapsed reporters with a model size) with an explicit inline tag: .extent on the main walk, .rectOnly through a portal.
- MaskedRegion's view and layer initializers keep #1000's drawn outsets with an explicit .owned tag.
- contentsKeys stays internal for inline masking and gains #911's contentsTransformKey.
- GPU capture tests take #911's FrameOutcome-based expectNoUnmaskedMagenta, with noCaptureView from GPUCaptureTestSupport.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019xLynxqQ97w5iqqo4Dxyi5
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