Harden installation script

[kvalkhof75]

Is your feature request related to a problem? Please describe.
The jfrog-cli installation scripts fail our QA checks. This is annoying because we are supposed to run downloaded scripts. And you should always check scripts from the internet before running them.
Additionally, they do not run out of the box, but require a chmod +x "${FILE_NAME}" to actually work.

As a customer from JFrog I reported this issue as #264465.
I see the merged pull request #2202.

It did not solve the following (minor) issues:

• shellcheck still reports QA issues.
• Downloads with curl cause annoying characters in logs, preventable with -# or -s -S.
• When the script runs as root, the jfrog executable cannot be run as non-root. This happens in our base images where installation happens as root, but the images run as non-root. Solvable by changing chmod from u+x to +x.

Describe the solution you'd like to see
I propose you change the script into the following:

#!/bin/sh
set -eu

CLI_MAJOR_VER="v2"
VERSION="[RELEASE]"
FILE_NAME="jfrog"
# Order is by destination priority.
DESTINATION_PATHS="/usr/local/bin /usr/bin /opt/bin"

if [ $# -eq 1 ]; then
    VERSION="$1"
    echo "Downloading version ${VERSION} of JFrog CLI..."
else
    echo "Downloading the latest version of JFrog CLI..."
fi
echo ""

if uname -s | grep -q -E -i "(cygwin|mingw|msys|windows)"; then
    CLI_OS="windows"
    FILE_NAME="${FILE_NAME}.exe"
elif uname -s | grep -q -i "darwin"; then
    CLI_OS="mac"
else
    CLI_OS="linux"
fi

MACHINE_TYPE="$(uname -m)"
case "${MACHINE_TYPE}" in
i386 | i486 | i586 | i686 | i786 | x86)
    ARCH="386"
    ;;
amd64 | x86_64 | x64)
    ARCH="amd64"
    ;;
arm | armv7l)
    ARCH="arm"
    ;;
aarch64)
    ARCH="arm64"
    ;;
s390x)
    ARCH="s390x"
    ;;
ppc64)
    ARCH="ppc64"
    ;;
ppc64le)
    ARCH="ppc64le"
    ;;
*)
    echo "Unknown machine type: ${MACHINE_TYPE}"
    exit 1
    ;;
esac

URL="https://releases.jfrog.io/artifactory/jfrog-cli/${CLI_MAJOR_VER}/${VERSION}/jfrog-cli-${CLI_OS}-${ARCH}/${FILE_NAME}"
curl -# -f -g -L -O --url "${URL}"
chmod +x "${FILE_NAME}"

# Move executable to a destination in path.
eval set -- "${DESTINATION_PATHS}"
while [ -n "$1" ]; do
    # Check if destination is in path.
    if echo "${PATH}" | grep -q "$1"; then
        if mv "${FILE_NAME}" "$1"; then
            echo ""
            echo "The ${FILE_NAME} executable was installed in $1"
            exit 0
        else
            echo ""
            echo "We'd like to install the JFrog CLI executable in $1. Please approve this installation by entering your password."
            if sudo mv "${FILE_NAME}" "$1"; then
                echo ""
                echo "The ${FILE_NAME} executable was installed in $1"
                exit 0
            fi
        fi
    fi
    shift
done

echo "could not find supported destination path in \$PATH"
exit 1

Describe alternatives you've considered
I considered re-raising the issue, but this way may just be faster.
I considered creating a pull request, but I have never done such a thing. If you are willing to guide me through the process, then I will do the work.

Additional context

[sverdlov93]

Hi @kvalkhof75,
Thank you for reaching out!
I wanted to let you know that since the last PR you mentioned, we have made another one PR that includes comprehensive ShellCheck fixes and enhancements to our bash scripting.

We are currently in the process of preparing the upcoming release of JFrog CLI, which will incorporate these improvements. If you have any suggestions or feedback, we would greatly appreciate hearing from you!

[kvalkhof75]

Thx. You could consider the changes I demonstrated above. :-)
And I indeed see a lot of improvements in that PR. Good!

[sverdlov93]

The logs without -# looks like this:

Why do you think they are annoying?

[sverdlov93]

@kvalkhof75
I took some of your suggestions and implemented them here - #2227
Will be glad to hear your feedback!
Also, can you elaborate more on uname -s | grep -q -E -i "(cygwin|mingw|msys|windows) vs echo "${OSTYPE}" | grep -q msys ?

[kvalkhof75]

These scripts are almost fully compatible with sh. OSTYPE is not sh compatible. And my suggestion is the sh compatible replacement. (OSTYPE="$(uname -o)", but the -o option is not guaranteed to be supported.) You could even check for Linux os-es by generating an error if grep -q -E -i "(linux|gnu|bsd)" does not match, just like for unsupported architectures.
I used the following documentation for creating the regex list.

If you are sh compatible, you effectively support more exotic and dressed out distributions.

You can verify this by changing the shebang from bash to sh and run shellcheck. It will give you the documentation references. And of course verify full sh compliance.

[sverdlov93]

@kvalkhof75
So you are saying that the following code is the exact same behavior we have today?

if uname -s | grep -q -i msys; then
...
elif uname -s | grep -q -i darwin; then
...
else
...

[kvalkhof75]

The logs from curl look like this on our Gitlab build runners:

Downloading the latest version of JFrog CLI...

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0

 11 25.4M   11 3061k    0     0  2105k      0  0:00:12  0:00:01  0:00:11 2105k
 87 25.4M   87 22.1M    0     0  9238k      0  0:00:02  0:00:02 --:--:-- 19.1M
100 25.4M  100 25.4M    0     0  9962k      0  0:00:02  0:00:02 --:--:-- 19.3M

Which is not that nice. As logs typically do not allow overwriting previous lines. As such I (and who am I ;-) ) always disable progress bars completely (-s -S). But -# will not cause issues. Here is our log output when used:

######################################################################## 100.0%

[kvalkhof75]

If the documentation on the internet as far as I can find it is correct then:
OSTYPE="$(uname -o)"

Meaning that:

if uname -o | grep -q -i msys; then
...
elif uname -o | grep -q -i darwin; then
...
else
...

will give hard equal results. (Mind the -o!)

I used -s instead of -o and added the other matches due to the desire to be more complete based on this documentation.
I can fully understand that being more complete is undesirable for this change.

[sverdlov93]

@kvalkhof75
After more research on the topic, I implemented most of your suggestions, Thanks for the great feedback!
https://github.com/jfrog/jfrog-cli/pull/2227/files

[sverdlov93]

@kvalkhof75
JFrog CLI 2.52.10 released containing most of the topics discussed here. Thanks again!