Can v4 still securely work with pull requests?

[alexdlaird]

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?

[jamshale]

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.

[rohan-at-sentry]

@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

[jamshale]

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.

[matt-codecov]

@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

[alexdlaird]

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 pull_request trigger, only push and pull_request_target.

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 pull_request trigger, but since that change, I think that's where a lot of this confusion is coming from, and I'm just trying to understand what the solutions are going forward, for this tool, and for other tools.

[alexdlaird]

And yes, I have the Codecov integration installed on my GitHub account, and it is enabled for all repos.

[alexdlaird]

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: https://github.com/codecov/codecov-action/pull/1410#issuecomment-2096842641)

And now I'm wondering if I'm crazy. I went to find the docs I was referencing about pull_request not passing secrets, and I see what you're saying now about the docs saying that applies to forked repos. But ... I dunno what to say, PRs from my own repo don't have access to secrets unless I see pull_request_target (which I don't want to do), sooooo ... not sure what I'm missing here.

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflows-in-forked-repositories-1

[rohan-at-sentry]

@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.

[alexdlaird]

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 dependabot[bot] actor is only granted a read-only token and is not passed secrets (this changed in 2021, so it's not "new" behavior by any means, but if others are confused, it has changed) unless you trigger the workflow with pull_request_target, which isn't a secure solution—this is where I was getting confused about the "fork" thing, since the behavior is similar. I actually discovered this while troubleshooting an auto-merge action I was using (sidebar on that in case that also brings others here, with GitHub's auto-merge feature now being a thing, a simple CLI command step is all you need for this now, not third-party action, which I've also found to be pretty flaky. CLI example here).

So, assuming you set CODECOV_TOKEN as a secret on the repo, and assuming the PRs triggering the workflow aren't Dependabot PRs, the v4 Codecov Action is working for me now on regular PRs, just not ones opened by Dependabot (but that's fine, don't care about coverage in that case). If I remove the token, I do still see 429s, even on a single execution, but I don't really need to troubleshoot that issue if token uploads are working for me, which they are now. Hopefully this context helps someone else too. Thanks!

[jotak]

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? My understanding was that secrets are not available on pull_request events, hence it just cannot work, or is that assumption wrong?

Personally I'm still seeing this issue, with this log that shows that tokenless is being used:

Run codecov/codecov-action@v4
  with:
    files: ./cover.out
    flags: unittests
    fail_ci_if_error: true
    verbose: true
  env:
    CODECOV_TOKEN: 
evenName: pull_request
baseRef: netobserv:main | headRef: jpinsonneau:740
==> Fork detected, tokenless uploading 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 push events?

PS: I tried with both v4 and v4.4.1 ... not sure if "v4" acts as a pointer to latest

[rohan-at-sentry]

@jotak

are you saying v4 works even on PRs coming from forked repos? Or does the PR have to come from the base repo itself?

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.

Related side question: is it necessary to have a report upload in order to have the codecov comment posted in the PR?

This is correct - Codecov will post a comment after it receives and successfully processes a coverage report.

[jotak]

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.