From b5832fc94a533e213163fb673c0576bf293d021c Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Thu, 27 Aug 2026 17:31:46 -0400 Subject: [PATCH] Fix installer state and launcher freshness checks --- install | 28 +++++++---- setup/lib/artifact-provenance | 11 ++++- setup/scripts/link-vicinae-scripts | 12 +++-- tests/setup/launcher-search-contract | 63 ++++++++++++++++++++++++- tests/setup/package-provenance-contract | 25 ++++++++++ tests/setup/update-command-contract | 36 ++++++++++++-- 6 files changed, 156 insertions(+), 19 deletions(-) diff --git a/install b/install index 6ae336c..993cec5 100755 --- a/install +++ b/install @@ -71,7 +71,7 @@ source "$PANAMA_PATH/bin/ascii" # time. On an upgrade it is worth running only when its package lists or # reviewed installer trust inputs changed, so this hashes them and remembers # the result. The framed, sorted stream includes top-level package files, the -# package-stage adapter, the provenance helper, and regular provenance files; +# package-stage adapter, every helper it sources, and regular provenance files; # both relative paths and bytes are part of the state. # # A content hash rather than a git range, because Panama is developed in place: @@ -93,7 +93,10 @@ hash_packages() { for fixed_input in \ "$PANAMA_PATH/setup/scripts/install-packages" \ - "$PANAMA_PATH/setup/lib/artifact-provenance"; do + "$PANAMA_PATH/setup/lib/artifact-provenance" \ + "$PANAMA_PATH/setup/lib/chatgpt-package" \ + "$PANAMA_PATH/setup/lib/extras-catalog" \ + "$PANAMA_PATH/setup/lib/machine-role"; do [[ -f "$fixed_input" && ! -L "$fixed_input" && -r "$fixed_input" ]] || return 1 done @@ -102,7 +105,10 @@ hash_packages() { find "$PANAMA_PATH/setup/packages" -maxdepth 1 -type f -print0 || exit 1 printf '%s\0' \ "$PANAMA_PATH/setup/scripts/install-packages" \ - "$PANAMA_PATH/setup/lib/artifact-provenance" || exit 1 + "$PANAMA_PATH/setup/lib/artifact-provenance" \ + "$PANAMA_PATH/setup/lib/chatgpt-package" \ + "$PANAMA_PATH/setup/lib/extras-catalog" \ + "$PANAMA_PATH/setup/lib/machine-role" || exit 1 find "$PANAMA_PATH/setup/provenance" -type f -print0 || exit 1 } | LC_ALL=C sort -z | while IFS= read -r -d '' file; do relative="${file#"$PANAMA_PATH"/}" @@ -116,12 +122,11 @@ hash_packages() { } packages_needed() { - local current_hash recorded_hash + local current_hash="$1" recorded_hash (( FORCE_PACKAGES )) && return 0 (( UPGRADE )) || return 0 [[ -r "$PACKAGES_HASH" ]] || return 0 - current_hash="$(hash_packages)" || return 2 recorded_hash="$(cat "$PACKAGES_HASH")" || return 2 [[ "$current_hash" != "$recorded_hash" ]] } @@ -130,10 +135,12 @@ packages_needed() { # documents for its markers: a step that did not complete has not happened, and # recording it as done hides it forever. record_packages_hash() { - local temporary_hash + local starting_hash="$1" current_hash temporary_hash + current_hash="$(hash_packages)" || return 1 + [[ "$current_hash" == "$starting_hash" ]] || return 1 mkdir -p "$STATE_DIR" temporary_hash="$(mktemp "$STATE_DIR/.packages-hash.XXXXXX")" || return 1 - if hash_packages >"$temporary_hash"; then + if printf '%s\n' "$starting_hash" >"$temporary_hash"; then mv -f -- "$temporary_hash" "$PACKAGES_HASH" else rm -f -- "$temporary_hash" @@ -344,7 +351,10 @@ for stage in "${STAGES[@]}"; do printf '\n=== %s ===\n' "$stage" if [[ "$stage" == install-packages ]]; then package_state_status=0 - packages_needed || package_state_status=$? + package_start_hash="$(hash_packages)" || package_state_status=2 + if (( package_state_status == 0 )); then + packages_needed "$package_start_hash" || package_state_status=$? + fi if (( package_state_status == 1 )); then echo "The package lists have not changed since the last run; skipping." echo "Run with --packages to install them anyway." @@ -357,7 +367,7 @@ for stage in "${STAGES[@]}"; do fi if "$script"; then if [[ "$stage" == install-packages ]]; then - if ! record_packages_hash; then + if ! record_packages_hash "$package_start_hash"; then failed+=("$stage") printf '!!! %s could not record its tracked installation inputs\n' "$stage" >&2 fi diff --git a/setup/lib/artifact-provenance b/setup/lib/artifact-provenance index 317d51a..615632a 100755 --- a/setup/lib/artifact-provenance +++ b/setup/lib/artifact-provenance @@ -8,6 +8,7 @@ declare -gA INSTALLER_PROVENANCE=() _primary_key_fingerprints() ( local home + set -o pipefail home="$(mktemp -d)" || exit 1 chmod 700 "$home" trap 'rm -rf -- "$home"' EXIT @@ -19,14 +20,20 @@ _primary_key_fingerprints() ( key_fingerprint_matches() { local file="$1" expected="$2" + local output local -a primary_fingerprints=() - mapfile -t primary_fingerprints < <(_primary_key_fingerprints "$file") + output="$(_primary_key_fingerprints "$file")" || return 1 + [[ -n "$output" ]] || return 1 + mapfile -t primary_fingerprints <<<"$output" [[ ${#primary_fingerprints[@]} -eq 1 && "${primary_fingerprints[0]}" == "$expected" ]] } _key_has_one_primary() { + local output local -a primary_fingerprints=() - mapfile -t primary_fingerprints < <(_primary_key_fingerprints "$1") + output="$(_primary_key_fingerprints "$1")" || return 1 + [[ -n "$output" ]] || return 1 + mapfile -t primary_fingerprints <<<"$output" [[ ${#primary_fingerprints[@]} -eq 1 ]] } diff --git a/setup/scripts/link-vicinae-scripts b/setup/scripts/link-vicinae-scripts index 843ee33..40ac28e 100755 --- a/setup/scripts/link-vicinae-scripts +++ b/setup/scripts/link-vicinae-scripts @@ -85,9 +85,15 @@ if [[ -d "$extensions_source" ]] && command -v npm >/dev/null 2>&1; then # alone takes long enough to be worth not repeating on every re-run of # a stage that is otherwise nearly instant. built="$vicinae_data_dir/extensions/$name" - if [[ -d "$built" && "$extension/src" -ot "$built" ]]; then - printf 'Vicinae extension %s is already built\n' "$name" - continue + if [[ -d "$built" ]]; then + newer_source='' + if newer_source="$(find "$extension/src" -type f -newer "$built" -print -quit)" \ + && [[ -z "$newer_source" \ + && ! "$extension/package.json" -nt "$built" \ + && ! "$extension/package-lock.json" -nt "$built" ]]; then + printf 'Vicinae extension %s is already built\n' "$name" + continue + fi fi printf 'Building Vicinae extension %s\n' "$name" diff --git a/tests/setup/launcher-search-contract b/tests/setup/launcher-search-contract index 1b9275e..204934d 100755 --- a/tests/setup/launcher-search-contract +++ b/tests/setup/launcher-search-contract @@ -123,7 +123,8 @@ grep -q '/etc/profile.d/nvm.sh' "$stage" \ || note 'the extension build never sources nvm, so npm is missing on any machine without a system node' # node_modules is a dependency tree, not configuration. -git -C "$repo_dir" check-ignore -q "$extension/node_modules" 2>/dev/null \ +git -C "$repo_dir" check-ignore --no-index -q \ + "$extension/node_modules/package.json" 2>/dev/null \ || note 'the extension node_modules is not gitignored' # npm must honour the committed dependency graph. This disposable fixture @@ -167,6 +168,66 @@ stage_output="$(PATH="$fixture_root/bin:$PATH" PANAMA_PATH="$fixture_root" \ cmp -s -- "$lock_before" "$lockfile" \ || note 'a rejected Vicinae lockfile mismatch changed package-lock.json' +# Editing an existing source file does not change its parent directory's +# timestamp, so freshness must inspect files rather than the src directory. +freshness_root="$fixture_root/freshness" +mkdir -p "$freshness_root/config/local/share/vicinae/scripts" \ + "$freshness_root/config/local/share/vicinae/extensions/panama-search/src" \ + "$freshness_root/bin" +freshness_extension="$freshness_root/config/local/share/vicinae/extensions/panama-search" +cp -- "$manifest" "$freshness_extension/package.json" +cp -- "$repo_dir/config/local/share/vicinae/extensions/panama-search/package-lock.json" \ + "$freshness_extension/package-lock.json" +cp -- "$extension/src/search.tsx" "$freshness_extension/src/search.tsx" +cat >"$freshness_root/bin/npm" <<'EOF' +#!/usr/bin/env bash +printf '%s\n' "$*" >>"${NPM_LOG:?}" +if [[ "${1:-}" == ci ]]; then + exit 0 +fi +if [[ "${1:-} ${2:-}" == 'run build' ]]; then + built="${VICINAE_DATA_DIR:?}/extensions/$(basename "$PWD")" + mkdir -p "$built" + touch "$built" + exit 0 +fi +exit 64 +EOF +chmod +x "$freshness_root/bin/npm" +: >"$freshness_root/npm.log" +run_freshness_stage() { + PATH="$freshness_root/bin:$PATH" PANAMA_PATH="$freshness_root" \ + VICINAE_DATA_DIR="$freshness_root/vicinae-data" \ + NPM_LOG="$freshness_root/npm.log" bash "$stage" >/dev/null 2>&1 +} +run_freshness_stage || note 'the Vicinae freshness fixture did not build initially' +initial_builds="$(grep -c '^run build$' "$freshness_root/npm.log")" +run_freshness_stage || note 'the unchanged Vicinae freshness fixture failed' +unchanged_builds="$(grep -c '^run build$' "$freshness_root/npm.log")" +[[ "$unchanged_builds" == "$initial_builds" ]] \ + || note 'an unchanged Vicinae extension rebuilt unnecessarily' +touch -d '2030-01-01 UTC' "$freshness_extension/src/search.tsx" +run_freshness_stage || note 'the source-changed Vicinae freshness fixture failed' +source_changed_builds="$(grep -c '^run build$' "$freshness_root/npm.log")" +[[ "$source_changed_builds" -eq $(( initial_builds + 1 )) ]] \ + || note 'editing an existing Vicinae source file did not trigger a rebuild' + +built_extension="$freshness_root/vicinae-data/extensions/panama-search" +touch -r "$built_extension" "$freshness_extension/src/search.tsx" +touch -d '2031-01-01 UTC' "$freshness_extension/package.json" +run_freshness_stage || note 'the manifest-changed Vicinae freshness fixture failed' +manifest_changed_builds="$(grep -c '^run build$' "$freshness_root/npm.log")" +[[ "$manifest_changed_builds" -eq $(( source_changed_builds + 1 )) ]] \ + || note 'changing a Vicinae package.json did not trigger a rebuild' + +touch -r "$built_extension" "$freshness_extension/package.json" \ + "$freshness_extension/src/search.tsx" +touch -d '2032-01-01 UTC' "$freshness_extension/package-lock.json" +run_freshness_stage || note 'the lockfile-changed Vicinae freshness fixture failed' +lockfile_changed_builds="$(grep -c '^run build$' "$freshness_root/npm.log")" +[[ "$lockfile_changed_builds" -eq $(( manifest_changed_builds + 1 )) ]] \ + || note 'changing a Vicinae package-lock.json did not trigger a rebuild' + # ── Report ─────────────────────────────────────────────────────────────────── if (( ${#findings[@]} > 0 )); then diff --git a/tests/setup/package-provenance-contract b/tests/setup/package-provenance-contract index 2284101..faca7c9 100755 --- a/tests/setup/package-provenance-contract +++ b/tests/setup/package-provenance-contract @@ -164,6 +164,31 @@ expect_failure key_fingerprint_matches "$fixtures/fixture-key.asc" '000000000000 cat "$fixtures/fixture-key.asc" "$fixtures/wrong-signer-key.asc" > "$test_tmp/combined-key.asc" expect_failure key_fingerprint_matches "$test_tmp/combined-key.asc" "$fixture_fingerprint" +# A parser must not accept plausible output from a GPG process that failed. +# The later import and verify calls succeed so both public helpers depend on +# the show-only producer's status rather than failing for an unrelated reason. +producer_failure_bin="$test_tmp/gpg-producer-failure-bin" +mkdir "$producer_failure_bin" +cat > "$producer_failure_bin/gpg" <>"$tmp/a/$relative" install_status=0 ran_input_changed="$(run_install "$tmp/a" --upgrade)" || install_status=$? @@ -282,7 +290,7 @@ done # Fixed hash inputs must not silently disappear or degrade into a directory or # link. An unreadable package input also proves a failed content read cannot be # hidden by the final digest command. -for fixed_input in setup/scripts/install-packages setup/lib/artifact-provenance; do +for fixed_input in setup/scripts/install-packages "${PACKAGE_BEHAVIOR_INPUTS[@]}"; do for case_name in missing directory symlink unreadable; do case_root="$tmp/hash-${fixed_input//\//-}-$case_name" build_fixture "$case_root" @@ -320,6 +328,26 @@ grep -qx 'install-packages' <<<"$ran_hash_failure" \ cmp -s -- "$tmp/hash-failure/stamp-before" "$tmp/hash-failure/state/panama/packages-hash" \ || note 'a failed package-state hash wrote a new packages-hash stamp' +# Inputs changed while install-packages was running were not the inputs it +# consumed at the start. Do not stamp the later bytes as successfully applied. +build_fixture "$tmp/hash-mid-stage-drift" +cat >"$tmp/hash-mid-stage-drift/setup/scripts/install-packages" <<'EOF' +#!/usr/bin/env bash +if [[ "${1:-}" == --trust-preflight ]]; then + printf 'trust-preflight\n' >>"$PANAMA_RAN" + exit 0 +fi +printf 'install-packages\n' >>"$PANAMA_RAN" +printf '\nchanged during package installation\n' >>"$PANAMA_PATH/setup/lib/machine-role" +EOF +chmod +x "$tmp/hash-mid-stage-drift/setup/scripts/install-packages" +install_status=0 +run_install "$tmp/hash-mid-stage-drift" --upgrade >/dev/null || install_status=$? +[[ "$install_status" -ne 0 ]] \ + || note 'mid-stage package input drift returned success' +[[ ! -e "$tmp/hash-mid-stage-drift/state/panama/packages-hash" ]] \ + || note 'mid-stage package input drift stamped bytes the stage did not start with' + # A failing stage must not record the hash, or the failure is hidden forever. build_fixture "$tmp/c" 1 install_status=0