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

Sudo alias in shell integration breaks sudo --edit #6754

Closed
kidonng opened this issue Oct 25, 2023 · 6 comments
Closed

Sudo alias in shell integration breaks sudo --edit #6754

kidonng opened this issue Oct 25, 2023 · 6 comments
Labels

Comments

@kidonng
Copy link
Contributor

kidonng commented Oct 25, 2023

Describe the bug

d96fdb8 introduced sudo alias which does not work with sudo --edit (or sudo -e).

The integration will need to check for the flag before adding the environment variable.

To Reproduce
Steps to reproduce the behavior:

  1. Enable shell integration, without no-sudo
  2. Run sudo -e
  3. See error
$ sudo -e /etc/pacman.conf
sudo: you may not specify environment variables in edit mode

Screenshots
N/A

Environment details
N/A

Additional context
N/A

@kidonng kidonng added the bug label Oct 25, 2023
@kovidgoyal
Copy link
Owner

It's a simple alias, changing it to a function and parsing sudo command
line arguments is way more brittle. You have various options:

  1. no-sudo in kitty.conf
  2. Run sudoedit instead of sudo -e
  3. Run sudo vim file instead of sudo -e file

Although I have to say, sudo -e is absurdly insecure. It allows any
process running as the user on the computer to edit the file, since it
works by creating a temp file owned by the user and then copying that
to the actual file.

@kidonng
Copy link
Contributor Author

kidonng commented Oct 25, 2023

It's a simple alias, changing it to a function and parsing sudo command
line arguments is way more brittle.

For this case the solution is simple enough. I can't speak for other shells, but for fish it's to change

if test -n "$TERMINFO"
alias sudo="sudo TERMINFO=\"$TERMINFO\""
end

to

if test -n "$TERMINFO"
    function sudo
        if ! contains -- --edit $argv && ! contains -- -e $argv
            set --prepend argv TERMINFO="$TERMINFO"
        end
        command sudo $argv
    end
end

(The shell integration should better use function instead of alias anyway, but that's another topic)

Although I have to say, sudo -e is absurdly insecure.

I won't argue with that, just the error message is too cryptic unless you already know kitty will alias sudo (as I suppose few people would do that themselves).

@kovidgoyal
Copy link
Owner

Nope, that will break when using sudo to run a command that happens to
take the -e option such as

sudo grep -e whatever

@mil-ad
Copy link

mil-ad commented Nov 1, 2023

Maybe this should go into a separate issue but this integration also breaks wg-quick that interfaces with Wireguard with this error message:

sudo: sorry, you are not allowed to set the following environment variables: TERMINFO

@kovidgoyal
Copy link
Owner

If your sudo is configured to not allow setting TERMINFO at all, then
either reconfigure it or disable sudo aliasing in kitty.conf

@SevereOverfl0w
Copy link

As an alternative to disabling aliasing in kitty.conf you can also enable TERMINFO to be conveyed via sudo by placing this in your /etc/sudoers or (or in my case I was able to place it in /etc/sudoers.d/env_kitty):

Defaults env_keep += "TERMINFO"

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

No branches or pull requests

4 participants