From bb79716a5c50a60421a2c8beed67eb92f49f2c01 Mon Sep 17 00:00:00 2001 From: Ryan Curran Date: Wed, 23 Sep 2026 09:42:45 -0400 Subject: [PATCH] screencapture grant: also grant /bin/bash for failure screenshots (RELOPS-2454) The failure-screenshot LaunchAgent (ronin macos_screenshot_helper) runs a bash script, so TCC attributes its captures to /bin/bash, not the worker binaries. On SIP-on hosts nothing grants bash (macos_tcc_perms writes it only with SIP off), so screencapture returns wallpaper + app menus only: 475/475 failure screenshots from SIP-on hosts were blank over 2026-09-09..23, vs 0/1,542 on SIP-off hosts. bash is not listed in the Screen Recording pane until added, so it goes in via the "+" button + Go-to-Folder. It is now required by both the already-granted short-circuit and the final verify. Proven on macmini-m4-118: 59KB blank -> 400KB capture with window + clock, row /bin/bash 2/4/0, survives reboot. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../data/screencapture-approve.sh | 88 ++++++++++++++++++- orchestrator/orchestrator/workflow.py | 8 +- .../tests/test_screencapture_grant.py | 12 +++ 3 files changed, 103 insertions(+), 5 deletions(-) diff --git a/orchestrator/orchestrator/data/screencapture-approve.sh b/orchestrator/orchestrator/data/screencapture-approve.sh index 22a7f64..22f21e6 100644 --- a/orchestrator/orchestrator/data/screencapture-approve.sh +++ b/orchestrator/orchestrator/data/screencapture-approve.sh @@ -42,6 +42,17 @@ # user-approved row, honoured by TCC and durable across reboots. # # EACS re-enables SIP and wipes TCC, so this has to run on every reprovision. +# +# /bin/bash (RELOPS-2454). The failure-screenshot LaunchAgent +# (ronin macos_screenshot_helper, com.mozilla.screencapture) runs a bash script, +# so TCC attributes its captures to /bin/bash, not to the worker binaries. Without +# a /bin/bash grant, screencapture "succeeds" but returns only wallpaper and app +# menus -- no windows, no menu-bar clock: 475/475 failure screenshots from SIP-on +# hosts were blank over 2026-09-09..23, against 0/1,542 on SIP-off hosts, where +# macos_tcc_perms writes the same grant. bash is not listed in the pane until it +# is added, so it goes in through the "+" button and Go-to-Folder rather than a +# checkbox; it lands ticked. This gives SIP-on hosts parity with SIP-off. A +# narrower Developer-ID-signed capture helper is the planned follow-up. set -u @@ -52,6 +63,7 @@ TCC_DB="/Library/Application Support/com.apple.TCC/TCC.db" OVERRIDES="/Library/Application Support/com.apple.TCC/MDMOverrides.plist" SESSION_USER="cltbld" CLIENTS=(/usr/local/bin/generic-worker-multiuser /usr/local/bin/start-worker) +SCREENSHOT_CLIENT=/bin/bash log() { echo "[screencapture] $*"; } fail() { echo "[ERROR] $*" >&2; exit 1; } @@ -66,7 +78,7 @@ row() { } granted() { - for c in "${CLIENTS[@]}"; do + for c in "${CLIENTS[@]}" "$SCREENSHOT_CLIENT"; do case "$(row "$c")" in 2/0|2/4) ;; *) return 1 ;; @@ -81,7 +93,7 @@ granted() { skip "SIP is off — macos_tcc_perms already grants this host" if granted; then - log "already granted ($(row "${CLIENTS[0]}"))" + log "already granted ($(row "${CLIENTS[0]}"), bash $(row "$SCREENSHOT_CLIENT"))" exit 0 fi @@ -152,10 +164,80 @@ on run argv delay 2 end if end repeat + + -- /bin/bash, for the screenshot LaunchAgent. Not listed until added, so add it + -- with "+" (the first unlabelled 10x10 button, under Screen & System Audio + -- Recording), answer the admin sheet, then Go-to-Folder in the open panel. It + -- lands ticked. If it is already listed but unticked, tick it instead. + set cb to my findCB(window 1, "bash", 0) + if cb is missing value then + set plusBtn to my findPlus(window 1, 0) + if plusBtn is missing value then error "add (+) button not found" + click plusBtn + delay 3 + my answerSheet(adminUser, adminPass) + keystroke "g" using {command down, shift down} + delay 2 + keystroke "/bin/bash" + delay 2 + keystroke return + delay 2 + keystroke return + delay 5 + my answerSheet(adminUser, adminPass) + else if value of cb is 0 then + click cb + delay 3 + my answerSheet(adminUser, adminPass) + end if end tell return "done" end run +on answerSheet(adminUser, adminPass) + tell application "System Events" to tell process "System Settings" + try + if (count of sheets of window 1) is 0 then return false + tell sheet 1 of window 1 + try + set value of (first text field whose subrole is not "AXSecureTextField") to adminUser + end try + set pw to (first text field whose subrole is "AXSecureTextField") + set focused of pw to true + set value of pw to adminPass + delay 1 + keystroke return + end tell + delay 6 + return true + on error + return false + end try + end tell +end answerSheet + +-- The add/remove buttons under each list carry no name, title or description +-- beyond "button"; the "+" glyph is 10x10 and the "-" glyph 10x2. The Screen & +-- System Audio Recording list comes first, so the first 10x10 match is its "+". +on findPlus(el, depth) + if depth > 14 then return missing value + tell application "System Events" + try + set kids to UI elements of el + on error + return missing value + end try + repeat with k in kids + try + if (class of k as string) is "button" and (description of k) is "button" and (size of k) is {10, 10} then return k + end try + set f to my findPlus(k, depth + 1) + if f is not missing value then return f + end repeat + end tell + return missing value +end findPlus + -- Deliberately NOT `entire contents of window 1`: on macOS 15.3 that returns an -- empty list against this pane even when it is loaded. And the left-hand category -- sidebar is ALSO an outline, reached first by a depth-first search, so we search @@ -187,7 +269,7 @@ sleep 3; /usr/bin/pkill -x "System Settings" >/dev/null 2>&1 # --- verify ------------------------------------------------------------------ -for c in "${CLIENTS[@]}"; do +for c in "${CLIENTS[@]}" "$SCREENSHOT_CLIENT"; do r=$(row "$c") case "$r" in 2/0|2/4) log "granted $c ($r)" ;; diff --git a/orchestrator/orchestrator/workflow.py b/orchestrator/orchestrator/workflow.py index 1821f66..fdf6ae4 100644 --- a/orchestrator/orchestrator/workflow.py +++ b/orchestrator/orchestrator/workflow.py @@ -1334,7 +1334,7 @@ def _screencapture_script() -> str: def step_screencapture_grant(ctx: HostContext) -> None: - """Grant Screen Recording to the worker binaries. SIP-on hosts only; no-op elsewhere. + """Grant Screen Recording to the worker binaries and /bin/bash. SIP-on hosts only. Bug 2073303. kTCCServiceScreenCapture is system-scoped, so the grant lives only in the SIP-protected system TCC database. ronin's macos_tcc_perms writes that database @@ -1345,6 +1345,10 @@ def step_screencapture_grant(ctx: HostContext) -> None: orange -- 42 of 174 hosts in gecko-t-osx-1500-m4 were in that state, which is what made bug 1937556 look like flakiness for 30 days. + /bin/bash is for the failure-screenshot LaunchAgent (RELOPS-2454), which runs a bash + script and so is attributed to bash, not the worker. Without it every failure + screenshot from a SIP-on host is wallpaper-only. + This belongs in the provisioning path rather than in puppet for two reasons: the approval needs an administrator-authenticated click that puppet has no credential for, and EACS re-enables SIP and wipes TCC, so a reprovisioned host comes back @@ -1357,7 +1361,7 @@ def step_screencapture_grant(ctx: HostContext) -> None: """ ui.step( "SCREEN RECORDING", - "grant the worker binaries ScreenCapture TCC (SIP-on hosts only)", + "grant the worker binaries + /bin/bash ScreenCapture TCC (SIP-on hosts only)", ) ui.wire( f"scp -> {SCREENCAPTURE_REMOTE} (0700, credential substituted from the vault)" diff --git a/orchestrator/tests/test_screencapture_grant.py b/orchestrator/tests/test_screencapture_grant.py index fa5bfed..4223950 100644 --- a/orchestrator/tests/test_screencapture_grant.py +++ b/orchestrator/tests/test_screencapture_grant.py @@ -142,6 +142,18 @@ def test_script_substitutes_the_credential_placeholders(): assert '[ "$ADMIN_PASSWORD" = "INSERT_HERE" ]' in body +def test_script_grants_and_verifies_bash(): + """RELOPS-2454: the screenshot LaunchAgent runs as /bin/bash, so bash must be both + granted and part of the success check. Dropping it from either leaves every failure + screenshot on a SIP-on host wallpaper-only, with the step still reporting success. + """ + with patch("orchestrator.workflow.ssh_admin_password", return_value="s3cr3t"): + body = workflow._screencapture_script() + assert "SCREENSHOT_CLIENT=/bin/bash" in body + assert body.count('"${CLIENTS[@]}" "$SCREENSHOT_CLIENT"') == 2 # granted() + verify + assert 'keystroke "/bin/bash"' in body + + def test_step_is_in_both_flows(): """Regression guard: the grant must not silently drop out of the sequences.