search

USBGuard / usbguard

USBGuard is a software framework for implementing USB device authorization policies (what kind of USB devices are authorized) as well as method of use policies (how a USB device may interact with the system)
https://usbguard.github.io/
GNU General Public License v2.0
1.1k stars 133 forks source link

scripts: Use bash-builtin `command -v` instead of external `which` #555

Open a1346054 opened 1 year ago

a1346054 commented 1 year ago

command -v is a bash builtin and is a standardized version of which

a1346054 commented 1 year ago

The behavior of command -v regarding functions and aliases should not be a problem because user-defined functions and aliases don't persist into scripts.

Additionally, command -v is not just a bash builtin, it's defined by POSIX, so it behaves the same everywhere.

type -P is not POSIX

hartwork commented 1 year ago

The behavior of command -v regarding functions and aliases should not be a problem because user-defined functions and aliases don't persist into scripts.

True, but someone could add functions right into these scripts. I'm not saying it's super likely to happen but command is not a fit in that regard. It's not a drop-in replacment to which.

Additionally, command -v is not just a bash builtin, it's defined by POSIX, so it behaves the same everywhere.

type -P is not POSIX

Our target is Bash, not POSIX.

a1346054 commented 1 year ago

True, but someone could add functions right into these scripts.

then someone could also do type() { echo "something"; }; and break stuff, so we'd need to protect it by calling:

builtin type -P curl

but it gets really crazy if we assume the scenario that users will edit the provided scripts in crazy ways.

Wonder how to fix it if the user did something like:

type() { echo "something"; };
builtin() { echo "anything"; };
hartwork commented 1 year ago

True, but someone could add functions right into these scripts.

then someone could also do type() { echo "something"; }; and break stuff, so we'd need to protect it by calling:

builtin type -P curl

It doesn't seem to stop there, even builtin() { :; } seems possible, just tried with Bash 5.1.16.

but it gets really crazy if we assume the scenario that users will edit the provided scripts in crazy ways.

I was not speaking of end users. I meant people adjusting these scripts in Git upstream over time.

I think we have both made our points. I would use type -P over command -v because it's more to the point and because we have Bash features available. We can leave the decision to someone with actual merge permissions. @Cropi @radosroka what do you think?

a1346054 commented 1 year ago

I was not speaking of end users. I meant people adjusting these scripts in Git upstream over time.

In that case, it might be of interest to note that the behavior of command -v curl is correct there.

If some maintainer for example does:

curl() { wget "$@"; };

with the intention that calling curl is now overridden, then command -v curl will respect that whereas type -P curl will do something else that the maintainer might not have realized.