Skip to content

fix(cua-driver-rs): drop bogus window-enum fallback in screen-recording probe - #1562

Merged
f-trycua merged 1 commit into
mainfrom
fix/cua-driver-rs-check-permissions-sr-fallback
May 18, 2026
Merged

fix(cua-driver-rs): drop bogus window-enum fallback in screen-recording probe#1562
f-trycua merged 1 commit into
mainfrom
fix/cua-driver-rs-check-permissions-sr-fallback

Conversation

@f-trycua

@f-trycua f-trycua commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

permissions::status::screen_recording_granted() had a heuristic fallback when CGPreflightScreenCaptureAccess returned false: !crate::windows::all_windows().is_empty(). The theory was that getting non-empty results from CGWindowListCopyWindowInfo implied the Screen Recording grant was active. That theory is wrong — CGWindowListCopyWindowInfo returns window IDs and bounds for any process without requiring the Screen Recording grant; only window titles are gated. So the fallback returned `true` on any populated desktop regardless of grant state.

Two user-visible bugs follow from this:

  1. `check_permissions` false positive after `tccutil reset` — observed live: after `tccutil reset ScreenCapture com.trycua.driver` clears the SR row, the freshly-spawned daemon still reports `screen_recording: true`. This is half of cua-driver check_permissions reports stale accessibility=true after tccutil reset Accessibility com.trycua.driver (macOS) #1561 — the SR half, anyway; the AX half is a separate concern (see below).
  2. Startup permissions gate skipped — `permissions::gate::run_if_needed` short-circuits when `status.all_granted()` (`gate.rs:213`). With the bogus fallback, users who had never granted SR would have the gate silently no-op on first `cua-driver serve` start, and only hit the failure inside `screenshot` / `record` tool calls later.

The fix is to remove the fallback entirely and trust `CGPreflightScreenCaptureAccess` — Apple's documented preflight API for this grant, accurate on macOS 11+.

Files changed

  • `libs/cua-driver-rs/crates/platform-macos/src/permissions/status.rs` — strip the `all_windows()` fallback in `screen_recording_granted()`; expand the doc-comment to record the rationale so the heuristic doesn't get re-introduced.
  • `libs/cua-driver-rs/Cargo.lock` — refreshed to match the v0.2.4 workspace version (the `release-bump-version.yml` workflow didn't update the lock when it bumped `Cargo.toml`; `cargo check` on this branch catches that up). No new direct deps; only the workspace package version + transitive dep additions that were already present in `crates/cua-driver/Cargo.toml` (`tar`, `flate2` from the skills tarball reader).

On the AX side of #1561

The umbrella issue (#1561) reported that both `accessibility` and `screen_recording` come back `true` after `tccutil reset`. The SR side is a probe bug, fixed here. The AX side is a TCC attribution question: `AXIsProcessTrusted()` is the right API, but TCC attributes shell-invoked CLI processes to the parent terminal — so any shell that already has AX granted (Terminal, Claude Code, iTerm) will make `cua-driver check_permissions` see `accessibility: true` no matter what `com.trycua.driver` has in TCC. That's a fundamentally separate problem from this fix; tracking it on #1561.

Test plan

  • `cargo check -p platform-macos` — clean, no new warnings on the changed file
  • Reproduced original bug on installed v0.2.4: `tccutil reset ScreenCapture com.trycua.driver` then `cua-driver check_permissions '{"prompt":false}'` → `screen_recording: true` (BUG)
  • Debug binary built from this branch ran against shell-attributed process (shell has SR) → `true` (correct: shell-attributed probe sees real shell grant)
  • End-to-end `com.trycua.driver`-attributed validation (run inside `/Applications/CuaDriver.app` after `tccutil reset`, expect `false`) — pending the released artifact, easiest to verify post-merge via the next CD-published `cua-driver-rs` build

Related

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved accuracy of Screen Recording permission detection on macOS 11+
    • Fixed an issue where the system could incorrectly report Screen Recording access as granted

Review Change Stack

…ng probe

`screen_recording_granted()` fell back to `!all_windows().is_empty()` when
`CGPreflightScreenCaptureAccess` reported false, on the theory that
`CGWindowListCopyWindowInfo` returning real windows implied the grant
was active. That theory is wrong: `CGWindowListCopyWindowInfo` returns
window IDs and bounds for any process without requiring the Screen
Recording grant — only window titles are gated. The fallback therefore
returned `true` on any populated desktop regardless of grant state,
which:

  - made `check_permissions` report a false positive after
    `tccutil reset ScreenCapture com.trycua.driver`, and
  - short-circuited the startup permissions gate's prompt
    (`gate.rs:213` early-returns on `status.all_granted()`) for users
    who had never granted Screen Recording.

Drop the fallback entirely — `CGPreflightScreenCaptureAccess` is
Apple's documented preflight API for this grant and is accurate on
macOS 11+. The Cargo.lock churn is the v0.2.4 workspace bump
catching up — `release-bump-version.yml` didn't refresh the lock when
it pushed the version commit; `cargo check` on this branch did.

Related to #1561 (`check_permissions` stale-state report). The AX
side of that issue is a separate concern: `AXIsProcessTrusted()` is
the right API, but TCC attributes shell-invoked CLI processes to the
parent terminal — so a shell with AX granted will make
`cua-driver check_permissions` always see `accessibility: true`,
regardless of `com.trycua.driver`'s grant. That's a process-attribution
question, not a probe bug, and warrants its own investigation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Ignored Ignored May 18, 2026 2:26pm

Request Review

@coderabbitai

coderabbitai Bot commented May 18, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR refactors macOS Screen Recording permission detection to use Apple's CGPreflightScreenCaptureAccess() API directly. The implementation removes a prior window-enumeration heuristic fallback that could incorrectly report permission as granted, improving accuracy. Documentation is updated to describe the new approach and its availability on macOS 11+.

Changes

Screen Recording Permission Probe Refactor

Layer / File(s) Summary
Screen Recording permission probe refactor
libs/cua-driver-rs/crates/platform-macos/src/permissions/status.rs
screen_recording_granted() now links CoreGraphics and returns only the result of CGPreflightScreenCaptureAccess(), eliminating the window-enumeration fallback. Documentation for current_status() is updated to clarify the probe behavior and availability on macOS 11+.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • trycua/cua#1529: Introduces the permissions-gate flow that directly depends on the Screen Recording probe refactored here.
  • trycua/cua#1396: Updates macOS permission handling logic that relies on the screen_recording_granted() status computation being more accurate.

Poem

🐰 With CGPreflightScreenCaptureAccess now in hand,
No more guessing by windows across the land—
The heuristic fallback takes its final bow,
True permission detection takes the stage now! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: removing a flawed fallback heuristic in the screen-recording permission probe.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cua-driver-rs-check-permissions-sr-fallback

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@f-trycua
f-trycua merged commit e00840a into main May 18, 2026
4 of 5 checks passed
@f-trycua
f-trycua deleted the fix/cua-driver-rs-check-permissions-sr-fallback branch May 18, 2026 14:30

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@libs/cua-driver-rs/crates/platform-macos/src/permissions/status.rs`:
- Around line 30-35: The CGPreflightScreenCaptureAccess call in
libs/cua-driver-rs/crates/platform-macos/src/permissions/status.rs is only
available on macOS 11+, but the crate still exposes older macOS feature flags
(e.g., "elcapitan", "highsierra") causing link failures; either update the
crate's supported macOS minimum to 11.0 by removing/adjusting those older
feature flags in Cargo.toml and any platform feature gating, or add a version
gate/fallback in permissions/status.rs so calls to
CGPreflightScreenCaptureAccess are only compiled/linked for macOS >= 11.0 (and
provide an alternative code path for older macOS targets); refer to the
CGPreflightScreenCaptureAccess symbol and the permissions/status.rs module when
making the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 55de892f-e272-42e9-9903-5129ab9ffd20

📥 Commits

Reviewing files that changed from the base of the PR and between 96b2759 and cdfde73.

⛔ Files ignored due to path filters (1)
  • libs/cua-driver-rs/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (1)
  • libs/cua-driver-rs/crates/platform-macos/src/permissions/status.rs

Comment on lines 30 to +35
/// Mirrors Swift `Permissions.currentStatus()`. Difference: Swift uses
/// `SCShareableContent.excludingDesktopWindows` (ScreenCaptureKit) for
/// the screen recording probe, which is unavailable from Rust without
/// large bindings — same caveat documented in `check_permissions.rs`.
/// large bindings. `CGPreflightScreenCaptureAccess` is Apple's
/// documented preflight API for the same grant and is accurate on
/// macOS 11+.

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.

⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff

🧩 Analysis chain

🌐 Web query:

What is the minimum macOS version that supports CGPreflightScreenCaptureAccess API?

💡 Result:

The CGPreflightScreenCaptureAccess API was introduced in macOS 11.0 [1]. It is used to check whether an application has permission to capture the screen [1]. As of May 2026, Apple recommends that developers transition to the ScreenCaptureKit framework for screen recording functionality [2].

Citations:


🏁 Script executed:

find . -name "Cargo.toml" -o -name ".github" -type d -o -name "*.yml" -path "*/.github/*" | head -20

Repository: trycua/cua

Length of output: 824


🏁 Script executed:

rg "macos|rust-version|minimum" -i $(find . -name "Cargo.toml" | head -5) -A 2

Repository: trycua/cua

Length of output: 899


🏁 Script executed:

cat -n libs/cua-driver-rs/crates/platform-macos/Cargo.toml

Repository: trycua/cua

Length of output: 2048


🏁 Script executed:

rg "macos|deployment|target" .github/workflows/*.yml | grep -i macos | head -20

Repository: trycua/cua

Length of output: 1583


🏁 Script executed:

sed -n '48,67p' libs/cua-driver-rs/crates/platform-macos/src/permissions/status.rs

Repository: trycua/cua

Length of output: 1101


Add macOS version check or update minimum version requirement.

CGPreflightScreenCaptureAccess is only available on macOS 11.0+, but your project's Cargo.toml includes features for older macOS versions (e.g., "elcapitan" for 10.11, "highsierra" for 10.13). The current implementation will fail to link on macOS 10.15 and earlier.

Either:

  1. Drop support for older macOS and update minimum version to 11.0, or
  2. Add a version gate or fallback for older versions
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@libs/cua-driver-rs/crates/platform-macos/src/permissions/status.rs` around
lines 30 - 35, The CGPreflightScreenCaptureAccess call in
libs/cua-driver-rs/crates/platform-macos/src/permissions/status.rs is only
available on macOS 11+, but the crate still exposes older macOS feature flags
(e.g., "elcapitan", "highsierra") causing link failures; either update the
crate's supported macOS minimum to 11.0 by removing/adjusting those older
feature flags in Cargo.toml and any platform feature gating, or add a version
gate/fallback in permissions/status.rs so calls to
CGPreflightScreenCaptureAccess are only compiled/linked for macOS >= 11.0 (and
provide an alternative code path for older macOS targets); refer to the
CGPreflightScreenCaptureAccess symbol and the permissions/status.rs module when
making the change.

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