From b897b0ea88d5f6c09c0003f141696ae5a6a9795e Mon Sep 17 00:00:00 2001 From: Ochenstarik Date: Tue, 11 Aug 2026 01:05:22 +0700 Subject: [PATCH] fix(bootstrap): compare release versions semantically against the installed one The update guard compared the archive version with PROGRAM_VERSION using shell string ordering. Both halves were wrong. PROGRAM_VERSION is a constant describing the bootstrap source tree ("0.2.0-dev"), never the deployed component, so it could not represent what is installed. The cross-role compatibility check compared it with a manifest field such as "v0.1.0-alpha.9"; those can never be equal, so update-control on a host that also runs the agent always failed. The downgrade guard compared the same mismatched pair and passed only by accident, because "v" sorts above "0" in ASCII. String ordering is also wrong for the version scheme in use: "0.1.0-alpha.10" sorts below "0.1.0-alpha.9", so the next release after the ninth would have been rejected as a downgrade. - record the installed version per role at install and update time, and compare against that instead of PROGRAM_VERSION; - order versions with sort -V after stripping the leading "v", so prerelease numbering and tag prefixes compare correctly; - treat an unknown peer version as a warning rather than a failure, because installations predating version recording have nothing to compare against; - guard all of the above in the bootstrap contract test, including the six ordering cases and a check that the lexicographic comparison is not restored. Verified by deliberately reintroducing each defect: lexicographic comparison, sort without -V, and a missing version record are all caught by the contract test. Co-Authored-By: Claude Opus 5 --- deploy/ochenstarik-server-monitor-manager.sh | 99 ++++++++++++++++---- tests/bootstrap/test-bootstrap-contract.sh | 38 ++++++++ 2 files changed, 121 insertions(+), 16 deletions(-) diff --git a/deploy/ochenstarik-server-monitor-manager.sh b/deploy/ochenstarik-server-monitor-manager.sh index 5acc5be..10f46ca 100755 --- a/deploy/ochenstarik-server-monitor-manager.sh +++ b/deploy/ochenstarik-server-monitor-manager.sh @@ -140,6 +140,54 @@ validate_platform() { [[ "$(ps -p 1 -o comm=)" == "systemd" ]] || fail "systemd must be PID 1." } +# Release tags carry a leading "v" while some manifest fields and recorded values may not. +normalize_version() { + printf '%s' "${1#v}" +} + +# True when the first version orders strictly before the second. Lexicographic comparison is +# wrong here: "0.1.0-alpha.10" sorts before "0.1.0-alpha.9" as a string, which would reject +# every release after the ninth as a downgrade. +version_lt() { + local left right first + left="$(normalize_version "$1")" + right="$(normalize_version "$2")" + [[ "$left" != "$right" ]] || return 1 + first="$(printf '%s\n%s\n' "$left" "$right" | sort -V | head -n1)" + [[ "$first" == "$left" ]] +} + +installed_version_file() { + printf '%s' "$ETC_DIR/installed-version-$1" +} + +read_installed_version() { + local file + file="$(installed_version_file "$1")" + [[ -r "$file" ]] || return 0 + tr -d '\r\n' <"$file" +} + +record_installed_version() { + local role="$1" version="$2" file + [[ -n "$version" ]] || return 0 + file="$(installed_version_file "$role")" + install -d -m 0755 "$ETC_DIR" + printf '%s\n' "$version" >"$file" + chmod 0644 "$file" +} + +manifest_version_field() { + local manifest="$1" field="$2" + [[ -r "$manifest" ]] || return 0 + awk -F'"' -v key="$field" '$2 == key { print $4; exit }' "$manifest" || true +} + +# Version recorded for an archive being installed, empty when the release predates manifests. +archive_version() { + manifest_version_field "$(dirname "$1")/server-monitor-manager-manifest.json" version +} + validate_node_id() { [[ "$1" =~ ^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$ ]] \ || fail "Node id must contain 1-63 lowercase letters, digits, or hyphens." @@ -799,6 +847,7 @@ EOF systemctl status --no-pager "$CONTROL_UNIT" >&2 || true fail "Control service failed; backup is $backup_id" } + record_installed_version control "$(archive_version "$archive")" log "Control installed. Backup: $backup_id" log "CA fingerprint: $(openssl x509 -in "$ETC_DIR/control-ca.crt" -noout -fingerprint -sha256 | cut -d= -f2)" } @@ -895,6 +944,7 @@ EOF systemctl status --no-pager "$AGENT_UNIT" >&2 || true fail "Agent service failed; backup is $backup_id" } + record_installed_version agent "$(archive_version "$archive")" log "Agent $node_id installed and enrolled. Backup: $backup_id" } @@ -1012,25 +1062,41 @@ update_role() { esac [[ -x "$TEMP_DIR/$role/$binary" ]] || fail "$role binary is missing." - local manifest="$(dirname "$archive")/server-monitor-manager-manifest.json" + local manifest new_version m_control m_agent installed peer_role peer_expected + manifest="$(dirname "$archive")/server-monitor-manager-manifest.json" if [[ -f "$manifest" ]]; then - local new_version m_control m_agent m_helper - new_version="$(awk -F'"' '/"version":/ {print $4}' "$manifest" || true)" - m_control="$(awk -F'"' '/"control":/ {print $4}' "$manifest" || true)" - m_agent="$(awk -F'"' '/"agent":/ {print $4}' "$manifest" || true)" - m_helper="$(awk -F'"' '/"helper":/ {print $4}' "$manifest" || true)" + new_version="$(manifest_version_field "$manifest" version)" + m_control="$(manifest_version_field "$manifest" control)" + m_agent="$(manifest_version_field "$manifest" agent)" if [[ -n "$new_version" ]]; then - if [[ "$new_version" < "$PROGRAM_VERSION" && "${SMM_ALLOW_DOWNGRADE:-0}" != "1" ]]; then - fail "Downgrade from $PROGRAM_VERSION to $new_version is not allowed. Set SMM_ALLOW_DOWNGRADE=1 to bypass." + # Compared against the version recorded when this role was installed, not against + # PROGRAM_VERSION: that constant describes the bootstrap source tree and never the + # deployed component, so it can neither detect a downgrade nor match a release tag. + installed="$(read_installed_version "$role")" + if [[ -n "$installed" ]] \ + && version_lt "$new_version" "$installed" \ + && [[ "${SMM_ALLOW_DOWNGRADE:-0}" != "1" ]]; then + fail "Downgrade of $role from $installed to $new_version is not allowed. Set SMM_ALLOW_DOWNGRADE=1 to bypass." fi - - if [[ "$role" == "control" ]] && systemctl list-unit-files | grep -q "^${AGENT_UNIT}"; then - if [[ "$PROGRAM_VERSION" != "$m_agent" ]]; then - fail "Incompatible versions: installed agent is $PROGRAM_VERSION, but archive requires agent $m_agent. Update rejected." - fi - elif [[ "$role" == "agent" ]] && systemctl list-unit-files | grep -q "^${CONTROL_UNIT}"; then - if [[ "$PROGRAM_VERSION" != "$m_control" ]]; then - fail "Incompatible versions: installed control is $PROGRAM_VERSION, but archive requires control $m_control. Update rejected." + + # Cross-role compatibility: the peer component already on this host must run the + # version this archive expects of it. An unknown peer version is reported but not + # treated as failure, because installations predating version recording have + # nothing to compare against. + case "$role" in + control) peer_role="agent"; peer_expected="$m_agent" ;; + agent) peer_role="control"; peer_expected="$m_control" ;; + *) peer_role="" ;; + esac + if [[ -n "$peer_role" ]] \ + && systemctl list-unit-files 2>/dev/null | grep -q "^ochenstarik-smm-${peer_role}.service"; then + local peer_actual + peer_actual="$(read_installed_version "$peer_role")" + if [[ -z "$peer_actual" ]]; then + log "Warning: installed $peer_role version is unknown; compatibility check skipped." + elif [[ -n "$peer_expected" ]] \ + && [[ "$(normalize_version "$peer_actual")" != "$(normalize_version "$peer_expected")" ]]; then + fail "Incompatible versions: installed $peer_role is $peer_actual, but this archive expects $peer_role $peer_expected. Update rejected." fi fi @@ -1083,6 +1149,7 @@ update_role() { CONTROL_UPDATE_BACKUP_ID="" CONTROL_UPDATE_LEGACY_ITEMS=() fi + record_installed_version "$role" "$(archive_version "$archive")" log "$role updated. Backup: $backup_id" } diff --git a/tests/bootstrap/test-bootstrap-contract.sh b/tests/bootstrap/test-bootstrap-contract.sh index 260e3bc..4072be6 100755 --- a/tests/bootstrap/test-bootstrap-contract.sh +++ b/tests/bootstrap/test-bootstrap-contract.sh @@ -830,5 +830,43 @@ else fi fi +# Version ordering. Lexicographic comparison would rank "0.1.0-alpha.10" below +# "0.1.0-alpha.9" and reject every release after the ninth as a downgrade. +version_helpers="$(sed -n '/^normalize_version()/,/^}/p;/^version_lt()/,/^}/p' "$bootstrap")" +version_case() { + local left="$1" right="$2" expected="$3" actual + if bash -c "$version_helpers +version_lt \"\$1\" \"\$2\"" _ "$left" "$right"; then actual="lt"; else actual="ge"; fi + if [[ "$actual" != "$expected" ]]; then + printf 'version_lt %s %s returned %s, expected %s\n' "$left" "$right" "$actual" "$expected" >&2 + exit 1 + fi +} +version_case "v0.1.0-alpha.9" "v0.1.0-alpha.10" lt +version_case "v0.1.0-alpha.10" "v0.1.0-alpha.9" ge +version_case "v0.1.0-alpha.9" "v0.1.0-alpha.9" ge +version_case "0.1.0-alpha.9" "v0.1.0-alpha.9" ge +version_case "v0.1.0-alpha.9" "v0.2.0" lt +version_case "v0.2.0" "v0.1.0-alpha.9" ge + +# The downgrade guard must compare against the recorded installed version, never against +# PROGRAM_VERSION: that constant tracks the source tree, not the deployed component. +grep -Fq 'installed="$(read_installed_version "$role")"' "$bootstrap" || { + printf '%s\n' "update_role must compare against the recorded installed version" >&2 + exit 1 +} +if grep -qE '"\$new_version" *< *"\$PROGRAM_VERSION"' "$bootstrap"; then + printf '%s\n' "lexicographic version comparison against PROGRAM_VERSION reintroduced" >&2 + exit 1 +fi +grep -Fq 'record_installed_version control' "$bootstrap" || { + printf '%s\n' "install-control must record the installed version" >&2 + exit 1 +} +grep -Fq 'record_installed_version agent' "$bootstrap" || { + printf '%s\n' "install-agent must record the installed version" >&2 + exit 1 +} + printf '%s\n' "BOOTSTRAP_CONTRACT=PASS"