sudo and commands only in /sbin

[Raizo62]

Hi

Sorry, i come back for an other problem with sudo

With "sudo ls" , "ls" is colorized correctly with green

with "sudo ifup" where "ifup" is only in "/sbin", "ifup" is colorized in red

[danielshahaf]

with "sudo ifup" where "ifup" is only in "/sbin", "ifup" is colorized in red

Known issue. z-sy-h looks for commands in $PATH, but sudoers(5) can define an alternative path for root. Both the path value and the directories specified therein may be unreadable by non-root users, so there's no way to fix this in the general case. Ideas are welcome though.

[Raizo62]

Can z-sy-h define a specific variable used only with "sudo" code of z-sy-h which contains default path of root's commands (as /sbin). If an user defines an alternative path for root in sudoers, then he can modify this variable ?

[danielshahaf]

Can z-sy-h define a specific variable used only with "sudo" code of z-sy-h which contains default path of root's commands (as /sbin). If an user defines an alternative path for root in sudoers, then he can modify this variable ?

Such users would also need to send the command-path style in the :completion::sudo: zstyle context. It would be nice to have completion and z-sy-h both obtain the value from the same place.

[Raizo62]

On my zsh, the completion of "ifup" fails, and the completion of "sudo ifup" successes. How the completion of zsh works ?

I want to install z-sy-h on OS for students in System Administration (Live Raizo), but it's very annoying if it shows a bad information : ifup is correct only behind sudo (if i am not root). And i can't add "/sbin" in the path of user (and it is not realistic)

[danielshahaf]

zstyle ":completion:*:sudo:*" command-path /bin /sbin .... Please ask further completion questions on zsh-users@ or #zsh on FreeNode to distribute the workload.

/sbin is in users' PATH on FreeBSD.

Good luck with your course!

[Raizo62]

zstyle ":completion:*:sudo:*" command-path /bin /sbin .... Please ask further completion questions on zsh-users@ or #zsh on FreeNode to distribute the workload.

Sorry, i believe that i was not been clear : I didn't ask how to do the completion on zsh for /sbin. I said that the completion of zsh can have different actions with the word "sudo" . When i am user, the completion of zsh doesn't show "ifup" , but if i use the word "sudo", zsh shows "ifup" Then my question was : why can z-sy-h not do the same ?

/sbin is in users' PATH on FreeBSD.

It is not in Debian, and i think that is a good thing ;-)

[danielshahaf]

Because that hasn't been implemented. Patches welcome.

[Raizo62]

I can try....

[danielshahaf]

Great! Let's open a new issue to discuss possible approaches.

[Raizo62]

I think to add in "_zsh_highlight_main__type" something as if $(sudo which "$1" >/dev/null) ; then; REPLY=command; fi But this command must be called only if i am in "sudo" command. Perhaps can i add a new parameter at the function _zsh_highlight_main__type ? But do you have already a variable to know if we are in sudo command ?

[danielshahaf]

-1. It is unacceptable to run sudo from z-sy-h.

[danielshahaf]

Knowing whether the command word is sudo is easy.

[Raizo62]

-1. It is unacceptable to run sudo from z-sy-h.

Ok, it's more complicated, but you're right.

And with this code ?

declare -a PATH_ROOT=("/sbin/" "/usr/sbin" "/usr/local/sbin")
if [ -n "$(find "$PATH_ROOT[@]" -maxdepth 1 -name "$1" -print -quit)" ]; then REPLY=command; fi

[danielshahaf]

This is plausible. We can't solve the general case, but a solution that's sound though not complete is worth consideration.

[danielshahaf]

@phy1729 @nicoulaj WDYT?

(Yes, it should be rewritten with a nullglob; I'm asking about the functionality and approach, not implementation.)

[danielshahaf]

@Raizo62 Thanks for putting up with my briefer replies that I sent from the mobile.

[nicoulaj]

Isn't this costly performance wise ? Also, not a big fan of hard coding the path like this, and subtle issues can occur because of the order of PATH entries, so it is seems tricky to get this right.

I think the zstyle approach is the best one (zstyle ":zsh-syntax-highlighting:*:sudo:*" command-path /bin /sbin ...). If you are in this situation you already have to hardcode root's PATH in your zshrc for completion anyway.

[danielshahaf]

Isn't this costly performance wise ? Also, not a big fan of hard coding the path like this, and subtle issues can occur because of the order of PATH entries, so it is seems tricky to get this right.

We could make it a bit fancier with a function that locally does path=( "${(@)path/\/bin/\/sbin}" ), runs 'rehash', and copies $commands to another name... but that's still guesswork and heuristics. How about a much KISSier approach: in 'sudo foo' always make 'foo' not highlighted at all. The rationale is that we can't know what sudoers(5) does: it could both remove and add $PATH entries.

I think the zstyle approach is the best one (zstyle ":zsh-syntax-highlighting:*:sudo:*" command-path /bin /sbin ...). If you are in this situation you already have to hardcode root's PATH in your zshrc for completion anyway.

(To clarify, z-sy-h does not currently honour any zstyles whatsoever, so no one should set that zstyle in their zshrc.)

Sure, that makes sense as a larger / longer-term change.

[Raizo62]

Isn't this costly performance wise ?

Not often costly, if we call this code only when we use "sudo"

The rationale is that we can't know what sudoers(5) does: it could both remove and add $PATH entries.

If user adds PATH entries in sudoes, he can also add them in config of z-sy-h

It is not a perfect solution, but often a good solution

[danielshahaf]

Not often costly, if we call this code only when we use "sudo"

You do realize that z-sy-h's main loop gets called for every single character typed at the prompt? That's why we should avoid stat()s in the main loop.

[Raizo62]

Not often costly, if we call this code only when we use "sudo" You do realize that z-sy-h's main loop gets called for every single character typed at the prompt? That's why we should avoid stat()s in the main loop.

Yes. I have tested the code that i have proposed ;-) Do you prefer a loop on $PATH_ROOT with test '-e' ? And i suggest to do this test at the end of _zsh_highlight_main__type, and only on the word(letter) after a "sudo"

[phy1729]

I'm not terribly fond of special casing sudo. Users could also be restricted to particular binaries even with particular arguments. Then there's doas and pkexec that could also be special cased.

Saying sudo will be treated just like every other precommand is a clear line, but adding this special case leads to more complexity to become more correct.

If we do special case sudo, we should probably use the completion zstyle, so users only have to configure it twice (once for sudo itself too).

[danielshahaf]

I'm not terribly fond of special casing sudo. Users could also be restricted to particular binaries even with particular arguments. Then there's doas and pkexec that could also be special cased.

Saying sudo will be treated just like every other precommand is a clear line, but adding this special case leads to more complexity to become more correct.

Then what do you think about the other option I suggested --- making the 'foo' in 'sudo foo' not highlighted at all? This needn't be a special case for sudo; that rule could be applied to other precommands, such as doas(1) and jexec(1).

On the other hand, the behaviour of master is correct for precommands such as nice(1) and moreutils' chronic(1), so perhaps we should have two classes of precommands: those that resolve their argv[0]-to-be argument in the same way this zsh process would, and those that don't --- but only two classes, not N.

?

[Raizo62]

"sudo" is a command very used and z-sy-h will be a great help for users who write commands.

Other idea to code :

_zsh_highlight_main__type()
{
...
  local OLDPATH="${PATH}"
  PATH="${PATH}:${PATH_ROOT}"

  if zmodload -e zsh/parameter; then
...
  PATH="${OLDPATH}"
...
}

User must define : export ROOT_PATH="/sbin:/usr/sbin:/usr/local/sbin"

[Raizo62]

Could you create a new branch where i can beginning to write a proposition of patch ? I will can do on my own repository, but my second ask will be less easy :

But do you have already a variable to know if we are in sudo command ?

Knowing whether the command word is sudo is easy.

Could you show me how (and where) to do that ? (i can, but i think that will not be very tidy)

[danielshahaf]

For a branch please use one in your own repository and PR here for reviews. However, please remember we haven't yet agreed what we want the functionality to end up being — whether we'd not highlight the ls in sudo ls ever, or whether we'd accept some sort of "root's $PATH" input, and if so whether that'd be via zstyles or via parameters, and so on.

I don't want to discourage you, but I don't want you to burn out or be frustrated either; by all means do feel free to experiment and to sketch out proposals, but remember consensus hasn't yet emerged from this discussion.

To figure out whether sudo was the command word you'd use a state flag and reset it at the end of every simple command, when a | or ; or & or && (or one of their friends) is detected. It'd be somewhere in _zsh_highlight_main_highlighter_highlight_list; there's already code for detecting sudo (you'll see it add :sudo_opt: to the state) and for detecting the end of the simple command (that resets highlight_glob and some other state) that you'll just need to extend.

[danielshahaf]

... and I'm glad to see new people being interested in writing code for z-sy-h. :)

[Raizo62]

I propose a code here : https://github.com/Raizo62/zsh-syntax-highlighting/tree/sudo

If you use z-sy-h as before (without define new varable), you have your old result : sudo ifup # ifup stays red (if ifup is not in your path)

if you define the variable ZSH_HIGHLIGHT_SPECIAL_PATH

ZSH_HIGHLIGHT_SPECIAL_PATH="/sbin:/usr/sbin:/usr/local/sbin"
sudo ifup     # ifup is green

and i believe that the new code is very efficient :

  if [ "$this_word" = ":sudo_opt::start:" ]; then
    local PATH="${PATH}:${ZSH_HIGHLIGHT_SPECIAL_PATH}"
  fi

[danielshahaf]

Thanks for writing that! As I said before, we still have to achieve consensus on what to do before we decide how to do it, but having a proof of concept will be helpful.

[Raizo62]

Yes, i understand.

In your thoughts, don't forget that : sudo is very used and a big distribution like Debian doesn't add "/sbin" in the path of user. With this code :

• you show to user that his command has a bad syntax even behind sudo : "sudo ifpu"
• your indications are less "bad" : "sudo ifup" is not a bad command (even if /sbin is not in your path)

This is a real need.

Also, perhaps that z-sy-h can initialize ZSH_HIGHLIGHT_SPECIAL_PATH if it doesn't detect sbin in the PATH of user ?

[danielshahaf]

• you show to user that his command has a bad syntax even behind sudo : "sudo ifpu"
• your indications are less "bad" : "sudo ifup" is not a bad command (even if /sbin is not in your path)

But on the other hand:

Your patch also causes sudo -u alice ifup to show in green even though it currently shows in red and red is correct.

If ~/bin/foo exists and is in $PATH, then both sudo foo and sudo -u alice foo show in green (both with and without your patch), but they should show in red.

That's why I suggested to give up and show the word as white: because we can't rule out either false positives or false negatives.

This is a real need.

Agreed. It annoys me too; I run into it every time I invoke ifconfig.

Also, perhaps that z-sy-h can initialize ZSH_HIGHLIGHT_SPECIAL_PATH if it doesn't detect sbin in the PATH of user ?

It's technically possible but ultimately it's a heuristic. I'm not fond of this idea, at least not as default behaviour.

[Raizo62]

You are right, but do we use often "sudo -u alice cmd" ? I think that "sudo cmd" is the more used syntax.

You can't code all cases (easily)

I think that is not a good choose to remove a generic and used functionality because few specific cases...

[danielshahaf]

We can't highlight all cases correctly, so we can choose between not highlighting at all, or highlighting and knowing that there might be both false positives and false negatives. Personally, I prefer the former (if the highlighting is sometimes wrong, then the user always has to wonder whether their command line is valid). So, how about:

The command word after sudo shall only be subject to "green if found, red otherwise" highlighting if the user has opted in to such behaviour by specifying the value of $PATH to use in that context. The user-supplied value will be used instead of the environmental value of $PATH for looking up commands in that context.

?

[Raizo62]

So, how about: The command word after sudo shall only be subject to "green if found, red otherwise" highlighting if the user has opted in to such behaviour by specifying the value of $PATH to use in that context. The user-supplied value will be used instead of the environmental value of $PATH for looking up commands in that context. ?

Sorry, i m not sure to understand : by "The user-supplied value", do you speak of a variable like "ZSH_HIGHLIGHT_SPECIAL_PATH" and a code like :

if [ "$this_word" = ":sudo_opt::start:" ]; then
    if [  -n  "${ZSH_HIGHLIGHT_SPECIAL_PATH}" ]
    then
       local PATH="${ZSH_HIGHLIGHT_SPECIAL_PATH}"
    fi
fi

[danielshahaf]

Yes, but phrased more generally since we wouldn't use a scalar parameter but an array parameter/style.

[Raizo62]

Hi

I use this code since my last post without problem :

if [ "$this_word" = ":sudo_opt::start:" ]; then
    if [  -n  "${ZSH_HIGHLIGHT_SPECIAL_PATH}" ]
    then
       local PATH="${ZSH_HIGHLIGHT_SPECIAL_PATH}"
    fi
fi

Could you show me an example of array parameter/style that you want here ?

[danielshahaf]

Could you show me an example of array parameter/style that you want here ?

By default, $path is an array version of $PATH. I'm simply saying it would be cleaner to set $path than to set $PATH.

I can't show you a full example of a style setting because we haven't resolved #362. Please don't worry about styles for now; we can cross that bridge when we come to it. Right now, this issue is still at the stage of agreeing on what the new behaviour should be (see my next-to-last comment).

[Raizo62]

The advantage of my code is that your preferred choose stays the default. You change it only if a user defines the variable ZSH_HIGHLIGHT_SPECIAL_PATH.

[danielshahaf]

That's orthogonal to my feedback.

[Raizo62]

I don't understand : If a user doesn't define the variable "ZSH_HIGHLIGHT_SPECIAL_PATH" (it is the default case), then zsh doesn't highlight commands behind "sudo". It is that you want, doesn't you ?

[danielshahaf]

I don't understand : If a user doesn't define the variable "ZSH_HIGHLIGHT_SPECIAL_PATH" (it is the default case), then zsh doesn't highlight commands behind "sudo". It is that you want, doesn't you ?

That is what I proposed, but it's not what your code does. With your code, if the variable is unset, commands after sudo would be highlighted according to the user's default $PATH.

As I said, the next step is for this thread to consense on what the new behaviour should be. You propose that there should be a new parameter that simply modifies PATH. There are a number of questions about that (summarizing from this thread):

• What about precommands other than sudo?
• What about sudo -u alice ifup? (which wouldn't use root's PATH)
• What about sudoers(5) configurations that also restrict the allowed commands and/or arguments?
• Can we interoperate with the zshcompsys(1) command-path style, that may be already set?
• If we change $PATH, we have to rehash… but if we rehash too often, performance will suffer, and if we rehash too seldom, highlighting will be wrong. How to implement this both correctly and performantly?
• What should the default behaviour be (when no custom paths are set)?

Also, your patch would cause /usr/sbin/grep to be called in preference to /usr/bin/grep, if _zsh_highlight_main__type started calling grep for some reason. I'm not really comfortable with that: the user-specified $PATH should not affect z-sy-h's own code in any way. (Sorry, I should have noticed this sooner.)

[Raizo62]

I don't understand : If a user doesn't define the variable "ZSH_HIGHLIGHT_SPECIAL_PATH" (it is the default case), then zsh doesn't highlight commands behind "sudo". It is that you want, doesn't you ?

That is what I proposed, but it's not what your code does. With your code, if the variable is unset, commands after sudo would be highlighted according to the user's default $PATH.

This is also what your current code does. With this patch, it can do a little more. And it can answer to the very recurrent problem : sudo ifpu Then i am not agree with you and i don't prefer to not highlighting at all.

As I said, the next step is for this thread to consense on what the new behaviour should be.

Yes, but, after 3 months, there is no idea, proposition, arguments :-(

You can also have a variable with a list of the "special" commands. Zsh will show in green the commands in this list, if user uses "sudo". But it is longer to create this list.

[danielshahaf]

With this patch, it can do a little more.

I don't mean to be rude, but I suppose I'd better say it plainly. Your code is not ready to be merged. It doesn't have docs, it doesn't have tests, it doesn't address the concern from the last paragraph of my previous comment, and it doesn't test the bitfield's value correctly. It could form the core of a solution, but it's not there yet.

Then i am not agree with you and i don't prefer to not highlighting at all.

Would you care to explain why you think that would be better?

You can also have a variable with a list of the "special" commands. Zsh will show in green the commands in this list, if user uses "sudo".

Yes, that could work. There's a possible problem here if root's path contains directories that are access(X_OK) but not access(R_OK) to the current user — that is, directories whose contents can be random-accessed but not enumerated — but I don't think that's a common setup.

[danielshahaf]

Yes, that could work. There's a possible problem here if root's path contains directories that are access(X_OK) but not access(R_OK) to the current user — that is, directories whose contents can be random-accessed but not enumerated — but I don't think that's a common setup.

Also, there's the complexity question. Enumerating the root's path is O(N) work where N is the number of binaries in root's path; testing existence of a single binary is O(M log N) where M is the number of directories in root's path… and that's without considering performance of network filesystems.

That is: obtaining the list of permitted commands ahead of time might not work performantly in some setups.

[Raizo62]

With this patch, it can do a little more. I don't mean to be rude, but I suppose I'd better say it plainly. Your code is not ready to be merged. It doesn't have docs, it doesn't have tests, it doesn't address the concern from the last paragraph of my previous comment, and it doesn't test the bitfield's value correctly. It could form the core of a solution, but it's not there yet.

I know. But i don't use my time to write docs or optimize code, if, already, you don't want this idea. And I have not proposed any merge. And this code works for a normal usage. I have tested it.

For your last paragraph, i see solution : When you will have the problem, you can save the original value of PATH before to modify it. After you can restore it before your "grep".

Then i am not agree with you and i don't prefer to not highlighting at all. Would you care to explain why you think that would be better?

Because that will help very often (we do very often "sudo cmd"), and mislead very rarely ("sudo -u alice"). In the second case, we can remove the variable ZSH_HIGHLIGHT_SPECIAL_PATH, or change his value (local folder with files with good name)...

That is: obtaining the list of permitted commands ahead of time might not work performantly in some setups.

The second method doesn't enumerate path. It is only a list of "word". If the first parameter of sudo is in this list then zsh shows it with green. This method can also completed this first ("ZSH_HIGHLIGHT_SPECIAL_PATH")

[danielshahaf]

I know. But i don't use my time to write docs or optimize code, if, already, you don't want this idea.

Sure.

And I have not proposed any merge. And this code works for a normal usage. I have tested it.

Sure.

For your last paragraph, i see solution : When you will have the problem, you can save the original value of PATH before to modify it. After you can restore it before your "grep".

Sorry, but no. It's not good design to require hoop jumping for a common operation that usually requires none.

Because that will help very often (we do very often "sudo cmd"), and mislead very rarely ("sudo -u alice").

Are we talking about the same thing? I wasn't asking you to justify the user-settable knob. I was asking you to justify why sudo foo should highlight foo according to the default PATH when the knob is unset, as opposed to leaving the foo unhighlighted in that case. This behaviour is exactly why sudo ifup doesn't DTRT by default on Debian derivatives.

The second method doesn't enumerate path. It is only a list of "word".

Yes, I understood that. It's just that I anticipate people would set that list to /usr/local/sbin/*(:t) /usr/sbin/*(:t) /sbin/*(:t).

[Raizo62]

Also, your patch would cause /usr/sbin/grep to be called in preference to /usr/bin/grep, if _zsh_highlight_main__type started calling grep for some reason. I'm not really comfortable with that: the user-specified $PATH should not affect z-sy-h's own code in any way. (Sorry, I should have noticed this sooner.)

You can also move the function which detects if a command is known in a new function. This function will do the local PATH.

I was asking you to justify why sudo foo should highlight foo according to the default PATH when the knob is unset, as opposed to leaving the foo unhighlighted in that case. This behaviour is exactly why sudo ifup doesn't DTRT by default on Debian derivatives.

(Sorry, i don't understand the word "DTRT" or found the correct spell) But on Debian, ifconfig is not found without sudo, and works with sudo.
I think that will be a good thing if zsh helps when we use a bad spelling even when we use sudo. Zsh is do for help, doesn't it. If it can do it then why it would not do it?

[danielshahaf]

You can also move the function which detects if a command is known in a new function. This function will do the local PATH.

Yes, that has been suggested upthread. It could work, although thought will need to be put into the cache invalidation question (see the fifth bullet in my comment from yesterday).

(Sorry, i don't understand the word "DTRT" or found the correct spell)

It's an acronym of "do the right thing".

But on Debian, ifconfig is not found without sudo, and works with sudo.

That's neither here nor there; we aren't talking about how to highlight % ifconfig. We're talking about how to highlight % sudo ifconfig by default. You said you thought that should highlight ifconfig in red and I asked you to explain the thinking behind the design choice you proposed.

[Raizo62]

That's neither here nor there; we aren't talking about how to highlight % ifconfig. We're talking about how to highlight % sudo ifconfig by default. You said you thought that should highlight ifconfig in red and I asked you to explain the thinking behind the design choice you proposed.

I wish to highlight ifconfig in green (behind sudo) because the command sudo ifconfig is correct (very often, without "rarely" case as sudo -u alice) : the shell command shows the good result. But if I do a spelling error (sudo ficonfig), the command is uncorrect and zsh can alerte me (with red word) before the shell. If i have no color behind sudo, i must wait the error of the shell to know that i have do an spelling error.

This behaviour is exactly why sudo ifup doesn't DTRT by default on Debian derivatives.

Why do you say that sudo ifup doesn't do the right thing ?

[danielshahaf]

This behaviour is exactly why sudo ifup doesn't DTRT by default on Debian derivatives.

Why do you say that sudo ifup doesn't do the right thing ?

Because it highlights ifup in red...

I wish to highlight ifconfig in green (behind sudo) because the command sudo ifconfig is correct (very often, without "rarely" case as sudo -u alice) : the shell command shows the good result. But if I do a spell error (sudo ficonfig), the command is uncorrect and zsh can alerte me (with red word) before the shell. If i have no color behind sudo, i must wait the error of the shell to know that i have do an spelling error.

You don't have to convince me that the green/red distinction is useful. I agree that it would be nice for 'sudo ifup' to highlight 'ifup' in green. However, I'm asking a different question: what algorithm should 'sudo foo' follow to decide what color to highlight 'foo' in when root's path is NOT configured.