From 7af2018025c8f91cfce8803807dc034df67dc772 Mon Sep 17 00:00:00 2001 From: Miguel Sanchez Gonzalez Date: Mon, 27 Apr 2026 18:45:50 +0200 Subject: [PATCH 1/9] feat(config:install): improve destination directory detection --- commands/config_install.go | 3 +- internal/config/alt/fs.go | 123 +++++++++++++++++++++++--- internal/config/alt/fs_test.go | 153 ++++++++++++++++++++++++++------- 3 files changed, 236 insertions(+), 43 deletions(-) diff --git a/commands/config_install.go b/commands/config_install.go index 6ebc1ded4..ec464f09f 100644 --- a/commands/config_install.go +++ b/commands/config_install.go @@ -12,6 +12,7 @@ import ( "github.com/fatih/color" "github.com/spf13/cobra" + "github.com/spf13/viper" "github.com/symfony-cli/terminal" "github.com/upsun/cli/internal/config" @@ -102,7 +103,7 @@ func runConfigInstall(cmd *cobra.Command, args []string) error { cmd.PrintErrf("The executable runs %s with the new configuration.\n", color.CyanString(formatPath(target))) cmd.PrintErrln() - if terminal.Stdin.IsInteractive() { + if terminal.Stdin.IsInteractive() && !viper.GetBool("no-interaction") { if !terminal.AskConfirmation("Are you sure you want to continue?", true) { os.Exit(1) } diff --git a/internal/config/alt/fs.go b/internal/config/alt/fs.go index 37f33718d..9b79a8435 100644 --- a/internal/config/alt/fs.go +++ b/internal/config/alt/fs.go @@ -14,8 +14,26 @@ const ( homeSubDir = ".platform-alt" ) +// executableFn is the source of the current executable path. It is a variable so tests can stub +// out os.Executable. +var executableFn = os.Executable + // FindConfigDir finds an appropriate destination directory for an "alt" CLI configuration YAML file. +// +// XDG_CONFIG_HOME takes precedence on all platforms, since os.UserConfigDir does not honor it on +// macOS or Windows. If neither XDG_CONFIG_HOME nor os.UserConfigDir yields an existing directory, +// it falls back to ~/.platform-alt. func FindConfigDir() (string, error) { + if xdg := os.Getenv("XDG_CONFIG_HOME"); xdg != "" { + isDir, err := isExistingDirectory(xdg) + if err != nil { + return "", err + } + if isDir { + return filepath.Join(xdg, subDir), nil + } + } + userConfigDir, err := os.UserConfigDir() if err != nil { return "", err @@ -37,35 +55,116 @@ func FindConfigDir() (string, error) { } // FindBinDir finds an appropriate destination directory for an "alt" CLI executable. +// +// The selection rules are: +// +// 1. If the currently running CLI executable lives in a directory on the per-OS allowlist, and +// that directory is writable and on PATH, install the alt there. This co-locates the alt with +// the source binary in package-manager installs (e.g. Homebrew). +// 2. Otherwise, pick the first allowlist entry that is writable and on PATH. +// 3. Otherwise, fall back to ~/.platform-alt/bin (the caller is expected to print PATH +// instructions in that case). +// +// The allowlist exists to avoid installing alongside binaries in version-scoped or developer +// locations such as ~/.nvm/versions/node//bin or a local ./dist build directory. func FindBinDir() (string, error) { homeDir, err := os.UserHomeDir() if err != nil { return "", fmt.Errorf("could not determine home directory: %w", err) } - var candidates []string - if runtime.GOOS == "windows" { - candidates = []string{ - filepath.Join(homeDir, "AppData", "Local", "Programs"), + candidates := binDirAllowlist(homeDir) + pathValue := os.Getenv("PATH") + + if exe, err := executableFn(); err == nil { + exeDir := filepath.Dir(exe) + if matchesAllowlist(exeDir, candidates) && inPathValue(exeDir, pathValue) && isWritableDir(exeDir) { + return exeDir, nil + } + } + + for _, c := range candidates { + if inPathValue(c, pathValue) && isWritableDir(c) { + return c, nil + } + } + + return filepath.Join(homeDir, homeSubDir, "bin"), nil +} + +// binDirAllowlist returns the per-OS list of acceptable bin directories, in priority order. +// Empty entries (e.g. unset XDG_BIN_HOME) and duplicates are removed. +func binDirAllowlist(homeDir string) []string { + xdgBinHome := os.Getenv("XDG_BIN_HOME") + + var raw []string + switch runtime.GOOS { + case "darwin": + raw = []string{ + "/opt/homebrew/bin", + "/usr/local/bin", + xdgBinHome, filepath.Join(homeDir, ".local", "bin"), filepath.Join(homeDir, "bin"), } - } else { - candidates = []string{ + case "windows": + raw = []string{ + filepath.Join(homeDir, "scoop", "shims"), + filepath.Join(homeDir, "AppData", "Local", "Programs"), + filepath.Join(homeDir, ".local", "bin"), + } + default: + raw = []string{ + "/home/linuxbrew/.linuxbrew/bin", + xdgBinHome, filepath.Join(homeDir, ".local", "bin"), filepath.Join(homeDir, "bin"), } } - // Use the first candidate that is in the PATH. - pathValue := os.Getenv("PATH") - for _, c := range candidates { - if inPathValue(c, pathValue) { - return c, nil + out := make([]string, 0, len(raw)) + seen := make(map[string]struct{}, len(raw)) + for _, p := range raw { + if p == "" { + continue + } + norm := normalizePathEntry(p, homeDir) + if _, ok := seen[norm]; ok { + continue + } + seen[norm] = struct{}{} + out = append(out, p) + } + return out +} + +// matchesAllowlist reports whether dir matches any allowlist entry after normalization. +func matchesAllowlist(dir string, allowlist []string) bool { + homeDir, _ := os.UserHomeDir() + target := normalizePathEntry(dir, homeDir) + for _, c := range allowlist { + if normalizePathEntry(c, homeDir) == target { + return true } } + return false +} - return filepath.Join(homeDir, homeSubDir, "bin"), nil +// isWritableDir reports whether path is an existing directory that the current process can write +// to. Detection is best-effort: it tries to create and remove a temp file in the directory. +func isWritableDir(path string) bool { + stat, err := os.Stat(path) + if err != nil || !stat.IsDir() { + return false + } + f, err := os.CreateTemp(path, ".platform-alt-write-check-*") + if err != nil { + return false + } + name := f.Name() + _ = f.Close() + _ = os.Remove(name) + return true } // isExistingDirectory checks if a path exists and is a directory. diff --git a/internal/config/alt/fs_test.go b/internal/config/alt/fs_test.go index caeeec71e..6accaac03 100644 --- a/internal/config/alt/fs_test.go +++ b/internal/config/alt/fs_test.go @@ -14,16 +14,11 @@ func TestFindConfigDir(t *testing.T) { tempDir := t.TempDir() t.Run("XDG_CONFIG_HOME exists", func(t *testing.T) { - switch runtime.GOOS { - case "windows", "darwin", "ios", "plan9": + if runtime.GOOS == "plan9" { t.Skip() } - err := os.Setenv("XDG_CONFIG_HOME", tempDir) - require.NoError(t, err) - defer os.Unsetenv("XDG_CONFIG_HOME") - - err = os.Mkdir(filepath.Join(tempDir, subDir), 0o755) - require.NoError(t, err) + require.NoError(t, os.Setenv("XDG_CONFIG_HOME", tempDir)) + t.Cleanup(func() { _ = os.Unsetenv("XDG_CONFIG_HOME") }) result, err := FindConfigDir() assert.NoError(t, err) @@ -31,39 +26,137 @@ func TestFindConfigDir(t *testing.T) { }) t.Run("HOME fallback", func(t *testing.T) { - err := os.Setenv("HOME", tempDir) - require.NoError(t, err) - defer os.Unsetenv("HOME") + _ = os.Unsetenv("XDG_CONFIG_HOME") + require.NoError(t, os.Setenv("HOME", tempDir)) + t.Cleanup(func() { _ = os.Unsetenv("HOME") }) result, err := FindConfigDir() assert.NoError(t, err) - assert.Equal(t, filepath.Join(tempDir, homeSubDir), result) + // On platforms where os.UserConfigDir resolves to an existing directory under the test + // HOME, that directory wins. Only assert the home fallback when it doesn't. + ucd, ucdErr := os.UserConfigDir() + isDir, _ := isExistingDirectory(ucd) + if ucdErr != nil || !isDir { + assert.Equal(t, filepath.Join(tempDir, homeSubDir), result) + } }) } func TestFindBinDir(t *testing.T) { - tempDir := t.TempDir() + if runtime.GOOS == "windows" { + t.Skip("path setup in this test is unix-style") + } - err := os.Setenv("HOME", tempDir) - require.NoError(t, err) - defer os.Unsetenv("HOME") + originalPath := os.Getenv("PATH") + originalHome := os.Getenv("HOME") + originalXDGBin := os.Getenv("XDG_BIN_HOME") + originalExecFn := executableFn + t.Cleanup(func() { + _ = os.Setenv("PATH", originalPath) + _ = os.Setenv("HOME", originalHome) + _ = os.Setenv("XDG_BIN_HOME", originalXDGBin) + executableFn = originalExecFn + }) - result, err := FindBinDir() - assert.NoError(t, err) - assert.Equal(t, filepath.Join(tempDir, homeSubDir, "bin"), result) + t.Run("home fallback when nothing on PATH", func(t *testing.T) { + tempDir := t.TempDir() + require.NoError(t, os.Setenv("HOME", tempDir)) + require.NoError(t, os.Setenv("PATH", "/nonexistent/dir")) + _ = os.Unsetenv("XDG_BIN_HOME") + executableFn = func() (string, error) { return filepath.Join(tempDir, "exe"), nil } - var standardDir string - if runtime.GOOS == "windows" { - standardDir = filepath.Join("AppData", "Local", "Programs") - } else { - standardDir = filepath.Join(".local", "bin") - } - err = os.Setenv("PATH", os.Getenv("PATH")+string(os.PathListSeparator)+filepath.Join(tempDir, standardDir)) - require.NoError(t, err) + result, err := FindBinDir() + assert.NoError(t, err) + assert.Equal(t, filepath.Join(tempDir, homeSubDir, "bin"), result) + }) - result, err = FindBinDir() - assert.NoError(t, err) - assert.Equal(t, filepath.Join(tempDir, standardDir), result) + t.Run("allowlist fallback picks first writable PATH entry", func(t *testing.T) { + tempDir := t.TempDir() + localBin := filepath.Join(tempDir, ".local", "bin") + require.NoError(t, os.MkdirAll(localBin, 0o755)) + require.NoError(t, os.Setenv("HOME", tempDir)) + require.NoError(t, os.Setenv("PATH", localBin)) + _ = os.Unsetenv("XDG_BIN_HOME") + // Executable lives outside the allowlist; should not be selected. + executableFn = func() (string, error) { + return filepath.Join(tempDir, "build", "exe"), nil + } + + result, err := FindBinDir() + assert.NoError(t, err) + assert.Equal(t, localBin, result) + }) + + t.Run("source-dir match on allowlist co-locates", func(t *testing.T) { + tempDir := t.TempDir() + sourceBin := filepath.Join(tempDir, "bin") + higherPriorityBin := filepath.Join(tempDir, ".local", "bin") + require.NoError(t, os.MkdirAll(sourceBin, 0o755)) + require.NoError(t, os.MkdirAll(higherPriorityBin, 0o755)) + require.NoError(t, os.Setenv("HOME", tempDir)) + require.NoError(t, os.Setenv("PATH", higherPriorityBin+string(os.PathListSeparator)+sourceBin)) + _ = os.Unsetenv("XDG_BIN_HOME") + // Executable lives in the lower-priority allowlist entry; we must pick that one over the + // higher-priority one. + executableFn = func() (string, error) { return filepath.Join(sourceBin, "exe"), nil } + + result, err := FindBinDir() + assert.NoError(t, err) + assert.Equal(t, sourceBin, result) + }) + + t.Run("nvm-style source dir is not selected", func(t *testing.T) { + tempDir := t.TempDir() + nvmBin := filepath.Join(tempDir, ".nvm", "versions", "node", "v20.0.0", "bin") + localBin := filepath.Join(tempDir, ".local", "bin") + require.NoError(t, os.MkdirAll(nvmBin, 0o755)) + require.NoError(t, os.MkdirAll(localBin, 0o755)) + require.NoError(t, os.Setenv("HOME", tempDir)) + require.NoError(t, os.Setenv("PATH", nvmBin+string(os.PathListSeparator)+localBin)) + _ = os.Unsetenv("XDG_BIN_HOME") + executableFn = func() (string, error) { return filepath.Join(nvmBin, "upsun"), nil } + + result, err := FindBinDir() + assert.NoError(t, err) + // Must skip the nvm dir (not on allowlist) and pick the allowlist entry. + assert.Equal(t, localBin, result) + }) + + t.Run("XDG_BIN_HOME is honored", func(t *testing.T) { + tempDir := t.TempDir() + xdgBin := filepath.Join(tempDir, "xdg-bin") + require.NoError(t, os.MkdirAll(xdgBin, 0o755)) + require.NoError(t, os.Setenv("HOME", tempDir)) + require.NoError(t, os.Setenv("XDG_BIN_HOME", xdgBin)) + require.NoError(t, os.Setenv("PATH", xdgBin)) + executableFn = func() (string, error) { return filepath.Join(tempDir, "exe"), nil } + + result, err := FindBinDir() + assert.NoError(t, err) + assert.Equal(t, xdgBin, result) + }) + + t.Run("non-writable PATH entry is skipped", func(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("running as root; cannot create a non-writable directory") + } + tempDir := t.TempDir() + readOnlyBin := filepath.Join(tempDir, ".local", "bin") + writableBin := filepath.Join(tempDir, "bin") + require.NoError(t, os.MkdirAll(readOnlyBin, 0o755)) + require.NoError(t, os.MkdirAll(writableBin, 0o755)) + require.NoError(t, os.Chmod(readOnlyBin, 0o555)) + t.Cleanup(func() { _ = os.Chmod(readOnlyBin, 0o755) }) + + require.NoError(t, os.Setenv("HOME", tempDir)) + require.NoError(t, os.Setenv("PATH", readOnlyBin+string(os.PathListSeparator)+writableBin)) + _ = os.Unsetenv("XDG_BIN_HOME") + executableFn = func() (string, error) { return filepath.Join(tempDir, "exe"), nil } + + result, err := FindBinDir() + assert.NoError(t, err) + assert.Equal(t, writableBin, result) + }) } func TestFSHelpers(t *testing.T) { From fcf4366f1ea0c64d707897c1cc3074b863626073 Mon Sep 17 00:00:00 2001 From: Miguel Sanchez Gonzalez Date: Mon, 27 Apr 2026 19:38:42 +0200 Subject: [PATCH 2/9] fix(config:install): resolve symlinks when matching exe to bin allowlist Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/config/alt/fs.go | 64 ++++++++++++++++++++++++---------- internal/config/alt/fs_test.go | 31 ++++++++++++++++ 2 files changed, 77 insertions(+), 18 deletions(-) diff --git a/internal/config/alt/fs.go b/internal/config/alt/fs.go index 9b79a8435..7d337203a 100644 --- a/internal/config/alt/fs.go +++ b/internal/config/alt/fs.go @@ -58,13 +58,20 @@ func FindConfigDir() (string, error) { // // The selection rules are: // -// 1. If the currently running CLI executable lives in a directory on the per-OS allowlist, and -// that directory is writable and on PATH, install the alt there. This co-locates the alt with -// the source binary in package-manager installs (e.g. Homebrew). +// 1. If the currently running CLI executable lives in (or is reachable via a symlink from) a +// directory on the per-OS allowlist, and that directory is writable and on PATH, install +// the alt there. This co-locates the alt with the source binary in package-manager installs +// (e.g. Homebrew, Linuxbrew, Scoop). // 2. Otherwise, pick the first allowlist entry that is writable and on PATH. // 3. Otherwise, fall back to ~/.platform-alt/bin (the caller is expected to print PATH // instructions in that case). // +// The symlink reachability check matters because os.Executable behaves differently per OS: on +// macOS and Windows it returns the path used to invoke the binary (preserving symlinks), while +// on Linux it returns /proc/self/exe fully resolved. Linuxbrew installs a symlink in +// /home/linuxbrew/.linuxbrew/bin pointing into a versioned Cellar directory, so the resolved +// exe path doesn't match the allowlist directly — we have to compare resolved targets instead. +// // The allowlist exists to avoid installing alongside binaries in version-scoped or developer // locations such as ~/.nvm/versions/node//bin or a local ./dist build directory. func FindBinDir() (string, error) { @@ -77,9 +84,8 @@ func FindBinDir() (string, error) { pathValue := os.Getenv("PATH") if exe, err := executableFn(); err == nil { - exeDir := filepath.Dir(exe) - if matchesAllowlist(exeDir, candidates) && inPathValue(exeDir, pathValue) && isWritableDir(exeDir) { - return exeDir, nil + if dir, ok := findCoLocatedBinDir(exe, candidates, pathValue, homeDir); ok { + return dir, nil } } @@ -92,6 +98,40 @@ func FindBinDir() (string, error) { return filepath.Join(homeDir, homeSubDir, "bin"), nil } +// findCoLocatedBinDir returns the allowlist entry that holds the running executable, either +// directly or via a symlink, provided the entry is writable and on PATH. It handles the +// platform-specific behavior of os.Executable described on FindBinDir. +func findCoLocatedBinDir(exe string, allowlist []string, pathValue, homeDir string) (string, bool) { + exeDir := filepath.Dir(exe) + exeBase := filepath.Base(exe) + resolvedExe, resolveExeErr := filepath.EvalSymlinks(exe) + + for _, c := range allowlist { + if !inPathValue(c, pathValue) || !isWritableDir(c) { + continue + } + // Direct match: exe's directory equals this allowlist entry. Covers macOS Homebrew + // (where os.Executable preserves the bin-dir symlink path) and plain copies. + if normalizePathEntry(exeDir, homeDir) == normalizePathEntry(c, homeDir) { + return c, true + } + // Symlink-resolved match: / resolves to the same file as exe. Covers + // Linuxbrew, where os.Executable returns the resolved Cellar path but the allowlist + // entry points at the bin dir that holds the symlink. + if resolveExeErr != nil { + continue + } + resolvedCandidate, err := filepath.EvalSymlinks(filepath.Join(c, exeBase)) + if err != nil { + continue + } + if resolvedCandidate == resolvedExe { + return c, true + } + } + return "", false +} + // binDirAllowlist returns the per-OS list of acceptable bin directories, in priority order. // Empty entries (e.g. unset XDG_BIN_HOME) and duplicates are removed. func binDirAllowlist(homeDir string) []string { @@ -138,18 +178,6 @@ func binDirAllowlist(homeDir string) []string { return out } -// matchesAllowlist reports whether dir matches any allowlist entry after normalization. -func matchesAllowlist(dir string, allowlist []string) bool { - homeDir, _ := os.UserHomeDir() - target := normalizePathEntry(dir, homeDir) - for _, c := range allowlist { - if normalizePathEntry(c, homeDir) == target { - return true - } - } - return false -} - // isWritableDir reports whether path is an existing directory that the current process can write // to. Detection is best-effort: it tries to create and remove a temp file in the directory. func isWritableDir(path string) bool { diff --git a/internal/config/alt/fs_test.go b/internal/config/alt/fs_test.go index 6accaac03..c5fac4533 100644 --- a/internal/config/alt/fs_test.go +++ b/internal/config/alt/fs_test.go @@ -105,6 +105,37 @@ func TestFindBinDir(t *testing.T) { assert.Equal(t, sourceBin, result) }) + t.Run("linuxbrew-style symlinked source dir is co-located via allowlist", func(t *testing.T) { + // Reproduces the Linuxbrew layout. On Linux, os.Executable returns /proc/self/exe + // fully resolved, so the running process sees the Cellar path — not the bin dir + // holding the symlink. We must still detect that the alt should be installed in the + // bin dir alongside the symlink. + tempDir := t.TempDir() + brewBin := filepath.Join(tempDir, ".linuxbrew", "bin") + cellarBin := filepath.Join(tempDir, ".linuxbrew", "Cellar", "upsun", "1.0.0", "bin") + require.NoError(t, os.MkdirAll(brewBin, 0o755)) + require.NoError(t, os.MkdirAll(cellarBin, 0o755)) + cellarExe := filepath.Join(cellarBin, "exe") + require.NoError(t, os.WriteFile(cellarExe, []byte("#!/bin/sh\n"), 0o755)) + require.NoError(t, os.Symlink(cellarExe, filepath.Join(brewBin, "exe"))) + + // PATH contains both a higher-priority allowlist entry and the brew bin dir; the + // co-location rule must win over the higher-priority writable entry. + higherPriorityBin := filepath.Join(tempDir, ".local", "bin") + require.NoError(t, os.MkdirAll(higherPriorityBin, 0o755)) + require.NoError(t, os.Setenv("HOME", tempDir)) + require.NoError(t, os.Setenv("PATH", higherPriorityBin+string(os.PathListSeparator)+brewBin)) + // Point XDG_BIN_HOME at the brew bin so it appears on the allowlist regardless of + // the test host's GOOS (the hardcoded /home/linuxbrew/.linuxbrew/bin entry only + // matches on a real Linuxbrew install). + require.NoError(t, os.Setenv("XDG_BIN_HOME", brewBin)) + executableFn = func() (string, error) { return cellarExe, nil } + + result, err := FindBinDir() + assert.NoError(t, err) + assert.Equal(t, brewBin, result) + }) + t.Run("nvm-style source dir is not selected", func(t *testing.T) { tempDir := t.TempDir() nvmBin := filepath.Join(tempDir, ".nvm", "versions", "node", "v20.0.0", "bin") From 981da8763f0a8efd2d3fbadf222c5f5c213dfa7b Mon Sep 17 00:00:00 2001 From: Miguel Sanchez Gonzalez Date: Mon, 27 Apr 2026 19:48:35 +0200 Subject: [PATCH 3/9] refactor(config:install): collapse bin-dir candidate scan into one pass Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/config/alt/fs.go | 92 +++++++++++--------------------- internal/config/alt/fs_test.go | 97 +++++++++++++--------------------- 2 files changed, 68 insertions(+), 121 deletions(-) diff --git a/internal/config/alt/fs.go b/internal/config/alt/fs.go index 7d337203a..b8e94612b 100644 --- a/internal/config/alt/fs.go +++ b/internal/config/alt/fs.go @@ -14,15 +14,12 @@ const ( homeSubDir = ".platform-alt" ) -// executableFn is the source of the current executable path. It is a variable so tests can stub -// out os.Executable. +// var so tests can stub os.Executable. var executableFn = os.Executable // FindConfigDir finds an appropriate destination directory for an "alt" CLI configuration YAML file. // -// XDG_CONFIG_HOME takes precedence on all platforms, since os.UserConfigDir does not honor it on -// macOS or Windows. If neither XDG_CONFIG_HOME nor os.UserConfigDir yields an existing directory, -// it falls back to ~/.platform-alt. +// XDG_CONFIG_HOME is honored explicitly because os.UserConfigDir ignores it on macOS and Windows. func FindConfigDir() (string, error) { if xdg := os.Getenv("XDG_CONFIG_HOME"); xdg != "" { isDir, err := isExistingDirectory(xdg) @@ -54,26 +51,13 @@ func FindConfigDir() (string, error) { return filepath.Join(homeDir, homeSubDir), nil } -// FindBinDir finds an appropriate destination directory for an "alt" CLI executable. +// FindBinDir picks a bin directory from a per-OS allowlist. It prefers the entry that already +// holds the running executable (so the alt installs alongside its source binary in +// package-manager layouts), falling back to the first writable PATH entry, then ~/.platform-alt/bin. // -// The selection rules are: -// -// 1. If the currently running CLI executable lives in (or is reachable via a symlink from) a -// directory on the per-OS allowlist, and that directory is writable and on PATH, install -// the alt there. This co-locates the alt with the source binary in package-manager installs -// (e.g. Homebrew, Linuxbrew, Scoop). -// 2. Otherwise, pick the first allowlist entry that is writable and on PATH. -// 3. Otherwise, fall back to ~/.platform-alt/bin (the caller is expected to print PATH -// instructions in that case). -// -// The symlink reachability check matters because os.Executable behaves differently per OS: on -// macOS and Windows it returns the path used to invoke the binary (preserving symlinks), while -// on Linux it returns /proc/self/exe fully resolved. Linuxbrew installs a symlink in -// /home/linuxbrew/.linuxbrew/bin pointing into a versioned Cellar directory, so the resolved -// exe path doesn't match the allowlist directly — we have to compare resolved targets instead. -// -// The allowlist exists to avoid installing alongside binaries in version-scoped or developer -// locations such as ~/.nvm/versions/node//bin or a local ./dist build directory. +// The symlink-resolved match exists for Linuxbrew: on Linux, os.Executable returns the resolved +// Cellar path rather than the bin-dir symlink that PATH points at, so a string-compare against +// the allowlist entry would miss. func FindBinDir() (string, error) { homeDir, err := os.UserHomeDir() if err != nil { @@ -83,57 +67,43 @@ func FindBinDir() (string, error) { candidates := binDirAllowlist(homeDir) pathValue := os.Getenv("PATH") - if exe, err := executableFn(); err == nil { - if dir, ok := findCoLocatedBinDir(exe, candidates, pathValue, homeDir); ok { - return dir, nil - } + exe, exeErr := executableFn() + var normExeDir, exeBase, resolvedExe string + var resolveExeErr error + if exeErr == nil { + normExeDir = normalizePathEntry(filepath.Dir(exe), homeDir) + exeBase = filepath.Base(exe) + resolvedExe, resolveExeErr = filepath.EvalSymlinks(exe) } + var firstValid string for _, c := range candidates { - if inPathValue(c, pathValue) && isWritableDir(c) { - return c, nil - } - } - - return filepath.Join(homeDir, homeSubDir, "bin"), nil -} - -// findCoLocatedBinDir returns the allowlist entry that holds the running executable, either -// directly or via a symlink, provided the entry is writable and on PATH. It handles the -// platform-specific behavior of os.Executable described on FindBinDir. -func findCoLocatedBinDir(exe string, allowlist []string, pathValue, homeDir string) (string, bool) { - exeDir := filepath.Dir(exe) - exeBase := filepath.Base(exe) - resolvedExe, resolveExeErr := filepath.EvalSymlinks(exe) - - for _, c := range allowlist { if !inPathValue(c, pathValue) || !isWritableDir(c) { continue } - // Direct match: exe's directory equals this allowlist entry. Covers macOS Homebrew - // (where os.Executable preserves the bin-dir symlink path) and plain copies. - if normalizePathEntry(exeDir, homeDir) == normalizePathEntry(c, homeDir) { - return c, true + if firstValid == "" { + firstValid = c } - // Symlink-resolved match: / resolves to the same file as exe. Covers - // Linuxbrew, where os.Executable returns the resolved Cellar path but the allowlist - // entry points at the bin dir that holds the symlink. - if resolveExeErr != nil { + if exeErr != nil { continue } - resolvedCandidate, err := filepath.EvalSymlinks(filepath.Join(c, exeBase)) - if err != nil { + if normalizePathEntry(c, homeDir) == normExeDir { + return c, nil + } + if resolveExeErr != nil { continue } - if resolvedCandidate == resolvedExe { - return c, true + if resolved, err := filepath.EvalSymlinks(filepath.Join(c, exeBase)); err == nil && resolved == resolvedExe { + return c, nil } } - return "", false + + if firstValid != "" { + return firstValid, nil + } + return filepath.Join(homeDir, homeSubDir, "bin"), nil } -// binDirAllowlist returns the per-OS list of acceptable bin directories, in priority order. -// Empty entries (e.g. unset XDG_BIN_HOME) and duplicates are removed. func binDirAllowlist(homeDir string) []string { xdgBinHome := os.Getenv("XDG_BIN_HOME") @@ -178,8 +148,6 @@ func binDirAllowlist(homeDir string) []string { return out } -// isWritableDir reports whether path is an existing directory that the current process can write -// to. Detection is best-effort: it tries to create and remove a temp file in the directory. func isWritableDir(path string) bool { stat, err := os.Stat(path) if err != nil || !stat.IsDir() { diff --git a/internal/config/alt/fs_test.go b/internal/config/alt/fs_test.go index c5fac4533..23c5b4078 100644 --- a/internal/config/alt/fs_test.go +++ b/internal/config/alt/fs_test.go @@ -17,8 +17,7 @@ func TestFindConfigDir(t *testing.T) { if runtime.GOOS == "plan9" { t.Skip() } - require.NoError(t, os.Setenv("XDG_CONFIG_HOME", tempDir)) - t.Cleanup(func() { _ = os.Unsetenv("XDG_CONFIG_HOME") }) + t.Setenv("XDG_CONFIG_HOME", tempDir) result, err := FindConfigDir() assert.NoError(t, err) @@ -26,14 +25,13 @@ func TestFindConfigDir(t *testing.T) { }) t.Run("HOME fallback", func(t *testing.T) { - _ = os.Unsetenv("XDG_CONFIG_HOME") - require.NoError(t, os.Setenv("HOME", tempDir)) - t.Cleanup(func() { _ = os.Unsetenv("HOME") }) + t.Setenv("XDG_CONFIG_HOME", "") + t.Setenv("HOME", tempDir) result, err := FindConfigDir() assert.NoError(t, err) // On platforms where os.UserConfigDir resolves to an existing directory under the test - // HOME, that directory wins. Only assert the home fallback when it doesn't. + // HOME, that directory wins. ucd, ucdErr := os.UserConfigDir() isDir, _ := isExistingDirectory(ucd) if ucdErr != nil || !isDir { @@ -47,23 +45,17 @@ func TestFindBinDir(t *testing.T) { t.Skip("path setup in this test is unix-style") } - originalPath := os.Getenv("PATH") - originalHome := os.Getenv("HOME") - originalXDGBin := os.Getenv("XDG_BIN_HOME") originalExecFn := executableFn - t.Cleanup(func() { - _ = os.Setenv("PATH", originalPath) - _ = os.Setenv("HOME", originalHome) - _ = os.Setenv("XDG_BIN_HOME", originalXDGBin) - executableFn = originalExecFn - }) + t.Cleanup(func() { executableFn = originalExecFn }) + + setExe := func(p string) { executableFn = func() (string, error) { return p, nil } } t.Run("home fallback when nothing on PATH", func(t *testing.T) { tempDir := t.TempDir() - require.NoError(t, os.Setenv("HOME", tempDir)) - require.NoError(t, os.Setenv("PATH", "/nonexistent/dir")) - _ = os.Unsetenv("XDG_BIN_HOME") - executableFn = func() (string, error) { return filepath.Join(tempDir, "exe"), nil } + t.Setenv("HOME", tempDir) + t.Setenv("PATH", "/nonexistent/dir") + t.Setenv("XDG_BIN_HOME", "") + setExe(filepath.Join(tempDir, "exe")) result, err := FindBinDir() assert.NoError(t, err) @@ -74,13 +66,10 @@ func TestFindBinDir(t *testing.T) { tempDir := t.TempDir() localBin := filepath.Join(tempDir, ".local", "bin") require.NoError(t, os.MkdirAll(localBin, 0o755)) - require.NoError(t, os.Setenv("HOME", tempDir)) - require.NoError(t, os.Setenv("PATH", localBin)) - _ = os.Unsetenv("XDG_BIN_HOME") - // Executable lives outside the allowlist; should not be selected. - executableFn = func() (string, error) { - return filepath.Join(tempDir, "build", "exe"), nil - } + t.Setenv("HOME", tempDir) + t.Setenv("PATH", localBin) + t.Setenv("XDG_BIN_HOME", "") + setExe(filepath.Join(tempDir, "build", "exe")) result, err := FindBinDir() assert.NoError(t, err) @@ -93,12 +82,10 @@ func TestFindBinDir(t *testing.T) { higherPriorityBin := filepath.Join(tempDir, ".local", "bin") require.NoError(t, os.MkdirAll(sourceBin, 0o755)) require.NoError(t, os.MkdirAll(higherPriorityBin, 0o755)) - require.NoError(t, os.Setenv("HOME", tempDir)) - require.NoError(t, os.Setenv("PATH", higherPriorityBin+string(os.PathListSeparator)+sourceBin)) - _ = os.Unsetenv("XDG_BIN_HOME") - // Executable lives in the lower-priority allowlist entry; we must pick that one over the - // higher-priority one. - executableFn = func() (string, error) { return filepath.Join(sourceBin, "exe"), nil } + t.Setenv("HOME", tempDir) + t.Setenv("PATH", higherPriorityBin+string(os.PathListSeparator)+sourceBin) + t.Setenv("XDG_BIN_HOME", "") + setExe(filepath.Join(sourceBin, "exe")) result, err := FindBinDir() assert.NoError(t, err) @@ -106,30 +93,23 @@ func TestFindBinDir(t *testing.T) { }) t.Run("linuxbrew-style symlinked source dir is co-located via allowlist", func(t *testing.T) { - // Reproduces the Linuxbrew layout. On Linux, os.Executable returns /proc/self/exe - // fully resolved, so the running process sees the Cellar path — not the bin dir - // holding the symlink. We must still detect that the alt should be installed in the - // bin dir alongside the symlink. tempDir := t.TempDir() brewBin := filepath.Join(tempDir, ".linuxbrew", "bin") cellarBin := filepath.Join(tempDir, ".linuxbrew", "Cellar", "upsun", "1.0.0", "bin") require.NoError(t, os.MkdirAll(brewBin, 0o755)) require.NoError(t, os.MkdirAll(cellarBin, 0o755)) cellarExe := filepath.Join(cellarBin, "exe") - require.NoError(t, os.WriteFile(cellarExe, []byte("#!/bin/sh\n"), 0o755)) + require.NoError(t, os.WriteFile(cellarExe, []byte("#!/bin/sh\n"), 0o600)) require.NoError(t, os.Symlink(cellarExe, filepath.Join(brewBin, "exe"))) - // PATH contains both a higher-priority allowlist entry and the brew bin dir; the - // co-location rule must win over the higher-priority writable entry. higherPriorityBin := filepath.Join(tempDir, ".local", "bin") require.NoError(t, os.MkdirAll(higherPriorityBin, 0o755)) - require.NoError(t, os.Setenv("HOME", tempDir)) - require.NoError(t, os.Setenv("PATH", higherPriorityBin+string(os.PathListSeparator)+brewBin)) - // Point XDG_BIN_HOME at the brew bin so it appears on the allowlist regardless of - // the test host's GOOS (the hardcoded /home/linuxbrew/.linuxbrew/bin entry only - // matches on a real Linuxbrew install). - require.NoError(t, os.Setenv("XDG_BIN_HOME", brewBin)) - executableFn = func() (string, error) { return cellarExe, nil } + t.Setenv("HOME", tempDir) + t.Setenv("PATH", higherPriorityBin+string(os.PathListSeparator)+brewBin) + // XDG_BIN_HOME pulls brewBin onto the allowlist regardless of host GOOS — the hardcoded + // /home/linuxbrew/.linuxbrew/bin entry only matches on a real Linuxbrew install. + t.Setenv("XDG_BIN_HOME", brewBin) + setExe(cellarExe) result, err := FindBinDir() assert.NoError(t, err) @@ -142,14 +122,13 @@ func TestFindBinDir(t *testing.T) { localBin := filepath.Join(tempDir, ".local", "bin") require.NoError(t, os.MkdirAll(nvmBin, 0o755)) require.NoError(t, os.MkdirAll(localBin, 0o755)) - require.NoError(t, os.Setenv("HOME", tempDir)) - require.NoError(t, os.Setenv("PATH", nvmBin+string(os.PathListSeparator)+localBin)) - _ = os.Unsetenv("XDG_BIN_HOME") - executableFn = func() (string, error) { return filepath.Join(nvmBin, "upsun"), nil } + t.Setenv("HOME", tempDir) + t.Setenv("PATH", nvmBin+string(os.PathListSeparator)+localBin) + t.Setenv("XDG_BIN_HOME", "") + setExe(filepath.Join(nvmBin, "upsun")) result, err := FindBinDir() assert.NoError(t, err) - // Must skip the nvm dir (not on allowlist) and pick the allowlist entry. assert.Equal(t, localBin, result) }) @@ -157,10 +136,10 @@ func TestFindBinDir(t *testing.T) { tempDir := t.TempDir() xdgBin := filepath.Join(tempDir, "xdg-bin") require.NoError(t, os.MkdirAll(xdgBin, 0o755)) - require.NoError(t, os.Setenv("HOME", tempDir)) - require.NoError(t, os.Setenv("XDG_BIN_HOME", xdgBin)) - require.NoError(t, os.Setenv("PATH", xdgBin)) - executableFn = func() (string, error) { return filepath.Join(tempDir, "exe"), nil } + t.Setenv("HOME", tempDir) + t.Setenv("XDG_BIN_HOME", xdgBin) + t.Setenv("PATH", xdgBin) + setExe(filepath.Join(tempDir, "exe")) result, err := FindBinDir() assert.NoError(t, err) @@ -179,10 +158,10 @@ func TestFindBinDir(t *testing.T) { require.NoError(t, os.Chmod(readOnlyBin, 0o555)) t.Cleanup(func() { _ = os.Chmod(readOnlyBin, 0o755) }) - require.NoError(t, os.Setenv("HOME", tempDir)) - require.NoError(t, os.Setenv("PATH", readOnlyBin+string(os.PathListSeparator)+writableBin)) - _ = os.Unsetenv("XDG_BIN_HOME") - executableFn = func() (string, error) { return filepath.Join(tempDir, "exe"), nil } + t.Setenv("HOME", tempDir) + t.Setenv("PATH", readOnlyBin+string(os.PathListSeparator)+writableBin) + t.Setenv("XDG_BIN_HOME", "") + setExe(filepath.Join(tempDir, "exe")) result, err := FindBinDir() assert.NoError(t, err) From e659507e4eca0a73f30ec30744d443666cc4a431 Mon Sep 17 00:00:00 2001 From: Miguel Sanchez Gonzalez Date: Mon, 27 Apr 2026 20:13:46 +0200 Subject: [PATCH 4/9] feat(config:install): honor XDG_BIN_HOME on Windows --- internal/config/alt/fs.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/config/alt/fs.go b/internal/config/alt/fs.go index b8e94612b..d935dcdce 100644 --- a/internal/config/alt/fs.go +++ b/internal/config/alt/fs.go @@ -121,6 +121,7 @@ func binDirAllowlist(homeDir string) []string { raw = []string{ filepath.Join(homeDir, "scoop", "shims"), filepath.Join(homeDir, "AppData", "Local", "Programs"), + xdgBinHome, filepath.Join(homeDir, ".local", "bin"), } default: From 55a52302dd3d89f48a517ea70e039438630c6605 Mon Sep 17 00:00:00 2001 From: Miguel Sanchez Gonzalez Date: Mon, 27 Apr 2026 20:15:35 +0200 Subject: [PATCH 5/9] fix(config:install): honor explicit XDG_CONFIG_HOME regardless of existence --- internal/config/alt/fs.go | 17 +++++++---------- internal/config/alt/fs_test.go | 18 ++++++++++++++++-- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/internal/config/alt/fs.go b/internal/config/alt/fs.go index d935dcdce..8a5591b8a 100644 --- a/internal/config/alt/fs.go +++ b/internal/config/alt/fs.go @@ -20,15 +20,11 @@ var executableFn = os.Executable // FindConfigDir finds an appropriate destination directory for an "alt" CLI configuration YAML file. // // XDG_CONFIG_HOME is honored explicitly because os.UserConfigDir ignores it on macOS and Windows. +// Per the XDG Base Directory spec, an explicitly set value is honored regardless of whether the +// directory already exists — writeFile creates parents as needed. func FindConfigDir() (string, error) { if xdg := os.Getenv("XDG_CONFIG_HOME"); xdg != "" { - isDir, err := isExistingDirectory(xdg) - if err != nil { - return "", err - } - if isDir { - return filepath.Join(xdg, subDir), nil - } + return filepath.Join(xdg, subDir), nil } userConfigDir, err := os.UserConfigDir() @@ -51,9 +47,10 @@ func FindConfigDir() (string, error) { return filepath.Join(homeDir, homeSubDir), nil } -// FindBinDir picks a bin directory from a per-OS allowlist. It prefers the entry that already -// holds the running executable (so the alt installs alongside its source binary in -// package-manager layouts), falling back to the first writable PATH entry, then ~/.platform-alt/bin. +// FindBinDir picks a bin directory from a per-OS allowlist. It prefers the allowlist entry that +// already holds the running executable (so the alt installs alongside its source binary in +// package-manager layouts), falling back to the first allowlist entry that is on PATH and +// writable, then ~/.platform-alt/bin. // // The symlink-resolved match exists for Linuxbrew: on Linux, os.Executable returns the resolved // Cellar path rather than the bin-dir symlink that PATH points at, so a string-compare against diff --git a/internal/config/alt/fs_test.go b/internal/config/alt/fs_test.go index 23c5b4078..26654e52f 100644 --- a/internal/config/alt/fs_test.go +++ b/internal/config/alt/fs_test.go @@ -13,7 +13,7 @@ import ( func TestFindConfigDir(t *testing.T) { tempDir := t.TempDir() - t.Run("XDG_CONFIG_HOME exists", func(t *testing.T) { + t.Run("XDG_CONFIG_HOME set", func(t *testing.T) { if runtime.GOOS == "plan9" { t.Skip() } @@ -24,6 +24,18 @@ func TestFindConfigDir(t *testing.T) { assert.Equal(t, filepath.Join(tempDir, subDir), result) }) + t.Run("XDG_CONFIG_HOME honored even when target does not yet exist", func(t *testing.T) { + if runtime.GOOS == "plan9" { + t.Skip() + } + nonExistent := filepath.Join(tempDir, "does-not-exist-yet") + t.Setenv("XDG_CONFIG_HOME", nonExistent) + + result, err := FindConfigDir() + assert.NoError(t, err) + assert.Equal(t, filepath.Join(nonExistent, subDir), result) + }) + t.Run("HOME fallback", func(t *testing.T) { t.Setenv("XDG_CONFIG_HOME", "") t.Setenv("HOME", tempDir) @@ -34,7 +46,9 @@ func TestFindConfigDir(t *testing.T) { // HOME, that directory wins. ucd, ucdErr := os.UserConfigDir() isDir, _ := isExistingDirectory(ucd) - if ucdErr != nil || !isDir { + if ucdErr == nil && isDir { + assert.Equal(t, filepath.Join(ucd, subDir), result) + } else { assert.Equal(t, filepath.Join(tempDir, homeSubDir), result) } }) From 3ea3489a381b57dd2ff79f9df898dd0dbdc30630 Mon Sep 17 00:00:00 2001 From: Miguel Sanchez Gonzalez Date: Mon, 27 Apr 2026 20:26:58 +0200 Subject: [PATCH 6/9] fix(config:install): fail writability check when temp file cleanup fails --- internal/config/alt/fs.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/config/alt/fs.go b/internal/config/alt/fs.go index 8a5591b8a..714c094b3 100644 --- a/internal/config/alt/fs.go +++ b/internal/config/alt/fs.go @@ -156,8 +156,13 @@ func isWritableDir(path string) bool { return false } name := f.Name() - _ = f.Close() - _ = os.Remove(name) + if err := f.Close(); err != nil { + _ = os.Remove(name) + return false + } + if err := os.Remove(name); err != nil { + return false + } return true } From 8b2797f5da16cf7a01436bdbeac259e9769528de Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Mon, 27 Apr 2026 22:01:31 +0100 Subject: [PATCH 7/9] refactor(config:install): extract co-location check from FindBinDir loop Move the running-executable match into an exeMatcher closure so the loop in FindBinDir reads as one straightforward pass: skip if not on PATH or not writable, return on co-location, otherwise track first-valid for the fallback. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/config/alt/fs.go | 54 +++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/internal/config/alt/fs.go b/internal/config/alt/fs.go index 714c094b3..f42ec2299 100644 --- a/internal/config/alt/fs.go +++ b/internal/config/alt/fs.go @@ -51,10 +51,6 @@ func FindConfigDir() (string, error) { // already holds the running executable (so the alt installs alongside its source binary in // package-manager layouts), falling back to the first allowlist entry that is on PATH and // writable, then ~/.platform-alt/bin. -// -// The symlink-resolved match exists for Linuxbrew: on Linux, os.Executable returns the resolved -// Cellar path rather than the bin-dir symlink that PATH points at, so a string-compare against -// the allowlist entry would miss. func FindBinDir() (string, error) { homeDir, err := os.UserHomeDir() if err != nil { @@ -63,35 +59,18 @@ func FindBinDir() (string, error) { candidates := binDirAllowlist(homeDir) pathValue := os.Getenv("PATH") - - exe, exeErr := executableFn() - var normExeDir, exeBase, resolvedExe string - var resolveExeErr error - if exeErr == nil { - normExeDir = normalizePathEntry(filepath.Dir(exe), homeDir) - exeBase = filepath.Base(exe) - resolvedExe, resolveExeErr = filepath.EvalSymlinks(exe) - } + matchExe := exeMatcher(homeDir) var firstValid string for _, c := range candidates { if !inPathValue(c, pathValue) || !isWritableDir(c) { continue } - if firstValid == "" { - firstValid = c - } - if exeErr != nil { - continue - } - if normalizePathEntry(c, homeDir) == normExeDir { + if matchExe(c) { return c, nil } - if resolveExeErr != nil { - continue - } - if resolved, err := filepath.EvalSymlinks(filepath.Join(c, exeBase)); err == nil && resolved == resolvedExe { - return c, nil + if firstValid == "" { + firstValid = c } } @@ -101,6 +80,31 @@ func FindBinDir() (string, error) { return filepath.Join(homeDir, homeSubDir, "bin"), nil } +// exeMatcher returns a predicate that reports whether a candidate bin directory holds the +// running executable. The symlink branch handles package-manager layouts like Linuxbrew, where +// os.Executable returns the resolved Cellar path rather than the bin-dir symlink that PATH +// points at — a string compare against the allowlist entry would otherwise miss. +func exeMatcher(homeDir string) func(string) bool { + exe, err := executableFn() + if err != nil { + return func(string) bool { return false } + } + normExeDir := normalizePathEntry(filepath.Dir(exe), homeDir) + exeBase := filepath.Base(exe) + resolvedExe, resolveErr := filepath.EvalSymlinks(exe) + + return func(c string) bool { + if normalizePathEntry(c, homeDir) == normExeDir { + return true + } + if resolveErr != nil { + return false + } + resolved, err := filepath.EvalSymlinks(filepath.Join(c, exeBase)) + return err == nil && resolved == resolvedExe + } +} + func binDirAllowlist(homeDir string) []string { xdgBinHome := os.Getenv("XDG_BIN_HOME") From 92ed9718dfd124e20a4c5c02f9e06afa83054374 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Mon, 27 Apr 2026 22:02:36 +0100 Subject: [PATCH 8/9] refactor(config:install): use access(2) for bin-dir writability check Replace the temp-file probe in isWritableDir with golang.org/x/sys/unix Access on Unix, falling back to the probe file only on Windows where no equivalent syscall is available. The temp file was created and deleted on every config:install invocation across each allowlist entry; the permission-bit check has the same semantics without the side effect. Co-Authored-By: Claude Opus 4.7 (1M context) --- go.mod | 2 +- internal/config/alt/fs.go | 20 -------------------- internal/config/alt/fs_unix.go | 19 +++++++++++++++++++ internal/config/alt/fs_windows.go | 25 +++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 21 deletions(-) create mode 100644 internal/config/alt/fs_unix.go create mode 100644 internal/config/alt/fs_windows.go diff --git a/go.mod b/go.mod index 6c5c820ff..5ee5123af 100644 --- a/go.mod +++ b/go.mod @@ -23,6 +23,7 @@ require ( golang.org/x/crypto v0.42.0 golang.org/x/oauth2 v0.31.0 golang.org/x/sync v0.17.0 + golang.org/x/sys v0.36.0 golang.org/x/term v0.35.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -128,7 +129,6 @@ require ( golang.org/x/exp v0.0.0-20250819193227-8b4c13bb791b // indirect golang.org/x/mod v0.27.0 // indirect golang.org/x/net v0.43.0 // indirect - golang.org/x/sys v0.36.0 // indirect golang.org/x/text v0.29.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250818200422-3122310a409c // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20250818200422-3122310a409c // indirect diff --git a/internal/config/alt/fs.go b/internal/config/alt/fs.go index f42ec2299..afe3f7615 100644 --- a/internal/config/alt/fs.go +++ b/internal/config/alt/fs.go @@ -150,26 +150,6 @@ func binDirAllowlist(homeDir string) []string { return out } -func isWritableDir(path string) bool { - stat, err := os.Stat(path) - if err != nil || !stat.IsDir() { - return false - } - f, err := os.CreateTemp(path, ".platform-alt-write-check-*") - if err != nil { - return false - } - name := f.Name() - if err := f.Close(); err != nil { - _ = os.Remove(name) - return false - } - if err := os.Remove(name); err != nil { - return false - } - return true -} - // isExistingDirectory checks if a path exists and is a directory. func isExistingDirectory(path string) (bool, error) { stat, err := os.Stat(path) diff --git a/internal/config/alt/fs_unix.go b/internal/config/alt/fs_unix.go new file mode 100644 index 000000000..f1e4a4e1f --- /dev/null +++ b/internal/config/alt/fs_unix.go @@ -0,0 +1,19 @@ +//go:build !windows + +package alt + +import ( + "os" + + "golang.org/x/sys/unix" +) + +// isWritableDir reports whether path is a directory the current user can write to. The check is +// permission-based (no temp file is created), so it is safe to call repeatedly. +func isWritableDir(path string) bool { + stat, err := os.Stat(path) + if err != nil || !stat.IsDir() { + return false + } + return unix.Access(path, unix.W_OK) == nil +} diff --git a/internal/config/alt/fs_windows.go b/internal/config/alt/fs_windows.go new file mode 100644 index 000000000..d4cbf02ae --- /dev/null +++ b/internal/config/alt/fs_windows.go @@ -0,0 +1,25 @@ +//go:build windows + +package alt + +import "os" + +// isWritableDir reports whether path is a directory the current user can write to. Windows +// has no equivalent of access(2), so the check is performed by creating and removing a probe +// file. +func isWritableDir(path string) bool { + stat, err := os.Stat(path) + if err != nil || !stat.IsDir() { + return false + } + f, err := os.CreateTemp(path, ".platform-alt-write-check-*") + if err != nil { + return false + } + name := f.Name() + if cerr := f.Close(); cerr != nil { + _ = os.Remove(name) + return false + } + return os.Remove(name) == nil +} From 5b5349bc307e233996964edfcb5d5a1ca8fc8356 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Mon, 27 Apr 2026 22:03:39 +0100 Subject: [PATCH 9/9] feat(config:install): drop Homebrew bin dirs from the allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove /opt/homebrew/bin (macOS) and /home/linuxbrew/.linuxbrew/bin (Linux) from the bin-directory allowlist. Those directories belong to Homebrew and should not be written to by config:install — when the running CLI was itself installed via Homebrew, the alt now lands in /usr/local/bin or ~/.local/bin instead. The symlink-resolution branch in exeMatcher is kept: a Cellar-style layout is still reachable when the user opts in via XDG_BIN_HOME, and the existing test exercises that path. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/config/alt/fs.go | 12 +++++++----- internal/config/alt/fs_test.go | 29 +++++++++++++++-------------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/internal/config/alt/fs.go b/internal/config/alt/fs.go index afe3f7615..7ed448bc6 100644 --- a/internal/config/alt/fs.go +++ b/internal/config/alt/fs.go @@ -81,9 +81,10 @@ func FindBinDir() (string, error) { } // exeMatcher returns a predicate that reports whether a candidate bin directory holds the -// running executable. The symlink branch handles package-manager layouts like Linuxbrew, where -// os.Executable returns the resolved Cellar path rather than the bin-dir symlink that PATH -// points at — a string compare against the allowlist entry would otherwise miss. +// running executable. The symlink branch handles bin directories injected via XDG_BIN_HOME +// that contain a symlink into a separate install location (e.g. a Cellar-style layout): the +// candidate's path doesn't match os.Executable directly, but the symlinked entry resolves to +// the same file. func exeMatcher(homeDir string) func(string) bool { exe, err := executableFn() if err != nil { @@ -108,11 +109,13 @@ func exeMatcher(homeDir string) func(string) bool { func binDirAllowlist(homeDir string) []string { xdgBinHome := os.Getenv("XDG_BIN_HOME") + // Homebrew bin directories (/opt/homebrew/bin on macOS, /home/linuxbrew/.linuxbrew/bin on + // Linux) are deliberately omitted: those directories are managed by Homebrew, and we should + // not deposit binaries there. var raw []string switch runtime.GOOS { case "darwin": raw = []string{ - "/opt/homebrew/bin", "/usr/local/bin", xdgBinHome, filepath.Join(homeDir, ".local", "bin"), @@ -127,7 +130,6 @@ func binDirAllowlist(homeDir string) []string { } default: raw = []string{ - "/home/linuxbrew/.linuxbrew/bin", xdgBinHome, filepath.Join(homeDir, ".local", "bin"), filepath.Join(homeDir, "bin"), diff --git a/internal/config/alt/fs_test.go b/internal/config/alt/fs_test.go index 26654e52f..11642dc8b 100644 --- a/internal/config/alt/fs_test.go +++ b/internal/config/alt/fs_test.go @@ -106,28 +106,29 @@ func TestFindBinDir(t *testing.T) { assert.Equal(t, sourceBin, result) }) - t.Run("linuxbrew-style symlinked source dir is co-located via allowlist", func(t *testing.T) { + t.Run("symlinked source dir is co-located via allowlist", func(t *testing.T) { tempDir := t.TempDir() - brewBin := filepath.Join(tempDir, ".linuxbrew", "bin") - cellarBin := filepath.Join(tempDir, ".linuxbrew", "Cellar", "upsun", "1.0.0", "bin") - require.NoError(t, os.MkdirAll(brewBin, 0o755)) - require.NoError(t, os.MkdirAll(cellarBin, 0o755)) - cellarExe := filepath.Join(cellarBin, "exe") - require.NoError(t, os.WriteFile(cellarExe, []byte("#!/bin/sh\n"), 0o600)) - require.NoError(t, os.Symlink(cellarExe, filepath.Join(brewBin, "exe"))) + shimBin := filepath.Join(tempDir, "shim", "bin") + realBin := filepath.Join(tempDir, "real", "bin") + require.NoError(t, os.MkdirAll(shimBin, 0o755)) + require.NoError(t, os.MkdirAll(realBin, 0o755)) + realExe := filepath.Join(realBin, "exe") + require.NoError(t, os.WriteFile(realExe, []byte("#!/bin/sh\n"), 0o600)) + require.NoError(t, os.Symlink(realExe, filepath.Join(shimBin, "exe"))) higherPriorityBin := filepath.Join(tempDir, ".local", "bin") require.NoError(t, os.MkdirAll(higherPriorityBin, 0o755)) t.Setenv("HOME", tempDir) - t.Setenv("PATH", higherPriorityBin+string(os.PathListSeparator)+brewBin) - // XDG_BIN_HOME pulls brewBin onto the allowlist regardless of host GOOS — the hardcoded - // /home/linuxbrew/.linuxbrew/bin entry only matches on a real Linuxbrew install. - t.Setenv("XDG_BIN_HOME", brewBin) - setExe(cellarExe) + t.Setenv("PATH", higherPriorityBin+string(os.PathListSeparator)+shimBin) + // XDG_BIN_HOME pulls shimBin onto the allowlist; the running exe lives outside the + // allowlist but is reachable via a symlink from shimBin, so co-location should still + // pick shimBin. + t.Setenv("XDG_BIN_HOME", shimBin) + setExe(realExe) result, err := FindBinDir() assert.NoError(t, err) - assert.Equal(t, brewBin, result) + assert.Equal(t, shimBin, result) }) t.Run("nvm-style source dir is not selected", func(t *testing.T) {