From 8c3db0a311f172331fe6d0e2d5a8ba2c011b00e4 Mon Sep 17 00:00:00 2001 From: Ryan Curran Date: Mon, 21 Sep 2026 14:16:02 -0400 Subject: [PATCH 1/2] Bug 2073303 - grant Screen Recording during provisioning EACS re-enables SIP and wipes TCC, so a reprovisioned host comes back without the ScreenCapture grant and silently fails every getDisplayMedia() call with SCStreamErrorUserDeclined (-3801) for the rest of its life. The only symptom is an intermittent orange on a random-looking subset of the pool: 42 of the 174 hosts in gecko-t-osx-1500-m4 were in that state, which is why bug 1937556 reached 499 failures in 30 days and was sheriff-escalated before anyone established it was the hosts and not Gecko. Those 42 have been granted by hand and verified (0 occurrences of -3801 across a 40-run try push). Without this step that fix decays one host at a time, invisibly, as machines go through refresh waves. This belongs here rather than in ronin for two reasons. The approval needs an administrator-authenticated click in System Settings and puppet has no credential for it; and the lifecycle is exactly this one -- the grant is destroyed by the wipe we perform, and has to be re-applied after the bootstrap, once the worker binaries exist and cltbld owns the console session. ronin gets the detector half instead (macos_screencapture_check, ronin_puppet#1412). MDM cannot supply the grant. Apple permits only AllowStandardUserToSetSystemService for this service in a PPPC payload, which authorises a standard user to approve it rather than approving it -- and an approval made while such a profile is installed is recorded as MDM-managed (flags 12) and then ignored by TCC. There is no consent dialog to automate either: tccd logs "Service kTCCServiceScreenCapture does not allow prompting; returning denied". Driving the Screen Recording pane is the only route, which is why this is GUI automation rather than a defaults write. The payload enforces four preconditions, each of which was learned by getting it wrong on the fleet: - SIP on. SIP-off hosts are already covered by macos_tcc_perms. - Developer-ID-signed worker binaries. An ad-hoc binary is Identifier=a.out with no TeamIdentifier and cannot satisfy a code requirement, so the grant is stored and ignored. - No ScreenCapture PPPC override, or the resulting row is flags=12 and inert. - Host idle. A running test owns the GUI session: the click does not land, and opening System Settings can corrupt that test. 16 of 18 failures on the first fleet pass were mid-mochitest. Exit 3 from the payload means "not applicable / not now" and is reported rather than raised, so a host that is merely busy does not abort a reprovision. A genuine failure -- unsigned binary, override present, flags=12, or a row that never reached auth_value 2 -- raises, because handing back a host that cannot screen-capture and saying nothing is the failure mode this whole bug is made of. The credential is substituted from the vault at fire time and written over ssh, same as _os_upgrade_script, so it never sits in SimpleMDM. On the host it goes into a 0600 file owned by cltbld, read once by the AppleScript and deleted immediately, so it is never an argv and never in the environment. The staged script is removed afterwards. Co-Authored-By: Claude Opus 5 (1M context) --- orchestrator/orchestrator/cli.py | 94 +++- .../data/screencapture-approve.sh | 199 ++++++++ orchestrator/orchestrator/workflow.py | 479 ++++++++++++++---- orchestrator/tests/test_mint.py | 218 +++++--- .../tests/test_screencapture_grant.py | 154 ++++++ 5 files changed, 960 insertions(+), 184 deletions(-) create mode 100644 orchestrator/orchestrator/data/screencapture-approve.sh create mode 100644 orchestrator/tests/test_screencapture_grant.py diff --git a/orchestrator/orchestrator/cli.py b/orchestrator/orchestrator/cli.py index b7e3f15..4b842c1 100644 --- a/orchestrator/orchestrator/cli.py +++ b/orchestrator/orchestrator/cli.py @@ -36,10 +36,14 @@ def run( _EXPECTED_OS_OPT = typer.Option( - "", "--expected-os", help="Required macOS version (default: REPROVISION_PROVISION_EXPECTED_OS, 15.3)." + "", + "--expected-os", + help="Required macOS version (default: REPROVISION_PROVISION_EXPECTED_OS, 15.3).", ) _ALLOW_SIP_OPT = typer.Option( - False, "--allow-sip-enabled", help="Don't require SIP to be disabled (for the SIP-on flow)." + False, + "--allow-sip-enabled", + help="Don't require SIP to be disabled (for the SIP-on flow).", ) _QUARANTINE_ON_REGISTER_OPT = typer.Option( False, @@ -55,7 +59,9 @@ def provision( expected_os: str = _EXPECTED_OS_OPT, allow_sip_enabled: bool = _ALLOW_SIP_OPT, no_wait: bool = typer.Option( - False, "--no-wait", help="Stop after mint + BST escrow; don't block on the bootstrap sentinel." + False, + "--no-wait", + help="Stop after mint + BST escrow; don't block on the bootstrap sentinel.", ), quarantine_on_register: bool = _QUARANTINE_ON_REGISTER_OPT, ) -> None: @@ -132,7 +138,9 @@ def preflight( @_app.command() def batch( - hosts_file: str = typer.Argument(..., help="File with one short hostname per line ('#' comments ok)."), + hosts_file: str = typer.Argument( + ..., help="File with one short hostname per line ('#' comments ok)." + ), action: str = typer.Option( "provision", "--action", @@ -140,13 +148,20 @@ def batch( "quarantine-on-register | validate | provision.", ), concurrency: int = typer.Option( - 0, "--concurrency", "-j", help="How many hosts in flight (default 3 — MDC1 throughput, not CPU)." + 0, + "--concurrency", + "-j", + help="How many hosts in flight (default 3 — MDC1 throughput, not CPU).", ), expected_os: str = _EXPECTED_OS_OPT, allow_sip_enabled: bool = _ALLOW_SIP_OPT, - no_wait: bool = typer.Option(False, "--no-wait", help="For --action provision: skip the sentinel wait."), + no_wait: bool = typer.Option( + False, "--no-wait", help="For --action provision: skip the sentinel wait." + ), quarantine_on_register: bool = _QUARANTINE_ON_REGISTER_OPT, - dry_run: bool = typer.Option(False, "--dry-run", help="Print the per-host commands and exit."), + dry_run: bool = typer.Option( + False, "--dry-run", help="Print the per-host commands and exit." + ), ) -> None: """Run one action across a list of hosts, a few at a time, with per-host logs. @@ -174,8 +189,12 @@ def batch( @_app.command() def quarantine( hostname: str, - until: str = typer.Option("", "--until", help="ISO-8601 quarantineUntil (default: 365 days out)."), - info: str = typer.Option("", "--info", help="Audit reason stored as quarantineInfo."), + until: str = typer.Option( + "", "--until", help="ISO-8601 quarantineUntil (default: 365 days out)." + ), + info: str = typer.Option( + "", "--info", help="Audit reason stored as quarantineInfo." + ), ) -> None: workflow.step_quarantine(workflow.resolve(hostname), until=until or None, info=info) @@ -216,17 +235,33 @@ def escrow_bst(hostname: str) -> None: workflow.step_escrow_bst(workflow.resolve_offline(hostname)) # SSH-only; see mint() +@_app.command() +def screencapture_grant(hostname: str) -> None: + """Grant Screen Recording (ScreenCapture TCC) to the worker binaries. SIP-on hosts only. + + Re-runnable and idempotent. Skips rather than fails when the host is mid-task, has no + console session yet, or is SIP-off (macos_tcc_perms covers those). Needed after every + reprovision because EACS re-enables SIP and wipes TCC -- see bug 2073303. + """ + workflow.step_screencapture_grant( + workflow.resolve_offline(hostname) + ) # SSH-only; see mint() + @_app.command() def wait_sentinel(hostname: str) -> None: - workflow.step_wait_for_sentinel(workflow.resolve_offline(hostname)) # SSH-only; see mint() + workflow.step_wait_for_sentinel( + workflow.resolve_offline(hostname) + ) # SSH-only; see mint() @_app.command() def add_to_group( hostname: str, group_id: int = typer.Option( - 0, "--group-id", help="Assignment group to ADD to (default: settings.bootstrap_group_id)." + 0, + "--group-id", + help="Assignment group to ADD to (default: settings.bootstrap_group_id).", ), quarantine_on_register: bool = typer.Option( False, @@ -258,8 +293,10 @@ def add_to_group( @_app.command() def pkg_audit( include_store: bool = typer.Option( - False, "--include-store", help="Also consider apple-store apps (noisy; they reach devices " - "by other means)." + False, + "--include-store", + help="Also consider apple-store apps (noisy; they reach devices " + "by other means).", ), ) -> None: """Which uploaded pkgs is no assignment group carrying? (read-only) @@ -273,9 +310,13 @@ def pkg_audit( @_app.command() def pkg_attach( - app: str = typer.Argument(..., help="App id, or a unique substring of its name/bundle id."), + app: str = typer.Argument( + ..., help="App id, or a unique substring of its name/bundle id." + ), group_id: int = typer.Option( - 0, "--group-id", help="Group to attach to (default: settings.bootstrap_group_id)." + 0, + "--group-id", + help="Group to attach to (default: settings.bootstrap_group_id).", ), push: bool = typer.Option( False, @@ -298,16 +339,24 @@ def group_parity( 0, "--group-id", help="Group to check (default: settings.bootstrap_group_id)." ), reference_group_id: int = typer.Option( - 0, "--reference-group-id", help="Group to measure against (default: settings.reference_group_id)." + 0, + "--reference-group-id", + help="Group to measure against (default: settings.reference_group_id).", ), reference_sample: int = typer.Option( - 0, "--reference-sample", help="Reference devices to intersect for the baseline (default 5)." + 0, + "--reference-sample", + help="Reference devices to intersect for the baseline (default 5).", ), max_devices: int = typer.Option( - 0, "--max-devices", help="Check only the first N devices of the group (default: all)." + 0, + "--max-devices", + help="Check only the first N devices of the group (default: all).", ), host: str = typer.Option( - "", "--host", help="Check one host instead of the whole group (needs SSH, to read its serial)." + "", + "--host", + help="Check one host instead of the whole group (needs SSH, to read its serial).", ), ) -> None: """Do this group's hosts get the profiles a working production host gets? (read-only) @@ -344,7 +393,8 @@ def validate( the last puppet run, and the worker. Exits 2 if the host hasn't bootstrapped yet, 1 if unfit. """ workflow.step_validate( - workflow.resolve_offline(hostname), expected_refresh_hz=expected_refresh_hz or None + workflow.resolve_offline(hostname), + expected_refresh_hz=expected_refresh_hz or None, ) @@ -372,7 +422,9 @@ def demo( help="Which replay: reprovision (EACS an existing host) | provision (fresh DEP host) | " "batch (the hardware-refresh rollout).", ), - host: str = typer.Option("", "--host", help="Hostname to show on screen (default: per-flow)."), + host: str = typer.Option( + "", "--host", help="Hostname to show on screen (default: per-flow)." + ), ) -> None: """Play a safe, no-host replay of a flow — for live demos (touches nothing). diff --git a/orchestrator/orchestrator/data/screencapture-approve.sh b/orchestrator/orchestrator/data/screencapture-approve.sh new file mode 100644 index 0000000..22a7f64 --- /dev/null +++ b/orchestrator/orchestrator/data/screencapture-approve.sh @@ -0,0 +1,199 @@ +#!/bin/bash +# +# screencapture-approve.sh — grant Screen Recording to the Taskcluster worker +# binaries on a SIP-enabled macOS host. Bug 2073303. +# +# Staged to the host by `reprovision screencapture-grant` (and by the provision / +# reprovision flows) with ADMIN_PASSWORD substituted from the vault at fire time, +# then run as root. Not a SimpleMDM script: the credential must not sit in the MDM +# UI, and we want a real exit code. +# +# WHY THIS EXISTS +# +# kTCCServiceScreenCapture is a system-scoped TCC service, read only from +# /Library/Application Support/com.apple.TCC/TCC.db, which SIP protects. ronin's +# macos_tcc_perms writes that database directly, which works only while SIP is off. +# On a SIP-on host the write silently fails and the fallback it takes -- writing +# the grant into cltbld's USER database -- is inert, because TCC never reads this +# service from a user database. The host then fails every getDisplayMedia() call +# with SCStreamErrorUserDeclined (-3801) for its entire life, showing up only as an +# intermittent orange (bug 1937556: 499 failures in 30 days). +# +# MDM cannot supply the grant. Apple permits only +# AllowStandardUserToSetSystemService for this service in a PPPC payload, which +# authorises a standard user to approve it rather than approving it -- and an +# approval made while such a profile is installed is recorded as MDM-managed +# (flags 12) and is then ignored by TCC. There is no consent dialog to automate +# either; tccd logs "Service kTCCServiceScreenCapture does not allow prompting; +# returning denied". The Screen Recording pane of System Settings is the only route. +# +# Three preconditions, all enforced below, all of which were learned the hard way: +# 1. SIP on -- SIP-off hosts are already handled by macos_tcc_perms +# 2. Developer-ID-signed worker binaries -- an ad-hoc binary is Identifier=a.out +# with no TeamIdentifier and can never satisfy a code +# requirement, so the grant lands and does nothing +# 3. No ScreenCapture PPPC override -- see above; it poisons the row to flags 12 +# 4. Host idle -- a running test owns the GUI session: the click never +# lands AND opening System Settings can corrupt that +# test. 16 of 18 failures on the first fleet pass were +# mid-mochitest. +# +# Result on success: auth_value 2, auth_reason 4, flags 0 -- an ordinary +# 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. + +set -u + +ADMIN_USER="INSERT_USER_HERE" +ADMIN_PASSWORD="INSERT_HERE" + +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) + +log() { echo "[screencapture] $*"; } +fail() { echo "[ERROR] $*" >&2; exit 1; } +skip() { echo "[SKIP] $*"; exit 3; } + +[ "$ADMIN_PASSWORD" = "INSERT_HERE" ] && fail "credential placeholder was not substituted" + +row() { + /usr/bin/sqlite3 -cmd ".timeout 5000" "$TCC_DB" \ + "SELECT auth_value || '/' || flags FROM access + WHERE service = 'kTCCServiceScreenCapture' AND client = '$1';" 2>/dev/null +} + +granted() { + for c in "${CLIENTS[@]}"; do + case "$(row "$c")" in + 2/0|2/4) ;; + *) return 1 ;; + esac + done + return 0 +} + +# --- preconditions ----------------------------------------------------------- + +/usr/bin/csrutil status 2>/dev/null | grep -qi disabled && \ + skip "SIP is off — macos_tcc_perms already grants this host" + +if granted; then + log "already granted ($(row "${CLIENTS[0]}"))" + exit 0 +fi + +ident=$(/usr/bin/codesign -dvvv "${CLIENTS[0]}" 2>&1 | /usr/bin/awk -F= '/^Identifier=/{print $2; exit}') +[ "$ident" = "generic-worker-multiuser-darwin-arm64" ] || \ + fail "worker binary is not Developer-ID signed (Identifier=${ident:-unknown}); the grant would be stored and ignored" + +overrides=$(/usr/bin/plutil -p "$OVERRIDES" 2>/dev/null | /usr/bin/grep -c kTCCServiceScreenCapture) +[ "$overrides" = "0" ] || \ + fail "a ScreenCapture PPPC override is installed ($overrides entries); approving now yields a flags=12 row that TCC ignores" + +/usr/bin/pgrep -f '/opt/worker/tasks/' >/dev/null 2>&1 && \ + skip "host is running a task — retry when idle (driving System Settings mid-test can corrupt it)" + +uid=$(/usr/bin/id -u "$SESSION_USER" 2>/dev/null) || fail "no $SESSION_USER user" +[ "$(/usr/bin/stat -f%Su /dev/console)" = "$SESSION_USER" ] || \ + skip "$SESSION_USER does not own the console session yet" + +# --- approve ----------------------------------------------------------------- + +asuser() { /bin/launchctl asuser "$uid" /usr/bin/sudo -u "$SESSION_USER" "$@"; } + +# Credential handoff: 0600, owned by the session user, read once and removed by the +# AppleScript itself. Never an argv (invisible to ps) and never in the environment. +creds=$(/usr/bin/sudo -u "$SESSION_USER" /usr/bin/mktemp "/Users/${SESSION_USER}/.sc-creds.XXXXXX") \ + || fail "could not create credential file" +trap '/bin/rm -f "$creds"' EXIT +/usr/bin/printf '%s\n%s\n' "$ADMIN_USER" "$ADMIN_PASSWORD" > "$creds" +/usr/sbin/chown "$SESSION_USER" "$creds"; /bin/chmod 600 "$creds" + +asuser /usr/bin/osascript -e 'tell application "System Settings" to quit' >/dev/null 2>&1 +sleep 3; /usr/bin/pkill -x "System Settings" >/dev/null 2>&1; sleep 2 +asuser /usr/bin/open "x-apple.systempreferences:com.apple.preference.security?Privacy_ScreenCapture" +sleep 12 + +asuser /usr/bin/osascript - "$creds" 2>&1 <<'OSA' +on run argv + set credFile to item 1 of argv + set adminUser to do shell script "head -1 " & quoted form of credFile + set adminPass to do shell script "sed -n 2p " & quoted form of credFile + do shell script "rm -f " & quoted form of credFile + + tell application "System Events" to tell process "System Settings" + set frontmost to true + delay 2 + repeat with nm in {"generic-worker-multiuser", "start-worker"} + set cb to my findCB(window 1, nm as string, 0) + if cb is missing value then error "checkbox not found: " & (nm as string) + if value of cb is 0 then + click cb + delay 3 + -- system.preferences.security requires an admin, so a sheet appears. + try + if (count of sheets of window 1) > 0 then + 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 + end if + end try + delay 2 + end if + end repeat + end tell + return "done" +end run + +-- 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 +-- for the checkbox by name rather than locating "the outline". +on findCB(el, nm, depth) + if depth > 14 then return missing value + tell application "System Events" + set kids to {} + 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 is checkbox and name of k is nm then return k + end try + set f to my findCB(k, nm, depth + 1) + if f is not missing value then return f + end repeat + end tell + return missing value +end findCB +OSA + +sleep 4 +asuser /usr/bin/osascript -e 'tell application "System Settings" to quit' >/dev/null 2>&1 +sleep 3; /usr/bin/pkill -x "System Settings" >/dev/null 2>&1 + +# --- verify ------------------------------------------------------------------ + +for c in "${CLIENTS[@]}"; do + r=$(row "$c") + case "$r" in + 2/0|2/4) log "granted $c ($r)" ;; + 2/12) fail "$c landed flags=12 (MDM-managed, TCC ignores it) — was the override really gone?" ;; + *) fail "$c not granted (got ${r:-none})" ;; + esac +done +log "Screen Recording granted" +exit 0 diff --git a/orchestrator/orchestrator/workflow.py b/orchestrator/orchestrator/workflow.py index 68117a7..1821f66 100644 --- a/orchestrator/orchestrator/workflow.py +++ b/orchestrator/orchestrator/workflow.py @@ -20,7 +20,12 @@ from .errors import NotReadyError, ReprovisionError from .hostnames import validate_short from .role_map import role_for_hostname -from .secrets import simplemdm_api_key, ssh_admin_key, ssh_admin_password, tc_credentials +from .secrets import ( + simplemdm_api_key, + ssh_admin_key, + ssh_admin_password, + tc_credentials, +) @dataclass @@ -31,8 +36,12 @@ class HostContext: worker_pool_id: str # e.g. releng-hardware/gecko-t-osx-1500-m4 worker_group: str = "mdc1" simplemdm_device_id: int | None = None - pre_wipe_enrolled_at: str | None = None # captured by step_wipe; used to detect a *fresh* re-enroll - registered: bool = True # is the worker currently registered in TC? False => skip quarantine/drain + pre_wipe_enrolled_at: str | None = ( + None # captured by step_wipe; used to detect a *fresh* re-enroll + ) + registered: bool = ( + True # is the worker currently registered in TC? False => skip quarantine/drain + ) _PROD_POOL_BY_ROLE = { @@ -130,7 +139,9 @@ def _try(label: str, getter, *, required: bool = True) -> None: ui.err(f"{label}: not configured (empty)") problems += 1 else: - ui.warn(f"{label}: not configured (optional — only needed for quarantine/drain)") + ui.warn( + f"{label}: not configured (optional — only needed for quarantine/drain)" + ) _try("admin password", ssh_admin_password) _try("admin SSH key", ssh_admin_key) @@ -139,7 +150,9 @@ def _try(label: str, getter, *, required: bool = True) -> None: _try("Taskcluster token", lambda: tc_credentials()[1], required=False) if problems: - raise ReprovisionError(f"{problems} credential(s) didn't resolve — see the ✗ line(s) above") + raise ReprovisionError( + f"{problems} credential(s) didn't resolve — see the ✗ line(s) above" + ) ui.ok("all credentials resolve — you're good to go") @@ -155,6 +168,7 @@ def _try(label: str, getter, *, required: bool = True) -> None: # Where the OS-upgrade script is staged when driven over SSH instead of as an MDM script job. # /var/root so it is root-only by location as well as by mode. OS_UPGRADE_REMOTE = "/var/root/macos-upgrade.sh" +SCREENCAPTURE_REMOTE = "/var/root/screencapture-approve.sh" def _os_version_matches(actual: str, expected: str) -> bool: @@ -194,7 +208,10 @@ def step_preflight( """ s = get_settings() expected_os = expected_os or s.provision_expected_os - ui.step("PREFLIGHT", "verify the host is at its target OS + SIP state before we commit to it") + ui.step( + "PREFLIGHT", + "verify the host is at its target OS + SIP state before we commit to it", + ) ui.wire(f"tcp connect {ctx.fqdn}:22 (fresh DEP hosts come up over ~15 min)") try: @@ -210,7 +227,9 @@ def step_preflight( cp = ssh.run(ctx.fqdn, "sw_vers -productVersion", check=False) actual_os = cp.stdout.decode(errors="replace").strip() if cp.returncode != 0 or not actual_os: - raise NotReadyError(f"{ctx.fqdn}: couldn't read the OS version over ssh (admin key installed yet?)") + raise NotReadyError( + f"{ctx.fqdn}: couldn't read the OS version over ssh (admin key installed yet?)" + ) if not _os_version_matches(actual_os, expected_os): raise NotReadyError( f"{ctx.fqdn}: macOS {actual_os}, expected {expected_os} — let the MDM in-place update " @@ -231,7 +250,11 @@ def step_preflight( "PPPC/system-DB-read-only branch on a role that has no PPPC profile. Disable SIP in " "Recovery first, or pass --allow-sip-enabled if this host is meant to be SIP-on" ) - ui.ok("SIP disabled" if sip_disabled else f"SIP enabled — allowed by request ({sip_raw})") + ui.ok( + "SIP disabled" + if sip_disabled + else f"SIP enabled — allowed by request ({sip_raw})" + ) # Informational: mint/escrow handle both of these, so they gate nothing. Printed because # "already ENABLED / already escrowed" is the difference between a fresh host and one @@ -240,7 +263,9 @@ def step_preflight( ui.info(f"admin SecureToken: {token or 'unknown'} (mint will grant it if needed)") cp = ssh.run(ctx.fqdn, "sudo profiles status -type bootstraptoken", check=False) escrowed = b"escrowed to server: YES" in cp.stdout - ui.info(f"Bootstrap Token escrowed: {'YES' if escrowed else 'no — escrow step will fix'}") + ui.info( + f"Bootstrap Token escrowed: {'YES' if escrowed else 'no — escrow step will fix'}" + ) # Reported, NOT gated. In the intended rollout order the readiness sweep runs BEFORE hosts # are moved into the bootstrap group, so the pkg is legitimately absent here and failing on @@ -252,7 +277,9 @@ def step_preflight( " (installed by bootstrap-group membership)" ) if ssh.file_exists(ctx.fqdn, SENTINEL): - ui.warn(f"sentinel {SENTINEL} already present — this host has bootstrapped before") + ui.warn( + f"sentinel {SENTINEL} already present — this host has bootstrapped before" + ) def _resolve_mdm_device(ctx: HostContext) -> dict: @@ -271,7 +298,9 @@ def _resolve_mdm_device(ctx: HostContext) -> dict: """ serial = ssh.platform_serial(ctx.fqdn) if serial: - ui.info(f"serial {serial} (from the host — SimpleMDM doesn't know its hostname)") + ui.info( + f"serial {serial} (from the host — SimpleMDM doesn't know its hostname)" + ) device = simplemdm.find_device_by_serial(serial) if device is not None: return device @@ -311,7 +340,10 @@ def step_add_to_group( """ s = get_settings() gid = group_id or s.bootstrap_group_id - ui.step("ADD TO GROUP", f"SimpleMDM assignment group {gid} — this is what triggers the bootstrap") + ui.step( + "ADD TO GROUP", + f"SimpleMDM assignment group {gid} — this is what triggers the bootstrap", + ) group = simplemdm.get_assignment_group(gid) name = group.get("attributes", {}).get("name", "?") @@ -342,7 +374,9 @@ def step_add_to_group( "ran on this host. Push the group's apps from SimpleMDM, or remove and re-add it." ) else: - ui.wire(f"POST /assignment_groups/{gid}/devices/{device_id} (additive; never a move)") + ui.wire( + f"POST /assignment_groups/{gid}/devices/{device_id} (additive; never a move)" + ) simplemdm.add_device_to_assignment_group(gid, device_id) ui.wire(f"POST /assignment_groups/{gid}/push_apps") simplemdm.push_apps(gid) @@ -356,7 +390,8 @@ def step_add_to_group( s2 = get_settings() step_quarantine_on_register( ctx, - max_wait_seconds=s2.bootstrap_max_wait_seconds + s2.quarantine_on_register_max_wait_seconds, + max_wait_seconds=s2.bootstrap_max_wait_seconds + + s2.quarantine_on_register_max_wait_seconds, ) @@ -411,7 +446,10 @@ def _membership_outliers( other = int(group["id"]) if other == gid: continue - ids = {int(d["id"]) for d in group.get("relationships", {}).get("devices", {}).get("data", [])} + ids = { + int(d["id"]) + for d in group.get("relationships", {}).get("devices", {}).get("data", []) + } if len([d for d in target_ids if d in ids]) < quorum: continue missing = [d for d in target_ids if d not in ids] @@ -468,7 +506,10 @@ def step_group_parity( ref_gid = reference_group_id or s.reference_group_id n_sample = reference_sample or s.group_parity_reference_sample - ui.step("GROUP PARITY", "do these hosts get the profiles a working prod host gets? (read-only)") + ui.step( + "GROUP PARITY", + "do these hosts get the profiles a working prod host gets? (read-only)", + ) if gid == ref_gid: raise ReprovisionError( @@ -476,7 +517,9 @@ def step_group_parity( "--reference-group-id to measure against a different group." ) - ref_name = simplemdm.get_assignment_group(ref_gid).get("attributes", {}).get("name", "?") + ref_name = ( + simplemdm.get_assignment_group(ref_gid).get("attributes", {}).get("name", "?") + ) ref_devices = simplemdm.assignment_group_device_ids(ref_gid)[:n_sample] if not ref_devices: raise ReprovisionError( @@ -487,7 +530,11 @@ def step_group_parity( baseline: dict[int, str] | None = None for did in ref_devices: profiles = simplemdm.device_profiles(did) - baseline = profiles if baseline is None else {i: n for i, n in baseline.items() if i in profiles} + baseline = ( + profiles + if baseline is None + else {i: n for i, n in baseline.items() if i in profiles} + ) assert baseline is not None ui.info( f"baseline: {len(baseline)} profile(s) common to {len(ref_devices)} device(s) " @@ -505,7 +552,9 @@ def step_group_parity( targets = [(hostname, int(device["id"]))] ui.info(f"checking {hostname} (device {targets[0][1]})") else: - name = simplemdm.get_assignment_group(gid).get("attributes", {}).get("name", "?") + name = ( + simplemdm.get_assignment_group(gid).get("attributes", {}).get("name", "?") + ) ids = simplemdm.assignment_group_device_ids(gid) if not ids: raise ReprovisionError(f"group {gid} ({name}) has no devices to check") @@ -534,9 +583,15 @@ def step_group_parity( # Second, independent question: is any device missing a GROUP its peers are all in? Catches # the mis-clicked move, including the app-bearing groups a profile diff cannot see. - outliers = _membership_outliers(gid, [did for _label, did in targets]) if not hostname else {} + outliers = ( + _membership_outliers(gid, [did for _label, did in targets]) + if not hostname + else {} + ) if outliers: - ui.warn(f"{len(outliers)} group(s) that most of these devices are in, some are not") + ui.warn( + f"{len(outliers)} group(s) that most of these devices are in, some are not" + ) elif not hostname: ui.ok("group membership is consistent across the group") @@ -548,9 +603,15 @@ def step_group_parity( if gaps: lines = [] - for pid, (pname, lacking) in sorted(gaps.items(), key=lambda kv: -len(kv[1][1])): - why = next((story for stem, story in _LOAD_BEARING_PROFILES if stem in pname), "") - line = f"{pname} (profile {pid}) — missing on {len(lacking)}/{total} device(s)" + for pid, (pname, lacking) in sorted( + gaps.items(), key=lambda kv: -len(kv[1][1]) + ): + why = next( + (story for stem, story in _LOAD_BEARING_PROFILES if stem in pname), "" + ) + line = ( + f"{pname} (profile {pid}) — missing on {len(lacking)}/{total} device(s)" + ) if len(lacking) <= 3: line += "\n " + "\n ".join( _device_label(d) for _l, d in targets if _l in lacking @@ -559,12 +620,15 @@ def step_group_parity( line += f"\n ^ {why}" lines.append(line) sections.append( - f"profile parity gap against {ref_gid} ({ref_name}):\n - " + "\n - ".join(lines) + f"profile parity gap against {ref_gid} ({ref_name}):\n - " + + "\n - ".join(lines) ) if outliers: lines = [] - for other, (oname, missing) in sorted(outliers.items(), key=lambda kv: -len(kv[1][1])): + for other, (oname, missing) in sorted( + outliers.items(), key=lambda kv: -len(kv[1][1]) + ): lines.append( f"{oname} ({other}) — {total - len(missing)}/{total} of these devices are in it, " f"{len(missing)} are not:\n " @@ -595,12 +659,15 @@ def _resolve_app(spec: str) -> dict: needle = spec.lower() hits = [ - a for a in catalog + a + for a in catalog if needle in (a.get("attributes", {}).get("name") or "").lower() or needle in (a.get("attributes", {}).get("bundle_identifier") or "").lower() ] if not hits: - raise ReprovisionError(f"no app matching {spec!r} — check the name or pass the numeric id") + raise ReprovisionError( + f"no app matching {spec!r} — check the name or pass the numeric id" + ) if len(hits) > 1: listed = "\n ".join( f"{a['id']} {a['attributes'].get('name')!r} {a['attributes'].get('bundle_identifier')}" @@ -644,9 +711,13 @@ def step_pkg_audit(*, include_store: bool = False) -> None: carried.setdefault(int(app["id"]), []).append(f"{group['id']} ({gname})") everything = simplemdm.apps() - catalog = everything if include_store else [ - a for a in everything if a.get("attributes", {}).get("app_type") == "custom" - ] + catalog = ( + everything + if include_store + else [ + a for a in everything if a.get("attributes", {}).get("app_type") == "custom" + ] + ) scope = "app(s)" if include_store else "custom pkg(s)" ui.info( f"{len(catalog)} {scope} in the account " @@ -667,17 +738,22 @@ def step_pkg_audit(*, include_store: bool = False) -> None: # ff-ent), and puppet-agent's ARM and Intel builds share com.puppetlabs.puppet-agent. Flagging # those buried the one real case. A duplicate that includes a stray upload is the smell. dupes = { - b: v for b, v in by_bundle.items() + b: v + for b, v in by_bundle.items() if len(v) > 1 and any(int(a["id"]) not in carried for a in v) } if dupes: - ui.warn(f"{len(dupes)} bundle id(s) uploaded more than once with a copy attached to nothing:") + ui.warn( + f"{len(dupes)} bundle id(s) uploaded more than once with a copy attached to nothing:" + ) for bundle, group in sorted(dupes.items()): ui.warn(f" {bundle}") for a in sorted(group, key=lambda a: int(a["id"])): where = carried.get(int(a["id"])) - ui.warn(f" {a['id']} {a['attributes'].get('name')!r} " - f"{'carried by ' + ', '.join(where) if where else 'ATTACHED TO NOTHING'}") + ui.warn( + f" {a['id']} {a['attributes'].get('name')!r} " + f"{'carried by ' + ', '.join(where) if where else 'ATTACHED TO NOTHING'}" + ) orphans = [a for a in catalog if int(a["id"]) not in carried] if not orphans: @@ -687,11 +763,15 @@ def step_pkg_audit(*, include_store: bool = False) -> None: ui.warn(f"{len(orphans)} {scope} attached to NOTHING — uploaded but inert:") for a in sorted(orphans, key=lambda a: int(a["id"])): at = a.get("attributes", {}) - ui.warn(f" {a['id']} {at.get('name')!r} bundle={at.get('bundle_identifier')}") + ui.warn( + f" {a['id']} {at.get('name')!r} bundle={at.get('bundle_identifier')}" + ) ui.info("attach one with: reprovision pkg-attach --group-id ") -def step_pkg_attach(app_spec: str, *, group_id: int | None = None, push: bool = False) -> None: +def step_pkg_attach( + app_spec: str, *, group_id: int | None = None, push: bool = False +) -> None: """Attach an uploaded pkg to an assignment group, then VERIFY the group really carries it. Verifies by re-reading the group rather than trusting the POST, for the same reason @@ -745,7 +825,9 @@ def step_pkg_attach(app_spec: str, *, group_id: int | None = None, push: bool = ui.ok(f"verified: {gname} carries app {aid} (group now has {len(after)} app(s))") -def step_validate(ctx: HostContext, *, expected_refresh_hz: float | None = None) -> None: +def step_validate( + ctx: HostContext, *, expected_refresh_hz: float | None = None +) -> None: """Read-only fitness check on a bootstrapped host: is it actually able to run tasks? This fills the gap the quarantine message already promises. `--quarantine-on-register` holds a @@ -776,7 +858,9 @@ def step_validate(ctx: HostContext, *, expected_refresh_hz: float | None = None) problems: list[str] = [] # The display check first: it's the one that passes every other signal and still eats tasks. - ui.wire(f"ssh admin@{ctx.hostname} launchctl asuser $(id -u cltbld) … CGDisplayModeGetRefreshRate") + ui.wire( + f"ssh admin@{ctx.hostname} launchctl asuser $(id -u cltbld) … CGDisplayModeGetRefreshRate" + ) mode = ssh.display_mode(ctx.fqdn) if mode is None: # Unknown, not fine. A host whose GUI session we can't reach can't run tests either. @@ -794,26 +878,38 @@ def step_validate(ctx: HostContext, *, expected_refresh_hz: float | None = None) "task on this before running a single test. Usually the KVM isn't set correctly." ) - puppet_ok = ssh.run( - ctx.fqdn, - "sudo grep -o '\"success\": [a-z]*' /opt/puppet_environments/last_run_metadata.json " - "2>/dev/null | head -1 | awk '{print $2}'", - check=False, - ).stdout.decode(errors="replace").strip() + puppet_ok = ( + ssh.run( + ctx.fqdn, + "sudo grep -o '\"success\": [a-z]*' /opt/puppet_environments/last_run_metadata.json " + "2>/dev/null | head -1 | awk '{print $2}'", + check=False, + ) + .stdout.decode(errors="replace") + .strip() + ) if puppet_ok == "true": ui.ok("last puppet run succeeded") else: problems.append(f"last puppet run reported success={puppet_ok or 'unknown'}") - worker_up = ssh.run( - ctx.fqdn, "pgrep -f 'start-worker ' >/dev/null && echo up || echo down", check=False - ).stdout.decode(errors="replace").strip() + worker_up = ( + ssh.run( + ctx.fqdn, + "pgrep -f 'start-worker ' >/dev/null && echo up || echo down", + check=False, + ) + .stdout.decode(errors="replace") + .strip() + ) if worker_up == "up": ui.ok("generic-worker is running") else: # Not fatal on its own: these hosts reboot between tasks, so a down worker can just mean # we caught it mid-cycle. Report it without failing the host on timing alone. - ui.warn("generic-worker isn't running right now (may be mid-reboot between tasks)") + ui.warn( + "generic-worker isn't running right now (may be mid-reboot between tasks)" + ) if problems: raise ReprovisionError( @@ -845,7 +941,10 @@ def step_wait_for_bootstrap_pkg(ctx: HostContext) -> None: ui.ok("host has already bootstrapped — pkg check not needed") return - ui.step("BOOTSTRAP PKG", "confirm the signed pkg landed — i.e. the host is in the bootstrap group") + ui.step( + "BOOTSTRAP PKG", + "confirm the signed pkg landed — i.e. the host is in the bootstrap group", + ) ui.wire(f"ssh admin@{ctx.hostname} test -f {BOOTSTRAP_PKG_PAYLOAD}") deadline = time.monotonic() + s.bootstrap_pkg_max_wait_seconds found = False @@ -878,9 +977,14 @@ def _os_upgrade_script(expected_os: str) -> str: from importlib import resources body = (resources.files("orchestrator") / "data" / "macos-upgrade.sh").read_text() - body = body.replace('ADMIN_PASSWORD="INSERT_HERE"', f'ADMIN_PASSWORD={shlex.quote(ssh_admin_password())}') + body = body.replace( + 'ADMIN_PASSWORD="INSERT_HERE"', + f"ADMIN_PASSWORD={shlex.quote(ssh_admin_password())}", + ) if expected_os: - body = body.replace('TARGET_VERSION="15.3"', f'TARGET_VERSION={shlex.quote(expected_os)}') + body = body.replace( + 'TARGET_VERSION="15.3"', f"TARGET_VERSION={shlex.quote(expected_os)}" + ) return body @@ -904,7 +1008,10 @@ def step_os_update(ctx: HostContext, *, expected_os: str = "") -> None: """ s = get_settings() expected_os = expected_os or s.provision_expected_os - ui.step("OS UPDATE", f"in-place upgrade to macOS {expected_os} — launches, then the host reboots itself") + ui.step( + "OS UPDATE", + f"in-place upgrade to macOS {expected_os} — launches, then the host reboots itself", + ) with ui.waiting("waiting for sshd"): ssh.wait_for_sshd(ctx.fqdn, timeout=s.preflight_sshd_wait_seconds) @@ -916,44 +1023,72 @@ def step_os_update(ctx: HostContext, *, expected_os: str = "") -> None: return ui.wire(f"scp → {OS_UPGRADE_REMOTE} (0700, credential substituted from the vault)") - ssh.write_file_as_root(ctx.fqdn, OS_UPGRADE_REMOTE, _os_upgrade_script(expected_os).encode(), mode="0700") + ssh.write_file_as_root( + ctx.fqdn, + OS_UPGRADE_REMOTE, + _os_upgrade_script(expected_os).encode(), + mode="0700", + ) # Detached: the download alone outlives any sane ssh timeout, and the script ends in a # reboot that would kill the channel anyway. setsid+nohup so it survives our disconnect. - ui.wire(f"ssh admin@{ctx.hostname} sudo nohup {OS_UPGRADE_REMOTE} (detached; log /var/log/macos-upgrade.log)") - ssh.run(ctx.fqdn, f"sudo /usr/bin/nohup {OS_UPGRADE_REMOTE} >/dev/null 2>&1 & echo launched", check=False) + ui.wire( + f"ssh admin@{ctx.hostname} sudo nohup {OS_UPGRADE_REMOTE} (detached; log /var/log/macos-upgrade.log)" + ) + ssh.run( + ctx.fqdn, + f"sudo /usr/bin/nohup {OS_UPGRADE_REMOTE} >/dev/null 2>&1 & echo launched", + check=False, + ) # Confirm it actually started rather than dying on a precondition — the script's own guards # (placeholder credential, no SecureToken, low disk) all fail within a second or two. time.sleep(5) - cp = ssh.run(ctx.fqdn, "sudo tail -5 /var/log/macos-upgrade.log 2>/dev/null", check=False) + cp = ssh.run( + ctx.fqdn, "sudo tail -5 /var/log/macos-upgrade.log 2>/dev/null", check=False + ) tail = cp.stdout.decode(errors="replace").strip() if "[ERROR]" in tail: - raise NotReadyError(f"{ctx.fqdn}: upgrade refused to start —\n " + tail.replace("\n", "\n ")) + raise NotReadyError( + f"{ctx.fqdn}: upgrade refused to start —\n " + + tail.replace("\n", "\n ") + ) ui.ok(f"upgrade launched — macOS {current or 'unknown'} → {expected_os}") - ui.info("host downloads ~14GB, installs, then reboots into startosinstall (tens of minutes)") + ui.info( + "host downloads ~14GB, installs, then reboots into startosinstall (tens of minutes)" + ) ui.info("confirm arrival later with: reprovision batch --action preflight") def step_quarantine(ctx: HostContext, until: str | None = None, info: str = "") -> None: if not until: - until = (datetime.now(timezone.utc) + timedelta(days=365)).strftime("%Y-%m-%dT%H:%M:%S.000Z") + until = (datetime.now(timezone.utc) + timedelta(days=365)).strftime( + "%Y-%m-%dT%H:%M:%S.000Z" + ) ui.step("QUARANTINE", "tell Taskcluster to stop scheduling tasks on this worker") - ui.wire(f"PUT queue/v1 quarantineWorker {ctx.worker_pool_id}/{ctx.worker_group}/{ctx.hostname}") - taskcluster.quarantine(ctx.worker_pool_id, ctx.worker_group, ctx.hostname, until, info) + ui.wire( + f"PUT queue/v1 quarantineWorker {ctx.worker_pool_id}/{ctx.worker_group}/{ctx.hostname}" + ) + taskcluster.quarantine( + ctx.worker_pool_id, ctx.worker_group, ctx.hostname, until, info + ) ui.ok(f"quarantined until {until[:10]}") def step_drain(ctx: HostContext) -> None: s = get_settings() - ui.step("DRAIN", "let the worker finish its in-flight task (2 consecutive idle polls)") + ui.step( + "DRAIN", "let the worker finish its in-flight task (2 consecutive idle polls)" + ) ui.wire(f"queue.getWorker {ctx.hostname} → inspect recentTasks run states") deadline = time.monotonic() + s.drain_max_wait_seconds consecutive_idle = 0 drained = False with ui.waiting("checking for an active task") as tick: while time.monotonic() < deadline: - busy = taskcluster.is_currently_busy(ctx.worker_pool_id, ctx.worker_group, ctx.hostname) + busy = taskcluster.is_currently_busy( + ctx.worker_pool_id, ctx.worker_group, ctx.hostname + ) if not busy: consecutive_idle += 1 # Require 2 consecutive idle polls so we don't race a worker that's @@ -977,7 +1112,10 @@ def step_wipe(ctx: HostContext) -> None: raise ReprovisionError(f"{ctx.hostname} not found in SimpleMDM") # A prior EACS may have rotated this host's SSH key; clear any stale entry from the tool's # known_hosts so the verify connection accept-new's the current key instead of failing. - ui.step("WIPE · EACS", "Erase All Content & Settings — DoNotObliterate (fails safe, never obliterates)") + ui.step( + "WIPE · EACS", + "Erase All Content & Settings — DoNotObliterate (fails safe, never obliterates)", + ) ssh.forget_host_key(ctx.fqdn) # Guard: EACS needs an escrowed Bootstrap Token. Without it, the erase either fails # (DoNotObliterate) or full-obliterates into a long headless macOS reinstall. Refuse to @@ -1020,7 +1158,9 @@ def step_wipe(ctx: HostContext) -> None: # (Previously this failed OPEN — warn + proceed — which let a running worker get wiped.) ui.wire(f"queue.getWorker {ctx.hostname} → confirm no task in flight") try: - busy = taskcluster.is_currently_busy(ctx.worker_pool_id, ctx.worker_group, ctx.hostname) + busy = taskcluster.is_currently_busy( + ctx.worker_pool_id, ctx.worker_group, ctx.hostname + ) except Exception as e: # noqa: BLE001 — any TC/auth failure → can't verify idle → refuse to wipe raise ReprovisionError( f"{ctx.hostname}: couldn't confirm the worker is idle via Taskcluster ({e}) — refusing " @@ -1035,8 +1175,14 @@ def step_wipe(ctx: HostContext) -> None: ui.ok("no task in flight") # Record the current enrolled_at so wait_for_reenroll can detect a *fresh* enrollment # (status alone is unreliable: it stays "enrolled" until the erase actually executes). - ctx.pre_wipe_enrolled_at = simplemdm.get_device(ctx.simplemdm_device_id).get("attributes", {}).get("enrolled_at") - ui.wire(f"SimpleMDM POST /devices/{ctx.simplemdm_device_id}/wipe obliteration_behavior=DoNotObliterate") + ctx.pre_wipe_enrolled_at = ( + simplemdm.get_device(ctx.simplemdm_device_id) + .get("attributes", {}) + .get("enrolled_at") + ) + ui.wire( + f"SimpleMDM POST /devices/{ctx.simplemdm_device_id}/wipe obliteration_behavior=DoNotObliterate" + ) simplemdm.wipe(ctx.simplemdm_device_id) ui.ok("erase command accepted by SimpleMDM") @@ -1048,9 +1194,15 @@ def step_wait_for_reenroll(ctx: HostContext) -> None: # so we don't false-return on the pre-wipe enrollment (status lags the erase). baseline = ctx.pre_wipe_enrolled_at if baseline is None: - baseline = simplemdm.get_device(ctx.simplemdm_device_id).get("attributes", {}).get("enrolled_at") + baseline = ( + simplemdm.get_device(ctx.simplemdm_device_id) + .get("attributes", {}) + .get("enrolled_at") + ) ui.step("RE-ENROLL", "erase → reboot → DEP re-enrollment · typically ~5 min") - ui.wire(f"SimpleMDM GET /devices/{ctx.simplemdm_device_id} (poll enrolled_at ≠ {baseline})") + ui.wire( + f"SimpleMDM GET /devices/{ctx.simplemdm_device_id} (poll enrolled_at ≠ {baseline})" + ) deadline = time.monotonic() + s.wipe_max_wait_seconds start = time.monotonic() next_poll = 0.0 @@ -1096,7 +1248,10 @@ def step_mint(ctx: HostContext) -> None: by A/B on m4-81 (2026-07-02): with this login the bootstrap finishes; without it, it wedges at the BST wait-loop and times out. Idempotent — skips if already ENABLED. """ - ui.step("MINT SECURETOKEN", "DEP skips Setup Assistant, so admin has no token until an interactive login") + ui.step( + "MINT SECURETOKEN", + "DEP skips Setup Assistant, so admin has no token until an interactive login", + ) # The box just re-enrolled post-EACS with a fresh host key; forget the old one so the # SecureToken status check (which uses ssh.run) doesn't fail on a key mismatch. ssh.forget_host_key(ctx.fqdn) @@ -1105,7 +1260,9 @@ def step_mint(ctx: HostContext) -> None: if "ENABLED" in ssh.secure_token_status(ctx.fqdn): ui.ok("admin already holds a SecureToken — skipping mint") return - ui.wire(f"expect: ssh admin@{ctx.hostname} (keyboard-interactive PAM login → grants first SecureToken)") + ui.wire( + f"expect: ssh admin@{ctx.hostname} (keyboard-interactive PAM login → grants first SecureToken)" + ) ssh.password_login(ctx.fqdn) enabled = False with ui.waiting("verifying the SecureToken came up ENABLED") as tick: @@ -1130,8 +1287,12 @@ def step_escrow_bst(ctx: HostContext) -> None: exists, so on the pre-minted path this step is what actually escrows the BST. """ s = get_settings() - ui.step("ESCROW BOOTSTRAP TOKEN", "escrow the BST so this box is EACS-able next cycle") - ui.wire(f"ssh admin@{ctx.hostname} sudo profiles install -type bootstraptoken -user {s.ssh_admin_user} -password ••••••") + ui.step( + "ESCROW BOOTSTRAP TOKEN", "escrow the BST so this box is EACS-able next cycle" + ) + ui.wire( + f"ssh admin@{ctx.hostname} sudo profiles install -type bootstraptoken -user {s.ssh_admin_user} -password ••••••" + ) install_cmd = ( f"sudo profiles install -type bootstraptoken " f"-user {s.ssh_admin_user} -password {shlex.quote(ssh_admin_password())}" @@ -1140,21 +1301,120 @@ def step_escrow_bst(ctx: HostContext) -> None: ssh.run(ctx.fqdn, install_cmd) except ReprovisionError as e: # ssh.run already scrubs the command (which embeds the password); add a mint hint. - raise ReprovisionError(f"{e}\n (has admin minted a SecureToken? run `reprovision mint` first)") from None + raise ReprovisionError( + f"{e}\n (has admin minted a SecureToken? run `reprovision mint` first)" + ) from None cp = ssh.run(ctx.fqdn, "sudo profiles status -type bootstraptoken") if b"escrowed to server: YES" not in cp.stdout: raise ReprovisionError(f"BST escrow check failed:\n{cp.stdout.decode()}") ui.ok("Bootstrap Token escrowed to server") +def _screencapture_script() -> str: + """The packaged approval script with the admin credential substituted in. + + Same delivery as _os_upgrade_script: resolved from the vault at fire time and + written to the host over ssh, so the password never sits in SimpleMDM and never + appears in an argv. + """ + from importlib import resources + + s = get_settings() + body = ( + resources.files("orchestrator") / "data" / "screencapture-approve.sh" + ).read_text() + body = body.replace( + 'ADMIN_USER="INSERT_USER_HERE"', f"ADMIN_USER={shlex.quote(s.ssh_admin_user)}" + ) + body = body.replace( + 'ADMIN_PASSWORD="INSERT_HERE"', + f"ADMIN_PASSWORD={shlex.quote(ssh_admin_password())}", + ) + return body + + +def step_screencapture_grant(ctx: HostContext) -> None: + """Grant Screen Recording to the worker binaries. SIP-on hosts only; no-op elsewhere. + + 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 + directly, which works only while SIP is off; on a SIP-on host the write fails + silently and its user-database fallback is inert, because TCC never reads this + service from a user database. The host then fails every getDisplayMedia() call with + SCStreamErrorUserDeclined (-3801) for its whole life, visible only as an intermittent + 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. + + 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 + without the grant. Running it here is what stops today's fleet-wide fix decaying + one host at a time. + + Exit 3 from the script means "not applicable / not now" (SIP off, host busy, no + console session) and is reported, not raised -- the host is still fine to hand back, + and the ronin detector (macos_screencapture_check) will keep the gap visible. + """ + ui.step( + "SCREEN RECORDING", + "grant the worker binaries ScreenCapture TCC (SIP-on hosts only)", + ) + ui.wire( + f"scp -> {SCREENCAPTURE_REMOTE} (0700, credential substituted from the vault)" + ) + ssh.write_file_as_root( + ctx.fqdn, SCREENCAPTURE_REMOTE, _screencapture_script().encode(), mode="0700" + ) + + ui.wire( + f"ssh admin@{ctx.hostname} sudo {SCREENCAPTURE_REMOTE} (drives System Settings as cltbld)" + ) + cp = ssh.run(ctx.fqdn, f"sudo {SCREENCAPTURE_REMOTE}; echo rc=$?", check=False) + out = cp.stdout.decode(errors="replace").strip() + ssh.run(ctx.fqdn, f"sudo rm -f {SCREENCAPTURE_REMOTE}", check=False) + + rc = 1 + for line in out.splitlines(): + if line.startswith("rc="): + rc = int(line[3:] or 1) + + if rc == 0: + ui.ok("Screen Recording granted (auth_value 2, flags 0)") + return + if rc == 3: + reason = next( + (ln for ln in out.splitlines() if ln.startswith("[SKIP]")), + "[SKIP] not applicable", + ) + ui.warn(reason.replace("[SKIP] ", "skipped: ")) + return + raise ReprovisionError( + f"{ctx.fqdn}: Screen Recording grant failed -\n " + + out.replace("\n", "\n ") + ) + + def step_wait_for_sentinel(ctx: HostContext) -> None: s = get_settings() - ui.step("BOOTSTRAP", "the freshly-enrolled host provisions itself — zero operator SSH from here") - ui.wire("signed bootstrap PKG (managed install) lands via SimpleMDM during DEP convergence") - ui.wire("→ host fetches its vault.yaml over mTLS from the forge LB (step-ca SCEP client cert)") - ui.wire(f"→ puppet apply: role {ctx.role} — generic-worker, users, TCC perms, launch daemons") - ui.wire("→ generic-worker self-registers with Taskcluster (Hawk) and starts claiming work") - ui.wire(f"ssh admin@{ctx.hostname} test -f {SENTINEL} (poll for the sentinel it writes)") + ui.step( + "BOOTSTRAP", + "the freshly-enrolled host provisions itself — zero operator SSH from here", + ) + ui.wire( + "signed bootstrap PKG (managed install) lands via SimpleMDM during DEP convergence" + ) + ui.wire( + "→ host fetches its vault.yaml over mTLS from the forge LB (step-ca SCEP client cert)" + ) + ui.wire( + f"→ puppet apply: role {ctx.role} — generic-worker, users, TCC perms, launch daemons" + ) + ui.wire( + "→ generic-worker self-registers with Taskcluster (Hawk) and starts claiming work" + ) + ui.wire( + f"ssh admin@{ctx.hostname} test -f {SENTINEL} (poll for the sentinel it writes)" + ) deadline = time.monotonic() + s.bootstrap_max_wait_seconds found = False with ui.waiting("waiting for the bootstrap sentinel") as tick: @@ -1169,7 +1429,9 @@ def step_wait_for_sentinel(ctx: HostContext) -> None: ui.ok(f"bootstrap complete — {SENTINEL} present") -def step_quarantine_on_register(ctx: HostContext, *, max_wait_seconds: int | None = None) -> None: +def step_quarantine_on_register( + ctx: HostContext, *, max_wait_seconds: int | None = None +) -> None: """Wait for a fresh worker to appear in Taskcluster, then quarantine it on sight. `max_wait_seconds` overrides the default budget. The default is sized for a watch started @@ -1207,18 +1469,27 @@ def step_quarantine_on_register(ctx: HostContext, *, max_wait_seconds: int | Non budget = max_wait_seconds or s.quarantine_on_register_max_wait_seconds pools = candidate_pools(ctx.role) - ui.step("QUARANTINE ON REGISTER", "hold the fresh worker out of the pool the moment it appears") - ui.wire(f"queue.getWorker {' | '.join(pools)} / {ctx.worker_group} / {ctx.hostname} (poll)") + ui.step( + "QUARANTINE ON REGISTER", + "hold the fresh worker out of the pool the moment it appears", + ) + ui.wire( + f"queue.getWorker {' | '.join(pools)} / {ctx.worker_group} / {ctx.hostname} (poll)" + ) ui.info(f"watch budget {budget}s") deadline = time.monotonic() + budget found_pool: str | None = None with ui.waiting("waiting for the worker to register with Taskcluster") as tick: while time.monotonic() < deadline: - found_pool = taskcluster.find_registered_pool(pools, ctx.worker_group, ctx.hostname) + found_pool = taskcluster.find_registered_pool( + pools, ctx.worker_group, ctx.hostname + ) if found_pool: break - tick("not in a pool yet — worker-runner starts generic-worker after the sentinel") + tick( + "not in a pool yet — worker-runner starts generic-worker after the sentinel" + ) time.sleep(s.quarantine_on_register_poll_seconds) if not found_pool: @@ -1230,7 +1501,9 @@ def step_quarantine_on_register(ctx: HostContext, *, max_wait_seconds: int | Non ctx.worker_pool_id = found_pool ui.ok(f"registered in {found_pool}") - step_quarantine(ctx, info="fresh host — quarantined on registration pending validation") + step_quarantine( + ctx, info="fresh host — quarantined on registration pending validation" + ) def step_unquarantine(ctx: HostContext) -> None: @@ -1240,7 +1513,9 @@ def step_unquarantine(ctx: HostContext) -> None: ui.ok("returned to service") -def reprovision(hostname: str, *, skip_wipe: bool = False, unquarantine: bool = False) -> None: +def reprovision( + hostname: str, *, skip_wipe: bool = False, unquarantine: bool = False +) -> None: """Full E2E workflow. skip_wipe lets operators re-run later steps after a wipe. unquarantine defaults to False: by design a host stays quarantined through wipe + @@ -1263,7 +1538,7 @@ def reprovision(hostname: str, *, skip_wipe: bool = False, unquarantine: bool = phases += ["QUARANTINE", "DRAIN"] if not skip_wipe: phases += ["WIPE", "RE-ENROLL"] - phases += ["MINT", "ESCROW BST", "BOOTSTRAP"] + phases += ["MINT", "ESCROW BST", "BOOTSTRAP", "SCREEN RECORDING"] if unquarantine and ctx.registered: phases += ["UNQUARANTINE"] ui.flow(phases) @@ -1272,7 +1547,9 @@ def reprovision(hostname: str, *, skip_wipe: bool = False, unquarantine: bool = step_quarantine(ctx) step_drain(ctx) else: - ui.warn(f"{ctx.hostname} isn't registered in Taskcluster — skipping quarantine/drain (nothing to drain)") + ui.warn( + f"{ctx.hostname} isn't registered in Taskcluster — skipping quarantine/drain (nothing to drain)" + ) if not skip_wipe: step_wipe(ctx) step_wait_for_reenroll(ctx) @@ -1284,6 +1561,11 @@ def reprovision(hostname: str, *, skip_wipe: bool = False, unquarantine: bool = # convergence once admin logs in (the mint), so nothing needs to trigger it. We just # wait for the sentinel it writes. step_wait_for_sentinel(ctx) + # After the bootstrap, because the grant is anchored to the worker binaries and + # needs cltbld's console session -- neither exists until puppet has run. EACS wiped + # TCC, so without this the host returns to service silently unable to screen-capture + # (bug 2073303). + step_screencapture_grant(ctx) # Default: leave the host quarantined (matches current fleet reality; no un-quarantine # key wired). Only return it to service when explicitly asked — the eventual prod flow. # Skip if we never quarantined it (host was unregistered at start). @@ -1351,12 +1633,14 @@ def provision( phases = ["PREFLIGHT", "MINT", "ESCROW BST"] if wait: - phases += ["BOOTSTRAP PKG", "BOOTSTRAP"] + phases += ["BOOTSTRAP PKG", "BOOTSTRAP", "SCREEN RECORDING"] if quarantine_on_register: phases.append("QUARANTINE ON REGISTER") ui.flow(phases) - step_preflight(ctx, expected_os=expected_os, require_sip_disabled=require_sip_disabled) + step_preflight( + ctx, expected_os=expected_os, require_sip_disabled=require_sip_disabled + ) step_mint(ctx) # mint SecureToken (must precede escrow_bst) step_escrow_bst(ctx) if wait: @@ -1366,11 +1650,18 @@ def provision( # The bootstrap pkg is a managed install driven by group membership, so there is # nothing to trigger — we only wait for the sentinel it writes. step_wait_for_sentinel(ctx) + # Needs the worker binaries and cltbld's console session, so only after the + # bootstrap has run. No-op on SIP-off hosts (bug 2073303). + step_screencapture_grant(ctx) else: - ui.info("--no-wait: credentials are in place; sweep the sentinel later with `wait-sentinel`") + ui.info( + "--no-wait: credentials are in place; sweep the sentinel later with `wait-sentinel`" + ) if quarantine_on_register: step_quarantine_on_register(ctx) elapsed = time.monotonic() - started - ui.provisioned(ctx.hostname, elapsed, waited=wait, quarantined=quarantine_on_register) + ui.provisioned( + ctx.hostname, elapsed, waited=wait, quarantined=quarantine_on_register + ) diff --git a/orchestrator/tests/test_mint.py b/orchestrator/tests/test_mint.py index 1d868df..4bb6c45 100644 --- a/orchestrator/tests/test_mint.py +++ b/orchestrator/tests/test_mint.py @@ -26,12 +26,18 @@ def _ctx(): # --- mint --- + def test_mint_is_idempotent_when_already_enabled(): """If admin already holds a SecureToken, we must NOT attempt a password login.""" - with patch("orchestrator.workflow.ssh.forget_host_key"), \ - patch("orchestrator.workflow.ssh.wait_for_sshd"), \ - patch("orchestrator.workflow.ssh.secure_token_status", return_value="ENABLED for user admin"), \ - patch("orchestrator.workflow.ssh.password_login") as login: + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + patch("orchestrator.workflow.ssh.wait_for_sshd"), + patch( + "orchestrator.workflow.ssh.secure_token_status", + return_value="ENABLED for user admin", + ), + patch("orchestrator.workflow.ssh.password_login") as login, + ): workflow.step_mint(_ctx()) login.assert_not_called() @@ -39,45 +45,62 @@ def test_mint_is_idempotent_when_already_enabled(): def test_mint_logs_in_then_verifies_enabled(): """DISABLED -> password_login -> then status flips to ENABLED -> success.""" statuses = iter(["DISABLED for user admin", "ENABLED for user admin"]) - with patch("orchestrator.workflow.ssh.forget_host_key"), \ - patch("orchestrator.workflow.ssh.wait_for_sshd"), \ - patch("orchestrator.workflow.ssh.secure_token_status", side_effect=lambda *_: next(statuses)), \ - patch("orchestrator.workflow.ssh.password_login") as login: + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + patch("orchestrator.workflow.ssh.wait_for_sshd"), + patch( + "orchestrator.workflow.ssh.secure_token_status", + side_effect=lambda *_: next(statuses), + ), + patch("orchestrator.workflow.ssh.password_login") as login, + ): workflow.step_mint(_ctx()) login.assert_called_once() def test_mint_raises_if_token_never_enables(): """If the token never comes up ENABLED after the login, the step must fail loudly.""" - with patch("orchestrator.workflow.ssh.forget_host_key"), \ - patch("orchestrator.workflow.ssh.wait_for_sshd"), \ - patch("orchestrator.workflow.ssh.secure_token_status", return_value="DISABLED for user admin"), \ - patch("orchestrator.workflow.ssh.password_login"), \ - patch("orchestrator.workflow.time.sleep"): # don't wait through the retries + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + patch("orchestrator.workflow.ssh.wait_for_sshd"), + patch( + "orchestrator.workflow.ssh.secure_token_status", + return_value="DISABLED for user admin", + ), + patch("orchestrator.workflow.ssh.password_login"), + patch("orchestrator.workflow.time.sleep"), + ): # don't wait through the retries with pytest.raises(RuntimeError): workflow.step_mint(_ctx()) # --- escrow_bst --- + def test_escrow_bst_is_non_interactive(): """escrow_bst must pass -user/-password (the bare form prompts and fails).""" cmds = [] def fake_run(host, cmd, **_): cmds.append(cmd) + class CP: stdout = b"profiles: Bootstrap Token escrowed to server: YES" + return CP() - with patch("orchestrator.workflow.ssh.run", side_effect=fake_run), \ - patch("orchestrator.workflow.ssh_admin_password", return_value="s3cr3t-pw"): + with ( + patch("orchestrator.workflow.ssh.run", side_effect=fake_run), + patch("orchestrator.workflow.ssh_admin_password", return_value="s3cr3t-pw"), + ): workflow.step_escrow_bst(_ctx()) install = next(c for c in cmds if "profiles install" in c) assert "-type bootstraptoken" in install assert "-user admin" in install assert "-password" in install - assert "s3cr3t-pw" in install # the resolved password is passed, not an empty string + assert ( + "s3cr3t-pw" in install + ) # the resolved password is passed, not an empty string def test_escrow_bst_ssh_error_does_not_leak_password(): @@ -86,19 +109,27 @@ def test_escrow_bst_ssh_error_does_not_leak_password(): def boom(host, cmd, **_): # ssh.run scrubs the command itself; it raises without the password in the message. - raise ReprovisionError(f"remote command on {host} failed (exit 1): profiles: error") - - with patch("orchestrator.workflow.ssh.run", side_effect=boom), \ - patch("orchestrator.workflow.ssh_admin_password", return_value="s3cr3t-pw"): + raise ReprovisionError( + f"remote command on {host} failed (exit 1): profiles: error" + ) + + with ( + patch("orchestrator.workflow.ssh.run", side_effect=boom), + patch("orchestrator.workflow.ssh_admin_password", return_value="s3cr3t-pw"), + ): with pytest.raises(ReprovisionError) as ei: workflow.step_escrow_bst(_ctx()) assert "s3cr3t-pw" not in str(ei.value) - assert ei.value.__suppress_context__ # `from None` — no chained exception leaks the cmd + assert ( + ei.value.__suppress_context__ + ) # `from None` — no chained exception leaks the cmd def test_escrow_bst_raises_when_not_escrowed(): - with patch("orchestrator.workflow.ssh.run") as run, \ - patch("orchestrator.workflow.ssh_admin_password", return_value="s3cr3t-pw"): + with ( + patch("orchestrator.workflow.ssh.run") as run, + patch("orchestrator.workflow.ssh_admin_password", return_value="s3cr3t-pw"), + ): run.return_value.stdout = b"profiles: Bootstrap Token escrowed to server: NO" with pytest.raises(RuntimeError): workflow.step_escrow_bst(_ctx()) @@ -106,6 +137,7 @@ def test_escrow_bst_raises_when_not_escrowed(): # --- wipe guard --- + def test_wipe_auto_escrows_bst_then_proceeds(): """BST not escrowed at first → step_wipe escrows it in place, then wipes (self-heal).""" n = {"status": 0} @@ -117,29 +149,43 @@ def fake_run(fqdn, cmd, check=True, **kw): if "profiles status" in cmd: n["status"] += 1 # 1st status = the step_wipe guard (NO); 2nd = step_escrow_bst's re-verify (YES) - m.stdout = b"escrowed to server: NO" if n["status"] == 1 else b"escrowed to server: YES" + m.stdout = ( + b"escrowed to server: NO" + if n["status"] == 1 + else b"escrowed to server: YES" + ) else: m.stdout = b"" # the `profiles install -type bootstraptoken` command return m - with patch("orchestrator.workflow.ssh.forget_host_key"), \ - patch("orchestrator.workflow.ssh.run", side_effect=fake_run), \ - patch("orchestrator.workflow.ssh_admin_password", return_value="pw"), \ - patch("orchestrator.workflow.taskcluster.is_currently_busy", return_value=False), \ - patch("orchestrator.workflow.simplemdm.get_device", - return_value={"attributes": {"enrolled_at": "2026-01-01T00:00:00Z"}}), \ - patch("orchestrator.workflow.simplemdm.wipe") as wipe: + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + patch("orchestrator.workflow.ssh.run", side_effect=fake_run), + patch("orchestrator.workflow.ssh_admin_password", return_value="pw"), + patch( + "orchestrator.workflow.taskcluster.is_currently_busy", return_value=False + ), + patch( + "orchestrator.workflow.simplemdm.get_device", + return_value={"attributes": {"enrolled_at": "2026-01-01T00:00:00Z"}}, + ), + patch("orchestrator.workflow.simplemdm.wipe") as wipe, + ): workflow.step_wipe(_ctx()) wipe.assert_called_once() def test_wipe_aborts_when_bst_unescrowable(): """No escrowed BST AND auto-escrow can't fix it → refuse to wipe (never obliterate).""" - with patch("orchestrator.workflow.ssh.forget_host_key"), \ - patch("orchestrator.workflow.ssh.run") as run, \ - patch("orchestrator.workflow.ssh_admin_password", return_value="pw"), \ - patch("orchestrator.workflow.simplemdm.wipe") as wipe: - run.return_value.returncode = 0 # ssh works; box genuinely has no BST and escrow won't take + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + patch("orchestrator.workflow.ssh.run") as run, + patch("orchestrator.workflow.ssh_admin_password", return_value="pw"), + patch("orchestrator.workflow.simplemdm.wipe") as wipe, + ): + run.return_value.returncode = ( + 0 # ssh works; box genuinely has no BST and escrow won't take + ) run.return_value.stdout = b"profiles: Bootstrap Token escrowed to server: NO" with pytest.raises(RuntimeError, match="auto-escrow failed"): workflow.step_wipe(_ctx()) @@ -148,9 +194,11 @@ def test_wipe_aborts_when_bst_unescrowable(): def test_wipe_aborts_when_ssh_check_fails_without_claiming_no_bst(): """An ssh failure must NOT masquerade as a missing Bootstrap Token, and must not wipe.""" - with patch("orchestrator.workflow.ssh.forget_host_key"), \ - patch("orchestrator.workflow.ssh.run") as run, \ - patch("orchestrator.workflow.simplemdm.wipe") as wipe: + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + patch("orchestrator.workflow.ssh.run") as run, + patch("orchestrator.workflow.simplemdm.wipe") as wipe, + ): run.return_value.returncode = 255 # e.g. first-connection host key / VPN down run.return_value.stdout = b"" run.return_value.stderr = b"Host key verification failed." @@ -161,11 +209,18 @@ def test_wipe_aborts_when_ssh_check_fails_without_claiming_no_bst(): def test_wipe_forgets_stale_host_key_before_verifying(): """EACS rotates the host key; step_wipe must clear the stale entry before the ssh check.""" - with patch("orchestrator.workflow.ssh.forget_host_key") as forget, \ - patch("orchestrator.workflow.ssh.run") as run, \ - patch("orchestrator.workflow.taskcluster.is_currently_busy", return_value=False), \ - patch("orchestrator.workflow.simplemdm.get_device", return_value={"attributes": {"enrolled_at": "x"}}), \ - patch("orchestrator.workflow.simplemdm.wipe"): + with ( + patch("orchestrator.workflow.ssh.forget_host_key") as forget, + patch("orchestrator.workflow.ssh.run") as run, + patch( + "orchestrator.workflow.taskcluster.is_currently_busy", return_value=False + ), + patch( + "orchestrator.workflow.simplemdm.get_device", + return_value={"attributes": {"enrolled_at": "x"}}, + ), + patch("orchestrator.workflow.simplemdm.wipe"), + ): run.return_value.returncode = 0 run.return_value.stdout = b"profiles: Bootstrap Token escrowed to server: YES" workflow.step_wipe(_ctx()) @@ -175,10 +230,13 @@ def test_wipe_forgets_stale_host_key_before_verifying(): def test_wipe_aborts_when_worker_busy(): """Never EACS a worker that's mid-task, even with an escrowed BST.""" from orchestrator.errors import ReprovisionError - with patch("orchestrator.workflow.ssh.forget_host_key"), \ - patch("orchestrator.workflow.ssh.run") as run, \ - patch("orchestrator.workflow.taskcluster.is_currently_busy", return_value=True), \ - patch("orchestrator.workflow.simplemdm.wipe") as wipe: + + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + patch("orchestrator.workflow.ssh.run") as run, + patch("orchestrator.workflow.taskcluster.is_currently_busy", return_value=True), + patch("orchestrator.workflow.simplemdm.wipe") as wipe, + ): run.return_value.returncode = 0 run.return_value.stdout = b"profiles: Bootstrap Token escrowed to server: YES" with pytest.raises(ReprovisionError, match="still running a task"): @@ -187,11 +245,18 @@ def test_wipe_aborts_when_worker_busy(): def test_wipe_proceeds_with_escrowed_bst(): - with patch("orchestrator.workflow.ssh.forget_host_key"), \ - patch("orchestrator.workflow.ssh.run") as run, \ - patch("orchestrator.workflow.taskcluster.is_currently_busy", return_value=False), \ - patch("orchestrator.workflow.simplemdm.get_device", return_value={"attributes": {"enrolled_at": "x"}}), \ - patch("orchestrator.workflow.simplemdm.wipe") as wipe: + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + patch("orchestrator.workflow.ssh.run") as run, + patch( + "orchestrator.workflow.taskcluster.is_currently_busy", return_value=False + ), + patch( + "orchestrator.workflow.simplemdm.get_device", + return_value={"attributes": {"enrolled_at": "x"}}, + ), + patch("orchestrator.workflow.simplemdm.wipe") as wipe, + ): run.return_value.returncode = 0 run.return_value.stdout = b"profiles: Bootstrap Token escrowed to server: YES" workflow.step_wipe(_ctx()) @@ -200,6 +265,7 @@ def test_wipe_proceeds_with_escrowed_bst(): # --- reprovision() sequence --- + def _run_reprovision_capturing(**kwargs) -> list[str]: """Run reprovision() with every step stubbed; return the ordered list of step names.""" calls: list[str] = [] @@ -207,15 +273,18 @@ def _run_reprovision_capturing(**kwargs) -> list[str]: def rec(name): return lambda ctx: calls.append(name) - with patch("orchestrator.workflow.resolve", return_value=_ctx()), \ - patch("orchestrator.workflow.step_quarantine", rec("quarantine")), \ - patch("orchestrator.workflow.step_drain", rec("drain")), \ - patch("orchestrator.workflow.step_wipe", rec("wipe")), \ - patch("orchestrator.workflow.step_wait_for_reenroll", rec("reenroll")), \ - patch("orchestrator.workflow.step_mint", rec("mint")), \ - patch("orchestrator.workflow.step_escrow_bst", rec("escrow")), \ - patch("orchestrator.workflow.step_wait_for_sentinel", rec("sentinel")), \ - patch("orchestrator.workflow.step_unquarantine", rec("unquarantine")): + with ( + patch("orchestrator.workflow.resolve", return_value=_ctx()), + patch("orchestrator.workflow.step_quarantine", rec("quarantine")), + patch("orchestrator.workflow.step_drain", rec("drain")), + patch("orchestrator.workflow.step_wipe", rec("wipe")), + patch("orchestrator.workflow.step_wait_for_reenroll", rec("reenroll")), + patch("orchestrator.workflow.step_mint", rec("mint")), + patch("orchestrator.workflow.step_escrow_bst", rec("escrow")), + patch("orchestrator.workflow.step_wait_for_sentinel", rec("sentinel")), + patch("orchestrator.workflow.step_screencapture_grant", rec("screencapture")), + patch("orchestrator.workflow.step_unquarantine", rec("unquarantine")), + ): workflow.reprovision("macmini-m4-81", **kwargs) return calls @@ -225,8 +294,15 @@ def test_reprovision_default_flow(): calls = _run_reprovision_capturing() assert "unquarantine" not in calls # quarantine persists by default assert calls.index("mint") < calls.index("escrow") # mint precedes BST escrow - assert not hasattr(workflow, "step_deliver_vault") # no 1Password vault drop (bootstrap self-fetches over mTLS) - assert not hasattr(workflow, "step_rotate_admin_password") # rotation is a DEP-config concern + assert not hasattr( + workflow, "step_deliver_vault" + ) # no 1Password vault drop (bootstrap self-fetches over mTLS) + assert not hasattr( + workflow, "step_rotate_admin_password" + ) # rotation is a DEP-config concern + # EACS wipes TCC, so the ScreenCapture grant has to be re-applied every cycle and must + # come after the bootstrap (it needs the worker binaries and cltbld's console session). + assert calls.index("sentinel") < calls.index("screencapture") # bug 2073303 def test_reprovision_unquarantine_flag_returns_to_service(): @@ -234,15 +310,19 @@ def test_reprovision_unquarantine_flag_returns_to_service(): assert calls[-1] == "unquarantine" # runs, and last - def test_wipe_aborts_when_idle_cannot_be_confirmed(): """Fail CLOSED: if Taskcluster can't confirm idle (e.g. bad creds / 'Bad mac'), refuse to wipe — must NOT warn-and-proceed. This was the 2026-07-10 incident path (a running worker got EACS'd because the idle check failed open).""" - with patch("orchestrator.workflow.ssh.forget_host_key"), \ - patch("orchestrator.workflow.ssh.run") as run, \ - patch("orchestrator.workflow.taskcluster.is_currently_busy", side_effect=RuntimeError("Bad mac")), \ - patch("orchestrator.workflow.simplemdm.wipe") as wipe: + with ( + patch("orchestrator.workflow.ssh.forget_host_key"), + patch("orchestrator.workflow.ssh.run") as run, + patch( + "orchestrator.workflow.taskcluster.is_currently_busy", + side_effect=RuntimeError("Bad mac"), + ), + patch("orchestrator.workflow.simplemdm.wipe") as wipe, + ): run.return_value.returncode = 0 run.return_value.stdout = b"profiles: Bootstrap Token escrowed to server: YES" with pytest.raises(ReprovisionError): diff --git a/orchestrator/tests/test_screencapture_grant.py b/orchestrator/tests/test_screencapture_grant.py new file mode 100644 index 0000000..fa5bfed --- /dev/null +++ b/orchestrator/tests/test_screencapture_grant.py @@ -0,0 +1,154 @@ +""" +Tests for the Screen Recording (ScreenCapture TCC) grant step. Bug 2073303. + +The ssh layer is mocked throughout; nothing touches a real host. + +The behaviour that matters here is the three-way split on the payload's exit code. +Getting it wrong is expensive in opposite directions: treating a real failure as a +skip hands back a host that silently cannot screen-capture (the original bug, which +took 30 days and 499 oranges to notice), while treating a skip as a failure aborts a +reprovision over a host that merely happened to be mid-task. +""" + +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from orchestrator import workflow +from orchestrator.errors import ReprovisionError + + +def _ctx(): + return workflow.HostContext( + hostname="macmini-m4-265", + fqdn="macmini-m4-265.test.releng.mdc1.mozilla.com", + role="gecko_t_osx_1500_m4", + worker_pool_id="releng-hardware/gecko-t-osx-1500-m4", + ) + + +class _CP: + def __init__(self, out: str): + self.stdout = out.encode() + self.returncode = 0 + + +def _run_with(output: str): + """Patch ssh so the payload 'returns' output; yields the run mock.""" + return patch( + "orchestrator.workflow.ssh.run", side_effect=lambda *a, **k: _CP(output) + ) + + +def test_granted_is_success(): + with ( + patch("orchestrator.workflow.ssh.write_file_as_root"), + patch( + "orchestrator.workflow._screencapture_script", return_value="#!/bin/bash\n" + ), + _run_with("[screencapture] granted /usr/local/bin/start-worker (2/0)\nrc=0"), + ): + workflow.step_screencapture_grant(_ctx()) # must not raise + + +@pytest.mark.parametrize( + "reason", + [ + "[SKIP] SIP is off — macos_tcc_perms already grants this host", + "[SKIP] host is running a task — retry when idle", + "[SKIP] cltbld does not own the console session yet", + ], +) +def test_skip_conditions_do_not_raise(reason): + """Exit 3 is 'not applicable / not now'. The host is still fine to hand back.""" + with ( + patch("orchestrator.workflow.ssh.write_file_as_root"), + patch( + "orchestrator.workflow._screencapture_script", return_value="#!/bin/bash\n" + ), + _run_with(f"{reason}\nrc=3"), + ): + workflow.step_screencapture_grant(_ctx()) # must not raise + + +@pytest.mark.parametrize( + "output", + [ + "[ERROR] worker binary is not Developer-ID signed (Identifier=a.out)\nrc=1", + "[ERROR] a ScreenCapture PPPC override is installed (2 entries)\nrc=1", + "[ERROR] /usr/local/bin/start-worker landed flags=12 (MDM-managed, TCC ignores it)\nrc=1", + "[ERROR] /usr/local/bin/start-worker not granted (got 0/6)\nrc=1", + ], +) +def test_real_failures_raise(output): + """A host that cannot hold the grant must fail loudly, not be quietly returned to the pool.""" + with ( + patch("orchestrator.workflow.ssh.write_file_as_root"), + patch( + "orchestrator.workflow._screencapture_script", return_value="#!/bin/bash\n" + ), + _run_with(output), + ): + with pytest.raises(ReprovisionError): + workflow.step_screencapture_grant(_ctx()) + + +def test_missing_rc_is_treated_as_failure(): + """Truncated/garbled output must not be read as success.""" + with ( + patch("orchestrator.workflow.ssh.write_file_as_root"), + patch( + "orchestrator.workflow._screencapture_script", return_value="#!/bin/bash\n" + ), + _run_with("something went sideways"), + ): + with pytest.raises(ReprovisionError): + workflow.step_screencapture_grant(_ctx()) + + +def test_payload_is_cleaned_up_from_the_host(): + """The staged script carries the admin credential; it must not be left behind.""" + calls = [] + + def _run(fqdn, cmd, **kw): + calls.append(cmd) + return _CP("rc=0") + + with ( + patch("orchestrator.workflow.ssh.write_file_as_root"), + patch( + "orchestrator.workflow._screencapture_script", return_value="#!/bin/bash\n" + ), + patch("orchestrator.workflow.ssh.run", side_effect=_run), + ): + workflow.step_screencapture_grant(_ctx()) + + assert any( + c.startswith("sudo rm -f ") and workflow.SCREENCAPTURE_REMOTE in c + for c in calls + ), calls + + +def test_script_substitutes_the_credential_placeholders(): + """The packaged body must not reach the host with placeholders intact.""" + with patch("orchestrator.workflow.ssh_admin_password", return_value="s3cr3t"): + body = workflow._screencapture_script() + assert 'ADMIN_PASSWORD="INSERT_HERE"' not in body + assert 'ADMIN_USER="INSERT_USER_HERE"' not in body + assert "s3cr3t" in body + # The payload's own guard against an unsubstituted placeholder must survive. + assert '[ "$ADMIN_PASSWORD" = "INSERT_HERE" ]' in body + + +def test_step_is_in_both_flows(): + """Regression guard: the grant must not silently drop out of the sequences. + + EACS wipes TCC, so a reprovision that skips this hands back a host that cannot + screen-capture and says nothing about it. + """ + import inspect + + assert "step_screencapture_grant" in inspect.getsource(workflow.reprovision) + assert "step_screencapture_grant" in inspect.getsource(workflow.provision) From 432c27e15a2d6c469e79666d0d81e1344de7ff71 Mon Sep 17 00:00:00 2001 From: Ryan Curran Date: Mon, 21 Sep 2026 14:26:29 -0400 Subject: [PATCH 2/2] Mock the new grant step in the provision() sequence tests CI caught what the local run could not. tests/test_provision.py patches the step list but did not know about step_screencapture_grant, so the real step ran and tried to resolve the admin password from 1Password. That passes on a laptop with `op` installed and authenticated, and fails on a runner without it -- a false green of exactly the kind this suite exists to prevent. Mocked in all five provision() sequence tests, and the ordering test now asserts the grant runs last, after the bootstrap sentinel: it needs the worker binaries in place and cltbld owning the console session, neither of which exists before puppet has run. Verified by reproducing the CI condition locally -- running the suite with the 1Password CLI removed from PATH. 285 passed. Co-Authored-By: Claude Opus 5 (1M context) --- orchestrator/tests/test_provision.py | 315 ++++++++++++++++++--------- 1 file changed, 215 insertions(+), 100 deletions(-) diff --git a/orchestrator/tests/test_provision.py b/orchestrator/tests/test_provision.py index 260ea3a..d90bd41 100644 --- a/orchestrator/tests/test_provision.py +++ b/orchestrator/tests/test_provision.py @@ -14,7 +14,9 @@ def _cp(stdout: bytes = b"", returncode: int = 0) -> subprocess.CompletedProcess: - return subprocess.CompletedProcess(args=["ssh"], returncode=returncode, stdout=stdout, stderr=b"") + return subprocess.CompletedProcess( + args=["ssh"], returncode=returncode, stdout=stdout, stderr=b"" + ) def _ctx(): @@ -24,8 +26,15 @@ def _ctx(): class _Host: """Canned responses for the handful of remote reads preflight makes.""" - def __init__(self, *, os_version="15.3", sip="System Integrity Protection status: disabled.", - token="ENABLED", escrowed=True, sentinel=False): + def __init__( + self, + *, + os_version="15.3", + sip="System Integrity Protection status: disabled.", + token="ENABLED", + escrowed=True, + sentinel=False, + ): self.os_version = os_version self.sip = sip self.token = token @@ -38,7 +47,11 @@ def run(self, _fqdn, command, **_kw): if "csrutil" in command: return _cp(self.sip.encode()) if "bootstraptoken" in command: - body = b"escrowed to server: YES" if self.escrowed else b"escrowed to server: NO" + body = ( + b"escrowed to server: YES" + if self.escrowed + else b"escrowed to server: NO" + ) return _cp(body) return _cp() @@ -46,7 +59,9 @@ def install(self): return ( patch("orchestrator.workflow.ssh.wait_for_sshd", return_value=None), patch("orchestrator.workflow.ssh.run", side_effect=self.run), - patch("orchestrator.workflow.ssh.secure_token_status", return_value=self.token), + patch( + "orchestrator.workflow.ssh.secure_token_status", return_value=self.token + ), patch("orchestrator.workflow.ssh.file_exists", return_value=self.sentinel), ) @@ -67,8 +82,10 @@ def _preflight(host: _Host, **kwargs): def test_resolve_offline_makes_no_api_calls(): # A 55-host readiness sweep must not need a SimpleMDM key or TC creds. - with patch("orchestrator.workflow.simplemdm.find_device_by_name") as mdm, \ - patch("orchestrator.workflow.taskcluster.find_registered_pool") as tc: + with ( + patch("orchestrator.workflow.simplemdm.find_device_by_name") as mdm, + patch("orchestrator.workflow.taskcluster.find_registered_pool") as tc, + ): ctx = workflow.resolve_offline(HOST) mdm.assert_not_called() tc.assert_not_called() @@ -92,11 +109,11 @@ def test_resolve_offline_rejects_a_bad_hostname(): "actual,expected,ok", [ ("15.3", "15.3", True), - ("15.3.1", "15.3", True), # point release of the target is fine - ("15.30", "15.3", False), # must not match on a bare prefix + ("15.3.1", "15.3", True), # point release of the target is fine + ("15.30", "15.3", False), # must not match on a bare prefix ("15.4", "15.3", False), ("15.2", "15.3", False), - ("26.1", "15.3", False), # shipped-with OS on new hardware + ("26.1", "15.3", False), # shipped-with OS on new hardware ], ) def test_os_version_matching(actual, expected, ok): @@ -123,12 +140,17 @@ def test_preflight_skips_when_sip_is_enabled(): def test_preflight_allows_sip_on_when_asked(): - _preflight(_Host(sip="System Integrity Protection status: enabled."), require_sip_disabled=False) + _preflight( + _Host(sip="System Integrity Protection status: enabled."), + require_sip_disabled=False, + ) def test_preflight_skips_when_sshd_is_not_up(): # Fresh DEP hosts appear over ~15 min; not-up-yet is "skip and re-run", not a failure. - with patch("orchestrator.workflow.ssh.wait_for_sshd", side_effect=TimeoutError("nope")): + with patch( + "orchestrator.workflow.ssh.wait_for_sshd", side_effect=TimeoutError("nope") + ): with pytest.raises(NotReadyError, match="sshd not reachable"): workflow.step_preflight(_ctx()) @@ -152,34 +174,70 @@ def test_not_ready_is_a_reprovision_error(): def test_provision_runs_the_steps_in_order_and_never_wipes(): calls: list[str] = [] - with patch("orchestrator.workflow.step_preflight", side_effect=lambda *a, **k: calls.append("preflight")), \ - patch("orchestrator.workflow.step_mint", side_effect=lambda *a, **k: calls.append("mint")), \ - patch("orchestrator.workflow.step_escrow_bst", side_effect=lambda *a, **k: calls.append("escrow")), \ - patch("orchestrator.workflow.step_wait_for_bootstrap_pkg"), \ - patch("orchestrator.workflow.step_wait_for_sentinel", side_effect=lambda *a, **k: calls.append("sentinel")), \ - patch("orchestrator.workflow.step_wipe", side_effect=AssertionError("provision must never wipe")), \ - patch("orchestrator.workflow.step_quarantine", side_effect=AssertionError("no pool calls")): + with ( + patch( + "orchestrator.workflow.step_preflight", + side_effect=lambda *a, **k: calls.append("preflight"), + ), + patch( + "orchestrator.workflow.step_mint", + side_effect=lambda *a, **k: calls.append("mint"), + ), + patch( + "orchestrator.workflow.step_escrow_bst", + side_effect=lambda *a, **k: calls.append("escrow"), + ), + patch("orchestrator.workflow.step_wait_for_bootstrap_pkg"), + patch( + "orchestrator.workflow.step_wait_for_sentinel", + side_effect=lambda *a, **k: calls.append("sentinel"), + ), + patch( + "orchestrator.workflow.step_screencapture_grant", + side_effect=lambda *a, **k: calls.append("screencapture"), + ), + patch( + "orchestrator.workflow.step_wipe", + side_effect=AssertionError("provision must never wipe"), + ), + patch( + "orchestrator.workflow.step_quarantine", + side_effect=AssertionError("no pool calls"), + ), + ): workflow.provision(HOST) # mint before escrow is load-bearing: the BST escrow needs an existing SecureToken holder. # (the pkg gate's own position is pinned by test_provision_gates_on_the_pkg_before_the_sentinel_wait) - assert calls == ["preflight", "mint", "escrow", "sentinel"] + # The grant runs last, after the bootstrap: it needs the worker binaries in place + # and cltbld owning the console session, neither of which exists before puppet has + # run. EACS wiped TCC, so skipping it returns a host that cannot screen-capture and + # says nothing about it (bug 2073303). + assert calls == ["preflight", "mint", "escrow", "sentinel", "screencapture"] def test_provision_no_wait_stops_after_escrow(): - with patch("orchestrator.workflow.step_preflight"), \ - patch("orchestrator.workflow.step_mint"), \ - patch("orchestrator.workflow.step_escrow_bst"), \ - patch("orchestrator.workflow.step_wait_for_sentinel") as sentinel: + with ( + patch("orchestrator.workflow.step_preflight"), + patch("orchestrator.workflow.step_mint"), + patch("orchestrator.workflow.step_escrow_bst"), + patch("orchestrator.workflow.step_screencapture_grant"), + patch("orchestrator.workflow.step_wait_for_sentinel") as sentinel, + ): workflow.provision(HOST, wait=False) sentinel.assert_not_called() def test_provision_stops_at_the_gate(): # A host that fails preflight must not get mint/escrow attempted anyway. - with patch("orchestrator.workflow.step_preflight", side_effect=NotReadyError("macOS 26.1")), \ - patch("orchestrator.workflow.step_mint") as mint, \ - patch("orchestrator.workflow.step_escrow_bst") as escrow: + with ( + patch( + "orchestrator.workflow.step_preflight", + side_effect=NotReadyError("macOS 26.1"), + ), + patch("orchestrator.workflow.step_mint") as mint, + patch("orchestrator.workflow.step_escrow_bst") as escrow, + ): with pytest.raises(NotReadyError): workflow.provision(HOST) mint.assert_not_called() @@ -200,9 +258,16 @@ def _pkg_settings(settings, *, max_wait=60): def test_pkg_gate_passes_once_the_managed_install_lands(): # A group move that just happened leaves an MDM check-in pending, so the first polls # legitimately find nothing. - seen = [False, False, False, True] # sentinel absent, then pkg absent twice, then present - with patch("orchestrator.workflow.ssh.file_exists", side_effect=seen), \ - patch("orchestrator.workflow.get_settings") as settings: + seen = [ + False, + False, + False, + True, + ] # sentinel absent, then pkg absent twice, then present + with ( + patch("orchestrator.workflow.ssh.file_exists", side_effect=seen), + patch("orchestrator.workflow.get_settings") as settings, + ): _pkg_settings(settings) workflow.step_wait_for_bootstrap_pkg(_ctx()) # no raise @@ -210,8 +275,10 @@ def test_pkg_gate_passes_once_the_managed_install_lands(): def test_pkg_gate_skips_a_host_in_the_wrong_group(): # Without this the sentinel wait burns the full hour before failing with a message that # points at the bootstrap instead of the group assignment. - with patch("orchestrator.workflow.ssh.file_exists", return_value=False), \ - patch("orchestrator.workflow.get_settings") as settings: + with ( + patch("orchestrator.workflow.ssh.file_exists", return_value=False), + patch("orchestrator.workflow.get_settings") as settings, + ): _pkg_settings(settings, max_wait=0) with pytest.raises(NotReadyError, match="is this host in the bootstrap group"): workflow.step_wait_for_bootstrap_pkg(_ctx()) @@ -220,8 +287,10 @@ def test_pkg_gate_skips_a_host_in_the_wrong_group(): def test_pkg_gate_is_a_noop_on_an_already_bootstrapped_host(): # Sentinel present => the pkg obviously ran; don't re-poll for a payload that may predate # the check. - with patch("orchestrator.workflow.ssh.file_exists", return_value=True) as exists, \ - patch("orchestrator.workflow.get_settings") as settings: + with ( + patch("orchestrator.workflow.ssh.file_exists", return_value=True) as exists, + patch("orchestrator.workflow.get_settings") as settings, + ): _pkg_settings(settings) workflow.step_wait_for_bootstrap_pkg(_ctx()) # One call: the sentinel probe. It must not go on to poll for the payload. @@ -237,7 +306,9 @@ def test_preflight_reports_the_pkg_but_does_not_gate_on_it(): for p in patches: p.start() try: - workflow.step_preflight(_ctx()) # pkg absent (file_exists -> False), must not raise + workflow.step_preflight( + _ctx() + ) # pkg absent (file_exists -> False), must not raise finally: for p in patches: p.stop() @@ -245,24 +316,33 @@ def test_preflight_reports_the_pkg_but_does_not_gate_on_it(): def test_provision_gates_on_the_pkg_before_the_sentinel_wait(): calls: list[str] = [] - with patch("orchestrator.workflow.step_preflight"), \ - patch("orchestrator.workflow.step_mint"), \ - patch("orchestrator.workflow.step_escrow_bst"), \ - patch( - "orchestrator.workflow.step_wait_for_bootstrap_pkg", - side_effect=lambda *a: calls.append("pkg"), - ), \ - patch("orchestrator.workflow.step_wait_for_sentinel", side_effect=lambda *a: calls.append("sentinel")): + with ( + patch("orchestrator.workflow.step_preflight"), + patch("orchestrator.workflow.step_mint"), + patch("orchestrator.workflow.step_escrow_bst"), + patch( + "orchestrator.workflow.step_wait_for_bootstrap_pkg", + side_effect=lambda *a: calls.append("pkg"), + ), + patch("orchestrator.workflow.step_screencapture_grant"), + patch( + "orchestrator.workflow.step_wait_for_sentinel", + side_effect=lambda *a: calls.append("sentinel"), + ), + ): workflow.provision(HOST) assert calls == ["pkg", "sentinel"] def test_provision_no_wait_skips_the_pkg_gate_too(): # Nothing is going to wait on the sentinel, so there's nothing to protect. - with patch("orchestrator.workflow.step_preflight"), \ - patch("orchestrator.workflow.step_mint"), \ - patch("orchestrator.workflow.step_escrow_bst"), \ - patch("orchestrator.workflow.step_wait_for_bootstrap_pkg") as pkg: + with ( + patch("orchestrator.workflow.step_preflight"), + patch("orchestrator.workflow.step_mint"), + patch("orchestrator.workflow.step_escrow_bst"), + patch("orchestrator.workflow.step_screencapture_grant"), + patch("orchestrator.workflow.step_wait_for_bootstrap_pkg") as pkg, + ): workflow.provision(HOST, wait=False) pkg.assert_not_called() @@ -284,10 +364,14 @@ def test_candidate_pools_rejects_an_unmapped_role(): def test_quarantine_on_register_waits_then_quarantines_in_the_discovered_pool(): # Registration trails the sentinel, so the first few polls legitimately find nothing. seen = [None, None, POOL] - with patch("orchestrator.workflow.tc_credentials", return_value=("cid", "tok")), \ - patch("orchestrator.workflow.taskcluster.find_registered_pool", side_effect=seen), \ - patch("orchestrator.workflow.get_settings") as settings, \ - patch("orchestrator.workflow.step_quarantine") as quarantine: + with ( + patch("orchestrator.workflow.tc_credentials", return_value=("cid", "tok")), + patch( + "orchestrator.workflow.taskcluster.find_registered_pool", side_effect=seen + ), + patch("orchestrator.workflow.get_settings") as settings, + patch("orchestrator.workflow.step_quarantine") as quarantine, + ): settings.return_value.quarantine_on_register_poll_seconds = 0 settings.return_value.quarantine_on_register_max_wait_seconds = 60 ctx = _ctx() @@ -301,9 +385,11 @@ def test_quarantine_on_register_waits_then_quarantines_in_the_discovered_pool(): def test_quarantine_on_register_fails_closed_without_tc_credentials(): # Refuse up front rather than spend the bootstrap window discovering we can't quarantine. - with patch("orchestrator.workflow.tc_credentials", return_value=("", "")), \ - patch("orchestrator.workflow.taskcluster.find_registered_pool") as find, \ - patch("orchestrator.workflow.step_quarantine") as quarantine: + with ( + patch("orchestrator.workflow.tc_credentials", return_value=("", "")), + patch("orchestrator.workflow.taskcluster.find_registered_pool") as find, + patch("orchestrator.workflow.step_quarantine") as quarantine, + ): with pytest.raises(ReprovisionError, match="needs Taskcluster credentials"): workflow.step_quarantine_on_register(_ctx()) find.assert_not_called() @@ -311,10 +397,14 @@ def test_quarantine_on_register_fails_closed_without_tc_credentials(): def test_quarantine_on_register_raises_if_the_worker_never_appears(): - with patch("orchestrator.workflow.tc_credentials", return_value=("cid", "tok")), \ - patch("orchestrator.workflow.taskcluster.find_registered_pool", return_value=None), \ - patch("orchestrator.workflow.get_settings") as settings, \ - patch("orchestrator.workflow.step_quarantine") as quarantine: + with ( + patch("orchestrator.workflow.tc_credentials", return_value=("cid", "tok")), + patch( + "orchestrator.workflow.taskcluster.find_registered_pool", return_value=None + ), + patch("orchestrator.workflow.get_settings") as settings, + patch("orchestrator.workflow.step_quarantine") as quarantine, + ): settings.return_value.quarantine_on_register_poll_seconds = 0 settings.return_value.quarantine_on_register_max_wait_seconds = 0 with pytest.raises(ReprovisionError, match="never registered"): @@ -324,16 +414,22 @@ def test_quarantine_on_register_raises_if_the_worker_never_appears(): def test_provision_quarantines_after_the_sentinel(): calls: list[str] = [] - with patch("orchestrator.workflow.tc_credentials", return_value=("cid", "tok")), \ - patch("orchestrator.workflow.step_preflight"), \ - patch("orchestrator.workflow.step_mint"), \ - patch("orchestrator.workflow.step_escrow_bst"), \ - patch("orchestrator.workflow.step_wait_for_bootstrap_pkg"), \ - patch("orchestrator.workflow.step_wait_for_sentinel", side_effect=lambda *a: calls.append("sentinel")), \ - patch( - "orchestrator.workflow.step_quarantine_on_register", - side_effect=lambda *a: calls.append("quarantine"), - ): + with ( + patch("orchestrator.workflow.tc_credentials", return_value=("cid", "tok")), + patch("orchestrator.workflow.step_preflight"), + patch("orchestrator.workflow.step_mint"), + patch("orchestrator.workflow.step_escrow_bst"), + patch("orchestrator.workflow.step_wait_for_bootstrap_pkg"), + patch( + "orchestrator.workflow.step_wait_for_sentinel", + side_effect=lambda *a: calls.append("sentinel"), + ), + patch("orchestrator.workflow.step_screencapture_grant"), + patch( + "orchestrator.workflow.step_quarantine_on_register", + side_effect=lambda *a: calls.append("quarantine"), + ), + ): workflow.provision(HOST, quarantine_on_register=True) # Order matters: the worker only registers after the bootstrap finishes. @@ -351,9 +447,11 @@ def test_provision_rejects_quarantine_on_register_with_no_wait(): def test_provision_checks_tc_credentials_before_touching_the_host(): # The flag's whole value is that the host doesn't take work; finding out we can't # quarantine only after a 40-minute bootstrap defeats it. - with patch("orchestrator.workflow.tc_credentials", return_value=("", "")), \ - patch("orchestrator.workflow.step_preflight") as pre, \ - patch("orchestrator.workflow.step_mint") as mint: + with ( + patch("orchestrator.workflow.tc_credentials", return_value=("", "")), + patch("orchestrator.workflow.step_preflight") as pre, + patch("orchestrator.workflow.step_mint") as mint, + ): with pytest.raises(ReprovisionError, match="fails now rather than after"): workflow.provision(HOST, quarantine_on_register=True) pre.assert_not_called() @@ -361,26 +459,35 @@ def test_provision_checks_tc_credentials_before_touching_the_host(): def test_provision_does_not_quarantine_by_default(): - with patch("orchestrator.workflow.step_preflight"), \ - patch("orchestrator.workflow.step_mint"), \ - patch("orchestrator.workflow.step_escrow_bst"), \ - patch("orchestrator.workflow.step_wait_for_bootstrap_pkg"), \ - patch("orchestrator.workflow.step_wait_for_sentinel"), \ - patch("orchestrator.workflow.step_quarantine_on_register") as quarantine, \ - patch("orchestrator.workflow.tc_credentials") as creds: + with ( + patch("orchestrator.workflow.step_preflight"), + patch("orchestrator.workflow.step_mint"), + patch("orchestrator.workflow.step_escrow_bst"), + patch("orchestrator.workflow.step_wait_for_bootstrap_pkg"), + patch("orchestrator.workflow.step_wait_for_sentinel"), + patch("orchestrator.workflow.step_screencapture_grant"), + patch("orchestrator.workflow.step_quarantine_on_register") as quarantine, + patch("orchestrator.workflow.tc_credentials") as creds, + ): workflow.provision(HOST) quarantine.assert_not_called() creds.assert_not_called() # and no credential fetch when the flag is off def test_provision_passes_the_gate_options_through(): - with patch("orchestrator.workflow.step_preflight") as pre, \ - patch("orchestrator.workflow.step_mint"), \ - patch("orchestrator.workflow.step_escrow_bst"), \ - patch("orchestrator.workflow.step_wait_for_bootstrap_pkg"), \ - patch("orchestrator.workflow.step_wait_for_sentinel"): + with ( + patch("orchestrator.workflow.step_preflight") as pre, + patch("orchestrator.workflow.step_mint"), + patch("orchestrator.workflow.step_escrow_bst"), + patch("orchestrator.workflow.step_wait_for_bootstrap_pkg"), + patch("orchestrator.workflow.step_screencapture_grant"), + patch("orchestrator.workflow.step_wait_for_sentinel"), + ): workflow.provision(HOST, expected_os="15.6", require_sip_disabled=False) - assert pre.call_args.kwargs == {"expected_os": "15.6", "require_sip_disabled": False} + assert pre.call_args.kwargs == { + "expected_os": "15.6", + "require_sip_disabled": False, + } # --- os-update --- @@ -415,12 +522,14 @@ def test_os_upgrade_script_keeps_the_placeholder_guard(): def test_os_update_is_a_noop_when_already_at_target(): - with patch("orchestrator.workflow.ssh.wait_for_sshd"), \ - patch("orchestrator.workflow.ssh.run", return_value=_cp(b"15.3")) as run, \ - patch("orchestrator.workflow.ssh.write_file_as_root") as write: + with ( + patch("orchestrator.workflow.ssh.wait_for_sshd"), + patch("orchestrator.workflow.ssh.run", return_value=_cp(b"15.3")) as run, + patch("orchestrator.workflow.ssh.write_file_as_root") as write, + ): workflow.step_os_update(_ctx(), expected_os="15.3") - write.assert_not_called() # nothing staged - assert run.call_count == 1 # just the version probe + write.assert_not_called() # nothing staged + assert run.call_count == 1 # just the version probe def test_os_update_stages_and_launches(): @@ -434,11 +543,13 @@ def _run(_fqdn, command, **_kw): return _cp(b"[INFO] downloading http://releng-pxe1...") return _cp(b"launched") - with patch("orchestrator.workflow.ssh.wait_for_sshd"), \ - patch("orchestrator.workflow.ssh.run", side_effect=_run), \ - patch("orchestrator.workflow.ssh.write_file_as_root") as write, \ - patch("orchestrator.workflow.ssh_admin_password", return_value="pw"), \ - patch("orchestrator.workflow.time.sleep"): + with ( + patch("orchestrator.workflow.ssh.wait_for_sshd"), + patch("orchestrator.workflow.ssh.run", side_effect=_run), + patch("orchestrator.workflow.ssh.write_file_as_root") as write, + patch("orchestrator.workflow.ssh_admin_password", return_value="pw"), + patch("orchestrator.workflow.time.sleep"), + ): workflow.step_os_update(_ctx(), expected_os="15.3") # Staged root-only, at a root-only path. @@ -456,14 +567,18 @@ def _run(_fqdn, command, **_kw): if "sw_vers" in command: return _cp(b"15.1") if "tail" in command: - return _cp(b"[ERROR] admin holds no SecureToken, so it is not a volume owner") + return _cp( + b"[ERROR] admin holds no SecureToken, so it is not a volume owner" + ) return _cp(b"launched") - with patch("orchestrator.workflow.ssh.wait_for_sshd"), \ - patch("orchestrator.workflow.ssh.run", side_effect=_run), \ - patch("orchestrator.workflow.ssh.write_file_as_root"), \ - patch("orchestrator.workflow.ssh_admin_password", return_value="pw"), \ - patch("orchestrator.workflow.time.sleep"): + with ( + patch("orchestrator.workflow.ssh.wait_for_sshd"), + patch("orchestrator.workflow.ssh.run", side_effect=_run), + patch("orchestrator.workflow.ssh.write_file_as_root"), + patch("orchestrator.workflow.ssh_admin_password", return_value="pw"), + patch("orchestrator.workflow.time.sleep"), + ): with pytest.raises(NotReadyError, match="SecureToken"): workflow.step_os_update(_ctx(), expected_os="15.3")