Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elevation broke between 1.1.7 and 1.1.9 #767

Closed
WORMSS opened this issue Mar 1, 2022 · 28 comments
Closed

Elevation broke between 1.1.7 and 1.1.9 #767

WORMSS opened this issue Mar 1, 2022 · 28 comments

Comments

@WORMSS
Copy link
Contributor

WORMSS commented Mar 1, 2022

Issue:
1.1.9 no longer pops up UAC to asks to to change node/npm, it just FAILS straight away with exit status 5: Access is denied

How To Reproduce:

  1. Have version 1.1.7 installed.
  2. nvm use 17.4.0
  3. UAC comes up, do you want to change node (yes)
  4. UAC comes up, do you want to change npm (yes)
  5. (observe everything is good with the world)
  6. Install NVM 1.1.9 from nvm-setup.zip or nvm-update.zip
  7. nvm use 17.6.0
  8. 💥 exit status 5: Access is denied. 💥
  9. 😢 cry because the world is caving in 😢

Expected Behavior:
To do step 3 and step 4 in 1.1.9, just like 1.1.7 used too.

image
Just to note, this does the EXACT same thing in CMD also..

Work around, I open a CMD window as administrator, and it works. But this is a major ballache to do.

Windows: 10.0.19044.1526

@WORMSS
Copy link
Contributor Author

WORMSS commented Mar 1, 2022

Just to note... Installing 1.1.7 makes things work again..

It doesn't matter if you use nvm-update.zip and select "1.1.9" or nvm-setup.zip (1.1.9) they both break.

But rolling back to 1.1.7 works wonderfully
image

@dgchrt
Copy link

dgchrt commented Mar 10, 2022

Work around, I open a CMD window as administrator, and it works. But this is a major ballache to do.

Probably not what you want though, as this will potentially prevent the unprivileged user account from managing it in the future.

@WORMSS
Copy link
Contributor Author

WORMSS commented Mar 10, 2022

Well... yeah... that is why I opened it as administrator... to get around the fact that the unprivileged user account couldn't do it..
I didn't install node as admin.. I just needed admin to use nvm.. Hence it was a ballache.
But as I said.. the best work around I found was installing nvm-windows 1.1.7.. works a treat.

@poindexterous
Copy link

poindexterous commented Mar 10, 2022

I've run into this also, I was upgrading from NVM 1.1.6 to 1.1.9 (upgraded to try and fix some bugs I was running into) and I ran into the same problem as @WORMSS when trying to switch node versions in powershell where the UAC prompt no longer popped up for NVM and gave me the exit status 1: Access is denied. message.

I'm trying to avoid opening up a separate elevated powershell just to switch my node version, but right now it seems like the only way without downgrading. Sadly I can't run everything from an elevated powershell due to user ssh key differences, and user file ownership issues with my everyday work user account and the separate admin account I have (which should only be used for elevation as needed). My company's IT department gets - understandably -nervous if I run everything on the local admin account, they prefer I use it sparingly.

@dgchrt
Copy link

dgchrt commented Mar 11, 2022

Just use 1.1.7 for now, as I've pointed out here: #700 (comment)

@ojintoad
Copy link

ojintoad commented Mar 11, 2022

@diogoeichert That's a good suggestion for those who want to go that route, but there are a number of benefits of using post 1.1.7 releases according to the release notes. A signed installer is a big one for some orgs.

I'm sure Corey is busy with his rt tool - this might be a good item for someone else to try to work out. 🤞

As an additional workaround, there are tools that enable "sudo" like functionality that present UAC prompts. I just tested successfully with gsudo for example:

C:\Users\ojint
> gsudo -d nvm use 10.17.0
Now using node v10.17.0 (64-bit)
C:\Users\ojint
> nvm list

    10.24.1
  * 10.17.0 (Currently using 64-bit executable)
C:\Users\ojint
> gsudo -d nvm use 10.24.1
Now using node v10.24.1 (64-bit)
C:\Users\ojint
> nvm list

  * 10.24.1 (Currently using 64-bit executable)
    10.17.0
C:\Users\ojint
>

Each time I ran gsudo it presented me with a UAC prompt. The tool is sophisticated enough to not reprompt either, but that has its own security risks/concerns. Personally if I were to continue using it I'd try to stick with constant prompting.

There are other similar tools besides gsudo worth considering, though gsudo seems like it's at least more recently maintained. Depending on your situation this may be a decent workaround as well.

@dgchrt
Copy link

dgchrt commented Mar 12, 2022

@ojintoad Nice, I wasn't aware such tools existed for Windows already. Thanks for sharing.

@ojintoad
Copy link

ojintoad commented Mar 14, 2022

I need to go to bed but figure I would speak to how far I got in trying to understand this. I'm not quite sure I want to open a PR. This is the first time I've touched Go as a language and the first time I've touched this library so any assumptions I'm making come with a big caveat.

First, I am slightly confused about what the runElevated command is supposed to try to do. I think that it might have been intended to be slightly differently written and to call itself in a way I'm not sure it does now.

nvm-windows/src/nvm.go

Lines 891 to 897 in 5803c4f

if err != nil {
msg := stderr.String()
if strings.Contains(msg, "not have sufficient privilege") && uac {
return runElevated(command, false)
}
// fmt.Println(fmt.Sprint(err) + ": " + stderr.String())
return false, errors.New(fmt.Sprint(err) + ": " + msg)

I think the intent might have been to have 893 have a !uac check and for 894 to pass in true for the forceUAC parameter. That would sort of make sense - if we failed to run the command without the uac check process, then let's force the issue. I started out trying that and sure enough it entered the earlier block in the method that tries to execute the command a different way.

nvm-windows/src/nvm.go

Lines 866 to 877 in 5803c4f

if uac {
// Alternative elevation option at stackoverflow.com/questions/31558066/how-to-ask-for-administer-privileges-on-windows-with-go
cmd := exec.Command(filepath.Join(env.root, "elevate.cmd"), command)
var output bytes.Buffer
var _stderr bytes.Buffer
cmd.Stdout = &output
cmd.Stderr = &_stderr
perr := cmd.Run()
if perr != nil {
return false, errors.New(fmt.Sprint(perr) + ": " + _stderr.String())
}
}

That hits a new problem when trying to do nvm use. First, the variable command already has the path to elevate.cmd in it from earlier so now it's going to get double included. So you get an error like this:

image

But even if you remove that, it appears to fail to call correctly in a different way since the elevate.cmd really doesn't appear to like having parameters passed in as a straight string. So you get an error like this:

image

Edit - additionally if you just pass the command without passing the joined path to the elevate.cmd file, it still errors in a different way:

exec: "\"[C:\\Users\\ojint\\AppData\\Roaming\\nvm\\elevate.cmd\]()" cmd /C mklink /D \"[c:\\nodejs\]()" \"[C:\\Users\\ojint\\AppData\\Roaming\\nvm\\v10.17.0\]()"": file does not exist:

That somewhat surprises me as I think the other commands do get passed that way, so there's something specific about that command or I'm missing something about how it works.

Regardless of why it's failing, we want to still try to call that command to make the symlink in a way we know that works. We know that 1.1.7 works. If we look at the 1.1.7 version, you can see it calls exec.Command with a list of variable args instead of a pre-assembled string:

c := exec.Command(filepath.Join(env.root, "elevate.cmd"), "cmd", "/C", "mklink", "/D", filepath.Clean(env.symlink), filepath.Join(env.root, "v"+version))

Since I don't quite know the intent of the runElevated method and it clearly works for almost all the other times it's called, I figured it made more sense to avoid changing that. So I walked back up the call stack to where the use method tries to actually create the symlink and patch it there. I ended up with this change starting at around line 562 here:

var ok bool

ok, err = runElevated(fmt.Sprintf(`"%s" cmd /C mklink /D "%s" "%s"`, filepath.Join(env.root, "elevate.cmd"), filepath.Clean(env.symlink), filepath.Join(env.root, "v"+version)))
	if err != nil {
		if strings.Contains(err.Error(), "not have sufficient privilege") {
			cmd := exec.Command(filepath.Join(env.root, "elevate.cmd"), "cmd", "/C", "mklink", "/D", filepath.Clean(env.symlink), filepath.Join(env.root, "v"+version))
			var output bytes.Buffer
			var _stderr bytes.Buffer
			cmd.Stdout = &output
			cmd.Stderr = &_stderr
			perr := cmd.Run()
			if perr != nil {
				ok = false
				fmt.Println(fmt.Sprint(perr) + ": " + _stderr.String())
			} else {
				ok = true
			}
		} else if strings.Contains(err.Error(), "file already exists") {
			ok, err = runElevated(fmt.Sprintf(`"%s" cmd /C rmdir "%s"`, filepath.Join(env.root, "elevate.cmd"), filepath.Clean(env.symlink)))

This seemed to generate the UAC prompt and update the symlink appropriately. I don't think this perfectly captures every error case - if the "file already exists" for example it may not behave quite right or something. I didn't actually ever see that happen in my testing though.

I did notice that as a result I could set an "admin" version and a standard user version distinctly. That is, from an admin prompt, nvm use couldn't influence what nvm use had set for a standard user and vice-versa. This was a little surprising. I'm not sure if the 1.1.7 version currently behaves like that.

I don't really know if this is a good solution or not. I'm gonna stop here but am curious if anyone has any feedback for me. If not, at least I learned some Go.

@1011025m
Copy link

1011025m commented Mar 18, 2022

To add onto this, the error message that this gives has an encoding error (error in an error lol) if you don't use English as the system language. Minor issue, but you know what I mean
image

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale Stale label Apr 18, 2022
@ViliusS
Copy link

ViliusS commented Apr 18, 2022

ping

@github-actions github-actions bot removed the Stale Stale label Apr 19, 2022
@WORMSS
Copy link
Contributor Author

WORMSS commented Apr 26, 2022

  1. Uninstalled nvm completely, confirmed by finding zero %appdata%/.nvm folder

  2. Installed fresh copy of 1.1.9 from Github

  3. opened a new CMD window. (Not admin)

C:\WINDOWS\system32>nvm version
1.1.9

C:\WINDOWS\system32>nvm install latest
Downloading node.js version 18.0.0 (64-bit)...
Extracting...
Complete

Installation complete. If you want to use this version, type

nvm use 18.0.0

C:\WINDOWS\system32>nvm use 18.0.0
exit status 1: Access is denied.

C:\WINDOWS\system32>nvm uninstall 18.0.0
Uninstalling node v18.0.0... done
  1. Uninstalled nvm completely, confirmed by finding zero %appdata%/.nvm folder

  2. Installed 1.1.7

  3. Opened a new CMD window. (Not admin)

C:\WINDOWS\system32>nvm version
1.1.7

C:\WINDOWS\system32>nvm install latest
Downloading node.js version 18.0.0 (64-bit)...
Complete
Creating C:\Users\CRichardson\AppData\Roaming\nvm\temp

Downloading npm version 8.6.0... Complete
Installing npm v8.6.0...

Installation complete. If you want to use this version, type

nvm use 18.0.0

C:\WINDOWS\system32>nvm use 18.0.0
Now using node v18.0.0 (64-bit)

C:\WINDOWS\system32>node --version
v18.0.0

So, the problem between 1.1.7 and later versions is still happening.

ojintoad added a commit to ojintoad/nvm-windows that referenced this issue Apr 27, 2022
@neverendingqs
Copy link

For anyone upgrading past 1.1.7 because of npm/cli#4234 (comment), it seems like you can install with 1.1.9 and then switch with 1.1.7.

I didn't do thorough testing, but if you take nvm.exe from 1.1.9's nvm-noinstall.zip, rename it to nvm119.exe, put it in the same directory as your nvm installation, and then use that to install Node, you can use nvm.exe to switch to it. E.g.

nvm119 install 18.0.0
nvm use 18.0.0

@Maxim-Mazurok
Copy link

Same issue, Win 11, Dev mode enabled, exit status 1: Access is denied. on 1.1.9 and 1.1.8, 1.1.7 works.

@tylerc
Copy link

tylerc commented Jun 8, 2022

I found this helpful: https://github.com/coreybutler/nvm-windows/wiki/Common-Issues#permissions-exit-1-exit-5-access-denied-exit-145

I got rid of this error on my machine by:

  1. Enabling developer mode.
  2. Updating C:\Users\<USERNAME>\AppData\Roaming\nvm\settings.txt to say path: C:\nodejs instead of path: C:\Program Files\nodejs
  3. Updating the environment variable NVM_SYMLINK to C:\nodejs
  4. Closing out the old Command Prompt I was using and opening a fresh one.

This worked for me on a fresh install of Windows 11 21H2 (22000.675).

@thany
Copy link

thany commented Jun 21, 2022

This issue has been reported almost 4 months ago and remains unresolved. Even better, the current release is even older, so the issue has technically existed for well over half a year.

How hard would it be, dear developer, to compare between 1.1.8 and 1.1.7 what has likely caused UAC prompting to stop working? I say revert whatever change caused this bug and be done with it.

I'm not an impatient guy, but half a year is a teeny tiny tad on the long side, for an issue that was provably introduced from one minor version into the next. Do you agree?

@mizu030691
Copy link

mizu030691 commented Jul 12, 2022

Happen have this issue, so I'm using this approach to nvm use. It'll prompt UAC. especially if use powershell.

"start powershell -v runas -Wait -ArgumentList {/c nvm use 10.24.1} -WindowStyle Hidden"

@ojintoad
Copy link

Indirectly related to this - there is a group policy that you can enable for just the permission to create symlinks. I get that developer mode resolves this, but it seems wild to want to enable that over a specific group policy that enables the single permission in particular. That resolved this for me.

https://social.technet.microsoft.com/Forums/windows/en-US/5ccc98e8-8f0b-4929-913f-085e32322050/allow-creation-of-symbolic-links-to-none-administrators

@Treeless
Copy link

After upgrading from 1.1.7, had this permission issue. Attempting to fix, I even tried doing a full uninstall and fresh install of 1.1.9 with a fresh nodejs install. No luck.
image

@thoys
Copy link

thoys commented Aug 4, 2022

Same happening here

@svashish305
Copy link

same, things were fine with 1.1.7, now on 1.1.9 i keep retrying but doesn't work

@thany
Copy link

thany commented Aug 19, 2022

A bug like this makes this project feel abandoned.

Setting up tricks to work around the UAC prompt is insane. A program that requires elevation, should ask for it. Users should not have to muddle around this for months.

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale Stale label Sep 19, 2022
@Treeless
Copy link

Bump. Not stale

@WORMSS
Copy link
Contributor Author

WORMSS commented Sep 20, 2022

I've managed to make quite a nice work around since he has publicly stated he isn't going to fix the issue himself.

My Idea:
#671 (comment)
My proof it works:
#671 (comment)

@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale Stale label Oct 22, 2022
@thoys
Copy link

thoys commented Oct 22, 2022

bump, not fixed yet

@github-actions github-actions bot removed the Stale Stale label Oct 23, 2022
@coreybutler
Copy link
Owner

I believe this is taken care of in v1.1.10. Testing was minimal, but yielded the desired results. If you're still struggling with this, @WORMSS provided a workaround that seems to do the job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

16 participants