quickstart.sh: unintentional command execution?

[rjendoubi]

quickstart.sh includes the following checks:

[...]

if [ -n "${CLDR_PYTHON_PATH}" ]; then
  echo "Path to custom Python sourcecode supplied as ${CLDR_PYTHON_PATH}, setting as System PYTHONPATH"
  PYTHONPATH="${CLDR_PYTHON_PATH}"
else
  echo "'CLDR_PYTHON_PATH' is not set, skipping setup of PYTHONPATH in execution container"
fi

echo "Checking if ssh-agent is running..."
if pgrep -x "ssh-agent" >/dev/null
then
    echo "ssh-agent OK"
else
    echo "ssh-agent is stopped, please start it by running: eval `ssh-agent -s` "
    #eval `ssh-agent -s`
fi

echo "Checking OS"
if [ ! -f "/run/host-services/ssh-auth.sock" ];
then
   if [ -n "${SSH_AUTH_SOCK}" ];
   then
        SSH_AUTH_SOCK=${SSH_AUTH_SOCK}
   else
    echo "ERROR: SSH_AUTH_SOCK is empty or not set, unable to proceed. Exiting"
    exit 1
   fi
else
    SSH_AUTH_SOCK=${SSH_AUTH_SOCK}
fi

[...]

The first check looks for an env var being set, and expliticly skips some setup if that env var is not found, i.e. that setting is explicitly optional. Fine.

The third check looks for an env var being set, and explicitly exits with an error if it's not present. Also fine.

The second check is to see if ssh-agent is running. If it is not running, the code looks like it intends to print a helpful error message for the user, to empower the user to manually do something before coming back to try this script again. There are a couple of issues here:

1. If ssh-agent is required, shouldn't the else block here explicitly exit 1, like the next check for $SSH_AUTH_SOCK does? (Otherwise, the intention would be clearer if it stated explicitly that it's carrying on regardless, like the first check does, but I don't think that's the idea here).
2. (main issue) Because the backticks aren't escaped, printing the error message actually runs the ssh-agent -s command. Pretty sure that's not what's intended, since in that case the actual message to the user doesn't make sense. Also, judging by the fact the next line is a commented-out version of the same command, seems it was considered to do automatically, but chose not to. Again, this is where I think an explicit exit 1 should be here instead.

If my read is right, happy to submit a PR?

[Chaffelson]

I think you are correct - it should detect that the ssh-agent is not running, give instruction to start it, and exit with a 1.
@wmudge what do you think?

[wmudge]

Absolutely!