diff --git a/.github/workflows/check-updates.yml b/.github/workflows/check-updates.yml index 968d65b..33d04fa 100644 --- a/.github/workflows/check-updates.yml +++ b/.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 diff --git a/docs/WORKFLOW.md b/docs/WORKFLOW.md index 8a42525..1e86f5e 100644 --- a/docs/WORKFLOW.md +++ b/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 ` runs `verify-pkg.sh` for every package in `packages/` that has a directory in `/prepared/`, and runs no strategy. An update that fails is removed from `/prepared/`, its status in `/status/` becomes `update_failed`, and `/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 ` 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 `/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 `/status/`, or `update_failed: prepare handed over no update`. Nothing else is taken from `/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 ` 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 `/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 `/status/`, or `update_failed: prepare and verify handed over no update`. Nothing else is taken from `/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//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//PKGBUILD`, `prepared//.SRCINFO`, `prepared//base` and `status/` 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//PKGBUILD`, `prepared//.SRCINFO`, `prepared//base` and `status/` 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`: diff --git a/scripts/check-all.sh b/scripts/check-all.sh index 4291964..baaea95 100755 --- a/scripts/check-all.sh +++ b/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 diff --git a/tests/publish.bats b/tests/publish.bats index 8c83c39..ae8077d 100644 --- a/tests/publish.bats +++ b/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/