From d2a303ed0c92f866cc0997f45b9b70174ae79edd Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 26 Nov 2025 20:16:09 +0000 Subject: [PATCH] fix: resolve symlinks for agent self-update to avoid cross-device rename When the agent binary is symlinked (e.g., from /opt/pulse/bin to /usr/local/bin), the self-update would fail with "invalid cross-device link" because os.Rename() doesn't work across filesystems. Now resolves symlinks before update and creates the temp file in the same directory as the real binary. Related to #737 --- internal/agentupdate/update.go | 24 ++++++++++---- internal/agentupdate/update_test.go | 50 +++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 7 deletions(-) diff --git a/internal/agentupdate/update.go b/internal/agentupdate/update.go index 2cea345..6c3bcf7 100644 --- a/internal/agentupdate/update.go +++ b/internal/agentupdate/update.go @@ -15,6 +15,7 @@ import ( "net/http" "os" "os/exec" + "path/filepath" "runtime" "strings" "syscall" @@ -327,8 +328,17 @@ func (u *Updater) performUpdate(ctx context.Context) error { // Verify checksum if provided checksumHeader := strings.TrimSpace(resp.Header.Get("X-Checksum-Sha256")) - // Create temporary file - tmpFile, err := os.CreateTemp("", u.cfg.AgentName+"-*.tmp") + // Resolve symlinks to get the real path for atomic rename + realExecPath, err := filepath.EvalSymlinks(execPath) + if err != nil { + // Fall back to original path if symlink resolution fails + realExecPath = execPath + } + + // Create temporary file in the same directory as the target binary + // to ensure atomic rename works (os.Rename fails across filesystems) + targetDir := filepath.Dir(realExecPath) + tmpFile, err := os.CreateTemp(targetDir, u.cfg.AgentName+"-*.tmp") if err != nil { return fmt.Errorf("failed to create temp file: %w", err) } @@ -374,15 +384,15 @@ func (u *Updater) performUpdate(ctx context.Context) error { return fmt.Errorf("failed to chmod: %w", err) } - // Atomic replacement with backup - backupPath := execPath + ".backup" - if err := os.Rename(execPath, backupPath); err != nil { + // Atomic replacement with backup (use realExecPath for rename operations) + backupPath := realExecPath + ".backup" + if err := os.Rename(realExecPath, backupPath); err != nil { return fmt.Errorf("failed to backup current binary: %w", err) } - if err := os.Rename(tmpPath, execPath); err != nil { + if err := os.Rename(tmpPath, realExecPath); err != nil { // Restore backup on failure - os.Rename(backupPath, execPath) + os.Rename(backupPath, realExecPath) return fmt.Errorf("failed to replace binary: %w", err) } diff --git a/internal/agentupdate/update_test.go b/internal/agentupdate/update_test.go index 6f09879..b9ff449 100644 --- a/internal/agentupdate/update_test.go +++ b/internal/agentupdate/update_test.go @@ -340,3 +340,53 @@ func TestConstants(t *testing.T) { t.Error("downloadTimeout should be positive") } } + +func TestSymlinkResolution(t *testing.T) { + // Test that filepath.EvalSymlinks works as expected for our use case + tmpDir := t.TempDir() + + // Create a real file + realFile := filepath.Join(tmpDir, "real", "binary") + if err := os.MkdirAll(filepath.Dir(realFile), 0755); err != nil { + t.Fatalf("failed to create dir: %v", err) + } + if err := os.WriteFile(realFile, []byte("test"), 0755); err != nil { + t.Fatalf("failed to create file: %v", err) + } + + // Create a symlink to it + linkFile := filepath.Join(tmpDir, "link", "binary") + if err := os.MkdirAll(filepath.Dir(linkFile), 0755); err != nil { + t.Fatalf("failed to create link dir: %v", err) + } + if err := os.Symlink(realFile, linkFile); err != nil { + t.Fatalf("failed to create symlink: %v", err) + } + + // EvalSymlinks should resolve to the real path + resolved, err := filepath.EvalSymlinks(linkFile) + if err != nil { + t.Fatalf("EvalSymlinks failed: %v", err) + } + + if resolved != realFile { + t.Errorf("EvalSymlinks(%q) = %q, want %q", linkFile, resolved, realFile) + } + + // Verify temp file in same dir as resolved path allows rename + targetDir := filepath.Dir(resolved) + tmpFile, err := os.CreateTemp(targetDir, "test-*.tmp") + if err != nil { + t.Fatalf("failed to create temp file: %v", err) + } + tmpPath := tmpFile.Name() + tmpFile.Close() + defer os.Remove(tmpPath) + + // Rename should work since both files are on the same filesystem + newPath := filepath.Join(targetDir, "renamed") + if err := os.Rename(tmpPath, newPath); err != nil { + t.Errorf("Rename failed (same filesystem): %v", err) + } + os.Remove(newPath) +}