From acce9cf13d0606827373ba9bbe3e65c8cabe18db Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:30:49 +0100 Subject: [PATCH 1/2] Discover existing update timers before consent-preserving refresh Match complete systemctl inventory rows by exact unit field with bounded reads, so updates and reinstalls reach the existing helper refresh. Preserve deliberate timer and updater masks instead of replacing their symlinks. Exercise all five installer consumers, unchanged settings and enablement, failure/timeout controls and actual fixture-root systemctl inventory. Change-source: pulse-maintainer --- .../subsystems/deployment-installability.md | 23 ++ install.sh | 19 +- scripts/installtests/install_sh_test.go | 13 + scripts/tests/test_server_update_timer.py | 240 ++++++++++++++++++ 4 files changed, 294 insertions(+), 1 deletion(-) create mode 100644 scripts/tests/test_server_update_timer.py diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 5d96e129fe..d0d2c34bc3 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -15,6 +15,29 @@ ## Purpose +### Existing auto-update timers are discovered without changing consent + +Server installer timer discovery is a bounded, exact-unit `systemctl` inventory. +It compares the first field of complete rows, including their state/preset +columns, without regex matching or truncating custom instance names. A failed +inventory's output is not proof of an installed timer. + +All five existing/new-install refresh consumers can therefore replace stale +auto-update assets when a timer exists, without enabling/starting it or changing +`system.json`. A configured timer or updater masked by a `/dev/null` symlink is +an explicit operator stop: refresh retains the mask and all existing assets. +Discovery is separate from enablement; version changes still require affirmative +consent to set up automatic updates. + +`TestRootInstallTimerDiscoveryReachesConsentPreservingRefresh` runs actual +discovery, installer `main` and asset refresh together for version pins, source +builds, menu updates/reinstalls and leftover fresh-install timers. Its controls +cover enabled/disabled rows, exact-name impostors, long instance names, missing/ +failed/hung inventory, byte-preserved preferences and both unit masks. Where +available, real `systemctl --root` reads only a temporary fixture filesystem. +Manager operations and server installation remain doubles: this is source/flow +proof, not native systemd activation, published installation or customer recovery. + ### Private, authenticated server-installer inputs Release archive downloads and their signature sidecars stay inside the same diff --git a/install.sh b/install.sh index 93eb9c6fb9..06db4797ef 100755 --- a/install.sh +++ b/install.sh @@ -772,7 +772,15 @@ detect_service_name() { } update_timer_exists() { - command -v systemctl >/dev/null 2>&1 && systemctl list-unit-files --no-legend 2>/dev/null | grep -q "^${UPDATE_TIMER_UNIT}$" + command -v systemctl >/dev/null 2>&1 || return 1 + local unit_files="" + # Rows also contain enablement/preset columns. Match the exact first field, + # not the whole row or a regex, and keep long custom unit names untruncated. + # A failed inventory must not admit even plausible partial stdout. + if ! unit_files=$(LC_ALL=C timeout -k 1 5 systemctl list-unit-files --no-legend --no-pager --full -- "$UPDATE_TIMER_UNIT" 2>/dev/null); then + return 1 + fi + awk -v unit="$UPDATE_TIMER_UNIT" '$1 == unit { found = 1 } END { exit !found }' <<< "$unit_files" } update_timer_enabled() { @@ -4731,6 +4739,15 @@ setup_auto_updates() { # Refresh the installed assets unconditionally instead, without changing the # user's choices: system.json and the timer's enabled state stay untouched. refresh_auto_updates() { + # Discovering an existing masked unit is not consent to unmask it. The + # staged unit rename would otherwise replace its /dev/null symlink. + local asset + for asset in "${UPDATE_TIMER_PATH:-${PULSE_UPDATE_TIMER_PATH:-}}" "${UPDATE_SERVICE_PATH:-${PULSE_UPDATE_SERVICE_PATH:-}}"; do + if [[ -L "$asset" && "$asset" -ef /dev/null ]]; then + print_info "Auto-update unit is deliberately masked ($asset); preserving the existing helper and units." + return 0 + fi + done print_info "Refreshing the installed auto-update helper..." if ! install_auto_update_assets; then print_warn "Could not refresh the auto-update helper; the previously installed one may be stale." diff --git a/scripts/installtests/install_sh_test.go b/scripts/installtests/install_sh_test.go index 4480416539..d3b05e5bab 100644 --- a/scripts/installtests/install_sh_test.go +++ b/scripts/installtests/install_sh_test.go @@ -4540,6 +4540,19 @@ func TestRootInstallTemporaryInputsRemainPrivateAndAuthenticated(t *testing.T) { } } +// Execute discovery and every installer refresh consumer together. systemd +// manager operations are confined doubles; real inventory uses a fixture root. +func TestRootInstallTimerDiscoveryReachesConsentPreservingRefresh(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, "python3", repoFile("scripts", "tests", "test_server_update_timer.py"), "-v") + cmd.Env = append(os.Environ(), "PULSE_INSTALLER_UNDER_TEST="+repoFile("install.sh")) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("installer timer discovery/refresh controls: %v\n%s", err, out) + } +} + func TestRootInstallStableReleaseTagRejectsPrereleaseShapes(t *testing.T) { script := ` ` + extractRootInstallShellFunction(t, "is_stable_release_tag") + ` diff --git a/scripts/tests/test_server_update_timer.py b/scripts/tests/test_server_update_timer.py new file mode 100644 index 0000000000..2629e9093b --- /dev/null +++ b/scripts/tests/test_server_update_timer.py @@ -0,0 +1,240 @@ +#!/usr/bin/env python3 +"""Actual installer timer discovery and refresh, confined to temporary files. + +The server/install producers and systemd manager operations are doubles. The +optional real systemctl inventory uses --root, never the host's unit manager. +""" + +import json +import os +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + + +ROOT = Path(__file__).resolve().parents[2] +INSTALLER = Path(os.environ.get("PULSE_INSTALLER_UNDER_TEST", ROOT / "install.sh")) +REAL_SYSTEMCTL = shutil.which("systemctl") + +SYSTEMCTL = r'''#!/usr/bin/env bash +set -eu +printf '%s\n' "$*" >> "$FIXTURE/calls" +case "$1" in + list-unit-files) + if [[ "$INVENTORY" == real ]]; then + exec "$REAL_SYSTEMCTL" --root="$FIXTURE/root" "$@" + fi + if [[ "$INVENTORY" == hang ]]; then exec sleep 30; fi + printf '%s' "$ROWS" + exit "$QUERY_EXIT" + ;; + daemon-reload) exit 0 ;; + *) echo "Unexpected manager operation: $*" >&2; exit 97 ;; +esac +''' + +HARNESS = r''' +source "$INSTALLER" +IN_CONTAINER=true +IN_DOCKER=false +CURRENT_VERSION=v6.5.0 +print_header() { :; } +print_info() { echo "$*"; } +print_warn() { echo "$*"; } +print_success() { echo "$*"; } +print_error() { echo "$*" >&2; } +check_root() { :; } +detect_os() { :; } +check_docker_environment() { :; } +check_existing_installation() { [[ "$FLOW" != fresh ]]; } +detect_service_name() { echo "$SERVICE_NAME"; } +run_upgrade_readiness_preflight() { :; } +backup_existing() { :; } +create_user() { :; } +install_dependencies() { :; } +setup_directories() { :; } +download_pulse() { :; } +build_from_source() { :; } +setup_update_command() { :; } +ensure_systemd_service_installed() { :; } +install_systemd_service() { :; } +start_pulse() { :; } +create_marker_file() { :; } +print_completion() { :; } +resolve_latest_release_tag_for_channel() { + if [[ "$1" == stable ]]; then echo v6.6.0; else echo v6.7.0-rc.1; fi +} +safe_read() { + printf '%s\n' "$1" >> "$FIXTURE/prompts" + if [[ "$2" == enable_updates ]]; then + printf -v "$2" '%s' n + elif [[ "$FLOW" == reinstall ]]; then + printf -v "$2" '%s' 3 + else + printf -v "$2" '%s' 1 + fi +} +case "$FLOW" in + query) + if update_timer_exists; then echo PRESENT; else echo ABSENT; fi + exit 0 ;; + unavailable) + command() { + [[ "$*" != '-v systemctl' ]] || return 1 + builtin command "$@" + } + if update_timer_exists; then echo PRESENT; else echo ABSENT; fi + exit 0 ;; + version) FORCE_VERSION=v6.6.0 ;; + source) BUILD_FROM_SOURCE=true ;; + update|reinstall|fresh) : ;; + *) exit 98 ;; +esac +main +''' + + +class ServerUpdateTimerTest(unittest.TestCase): + def exercise(self, *, flow="query", state="enabled", service="pulse", + rows=None, query_exit=0, inventory="double", mask="", + config_enabled=True): + with tempfile.TemporaryDirectory() as temporary: + fixture = Path(temporary) + config = fixture / "config" + install = fixture / "install" + units = fixture / "root/etc/systemd/system" + tools = fixture / "tools" + for directory in (config, install / "scripts", units, tools): + directory.mkdir(parents=True) + helper = tools / "auto-update" + helper.write_text("#!/bin/bash\necho stale-helper\n") + (install / "scripts/pulse-auto-update.sh").write_text( + "#!/bin/bash\necho current-helper\n") + settings = json.dumps({"autoUpdateEnabled": config_enabled, "updateChannel": "stable"}) + (config / "system.json").write_text(settings) + timer_name = f"{service}-update.timer" + timer = units / timer_name + updater = units / f"{service}-update.service" + timer.write_text("[Timer]\nOnCalendar=daily\n[Install]\nWantedBy=timers.target\n") + updater.write_text("[Service]\nExecStart=/bin/true\n") + if state == "enabled": + wants = units / "timers.target.wants" + wants.mkdir() + (wants / timer_name).symlink_to(f"../{timer_name}") + masked = timer if mask == "timer" else updater if mask == "service" else None + if masked: + masked.unlink() + masked.symlink_to("/dev/null") + systemctl = tools / "systemctl" + systemctl.write_text(SYSTEMCTL) + systemctl.chmod(0o755) + if rows is None: + rows = f"{timer_name} {state} enabled\n" + env = {**os.environ, "INSTALLER": str(INSTALLER), "FIXTURE": str(fixture), + "FLOW": flow, "ROWS": rows, "QUERY_EXIT": str(query_exit), + "INVENTORY": inventory, "REAL_SYSTEMCTL": REAL_SYSTEMCTL or "", + "PATH": f"{tools}:{os.environ['PATH']}", "PULSE_SERVICE_NAME": service, + "PULSE_INSTALL_DIR": str(install), "PULSE_CONFIG_DIR": str(config), + "PULSE_UPDATE_SERVICE_PATH": str(updater), "PULSE_UPDATE_TIMER_PATH": str(timer), + "PULSE_AUTO_UPDATE_DEST": str(helper)} + result = subprocess.run(["bash", "-c", HARNESS], env=env, + capture_output=True, text=True, timeout=12) + def lines(name): + path = fixture / name + return path.read_text().splitlines() if path.exists() else [] + return {"result": result, "helper": helper.read_text(), + "settings": (config / "system.json").read_text(), "original_settings": settings, + "calls": lines("calls"), "prompts": lines("prompts"), + "timer_masked": timer.is_symlink() and os.readlink(timer) == "/dev/null", + "service_masked": updater.is_symlink() and os.readlink(updater) == "/dev/null", + "timer_enabled": (units / "timers.target.wants" / timer_name).is_symlink()} + + def assert_discovery(self, evidence, present): + r = evidence["result"] + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + self.assertEqual(r.stdout.strip(), "PRESENT" if present else "ABSENT") + + def assert_refreshed(self, evidence, enabled): + r = evidence["result"] + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + self.assertIn("current-helper", evidence["helper"]) + self.assertNotIn("stale-helper", evidence["helper"]) + self.assertEqual(evidence["settings"], evidence["original_settings"]) + self.assertEqual(evidence["timer_enabled"], enabled) + mutations = [call for call in evidence["calls"] if not call.startswith("list-unit-files")] + self.assertEqual(mutations, ["daemon-reload"]) + + def test_discovers_full_systemctl_rows_regardless_of_enablement(self): + for state in ("enabled", "enabled-runtime", "disabled", "static", "indirect", + "linked", "linked-runtime", "masked", "masked-runtime"): + with self.subTest(state=state): + self.assert_discovery(self.exercise(state=state), True) + + def test_exact_unit_field_rejects_prefixes_and_regex_impostors(self): + for rows in ("", "pulse-update.timer-other enabled enabled\n", + "pulse-updateXtimer enabled enabled\n", "other-update.timer enabled enabled\n"): + with self.subTest(rows=rows): + self.assert_discovery(self.exercise(rows=rows), False) + self.assert_discovery(self.exercise(rows="other.timer enabled enabled\n" + "\tpulse-update.timer\tdisabled\tenabled\n"), True) + + def test_query_is_bounded_and_scoped_to_full_exact_instance_name(self): + service = "pulse-" + "department-" * 12 + e = self.exercise(service=service) + self.assert_discovery(e, True) + self.assertEqual(e["calls"], [f"list-unit-files --no-legend --no-pager --full -- {service}-update.timer"]) + + def test_failed_query_never_accepts_its_stdout(self): + self.assert_discovery(self.exercise(query_exit=1), False) + + def test_missing_systemctl_never_runs_a_query(self): + e = self.exercise(flow="unavailable") + self.assert_discovery(e, False) + self.assertEqual(e["calls"], []) + + def test_hung_inventory_times_out_without_manager_mutation(self): + e = self.exercise(inventory="hang") + self.assert_discovery(e, False) + self.assertEqual(len(e["calls"]), 1) + + def test_all_install_flows_reach_real_refresh_without_changing_consent(self): + for flow in ("version", "source", "update", "reinstall", "fresh"): + for state, enabled in (("enabled", True), ("disabled", False)): + with self.subTest(flow=flow, state=state): + e = self.exercise(flow=flow, state=state, config_enabled=enabled) + self.assert_refreshed(e, enabled) + self.assertNotIn("Automatic updates are not configured.", e["result"].stdout) + if enabled: + self.assertFalse(any("Enable auto-updates?" in p for p in e["prompts"])) + + def test_missing_or_failed_inventory_does_not_refresh_or_enable(self): + for rows, code in (("", 0), ("pulse-update.timer enabled enabled\n", 1)): + e = self.exercise(flow="version", rows=rows, query_exit=code) + self.assertEqual(e["result"].returncode, 0) + self.assertIn("stale-helper", e["helper"]) + self.assertEqual(e["settings"], e["original_settings"]) + self.assertFalse(any(call == "daemon-reload" for call in e["calls"])) + + def test_timer_and_updater_masks_are_not_replaced_during_refresh(self): + for mask in ("timer", "service"): + with self.subTest(mask=mask): + e = self.exercise(flow="version", mask=mask) + self.assertEqual(e["result"].returncode, 0) + self.assertTrue(e[f"{mask}_masked"]) + self.assertIn("stale-helper", e["helper"]) + self.assertIn("masked", e["result"].stdout) + self.assertEqual(e["settings"], e["original_settings"]) + self.assertFalse(any(call == "daemon-reload" for call in e["calls"])) + + @unittest.skipUnless(REAL_SYSTEMCTL, "systemctl filesystem inventory is unavailable") + def test_real_systemctl_inventory_and_complete_flow_use_only_fixture_root(self): + for state, enabled in (("enabled", True), ("disabled", False)): + with self.subTest(state=state): + self.assert_discovery(self.exercise(inventory="real", state=state), True) + self.assert_refreshed(self.exercise(flow="version", inventory="real", state=state), enabled) + + +if __name__ == "__main__": + unittest.main() From 595ee454a89bfd5a36651492009932548db314d4 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 9 Oct 2026 11:42:50 +0100 Subject: [PATCH 2/2] Check local-only temperature cleanup instead of retired SSH guidance The complete installer suite failed because its older guide check still required --ssh-known-hosts after reviewed 953e0c6dec deliberately made cleanup local-only. Check the actual copied command and reject remote scope, purge, account deletion, extra actions and prose-only impostors; preserve all existing bounded/private diagnostic checks. Contract-Neutral: Test-only reconciliation with the existing local-only guide; no runtime, shipped document, access or cleanup contract change. Change-source: pulse-maintainer --- .../installtests/temperature_guidance_test.go | 64 ++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/scripts/installtests/temperature_guidance_test.go b/scripts/installtests/temperature_guidance_test.go index 231014120d..2218e604c3 100644 --- a/scripts/installtests/temperature_guidance_test.go +++ b/scripts/installtests/temperature_guidance_test.go @@ -99,11 +99,73 @@ func TestTemperatureGuidanceKeepsExistingSafetyAndSourceMeaning(t *testing.T) { if err != nil { t.Fatal(err) } - for _, text := range []string{"thermal_zone0` is not necessarily the CPU", "millidegrees", "not physical-disk SMART", "--token-file", "--local-only", "--ssh-known-hosts", "--remove-proxmox-access", "not a signed release asset", "not an empty reading"} { + for _, text := range []string{"thermal_zone0` is not necessarily the CPU", "millidegrees", "not physical-disk SMART", "--token-file", "--local-only", "--remove-proxmox-access", "not a signed release asset", "not an empty reading"} { if !strings.Contains(string(doc), text) { t.Fatalf("temperature guide loses existing scope or source distinction %q", text) } } + if err := temperatureCleanupRecipeScope(string(doc)); err != nil { + t.Fatal(err) + } +} + +// The current guide deliberately uses local-only cleanup, not remote key +// cleanup or trust enrolment. Check the executable recipe, not whether prose +// happens to mention an optional cluster-SSH flag from older instructions. +func temperatureCleanupRecipeScope(doc string) error { + const helper = `"$HOME/.config/pulse/sensor-proxy-uninstall.sh"` + count := 0 + for _, match := range regexp.MustCompile("(?s)```bash\n(.*?)```").FindAllStringSubmatch(doc, -1) { + block := strings.ReplaceAll(match[1], "\\\n", " ") + if !strings.Contains(block, "bash "+helper) { + continue + } + count++ + fields := strings.Fields(block) + if len(fields) != 4 || fields[0] != "bash" || fields[1] != helper || + !((fields[2] == "--uninstall" && fields[3] == "--local-only") || + (fields[2] == "--local-only" && fields[3] == "--uninstall")) { + return fmt.Errorf("temperature cleanup recipe must preserve local-only scope without extra operations") + } + } + if count != 1 { + return fmt.Errorf("temperature guide needs one explicit local-only cleanup recipe, got %d", count) + } + return nil +} + +func TestTemperatureCleanupRecipeScopeRejectsBroaderOrMissingActions(t *testing.T) { + const command = `bash "$HOME/.config/pulse/sensor-proxy-uninstall.sh" --uninstall --local-only` + wrap := func(command string) string { + return "```bash\n" + command + "\n```\n" + } + for _, recipe := range []string{ + command, + `bash "$HOME/.config/pulse/sensor-proxy-uninstall.sh" --local-only --uninstall`, + "bash \"$HOME/.config/pulse/sensor-proxy-uninstall.sh\" \\\n --uninstall --local-only", + } { + // Warning prose can name dangerous flags without proposing their use. + doc := wrap(recipe) + "Do not add --purge or --remove-proxmox-access.\n" + if err := temperatureCleanupRecipeScope(doc); err != nil { + t.Fatalf("safe local cleanup rejected: %v", err) + } + } + for name, doc := range map[string]string{ + "no action": "Use local-only cleanup after planning maintenance.\n", + "remote cleanup": wrap(strings.Replace(command, " --local-only", "", 1)), + "purge": wrap(command + " --purge"), + "remove access": wrap(command + " --remove-proxmox-access"), + "cluster trust": wrap(command + " --ssh-known-hosts /path/to/known_hosts"), + "replay": wrap(command) + wrap(command), + "chained action": wrap(command + " && echo done"), + "prose impostor": wrap(strings.Replace(command, " --local-only", "", 1)) + "Keep --local-only.\n", + } { + t.Run(name, func(t *testing.T) { + if temperatureCleanupRecipeScope(doc) == nil { + t.Fatal("unsafe or absent cleanup recipe passed") + } + }) + } } func TestTemperatureGuidanceReadingCommandsArePrivateAndBounded(t *testing.T) {