From 1d7e14af31f5eb22db2c847fff19e6efe6edc4c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Apr 2026 08:02:01 +0000 Subject: [PATCH 1/3] fix: wrap bare returned errors with context Add fmt.Errorf("context: %w", err) wrapping on previously bare `return err` sites in auth, shell, npm, and dotfiles. Per CLAUDE.md convention, callers should get contextual error chains. --- internal/auth/auth.go | 6 +++--- internal/dotfiles/dotfiles.go | 21 ++++++++++++--------- internal/npm/npm.go | 2 +- internal/shell/shell.go | 13 ++++++++----- 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/internal/auth/auth.go b/internal/auth/auth.go index 7afb367..41d346d 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -26,7 +26,7 @@ func TokenPath() (string, error) { func LoadToken() (*StoredAuth, error) { path, err := TokenPath() if err != nil { - return nil, err + return nil, fmt.Errorf("load token: %w", err) } data, err := os.ReadFile(path) if err != nil { @@ -51,7 +51,7 @@ func LoadToken() (*StoredAuth, error) { func SaveToken(auth *StoredAuth) error { path, err := TokenPath() if err != nil { - return err + return fmt.Errorf("save token: %w", err) } dir := filepath.Dir(path) @@ -74,7 +74,7 @@ func SaveToken(auth *StoredAuth) error { func DeleteToken() error { path, err := TokenPath() if err != nil { - return err + return fmt.Errorf("delete token: %w", err) } err = os.Remove(path) if err != nil && !os.IsNotExist(err) { diff --git a/internal/dotfiles/dotfiles.go b/internal/dotfiles/dotfiles.go index 38a17de..c49686b 100644 --- a/internal/dotfiles/dotfiles.go +++ b/internal/dotfiles/dotfiles.go @@ -43,15 +43,18 @@ func Clone(repoURL string, dryRun bool) error { home, err := system.HomeDir() if err != nil { - return err + return fmt.Errorf("clone dotfiles: %w", err) } dotfilesPath := filepath.Join(home, defaultDotfilesDir) if _, err := os.Stat(dotfilesPath); err == nil { // Dotfiles directory already exists — sync or re-clone as appropriate. needsClone, err := handleExistingDotfiles(dotfilesPath, repoURL, dryRun) - if err != nil || !needsClone { - return err + if err != nil { + return fmt.Errorf("handle existing dotfiles: %w", err) + } + if !needsClone { + return nil } } @@ -210,7 +213,7 @@ func confirmResetIfDirty(dotfilesPath, branch string) bool { func Link(dryRun bool) error { home, err := system.HomeDir() if err != nil { - return err + return fmt.Errorf("link dotfiles: %w", err) } dotfilesPath := filepath.Join(home, defaultDotfilesDir) @@ -332,17 +335,17 @@ func backupConflicts(pkgDir, targetDir string) ([][2]string, error) { func linkWithStow(dotfilesPath string, dryRun bool) error { if err := ensureStow(dryRun); err != nil { - return err + return fmt.Errorf("ensure stow: %w", err) } entries, err := os.ReadDir(dotfilesPath) if err != nil { - return err + return fmt.Errorf("read dotfiles dir: %w", err) } home, err := system.HomeDir() if err != nil { - return err + return fmt.Errorf("link with stow: %w", err) } var errs []error @@ -399,12 +402,12 @@ func linkWithStow(dotfilesPath string, dryRun bool) error { func linkDirect(dotfilesPath string, dryRun bool) error { home, err := system.HomeDir() if err != nil { - return err + return fmt.Errorf("link direct: %w", err) } entries, err := os.ReadDir(dotfilesPath) if err != nil { - return err + return fmt.Errorf("read dotfiles dir: %w", err) } for _, entry := range entries { diff --git a/internal/npm/npm.go b/internal/npm/npm.go index 5757790..c7b542b 100644 --- a/internal/npm/npm.go +++ b/internal/npm/npm.go @@ -124,7 +124,7 @@ func Install(packages []string, dryRun bool) error { failed, err := installBatch(toInstall) if err != nil { - return err + return fmt.Errorf("install npm packages: %w", err) } if len(failed) > 0 { diff --git a/internal/shell/shell.go b/internal/shell/shell.go index 797a1d0..6533011 100644 --- a/internal/shell/shell.go +++ b/internal/shell/shell.go @@ -119,7 +119,7 @@ func EnsureBrewShellenv(dryRun bool) error { home, err := system.HomeDir() if err != nil { - return err + return fmt.Errorf("ensure brew shellenv: %w", err) } zshrcPath := filepath.Join(home, ".zshrc") @@ -129,7 +129,10 @@ func EnsureBrewShellenv(dryRun bool) error { fmt.Printf("[DRY-RUN] Would create %s with Homebrew shellenv\n", zshrcPath) return nil } - return os.WriteFile(zshrcPath, []byte(brewShellenvLine+"\n"), 0600) + if err := os.WriteFile(zshrcPath, []byte(brewShellenvLine+"\n"), 0600); err != nil { + return fmt.Errorf("create .zshrc: %w", err) + } + return nil } raw, err := os.ReadFile(zshrcPath) @@ -262,7 +265,7 @@ func RestoreFromSnapshot(ohMyZsh bool, theme string, plugins []string, dryRun bo home, err := system.HomeDir() if err != nil { - return err + return fmt.Errorf("configure zshrc: %w", err) } zshrcPath := filepath.Join(home, ".zshrc") @@ -272,11 +275,11 @@ func RestoreFromSnapshot(ohMyZsh bool, theme string, plugins []string, dryRun bo return nil } if err := validateShellIdentifier(theme, "ZSH_THEME"); err != nil { - return err + return fmt.Errorf("validate theme: %w", err) } for _, p := range plugins { if err := validateShellIdentifier(p, "plugin"); err != nil { - return err + return fmt.Errorf("validate plugin: %w", err) } } template := fmt.Sprintf(`export ZSH="$HOME/.oh-my-zsh" From 13ec13627502f527cc639b24fad94f34693e7ce4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Apr 2026 08:02:08 +0000 Subject: [PATCH 2/3] test: skip filesystem-permission tests when running as root Tests that rely on chmod 0500/0000 to trigger permission denied silently succeed under root because root bypasses DAC checks, leaving `err == nil` where the test expected a non-nil error and panicking on the follow-up nil dereference. Guard these with `os.Geteuid() == 0` skips; they still run as a non-root user or in standard CI. --- internal/auth/auth_test.go | 8 ++++++++ internal/auth/login_test.go | 3 +++ internal/cli/login_test.go | 3 +++ internal/state/reminder_test.go | 3 +++ 4 files changed, 17 insertions(+) diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index 26b20e2..e83199b 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -258,6 +258,10 @@ func TestDeleteToken_FileNotExist(t *testing.T) { } func TestDeleteToken_PermissionError(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses filesystem permission checks") + } + tmpDir := t.TempDir() authDir := filepath.Join(tmpDir, ".openboot") authFile := filepath.Join(authDir, "auth.json") @@ -378,6 +382,10 @@ func TestStoredAuth_JSONMarshaling(t *testing.T) { } func TestLoadToken_ReadPermissionError(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses filesystem permission checks") + } + tmpDir := t.TempDir() authDir := filepath.Join(tmpDir, ".openboot") authFile := filepath.Join(authDir, "auth.json") diff --git a/internal/auth/login_test.go b/internal/auth/login_test.go index fc1700a..eafa8c8 100644 --- a/internal/auth/login_test.go +++ b/internal/auth/login_test.go @@ -521,6 +521,9 @@ func TestLoginInteractive_InvalidExpirationFormat(t *testing.T) { } func TestLoginInteractive_SaveTokenError(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses filesystem permission checks") + } withFastPoll(t) withNoBrowser(t) tmpDir := t.TempDir() diff --git a/internal/cli/login_test.go b/internal/cli/login_test.go index a81b65f..367026a 100644 --- a/internal/cli/login_test.go +++ b/internal/cli/login_test.go @@ -90,6 +90,9 @@ func TestLogoutCmd_WhenNotAuthenticated(t *testing.T) { } func TestLogoutCmd_DeleteError(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses filesystem permission checks") + } tmpDir := setupTestAuth(t, true) authDir := filepath.Join(tmpDir, ".openboot") diff --git a/internal/state/reminder_test.go b/internal/state/reminder_test.go index d9027cc..ec5fd1b 100644 --- a/internal/state/reminder_test.go +++ b/internal/state/reminder_test.go @@ -176,6 +176,9 @@ func TestRoundTrip_DefaultState(t *testing.T) { } func TestLoadState_ReadError(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root bypasses filesystem permission checks") + } tmpDir := t.TempDir() statePath := filepath.Join(tmpDir, "state.json") From c2906082ae59efeafd1aaf9dde662a75482ed14d Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Apr 2026 08:02:13 +0000 Subject: [PATCH 3/3] refactor: make brewInstallURL const The existing comment described it as a test seam, but no test reassigns it. A const prevents accidental mutation; if a test ever needs to override the URL, add a proper seam then. --- internal/system/system.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/system/system.go b/internal/system/system.go index bb569f2..4db71c0 100644 --- a/internal/system/system.go +++ b/internal/system/system.go @@ -77,8 +77,7 @@ func RunCommandOutput(name string, args ...string) (string, error) { // curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh | sha256sum const knownBrewInstallHash = "dfd5145fe2aa5956a600e35848765273f5798ce6def01bd08ecec088a1268d91" -// brewInstallURL is a var so tests can redirect it without a real server. -var brewInstallURL = "https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh" +const brewInstallURL = "https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh" // brewHTTPClient is a var so tests can inject a mock transport. var brewHTTPClient *http.Client = http.DefaultClient