diff --git a/CLAUDE.md b/CLAUDE.md index 3c5b80c..b51d0d2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -397,3 +397,4 @@ Use `internal/ui` for all user-facing output: - **Version strings**: Strip `v` prefix (e.g., "v22.0.0" → "22.0.0") - **Registry is global**: Providers auto-register on import via `init()` - **Verbose mode**: Set `DTVEM_VERBOSE=1` for debug output +- **Windows PATH in the registry**: write it with `writePathValue` (`internal/path/registry_windows.go`), never `registry.Key.SetStringValue` or .NET's `[Environment]::SetEnvironmentVariable`. Those write `REG_SZ`, and Windows only expands `%VAR%` entries such as `%SystemRoot%\system32` in a `REG_EXPAND_SZ` value. `dtvem doctor` (`path-registry-type`) repairs a converted User PATH. diff --git a/install.ps1 b/install.ps1 index d2d7fde..2b63f2b 100644 --- a/install.ps1 +++ b/install.ps1 @@ -125,6 +125,46 @@ function Test-Checksum { return $true } +function Send-EnvironmentChange { + # Tell running applications (Explorer, new terminals) that the + # environment changed, as [Environment]::SetEnvironmentVariable would. + $signature = '[DllImport("user32.dll", SetLastError = true, CharSet = CharSet.Auto)] public static extern IntPtr SendMessageTimeout(IntPtr hWnd, uint Msg, UIntPtr wParam, string lParam, uint fuFlags, uint uTimeout, out UIntPtr lpdwResult);' + $native = Add-Type -MemberDefinition $signature -Name 'NativeMethods' -Namespace 'DtvemInstaller' -PassThru + $result = [UIntPtr]::Zero + [void]$native::SendMessageTimeout([IntPtr]0xffff, 0x1A, [UIntPtr]::Zero, 'Environment', 2, 5000, [ref]$result) +} + +function Add-ToUserPath { + param([string]$Directory) + + # Edit the User PATH in the registry directly. + # [Environment]::GetEnvironmentVariable returns it with %VAR% entries + # expanded and SetEnvironmentVariable writes REG_SZ, which together would + # freeze entries like %USERPROFILE%\.dotnet\tools and change the value's + # type. Windows stores PATH as REG_EXPAND_SZ. + $key = [Microsoft.Win32.Registry]::CurrentUser.OpenSubKey('Environment', $true) + if (-not $key) { + throw "Could not open HKCU\Environment" + } + try { + $userPath = $key.GetValue('Path', '', [Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames) + $entries = [Environment]::ExpandEnvironmentVariables($userPath) -split ';' + if ($entries -contains $Directory) { + return $false + } + + # Add to BEGINNING of PATH for priority over system versions + $newPath = if ($userPath) { "$Directory;$userPath" } else { $Directory } + $key.SetValue('Path', $newPath, [Microsoft.Win32.RegistryValueKind]::ExpandString) + } + finally { + $key.Close() + } + + Send-EnvironmentChange + return $true +} + function Main { param([switch]$UserInstall) Write-Host "" @@ -250,24 +290,18 @@ function Main { # Add install directory to PATH Write-Host "" - $userPath = [Environment]::GetEnvironmentVariable("Path", "User") - if ($userPath -notlike "*$INSTALL_DIR*") { - Write-Info "Adding $INSTALL_DIR to PATH..." - - try { - # Add to BEGINNING of PATH for priority over system versions - $newPath = if ($userPath) { "$INSTALL_DIR;$userPath" } else { $INSTALL_DIR } - [Environment]::SetEnvironmentVariable("Path", $newPath, "User") - Write-Success "Added to PATH (at beginning for priority)" + try { + if (Add-ToUserPath -Directory $INSTALL_DIR) { + Write-Success "Added $INSTALL_DIR to PATH (at beginning for priority)" Write-Warning-Custom "Please restart your terminal for PATH changes to take effect" } - catch { - Write-Warning-Custom "Failed to add to PATH automatically: $_" - Write-Info "Please add manually: $INSTALL_DIR" + else { + Write-Info "$INSTALL_DIR is already in PATH" } } - else { - Write-Info "$INSTALL_DIR is already in PATH" + catch { + Write-Warning-Custom "Failed to add to PATH automatically: $_" + Write-Info "Please add manually: $INSTALL_DIR" } # Run init to add shims directory to PATH diff --git a/src/internal/doctor/check_path_registry_type.go b/src/internal/doctor/check_path_registry_type.go new file mode 100644 index 0000000..044da4e --- /dev/null +++ b/src/internal/doctor/check_path_registry_type.go @@ -0,0 +1,99 @@ +package doctor + +import ( + "strings" + + "github.com/CodingWithCalvin/dtvem.cli/src/internal/path" +) + +// systemPathRepairCommand rewrites the System PATH as REG_EXPAND_SZ with its +// entries unchanged. It needs an elevated PowerShell, so doctor only shows it. +const systemPathRepairCommand = `$k = [Microsoft.Win32.Registry]::LocalMachine.OpenSubKey('SYSTEM\CurrentControlSet\Control\Session Manager\Environment', $true); ` + + `$k.SetValue('Path', $k.GetValue('Path', '', 'DoNotExpandEnvironmentNames'), 'ExpandString')` + +// pathRegistryTypeCheck flags a Windows PATH value that holds %VAR% +// references but is stored as REG_SZ. Windows only expands references in a +// REG_EXPAND_SZ value, so entries such as %SystemRoot%\system32 or +// %USERPROFILE%\.dotnet\tools stay literal and their directories are +// missing from PATH. +// +// dtvem releases before #295 wrote PATH as REG_SZ from `dtvem init` and the +// stale-shims fix, and .NET's Environment.SetEnvironmentVariable does too, +// so a converted value may already be in place. +// +// Rewriting the User PATH is safe to automate. The System PATH needs an +// elevated terminal, so for it doctor shows the command instead. +type pathRegistryTypeCheck struct { + readPaths func() ([]path.RegistryPath, error) + repairUser func() error +} + +func newPathRegistryTypeCheck() *pathRegistryTypeCheck { + return &pathRegistryTypeCheck{ + readPaths: path.RegistryPaths, + repairUser: path.RepairUserPathType, + } +} + +func (pathRegistryTypeCheck) Name() string { return "path-registry-type" } + +func (c pathRegistryTypeCheck) Run() Finding { + paths, err := c.readPaths() + if err != nil { + return Finding{ + Severity: SeverityWarning, + Title: "Could not read PATH from the registry", + Details: []Detail{{Key: "Error", Value: err.Error()}}, + Resolution: "Check that the Environment registry keys are readable.", + } + } + if len(paths) == 0 { + return Finding{OK: true, Title: "PATH is not stored in the registry on this platform"} + } + + var details []Detail + userUnexpanded, systemUnexpanded := false, false + for _, p := range paths { + if !p.Unexpanded() { + continue + } + details = append(details, Detail{ + Key: p.Scope + " PATH", + Value: "stored as REG_SZ, so its %VAR% entries are not expanded", + }) + switch p.Scope { + case "User": + userUnexpanded = true + case "System": + systemUnexpanded = true + } + } + if len(details) == 0 { + return Finding{OK: true, Title: "PATH registry values expand their %VAR% entries"} + } + + var resolution []string + var fix func() error + if userUnexpanded { + resolution = append(resolution, "Rewrite the User PATH as REG_EXPAND_SZ; its entries stay as they are.") + fix = c.repairUser + } + if systemUnexpanded { + resolution = append(resolution, + "The System PATH needs an elevated PowerShell:", + " "+systemPathRepairCommand) + } + resolution = append(resolution, "Restart your terminal afterwards.") + + return Finding{ + Severity: SeverityWarning, + Title: "PATH entries that use %VAR% are not being expanded", + Details: details, + Resolution: strings.Join(resolution, "\n"), + Fix: fix, + } +} + +func init() { + Register(newPathRegistryTypeCheck()) +} diff --git a/src/internal/doctor/check_path_registry_type_test.go b/src/internal/doctor/check_path_registry_type_test.go new file mode 100644 index 0000000..7b5ff3c --- /dev/null +++ b/src/internal/doctor/check_path_registry_type_test.go @@ -0,0 +1,141 @@ +package doctor + +import ( + "errors" + "strings" + "testing" + + "github.com/CodingWithCalvin/dtvem.cli/src/internal/path" +) + +func TestPathRegistryTypeCheck(t *testing.T) { + expandable := path.RegistryPath{Expandable: true, HasReferences: true} + unexpanded := path.RegistryPath{Expandable: false, HasReferences: true} + plain := path.RegistryPath{Expandable: false, HasReferences: false} + scoped := func(scope string, p path.RegistryPath) path.RegistryPath { + p.Scope = scope + return p + } + + tests := []struct { + name string + paths []path.RegistryPath + wantOK bool + wantFixable bool + wantDetails []string + wantInResolve []string + }{ + { + name: "not Windows", + paths: nil, + wantOK: true, + }, + { + name: "both expandable", + paths: []path.RegistryPath{scoped("User", expandable), scoped("System", expandable)}, + wantOK: true, + }, + { + name: "REG_SZ without references is fine", + paths: []path.RegistryPath{scoped("User", plain), scoped("System", expandable)}, + wantOK: true, + }, + { + name: "User PATH unexpanded", + paths: []path.RegistryPath{scoped("User", unexpanded), scoped("System", expandable)}, + wantFixable: true, + wantDetails: []string{"User PATH"}, + wantInResolve: []string{"Rewrite the User PATH as REG_EXPAND_SZ"}, + }, + { + name: "System PATH unexpanded", + paths: []path.RegistryPath{scoped("User", expandable), scoped("System", unexpanded)}, + wantFixable: false, + wantDetails: []string{"System PATH"}, + wantInResolve: []string{"elevated PowerShell", "'ExpandString'"}, + }, + { + name: "both unexpanded", + paths: []path.RegistryPath{scoped("User", unexpanded), scoped("System", unexpanded)}, + wantFixable: true, + wantDetails: []string{"User PATH", "System PATH"}, + wantInResolve: []string{"Rewrite the User PATH", "elevated PowerShell"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := pathRegistryTypeCheck{ + readPaths: func() ([]path.RegistryPath, error) { return tt.paths, nil }, + repairUser: func() error { return nil }, + } + + f := c.Run() + if f.OK != tt.wantOK { + t.Fatalf("Run().OK = %v, want %v (%+v)", f.OK, tt.wantOK, f) + } + if tt.wantOK { + return + } + if f.Severity != SeverityWarning { + t.Errorf("Severity = %v, want warning", f.Severity) + } + if f.Fixable() != tt.wantFixable { + t.Errorf("Fixable() = %v, want %v", f.Fixable(), tt.wantFixable) + } + var gotDetails []string + for _, d := range f.Details { + gotDetails = append(gotDetails, d.Key) + } + if strings.Join(gotDetails, ",") != strings.Join(tt.wantDetails, ",") { + t.Errorf("Details = %v, want %v", gotDetails, tt.wantDetails) + } + for _, s := range tt.wantInResolve { + if !strings.Contains(f.Resolution, s) { + t.Errorf("Resolution %q does not mention %q", f.Resolution, s) + } + } + }) + } +} + +func TestPathRegistryTypeCheck_FixRepairsUserPath(t *testing.T) { + repaired := false + c := pathRegistryTypeCheck{ + readPaths: func() ([]path.RegistryPath, error) { + return []path.RegistryPath{{Scope: "User", HasReferences: true}}, nil + }, + repairUser: func() error { repaired = true; return nil }, + } + + f := c.Run() + if !f.Fixable() { + t.Fatal("finding is not fixable") + } + if err := f.Fix(); err != nil { + t.Fatalf("Fix() error: %v", err) + } + if !repaired { + t.Error("Fix() did not repair the User PATH") + } +} + +func TestPathRegistryTypeCheck_ReadError(t *testing.T) { + c := pathRegistryTypeCheck{ + readPaths: func() ([]path.RegistryPath, error) { return nil, errors.New("access denied") }, + } + + f := c.Run() + if f.OK || f.Severity != SeverityWarning || f.Fixable() { + t.Errorf("Run() = %+v, want an unfixable warning", f) + } +} + +func TestPathRegistryTypeCheck_Registered(t *testing.T) { + for _, c := range All() { + if c.Name() == "path-registry-type" { + return + } + } + t.Error("path-registry-type check is not registered") +} diff --git a/src/internal/path/path.go b/src/internal/path/path.go index ceabb34..078a347 100644 --- a/src/internal/path/path.go +++ b/src/internal/path/path.go @@ -17,6 +17,21 @@ import ( // line above, so we never touch lines the user wrote themselves. const dtvemMarkerComment = "# Added by dtvem" +// RegistryPath describes how a PATH value is stored in the Windows +// registry. See RegistryPaths. +type RegistryPath struct { + Scope string // "User" or "System" + Expandable bool // stored as REG_EXPAND_SZ + HasReferences bool // contains %VAR% references +} + +// Unexpanded reports whether the value's %VAR% references are left +// literal, and so their directories are missing from PATH, because it is +// stored as REG_SZ rather than REG_EXPAND_SZ. +func (p RegistryPath) Unexpanded() bool { + return p.HasReferences && !p.Expandable +} + // IsInPath checks if a directory is in the system PATH func IsInPath(dir string) bool { pathEnv := os.Getenv("PATH") diff --git a/src/internal/path/path_unix.go b/src/internal/path/path_unix.go index eb45666..d9979b5 100644 --- a/src/internal/path/path_unix.go +++ b/src/internal/path/path_unix.go @@ -178,3 +178,15 @@ func containsPathModification(configFile, shimsDir string) bool { return false } + +// RegistryPaths reports how the PATH values are stored in the Windows +// registry. There is no such registry on this platform. +func RegistryPaths() ([]RegistryPath, error) { + return nil, nil +} + +// RepairUserPathType rewrites the User PATH as REG_EXPAND_SZ on Windows. +// There is no such registry on this platform. +func RepairUserPathType() error { + return fmt.Errorf("the PATH registry only exists on Windows") +} diff --git a/src/internal/path/path_windows.go b/src/internal/path/path_windows.go index 3556e15..55d79d6 100644 --- a/src/internal/path/path_windows.go +++ b/src/internal/path/path_windows.go @@ -348,127 +348,39 @@ func relaunchElevated(shimsDir string) error { // modifySystemPath modifies the System PATH (requires admin privileges) func modifySystemPath(shimsDir, action string) error { - key, err := registry.OpenKey(registry.LOCAL_MACHINE, `SYSTEM\CurrentControlSet\Control\Session Manager\Environment`, registry.QUERY_VALUE|registry.SET_VALUE) - if err != nil { - return fmt.Errorf("failed to open System PATH registry key for writing: %w", err) - } - defer func() { _ = key.Close() }() - - currentPath, _, err := key.GetStringValue("Path") - if err != nil && !errors.Is(err, registry.ErrNotExist) { - return fmt.Errorf("failed to read System PATH: %w", err) - } - - // Parse and filter current PATH entries - paths := strings.Split(currentPath, ";") - var filteredPaths []string - var removedStale []string - - for _, p := range paths { - trimmed := strings.TrimSpace(p) - if trimmed == "" { - continue - } - // Skip if it's the shims dir (we'll prepend it) - if strings.EqualFold(trimmed, shimsDir) { - continue - } - // Skip stale dtvem shims entries from prior installs - if IsDtvemShimsPath(trimmed) { - removedStale = append(removedStale, trimmed) - continue - } - filteredPaths = append(filteredPaths, trimmed) - } - - // Build new PATH with shimsDir at the beginning - newPath := shimsDir - if len(filteredPaths) > 0 { - newPath += ";" + strings.Join(filteredPaths, ";") - } - - // Write back to registry - err = key.SetStringValue("Path", newPath) - if err != nil { - return fmt.Errorf("failed to update System PATH in registry: %w", err) - } - - // Broadcast WM_SETTINGCHANGE to notify running processes - broadcastSettingChange() - - for _, stale := range removedStale { - ui.Success("Removed stale dtvem shims entry from System PATH: %s", stale) - } - - if action == pathActionMove { - ui.Success("Moved %s to the beginning of your System PATH", shimsDir) - } else { - ui.Success("Added %s to your System PATH", shimsDir) - } - ui.Warning("Please restart your terminal for the changes to take effect") - - return nil + return modifyPath(registry.LOCAL_MACHINE, systemEnvironmentKey, "System", shimsDir, action) } // modifyUserPath modifies the User PATH (no admin privileges required) func modifyUserPath(shimsDir, action string) error { - key, err := registry.OpenKey(registry.CURRENT_USER, `Environment`, registry.QUERY_VALUE|registry.SET_VALUE) + return modifyPath(registry.CURRENT_USER, userEnvironmentKey, "User", shimsDir, action) +} + +// modifyPath puts shimsDir first on the System or User PATH (scope), held +// in the registry under root\keyPath, and reports what changed. +func modifyPath(root registry.Key, keyPath, scope, shimsDir, action string) error { + key, err := registry.OpenKey(root, keyPath, registry.QUERY_VALUE|registry.SET_VALUE) if err != nil { - return fmt.Errorf("failed to open User PATH registry key for writing: %w", err) + return fmt.Errorf("failed to open %s PATH registry key for writing: %w", scope, err) } defer func() { _ = key.Close() }() - currentPath, _, err := key.GetStringValue("Path") - if err != nil && !errors.Is(err, registry.ErrNotExist) { - return fmt.Errorf("failed to read User PATH: %w", err) - } - - // Parse and filter current PATH entries - var filteredPaths []string - var removedStale []string - if currentPath != "" { - paths := strings.Split(currentPath, ";") - for _, p := range paths { - trimmed := strings.TrimSpace(p) - if trimmed == "" { - continue - } - // Skip if it's the shims dir (we'll prepend it) - if strings.EqualFold(trimmed, shimsDir) { - continue - } - // Skip stale dtvem shims entries from prior installs - if IsDtvemShimsPath(trimmed) { - removedStale = append(removedStale, trimmed) - continue - } - filteredPaths = append(filteredPaths, trimmed) - } - } - - // Build new PATH with shimsDir at the beginning - newPath := shimsDir - if len(filteredPaths) > 0 { - newPath += ";" + strings.Join(filteredPaths, ";") - } - - // Write back to registry - err = key.SetStringValue("Path", newPath) + removedStale, err := prependShimsDir(key, scope, shimsDir) if err != nil { - return fmt.Errorf("failed to update User PATH in registry: %w", err) + return err } // Broadcast WM_SETTINGCHANGE to notify running processes broadcastSettingChange() for _, stale := range removedStale { - ui.Success("Removed stale dtvem shims entry from User PATH: %s", stale) + ui.Success("Removed stale dtvem shims entry from %s PATH: %s", scope, stale) } if action == pathActionMove { - ui.Success("Moved %s to the beginning of your User PATH", shimsDir) + ui.Success("Moved %s to the beginning of your %s PATH", shimsDir, scope) } else { - ui.Success("Added %s to your User PATH", shimsDir) + ui.Success("Added %s to your %s PATH", shimsDir, scope) } ui.Warning("Please restart your terminal for the changes to take effect") @@ -635,30 +547,15 @@ func IsSetxAvailable() bool { // administrator privileges and aren't touched here; doctor surfaces // that case separately in its resolution text. func RemoveStaleShimsFromUserPath(currentShimsDir string) ([]string, error) { - key, err := registry.OpenKey(registry.CURRENT_USER, `Environment`, registry.QUERY_VALUE|registry.SET_VALUE) + key, err := registry.OpenKey(registry.CURRENT_USER, userEnvironmentKey, registry.QUERY_VALUE|registry.SET_VALUE) if err != nil { return nil, fmt.Errorf("failed to open User PATH registry key: %w", err) } defer func() { _ = key.Close() }() - currentPath, _, err := key.GetStringValue("Path") - if err != nil && !errors.Is(err, registry.ErrNotExist) { - return nil, fmt.Errorf("failed to read User PATH: %w", err) - } - if errors.Is(err, registry.ErrNotExist) || currentPath == "" { - // No User PATH set at all means there can't be any stale - // entries to remove. - return nil, nil - } - - entries := strings.Split(currentPath, ";") - kept, removed := PartitionStaleShimsEntries(entries, currentShimsDir) - if len(removed) == 0 { - return nil, nil - } - - if err := key.SetStringValue("Path", strings.Join(kept, ";")); err != nil { - return nil, fmt.Errorf("failed to update User PATH in registry: %w", err) + removed, err := removeStaleShims(key, currentShimsDir) + if err != nil || len(removed) == 0 { + return nil, err } // Tell the shell that PATH changed so newly-spawned processes pick diff --git a/src/internal/path/registry_windows.go b/src/internal/path/registry_windows.go new file mode 100644 index 0000000..04d0a6b --- /dev/null +++ b/src/internal/path/registry_windows.go @@ -0,0 +1,166 @@ +//go:build windows + +package path + +import ( + "errors" + "fmt" + "regexp" + "strings" + + "golang.org/x/sys/windows/registry" +) + +const ( + userEnvironmentKey = `Environment` + systemEnvironmentKey = `SYSTEM\CurrentControlSet\Control\Session Manager\Environment` +) + +// envReferencePattern matches a %VAR% reference in a PATH value. +var envReferencePattern = regexp.MustCompile(`%[^%;]+%`) + +// pathKey is the part of registry.Key the PATH helpers use, so tests can +// use a fake instead of the real environment. +type pathKey interface { + GetStringValue(name string) (val string, valtype uint32, err error) + SetStringValue(name, value string) error + SetExpandStringValue(name, value string) error +} + +// readPathValue returns the PATH value under key, with %VAR% references +// unexpanded, and its registry type. A missing value reads as empty with +// REG_EXPAND_SZ, the type Windows gives PATH. +func readPathValue(key pathKey) (string, uint32, error) { + value, valType, err := key.GetStringValue("Path") + if errors.Is(err, registry.ErrNotExist) { + return "", registry.EXPAND_SZ, nil + } + return value, valType, err +} + +// writePathValue writes the PATH value under key, as REG_EXPAND_SZ when +// it was stored that way or when value holds a %VAR% reference. Windows +// only expands references in a REG_EXPAND_SZ value, so writing REG_SZ would +// leave entries like %SystemRoot%\system32 literal and off PATH. +func writePathValue(key pathKey, value string, valType uint32) error { + if valType == registry.EXPAND_SZ || hasEnvReference(value) { + return key.SetExpandStringValue("Path", value) + } + return key.SetStringValue("Path", value) +} + +func hasEnvReference(value string) bool { + return envReferencePattern.MatchString(value) +} + +// prependShimsDir puts shimsDir first on the PATH under key, dropping any +// other copy of it and any stale dtvem shims entries, and returns the +// stale entries it removed. scope names the PATH ("User" or "System") in +// errors. +func prependShimsDir(key pathKey, scope, shimsDir string) ([]string, error) { + current, valType, err := readPathValue(key) + if err != nil { + return nil, fmt.Errorf("failed to read %s PATH: %w", scope, err) + } + + entries := []string{shimsDir} + var removedStale []string + for _, p := range strings.Split(current, ";") { + trimmed := strings.TrimSpace(p) + switch { + case trimmed == "", strings.EqualFold(trimmed, shimsDir): + continue + case IsDtvemShimsPath(trimmed): + removedStale = append(removedStale, trimmed) + default: + entries = append(entries, trimmed) + } + } + + if err := writePathValue(key, strings.Join(entries, ";"), valType); err != nil { + return nil, fmt.Errorf("failed to update %s PATH in registry: %w", scope, err) + } + return removedStale, nil +} + +// removeStaleShims removes stale dtvem shims entries from the PATH under +// key, keeping currentShimsDir and the order of everything else, and +// returns the entries it removed. PATH is only rewritten when something +// was removed. +func removeStaleShims(key pathKey, currentShimsDir string) ([]string, error) { + current, valType, err := readPathValue(key) + if err != nil { + return nil, fmt.Errorf("failed to read User PATH: %w", err) + } + if current == "" { + return nil, nil + } + + kept, removed := PartitionStaleShimsEntries(strings.Split(current, ";"), currentShimsDir) + if len(removed) == 0 { + return nil, nil + } + if err := writePathValue(key, strings.Join(kept, ";"), valType); err != nil { + return nil, fmt.Errorf("failed to update User PATH in registry: %w", err) + } + return removed, nil +} + +// RegistryPaths reports how the User and System PATH values are stored. +func RegistryPaths() ([]RegistryPath, error) { + scopes := []struct { + name string + root registry.Key + path string + }{ + {"User", registry.CURRENT_USER, userEnvironmentKey}, + {"System", registry.LOCAL_MACHINE, systemEnvironmentKey}, + } + + paths := make([]RegistryPath, 0, len(scopes)) + for _, s := range scopes { + key, err := registry.OpenKey(s.root, s.path, registry.QUERY_VALUE) + if err != nil { + return nil, fmt.Errorf("failed to open %s environment: %w", s.name, err) + } + value, valType, err := readPathValue(key) + _ = key.Close() + if err != nil { + return nil, fmt.Errorf("failed to read %s PATH: %w", s.name, err) + } + paths = append(paths, RegistryPath{ + Scope: s.name, + Expandable: valType == registry.EXPAND_SZ, + HasReferences: hasEnvReference(value), + }) + } + return paths, nil +} + +// RepairUserPathType rewrites the User PATH as REG_EXPAND_SZ, with its +// entries unchanged, so its %VAR% references expand again. +func RepairUserPathType() error { + key, err := registry.OpenKey(registry.CURRENT_USER, userEnvironmentKey, registry.QUERY_VALUE|registry.SET_VALUE) + if err != nil { + return fmt.Errorf("failed to open User PATH registry key: %w", err) + } + defer func() { _ = key.Close() }() + + if err := repairPathType(key); err != nil { + return err + } + broadcastSettingChange() + return nil +} + +// repairPathType rewrites the PATH under key as REG_EXPAND_SZ. +func repairPathType(key pathKey) error { + value, _, err := readPathValue(key) + if err != nil { + return fmt.Errorf("failed to read User PATH: %w", err) + } + if err := key.SetExpandStringValue("Path", value); err != nil { + return fmt.Errorf("failed to update User PATH in registry: %w", err) + } + return nil +} diff --git a/src/internal/path/registry_windows_test.go b/src/internal/path/registry_windows_test.go new file mode 100644 index 0000000..82ec95a --- /dev/null +++ b/src/internal/path/registry_windows_test.go @@ -0,0 +1,225 @@ +//go:build windows + +package path + +import ( + "errors" + "fmt" + "os" + "reflect" + "strings" + "testing" + + "golang.org/x/sys/windows/registry" +) + +// fakePathKey is an in-memory pathKey holding one Path value. +type fakePathKey struct { + value string + valType uint32 + missing bool + readErr error + writes int +} + +func (k *fakePathKey) GetStringValue(name string) (string, uint32, error) { + if k.readErr != nil { + return "", 0, k.readErr + } + if k.missing { + return "", 0, registry.ErrNotExist + } + return k.value, k.valType, nil +} + +func (k *fakePathKey) SetStringValue(name, value string) error { + k.value, k.valType, k.missing = value, registry.SZ, false + k.writes++ + return nil +} + +func (k *fakePathKey) SetExpandStringValue(name, value string) error { + k.value, k.valType, k.missing = value, registry.EXPAND_SZ, false + k.writes++ + return nil +} + +func typeName(valType uint32) string { + switch valType { + case registry.SZ: + return "REG_SZ" + case registry.EXPAND_SZ: + return "REG_EXPAND_SZ" + default: + return fmt.Sprintf("type %d", valType) + } +} + +func TestWritePathValue(t *testing.T) { + tests := []struct { + name string + original uint32 + value string + want uint32 + }{ + {name: "REG_EXPAND_SZ stays expandable", original: registry.EXPAND_SZ, value: `C:\bin`, want: registry.EXPAND_SZ}, + {name: "REG_SZ without references stays REG_SZ", original: registry.SZ, value: `C:\bin;C:\tools`, want: registry.SZ}, + {name: "REG_SZ with a reference becomes expandable", original: registry.SZ, value: `C:\bin;%USERPROFILE%\.dotnet\tools`, want: registry.EXPAND_SZ}, + {name: "a lone percent sign is not a reference", original: registry.SZ, value: `C:\100%\bin`, want: registry.SZ}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + key := &fakePathKey{} + if err := writePathValue(key, tt.value, tt.original); err != nil { + t.Fatalf("writePathValue() error: %v", err) + } + if key.valType != tt.want || key.value != tt.value { + t.Errorf("wrote %q as %s, want %q as %s", key.value, typeName(key.valType), tt.value, typeName(tt.want)) + } + }) + } +} + +func TestReadPathValue_Missing(t *testing.T) { + value, valType, err := readPathValue(&fakePathKey{missing: true}) + if err != nil || value != "" || valType != registry.EXPAND_SZ { + t.Errorf("readPathValue(missing) = (%q, %s, %v), want (\"\", REG_EXPAND_SZ, nil)", value, typeName(valType), err) + } +} + +func TestPrependShimsDir(t *testing.T) { + const shims = `C:\Users\me\.dtvem\shims` + key := &fakePathKey{ + value: `%SystemRoot%\system32;C:\old\.dtvem\shims;C:\tools;` + shims + `;%USERPROFILE%\.dotnet\tools;`, + valType: registry.EXPAND_SZ, + } + + removed, err := prependShimsDir(key, "User", shims) + if err != nil { + t.Fatalf("prependShimsDir() error: %v", err) + } + + want := shims + `;%SystemRoot%\system32;C:\tools;%USERPROFILE%\.dotnet\tools` + if key.value != want { + t.Errorf("PATH = %q, want %q", key.value, want) + } + if key.valType != registry.EXPAND_SZ { + t.Errorf("PATH written as %s, want REG_EXPAND_SZ", typeName(key.valType)) + } + if !reflect.DeepEqual(removed, []string{`C:\old\.dtvem\shims`}) { + t.Errorf("removed = %v, want the stale shims entry", removed) + } +} + +func TestPrependShimsDir_KeepsType(t *testing.T) { + const shims = `C:\Users\me\.dtvem\shims` + tests := []struct { + name string + key *fakePathKey + want uint32 + value string + }{ + {name: "REG_SZ without references", key: &fakePathKey{value: `C:\tools`, valType: registry.SZ}, want: registry.SZ, value: shims + `;C:\tools`}, + {name: "REG_SZ with a reference is repaired", key: &fakePathKey{value: `%SystemRoot%\system32`, valType: registry.SZ}, want: registry.EXPAND_SZ, value: shims + `;%SystemRoot%\system32`}, + {name: "no PATH yet", key: &fakePathKey{missing: true}, want: registry.EXPAND_SZ, value: shims}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, err := prependShimsDir(tt.key, "User", shims); err != nil { + t.Fatalf("prependShimsDir() error: %v", err) + } + if tt.key.value != tt.value || tt.key.valType != tt.want { + t.Errorf("wrote %q as %s, want %q as %s", tt.key.value, typeName(tt.key.valType), tt.value, typeName(tt.want)) + } + }) + } +} + +func TestPrependShimsDir_ReadError(t *testing.T) { + key := &fakePathKey{readErr: errors.New("access denied")} + + _, err := prependShimsDir(key, "System", `C:\shims`) + if err == nil || !strings.Contains(err.Error(), "System PATH") { + t.Errorf("prependShimsDir() error = %v, want a System PATH read error", err) + } + if key.writes != 0 { + t.Error("prependShimsDir() wrote PATH after a read error") + } +} + +func TestRemoveStaleShims(t *testing.T) { + const current = `C:\Users\me\.dtvem\shims` + key := &fakePathKey{ + value: `C:\Users\me\.cargo\bin;C:\old\.dtvem\shims;` + current + `;%USERPROFILE%\.dotnet\tools`, + valType: registry.EXPAND_SZ, + } + + removed, err := removeStaleShims(key, current) + if err != nil { + t.Fatalf("removeStaleShims() error: %v", err) + } + + want := `C:\Users\me\.cargo\bin;` + current + `;%USERPROFILE%\.dotnet\tools` + if key.value != want || key.valType != registry.EXPAND_SZ { + t.Errorf("wrote %q as %s, want %q as REG_EXPAND_SZ", key.value, typeName(key.valType), want) + } + if !reflect.DeepEqual(removed, []string{`C:\old\.dtvem\shims`}) { + t.Errorf("removed = %v, want the stale shims entry", removed) + } +} + +func TestRemoveStaleShims_NothingToRemove(t *testing.T) { + key := &fakePathKey{value: `C:\tools;%USERPROFILE%\bin`, valType: registry.SZ} + + removed, err := removeStaleShims(key, `C:\Users\me\.dtvem\shims`) + if err != nil || len(removed) != 0 { + t.Fatalf("removeStaleShims() = %v, %v; want nothing removed", removed, err) + } + if key.writes != 0 { + t.Error("removeStaleShims() rewrote PATH with nothing to remove") + } +} + +func TestRepairPathType(t *testing.T) { + const value = `C:\tools;%USERPROFILE%\.dotnet\tools` + key := &fakePathKey{value: value, valType: registry.SZ} + + if err := repairPathType(key); err != nil { + t.Fatalf("repairPathType() error: %v", err) + } + if key.value != value || key.valType != registry.EXPAND_SZ { + t.Errorf("wrote %q as %s, want %q as REG_EXPAND_SZ", key.value, typeName(key.valType), value) + } +} + +// TestPrependShimsDir_Registry runs the helper against a real registry +// value: a scratch key under HKCU\Software, never the real environment. +func TestPrependShimsDir_Registry(t *testing.T) { + scratch := fmt.Sprintf(`Software\dtvem-test-path-%d`, os.Getpid()) + key, _, err := registry.CreateKey(registry.CURRENT_USER, scratch, registry.ALL_ACCESS) + if err != nil { + t.Skipf("cannot create scratch registry key: %v", err) + } + t.Cleanup(func() { + _ = key.Close() + _ = registry.DeleteKey(registry.CURRENT_USER, scratch) + }) + + if err := key.SetExpandStringValue("Path", `%SystemRoot%\system32;C:\tools`); err != nil { + t.Fatalf("seeding scratch Path: %v", err) + } + if _, err := prependShimsDir(key, "User", `C:\Users\me\.dtvem\shims`); err != nil { + t.Fatalf("prependShimsDir() error: %v", err) + } + + value, valType, err := key.GetStringValue("Path") + if err != nil { + t.Fatalf("reading scratch Path: %v", err) + } + if valType != registry.EXPAND_SZ { + t.Errorf("scratch Path stored as %s, want REG_EXPAND_SZ", typeName(valType)) + } + if want := `C:\Users\me\.dtvem\shims;%SystemRoot%\system32;C:\tools`; value != want { + t.Errorf("scratch Path = %q, want %q", value, want) + } +}