3
\$\begingroup\$

I have a server where shell commands to the supervisor-service must be executed by the specific user lx. I have written a bash function that does exactly that.

function supervisor {
if [ `id -nu` != lx ]; then
    sudo su - lx -c "supervisor \"\$@\"" -- supervisor "$@"
else
    sh -s "$@" <<EOF
supervisor "$@"
EOF
fi
}

It works, but it's doesn't look clean and I'm not really comfortable with half the things I'm using here.

How can that function be written in a less convoluted way?

\$\endgroup\$
3
\$\begingroup\$

The only simpler way to do this I've come up with is just to sudo every time and not worry about whether you're already the right user or not. But overall it seems pretty straight forward and it should work fine as-is. Some other suggestions:

  • Indent your code with the function. Leaving the heredoc alone is ok.
  • Use [[ for conditionals. See https://unix.stackexchange.com/a/306115/79839 for more context.
  • It is more modern to use $() for subcommands instead of backticks (``). Parenthesis () are also used for creating subshells so the $() is just substituting the result of a subshell.
\$\endgroup\$
  • \$\begingroup\$ I don't think that Unix & Linux answer provides any argument that [[ should be preferred to [. I'd argue against using [[ if that's the only Bashism (which it can be, if we remove the unnecessary function keyword). Then we can make it a plain POSIX shell script and let /bin/sh execute it (for portability and efficiency). \$\endgroup\$ – Toby Speight Feb 21 at 15:10
  • \$\begingroup\$ The [[ syntax came from ksh and it is a good habit to get into. Maintainability matters more to me than portability to things that don't have bash or ksh. \$\endgroup\$ – chicks Feb 21 at 15:52
  • 1
    \$\begingroup\$ In what way do you consider [[ better than [? Most of the time, I find its unusual parsing rules annoying rather than helpful (e.g. variable names expanded without any $). \$\endgroup\$ – Toby Speight Feb 21 at 16:30
  • 1
    \$\begingroup\$ I still think avoiding the problem of empty variables causing missing argument errors is a good idea. tothenew.com/blog/… and github.com/progrium/bashstyle agree with me. \$\endgroup\$ – chicks Feb 21 at 17:50
  • 1
    \$\begingroup\$ Running all commands with sudo won't let me start supervisor as lx. But it must be lx to correctly run supervisor. That is the reason why I've written that function to start with. \$\endgroup\$ – MadMike Feb 22 at 12:53
2
\$\begingroup\$

I think it's better to simply give an error if it's invoked as the wrong user (or even better, not provide the function at all, similar to hiding admin programs away from non-root users in /sbin rather than /bin). That allows a user to then consider whether they are typing into the right terminal (and any other issues), and if appropriate, issue their own sudo or fakeroot.

Consider this version (with the single Bashism removed, to make a portable script, and with shellcheck errors addressed):

supervisor() {
    if [ "$(id -nu)" != lx ]
    then
        echo "supervisor: can only be run by user 'lx'" >&2
        return 1
    fi

    command supervisor "$@"
}

I removed the use of the pointless inner shell - that just serves to expand arguments a second time (similar to using $* instead of "$@") which probably isn't what you want. Instead, I use the command builtin to make the shell find a non-function version of supervisor.


If you really must invoke sudo from within the script, then we can simplify a bit. If users are allowed to sudo an arbitrary command (which they should be able to, if they can sudo su), then we can make the function invoke it like this:

sudo -u lx -- supervisor "$@"

That seems cleaner than creating a shell and funnelling arguments through it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.