aquasecurity / tfsec-pr-commenter-action

Add comments to pull requests where tfsec checks have failed
MIT License
164 stars 64 forks source link

Frequent usage causes rate limit errors #61

Open adamdecaf opened 2 years ago

adamdecaf commented 2 years ago

Thanks for the handy Action to run on projects. We've found it useful in several projects.

There's an issue we've ran into when opening PR's during normal working hours. After looking at the script I can see there are 8 wget calls to github services, which can pretty easily exceed the limit of 60/hour from a single IP address.

Would y'all be open to using secrets.GITHUB_TOKEN (I think this would work) and better management of the wget calls in the script?

jnauska commented 2 years ago

This is an issue for us aswell. We are using matrix in our github actions to scan the different folders, because otherwise it will always fail to comments as it reports that the issue is not part of this PR.

So as we invoke this in matrix, we run the action multiple times (16 in our case), which then exceeds the rate limit. And as the API calls are part of the entrypoint, we have no way of using user/app tokens to properly authenticate and bypass this limitation in the current state.

owenrumney commented 2 years ago

Hey @adamdecaf - I'm not sure I follow, GITHUB_TOKEN is required for this action already.

I'm likely being dim, could you help me understand your ask?

harmw commented 2 years ago

Those wget calls are running in regular anonymous mode, where they are subject to rate-limiting rules. If we change these to use the GITHUB_TOKEN, GitHub will see them as trusted, thus easing the rate-limiting rules. Iirc this can be done with a query string parameter and/or some HTTP request header.

While at it, may I suggest getting all required files in at build time instead? this would speed up execution and decouple the runtime image from these additional binaries. Having the logic only do these downloads if INPUT_COMMENTER_VERSION or INPUT_TFSEC_VERSION are defined (and not set to latest). Wdyt?

owenrumney commented 2 years ago

I used to build the action with the binaries already baked in but then being able to switch out the version of tfsec was requested.

I think maybe it is the lesser of two evils to get everything in place at build time, I'll look to chain the build of the action from the back of a new release of tfsec and synchronise the versions in the process..... sounds like a fun friday task

adamdecaf commented 2 years ago

Those wget calls are running in regular anonymous mode, where they are subject to rate-limiting rules. If we change these to use the GITHUB_TOKEN, GitHub will see them as trusted, thus easing the rate-limiting rules. Iirc this can be done with a query string parameter and/or some HTTP request header.

This is exactly right. It's a large difference from anonymous to trusted rate limits. You can set it in an HTTP header.

This branch wasn't working, but the idea is here: https://github.com/adamdecaf/tfsec-pr-commenter-action/commit/20567c779823f163f8d5568e6c8838c3141bfdce

iaacautomation commented 2 years ago

I am running the same issue inpulling image from dockerhub. Rate limited. IS it possible to pass the default image name? change the location of "alpine" image?

We are running on self-hosted gh-runner on kubernetes with ephemeral runners.

rauny-brandao commented 2 years ago

@iaacautomation as a workaround I'm doing this: https://github.com/aquasecurity/tfsec-pr-commenter-action/pull/59/files

lloydnye commented 2 years ago

I think this is fixed in the latest release 1.3.0 by https://github.com/aquasecurity/tfsec-pr-commenter-action/pull/82