fix(scripts): give each package a time budget

.github/workflows/check-updates.yml

@@ -139,9 +139,31 @@ jobs:
       - name: Hand over
         run: |
           # A process started by a PKGBUILD can outlive its step. Stop all
-          # of them before root reads anything builder wrote.
-          pkill -KILL -u builder || true
-          while pgrep -u builder >/dev/null; do sleep 1; done
+          # of them before root reads anything builder wrote. A zombie runs
+          # nothing, and pid 1 of a job container never reaps one, so only
+          # the other processes count, by effective or real user. live exits
+          # with 0 for a running process, 1 for none and 2 when the list
+          # fails, and only 1 lets the step go on.
+          uid=$(id -u builder)
+          live() {
+            procs=$(ps -e -o uid=,ruid=,stat=) || return 2
+            printf '%s\n' "$procs" |
+              awk -v u="$uid" '($1 == u || $2 == u) && $3 !~ /^Z/ { n++ } END { exit !n }'
+          }
+          for _ in $(seq 30); do
+            rc=0
+            live || rc=$?
+            [ "$rc" -eq 1 ] && break
+            pkill -KILL -u builder || true
+            pkill -KILL -U builder || true
+            sleep 1
+          done
+          rc=0
+          live || rc=$?
+          if [ "$rc" -ne 1 ]; then
+            echo "::error::A process of builder is still running, or the process list failed"
+            exit 1
+          fi
 
           # Copy regular files from the expected paths only, and nothing
           # else gets uploaded. find does not follow symlinks; -print0,
@@ -211,16 +233,14 @@ jobs:
           retention-days: 1
 
   # Pushes what verify passed on and records the statuses of the check job.
-  # Runs also when prepare is skipped, for a change in the status table.
-  # Nothing here executes a PKGBUILD: publish-pkg.sh checks the prepared
-  # files against the AUR copy.
+  # Runs also when prepare is skipped, for a change in the status table, and
+  # when prepare or verify failed: then it publishes nothing, and every
+  # package with a new version gets update_failed and an issue. Nothing here
+  # executes a PKGBUILD: publish-pkg.sh checks the prepared files against
+  # the AUR copy.
   publish:
     needs: [image, check, prepare, verify]
-    if: >-
-      !cancelled() &&
-      needs.check.outputs.work == 'true' &&
-      contains(fromJSON('["success", "skipped"]'), needs.prepare.result) &&
-      contains(fromJSON('["success", "skipped"]'), needs.verify.result)
+    if: "!cancelled() && needs.check.outputs.work == 'true'"
     runs-on: ubuntu-latest
     container: ${{ needs.image.outputs.ref }}
     timeout-minutes: 30

docs/WORKFLOW.md

@@ -64,7 +64,7 @@ flowchart TD
 - **Strategy-based version detection.** Each type of upstream source has its own script. A new source is a new script plus its entry in `scripts/validate-pkg.sh`.
 - **Patch, do not generate.** aurtomator trusts your PKGBUILD and changes only `pkgver`, `pkgrel`, checksums and `.SRCINFO`. Build steps, dependencies and install files stay untouched.
 - **Four jobs, one with secrets.** A PKGBUILD is code that every step reading it executes. `check` runs the strategies with the read-only workflow token, executes no PKGBUILD and decides what the run does. The jobs after it take the status of every package from `check`. `prepare` runs the PKGBUILD steps without secrets and without a token, and hands over only prepared files and update failures, because code from one PKGBUILD can write the files of every package there. `verify` downloads the sources of each prepared update and checks its checksums, without secrets, so code from one PKGBUILD cannot forge the checksums of another package. `publish` holds the SSH key, the GPG key and the write token, never executes a PKGBUILD and never downloads a source, and checks what `verify` passes on before it pushes. See [4.4](#44-prepare-and-publish).
-- **Failure isolation.** A broken package does not block the others. Each failure is logged for its package and opens a GitHub Issue.
+- **Failure isolation.** A broken package does not block the others. Each failure is logged for its package and opens a GitHub Issue. Each package has a time budget, so one that hangs does not hold up the run either.
 
 ## 2. Package YAML Format
 
@@ -542,6 +542,8 @@ strategy: kde-tarball
 
 `check-all.sh --update` calls `update-pkg.sh` for every package with a new version. `--prepare`, `--verify` and `--publish` split the same update into three runs, see [4.4](#44-prepare-and-publish).
 
+Outside `--update`, `check-all.sh` gives each package a time budget: 120 seconds for `check-update.sh`, and 300 seconds each for `update-pkg.sh`, `verify-pkg.sh` and `publish-pkg.sh`. `timeout` ends the script together with everything it started, makepkg, git and curl included, and the package gets `check_failed: timed out after 120 s` or `update_failed: timed out after 300 s`. `AURTOMATOR_CHECK_TIMEOUT` and `AURTOMATOR_UPDATE_TIMEOUT` change the budgets. `--update` has none, because it runs on a terminal, where git and ssh may ask for a passphrase, and a command under `timeout` cannot read the terminal.
+
 ### 4.1 Arguments
 
 ```text
@@ -641,7 +643,7 @@ A PKGBUILD is a bash script, and every step that reads it executes it: the audit
 
 `check-all.sh --verify <dir>` runs `verify-pkg.sh` for every package in `packages/` that has a directory in `<dir>/prepared/`, and runs no strategy. An update that fails is removed from `<dir>/prepared/`, its status in `<dir>/status/` becomes `update_failed`, and `<dir>/status/_error_count` grows by one. Exit code 3 counts as a failure as well: the prepare run writes `base` for every package, so a moved AUR repo looks the same as a wrong `base`. The next run prepares the package again.
 
-`check-all.sh --publish <dir>` takes the statuses of the check run from `.status/`, as `--prepare` does, and writes `.status/` anew. `up_to_date` and `check_failed` stay as they are. A missing status, and one the check run does not write, become `check_failed`. For a package with `new_version` or `available_downgrade` and a directory in `<dir>/prepared/`, it runs the strategy once more and publishes with the version the strategy reports now. Exit code 3 leaves the status at `new_version` or `available_downgrade` without an error, and the next run prepares the package again. A package with `new_version` or `available_downgrade` and no prepared directory gets the `update_failed` status that the prepare or the verify run wrote for it to `<dir>/status/`, or `update_failed: prepare handed over no update`. Nothing else is taken from `<dir>/status/`, and only a regular file without a NUL byte: the prepare run executes PKGBUILDs, and code from one of them can write the files of every package. `_error_count` is the number of `check_failed` and `update_failed` statuses.
+`check-all.sh --publish <dir>` takes the statuses of the check run from `.status/`, as `--prepare` does, and writes `.status/` anew. `up_to_date` and `check_failed` stay as they are. A missing status, and one the check run does not write, become `check_failed`. For a package with `new_version` or `available_downgrade` and a directory in `<dir>/prepared/`, it runs the strategy once more and publishes with the version the strategy reports now. Exit code 3 leaves the status at `new_version` or `available_downgrade` without an error, and the next run prepares the package again. A package with `new_version` or `available_downgrade` and no prepared directory gets the `update_failed` status that the prepare or the verify run wrote for it to `<dir>/status/`, or `update_failed: prepare and verify handed over no update`. Nothing else is taken from `<dir>/status/`, and only a regular file without a NUL byte: the prepare run executes PKGBUILDs, and code from one of them can write the files of every package. `_error_count` is the number of `check_failed` and `update_failed` statuses.
 
 ## 5. CI Workflow: check-updates.yml
 
@@ -661,7 +663,7 @@ The workflow has four jobs after a short `image` job. `image` reads the image fr
 | `verify` | `contents: read` | none |
 | `publish` | `contents: write` for the status commit, `issues: write` for failure issues | `AUR_SSH_KEY`, `GPG_SIGNING_KEY`, `AUR_GIT_NAME`, `AUR_GIT_EMAIL` |
 
-`check` decides the run. `prepare` and `verify` run only when a package has a new version. `publish` runs for a new version and also for a change in the status table of `README.md`, but not after `prepare` or `verify` failed. In most hours nothing changes, and all three are skipped.
+`check` decides the run. `prepare` and `verify` run only when a package has a new version. `publish` runs for a new version and also for a change in the status table of `README.md`. It runs after a failure of `prepare` or `verify` as well: then it publishes nothing, and every package with a new version gets `update_failed` and an issue. In most hours nothing changes, and all three are skipped.
 
 ### 5.3 Steps
 
@@ -684,7 +686,7 @@ Every job starts with **Checkout:** `actions/checkout`, pinned by commit SHA lik
 1. **Install dependencies:** `sudo git openssh namcap jq go-yq pacman-contrib`, and the `builder` user.
 2. **Download statuses:** the `checked` artifact to `.status/`.
 3. **Prepare updates:** gives the workspace to `builder` and runs `check-all.sh --prepare handoff` as `builder`, without a token. Code from a PKGBUILD can read the environment of every running process of `builder` through `/proc/<pid>/environ`, so the token stays in `check`. Root prefixes every line of the output with `|` and a space, so nothing a PKGBUILD prints reaches the runner as a workflow command such as `::add-mask::`. The step runs in bash with `pipefail`, so a failure of `check-all.sh` fails it.
-4. **Hand over:** code from a PKGBUILD has run in the workspace by now, so the step runs nothing from it. A process started by a PKGBUILD can outlive its step, so the step kills every process of `builder`. Only after that does root copy the regular files at `prepared/<package>/PKGBUILD`, `prepared/<package>/.SRCINFO`, `prepared/<package>/base` and `status/<package>` from `handoff/` to `upload/`. `find` does not follow symlinks and separates paths with NUL, because a name may hold a newline, so a link to a file outside the workspace does not reach the artifact. A package name never starts with a dot, so `.` and `..` match no path.
+4. **Hand over:** code from a PKGBUILD has run in the workspace by now, so the step runs nothing from it. A process started by a PKGBUILD can outlive its step, so the step kills every process whose effective or real user is `builder` and waits up to 30 seconds for them to end. A zombie does not count: it runs nothing, and pid 1 of a job container never reaps one. A process that is still running after that fails the step, and so does a process list that `ps` cannot produce. Only after that does root copy the regular files at `prepared/<package>/PKGBUILD`, `prepared/<package>/.SRCINFO`, `prepared/<package>/base` and `status/<package>` from `handoff/` to `upload/`. `find` does not follow symlinks and separates paths with NUL, because a name may hold a newline, so a link to a file outside the workspace does not reach the artifact. A package name never starts with a dot, so `.` and `..` match no path.
 5. **Upload handoff:** uploads `upload/` as the `handoff` artifact with `include-hidden-files: true`, because `.SRCINFO` starts with a dot, and a retention of 1 day.
 
 `verify`:

scripts/check-all.sh

@@ -28,6 +28,35 @@ MODE=check
 WORK_DIR=""
 DRY_RUN=""
 
+# Time budget of one package, in seconds: for its version check, and for
+# its update, verify or publish
+CHECK_TIMEOUT="${AURTOMATOR_CHECK_TIMEOUT:-120}"
+UPDATE_TIMEOUT="${AURTOMATOR_UPDATE_TIMEOUT:-300}"
+
+# Runs a per-package script within a budget. timeout runs it in a process
+# group of its own and ends the whole group, makepkg, git and curl below it
+# included. Such a group cannot read the terminal, where git and ssh may ask
+# for a passphrase, so --update, which runs on a terminal, has no budget.
+# Args: $1=budget in seconds, the command after it
+bounded() {
+  local budget="$1"
+  shift
+  if [[ "$MODE" == "update" ]]; then
+    "$@"
+  else
+    timeout --kill-after=30 "$budget" "$@"
+  fi
+}
+
+# Prints the error of a per-package script: the budget it ran out of, or
+# its stderr. Args: $1=exit code, $2=stderr file, $3=budget in seconds
+script_error() {
+  case "$1" in
+    124 | 137) echo "timed out after $3 s" ;;
+    *) cat "$2" ;;
+  esac
+}
+
 while [[ $# -gt 0 ]]; do
   case "$1" in
     --update)
@@ -76,8 +105,8 @@ if [[ "$MODE" == "verify" ]]; then
 
     err_file=$(mktemp)
     verify_rc=0
-    "${AURTOMATOR_DIR}/scripts/verify-pkg.sh" "$pkg_name" "$prepared" 2>"$err_file" || verify_rc=$?
-    verify_err=$(<"$err_file")
+    bounded "$UPDATE_TIMEOUT" "${AURTOMATOR_DIR}/scripts/verify-pkg.sh" "$pkg_name" "$prepared" 2>"$err_file" || verify_rc=$?
+    verify_err=$(script_error "$verify_rc" "$err_file" "$UPDATE_TIMEOUT")
     rm -f "$err_file"
     [[ -z "$verify_err" ]] || printf '%s\n' "$verify_err" >&2
 
@@ -140,7 +169,7 @@ handed_failure() {
       return
     fi
   fi
-  echo "update_failed: prepare handed over no update"
+  echo "update_failed: prepare and verify handed over no update"
 }
 
 updated=0
@@ -206,19 +235,21 @@ for pkg_file in $(printf '%s\n' "${AURTOMATOR_DIR}"/packages/*.yml | sort); do
   if [[ "$MODE" == "prepare" ]]; then
     check_cmd=(checked_version "$pkg_name")
   else
-    check_cmd=("${AURTOMATOR_DIR}/scripts/check-update.sh" "$pkg_name")
+    check_cmd=(bounded "$CHECK_TIMEOUT" "${AURTOMATOR_DIR}/scripts/check-update.sh" "$pkg_name")
   fi
 
   # Capture stderr separately for error reporting
   err_file=$(mktemp)
-  latest=$("${check_cmd[@]}" 2>"$err_file") || {
-    check_err=$(cat "$err_file")
+  check_rc=0
+  latest=$("${check_cmd[@]}" 2>"$err_file") || check_rc=$?
+  if ((check_rc != 0)); then
+    check_err=$(script_error "$check_rc" "$err_file" "$CHECK_TIMEOUT")
     rm -f "$err_file"
     log_err "Failed to check $pkg_name: $check_err"
     echo "check_failed: ${check_err}" >"${STATUS_DIR}/${pkg_name}"
     ((errors++)) || true
     continue
-  }
+  fi
   rm -f "$err_file"
 
   if [[ -z "$latest" ]]; then
@@ -267,8 +298,8 @@ for pkg_file in $(printf '%s\n' "${AURTOMATOR_DIR}"/packages/*.yml | sort); do
 
   err_file=$(mktemp)
   update_rc=0
-  "${cmd[@]}" 2>"$err_file" || update_rc=$?
-  update_err=$(cat "$err_file")
+  bounded "$UPDATE_TIMEOUT" "${cmd[@]}" 2>"$err_file" || update_rc=$?
+  update_err=$(script_error "$update_rc" "$err_file" "$UPDATE_TIMEOUT")
   rm -f "$err_file"
 
   case "$MODE:$update_rc" in

tests/publish.bats

@@ -769,7 +769,7 @@ current: '1.0.0'"
   checked_status pkg "new_version: 2.0.0"
   run "$TEST_TMPDIR/scripts/check-all.sh" --publish "$TEST_TMPDIR/out"
   [ "$status" -eq 0 ]
-  [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "update_failed: prepare handed over no update" ]
+  [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "update_failed: prepare and verify handed over no update" ]
   [ "$(cat "$TEST_TMPDIR/.status/_error_count")" = "1" ]
 }
 
@@ -797,7 +797,7 @@ current: '2.0.0'"
   echo "up_to_date" >"$TEST_TMPDIR/out/status/pkg"
   echo "check_failed: forged" >"$TEST_TMPDIR/out/status/other"
   run "$TEST_TMPDIR/scripts/check-all.sh" --publish "$TEST_TMPDIR/out"
-  [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "update_failed: prepare handed over no update" ]
+  [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "update_failed: prepare and verify handed over no update" ]
   [ "$(cat "$TEST_TMPDIR/.status/other")" = "up_to_date" ]
 }
 
@@ -847,7 +847,7 @@ current: '1.0.0'"
   stub_script publish-pkg.sh 0
   run "$TEST_TMPDIR/scripts/check-all.sh" --publish "$TEST_TMPDIR/out"
   [ ! -f "$TEST_TMPDIR/publish-pkg.sh.args" ]
-  [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "update_failed: prepare handed over no update" ]
+  [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "update_failed: prepare and verify handed over no update" ]
 }
 
 @test "check-all --publish: does not import statuses of names that are not packages" {
@@ -871,7 +871,7 @@ current: '1.0.0'"
   echo "update_failed: from elsewhere" >"$TEST_TMPDIR/elsewhere/pkg"
   ln -s "$TEST_TMPDIR/elsewhere" "$TEST_TMPDIR/out/status"
   run "$TEST_TMPDIR/scripts/check-all.sh" --publish "$TEST_TMPDIR/out"
-  [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "update_failed: prepare handed over no update" ]
+  [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "update_failed: prepare and verify handed over no update" ]
 }
 
 @test "check-all --publish: does not follow symlinks among the handed over statuses" {
@@ -894,7 +894,7 @@ current: '1.0.0'"
   [ "$(cat "$TEST_TMPDIR/target-status")" = "update_failed: from a link" ]
   [ "$(cat "$TEST_TMPDIR/target-count")" = "keep" ]
   [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "updated: 2.0.0" ]
-  [ "$(cat "$TEST_TMPDIR/.status/other")" = "update_failed: prepare handed over no update" ]
+  [ "$(cat "$TEST_TMPDIR/.status/other")" = "update_failed: prepare and verify handed over no update" ]
 }
 
 @test "check-all --publish: does not read a checked status that is a symlink" {
@@ -943,6 +943,91 @@ current: '1.0.0'"
   [ "$(cat "$TEST_TMPDIR/.status/_error_count")" = "1" ]
 }
 
+# === check-all.sh time budgets ===
+
+# Helper: replace a script with a stub that starts a background sleep,
+# records its pid in $TEST_TMPDIR/<script>.child, and then hangs
+# Usage: hanging_script <script name>
+hanging_script() {
+  rm -f "$TEST_TMPDIR/scripts/$1"
+  cat >"$TEST_TMPDIR/scripts/$1" <<EOF
+#!/usr/bin/env bash
+sleep 60 &
+echo \$! >"$TEST_TMPDIR/$1.child"
+sleep 60
+EOF
+  chmod +x "$TEST_TMPDIR/scripts/$1"
+}
+
+# Succeeds when the child that hanging_script started has ended. In a job
+# container pid 1 never reaps an orphan, so an ended child can stay a zombie.
+child_ended() {
+  local state
+  state=$(ps -o stat= -p "$(cat "$TEST_TMPDIR/$1.child")") || return 0
+  [[ "$state" == Z* ]]
+}
+
+@test "check-all: a strategy that hangs fails after its time budget and the others go on" {
+  create_test_package "slow" "github-release" "upstream:
+  project: owner/repo
+current: '1.0.0'"
+  create_test_package "zzz" "github-release" "upstream:
+  project: owner/repo
+current: '1.0.0'"
+  rm -f "$TEST_TMPDIR/scripts/check-update.sh"
+  cat >"$TEST_TMPDIR/scripts/check-update.sh" <<EOF
+#!/usr/bin/env bash
+[[ "\$1" == "slow" ]] && exec "$TEST_TMPDIR/scripts/hang.sh"
+exit 0
+EOF
+  chmod +x "$TEST_TMPDIR/scripts/check-update.sh"
+  hanging_script hang.sh
+  SECONDS=0
+  AURTOMATOR_CHECK_TIMEOUT=1 run "$TEST_TMPDIR/scripts/check-all.sh"
+  [ "$SECONDS" -lt 10 ]
+  [ "$(cat "$TEST_TMPDIR/.status/slow")" = "check_failed: timed out after 1 s" ]
+  [ "$(cat "$TEST_TMPDIR/.status/zzz")" = "up_to_date" ]
+  child_ended hang.sh
+}
+
+@test "check-all --prepare: an update that hangs fails after its time budget" {
+  create_test_package "pkg" "github-release" "upstream:
+  project: owner/repo
+current: '1.0.0'"
+  checked_status pkg "new_version: 2.0.0"
+  hanging_script update-pkg.sh
+  SECONDS=0
+  AURTOMATOR_UPDATE_TIMEOUT=1 run "$TEST_TMPDIR/scripts/check-all.sh" --prepare "$TEST_TMPDIR/out"
+  [ "$SECONDS" -lt 10 ]
+  [ "$(cat "$TEST_TMPDIR/out/status/pkg")" = "update_failed: timed out after 1 s" ]
+  child_ended update-pkg.sh
+}
+
+@test "check-all --verify: a verify that hangs fails after its time budget" {
+  setup_check_verify
+  hanging_script verify-pkg.sh
+  SECONDS=0
+  AURTOMATOR_UPDATE_TIMEOUT=1 run "$TEST_TMPDIR/scripts/check-all.sh" --verify "$TEST_TMPDIR/out"
+  [ "$SECONDS" -lt 10 ]
+  [ ! -e "$TEST_TMPDIR/out/prepared/pkg" ]
+  [ "$(cat "$TEST_TMPDIR/out/status/pkg")" = "update_failed: timed out after 1 s" ]
+  child_ended verify-pkg.sh
+}
+
+@test "check-all --publish: a publish that hangs fails after its time budget" {
+  create_test_package "pkg" "github-release" "upstream:
+  project: owner/repo
+current: '1.0.0'"
+  checked_status pkg "new_version: 2.0.0"
+  mkdir -p "$TEST_TMPDIR/out/prepared/pkg"
+  hanging_script publish-pkg.sh
+  SECONDS=0
+  AURTOMATOR_UPDATE_TIMEOUT=1 run "$TEST_TMPDIR/scripts/check-all.sh" --publish "$TEST_TMPDIR/out"
+  [ "$SECONDS" -lt 10 ]
+  [ "$(cat "$TEST_TMPDIR/.status/pkg")" = "update_failed: timed out after 1 s" ]
+  child_ended publish-pkg.sh
+}
+
 # === check-all.sh --verify ===
 
 # Helper: one package with a prepared update and its handed over status