iOS: keep secure rectangles and the view tree on windows away from the screen's origin - #30
dsheikherev wants to merge 6 commits into
Conversation
…screen
JS measures BugseeSecure views and published rectangles with
measureInWindow, in the React root's window, and iOS handed them to the
SDK as they were. The SDK's contract puts secure rectangles in screen
points, and the SDK that composes every window of the app on the screen
(iPad Stage Manager and Split View, iPhone Duo side by side) draws them
there: in a window away from the screen's origin each mask landed that
far up and left of its view, which was recorded in the clear. Android
already moves its rectangles by the root's display origin
(ReactRootOriginTracker).
The store now keeps JS's rectangles and serves them moved by where the
React root's window starts in the frame the SDK records: its place on
the screen where the SDK composes, its frame.origin on a Mac, where the
SDK records the key window's scene alone. A fractional origin only grows
a rectangle, edges saturate, and the version moves only when what is
served changes. BGSRNReactRootOriginTracker keeps the root view weakly
and reads its window's place; it searches for the root only on a JS
publish, so the SDK's pulls never walk the windows.
BGSRNSecureRectanglePulls refreshes the origin on the SDK's pull at most
every 100 ms, before the snapshot when on main, as Android's
SecureRectanglePulls does.
Support tests 246/246: the store (origin, version, rounding, saturation,
displays), the tracker (root cache, no search on pulls, the last origin
kept, exceptions) and the pulls (throttle, off main, the served origin).
The example builds with CocoaPods and SPM; launch, secure-component and
view-tree e2e pass on iOS 27 and 26.5 simulators. On an iPad Air 5 in
Stage Manager, window at {359, 64}, with the SDK built from bugsee-cocoa:
the mask lies on the component in the video and in the report
screenshot within 1-2 px, before and after a scroll, and is gone after
unmount.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… nodes
The vh origin was the hosting window's frame.origin, the offset the SDK
used to add to every native node. The SDK now places its nodes in the
frame it records, on iOS every window of the app at its place on the
screen, so in a Stage Manager window or the right-hand one side by side
the React tree sat that far from the native one: frame.origin is the
window's place in its scene, {0, 0} there.
The origin is now BGSRNWindowRecordedOrigin, the same one that moves the
secure rectangles. The tests compare a node plus the origin with the
SDK's own placement. view-tree e2e passes on iOS 27 and 26.5 simulators;
the simulator's scene fills its screen, so the old and the new origin
agree there and only a device with a window away from the screen's
origin tells them apart.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…s windows BGSRNSdkKeyWindow and BGSRNSdkWalkedWindows copied the SDK's earlier logic: the key window chosen by isKeyWindow over connectedScenes, which since the iOS 15 SDK every scene's key window reports, and only the key scene's windows walked. The SDK now takes the application's key window while its scene is in the foreground, the window the user brought forward last, and where it composes the app's windows on the screen it records and walks the windows of every foreground scene on that screen. Mirroring that keeps the React root found in a window the SDK records. Not unit tested: the test runner has no application and no scenes. The Support tests, the example build and the e2e above all ran with this change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Stale comment
Deep code review
This PR moves iOS secure rectangles and the managed view-tree origin from the React root window’s points onto the screen, matching Android’s tracker/pulls split and the unreleased cocoa SDK that composites every window. The store, rounding, versioning, and on-main pull refresh are carefully built. Two issues should be fixed before merge.
Findings
P1 High —
BGSRNSdkComposesScreen()treats every real iOS device as the new compositor, but this repo still pins 7.0.0-beta3. Beta3 records the key window’s scene and draws rectangles / native nodes in scene points (frame.origin). Adding the screen origin inverts the Stage Manager / Split View bug this PR is fixing: masks and the React tree sit one window-offset away from the content, so secret UI is recorded in the clear. iPhone (origin {0,0}) is unchanged; iPad multitasking is not. Task 6.6 review I1 previously requiredframe.originfor this reason.P2 Medium —
setSecureRectanglespublishes coordinates on the TurboModule queue (origin still {0,0} until a root has been found) and only thendispatch_asyncs the origin read. The SDK pulls on main and will re-read on the version bump from that publish. Until the hop runs, Stage Manager windows are served unmoved.Overall risk: High
High because the P1 is a privacy defect on the currently declared SDK, in the exact iPad configuration the PR exists to fix. Against the unreleased compositor SDK the design looks right (and the iPad Stage Manager
secure-componentrun is good evidence). I did not run the Support XCTests or e2e here (Linux agent, no iOS toolchain).Merge recommendation
Do not merge onto main while
native-versions.json/ios/Support/Package.swiftstill pin 7.0.0-beta3. Land it with the compositor SDK (bugsee-cocoa #170–#172), or gateBGSRNWindowRecordedOrigin/ window walking on that SDK so beta3 keepsframe.origin.Most important to fix
- Tie screen-space origin to the SDK that actually composites, and bump the pin in the same change.
- Publish rectangles only after the origin has been applied, on main, so the SDK never observes an unmoved version.
- Still missing: managed view-tree on a device window that is not at the screen origin (called out in the test plan). Simulator e2e cannot distinguish old vs new origin.
What looks solid
- Served-buffer versioning only when the moved rectangles change; outward floor/ceil for a fractional origin; int32 saturation.
- Weak cached root and no search on the SDK pull (brownfield-safe); on-main refresh-before-snapshot is stricter than Android’s post-to-UI-thread model.
- Support tests cover store origin/version, tracker cache vs search, and pull throttling.
Sent by Cursor Automation: Bugsee code review
| const CGRect inFixedSpace = [window convertRect:window.bounds toCoordinateSpace:fixedSpace]; | ||
| const CGRect onScreen = [fixedSpace convertRect:inFixedSpace | ||
| toCoordinateSpace:screen.coordinateSpace]; | ||
| return [NSValue valueWithCGPoint:onScreen.origin]; |
There was a problem hiding this comment.
P1 High — This path assumes the iOS SDK composites every window onto the screen. That is not 7.0.0-beta3, which is still what native-versions.json, ios/Support/Package.swift, and Package.resolved pin.
BGSRNSdkComposesScreen() is only “not Catalyst / not iOS-on-Mac”. On a real iPhone or iPad it always returns YES, so this function (and the vh origin that calls it) always adds the window’s place on the screen.
Impact: Beta3 records the key window’s scene and draws secure rectangles and native nodes in that scene’s points. frame.origin is {0,0} for a Stage Manager / Split View window; the correct origin for beta3 is therefore {0,0}. After this change the wrapper serves screen points (e.g. {359, 64} from the PR’s iPad run). Masks and the React tree sit that far down and right of the views. Secret UI is recorded in the clear. Task 6.6 review I1 switched to frame.origin for this contract (BGSCaptureViewHierarchyEngine adding frame.origin on SDK 0d9c9d0a3).
Scenario: Consumer builds this wrapper against the declared 7.0.0-beta3 pin (CocoaPods / SPM). iPad Stage Manager or the right-hand Split View window. <BugseeSecure> and vh both go through BGSRNWindowRecordedOrigin. iPhone is fine ({0,0} == {0,0}); simulator e2e is also fine because the scene fills the screen. The PR’s own Stage Manager check used a locally built cocoa SDK, not the pin.
Fix: Ship this origin only with the compositor SDK (bump the pin in this PR), or detect that SDK and keep window.frame.origin on beta3. Do not merge a main that still resolves 7.0.0-beta3.
There was a problem hiding this comment.
Agreed, this is the gate the PR body states: the screen origin is right only with an SDK that composes the app's windows on the screen (bugsee-cocoa #170–#172). This PR does not merge while the pin is 7.0.0-beta3; the pin moves to the release with #170–#172 in the same change. Detecting the SDK at runtime would be dead code: the wrapper vendors exactly the pinned SDK. Converted to draft until then.
setSecureRectangles wrote the rectangles to the store on the JS thread
and only then asked main to read where the React root's window sits on
the screen. The SDK pulls on main, so a pull between the two got the new
rectangles moved by the origin the store held before: CGPointZero until
the first read. For a window away from the screen's origin (Stage
Manager, Split View, an iPad scene not at {0, 0}) a region JS had just
secured was recorded in the clear for that frame.
The tracker now takes the publish: on main, it reads the window's place
first and then writes the rectangles, so no pull sees one without the
other. Android has no such gap; it reads its origin on host resume and
on every layout of the root, before JS can measure anything.
A test reads the store from inside the origin read and fails if the
rectangles are already there.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The comments gave two rates for the same pull, "2-3 times a second" (Android's) and "ten times a second". The SDK's contract for secureRectanglesForDisplay: is once per captured frame and per report screenshot, on main, and from the touch filter at most every 100 ms while nothing captures. Every iOS comment now says that. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With React roots in two windows the tracker follows the first one it finds until that one leaves its window, and the rectangles JS measured in the other window are moved by the wrong origin. Android's tracker holds one root too. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Deep code review
Re-review of 282233b (prior review was 4af4f55). The store, rounding, versioning, weak root cache, and on-main pull refresh are still carefully built. One of the two earlier issues is fixed; the other is not.
Prior findings
P2 — fully addressed in 99012d7. setSecureRectangles now hops to main and -publishCoordinates:forDisplay: reads the window’s place before it writes the rectangles. testAPublishReadsThePlaceBeforeItWritesTheRectangles fails if the store already has the new rects during the origin read. That publish-order leak is gone.
P1 High — not addressed. BGSRNSdkComposesScreen() is still “not Catalyst / not iOS-on-Mac”. The pin is still 7.0.0-beta3. Beta3 records the key window’s scene and draws rectangles / native nodes in scene points (frame.origin). Adding the screen origin inverts the Stage Manager / Split View bug this PR exists to fix: masks and the React tree sit one window-offset away from the content, so secret UI is recorded in the clear. iPhone (origin {0,0}) is unchanged; simulator e2e cannot tell the origins apart. The PR body states the compositor SDK requirement; the later commits only document pull cadence and the one-root tracker limit.
No new P0–P3 on the tracker, pulls, or store. I did not run the Support XCTests or e2e here (Linux agent, no iOS toolchain). CI on this head was still pending at review time.
Overall risk: High
High because the remaining P1 is a privacy defect on the currently declared SDK, in the exact iPad configuration the PR is meant to fix. Against the unreleased compositor SDK the design looks right (the iPad Stage Manager secure-component run is good evidence).
Merge recommendation
Do not merge onto main while native-versions.json / ios/Support/Package.swift still pin 7.0.0-beta3. Land it with the compositor SDK (bugsee-cocoa #170–#172), or gate BGSRNWindowRecordedOrigin / window walking on that SDK so beta3 keeps frame.origin.
Most important to fix
- Tie screen-space origin to the SDK that actually composites, and bump the pin in the same change.
- Still missing: managed view-tree on a device window that is not at the screen origin (called out in the test plan).
What looks solid
- Publish is origin-then-write on main; a pull cannot observe new rectangles at a stale origin.
- Served-buffer versioning only when the moved rectangles change; outward floor/ceil for a fractional origin; int32 saturation.
- Weak cached root and no search on the SDK pull (brownfield-safe); on-main refresh-before-snapshot is stricter than Android’s post-to-UI-thread model.
- Support tests cover store origin/version, tracker cache vs search, publish order, and pull throttling.
Sent by Cursor Automation: Bugsee code review
| #if TARGET_OS_MACCATALYST | ||
| return NO; | ||
| #else | ||
| return !NSProcessInfo.processInfo.isiOSAppOnMac; |
There was a problem hiding this comment.
P1 High — Still open after 99012d7 / 282233b. This helper is only “not Catalyst / not iOS-on-Mac”, so on a real iPhone or iPad BGSRNWindowRecordedOrigin always adds the window’s place on the screen (and BGSRNSdkWalkedWindows takes the multi-scene walk).
The declared pin is still 7.0.0-beta3 (native-versions.json, ios/Support/Package.swift, Package.resolved). Beta3 records the key window’s scene and draws secure rectangles and native nodes in that scene’s points. frame.origin is {0,0} for a Stage Manager / Split View window; the correct origin for beta3 is therefore {0,0}. After this change the wrapper serves screen points (e.g. {359, 64} from the PR’s iPad run). Masks and the React tree sit that far down and right of the views. Secret UI is recorded in the clear. Task 6.6 review I1 switched to frame.origin for this contract (BGSCaptureViewHierarchyEngine adding frame.origin on SDK 0d9c9d0a3).
The PR body already says ship with cocoa #170–#172. That SDK is not what this branch resolves. Comments and the Stage Manager check (run against a locally built cocoa SDK) do not change what a consumer of main gets.
Scenario: Consumer builds this wrapper against the declared 7.0.0-beta3 pin. iPad Stage Manager or the right-hand Split View window. <BugseeSecure> and vh both go through BGSRNWindowRecordedOrigin. iPhone is fine ({0,0} == {0,0}); simulator e2e is also fine because the scene fills the screen.
Fix: Ship this origin only with the compositor SDK (bump the pin in this PR), or detect that SDK and keep window.frame.origin on beta3. Do not merge onto a main that still resolves 7.0.0-beta3.
There was a problem hiding this comment.
Agreed, this is the gate the PR body states: the screen origin is right only with an SDK that composes the app's windows on the screen (bugsee-cocoa #170–#172). This PR does not merge while the pin is 7.0.0-beta3; the pin moves to the release with #170–#172 in the same change. Detecting the SDK at runtime would be dead code: the wrapper vendors exactly the pinned SDK. Converted to draft until then.


Summary
measureInWindow), and the SDK draws secure rectangles in screen points. The store now serves them moved by where that window starts in the frame the SDK records, as Android'sReactRootOriginTrackerdoes. NewBGSRNReactRootOriginTrackerkeeps the root view weakly and searches for it only on a JS publish. NewBGSRNSecureRectanglePullsrefreshes the origin on the SDK's pull, at most every 100 ms. A JS publish reads the window's place first and only then writes the rectangles, both on main, where the SDK pulls, so no pull serves new rectangles at an origin not yet read.vhorigin is that same window origin instead ofwindow.frame.origin, the window's place in its scene.BGSRNSdkKeyWindow/BGSRNSdkWalkedWindowsfollow the SDK's current key-window pick and composed window list.Needs the iOS SDK that composes the app's windows on the screen (bugsee-cocoa #170–#172, not released yet). 7.0.0-beta3 records only the key window's scene and draws rectangles in its points, so with beta3 a window away from the screen's origin would be off the other way. Ship with that SDK.
Test plan
window.frame.originfor the view tree)yarn test(88 suites, 1651 passed, 1 skipped),yarn lint,yarn typechecklaunch,secure-component,view-treeon an iOS 26.5 simulator with 7.0.0-beta3launch,view-treeandsecure-component(5/5) on iOS 27 and 26.5 simulators;secure-component3 more times on iOS 27 after the publish-order fix, 5/5 each. That SDK writesvideo.auxv2, sosecure-component's iOS check that novideo.auxexists was adapted for the run; the adaptation is not in this PRsecure-componentscenario against the dead endpoint: the mask lies on the component in the video and in the report screenshot within 1–2 px, mounted and after the scroll, and is gone after unmount. Without this change it would sit at {60, 260} on the screen, outside the windowe2e on iOS 27 simulators also needs #31 (the example traps at launch there without the UIScene lifecycle).
🤖 Generated with Claude Code