Stage the macOS payload where only root can change it

CHANGELOG.md

@@ -5,6 +5,7 @@ All notable OpenBeat changes should be summarized here for users. GitHub Release
 ## Unreleased
 
 - Installer updates now prepare and verify the new files before replacing the installed ones. If an update fails or is interrupted, the previous installation is kept or restored by running the installer again.
+- The macOS installer stages its files in a folder only root can change, instead of a shared temporary folder, and stops with a clear error if it cannot tell which user to install for.
 - Release builds use one resolved commit for testing, building, and tagging, and pinned build dependencies.
 - When beat analysis or click-track rendering fails, OpenBeat now shows the helper's error instead of a missing-file error, and no longer imports an older click track as if it were new.
 - On Windows, OpenBeat should now work when the Windows user name or the media path contains non-English characters, such as `José` or `Música`. Subtitle export to such folders is not fixed yet.

docs/development.md

@@ -53,6 +53,8 @@ Replace `--upgrade` with `--upgrade-package <name>` to move a single package, or
 
 Both installers stage the new `Modules/OpenBeat` and `Scripts/Utility/OpenBeat` folders next to the installed ones as `.OpenBeat.new`, verify them against SHA-256 manifests written by `build_installers.py`, and only then rename the installed folders to `.OpenBeat.old` and the staged folders into place. Any failure renames `.OpenBeat.old` back, and the next run restores a `.OpenBeat.old` folder left by an interrupted update before it stages anything. The macOS package does this in `scripts/macos_postinstall.sh` and the Windows installer in `scripts/staged_install.py`; `tests/test_installer_updates.py` runs the same failed-update scenarios against both.
 
+The macOS package installs its payload into `/Library/Application Support/OpenBeat/Installer`, and the postinstall script, which runs as root, checks that folder and works out the logged-in user, then copies the payload into that user's Resolve folder as the user (through `sudo -u`), so nothing in their home folder is changed with root's permissions. The script refuses to run unless that folder and every folder above it belong to root and only root can write to them, so another user on the Mac cannot place or swap the files it copies, and it deletes the folder afterwards. With nobody logged in, for example `sudo installer -pkg ... -target /` over SSH, it installs for the user who ran `sudo`; it stops with an error rather than install for root or a system account.
+
 ## Run Tests
 
 Install Lua 5.1 or a later Lua release, then run `python -m unittest discover -s tests -v`. The suite tests timecode conversion, source selection, marker replacement and rollback, and subtitle timing without an open Resolve project. It also sends generated audio through the real CLI and checks both click-track modes.

docs/install.md

@@ -46,6 +46,8 @@ Install the newer package over the old one, then restart Resolve. The installer
 
 The installer copies the new files next to your current installation and checks them before replacing anything. If copying, checking, or replacing fails, for example because the disk is full or Resolve still has OpenBeat files open, your current installation is kept and the installer reports that it did not complete. Close Resolve and run the installer again. On macOS, Installer's log (**Window > Installer Log**) shows the reason.
 
+On macOS, OpenBeat 0.0.1 could leave some Resolve folders owned by root if Resolve had never been opened. If the installer log says OpenBeat cannot write to a folder, run the `sudo chown` command it shows in Terminal, then run the installer again.
+
 If an update is cut off part way through, for example by a power failure, run the installer again. It first puts back the previous installation it had set aside, then repeats the update. It also removes any `.OpenBeat.new`, `.OpenBeat.old`, or `.OpenBeat.discard` folders left next to the OpenBeat folders.
 
 ## Uninstall

scripts/build_installers.py

@@ -13,6 +13,10 @@
 # Installed folders, relative to the Resolve Fusion support folder. The installers
 # (staged_install.py and macos_postinstall.sh) replace exactly these folders.
 PAYLOAD_DIRS = ("Modules/OpenBeat", "Scripts/Utility/OpenBeat")
+# Where the macOS package installs its payload before the postinstall script copies
+# it into the user's Resolve folder. It must match OPENBEAT_STAGING in
+# macos_postinstall.sh, which refuses to run unless root alone can write to it.
+MACOS_STAGING = "/Library/Application Support/OpenBeat/Installer"
 
 
 def run(cmd: list[str], cwd: Path | None = None) -> None:
@@ -122,11 +126,21 @@ def build_payload(payload_root: Path, cli_binary: Path, platform: str) -> None:
         write_manifest(payload_root / relative)
 
 
+def restrict_modes(root: Path) -> None:
+    """Make every folder and file writable only by its owner, which the package sets to root."""
+    for path in [root, *root.rglob("*")]:
+        if path.is_symlink():
+            raise ValueError(f"The macOS package payload cannot contain symlinks: {path}")
+        executable = path.is_dir() or path.stat().st_mode & 0o111
+        path.chmod(0o755 if executable else 0o644)
+
+
 def create_macos_pkg(cli_binary: Path, version: str) -> Path:
     with tempfile.TemporaryDirectory(prefix="openbeat-macos-installer-") as tmp:
         tmp_path = Path(tmp)
         root_dir = tmp_path / "root"
         build_payload(root_dir / "payload", cli_binary, "macos")
+        restrict_modes(root_dir)
 
         scripts_dir = tmp_path / "scripts"
         scripts_dir.mkdir(parents=True, exist_ok=True)
@@ -144,10 +158,12 @@ def create_macos_pkg(cli_binary: Path, version: str) -> Path:
                 version,
                 "--root",
                 str(root_dir),
+                "--ownership",
+                "recommended",
                 "--scripts",
                 str(scripts_dir),
                 "--install-location",
-                "/private/tmp/OpenBeatInstaller",
+                MACOS_STAGING,
                 str(pkg_path),
             ]
         )

scripts/macos_postinstall.sh

@@ -14,13 +14,24 @@
 #    and discard the .OpenBeat.old folders. While that marker exists the update is
 #    complete, so a later run deletes leftover folders instead of restoring them.
 #
+# The package installs the payload under OPENBEAT_STAGING on the target volume.
+# That folder and every folder above it must belong to root and be writable only
+# by root, so no other user can place or swap files the script then copies as
+# root. The script refuses to run otherwise and removes the folder when done.
+#
 # The installer runs openbeat_main. The other functions take explicit paths so
 # tests can source this file and run them against temporary folders. macOS runs
 # this with bash 3.2, so avoid arrays and newer bash features.
 
 OPENBEAT_TARGETS="Modules/OpenBeat Scripts/Utility/OpenBeat"
+# Relative to the target volume; build_installers.py passes it to pkgbuild as
+# --install-location.
+OPENBEAT_STAGING="Library/Application Support/OpenBeat/Installer"
+# Owner required for the staging folders. Tests override it after sourcing.
+OPENBEAT_TRUSTED_UID=0
 OPENBEAT_ROOT=""
 OPENBEAT_CREATED=""
+OPENBEAT_USER=""
 
 openbeat_log() {
   echo "OpenBeat: $*"
@@ -41,7 +52,7 @@ openbeat_rename() {
 
 # Checks that directory $1 holds every file listed in manifest $2 with the recorded digest.
 openbeat_verify() {
-  local directory="$1" manifest="$2" line digest name result
+  local directory="$1" manifest="$2" line digest name result listed present
   if [ ! -s "$manifest" ]; then
     openbeat_log "The installer payload manifest $manifest is missing or empty"
     return 1
@@ -66,6 +77,14 @@ openbeat_verify() {
     printf '%s\n' "$result" | grep -v ': OK$'
     return 1
   fi
+  # The staging folder is a fixed path, so files left there by an earlier package
+  # could be copied along; install nothing the manifest does not list.
+  listed="$(sed 's/^[0-9a-f]*  //' "$manifest" | LC_ALL=C sort)"
+  present="$(cd "$directory" && find . ! -type d | sed 's|^\./||' | LC_ALL=C sort)" || return 1
+  if [ "$listed" != "$present" ]; then
+    openbeat_log "The prepared files in $directory include files the installer payload does not list"
+    return 1
+  fi
 }
 
 # Puts every .OpenBeat.old folder under Fusion root $1 back in place and removes
@@ -140,12 +159,39 @@ openbeat_swap() {
 }
 
 openbeat_write_config() {
-  printf 'return {\n  python_bin = "%s",\n}\n' "$2" > "$1/OpenBeatConfig.local.lua"
+  local path
+  # Escape the path for a Lua string. sed, because bash 3.2 and 5 treat backslashes
+  # in ${var//pattern/replacement} differently.
+  path="$(printf '%s' "$2" | sed -e 's/\\/\\\\/g' -e 's/"/\\"/g')" || return 1
+  printf 'return {\n  python_bin = "%s",\n}\n' "$path" > "$1/OpenBeatConfig.local.lua"
+}
+
+# Fails, explaining the fix, if a Resolve folder OpenBeat installs into exists but
+# the installing user cannot write to it. OpenBeat 0.0.1 created missing ones as
+# root. Root does not change their owner: it cannot tell which root-owned folders in
+# a home folder OpenBeat made, and the user can rename folders there.
+# Prints $1 single-quoted for pasting into a shell, whatever characters it contains.
+openbeat_shell_quote() {
+  printf "'%s'" "$(printf '%s' "$1" | sed "s/'/'\\\\''/g")"
+}
+
+openbeat_check_writable() {
+  local root="$1" folder blocked=""
+  for folder in "$root" "$root/Modules" "$root/Scripts" "$root/Scripts/Utility"; do
+    if [ -e "$folder" ] && [ ! -w "$folder" ]; then
+      blocked="$blocked $(openbeat_shell_quote "$folder")"
+    fi
+  done
+  if [ -n "$blocked" ]; then
+    openbeat_log "OpenBeat cannot write to some Resolve folders, which may belong to root after an earlier OpenBeat installer. In Terminal, run: sudo chown $(openbeat_shell_quote "$(id -un)")$blocked and then run this installer again."
+    return 1
+  fi
 }
 
 openbeat_install_steps() {
-  local payload="$1" root="$2" owner="$3" relative parent staged
+  local payload="$1" root="$2" relative parent staged
 
+  openbeat_check_writable "$root" || return 1
   if ! openbeat_recover "$root"; then
     openbeat_log "A previous update was interrupted and could not be completed or undone automatically"
     return 1
@@ -164,8 +210,7 @@ openbeat_install_steps() {
 
   staged="$root/Modules/.OpenBeat.new"
   if ! { chmod 755 "$staged/bin/openbeat" &&
-    openbeat_write_config "$staged" "$root/Modules/OpenBeat/bin/openbeat" &&
-    chown -R "$owner" "$staged" "$root/Scripts/Utility/.OpenBeat.new"; }; then
+    openbeat_write_config "$staged" "$root/Modules/OpenBeat/bin/openbeat"; }; then
     openbeat_log "Could not configure the new files; the existing installation was not changed"
     openbeat_restore "$root"
     return 1
@@ -193,7 +238,8 @@ openbeat_install_steps() {
   return 0
 }
 
-# Installs payload folder $1 into Resolve Fusion folder $2, owned by $3 (user:group).
+# Installs payload folder $1 into Resolve Fusion folder $2, with the permissions of
+# whoever runs it. openbeat_main runs it as the target user (see openbeat_as_user).
 openbeat_install() {
   local status=0
   OPENBEAT_ROOT="$2"
@@ -205,20 +251,133 @@ openbeat_install() {
   return "$status"
 }
 
+# What openbeat_install needs, passed to the shell openbeat_install_as_user starts.
+OPENBEAT_INSTALL_FUNCTIONS="openbeat_log openbeat_exists openbeat_rename openbeat_verify openbeat_restore
+  openbeat_finish openbeat_recover openbeat_abort openbeat_swap openbeat_write_config
+  openbeat_shell_quote openbeat_check_writable openbeat_install_steps openbeat_install"
+
+# Runs command $2... as user $1. Tests replace this function.
+openbeat_as_user() {
+  local user="$1"
+  shift
+  /usr/bin/sudo -u "$user" -H "$@"
+}
+
+# Installs payload $2 into Fusion root $3 as user $1. The Fusion folder is in the
+# user's home folder, where their own processes can swap files and symlinks at any
+# time, so root must not copy, verify, or change permissions there: a swapped path
+# could make root change or reveal a file the user cannot.
+openbeat_install_as_user() {
+  local definitions
+  # shellcheck disable=SC2086 # The list is split into function names on purpose.
+  definitions="$(declare -f $OPENBEAT_INSTALL_FUNCTIONS)" || return 1
+  openbeat_as_user "$1" /bin/bash -c "set -euo pipefail
+OPENBEAT_TARGETS='$OPENBEAT_TARGETS'
+$definitions
+openbeat_install \"\$1\" \"\$2\"" openbeat-postinstall "$2" "$3"
+}
+
+# Prints the access control entries of $1, one per line. Tests replace this function.
+openbeat_acl_entries() {
+  local listing
+  listing="$(ls -lde "$1")" || return 1
+  printf '%s\n' "$listing" | sed 1d
+}
+
+# Succeeds if folder $1 is a real folder owned by OPENBEAT_TRUSTED_UID that no other
+# user can write to, through its mode or an access control entry, so nobody else can
+# rename or replace anything in it.
+openbeat_check_trusted() {
+  local owner_mode owner mode acl
+  if [ -L "$1" ] || [ ! -d "$1" ]; then
+    openbeat_log "Refusing to install: $1 is not a folder"
+    return 1
+  fi
+  owner_mode="$(stat -f '%u %Lp' "$1")" || owner_mode=""
+  owner="${owner_mode%% *}"
+  mode="${owner_mode#* }"
+  case "$mode" in
+    "" | *[!0-7]*) mode=777 ;;
+  esac
+  if [ "$owner" != "$OPENBEAT_TRUSTED_UID" ] || [ $((8#$mode & 8#22)) -ne 0 ]; then
+    openbeat_log "Refusing to install: $1 must belong to root and be writable only by root"
+    return 1
+  fi
+  # macOS folders such as / and /Library carry only deny entries.
+  if ! acl="$(openbeat_acl_entries "$1")" ||
+    printf '%s\n' "$acl" | grep -Eq ' allow .*(write|append|add_file|add_subdirectory|delete|chown)'; then
+    openbeat_log "Refusing to install: an access control entry lets other users change $1"
+    return 1
+  fi
+}
+
+# Succeeds if volume $1, $1/$2, and every folder between them pass
+# openbeat_check_trusted. Checking the volume too means no folder on the path can be
+# swapped after it is checked.
+openbeat_check_staging() {
+  local path="${1%/}" remaining="$2" component
+  openbeat_check_trusted "${path:-/}" || return 1
+  while [ -n "$remaining" ]; do
+    component="${remaining%%/*}"
+    if [ "$component" = "$remaining" ]; then
+      remaining=""
+    else
+      remaining="${remaining#*/}"
+    fi
+    [ -n "$component" ] || continue
+    path="$path/$component"
+    openbeat_check_trusted "$path" || return 1
+  done
+}
+
+# Sets OPENBEAT_USER to the user to install for: the console user, or for a
+# command-line install with nobody logged in, the user who ran sudo. Fails for
+# root and system accounts rather than installing into their home folders.
+openbeat_target_user() {
+  local user uid
+  OPENBEAT_USER=""
+  user="$(stat -f %Su /dev/console 2>/dev/null)" || user=""
+  if [ -z "$user" ] || [ "$user" = "root" ] || [ "$user" = "loginwindow" ]; then
+    user="${SUDO_USER:-${USER:-}}"
+  fi
+  uid=""
+  if [ -n "$user" ] && [ "$user" != "root" ]; then
+    uid="$(id -u "$user" 2>/dev/null)" || uid=""
+  fi
+  case "$uid" in
+    "" | *[!0-9]*) uid=0 ;;
+  esac
+  if [ "$uid" -lt 500 ]; then
+    openbeat_log "Could not tell which user to install OpenBeat for. Log in as the user who runs DaVinci Resolve and run the installer again."
+    return 1
+  fi
+  OPENBEAT_USER="$user"
+}
+
 openbeat_main() {
   set -euo pipefail
 
-  local installer_root="$3/private/tmp/OpenBeatInstaller"
-  local console_user user_home resolve_root status=0
-  console_user="$(stat -f %Su /dev/console)"
-  if [[ -z "${console_user}" || "${console_user}" == "root" ]]; then
-    console_user="${SUDO_USER:-$USER}"
+  local volume="${3:-/}" staging user_home resolve_root status=0
+  staging="${volume%/}/$OPENBEAT_STAGING"
+  openbeat_check_staging "$volume" "$OPENBEAT_STAGING" || return 1
+
+  if ! openbeat_target_user; then
+    rm -rf "$staging"
+    return 1
+  fi
+  # dscl prints a value with spaces on the line after the attribute name.
+  if ! user_home="$(dscl . -read /Users/"$OPENBEAT_USER" NFSHomeDirectory |
+    awk 'NR == 1 { sub(/^NFSHomeDirectory:/, "") } { sub(/^ /, ""); printf "%s", $0 }')" ||
+    [ -z "$user_home" ] || [ "$user_home" = "/" ] || [ "$user_home" = "/var/root" ] || [ ! -d "$user_home" ]; then
+    openbeat_log "Could not find the home folder of $OPENBEAT_USER"
+    rm -rf "$staging"
+    return 1
   fi
-  user_home="$(dscl . -read /Users/"${console_user}" NFSHomeDirectory | awk '{print $2}')"
   resolve_root="$user_home/Library/Application Support/Blackmagic Design/DaVinci Resolve/Fusion"
 
-  openbeat_install "$installer_root/payload" "$resolve_root" "$console_user:staff" || status=$?
-  rm -rf "$installer_root"
+  openbeat_install_as_user "$OPENBEAT_USER" "$staging/payload" "$resolve_root" || status=$?
+  rm -rf "$staging"
+  rmdir "${staging%/*}" 2>/dev/null || true
   return "$status"
 }
 

tests/test_build_installers.py

@@ -48,6 +48,11 @@ def test_macos_package_runs_the_staged_postinstall_script(self) -> None:
             def fake_pkgbuild(cmd: list[str], cwd: Path | None = None) -> None:
                 package_root = Path(cmd[cmd.index("--root") + 1])
                 scripts = Path(cmd[cmd.index("--scripts") + 1])
+                self.assertEqual(cmd[cmd.index("--install-location") + 1], build_installers.MACOS_STAGING)
+                self.assertEqual(cmd[cmd.index("--ownership") + 1], "recommended")
+                for path in [package_root, *package_root.rglob("*")]:
+                    self.assertEqual(path.stat().st_mode & 0o022, 0, path)
+                self.assertEqual((package_root / "payload" / "Modules" / "OpenBeat" / "bin" / "openbeat").stat().st_mode & 0o777, 0o755)
                 postinstall = scripts / "postinstall"
                 self.assertEqual(postinstall.read_bytes(), (build_installers.ROOT / "scripts" / "macos_postinstall.sh").read_bytes())
                 self.assertTrue(postinstall.stat().st_mode & 0o111)
@@ -63,6 +68,12 @@ def fake_pkgbuild(cmd: list[str], cwd: Path | None = None) -> None:
                     result = build_installers.create_macos_pkg(binary, "0.0.2")
             self.assertEqual(result.read_bytes(), b"package")
 
+    def test_macos_package_stages_where_the_postinstall_script_checks(self) -> None:
+        self.assertTrue(build_installers.MACOS_STAGING.startswith("/Library/"))
+        postinstall = (build_installers.ROOT / "scripts" / "macos_postinstall.sh").read_text()
+        self.assertIn(f'OPENBEAT_STAGING="{build_installers.MACOS_STAGING.lstrip("/")}"', postinstall)
+        self.assertNotIn("/tmp", postinstall)
+
     def test_windows_cli_runtime_is_onedir_and_windowed(self) -> None:
         with TemporaryDirectory() as tmpdir:
             dist_root = Path(tmpdir) / "dist"