From ff770247115b61fd79752c1d4240c25cc8c3cca6 Mon Sep 17 00:00:00 2001 From: Corey Butler <770982+coreybutler@users.noreply.github.com> Date: Mon, 26 Oct 2026 13:47:48 -0500 Subject: [PATCH] fix(cli): put expanded LocalAppData .nodejs on HKCU PATH Electron copies PATH without expanding %LOCALAPPDATA%, so integrated terminals cannot find node/npm. Write only %LOCALAPPDATA%\Author Software\nvm\.nodejs. Signed-off-by: Corey Butler <770982+coreybutler@users.noreply.github.com> --- src/bootstrap/init.go | 7 ++ src/bootstrap/init_test.go | 45 ++++++++++ src/bootstrap/migration.go | 124 +++++++++++++++++++++++++++ src/bootstrap/migration_path_test.go | 71 +++++++++++++++ 4 files changed, 247 insertions(+) diff --git a/src/bootstrap/init.go b/src/bootstrap/init.go index 251ff3a..4aa6dda 100644 --- a/src/bootstrap/init.go +++ b/src/bootstrap/init.go @@ -82,6 +82,13 @@ func EnsureUserProfileInitialized() error { needsRepair = true } + // Electron apps (Cursor, VS Code) copy PATH without expanding %LOCALAPPDATA%. + // HKCU gets the expanded per-user LocalAppData .nodejs path. That location is + // fixed by the installer (user-controlled); custom InstallRoot does not move it. + if err := ensureExpandedUserNodejsPath(); err != nil { + fmt.Fprintf(os.Stderr, "nvm: warning: user PATH .nodejs entry skipped: %v\n", err) + } + // Fast path: profile already healthy. Avoid reshim/ACL/schtasks/seed on every command. // MaintainShimDirectory is a no-op unless Program Files shims are newer than data copies. if !needsRepair && state.version >= currentBootstrapVersion { diff --git a/src/bootstrap/init_test.go b/src/bootstrap/init_test.go index 04ad1fc..e563e7d 100644 --- a/src/bootstrap/init_test.go +++ b/src/bootstrap/init_test.go @@ -303,6 +303,7 @@ func TestEnsureUserProfileInitializedPreservesExistingSyncAssets(t *testing.T) { } func TestEnsureUserProfileInitializedCleansLegacyPayload(t *testing.T) { + snapshotUserEnvironment(t) root := t.TempDir() installRoot := filepath.Join(root, "installs") resetBootstrapState(t) @@ -732,6 +733,50 @@ func assertRegistryValueMissing(t *testing.T, keyPath, valueName string) { } } +func snapshotUserEnvironment(t *testing.T) { + t.Helper() + key, err := winreg.OpenKey(winreg.CURRENT_USER, `Environment`, winreg.QUERY_VALUE) + if err != nil { + t.Fatalf("OpenKey(Environment) error = %v", err) + } + path, pathKind, pathErr := key.GetStringValue("Path") + home, homeKind, homeErr := key.GetStringValue("NVM_HOME") + link, linkKind, linkErr := key.GetStringValue("NVM_SYMLINK") + key.Close() + + t.Cleanup(func() { + env, err := winreg.OpenKey(winreg.CURRENT_USER, `Environment`, winreg.QUERY_VALUE|winreg.SET_VALUE) + if err != nil { + t.Errorf("OpenKey(Environment) restore error = %v", err) + return + } + defer env.Close() + restoreEnvValue(t, env, "Path", path, pathKind, pathErr) + restoreEnvValue(t, env, "NVM_HOME", home, homeKind, homeErr) + restoreEnvValue(t, env, "NVM_SYMLINK", link, linkKind, linkErr) + }) +} + +func restoreEnvValue(t *testing.T, key winreg.Key, name, value string, kind uint32, readErr error) { + t.Helper() + if readErr == winreg.ErrNotExist { + _ = key.DeleteValue(name) + return + } + if readErr != nil { + return + } + var err error + if kind == winreg.EXPAND_SZ { + err = key.SetExpandStringValue(name, value) + } else { + err = key.SetStringValue(name, value) + } + if err != nil { + t.Errorf("restore %s: %v", name, err) + } +} + func readUserPath(t *testing.T) string { t.Helper() key, err := winreg.OpenKey(winreg.CURRENT_USER, `Environment`, winreg.QUERY_VALUE) diff --git a/src/bootstrap/migration.go b/src/bootstrap/migration.go index 6d2212e..3051363 100644 --- a/src/bootstrap/migration.go +++ b/src/bootstrap/migration.go @@ -131,6 +131,13 @@ func removeLegacyCurrentUserEnv(dataRoot string) error { userPath, _, pathErr := key.GetStringValue("Path") if pathErr != nil { if pathErr == winreg.ErrNotExist { + if nodejsPath, ok := canonicalUserNodejsPath(); ok { + ensured, _ := ensureUserNodejsPath("", nodejsPath) + if err := key.SetExpandStringValue("Path", ensured); err != nil { + return fmt.Errorf("failed to write current-user Path: %w", err) + } + changed = true + } if changed { legacy.BroadcastEnvironmentChange() } @@ -141,6 +148,9 @@ func removeLegacyCurrentUserEnv(dataRoot string) error { // Also strip community program-root PATH entries (keep .nodejs). cleaned := filterUserPath(userPath, dataRoot, forceRemove) + if nodejsPath, ok := canonicalUserNodejsPath(); ok { + cleaned, _ = ensureUserNodejsPath(cleaned, nodejsPath) + } if cleaned != userPath { if err := key.SetExpandStringValue("Path", cleaned); err != nil { return fmt.Errorf("failed to rewrite current-user Path: %w", err) @@ -180,6 +190,120 @@ func looksLikeAuthorNvmHome(value string) bool { return strings.HasSuffix(norm, `\author software\nvm`) } +const unexpandedUserNodejsPath = `%LOCALAPPDATA%\Author Software\nvm\.nodejs` + +var serviceProfileLocalAppDataSuffixes = []string{ + `\windows\system32\config\systemprofile\appdata\local`, + `\windows\serviceprofiles\localservice\appdata\local`, + `\windows\serviceprofiles\networkservice\appdata\local`, +} + +func canonicalUserDataRootFrom(lookupEnv func(string) (string, bool)) (string, bool) { + local, ok := lookupEnv("LOCALAPPDATA") + if !ok { + return "", false + } + local = strings.TrimSpace(local) + if local == "" || isServiceProfileLocalAppData(local) { + return "", false + } + return filepath.Join(local, "Author Software", "nvm"), true +} + +func canonicalUserNodejsPathFrom(lookupEnv func(string) (string, bool)) (string, bool) { + root, ok := canonicalUserDataRootFrom(lookupEnv) + if !ok { + return "", false + } + return filepath.Join(root, ".nodejs"), true +} + +func canonicalUserNodejsPath() (string, bool) { + return canonicalUserNodejsPathFrom(os.LookupEnv) +} + +func isServiceProfileLocalAppData(localAppData string) bool { + cleaned := strings.ToLower(filepath.Clean(localAppData)) + for _, root := range serviceProfileLocalAppDataSuffixes { + if strings.HasSuffix(cleaned, root) { + return true + } + } + return false +} + +// ensureUserNodejsPath prepends the expanded .nodejs directory and drops the +// unexpanded %LOCALAPPDATA% form. Cursor/VS Code copy PATH without expanding it. +func ensureUserNodejsPath(userPath, nodejsPath string) (string, bool) { + nodejsNorm := normalizePathMatch(nodejsPath) + unexpandedNorm := normalizePathMatch(unexpandedUserNodejsPath) + hasExpanded := false + kept := make([]string, 0) + for _, seg := range strings.Split(userPath, ";") { + trimmed := strings.TrimSpace(seg) + if trimmed == "" { + continue + } + norm := normalizePathMatch(trimmed) + expanded := normalizePathMatch(os.ExpandEnv(trimmed)) + if norm == unexpandedNorm { + continue + } + if nodejsNorm != "" && (norm == nodejsNorm || expanded == nodejsNorm) { + if !hasExpanded { + kept = append(kept, filepath.Clean(nodejsPath)) + hasExpanded = true + } + continue + } + kept = append(kept, trimmed) + } + if hasExpanded { + out := strings.Join(kept, ";") + return out, out != userPath + } + if nodejsNorm == "" { + return strings.Join(kept, ";"), false + } + out := filepath.Clean(nodejsPath) + if len(kept) > 0 { + out = out + ";" + strings.Join(kept, ";") + } + return out, true +} + +func ensureExpandedUserNodejsPath() error { + nodejsPath, ok := canonicalUserNodejsPath() + if !ok { + return nil + } + key, err := winreg.OpenKey(winreg.CURRENT_USER, `Environment`, winreg.QUERY_VALUE|winreg.SET_VALUE) + if err != nil { + if err == winreg.ErrNotExist { + return nil + } + return fmt.Errorf("failed to open current-user environment key: %w", err) + } + defer key.Close() + + userPath, _, pathErr := key.GetStringValue("Path") + if pathErr != nil { + if pathErr != winreg.ErrNotExist { + return fmt.Errorf("failed to read current-user Path: %w", pathErr) + } + userPath = "" + } + cleaned, changed := ensureUserNodejsPath(userPath, nodejsPath) + if !changed { + return nil + } + if err := key.SetExpandStringValue("Path", cleaned); err != nil { + return fmt.Errorf("failed to rewrite current-user Path: %w", err) + } + legacy.BroadcastEnvironmentChange() + return nil +} + // filterUserPath drops legacy NVM segments and the community program root for dataRoot // while keeping dataRoot\.nodejs. func filterUserPath(userPath, dataRoot string, forceRemove map[string]bool) string { diff --git a/src/bootstrap/migration_path_test.go b/src/bootstrap/migration_path_test.go index 88e0636..93c064f 100644 --- a/src/bootstrap/migration_path_test.go +++ b/src/bootstrap/migration_path_test.go @@ -45,6 +45,77 @@ func pathHasSegment(path, segment string) bool { return false } +func TestEnsureUserNodejsPath_PrependsExpandedAndDropsUnexpanded(t *testing.T) { + root := `C:\Users\a\AppData\Local\Author Software\nvm` + nodejs := filepath.Join(root, ".nodejs") + in := strings.Join([]string{ + unexpandedUserNodejsPath, + `C:\Windows\system32`, + `C:\Tools`, + }, ";") + + got, changed := ensureUserNodejsPath(in, nodejs) + if !changed { + t.Fatal("want PATH rewrite") + } + if !pathHasSegment(got, nodejs) { + t.Fatalf("expected expanded %q in %q", nodejs, got) + } + if pathHasSegment(got, unexpandedUserNodejsPath) { + t.Fatalf("unexpanded %%LOCALAPPDATA%% .nodejs still in %q", got) + } + parts := strings.Split(got, ";") + if normalizePathMatch(parts[0]) != normalizePathMatch(nodejs) { + t.Fatalf("expanded .nodejs should be first, got %q", got) + } +} + +func TestEnsureUserNodejsPath_IdempotentWhenExpandedPresent(t *testing.T) { + root := `C:\Users\a\AppData\Local\Author Software\nvm` + nodejs := filepath.Join(root, ".nodejs") + in := strings.Join([]string{nodejs, `C:\Windows\system32`}, ";") + got, changed := ensureUserNodejsPath(in, nodejs) + if changed { + t.Fatalf("PATH already expanded, got rewrite %q", got) + } + if got != in { + t.Fatalf("got %q want %q", got, in) + } +} + +func TestCanonicalUserDataRoot_UsesLocalAppDataOnly(t *testing.T) { + got, ok := canonicalUserDataRootFrom(func(name string) (string, bool) { + if name == "LOCALAPPDATA" { + return `C:\Users\a\AppData\Local`, true + } + return "", false + }) + if !ok { + t.Fatal("want LocalAppData data root") + } + want := filepath.Join(`C:\Users\a\AppData\Local`, "Author Software", "nvm") + if normalizePathMatch(got) != normalizePathMatch(want) { + t.Fatalf("got %q want %q", got, want) + } + + _, ok = canonicalUserDataRootFrom(func(name string) (string, bool) { + if name == "LOCALAPPDATA" { + return `C:\Windows\ServiceProfiles\LocalService\AppData\Local`, true + } + return "", false + }) + if ok { + t.Fatal("service profile LocalAppData must not be a data root") + } + + _, ok = canonicalUserDataRootFrom(func(string) (string, bool) { + return "", false + }) + if ok { + t.Fatal("empty LOCALAPPDATA must not yield a data root") + } +} + func TestLooksLikeLegacyNvmSymlink(t *testing.T) { if !looksLikeLegacyNvmSymlink(`C:\nodejs`) { t.Fatal("want classic C:\\nodejs")