-
Notifications
You must be signed in to change notification settings - Fork 199
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Can v4 still securely work with pull requests? #1431
Comments
Same problem here. It really doesn't seem like codecov with github actions is going to work for our project. https://github.com/hyperledger/aries-cloudagent-python I'm going to consider alternatives. Too many issues trying to get this working. |
@alexdlaird @jamshale - my assumption is that you're looking to find a way to ensure forks of your repos are able to upload to Codecov. This is currently permitted without a token see here. Let me know if I have misunderstood |
For us, it fails every time I've tried to upload because of the rate limiting, when doing tokenless. If I try to use the token then I get the problem @alexdlaird mentioned. |
@jamshale is the codecov GitHub app installed for your org / is it given access to the repo in question? in general across codecov installing the app should give your org its own rate limit, and that should apply here as well we're currently working on an improvement to upload authentication and tokenless behavior and will update docs accordingly soon |
No, I am not referring to PRs from forked repos. A PR from with your same repo still does not have access to secrets with Previously, I was using tokenless uploads. I recently upgraded to the GitHub Action v4, and in that version, you are now requiring the token—from this, I surmised tokenless upload is going away (though it sounds like maybe that's not true, from what you're describing)? In either case, once I upgraded to v4 of the action and was required to provide a token via a secret, PR coverage reports stopped working for the reasons I've described above. Obviously, I could roll back to an older version of the GitHub Action and use tokenless upload again, but that's why I raised this question (and like @jamshale, my experience for the last several years, on dozens of repos in my personal GitHub account, is very inconsistent behavior from tokenless uploads). What is the intent for the future here (since old versions of the GitHub Action also use legacy Node versions taht GitHub has stopped supporting)? This seems like it will be an issue for any coverage reporting tool that requires a token, so how are open source projects supposed to do this going forward? GitHub used to allow secrets to be passed down via a |
And yes, I have the Codecov integration installed on my GitHub account, and it is enabled for all repos. |
I see ya'll do this though: https://github.com/codecov/codecov-action/blob/main/.github/workflows/main.yml#L26C1-L27C1 And it looks like PR comments still work for ya'll (example: #1410 (comment)) And now I'm wondering if I'm crazy. I went to find the docs I was referencing about |
@alexdlaird would you be open to a call next week to help me understand how we could improve this? You can find time on my calendar if it's easier. |
Alright, I believe I've resolved my issue with some additional research. It's not clearly documented, so going to leave my findings here, in case others stumble across this with a similar issue. For clarity, the PRs I was specifically have issues with that appeared flaky to me were Dependabot PRs (though I didn't immediately make this correlation). Though Dependabot opens the PRs on the repo itself, they act similar to a forked repo in regards to permissions, which is to say, the So, assuming you set |
Hi @alexdlaird , thanks for your explanations, but there's still something confusing me: are you saying v4 works even on PRs coming from forked repos? Or does the PR have to come from the base repo itself? Personally I'm still seeing this issue, with this log that shows that tokenless is being used:
Related side question: is it necessary to have a report upload in order to have the codecov comment posted in the PR? Wouldn't it be possible to just disable uploads, but keep posting comments, and have uploads only on PS: I tried with both v4 and v4.4.1 ... not sure if "v4" acts as a pointer to latest |
V4 allows uploads from both forked as well as the base repo. PRs from forked repos don't need tokens for reports to be upload to codecov. PRs from the "upstream" repo, need a token today.
This is correct - Codecov will post a comment after it receives and successfully processes a coverage report. |
Thanks @rohan-at-sentry - but my questions were to take in the context of the GH rate-limiting issue when tokenless is used, so I'm trying to not use tokenless. If I understand correctly, @alexdlaird was saying that it's possible to pass tokens for forks PRs , which is what I don't understand because I can't make it work. |
See codecov/codecov-action#1431 Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Version 4.4.1 provides a bugfix to correctly detect tokenless upload for PRs from forks: codecov/codecov-action#1437 codecov/codecov-action#1431 Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Version 4.4.1 provides a bugfix to correctly detect tokenless upload for PRs from forks: codecov/codecov-action#1437 codecov/codecov-action#1431 Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
Version 4.4.1 provides a bugfix to correctly detect tokenless upload for PRs from forks: codecov/codecov-action#1437 codecov/codecov-action#1431 Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
This may be more of a question than a report. Since upgrading to v4, a token is required. However, the GitHub Action Target pull_request does not provide secrets to the action (and pull_request_target, which does, is inherently insecure for this reason). Is the expectation then that Codecov simply doesn’t work with pull requests anymore going forward? Or what is the proper, secure workaround for this, if any?
The text was updated successfully, but these errors were encountered: