Skip to content
This repository has been archived by the owner on Apr 24, 2020. It is now read-only.

Use SUDO_COMMAND to check for sudo #937

Closed
wants to merge 1 commit into from
Closed

Use SUDO_COMMAND to check for sudo #937

wants to merge 1 commit into from

Conversation

docwhat
Copy link
Contributor

@docwhat docwhat commented Jul 31, 2018

sudo -n true only checks that we could use sudo, not if we are in
a sudo session.

closes #852

This was referenced Aug 2, 2018
@dritter
Copy link
Member

dritter commented Aug 9, 2018

Thanks @docwhat for that PR.

As @onaforeignshore pointed out, the prompt_user segment has a sudo state as well. Could you change that as well?

`sudo -n true` only checks that we _could_ use `sudo`, not if we are in
a sudo session.

closes #852
@docwhat
Copy link
Contributor Author

docwhat commented Aug 10, 2018

@dritter Done.

@dritter
Copy link
Member

dritter commented Aug 10, 2018

Awesome! Will merge that later in #944 today.

@bhilburn
Copy link
Member

@docwhat - Thanks so much for putting this together. I really do appreciate it, and it'll be included in the v0.6.6 release that we will get out the door ASAP.

@dritter - nice job catching that comment from @onaforeignshore in the other PR! That would have been easy to miss, and thanks for staging this for v0.6.6 release =)

@bhilburn
Copy link
Member

Merged into master as part of #944, and will be part of the v0.6.6 release!

@tippl
Copy link
Contributor

tippl commented Nov 14, 2018

This might be kicking a dead thread, sorry about that, but it seems the SUDO state was misunderstood a bit.

Original idea of the state was to have a yellow prompt if the sudo has cached user creds, as such "danger, sudo in this terminal will not ask password" thus the "can i run sudo" implementation.

$SUDO_COMMAND is set by sudo for processes started by sudo, thus does not reflect the functionality.

For it to be set, zsh would have to run in a sudo session, however that means the process has root permissions, so the ROOT state is used instead here, making the SUDO state not used at all.

I do not know if it is possible to implement this feature without incurring the "password is required" line in log, so i am not against removing the feature entirely, just mentioning this here so we don't have dead code that does nothing.

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

Successfully merging this pull request may close these issues.

4 participants