Use `vi` as default editor, like C-based `sudo`?

[rhelder]

Describe the bug

(Disclaimer: as I say later, I'm aware that this is not a bug per se – I'm flagging it as a 'bug' just because it's closer to a bug than to a feature request.)

The choice to use /usr/bin/editor as the default for visudo (as of #663), rather than vi, can be a bummer if /usr/bin/editor does not exist (as on my system). If you're coming from C-based visudo, it's unexpected to find that /usr/bin/editor is the default. And, if you want to avoid errors that /usr/bin/editor could not be used, you may have to do a lot more typing than you would have to do with C-based visudo (at least if env_editor is disabled in sudoers, or you don't have the relevant environment variables set).

To Reproduce

[Edit: in what follows, note that if you have disabled env_editor, of course you need to whitelist /usr/bin/vi by setting editor to /usr/bin/vi.]

Suppose /usr/bin/editor does not exist on your system, and that you have not yet set editor in sudoers. Also suppose that env_editor is already disabled in sudoers, or you don't have any of the relevant environment variables set. (Also, in everything that follows, I'm supposing that you are not already operating with root privileges, so I always prefix visudo with sudo.) If you run sudo visudo /etc/sudoers, visudo will throw the following error: visudo: specified editor (/usr/bin/editor) could not be used. If you use C-based visudo under the same circumstances (sudo visudo /etc/sudoers), visudo will use vi.

Also, suppose you've already set editor to /usr/bin/vi in sudoers (having avoided errors by editing it with e.g. sudo EDITOR=/usr/bin/vi visudo /etc/sudoers). If you're only going to edit /etc/sudoers with visudo from now on, your problems are over, so that's good. But suppose you're editing a drop-in file in /etc/sudoers.d/, visudo won't pick up on the editor setting from /etc/sudoers – which means that it is necessary to do a lot of typing each time you edit a drop-in file: sudo EDITOR=/usr/bin/vi visudo -f sudoers.d/timeout, or whatever. Again, if you used C-based visudo under the same circumstances (sudo visudo -f sudoers.d/timeout), visudo would use vi.

Expected behavior

This is not a bug, per se – for me, it happens to be an annoyance, but it could be convenient for someone else. I also could easily fix it for myself (e.g., symlink /usr/bin/vi to /usr/bin/editor). The reason I raise the issue, and the reason I think it's closer to a bug report than to a feature request, is its departure from expected behavior. The expected behavior for Rust-based visudo, I think, is to behave more or less like C-based visudo. I will definitely stress the 'more or less' – there's no pressure for Rust-based visudo to be identical to C-based visudo – but for me, using /usr/bin/editor as the default is a big difference between Rust-based visudo and C-based visudo, not a small difference. I have always disabled env_editor, so I am very used to relying on C-based visudo's default fallback onto vi. And given that vi is in the name of visudo, it was a bit of a cold shower to see Rust-based visudo complaining about some /usr/bin/editor I had never heard of (until I searched your Github and found #663).

So although it might just be a matter of personal preference whether Rust-based visudo should fall back on vi or on /usr/bin/editor, there is at least the argument that Rust-based visudo should fall back on vi in order to be consistent with C-based visudo (and, perhaps, because vi is in the name!). Of course, it is totally up to you, and I'd be curious to hear your reasoning for using /usr/bin/editor instead. Thanks for your work on this exciting project!

Environment:

• Linux distribution: Arch Linux
• sudo-rs version: 0.2.10

[bjorn3]

While ogsudo itself does default to vi, it allows the packager to set the default. On Debian based systems the default is set to /usr/bin/editor. I wasn't aware that Arch doesn't override it.

[squell]

I don't think making this change globally would be a good move. Let's be honest, most people, when confronted with vi, would at most know about :q!. If I look at the fn select_editor() function we take quite some pains to find a suitably editor.

That said, if Arch doesn't provide /usr/bin/editor, I agree with you that is a bug in their package of sudo-rs. If they don't have /bin/editor, they should patch defaults/mod.rs to make vi the default editor for Arch Linux, so I would suggest you file the bug there. (CC @bertptrs, the location is const SYSTEM_EDITOR = ...).

What we could do to encourage this, I guess, is to add a unit test that SYSTEM_EDITOR exists.

[squell]

Having looked into this a bit further, I think the choice for /usr/bin/editor is also a remnant from the early days when the goal was explicitly to target Debian/Ubuntu; we should probably change the selection of what the 'system_editor' is by either detecting it at runtime or configuring it in build.rs (which we just got rid of).

[bertptrs]

/usr/bin/editor is a Debianism as far as I can tell so I personally wouldn't try to conform to that for other distros. ogsudo uses the first of:

• $SUDO_EDITOR
• $VISUAL
• $EDITOR
• The first editor configured in editor in the sudoersfile
  • On Debian, this is explicitly set to /usr/bin/editor at configure time.
  • Arch doesn't set this value, and in that case, upstream defaults to vi. (see the --with-editor configure flag)

sudo-rs implements the environment variables then defaults to /usr/bin/vi on everything but Linux, and /usr/bin/editor for Linux.

If there is an override flag to set it to something specific, that would make sense, otherwise, yes, we'd have to patch the source. FWIW there is, at this time, no editor that is guaranteed to be installed on Arch, not even vi, so technically there is no right answer, but falling back to vi is a pretty common decision. Git does so as well for its editor.

[squell]

For visudo, there is also a (admittedly obscure) security mechanism attached to editor choice for visudo (the reasoning being that “sudo visudo” can hide what program is being run as root from the system log), so we can’t just do an arbitrary userfriendly fallback making this choice even harder.

I will try to get this niggle fixed in the upcoming release. I think the fix can be to just use a richer default value for the “Defaults editor” value; we shouldn’t go looking for system fallbacks since only FreeBSD has a guaranteed editor in its base. And besides in the current code base the “system editor” is part of the default “edifor” value so it is not adding anything.

[squell]

Thanks for bringing this to our attention!

[rhelder]

And thanks to you for your quick and helpful response! I think this solution is great. Thanks again for your work on sudo-rs.