Version secure rectangles in native units

[krassx]

Summary

• SecureRectRegistry stores per-owner rectangles and publishes the per-display union.
• Conversion to Android pixels or iOS points happens before outward rounding, and a repeated publish does not bump the version.

Test plan

• DOTNET_ROLL_FORWARD=LatestMajor dotnet test Tests~/WrapperPolicy/Bugsee.WrapperPolicy.Tests.csproj --filter SecureRectRegistryTests

Made with Cursor

[cursor]

Stale comment

Deep review: Version secure rectangles in native units

Reviewed fc6d346 against the wrapper contract, not just the new files. Production privacy still goes through Bugsee.AddSecureRectangle → Android addSecureRectangle / iOS _bugsee_add_secure_rect (float divide by mainScreen.scale, no outward round). BugseeWrapperProxy.getSecureRectangles still returns a cached {0, 0} and ignores display. This PR is a policy kernel only; it does not change what gets masked today.

The kernel itself is the right shape: store Unity-pixel LTRB per (owner, display), convert with floor(left/top) / ceil(right/bottom) after the divide, sort owners so the packed buffer is stable, and bump version only when the converted ints change. That matches the existing iOS pixel→point divide and the packed [version, count, l,t,r,b, …] layout the Android wrapper already documents.

No P0/P1. Three P2s on this patch, plus one integration landmine in code this PR does not touch.

Could not run the listed dotnet test command here (dotnet is not installed).

Overall risk: Medium

Merge recommendation: Request changes

Most important to fix

1. Add Unity .meta files for Runtime/WrapperPolicy/ and SecureRectRegistry.cs (every other Runtime script has them).
2. Reject non-finite pixelsPerNativeUnit — NaN/∞ bypass <= 0 and publish 0-size rects, which drops the mask.
3. Normalize or reject inverted LTRB in Set (Android Rect with left > right is empty → no redaction).
4. When wiring: do not feed Snapshot() into BugseeWrapperProxy.SetSecureRectangles as written. That setter always does _secureVersion++ and overwrites arr[0], which destroys this PR’s “same rects → same version” invariant. Store the packed snapshot as-is, or have getSecureRectangles(display) call Snapshot and return it. Also: getContext is already documented as off the Unity main thread; getSecureRectangles will be too (capture). If Snapshot is the JNI implementation, the dictionaries need a lock or a published immutable int[]. If you Snapshot on main and only publish a frozen buffer, the registry can stay unsynchronized.

Do not send registry output through IOSBridge.AddSecureRectangle / _bugsee_add_secure_rect — that path divides by scale again.

Positive

• Convert-then-outward-round is the correct privacy bias (10px @ 3x → 4 points, not truncated 3).
• Version is keyed off converted native ints, so a 1px source tweak that still maps to the same points does not churn native.
• Owner sort (Ordinal) makes the packed list independent of dictionary order.
• RemoveOwner copies keys before mutating the dictionary.
• The two tests pin the iOS 3x case and the no-bump / other-owner-survives path.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review: Version secure rectangles in native units

Re-reviewed 9325bd9 (harden on top of fc6d346) against the Android 7.x wrapper pull contract, not just the new files.

Prior P2s are fixed: Runtime .metas, non-finite scale rejection, inverted-edge normalize, and the proxy no longer overwrites arr[0] / ignores display.

Production privacy still goes through Bugsee.AddSecureRectangle → Android addSecureRectangle / iOS _bugsee_add_secure_rect. This kernel is still unwired from the public facade. Native does already poll BugseeWrapper.getSecureRectangles after setWrapper (2–3 Hz on Android). The empty miss is now {1, 0}, which matches the RN store’s INITIAL_VERSION = 1 sentinel. That is correct — and it makes the remaining version bug load-bearing the moment anyone does SetSecureRectangles(registry.Snapshot(…)).

The Android/iOS RN stores are explicit: the SDK re-reads rects only when version differs from the last poll. Empty never-published is version 1; the first change of the served set must step off that sentinel (nextVersion(1) → 2). This registry assigns version 1 to the first snapshot even when it already has rects. The existing test locks that in ({1, 1, 0, 0, 4, 4}).

Could not run dotnet test here (dotnet is not installed).

Overall risk: Medium

Merge recommendation: Request changes

Most important to fix

1. P1: First non-empty Snapshot must not use version 1. Treat “never published” as empty@version 1, then bump when converted ints differ (first HUD rects → version 2). Skip 1 in the incrementer so wrap cannot collide with the empty sentinel. Update the iOS 3x test accordingly.
2. P2: getSecureRectangles runs on the SDK capture thread; SetSecureRectangles will run on Unity main. Dictionary is not a safe publication. Publish an immutable int[] (volatile field / ConcurrentDictionary) so a racy miss cannot serve {1, 0} after native has already latched a real version.

Do not send registry output through IOSBridge.AddSecureRectangle / _bugsee_add_secure_rect — that path divides by scale again.

Positive

• Prior four P2s from fc6d346 are addressed in 9325bd9.
• Convert-then-outward-round remains the right privacy bias (10px @ 3x → 4 points).
• Proxy now keeps the packed snapshot version and keys the cache by the display native passes.
• Owner sort (Ordinal) keeps the packed list stable; RemoveOwner still copies keys before mutating.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Stale comment

Deep review: Version secure rectangles in native units

Re-reviewed 5765e3d (reserve version 1, on top of 9325bd9 / fc6d346) against the Android 7.x wrapper pull contract and the RN SecureRectangleStore that already implements it, not just the new files.

Prior findings are fixed. Empty never-published is version 1 ({1, 0}). The first converted change — including a first snapshot that already has rects — stores version 2 and skips 1 on wrap. Ios_scale_3_rounds_outward_after_convert and Empty_snapshot_then_set_bumps_to_version_2 lock that in. The Android cache is a ConcurrentDictionary and getSecureRectangles returns a clone, so a capture-thread pull cannot see a racy miss as the empty sentinel after a real version was published.

Checked against RN SecureRectangleStore: packed [version, count, l, t, r, b, …], INITIAL_VERSION = 1, freshness is != (not >), nextVersion skips 1, pull is on a Bugsee background thread, copy on the way out. This kernel matches that contract.

No remaining P0–P3 on this patch. Production privacy still goes through Bugsee.AddSecureRectangle → Android addSecureRectangle / iOS _bugsee_add_secure_rect. That is expected for a policy kernel; it is not a defect in these files.

Could not run dotnet test here (dotnet is not installed).

Overall risk: Low

Merge recommendation: Approve

Most important to fix

Nothing on this patch. When wiring:

1. Publish registry.Snapshot(display, scale) into SetSecureRectangles(display, …) using the same display id native passes (Display.getDisplayId(), usually 0).
2. Do not send registry output through IOSBridge.AddSecureRectangle / _bugsee_add_secure_rect — that path divides by mainScreen.scale again. iOS BugseeUnityWrapper still has no getSecureRectangles.
3. Runtime/WrapperPolicy.meta GUID is b8153596…; #3/#5 use bd4df1b4…. Resolve on merge so the folder GUID does not churn.

Positive

• 5765e3d implements the exact first-publish latch from the last review; empty-then-set and first-non-empty-is-2 are both tested.
• Convert-then-outward-round remains the right privacy bias (10px @ 3x → 4 points).
• Proxy keeps the packed snapshot version, keys by display, and no longer synthesizes {1, 0} for a display that is already in the map.
• Owner sort (Ordinal) keeps the packed list stable; RemoveOwner still copies keys before mutating.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Deep review: Version secure rectangles in native units

Re-reviewed fbdc80b (scaffold merge on top of 5765e3d / 9325bd9 / fc6d346) against the live Android wrapper registration, not just the new files.

Prior findings stay fixed. Empty never-published is version 1 ({1, 0}). First converted change stores version 2 and skips 1 on wrap. Non-finite scale throws. Inverted LTRB is normalized. Runtime .metas exist. Runtime/WrapperPolicy.meta now uses GUID bd4df1b4…, matching the other wrapper-policy PRs — that was the point of this merge.

The kernel itself still matches the RN SecureRectangleStore contract: packed [version, count, l, t, r, b, …], freshness is !=, convert-then-outward-round.

One new P2 from the merge: this PR’s Android sink is no longer what native polls. AndroidBridge only calls UnityWrapper.refineContext; UnityWrapperProvider is the setWrapper target and getSecureRectangles is hardcoded {1, 0}. Nothing constructs BugseeWrapperProxy. The concurrent per-display cache added here is unreachable. Wiring Snapshot into SetSecureRectangles — the path this file still documents — will not mask anything on the wrapper pull path.

Production privacy is still the push APIs (Bugsee.AddSecureRectangle → Android addSecureRectangle / iOS _bugsee_add_secure_rect). Expected for a policy kernel, but this PR’s Android file is now a false integration surface.

Could not run dotnet test here (dotnet is not installed).

Overall risk: Medium

Merge recommendation: Approve (already merged). Fix the P2 in a follow-up before anyone wires HUD rects to the wrapper pull path.

Most important to fix

1. P2: Do not publish registry.Snapshot into BugseeWrapperProxy.SetSecureRectangles. Native polls Java UnityWrapper.getSecureRectangles, which always returns {1, 0}. Serve the packed buffer from that Java method (JNI to a frozen C# snapshot, clone on the way out), keyed by the display id native passes (Display.getDisplayId()).
2. Do not send registry output through IOSBridge.AddSecureRectangle / _bugsee_add_secure_rect — that path divides by mainScreen.scale again. iOS BugseeUnityWrapper still has no getSecureRectangles.

Positive

• fbdc80b aligns WrapperPolicy.meta with #3/#5/#7 (bd4df1b4bbeb491a9a4211ad1762f5bd) instead of churning a second folder GUID.
• Convert-then-outward-round remains the right privacy bias (10px @ 3x → 4 points).
• First-publish latch, owner sort, and RemoveOwner key copy are unchanged and still correct.
• Tests still pin iOS 3x, inverted edges, non-finite scale, and empty → version 2.

  

_{Sent by Cursor Automation: Bugsee code review}

[cursor]

Severity: P2 Medium
Location: BugseeWrapperProxy.getSecureRectangles / SetSecureRectangles

Problem: This PR’s Android publication path stores packed snapshots on BugseeWrapperProxy, whose comment still says it is registered via Bugsee.setWrapper. After the scaffold merge (fbdc80b, which includes PR #9), that is false. AndroidBridge.EnsureWrapperRegistered only calls UnityWrapper.refineContext. The object native actually polls is Plugins/Android/UnityWrapperProvider.java UnityWrapper, installed from a ContentProvider:

Bugsee.setWrapper(INSTANCE);
// ...
public int[] getSecureRectangles(int display) {
    return new int[] { 1, 0 };
}

Nothing in the repo constructs BugseeWrapperProxy (new BugseeWrapperProxy has no callers). RegistrationSourceTests even asserts that AndroidBridge must not call setWrapper.

Impact: The versioned per-display ConcurrentDictionary cache this PR added is unreachable. Following this file — registry.Snapshot(display, scale) → SetSecureRectangles(display, packed) — looks correct in C# and still delivers empty buffers to the SDK. Secret UI stays in the clear on the wrapper pull path. Native has already latched version 1 from the Java stub, so even a later C# publish would be ignored unless Java starts returning a different version.

Scenario: A follow-up wires LateUpdate SetSecureRectangles(0, registry.Snapshot(0, scale)) as the last review suggested, when this proxy was still the setWrapper target on this branch. On device, UnityWrapperProvider already registered the Java singleton at initOrder 200. Capture polls BugseeWrapper.getSecureRectangles(0) on that singleton. Every poll is {1, 0}. Native keeps lastVersion = 1 and never applies masks. The sample HUD still goes through Bugsee.AddSecureRectangle, so today’s push path is unchanged — this only fails the moment someone treats this proxy as live.

Fix: Serve packed snapshots from the registered wrapper: have UnityWrapper.getSecureRectangles return a frozen [version, count, l,t,r,b,…] buffer (JNI to C# or a Java-side copy of registry.Snapshot), clone on the way out, and key by the display id native passes (Display.getDisplayId()). Keep version in the buffer; do not synthesize {1, 0} for a display that has already been published. Update or delete this proxy so it is not documented as setWrapper.

Do not send registry output through IOSBridge.AddSecureRectangle / _bugsee_add_secure_rect — that path divides by mainScreen.scale again.