Closed rkitover closed 2 years ago
The ALLUSERSPROFILE
thing is by intention. If nano is started as a non-elevated process by an admin account, IsUserAnAdmin()
returns false, and the user's .nanorc
is used. If nano is started as an elevated process, All Users/.nanorc
is used. This is probably not satisfactory (Administrator/.nanorc
would be better) but it matches the behavior on Linux, where a sudo'd process reads /root/.nanorc
instead of the user's nanorc.
@akuropka Your input here would be appreciated.
The ALLUSERSPROFILE thing is by intention. If nano is started as a non-elevated process by an admin account, IsUserAnAdmin() returns false, and the user's .nanorc is used. If nano is started as an elevated process, All Users/.nanorc is used. This is probably not satisfactory (Administrator/.nanorc would be better) but it matches the behavior on Linux, where a sudo'd process reads /root/.nanorc instead of the user's nanorc.
Consider the usual case, user installs Windows and his user is an admin user, in an elevated shell his home directory is the same but the .nanorc
is not read. This is very confusing, see #34.
How about checking if the directory pointed to by getenv("USERPROFILE")
exists, and if not using getenv("ALLUSERPROFILE")
.
Consider the usual case, user installs Windows and his user is an admin user, in an elevated shell his home directory is the same but the
.nanorc
is not read. This is very confusing, see #34.
It's the same on Linux; a sudo'd nano reads /root/.nanorc
instead of the one of who calls sudo (unless it's root, of course).
As said above, it's probably better to use %USERPROFILE%/../Administrator/.nanorc
here. But anyway, the user's .nanorc should be ignored.
It is not the same on Linux, a Linux user is aware that sudo runs as root, while Windows allows actual admin users with their own profile directory. UNIX ports usually do not do anything special here.
Also the Administrator user is not equivalent to another admin user, they are separate users.
It is not the same on Linux, a Linux user is aware that sudo runs as root, while Windows allows actual admin users with their own profile directory. UNIX ports usually do not do anything special here.
This is correct. However, please take a look at the default .nanorc
file. There is a line saying:
## In root's .nanorc you might want to use:
which suggests a different color scheme be used for nano running as root for distinction.
Also the Administrator user is not equivalent to another admin user, they are separate users.
One notable difference is that if a user logs in as Administrator, they by default create elevated processes using the Run command.
One reason for not using Administrator/.nanorc
is that this account is disabled by default, and does not have a profile directory unless it is enabled and logged in once.
I just want my nanorc
to work, regardless of whether I am in an unelevated or elevated shell, without having to place copies or symlinks to files in weird places on the filesystem. I suspect other people have an interest in this too.
which suggests a different color scheme be used for nano running as root for distinction.
Is this really important right now? Perhaps in the future, when your project makes an installer package with a default nanorc
for elevated users in a standard location. But in the current situation, this is a huge inconvenience for no gain.
One reason for not using Administrator/.nanorc is that this account is disabled by default, and does not have a profile directory unless it is enabled and logged in once.
And for the reason that Administrator
is not equivalent to root
on UNIX. Few people even use this account anymore in the last two decades.
As getenv("ALLUSERSPROFILE")
on my system is C:\ProgramData
it would make more sense to use %ALLUSERSPROFILE%\nano\nanorc
as the equivalent of /etc/nanorc
like OpenSSH for Windows does, and make a default %ALLUSERSPROFILE%\nano\elevated_nanorc
for elevated users but use it ONLY if they don't have a profile nanorc
of their own.
I am wondering why not just using USERPROFILE
since this always points to the current user matters not if elevated or not. At this point we need to acknowledge the elevation system of Windows is different to the superuser of Linux.
However, I created a batch wrapper starting nano
with the --rcfile
option. This works out of the box, matters not which environment or if elevated or not.
However, I created a batch wrapper starting nano with the --rcfile option. This works out of the box, matters not which environment or if elevated or not.
That's what I'll end up doing for my installer script and possibly the chocolatey package if I ever update that if I can't get any traction here.
It is necessary to have a distinct color scheme for elevated instances. This is not specific to nano; almost all modern text editors have (Administrator)
annotation in their titlebars, if run elevated. The Linux behavior may be unintuitive, but there has to be a solution, rather than nothing at all.
Ok, but what is your solution to the much more annoying problem of your nanorc not being read in an elevated shell.
Why don't you make a copy of your nanorc to where an elevated instance will read, just like what others do on Linux?
Because I don't want to maintain or install two copies, and there is no reason to do this. I had some suggestions for more reasonable locations and behavior above.
Because I don't want to maintain or install two copies, and there is no reason to do this. I had some suggestions for more reasonable locations and behavior above.
While I understand your emergency, this behavior does not come without a reason. If maintaining two copies of nanorc is an issue for you, please file a feature request at https://savannah.gnu.org/bugs/?group=nano, or send a message to nano-devel@gnu.org.
While I understand your emergency, this behavior does not come without a reason. If maintaining two copies of nanorc is an issue for you, please file a feature request at https://savannah.gnu.org/bugs/?group=nano, or send a message to nano-devel@gnu.org.
This has nothing to do with Linux or nano as I explained in great detail here already. The behavior of your port is wrong and your code is wrong. Why are you purposefully insisting on a huge inconvenience for your users? There isn't even any documentation for this.
Because I don't want to maintain or install two copies, and there is no reason to do this. I had some suggestions for more reasonable locations and behavior above.
While I understand your emergency, this behavior does not come without a reason. If maintaining two copies of nanorc is an issue for you, please file a feature request at https://savannah.gnu.org/bugs/?group=nano, or send a message to nano-devel@gnu.org.
It is not clear which exact targets are supported by the upstream project. They will have legitimate reasons to reject any changes relying on the targets not ever supposed to be support officially, unless you merge this project to the upstream.
it matches the behavior on Linux, where a sudo'd process reads
/root/.nanorc
instead of the user's nanorc.
You are talking about sudo
, not su
. Are you really sure this is independent to sudo
's options and/or the configurations in /etc/sudoers
?
It is true that impersonating in an elevated process would be dangnerous for an editor which may occasionally overwrite some files unexpectedly, but the problem of the resulted file permissions in such cases should already be advertised and users should have been educated to use it anywhere. Ruling out the user profile directory (indicated by HOME
, USERPROFILE
or whatever) by enforcing its change during the impersonating by default seems stupid in general. This is just like that you refuse to use scoping rules (with inheriting) in resolving varaible names in the progamming language by default, but eval
with explicit environment (or implicitly the global environment, like root
for su
and sudo
) everywhere instead. (OK, there are actual languages do this, e.g. JavaScript, Lua, and many shell languages.)
The default behavior (with default configurations) chosen by sudo
is specific to itself. This is never a system-wide requirement. Except to be closer to su
, I see it inconvenient to many cases, where users actually want a impersonated shell instead a real su
session.
The real issue here is that this original code is a misguided effort to match the behavior on Linux, but is wrong because Windows works differently.
On Linux, sudo -E
will preserve the value of HOME
but the real home directory as returned by getpwent()
etc. will still be /root
.
On Windows, an admin user that is elevated does not become another user, he is simply elevated and both USERPROFILE
and his profile directory as returned by Windows APIs remains the same, hence the default home directory should remain USERPROFILE
.
Please fix this, either merge my PR or do your own fix. I can work with you on improving the whole situation with finding the config file, if you like. I had some suggestions above.
The real issue here is that this original code is a misguided effort to match the behavior on Linux, but is wrong because Windows works differently.
How do you define 'wrong'? Something that looks wrong to you might not look so to others.
On Linux,
sudo -E
will preserve the value ofHOME
but the real home directory as returned bygetpwent()
etc. will still be/root
.
This is valid. There are indeed issues in the current approach: All Users
might be a bad candidate for elevated processes, and relying on environment variables is unsafe.
But, as explained many times already, it is necessary for an elevated instance to load nanorc from a location that does not depend on the current user (one possible candidate could even be C:\WIndows\.nanorc
).
On Windows, an admin user that is elevated does not become another user, he is simply elevated and both
USERPROFILE
and his profile directory as returned by Windows APIs remains the same, hence the default home directory should remainUSERPROFILE
.Please fix this, either merge my PR or do your own fix. I can work with you on improving the whole situation with finding the config file, if you like. I had some suggestions above.
The XDG_CONFIG_HOME
part looks good to me; I suggest you remove the code path not in _WIN32
though. The other won't be accepted. If you don't like it you are free to fork.
The real issue here is that this original code is a misguided effort to match the behavior on Linux, but is wrong because Windows works differently.
How do you define 'wrong'? Something that looks wrong to you might not look so to others.
This looks wrong to anyone but you, because you refuse to admit that your code is wrong and needs to be fixed.
It's wrong because:
On Linux,
sudo -E
will preserve the value ofHOME
but the real home directory as returned bygetpwent()
etc. will still be/root
.This is valid. There are indeed issues in the current approach:
All Users
might be a bad candidate for elevated processes, and relying on environment variables is unsafe.
ALLUSERSPROFILE
is actually C:\ProgramData
, it should be C:\ProgramData\nano
, and that should be where the equivalent of /etc/nanorc
is.
Why is using an environment variable unsafe? You are just using another environment variable. If you prefer, you can use an API to get the user's profile directory.
But, as explained many times already, it is necessary for an elevated instance to load nanorc from a location that does not depend on the current user (one possible candidate could even be
C:\WIndows\.nanorc
).
Why should it not depend on the current user, that's exactly what it does on Linux. The root user uses root's home directory.
On Windows, an admin user that is elevated does not become another user, he is simply elevated and both
USERPROFILE
and his profile directory as returned by Windows APIs remains the same, hence the default home directory should remainUSERPROFILE
. Please fix this, either merge my PR or do your own fix. I can work with you on improving the whole situation with finding the config file, if you like. I had some suggestions above.The
XDG_CONFIG_HOME
part looks good to me; I suggest you remove the code path not in_WIN32
though. The other won't be accepted. If you don't like it you are free to fork.
That would still use the user's profile directory, C:\Users\<user>\AppData\Roaming\nano
, without the other part it makes no sense. I removed the #ifndef
.
I don't have time to fork this crap, I have enough to do, I was only interested in writing an installer script and possibly updating the chocolatey package, if this pile of crap would even be usable and not subject to incomprehensible undocumented behavior.
Bad enough it doesn't support Unicode, for an editor that's pretty bad.
This looks wrong to anyone but you, because you refuse to admit that your code is wrong and needs to be fixed.
It's wrong because:
- It makes no sense.
- No other UNIX port does this.
- It does not enhance security in any way.
It's too subjective. I mostly work on Linux, and it is reasonable to tell whether the editor is elevated (checking the the EUID) or not; OTOH, which UID that has effected the process is less important. I am not saying that sudo
leave the UID unchanged; looking at nano source, there is no reference to getuid()
, only geteuid()
.
Note it is possible to change the EUID. You may try this one yourself:
lh_mouse@lhmouse-xps ~ $ sudo UID=$UID EUID=0 bash -p
lhmouse-xps /home/lh_mouse # echo $UID $EUID
1000 0
This starts an elevated shell like on Windows. The real UID is me. I can start nano without becoming root, and it still reads /root/.nanorc
.
So, an instance, whose UID is not root, but is impersonating root, reads the root's nanorc. It's probably less common, but it does exist.
Why is using an environment variable unsafe? You are just using another environment variable. If you prefer, you can use an API to get the user's profile directory.
Because I don't think it is unsafe here. Someone else may think so.
That would still use the user's profile directory,
C:\Users\<user>\AppData\Roaming\nano
, without the other part it makes no sense. I removed the#ifndef
.
Thank you for the update.
I don't have time to fork this crap, I have enough to do, I was only interested in writing an installer script and possibly updating the chocolatey package, if this pile of crap would even be usable and not subject to incomprehensible undocumented behavior.
Bad enough it doesn't support Unicode, for an editor that's pretty bad.
MSVCRT doesn't support UTF-8. In order to get UTF-8 support it is necessary to link against UCRT and attach a manifest. Haven't tried it though.
This starts an elevated shell like on Windows. The real UID is me. I can start nano without becoming root, and it still reads
/root/.nanorc
.So, an instance, whose UID is not root, but is impersonating root, reads the root's nanorc. It's probably less common, but it does exist.
This is not the same thing as an elevated shell on Windows. Windows does not change the UID or EUID, if it even has the concept of EUID, the user remains the same but runs with admin privileges.
I don't see how any of this is relevant.
Vim/NVim on Windows does not do anything like this, nothing does anything like this.
Why is using an environment variable unsafe? You are just using another environment variable. If you prefer, you can use an API to get the user's profile directory.
Because I don't think it is unsafe here. Someone else may think so.
You first said it's unsafe now you're saying it's safe.
MSVCRT doesn't support UTF-8. In order to get UTF-8 support it is necessary to link against UCRT and attach a manifest. Haven't tried it though.
MSYS2 now has the UCRT64 environment for using MINGW with UCRT.
https://www.msys2.org/docs/environments/
Usage is almost exactly the same as MINGW64.
Ok, I'm closing this.
I recommend making this change, but for my purposes I'll just find the config in a .bat
file.
If you want help redoing the config locations more sanely, regardless of whether you want to use a separate admin config or not, I made some suggestions above, let me know.
Would please propose a PR for the change for XDG_CONFIG_HOME
? Actually I was unsure what it meant and left it alone; but your commit was of course explanatory and desirable, and shall be appreciated.
I will open an issue with some suggestions, and we can go from there.
Only use ALLUSERPROFILE for home dir when USERPROFILE is not set, even for admin users. Fix #34.
Also on Windows when looking for the rc, look in getenv("APPDATA") instead of getenv("XDG_CONFIG_HOME"), which is mostly equivalent.
Signed-off-by: Rafael Kitover rkitover@gmail.com