From 4e79e047f4d9daf2a8749b4e3490bde994d38931 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Sat, 10 Oct 2026 11:14:58 +0100 Subject: [PATCH] Bind unattended updater service discovery before installer handoff The helper selects and passes an explicit service name, bypassing installer discovery. Apply the same bounded exact-unit admission there and preserve its error before any prior-active query, backup, installer call or restart trap. Execute both producers and the real helper handoff in the registered unattended updater proof artifact. Refs: #2785 Change-source: pulse-maintainer --- .../subsystems/deployment-installability.md | 11 +++ scripts/installtests/install_sh_test.go | 8 ++- .../installtests/pulse_auto_update_test.go | 21 ++++++ scripts/pulse-auto-update.sh | 48 +++++++++++-- .../tests/test_server_service_discovery.py | 69 +++++++++++++++++-- 5 files changed, 144 insertions(+), 13 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index f28e79e9c6..1c4a607d02 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -26,6 +26,13 @@ partial, malformed, duplicate, impostor or unknown-state inventory does not. This avoids an early pipe close hiding an installed legacy service under `pipefail`. Discovery itself never stops, enables or unmasks a unit. +The standalone unattended updater uses the same bounded name/state admission +before passing its explicit service identity to the installer. It checks the +command-substitution result separately from local variable declaration, and +refuses unknown inventory before observing prior-active state, creating backup +files, downloading/invoking the installer or installing a restart trap. An +installer-only repair cannot cover this earlier explicit-name handoff. + Existing-install detection distinguishes an unavailable inventory from no installation. Main, archive/source replacement, unit creation, removal and reset propagate the refusal even when a caller disables Bash errexit: no @@ -40,6 +47,10 @@ optional real `systemctl --root` inventory. Only the unit-writer test relocates its fixed output path. It proves selection and refusal propagation, not release signature trust, real compilation, native systemd restart, an LXC stop cause or installed update/recovery. Those remain separate operational evidence. +`TestAutoUpdateServiceDiscoveryAndInstallerHandoff` uses the same fixture to +execute the actual unattended helper's selection and +`perform_update` entry/handoff with a fake installer, including timeout and +failed/partial inventory before any service observation or installer call. ### Core E2E images are built once and admitted in the same run diff --git a/scripts/installtests/install_sh_test.go b/scripts/installtests/install_sh_test.go index 39da655997..b8cf615de3 100644 --- a/scripts/installtests/install_sh_test.go +++ b/scripts/installtests/install_sh_test.go @@ -4556,13 +4556,15 @@ func TestRootInstallTimerDiscoveryReachesConsentPreservingRefresh(t *testing.T) } } -// Execute bounded service discovery with every mutation-owning consumer, -// including OR-list callers where Bash does not apply errexit to the function. +// Execute both standalone discovery producers and every mutation-owning +// consumer, including OR-list callers where Bash disables function errexit. func TestRootInstallServiceDiscoveryFailsClosedWithoutShortReads(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) defer cancel() cmd := exec.CommandContext(ctx, "python3", repoFile("scripts", "tests", "test_server_service_discovery.py"), "-v") - cmd.Env = append(os.Environ(), "PULSE_INSTALLER_UNDER_TEST="+repoFile("install.sh")) + cmd.Env = append(os.Environ(), + "PULSE_INSTALLER_UNDER_TEST="+repoFile("install.sh"), + "PULSE_AUTO_UPDATER_UNDER_TEST="+repoFile("scripts", "pulse-auto-update.sh")) out, err := cmd.CombinedOutput() if err != nil { t.Fatalf("installer service discovery/caller controls: %v\n%s", err, out) diff --git a/scripts/installtests/pulse_auto_update_test.go b/scripts/installtests/pulse_auto_update_test.go index 4e85267a6d..3401b84de4 100644 --- a/scripts/installtests/pulse_auto_update_test.go +++ b/scripts/installtests/pulse_auto_update_test.go @@ -1,13 +1,34 @@ package installtests import ( + "context" "os" "os/exec" "path/filepath" "strings" "testing" + "time" ) +// The standalone helper passes an explicit name to the installer. Exercise +// that producer and its real entry/handoff, not just installer discovery. +func TestAutoUpdateServiceDiscoveryAndInstallerHandoff(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 45*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "python3", repoFile("scripts", "tests", "test_server_service_discovery.py"), "-v", + "ServiceDiscovery.test_automatic_helper_preserves_exact_names_and_explicit_instances", + "ServiceDiscovery.test_automatic_helper_cannot_short_read_the_installer_handoff", + "ServiceDiscovery.test_automatic_helper_refuses_before_backup_or_installer_even_in_or_list", + "ServiceDiscovery.test_automatic_helper_timeout_stops_before_service_observation") + cmd.Env = append(os.Environ(), + "PULSE_INSTALLER_UNDER_TEST="+repoFile("install.sh"), + "PULSE_AUTO_UPDATER_UNDER_TEST="+repoFile("scripts", "pulse-auto-update.sh")) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("unattended service discovery/installer handoff controls: %v\n%s", err, out) + } +} + // TestIsPrereleaseTagRecognizesPrereleases asserts that any semver tag with // a hyphen after the patch component is flagged as a prerelease. This is the // pattern used for Pulse RCs (`-rc.N`), betas (`-beta.N`), alphas, nightlies, diff --git a/scripts/pulse-auto-update.sh b/scripts/pulse-auto-update.sh index 51fcb0634c..a166d4e6d4 100755 --- a/scripts/pulse-auto-update.sh +++ b/scripts/pulse-auto-update.sh @@ -316,12 +316,47 @@ version_greater_than() { detect_service_name() { if [[ -n "${PULSE_SERVICE_NAME:-}" ]]; then echo "$SERVICE_NAME" - elif systemctl list-unit-files --no-legend | grep -q "^pulse-backend.service"; then - echo "pulse-backend" - elif systemctl list-unit-files --no-legend | grep -q "^pulse.service"; then + return + fi + if ! command -v systemctl >/dev/null 2>&1; then echo "pulse" + return + fi + + local unit_files="" unit="" state="" preset="" extra="" + local seen_backend=false seen_pulse=false + # Keep the standalone helper aligned with the server installer's bounded + # discovery. Its explicit installer handoff must not carry a short-read + # default past the installer's own discovery gate. + if ! unit_files=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- pulse-backend.service pulse.service 2>/dev/null); then + log error "Cannot identify the Pulse systemd service: unit inventory failed. Reconcile service state before retrying." >&2 + return 1 + fi + while read -r unit state preset extra; do + [[ -n "$unit" ]] || continue + if [[ "$unit" == pulse-backend.service && "$seen_backend" == false ]]; then + seen_backend=true + elif [[ "$unit" == pulse.service && "$seen_pulse" == false ]]; then + seen_pulse=true + else + log error "Cannot identify the Pulse systemd service: unexpected unit inventory. No default service was selected." >&2 + return 1 + fi + case "$state" in + enabled|enabled-runtime|disabled|static|indirect|linked|linked-runtime|alias|generated|transient|masked|masked-runtime) ;; + *) + log error "Cannot identify the Pulse systemd service: unknown unit state. No default service was selected." >&2 + return 1 ;; + esac + if [[ -n "$extra" ]]; then + log error "Cannot identify the Pulse systemd service: malformed unit inventory. No default service was selected." >&2 + return 1 + fi + done <<< "$unit_files" + if [[ "$seen_backend" == true ]]; then + echo "pulse-backend" else - echo "pulse" # Default + echo "pulse" # Only a successful absent inventory permits the default. fi } @@ -379,7 +414,10 @@ ensure_service_restarted() { perform_update() { local new_version=$1 - local service_name=$(detect_service_name) + local service_name="" + # A local declaration hides command-substitution failure. Refuse before + # prior-active observation, backup, installer download or restart traps. + service_name=$(detect_service_name) || return 1 local installer_tmp="" local signature_tmp="" local backup_dir="" diff --git a/scripts/tests/test_server_service_discovery.py b/scripts/tests/test_server_service_discovery.py index 080bad7247..e5e0d4bc5b 100644 --- a/scripts/tests/test_server_service_discovery.py +++ b/scripts/tests/test_server_service_discovery.py @@ -20,6 +20,7 @@ import unittest ROOT = Path(__file__).resolve().parents[2] INSTALLER = Path(os.environ.get("PULSE_INSTALLER_UNDER_TEST", ROOT / "install.sh")) +UPDATER = Path(os.environ.get("PULSE_AUTO_UPDATER_UNDER_TEST", ROOT / "scripts/pulse-auto-update.sh")) REAL_SYSTEMCTL = shutil.which("systemctl") QUERY = ["list-unit-files", "--no-legend", "--no-pager", "--full", "--", "pulse-backend.service", "pulse.service"] @@ -68,11 +69,13 @@ sys.exit(97) HARNESS = r''' source "$INSTALLER" +if [[ "$PRODUCER" == updater ]]; then source "$UPDATER"; fi print_header() { :; } print_info() { echo "$*" >&2; } print_warn() { echo "$*" >&2; } print_error() { echo "$*" >&2; } print_success() { echo "$*" >&2; } +log() { echo "$*" >&2; } check_root() { :; } check_proxmox_host() { return 1; } detect_os() { :; } @@ -108,13 +111,24 @@ case "$FLOW" in check_existing_installation() { CURRENT_VERSION=v6.5.0; return 0; } fi ;; - archive|build) + archive|build|automatic) verify_release_signature() { :; } validate_pulse_binary_architecture() { :; } detect_pulse_architecture() { echo amd64; } chown() { :; } install_additional_agent_binaries() { :; } deploy_agent_scripts() { :; } + if [[ "$FLOW" == automatic ]]; then + curl() { + # Fixed fixture response, never a provider request. + local target="${@: -1}" + if [[ "$*" == *.sshsig* ]]; then + printf 'fixture signature\n' > "$target" + else + cp "$FIXTURE/served-installer.sh" "$target" + fi + } + fi if [[ "$FLOW" == build ]]; then apt-get() { :; } go() { echo 'go version go1.26.9 linux/amd64'; } @@ -152,6 +166,7 @@ case "$FLOW" in main|late-main) main || status=$? ;; archive) install_pulse_archive "$FIXTURE/pulse-v6.6.0-linux-amd64.tar.gz" v6.6.0 || status=$? ;; build) build_from_source main || status=$? ;; + automatic) perform_update v6.6.0 || status=$? ;; uninstall) uninstall_pulse || status=$? ;; reset) reset_pulse || status=$? ;; unit) install_systemd_service || status=$? ;; @@ -163,19 +178,24 @@ exit "$status" class ServiceDiscovery(unittest.TestCase): - def run_fixture(self, *, rows="", mode="rows", query_exit=0, flow="query", explicit=None): + def run_fixture(self, *, rows="", mode="rows", query_exit=0, flow="query", explicit=None, + producer="installer"): with tempfile.TemporaryDirectory(prefix="pulse-service-discovery-") as directory: f = Path(directory) for name in ["bin", "install/bin", "config", "units", "root/etc/systemd/system"]: (f / name).mkdir(parents=True) (f / "bin/systemctl").write_text(SYSTEMCTL) (f / "bin/systemctl").chmod(0o755) - old = b"#!/bin/sh\necho 'Pulse version v6.5.0'\n" - new = b"#!/bin/sh\necho 'Pulse version v6.6.0'\n" + old = b"#!/bin/sh\necho 'Pulse v6.5.0'\n" + new = b"#!/bin/sh\necho 'Pulse v6.6.0'\n" (f / "install/bin/pulse").write_bytes(old) (f / "install/bin/pulse").chmod(0o755) (f / "new-pulse").write_bytes(new) (f / "new-pulse").chmod(0o755) + (f / "served-installer.sh").write_text( + '#!/bin/bash\nset -eu\n' + 'printf "%s\\n" "$PULSE_SERVICE_NAME" > "$FIXTURE/installer-service"\n' + 'cp "$FIXTURE/new-pulse" "$PULSE_INSTALL_DIR/bin/pulse"\n') archive = f / "pulse-v6.6.0-linux-amd64.tar.gz" with tarfile.open(archive, "w:gz") as stream: entry = tarfile.TarInfo("pulse") @@ -191,7 +211,8 @@ class ServiceDiscovery(unittest.TestCase): (f / "root/etc/systemd/system/pulse-backend.service").write_text(unit) (f / "root/etc/systemd/system/pulse.service").write_text(unit) env = {k: v for k, v in os.environ.items() if not k.startswith("PULSE_")} - env.update(FIXTURE=str(f), INSTALLER=str(INSTALLER), FLOW=flow, MODE=mode, + env.update(FIXTURE=str(f), INSTALLER=str(INSTALLER), UPDATER=str(UPDATER), + PRODUCER=producer, FLOW=flow, MODE=mode, ROWS=rows, QUERY_EXIT=str(query_exit), REAL_SYSTEMCTL=REAL_SYSTEMCTL or "", PATH=str(f / "bin") + os.pathsep + os.environ["PATH"], PULSE_INSTALL_DIR=str(f / "install"), PULSE_CONFIG_DIR=str(f / "config"), @@ -207,6 +228,8 @@ class ServiceDiscovery(unittest.TestCase): mutations=(f / "mutations").read_text() if (f / "mutations").exists() else "", binary=(f / "install/bin/pulse").read_bytes(), old=old, new=new, units=[p.name for p in (f / "units").iterdir()], + installer_service=(f / "installer-service").read_text() + if (f / "installer-service").exists() else None, broken_pipe=(f / "broken-pipe").exists()) def assert_refused(self, e): @@ -303,6 +326,42 @@ class ServiceDiscovery(unittest.TestCase): self.assertEqual(e["result"].stdout, "SERVICE=pulse-backend\n") self.assertEqual(e["calls"], [QUERY]) + def test_automatic_helper_preserves_exact_names_and_explicit_instances(self): + for rows, explicit, expected in [("", None, "pulse"), + ("pulse.service disabled enabled\n", None, "pulse"), + ("pulse-backend.service masked-runtime enabled\n", None, "pulse-backend"), + ("", "pulse.department" + "x" * 140, "pulse.department" + "x" * 140)]: + with self.subTest(rows=rows, explicit=explicit): + e = self.run_fixture(producer="updater", rows=rows, explicit=explicit) + self.assertEqual(e["result"].returncode, 0, e["result"].stderr) + self.assertEqual(e["result"].stdout, "SERVICE=" + expected + "\n") + self.assertEqual(e["calls"], [] if explicit else [QUERY]) + + def test_automatic_helper_cannot_short_read_the_installer_handoff(self): + e = self.run_fixture(producer="updater", mode="large", flow="automatic") + self.assertEqual(e["result"].returncode, 0, e["result"].stderr) + self.assertEqual(e["installer_service"], "pulse-backend\n") + self.assertEqual(e["binary"], e["new"]) + self.assertEqual(e["calls"][0], QUERY) + self.assertFalse(e["broken_pipe"]) + + def test_automatic_helper_refuses_before_backup_or_installer_even_in_or_list(self): + for rows, status in [("", 1), ("pulse-backend.service enabled enabled\n", 1), + ("pulse-backend.service future-state\n", 0), + ("pulse-backend.service enabled\npulse-backend.service enabled\n", 0), + ("pulse-backendXservice enabled\n", 0), + ("pulse.service enabled enabled extra\n", 0)]: + with self.subTest(rows=rows, status=status): + e = self.run_fixture(producer="updater", rows=rows, query_exit=status, flow="automatic") + self.assert_refused(e) + self.assertIsNone(e["installer_service"]) + + def test_automatic_helper_timeout_stops_before_service_observation(self): + e = self.run_fixture(producer="updater", mode="hang", flow="automatic") + self.assert_refused(e) + self.assertLess(e["seconds"], 7) + self.assertIsNone(e["installer_service"]) + if __name__ == "__main__": unittest.main()