Skip to content

Commit d736fb1

Browse files
CalvinAllenCalvinAllen
committed
fix(runtime): filter shims directory from PATH in DetectInstalled
The DetectInstalled() functions in all runtime providers were using raw exec.LookPath() which could find dtvem's own shims instead of actual system installations when the shims directory is in PATH. This caused the migrate command to potentially detect our own shims as "system" installations, leading to confusing migration suggestions. Changes: - Add LookPathExcludingShims() to internal/path package - Update Node.js, Python, and Ruby providers to use filtered lookup - Update shim executable to use shared function (deduplication) - Add comprehensive unit tests for the new lookup function Fixes #125
1 parent 699a504 commit d736fb1

6 files changed

Lines changed: 256 additions & 58 deletions

File tree

‎src/cmd/shim/main.go‎

Lines changed: 2 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212

1313
"github.com/dtvem/dtvem/src/internal/config"
1414
"github.com/dtvem/dtvem/src/internal/constants"
15+
"github.com/dtvem/dtvem/src/internal/path"
1516
"github.com/dtvem/dtvem/src/internal/runtime"
1617
"github.com/dtvem/dtvem/src/internal/shim"
1718
"github.com/dtvem/dtvem/src/internal/ui"
@@ -111,7 +112,7 @@ func runShim() error {
111112
// It attempts to fallback to system PATH or prompts for installation
112113
func handleNoConfiguredVersion(shimName, runtimeName string, provider runtime.ShimProvider) error {
113114
// Try to find the executable deeper in PATH (system installation)
114-
systemPath := findInSystemPath(shimName)
115+
systemPath := path.LookPathExcludingShims(shimName)
115116

116117
if systemPath != "" {
117118
// Found system installation - use it
@@ -143,57 +144,6 @@ func handleNoConfiguredVersion(shimName, runtimeName string, provider runtime.Sh
143144
return fmt.Errorf("no version configured")
144145
}
145146

146-
// findInSystemPath searches for an executable in PATH, excluding dtvem's shims directory
147-
func findInSystemPath(execName string) string {
148-
// Get the shims directory to exclude it from search
149-
shimsDir := config.DefaultPaths().Shims
150-
151-
// Get PATH environment variable
152-
pathEnv := os.Getenv("PATH")
153-
if pathEnv == "" {
154-
return ""
155-
}
156-
157-
// Split PATH into directories
158-
pathDirs := filepath.SplitList(pathEnv)
159-
160-
// Search each directory
161-
for _, dir := range pathDirs {
162-
// Skip the dtvem shims directory
163-
if strings.EqualFold(dir, shimsDir) {
164-
continue
165-
}
166-
167-
// Try to find the executable in this directory
168-
var candidatePath string
169-
if os.PathSeparator == '\\' {
170-
// Windows: try .exe, .cmd, .bat extensions
171-
for _, ext := range []string{".exe", ".cmd", ".bat"} {
172-
candidate := filepath.Join(dir, execName+ext)
173-
if info, err := os.Stat(candidate); err == nil && !info.IsDir() {
174-
candidatePath = candidate
175-
break
176-
}
177-
}
178-
} else {
179-
// Unix: check if file exists and is executable
180-
candidate := filepath.Join(dir, execName)
181-
if info, err := os.Stat(candidate); err == nil && !info.IsDir() {
182-
// Check if executable (has execute permission)
183-
if info.Mode()&0111 != 0 {
184-
candidatePath = candidate
185-
}
186-
}
187-
}
188-
189-
if candidatePath != "" {
190-
return candidatePath
191-
}
192-
}
193-
194-
return ""
195-
}
196-
197147
// getShimName returns the name of this shim binary
198148
func getShimName() string {
199149
shimPath := os.Args[0]

‎src/internal/path/path.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,3 +43,61 @@ func ShimsDir() string {
4343
}
4444
return filepath.Join(home, ".dtvem", "shims")
4545
}
46+
47+
// LookPathExcludingShims searches for an executable in PATH, excluding dtvem's shims directory.
48+
// This prevents detecting our own shims as "system" installations during migration detection.
49+
// Returns the full path to the executable, or empty string if not found.
50+
func LookPathExcludingShims(execName string) string {
51+
// Get the shims directory to exclude it from search
52+
shimsDir := ShimsDir()
53+
54+
// Get PATH environment variable
55+
pathEnv := os.Getenv("PATH")
56+
if pathEnv == "" {
57+
return ""
58+
}
59+
60+
// Split PATH into directories
61+
pathDirs := filepath.SplitList(pathEnv)
62+
63+
// Search each directory
64+
for _, dir := range pathDirs {
65+
// Skip the dtvem shims directory (case-insensitive on Windows)
66+
if strings.EqualFold(dir, shimsDir) {
67+
continue
68+
}
69+
70+
// Try to find the executable in this directory
71+
candidatePath := findExecutableInDir(dir, execName)
72+
if candidatePath != "" {
73+
return candidatePath
74+
}
75+
}
76+
77+
return ""
78+
}
79+
80+
// findExecutableInDir looks for an executable with the given name in a directory.
81+
// On Windows, it tries .exe, .cmd, .bat extensions.
82+
// On Unix, it checks if the file exists and has execute permission.
83+
func findExecutableInDir(dir, execName string) string {
84+
if runtime.GOOS == "windows" {
85+
// Windows: try .exe, .cmd, .bat extensions
86+
for _, ext := range []string{".exe", ".cmd", ".bat"} {
87+
candidate := filepath.Join(dir, execName+ext)
88+
if info, err := os.Stat(candidate); err == nil && !info.IsDir() {
89+
return candidate
90+
}
91+
}
92+
} else {
93+
// Unix: check if file exists and is executable
94+
candidate := filepath.Join(dir, execName)
95+
if info, err := os.Stat(candidate); err == nil && !info.IsDir() {
96+
// Check if executable (has execute permission)
97+
if info.Mode()&0111 != 0 {
98+
return candidate
99+
}
100+
}
101+
}
102+
return ""
103+
}

‎src/internal/path/path_test.go‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,190 @@ func TestShimsDir(t *testing.T) {
108108
t.Errorf("ShimsDir() = %q, should be an absolute path", result)
109109
}
110110
}
111+
112+
func TestLookPathExcludingShims(t *testing.T) {
113+
originalPath := os.Getenv("PATH")
114+
defer func() { _ = os.Setenv("PATH", originalPath) }()
115+
116+
// Create temp directories for testing
117+
tempDir := t.TempDir()
118+
systemDir := filepath.Join(tempDir, "system")
119+
shimsDir := filepath.Join(tempDir, "shims")
120+
121+
if err := os.MkdirAll(systemDir, 0755); err != nil {
122+
t.Fatalf("Failed to create system dir: %v", err)
123+
}
124+
if err := os.MkdirAll(shimsDir, 0755); err != nil {
125+
t.Fatalf("Failed to create shims dir: %v", err)
126+
}
127+
128+
// Create test executables
129+
execName := "testexec"
130+
var systemExec, shimsExec string
131+
if runtime.GOOS == constants.OSWindows {
132+
systemExec = filepath.Join(systemDir, execName+".exe")
133+
shimsExec = filepath.Join(shimsDir, execName+".exe")
134+
} else {
135+
systemExec = filepath.Join(systemDir, execName)
136+
shimsExec = filepath.Join(shimsDir, execName)
137+
}
138+
139+
// Create dummy executables
140+
if err := os.WriteFile(systemExec, []byte("system"), 0755); err != nil {
141+
t.Fatalf("Failed to create system exec: %v", err)
142+
}
143+
if err := os.WriteFile(shimsExec, []byte("shim"), 0755); err != nil {
144+
t.Fatalf("Failed to create shims exec: %v", err)
145+
}
146+
147+
t.Run("Finds executable in system dir", func(t *testing.T) {
148+
separator := ":"
149+
if runtime.GOOS == constants.OSWindows {
150+
separator = ";"
151+
}
152+
testPath := strings.Join([]string{systemDir}, separator)
153+
_ = os.Setenv("PATH", testPath)
154+
155+
result := LookPathExcludingShims(execName)
156+
if result != systemExec {
157+
t.Errorf("LookPathExcludingShims(%q) = %q, want %q", execName, result, systemExec)
158+
}
159+
})
160+
161+
t.Run("Returns empty when not found", func(t *testing.T) {
162+
_ = os.Setenv("PATH", systemDir)
163+
164+
result := LookPathExcludingShims("nonexistent")
165+
if result != "" {
166+
t.Errorf("LookPathExcludingShims(%q) = %q, want empty string", "nonexistent", result)
167+
}
168+
})
169+
170+
t.Run("Returns empty with empty PATH", func(t *testing.T) {
171+
_ = os.Setenv("PATH", "")
172+
173+
result := LookPathExcludingShims(execName)
174+
if result != "" {
175+
t.Errorf("LookPathExcludingShims(%q) with empty PATH = %q, want empty string", execName, result)
176+
}
177+
})
178+
}
179+
180+
func TestLookPathExcludingShims_SkipsShimsDir(t *testing.T) {
181+
originalPath := os.Getenv("PATH")
182+
defer func() { _ = os.Setenv("PATH", originalPath) }()
183+
184+
// Get the actual shims directory that will be excluded
185+
shimsDir := ShimsDir()
186+
187+
// Create temp directory for "system" install
188+
tempDir := t.TempDir()
189+
systemDir := filepath.Join(tempDir, "system")
190+
if err := os.MkdirAll(systemDir, 0755); err != nil {
191+
t.Fatalf("Failed to create system dir: %v", err)
192+
}
193+
194+
// Create shims directory if it doesn't exist (for testing)
195+
if err := os.MkdirAll(shimsDir, 0755); err != nil {
196+
t.Fatalf("Failed to create shims dir: %v", err)
197+
}
198+
199+
execName := "lookuptest"
200+
var systemExec, shimsExec string
201+
if runtime.GOOS == constants.OSWindows {
202+
systemExec = filepath.Join(systemDir, execName+".exe")
203+
shimsExec = filepath.Join(shimsDir, execName+".exe")
204+
} else {
205+
systemExec = filepath.Join(systemDir, execName)
206+
shimsExec = filepath.Join(shimsDir, execName)
207+
}
208+
209+
// Create dummy executables
210+
if err := os.WriteFile(systemExec, []byte("system"), 0755); err != nil {
211+
t.Fatalf("Failed to create system exec: %v", err)
212+
}
213+
if err := os.WriteFile(shimsExec, []byte("shim"), 0755); err != nil {
214+
t.Fatalf("Failed to create shims exec: %v", err)
215+
}
216+
// Clean up shims exec after test
217+
defer func() { _ = os.Remove(shimsExec) }()
218+
219+
separator := ":"
220+
if runtime.GOOS == constants.OSWindows {
221+
separator = ";"
222+
}
223+
224+
t.Run("Skips shims dir and finds system install", func(t *testing.T) {
225+
// Put shims dir FIRST in PATH, then system dir
226+
testPath := strings.Join([]string{shimsDir, systemDir}, separator)
227+
_ = os.Setenv("PATH", testPath)
228+
229+
result := LookPathExcludingShims(execName)
230+
231+
// Should find the system exec, NOT the shims exec
232+
if result != systemExec {
233+
t.Errorf("LookPathExcludingShims(%q) = %q, want %q (should skip shims dir)", execName, result, systemExec)
234+
}
235+
})
236+
237+
t.Run("Returns empty when only in shims dir", func(t *testing.T) {
238+
// Put ONLY shims dir in PATH
239+
_ = os.Setenv("PATH", shimsDir)
240+
241+
result := LookPathExcludingShims(execName)
242+
243+
// Should return empty since shims dir is excluded
244+
if result != "" {
245+
t.Errorf("LookPathExcludingShims(%q) = %q, want empty (shims dir should be excluded)", execName, result)
246+
}
247+
})
248+
}
249+
250+
func TestFindExecutableInDir(t *testing.T) {
251+
tempDir := t.TempDir()
252+
253+
execName := "findtest"
254+
var execPath string
255+
if runtime.GOOS == constants.OSWindows {
256+
execPath = filepath.Join(tempDir, execName+".exe")
257+
} else {
258+
execPath = filepath.Join(tempDir, execName)
259+
}
260+
261+
// Create dummy executable
262+
if err := os.WriteFile(execPath, []byte("test"), 0755); err != nil {
263+
t.Fatalf("Failed to create exec: %v", err)
264+
}
265+
266+
t.Run("Finds executable", func(t *testing.T) {
267+
result := findExecutableInDir(tempDir, execName)
268+
if result != execPath {
269+
t.Errorf("findExecutableInDir(%q, %q) = %q, want %q", tempDir, execName, result, execPath)
270+
}
271+
})
272+
273+
t.Run("Returns empty for nonexistent", func(t *testing.T) {
274+
result := findExecutableInDir(tempDir, "nonexistent")
275+
if result != "" {
276+
t.Errorf("findExecutableInDir(%q, %q) = %q, want empty", tempDir, "nonexistent", result)
277+
}
278+
})
279+
280+
t.Run("Returns empty for directory with same name", func(t *testing.T) {
281+
dirName := "isdir"
282+
var dirPath string
283+
if runtime.GOOS == constants.OSWindows {
284+
dirPath = filepath.Join(tempDir, dirName+".exe")
285+
} else {
286+
dirPath = filepath.Join(tempDir, dirName)
287+
}
288+
if err := os.MkdirAll(dirPath, 0755); err != nil {
289+
t.Fatalf("Failed to create dir: %v", err)
290+
}
291+
292+
result := findExecutableInDir(tempDir, dirName)
293+
if result != "" {
294+
t.Errorf("findExecutableInDir should not return directories, got %q", result)
295+
}
296+
})
297+
}

‎src/runtimes/node/provider.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/dtvem/dtvem/src/internal/config"
1616
"github.com/dtvem/dtvem/src/internal/constants"
1717
"github.com/dtvem/dtvem/src/internal/download"
18+
"github.com/dtvem/dtvem/src/internal/path"
1819
"github.com/dtvem/dtvem/src/internal/runtime"
1920
"github.com/dtvem/dtvem/src/internal/shim"
2021
"github.com/dtvem/dtvem/src/internal/ui"
@@ -343,8 +344,8 @@ func (p *Provider) DetectInstalled() ([]runtime.DetectedVersion, error) {
343344
detected := make([]runtime.DetectedVersion, 0)
344345
seen := make(map[string]bool) // Track unique paths to avoid duplicates
345346

346-
// 1. Check node in PATH
347-
if nodePath, err := exec.LookPath("node"); err == nil {
347+
// 1. Check node in PATH (excluding dtvem's shims directory)
348+
if nodePath := path.LookPathExcludingShims("node"); nodePath != "" {
348349
if version, err := getNodeVersion(nodePath); err == nil {
349350
if !seen[nodePath] {
350351
detected = append(detected, runtime.DetectedVersion{

‎src/runtimes/python/provider.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/dtvem/dtvem/src/internal/config"
2020
"github.com/dtvem/dtvem/src/internal/constants"
2121
"github.com/dtvem/dtvem/src/internal/download"
22+
"github.com/dtvem/dtvem/src/internal/path"
2223
"github.com/dtvem/dtvem/src/internal/runtime"
2324
"github.com/dtvem/dtvem/src/internal/shim"
2425
"github.com/dtvem/dtvem/src/internal/ui"
@@ -557,9 +558,9 @@ func (p *Provider) DetectInstalled() ([]runtime.DetectedVersion, error) {
557558
detected := make([]runtime.DetectedVersion, 0)
558559
seen := make(map[string]bool) // Track unique paths to avoid duplicates
559560

560-
// 1. Check python and python3 in PATH
561+
// 1. Check python and python3 in PATH (excluding dtvem's shims directory)
561562
for _, cmd := range []string{"python", "python3"} {
562-
if pythonPath, err := exec.LookPath(cmd); err == nil {
563+
if pythonPath := path.LookPathExcludingShims(cmd); pythonPath != "" {
563564
if version, err := getPythonVersion(pythonPath); err == nil {
564565
if !seen[pythonPath] {
565566
detected = append(detected, runtime.DetectedVersion{

‎src/runtimes/ruby/provider.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/dtvem/dtvem/src/internal/config"
1818
"github.com/dtvem/dtvem/src/internal/constants"
1919
"github.com/dtvem/dtvem/src/internal/download"
20+
"github.com/dtvem/dtvem/src/internal/path"
2021
"github.com/dtvem/dtvem/src/internal/runtime"
2122
"github.com/dtvem/dtvem/src/internal/shim"
2223
"github.com/dtvem/dtvem/src/internal/ui"
@@ -556,8 +557,8 @@ func (p *Provider) DetectInstalled() ([]runtime.DetectedVersion, error) {
556557
detected := make([]runtime.DetectedVersion, 0)
557558
seen := make(map[string]bool) // Track unique paths to avoid duplicates
558559

559-
// 1. Check ruby in PATH
560-
if rubyPath, err := exec.LookPath("ruby"); err == nil {
560+
// 1. Check ruby in PATH (excluding dtvem's shims directory)
561+
if rubyPath := path.LookPathExcludingShims("ruby"); rubyPath != "" {
561562
if version, err := getRubyVersion(rubyPath); err == nil {
562563
if !seen[rubyPath] {
563564
detected = append(detected, runtime.DetectedVersion{

0 commit comments

Comments
 (0)