Repository navigation
fix(replay): mask image views by every image source they can draw - #956
Conversation
isImageViewSensitive only checked view.image, so a UIImageView drawing animationImages, highlightedImage or highlightedAnimationImages was recorded unmasked in screenshots even with maskAllImages on. It now masks the view when any of those sources is not an SF Symbol or asset-catalog image. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
posthog-ios Compliance ReportDate: 2026-10-23T15:41:43.147170+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ❌ | 249ms |
| Format Validation.Event Has Uuid | ✅ | 427ms |
| Format Validation.Event Has Lib Properties | ✅ | 2746ms |
| Format Validation.Distinct Id Is String | ✅ | 2718ms |
| Format Validation.Token Is Present | ✅ | 2790ms |
| Format Validation.Custom Properties Preserved | ✅ | 2851ms |
| Format Validation.Event Has Timestamp | ✅ | 2857ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 2808ms |
| Retry Behavior.Retries On 503 | ✅ | 5402ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 4802ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 4776ms |
| Retry Behavior.Respects Retry After Header | ✅ | 7754ms |
| Retry Behavior.Implements Backoff | ✅ | 21747ms |
| Retry Behavior.Retries On 500 | ✅ | 9250ms |
| Retry Behavior.Retries On 502 | ✅ | 9115ms |
| Retry Behavior.Retries On 504 | ✅ | 5326ms |
| Retry Behavior.Max Retries Respected | ❌ | 21741ms |
| Deduplication.Generates Unique Uuids | ✅ | 2917ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 9306ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 16742ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 7842ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 2814ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 3196ms |
| Compression.Sends Gzip When Enabled | ✅ | 2812ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 2715ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 294ms |
| Batch Format.Multiple Events Batched Together | ✅ | 3044ms |
| Error Handling.Does Not Retry On 403 | ✅ | 2372ms |
| Error Handling.Does Not Retry On 413 | ✅ | 4766ms |
| Error Handling.Retries On 408 | ✅ | 5261ms |
Failures
format_validation.event_has_required_fields
Expected 1 requests, got 0
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 | ✅ | 2874ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 2835ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 2673ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 2778ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 2808ms |
| Request Payload.Groups Round Trip | ✅ | 2729ms |
| Request Payload.Groups Default To Empty Object | ✅ | 3002ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 2702ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 2799ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 2752ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 64ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 2815ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 5474ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 2652ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 2811ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 2714ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 3046ms |
dustinbyrne
left a comment
There was a problem hiding this comment.
The expanded image-source checks fix the existing highlighted/animated-image gap. One privacy regression remains: newly sensitive image containers stop mask discovery before visible overflowing descendants are visited. Consider preserving descendant masking and adding the regression case described inline.
AI-assisted review.
A masked UIImageView stopped the mask walk, so a subview drawn outside its bounds (clipsToBounds = false) was left unmasked. Keep walking its visible subviews after adding the image view's own mask. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
marandaneto
left a comment
There was a problem hiding this comment.
worth checking #956 (review) tho
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Addressed in c97ca12: a masked image view now keeps walking its visible subviews, so an overflowing label gets its own mask. Reply with details in the thread. |
The macos-26 runner image now ships SwiftFormat 0.64, whose new rules (nestedIf, repeatedGuard, extensionAttributes, ...) fail unchanged code. Install the release binary .swiftformat is written for, checksum-verified, instead of relying on the preinstalled version. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
💡 Motivation and Context
Session replay screenshots could upload private images unmasked even with
maskAllImageson.isImageViewSensitiveonly checkedview.image, but aUIImageViewcan also drawanimationImages,highlightedImageandhighlightedAnimationImages. Running the new tests on unchangedmainshowed the private image unmasked in the uploaded frame in four cases:animationImageswith noimageanimationImagesplaying over an SF Symbol placeholderimage(the placeholder exempted the whole view)highlightedImagehighlightedAnimationImagesThe check now looks at all four sources and masks the view if any of them is not an SF Symbol or asset-catalog image.
ph-no-capturemasks the view if it has any image source. This matches thesession-replay-privacyspec (behaviors 6, 8 and 11): the asset/symbol exception now applies to every image the view can draw, and masking fails closed.A masked image view also no longer stops the mask search: its visible subviews are still checked, so a subview drawn outside the image view's bounds (
clipsToBounds = false) keeps its own mask. Before, that subview was left unmasked when the image view had a privateimage(already the case onmain), and this PR would have extended that to the newly masked image views.This came up in bot review on the 4.0 branch (#873). The gap is on
maintoo, so the fix lands here first.Behavior change
maskAllImageson (the default), more image views are masked: any view holding a private image in one of the four sources, even one it isn't drawing right now (for example a privatehighlightedImageon a view that isn't highlighted). Checking only the current state would be wrong for paths that render after the masks are collected, and animation frames change between collection and render anyway.screenshotMode = false), such a view'simageis now left out of the wireframe even when thatimageis an asset or symbol. This loses some detail but exposes nothing.postHogNoMask()/ph-no-mask, or globally withmaskAllImages = false.💚 How did you test it?
PostHogMaskPresentationPrivacyTest(run bymake testPresentationMasks). For each image source they take the same steps as a screenshot capture: collect the masks, render the window, bake the masks in throughRRWireframe.toDict(), then decode the uploaded image and sample the pixel at the image view.image, and a highlighted view falling back toimage, were already masked.collectMaskableRectstook 0.09 ms per pass, against 0.01 ms for an empty window (simulator).make testOniOSSimulator: 1567 tests pass.make testPresentationMasks: 31/31.make buildSdksucceeds.make lintpasses.make test(macOS): the only failures are the 13 group-container storage tests that also fail locally onmain.make maskSnapshotson the pinned Xcode 26.3 / iOS 26.2 and it passes. Locally on Xcode 26.4.1 / iOS 26.4, goldens recorded withmain's masking code match this branch on all 24 cases.mainand with this branch, then viewed in PostHog. Row 1 is a plain privateimage; rows 2–5 are an animating view with noimage, animation frames over an SF Symbol,highlightedImage, andhighlightedAnimationImages; row 6 is an SF Symbol.main: rows 2–5 show the private image (replay)📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Built with Claude Code. The tests were written first and run against unchanged
mainto confirm the leaks, then the fix was applied. A first attempt that checked only the source the view currently draws was dropped: it missed the highlighted fallback toimage, and let a bundled placeholder exempt private animation frames. Two pre-PR review cycles followed, each with an independent reviewer and triage agent. They kept the changeset wording and the asset-animation negative test, and left the wireframe-mode detail loss as documented above.🤖 Generated with Claude Code