nabijaczleweli / cargo-update

A cargo subcommand for checking and applying updates to installed executables
MIT License
1.22k stars 42 forks source link

'unknown error occurred': It implicitly requires to be run as Administrator on Windows #11

Closed liigo closed 7 years ago

liigo commented 7 years ago
C:\Users\liigo>cargo install-update
error: An unknown error occurred

To learn more, run the command again with --verbose.

C:\Users\liigo>cargo install-update --verbose
error: An unknown error occurred

To learn more, run the command again with --verbose.

If run it as administrator, it works.


Update: to know the reason and workaround, see comments after this

nabijaczleweli commented 7 years ago

Interesting, but I don't have a way to test that (due to how broken Windows is I somehow am the built-in admin account). We don't do anything that should require admin privs, though, either...

nabijaczleweli commented 7 years ago

Are you on the latest version?

liigo commented 7 years ago

yes, i met this error immediately after cargo install ...

nabijaczleweli commented 7 years ago

What happens if you pass -c as somewhere 100% public?

mzji commented 7 years ago

Weird. Don't have this problem on Win10 64-bit. Which version of Windows do you use?

nabijaczleweli commented 7 years ago

I'm on Win10 x86_64, too, so that might manifest only on earlier versions 🤔

liigo commented 7 years ago

I'm on Win10 x86_64 (build 1607), not an earlier version.

mzji commented 7 years ago

I'm on build 1607 too... @nabijaczleweli Do we write into any directory other than %USERPROFILE% and temporary directory on Windows?

nabijaczleweli commented 7 years ago

Well, we don't even write it, we open $CARGO_HOME/.crates.toml and $CARGO_HOME/registry/index for reading. All write ops are handled by cargo install subprocesses.

Edit: Okay, I lied, we rename and create a file in $CARGO_HOME/bin on self-updates (but so does our cargo install subprocess).

mzji commented 7 years ago

Usually, on windows, the $CARGO_HOME directory is %USERPROFILE%\.cargo. I don't think accessing files in the profile directory of a user requires administrator privilege. @liigo Do you install cargo elsewhere, like, at the root directory of C drive?

liigo commented 7 years ago

My cargo home is C:\Users\liigo\.cargo, installed by rustup.rs by default.

Yesterday I did a fresh install of cargo-update, i.e. uninstall & reinstall (cargo install ...), the same error occurred.


Just now I found that the icon of cargo-install-update.exe is very different to other .exes in C:\Users\liigo\.cargo\bin:

cargo-install-update


dir /Q list the .exes owner account:

C:\Users\liigo\.cargo\bin>dir /Q
 驱动器 C 中的卷是 C本地磁盘
 卷的序列号是 3639-D54A

 C:\Users\liigo\.cargo\bin 的目录

2016/11/14  16:15    <DIR>          liigo-PC\liigo         .
2016/11/14  16:15    <DIR>          liigo-PC\liigo         ..
2016/11/10  09:27         1,376,768 BUILTIN\Administrators cargo-add.exe
2016/11/10  09:14           210,944 BUILTIN\Administrators cargo-fmt.exe
2016/11/14  16:15         1,184,768 liigo-PC\liigo         cargo-install-update.exe
2016/11/10  09:27           931,840 BUILTIN\Administrators cargo-list.exe
2016/11/10  09:27           912,384 BUILTIN\Administrators cargo-rm.exe
2016/11/07  16:46         3,540,480 liigo-PC\liigo         cargo.exe
2016/10/25  11:19         2,839,040 liigo-PC\liigo         lalrpop.exe
2016/11/01  13:41         2,717,696 liigo-PC\liigo         racer.exe
2016/11/10  09:08         1,433,600 BUILTIN\Administrators rg.exe
2016/11/07  16:46         3,540,480 liigo-PC\liigo         rust-gdb.exe
2016/11/07  16:46         3,540,480 liigo-PC\liigo         rust-lldb.exe
2016/11/07  16:46         3,540,480 liigo-PC\liigo         rustc.exe
2016/11/07  16:46         3,540,480 liigo-PC\liigo         rustdoc.exe
2016/11/10  09:14         2,900,480 BUILTIN\Administrators rustfmt.exe
2016/11/01  11:15         1,814,016 liigo-PC\liigo         rustsym.exe
2016/11/07  16:46         3,540,480 liigo-PC\liigo         rustup.exe
2016/11/10  09:29         1,019,904 BUILTIN\Administrators tokei.exe
2016/11/10  09:18           473,600 BUILTIN\Administrators xargo.exe
              18 个文件     39,057,920 字节
               2 个目录 92,901,605,376 可用字节

I'm really sure that, all BUILTIN\Administrators's .exe are created/updated by cargo-update.


Let me know if I need to do something more.

liigo commented 7 years ago

I did a fresh install of cargo-edit too, it runs normally without Administrator privilege. So I don't think it's an issue of cargo install.

C:\Users\liigo>cargo -V
cargo 0.13.0-nightly (9c47815 2016-11-04)

C:\Users\liigo>rustc -V
rustc 1.14.0-nightly (cae6ab1c4 2016-11-05)
liigo commented 7 years ago

I cloned this repo, and cargo build it, the result cargo-install-update.exe do need Administrator privilege to be run.

liigo commented 7 years ago

Note

Hello everyone. Finally I did more investigation, and the result is a bit frustrating.

The short answer is, you can't name your executable as cargo-install-update.exe. (But rename to cargo-up, cargo-instup.exe or similar works normally.)

If an .exe's name contains the word 'install', 'update' or 'setup', it need Administrator privilege to be run.

For example, good name: hello.exe, bad names: hello-install.exe helloinstall.exe hello-update hello-setup hellosetupworld...

Enable or disable Windows Defender doesn't affect the result. I've no other security software installed.

I'm on Windows 10 Home, x86_64, version 1607.

I DON'T KNOW WHY.


Closing. This is not an issue direct related to this repo. May be an issue of Windows itself. Sorry. @nabijaczleweli @mzji

nabijaczleweli commented 7 years ago

If an .exe's name contains the word 'install', 'update' or 'setup', it need Administrator privilege to be run.

For example, good name: hello.exe, bad names: hello-install.exe helloinstall.exe hello-update hello-setup hellosetupworld...

Wow, now that's insane. Thanks for investigating this.

TheCatPlusPlus commented 7 years ago

The binaries need a manifest specifying asInvoker explicitly. All of the autodetection stuff is compatibility. Ping https://github.com/rust-lang/rfcs/issues/721 or something.

As a workaround you should be able to create cargo-install-update.exe.manifest (put it next to the binary) and override the heuristics:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">
    <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
        <security>
            <requestedPrivileges>
                <requestedExecutionLevel level="asInvoker" uiAccess="false"/>
            </requestedPrivileges>
        </security>
    </trustInfo>
</assembly>
nabijaczleweli commented 7 years ago

I'll put a notice for that in the README 👍

mzji commented 7 years ago

My windows is the education version, fresh installed. Don't have this issue. Is your windows upgraded from an old version, @liigo ?

liigo commented 7 years ago

Maybe yes or not. I can't remember that, sorry! It seams that the installer heuristics existed since Windows Vista. http://stackoverflow.com/questions/11573444/why-is-windows-asking-for-system-administrator-privileges-for-running-executable

liigo commented 7 years ago

Manifests may help. Another simple workaround is renaming the repo /.exe (I already hinted this in a previous comment). @nabijaczleweli

nabijaczleweli commented 7 years ago

Renaming's too much of a breaking change to be viable, but when RFC721 lands I'll embed the manifest to remove the need to make one yourself.

mzji commented 7 years ago

Or as for now @nabijaczleweli you could add some code in build.rs to write the manifest file to the bin directory.

nabijaczleweli commented 7 years ago

Into the install dir at install? I didn't think it possible, maybe that's the way to go.

mzji commented 7 years ago

When installing a package via cargo install package, cargo compiles that crate, and copies the built binaries to the bin directory. Since cargo will use the build.rs file (if exists) to build all crates, basically we could add any arbitrary codes to that file to Do Anything We Want (TM). That includes writing manifest file to the bin directory, of course. There are two things should be mentioned: getting the cargo home directory, and the created time and the modified time of the manifest file.

Cargo home directory

Obviously, if there exists $CARGO_HOME, use it, or just use $HOME/.cargo or %USERPROFILE%\.cargo (on Windows).

Created Time and Modified Time

I don't know does there exist functions in the std library which modify these metadata. If not, then we should use some Win32 APIs instead. Looks like we have a C library function to do this :).

P.S.: Creating manifest file for binaries is conceptually a part of build progress on Windows platform, so I think putting the manifest file writing code into build.rs is appropriate here.

TheCatPlusPlus commented 7 years ago

You can just embed the manifest properly with build.rs, e.g. https://github.com/TheCatPlusPlus/rust-manifest-test (though dunno whether you can archive MSVC's RC output and have it work the same way)

nabijaczleweli commented 7 years ago

I sought for manifest in resources but didn't succeed, good to see that it's doable.

mzji commented 7 years ago

@nabijaczleweli The demonstration gist of writing manifest file in build.rs: https://gist.github.com/mzji/9674ef74aad7b83dafdfb18c30b3b3e3 Hope this helps. It is a bit dirty, but absolutely works. Edit: Not dirty anymore.

mzji commented 7 years ago

@TheCatPlusPlus The manifest file we need here is just a kind of static file (the contents won't be changed), so just inlining its content into build.rs is fine.

nabijaczleweli commented 7 years ago

Released in v0.5.1

liigo commented 7 years ago

Embedding a manifest resource can resolve this issue (see comments above).

Done on gnu toolchain, work in progress on msvc toolchain.

Reopening...