feat(installer): add github token support

[jpeeler]

This adds support for utilizing the env var GITHUB_TOKEN if it exists for both curl/wget download utils.
fixes #2664

I think this would be a nice way to help alleviate rate limiting issues for those who can provide a token. My only concern is that perhaps some people may be upset if they are running this script unpiped and are utilizing github actions such that the -x output shows the token in some logs. Otherwise, perhaps this is a nice feature enhancement?
Edit: this is handled now.

Would be nice to not have to fork the installer, though it seems that changes are pretty rare.

[casey]

This seems like a nice improvement! I think it definitely needs to be kept out of output. Ideally it should also be kept out of command arguments as well, since it could be observed by ps. I'm not sure what the best way to do this is.

[jpeeler]

What do you think now? It uses a config file to avoid ps output and turns off xtrace for the temp config file writing that only occurs when GITHUB_TOKEN is set.

[casey]

I'm still not super comfortable with this as is. It's better to keep it out of logs, but it's not good to be writing it to a file. And the code is getting a little crazy. I think maybe we should just pass it as an argument, and make sure that command echoing is turned off.

[jpeeler]

Less crazy?

[casey]

Yah, still a little crazy ;)

I think it's probably a bad idea to pass GIHUB_TOKEN to install.sh, because that actually makes it more likely, I think, that the token will wind up in CI logs.

So, here's what I would like to see:

• This PR's modifications should be local to the download() function
• wget and curl use the same syntax for --header, so we can use the same variable for both, which is either set to the empty string (no auth) or --header=….
• I think we should automatically pass the auth token if it's found

So, to sum up, something like:

download() {
  url="$1"
  output="$2"
  
  set +x

  if [ -v GITHUB_TOKEN ]; then
    authorization="--header='Authorization: Bearer $GITHUB_TOKEN'"
  else
    authorization=""
  fi

  if command -v curl > /dev/null; then
    curl --proto =https --tlsv1.2 -sSfL "$authorization" "$url" "-o$output"
  else
    wget --https-only --secure-protocol=TLSv1_2 --quiet "$authorization" "$url" "-O$output"
  fi

  if [ -n "${GITHUB_ACTIONS-}" ]; then
    set -x
  fi
}

[jpeeler]

I could not get your version to quite work correctly. I read that "wget tries to interpret any non-option argument as a URL" so was seeing a spurious "Prepended http:// to ''" when the token wasn't set. And then curl's parsing also seemed to not like an empty argument, so I gave up and conditioned it all. If this is unacceptable I'll just fork. (But I do think that it's not a coincidence that this issue came up from somebody else - I assume github changed their rate limiting.)

[casey]

@jpeeler I believe that the Prepended http:// to message can be avoided.

If you have an empty variable which is unquoted, then it won't be passed as an argument at all. args is a script which prints the arguments it is passed. In the first call, $x is unquoted, so args doesn't see it at all. In the second call, $x is quoted, so args receives the empty string:

x=
$ args $x
0: /Users/rodarmor/bin/args
$ args "$x"
0: /Users/rodarmor/bin/args
1:

So we could use an unquoted string variable. However, I think the best answer is actually to use an array variable, conditionally push the additional --header argument, and then unconditionally supply the array variable to the invocation.

[jpeeler]

I can't get any combination that works correctly for both when the token is defined and when it isn't (aside from my latest revision). Also, array variables are bash only I think.

[jpeeler]

Hrm, I just now noticed that the standard install command line you have documented is actually using bash. I hadn't been testing that way, so I'll adjust and try yet again to get this right.

[jpeeler]

@casey Simple/good enough?

[jpeeler]

@casey One more ping since I think you do want this little feature.