From 49418942c898c8ed9ea604c35a71f0d5275ae26c Mon Sep 17 00:00:00 2001
From: Adolanium <94890352+Adolanium@users.noreply.github.com>
Date: Tue, 15 Sep 2026 18:50:20 +0300
Subject: [PATCH] Merge pull request #7807 from Adolanium/keyring-fail-loud

Stop update-keyring from claiming success when key operations fail
---
 bin/omarchy-update-keyring          |   9 ++
 test/shell.d/update-keyring-test.sh | 137 ++++++++++++++++++++++++++++
 2 files changed, 146 insertions(+)
 create mode 100644 test/shell.d/update-keyring-test.sh

diff --git a/bin/omarchy-update-keyring b/bin/omarchy-update-keyring
index d561e22a..3a1c1c17 100755
--- a/bin/omarchy-update-keyring
+++ b/bin/omarchy-update-keyring
@@ -3,6 +3,11 @@
 # omarchy:summary=Ensure the Omarchy and Arch keyring packages are installed and populated
 # omarchy:requires-sudo=true
 
+# omarchy-update runs this under set -e as a trusted pre-step, so a failed recv
+# or a broken keyring has to stop this script here, not surface later as
+# signature errors in the middle of the main transaction.
+set -euo pipefail
+
 if omarchy-pkg-missing omarchy-keyring || ! sudo pacman-key --list-keys 40DFB630FF42BCFFB047046CF0134EE680CAC571 &>/dev/null; then
   sudo pacman-key --recv-keys 40DFB630FF42BCFFB047046CF0134EE680CAC571 --keyserver keys.openpgp.org
   sudo pacman-key --lsign-key 40DFB630FF42BCFFB047046CF0134EE680CAC571
@@ -19,4 +24,8 @@ fi
 # Always reinstall, as the keyring can be updated without a package version bump.
 echo -e "\e[32m\nUpdate Arch signing keys\e[0m"
 sudo pacman -Sy --noconfirm archlinux-keyring >/dev/null 2> >(grep -vE '^warning: archlinux-keyring-[^ ]+ is up to date -- reinstalling$' >&2)
+
+# Say "correct" only once the key verifiably is: before the failure checks
+# above, a failed recv or reinstall still ended here with exit 0.
+sudo pacman-key --list-keys 40DFB630FF42BCFFB047046CF0134EE680CAC571 >/dev/null
 echo "Keys are correct"
diff --git a/test/shell.d/update-keyring-test.sh b/test/shell.d/update-keyring-test.sh
new file mode 100644
index 00000000..15d5bb52
--- /dev/null
+++ b/test/shell.d/update-keyring-test.sh
@@ -0,0 +1,137 @@
+#!/bin/bash
+
+set -euo pipefail
+
+source "$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)/base-test.sh"
+
+test_tmp=$(mktemp -d)
+trap 'rm -rf "$test_tmp"' EXIT
+
+stub_bin="$test_tmp/bin"
+log_file="$test_tmp/keyring.log"
+mkdir -p "$stub_bin"
+
+# Behavior is driven by env vars so each case can pick its failure point:
+# KEYRING_TEST_PKG_MISSING     exit status of omarchy-pkg-missing (default 1: installed)
+# KEYRING_TEST_LIST_FAIL_ON    which --list-keys call fails, counted per run (default: none)
+# KEYRING_TEST_RECV_STATUS     exit status of --recv-keys (default 0)
+# KEYRING_TEST_REINSTALL_STATUS exit status of the archlinux-keyring reinstall (default 0)
+cat >"$stub_bin/sudo" <<'SH'
+#!/bin/bash
+
+printf 'sudo' >>"$KEYRING_TEST_LOG"
+for arg in "$@"; do
+  printf '\t%s' "$arg" >>"$KEYRING_TEST_LOG"
+done
+printf '\n' >>"$KEYRING_TEST_LOG"
+
+if [[ $1 == "pacman-key" && $2 == "--list-keys" ]]; then
+  calls_file="$KEYRING_TEST_DIR/list-calls"
+  calls=$(( $(cat "$calls_file" 2>/dev/null || echo 0) + 1 ))
+  echo "$calls" >"$calls_file"
+  if [[ ${KEYRING_TEST_LIST_FAIL_ON:-} == "$calls" ]]; then
+    exit 1
+  fi
+  exit 0
+fi
+
+if [[ $1 == "pacman-key" && $2 == "--recv-keys" ]]; then
+  exit "${KEYRING_TEST_RECV_STATUS:-0}"
+fi
+
+if [[ $1 == "pacman-key" && $2 == "--lsign-key" ]]; then
+  exit 0
+fi
+
+if [[ $1 == "pacman" && $* == *archlinux-keyring* ]]; then
+  exit "${KEYRING_TEST_REINSTALL_STATUS:-0}"
+fi
+
+exit 0
+SH
+chmod +x "$stub_bin/sudo"
+
+cat >"$stub_bin/omarchy-pkg-missing" <<'SH'
+#!/bin/bash
+
+exit "${KEYRING_TEST_PKG_MISSING:-1}"
+SH
+chmod +x "$stub_bin/omarchy-pkg-missing"
+
+cat >"$stub_bin/omarchy-pkg-add" <<'SH'
+#!/bin/bash
+
+printf 'pkg-add\t%s\n' "$1" >>"$KEYRING_TEST_LOG"
+exit 0
+SH
+chmod +x "$stub_bin/omarchy-pkg-add"
+
+run_keyring() {
+  KEYRING_TEST_LOG="$log_file" \
+    KEYRING_TEST_DIR="$test_tmp" \
+    PATH="$stub_bin:$PATH" \
+    "$ROOT/bin/omarchy-update-keyring" "$@"
+}
+
+# Everything healthy: the key and package are present, the reinstall works.
+: >"$log_file"
+rm -f "$test_tmp/list-calls"
+run_keyring >"$test_tmp/ok.out"
+
+grep -F "Keys are correct" "$test_tmp/ok.out" >/dev/null ||
+  fail "update-keyring reports success when the keyring is healthy" "$(cat "$test_tmp/ok.out")"
+pass "update-keyring reports success when the keyring is healthy"
+
+grep -Eq $'^sudo\tpacman\t-Sy\t--noconfirm\tarchlinux-keyring$' "$log_file" ||
+  fail "update-keyring still reinstalls archlinux-keyring" "$(cat "$log_file")"
+pass "update-keyring still reinstalls archlinux-keyring"
+
+# Key and package missing: the full populate path runs and verifies at the end.
+: >"$log_file"
+rm -f "$test_tmp/list-calls"
+KEYRING_TEST_PKG_MISSING=0 run_keyring >"$test_tmp/populate.out"
+
+grep -F "Keys are correct" "$test_tmp/populate.out" >/dev/null ||
+  fail "update-keyring populates a missing keyring and reports success" "$(cat "$test_tmp/populate.out")"
+for expected in 'recv-keys' 'lsign-key' $'pkg-add\tomarchy-keyring'; do
+  grep -Eq "$expected" "$log_file" ||
+    fail "update-keyring populates a missing keyring and reports success" "$(cat "$log_file")"
+done
+pass "update-keyring populates a missing keyring and reports success"
+
+# recv-keys failing must stop the script, not end in "Keys are correct".
+: >"$log_file"
+rm -f "$test_tmp/list-calls"
+if KEYRING_TEST_PKG_MISSING=0 KEYRING_TEST_RECV_STATUS=1 run_keyring >"$test_tmp/recv.out" 2>&1; then
+  fail "update-keyring fails when recv-keys fails"
+fi
+if grep -F "Keys are correct" "$test_tmp/recv.out" >/dev/null; then
+  fail "update-keyring fails when recv-keys fails" "$(cat "$test_tmp/recv.out")"
+fi
+if grep -q 'lsign-key' "$log_file"; then
+  fail "update-keyring stops at the failed recv instead of signing anyway" "$(cat "$log_file")"
+fi
+pass "update-keyring fails when recv-keys fails"
+
+# A failed archlinux-keyring reinstall must not end in success either.
+: >"$log_file"
+rm -f "$test_tmp/list-calls"
+if KEYRING_TEST_REINSTALL_STATUS=1 run_keyring >"$test_tmp/reinstall.out" 2>&1; then
+  fail "update-keyring fails when the archlinux-keyring reinstall fails"
+fi
+if grep -F "Keys are correct" "$test_tmp/reinstall.out" >/dev/null; then
+  fail "update-keyring fails when the archlinux-keyring reinstall fails" "$(cat "$test_tmp/reinstall.out")"
+fi
+pass "update-keyring fails when the archlinux-keyring reinstall fails"
+
+# The closing check is what backs the success line: the first --list-keys
+# passes (key present, populate skipped), the verifying one fails.
+: >"$log_file"
+rm -f "$test_tmp/list-calls"
+if KEYRING_TEST_LIST_FAIL_ON=2 run_keyring >"$test_tmp/verify.out" 2>&1; then
+  fail "update-keyring fails when the final key check fails"
+fi
+if grep -F "Keys are correct" "$test_tmp/verify.out" >/dev/null; then
+  fail "update-keyring fails when the final key check fails" "$(cat "$test_tmp/verify.out")"
+fi
+pass "update-keyring fails when the final key check fails"