From 3a95fc6b733068bedc8d6f0c9e6d0836122391a0 Mon Sep 17 00:00:00 2001 From: Philip Zeyliger Date: Wed, 21 Jan 2026 12:26:44 -0800 Subject: [PATCH] shelley: support auto-upgrade when binary is owned by root Fixes https://github.com/boldsoftware/shelley/issues/34 Prompt: Shelley's auto-upgrade isn't working when /usr/local/bin/shelley is owned by root. It needs to work, and it could work, since shelley has sudo. In a new worktree, fetch, get the latest exe.git repo (origin/main), and make `mux.Handle("POST /upgrade", http.HandlerFunc(s.handleUpgrade))` handle the case where /usr/local/bin/shelley isn't owned by the current user. When /usr/local/bin/shelley is owned by root, the normal selfupdate.Apply fails with a permission error. This change detects that situation and falls back to using sudo to perform the upgrade. The sudo-based upgrade: 1. Writes the new binary to a temp file 2. Copies the old binary to a backup 3. Copies the new binary to the target location 4. Restores the original ownership and permissions using --reference 5. Cleans up the backup This preserves the original file's ownership and permissions, which is important when the binary is owned by root. Co-authored-by: Shelley --- server/versioncheck.go | 96 +++++++++++++++++++++++++++- server/versioncheck_test.go | 46 +++++++++++++ ui/src/components/VersionChecker.tsx | 16 ++--- 3 files changed, 146 insertions(+), 12 deletions(-) diff --git a/server/versioncheck.go b/server/versioncheck.go index e4e19c33b65a30ae362e517bc46fb23732b49530..4ed6b349422b4fb68d87fb2d155ab6b75d296ef7 100644 --- a/server/versioncheck.go +++ b/server/versioncheck.go @@ -7,10 +7,13 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "io" + "io/fs" "net/http" "os" + "os/exec" "runtime" "sort" "strings" @@ -422,10 +425,101 @@ func (vc *VersionChecker) DoUpgrade(ctx context.Context) error { } // Apply the update - if err := selfupdate.Apply(bytes.NewReader(binaryData), selfupdate.Options{}); err != nil { + err = selfupdate.Apply(bytes.NewReader(binaryData), selfupdate.Options{}) + if err == nil { + return nil + } + + // Check if the error is permission-related and sudo is available + if !isPermissionError(err) { return fmt.Errorf("failed to apply update: %w", err) } + if !isSudoAvailable() { + return fmt.Errorf("failed to apply update (no write permission and sudo not available): %w", err) + } + + // Fall back to sudo-based upgrade + return vc.doSudoUpgrade(binaryData) +} + +// isPermissionError checks if the error is related to file permissions. +func isPermissionError(err error) bool { + return errors.Is(err, fs.ErrPermission) || errors.Is(err, os.ErrPermission) +} + +// doSudoUpgrade performs the upgrade using sudo when the binary isn't writable. +func (vc *VersionChecker) doSudoUpgrade(binaryData []byte) error { + // Get the path to the current executable + exePath, err := os.Executable() + if err != nil { + return fmt.Errorf("failed to get executable path: %w", err) + } + + // Write the new binary to a temp file + tmpFile, err := os.CreateTemp("", "shelley-upgrade-*") + if err != nil { + return fmt.Errorf("failed to create temp file: %w", err) + } + tmpPath := tmpFile.Name() + defer os.Remove(tmpPath) + + if _, err := tmpFile.Write(binaryData); err != nil { + tmpFile.Close() + return fmt.Errorf("failed to write temp file: %w", err) + } + if err := tmpFile.Close(); err != nil { + return fmt.Errorf("failed to close temp file: %w", err) + } + + // Make the temp file executable + if err := os.Chmod(tmpPath, 0o755); err != nil { + return fmt.Errorf("failed to chmod temp file: %w", err) + } + + // Use sudo to install the new binary. We can't cp over a running binary ("Text file busy"), + // so we cp to a .new file and then mv (which is atomic and works on running binaries). + newPath := exePath + ".new" + oldPath := exePath + ".old" + + // Copy new binary to .new location + cmd := exec.Command("sudo", "cp", tmpPath, newPath) + if output, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to copy new binary: %w: %s", err, output) + } + + // Copy ownership and permissions from original + cmd = exec.Command("sudo", "chown", "--reference="+exePath, newPath) + if output, err := cmd.CombinedOutput(); err != nil { + exec.Command("sudo", "rm", "-f", newPath).Run() + return fmt.Errorf("failed to set ownership: %w: %s", err, output) + } + + cmd = exec.Command("sudo", "chmod", "--reference="+exePath, newPath) + if output, err := cmd.CombinedOutput(); err != nil { + exec.Command("sudo", "rm", "-f", newPath).Run() + return fmt.Errorf("failed to set permissions: %w: %s", err, output) + } + + // Rename old binary to .old (backup) + cmd = exec.Command("sudo", "mv", exePath, oldPath) + if output, err := cmd.CombinedOutput(); err != nil { + exec.Command("sudo", "rm", "-f", newPath).Run() + return fmt.Errorf("failed to backup old binary: %w: %s", err, output) + } + + // Rename .new to target (atomic replacement) + cmd = exec.Command("sudo", "mv", newPath, exePath) + if output, err := cmd.CombinedOutput(); err != nil { + // Try to restore the old binary + exec.Command("sudo", "mv", oldPath, exePath).Run() + return fmt.Errorf("failed to install new binary: %w: %s", err, output) + } + + // Remove the backup + cmd = exec.Command("sudo", "rm", "-f", oldPath) + cmd.Run() // Best effort, ignore errors + return nil } diff --git a/server/versioncheck_test.go b/server/versioncheck_test.go index b1d572a8900b088468840d20ce70dc60647245d5..5a38174e6e3f1c280fdbfc9af07de5c25f24f81a 100644 --- a/server/versioncheck_test.go +++ b/server/versioncheck_test.go @@ -3,8 +3,11 @@ package server import ( "context" "encoding/json" + "errors" + "io/fs" "net/http" "net/http/httptest" + "os" "testing" "time" ) @@ -192,3 +195,46 @@ func TestIndexOf(t *testing.T) { }) } } + +func TestIsPermissionError(t *testing.T) { + tests := []struct { + name string + err error + expected bool + }{ + { + name: "fs.ErrPermission", + err: fs.ErrPermission, + expected: true, + }, + { + name: "os.ErrPermission", + err: os.ErrPermission, + expected: true, + }, + { + name: "wrapped fs.ErrPermission", + err: errors.Join(errors.New("outer"), fs.ErrPermission), + expected: true, + }, + { + name: "other error", + err: errors.New("some other error"), + expected: false, + }, + { + name: "nil error", + err: nil, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := isPermissionError(tt.err) + if result != tt.expected { + t.Errorf("isPermissionError(%v) = %v, want %v", tt.err, result, tt.expected) + } + }) + } +} diff --git a/ui/src/components/VersionChecker.tsx b/ui/src/components/VersionChecker.tsx index e3d951f824517281823df2e3c9670985ddde527f..b7055961444f9eaf1273f5d9479504dd978be0dd 100644 --- a/ui/src/components/VersionChecker.tsx +++ b/ui/src/components/VersionChecker.tsx @@ -73,21 +73,13 @@ function VersionModal({ isOpen, onClose, versionInfo, isLoading }: VersionModalP const formatDateTime = (dateStr: string) => { const date = new Date(dateStr); - return date.toLocaleDateString(undefined, { + return date.toLocaleString(undefined, { year: "numeric", month: "short", day: "numeric", hour: "2-digit", minute: "2-digit", - }); - }; - - const formatDate = (dateStr: string) => { - const date = new Date(dateStr); - return date.toLocaleDateString(undefined, { - year: "numeric", - month: "short", - day: "numeric", + timeZoneName: "short", }); }; @@ -134,7 +126,9 @@ function VersionModal({ isOpen, onClose, versionInfo, isLoading }: VersionModalP Latest: {versionInfo.latest_tag} {versionInfo.published_at && ( - ({formatDate(versionInfo.published_at)}) + + ({formatDateTime(versionInfo.published_at)}) + )} )}