From 4cee5d9f633c41b89e1fa3a3760749ca4a8bf83e Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Sat, 5 Sep 2026 19:25:51 -0400 Subject: [PATCH] Refuse to read package metadata as empty when the file exists but cannot be opened MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The version check runs as root; the build container runs as an unprivileged user. Forty-two .omarchy/package.json files on the release host were mode 0600, so root read release_ring=fast and queued dropbox-cli for rc and stable while the container, unable to open the same file, read no ring and skipped it. Each release ran with nothing to publish, auto-release counted that as success and cleared the queue, and the next version check wrote it straight back — 34 empty runs in two hours, one chat report per run. package_require_readable_metadata aborts with status 2 and a message naming the file whenever metadata exists but is unreadable, and every reader in package-metadata.sh goes through it. The package enumerators materialize package_dirs into a variable instead of piping it into a loop, so that abort reaches the caller rather than ending a subshell and leaving a silently shorter list. check-versions exits without touching the queue, and build.sh refuses to build a partial list, when enumeration fails. A self-test covers the unreadable case end to end. --- bin/check-versions | 11 ++++- bin/sync-rebuilds | 17 +++++++ build/build.sh | 13 +++++- helpers/package-metadata.sh | 88 +++++++++++++++++++++++++++++++------ 4 files changed, 113 insertions(+), 16 deletions(-) diff --git a/bin/check-versions b/bin/check-versions index 925570f61..ee974b2b0 100755 --- a/bin/check-versions +++ b/bin/check-versions @@ -189,13 +189,22 @@ check_mirror() { print_info "Checking $mirror packages for $ARCH..." + # Enumerate first and stop on failure: an unreadable metadata file must not + # produce a shorter list that the queue then treats as the truth. + local candidates + if ! candidates=$(packages_for_unscoped_build "$mirror" "$ARCH"); then + print_error "Could not enumerate $mirror packages for $ARCH — leaving its queue untouched" + exit 2 + fi + while IFS= read -r pkg; do + [[ -n "$pkg" ]] || continue local pkgdir="$PKGBUILDS_DIR/$pkg" if check_package "$pkg" "$pkgdir" "$mirror"; then needs_build=true packages+=("$pkg") fi - done < <(packages_for_unscoped_build "$mirror" "$ARCH") + done <<<"$candidates" echo "" diff --git a/bin/sync-rebuilds b/bin/sync-rebuilds index 3009b8983..6189f64b3 100755 --- a/bin/sync-rebuilds +++ b/bin/sync-rebuilds @@ -781,6 +781,23 @@ cmd_self_test() { unset SELFTEST_ARCHES check "pkgrel untouched" 1 "$(pkgrel_of "$root/pkgbuilds/t-x86")" + echo "Metadata that exists but cannot be read aborts instead of reading as empty:" + root=$(selftest_root unreadable-metadata) + selftest_package "$root" t-locked 1 '{"source":"local","release_ring":"fast"}' + if [[ "$(id -u)" == 0 ]]; then + echo " skip: running as root, every file is readable" + else + chmod 000 "$root/pkgbuilds/t-locked/.omarchy/package.json" + check "package_release_ring aborts with status 2" 2 "$( (package_release_ring "$root/pkgbuilds/t-locked" >/dev/null 2>&1); echo $? )" + check "package_builds_for_mirror aborts with status 2" 2 "$( (package_builds_for_mirror "$root/pkgbuilds/t-locked" rc >/dev/null 2>&1); echo $? )" + check "packages_for_unscoped_build aborts with status 2" 2 "$( (PKGBUILDS_DIR="$root/pkgbuilds" packages_for_unscoped_build rc >/dev/null 2>&1); echo $? )" + local guard_message + guard_message=$( (package_release_ring "$root/pkgbuilds/t-locked" 2>&1 1>/dev/null) || true ) + check "message names the file" "true" "$( [[ "$guard_message" == *"cannot be read"* && "$guard_message" == *t-locked* ]] && echo true || echo false )" + chmod 644 "$root/pkgbuilds/t-locked/.omarchy/package.json" + check "readable again reads the ring" "fast" "$(package_release_ring "$root/pkgbuilds/t-locked")" + fi + echo "A PKGBUILD that branches on CARCH at file scope still reads its version:" root=$(selftest_root carch-branch) mkdir -p "$root/pkgbuilds/t-carch/.omarchy" diff --git a/build/build.sh b/build/build.sh index 9be271628..d0dd86436 100755 --- a/build/build.sh +++ b/build/build.sh @@ -567,15 +567,24 @@ if [[ -n "$PACKAGES" ]]; then fi done else - # Build all packages that need updates from the relevant directories + # Build all packages that need updates from the relevant directories. + # Enumerate first and stop on failure: the version check queued this run + # from its own reading of the metadata, and a shorter list here (an + # unreadable file for the container user, say) would build nothing and + # report success — the queue then refills on the next tick, forever. + if ! CANDIDATE_PACKAGES=$(collect_packages); then + echo "==> ERROR: could not enumerate packages for $MIRROR ($ARCH); refusing to build a partial list" + exit 1 + fi while IFS= read -r pkg; do + [[ -n "$pkg" ]] || continue if check_needs_build "$pkg"; then PACKAGES_TO_BUILD+=("$pkg") else echo " + $pkg - already up to date" SKIPPED_PACKAGES="$SKIPPED_PACKAGES $pkg" fi - done < <(collect_packages) + done <<<"$CANDIDATE_PACKAGES" fi if [[ ${#PACKAGES_TO_BUILD[@]} -eq 0 ]]; then diff --git a/helpers/package-metadata.sh b/helpers/package-metadata.sh index 564cd2d6c..dc41cd5f1 100644 --- a/helpers/package-metadata.sh +++ b/helpers/package-metadata.sh @@ -47,6 +47,26 @@ package_dir_for_name() { echo "$pkgdir" } +# A metadata file that exists but cannot be opened is not "no metadata": it is +# a broken checkout, and reading it as empty makes every decision that depends +# on it silently wrong. The version check runs as root and the build container +# as an unprivileged user, so a mode-0600 package.json is readable by one and +# not the other — the two halves then disagree about a package forever, and +# each release runs with nothing to publish while the queue refills. +# +# Every reader below goes through this: a file that is present but unreadable +# aborts the calling script with a message naming the file and the fix. +package_require_readable_metadata() { + local metadata="$1" + + [[ -e "$metadata" ]] || return 0 + [[ -r "$metadata" ]] && return 0 + + echo "ERROR: package metadata exists but cannot be read as $(id -un): $metadata" >&2 + echo " fix the file mode on the checkout (chmod 644) and rerun" >&2 + exit 2 +} + package_metadata_value() { local pkgdir="$1" local jq_filter="$2" @@ -54,6 +74,7 @@ package_metadata_value() { local metadata metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" if [[ ! -f "$metadata" ]]; then echo "$default" return 0 @@ -67,6 +88,7 @@ package_sync_enabled() { local metadata source sync metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" [[ -f "$metadata" ]] || return 1 source=$(jq -r '.source // ""' "$metadata") @@ -83,6 +105,10 @@ package_release_ring() { package_is_fast_ring() { local pkgdir="$1" + # The ring is read inside a command substitution, where an abort would only + # end the subshell; check readability here so an unreadable file stops the + # caller rather than reading as "not fast". + package_require_readable_metadata "$(metadata_file_for_dir "$pkgdir")" [[ "$(package_release_ring "$pkgdir")" == "fast" ]] } @@ -95,6 +121,7 @@ package_is_fast_ring() { package_min_release_age_seconds() { local pkgdir="$1" metadata raw metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" if [[ ! -f "$metadata" ]]; then echo 0 return 0 @@ -128,6 +155,7 @@ package_build_skipped() { local metadata skip_build metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" [[ -f "$metadata" ]] || return 1 skip_build=$(jq -r 'if has("skip_build") then .skip_build else false end' "$metadata") @@ -136,7 +164,11 @@ package_build_skipped() { package_has_metadata() { local pkgdir="$1" - [[ -f "$(metadata_file_for_dir "$pkgdir")" ]] + local metadata + + metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" + [[ -f "$metadata" ]] } package_has_pkgbuild() { @@ -198,6 +230,7 @@ package_supports_arch() { package_has_channels() { local pkgdir="$1" metadata metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" [[ -f "$metadata" ]] || return 1 jq -e 'has("channels")' "$metadata" >/dev/null } @@ -205,6 +238,7 @@ package_has_channels() { package_channels() { local pkgdir="$1" metadata metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" [[ -f "$metadata" ]] || return 0 jq -r '(.channels // [])[]' "$metadata" } @@ -223,6 +257,7 @@ package_in_channel() { package_is_pinned() { local pkgdir="$1" metadata metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" [[ -f "$metadata" ]] || return 1 [[ "$(jq -r 'if has("pinned") then .pinned else false end' "$metadata")" == "true" ]] } @@ -275,19 +310,26 @@ package_moves_to_channel() { package_dirs() { [[ -d "$PKGBUILDS_DIR" ]] || return 0 - find "$PKGBUILDS_DIR" -mindepth 1 -maxdepth 1 -type d -print | sort | while IFS= read -r pkgdir; do + # Read from a process substitution rather than piping into the loop, so the + # readability guard inside package_has_metadata aborts this function and + # not just a pipeline subshell. + local pkgdir + while IFS= read -r pkgdir; do package_has_pkgbuild "$pkgdir" || continue package_has_metadata "$pkgdir" || continue echo "$pkgdir" - done + done < <(find "$PKGBUILDS_DIR" -mindepth 1 -maxdepth 1 -type d -print | sort) } packages_for_aur_sync() { - package_dirs | while IFS= read -r pkgdir; do + local pkgdir dirs + dirs=$(package_dirs) || return $? + while IFS= read -r pkgdir; do + [[ -n "$pkgdir" ]] || continue if package_sync_enabled "$pkgdir"; then basename "$pkgdir" fi - done + done <<<"$dirs" } package_has_upstream_hook() { @@ -298,6 +340,7 @@ package_has_upstream_hook() { package_has_upstream_provider() { local pkgdir="$1" metadata metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" [[ -f "$metadata" ]] || return 1 # Any upstream key counts, valid or not: a malformed declaration must reach # bin/sync-upstream and fail loudly there, not vanish from discovery. @@ -305,11 +348,14 @@ package_has_upstream_provider() { } packages_for_upstream_sync() { - package_dirs | while IFS= read -r pkgdir; do + local pkgdir dirs + dirs=$(package_dirs) || return $? + while IFS= read -r pkgdir; do + [[ -n "$pkgdir" ]] || continue if package_has_upstream_hook "$pkgdir" || package_has_upstream_provider "$pkgdir"; then basename "$pkgdir" fi - done + done <<<"$dirs" } # Packages that must be rebuilt when a dependency they link against changes, @@ -321,6 +367,7 @@ package_rebuild_triggers() { local metadata metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" [[ -f "$metadata" ]] || return 0 jq -r '(.rebuild_on // [])[]' "$metadata" @@ -332,36 +379,50 @@ package_has_rebuild_triggers() { } packages_for_rebuild_sync() { - package_dirs | while IFS= read -r pkgdir; do + local pkgdir dirs + dirs=$(package_dirs) || return $? + while IFS= read -r pkgdir; do + [[ -n "$pkgdir" ]] || continue if package_has_rebuild_triggers "$pkgdir"; then basename "$pkgdir" fi - done + done <<<"$dirs" } packages_for_mirror() { local mirror="$1" - package_dirs | while IFS= read -r pkgdir; do + local pkgdir dirs + dirs=$(package_dirs) || return $? + while IFS= read -r pkgdir; do + [[ -n "$pkgdir" ]] || continue if package_builds_for_mirror "$pkgdir" "$mirror"; then basename "$pkgdir" fi - done + done <<<"$dirs" } packages_for_unscoped_build() { local mirror="$1" local arch="${2:-${ARCH:-x86_64}}" - package_dirs | while IFS= read -r pkgdir; do + local pkgdir dirs + # Enumerate into a variable and loop over that, not `package_dirs | while` + # or `done < <(package_dirs)`: both discard the producer's exit status, so + # the readability guard aborting inside package_dirs would leave the caller + # with a silently shorter list. This way the abort reaches the caller. + dirs=$(package_dirs) || return $? + while IFS= read -r pkgdir; do + [[ -n "$pkgdir" ]] || continue if package_builds_for_mirror "$pkgdir" "$mirror" && ! package_build_skipped "$pkgdir" && package_supports_arch "$pkgdir" "$arch"; then basename "$pkgdir" fi - done + done <<<"$dirs" } + package_extract_vcs_hash_from_version() { local version="$1" local version_no_pkgrel="${version%-*}" @@ -444,6 +505,7 @@ validate_package_metadata() { local metadata source sync skip_build aur ring pkgrel_type metadata=$(metadata_file_for_dir "$pkgdir") + package_require_readable_metadata "$metadata" [[ -f "$metadata" ]] || { echo "missing metadata: $metadata"; return 1; } jq empty "$metadata" >/dev/null || return 1