Preserve updater assets when repository configuration fails

Return actual allocation, syntax, mode and rename failures before publishing a configured updater. Prepare the rewrite beside its input and leave the installed helper and units unchanged when configuration cannot complete. Exercise the real rewriter and its containing asset installer under the conditional callers that suppress errexit. Contract-Neutral: Repairs the existing configured-helper and failure-preservation guarantees without changing update consent, source selection, service policy or deployment contracts. Change-source: pulse-maintainer
2026-10-26 05:29:45 +00:00 · 2026-10-25 21:47:30 +00:00
parent d57c20b421
commit aa4f3bbf8d
2 changed files with 172 additions and 3 deletions
@@ -4495,8 +4495,14 @@ configure_auto_update_script_repo() {
        return 1
    fi

+    # Both callers suppress errexit with `if !`: every preparation and
+    # publication failure must be returned explicitly. Keep the rewrite beside
+    # the supplied staging file so its final rename cannot become a cross-
+    # filesystem copy, and do not chmod or replace the input before it is ready.
    local tmp
-    tmp=$(mktemp /tmp/pulse-auto-update-script.XXXXXX)
+    if ! tmp=$(mktemp "${dest}.repo.XXXXXX"); then
+        return 1
+    fi
    if ! awk -v repo="$GITHUB_REPO" '
        BEGIN { inserted = 0 }
        /^#!/ {
@@ -4525,8 +4531,11 @@ configure_auto_update_script_repo() {
        return 1
    fi

-    mv "$tmp" "$dest"
-    chmod +x "$dest"
+    if ! bash -n "$tmp" || ! chmod 0755 "$tmp" || ! mv -Tf -- "$tmp" "$dest"; then
+        rm -f -- "$tmp"
+        return 1
+    fi
+    return 0
 }

 # Probes whether new entries can be created in a directory. This is exactly
@@ -0,0 +1,160 @@
+package installtests
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// Run the real rewriter in the conditional context used by its installer
+// caller. A trailing successful chmod must not hide a failed allocation/mv.
+// All data and failure doubles are ordinary-user temporary fixtures.
+func TestConfigureAutoUpdateRepositoryPublication(t *testing.T) {
+	for _, fault := range []string{"allocation", "write", "mode", "rename", "syntax", "none"} {
+		t.Run(fault, func(t *testing.T) {
+			dir := t.TempDir()
+			dest := filepath.Join(dir, "helper")
+			original := "#!/usr/bin/env bash\nGITHUB_REPO=old/repository\nprintf '%s\\n' \"$GITHUB_REPO\"\n"
+			if fault == "syntax" {
+				original += "if\n"
+			}
+			if err := os.WriteFile(dest, []byte(original), 0751); err != nil {
+				t.Fatal(err)
+			}
+			script := `set -euo pipefail
+GITHUB_REPO=fixture/selected
+` + extractRootInstallShellFunction(t, "configure_auto_update_script_repo") + `
+mktemp() {
+    [[ "$FAULT" != allocation ]] || { echo FAULT >&2; return 71; }
+    # Confine the pre-repair /tmp allocation too, for the parent control.
+    if [[ "$1" == /tmp/pulse-auto-update-script.XXXXXX ]]; then
+        command mktemp "$FIXTURE/legacy.XXXXXX"
+    else
+        command mktemp "$@"
+    fi
+}
+awk() {
+    if [[ "$FAULT" == write ]]; then echo partial; echo FAULT >&2; return 72; fi
+    command awk "$@"
+}
+chmod() {
+    [[ "$FAULT" != mode ]] || { echo FAULT >&2; return 73; }
+    command chmod "$@"
+}
+mv() {
+    [[ "$FAULT" != rename ]] || { echo FAULT >&2; return 74; }
+    command mv "$@"
+}
+if configure_auto_update_script_repo "$FIXTURE/helper"; then
+    echo CONFIGURATION_SUCCESS
+else
+    echo CONFIGURATION_FAILURE
+fi
+`
+			cmd := exec.Command("bash", "-c", script)
+			cmd.Env = append(os.Environ(), "FAULT="+fault, "FIXTURE="+dir)
+			out, err := cmd.CombinedOutput()
+			if err != nil {
+				t.Fatalf("conditional caller aborted: %v\n%s", err, out)
+			}
+			wantSuccess := fault == "none"
+			if strings.Contains(string(out), "CONFIGURATION_SUCCESS") != wantSuccess {
+				t.Errorf("incorrect configuration result:\n%s", out)
+			}
+			if fault != "none" && fault != "syntax" && !strings.Contains(string(out), "FAULT") {
+				t.Errorf("injected failure was not exercised:\n%s", out)
+			}
+			after, err := os.ReadFile(dest)
+			if err != nil {
+				t.Fatal(err)
+			}
+			info, err := os.Stat(dest)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !wantSuccess {
+				if string(after) != original || info.Mode().Perm() != 0751 {
+					t.Errorf("failed rewrite changed input bytes or mode: mode=%o\n%s", info.Mode().Perm(), after)
+				}
+			} else {
+				want := "#!/usr/bin/env bash\nGITHUB_REPO=\"fixture/selected\"\nprintf '%s\\n' \"$GITHUB_REPO\"\n"
+				if string(after) != want || info.Mode().Perm() != 0755 {
+					t.Errorf("incorrect configured helper: mode=%o\n%s", info.Mode().Perm(), after)
+				}
+				result, err := exec.Command("bash", dest).CombinedOutput()
+				if err != nil || string(result) != "fixture/selected\n" {
+					t.Errorf("configured helper did not consume selected repository: %v\n%s", err, result)
+				}
+			}
+			entries, err := os.ReadDir(dir)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(entries) != 1 || entries[0].Name() != "helper" {
+				t.Errorf("configuration left temporary inputs: %v", entries)
+			}
+		})
+	}
+}
+
+// A configuration mv failure used to fall through to chmod and return zero.
+// The real containing caller then published the wrong-repository helper and
+// rewrote its units. Refuse only that inner mv, not the outer publication.
+func TestInstallAutoUpdateAssetsRejectsRepositoryRewriteFailure(t *testing.T) {
+	dir := t.TempDir()
+	helper, service, timer := prepareAutoUpdatePaths(t, dir)
+	before := map[string]string{
+		helper:  "#!/usr/bin/env bash\necho previously-working\n",
+		service: "previous-service\n",
+		timer:   "previous-timer\n",
+	}
+	for path, data := range before {
+		if err := os.WriteFile(path, []byte(data), 0751); err != nil {
+			t.Fatal(err)
+		}
+	}
+	script := "set -euo pipefail\n" + autoUpdateAssetsHarness(t, dir, helper, service, timer, `
+GITHUB_REPO=fixture/selected
+mktemp() {
+    if [[ "$1" == /tmp/pulse-auto-update-script.XXXXXX ]]; then
+        command mktemp "${PULSE_AUTO_UPDATE_DEST}.legacy.XXXXXX"
+    else
+        command mktemp "$@"
+    fi
+}
+mv() {
+    case "${@: -1}" in *.staged.*) echo CONFIGURATION_RENAME_FAULT; return 74 ;; esac
+    command mv "$@"
+}
+safe_systemctl() { echo UNEXPECTED_SYSTEMCTL; }
+`)
+	out, err := exec.Command("bash", "-c", script).CombinedOutput()
+	if err != nil {
+		t.Fatalf("caller aborted: %v\n%s", err, out)
+	}
+	if !strings.Contains(string(out), "CONFIGURATION_RENAME_FAULT") ||
+		!strings.Contains(string(out), "FAILED_AS_EXPECTED") ||
+		strings.Contains(string(out), "UNEXPECTED_SUCCESS") ||
+		strings.Contains(string(out), "UNEXPECTED_SYSTEMCTL") {
+		t.Errorf("configuration failure was not contained before publication:\n%s", out)
+	}
+	for path, data := range before {
+		after, err := os.ReadFile(path)
+		if err != nil || string(after) != data {
+			t.Errorf("configuration failure changed %s: %v\n%s", path, err, after)
+		}
+		info, err := os.Stat(path)
+		if err != nil || info.Mode().Perm() != 0751 {
+			t.Errorf("configuration failure changed mode of %s: %v", path, err)
+		}
+	}
+	entries, err := os.ReadDir(filepath.Dir(helper))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(entries) != 1 || entries[0].Name() != filepath.Base(helper) {
+		t.Errorf("failed containing caller left staging files: %v", entries)
+	}
+}