From 4eee055275a146b2a5087beaa21930242164b2d5 Mon Sep 17 00:00:00 2001 From: Amolith Date: Tue, 9 Dec 2025 14:16:00 -0700 Subject: [PATCH] fix(update): various bug fixes and improvements - Fix go install pseudo-version regexp to match actual format - Add nil guard for release in async update check - Improve temp file handling (close immediately after write) - Add write permission check on Windows before copying - Make lock file handling more resilient on Windows - Add clarifying comment for AUR update instruction Assisted-by: Claude Opus 4.5 via Crush --- internal/cmd/root.go | 5 +++++ internal/update/update.go | 21 +++++++++------------ internal/update/update_test.go | 4 ++-- internal/update/update_windows.go | 14 ++++++++++---- 4 files changed, 26 insertions(+), 18 deletions(-) diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 27e48d415c87cf55dabfe4f72970f57c5f82a017..b511bb4949b6a1e8dcb8a4da85afefcc38fcb3ae 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -240,6 +240,11 @@ func checkForUpdateAsync(ctx context.Context, program *tea.Program) { return } + // Guard against nil release (shouldn't happen, but defensive). + if info.Release == nil { + return + } + // Check install method. method := update.DetectInstallMethod() if !method.CanSelfUpdate() { diff --git a/internal/update/update.go b/internal/update/update.go index 334646cdaea6828c8e1493e4f95d964faf4a3522..d1dac71f4c6cb33f763a9e31a486ba1c38e3b067 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -55,8 +55,8 @@ type Info struct { } // goInstallRegexp matches pseudo-versions from go install: -// v0.0.0-20251231235959-06c807842604 -var goInstallRegexp = regexp.MustCompile(`^v?\d+\.\d+\.\d+-\d{14}-[0-9a-f]{12}$`) +// v0.0.0-0.20251231235959-06c807842604 +var goInstallRegexp = regexp.MustCompile(`^v?\d+\.\d+\.\d+-\d+\.\d{14}-[0-9a-f]{12}$`) // gitDescribeRegexp matches git describe versions: // v0.19.0-15-g1a2b3c4d (tag-commits-ghash) @@ -181,7 +181,7 @@ func (m InstallMethod) UpdateInstructions() string { case InstallMethodNPM: return "npm update -g @charmland/crush" case InstallMethodAUR: - return "yay -Syu crush-bin" + return "yay -Syu crush-bin # or your preferred AUR helper" case InstallMethodNix: return "nix flake update # or update your NUR channel" case InstallMethodWinget: @@ -333,9 +333,7 @@ func FindAsset(assets []Asset) (*Asset, error) { // Download downloads and extracts the crush binary from the given asset. // Returns the path to the extracted binary. func Download(ctx context.Context, asset *Asset, release *Release) (string, error) { - client := &http.Client{ - Timeout: 5 * time.Minute, - } + client := &http.Client{} // Find and download checksums.txt. var checksumsAsset *Asset @@ -380,22 +378,21 @@ func Download(ctx context.Context, asset *Asset, release *Release) (string, erro if err != nil { return "", fmt.Errorf("failed to create temp file: %w", err) } - defer os.Remove(tmpFile.Name()) + tmpFileName := tmpFile.Name() + defer os.Remove(tmpFileName) // Download to temp file while computing checksum. // Use LimitReader to prevent DoS from oversized downloads (Content-Length can be spoofed). hash := sha256.New() limitedBody := io.LimitReader(resp.Body, maxArchiveSize+1) written, err := copyWithContext(ctx, io.MultiWriter(tmpFile, hash), limitedBody) + tmpFile.Close() // Close immediately after writing, before size check. if err != nil { - tmpFile.Close() return "", fmt.Errorf("failed to download: %w", err) } if written > maxArchiveSize { - tmpFile.Close() return "", fmt.Errorf("archive size %d exceeds maximum allowed size of %d bytes", written, maxArchiveSize) } - tmpFile.Close() // Verify checksum. actualSum := hex.EncodeToString(hash.Sum(nil)) @@ -410,9 +407,9 @@ func Download(ctx context.Context, asset *Asset, release *Release) (string, erro // Extract binary based on archive type. var binaryPath string if strings.HasSuffix(asset.Name, ".zip") { - binaryPath, err = extractZip(tmpFile.Name()) + binaryPath, err = extractZip(tmpFileName) } else { - binaryPath, err = extractTarGz(tmpFile.Name()) + binaryPath, err = extractTarGz(tmpFileName) } if err != nil { if binaryPath != "" { diff --git a/internal/update/update_test.go b/internal/update/update_test.go index 1a44b58ac45e75b3ebc83e3662f578753ef0b110..dee04c51f1ddc2293ce1fbd2cc6a7982aa93c2d0 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -126,7 +126,7 @@ func TestIsDevelopment(t *testing.T) { {"unknown version", "unknown", true}, {"dirty version", "0.19.0-dirty", true}, {"dirty with suffix", "0.19.0-10-g1234567-dirty", true}, - {"go install pseudo-version", "v0.0.0-20251231235959-06c807842604", true}, + {"go install pseudo-version", "v0.0.0-0.20251231235959-06c807842604", true}, {"git describe version", "v0.19.0-15-g1a2b3c4d", true}, {"git describe short hash", "0.19.0-3-gabcdef0", true}, {"git describe long hash", "v1.0.0-100-g0123456789ab", true}, @@ -512,7 +512,7 @@ func TestInstallMethod_UpdateInstructions(t *testing.T) { }{ {InstallMethodHomebrew, "brew upgrade"}, {InstallMethodNPM, "npm update"}, - {InstallMethodAUR, "yay"}, + {InstallMethodAUR, "yay -Syu crush-bin"}, {InstallMethodNix, "nix"}, {InstallMethodWinget, "winget upgrade"}, {InstallMethodScoop, "scoop update"}, diff --git a/internal/update/update_windows.go b/internal/update/update_windows.go index 974c722a7fb95c52cec2992798c7f473123c4c64..cc19ca220d9facfbd8a92d314ba3b5ace08ae94a 100644 --- a/internal/update/update_windows.go +++ b/internal/update/update_windows.go @@ -3,6 +3,7 @@ package update import ( + "fmt" "os" "path/filepath" "strings" @@ -76,6 +77,12 @@ func Apply(binaryPath string) error { return err } + // Check write permission before attempting to copy. + exeDir := filepath.Dir(pendingPath) + if err := checkWritePermission(exeDir); err != nil { + return fmt.Errorf("cannot write to %s: %w (you may need to run as administrator)", exeDir, err) + } + // Copy the new binary to the pending location. if err := copyFile(binaryPath, pendingPath); err != nil { return err @@ -128,10 +135,9 @@ func ApplyPendingUpdate() error { lockPath := exe + ".lock" lock, err := os.OpenFile(lockPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o644) if err != nil { - if os.IsExist(err) { - return nil // Another process is handling this. - } - return err + // Lock exists or can't be created—another process is likely handling this, + // or we lack permissions. Either way, silently skip to avoid blocking startup. + return nil } defer func() { lock.Close()