From 5777573a84e4772898f4b721d5a0e56dbf348ac1 Mon Sep 17 00:00:00 2001 From: Ryan Hughes Date: Mon, 24 Aug 2026 20:07:10 -0400 Subject: [PATCH] Harden the provider per Momus review and prove it with offline fixtures bin/sync-upstream self-test swaps the two network fetches in helpers/upstream-github.sh for fixture readers and runs the production code paths: fallback past a quarantined release, draft/prerelease filtering, the deliberate bypass, unchanged-version and all-quarantined no-update paths, unusable tags/timestamps and missing checksums failing the sync, {tag} and {pkgver} asset templates with ./ and * manifest prefixes across both architectures, the min_release_age backstop verdicts (now a testable release_age_status function), the duration parser, and manifest validation. Also fixes from the review: the duration parser forces base-10 arithmetic (leading zeros no longer parse as octal) and bounds values to nine digits so no suffix can overflow; jq // treating false as absent can no longer let "min_release_age": false or "upstream": false slip through as unset; the release feed page grew to the API maximum of 100 with the bounded search documented; and the README package-metadata section documents the upstream block, min_release_age, the bypass, and provider-versus-hook exclusivity. --- README.md | 43 ++++++- bin/sync-upstream | 221 +++++++++++++++++++++++++++++++++--- helpers/package-metadata.sh | 36 ++++-- helpers/upstream-github.sh | 29 ++++- 4 files changed, 294 insertions(+), 35 deletions(-) diff --git a/README.md b/README.md index 3530dab..f1dadf6 100644 --- a/README.md +++ b/README.md @@ -258,8 +258,41 @@ bin/sync-upstream openai-codex-desktop # Update specific packages Some vendors publish a release feed of their own that is faster and more precise than the AUR packaging of it. Those packages are `source: local` — Omarchy owns -the PKGBUILD — and provide `.omarchy/upstream.sh`, a hook that reports the newest -upstream release as JSON on stdout: +the PKGBUILD — and declare where releases come from in one of two ways. + +A vendor shipping tagged GitHub releases with a checksum manifest asset is pure +data, declared as `upstream` in `.omarchy/package.json` with no code at all: + +```json +"upstream": { + "github": "jdx/mise", + "checksums": "SHASUMS256.txt", + "assets": { + "x86_64": "mise-{tag}-linux-x64.tar.xz", + "aarch64": "mise-{tag}-linux-arm64.tar.xz" + } +} +``` + +`{tag}` and `{pkgver}` interpolate into asset names; a leading `v` on the tag is +stripped for `pkgver`; drafts and prereleases are ignored. Only the 100 most +recent releases are considered. The provider fails closed on anything it cannot +read — an unusable tag, timestamp, or checksum stops the sync rather than being +skipped. + +A package may also declare `"min_release_age": "24h"` (`s`/`m`/`h`/`d` suffix or +bare seconds) to quarantine fresh releases until maintainers have had time to +pull a bad or compromised one. The newest release that has cleared the window +ships, so a fast release cadence cannot starve updates. The window is enforced +centrally: whatever reports the release must prove its age via `published_at`, +or the sync fails. A maintainer deliberately shipping inside the window runs +`BYPASS_MIN_RELEASE_AGE=1 bin/sync-upstream ` locally and merges the +result through a normal PR; scheduled automation never sets the bypass. + +A vendor whose feed fits no convention (a Debian package index, a bare +version.txt) instead provides `.omarchy/upstream.sh`, a hook that reports the +newest upstream release as JSON on stdout — declaring both an `upstream` block +and a hook is an error: ```json { @@ -281,7 +314,11 @@ back cannot walk the repository backwards. Hooks should read checksums from whatever manifest the vendor publishes rather than downloading the artifacts — see `pkgbuilds/openai-codex-desktop/.omarchy/upstream.sh`, which reads OpenAI's Debian package index and never fetches the 750 MB of debs -it describes. +it describes. Hooks honoring `min_release_age` receive the window as +`MIN_RELEASE_AGE_SECONDS` and report `published_at` alongside `pkgver`. + +`bin/sync-upstream self-test` runs offline fixture tests over the release +selection, quarantine backstop, duration parsing, and manifest validation. ### Sync Rebuild Triggers diff --git a/bin/sync-upstream b/bin/sync-upstream index 206350e..c06f1a3 100755 --- a/bin/sync-upstream +++ b/bin/sync-upstream @@ -48,8 +48,12 @@ the bypass, so the resulting change still goes through a reviewed PR. Arguments: PACKAGE One or more package names to update (optional) +Commands: + self-test Run the offline fixture tests for release selection, the + quarantine backstop, and metadata parsing + Examples: - $0 # Update every package with an upstream hook + $0 # Update every package with an upstream source $0 openai-codex-desktop # Update specific packages EOF } @@ -157,6 +161,22 @@ set_pkgbuild_array() { mv "$rewritten" "$pkgbuild" } +# Backstop verdict for a reported release against min_release_age. Returns 0 +# when old enough (or no policy is set, or the bypass is deliberate), 1 when +# the release is younger than the window, 2 when the report carries no usable +# published_at and the age cannot be established at all. +release_age_status() { + local release="$1" min_age="$2" + (( min_age > 0 )) || return 0 + [[ "${BYPASS_MIN_RELEASE_AGE:-}" != "1" ]] || return 0 + local published_at published_epoch + published_at=$(jq -r '.published_at // empty' <<<"$release") + if [[ -z "$published_at" ]] || ! published_epoch=$(date --date="$published_at" +%s 2>/dev/null); then + return 2 + fi + (( $(date +%s) - published_epoch >= min_age )) || return 1 +} + # pacman's own comparator, because nothing else agrees with it at the corners: # sort -V calls 1.0a newer than 1.0, vercmp calls it older, and pacman is what # decides whether a published package is an upgrade. @@ -371,23 +391,23 @@ sync_package() { return 0 fi - # Backstop for min_release_age: the hook already selects within the window, - # but a hook bug must not be able to ship a release younger than the policy. - if (( min_age > 0 )) && [[ "${BYPASS_MIN_RELEASE_AGE:-}" != "1" ]]; then - local published_at published_epoch age - published_at=$(jq -r '.published_at // empty' <<<"$release") - if [[ -z "$published_at" ]] || ! published_epoch=$(date --date="$published_at" +%s 2>/dev/null); then - print_error "min_release_age is set for $package but its hook reported no usable published_at; refusing an unverifiable release" - ((++FAILED)) - return 0 - fi - age=$(( $(date +%s) - published_epoch )) - if (( age < min_age )); then - print_warning " Hook reported a release only $((age / 3600))h old, inside the ${min_age}s minimum age; leaving it alone" + # Backstop for min_release_age: the selection already honors the window, + # but a provider or hook bug must not be able to ship a release younger + # than the policy. + local age_status=0 + release_age_status "$release" "$min_age" || age_status=$? + case "$age_status" in + 1) + print_warning " Reported release is inside the ${min_age}s minimum release age; leaving it alone" ((++SKIPPED)) return 0 - fi - fi + ;; + 2) + print_error "min_release_age is set for $package but its source reported no usable published_at; refusing an unverifiable release" + ((++FAILED)) + return 0 + ;; + esac local pkgver current_pkgver pkgver=$(jq -r '.pkgver' <<<"$release") @@ -421,6 +441,175 @@ sync_package() { ((++UPDATED)) } +# Offline fixture tests: the network fetches in helpers/upstream-github.sh +# are swapped for fixture readers, everything else runs the production code +# paths. Covers release selection (fallback past quarantined releases, +# draft/prerelease filtering, bypass, unchanged version), failure paths +# (unusable tags/timestamps, missing checksums), checksum template mapping +# for both architectures, the min_release_age backstop, the duration parser, +# and manifest validation. +cmd_self_test() { + local failures=0 + + check() { + local desc="$1" expected="$2" got="$3" + if [[ "$expected" == "$got" ]]; then + echo " ok: $desc" + else + echo " FAIL: $desc (expected '$expected', got '$got')" + failures=$((failures + 1)) + fi + } + + local pkg="$TEMP_DIR/selftest-pkg" + mkdir -p "$pkg/.omarchy" + printf 'pkgver=1.0.0\npkgrel=1\n' > "$pkg/PKGBUILD" + cat > "$pkg/.omarchy/package.json" <<'EOF' +{ + "source": "local", + "min_release_age": "24h", + "upstream": { + "github": "example/tool", + "checksums": "SHASUMS256.txt", + "assets": { + "x86_64": "tool-{tag}-x64.tar.xz", + "aarch64": "tool-v{pkgver}-arm64.tar.xz" + } + } +} +EOF + + local young old2d old3d + young=$(date -u -d '1 hour ago' +%Y-%m-%dT%H:%M:%SZ) + old2d=$(date -u -d '2 days ago' +%Y-%m-%dT%H:%M:%SZ) + old3d=$(date -u -d '3 days ago' +%Y-%m-%dT%H:%M:%SZ) + + # v2.0.0 is inside the 24h window; v1.9.9/v1.9.8 are a prerelease and a + # draft that would outrank v1.9.0 if the filters failed. + local sum_x19 sum_a19 sum_x20 sum_a20 + sum_x19=$(printf 'a%.0s' {1..64}) + sum_a19=$(printf 'b%.0s' {1..64}) + sum_x20=$(printf 'c%.0s' {1..64}) + sum_a20=$(printf 'd%.0s' {1..64}) + + FIXTURE_RELEASES=$(jq -n --arg young "$young" --arg old2 "$old2d" --arg old3 "$old3d" '[ + {tag_name: "v2.0.0", published_at: $young, draft: false, prerelease: false}, + {tag_name: "v1.9.9", published_at: $old2, draft: false, prerelease: true}, + {tag_name: "v1.9.8", published_at: $old2, draft: true, prerelease: false}, + {tag_name: "v1.9.0", published_at: $old2, draft: false, prerelease: false}, + {tag_name: "v1.8.0", published_at: $old3, draft: false, prerelease: false} + ]') + FIXTURE_CHECKSUMS=$(printf '%s\n' \ + "$sum_x19 ./tool-v1.9.0-x64.tar.xz" \ + "$sum_a19 tool-v1.9.0-arm64.tar.xz" \ + "$sum_x20 *tool-v2.0.0-x64.tar.xz" \ + "$sum_a20 tool-v2.0.0-arm64.tar.xz") + + github_fetch_releases() { printf '%s' "$FIXTURE_RELEASES"; } + github_fetch_checksums() { printf '%s\n' "$FIXTURE_CHECKSUMS"; } + + echo "Release selection:" + local out + out=$(github_upstream_release "$pkg" 86400 2>/dev/null) || out="" + check "quarantine falls back past the young v2.0.0" "1.9.0" "$(jq -r '.pkgver // ""' <<<"$out")" + check "selected release reports its published_at" "$old2d" "$(jq -r '.published_at // ""' <<<"$out")" + check "x86_64 checksum via {tag} template and ./ prefix" "$sum_x19" "$(jq -r '.sha256sums.x86_64[0] // ""' <<<"$out")" + check "aarch64 checksum via {pkgver} template" "$sum_a19" "$(jq -r '.sha256sums.aarch64[0] // ""' <<<"$out")" + + out=$(github_upstream_release "$pkg" 0 2>/dev/null) || out="" + check "no policy selects the newest stable release" "2.0.0" "$(jq -r '.pkgver // ""' <<<"$out")" + check "prerelease v1.9.9 and draft v1.9.8 are never selected" "" "$(jq -r 'select(.pkgver == "1.9.9" or .pkgver == "1.9.8") | .pkgver' <<<"$out")" + + out=$(BYPASS_MIN_RELEASE_AGE=1 github_upstream_release "$pkg" 86400 2>/dev/null) || out="" + check "bypass lifts the quarantine" "2.0.0" "$(jq -r '.pkgver // ""' <<<"$out")" + check "x86_64 checksum via * binary-mode prefix" "$sum_x20" "$(jq -r '.sha256sums.x86_64[0] // ""' <<<"$out")" + + out=$(github_upstream_release "$pkg" 8640000 2>/dev/null) || out="" + check "everything quarantined reports no update" "{}" "$(jq -c . <<<"$out")" + + printf 'pkgver=1.9.0\npkgrel=1\n' > "$pkg/PKGBUILD" + out=$(github_upstream_release "$pkg" 86400 2>/dev/null) || out="" + check "already checked in reports no update" "{}" "$(jq -c . <<<"$out")" + printf 'pkgver=1.0.0\npkgrel=1\n' > "$pkg/PKGBUILD" + + echo "Failure paths:" + local rc + FIXTURE_RELEASES=$(jq -n '[{tag_name: "v1.9.0", published_at: "not-a-date", draft: false, prerelease: false}]') + rc=0; github_upstream_release "$pkg" 86400 >/dev/null 2>&1 || rc=$? + check "invalid published_at fails the sync" "1" "$rc" + + FIXTURE_RELEASES=$(jq -n --arg old "$old2d" '[{tag_name: "release 1.9!", published_at: $old, draft: false, prerelease: false}]') + rc=0; github_upstream_release "$pkg" 86400 >/dev/null 2>&1 || rc=$? + check "unusable tag fails the sync" "1" "$rc" + + FIXTURE_RELEASES=$(jq -n --arg old "$old2d" '[{tag_name: "v1.9.0", published_at: $old, draft: false, prerelease: false}]') + FIXTURE_CHECKSUMS="$sum_x19 ./tool-v1.9.0-x64.tar.xz" + rc=0; github_upstream_release "$pkg" 86400 >/dev/null 2>&1 || rc=$? + check "missing aarch64 checksum fails the sync" "1" "$rc" + + echo "Quarantine backstop:" + local rel st + rel=$(jq -n --arg p "$old2d" '{pkgver: "1.9.0", published_at: $p, sha256sums: {}}') + st=0; release_age_status "$rel" 86400 || st=$? + check "old enough passes" "0" "$st" + rel=$(jq -n --arg p "$young" '{pkgver: "2.0.0", published_at: $p, sha256sums: {}}') + st=0; release_age_status "$rel" 86400 || st=$? + check "too young is held" "1" "$st" + st=0; release_age_status "$rel" 0 || st=$? + check "no policy passes anything" "0" "$st" + st=0; BYPASS_MIN_RELEASE_AGE=1 release_age_status "$rel" 86400 || st=$? + check "deliberate bypass passes" "0" "$st" + rel=$(jq -n '{pkgver: "2.0.0", sha256sums: {}}') + st=0; release_age_status "$rel" 86400 || st=$? + check "missing published_at is unprovable" "2" "$st" + + echo "Duration parser:" + local agepkg="$TEMP_DIR/selftest-age" + mkdir -p "$agepkg/.omarchy" + check_age() { + local json_value="$1" expected="$2" got + jq -n "{source: \"local\", min_release_age: $json_value}" > "$agepkg/.omarchy/package.json" + got=$(package_min_release_age_seconds "$agepkg") || got="" + check "min_release_age $json_value" "$expected" "$got" + } + check_age '"24h"' 86400 + check_age '"90m"' 5400 + check_age '"2d"' 172800 + check_age '3600' 3600 + check_age '"600s"' 600 + check_age '"010h"' 36000 + check_age '"abc"' "" + check_age '"24hh"' "" + check_age 'false' "" + check_age '"9999999999"' "" + + echo "Manifest validation:" + printf 'pkgver=1.0.0\n' > "$agepkg/PKGBUILD" + local vst + echo '{"source": "local", "upstream": false}' > "$agepkg/.omarchy/package.json" + vst=0; validate_package_metadata "$agepkg" >/dev/null || vst=$? + check "upstream: false is rejected" "1" "$vst" + echo '{"source": "local", "upstream": {"github": "example/tool"}}' > "$agepkg/.omarchy/package.json" + vst=0; validate_package_metadata "$agepkg" >/dev/null || vst=$? + check "upstream without checksums/assets is rejected" "1" "$vst" + cp "$pkg/.omarchy/package.json" "$agepkg/.omarchy/package.json" + vst=0; validate_package_metadata "$agepkg" >/dev/null || vst=$? + check "the real declaration shape is accepted" "0" "$vst" + + echo "" + if [[ "$failures" -eq 0 ]]; then + print_success "Self-test passed" + else + print_error "$failures self-test failure(s)" + exit 1 + fi +} + +if [[ ${#SPECIFIC_PACKAGES[@]} -gt 0 && "${SPECIFIC_PACKAGES[0]}" == "self-test" ]]; then + cmd_self_test + exit 0 +fi + if [[ ${#SPECIFIC_PACKAGES[@]} -gt 0 ]]; then SPECIFIC_MODE=true for package in "${SPECIFIC_PACKAGES[@]}"; do diff --git a/helpers/package-metadata.sh b/helpers/package-metadata.sh index 561c3de..01f828e 100644 --- a/helpers/package-metadata.sh +++ b/helpers/package-metadata.sh @@ -82,17 +82,29 @@ package_is_fast_ring() { # Quarantine window for upstream releases, in seconds. Accepts a bare number # of seconds or a number suffixed s/m/h/d ("24h", "2d"). Unset means 0 (no -# hold); an unparseable value returns 1 so callers fail closed instead of -# silently dropping the hold. +# hold); an unparseable value -- including a non-string/non-number JSON type +# like false -- returns 1 so callers fail closed instead of silently dropping +# the hold. At most 9 digits: enough for three decades in seconds, and small +# enough that no suffix multiplication can overflow 64-bit arithmetic. package_min_release_age_seconds() { - local pkgdir="$1" raw - raw=$(package_metadata_value "$pkgdir" '.min_release_age' "") + local pkgdir="$1" metadata raw + metadata=$(metadata_file_for_dir "$pkgdir") + if [[ ! -f "$metadata" ]]; then + echo 0 + return 0 + fi + raw=$(jq -r ' + if has("min_release_age") then + .min_release_age | if type == "string" or type == "number" then tostring else "unparseable" end + else "" end + ' "$metadata") if [[ -z "$raw" ]]; then echo 0 return 0 fi - [[ "$raw" =~ ^([0-9]+)([smhd]?)$ ]] || return 1 - local n=${BASH_REMATCH[1]} + [[ "$raw" =~ ^([0-9]{1,9})([smhd]?)$ ]] || return 1 + # Forced base 10: bash arithmetic would otherwise read "010" as octal. + local n=$((10#${BASH_REMATCH[1]})) case "${BASH_REMATCH[2]}" in ""|s) echo "$n" ;; m) echo $((n * 60)) ;; @@ -167,7 +179,8 @@ package_has_upstream_hook() { package_has_upstream_provider() { local pkgdir="$1" - [[ -n "$(package_metadata_value "$pkgdir" '.upstream.github' "")" ]] + # `objects` drops a non-object upstream value instead of erroring jq. + [[ -n "$(package_metadata_value "$pkgdir" '(.upstream? | objects | .github)' "")" ]] } packages_for_upstream_sync() { @@ -346,15 +359,18 @@ validate_package_metadata() { return 1 fi + # `has` rather than `// {}`: jq's // treats false as absent, which would + # let "upstream": false slip through as an empty declaration. if ! jq -e ' - (.upstream // {}) | type == "object" - and (if . == {} then true else + if has("upstream") | not then true + elif (.upstream | type) != "object" then false + else .upstream | ((.github // "") | type == "string" and test("\\A[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+\\z")) and ((.checksums // "") | type == "string" and length > 0) and ((.assets // {}) | type == "object" and length > 0 and (to_entries | all( (.key | test("\\A[a-z0-9_]+\\z")) and (.value | type == "string" and length > 0) ))) - end) + end ' "$metadata" >/dev/null; then echo "invalid upstream for $(basename "$pkgdir"): needs github owner/repo, checksums asset name, and an assets arch->name map" return 1 diff --git a/helpers/upstream-github.sh b/helpers/upstream-github.sh index adf2b4b..297056c 100644 --- a/helpers/upstream-github.sh +++ b/helpers/upstream-github.sh @@ -21,7 +21,24 @@ package_upstream_github_repo() { local pkgdir="$1" - package_metadata_value "$pkgdir" '.upstream.github' "" + # `objects` drops a non-object upstream value (validation rejects those + # separately) instead of erroring the jq pipeline. + package_metadata_value "$pkgdir" '(.upstream? | objects | .github)' "" +} + +# Fetches sit behind functions so the self-test can replace them with fixture +# readers; everything below the fetch is deterministic and testable offline. +# Only the 100 most recent releases are considered -- a bounded search, not +# pagination. A feed whose entire first page is drafts, prereleases, or +# quarantined releases reports no update and waits for the next run. +github_fetch_releases() { + local repo="$1" + curl -fsSL "https://api.github.com/repos/$repo/releases?per_page=100" +} + +github_fetch_checksums() { + local repo="$1" tag="$2" asset="$3" + curl -fsSL "https://github.com/$repo/releases/download/$tag/$asset" } # Emits the newest qualifying release as hook-contract JSON. min_release_age @@ -35,18 +52,18 @@ github_upstream_release() { local metadata repo checksums_name metadata=$(metadata_file_for_dir "$package_dir") - repo=$(jq -r '.upstream.github // ""' "$metadata") + repo=$(jq -r '(.upstream? | objects | .github) // ""' "$metadata") if [[ ! "$repo" =~ ^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$ ]]; then echo "invalid upstream.github repository: '${repo:-}'" >&2 return 1 fi - checksums_name=$(jq -r '.upstream.checksums // ""' "$metadata") + checksums_name=$(jq -r '(.upstream? | objects | .checksums) // ""' "$metadata") if [[ -z "$checksums_name" ]]; then echo "upstream.checksums names the checksum manifest asset and is required" >&2 return 1 fi local arches - mapfile -t arches < <(jq -r '.upstream.assets // {} | keys[]' "$metadata") + mapfile -t arches < <(jq -r '(.upstream? | objects | .assets) // {} | keys[]' "$metadata") if [[ ${#arches[@]} -eq 0 ]]; then echo "upstream.assets must map at least one architecture to an asset name" >&2 return 1 @@ -54,7 +71,7 @@ github_upstream_release() { local releases now now=$(date +%s) - if ! releases=$(curl -fsSL "https://api.github.com/repos/$repo/releases?per_page=20"); then + if ! releases=$(github_fetch_releases "$repo"); then echo "could not fetch the release feed for $repo" >&2 return 1 fi @@ -108,7 +125,7 @@ github_upstream_release() { fi local checksums - if ! checksums=$(curl -fsSL "https://github.com/$repo/releases/download/$best_tag/$checksums_name"); then + if ! checksums=$(github_fetch_checksums "$repo" "$best_tag" "$checksums_name"); then echo "could not fetch $checksums_name for $repo $best_tag" >&2 return 1 fi