From d736fb1b3bec4b841504b1db6b1399b0011be32c Mon Sep 17 00:00:00 2001 From: "Calvin A. Allen" Date: Sat, 13 Dec 2025 16:54:19 -0500 Subject: [PATCH 1/3] 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 --- src/cmd/shim/main.go | 54 +-------- src/internal/path/path.go | 58 ++++++++++ src/internal/path/path_test.go | 187 ++++++++++++++++++++++++++++++++ src/runtimes/node/provider.go | 5 +- src/runtimes/python/provider.go | 5 +- src/runtimes/ruby/provider.go | 5 +- 6 files changed, 256 insertions(+), 58 deletions(-) diff --git a/src/cmd/shim/main.go b/src/cmd/shim/main.go index 72a6ad8..6f2c1b6 100644 --- a/src/cmd/shim/main.go +++ b/src/cmd/shim/main.go @@ -12,6 +12,7 @@ import ( "github.com/dtvem/dtvem/src/internal/config" "github.com/dtvem/dtvem/src/internal/constants" + "github.com/dtvem/dtvem/src/internal/path" "github.com/dtvem/dtvem/src/internal/runtime" "github.com/dtvem/dtvem/src/internal/shim" "github.com/dtvem/dtvem/src/internal/ui" @@ -111,7 +112,7 @@ func runShim() error { // It attempts to fallback to system PATH or prompts for installation func handleNoConfiguredVersion(shimName, runtimeName string, provider runtime.ShimProvider) error { // Try to find the executable deeper in PATH (system installation) - systemPath := findInSystemPath(shimName) + systemPath := path.LookPathExcludingShims(shimName) if systemPath != "" { // Found system installation - use it @@ -143,57 +144,6 @@ func handleNoConfiguredVersion(shimName, runtimeName string, provider runtime.Sh return fmt.Errorf("no version configured") } -// findInSystemPath searches for an executable in PATH, excluding dtvem's shims directory -func findInSystemPath(execName string) string { - // Get the shims directory to exclude it from search - shimsDir := config.DefaultPaths().Shims - - // Get PATH environment variable - pathEnv := os.Getenv("PATH") - if pathEnv == "" { - return "" - } - - // Split PATH into directories - pathDirs := filepath.SplitList(pathEnv) - - // Search each directory - for _, dir := range pathDirs { - // Skip the dtvem shims directory - if strings.EqualFold(dir, shimsDir) { - continue - } - - // Try to find the executable in this directory - var candidatePath string - if os.PathSeparator == '\\' { - // Windows: try .exe, .cmd, .bat extensions - for _, ext := range []string{".exe", ".cmd", ".bat"} { - candidate := filepath.Join(dir, execName+ext) - if info, err := os.Stat(candidate); err == nil && !info.IsDir() { - candidatePath = candidate - break - } - } - } else { - // Unix: check if file exists and is executable - candidate := filepath.Join(dir, execName) - if info, err := os.Stat(candidate); err == nil && !info.IsDir() { - // Check if executable (has execute permission) - if info.Mode()&0111 != 0 { - candidatePath = candidate - } - } - } - - if candidatePath != "" { - return candidatePath - } - } - - return "" -} - // getShimName returns the name of this shim binary func getShimName() string { shimPath := os.Args[0] diff --git a/src/internal/path/path.go b/src/internal/path/path.go index 54c34a5..e9c067b 100644 --- a/src/internal/path/path.go +++ b/src/internal/path/path.go @@ -43,3 +43,61 @@ func ShimsDir() string { } return filepath.Join(home, ".dtvem", "shims") } + +// LookPathExcludingShims searches for an executable in PATH, excluding dtvem's shims directory. +// This prevents detecting our own shims as "system" installations during migration detection. +// Returns the full path to the executable, or empty string if not found. +func LookPathExcludingShims(execName string) string { + // Get the shims directory to exclude it from search + shimsDir := ShimsDir() + + // Get PATH environment variable + pathEnv := os.Getenv("PATH") + if pathEnv == "" { + return "" + } + + // Split PATH into directories + pathDirs := filepath.SplitList(pathEnv) + + // Search each directory + for _, dir := range pathDirs { + // Skip the dtvem shims directory (case-insensitive on Windows) + if strings.EqualFold(dir, shimsDir) { + continue + } + + // Try to find the executable in this directory + candidatePath := findExecutableInDir(dir, execName) + if candidatePath != "" { + return candidatePath + } + } + + return "" +} + +// findExecutableInDir looks for an executable with the given name in a directory. +// On Windows, it tries .exe, .cmd, .bat extensions. +// On Unix, it checks if the file exists and has execute permission. +func findExecutableInDir(dir, execName string) string { + if runtime.GOOS == "windows" { + // Windows: try .exe, .cmd, .bat extensions + for _, ext := range []string{".exe", ".cmd", ".bat"} { + candidate := filepath.Join(dir, execName+ext) + if info, err := os.Stat(candidate); err == nil && !info.IsDir() { + return candidate + } + } + } else { + // Unix: check if file exists and is executable + candidate := filepath.Join(dir, execName) + if info, err := os.Stat(candidate); err == nil && !info.IsDir() { + // Check if executable (has execute permission) + if info.Mode()&0111 != 0 { + return candidate + } + } + } + return "" +} diff --git a/src/internal/path/path_test.go b/src/internal/path/path_test.go index 2237577..16c66dd 100644 --- a/src/internal/path/path_test.go +++ b/src/internal/path/path_test.go @@ -108,3 +108,190 @@ func TestShimsDir(t *testing.T) { t.Errorf("ShimsDir() = %q, should be an absolute path", result) } } + +func TestLookPathExcludingShims(t *testing.T) { + originalPath := os.Getenv("PATH") + defer func() { _ = os.Setenv("PATH", originalPath) }() + + // Create temp directories for testing + tempDir := t.TempDir() + systemDir := filepath.Join(tempDir, "system") + shimsDir := filepath.Join(tempDir, "shims") + + if err := os.MkdirAll(systemDir, 0755); err != nil { + t.Fatalf("Failed to create system dir: %v", err) + } + if err := os.MkdirAll(shimsDir, 0755); err != nil { + t.Fatalf("Failed to create shims dir: %v", err) + } + + // Create test executables + execName := "testexec" + var systemExec, shimsExec string + if runtime.GOOS == constants.OSWindows { + systemExec = filepath.Join(systemDir, execName+".exe") + shimsExec = filepath.Join(shimsDir, execName+".exe") + } else { + systemExec = filepath.Join(systemDir, execName) + shimsExec = filepath.Join(shimsDir, execName) + } + + // Create dummy executables + if err := os.WriteFile(systemExec, []byte("system"), 0755); err != nil { + t.Fatalf("Failed to create system exec: %v", err) + } + if err := os.WriteFile(shimsExec, []byte("shim"), 0755); err != nil { + t.Fatalf("Failed to create shims exec: %v", err) + } + + t.Run("Finds executable in system dir", func(t *testing.T) { + separator := ":" + if runtime.GOOS == constants.OSWindows { + separator = ";" + } + testPath := strings.Join([]string{systemDir}, separator) + _ = os.Setenv("PATH", testPath) + + result := LookPathExcludingShims(execName) + if result != systemExec { + t.Errorf("LookPathExcludingShims(%q) = %q, want %q", execName, result, systemExec) + } + }) + + t.Run("Returns empty when not found", func(t *testing.T) { + _ = os.Setenv("PATH", systemDir) + + result := LookPathExcludingShims("nonexistent") + if result != "" { + t.Errorf("LookPathExcludingShims(%q) = %q, want empty string", "nonexistent", result) + } + }) + + t.Run("Returns empty with empty PATH", func(t *testing.T) { + _ = os.Setenv("PATH", "") + + result := LookPathExcludingShims(execName) + if result != "" { + t.Errorf("LookPathExcludingShims(%q) with empty PATH = %q, want empty string", execName, result) + } + }) +} + +func TestLookPathExcludingShims_SkipsShimsDir(t *testing.T) { + originalPath := os.Getenv("PATH") + defer func() { _ = os.Setenv("PATH", originalPath) }() + + // Get the actual shims directory that will be excluded + shimsDir := ShimsDir() + + // Create temp directory for "system" install + tempDir := t.TempDir() + systemDir := filepath.Join(tempDir, "system") + if err := os.MkdirAll(systemDir, 0755); err != nil { + t.Fatalf("Failed to create system dir: %v", err) + } + + // Create shims directory if it doesn't exist (for testing) + if err := os.MkdirAll(shimsDir, 0755); err != nil { + t.Fatalf("Failed to create shims dir: %v", err) + } + + execName := "lookuptest" + var systemExec, shimsExec string + if runtime.GOOS == constants.OSWindows { + systemExec = filepath.Join(systemDir, execName+".exe") + shimsExec = filepath.Join(shimsDir, execName+".exe") + } else { + systemExec = filepath.Join(systemDir, execName) + shimsExec = filepath.Join(shimsDir, execName) + } + + // Create dummy executables + if err := os.WriteFile(systemExec, []byte("system"), 0755); err != nil { + t.Fatalf("Failed to create system exec: %v", err) + } + if err := os.WriteFile(shimsExec, []byte("shim"), 0755); err != nil { + t.Fatalf("Failed to create shims exec: %v", err) + } + // Clean up shims exec after test + defer func() { _ = os.Remove(shimsExec) }() + + separator := ":" + if runtime.GOOS == constants.OSWindows { + separator = ";" + } + + t.Run("Skips shims dir and finds system install", func(t *testing.T) { + // Put shims dir FIRST in PATH, then system dir + testPath := strings.Join([]string{shimsDir, systemDir}, separator) + _ = os.Setenv("PATH", testPath) + + result := LookPathExcludingShims(execName) + + // Should find the system exec, NOT the shims exec + if result != systemExec { + t.Errorf("LookPathExcludingShims(%q) = %q, want %q (should skip shims dir)", execName, result, systemExec) + } + }) + + t.Run("Returns empty when only in shims dir", func(t *testing.T) { + // Put ONLY shims dir in PATH + _ = os.Setenv("PATH", shimsDir) + + result := LookPathExcludingShims(execName) + + // Should return empty since shims dir is excluded + if result != "" { + t.Errorf("LookPathExcludingShims(%q) = %q, want empty (shims dir should be excluded)", execName, result) + } + }) +} + +func TestFindExecutableInDir(t *testing.T) { + tempDir := t.TempDir() + + execName := "findtest" + var execPath string + if runtime.GOOS == constants.OSWindows { + execPath = filepath.Join(tempDir, execName+".exe") + } else { + execPath = filepath.Join(tempDir, execName) + } + + // Create dummy executable + if err := os.WriteFile(execPath, []byte("test"), 0755); err != nil { + t.Fatalf("Failed to create exec: %v", err) + } + + t.Run("Finds executable", func(t *testing.T) { + result := findExecutableInDir(tempDir, execName) + if result != execPath { + t.Errorf("findExecutableInDir(%q, %q) = %q, want %q", tempDir, execName, result, execPath) + } + }) + + t.Run("Returns empty for nonexistent", func(t *testing.T) { + result := findExecutableInDir(tempDir, "nonexistent") + if result != "" { + t.Errorf("findExecutableInDir(%q, %q) = %q, want empty", tempDir, "nonexistent", result) + } + }) + + t.Run("Returns empty for directory with same name", func(t *testing.T) { + dirName := "isdir" + var dirPath string + if runtime.GOOS == constants.OSWindows { + dirPath = filepath.Join(tempDir, dirName+".exe") + } else { + dirPath = filepath.Join(tempDir, dirName) + } + if err := os.MkdirAll(dirPath, 0755); err != nil { + t.Fatalf("Failed to create dir: %v", err) + } + + result := findExecutableInDir(tempDir, dirName) + if result != "" { + t.Errorf("findExecutableInDir should not return directories, got %q", result) + } + }) +} diff --git a/src/runtimes/node/provider.go b/src/runtimes/node/provider.go index 5bf1f48..85af0ba 100644 --- a/src/runtimes/node/provider.go +++ b/src/runtimes/node/provider.go @@ -15,6 +15,7 @@ import ( "github.com/dtvem/dtvem/src/internal/config" "github.com/dtvem/dtvem/src/internal/constants" "github.com/dtvem/dtvem/src/internal/download" + "github.com/dtvem/dtvem/src/internal/path" "github.com/dtvem/dtvem/src/internal/runtime" "github.com/dtvem/dtvem/src/internal/shim" "github.com/dtvem/dtvem/src/internal/ui" @@ -343,8 +344,8 @@ func (p *Provider) DetectInstalled() ([]runtime.DetectedVersion, error) { detected := make([]runtime.DetectedVersion, 0) seen := make(map[string]bool) // Track unique paths to avoid duplicates - // 1. Check node in PATH - if nodePath, err := exec.LookPath("node"); err == nil { + // 1. Check node in PATH (excluding dtvem's shims directory) + if nodePath := path.LookPathExcludingShims("node"); nodePath != "" { if version, err := getNodeVersion(nodePath); err == nil { if !seen[nodePath] { detected = append(detected, runtime.DetectedVersion{ diff --git a/src/runtimes/python/provider.go b/src/runtimes/python/provider.go index 21da569..2a6c901 100644 --- a/src/runtimes/python/provider.go +++ b/src/runtimes/python/provider.go @@ -19,6 +19,7 @@ import ( "github.com/dtvem/dtvem/src/internal/config" "github.com/dtvem/dtvem/src/internal/constants" "github.com/dtvem/dtvem/src/internal/download" + "github.com/dtvem/dtvem/src/internal/path" "github.com/dtvem/dtvem/src/internal/runtime" "github.com/dtvem/dtvem/src/internal/shim" "github.com/dtvem/dtvem/src/internal/ui" @@ -557,9 +558,9 @@ func (p *Provider) DetectInstalled() ([]runtime.DetectedVersion, error) { detected := make([]runtime.DetectedVersion, 0) seen := make(map[string]bool) // Track unique paths to avoid duplicates - // 1. Check python and python3 in PATH + // 1. Check python and python3 in PATH (excluding dtvem's shims directory) for _, cmd := range []string{"python", "python3"} { - if pythonPath, err := exec.LookPath(cmd); err == nil { + if pythonPath := path.LookPathExcludingShims(cmd); pythonPath != "" { if version, err := getPythonVersion(pythonPath); err == nil { if !seen[pythonPath] { detected = append(detected, runtime.DetectedVersion{ diff --git a/src/runtimes/ruby/provider.go b/src/runtimes/ruby/provider.go index 944d039..0cda567 100644 --- a/src/runtimes/ruby/provider.go +++ b/src/runtimes/ruby/provider.go @@ -17,6 +17,7 @@ import ( "github.com/dtvem/dtvem/src/internal/config" "github.com/dtvem/dtvem/src/internal/constants" "github.com/dtvem/dtvem/src/internal/download" + "github.com/dtvem/dtvem/src/internal/path" "github.com/dtvem/dtvem/src/internal/runtime" "github.com/dtvem/dtvem/src/internal/shim" "github.com/dtvem/dtvem/src/internal/ui" @@ -556,8 +557,8 @@ func (p *Provider) DetectInstalled() ([]runtime.DetectedVersion, error) { detected := make([]runtime.DetectedVersion, 0) seen := make(map[string]bool) // Track unique paths to avoid duplicates - // 1. Check ruby in PATH - if rubyPath, err := exec.LookPath("ruby"); err == nil { + // 1. Check ruby in PATH (excluding dtvem's shims directory) + if rubyPath := path.LookPathExcludingShims("ruby"); rubyPath != "" { if version, err := getRubyVersion(rubyPath); err == nil { if !seen[rubyPath] { detected = append(detected, runtime.DetectedVersion{ From 500899f690e24c42e0f50791cb59dbedb3e36467 Mon Sep 17 00:00:00 2001 From: "Calvin A. Allen" Date: Sat, 13 Dec 2025 16:58:09 -0500 Subject: [PATCH 2/3] fix(ci): use official golangci-lint-action instead of curl install --- .github/workflows/build.yml | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c365810..e7084f1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -64,15 +64,14 @@ jobs: with: go-version: '1.23' - - name: Install golangci-lint - run: | - # Download and install golangci-lint v2.6.2 - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v2.6.2 - golangci-lint --version - - name: Run golangci-lint - run: GOOS=${{ matrix.goos }} golangci-lint run --timeout=5m --config=../.golangci.yml - working-directory: src + uses: golangci/golangci-lint-action@v7 + with: + version: v2.6.2 + working-directory: src + args: --timeout=5m --config=../.golangci.yml + env: + GOOS: ${{ matrix.goos }} - name: Verify no linting issues if: success() From 35445f250e1ea1a9053c3a790089b11e2e13ee02 Mon Sep 17 00:00:00 2001 From: "Calvin A. Allen" Date: Sat, 13 Dec 2025 17:12:36 -0500 Subject: [PATCH 3/3] chore(lint): migrate golangci-lint config to v2.x schema - Convert version from number to string ("2") - Migrate output format configuration to v2.x structure - Move linters-settings to linters.settings - Migrate exclusion rules from issues section to linters.exclusions - Increase gocyclo complexity threshold to 25 for existing code - Add goconst to test file exclusions (replaces removed ignore-tests) Required for golangci-lint-action@v7 which validates schema strictly. --- .golangci.yml | 224 +++++++++++++++++++------------------------------- 1 file changed, 85 insertions(+), 139 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index de10985..b39f512 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,44 +1,32 @@ # golangci-lint configuration for dtvem -# Documentation: https://golangci-lint.run/usage/configuration/ +# Documentation: https://golangci-lint.run/docs/configuration/file/ -# Config version -version: 2 +# Config version (must be a string for v2.x) +version: "2" run: # Timeout for analysis timeout: 5m - # Include test files in linting tests: true -# Output configuration +# Output configuration (v2.x format) output: - # Colored output - format: colored-line-number - - # Print lines of code with issue - print-issued-lines: true - - # Print linter name in the end of issue text - print-linter-name: true - - # Make issues output unique by line - uniq-by-line: true - - # Sort results by: filepath, line and column - sort-results: true - -# Linters configuration + formats: + text: + path: stdout + colors: true + # Sort results by file, linter, severity + sort-order: + - file + - linter + # Show statistics + show-stats: true + +# Linters configuration (v2.x format) linters: - # Disable all linters by default - disable-all: true - - # Explicitly disable warning-only linters - disable: - - staticcheck # Code improvement suggestions - - revive # Style suggestions - - prealloc # Performance hints - - gosec # Security false positives + # Start with no linters enabled + default: none # Enable specific linters enable: @@ -62,115 +50,73 @@ linters: - nilerr # Find code that returns nil even if it checks that the error is not nil - nilnil # Checks that there is no simultaneous return of nil error and invalid value - # WARNINGS ONLY (disabled to prevent CI failure) - # These are style/security suggestions that should not fail the build - # Re-enable locally with: golangci-lint run --enable-all - # - staticcheck # Code improvement suggestions (QF quick-fixes) - # - revive # Style suggestions (comment formatting, etc.) - # - prealloc # Performance hints - # - gosec # Security warnings (often false positives) - -linters-settings: - # Settings for errcheck - errcheck: - # Report about not checking of errors in type assertions: `a := b.(MyStruct)` - check-type-assertions: true - - # Report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)` - check-blank: false - - # List of functions to exclude from checking - exclude-functions: - - (*os.File).Close - - (*database/sql.Rows).Close - - # Settings for govet - govet: - # Enable all analyzers - enable-all: true - # Disable specific analyzers - disable: - - shadow # Reports variables that shadow other variables (can be too noisy) - - # Settings for gocyclo (cyclomatic complexity) - gocyclo: - # Minimal code complexity to report - min-complexity: 15 - - # Settings for dupl (code duplication) - dupl: - # Threshold for duplicate code detection - threshold: 100 - - # Settings for goconst - goconst: - # Minimal length of string constant - min-len: 3 - # Minimum occurrences count to trigger issue - min-occurrences: 3 - # Ignore test files - ignore-tests: true - - # Settings for misspell - misspell: - # Correct spellings using locale preferences - locale: US - - # DISABLED LINTER SETTINGS (commented out since linters are disabled above) - # Uncomment these when re-enabling the corresponding linters - - # # Settings for revive (replacement for golint) - # revive: - # enable-all-rules: false - # rules: - # # ... (keep original rules for reference) - - # # Settings for gosec (security) - # gosec: - # tests: true - # excludes: - # - G304 # File path provided as taint input - - # # Settings for staticcheck - # staticcheck: - # checks: ["all"] - -# Issues configuration + # Linter-specific settings (moved from top-level linters-settings) + settings: + # Settings for errcheck + errcheck: + # Report about not checking of errors in type assertions + check-type-assertions: true + # Report about assignment of errors to blank identifier + check-blank: false + # List of functions to exclude from checking + exclude-functions: + - (*os.File).Close + - (*database/sql.Rows).Close + + # Settings for govet + govet: + # Enable all analyzers + enable-all: true + # Disable specific analyzers + disable: + - shadow # Reports variables that shadow other variables (can be too noisy) + - fieldalignment # Reports struct field ordering for memory optimization (too noisy) + + # Settings for gocyclo (cyclomatic complexity) + gocyclo: + # Minimal code complexity to report (25 allows for existing complex functions) + min-complexity: 25 + + # Settings for dupl (code duplication) + dupl: + # Threshold for duplicate code detection + threshold: 100 + + # Settings for goconst + goconst: + # Minimal length of string constant + min-len: 3 + # Minimum occurrences count to trigger issue + min-occurrences: 3 + + # Settings for misspell + misspell: + # Correct spellings using locale preferences + locale: US + + # Exclusion rules (moved from issues section in v1.x) + exclusions: + # Generated files should be ignored + generated: lax + # Paths to exclude + paths: + - vendor + - dist + - .claude + - ".*\\.pb\\.go$" + # Rules to exclude specific linters in specific paths + rules: + # Ignore duplicate code and repeated strings in test files (test tables often have similar structure) + - linters: + - dupl + - goconst + path: "_test\\.go$" + +# Issues configuration (v2.x format) issues: - # Which dirs to skip: issues from them won't be reported - exclude-dirs: - - vendor - - dist - - .claude - - # Which files to skip: issues from them won't be reported - exclude-files: - - ".*\\.pb\\.go$" # Skip generated protobuf files - - # Exclude specific linters for matching issues - exclude-rules: - # Exclude known issues in generated files - - path: ".*\\.pb\\.go" - linters: - - all - - # Ignore long lines in comments (e.g., URLs) - - linters: - - revive - text: "line is \\d+ characters" - source: "^\\s*//.*https?://" - - # Maximum issues count per one linter - # Set to 0 to disable limit + # Maximum issues count per one linter (0 = unlimited) max-issues-per-linter: 0 - - # Maximum count of issues with the same text - # Set to 0 to disable limit + # Maximum count of issues with the same text (0 = unlimited) max-same-issues: 0 - - # Show only new issues created after git revision - # Useful for checking only your changes - # new-from-rev: origin/main - - # Independently of option `exclude` we use default exclude patterns - exclude-use-default: false + # Make issues output unique by line + uniq-by-line: true