From e3ccaf49f9d13e65f325bda0116574e26e3ba4c8 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Thu, 16 Jul 2026 16:24:50 -0700 Subject: [PATCH 01/23] feat: add macOS (darwin) support to the worker agent installer install-deadline-worker rejected every platform except Linux and Windows, so macOS hosts could not be configured as workers in a customer-managed fleet even though the CMF fleet OS enum already accepts MACOS. - __init__.py: allow 'darwin' through the platform gate; add a 'darwin' entry to INSTALLER_PATH so install() dispatches through the existing sudo path to a new install_darwin.sh; reject --vfs-install-path on macOS (VFS is Linux-only). - install_darwin.sh (new): macOS port of install.sh. Creates the agent user and job group via Directory Services (dscl/dseditgroup) instead of useradd/groupadd, preserving the jobRunAsUser isolation model; provisions the same directories/permissions; writes worker.toml via the same config module; installs a launchd LaunchDaemon instead of a systemd unit. Uses a portable while-loop for argument parsing (macOS ships BSD getopt, which lacks --longoptions). - settings.py + arg-parser default: add DEFAULT_MACOS_SESSION_ROOT_DIR (/var/lib/deadline/sessions) and select it on darwin, because the macOS root volume is sealed read-only and /sessions cannot be created there. - Tests: cover the darwin dispatch path, --vfs-install-path rejection, and the platform-correct session-root default; drop darwin from the unsupported-platform test. Linux and Windows install paths are unchanged. Running a job as a jobRunAsUser on macOS additionally requires the companion openjd-sessions changes. Validated end-to-end on macOS 26.5 (arm64) against a live customer-managed fleet: install, worker registration, running a job as the jobRunAsUser, and cancellation. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- src/deadline_worker_agent/config/settings.py | 12 +- .../installer/__init__.py | 16 +- .../installer/install_darwin.sh | 638 ++++++++++++++++++ test/unit/config/test_settings.py | 11 +- test/unit/install/test_install.py | 100 ++- 5 files changed, 770 insertions(+), 7 deletions(-) create mode 100755 src/deadline_worker_agent/installer/install_darwin.sh diff --git a/src/deadline_worker_agent/config/settings.py b/src/deadline_worker_agent/config/settings.py index 7d16dada..478b960c 100644 --- a/src/deadline_worker_agent/config/settings.py +++ b/src/deadline_worker_agent/config/settings.py @@ -13,6 +13,7 @@ from .config_file import ConfigFile import os +import sys # Default path for the worker's logs. @@ -28,6 +29,9 @@ ) DEFAULT_POSIX_SESSION_ROOT_DIR = Path("/sessions") +# macOS seals the root volume read-only (macOS 10.15+), so a top-level directory like +# /sessions cannot be created. Use a path under /var (writable) instead. +DEFAULT_MACOS_SESSION_ROOT_DIR = Path("/var/lib/deadline/sessions") DEFAULT_WINDOWS_SESSION_ROOT_DIR: Path = ( Path(os.getenv("PROGRAMDATA", "C:\\ProgramData")) / "Amazon" / "OpenJD" ) @@ -129,7 +133,13 @@ class WorkerSettings(BaseSettings): session_runtime: SessionRuntimeKind = SessionRuntimeKind.PYTHON telemetry_opt_out: bool = False session_root_dir: Path = ( - DEFAULT_WINDOWS_SESSION_ROOT_DIR if os.name == "nt" else DEFAULT_POSIX_SESSION_ROOT_DIR + DEFAULT_WINDOWS_SESSION_ROOT_DIR + if os.name == "nt" + else ( + DEFAULT_MACOS_SESSION_ROOT_DIR + if sys.platform == "darwin" + else DEFAULT_POSIX_SESSION_ROOT_DIR + ) ) class Config: diff --git a/src/deadline_worker_agent/installer/__init__.py b/src/deadline_worker_agent/installer/__init__.py index 8d739e1a..b7c2eee5 100644 --- a/src/deadline_worker_agent/installer/__init__.py +++ b/src/deadline_worker_agent/installer/__init__.py @@ -11,6 +11,7 @@ import sysconfig from deadline_worker_agent.config.settings import ( + DEFAULT_MACOS_SESSION_ROOT_DIR, DEFAULT_POSIX_SESSION_ROOT_DIR, DEFAULT_WINDOWS_SESSION_ROOT_DIR, ) @@ -25,6 +26,7 @@ INSTALLER_PATH = { "linux": Path(__file__).parent / "install.sh", + "darwin": Path(__file__).parent / "install_darwin.sh", } @@ -70,7 +72,7 @@ def _get_ec2_region() -> Optional[str]: def install() -> None: """Installer entrypoint for the AWS Deadline Cloud Worker Agent""" - if sys.platform not in ["linux", "win32"]: + if sys.platform not in ["linux", "darwin", "win32"]: print(f"ERROR: Unsupported platform {sys.platform}") sys.exit(1) @@ -78,6 +80,12 @@ def install() -> None: args = arg_parser.parse_args(namespace=ParsedCommandLineArguments()) scripts_path = Path(sysconfig.get_path("scripts")) + # The Deadline Virtual File System (VFS) is not supported on macOS. Reject the option here + # so the error surfaces before we shell out to install_darwin.sh (which also rejects it). + if sys.platform == "darwin" and args.vfs_install_path: + print("ERROR: --vfs-install-path is not supported on macOS.") + sys.exit(1) + if args.region is None: args.region = _get_ec2_region() if args.region is None: @@ -275,7 +283,11 @@ def get_argument_parser() -> ArgumentParser: # pragma: no cover default=( str(DEFAULT_WINDOWS_SESSION_ROOT_DIR) if sys.platform == "win32" - else str(DEFAULT_POSIX_SESSION_ROOT_DIR) + else ( + str(DEFAULT_MACOS_SESSION_ROOT_DIR) + if sys.platform == "darwin" + else str(DEFAULT_POSIX_SESSION_ROOT_DIR) + ) ), # pragma: nocover ) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_darwin.sh new file mode 100755 index 00000000..17d25e99 --- /dev/null +++ b/src/deadline_worker_agent/installer/install_darwin.sh @@ -0,0 +1,638 @@ +#!/usr/bin/env bash + +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + +# +# AWS Deadline Cloud Worker Agent Installer (macOS / darwin) +# +# This is the macOS port of install.sh. It mirrors the Linux installer's flag surface and +# overall structure so the deadline_worker_agent.installer dispatcher can invoke it identically +# (same getopt arguments, including --python-interpreter-path and --scripts-path). +# +# The installer: +# 1. Creates a hidden system OS user for the worker agent if required (via Directory Services) +# 2. Creates an OS group for all job users if required (via Directory Services) +# 3. Provisions directories used by the worker agent at runtime (identical to Linux). +# 4. Creates an agent configuration file if required and installs an example configuration file. +# 5. Updates the configuration file with arguments passed to the installer. +# 6. Creates, enables, and (optionally) starts a launchd LaunchDaemon that runs the worker +# agent and restarts it upon failure. +# +# PORTING NOTES (macOS differs from Linux): +# * Users/groups: dscl/dseditgroup instead of useradd/groupadd/getent/usermod. +# * Service: launchd LaunchDaemon instead of a systemd unit. +# * Shutdown: /sbin/shutdown -h now instead of /usr/sbin/shutdown now. +# * VFS: NOT supported on macOS -- hard error if requested. +# * Directories, permission modes, worker.toml handling, and the python config call are +# kept IDENTICAL to the Linux installer (they are portable to macOS/BSD). + +set -euo pipefail + +SCRIPT_DIR=$(cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) + +# Defaults +default_wa_user=deadline-worker +default_job_group=deadline-job-users +farm_id="unset" +fleet_id="unset" +wa_user=$default_wa_user +confirm="" +region="unset" +scripts_path="unset" +worker_agent_program="deadline-worker-agent" +allow_shutdown="no" +disallow_instance_profile="no" +no_install_service="no" +start_service="no" +telemetry_opt_out="no" +warning_lines=() +vfs_install_path="unset" +python_interpreter_path="unset" +# macOS seals the root volume read-only (macOS 10.15+), so /sessions (a top-level directory) +# cannot be created. Default to a writable path under /var. The dispatcher normally passes +# --session-root-dir explicitly; this is the fallback for a direct script invocation. +session_root_dir="/var/lib/deadline/sessions" + +# macOS-specific constants +# NOTE: launchd label + plist path. Uses reverse-DNS label convention that launchd expects. +# UNVERIFIED: ensure no other daemon on the fleet image already uses this label. +launchd_label="com.amazon.deadline.worker-agent" +launchd_plist="/Library/LaunchDaemons/${launchd_label}.plist" +# NOTE: macOS has no `useradd -m`; we must create and own a home directory ourselves. +worker_agent_homedir="/var/lib/deadline-worker" + +usage() +{ + echo "Usage: install_darwin.sh --farm-id FARM_ID" + echo " --fleet-id FLEET_ID" + echo " --scripts-path SCRIPTS_PATH" + echo " --python-interpreter-path PYTHON_INTERPRETER_PATH" + echo " --region REGION" + echo " [--user USER]" + echo " [--group GROUP]" + echo " [-y]" + echo " [--disallow-instance-profile]" + echo " [--no-install-service]" + echo " [--allow-shutdown]" + echo " [--session-root-dir SESSION_ROOT_DIR]" + echo "" + echo "Arguments" + echo "---------" + echo " --farm-id FARM_ID" + echo " The AWS Deadline Cloud Farm ID that the Worker belongs to." + echo " --fleet-id FLEET_ID" + echo " The AWS Deadline Cloud Fleet ID that the Worker belongs to." + echo " --region REGION" + echo " The AWS region of the AWS Deadline Cloud farm." + echo " --user USER" + echo " A user name that the AWS Deadline Cloud Worker Agent will run as. Defaults to $default_wa_user." + echo " --group GROUP" + echo " A group name that the Worker Agent shares with the user(s) that Jobs will be running as." + echo " Do not use the primary/effective group of the Worker Agent user specified in --user as" + echo " this is not a secure configuration. Defaults to $default_job_group." + echo " --scripts-path SCRIPTS_PATH" + echo " An optional path to the directory that the Worker Agent is installed. This is used as the" + echo " program path when creating the launchd service for the Worker Agent." + echo " --python-interpreter-path" + echo " Path to the Python interpreter for the worker agent." + echo " --allow-shutdown" + echo " Dictates whether a sudoers rule is created/deleted allowing the worker agent the" + echo " ability to shutdown the host system." + echo " --no-install-service" + echo " Skips the worker agent launchd service installation." + echo " --telemetry-opt-out" + echo " Opts out of telemetry collection for the worker agent." + echo " --start" + echo " Starts the launchd service as part of the installation." + echo " -y" + echo " Skips a confirmation prompt before performing the installation." + echo " --vfs-install-path VFS_INSTALL_PATH" + echo " NOT SUPPORTED on macOS. Providing this option is an error." + echo " --disallow-instance-profile" + echo " Disallow running the worker agent with an EC2 instance profile." + echo " --session-root-dir SESSION_ROOT_DIR" + echo " The root directory under which the worker agent will create session directories." + + exit 2 +} + +banner() { + echo "===========================================================" + echo "| AWS Deadline Cloud Worker Agent Installer |" + echo "| (macOS / darwin) |" + echo "===========================================================" +} + +# --- macOS Directory Services helpers ------------------------------------------------- +# macOS has no /etc/passwd-backed `id`/`getent` semantics for lookups the way Linux does; +# we query the local Directory Services node ("." = /Local/Default) with dscl. + +user_exists() { + # `dscl . -read /Users/` exits non-zero if the record does not exist. + dscl . -read /Users/"$1" &> /dev/null +} + +group_exists() { + dscl . -read /Groups/"$1" &> /dev/null +} + +# Primary group NAME of a user (Linux `id -gn` equivalent). Resolves PrimaryGroupID -> group name. +user_primary_group_name() { + local u="$1" gid + gid=$(dscl . -read /Users/"$u" PrimaryGroupID 2>/dev/null | awk '{print $2}') + if [[ -n "${gid}" ]]; then + dscl . -search /Groups PrimaryGroupID "${gid}" 2>/dev/null | awk 'NR==1{print $1}' + fi +} + +# Allocate the lowest unused ID in [200,500) from a dscl attribute listing. +# NOTE: macOS reserves IDs < 500 for hidden/system accounts. Combined with IsHidden=1 this keeps +# the agent account out of the login window. +# UNVERIFIED: the free range on the target image; MDM-managed fleets may already occupy low IDs. +# This allocation is also not race-safe against concurrent account creation. +allocate_system_id() { + local kind="$1" # "Users" or "Groups" + local attr="$2" # "UniqueID" or "PrimaryGroupID" + local used candidate + used=$(dscl . -list /"${kind}" "${attr}" 2>/dev/null | awk '{print $2}' | sort -n) + for candidate in $(seq 499 -1 200); do + if ! grep -qx "${candidate}" <<< "${used}"; then + echo "${candidate}" + return 0 + fi + done + echo "ERROR: Could not allocate a free system ${attr} in range [200,500)." >&2 + return 1 +} + +validate_deadline_id() { + prefix="$1" + input="$2" + [[ "${input}" =~ ^$prefix-[a-f0-9]{32}$ ]] +} + +# Validate arguments +# macOS ships only BSD getopt, which does NOT support `--longoptions` -- it silently treats the +# long option spec as positional args and drops every real flag, leaving values "unset". Rather +# than depend on GNU getopt being on PATH, we parse the long options directly with a portable +# while-loop. + +while [[ $# -gt 0 ]] +do + case "${1}" in + --farm-id) farm_id="$2" ; shift 2 ;; + --fleet-id) fleet_id="$2" ; shift 2 ;; + --region) region="$2" ; shift 2 ;; + --user) wa_user="$2" ; shift 2 ;; + --group) job_group="$2" ; shift 2 ;; + --scripts-path) scripts_path="$2" ; shift 2 ;; + --python-interpreter-path) python_interpreter_path="$2" ; shift 2 ;; + --vfs-install-path) vfs_install_path="$2" ; shift 2 ;; + --session-root-dir) session_root_dir="$2" ; shift 2 ;; + --allow-shutdown) allow_shutdown="yes" ; shift ;; + --disallow-instance-profile) disallow_instance_profile="yes" ; shift ;; + --no-install-service) no_install_service="yes" ; shift ;; + --telemetry-opt-out) telemetry_opt_out="yes" ; shift ;; + --start) start_service="yes" ; shift ;; + -y) confirm="-y" ; shift ;; + --) shift; break ;; + *) echo "ERROR: Unexpected option: $1" + usage ;; + esac +done + +# Require root (sudo), like the Linux installer. macOS account/plist operations need root. +if [[ "$(id -u)" -ne 0 ]]; then + echo "ERROR: This installer must be run as root (via sudo)." + exit 1 +fi + +# Validate required command-line arguments +if [[ "${farm_id}" == "unset" ]]; then + echo "ERROR: --farm-id not specified" + usage +elif ! validate_deadline_id farm "${farm_id}"; then + echo "ERROR: Not a valid value for --farm-id: ${farm_id}" + usage +fi + +if [[ "${fleet_id}" == "unset" ]]; then + echo "ERROR: --fleet-id not specified" + usage +elif ! validate_deadline_id fleet "${fleet_id}"; then + echo "ERROR: Not a valid value for --fleet-id: ${fleet_id}" + usage +fi + +if [[ "${scripts_path}" == "unset" ]]; then + echo "ERROR: --scripts-path is not specified" + usage +elif [[ ! -d "${scripts_path}" ]]; then + echo "ERROR: The specified scripts path is not found: \"${scripts_path}\"" + usage +else + set +e + worker_agent_program="${scripts_path}"/deadline-worker-agent + if [[ ! -f "${worker_agent_program}" ]]; then + echo "ERROR: Could not find deadline-worker-agent in scripts path: \"${worker_agent_program}\"" + exit 1 + fi + set -e +fi + +if [[ "${python_interpreter_path}" == "unset" ]]; then + echo "ERROR: --python-interpreter-path is not specified" + usage +elif [[ ! -f "${python_interpreter_path}" ]]; then + echo "ERROR: The Python interpreter path is not found: \"${python_interpreter_path}\"" + usage +fi + +if [[ "${region}" == "unset" ]]; then + echo "ERROR: --region not specified" + usage +fi +if [[ ! "${region}" =~ ^[a-z]+-[a-z]+-([a-z]+-)?[0-9]+$ ]]; then + echo "ERROR: Not a valid value for --region: ${region}" + usage +fi +if [[ ! -z "${wa_user}" ]] && [[ ! "${wa_user}" =~ ^[a-z_]([a-z0-9_-]{0,31}|[a-z0-9_-]{0,30}\$)$ ]]; then + echo "ERROR: Not a valid value for --user: ${wa_user}" + usage +fi + +# --- VFS is not supported on macOS: fail loudly ------------------------------------- +# DESIGN CHOICE: hard error (exit non-zero) rather than a silent warning, so that a mistaken +# VFS request surfaces immediately instead of producing a silently-degraded install. The +# dispatcher (__init__.py) also rejects --vfs-install-path on darwin before we ever get here; +# this is defense-in-depth for direct script invocation. +if [[ "${vfs_install_path}" != "unset" ]]; then + echo "ERROR: The Deadline Virtual File System (VFS) is not supported on macOS." + echo " Remove the --vfs-install-path option and re-run the installer." + exit 1 +fi + +# Determine the worker agent's PRIMARY group. +# CRITICAL SECURITY INVARIANT: the agent user's PRIMARY group must NOT be the job group. The job +# group is a SECONDARY membership only (see below). If the user already exists we read its current +# primary group; if we are creating the user we will give it a dedicated primary group named after +# the user (never the job group). +if user_exists "${wa_user}"; then + wa_group=$(user_primary_group_name "${wa_user}") + if [[ -z "${wa_group}" ]]; then + # Fall back to the user name if the primary group name could not be resolved. + wa_group="${wa_user}" + fi +else + # Newly created user -> its dedicated primary group has the same name as the user. + wa_group="${wa_user}" +fi + +# Default the job group if not provided via --group. +job_group=${job_group:-${default_job_group}} +if [[ ! -z "${job_group}" ]] && [[ ! "${job_group}" =~ ^[a-z_]([a-z0-9_-]{0,31}|[a-z0-9_-]{0,30}\$)$ ]]; then + echo "ERROR: Not a valid value for --group: ${job_group}" + usage +fi + +banner +echo + +# Output configuration +echo "Farm ID: ${farm_id}" +echo "Fleet ID: ${fleet_id}" +echo "Region: ${region}" +echo "Worker agent user: ${wa_user}" +echo "Worker agent group: ${wa_group}" +echo "Worker job group: ${job_group}" +echo "Scripts path: ${scripts_path}" +echo "Session root directory: ${session_root_dir}" +echo "Worker agent program path: ${worker_agent_program}" +echo "Allow worker agent shutdown: ${allow_shutdown}" +echo "Start launchd service: ${start_service}" +echo "Telemetry opt-out: ${telemetry_opt_out}" +echo "Disallow EC2 instance profile: ${disallow_instance_profile}" + +# Confirmation prompt +if [ -z "$confirm" ]; then + while : + do + read -p "Confirm install with the above settings (y/n):" confirm + if [[ "${confirm}" == "y" ]]; then + break + elif [[ "${confirm}" == "n" ]]; then + echo "Installation aborted" + exit 1 + else + echo "Not a valid choice (${confirm}). Please try again." + fi + done +fi + +echo "" + +# --- Create the worker agent user (hidden system account) --------------------------- +# DESIGN CHOICE: dscl (low-level) instead of `sysadminctl -addUser`. +# * sysadminctl auto-assigns a UID >= 501 (a normal, login-visible account) and offers no +# supported flag to force a hidden sub-500 system UID. +# * dscl lets us explicitly set UniqueID (<500), IsHidden, NFSHomeDirectory, UserShell, and +# PrimaryGroupID -- exactly what a headless daemon account needs, and it keeps the account +# out of the login window. +# Idempotent: only create when the record is absent. +if ! user_exists "${wa_user}"; then + echo "Creating worker agent user (${wa_user})" + + # First ensure the user's DEDICATED primary group exists (named after the user). + # This group -- NOT the job group -- becomes the user's PrimaryGroupID (security invariant). + if ! group_exists "${wa_user}"; then + wa_primary_gid=$(allocate_system_id Groups PrimaryGroupID) + dscl . -create /Groups/"${wa_user}" + dscl . -create /Groups/"${wa_user}" PrimaryGroupID "${wa_primary_gid}" + dscl . -create /Groups/"${wa_user}" RealName "${wa_user}" + else + wa_primary_gid=$(dscl . -read /Groups/"${wa_user}" PrimaryGroupID 2>/dev/null | awk '{print $2}') + fi + + wa_uid=$(allocate_system_id Users UniqueID) + dscl . -create /Users/"${wa_user}" + dscl . -create /Users/"${wa_user}" UniqueID "${wa_uid}" + dscl . -create /Users/"${wa_user}" PrimaryGroupID "${wa_primary_gid}" + dscl . -create /Users/"${wa_user}" NFSHomeDirectory "${worker_agent_homedir}" + # /usr/bin/false prevents interactive login (macOS equivalent of a nologin shell). + dscl . -create /Users/"${wa_user}" UserShell /usr/bin/false + dscl . -create /Users/"${wa_user}" RealName "AWS Deadline Cloud Worker Agent" + # IsHidden=1 keeps a UID<500 account out of the login window / user pickers. + dscl . -create /Users/"${wa_user}" IsHidden 1 + # Disable password auth entirely for this service account. + # UNVERIFIED: on some macOS versions a service account also needs `dscl . -passwd` or an + # AuthenticationAuthority reset; '*' matches the /etc/master.passwd disabled-account convention. + dscl . -create /Users/"${wa_user}" Password '*' + + wa_group="${wa_user}" + echo "Done creating worker agent user (${wa_user})" +else + echo "Worker agent user ${wa_user} already exists" +fi + +# --- Create the home directory (macOS does not auto-create it) ---------------------- +if [[ ! -d "${worker_agent_homedir}" ]]; then + echo "Creating worker agent home directory (${worker_agent_homedir})" + mkdir -p "${worker_agent_homedir}" +fi +chown "${wa_user}:${wa_group}" "${worker_agent_homedir}" +chmod 750 "${worker_agent_homedir}" + +# --- Create the job group ----------------------------------------------------------- +# dseditgroup allocates a system GID and creates the group record. Idempotent via group_exists. +if ! group_exists "${job_group}"; then + echo "Creating job group (${job_group})" + dseditgroup -o create "${job_group}" + echo "Done creating job group (${job_group})" +else + echo "Job group ${job_group} already exists" +fi + +# --- Enforce/verify the primary-group security invariant ---------------------------- +# The job group must be a SECONDARY membership only, never the agent user's primary group. +current_primary_group=$(user_primary_group_name "${wa_user}") +if [[ "${current_primary_group}" == "${job_group}" ]]; then + warning_lines+=( + "The job group (${job_group}) is the primary group of worker agent user (${wa_user}). This is not a secure setup." + "Consider re-installing and using a dedicated job group." + ) +else + # Add the agent user to the job group as a SECONDARY member (Linux `usermod -a -G` equivalent). + # `dseditgroup -o checkmember` reports current membership so this stays idempotent. + if ! dseditgroup -o checkmember -m "${wa_user}" "${job_group}" &> /dev/null; then + echo "Adding worker agent user (${wa_user}) to job group (${job_group})" + dseditgroup -o edit -a "${wa_user}" -t user "${job_group}" + echo "Done adding worker agent user (${wa_user}) to job group (${job_group})" + else + echo "Worker agent user (${wa_user}) is already in job group (${job_group})" + fi +fi + +# --- Sudoers configuration (--allow-shutdown) --------------------------------------- +# macOS shutdown binary lives at /sbin/shutdown (BSD shutdown). The Linux line used +# `/usr/sbin/shutdown now`; the BSD invocation is `/sbin/shutdown -h now` (-h = halt/power off). +# UNVERIFIED: confirm the worker agent actually invokes `/sbin/shutdown -h now` on macOS. The +# sudoers command MUST match the invoked argv EXACTLY or the NOPASSWD rule will not apply. +if [[ "${allow_shutdown}" == "yes" ]]; then + echo "Setting up sudoers shutdown rule at /etc/sudoers.d/deadline-worker-shutdown" + # /etc/sudoers.d exists and is included by default on macOS. + mkdir -p /etc/sudoers.d + cat > /etc/sudoers.d/deadline-worker-shutdown </dev/null; then + sed -i '' '/^\[telemetry\]/,/^\[/{s/^opt_out.*/opt_out = true/;}' "$worker_config" + if ! grep -q '^opt_out' "$worker_config"; then + sed -i '' '/^\[telemetry\]/a\ +opt_out = true +' "$worker_config" + fi + else + printf '\n[telemetry]\nopt_out = true\n' >> "$worker_config" + fi +fi + +# --- launchd LaunchDaemon (replaces the systemd unit) ------------------------------- +if ! [[ "${no_install_service}" == "yes" ]]; then + echo "Installing launchd LaunchDaemon to ${launchd_plist}" + + # Split the program path into ProgramArguments array elements. worker_agent_program is a bare + # path with no arguments, so this yields a single-element array. + # UNVERIFIED: assumes the program path contains no whitespace (true for standard scripts-path). + prog_args_xml="" + for token in ${worker_agent_program}; do + prog_args_xml+=" ${token}"$'\n' + done + + # RunAtLoad controls whether the daemon runs immediately when it is bootstrapped and on + # every boot. We gate it on --start so macOS matches the Linux installer's behavior: without + # --start the daemon is registered but not started now (Linux runs `systemctl enable` only), + # and with --start it starts immediately and on boot (Linux runs `systemctl start` too). + if [[ "${start_service}" == "yes" ]]; then + run_at_load="true" + else + run_at_load="false" + fi + + # NOTE ON MAPPINGS from the systemd unit: + # User= -> UserName + # WorkingDirectory= -> WorkingDirectory + # Environment=AWS_* -> EnvironmentVariables dict + # ExecStart= -> ProgramArguments (array) + # Restart=on-failure -> KeepAlive { SuccessfulExit = false } (restart only on failure) + # WantedBy=multi-user.target + (systemctl start when --start) -> RunAtLoad, gated on --start + # StandardOutput/Error=null -> StandardOutPath/StandardErrorPath = /dev/null + # AmbientCapabilities=CAP_KILL -> OMITTED. No macOS equivalent. The worker agent runtime + # already falls back to `pgrep` + `sudo kill` for process cleanup on non-Linux platforms, + # so no ambient capability is required here. + # VFS env vars (FUS3_PATH/DEADLINE_VFS_PATH) -> OMITTED. VFS is unsupported on macOS. + cat > "${launchd_plist}" < + + + + Label + ${launchd_label} + UserName + ${wa_user} + WorkingDirectory + ${worker_agent_homedir} + ProgramArguments + +${prog_args_xml} + EnvironmentVariables + + AWS_REGION + ${region} + AWS_DEFAULT_REGION + ${region} + + KeepAlive + + SuccessfulExit + + + RunAtLoad + <${run_at_load}/> + StandardOutPath + /dev/null + StandardErrorPath + /dev/null + + +EOF + + # launchd REJECTS a plist that is group- or other-writable, so mode is 644 (NOT 640 like the + # Linux systemd unit). Ownership must be root:wheel. + chown root:wheel "${launchd_plist}" + chmod 644 "${launchd_plist}" + echo "Done installing launchd LaunchDaemon" + + # Idempotent (re)load: bootout an already-loaded instance before bootstrap, otherwise + # `bootstrap` fails with "service already bootstrapped". + # UNVERIFIED: `bootout` returns non-zero if the service is not loaded; we tolerate that with `|| true`. + if launchctl print "system/${launchd_label}" &> /dev/null; then + echo "Existing LaunchDaemon detected; unloading before reload" + launchctl bootout system "${launchd_plist}" &> /dev/null || true + fi + + # bootstrap loads the daemon and enables start-on-boot; enable keeps it eligible to run. + # Whether it also starts *now* is governed by RunAtLoad (gated on --start above), matching + # the Linux installer where `systemctl enable` always runs but `systemctl start` is --start-only. + echo "Bootstrapping and enabling the LaunchDaemon" + launchctl bootstrap system "${launchd_plist}" + launchctl enable "system/${launchd_label}" + + if [[ "${start_service}" == "yes" ]]; then + # RunAtLoad=true means bootstrap already started it; kickstart -k guarantees an immediate + # (re)start even if bootstrap raced or the service was previously loaded. + echo "Starting the service" + launchctl kickstart -k "system/${launchd_label}" + echo "Done starting the service" + fi +fi + +echo "Done" + +# Output warning lines if any +if [ ${#warning_lines[@]} -gt 0 ]; then + echo + echo "!!!! WARNING !!!" + echo + for i in "${!warning_lines[@]}"; do + echo "${warning_lines[i]}" + done + echo +fi + +# UNVERIFIED (macOS platform integration, not scriptable here -- operator checklist): +# * TCC / Full Disk Access: a headless LaunchDaemon may be blocked by TCC from protected paths +# (Desktop/Documents/removable volumes) and cannot present the consent UI. Fleets likely need +# an MDM PPPC profile granting Full Disk Access to the ${worker_agent_program} binary. +# * Code signing / Gatekeeper: an unsigned/unnotarized agent binary may be quarantined. Ensure +# the binary is signed + notarized (or delivered without the com.apple.quarantine xattr). diff --git a/test/unit/config/test_settings.py b/test/unit/config/test_settings.py index 1f7e92eb..60b4cd3d 100644 --- a/test/unit/config/test_settings.py +++ b/test/unit/config/test_settings.py @@ -7,6 +7,7 @@ from typing import Any, Generator, NamedTuple, Type import pytest import os +import sys from pathlib import Path from pydantic.v1 import ConstrainedStr @@ -192,9 +193,13 @@ class FieldTestCaseParams(NamedTuple): expected_type=Path, expected_required=False, expected_default=( - Path("/sessions") - if os.name == "posix" - else Path(os.getenv("PROGRAMDATA", "C:\\ProgramData")) / "Amazon" / "OpenJD" + settings_mod.DEFAULT_WINDOWS_SESSION_ROOT_DIR + if os.name == "nt" + else ( + settings_mod.DEFAULT_MACOS_SESSION_ROOT_DIR + if sys.platform == "darwin" + else settings_mod.DEFAULT_POSIX_SESSION_ROOT_DIR + ) ), expected_default_factory_return_value=None, ), diff --git a/test/unit/install/test_install.py b/test/unit/install/test_install.py index 0bdf3351..d1a2e207 100644 --- a/test/unit/install/test_install.py +++ b/test/unit/install/test_install.py @@ -198,7 +198,6 @@ def test_runs_expected_subprocess( "emscripten", "wasi", "cygwin", - "darwin", ), ) def test_unsupported_platform_raises(platform: str, capsys: pytest.CaptureFixture) -> None: @@ -328,3 +327,102 @@ def test_fails_if_nonvalid_az_received( f"AWS region could not be detected, got unexpected availability zone from IMDS: {az}" in out ) + + +class TestMacOSInstall: + """Test cases for macOS (darwin) support in the installer dispatcher.""" + + @pytest.fixture(autouse=True) + def mock_darwin_platform(self) -> Generator[str, None, None]: + with patch.object(installer_mod.sys, "platform", new="darwin") as m: + yield m + + def test_installer_path_has_darwin_entry(self) -> None: + # THEN + assert ( + installer_mod.INSTALLER_PATH["darwin"] + == Path(installer_mod.__file__).parent / "install_darwin.sh" + ) + + def test_runs_expected_subprocess_on_darwin( + self, + mock_subprocess_run: MagicMock, + ) -> None: + # GIVEN + parsed_args = ParsedCommandLineArguments( + farm_id="farm-1", + fleet_id="fleet-1", + region="us-west-2", + user="wa-user", + group="job-group", + service_start=False, + confirmed=True, + allow_shutdown=False, + install_service=True, + telemetry_opt_out=False, + vfs_install_path=None, + disallow_instance_profile=False, + session_root_dir=Path("/var/lib/deadline/sessions"), + ) + + expected_cmd = [ + "sudo", + str(installer_mod.INSTALLER_PATH["darwin"]), + "--farm-id", + "farm-1", + "--fleet-id", + "fleet-1", + "--region", + "us-west-2", + "--user", + "wa-user", + "--scripts-path", + sysconfig.get_path("scripts"), + "--python-interpreter-path", + sys.executable, + "--session-root-dir", + # install() passes str(args.session_root_dir); a Path stringifies with the host + # separator, so build the expected value the same way rather than hard-coding it. + str(Path("/var/lib/deadline/sessions")), + "--group", + "job-group", + "-y", + ] + + with patch.object(installer_mod, "get_argument_parser") as mock_get_arg_parser: + arg_parser: MagicMock = mock_get_arg_parser.return_value + arg_parser.parse_args.return_value = parsed_args + + # WHEN + install() + + # THEN + mock_subprocess_run.assert_called_once_with(expected_cmd, check=True) + + def test_vfs_install_path_rejected_on_darwin( + self, + mock_subprocess_run: MagicMock, + capsys: pytest.CaptureFixture, + ) -> None: + """--vfs-install-path is unsupported on macOS and must be rejected before dispatch.""" + # GIVEN + parsed_args = ParsedCommandLineArguments( + farm_id="farm-1", + fleet_id="fleet-1", + region="us-west-2", + user="wa-user", + vfs_install_path="/opt/deadline_vfs", + ) + + with patch.object(installer_mod, "get_argument_parser") as mock_get_arg_parser: + arg_parser: MagicMock = mock_get_arg_parser.return_value + arg_parser.parse_args.return_value = parsed_args + + # WHEN / THEN + with pytest.raises(SystemExit) as raise_ctx: + install() + + assert raise_ctx.value.code == 1 + assert "--vfs-install-path is not supported on macOS." in capsys.readouterr().out + # AND the install script must not be invoked + mock_subprocess_run.assert_not_called() From c123104cb4aef7ff3582ad4687104b4540b3f7d8 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Fri, 17 Jul 2026 12:23:09 -0700 Subject: [PATCH 02/23] fix: gate immediate launchd start on --start, keep start-on-boot always The plist previously combined RunAtLoad=true with an unconditional `launchctl bootstrap`, so the agent started as soon as the installer ran even without --start. An intermediate fix gated RunAtLoad instead, but that broke start-on-boot for non---start installs (launchd loads /Library/LaunchDaemons plists at every boot, and with RunAtLoad=false plus no other trigger the daemon would never run at all). launchd has no separate start-now/start-on-boot controls the way systemd separates `systemctl start` from `systemctl enable`, so the two must be split differently: - The plist is always written boot-ready (RunAtLoad=true with KeepAlive/SuccessfulExit for restart-on-failure); installing it into /Library/LaunchDaemons is the systemctl-enable analog: launchd loads and starts it on the next boot. - `launchctl bootstrap` (load now, which per RunAtLoad also starts now) only runs with --start -- the systemctl-start analog. Caught by the macOS installer CI workflow asserting on the process table and launchd registration state after installs with and without --start. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_darwin.sh | 45 ++++++++++--------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_darwin.sh index 17d25e99..86346124 100755 --- a/src/deadline_worker_agent/installer/install_darwin.sh +++ b/src/deadline_worker_agent/installer/install_darwin.sh @@ -529,15 +529,16 @@ if ! [[ "${no_install_service}" == "yes" ]]; then prog_args_xml+=" ${token}"$'\n' done - # RunAtLoad controls whether the daemon runs immediately when it is bootstrapped and on - # every boot. We gate it on --start so macOS matches the Linux installer's behavior: without - # --start the daemon is registered but not started now (Linux runs `systemctl enable` only), - # and with --start it starts immediately and on boot (Linux runs `systemctl start` too). - if [[ "${start_service}" == "yes" ]]; then - run_at_load="true" - else - run_at_load="false" - fi + # launchd has no separate "start on boot" and "start on load" controls: RunAtLoad governs + # both, and loading happens at every boot for /Library/LaunchDaemons plists as well as at + # `launchctl bootstrap`. So to reproduce the Linux installer's semantics -- + # * `systemctl enable` always: start on (next) boot + # * `systemctl start` only with --start: start now + # -- the plist is always written boot-ready (RunAtLoad=true plus KeepAlive/SuccessfulExit, + # the systemd Restart=on-failure analog; note KeepAlive implies a load-time start per + # launchd.plist(5), which is fine because we want every load to start the daemon), and it + # is the `launchctl bootstrap` (load-now) that is gated on --start below. Installing the + # plist into /Library/LaunchDaemons is itself the boot-time registration. # NOTE ON MAPPINGS from the systemd unit: # User= -> UserName @@ -578,7 +579,7 @@ ${prog_args_xml} RunAtLoad - <${run_at_load}/> + StandardOutPath /dev/null StandardErrorPath @@ -593,27 +594,27 @@ EOF chmod 644 "${launchd_plist}" echo "Done installing launchd LaunchDaemon" - # Idempotent (re)load: bootout an already-loaded instance before bootstrap, otherwise - # `bootstrap` fails with "service already bootstrapped". + # Idempotent (re)load: bootout an already-loaded instance so a re-install picks up the new + # plist. Without --start we leave the service unloaded -- the plist in /Library/LaunchDaemons + # makes launchd load (and, per RunAtLoad, start) it on the next boot, which is exactly the + # Linux `systemctl enable`-without-`systemctl start` behavior. # UNVERIFIED: `bootout` returns non-zero if the service is not loaded; we tolerate that with `|| true`. if launchctl print "system/${launchd_label}" &> /dev/null; then - echo "Existing LaunchDaemon detected; unloading before reload" + echo "Existing LaunchDaemon detected; unloading" launchctl bootout system "${launchd_plist}" &> /dev/null || true fi - - # bootstrap loads the daemon and enables start-on-boot; enable keeps it eligible to run. - # Whether it also starts *now* is governed by RunAtLoad (gated on --start above), matching - # the Linux installer where `systemctl enable` always runs but `systemctl start` is --start-only. - echo "Bootstrapping and enabling the LaunchDaemon" - launchctl bootstrap system "${launchd_plist}" launchctl enable "system/${launchd_label}" if [[ "${start_service}" == "yes" ]]; then - # RunAtLoad=true means bootstrap already started it; kickstart -k guarantees an immediate - # (re)start even if bootstrap raced or the service was previously loaded. - echo "Starting the service" + # Load now; RunAtLoad=true makes bootstrap start the daemon immediately (the Linux + # `systemctl start` analog). kickstart -k guarantees an immediate (re)start even if + # bootstrap raced. + echo "Bootstrapping and starting the LaunchDaemon" + launchctl bootstrap system "${launchd_plist}" launchctl kickstart -k "system/${launchd_label}" echo "Done starting the service" + else + echo "LaunchDaemon installed; it will start on the next boot (use --start to start it now)" fi fi From 06c03e8c39ccf328367dcc2dd56de7131b350a0c Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Fri, 17 Jul 2026 12:23:20 -0700 Subject: [PATCH 03/23] test: add install_darwin.sh integration tests, run them on macOS in CI Adds test/integ/macos/test_installer.py, following the pattern of the Windows installer integration tests: pytest tests that run the real installer (via sudo) and assert the invariants it must establish: - agent user hidden with no interactive shell; its PRIMARY group is a dedicated per-user group and the job group is secondary-only (the credential-isolation boundary) - credentials dir 700 and denied to unprivileged users, config dir/worker.toml 750/640 root-owned and well-formed TOML with the configured farm/fleet ids, session root under /var, logs 750 - LaunchDaemon plist 644 root:wheel, parses, runs as the agent user, boot-ready (RunAtLoad=true + KeepAlive/SuccessfulExit); without --start the service is not registered with launchd and no agent process runs; with --start it is registered (which also proves launchd accepts the plist), then booted out - sudoers file 440, validates with visudo, grants exactly '/sbin/shutdown -h now', and is revoked on a re-run without --allow-shutdown - --vfs-install-path is rejected before any system mutation - a second run is idempotent The tests mutate host state (users, groups, LaunchDaemon, sudoers) so they are gated behind RUN_INSTALLER_TESTS=true and skip everywhere else, including 'hatch run integ-test' on a developer Mac. The macos_installer_test.yml workflow sets the gate and runs them on macOS runners, with the agent installed from the checkout into a venv and never successfully starting (fake farm/fleet ids, no AWS access). Actions are hash-pinned per the repository's zizmor blanket policy. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .github/workflows/macos_installer_test.yml | 58 +++++ pyproject.toml | 1 + test/integ/conftest.py | 4 +- test/integ/macos/__init__.py | 1 + test/integ/macos/test_installer.py | 267 +++++++++++++++++++++ 5 files changed, 329 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/macos_installer_test.yml create mode 100644 test/integ/macos/__init__.py create mode 100644 test/integ/macos/test_installer.py diff --git a/.github/workflows/macos_installer_test.yml b/.github/workflows/macos_installer_test.yml new file mode 100644 index 00000000..8c450027 --- /dev/null +++ b/.github/workflows/macos_installer_test.yml @@ -0,0 +1,58 @@ +name: macOS Installer Test + +# Runs the install_darwin.sh integration tests (test/integ/macos/test_installer.py) +# on a macOS runner. The tests execute the installer for real (as root) and assert +# the security invariants it must establish: user/group isolation, file modes, the +# LaunchDaemon plist, and the sudoers rule. They mutate host state, so they are +# gated behind RUN_INSTALLER_TESTS=true and belong on throwaway CI runners only. +# No AWS access is needed: the farm/fleet ids are fakes and the agent never +# successfully starts. + +on: + workflow_dispatch: + pull_request: + branches: [ mainline, release ] + paths: + - 'src/deadline_worker_agent/installer/**' + - 'test/integ/macos/**' + - '.github/workflows/macos_installer_test.yml' + +jobs: + macos-installer: + name: Python ${{ matrix.python-version }} + runs-on: macos-latest + timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + python-version: ['3.11', '3.13'] + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - uses: actions/setup-python@ece7cb06caefa5fff74198d8649806c4678c61a1 # v6.3.0 + with: + python-version: ${{ matrix.python-version }} + + - name: Install the worker agent from this checkout + run: | + set -euxo pipefail + sudo mkdir -p /opt/wa-venv + sudo chown "$(id -un)" /opt/wa-venv + python -m venv /opt/wa-venv + /opt/wa-venv/bin/pip install --quiet . + # The installer resolves deadline-worker-agent in --scripts-path and + # validates --python-interpreter-path exists. + test -x /opt/wa-venv/bin/deadline-worker-agent + + - name: Install Hatch + run: pip install --upgrade hatch + + - name: Run integration tests + env: + RUN_INSTALLER_TESTS: "true" + WA_VENV_BIN: /opt/wa-venv/bin + # Runs the whole integ suite (the hatch script targets test/integ); the + # installer tests activate via RUN_INSTALLER_TESTS and rely on their + # in-file order (VFS rejection asserts a pristine system, install + # fixtures build on it), which is pytest's default ordering. + run: hatch run integ-test -v diff --git a/pyproject.toml b/pyproject.toml index 787707e8..f31eed08 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -140,6 +140,7 @@ ignore = [ # This causes imports to come after regular Python statements causing flake8 rule E402 to be flagged "src/deadline_worker_agent/**/*win*.py" = ["E402"] "test/**/*windows*.py" = ["E402"] +"test/integ/macos/*.py" = ["E402"] [tool.ruff.lint.isort] known-first-party = [ diff --git a/test/integ/conftest.py b/test/integ/conftest.py index 56224656..f701bd25 100644 --- a/test/integ/conftest.py +++ b/test/integ/conftest.py @@ -7,5 +7,5 @@ collect_ignore: list[str] = [] if sys.platform != "win32": collect_ignore.append("windows") -elif sys.platform != "linux": - collect_ignore.append("linux") +if sys.platform != "darwin": + collect_ignore.append("macos") diff --git a/test/integ/macos/__init__.py b/test/integ/macos/__init__.py new file mode 100644 index 00000000..8d929cc8 --- /dev/null +++ b/test/integ/macos/__init__.py @@ -0,0 +1 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py new file mode 100644 index 00000000..239668dc --- /dev/null +++ b/test/integ/macos/test_installer.py @@ -0,0 +1,267 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + +"""Integration tests for install_darwin.sh. + +These tests run the macOS installer for real (via sudo) and assert the +invariants it must establish. They mutate host state -- users, groups, +directories, a LaunchDaemon plist, and a sudoers file -- so they only run when +RUN_INSTALLER_TESTS=true (set by the macos_installer_test.yml workflow, whose +runners are throwaway VMs). The tests are ordered: installation happens in +module-scoped fixtures and later tests build on earlier runs' state. + +The agent is installed from the repository checkout into a venv. The service is +never left running: the farm/fleet ids are fakes, and the one test that loads +the service with --start boots it out again. +""" + +# This assertion short-circuits mypy from type checking this module on platforms other than macOS +# https://mypy.readthedocs.io/en/stable/common_issues.html#python-version-and-system-platform-checks +import sys + +assert sys.platform == "darwin" + +import os +import plistlib +import re +import subprocess +from pathlib import Path + +import pytest + +try: + from tomllib import load as load_toml +except ModuleNotFoundError: + from tomli import load as load_toml + +FARM_ID = "farm-aabbccddeeff11223344556677889900" +FLEET_ID = "fleet-00998877665544332211ffeeddccbbaa" +REGION = "us-west-2" +WA_USER = "deadline-worker" +JOB_GROUP = "deadline-job-users" +LAUNCHD_LABEL = "com.amazon.deadline.worker-agent" +PLIST_PATH = Path("/Library/LaunchDaemons") / f"{LAUNCHD_LABEL}.plist" +SUDOERS_PATH = Path("/etc/sudoers.d/deadline-worker-shutdown") + +REPO_ROOT = Path(__file__).parent.parent.parent.parent +INSTALLER = REPO_ROOT / "src" / "deadline_worker_agent" / "installer" / "install_darwin.sh" + +pytestmark = pytest.mark.skipif( + os.environ.get("RUN_INSTALLER_TESTS", "").lower() != "true", + reason=( + "Skipping installer integration tests: they mutate host state (users, groups, " + "LaunchDaemon, sudoers) and only run when RUN_INSTALLER_TESTS=true" + ), +) + + +def run_installer(*extra_args: str, check: bool = True) -> subprocess.CompletedProcess: + """Runs install_darwin.sh via sudo with the standard test arguments.""" + venv_bin = Path(os.environ["WA_VENV_BIN"]) + cmd = [ + "sudo", + "bash", + str(INSTALLER), + "--farm-id", + FARM_ID, + "--fleet-id", + FLEET_ID, + "--region", + REGION, + "--scripts-path", + str(venv_bin), + "--python-interpreter-path", + str(venv_bin / "python"), + "-y", + *extra_args, + ] + return subprocess.run(cmd, capture_output=True, text=True, check=check) + + +def sudo_output(*cmd: str) -> str: + return subprocess.run(["sudo", *cmd], capture_output=True, text=True, check=True).stdout.strip() + + +def dscl_read(path: str, key: str) -> str: + out = subprocess.run( + ["dscl", ".", "-read", path, key], capture_output=True, text=True, check=True + ).stdout + # Standard attributes print as "Key: value"; native ones as + # "dsAttrTypeNative:Key: value" -- take everything after the last colon. + return out.rsplit(":", 1)[1].strip() + + +def user_exists(user: str) -> bool: + return ( + subprocess.run(["dscl", ".", "-read", f"/Users/{user}"], capture_output=True).returncode + == 0 + ) + + +def service_is_registered() -> bool: + return ( + subprocess.run( + ["sudo", "launchctl", "print", f"system/{LAUNCHD_LABEL}"], capture_output=True + ).returncode + == 0 + ) + + +def agent_process_running() -> bool: + return ( + subprocess.run(["pgrep", "-f", "deadline-worker-agent"], capture_output=True).returncode + == 0 + ) + + +class TestVfsRejection: + """--vfs-install-path is unsupported on macOS; it must fail before any mutation. + + Runs first: it asserts that the agent user does not exist yet, which is only + meaningful before TestInstall's fixture has run the real installation. + """ + + def test_vfs_install_path_rejected_without_side_effects(self) -> None: + # GIVEN a system that has not had the installer run on it + assert not user_exists(WA_USER), "test ordering violated: installer already ran" + + # WHEN + result = run_installer("--vfs-install-path", "/opt/deadline_vfs", check=False) + + # THEN + assert result.returncode != 0 + # AND nothing was created + assert not user_exists(WA_USER) + assert not PLIST_PATH.exists() + + +@pytest.fixture(scope="module") +def installed() -> subprocess.CompletedProcess: + """Runs the installer (with --allow-shutdown, without --start) once for this module.""" + return run_installer("--allow-shutdown") + + +class TestInstall: + """Invariants established by a plain install (no --start).""" + + def test_agent_user_is_hidden_service_account(self, installed) -> None: + assert user_exists(WA_USER) + assert dscl_read(f"/Users/{WA_USER}", "IsHidden") == "1" + assert "/usr/bin/false" in dscl_read(f"/Users/{WA_USER}", "UserShell") + + def test_agent_primary_group_is_not_job_group(self, installed) -> None: + """SECURITY INVARIANT: the agent user's PRIMARY group is its dedicated + per-user group -- never the job group, which is secondary-only. If this + inverts, job-user processes could read agent credentials.""" + wa_gid = dscl_read(f"/Users/{WA_USER}", "PrimaryGroupID") + job_gid = dscl_read(f"/Groups/{JOB_GROUP}", "PrimaryGroupID") + assert wa_gid != job_gid + # The primary gid resolves to the agent's dedicated self-named group + assert dscl_read(f"/Groups/{WA_USER}", "PrimaryGroupID") == wa_gid + # Job group membership is secondary + groups = subprocess.run( + ["id", "-Gn", WA_USER], capture_output=True, text=True, check=True + ).stdout.split() + assert JOB_GROUP in groups + + @pytest.mark.parametrize( + ("path", "expected_mode", "expected_owner"), + [ + ("/var/lib/deadline/credentials", "700", WA_USER), + ("/etc/amazon/deadline", "750", "root"), + ("/etc/amazon/deadline/worker.toml", "640", "root"), + ("/var/log/amazon/deadline", "750", WA_USER), + ], + ) + def test_file_modes( + self, installed, path: str, expected_mode: str, expected_owner: str + ) -> None: + # sudo is required to stat inside these directories: the test user is in + # neither the agent group nor the job group, so unprivileged access is + # denied -- which is itself the isolation working (asserted below). + assert sudo_output("stat", "-f", "%Lp %Su", path) == f"{expected_mode} {expected_owner}" + + def test_session_root_is_under_var(self, installed) -> None: + # /var is writable; the sealed read-only root volume is not (macOS 10.15+) + assert ( + subprocess.run( + ["sudo", "test", "-d", "/var/lib/deadline/sessions"], capture_output=True + ).returncode + == 0 + ) + + def test_credentials_denied_to_unprivileged_user(self, installed) -> None: + with pytest.raises(PermissionError): + os.stat("/var/lib/deadline/credentials/anything") + + def test_worker_toml_contains_configuration(self, installed) -> None: + raw = sudo_output("cat", "/etc/amazon/deadline/worker.toml") + # Parse rather than grep so we validate the file is well-formed TOML too + import io + + config = load_toml(io.BytesIO(raw.encode())) + assert config["worker"]["farm_id"] == FARM_ID + assert config["worker"]["fleet_id"] == FLEET_ID + + def test_plist_is_boot_ready_and_root_owned(self, installed) -> None: + # launchd rejects group/other-writable daemon plists + assert sudo_output("stat", "-f", "%Lp %Su %Sg", str(PLIST_PATH)) == "644 root wheel" + plist = plistlib.loads(PLIST_PATH.read_bytes()) + assert plist["Label"] == LAUNCHD_LABEL + # Runs as the agent user, not root + assert plist["UserName"] == WA_USER + # Boot-ready: RunAtLoad=true starts it whenever launchd loads it (every + # boot -- the systemctl-enable analog), KeepAlive restarts it on failure + assert plist["RunAtLoad"] is True + assert plist["KeepAlive"] == {"SuccessfulExit": False} + + def test_service_not_loaded_without_start(self, installed) -> None: + """Without --start the installer must NOT load (bootstrap) the service: + not registered with launchd, and no agent process running.""" + assert not service_is_registered() + assert not agent_process_running() + + def test_sudoers_grants_exactly_shutdown(self, installed) -> None: + assert sudo_output("stat", "-f", "%Lp %Su", str(SUDOERS_PATH)) == "440 root" + # visudo validates the syntax + subprocess.run(["sudo", "visudo", "-cf", str(SUDOERS_PATH)], check=True) + content = sudo_output("cat", str(SUDOERS_PATH)) + assert re.search( + rf"^{WA_USER} ALL=\(root\) NOPASSWD: /sbin/shutdown -h now$", content, re.MULTILINE + ) + + +class TestReinstall: + """Behavior of running the installer again over an existing installation.""" + + def test_reinstall_is_idempotent(self, installed) -> None: + run_installer("--allow-shutdown") + # Spot-check that the security invariants survived the re-run + wa_gid = dscl_read(f"/Users/{WA_USER}", "PrimaryGroupID") + job_gid = dscl_read(f"/Groups/{JOB_GROUP}", "PrimaryGroupID") + assert wa_gid != job_gid + assert ( + sudo_output("stat", "-f", "%Lp %Su", "/var/lib/deadline/credentials") + == f"700 {WA_USER}" + ) + plistlib.loads(PLIST_PATH.read_bytes()) + subprocess.run(["sudo", "visudo", "-cf", str(SUDOERS_PATH)], check=True) + + def test_reinstall_without_allow_shutdown_revokes_sudoers(self, installed) -> None: + run_installer() + assert not SUDOERS_PATH.exists() + + def test_install_with_start_registers_service(self, installed) -> None: + """--start loads the service with launchd, which also proves launchd + accepts the generated plist beyond XML well-formedness. The agent + process itself cannot authenticate against the fake farm/fleet and may + be mid-restart when we look, so only registration is asserted.""" + run_installer("--start") + try: + assert service_is_registered() + finally: + # The crash-looping agent must not outlive the test + subprocess.run( + ["sudo", "launchctl", "bootout", f"system/{LAUNCHD_LABEL}"], + capture_output=True, + ) + assert not service_is_registered() From d1ef8393a26915a6d4dcd59f46ce6aeb40b5e3c0 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Mon, 20 Jul 2026 07:49:47 -0700 Subject: [PATCH 04/23] fix: restore a previously-loaded service on re-install and tolerate the bootout/bootstrap race Two related re-install problems in the launchd load logic: 1. A config-only re-run (no --start) over a loaded service booted the service out to pick up the new plist and never put it back, leaving the worker offline until the next reboot. On Linux the equivalent re-run leaves a running service running. Track whether the service was loaded before the bootout and re-bootstrap it afterward even without --start. A re-run over an unloaded service still leaves it unloaded. 2. launchctl bootout is asynchronous: bootstrap immediately after it can fail transiently while the old instance is still unloading, aborting the installer under set -e. Retry bootstrap for up to ten seconds, then run it unguarded once more so a persistent failure still surfaces its real error message. Both caught by review on the macOS installer PR; covered by new integration tests (reinstall-with---start race, config-only re-run restore, and unloaded-stays-unloaded). Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_darwin.sh | 34 ++++++++++---- test/integ/macos/test_installer.py | 47 +++++++++++++++++++ 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_darwin.sh index 86346124..ed1e960c 100755 --- a/src/deadline_worker_agent/installer/install_darwin.sh +++ b/src/deadline_worker_agent/installer/install_darwin.sh @@ -595,22 +595,40 @@ EOF echo "Done installing launchd LaunchDaemon" # Idempotent (re)load: bootout an already-loaded instance so a re-install picks up the new - # plist. Without --start we leave the service unloaded -- the plist in /Library/LaunchDaemons - # makes launchd load (and, per RunAtLoad, start) it on the next boot, which is exactly the - # Linux `systemctl enable`-without-`systemctl start` behavior. - # UNVERIFIED: `bootout` returns non-zero if the service is not loaded; we tolerate that with `|| true`. + # plist. Remember whether it was loaded: a config-only re-run (no --start) over a loaded + # service must put the service back afterward, matching Linux where a re-run without + # `systemctl start` leaves a running service running. + was_loaded="no" if launchctl print "system/${launchd_label}" &> /dev/null; then echo "Existing LaunchDaemon detected; unloading" + was_loaded="yes" + # bootout returns non-zero if the service is not loaded; tolerate the race. launchctl bootout system "${launchd_plist}" &> /dev/null || true fi launchctl enable "system/${launchd_label}" - if [[ "${start_service}" == "yes" ]]; then + if [[ "${start_service}" == "yes" ]] || [[ "${was_loaded}" == "yes" ]]; then # Load now; RunAtLoad=true makes bootstrap start the daemon immediately (the Linux - # `systemctl start` analog). kickstart -k guarantees an immediate (re)start even if - # bootstrap raced. + # `systemctl start` analog -- or, in the re-install case, the restore of the + # previously-loaded service). + # + # `bootout` above is asynchronous: launchd may still be unloading the old instance + # when we bootstrap, which fails transiently ("service already loaded" / EIO). Retry + # briefly rather than aborting the install on the race. echo "Bootstrapping and starting the LaunchDaemon" - launchctl bootstrap system "${launchd_plist}" + bootstrap_ok="no" + for _ in $(seq 1 10); do + if launchctl bootstrap system "${launchd_plist}" &> /dev/null; then + bootstrap_ok="yes" + break + fi + sleep 1 + done + if [[ "${bootstrap_ok}" != "yes" ]]; then + # Surface the real error for the operator. + launchctl bootstrap system "${launchd_plist}" + fi + # kickstart -k guarantees an immediate (re)start even if bootstrap raced. launchctl kickstart -k "system/${launchd_label}" echo "Done starting the service" else diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py index 239668dc..58974cdb 100644 --- a/test/integ/macos/test_installer.py +++ b/test/integ/macos/test_installer.py @@ -265,3 +265,50 @@ def test_install_with_start_registers_service(self, installed) -> None: capture_output=True, ) assert not service_is_registered() + + def test_reinstall_with_start_survives_bootout_race(self, installed) -> None: + """A re-install with --start over an already-loaded service must not + abort on the asynchronous bootout/bootstrap race: launchd may still be + unloading the old instance when the new bootstrap runs.""" + run_installer("--start") + try: + assert service_is_registered() + # Immediately re-install over the loaded (crash-looping) service + result = run_installer("--start", check=False) + assert result.returncode == 0, result.stdout + result.stderr + assert service_is_registered() + finally: + subprocess.run( + ["sudo", "launchctl", "bootout", f"system/{LAUNCHD_LABEL}"], + capture_output=True, + ) + assert not service_is_registered() + + def test_reinstall_without_start_restores_loaded_service(self, installed) -> None: + """A config-only re-run (no --start) over a LOADED service must put the + service back afterward -- matching Linux, where a re-run without + `systemctl start` leaves a running service running. Without this, a + config change (e.g. toggling --allow-shutdown) would silently take the + worker offline until the next reboot.""" + run_installer("--start") + try: + assert service_is_registered() + # Config-only re-run: no --start + run_installer() + assert service_is_registered(), ( + "re-install without --start left the previously-loaded service stopped" + ) + finally: + subprocess.run( + ["sudo", "launchctl", "bootout", f"system/{LAUNCHD_LABEL}"], + capture_output=True, + ) + assert not service_is_registered() + + def test_reinstall_without_start_over_unloaded_service_stays_unloaded(self, installed) -> None: + """The restore logic must not overreach: a re-run without --start over + an UNLOADED service must leave it unloaded (only boot starts it).""" + assert not service_is_registered(), "test precondition: service must be unloaded" + run_installer() + assert not service_is_registered() + assert not agent_process_running() From f27caceccabff756e4005fe37308a9fdbe1e934f Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Mon, 20 Jul 2026 07:49:48 -0700 Subject: [PATCH 05/23] fix: stop mypy from following imports into numpy type stubs deadline-cloud-test-fixtures >= 0.18.16 (2026-07-17) added a numpy dependency, and mypy reaches numpy's stubs through pytest's approx implementation when checking the test tree. numpy 2.5+ stubs use PEP 695 'type' statements, which mypy rejects with a syntax error while python_version is pinned to 3.10, failing 'hatch run lint' on every platform. numpy is not used by this package, so skip following imports into it (follow_imports_for_stubs is required for the setting to apply to .pyi files). Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- pyproject.toml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index f31eed08..5f1c292b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -124,6 +124,17 @@ module = [ "botocore.*" ] +# numpy is not used by this package; it arrives transitively (via +# deadline-cloud-test-fixtures >= 0.18.16) and mypy reaches its stubs through +# pytest's `approx` implementation. numpy 2.5+ type stubs use PEP 695 `type` +# statements, which mypy rejects while python_version is pinned below 3.12, so +# do not follow imports into it. follow_imports_for_stubs is required for +# follow_imports to apply to .pyi files like numpy/__init__.pyi. +[[tool.mypy.overrides]] +module = [ "numpy", "numpy.*" ] +follow_imports = "skip" +follow_imports_for_stubs = true + [tool.ruff] line-length = 100 From a1ac6303f16d3315b1d9f202234b1dc092cfb978 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:46:33 -0700 Subject: [PATCH 06/23] fix: emit ProgramArguments without word-splitting and XML-escape the program path The launchd plist's ProgramArguments array was built by word-splitting the agent program path, so a venv under a path containing spaces (e.g. /Users/My Name/venv) fractured into multiple argv elements and launchd could not exec the daemon. The path is a single argument, so emit exactly one element, XML-escaped so paths containing &, <, or > cannot corrupt the plist. Also resolves the installer's remaining UNVERIFIED markers: - the sudoers rule matches the agent's actual invocation (verified against startup/entrypoint.py:_host_shutdown -- sudo resolves shutdown to /sbin/shutdown via PATH and matches the rule's full path) - password-disable via Password '*' verified: with no AuthenticationAuthority attribute Directory Services rejects authentication outright (eDSAuthMethodNotSupported) - the system-ID allocator scans the live directory (dscl -list) so MDM-occupied IDs are skipped by construction - TCC/Gatekeeper notes reframed as an explicit operator checklist Integration tests: assert ProgramArguments is exactly one element, and add a spaced-path installer run asserting the plist survives intact. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_darwin.sh | 55 ++++++++++++------- test/integ/macos/test_installer.py | 44 +++++++++++++++ 2 files changed, 79 insertions(+), 20 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_darwin.sh index ed1e960c..5a1d1f20 100755 --- a/src/deadline_worker_agent/installer/install_darwin.sh +++ b/src/deadline_worker_agent/installer/install_darwin.sh @@ -54,8 +54,10 @@ python_interpreter_path="unset" session_root_dir="/var/lib/deadline/sessions" # macOS-specific constants -# NOTE: launchd label + plist path. Uses reverse-DNS label convention that launchd expects. -# UNVERIFIED: ensure no other daemon on the fleet image already uses this label. +# NOTE: launchd label + plist path. Uses the reverse-DNS label convention that launchd expects; +# the vendor-namespaced label makes collision with another daemon on a fleet image as unlikely +# as the systemd unit name collision on Linux. The installer's bootout-before-bootstrap reload +# path treats an existing service with this label as a prior install of this agent. launchd_label="com.amazon.deadline.worker-agent" launchd_plist="/Library/LaunchDaemons/${launchd_label}.plist" # NOTE: macOS has no `useradd -m`; we must create and own a home directory ourselves. @@ -145,11 +147,15 @@ user_primary_group_name() { fi } -# Allocate the lowest unused ID in [200,500) from a dscl attribute listing. +# Allocate the highest unused ID in [200,500) from a dscl attribute listing. # NOTE: macOS reserves IDs < 500 for hidden/system accounts. Combined with IsHidden=1 this keeps -# the agent account out of the login window. -# UNVERIFIED: the free range on the target image; MDM-managed fleets may already occupy low IDs. -# This allocation is also not race-safe against concurrent account creation. +# the agent account out of the login window. The allocator scans the live directory +# (dscl -list) at install time, so IDs already taken on the image -- including by +# MDM-provisioned accounts -- are skipped; searching downward from 499 also stays clear of +# Apple's own low-numbered system accounts. The install must fail if the range is +# exhausted rather than pick a UID >= 500 (which would appear in the login window). +# NOTE: not race-safe against concurrent account creation, matching the Linux installer's +# useradd behavior under the same (root, single-installer) assumption. allocate_system_id() { local kind="$1" # "Users" or "Groups" local attr="$2" # "UniqueID" or "PrimaryGroupID" @@ -363,9 +369,10 @@ if ! user_exists "${wa_user}"; then dscl . -create /Users/"${wa_user}" RealName "AWS Deadline Cloud Worker Agent" # IsHidden=1 keeps a UID<500 account out of the login window / user pickers. dscl . -create /Users/"${wa_user}" IsHidden 1 - # Disable password auth entirely for this service account. - # UNVERIFIED: on some macOS versions a service account also needs `dscl . -passwd` or an - # AuthenticationAuthority reset; '*' matches the /etc/master.passwd disabled-account convention. + # Disable password auth entirely for this service account. '*' matches the + # /etc/master.passwd disabled-account convention; because the account is created without an + # AuthenticationAuthority attribute, Directory Services rejects authentication attempts + # outright (eDSAuthMethodNotSupported) rather than comparing against a password. dscl . -create /Users/"${wa_user}" Password '*' wa_group="${wa_user}" @@ -415,8 +422,9 @@ fi # --- Sudoers configuration (--allow-shutdown) --------------------------------------- # macOS shutdown binary lives at /sbin/shutdown (BSD shutdown). The Linux line used # `/usr/sbin/shutdown now`; the BSD invocation is `/sbin/shutdown -h now` (-h = halt/power off). -# UNVERIFIED: confirm the worker agent actually invokes `/sbin/shutdown -h now` on macOS. The -# sudoers command MUST match the invoked argv EXACTLY or the NOPASSWD rule will not apply. +# The agent invokes `sudo shutdown -h now` on darwin (startup/entrypoint.py:_host_shutdown); +# sudo resolves `shutdown` to /sbin/shutdown via PATH and matches this rule by full path. +# The sudoers command MUST continue to match that argv exactly for the NOPASSWD rule to apply. if [[ "${allow_shutdown}" == "yes" ]]; then echo "Setting up sudoers shutdown rule at /etc/sudoers.d/deadline-worker-shutdown" # /etc/sudoers.d exists and is included by default on macOS. @@ -521,13 +529,18 @@ fi if ! [[ "${no_install_service}" == "yes" ]]; then echo "Installing launchd LaunchDaemon to ${launchd_plist}" - # Split the program path into ProgramArguments array elements. worker_agent_program is a bare - # path with no arguments, so this yields a single-element array. - # UNVERIFIED: assumes the program path contains no whitespace (true for standard scripts-path). - prog_args_xml="" - for token in ${worker_agent_program}; do - prog_args_xml+=" ${token}"$'\n' - done + # worker_agent_program is a single path with no embedded arguments, so ProgramArguments is a + # single-element array. Do NOT word-split it: the venv scripts path can contain spaces on + # macOS (e.g. a venv under "/Users/My Name/..."). XML-escape it so paths containing + # &, <, or > cannot corrupt the plist. + xml_escape() { + local s="$1" + s="${s//&/&}" + s="${s///>}" + printf '%s' "${s}" + } + prog_args_xml=" $(xml_escape "${worker_agent_program}")"$'\n' # launchd has no separate "start on boot" and "start on load" controls: RunAtLoad governs # both, and loading happens at every boot for /Library/LaunchDaemons plists as well as at @@ -649,9 +662,11 @@ if [ ${#warning_lines[@]} -gt 0 ]; then echo fi -# UNVERIFIED (macOS platform integration, not scriptable here -- operator checklist): +# OPERATOR NOTES (macOS platform integration; environment-dependent, not verifiable here): # * TCC / Full Disk Access: a headless LaunchDaemon may be blocked by TCC from protected paths # (Desktop/Documents/removable volumes) and cannot present the consent UI. Fleets likely need -# an MDM PPPC profile granting Full Disk Access to the ${worker_agent_program} binary. +# an MDM PPPC profile granting Full Disk Access to the ${worker_agent_program} binary. Not an +# issue for the default session root (/var/lib/deadline/sessions), which is not TCC-protected. # * Code signing / Gatekeeper: an unsigned/unnotarized agent binary may be quarantined. Ensure # the binary is signed + notarized (or delivered without the com.apple.quarantine xattr). +# pip-installed console scripts (the standard install path) do not carry the quarantine xattr. diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py index 58974cdb..40625c35 100644 --- a/test/integ/macos/test_installer.py +++ b/test/integ/macos/test_installer.py @@ -213,6 +213,50 @@ def test_plist_is_boot_ready_and_root_owned(self, installed) -> None: # boot -- the systemctl-enable analog), KeepAlive restarts it on failure assert plist["RunAtLoad"] is True assert plist["KeepAlive"] == {"SuccessfulExit": False} + # ProgramArguments is exactly one element: the agent binary path. A + # word-split bug would fracture a path containing spaces into multiple + # argv elements (see test_plist_program_arguments_survive_spaced_path). + venv_bin = Path(os.environ["WA_VENV_BIN"]) + assert plist["ProgramArguments"] == [str(venv_bin / "deadline-worker-agent")] + + def test_plist_program_arguments_survive_spaced_path(self, installed, tmp_path) -> None: + """A scripts path containing a space (e.g. a venv under '/Users/My Name') + must produce a single, intact ProgramArguments element.""" + spaced_dir = tmp_path / "spaced dir" / "bin" + spaced_dir.mkdir(parents=True) + venv_bin = Path(os.environ["WA_VENV_BIN"]) + agent_program = spaced_dir / "deadline-worker-agent" + agent_program.symlink_to(venv_bin / "deadline-worker-agent") + # World-traversable so the installer's checks and launchd can read it + tmp_path.chmod(0o755) + (tmp_path / "spaced dir").chmod(0o755) + spaced_dir.chmod(0o755) + + result = subprocess.run( + [ + "sudo", + "bash", + str(INSTALLER), + "--farm-id", + FARM_ID, + "--fleet-id", + FLEET_ID, + "--region", + REGION, + "--scripts-path", + str(spaced_dir), + "--python-interpreter-path", + str(venv_bin / "python"), + "-y", + ], + capture_output=True, + text=True, + ) + assert result.returncode == 0, result.stdout + result.stderr + plist = plistlib.loads(PLIST_PATH.read_bytes()) + assert plist["ProgramArguments"] == [str(agent_program)] + # Restore the standard plist for subsequent tests + run_installer("--allow-shutdown") def test_service_not_loaded_without_start(self, installed) -> None: """Without --start the installer must NOT load (bootstrap) the service: From 82e394503c8614ccbec781c9ffe2df3d61fc839b Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:54:23 -0700 Subject: [PATCH 07/23] test: plumb OPERATING_SYSTEM=macos through the e2e suite Prepares the E2E suite to run against macOS workers so that a macOS leg only needs the CodeBuild/CloudFormation infrastructure and a deadline-cloud-test-fixtures release that can provision EC2 Mac instances; the suite-side plumbing is ready ahead of that. - conftest: the operating_system fixture accepts OPERATING_SYSTEM=macos (OperatingSystem(name="MACOS")) - Windows-specific tests were gated with == "linux", which would have RUN them on a macOS worker; they are now gated != "windows" - Genuinely Linux-only tests (systemd management in worker_status and override_job_user, CAP_KILL/procfs in cap_kill) are now gated != "linux" with reasons updated to name the Linux-ism - POSIX-portable tests keep their == "windows" skip and now also run on macOS (credential-file isolation, queue-credential security, secure log writes: same /var/lib/deadline and /var/log/amazon paths) - Inline bash-vs-powershell ternaries and branches keyed == "linux" chose the POWERSHELL arm on macOS; all flipped to key off != "windows" / == "windows" so macOS takes the POSIX arm - The installer sudoers e2e check is parametrized per OS: BSD shutdown is '/sbin/shutdown -h now' vs Linux '/usr/sbin/shutdown now' - get_shutdown_on_stop_status_from_toml dispatches windows-first with a shared POSIX arm (worker.toml path is identical on linux/macos) The full suite collects cleanly under OPERATING_SYSTEM=linux, windows, and macos (72 tests each). Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- test/e2e/conftest.py | 8 +++- test/e2e/test_installer.py | 15 +++++-- test/e2e/test_job_attachments.py | 30 +++++++------- test/e2e/test_job_submissions.py | 64 +++++++++++++++--------------- test/e2e/test_override_job_user.py | 6 +-- test/e2e/test_worker_config.py | 4 +- test/e2e/test_worker_status.py | 6 +-- test/e2e/utils.py | 27 ++++++------- 8 files changed, 86 insertions(+), 74 deletions(-) diff --git a/test/e2e/conftest.py b/test/e2e/conftest.py index 3447f9dc..cd084f74 100644 --- a/test/e2e/conftest.py +++ b/test/e2e/conftest.py @@ -577,9 +577,15 @@ def operating_system() -> OperatingSystem: return OperatingSystem(name="AL2023") elif os_env_var == "windows": return OperatingSystem(name="WIN2022") + elif os_env_var == "macos": + # NOTE: requires a deadline-cloud-test-fixtures release whose + # OperatingSystem/worker fixtures accept a macOS platform (EC2 Mac + # dedicated hosts). The test-suite plumbing here is ready ahead of that. + return OperatingSystem(name="MACOS") else: assert False, ( - f'Expected OPERATING_SYSTEM env var to be "linux" or "windows", but got {os_env_var}' + f'Expected OPERATING_SYSTEM env var to be "linux", "windows", or "macos", ' + f"but got {os_env_var}" ) diff --git a/test/e2e/test_installer.py b/test/e2e/test_installer.py index 9f8b66ae..b1af7f3c 100644 --- a/test/e2e/test_installer.py +++ b/test/e2e/test_installer.py @@ -32,23 +32,30 @@ @pytest.mark.skipif( os.environ["OPERATING_SYSTEM"] == "windows", - reason="Linux specific test", + reason="POSIX specific test", ) class TestInstaller: def test_installer_shutdown_permission( self, session_worker: EC2InstanceWorker, ) -> None: + # The sudoers rule grants exactly the shutdown command the agent invokes, + # which differs by platform: BSD shutdown requires -h (halt). + expected_rule = ( + "^deadline-worker ALL=\\(root\\) NOPASSWD: /sbin/shutdown -h now$" + if os.environ["OPERATING_SYSTEM"] == "macos" + else "^deadline-worker ALL=\\(root\\) NOPASSWD: /usr/sbin/shutdown now$" + ) cmd_result = session_worker.send_command( - "egrep \ - '^deadline-worker ALL=\\(root\\) NOPASSWD: /usr/sbin/shutdown now$' \ + f"egrep \ + '{expected_rule}' \ /etc/sudoers.d/deadline-worker-shutdown" ) assert cmd_result.exit_code == 0, f"Shutdown WA permission do not exist: {cmd_result}" -@pytest.mark.skipif(os.environ["OPERATING_SYSTEM"] == "linux", reason="Windows specific tests") +@pytest.mark.skipif(os.environ["OPERATING_SYSTEM"] != "windows", reason="Windows specific tests") @pytest.mark.usefixtures("test_job") class TestWindowsInstaller: # Names for tests diff --git a/test/e2e/test_job_attachments.py b/test/e2e/test_job_attachments.py index ba673e65..c02c0706 100644 --- a/test/e2e/test_job_attachments.py +++ b/test/e2e/test_job_attachments.py @@ -95,10 +95,10 @@ def test_worker_job_attachment_storage_profile_path_mapping( assert worker_os is not None, ( "OPERATING_SYSTEM environment variable is required but was not provided" ) - if worker_os == "linux": - fleet_storage_profile_id = deadline_resources.fleet_storage_profile_id - else: + if worker_os == "windows": fleet_storage_profile_id = deadline_resources.windows_fleet_storage_profile_id + else: + fleet_storage_profile_id = deadline_resources.fleet_storage_profile_id queue_storage_profile_res = deadline_client.get_storage_profile( farmId=deadline_resources.farm.id, @@ -197,7 +197,7 @@ def test_worker_job_attachment_storage_profile_path_mapping( "script": { "actions": { "onRun": { - "command": "python3" if worker_os == "linux" else "python", + "command": "python" if worker_os == "windows" else "python3", "args": ["{{ Task.File.runScript }}"], }, }, @@ -358,7 +358,7 @@ def test_job_submission_many_small_files_download_does_not_cancel( "Small files download test completed successfully" ], } - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": [ @@ -484,7 +484,7 @@ def test_job_submission_many_small_files_download_does_not_cancel( [ ( "#!/usr/bin/env bash\n\n echo -n $(cat {{Param.DataDir}}/files/test_input_file){{Param.StringToAppend}} > {{Param.DataDir}}/output_file\n" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else '''set /p input=<"{{Param.DataDir}}\\files\\test_input_file"\n powershell -Command "echo ($env:input+\'{{Param.StringToAppend}}\') | Out-File -encoding utf8 {{Param.DataDir}}\\output_file -NoNewLine"''' ) ], @@ -793,7 +793,7 @@ def test_worker_job_attachments_dep_data_flow_linux( ) @pytest.mark.skipif( - os.environ["OPERATING_SYSTEM"] == "linux", + os.environ["OPERATING_SYSTEM"] != "windows", reason="Windows specific job bundle to test job attachments dependency data flow", ) def test_worker_job_attachments_dep_data_flow_windows( @@ -877,7 +877,7 @@ def test_worker_fails_job_attachment_sync_when_non_valid_queue_role( ] append_string_script = ( "#!/usr/bin/env bash\n\n echo -n $(cat {{Param.DataDir}}/files/test_input_file)hi > {{Param.DataDir}}/output_file\n" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else '''set /p input=<"{{Param.DataDir}}\\files\\test_input_file"\n powershell -Command "echo ($env:input+\'hi\') | Out-File -encoding utf8 {{Param.DataDir}}\\output_file -NoNewLine"''' ) @@ -1005,7 +1005,7 @@ def sync_input_job_attachments_failed(current_job: Job) -> bool: "done\n" "sha256_hash=$(echo -n \"$combined_contents\" | sha256sum | awk '{ print $1 }')\n" 'echo -n "$sha256_hash" > {{Param.DataDir}}/output_file.txt' - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else '$InputFolder = "{{Param.DataDir}}\\files"\n' '$OutputFile = "{{Param.DataDir}}\\output_file.txt"\n' '$combinedContent = ""\n' @@ -1094,7 +1094,7 @@ def test_worker_uses_job_attachment_sync( "actions": { "onRun": ( {"command": "{{ Task.File.runScript }}"} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": ["{{ Task.File.runScript }}"], @@ -1234,13 +1234,13 @@ def test_worker_uses_step_step_dependencies( append_string_script_step_one = ( "#!/usr/bin/env bash\n\n echo -n $(cat {{Param.DataDir}}/files/test_input_file)Hello > {{Param.DataDir}}/files/step_one_output\n" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else '''set /p input=<"{{Param.DataDir}}\\files\\test_input_file"\n powershell -Command "echo ($env:input+\'Hello\') | Out-File -encoding utf8 {{Param.DataDir}}\\files\\step_one_output -NoNewLine"''' ) append_string_script_step_two = ( "#!/usr/bin/env bash\n\n echo -n $(cat {{Param.DataDir}}/files/step_one_output)Hello > {{Param.DataDir}}/files/output_file\n" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else '''set /p input=<"{{Param.DataDir}}\\files\\step_one_output"\n powershell -Command "echo ($env:input+\'Hello\') | Out-File -encoding utf8 {{Param.DataDir}}\\files\\output_file -NoNewLine"''' ) @@ -1668,7 +1668,7 @@ def test_job_submission_asset_sync_behaviour_expected_without_errors( "python3 {{ Task.File.upload }} && python3 {{ Task.File.download }}", ], } - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": [ @@ -1871,7 +1871,7 @@ def test_worker_create_job_API_call_linux( ) @pytest.mark.skipif( - os.environ["OPERATING_SYSTEM"] == "linux", + os.environ["OPERATING_SYSTEM"] != "windows", reason="Windows specific job bundle to test create job API call", ) def test_worker_create_job_API_call_windows( @@ -2035,7 +2035,7 @@ def test_job_attachments_no_output_relative_directories( ), ], } - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "cmd", "args": [ diff --git a/test/e2e/test_job_submissions.py b/test/e2e/test_job_submissions.py index 9588f0ea..b74f5f33 100644 --- a/test/e2e/test_job_submissions.py +++ b/test/e2e/test_job_submissions.py @@ -347,7 +347,7 @@ def test_worker_writes_logs_to_disk_securely( { "onEnter": ( {"command": "echo", "args": ["PASS: Environment entered"]} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": ["Write-Output 'PASS: Environment entered'"], @@ -360,7 +360,7 @@ def test_worker_writes_logs_to_disk_securely( { "onRun": ( {"command": "echo", "args": ["PASS: Task ran"]} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else {"command": "powershell", "args": ["Write-Output 'PASS: Task ran'"]} ), }, @@ -375,14 +375,14 @@ def test_worker_writes_logs_to_disk_securely( { "onRun": ( {"command": "echo", "args": ["PASS: Task ran"]} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else {"command": "powershell", "args": ["Write-Output 'PASS: Task ran'"]} ), }, { "onEnter": ( {"command": "echo", "args": ["PASS: Environment entered"]} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": ["Write-Output 'PASS: Environment entered'"], @@ -501,12 +501,12 @@ def test_worker_fails_session_action_timeout( "onRun": { "command": ( "/bin/sleep" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else "powershell" ), "args": ( ["40"] - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else ["ping", "localhost", "-n", "40"] ), "timeout": 1, # Times out in 1 second @@ -571,12 +571,12 @@ def test_worker_fails_session_action_timeout( "onRun": { "command": ( "/bin/sleep" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else "powershell" ), "args": ( ["300"] - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else ["ping", "localhost", "-n", "300"] ), "cancelation": { @@ -588,7 +588,7 @@ def test_worker_fails_session_action_timeout( { "onEnter": ( {"command": "echo", "args": ["PASS: Environment entered"]} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": ["Write-Output 'PASS: Environment entered'"], @@ -601,7 +601,7 @@ def test_worker_fails_session_action_timeout( { "onRun": ( {"command": "echo", "args": ["PASS: Task ran"]} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else {"command": "powershell", "args": ["Write-Output 'PASS: Task ran'"]} ), }, @@ -609,12 +609,12 @@ def test_worker_fails_session_action_timeout( "onEnter": { "command": ( "/bin/sleep" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else "powershell" ), "args": ( ["300"] - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else ["ping", "localhost", "-n", "300"] ), "cancelation": { @@ -763,7 +763,7 @@ def test_worker_reports_canceled_session_actions_as_canceled( "bash\n\nsleep 300\n" "echo 'FAIL: Sleep completed without cancellation'\n" "exit 1\n" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else """Write-Output '--- STEP: Long sleep with cancel trap ---' Write-Output 'Sleeping 300s, waiting for cancellation' try @@ -804,7 +804,7 @@ def test_worker_reports_canceled_session_actions_as_canceled( "actions": { "onRun": ( {"command": "{{ Task.File.runScript }}"} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": ["{{ Task.File.runScript }}"], # type: ignore[dict-item] @@ -821,7 +821,7 @@ def test_worker_reports_canceled_session_actions_as_canceled( if expected_canceled_action == "taskRun" else ( "#!/usr/bin/env bash\necho 'PASS: Task ran'\n" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else "Write-Output 'PASS: Task ran'\n" ) ), @@ -843,7 +843,7 @@ def test_worker_reports_canceled_session_actions_as_canceled( "onEnter": ( ( {"command": "{{ Env.File.runScript }}"} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": ["{{ Env.File.runScript }}"], # type: ignore[dict-item] @@ -852,7 +852,7 @@ def test_worker_reports_canceled_session_actions_as_canceled( if expected_canceled_action == "envEnter" else ( {"command": "echo", "args": ["PASS: Environment entered"]} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": ["Write-Output 'PASS: Environment entered'"], @@ -864,7 +864,7 @@ def test_worker_reports_canceled_session_actions_as_canceled( "command": "echo", "args": ["Environment exit ran successfully"], } - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": [ @@ -883,7 +883,7 @@ def test_worker_reports_canceled_session_actions_as_canceled( if expected_canceled_action == "envEnter" else ( "#!/usr/bin/env bash\necho 'PASS: Environment entered'\n" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else "Write-Output 'PASS: Environment entered'\n" ) ), @@ -1251,12 +1251,12 @@ def test_worker_reports_never_attempted_tasks_if_task_is_canceled( "onRun": { "command": ( "/bin/sleep" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else "powershell" ), "args": ( ["1"] - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else ["ping", "localhost", "-n", "1"] ), }, @@ -1279,12 +1279,12 @@ def test_worker_reports_never_attempted_tasks_if_task_is_canceled( "onRun": { "command": ( "/bin/sleep" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else "powershell" ), "args": ( ["120"] - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else ["ping", "localhost", "-n", "120"] ), "cancelation": { @@ -1549,7 +1549,7 @@ def check_environment_action_statuses_are_expected() -> None: "--- STEP: Env 1 enter --- Entering environment_1 PASS: environment_1 entered" ], } - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": [ @@ -1575,7 +1575,7 @@ def check_environment_action_statuses_are_expected() -> None: "--- STEP: Env 1 enter --- Entering environment_1 PASS: environment_1 entered" ], } - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": [ @@ -1597,7 +1597,7 @@ def check_environment_action_statuses_are_expected() -> None: "--- STEP: Env 2 enter --- Entering environment_2 PASS: environment_2 entered" ], } - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": [ @@ -1619,7 +1619,7 @@ def check_environment_action_statuses_are_expected() -> None: "--- STEP: Env 3 enter --- Entering environment_3 PASS: environment_3 entered" ], } - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": [ @@ -1665,7 +1665,7 @@ def test_worker_run_with_number_of_environments( "--- STEP: Task run --- Running task PASS: Task completed" ], } - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": [ @@ -1726,7 +1726,7 @@ def test_worker_streams_logs_to_cloudwatch( "actions": { "onRun": ( {"command": "echo", "args": ["HelloWorld"]} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": ['"Hello"', "+", '"World"'], @@ -1801,7 +1801,7 @@ def test_worker_reports_task_progress_and_status_message( sleep 6 done """ - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else f""" $percent = 0 while ($percent -le 100) {{ @@ -1910,7 +1910,7 @@ def test_worker_enters_stopping_state_while_draining( #!/usr/bin/env bash sleep 600 """ - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else """ Start-Sleep -Seconds 600 """ @@ -1924,7 +1924,7 @@ def test_worker_enters_stopping_state_while_draining( run_script=sleep_script, ) - if os.environ["OPERATING_SYSTEM"] == "linux": + if os.environ["OPERATING_SYSTEM"] != "windows": cmd_result = function_worker.send_command("sudo systemctl stop deadline-worker") else: cmd_result = function_worker.send_command("sc.exe stop DeadlineWorker") diff --git a/test/e2e/test_override_job_user.py b/test/e2e/test_override_job_user.py index b3bd8802..b6d37d62 100644 --- a/test/e2e/test_override_job_user.py +++ b/test/e2e/test_override_job_user.py @@ -32,7 +32,7 @@ @pytest.mark.skipif( - os.environ["OPERATING_SYSTEM"] == "linux", + os.environ["OPERATING_SYSTEM"] != "windows", reason="Windows Specific Job User Override Tests.", ) class TestWindowsJobUserOverride: @@ -314,8 +314,8 @@ def test_env_var_user_override( @pytest.mark.skipif( - os.environ["OPERATING_SYSTEM"] == "windows", - reason="Linux specific Job User Override tests", + os.environ["OPERATING_SYSTEM"] != "linux", + reason="Linux (systemd) specific Job User Override tests", ) class TestLinuxJobUserOverride: @staticmethod diff --git a/test/e2e/test_worker_config.py b/test/e2e/test_worker_config.py index 385474af..76bf33f1 100644 --- a/test/e2e/test_worker_config.py +++ b/test/e2e/test_worker_config.py @@ -121,14 +121,14 @@ def test_worker_local_session_logs_can_be_turned_off( session_id: str = session["sessionId"] session_logs_file_path: str = ( os.path.join("/var/log/amazon/deadline", job.queue.id, f"{session_id}.log") - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else os.path.join( "C:/ProgramData/Amazon/Deadline/Logs", job.queue.id, f"{session_id}.log", ) ) - if os.environ["OPERATING_SYSTEM"] == "linux": + if os.environ["OPERATING_SYSTEM"] != "windows": # Linux worker check_log_exists_result = worker_with_local_session_logs_off.send_command( command=f'[ -e "{session_logs_file_path}" ]' diff --git a/test/e2e/test_worker_status.py b/test/e2e/test_worker_status.py index ba4be5da..4628f3ae 100644 --- a/test/e2e/test_worker_status.py +++ b/test/e2e/test_worker_status.py @@ -19,8 +19,8 @@ class TestWorkerStatus: @pytest.mark.skipif( - os.environ["OPERATING_SYSTEM"] == "windows", - reason="Linux specific test", + os.environ["OPERATING_SYSTEM"] != "linux", + reason="Linux (systemd) specific test", ) def test_linux_worker_restarts_process( self, @@ -110,7 +110,7 @@ def check_worker_processes_exist() -> None: check_worker_processes_exist() @pytest.mark.skipif( - os.environ["OPERATING_SYSTEM"] == "linux", + os.environ["OPERATING_SYSTEM"] != "windows", reason="Windows specific test", ) def test_windows_worker_restarts_process( diff --git a/test/e2e/utils.py b/test/e2e/utils.py index 2b23bfb5..714a0d52 100644 --- a/test/e2e/utils.py +++ b/test/e2e/utils.py @@ -116,12 +116,12 @@ def submit_sleep_job( "onRun": { "command": ( "/bin/sleep" - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else "powershell" ), "args": ( ["5"] - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else ["ping", "localhost"] ), }, @@ -302,7 +302,7 @@ def submit_custom_job( "actions": { "onRun": ( {"command": "{{ Task.File.runScript }}"} - if os.environ["OPERATING_SYSTEM"] == "linux" + if os.environ["OPERATING_SYSTEM"] != "windows" else { "command": "powershell", "args": ["{{ Task.File.runScript }}"], # type: ignore[dict-item] @@ -382,29 +382,28 @@ def is_worker_stopped( def get_shutdown_on_stop_status_from_toml( worker: EC2InstanceWorker, ) -> str: - if os.environ["OPERATING_SYSTEM"] == "linux": + if os.environ["OPERATING_SYSTEM"] == "windows": cmd_result = worker.send_command( command=""" -grep -E \ - '^(# shutdown_on_stop =|shutdown_on_stop =)' \ - /etc/amazon/deadline/worker.toml +$content = Get-Content "C:\\ProgramData\\Amazon\\Deadline\\Config\\worker.toml" +$content | Select-String -Pattern "^# shutdown_on_stop =|^shutdown_on_stop =" """ ) - assert cmd_result.exit_code == 0, "Failed to execute Linux command on .toml" + assert cmd_result.exit_code == 0, "Failed to execute Windows command on .toml" assert "shutdown_on_stop" in cmd_result.stdout, "shutdown_on_stop not found in .toml" return cmd_result.stdout.strip() - elif os.environ["OPERATING_SYSTEM"] == "windows": + else: + # POSIX (linux and macos): the worker.toml path is identical on both. cmd_result = worker.send_command( command=""" -$content = Get-Content "C:\\ProgramData\\Amazon\\Deadline\\Config\\worker.toml" -$content | Select-String -Pattern "^# shutdown_on_stop =|^shutdown_on_stop =" +grep -E \ + '^(# shutdown_on_stop =|shutdown_on_stop =)' \ + /etc/amazon/deadline/worker.toml """ ) - assert cmd_result.exit_code == 0, "Failed to execute Windows command on .toml" + assert cmd_result.exit_code == 0, "Failed to execute POSIX command on .toml" assert "shutdown_on_stop" in cmd_result.stdout, "shutdown_on_stop not found in .toml" return cmd_result.stdout.strip() - else: - raise Exception(f"Unsupported operating system: {os.environ['OPERATING_SYSTEM']}") def submit_job_from_create_job_API( From f6e5f84567e43312106097d3a4195fe7d269289b Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:31:29 -0700 Subject: [PATCH 08/23] test: add macOS GPU (Metal) compute e2e test Validates that a job launched through the full worker-agent session path (launchd daemon context -> sudo -u -> setsid shim) can reach the GPU and run a compute workload. The agent does not mediate GPU access -- the job talks to Metal directly -- so this confirms the session machinery does not block GPU use from a system daemon that has no window-server/GUI login session. The probe creates the default Metal device, compiles a compute kernel at runtime via swift, dispatches it, and verifies the result, exiting non-zero on any failure -- so a SUCCEEDED task status is the assertion. macOS-gated (OPERATING_SYSTEM=macos); skips on linux/windows. Validated in substance against a live CMF fleet: the same probe ran as deadline-job-user under a launchd-managed agent and returned the correct compute result on an Apple M4 Pro. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- test/e2e/test_gpu.py | 97 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 test/e2e/test_gpu.py diff --git a/test/e2e/test_gpu.py b/test/e2e/test_gpu.py new file mode 100644 index 00000000..c9994999 --- /dev/null +++ b/test/e2e/test_gpu.py @@ -0,0 +1,97 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +""" +GPU compute E2E tests. + +Validates that a job launched through the full worker-agent session path +(launchd daemon context -> sudo -u -> setsid shim -> task) can +actually reach the GPU and run a compute workload. The agent itself does not +mediate GPU access -- the job process talks to the graphics API directly -- so +this test confirms the session machinery does not get in the way (no window +server / GUI login session is available to a system daemon). + +macOS: uses Metal via `swift`. The probe creates the default Metal device, +compiles a compute kernel at runtime, dispatches it, and verifies the result; +it exits non-zero on any failure, so a SUCCEEDED task status is the assertion. +""" + +import os + +import pytest +from deadline_test_fixtures import ( + DeadlineClient, + Job, + TaskStatus, +) + +from e2e.conftest import DeadlineResources +from e2e.utils import submit_custom_job, job_failure_message + + +# A self-contained Metal compute probe. Written to the session working directory +# (the task's CWD) rather than $TMPDIR, which is empty in the `sudo -u ... -i` +# login shell and would resolve to the sealed read-only root volume. +_METAL_PROBE = r"""#!/bin/zsh +set -e +echo "=== identity ===" +whoami +echo "=== GPU inventory ===" +system_profiler SPDisplaysDataType | grep -E "Chipset Model|Metal Support" +echo "=== Metal compute ===" +cat > ./metalprobe.swift <<'SWIFT' +import Metal +guard let dev = MTLCreateSystemDefaultDevice() else { print("FAIL: no Metal device"); exit(1) } +print("device:", dev.name) +guard let q = dev.makeCommandQueue() else { print("FAIL: no command queue"); exit(2) } +let src = "kernel void doub(device float* o [[buffer(0)]], uint i [[thread_position_in_grid]]) { o[i] = float(i) * 2.0; }" +let lib = try! dev.makeLibrary(source: src, options: nil) +let pipe = try! dev.makeComputePipelineState(function: lib.makeFunction(name: "doub")!) +let n = 16 +let buf = dev.makeBuffer(length: n * 4, options: .storageModeShared)! +let cb = q.makeCommandBuffer()! +let enc = cb.makeComputeCommandEncoder()! +enc.setComputePipelineState(pipe) +enc.setBuffer(buf, offset: 0, index: 0) +enc.dispatchThreads(MTLSize(width: n, height: 1, depth: 1), + threadsPerThreadgroup: MTLSize(width: n, height: 1, depth: 1)) +enc.endEncoding() +cb.commit() +cb.waitUntilCompleted() +let p = buf.contents().bindMemory(to: Float.self, capacity: n) +print("compute result[8] =", p[8], "(expect 16.0)") +guard p[8] == 16.0 else { print("FAIL: wrong compute result"); exit(3) } +print("PASS: GPU compute works from session") +SWIFT +/usr/bin/swift ./metalprobe.swift +""" + + +@pytest.mark.skipif( + os.environ["OPERATING_SYSTEM"] != "macos", + reason="macOS (Metal) specific GPU test", +) +class TestMacGPU: + def test_metal_compute_runs_in_session( + self, + deadline_client: DeadlineClient, + deadline_resources: DeadlineResources, + session_worker, + ) -> None: + """A task can create a Metal device and run a compute kernel from within + the worker-agent session (daemon context, as the jobRunAsUser). The + probe exits non-zero on any GPU failure, so SUCCEEDED is the assertion.""" + job: Job = submit_custom_job( + job_name="macOS: Metal GPU compute", + deadline_client=deadline_client, + farm=deadline_resources.farm, + queue=deadline_resources.queue_a, + run_script=_METAL_PROBE, + description=( + "Validates GPU compute works from a job session on macOS. " + "Expected status: SUCCEEDED if the Metal compute kernel runs correctly." + ), + ) + job.wait_until_complete(client=deadline_client) + + assert job.task_run_status == TaskStatus.SUCCEEDED, job_failure_message( + job, deadline_client, deadline_resources.queue_a, deadline_resources + ) From 4e1660063ffd46379db28916a58b6ba3310c3a04 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 4 Aug 2026 08:20:13 -0700 Subject: [PATCH 09/23] fix(test): use platform-correct commands in e2e tests on macOS Two e2e tests had their platform guards widened from `== "linux"` to `!= "windows"` so macOS would exercise them, but their POSIX branches contained Linux-only commands. test_worker_enters_stopping_state_while_draining stopped the service with `systemctl`, which does not exist on macOS. Make it a three-way branch so macOS boots out the LaunchDaemon that install_darwin.sh registers. The other `systemctl` call sites in the suite are inside classes already skipped unless OPERATING_SYSTEM == "linux", so they are unaffected. The job-attachments hash script used `sha256sum`, which is GNU coreutils and absent from older macOS. Select the digest tool at runtime, preferring `sha256sum` and falling back to `shasum -a 256`. Both print the digest as the first field, so the existing awk extraction is unchanged. Verified on macOS 26.5 that both branches produce the digest hashlib.sha256 expects. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- test/e2e/test_job_attachments.py | 8 +++++++- test/e2e/test_job_submissions.py | 12 +++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/test/e2e/test_job_attachments.py b/test/e2e/test_job_attachments.py index c02c0706..a95e6a0d 100644 --- a/test/e2e/test_job_attachments.py +++ b/test/e2e/test_job_attachments.py @@ -1003,7 +1003,13 @@ def sync_input_job_attachments_failed(current_job: Job) -> bool: ' combined_contents+="$(cat "$file" | tr -d \'\\n\')"\n' " fi\n" "done\n" - "sha256_hash=$(echo -n \"$combined_contents\" | sha256sum | awk '{ print $1 }')\n" + # sha256sum is GNU coreutils and is not present on macOS, which ships shasum + # instead. Both emit " -", so awk extracts the digest either way. + "if command -v sha256sum > /dev/null 2>&1; then\n" + " sha256_hash=$(echo -n \"$combined_contents\" | sha256sum | awk '{ print $1 }')\n" + "else\n" + " sha256_hash=$(echo -n \"$combined_contents\" | shasum -a 256 | awk '{ print $1 }')\n" + "fi\n" 'echo -n "$sha256_hash" > {{Param.DataDir}}/output_file.txt' if os.environ["OPERATING_SYSTEM"] != "windows" else '$InputFolder = "{{Param.DataDir}}\\files"\n' diff --git a/test/e2e/test_job_submissions.py b/test/e2e/test_job_submissions.py index b74f5f33..ea904c5a 100644 --- a/test/e2e/test_job_submissions.py +++ b/test/e2e/test_job_submissions.py @@ -34,6 +34,9 @@ LOG = logging.getLogger(__name__) +# launchd service label installed by installer/install_darwin.sh on macOS workers. +MACOS_LAUNCHD_LABEL = "com.amazon.deadline.worker-agent" + class TestJobSubmission: JOB_OUTPUT_PATH = os.path.join(os.getcwd(), "job_output") @@ -1924,8 +1927,15 @@ def test_worker_enters_stopping_state_while_draining( run_script=sleep_script, ) - if os.environ["OPERATING_SYSTEM"] != "windows": + # Stopping the service sends the agent a SIGTERM (or the Windows service stop + # equivalent), which is what triggers the drain. The command differs by service + # manager: systemd on Linux, launchd on macOS, and the Windows service manager. + if os.environ["OPERATING_SYSTEM"] == "linux": cmd_result = function_worker.send_command("sudo systemctl stop deadline-worker") + elif os.environ["OPERATING_SYSTEM"] == "macos": + cmd_result = function_worker.send_command( + f"sudo launchctl bootout system/{MACOS_LAUNCHD_LABEL}" + ) else: cmd_result = function_worker.send_command("sc.exe stop DeadlineWorker") From ba38cbd1ea9e775d51222e5841855f0df432669d Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 4 Aug 2026 08:20:13 -0700 Subject: [PATCH 10/23] fix(installer): stop restarting a healthy agent on macOS re-install The re-install path ran `launchctl kickstart -k` unconditionally after bootstrap. On a clean bootstrap, RunAtLoad has already started the daemon, so the `-k` killed and respawned a healthy process for no reason. Only kickstart when bootstrap never confirmed a clean load, which happens when the asynchronous bootout left the old instance loaded and it won the race. That path also now warns that launchd is still holding the previous plist, so the operator knows the new configuration has not taken effect and how to apply it. A bootstrap that fails while the service is genuinely unloaded still re-runs unguarded so launchd's real error surfaces and set -e aborts the install. Some restarting is unavoidable, since launchd has no in-place plist reload (the `systemctl daemon-reload` analog) and the service must be booted out and back in. That diverges from a Linux config-only re-run, which leaves the running process untouched, so a re-install over a loaded service now warns that the agent was restarted and any session it was running was interrupted rather than doing so silently. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_darwin.sh | 38 +++++++++++++++++-- test/integ/macos/test_installer.py | 36 ++++++++++++++++++ 2 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_darwin.sh index 5a1d1f20..fa4a74f7 100755 --- a/src/deadline_worker_agent/installer/install_darwin.sh +++ b/src/deadline_worker_agent/installer/install_darwin.sh @@ -611,10 +611,22 @@ EOF # plist. Remember whether it was loaded: a config-only re-run (no --start) over a loaded # service must put the service back afterward, matching Linux where a re-run without # `systemctl start` leaves a running service running. + # + # DIVERGENCE FROM LINUX: this restarts a running agent. launchd has no way to reload a + # changed plist in place (the systemd `daemon-reload` analog) -- the service must be + # booted out and back in -- so unlike a Linux config-only re-run, which leaves the + # running process untouched, a macOS re-install terminates the agent. Any session the + # agent is currently running is interrupted. Warn the operator rather than doing this + # silently. was_loaded="no" if launchctl print "system/${launchd_label}" &> /dev/null; then echo "Existing LaunchDaemon detected; unloading" was_loaded="yes" + warning_lines+=( + "The worker agent service was running and has been restarted to load the updated" + "configuration. Any session it was running was interrupted. (macOS/launchd cannot" + "reload a changed LaunchDaemon plist without restarting the service.)" + ) # bootout returns non-zero if the service is not loaded; tolerate the race. launchctl bootout system "${launchd_plist}" &> /dev/null || true fi @@ -638,11 +650,29 @@ EOF sleep 1 done if [[ "${bootstrap_ok}" != "yes" ]]; then - # Surface the real error for the operator. - launchctl bootstrap system "${launchd_plist}" + if launchctl print "system/${launchd_label}" &> /dev/null; then + # bootstrap kept failing because the service is still loaded: the old + # instance never fully unloaded and won the race. kickstart -k forces it to + # (re)start so the operator is not left with a stopped agent. launchd is + # still holding the OLD plist in this case, so the config we just wrote does + # not take effect until the service is reloaded -- say so. + echo "Service still loaded after bootout; forcing a restart" + launchctl kickstart -k "system/${launchd_label}" + warning_lines+=( + "The previous worker agent service could not be unloaded, so launchd is still" + "using the previous configuration. Reboot, or run" + "\`launchctl bootout system/${launchd_label}\` followed by" + "\`launchctl bootstrap system ${launchd_plist}\`, to apply the new configuration." + ) + else + # Not loaded, and bootstrap will not take it. Re-run unguarded so the real + # launchd error reaches the operator; set -e then aborts the install. + launchctl bootstrap system "${launchd_plist}" + fi fi - # kickstart -k guarantees an immediate (re)start even if bootstrap raced. - launchctl kickstart -k "system/${launchd_label}" + # After a clean bootstrap, RunAtLoad has already started the daemon -- no kickstart + # here, since an unconditional `-k` would kill and respawn a healthy process (and + # interrupt its session) for no reason. echo "Done starting the service" else echo "LaunchDaemon installed; it will start on the next boot (use --start to start it now)" diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py index 40625c35..93b214bc 100644 --- a/test/integ/macos/test_installer.py +++ b/test/integ/macos/test_installer.py @@ -356,3 +356,39 @@ def test_reinstall_without_start_over_unloaded_service_stays_unloaded(self, inst run_installer() assert not service_is_registered() assert not agent_process_running() + + def test_reinstall_over_loaded_service_warns_about_restart(self, installed) -> None: + """launchd cannot reload a changed plist in place, so a re-install + restarts a running agent and interrupts any session it is running -- + a divergence from Linux, where a config-only re-run leaves the running + process untouched. The installer must say so rather than doing it + silently.""" + run_installer("--start") + try: + assert service_is_registered() + result = run_installer() + assert "restarted" in result.stdout, ( + f"re-install over a loaded service did not warn about the restart: {result.stdout}" + ) + finally: + subprocess.run( + ["sudo", "launchctl", "bootout", f"system/{LAUNCHD_LABEL}"], + capture_output=True, + ) + assert not service_is_registered() + + def test_first_install_does_not_warn_about_restart(self, installed) -> None: + """The restart warning must be specific to the re-install-over-loaded + case: a first install has no running agent to interrupt.""" + assert not service_is_registered(), "test precondition: service must be unloaded" + result = run_installer("--start") + try: + assert "restarted" not in result.stdout, ( + f"install over an unloaded service warned about a restart: {result.stdout}" + ) + finally: + subprocess.run( + ["sudo", "launchctl", "bootout", f"system/{LAUNCHD_LABEL}"], + capture_output=True, + ) + assert not service_is_registered() From 0ccac4aab7f44f6323cc09fa7010fbf20e9f21b9 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 4 Aug 2026 12:13:55 -0700 Subject: [PATCH 11/23] fix(installer): grant the agent passwordless sudo to job users on macOS openjd-sessions runs a Session's actions as the queue's jobRunAsUser via `sudo -u -i ...`, which its README documents as requiring passwordless sudo ("host ALL=(actions) NOPASSWD: ALL"). Nothing satisfied that on macOS, so every impersonated action died with "sudo: a terminal is required to read the password" and the task retried forever, oscillating between READY and ASSIGNED. A stock macOS install therefore could not run a job under the default QUEUE_CONFIGURED_USER configuration. Linux needs no explicit rule only because the distro's stock sudoers already grants sudo to the agent user's groups. On macOS the agent is a hidden UID<500 service account in no admin group, and a LaunchDaemon has no TTY for sudo to prompt on, so the rule is mandatory rather than optional and is not gated behind a flag. The rule's runas target is the job GROUP, not a single user: the queue selects the jobRunAsUser and the installer cannot know which member of the group that will be. It grants the agent user no authority over root or over any account outside the job group. The generated file is validated with `visudo -cf` and removed if it does not parse, since a malformed file in /etc/sudoers.d breaks sudo host-wide. Verified on macOS 26.5 (arm64) against a live customer-managed fleet: a job now runs as the queue's jobRunAsUser (uid 498, gid 501), the job user cannot read the agent's cached credentials, and cancelling a running task reaps the whole process group with no orphaned processes. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_darwin.sh | 34 +++++++++++++++++++ test/integ/macos/test_installer.py | 30 ++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_darwin.sh index fa4a74f7..7152a92c 100755 --- a/src/deadline_worker_agent/installer/install_darwin.sh +++ b/src/deadline_worker_agent/installer/install_darwin.sh @@ -443,6 +443,40 @@ else echo "No prior sudoers shutdown rule at /etc/sudoers.d/deadline-worker-shutdown" fi +# --- Sudoers configuration (jobRunAsUser impersonation) ----------------------------- +# openjd-sessions runs a Session's actions as the queue's jobRunAsUser via +# `sudo -u -i ...`, so the agent user must be able to become a job user +# WITHOUT a password. Its README states the requirement directly: "You must ensure that +# the `host` user is able to run commands as the `actions` user with passwordless sudo". +# +# Linux does not need an explicit rule here only because the distro's stock sudoers +# already grants sudo to the agent user's groups. On macOS the agent is a hidden +# UID<500 service account that belongs to no admin group, so nothing grants it, and a +# LaunchDaemon has no TTY for sudo to prompt on -- every impersonated action would fail +# with "sudo: a terminal is required to read the password" and the job would retry +# forever. The rule is therefore mandatory on macOS, not optional. +# +# Scoped to the job GROUP (%group runas syntax), not a single user: the queue chooses the +# jobRunAsUser, which may be any member of the job group, and the installer does not know +# which. This grants the agent user no new authority over root or over any account +# outside the job group. +echo "Setting up sudoers jobRunAsUser rule at /etc/sudoers.d/deadline-worker-job-users" +mkdir -p /etc/sudoers.d +cat > /etc/sudoers.d/deadline-worker-job-users < -i ...). +${wa_user} ALL=(%${job_group}) NOPASSWD: ALL +EOF +chmod 440 /etc/sudoers.d/deadline-worker-job-users +# A malformed sudoers file can break sudo host-wide, so validate before leaving it in +# place and remove it rather than shipping something sudo will refuse to parse. +if ! visudo -cf /etc/sudoers.d/deadline-worker-job-users; then + rm -f /etc/sudoers.d/deadline-worker-job-users + echo "ERROR: generated an invalid sudoers file; removed it." >&2 + exit 1 +fi +echo "Done setting up sudoers jobRunAsUser rule" + # --- Directory provisioning (IDENTICAL to Linux: paths + modes are portable) -------- echo "Provisioning log directory (/var/log/amazon/deadline)" mkdir -p /var/log/amazon/deadline diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py index 93b214bc..12167fc6 100644 --- a/test/integ/macos/test_installer.py +++ b/test/integ/macos/test_installer.py @@ -41,6 +41,7 @@ LAUNCHD_LABEL = "com.amazon.deadline.worker-agent" PLIST_PATH = Path("/Library/LaunchDaemons") / f"{LAUNCHD_LABEL}.plist" SUDOERS_PATH = Path("/etc/sudoers.d/deadline-worker-shutdown") +JOB_USERS_SUDOERS_PATH = Path("/etc/sudoers.d/deadline-worker-job-users") REPO_ROOT = Path(__file__).parent.parent.parent.parent INSTALLER = REPO_ROOT / "src" / "deadline_worker_agent" / "installer" / "install_darwin.sh" @@ -264,6 +265,35 @@ def test_service_not_loaded_without_start(self, installed) -> None: assert not service_is_registered() assert not agent_process_running() + def test_sudoers_grants_job_user_impersonation(self, installed) -> None: + """openjd-sessions runs a Session's actions as the queue's jobRunAsUser via + `sudo -u -i ...`, which requires passwordless sudo. Unlike Linux, + where the distro's stock sudoers already covers the agent user, macOS grants + a hidden UID<500 service account nothing, and a LaunchDaemon has no TTY for + sudo to prompt on -- so without this rule every impersonated action fails + with "sudo: a terminal is required to read the password".""" + assert JOB_USERS_SUDOERS_PATH.exists(), ( + "installer did not create the jobRunAsUser sudoers rule" + ) + assert sudo_output("stat", "-f", "%Lp %Su", str(JOB_USERS_SUDOERS_PATH)) == "440 root" + # A malformed file here breaks sudo host-wide, so it must parse. + subprocess.run(["sudo", "visudo", "-cf", str(JOB_USERS_SUDOERS_PATH)], check=True) + content = sudo_output("cat", str(JOB_USERS_SUDOERS_PATH)) + # Scoped to the job GROUP: the queue picks the jobRunAsUser, and the installer + # cannot know which member of the group that will be. + assert re.search( + rf"^{WA_USER} ALL=\(%{JOB_GROUP}\) NOPASSWD: ALL$", content, re.MULTILINE + ), f"unexpected rule contents: {content}" + + def test_sudoers_job_user_rule_does_not_grant_root(self, installed) -> None: + """The impersonation rule must not become a general root escalation for the + agent user: it is scoped to the job group's runas list only.""" + content = sudo_output("cat", str(JOB_USERS_SUDOERS_PATH)) + assert "ALL=(ALL)" not in content + assert "(root)" not in content + # Absent --allow-shutdown there is no root-granting rule at all. + assert not SUDOERS_PATH.exists() or "root" not in sudo_output("cat", str(SUDOERS_PATH)) + def test_sudoers_grants_exactly_shutdown(self, installed) -> None: assert sudo_output("stat", "-f", "%Lp %Su", str(SUDOERS_PATH)) == "440 root" # visudo validates the syntax From 6cd5ccccaff966913157d457826ce5ea02d17f9f Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Wed, 5 Aug 2026 09:14:57 -0700 Subject: [PATCH 12/23] test: fix jobRunAsUser sudoers assertion contradicting its own fixture test_sudoers_job_user_rule_does_not_grant_root ended by asserting that the shutdown sudoers file either does not exist or contains no "root", on the stated grounds that "absent --allow-shutdown there is no root-granting rule". But the module fixture installs WITH --allow-shutdown, so that file exists and legitimately contains `ALL=(root) NOPASSWD: /sbin/shutdown -h now`. The test failed on both macOS CI jobs (3.11 and 3.13) for that reason alone. The assertion also reached across to a file the test is not about. Scope it to the jobRunAsUser rule and check the whole rule set rather than two known-bad substrings: exactly one non-comment line, equal to the group-scoped rule. That catches an added second rule or a widened runas list, which the previous `"ALL=(ALL)" not in content` / `"(root)" not in content` pair did not. Shutdown-rule coverage is unchanged and already lives in test_sudoers_grants_exactly_shutdown and (for its absence) test_reinstall_without_allow_shutdown_revokes_sudoers; the docstring now says so. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- test/integ/macos/test_installer.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py index 12167fc6..7d60a3eb 100644 --- a/test/integ/macos/test_installer.py +++ b/test/integ/macos/test_installer.py @@ -287,12 +287,24 @@ def test_sudoers_grants_job_user_impersonation(self, installed) -> None: def test_sudoers_job_user_rule_does_not_grant_root(self, installed) -> None: """The impersonation rule must not become a general root escalation for the - agent user: it is scoped to the job group's runas list only.""" + agent user: it is scoped to the job group's runas list only. + + Only the jobRunAsUser rule is examined here. The shutdown rule is a + deliberate, narrowly-scoped root grant gated behind --allow-shutdown; it is + covered by test_sudoers_grants_exactly_shutdown and (for its absence) + test_reinstall_without_allow_shutdown_revokes_sudoers.""" content = sudo_output("cat", str(JOB_USERS_SUDOERS_PATH)) - assert "ALL=(ALL)" not in content - assert "(root)" not in content - # Absent --allow-shutdown there is no root-granting rule at all. - assert not SUDOERS_PATH.exists() or "root" not in sudo_output("cat", str(SUDOERS_PATH)) + # Exactly one rule, and it is the job-group-scoped one. A second rule -- or a + # widened runas list -- would hand the agent user authority the queue's + # jobRunAsUser model does not require. + rules = [ + line.strip() + for line in content.splitlines() + if line.strip() and not line.strip().startswith("#") + ] + assert rules == [f"{WA_USER} ALL=(%{JOB_GROUP}) NOPASSWD: ALL"], ( + f"unexpected rule contents: {content}" + ) def test_sudoers_grants_exactly_shutdown(self, installed) -> None: assert sudo_output("stat", "-f", "%Lp %Su", str(SUDOERS_PATH)) == "440 root" From 0accccf981026e357b83a2f469b8e6f390f87afd Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Wed, 5 Aug 2026 09:15:09 -0700 Subject: [PATCH 13/23] docs: correct the sudoers rationale and state which queue modes it affects Two corrections to the jobRunAsUser sudoers comment block. No behavior change. 1. The claim that "Linux does not need an explicit rule here only because the distro's stock sudoers already grants sudo to the agent user's groups" was wrong. Linux needs an equivalent rule; it is a documented manual step that install.sh never automated -- the developer guide has the operator write `deadline-worker-agent ALL=(jobRunAsUser) NOPASSWD:ALL` by hand. The actual macOS divergence is that the installer writes the rule for you, justified because here the failure mode is silent as well as fatal: no TTY on a LaunchDaemon, so every impersonated action dies with "a terminal is required to read the password" and the task loops READY <-> ASSIGNED with nothing on the host explaining why. 2. Record which queue configurations the rule actually affects. It is consulted only for runAs = QUEUE_CONFIGURED_USER (and the agent's posix_job_user override), where openjd-sessions takes its cross-user sudo path. Under runAs = WORKER_AGENT_USER the agent passes no user, is_process_user() short-circuits the sudo branch, and actions run as the agent user directly -- the rule is never consulted. It is still written unconditionally because the installer cannot know which queues the fleet will be associated with, and association can change after install. Also note explicitly that the group-scoped runas target is broader than the per-user rule the Linux docs prescribe, so job group membership is the security boundary: adding a user to the job group makes it impersonable by the agent. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_darwin.sh | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_darwin.sh index 7152a92c..a0be6509 100755 --- a/src/deadline_worker_agent/installer/install_darwin.sh +++ b/src/deadline_worker_agent/installer/install_darwin.sh @@ -449,17 +449,33 @@ fi # WITHOUT a password. Its README states the requirement directly: "You must ensure that # the `host` user is able to run commands as the `actions` user with passwordless sudo". # -# Linux does not need an explicit rule here only because the distro's stock sudoers -# already grants sudo to the agent user's groups. On macOS the agent is a hidden -# UID<500 service account that belongs to no admin group, so nothing grants it, and a -# LaunchDaemon has no TTY for sudo to prompt on -- every impersonated action would fail -# with "sudo: a terminal is required to read the password" and the job would retry -# forever. The rule is therefore mandatory on macOS, not optional. +# WHICH QUEUE CONFIGURATIONS THIS AFFECTS: only jobRunAsUser -> runAs = +# QUEUE_CONFIGURED_USER (and the agent's own `posix_job_user` config override), where +# openjd-sessions takes its cross-user path. Under runAs = WORKER_AGENT_USER the agent +# passes no user to openjd-sessions, `PosixSessionUser.is_process_user()` short-circuits +# the `sudo` branch, and actions run as the agent user directly -- this rule is never +# consulted. It is written unconditionally because the installer cannot know which +# queues the fleet will be associated with, and association can change after install. +# +# Linux DOES need an equivalent rule; it is simply a documented manual step rather than +# something install.sh automates. The Deadline Cloud developer guide has the operator +# create it by hand (`deadline-worker-agent ALL=(jobRunAsUser) NOPASSWD:ALL`): +# https://docs.aws.amazon.com/deadline-cloud/latest/developerguide/worker-host.html +# The macOS divergence is that the installer writes the rule for you, because here the +# failure is both silent and fatal: the agent is a hidden UID<500 service account in no +# admin group, and a LaunchDaemon has no TTY for sudo to prompt on, so every impersonated +# action dies with "sudo: a terminal is required to read the password" and the task +# retries forever (READY <-> ASSIGNED) with nothing on the host explaining why. # # Scoped to the job GROUP (%group runas syntax), not a single user: the queue chooses the # jobRunAsUser, which may be any member of the job group, and the installer does not know # which. This grants the agent user no new authority over root or over any account -# outside the job group. +# outside the job group. Note this is BROADER than the per-user rule the Linux docs +# prescribe -- it covers any current or future member of ${job_group} -- which is the +# price of not knowing the queue's user at install time. Treat job group membership as +# the security boundary: adding a user to ${job_group} makes it impersonable by the +# agent. The docs require each jobRunAsUser to be in this group and to stay out of the +# agent's primary group, so group membership is already the intended boundary. echo "Setting up sudoers jobRunAsUser rule at /etc/sudoers.d/deadline-worker-job-users" mkdir -p /etc/sudoers.d cat > /etc/sudoers.d/deadline-worker-job-users < Date: Thu, 6 Aug 2026 09:44:51 -0700 Subject: [PATCH 14/23] fix(installer): validate the shutdown sudoers file before installing it The jobRunAsUser sudoers file was validated with `visudo -cf` and discarded if sudo could not parse it; the --allow-shutdown file was written with no such check. A malformed file in /etc/sudoers.d breaks sudo host-wide, so the protection needs to cover every file the installer puts there, not one of two. Write ordering made the gap worse: the shutdown file is written first, so if a later step aborted the install (the jobRunAsUser visudo check being the obvious candidate), an unvalidated shutdown file was already on disk. Both files now go through one install_sudoers_file helper that writes to a temporary path, validates, and only then moves the file into place. Validating before the file is visible to sudo is stronger than the previous write-then-remove-on-failure order, under which a rejected file briefly existed at the real path where a concurrent sudo could observe it. The helper also asserts root:wheel and 440 on the installed file, since sudo ignores a sudoers.d file that is not root-owned or is group-writable. No content change to either rule; the generated bytes are identical. Note the reviewing agent's specific example does not hold: a username ending in `$` is rejected by the --user regex (the `\$` in the second alternative is a literal-dollar escape inside a bracket-free group, but the alternative still has to match the whole name, and `deadline$` does not), and `visudo` parses such a name without complaint anyway. The asymmetry it identified is real regardless -- the fix is worth making on its own terms rather than for that scenario. Adds test_every_sudoers_file_is_root_owned_and_parses, parameterized over both files, asserting 440 root:wheel and that visudo accepts each. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_darwin.sh | 49 +++++++++++++------ test/integ/macos/test_installer.py | 10 ++++ 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_darwin.sh index a0be6509..6b976995 100755 --- a/src/deadline_worker_agent/installer/install_darwin.sh +++ b/src/deadline_worker_agent/installer/install_darwin.sh @@ -177,6 +177,31 @@ validate_deadline_id() { [[ "${input}" =~ ^$prefix-[a-f0-9]{32}$ ]] } +# Install a file into /etc/sudoers.d only if sudo can parse it. +# A malformed file in /etc/sudoers.d breaks sudo HOST-WIDE, not just for this agent, so every +# file this installer writes there goes through here: write to a temporary location, validate, +# and only then move it into place. Validating before the file is ever visible to sudo (rather +# than writing it and removing it on failure) means a rejected file never exists at the real +# path, so a concurrent sudo cannot observe the broken state. +# Args: $1 = destination path under /etc/sudoers.d, $2 = file content. +install_sudoers_file() { + local dest="$1" content="$2" tmp + tmp="$(mktemp)" + printf '%s' "${content}" > "${tmp}" + chmod 440 "${tmp}" + if ! visudo -cf "${tmp}" > /dev/null; then + rm -f "${tmp}" + echo "ERROR: generated an invalid sudoers file for ${dest}; not installing it." >&2 + return 1 + fi + # mv within the same filesystem is atomic; /etc/sudoers.d and mktemp's /var/folders are both + # on the root volume. Re-assert mode/owner after the move: mktemp created the file as root + # (the installer requires root) but be explicit rather than relying on it. + mv "${tmp}" "${dest}" + chown root:wheel "${dest}" + chmod 440 "${dest}" +} + # Validate arguments # macOS ships only BSD getopt, which does NOT support `--longoptions` -- it silently treats the # long option spec as positional args and drops every real flag, leaving values "unset". Rather @@ -429,11 +454,13 @@ if [[ "${allow_shutdown}" == "yes" ]]; then echo "Setting up sudoers shutdown rule at /etc/sudoers.d/deadline-worker-shutdown" # /etc/sudoers.d exists and is included by default on macOS. mkdir -p /etc/sudoers.d - cat > /etc/sudoers.d/deadline-worker-shutdown < /etc/sudoers.d/deadline-worker-job-users < -i ...). ${wa_user} ALL=(%${job_group}) NOPASSWD: ALL -EOF -chmod 440 /etc/sudoers.d/deadline-worker-job-users -# A malformed sudoers file can break sudo host-wide, so validate before leaving it in -# place and remove it rather than shipping something sudo will refuse to parse. -if ! visudo -cf /etc/sudoers.d/deadline-worker-job-users; then - rm -f /etc/sudoers.d/deadline-worker-job-users - echo "ERROR: generated an invalid sudoers file; removed it." >&2 - exit 1 -fi +" echo "Done setting up sudoers jobRunAsUser rule" # --- Directory provisioning (IDENTICAL to Linux: paths + modes are portable) -------- diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py index 7d60a3eb..d7dff50f 100644 --- a/test/integ/macos/test_installer.py +++ b/test/integ/macos/test_installer.py @@ -315,6 +315,16 @@ def test_sudoers_grants_exactly_shutdown(self, installed) -> None: rf"^{WA_USER} ALL=\(root\) NOPASSWD: /sbin/shutdown -h now$", content, re.MULTILINE ) + @pytest.mark.parametrize("path", [SUDOERS_PATH, JOB_USERS_SUDOERS_PATH]) + def test_every_sudoers_file_is_root_owned_and_parses(self, installed, path: Path) -> None: + """Both files this installer writes into /etc/sudoers.d go through the same + validate-then-install helper. A malformed file there breaks sudo host-wide, and a + non-root-owned or group-writable one is ignored by sudo outright, so assert both + properties for every file rather than only the jobRunAsUser one.""" + assert path.exists(), f"installer did not create {path}" + assert sudo_output("stat", "-f", "%Lp %Su %Sg", str(path)) == "440 root wheel" + subprocess.run(["sudo", "visudo", "-cf", str(path)], check=True) + class TestReinstall: """Behavior of running the installer again over an existing installation.""" From 933275bc785267a91977634442290ee9b1075ac6 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Fri, 7 Aug 2026 09:30:11 -0700 Subject: [PATCH 15/23] test: verify the installed LaunchDaemon starts the agent from cold Nothing covered the "host rebooted and the worker never came back" failure mode. The plist's RunAtLoad/KeepAlive combination is what makes the daemon come back on boot, and it went through several revisions during review (RunAtLoad was briefly gated on --start, which would have left a no---start install never starting the agent at all, even across reboots). That is worth a regression guard. A real reboot cannot run on a GitHub macOS runner, so the test reproduces the mechanism a reboot uses instead: at boot launchd loads every plist in /Library/LaunchDaemons and starts those with RunAtLoad=true, and `launchctl bootstrap system ` is that same load-and-start step. Installing without --start (so nothing is running and only the on-disk plist can bring the service back), then bootstrapping from that cold state, exercises the same path boot does. Asserts RunAtLoad is true, that the service registers, and that launchd actually spawned a process. The last check polls `launchctl print` for a "pid = N" line rather than sampling once, because the agent cannot authenticate against the fake farm/fleet and is only briefly resident between KeepAlive respawns. Regex verified against real `launchctl print` output (the line is tab-indented) and against negative controls for the no-pid and "ppid" cases. Explicitly scoped in the class docstring: this is the launchd half only. It does not prove re-registration with the Deadline service, which needs a live farm and an EC2 Mac dedicated host; that is tracked as separate follow-up work. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- test/integ/macos/test_installer.py | 83 ++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py index d7dff50f..ee21c31a 100644 --- a/test/integ/macos/test_installer.py +++ b/test/integ/macos/test_installer.py @@ -24,6 +24,7 @@ import plistlib import re import subprocess +import time from pathlib import Path import pytest @@ -444,3 +445,85 @@ def test_first_install_does_not_warn_about_restart(self, installed) -> None: capture_output=True, ) assert not service_is_registered() + + +class TestStartsOnBoot: + """The daemon must come back by itself after a reboot. + + SCOPE: this covers the launchd half of "reboot and re-attach" only. A real + reboot cannot run on a GitHub macOS runner, and re-registering with the + Deadline service needs a live farm/fleet, which these tests deliberately do + not have (the ids are fakes). What is reproduced here is the mechanism a + reboot uses: at boot launchd loads every plist in /Library/LaunchDaemons and + starts the ones with RunAtLoad=true. `launchctl bootstrap system ` is + that same load-and-start step, so a plist that starts under bootstrap from a + cold (unloaded) state is a plist that starts on boot. + + End-to-end reboot + re-attach against a real fleet is tracked separately; it + needs an EC2 Mac dedicated host and macOS support in + deadline-cloud-test-fixtures. + """ + + def test_installed_plist_starts_the_daemon_from_cold(self, installed) -> None: + """Install without --start, then load the plist the way boot does. + + This is the regression guard for the RunAtLoad/KeepAlive combination. If + RunAtLoad were false (or the plist were otherwise not boot-ready), the + install would leave a worker that never runs again after a restart -- + which is what an operator experiences as "the host rebooted and the + worker never came back".""" + # GIVEN a config-only install (no --start) over an unloaded service, so + # nothing is running and only the on-disk plist can bring it back + run_installer() + assert not service_is_registered(), "test precondition: service must be unloaded" + + plist = plistlib.loads(PLIST_PATH.read_bytes()) + assert plist["RunAtLoad"] is True, ( + "plist is not boot-ready: RunAtLoad must be true or the daemon will " + "not start after a reboot" + ) + + # WHEN launchd loads it, exactly as it does for /Library/LaunchDaemons at boot + try: + subprocess.run( + ["sudo", "launchctl", "bootstrap", "system", str(PLIST_PATH)], + capture_output=True, + check=True, + ) + + # THEN the service is registered and launchd actually spawned it. + # The agent cannot authenticate against the fake farm/fleet, so it + # exits and KeepAlive respawns it; poll for a PID rather than + # requiring one at an instant we might catch between restarts. + assert service_is_registered() + assert self._daemon_had_a_pid(), ( + "launchd loaded the plist but never spawned the process; " + "RunAtLoad did not take effect" + ) + finally: + subprocess.run( + ["sudo", "launchctl", "bootout", f"system/{LAUNCHD_LABEL}"], + capture_output=True, + ) + assert not service_is_registered() + + @staticmethod + def _daemon_had_a_pid(timeout_s: float = 20.0) -> bool: + """True if `launchctl print` ever reports a pid for the daemon. + + A crash-looping job is only briefly resident, so sample repeatedly + instead of once. A "pid = N" line means launchd started the process, + which is the property under test; whether that process then exits on the + fake credentials is irrelevant here. + """ + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + out = subprocess.run( + ["sudo", "launchctl", "print", f"system/{LAUNCHD_LABEL}"], + capture_output=True, + text=True, + ).stdout + if re.search(r"^\s*pid\s*=\s*\d+", out, re.MULTILINE): + return True + time.sleep(0.5) + return False From 433f15a9f3602549c8ca8c6e1b07eb7b535fae16 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Fri, 7 Aug 2026 13:35:07 -0700 Subject: [PATCH 16/23] fix(installer): do not configure jobRunAsUser sudo on macOS Removes the /etc/sudoers.d/deadline-worker-job-users rule the installer wrote (`deadline-worker ALL=(%deadline-job-users) NOPASSWD: ALL`) and its tests. install.sh does not grant the agent passwordless sudo to the queue's job user -- on Linux that is a documented manual step for the operator. macOS now matches. Automating it, with or without an installer flag, is a quality-of-life change that should land for both platforms together rather than diverging them here. Consequence: a stock macOS install cannot run jobs on a queue configured with runAs = QUEUE_CONFIGURED_USER until the operator adds the rule by hand, the same as Linux. Queues using runAs = WORKER_AGENT_USER are unaffected -- the agent passes no user to openjd-sessions, so its cross-user sudo path is never taken. The macOS failure mode is quieter than Linux's (a LaunchDaemon has no TTY, so sudo cannot prompt and the task just retries), which makes this worth calling out in the macOS worker-host docs follow-up. The --allow-shutdown rule is unchanged: it has a direct Linux counterpart and is flag-gated. It still installs through install_sudoers_file, which validates with `visudo -cf` before moving the file into place. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_darwin.sh | 48 ++--------------- test/integ/macos/test_installer.py | 52 +++---------------- 2 files changed, 10 insertions(+), 90 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_darwin.sh index 6b976995..c58a2b09 100755 --- a/src/deadline_worker_agent/installer/install_darwin.sh +++ b/src/deadline_worker_agent/installer/install_darwin.sh @@ -454,9 +454,9 @@ if [[ "${allow_shutdown}" == "yes" ]]; then echo "Setting up sudoers shutdown rule at /etc/sudoers.d/deadline-worker-shutdown" # /etc/sudoers.d exists and is included by default on macOS. mkdir -p /etc/sudoers.d - # Validated before being installed -- see install_sudoers_file. This file is written before - # the jobRunAsUser rule below, so validating it here keeps an unvalidated file from being - # left behind if a later step aborts the install. + # Validated before being installed -- see install_sudoers_file. A rejected file never + # appears at the real path, so a later step aborting the install cannot leave an + # unvalidated file behind in /etc/sudoers.d. install_sudoers_file /etc/sudoers.d/deadline-worker-shutdown \ "# Allow ${wa_user} user to shutdown the system ${wa_user} ALL=(root) NOPASSWD: /sbin/shutdown -h now @@ -470,48 +470,6 @@ else echo "No prior sudoers shutdown rule at /etc/sudoers.d/deadline-worker-shutdown" fi -# --- Sudoers configuration (jobRunAsUser impersonation) ----------------------------- -# openjd-sessions runs a Session's actions as the queue's jobRunAsUser via -# `sudo -u -i ...`, so the agent user must be able to become a job user -# WITHOUT a password. Its README states the requirement directly: "You must ensure that -# the `host` user is able to run commands as the `actions` user with passwordless sudo". -# -# WHICH QUEUE CONFIGURATIONS THIS AFFECTS: only jobRunAsUser -> runAs = -# QUEUE_CONFIGURED_USER (and the agent's own `posix_job_user` config override), where -# openjd-sessions takes its cross-user path. Under runAs = WORKER_AGENT_USER the agent -# passes no user to openjd-sessions, `PosixSessionUser.is_process_user()` short-circuits -# the `sudo` branch, and actions run as the agent user directly -- this rule is never -# consulted. It is written unconditionally because the installer cannot know which -# queues the fleet will be associated with, and association can change after install. -# -# Linux DOES need an equivalent rule; it is simply a documented manual step rather than -# something install.sh automates. The Deadline Cloud developer guide has the operator -# create it by hand (`deadline-worker-agent ALL=(jobRunAsUser) NOPASSWD:ALL`): -# https://docs.aws.amazon.com/deadline-cloud/latest/developerguide/worker-host.html -# The macOS divergence is that the installer writes the rule for you, because here the -# failure is both silent and fatal: the agent is a hidden UID<500 service account in no -# admin group, and a LaunchDaemon has no TTY for sudo to prompt on, so every impersonated -# action dies with "sudo: a terminal is required to read the password" and the task -# retries forever (READY <-> ASSIGNED) with nothing on the host explaining why. -# -# Scoped to the job GROUP (%group runas syntax), not a single user: the queue chooses the -# jobRunAsUser, which may be any member of the job group, and the installer does not know -# which. This grants the agent user no new authority over root or over any account -# outside the job group. Note this is BROADER than the per-user rule the Linux docs -# prescribe -- it covers any current or future member of ${job_group} -- which is the -# price of not knowing the queue's user at install time. Treat job group membership as -# the security boundary: adding a user to ${job_group} makes it impersonable by the -# agent. The docs require each jobRunAsUser to be in this group and to stay out of the -# agent's primary group, so group membership is already the intended boundary. -echo "Setting up sudoers jobRunAsUser rule at /etc/sudoers.d/deadline-worker-job-users" -mkdir -p /etc/sudoers.d -install_sudoers_file /etc/sudoers.d/deadline-worker-job-users \ -"# Allow ${wa_user} to run a Session's actions as any user in the ${job_group} group. -# Required by openjd-sessions' POSIX impersonation path (sudo -u -i ...). -${wa_user} ALL=(%${job_group}) NOPASSWD: ALL -" -echo "Done setting up sudoers jobRunAsUser rule" - # --- Directory provisioning (IDENTICAL to Linux: paths + modes are portable) -------- echo "Provisioning log directory (/var/log/amazon/deadline)" mkdir -p /var/log/amazon/deadline diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py index ee21c31a..482f0111 100644 --- a/test/integ/macos/test_installer.py +++ b/test/integ/macos/test_installer.py @@ -42,7 +42,6 @@ LAUNCHD_LABEL = "com.amazon.deadline.worker-agent" PLIST_PATH = Path("/Library/LaunchDaemons") / f"{LAUNCHD_LABEL}.plist" SUDOERS_PATH = Path("/etc/sudoers.d/deadline-worker-shutdown") -JOB_USERS_SUDOERS_PATH = Path("/etc/sudoers.d/deadline-worker-job-users") REPO_ROOT = Path(__file__).parent.parent.parent.parent INSTALLER = REPO_ROOT / "src" / "deadline_worker_agent" / "installer" / "install_darwin.sh" @@ -266,47 +265,6 @@ def test_service_not_loaded_without_start(self, installed) -> None: assert not service_is_registered() assert not agent_process_running() - def test_sudoers_grants_job_user_impersonation(self, installed) -> None: - """openjd-sessions runs a Session's actions as the queue's jobRunAsUser via - `sudo -u -i ...`, which requires passwordless sudo. Unlike Linux, - where the distro's stock sudoers already covers the agent user, macOS grants - a hidden UID<500 service account nothing, and a LaunchDaemon has no TTY for - sudo to prompt on -- so without this rule every impersonated action fails - with "sudo: a terminal is required to read the password".""" - assert JOB_USERS_SUDOERS_PATH.exists(), ( - "installer did not create the jobRunAsUser sudoers rule" - ) - assert sudo_output("stat", "-f", "%Lp %Su", str(JOB_USERS_SUDOERS_PATH)) == "440 root" - # A malformed file here breaks sudo host-wide, so it must parse. - subprocess.run(["sudo", "visudo", "-cf", str(JOB_USERS_SUDOERS_PATH)], check=True) - content = sudo_output("cat", str(JOB_USERS_SUDOERS_PATH)) - # Scoped to the job GROUP: the queue picks the jobRunAsUser, and the installer - # cannot know which member of the group that will be. - assert re.search( - rf"^{WA_USER} ALL=\(%{JOB_GROUP}\) NOPASSWD: ALL$", content, re.MULTILINE - ), f"unexpected rule contents: {content}" - - def test_sudoers_job_user_rule_does_not_grant_root(self, installed) -> None: - """The impersonation rule must not become a general root escalation for the - agent user: it is scoped to the job group's runas list only. - - Only the jobRunAsUser rule is examined here. The shutdown rule is a - deliberate, narrowly-scoped root grant gated behind --allow-shutdown; it is - covered by test_sudoers_grants_exactly_shutdown and (for its absence) - test_reinstall_without_allow_shutdown_revokes_sudoers.""" - content = sudo_output("cat", str(JOB_USERS_SUDOERS_PATH)) - # Exactly one rule, and it is the job-group-scoped one. A second rule -- or a - # widened runas list -- would hand the agent user authority the queue's - # jobRunAsUser model does not require. - rules = [ - line.strip() - for line in content.splitlines() - if line.strip() and not line.strip().startswith("#") - ] - assert rules == [f"{WA_USER} ALL=(%{JOB_GROUP}) NOPASSWD: ALL"], ( - f"unexpected rule contents: {content}" - ) - def test_sudoers_grants_exactly_shutdown(self, installed) -> None: assert sudo_output("stat", "-f", "%Lp %Su", str(SUDOERS_PATH)) == "440 root" # visudo validates the syntax @@ -316,12 +274,16 @@ def test_sudoers_grants_exactly_shutdown(self, installed) -> None: rf"^{WA_USER} ALL=\(root\) NOPASSWD: /sbin/shutdown -h now$", content, re.MULTILINE ) - @pytest.mark.parametrize("path", [SUDOERS_PATH, JOB_USERS_SUDOERS_PATH]) + @pytest.mark.parametrize("path", [SUDOERS_PATH]) def test_every_sudoers_file_is_root_owned_and_parses(self, installed, path: Path) -> None: - """Both files this installer writes into /etc/sudoers.d go through the same + """Every file this installer writes into /etc/sudoers.d goes through the same validate-then-install helper. A malformed file there breaks sudo host-wide, and a non-root-owned or group-writable one is ignored by sudo outright, so assert both - properties for every file rather than only the jobRunAsUser one.""" + properties for each file the installer creates. + + Parameterized over a single path today (the --allow-shutdown rule is the only + one) so that adding a second sudoers file picks up these checks by listing it + here rather than by writing a new test.""" assert path.exists(), f"installer did not create {path}" assert sudo_output("stat", "-f", "%Lp %Su %Sg", str(path)) == "440 root wheel" subprocess.run(["sudo", "visudo", "-cf", str(path)], check=True) From c44d368bb47662603127bdd067fc51b92e0ed7e1 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Mon, 10 Aug 2026 08:49:51 -0700 Subject: [PATCH 17/23] refactor(installer): rename to install_macos.sh and clarify id lookup Review feedback, no behaviour change. * install_darwin.sh -> install_macos.sh, and the user-facing "darwin" wording in the banner, usage and comments becomes "macOS". "darwin" only survives where it must: sys.platform returns it, so the INSTALLER_PATH key stays "darwin". Renamed with git mv so history follows the file. * allocate_system_id -> find_unused_system_id. The function never allocated anything: it returns the highest unused id and the caller then creates the record, so the id is unclaimed in between. The old name promised atomicity the code does not provide. The comment now leads with that instead of noting the race as an afterthought, and the error message matches. * macos_installer_test.yml runs on every PR instead of behind a paths filter. The installer's behaviour depends on more than the installer directory (config defaults, the settings model, the launchd label the e2e suite asserts on), and a filtered job that misses one of those reads as a pass. * Matrix expands from 3.11/3.13 to 3.9-3.13, matching code_quality.yml and requires-python >=3.9. That needs the virtualenv<21 pin for 3.9 that the other workflows in this repo already carry, or `hatch run` fails there with "Environment `default` is incompatible" -- the same failure this change hit on the companion openjd-sessions PR. Also fixes the banner box, whose borders were two characters wider than its text rows before this, and narrows the workflow header's "sudoers rule" to the --allow-shutdown rule, which is the only one the installer writes now. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .github/workflows/macos_installer_test.yml | 20 +++++++----- .../installer/__init__.py | 4 +-- .../{install_darwin.sh => install_macos.sh} | 32 +++++++++++-------- test/e2e/test_job_submissions.py | 2 +- test/integ/macos/test_installer.py | 6 ++-- test/unit/install/test_install.py | 2 +- 6 files changed, 37 insertions(+), 29 deletions(-) rename src/deadline_worker_agent/installer/{install_darwin.sh => install_macos.sh} (96%) diff --git a/.github/workflows/macos_installer_test.yml b/.github/workflows/macos_installer_test.yml index 8c450027..93cb639a 100644 --- a/.github/workflows/macos_installer_test.yml +++ b/.github/workflows/macos_installer_test.yml @@ -1,21 +1,21 @@ name: macOS Installer Test -# Runs the install_darwin.sh integration tests (test/integ/macos/test_installer.py) +# Runs the install_macos.sh integration tests (test/integ/macos/test_installer.py) # on a macOS runner. The tests execute the installer for real (as root) and assert # the security invariants it must establish: user/group isolation, file modes, the -# LaunchDaemon plist, and the sudoers rule. They mutate host state, so they are +# LaunchDaemon plist, and the --allow-shutdown sudoers rule. They mutate host state, so they are # gated behind RUN_INSTALLER_TESTS=true and belong on throwaway CI runners only. # No AWS access is needed: the farm/fleet ids are fakes and the agent never # successfully starts. +# Runs on every PR rather than behind a paths filter: the installer's behaviour depends +# on more than the installer directory (config defaults, the settings model, the launchd +# label used by the e2e suite), and a filtered job that misses one of those reads as a +# pass. on: workflow_dispatch: pull_request: branches: [ mainline, release ] - paths: - - 'src/deadline_worker_agent/installer/**' - - 'test/integ/macos/**' - - '.github/workflows/macos_installer_test.yml' jobs: macos-installer: @@ -25,7 +25,8 @@ jobs: strategy: fail-fast: false matrix: - python-version: ['3.11', '3.13'] + # Matches code_quality.yml and requires-python >=3.9. + python-version: ['3.9', '3.10', '3.11', '3.12', '3.13'] steps: - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -45,7 +46,10 @@ jobs: test -x /opt/wa-venv/bin/deadline-worker-agent - name: Install Hatch - run: pip install --upgrade hatch + # virtualenv 21 removed an API that the hatch version resolvable on 3.9 still calls, + # so `hatch run` fails there with "Environment `default` is incompatible". Same + # constraint the other workflows in this repo use. + run: pip install --upgrade hatch 'virtualenv<21; python_version < "3.10"' - name: Run integration tests env: diff --git a/src/deadline_worker_agent/installer/__init__.py b/src/deadline_worker_agent/installer/__init__.py index b7c2eee5..77dcc0d5 100644 --- a/src/deadline_worker_agent/installer/__init__.py +++ b/src/deadline_worker_agent/installer/__init__.py @@ -26,7 +26,7 @@ INSTALLER_PATH = { "linux": Path(__file__).parent / "install.sh", - "darwin": Path(__file__).parent / "install_darwin.sh", + "darwin": Path(__file__).parent / "install_macos.sh", } @@ -81,7 +81,7 @@ def install() -> None: scripts_path = Path(sysconfig.get_path("scripts")) # The Deadline Virtual File System (VFS) is not supported on macOS. Reject the option here - # so the error surfaces before we shell out to install_darwin.sh (which also rejects it). + # so the error surfaces before we shell out to install_macos.sh (which also rejects it). if sys.platform == "darwin" and args.vfs_install_path: print("ERROR: --vfs-install-path is not supported on macOS.") sys.exit(1) diff --git a/src/deadline_worker_agent/installer/install_darwin.sh b/src/deadline_worker_agent/installer/install_macos.sh similarity index 96% rename from src/deadline_worker_agent/installer/install_darwin.sh rename to src/deadline_worker_agent/installer/install_macos.sh index c58a2b09..50c0ac88 100755 --- a/src/deadline_worker_agent/installer/install_darwin.sh +++ b/src/deadline_worker_agent/installer/install_macos.sh @@ -3,7 +3,7 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. # -# AWS Deadline Cloud Worker Agent Installer (macOS / darwin) +# AWS Deadline Cloud Worker Agent Installer (macOS) # # This is the macOS port of install.sh. It mirrors the Linux installer's flag surface and # overall structure so the deadline_worker_agent.installer dispatcher can invoke it identically @@ -65,7 +65,7 @@ worker_agent_homedir="/var/lib/deadline-worker" usage() { - echo "Usage: install_darwin.sh --farm-id FARM_ID" + echo "Usage: install_macos.sh --farm-id FARM_ID" echo " --fleet-id FLEET_ID" echo " --scripts-path SCRIPTS_PATH" echo " --python-interpreter-path PYTHON_INTERPRETER_PATH" @@ -120,8 +120,8 @@ usage() banner() { echo "===========================================================" - echo "| AWS Deadline Cloud Worker Agent Installer |" - echo "| (macOS / darwin) |" + echo "| AWS Deadline Cloud Worker Agent Installer |" + echo "| (macOS) |" echo "===========================================================" } @@ -147,16 +147,20 @@ user_primary_group_name() { fi } -# Allocate the highest unused ID in [200,500) from a dscl attribute listing. +# Returns the highest unused ID in [200,500) from a dscl attribute listing. +# +# This only FINDS an unused id; it does not reserve one. The caller creates the record, so the +# id is unclaimed between this returning and that write. That is not race-safe against +# concurrent account creation, which matches the Linux installer's useradd behavior under the +# same (root, single-installer) assumption. +# # NOTE: macOS reserves IDs < 500 for hidden/system accounts. Combined with IsHidden=1 this keeps -# the agent account out of the login window. The allocator scans the live directory +# the agent account out of the login window. The search scans the live directory # (dscl -list) at install time, so IDs already taken on the image -- including by # MDM-provisioned accounts -- are skipped; searching downward from 499 also stays clear of # Apple's own low-numbered system accounts. The install must fail if the range is # exhausted rather than pick a UID >= 500 (which would appear in the login window). -# NOTE: not race-safe against concurrent account creation, matching the Linux installer's -# useradd behavior under the same (root, single-installer) assumption. -allocate_system_id() { +find_unused_system_id() { local kind="$1" # "Users" or "Groups" local attr="$2" # "UniqueID" or "PrimaryGroupID" local used candidate @@ -167,7 +171,7 @@ allocate_system_id() { return 0 fi done - echo "ERROR: Could not allocate a free system ${attr} in range [200,500)." >&2 + echo "ERROR: Could not find an unused system ${attr} in range [200,500)." >&2 return 1 } @@ -295,7 +299,7 @@ fi # --- VFS is not supported on macOS: fail loudly ------------------------------------- # DESIGN CHOICE: hard error (exit non-zero) rather than a silent warning, so that a mistaken # VFS request surfaces immediately instead of producing a silently-degraded install. The -# dispatcher (__init__.py) also rejects --vfs-install-path on darwin before we ever get here; +# dispatcher (__init__.py) also rejects --vfs-install-path on macOS before we ever get here; # this is defense-in-depth for direct script invocation. if [[ "${vfs_install_path}" != "unset" ]]; then echo "ERROR: The Deadline Virtual File System (VFS) is not supported on macOS." @@ -376,7 +380,7 @@ if ! user_exists "${wa_user}"; then # First ensure the user's DEDICATED primary group exists (named after the user). # This group -- NOT the job group -- becomes the user's PrimaryGroupID (security invariant). if ! group_exists "${wa_user}"; then - wa_primary_gid=$(allocate_system_id Groups PrimaryGroupID) + wa_primary_gid=$(find_unused_system_id Groups PrimaryGroupID) dscl . -create /Groups/"${wa_user}" dscl . -create /Groups/"${wa_user}" PrimaryGroupID "${wa_primary_gid}" dscl . -create /Groups/"${wa_user}" RealName "${wa_user}" @@ -384,7 +388,7 @@ if ! user_exists "${wa_user}"; then wa_primary_gid=$(dscl . -read /Groups/"${wa_user}" PrimaryGroupID 2>/dev/null | awk '{print $2}') fi - wa_uid=$(allocate_system_id Users UniqueID) + wa_uid=$(find_unused_system_id Users UniqueID) dscl . -create /Users/"${wa_user}" dscl . -create /Users/"${wa_user}" UniqueID "${wa_uid}" dscl . -create /Users/"${wa_user}" PrimaryGroupID "${wa_primary_gid}" @@ -447,7 +451,7 @@ fi # --- Sudoers configuration (--allow-shutdown) --------------------------------------- # macOS shutdown binary lives at /sbin/shutdown (BSD shutdown). The Linux line used # `/usr/sbin/shutdown now`; the BSD invocation is `/sbin/shutdown -h now` (-h = halt/power off). -# The agent invokes `sudo shutdown -h now` on darwin (startup/entrypoint.py:_host_shutdown); +# The agent invokes `sudo shutdown -h now` on macOS (startup/entrypoint.py:_host_shutdown); # sudo resolves `shutdown` to /sbin/shutdown via PATH and matches this rule by full path. # The sudoers command MUST continue to match that argv exactly for the NOPASSWD rule to apply. if [[ "${allow_shutdown}" == "yes" ]]; then diff --git a/test/e2e/test_job_submissions.py b/test/e2e/test_job_submissions.py index ea904c5a..821aa7a1 100644 --- a/test/e2e/test_job_submissions.py +++ b/test/e2e/test_job_submissions.py @@ -34,7 +34,7 @@ LOG = logging.getLogger(__name__) -# launchd service label installed by installer/install_darwin.sh on macOS workers. +# launchd service label installed by installer/install_macos.sh on macOS workers. MACOS_LAUNCHD_LABEL = "com.amazon.deadline.worker-agent" diff --git a/test/integ/macos/test_installer.py b/test/integ/macos/test_installer.py index 482f0111..e58533da 100644 --- a/test/integ/macos/test_installer.py +++ b/test/integ/macos/test_installer.py @@ -1,6 +1,6 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -"""Integration tests for install_darwin.sh. +"""Integration tests for install_macos.sh. These tests run the macOS installer for real (via sudo) and assert the invariants it must establish. They mutate host state -- users, groups, @@ -44,7 +44,7 @@ SUDOERS_PATH = Path("/etc/sudoers.d/deadline-worker-shutdown") REPO_ROOT = Path(__file__).parent.parent.parent.parent -INSTALLER = REPO_ROOT / "src" / "deadline_worker_agent" / "installer" / "install_darwin.sh" +INSTALLER = REPO_ROOT / "src" / "deadline_worker_agent" / "installer" / "install_macos.sh" pytestmark = pytest.mark.skipif( os.environ.get("RUN_INSTALLER_TESTS", "").lower() != "true", @@ -56,7 +56,7 @@ def run_installer(*extra_args: str, check: bool = True) -> subprocess.CompletedProcess: - """Runs install_darwin.sh via sudo with the standard test arguments.""" + """Runs install_macos.sh via sudo with the standard test arguments.""" venv_bin = Path(os.environ["WA_VENV_BIN"]) cmd = [ "sudo", diff --git a/test/unit/install/test_install.py b/test/unit/install/test_install.py index d1a2e207..14fdcb39 100644 --- a/test/unit/install/test_install.py +++ b/test/unit/install/test_install.py @@ -341,7 +341,7 @@ def test_installer_path_has_darwin_entry(self) -> None: # THEN assert ( installer_mod.INSTALLER_PATH["darwin"] - == Path(installer_mod.__file__).parent / "install_darwin.sh" + == Path(installer_mod.__file__).parent / "install_macos.sh" ) def test_runs_expected_subprocess_on_darwin( From 69e1052984179c139acaad5a9e8fb1f944c5bfa9 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:20:23 -0700 Subject: [PATCH 18/23] chore(deps): require openjd-sessions 0.10.14 for the macOS setsid shim The macOS cross-user path needs the pure-Python session-leader shim added in openjd-sessions#335, because macOS ships no setsid(1). That merged as 2df2d99, after the 0.10.13 release, so the previous `== 0.10.13` pin resolved to a version without it -- a macOS install would have had the installer support from this PR and no working impersonation, failing every action at `setsid` with exit 127. 0.10.14 is the first release containing it (see the changelog in openjd-sessions-for-python#347). Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 3018b6b1..c4d4c680 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,7 +15,7 @@ dependencies = [ "boto3 >= 1.34.75", "deadline-job-attachments == 0.1.3", # Pinned to patch version due to Host Config Script runner usage of private OpenJD Sessions API. - "openjd-sessions == 0.10.13", + "openjd-sessions == 0.10.14", "openjd-model >= 0.11.1, < 0.12", # tomli became tomllib in standard library in Python 3.11 "tomli == 2.0.* ; python_version<'3.11'", From 433ed0f6532cb21788c6e22d28d565c0d79453ad Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:39:15 -0700 Subject: [PATCH 19/23] fix(installer): refuse a shared primary group, adopt the account's home, widen id search Four review findings on install_macos.sh. 1. SECURITY: --user naming an existing normal account resolved wa_group to that account's primary group, which on macOS is `staff` (GID 20) for any Setup-Assistant or MDM-created user. wa_group is the group owner of /etc/amazon/deadline (worker.toml, mode 640) and /var/log/amazon/deadline, so that published the agent's config and logs to every local user. Verified on macOS 26.5: three unrelated local accounts share primary group staff. install.sh is safe doing the same thing only because Linux useradd guarantees a dedicated single-member primary group; that guarantee does not exist here. The installer now refuses staff/admin/everyone/wheel/_unknown/nogroup with an error naming the fix. The existing invariant check did not catch this: it compares the primary group against job_group only, so staff passed silently. 2. find_unused_system_id searched a single namespace, so the group and the user could be assigned the same number: on a stock image 499 is free as both a UID and a GID, so the first install took gid=499 then uid=499. It now searches the union of both namespaces. The group record is written before the UID lookup, so the second call sees the first allocation and the two cannot collide. 3. worker_agent_homedir was hardcoded even when --user named an existing account with a different NFSHomeDirectory. launchd derives the daemon's HOME from the user record, not from the plist's WorkingDirectory, so HOME and the CWD pointed at different directories: anything resolving ~ (botocore's ~/.aws, its caches) landed somewhere never provisioned or chowned, while the directory the installer did provision went unused. An existing account's home is now adopted. 4. Corrected the Password '*' comment, which claimed Directory Services returns eDSAuthMethodNotSupported because no AuthenticationAuthority is present. That is not documented and not safe to assert. Kept the mechanism -- `dscl . -read /Users/daemon` shows Apple's own service accounts use exactly Password '*' with no AuthenticationAuthority and UserShell=/usr/bin/false -- but the comment now credits the account's non-interactivity to that combination rather than to '*' being interpreted as "never matches". Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_macos.sh | 70 ++++++++++++++++--- 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_macos.sh b/src/deadline_worker_agent/installer/install_macos.sh index 50c0ac88..ecd73598 100755 --- a/src/deadline_worker_agent/installer/install_macos.sh +++ b/src/deadline_worker_agent/installer/install_macos.sh @@ -161,17 +161,21 @@ user_primary_group_name() { # Apple's own low-numbered system accounts. The install must fail if the range is # exhausted rather than pick a UID >= 500 (which would appear in the login window). find_unused_system_id() { - local kind="$1" # "Users" or "Groups" - local attr="$2" # "UniqueID" or "PrimaryGroupID" local used candidate - used=$(dscl . -list /"${kind}" "${attr}" 2>/dev/null | awk '{print $2}' | sort -n) + # Union of BOTH namespaces. Searching only one lets the group and the user land on the + # same number: on a stock image 499 is free as a UID and as a GID, so the first install + # would take gid=499 and then uid=499. A shared number between two unrelated principals + # is ambiguous for getpwuid/getgrgid round-trips and makes the account harder to audit. + # Callers pass no arguments: both want "an id unused by any principal". + used=$( { dscl . -list /Users UniqueID; dscl . -list /Groups PrimaryGroupID; } 2>/dev/null \ + | awk '{print $2}' | sort -n -u) for candidate in $(seq 499 -1 200); do if ! grep -qx "${candidate}" <<< "${used}"; then echo "${candidate}" return 0 fi done - echo "ERROR: Could not find an unused system ${attr} in range [200,500)." >&2 + echo "ERROR: Could not find an unused system UID/GID in range [200,500)." >&2 return 1 } @@ -312,17 +316,54 @@ fi # group is a SECONDARY membership only (see below). If the user already exists we read its current # primary group; if we are creating the user we will give it a dedicated primary group named after # the user (never the job group). +# SECOND INVARIANT, macOS-specific: wa_group must not be a broadly-shared group. It is used +# as the group owner of the config directory (worker.toml, mode 640) and the agent's logs, so +# every member of it can read them. On Linux `useradd` guarantees a dedicated single-member +# primary group, which is why install.sh can reuse the primary group safely. On macOS the +# primary group of any normal account -- including Setup Assistant and MDM-created ones -- is +# `staff` (GID 20), which contains every local user on the host. Reusing that would publish +# worker.toml and the logs to all of them. +broad_groups=(staff admin everyone wheel _unknown nogroup) +is_broad_group() { + local candidate="$1" g + for g in "${broad_groups[@]}"; do + [[ "${candidate}" == "${g}" ]] && return 0 + done + return 1 +} + if user_exists "${wa_user}"; then wa_group=$(user_primary_group_name "${wa_user}") if [[ -z "${wa_group}" ]]; then # Fall back to the user name if the primary group name could not be resolved. wa_group="${wa_user}" + elif is_broad_group "${wa_group}"; then + echo "ERROR: The primary group of --user ${wa_user} is ${wa_group}, which is shared by" + echo " other users on this host. The worker agent's configuration and logs are" + echo " group-owned by this group, so using it would make them readable by every" + echo " member of ${wa_group}." + echo " Use --user with a dedicated service account whose primary group has no" + echo " other members, or omit --user to let the installer create one." + exit 1 fi else # Newly created user -> its dedicated primary group has the same name as the user. wa_group="${wa_user}" fi +# An existing account already has a home directory recorded in NFSHomeDirectory, and launchd +# derives the daemon's HOME from that record -- not from the plist's WorkingDirectory. Adopt it +# rather than provisioning the default path: otherwise HOME and the CWD point at two different +# directories, anything resolving ~ (botocore's ~/.aws config and caches, for instance) lands +# somewhere the installer never created or chowned, and the directory we did provision goes +# unused. +if user_exists "${wa_user}"; then + existing_homedir=$(dscl . -read /Users/"${wa_user}" NFSHomeDirectory 2>/dev/null | awk '{print $2}') + if [[ -n "${existing_homedir}" ]]; then + worker_agent_homedir="${existing_homedir}" + fi +fi + # Default the job group if not provided via --group. job_group=${job_group:-${default_job_group}} if [[ ! -z "${job_group}" ]] && [[ ! "${job_group}" =~ ^[a-z_]([a-z0-9_-]{0,31}|[a-z0-9_-]{0,30}\$)$ ]]; then @@ -380,7 +421,7 @@ if ! user_exists "${wa_user}"; then # First ensure the user's DEDICATED primary group exists (named after the user). # This group -- NOT the job group -- becomes the user's PrimaryGroupID (security invariant). if ! group_exists "${wa_user}"; then - wa_primary_gid=$(find_unused_system_id Groups PrimaryGroupID) + wa_primary_gid=$(find_unused_system_id) dscl . -create /Groups/"${wa_user}" dscl . -create /Groups/"${wa_user}" PrimaryGroupID "${wa_primary_gid}" dscl . -create /Groups/"${wa_user}" RealName "${wa_user}" @@ -388,7 +429,7 @@ if ! user_exists "${wa_user}"; then wa_primary_gid=$(dscl . -read /Groups/"${wa_user}" PrimaryGroupID 2>/dev/null | awk '{print $2}') fi - wa_uid=$(find_unused_system_id Users UniqueID) + wa_uid=$(find_unused_system_id) dscl . -create /Users/"${wa_user}" dscl . -create /Users/"${wa_user}" UniqueID "${wa_uid}" dscl . -create /Users/"${wa_user}" PrimaryGroupID "${wa_primary_gid}" @@ -398,10 +439,19 @@ if ! user_exists "${wa_user}"; then dscl . -create /Users/"${wa_user}" RealName "AWS Deadline Cloud Worker Agent" # IsHidden=1 keeps a UID<500 account out of the login window / user pickers. dscl . -create /Users/"${wa_user}" IsHidden 1 - # Disable password auth entirely for this service account. '*' matches the - # /etc/master.passwd disabled-account convention; because the account is created without an - # AuthenticationAuthority attribute, Directory Services rejects authentication attempts - # outright (eDSAuthMethodNotSupported) rather than comparing against a password. + # No usable password for this service account. Password '*' with no + # AuthenticationAuthority is the shape Apple's own service accounts have -- `dscl . -read + # /Users/daemon` shows exactly "Password: *" and "UserShell: /usr/bin/false" with no + # AuthenticationAuthority -- so this follows the platform convention rather than inventing + # one. + # + # Deliberately not claiming more than that: what Directory Services does with a non-hash + # sentinel in that attribute is not documented, and it is the absence of an + # AuthenticationAuthority plus UserShell=/usr/bin/false and IsHidden=1 that actually keeps + # this account out of interactive use, not the '*' by itself. The explicit alternatives + # (AuthenticationAuthority ';DisabledUser;' or `pwpolicy -disableuser`) describe a + # *disabled* account, which is a different thing from one that never had credentials, and + # they add a second mechanism to keep working across releases for no clear gain here. dscl . -create /Users/"${wa_user}" Password '*' wa_group="${wa_user}" From cc4409ee4013c3630034d3eed9ce59dcbbab8848 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:54:34 -0700 Subject: [PATCH 20/23] fix(installer): do not take over a shared home directory; fix dscl parsing Three problems, all introduced by the NFSHomeDirectory adoption in 433ed0f. 1. Adopting the recorded home and then unconditionally chown/chmod-ing it could take over a SHARED system directory. Most of Apple's _-prefixed service accounts record /var/empty (root:wheel 0555, also sshd's privsep chroot) and `daemon` records /var/root, so a hand- or MDM-provisioned deadline-worker following that convention would have had /var/empty chowned to the agent user and narrowed to 750 -- a host-wide change well outside this installer's remit. Some service accounts use /dev/null, where `[[ ! -d ]]` passes and `mkdir -p` then fails with "File exists", aborting the install under set -e. The installer now only provisions a directory it creates itself, re-asserts ownership only when the directory is already owned by the agent user (the re-install case), and otherwise warns and leaves it alone. Verified against /var/empty (untouched, warned) and /dev/null (warned, no abort). 2. `awk '{print $2}'` truncated the home directory at the first space: "NFSHomeDirectory: /Users/Deadline Worker" yielded "/Users/Deadline", a different and probably nonexistent path that would then be provisioned and written into the plist while the account's real HOME stayed elsewhere. Uses sed to take the whole value. The same assumption in find_unused_system_id was worse: `dscl . -list` prints "nameid", so a record name containing a space made $2 a fragment of the NAME, dropping that id from the used set and allowing it to be handed out again -- a duplicate-UID install rather than a clean failure. Now takes $NF, and skips lines with no attribute rather than emitting an empty field. 3. worker_agent_homedir went into the plist unescaped while ProgramArguments was escaped, even though it is now read from a Directory Services record rather than being a hard-coded constant. A home directory containing & < or > would produce a plist that is not well-formed XML, which launchd rejects opaquely. Escaped with the same xml_escape helper; verified paths containing "&" and "<>" now lint clean and round-trip through plistlib intact. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_macos.sh | 52 ++++++++++++++++--- 1 file changed, 46 insertions(+), 6 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_macos.sh b/src/deadline_worker_agent/installer/install_macos.sh index ecd73598..261ec244 100755 --- a/src/deadline_worker_agent/installer/install_macos.sh +++ b/src/deadline_worker_agent/installer/install_macos.sh @@ -167,8 +167,12 @@ find_unused_system_id() { # would take gid=499 and then uid=499. A shared number between two unrelated principals # is ambiguous for getpwuid/getgrgid round-trips and makes the account harder to audit. # Callers pass no arguments: both want "an id unused by any principal". + # $NF, not $2: `dscl . -list` prints "nameid", so a record name containing a + # space makes $2 a fragment of the NAME. That id would then be missing from the used set + # and could be handed out again, producing a duplicate-UID install rather than a clean + # failure. The id is always the last field. used=$( { dscl . -list /Users UniqueID; dscl . -list /Groups PrimaryGroupID; } 2>/dev/null \ - | awk '{print $2}' | sort -n -u) + | awk 'NF>1 {print $NF}' | sort -n -u) for candidate in $(seq 499 -1 200); do if ! grep -qx "${candidate}" <<< "${used}"; then echo "${candidate}" @@ -358,7 +362,12 @@ fi # somewhere the installer never created or chowned, and the directory we did provision goes # unused. if user_exists "${wa_user}"; then - existing_homedir=$(dscl . -read /Users/"${wa_user}" NFSHomeDirectory 2>/dev/null | awk '{print $2}') + # sed, not awk '{print $2}': a home directory may contain spaces, and $2 would truncate + # "/Users/Deadline Worker" to "/Users/Deadline" -- a different, probably nonexistent path + # that would then be provisioned and written into the plist while the real HOME stayed + # elsewhere. Failing silently that way is worse than not adopting the value at all. + existing_homedir=$(dscl . -read /Users/"${wa_user}" NFSHomeDirectory 2>/dev/null \ + | sed -n 's/^NFSHomeDirectory: //p') if [[ -n "${existing_homedir}" ]]; then worker_agent_homedir="${existing_homedir}" fi @@ -461,12 +470,37 @@ else fi # --- Create the home directory (macOS does not auto-create it) ---------------------- -if [[ ! -d "${worker_agent_homedir}" ]]; then +# Only provision a directory this installer creates. When --user names a pre-existing account, +# worker_agent_homedir came from its NFSHomeDirectory, which for a service account is +# conventionally a SHARED system path: most of Apple's own _-prefixed accounts record +# /var/empty (root:wheel 0555, also sshd's privsep chroot), and `daemon` records /var/root. +# chown-ing one of those to the agent user and narrowing it to 750 is a host-wide change well +# outside this installer's remit. Some service accounts also use /dev/null, where the old +# `[[ ! -d ]]` test passes and `mkdir -p` then fails with "File exists", aborting the install +# under set -e with an opaque error. +if [[ -e "${worker_agent_homedir}" && ! -d "${worker_agent_homedir}" ]]; then + warning_lines+=( + "The home directory of ${wa_user} (${worker_agent_homedir}) is not a directory." + "Leaving it alone. The agent's HOME will not be writable, which may break anything" + "that resolves ~ (for example a botocore credentials cache)." + ) +elif [[ ! -d "${worker_agent_homedir}" ]]; then echo "Creating worker agent home directory (${worker_agent_homedir})" mkdir -p "${worker_agent_homedir}" + chown "${wa_user}:${wa_group}" "${worker_agent_homedir}" + chmod 750 "${worker_agent_homedir}" +elif [[ "$(stat -f %Su "${worker_agent_homedir}")" == "${wa_user}" ]]; then + # Already ours (a re-install): safe to re-assert ownership and mode. + chown "${wa_user}:${wa_group}" "${worker_agent_homedir}" + chmod 750 "${worker_agent_homedir}" +else + warning_lines+=( + "The home directory of ${wa_user} (${worker_agent_homedir}) already exists and is" + "owned by $(stat -f %Su "${worker_agent_homedir}"), not ${wa_user}. Leaving its" + "ownership and permissions unchanged rather than taking over a directory this" + "installer did not create." + ) fi -chown "${wa_user}:${wa_group}" "${worker_agent_homedir}" -chmod 750 "${worker_agent_homedir}" # --- Create the job group ----------------------------------------------------------- # dseditgroup allocates a system GID and creates the group record. Idempotent via group_exists. @@ -622,6 +656,12 @@ if ! [[ "${no_install_service}" == "yes" ]]; then printf '%s' "${s}" } prog_args_xml=" $(xml_escape "${worker_agent_program}")"$'\n' + # WorkingDirectory takes the same class of value as ProgramArguments and needs the same + # escaping. Since the NFSHomeDirectory adoption above it is read from a Directory Services + # record rather than being a hard-coded constant, so a home directory containing & < or > + # would produce a plist that is not well-formed XML -- which launchd rejects with an + # opaque error, or silently ignores until the next boot when --start was not passed. + working_directory_xml="$(xml_escape "${worker_agent_homedir}")" # launchd has no separate "start on boot" and "start on load" controls: RunAtLoad governs # both, and loading happens at every boot for /Library/LaunchDaemons plists as well as at @@ -656,7 +696,7 @@ if ! [[ "${no_install_service}" == "yes" ]]; then UserName ${wa_user} WorkingDirectory - ${worker_agent_homedir} + ${working_directory_xml} ProgramArguments ${prog_args_xml} From 3580cb66f73312a995ccccfd2728f124c1202c7b Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:37:25 -0700 Subject: [PATCH 21/23] fix(installer): close the broad-group bypasses, parse dscl values structurally Five review findings, most of them fallout from my own earlier changes. 1. SECURITY: the is_broad_group check only ran inside the `user_exists` branch, so `--user staff` bypassed it entirely -- the account does not exist, wa_group becomes "staff", and the group-creation block then adopts the existing staff record (GID 20) as the new account's primary group. Same exposure the check was added to prevent, by a different route. Hoisted out of the conditional so it runs on the resolved wa_group in both paths. Verified `--user admin` is also caught, since admin's primary group resolves to staff. 2. SECURITY: job_group had no such check at all, and it is group owner of /var/lib/deadline and queues/ (0750) and the session root (0755) -- modes deliberately loosened so job users can reach them. `--group staff` would let any local user list queues/ and traverse into session directories to read job attachment inputs and generated scripts. credentials/ stays 0700, so AWS credentials were never exposed. Now checked after its syntax validation. 3. `sed -n 's/^NFSHomeDirectory: //p'` failed for the exact case it was added for. `dscl -read` prints a whitespace-containing value on an indented CONTINUATION line, not inline -- confirmed: `RealName:` then " Choquette, Andy", versus "NFSHomeDirectory: /var/empty" inline. So a spaced home came back empty and the installer silently fell back to the default, provisioning a directory that is not the account's home. Replaced with a dscl_read_value helper using `dscl -plist` + `plutil -extract`, which is unambiguous for both shapes, and routed the PrimaryGroupID reads through it too so there is one parsing path. 4. An unusable WorkingDirectory is fatal, not cosmetic: launchd chdir()s before exec, so /dev/null (a real service-account home) makes every spawn fail with KeepAlive throttling the retry -- a service that never runs. It now falls back to /var/lib/deadline, which this installer provisions, and the warning says the real consequence instead of implying only ~ is affected. 5. Fail loudly when an adopted group record has no PrimaryGroupID, rather than running `dscl -create ... PrimaryGroupID ""`. Also documents the session-root traversal divergence from Linux (id 3762317005): nesting under 0750 /var/lib/deadline means traversal needs job_group membership, which /sessions on Linux does not. Left as-is deliberately -- it is tighter than Linux and matches the documented model where every jobRunAsUser is in the shared job group -- but a jobRunAsUser outside that group gets EACCES, which the worker-host docs need to state. NOT changed: the claim that `set -e` aborts before the `-z "${wa_group}"` fallback because user_primary_group_name returns 1. An `if` whose condition is false and which has no `else` returns 0, so the function returns 0 and the fallback is reachable; verified with a nonexistent user (reached the fallback, exit 0) and with a gid matching no group record (`dscl -search` exits 0 on no match). Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_macos.sh | 107 ++++++++++++++---- 1 file changed, 88 insertions(+), 19 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_macos.sh b/src/deadline_worker_agent/installer/install_macos.sh index 261ec244..e6377532 100755 --- a/src/deadline_worker_agent/installer/install_macos.sh +++ b/src/deadline_worker_agent/installer/install_macos.sh @@ -138,10 +138,24 @@ group_exists() { dscl . -read /Groups/"$1" &> /dev/null } +# Read a single-valued attribute from a Directory Services record. +# +# `dscl -read` has two output shapes and the value's content decides which: a value with no +# whitespace is printed inline ("NFSHomeDirectory: /var/empty"), while one containing spaces +# is printed on an indented CONTINUATION line under a bare "NFSHomeDirectory:" header. Any +# `awk '{print $2}'` or `sed 's/^Key: //'` parse therefore handles one shape and silently +# returns the wrong thing (or nothing) for the other. Ask for a plist and extract the value +# structurally instead, which is unambiguous for both. +dscl_read_value() { + local record="$1" attr="$2" + dscl -plist . -read "${record}" "${attr}" 2>/dev/null \ + | plutil -extract "dsAttrTypeStandard:${attr}.0" raw - 2>/dev/null || true +} + # Primary group NAME of a user (Linux `id -gn` equivalent). Resolves PrimaryGroupID -> group name. user_primary_group_name() { local u="$1" gid - gid=$(dscl . -read /Users/"$u" PrimaryGroupID 2>/dev/null | awk '{print $2}') + gid=$(dscl_read_value /Users/"$u" PrimaryGroupID) if [[ -n "${gid}" ]]; then dscl . -search /Groups PrimaryGroupID "${gid}" 2>/dev/null | awk 'NR==1{print $1}' fi @@ -341,20 +355,27 @@ if user_exists "${wa_user}"; then if [[ -z "${wa_group}" ]]; then # Fall back to the user name if the primary group name could not be resolved. wa_group="${wa_user}" - elif is_broad_group "${wa_group}"; then - echo "ERROR: The primary group of --user ${wa_user} is ${wa_group}, which is shared by" - echo " other users on this host. The worker agent's configuration and logs are" - echo " group-owned by this group, so using it would make them readable by every" - echo " member of ${wa_group}." - echo " Use --user with a dedicated service account whose primary group has no" - echo " other members, or omit --user to let the installer create one." - exit 1 fi else # Newly created user -> its dedicated primary group has the same name as the user. wa_group="${wa_user}" fi +# Checked on the RESOLVED wa_group, outside the branches above, so both paths are covered. +# Gating this on user_exists would miss `--user staff`: the account does not exist, so +# wa_group becomes "staff", and the group-creation block below then adopts the existing +# `staff` record (GID 20) as the new account's primary group -- the same exposure by a +# different route. +if is_broad_group "${wa_group}"; then + echo "ERROR: ${wa_group} would be the worker agent user's primary group, and it is shared" + echo " by other users on this host. The agent's configuration and logs are" + echo " group-owned by that group, so this would make them readable by every one of" + echo " its members." + echo " Use --user with a dedicated service account whose primary group has no other" + echo " members, or omit --user to let the installer create one." + exit 1 +fi + # An existing account already has a home directory recorded in NFSHomeDirectory, and launchd # derives the daemon's HOME from that record -- not from the plist's WorkingDirectory. Adopt it # rather than provisioning the default path: otherwise HOME and the CWD point at two different @@ -362,12 +383,12 @@ fi # somewhere the installer never created or chowned, and the directory we did provision goes # unused. if user_exists "${wa_user}"; then - # sed, not awk '{print $2}': a home directory may contain spaces, and $2 would truncate - # "/Users/Deadline Worker" to "/Users/Deadline" -- a different, probably nonexistent path - # that would then be provisioned and written into the plist while the real HOME stayed - # elsewhere. Failing silently that way is worse than not adopting the value at all. - existing_homedir=$(dscl . -read /Users/"${wa_user}" NFSHomeDirectory 2>/dev/null \ - | sed -n 's/^NFSHomeDirectory: //p') + # Read via dscl_read_value: a home directory containing spaces is printed by `dscl -read` + # on a continuation line rather than inline, so both a field-split and a "^Key: " sed parse + # come back empty for exactly the case that matters. Silently falling back to the default + # would provision and chown a directory that is not the account's home while launchd still + # took HOME from the record. + existing_homedir=$(dscl_read_value /Users/"${wa_user}" NFSHomeDirectory) if [[ -n "${existing_homedir}" ]]; then worker_agent_homedir="${existing_homedir}" fi @@ -380,6 +401,20 @@ if [[ ! -z "${job_group}" ]] && [[ ! "${job_group}" =~ ^[a-z_]([a-z0-9_-]{0,31}| usage fi +# Same broad-group check as wa_group, for the same reason. job_group owns the persistence tree +# (0750) and the session root (0755), and those modes are deliberately loosened so job users +# can reach them -- so a shared group here is worse, not better: `--group staff` would let any +# local user list queues/ and traverse into session directories to read job attachment inputs +# and generated scripts. (credentials/ stays 0700, so AWS credentials remain protected.) +# Syntax validation above does not catch this; `staff` is a perfectly well-formed group name. +if is_broad_group "${job_group}"; then + echo "ERROR: --group ${job_group} is shared by other users on this host. The worker agent's" + echo " persistence and session directories are group-owned by it, so this would give" + echo " every member of ${job_group} access to job data." + echo " Use a dedicated job group, or omit --group to use ${default_job_group}." + exit 1 +fi + banner echo @@ -435,7 +470,13 @@ if ! user_exists "${wa_user}"; then dscl . -create /Groups/"${wa_user}" PrimaryGroupID "${wa_primary_gid}" dscl . -create /Groups/"${wa_user}" RealName "${wa_user}" else - wa_primary_gid=$(dscl . -read /Groups/"${wa_user}" PrimaryGroupID 2>/dev/null | awk '{print $2}') + wa_primary_gid=$(dscl_read_value /Groups/"${wa_user}" PrimaryGroupID) + if [[ -z "${wa_primary_gid}" ]]; then + # Otherwise the `dscl -create ... PrimaryGroupID ""` below would write an empty + # value instead of failing with something an operator can act on. + echo "ERROR: group ${wa_user} exists but has no PrimaryGroupID." >&2 + exit 1 + fi fi wa_uid=$(find_unused_system_id) @@ -481,8 +522,10 @@ fi if [[ -e "${worker_agent_homedir}" && ! -d "${worker_agent_homedir}" ]]; then warning_lines+=( "The home directory of ${wa_user} (${worker_agent_homedir}) is not a directory." - "Leaving it alone. The agent's HOME will not be writable, which may break anything" - "that resolves ~ (for example a botocore credentials cache)." + "Leaving it alone. HOME will point at it regardless, since launchd takes HOME from" + "the account record, so anything resolving ~ (a botocore credentials cache, for" + "example) will fail. The service's WorkingDirectory falls back to a real directory" + "so the agent still starts." ) elif [[ ! -d "${worker_agent_homedir}" ]]; then echo "Creating worker agent home directory (${worker_agent_homedir})" @@ -584,6 +627,16 @@ if [ -f /var/lib/deadline/worker.json ]; then fi echo "Done provisioning persistence directory (/var/lib/deadline)" +# DIVERGENCE FROM LINUX, deliberate: the default session root is nested under +# /var/lib/deadline (0750 wa_user:job_group), whereas Linux's /sessions is top-level under / +# (0755). Traversing into a session directory here therefore requires membership in +# ${job_group}, which the 0755 on the session root itself cannot grant -- the search bit is +# missing one level up. That matches the documented model, where every jobRunAsUser is a member +# of the shared job group, and it is deliberately tighter than Linux: a job user outside the +# group cannot reach other queues' session directories. A jobRunAsUser that is NOT in +# ${job_group} will get EACCES on its own session path, which is a misconfiguration the +# worker-host documentation needs to state (macOS cannot put this at / because the root volume +# is sealed read-only). echo "Provisioning root directory for OpenJD Sessions (${session_root_dir})" mkdir -p "${session_root_dir}" chown "${wa_user}:${job_group}" "${session_root_dir}" @@ -661,7 +714,23 @@ if ! [[ "${no_install_service}" == "yes" ]]; then # record rather than being a hard-coded constant, so a home directory containing & < or > # would produce a plist that is not well-formed XML -- which launchd rejects with an # opaque error, or silently ignores until the next boot when --start was not passed. - working_directory_xml="$(xml_escape "${worker_agent_homedir}")" + # launchd chdir()s into WorkingDirectory before exec, so an unusable value is fatal rather + # than cosmetic: the spawn fails with ENOTDIR and KeepAlive throttles the retry, leaving a + # service that never runs and reports only a nonzero last exit status. worker_agent_homedir + # can be such a path -- an adopted NFSHomeDirectory of /dev/null, for instance, which the + # provisioning block above deliberately leaves alone. Fall back to a directory this + # installer definitely created so the daemon still starts. + working_directory="${worker_agent_homedir}" + if [[ ! -d "${working_directory}" ]]; then + working_directory=/var/lib/deadline + warning_lines+=( + "The home directory of ${wa_user} (${worker_agent_homedir}) is not a usable" + "directory, so the service's WorkingDirectory was set to ${working_directory}" + "instead. HOME still comes from the account record, so anything resolving ~ will" + "not work until that directory exists." + ) + fi + working_directory_xml="$(xml_escape "${working_directory}")" # launchd has no separate "start on boot" and "start on load" controls: RunAtLoad governs # both, and loading happens at every boot for /Library/LaunchDaemons plists as well as at From 6a9674976b9046f67d9c1f5f6adb41b420d3d626 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:07:34 -0700 Subject: [PATCH 22/23] fix(test): make the e2e suite type-check Two mypy errors, both surfacing in the E2E Lint job. 1. conftest.py:621 -- deadline-cloud-test-fixtures types OperatingSystem.name as Literal["AL2023", "WIN2022"], so `OperatingSystem(name="MACOS")` is rejected. That package needs macOS support before this can type-check properly (it also needs a MacInstanceWorker: the posix worker hardcodes an AL2023 AMI and provisions with useradd/groupadd, neither of which exists on macOS). Nothing sets OPERATING_SYSTEM=macos in CI yet, so the branch is unreachable today and is kept only so the plumbing is in place; narrow `# type: ignore[arg-type]` with a comment saying what removes it. 2. test_session_runtime.py:544 -- PRE-EXISTING, not from this branch: mainline alone fails this same check, in a file added by #1035 that this PR does not touch. `worker.worker_id` is `str | None` and `is_worker_started` wants `str`; the `assert ... is not None` above does not narrow inside the nested function, because an attribute could change between the assert and the call. Bound to a local so the narrowing holds. Fixed here rather than deferred because it blocks the E2E Lint job for every PR, this one included. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- test/e2e/conftest.py | 11 +++++++---- test/e2e/test_session_runtime.py | 6 +++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/test/e2e/conftest.py b/test/e2e/conftest.py index e7053394..2a4ab120 100644 --- a/test/e2e/conftest.py +++ b/test/e2e/conftest.py @@ -615,10 +615,13 @@ def operating_system() -> OperatingSystem: elif os_env_var == "windows": return OperatingSystem(name="WIN2022") elif os_env_var == "macos": - # NOTE: requires a deadline-cloud-test-fixtures release whose - # OperatingSystem/worker fixtures accept a macOS platform (EC2 Mac - # dedicated hosts). The test-suite plumbing here is ready ahead of that. - return OperatingSystem(name="MACOS") + # deadline-cloud-test-fixtures types this as Literal["AL2023", "WIN2022"], so mypy + # rejects "MACOS" until that package gains macOS support (it also needs a + # MacInstanceWorker: the posix worker hardcodes an AL2023 AMI and provisions with + # useradd/groupadd). Nothing sets OPERATING_SYSTEM=macos in CI yet, so this branch is + # unreachable today and kept only so the plumbing is in place; the ignore comes off + # with that release. + return OperatingSystem(name="MACOS") # type: ignore[arg-type] else: assert False, ( f'Expected OPERATING_SYSTEM env var to be "linux", "windows", or "macos", ' diff --git a/test/e2e/test_session_runtime.py b/test/e2e/test_session_runtime.py index deeabde0..bcdb5f79 100644 --- a/test/e2e/test_session_runtime.py +++ b/test/e2e/test_session_runtime.py @@ -529,6 +529,10 @@ def check_worker_service_stopped() -> None: # Wait for the worker to come back online assert worker.worker_id is not None + # Bound to a local: mypy does not carry the narrowing from the assert above into the + # nested function, since worker.worker_id is an attribute that could change between + # the assert and the call. + worker_id = worker.worker_id @backoff.on_exception( backoff.constant, @@ -541,7 +545,7 @@ def wait_worker_started() -> None: deadline_client=deadline_client, farm_id=deadline_resources.farm.id, fleet_id=deadline_resources.fleet.id, - worker_id=worker.worker_id, + worker_id=worker_id, ) wait_worker_started() From a8906ef9b7adb09eec2271756420ca6996e9f407 Mon Sep 17 00:00:00 2001 From: andychoquette <78888816+andychoquette@users.noreply.github.com> Date: Tue, 11 Aug 2026 20:24:41 -0700 Subject: [PATCH 23/23] fix(installer): resolve the primary group exactly, and fail early when it cannot be Two findings on user_primary_group_name and its caller. 1. `dscl . -search` is documented as a SUBSTRING match on the attribute value, so searching for gid 20 can also match 120/200/2000, and `awk 'NR==1{print $1}'` then took whichever record dscl emitted first. Replaced with the conventional `dscl . -list` plus an exact `$NF == gid` comparison, using $NF rather than $2 so a group name containing a space cannot shift the fields, and gave the function an explicit `return 0` so its status no longer depends on the last command run. A misresolution here is not cosmetic: the result becomes the group owner of /etc/amazon/deadline (0750), worker.toml (0640) and the agent's logs, and it gates the job-group-is-not-the-primary-group invariant. is_broad_group cannot catch it, because it checks the resolved NAME and a wrongly-resolved name is not in broad_groups. Note the substring behaviour did not reproduce on macOS 26.5 -- `-search 20` returned only staff, though eight groups on this host have GIDs containing "20" -- so this is hardening against documented behaviour rather than an observed misresolution. The `-list` form is deterministic either way, and the exit-status fix is real regardless. 2. The `[[ -z "${wa_group}" ]]` fallback named a group with no directory record: the block that creates /Groups/${wa_user} only runs on the create-USER path, so on this branch nothing ever creates it, and every later `chown "${wa_user}:${wa_group}"` would fail with "invalid group" -- aborting under set -e possibly after the job group, its membership, and the sudoers rule were already applied. Now a hard error before anything is mutated, naming the three ways out. Reachable without any resolver bug: a PrimaryGroupID pointing at a GID whose group record no longer exists resolves to nothing. Verified: gid 20 -> staff, 204 -> _developer (not staff), 0 -> wheel, 99999 -> empty; the full resolution flow survives set -e for a normal user, a service account and a to-be-created user; and the dangling-PrimaryGroupID case now exits 1 up front. Signed-off-by: andychoquette <78888816+andychoquette@users.noreply.github.com> --- .../installer/install_macos.sh | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/deadline_worker_agent/installer/install_macos.sh b/src/deadline_worker_agent/installer/install_macos.sh index e6377532..af36cef6 100755 --- a/src/deadline_worker_agent/installer/install_macos.sh +++ b/src/deadline_worker_agent/installer/install_macos.sh @@ -157,8 +157,23 @@ user_primary_group_name() { local u="$1" gid gid=$(dscl_read_value /Users/"$u" PrimaryGroupID) if [[ -n "${gid}" ]]; then - dscl . -search /Groups PrimaryGroupID "${gid}" 2>/dev/null | awk 'NR==1{print $1}' + # `dscl . -list` plus an exact field comparison, NOT `dscl . -search`. -search is + # documented as a substring match on the attribute value, so searching for gid 20 can + # also match 120/200/2000, and the previous `awk 'NR==1{print $1}'` then took whichever + # record dscl emitted first. A wrong answer here is not cosmetic: the result becomes the + # group owner of /etc/amazon/deadline (0750), worker.toml (0640) and the agent's logs, + # and it also gates the job-group-is-not-the-primary-group invariant. is_broad_group + # cannot catch a misresolution, because it checks the resolved NAME and a wrong name is + # not in broad_groups. + # + # $NF rather than $2 for the same reason find_unused_system_id uses it: `-list` prints + # "nameid", so a group name containing a space shifts the fields. + dscl . -list /Groups PrimaryGroupID 2>/dev/null \ + | awk -v want="${gid}" 'NF>1 && $NF == want {print $1; exit}' || true fi + # Always succeed: callers decide what an empty result means. Without this the function's + # status is whatever the last command happened to return. + return 0 } # Returns the highest unused ID in [200,500) from a dscl attribute listing. @@ -353,8 +368,18 @@ is_broad_group() { if user_exists "${wa_user}"; then wa_group=$(user_primary_group_name "${wa_user}") if [[ -z "${wa_group}" ]]; then - # Fall back to the user name if the primary group name could not be resolved. - wa_group="${wa_user}" + # Fail here rather than falling back to "${wa_user}". That name has no group record on + # this path -- the block that creates /Groups/${wa_user} only runs when the USER is + # being created -- so every later `chown ${wa_user}:${wa_group}` would fail with + # "invalid group", and under set -e the install would stop somewhere in the middle, + # possibly after the job group, the group membership and the sudoers rule were already + # in place. Reachable without any resolver bug: a PrimaryGroupID pointing at a GID whose + # group record no longer exists (a removed MDM group, say) resolves to nothing. + echo "ERROR: could not resolve the primary group of the existing user ${wa_user}." + echo " Its PrimaryGroupID does not correspond to any group on this host." + echo " Fix that account's PrimaryGroupID, or pass --user with a different" + echo " account, or omit --user to let the installer create one." + exit 1 fi else # Newly created user -> its dedicated primary group has the same name as the user.