Keep NAS agent update copies executable

Avoid rewriting an already-replaced QNAP executable through an aliased persistent path. Stage split QNAP and Unraid copies in owned same-directory temporaries, set execute permissions before atomic replacement, and clean every failed stage without changing saved state or update trust. Add authenticated HTTPS and real child-exec regressions plus persistence failure and recovery controls. Native appliance and old-installed-updater recovery remain separate.

Change-source: pulse-maintainer
This commit is contained in:
pulse-triage[bot]
2026-10-13 13:18:15 +01:00
parent 340a832faa
commit a38ef3734d
4 changed files with 405 additions and 9 deletions
@@ -7647,6 +7647,28 @@ that waits for the persistent data volume before launching the stored wrapper,
and `internal/agentupdate/update.go` keeps the persisted QNAP binary copy in
sync on self-update so reboot does not roll the runtime back to an older
binary.
The persistent replacement must also remain executable. If the verified runtime
and persistent path identify the same file (including directory or executable
symlink aliases), the updater must not copy over its already-installed binary.
Separate QNAP/Unraid copies use an exclusively created temporary in the
destination directory, closed and explicitly made `0755` before atomic rename.
They must not reuse or follow a pre-existing fixed `.tmp` path. Read, creation,
write, close, chmod or rename failure leaves the previous persistent binary
unchanged, closes/removes the owned temporary and logs the persistence failure.
Persistence remains best-effort after live replacement; a runnable new runtime
does not establish that the boot copy was updated. Saved identity, credentials,
transport trust, mandatory checksum/signature, downloaded-binary self-test and
main replacement/backup recovery remain unchanged.
`TestNASAuthenticatedUpdateKeepsExecutableCopies` exercises the real trusted
HTTPS/checksum/test-signature/self-test path with inert changed executables in
unified, aliased and split QNAP layouts, split Unraid and an ordinary Linux
control. Actual child execution checks both runtime and boot copies, preserved
saved state and temporary cleanup. `TestPersistentBinaryCopyFailuresRetainExecutable`
requires every persistence fault phase to retain an executable old copy and
clean its own temporary; unowned legacy temporaries and other hardlinks survive.
These Linux source controls do not establish native appliance, flash mount,
boot/watchdog, old-installed-updater recovery or release availability.
That appliance lifecycle must remain bounded by available storage. Before
downloading or replacing an agent, `scripts/install.sh` checks the effective
temporary and installation filesystems for enough free space, coalesces the
+141 -2
View File
@@ -19,6 +19,7 @@ import (
"time"
"github.com/rcourtman/pulse-go-rewrite/internal/securityutil"
"github.com/rs/zerolog"
)
type roundTripperFunc func(*http.Request) (*http.Response, error)
@@ -1418,6 +1419,7 @@ func TestPerformUpdateUnraidPaths(t *testing.T) {
origWrite := writeFileFn
origRename := renameFn
origRestart := restartProcessFn
origCreateTemp := createTempFn
t.Cleanup(func() {
unraidVersionPath = origUnraid
unraidPersistentPathFn = origPersist
@@ -1425,6 +1427,7 @@ func TestPerformUpdateUnraidPaths(t *testing.T) {
writeFileFn = origWrite
renameFn = origRename
restartProcessFn = origRestart
createTempFn = origCreateTemp
})
tmpDir := t.TempDir()
@@ -1453,10 +1456,17 @@ func TestPerformUpdateUnraidPaths(t *testing.T) {
t.Run("WriteError", func(t *testing.T) {
readFileFn = func(string) ([]byte, error) { return []byte("new"), nil }
writeFileFn = func(string, []byte, os.FileMode) error { return errors.New("write fail") }
createTempFn = func(dir, pattern string) (*os.File, error) {
file, err := os.CreateTemp(dir, pattern)
if err == nil && strings.Contains(pattern, "-persist-") {
err = file.Close() // Make the real persistent write fail.
}
return file, err
}
if err := u.performUpdateWithExecPath(context.Background(), execPath); err != nil {
t.Fatalf("expected update success, got %v", err)
}
createTempFn = origCreateTemp
})
t.Run("RenameError", func(t *testing.T) {
@@ -1504,6 +1514,7 @@ func TestPerformUpdateQNAPPaths(t *testing.T) {
origWrite := writeFileFn
origRename := renameFn
origRestart := restartProcessFn
origCreateTemp := createTempFn
t.Cleanup(func() {
qnapGetcfgPath = origQnapGetcfg
qnapQpkgConfPath = origQnapQpkgConf
@@ -1512,6 +1523,7 @@ func TestPerformUpdateQNAPPaths(t *testing.T) {
writeFileFn = origWrite
renameFn = origRename
restartProcessFn = origRestart
createTempFn = origCreateTemp
})
tmpDir := t.TempDir()
@@ -1532,11 +1544,18 @@ func TestPerformUpdateQNAPPaths(t *testing.T) {
t.Run("WriteError", func(t *testing.T) {
readFileFn = func(string) ([]byte, error) { return []byte("new"), nil }
writeFileFn = func(string, []byte, os.FileMode) error { return errors.New("write fail") }
createTempFn = func(dir, pattern string) (*os.File, error) {
file, err := os.CreateTemp(dir, pattern)
if err == nil && strings.Contains(pattern, "-persist-") {
err = file.Close()
}
return file, err
}
renameFn = os.Rename
if err := u.performUpdateWithExecPath(context.Background(), execPath); err != nil {
t.Fatalf("expected update success, got %v", err)
}
createTempFn = origCreateTemp
})
t.Run("RenameError", func(t *testing.T) {
@@ -1570,6 +1589,126 @@ func TestPerformUpdateQNAPPaths(t *testing.T) {
})
}
func TestPersistentBinaryCopyFailuresRetainExecutable(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("NAS executable persistence controls require Linux")
}
oldBinary := buildNASUpdateExecutable(t, "6.4.5")
newBinary := buildNASUpdateExecutable(t, "6.5.0")
for _, phase := range []string{"read", "create", "write", "close", "chmod", "rename", "success", "legacy-temp-file", "legacy-temp-symlink", "same-file", "hardlink"} {
t.Run(phase, func(t *testing.T) {
dir := t.TempDir()
execPath := filepath.Join(dir, "runtime")
persistPath := filepath.Join(dir, "pulse-agent")
if err := os.WriteFile(execPath, newBinary, 0755); err != nil {
t.Fatal(err)
}
if phase == "same-file" {
persistPath = execPath
} else if err := os.WriteFile(persistPath, oldBinary, 0755); err != nil {
t.Fatal(err)
}
if phase == "hardlink" {
if err := os.Link(persistPath, persistPath+".old-link"); err != nil {
t.Fatal(err)
}
}
const foreign = "unrelated recovery bytes"
if phase == "legacy-temp-file" {
if err := os.WriteFile(persistPath+".tmp", []byte(foreign), 0644); err != nil {
t.Fatal(err)
}
}
if phase == "legacy-temp-symlink" {
if err := os.WriteFile(filepath.Join(dir, "foreign"), []byte(foreign), 0600); err != nil {
t.Fatal(err)
}
if err := os.Symlink(filepath.Join(dir, "foreign"), persistPath+".tmp"); err != nil {
t.Fatal(err)
}
}
origRead, origCreate, origClose, origChmod, origRename := readFileFn, createTempFn, closeFileFn, chmodFn, renameFn
t.Cleanup(func() {
readFileFn, createTempFn, closeFileFn, chmodFn, renameFn = origRead, origCreate, origClose, origChmod, origRename
})
var temp *os.File
createTempFn = func(dir, pattern string) (*os.File, error) {
if phase == "create" {
return nil, errors.New("fixture create refused")
}
file, err := os.CreateTemp(dir, pattern)
temp = file
if err == nil && phase == "write" {
err = file.Close()
}
return file, err
}
if phase == "read" {
readFileFn = func(string) ([]byte, error) { return nil, errors.New("fixture read refused") }
}
if phase == "close" {
closeFileFn = func(file *os.File) error {
if err := file.Close(); err != nil {
t.Fatal(err)
}
return errors.New("fixture close refused")
}
}
if phase == "chmod" {
chmodFn = func(string, os.FileMode) error { return errors.New("fixture chmod refused") }
}
if phase == "rename" {
renameFn = func(string, string) error { return errors.New("fixture rename refused") }
}
var logs bytes.Buffer
logger := zerolog.New(&logs)
u := New(Config{Logger: &logger})
u.syncPersistentBinaryCopy(execPath, persistPath, "qnap")
failure := phase == "read" || phase == "create" || phase == "write" || phase == "close" || phase == "chmod" || phase == "rename"
wantBytes, wantVersion := newBinary, "6.5.0"
if failure {
wantBytes, wantVersion = oldBinary, "6.4.5"
if !strings.Contains(logs.String(), "failed to ") || !strings.Contains(logs.String(), "persistent") {
t.Fatalf("persistence failure was not logged: %s", logs.String())
}
}
got, err := os.ReadFile(persistPath)
if err != nil || checksum(got) != checksum(wantBytes) {
t.Fatalf("persistent recovery bytes changed: %v", err)
}
info, err := os.Stat(persistPath)
if err != nil || info.Mode().Perm() != 0755 {
t.Fatalf("persistent binary lost execute mode: %v", err)
}
runNASUpdateExecutable(t, persistPath, wantVersion)
runNASUpdateExecutable(t, execPath, "6.5.0")
if temp != nil {
if _, err := temp.Stat(); err == nil {
t.Fatal("persistent temporary descriptor was not closed")
}
if _, err := os.Stat(temp.Name()); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("owned persistent temporary was not cleaned: %v", err)
}
} else if phase == "same-file" && strings.Contains(logs.String(), "updated persistent binary") {
t.Fatal("already replaced executable was copied again")
}
if phase == "legacy-temp-file" || phase == "legacy-temp-symlink" {
got, err := os.ReadFile(persistPath + ".tmp")
if err != nil || string(got) != foreign {
t.Fatalf("unowned legacy temporary was changed: %v", err)
}
}
if phase == "hardlink" {
got, err := os.ReadFile(persistPath + ".old-link")
if err != nil || checksum(got) != checksum(oldBinary) {
t.Fatalf("atomic replace mutated another hardlink: %v", err)
}
runNASUpdateExecutable(t, persistPath+".old-link", "6.4.5")
}
})
}
}
type errorReader struct {
sent bool
}
+39 -7
View File
@@ -1058,7 +1058,7 @@ func (u *Updater) performUpdateWithExecPathForVersion(ctx context.Context, execP
if isUnraid() {
persistPath := unraidPersistentPathFn(agentName)
if _, err := os.Stat(persistPath); err == nil {
u.syncPersistentBinaryCopy(execPath, persistPath, "unraid")
u.syncPersistentBinaryCopy(realExecPath, persistPath, "unraid")
}
}
@@ -1066,7 +1066,7 @@ func (u *Updater) performUpdateWithExecPathForVersion(ctx context.Context, execP
persistPath := qnapPersistentPathFn(u.cfg.StateDir, agentName)
if persistPath != "" {
if _, err := os.Stat(filepath.Dir(persistPath)); err == nil {
u.syncPersistentBinaryCopy(execPath, persistPath, "qnap")
u.syncPersistentBinaryCopy(realExecPath, persistPath, "qnap")
}
}
}
@@ -1187,6 +1187,15 @@ func (u *Updater) performPrivilegedUpdate(ctx context.Context, execPath string,
}
func (u *Updater) syncPersistentBinaryCopy(execPath, persistPath, platform string) {
// QNAP normally runs directly from its persistent data volume. The main
// replacement has already installed the verified, executable binary there.
// Compare file identity, not path spelling, to include symlinked mounts.
if execInfo, err := os.Stat(execPath); err == nil {
if persistInfo, err := os.Stat(persistPath); err == nil && os.SameFile(execInfo, persistInfo) {
return
}
}
u.logger.Debug().Str("path", persistPath).Str("platform", platform).Msg("updating persistent binary")
newBinary, err := readFileFn(execPath)
@@ -1195,16 +1204,39 @@ func (u *Updater) syncPersistentBinaryCopy(execPath, persistPath, platform strin
return
}
tmpPersist := persistPath + ".tmp"
if err := writeFileFn(tmpPersist, newBinary, 0644); err != nil {
// Use an owned, exclusive temporary in the destination directory. A fixed
// .tmp path can retain old permissions or follow a leftover symlink.
tmpFile, err := createTempFn(filepath.Dir(persistPath), filepath.Base(persistPath)+"-persist-*.tmp")
if err != nil {
u.logger.Warn().Err(err).Str("path", persistPath).Str("platform", platform).Msg("failed to create temporary persistent binary")
return
}
tmpPersist := tmpFile.Name()
defer func() {
if removeErr := os.Remove(tmpPersist); removeErr != nil && !errors.Is(removeErr, os.ErrNotExist) {
u.logger.Warn().Err(removeErr).Str("path", tmpPersist).Str("platform", platform).Msg("failed to remove temporary persistent binary")
}
}()
if _, err := tmpFile.Write(newBinary); err != nil {
if closeErr := closeFileFn(tmpFile); closeErr != nil {
err = errors.Join(err, closeErr)
}
u.logger.Warn().Err(err).Str("path", tmpPersist).Str("platform", platform).Msg("failed to write persistent binary")
return
}
if err := closeFileFn(tmpFile); err != nil {
u.logger.Warn().Err(err).Str("path", tmpPersist).Str("platform", platform).Msg("failed to close persistent binary")
return
}
// WriteFile's mode does not repair an existing file, and a restrictive
// umask can remove execute bits. Establish them before the atomic rename.
if err := chmodFn(tmpPersist, 0755); err != nil {
u.logger.Warn().Err(err).Str("path", tmpPersist).Str("platform", platform).Msg("failed to chmod persistent binary")
return
}
if err := renameFn(tmpPersist, persistPath); err != nil {
u.logger.Warn().Err(err).Str("path", persistPath).Str("platform", platform).Msg("failed to replace persistent binary")
if removeErr := os.Remove(tmpPersist); removeErr != nil && !errors.Is(removeErr, os.ErrNotExist) {
u.logger.Warn().Err(removeErr).Str("path", tmpPersist).Str("platform", platform).Msg("failed to remove temporary persistent binary")
}
return
}
+203
View File
@@ -6,11 +6,14 @@ import (
"crypto/rand"
"encoding/base64"
"encoding/json"
"encoding/pem"
"errors"
"net/http"
"net/http/httptest"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"
"sync/atomic"
"testing"
@@ -19,6 +22,206 @@ import (
"github.com/rcourtman/pulse-go-rewrite/internal/updatesignature"
)
// These executables are inert, not Pulse agents. They let the actual signed
// HTTPS updater, self-test and child exec paths check Unix execute permissions
// without replacing the test process or touching an appliance/service.
func buildNASUpdateExecutable(t *testing.T, version string) []byte {
t.Helper()
dir := t.TempDir()
source := filepath.Join(dir, "main.go")
const program = `package main
import ("fmt"; "os")
var version string
func main() {
if len(os.Args) > 1 && os.Args[1] == "--self-test" {
if len(os.Args) != 4 || os.Args[2] != "--token-file" { os.Exit(10) }
info, err := os.Stat(os.Args[3]); if err != nil || info.Mode().Perm() != 0600 { os.Exit(11) }
token, err := os.ReadFile(os.Args[3]); if err != nil || string(token) != "fixture-token" { os.Exit(12) }
}
fmt.Println(version)
}
`
if err := os.WriteFile(source, []byte(program), 0600); err != nil {
t.Fatal(err)
}
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
binary := filepath.Join(dir, "fixture-agent")
cmd := exec.CommandContext(ctx, "go", "build", "-trimpath", "-ldflags=-X main.version="+version, "-o", binary, source)
cmd.Env = append(os.Environ(), "CGO_ENABLED=0", "GOPROXY=off", "GOWORK=off")
if output, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("build inert executable: %v\n%s", err, output)
}
data, err := os.ReadFile(binary)
if err != nil {
t.Fatal(err)
}
return data
}
func runNASUpdateExecutable(t *testing.T, path, wantVersion string) {
t.Helper()
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
output, err := exec.CommandContext(ctx, path).CombinedOutput()
if err != nil || strings.TrimSpace(string(output)) != wantVersion {
t.Fatalf("execute inert binary: error=%v, output=%q, want %q", err, output, wantVersion)
}
}
func TestNASAuthenticatedUpdateKeepsExecutableCopies(t *testing.T) {
if runtime.GOOS != "linux" {
t.Skip("QNAP/Unraid executable-layout controls require Linux")
}
oldBinary := buildNASUpdateExecutable(t, "6.4.5")
newBinary := buildNASUpdateExecutable(t, "6.5.0")
key := configureTrustedUpdateSigningKey(t)
signature := signedUpdateHeader(t, newBinary, key)
for _, layout := range []string{"qnap-unified", "qnap-directory-alias", "qnap-executable-alias", "qnap-persistent-alias", "qnap-split", "unraid-split", "ordinary"} {
t.Run(layout, func(t *testing.T) {
dir := t.TempDir()
stateDir := filepath.Join(dir, "state")
runtimeDir := filepath.Join(dir, "runtime")
for _, path := range []string{stateDir, runtimeDir} {
if err := os.Mkdir(path, 0700); err != nil {
t.Fatal(err)
}
}
execPath := filepath.Join(runtimeDir, "pulse-agent")
persistPath := filepath.Join(stateDir, "pulse-agent")
if layout == "qnap-unified" || layout == "qnap-directory-alias" || layout == "qnap-executable-alias" {
execPath = persistPath
}
for _, path := range []string{execPath, persistPath} {
if err := os.WriteFile(path, oldBinary, 0755); err != nil {
t.Fatal(err)
}
}
if layout == "qnap-directory-alias" {
alias := filepath.Join(dir, "state-alias")
if err := os.Symlink(stateDir, alias); err != nil {
t.Fatal(err)
}
persistPath = filepath.Join(alias, "pulse-agent")
}
if layout == "qnap-executable-alias" {
alias := filepath.Join(runtimeDir, "pulse-agent-link")
if err := os.Symlink(execPath, alias); err != nil {
t.Fatal(err)
}
execPath = alias
}
if layout == "qnap-persistent-alias" {
if err := os.Remove(persistPath); err != nil {
t.Fatal(err)
}
if err := os.Symlink(execPath, persistPath); err != nil {
t.Fatal(err)
}
}
state := map[string]string{"agent-id": "fixture-identity\n", "token": "saved-fixture-token\n", "agent.env": "PULSE_URL=https://fixture.invalid\n"}
for name, value := range state {
if err := os.WriteFile(filepath.Join(stateDir, name), []byte(value), 0600); err != nil {
t.Fatal(err)
}
}
origUnraid, origGetcfg, origQpkg := unraidVersionPath, qnapGetcfgPath, qnapQpkgConfPath
origUnraidPersist, origQnapPersist, origRestart := unraidPersistentPathFn, qnapPersistentPathFn, restartProcessFn
t.Cleanup(func() {
unraidVersionPath, qnapGetcfgPath, qnapQpkgConfPath = origUnraid, origGetcfg, origQpkg
unraidPersistentPathFn, qnapPersistentPathFn, restartProcessFn = origUnraidPersist, origQnapPersist, origRestart
})
unraidVersionPath, qnapGetcfgPath, qnapQpkgConfPath = filepath.Join(dir, "unraid-version"), filepath.Join(dir, "getcfg"), filepath.Join(dir, "qpkg.conf")
marker := ""
if strings.HasPrefix(layout, "qnap-") {
marker = qnapGetcfgPath
} else if layout == "unraid-split" {
marker = unraidVersionPath
}
if marker != "" {
if err := os.WriteFile(marker, []byte("fixture"), 0600); err != nil {
t.Fatal(err)
}
}
unraidPersistentPathFn = func(string) string { return persistPath }
qnapPersistentPathFn = func(string, string) string { return persistPath }
restarts := 0
restartProcessFn = func(path string) error {
restarts++
if path != execPath {
t.Errorf("restart changed executable path")
}
runNASUpdateExecutable(t, path, "6.5.0")
return nil
}
server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/download/pulse-agent" || r.URL.Query().Get("serverVersion") != "6.5.0" || r.Header.Get(apiTokenHeader) != "fixture-token" {
t.Error("missing download identity or authentication")
http.Error(w, "fixture refused", http.StatusForbidden)
return
}
w.Header().Set(checksumSHA256Header, checksum(newBinary))
w.Header().Set(signatureHeader, signature)
_, _ = w.Write(newBinary)
}))
defer server.Close()
caPath := filepath.Join(dir, "fixture-ca.pem")
if err := os.WriteFile(caPath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: server.Certificate().Raw}), 0600); err != nil {
t.Fatal(err)
}
u := New(Config{PulseURL: server.URL, CACertPath: caPath, APIToken: "fixture-token", AgentName: "pulse-agent", CurrentVersion: "6.4.5", StateDir: stateDir})
// Keep the real downloaded-binary self-test, checksum, signature and TLS.
if err := u.performUpdateWithExecPathForVersion(context.Background(), execPath, "6.5.0"); err != nil {
t.Fatal(err)
}
if restarts != 1 {
t.Fatalf("restart calls = %d, want 1", restarts)
}
for _, path := range []string{execPath, persistPath} {
wantBytes, wantVersion := newBinary, "6.5.0"
if layout == "ordinary" && path == persistPath {
wantBytes, wantVersion = oldBinary, "6.4.5"
}
data, err := os.ReadFile(path)
if err != nil || checksum(data) != checksum(wantBytes) {
t.Fatalf("binary bytes changed unexpectedly: %v", err)
}
info, err := os.Stat(path)
if err != nil || info.Mode().Perm() != 0755 {
t.Fatalf("binary is not 0755: %v", err)
}
runNASUpdateExecutable(t, path, wantVersion)
}
for name, value := range state {
path := filepath.Join(stateDir, name)
data, err := os.ReadFile(path)
if err != nil || string(data) != value {
t.Fatalf("saved %s changed: %v", name, err)
}
info, err := os.Stat(path)
if err != nil || info.Mode().Perm() != 0600 {
t.Fatalf("saved %s permissions changed: %v", name, err)
}
}
for _, path := range []string{stateDir, runtimeDir} {
entries, err := os.ReadDir(path)
if err != nil {
t.Fatal(err)
}
for _, entry := range entries {
if strings.HasSuffix(entry.Name(), ".tmp") || strings.HasSuffix(entry.Name(), ".backup") {
t.Fatalf("update left a temporary/backup: %s", entry.Name())
}
}
}
t.Logf("%s: authenticated replacement, real self-test, runtime/persistent exec and saved-state checks passed", layout)
})
}
}
const testPEMCertificate = `-----BEGIN CERTIFICATE-----
MIICuDCCAaACCQDptFpSdDdFNjANBgkqhkiG9w0BAQsFADAeMRwwGgYDVQQDDBNw
dWxzZS10ZXN0LWNhLmxvY2FsMB4XDTI2MDMxNDA4NTgyN1oXDTI3MDMxNDA4NTgy