Allow configuring the github-token used

[pbrisbin]

This is a pretty common pattern, so I don't know that it requires much justification, but for additional context I needed it to get this action working on a Forgejo Actions runner.

Those runners spoof the github context, meaning the github.token value is actually a token for interacting with the Forgejo repository, which of course results in a 401 when it's used to pull the GitHub artifacts (even though they're public). Being able to swap in my own GitHub token gets things working.

[brokenpip3]

thanks for your contribution, can you please do a small change?
Instead of passing directly the input in the curl command we should pass it as env for security reasons, something like:

  env:
    GITHUB_TOKEN: ${{ inputs.github-token }}
  run: |
    [...] curl -fsSL --retry 4 --retry-connrefused -H "Authorization: token ${GITHUB_TOKEN}" [...]

I need to do it for other inputs as well but since you are changing this one let's start from the good :)

[pbrisbin]

Great point, I was aware of that practice and should've known. IIRC, I think you're not supposed to clobber that actual env var name, but I'll look into it on Monday and make the change one way or another.

[pbrisbin]

Done. I couldn't find any documentation that said to avoid making your own variables named GITHUB_, so I went with your suggestion.

[brokenpip3]

Thanks for your contribution

[waldner]

Do you actually need GITHUB_TOKEN for those curls? My understanding is that the bats-core repo is public, so no token needed. Also, subsequent curl invocations in later steps do not use the token either.

[pbrisbin]

I think it's common for actions to use github.token because it's there and will avoid rate limits you hit when using completely un-authenticated requests. Usually this is a fine assumption, except for the Forgejo practicality. As long as it is only a default that can be overridden, this seems fine and good to me.

[waldner]

Well, yes but then you're still forced to supply a valid github token, which you might not necessarily have if you're working on other platforms like forgejo. Perhaps it could be made entirely optional?

[brokenpip3]

Do you actually need GITHUB_TOKEN for those curls? My understanding is that the bats-core repo is public, so no token needed.

https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#primary-rate-limit-for-unauthenticated-users

Also, subsequent curl invocations in later steps do not use the token either.

Indeed we should add it there as well, I mean in general, not only in case of forgejo

[brokenpip3]

Perhaps it could be made entirely optional?

if you pass github-token set to "" I believe you are going to remove the default which is the github action short lived token (in github case, not forgejo), not entire sure but you can try

[waldner]

Sure, but if a user wants to use unauthenticated requests and risk rate limiting, it's their choice, isn't it?

Also note that the rate limits are for API requests, not for other types of requests.

https://github.com/bats-core/bats-action/blob/main/action.yaml#L145 this requests is done against the API, true, but later requests like eg https://github.com/bats-core/bats-action/blob/main/action.yaml#L218 are not against the API.

The curl that runs against the api is used, in essence, to determine the latest available version of bats; if the bats-version input is supplied, that curl isn't even performed and the tar.gz is downloaded (here https://github.com/bats-core/bats-action/blob/main/action.yaml#L152) without calling the API, so strictly speaking it doesn't need the token either.

In the end, I doubt that a user could reach the max 60 API requests per hour (a single run of bats-action already probably takes more than one minute), and even then, if they want to risk, it should be their choice, IMHO.

[pbrisbin]

I think github-token: "" is part of the solution. The code would have to check, so that it doesn't add "Authorization: token " as the header when the token variable is empty.

[waldner]

Yes, but even so, it's never empty, since by default it's set to ${{ github.token }}

[waldner]

Just to be clear, I'm not suggesting to remove the use of the token altogther; in fact, I would leave the default as it is now, which is good; only offer an optional way to opt-out from using the token if the user knows what they're doing.

[waldner]

And to be clearer, I'm asking if this feature can be accepted; if so, I can do it myself and submit a PR.

[brokenpip3]

Also note that the rate limits are for API requests, not for other types of requests.

unfortunately this is not true, github introduced rate limits everywhere, even by visiting a web page, they called it "secondary rate limit"

Just to be clear, I'm not suggesting to remove the use of the token altogther; in fact, I would leave the default as it is now, which is good; only offer an optional way to opt-out from using the token if the user knows what they're doing.

why not, make sense, I'm happy to provide more freedom to the users

I can do it myself and submit a PR.

happy to review it, I believe the best way could be:

• removing the default, keeping the token to be optional
• add a check if we need to pass the token as header in case it's present
• use it in any curl call even later
• update the readme and show how to actually pass the token to avoid the rate limit so something like:

with:
    [...]
    github-token: ${{ secrets.github.token }}

[brokenpip3]

@waldner I created #65 for it, I can assign it to you if you comment the issue

[waldner]

For completeness, you can also get the latest release without invoking the API, by calling eg curl 'https://github.com/bats-core/bats-core/releases/latest' which returns a 302 to the actual release URL (eg location: https://github.com/bats-core/bats-core/releases/tag/v1.13.0).
Regarding secondary limits, the documentation isn't 100% clear but it would seem that they still are about API requests, at least according to https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2022-11-28#about-secondary-rate-limits ("web interface" is only mentioned in the "Create too much content on GitHub in a short amount of time" paragraph which doesn't apply here). A quick search shows users getting that "you have exceeded a secondary rate limit" error in the context of searches on the github web site, which again hopefully isn't applicable here.

[waldner]

Also I'm not really sure that just adding the token to a non-API request (ie one against https://github.com/blah) makes it magically authenticated, but I might be wrong on this, I would have to do some more research.

[brokenpip3]

Also I'm not really sure that just adding the token to a non-API request (ie one against https://github.com/blah) makes it magically authenticated, but I might be wrong on this, I would have to do some more research.

yes indeed you are right to underline this, we should use the tarball api everywhere, for instance https://api.github.com/repos/bats-core/bats-core/tarball/v1.13.0 so we can pass the authentication token or not depending on users preference