Skip to content

fix(stereo-camera): delegation never outranks the OS camera switch or a stored Deny - #1836

Merged
dfattal merged 1 commit into
mainfrom
fix/camera-consent-order
Oct 6, 2026
Merged

dfattal merged 1 commit into
mainfrom
fix/camera-consent-order

Conversation

@dfattal

@dfattal dfattal commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to #1819 (R3 consent). Not a release blocker (no installer registers a delegating client today), but it must land before one does.

The defect

As built, u_camera_consent_evaluate() allowed a registered consent-delegating client at step 4 — before the OS camera privacy switch (step 5) and a stored per-app Deny (step 6); tests_camera_consent pinned that order. Once a browser's installer registers it, it gets the 3D camera with the OS switch off, and a Deny the user stored for it is silently ignored. Roadmap §B.3 and the pre-R3 spec (OS consent applies to every consumer; delegation replaces only the DisplayXR prompt) say the opposite. No rationale was recorded, and "the browser enforces the OS switch itself" cannot hold: the service opens the camera, so the OS never sees the browser as a camera user.

The fix

Order (spec §7.1, the one statement of it): sharing off / DXR_STEREO_CAMERA=0 → DISABLED · dev override · no identity · OS switch → PERMISSION_INSUFFICIENT · stored Deny → CONSENT_REFUSED · delegating → allowed · stored Allow · Allow once · prompt. Delegation means exactly "no runtime prompt and no stored decision needed". Result codes are unchanged per refusal.

  • camera trust <exe> over a stored Deny clears the Deny and says so (user's newer explicit word, as allow over deny); an installer's registration never touches the user store. camera deny on a delegating exe notes the Deny wins.
  • CLI <exe> args are stored as the service sees the peer: realpath on POSIX; on Windows read from the UTF-16 command line and GetFullPathNameW'd (argv is ANSI).
  • Windows store reader: ANSI → wide registry API everywhere (UTF-8 ↔ UTF-16 explicit), so non-ASCII install paths match; buffers sized from RegQueryInfoKeyW, and an oversized / wrong-type / unconvertible value is skipped, never ends the scan. Matching is the pure u_camera_consent_list_find() + u_camera_consent_path_equal(), unit-tested on every OS.
  • Docs: spec §7.1/§8, developer guide, roadmap §B.3 + R3-as-built, ADR-043 Amendment 2.

Verification

  • tests_camera_consent (macOS): 18 cases / 161 assertions pass.
  • Mutation: old order restored → 2 cases / 7 assertions fail (OS switch ×2 sections, stored Deny). List scan made to stop on a skipped entry → the enumeration case fails.
  • scripts/build-mingw-check.sh aux_util passes (the Windows store compiles); the CLI's Windows argv snippet compiles + links under MinGW with -lshell32.
  • macOS CLI end to end on a scratch XDG_CONFIG_HOME: deny → trust (clears Deny) → deny (note printed).

Not run (Windows box)

Registry code is compile-checked only. On Windows: copy displayxr-cli.exe (+ its DLLs) into a non-ASCII directory (e.g. C:\Tëst\浏览器\displayxr-cli.exe) — the probe is the consumer; write HKLM\Software\DisplayXR\CameraConsent\Delegating with a 5000-char junk REG_SZ value first and then a REG_SZ whose data is that path; displayxr-cli camera consent must list the path; camera probe run from that copy must be allowed without a prompt; turn Settings → Privacy → Camera → desktop apps off → it must be refused with PERMISSION_INSUFFICIENT; turn it back on, displayxr-cli camera deny "C:\Tëst\浏览器\displayxr-cli.exe" → refused with CONSENT_REFUSED; camera trust it → "cleared the stored Deny".

🤖 Generated with Claude Code

Base automatically changed from feat/stereo-camera-r3 to main October 6, 2026 08:03
… a stored Deny

Follow-up to PR #1819 (R3 consent). As built, a registered consent-
delegating client was allowed at step 4 of the consent decision, before
the OS camera privacy switch (step 5) and before a stored per-app Deny
(step 6), and tests_camera_consent pinned that order. Once a browser's
installer registered it, it would get the 3D camera with the OS switch
off and a Deny the user had stored silently ignored. Roadmap §B.3 and the
pre-R3 spec say the opposite, and no rationale for the order was recorded;
"the browser enforces the OS switch itself" cannot hold, because the
service, not the browser, opens the camera, so the OS never sees the
browser as a camera user.

New order (spec §7.1 is the one statement of it): sharing off /
DXR_STEREO_CAMERA=0 -> DISABLED; dev override; no identity; OS switch ->
PERMISSION_INSUFFICIENT; stored Deny -> CONSENT_REFUSED; delegating ->
allowed; stored Allow; Allow once; prompt. Delegation keeps meaning
exactly "no runtime prompt and no stored decision needed". The decision's
`delegating` flag is now reported from step 3 on whatever the verdict.

`displayxr-cli camera trust <exe>` over a stored Deny clears the Deny and
says so (the user's newer explicit word, like `allow` over `deny`); an
installer's machine-wide registration never touches the user's store.
`camera deny` on a delegating exe notes that the Deny wins. Explicit <exe>
arguments are stored the way the service sees the peer: resolved with
realpath on POSIX; on Windows read from the UTF-16 command line (argv is
the ANSI code page) and made absolute with GetFullPathNameW.

Windows store reader (u_camera_consent_store.c), both bugs verified:
- it used the ANSI registry API while the runtime compares UTF-8 paths
  (QueryFullProcessImageNameW -> CP_UTF8), so a non-ASCII install path
  could never match an installer's UTF-16 entry. Every read, write, delete
  and enumeration (HKLM and HKCU, Apps, Delegating, Sharing, Secret, the
  ConsentStore webcam keys) now uses the wide API with explicit UTF-8
  conversion.
- fixed 256-char name / 1024-byte data buffers turned one oversized value
  into ERROR_MORE_DATA and `break`, hiding every later entry. Buffers are
  now sized from RegQueryInfoKeyW; an entry that still does not fit, has
  the wrong type or does not convert is skipped, never ends the scan.
The matching is the pure u_camera_consent_list_find() over an abstract
enumerator plus u_camera_consent_path_equal(), both unit-tested on every
OS.

Tests: the delegation-first assertions are rewritten; new cases for
delegating + OS switch off, + stored Deny, + sharing off / kill switch,
+ nothing stored (allowed, no prompt, nothing written), unregistered
neighbour unchanged, path rules, and skip-not-stop enumeration. Mutation:
restoring the old order fails 2 cases / 7 assertions; making the list
scan stop on a skipped entry fails the enumeration case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dfattal
dfattal force-pushed the fix/camera-consent-order branch from 986b531 to 54ef035 Compare October 6, 2026 08:03
@dfattal

dfattal commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator Author

Windows check — PASS (SR laptop, run by the Windows session, 2026-10-06)

54ef03583 built with MSVC, plain service (no DXR_STEREO_CAMERA_DEV_ALLOW):

  • tests_camera_consent 18 cases / 161 assertions and tests_stereo_camera pass on MSVC; selftest passes.
  • Registry reader: CLI copied to a directory with non-ASCII characters in its path; HKLM\Software\DisplayXR\CameraConsent\Delegating given a 5000-character junk value first, then that path. camera consent lists the path as delegating (system list) — the oversized value is skipped, the scan continues, the non-ASCII path round-trips.
  • Delegating + OS switch on → allowed, no tray prompt (exit 0).
  • Delegating + OS desktop-apps camera switch off → refused (exit 2).
  • camera deny <that exe> prints the 'stored Deny wins' note → probe CONSENT_REFUSED (exit 4).
  • camera trust <that exe> → 'cleared the stored Deny' → allowed (exit 0).
  • Also confirmed on main: no C4133 at service_tray_win.c; camera sharing off → probe DISABLED, exit 6.

One naming note from that run: for the OS-switch refusal the CLI prints the IPC-level name, -39 = NOT_AUTHORIZED, where this PR's text says PERMISSION_INSUFFICIENT. Both are right — XRT_ERROR_NOT_AUTHORIZED is what the service returns and XR_ERROR_PERMISSION_INSUFFICIENT is what an OpenXR caller sees — the CLI just speaks the lower layer.

Mergeable from the Windows side; merge is the project owner's call.

@dfattal
dfattal marked this pull request as ready for review October 6, 2026 09:05
@dfattal
dfattal requested a review from a team as a code owner October 6, 2026 09:05
@dfattal
dfattal merged commit b920228 into main Oct 6, 2026
46 of 56 checks passed
@dfattal
dfattal deleted the fix/camera-consent-order branch October 6, 2026 09:05
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