feat(cua-driver-rs)(macos): permissions UX — source labeling + live probe + permissions status|grant CLI verb - #1765
Conversation
… + live capture probe `check_permissions` answered in-process trusts CGPreflightScreenCaptureAccess / AXIsProcessTrusted, which answer for the macOS *responsible process* (the LaunchServices launching app), not the executable. So a standalone `cua-driver call check_permissions` from a terminal reports the TERMINAL's grants — it can read `accessibility: true, screen_recording: true` while `tccutil … com.trycua.driver` reports no record for the driver. A real false positive (reported live). Borrowing Peekaboo's two techniques, without changing the daemon path: - **(A) Live probe.** Add `screen_recording_capturable` from a `SCShareableContent::get()` query (screencapturekit is already a dep). Unlike the CGPreflight cache — which goes stale after `tccutil reset` (#1561) and is unreliable for CLI/child processes — this only returns displays when the answering process can genuinely capture. A preflight `true` with a probe `false` is the false-positive tell; the text summary flags the disagreement. - **(B) Source identity.** Add a `source` object: `attribution` (`driver-daemon` when we're the bundle binary reparented to launchd — the real driver status — vs `caller` otherwise), pid, responsible_ppid, executable, and a note explaining the booleans belong to the launching app, not com.trycua.driver, with the `open -n -g -a CuaDriver --args serve` path for the driver's own status. status.rs + the startup gate are untouched (zero risk to the forced bundle/daemon path). Verified live: a terminal `call check_permissions` now returns attribution=caller + the note; the daemon path reports attribution=driver-daemon. Refs #1491, #1561.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR enhances the macOS ChangesPermission Check Enhancement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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/rust/crates/platform-macos/src/tools/check_permissions.rs`:
- Around line 21-25: The probe function screen_recording_capturable() currently
swallows SCShareableContent::get() errors by using unwrap_or(false), making
transient probe failures indistinguishable from a true permission-denied result;
change it to return a Result<bool, E> (or an enum) so callers (e.g., invoke())
can distinguish probe errors from an actual non-capturable state and
log/propagate the underlying error instead of serializing false. Also, in the
source construction, don’t call the field responsible_ppid with libc::getppid()
if you intend the LaunchServices/TCC “responsible process” PID; rename the field
to immediate_ppid or add a new field (e.g., responsible_process_pid) that
attempts to resolve the true responsible PID, keeping libc::getppid() only for
the immediate parent identity.
🪄 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: 793728f1-92e1-4175-a2e1-c7ec91c85351
📒 Files selected for processing (1)
libs/cua-driver/rust/crates/platform-macos/src/tools/check_permissions.rs
| fn screen_recording_capturable() -> bool { | ||
| use screencapturekit::prelude::SCShareableContent; | ||
| SCShareableContent::get() | ||
| .map(|c| !c.displays().is_empty()) | ||
| .unwrap_or(false) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-macos/src/tools/check_permissions.rs"
echo "== File =="
ls -l "$FILE"
echo "== Relevant function: screen_recording_capturable() =="
rg -n "fn screen_recording_capturable" -n "$FILE" || true
# Print around it (small window)
python3 - <<'PY'
import itertools,sys,os
path="libs/cua-driver/rust/crates/platform-macos/src/tools/check_permissions.rs"
start=1
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
# find line numbers for the function
for i,l in enumerate(lines, start=1):
if "fn screen_recording_capturable" in l:
s=i-5
e=i+30
print(f"== lines {s}-{e} ==")
for j in range(max(1,s), min(len(lines),e)+1):
print(f"{j:4d}:{lines[j-1].rstrip()}")
break
PY
echo "== CheckPermissionsTool::invoke() warning + structured payload =="
python3 - <<'PY'
path="libs/cua-driver/rust/crates/platform-macos/src/tools/check_permissions.rs"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
def show_around(substr, window=120):
for i,l in enumerate(lines, start=1):
if substr in l:
s=i-20
e=i+window
print(f"\n== around line {i} ({substr}) => {s}-{e} ==")
for j in range(max(1,s), min(len(lines),e)+1):
print(f"{j:4d}:{lines[j-1].rstrip()}")
return True
return False
show_around("⚠️ Screen Recording reads granted")
show_around("screen_recording_capturable")
show_around("responsible_ppid")
PY
echo "== permission_source() responsible_ppid field =="
rg -n "permission_source\\(" "$FILE" || true
python3 - <<'PY'
path="libs/cua-driver/rust/crates/platform-macos/src/tools/check_permissions.rs"
with open(path,'r',encoding='utf-8') as f:
lines=f.readlines()
for i,l in enumerate(lines, start=1):
if "fn permission_source" in l:
s=i-5
e=i+80
print(f"\n== lines {s}-{e} ===============")
for j in range(max(1,s), min(len(lines),e)+1):
print(f"{j:4d}:{lines[j-1].rstrip()}")
break
PYRepository: trycua/cua
Length of output: 21994
Don’t collapse ScreenCaptureKit probe errors into false
screen_recording_capturable()turns anySCShareableContent::get()error intofalse(unwrap_or(false)), so transient probe failures are indistinguishable from a true permission miss;invoke()can then emit the “reads granted but a live capture probe failed” warning and serializescreen_recording_capturable: false. (libs/cua-driver/rust/crates/platform-macos/src/tools/check_permissions.rs:21-26, 145-165)sourcesetsresponsible_ppidtolibc::getppid()(immediate parent PID). If the intent is the LaunchServices/TCC “responsible process” PID, this can be wrong for indirect spawn/reparenting cases—rename or add the accurate identity. (40-75)
Possible fix
-fn screen_recording_capturable() -> bool {
+fn screen_recording_capturable() -> Result<bool, String> {
use screencapturekit::prelude::SCShareableContent;
SCShareableContent::get()
.map(|c| !c.displays().is_empty())
- .unwrap_or(false)
+ .map_err(|err| err.to_string())
}- let screen_recording_capturable = screen_recording_capturable();
+ let screen_recording_capturable = screen_recording_capturable();
+ let screen_recording_capturable_value =
+ screen_recording_capturable.as_ref().ok().copied();
- if screen_recording && !screen_recording_capturable {
+ if screen_recording && screen_recording_capturable_value == Some(false) {
summary.push_str(
"\n⚠️ Screen Recording reads granted but a live capture probe failed — \
the grant likely belongs to a different process, not this one.",
);
}
ToolResult::text(summary)
.with_structured(serde_json::json!({
"accessibility": accessibility,
"screen_recording": screen_recording,
- "screen_recording_capturable": screen_recording_capturable,
+ "screen_recording_capturable": screen_recording_capturable_value,
+ "screen_recording_capturable_error": screen_recording_capturable.err(),
"source": source,
}))🤖 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/rust/crates/platform-macos/src/tools/check_permissions.rs`
around lines 21 - 25, The probe function screen_recording_capturable() currently
swallows SCShareableContent::get() errors by using unwrap_or(false), making
transient probe failures indistinguishable from a true permission-denied result;
change it to return a Result<bool, E> (or an enum) so callers (e.g., invoke())
can distinguish probe errors from an actual non-capturable state and
log/propagate the underlying error instead of serializing false. Also, in the
source construction, don’t call the field responsible_ppid with libc::getppid()
if you intend the LaunchServices/TCC “responsible process” PID; rename the field
to immediate_ppid or add a new field (e.g., responsible_process_pid) that
attempts to resolve the true responsible PID, keeping libc::getppid() only for
the immediate parent identity.
Promote permission checking/granting from the clunky
`cua-driver call check_permissions '{"prompt":false}'` to a discoverable
verb group (mirrors `recording` / `skills` / `autostart`):
- `cua-driver permissions status [--json]` — report Accessibility + Screen
Recording. Read-only (never prompts). Routes through a running daemon
when one is up so the answer carries the com.trycua.driver identity;
otherwise reads in-process and labels the result as the caller's
(`source.attribution`). Human-formatted by default, `--json` for the raw
payload (incl. the `screen_recording_capturable` probe + `source`).
- `cua-driver permissions grant` — launch CuaDriver via LaunchServices
(`open -n -g -a CuaDriver --args serve`) so the dialog attributes to
com.trycua.driver, wait (user-paced, 180s) for the daemon's socket — it
only appears once the gate passes, i.e. the grant was given — then
confirm the driver's own status. The blessed grant path, no longer
tribal knowledge.
Wired into `--help`, telemetry (`cua_driver_permissions`), and both macOS
dispatch sites. The MCP `check_permissions` tool is unchanged. Updated the
tool's `source.note` to point at `cua-driver permissions grant` instead of
the raw `open` incantation.
Verified live: `permissions status` reports caller-attributed status + the
grant hint with no prompt/daemon; `--json` emits the full payload;
`permissions grant` finds/launches the daemon and confirms.
permissions status|grant CLI verb
…nce) (#1766) The install output + autostart docs pitched `--autostart` as bare "auto-start (optional)". On macOS it's more than convenience: a launchd-started daemon is attributed to com.trycua.driver (not the terminal that would otherwise spawn it), so permission prompts say "Cua Driver", you grant Accessibility + Screen Recording ONCE, and the grants persist — every `cua-driver call`/`mcp` then routes through the correctly-attributed always-on daemon. This is the clean fix for the terminal-attribution problem (#1491, PRs #1758–#1765). Changes: - install-local-rust.sh: macOS hint now "recommended on macOS" and explains the TCC attribution win; the --autostart --help text too. - autostart.mdx: new callout on the macOS TCC benefit; fixed the stale plist name (com.trycua.cua-driver-rs.plist → com.trycua.cua-driver.plist, matching what the script actually writes) and pointed at the new `cua-driver permissions status|grant` verbs. Kept opt-in (a standing computer-control daemon is a deliberate choice). Linux/systemd messaging unchanged — TCC is macOS-only.
Problem (reported live)
cua-driver call check_permissionsfrom a terminal can returnaccessibility: true, screen_recording: truewhiletccutil … com.trycua.driverreports no record for the driver. The preflight APIs (CGPreflightScreenCaptureAccess,AXIsProcessTrusted) answer for the macOS responsible process (the launching app), so a standalone call reports the terminal's grants — a false positive for the driver.Fix — Peekaboo's two techniques, without touching the daemon path
Peekaboo (a) doesn't trust the preflight API — it probes with
SCShareableContent— and (b) prints aSource:line so you know which identity the status reflects. Both adopted here:screen_recording_capturable— a liveSCShareableContent::get()probe (screencapturekitwas already a dep for recording). Only true when the answering process can genuinely capture. When it disagrees with the preflightscreen_recording, the text summary flags it — catching the stale-cache false positive (cua-driver check_permissions reports stale accessibility=true aftertccutil reset Accessibility com.trycua.driver(macOS) #1561) too.source—{ attribution: "driver-daemon" | "caller", pid, responsible_ppid, executable, note }.driver-daemonwhen we're the bundle binary reparented to launchd (the realcom.trycua.driverstatus — the forced bundle/daemon path),callerotherwise, with a note + theopen -n -g -a CuaDriver --args servehint.status.rsand the startup permissions gate are untouched — zero risk to the daemon path.Verified live (macOS 26)
Refs
tccutil reset Accessibility com.trycua.driver(macOS) #1561 (stale TCC cache), fix(cua-driver-rs)(install/uninstall): local install/uninstall use the current macOS layout (CuaDriver.app + ~/.cua-driver) #1758 (.app bundle), fix(cua-driver-rs)(macos): don't raise a terminal-attributed TCC prompt fromcall check_permissions#1760 (call report-only)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation