codecov / feedback

A place to discuss feedback about the pull request and web product experience.
36 stars 7 forks source link

Tokenless support in the new CLI (for public repos and forks) #112

Closed toddbaert closed 4 months ago

toddbaert commented 1 year ago

Hello! I've recently updated a number of projects in our org to use the latest versions of the codecov action.

I see now that the codecov CLI is used to power this action, and that it requires a token. No problem, I went and got the token for the repositories in question, and all seemed to work... until we got a PR from a fork (something that happens quite frequently in our case). This results in a problem: secrets aren't available in PRs from forks (unless we use the pull_request_target trigger, which would allow malicious actors to potentially exfiltrate our secret(s)).

So my question is, as an open-source project, how am I supposed to use the new action? I want to run the coverage analysis on the new code (which is untrusted) but in order to do that, I need to run in a trusted environment to get access to the secret. Am I missing something? :thinking:

thomasrockhu-codecov commented 1 year ago

@toddbaert this is a known and yet unsolved problem. We are investigating GitHub's OIDC to see if this will be a suitable replacement for token-less uploading (including forks)

toddbaert commented 1 year ago

@thomasrockhu-codecov Thanks!

For now we'll stick to v3! Happy to help you folks test v4 if you have a proposed solution for forks at some point.

Should I close this issue?

thomasrockhu-codecov commented 1 year ago

@toddbaert let's leave it open for now so that anyone watching will get an update if OIDC is the way to go for forks.

For everyone else, you can add the CODECOV_TOKEN from the Codecov repo settings page to your repo secrets. Then, update your GitHub Actions workflows:

...
uses: codecov/codecov-action@v4
env:
  CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
...
toddbaert commented 1 year ago

@toddbaert let's leave it open for now so that anyone watching will get an update if OIDC is the way to go for forks.

For everyone else, you can add the CODECOV_TOKEN from the Codecov repo settings page to your repo secrets. Then, update your GitHub Actions workflows:

...
uses: codecov/codecov-action@v4
env:
  CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
...

Yep, looks like there's some linked PRs/issues, so probably good to keep it open until there's a solution for forks. Thanks again!

dolfinus commented 1 year ago

you can add the CODECOV_TOKEN from the Codecov repo settings page to your repo secrets

But this still requires changing on:pull_request to on:pull_request_target to access secrets of original repo from a fork.

rohan-at-sentry commented 1 year ago

Hey all, thanks for this discussion. We're actively looking to solve this issue - https://github.com/codecov/engineering-team/issues/665

I'll post back here when we're ready! Thanks for your patience.

jonhoo commented 10 months ago

@rohan-at-sentry Now that https://github.com/codecov/engineering-team/issues/665 has been resolved, what's the state of affairs for this (and https://github.com/codecov/codecov-action/issues/1110)?

rohan-at-sentry commented 10 months ago

@jonhoo we're still waiting on a few related stories to wrap up, specifically around our API, following which we intend to bump up the version of the action.

I'll check back in here when this is done, but I expect things to wrap up fairly quick here

jonhoo commented 10 months ago

Nice, I see https://github.com/codecov/engineering-team/issues/736 was just completed! Steady steps forward :tada:

rohan-at-sentry commented 10 months ago

That's right. We actually released this under a flag that allowed us to test things out. Next steps are

  1. Remove the flag
  2. Update our GitHub Action, Bitrise Step and CircleCI orb
rohan-at-sentry commented 9 months ago

Hi everyone

We've released a new version of the codecov action and CLI that should support tokenless uploads from forks. Here's a link to our blog outlining the latest update including how to upgrade to the CLI. You can also visit our docs for more detailed information on how to get set up with the CLI, especially if you're not using our Github Action, Bitrise Step or CircleCI orb.

Currently, tokenless uploading is only available to forks attempting to create PRs on public repositories. This means that for most use cases, you will still need a token, even for public repositories. However, contributors creating PRs from their forks to the upstream repo will be able to get coverage information from Codecov.

jrfnl commented 9 months ago

@rohan-at-sentry If forks can get the codecov report without a token, why can't PRs from within the repo itself ? This sounds like the world upside-down.

LecrisUT commented 9 months ago

But PRs from forks to main repo have access to the main repo's secret so it will be using that token. The difference would be if they are making a PR on their own fork, in which case they should be uploading CODECOV_TOKEN to the forked repository as well

thomasrockhu-codecov commented 9 months ago

Hi all, to clear some things up about why tokenless is only available for forks to public repos.

The reason why tokenless has been so problematic over the past few years has been due to GitHub's rate limiting. Tokenless generally works by making an API call to GitHub to confirm that the repo and commit are the correct values. Making this call for thousands of repositories causes our GitHub token to hit the limit causing the issues that many of you have seen (e.g. Unable to locate build via Github Actions API).

As a result, we needed to make decisions on how best to serve our users. This means that tokenless uploading is not currently sustainable until GitHub allows for higher rate limited APIs.

The open source (OS) community is a major user and proponent of Codecov. A very common flow for them is to have outside contributors fork the repo, make changes, and open a PR to the upstream repository with that change. Unfortunately, GitHub does not have a way to share secrets with contributors. In order to provide support for these OS projects, we felt that it was necessary to allow for tokenless to forks.

We felt that this balance between low GitHub API usage and OS usability would allow for the least amount of pain for our users.


For forks using Codecov, you will need to use the fork's CODECOV_TOKEN when uploading to your own repository. However, when creating a PR upstream, that token will be ignored.

jrfnl commented 9 months ago

But PRs from forks to main repo have access to the main repo's secret so it will be using that token.

@LecrisUT No, they don't. They have access to the default GITHUB_TOKEN, but not to any repo secrets, which is what the CODECOV_TOKEN would be. (That's the whole point of this ticket)

jrfnl commented 9 months ago

Making this call for thousands of repositories causes our GitHub token to hit the limit causing the issues that many of you have seen (e.g. Unable to locate build via Github Actions API).

This means that tokenless uploading is not currently sustainable until GitHub allows for higher rate limited APIs.

That sounds like a "you" problem which you are dumping onto your end-users instead.

thomasrockhu-codecov commented 9 months ago

@jrfnl, I'm not sure what problem you're running into is. Public repos, yes, need to use a token, as do private repos to use Codecov. Public forks are now able to get coverage results back from Codecov on their PRs without the token. Are you having trouble adding the token?

LecrisUT commented 9 months ago

But PRs from forks to main repo have access to the main repo's secret so it will be using that token.

@LecrisUT No, they don't. They have access to the default GITHUB_TOKEN, but not to any repo secrets, which is what the CODECOV_TOKEN would be. (That's the whole point of this ticket)

I am confused then. How do tools like sonarcloud work on forks? random example. Basically isn't codecov supposed to mimic the same interface like those tools?

Andrew-S-Rosen commented 9 months ago

Thanks for the update, although it is an unfortunate one.

To confirm/clarify, in updating the codecov-action to 4.0, I am getting upload errors that no codecov token can be found. I was not getting this error with 3.0. Presumably the suggestion from the codecov team is to define my CODECOV_TOKEN in the GitHub Action, but this is obviously not practical when I have external contributors submitting PRs (which is kind of the whole point of open-source software in the first place).

thomasrockhu-codecov commented 9 months ago

@Andrew-S-Rosen, external contributors submitting PRs from forks without the token is allowed. Is this the use case you're talking about?

thomasrockhu-codecov commented 9 months ago

@LecrisUT I am unfamiliar with how sonarcloud works, so I can't speak on what API calls they need to do internally.

Andrew-S-Rosen commented 9 months ago

@Andrew-S-Rosen, external contributors submitting PRs from forks without the token is allowed. Is this the use case you're talking about?

Indeed it is. That would be great if so. However, I remain confused by some of the information here.

This means that for most use cases, you will still need a token, even for public repositories.

What are "most use cases" here then? I also was under the impression that repo secrets are not accessible from externally submitted PRs.

Is the suggestion to add

  env:
    token: ${{ secrets.CODECOV_TOKEN }}

where CODECOV_TOKEN is a defined repo secret. And then nothing else will need to be done? PRs submitted from forks will have their code coverage reported in the open PR as-is?

mattlord commented 9 months ago

It seems to be working for me here: https://github.com/vitessio/vitess/pull/14967

Thank you!

Now I have to figure out how the code coverage went DOWN with cross package testing enabled. 🙂 These numbers don't make much sense to me: https://github.com/vitessio/vitess/pull/14967#issuecomment-1919774260

But that's an unrelated issue. Thanks again!

thomasrockhu-codecov commented 9 months ago

@Andrew-S-Rosen.

This means that for most use cases, you will still need a token, even for public repositories.

This is true for all use cases except for PRs from forks to upstream repos. Sorry that this is a bit confusing, you're right.

The suggestion is to add the token as you have written out. PRs submitted from forks should have their code coverage reported as is.

Andrew-S-Rosen commented 9 months ago

@thomasrockhu-codecov --- alright, thank you for the clarification. I'll merge the change and see what happens when external contributors start opening PRs. Fingers crossed.

mrazauskas commented 9 months ago

Can it be that usage example in the blog post and current documentation of the codecov-action are incorrect?

They suggest passing token like this:

- name: Upload coverage reports to Codecov
- uses: codecov/codecov-action@v4
  env:
    token: ${{ secrets.CODECOV_TOKEN }}

That fails for me. But the following worked:

  - name: Upload coverage reports to Codecov
  - uses: codecov/codecov-action@v4
-   env:
+   with:
      token: ${{ secrets.CODECOV_TOKEN }}

With this change I see: "Process Upload complete" in the action log. Otherwise I get an error.

thomasrockhu-codecov commented 9 months ago

@mrazauskas that is unexpected, it should work with env (as well as with). Can you share the logs in the failure scenario?

I see my error, it should have been

env:
  CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

I will update this now

ljharb commented 9 months ago

I use codecov on nearly 500 repositories, spread across many orgs and users. Some of the repos I don't have admin on (they're user-owned repos that I maintain but don't control). I use this via a shared action, so I can make one change and affect every repo at once.

It kind of seems like this means i will literally never be able to upgrade to v4, since even if I was capable if installing a secret on every repo I need to manage, I would never want to (because of the quantity).

Is this accurate? If so, what are the plans to either never drop support for v3, or, fix v4 so this isn't a requirement?

(given that v3 uses node16, which is deprecated, the former may not be an option)

(cc @chadwhitacre for visibility)

thomasrockhu-codecov commented 9 months ago

@ljharb, I realize that that is a huge pain for you. The closest thing I can think of that is offered in v4 is the global upload token, which is set per organization. You can find it at a similar URL: https://app.codecov.io/account/gh/`orgname`/org-upload-token

I think for your use case, you could add that token at the organization level as CODECOV_ORG_TOKEN to each organization (as opposed to each repository). Then, the shared action could intake

env:
  CODECOV_TOKEN: ${{ secrets.CODECOV_ORG_TOKEN }}

It's obviously not perfect, but hopefully will allow you to continue to use the shared action. As of now, there are no plans to drop support for v3.

ljharb commented 9 months ago

Again, i have many repos that are owned by another user, which means i can’t set a secret at any level. Anything that requires repo configuration is a nonstarter for me.

thomasrockhu-codecov commented 9 months ago

@ljharb I don't have a good solution for you right now. The best solution I can think of is to continue using v3 where you can, if that has been usable for you. I'll sync up with the product team to see what we can do to help.

ljharb commented 9 months ago

Thanks! I’m not really clear on why the requirement exists, though - it can’t be for security if tokenless is available in the least secure context, a pr from a public fork.

What would be ideal in the meantime is to update v3 to node 20, which shouldn’t be a breaking change for anyone, but would silence the deprecation warnings.

thomasrockhu-codecov commented 9 months ago

@ljharb I thought so too, but this was actually a breaking change for self-hosted runners. See https://github.com/codecov/codecov-action/issues/1230.

3.1.5 does use node20 so if that helps remove deprecation warnings, so be it.

jrfnl commented 9 months ago

Thanks! I’m not really clear on why the requirement exists, though - it can’t be for security if tokenless is available in the least secure context, a pr from a public fork.

@ljharb See https://github.com/codecov/feedback/issues/112#issuecomment-1919718118

They have a problem with GH's rate limit and instead of solving their problem and talking with GH, they are dumping this problem on their end-users, as well, you know, it's much easier to require thousands and thousands of repos to make changes....

LecrisUT commented 9 months ago

Again, i have many repos that are owned by another user, which means i can’t set a secret at any level. Anything that requires repo configuration is a nonstarter for me.

Have a different branch/tag where you use v4. And on the main branch/tag you had, emit a deprecation warning that the consuming repo/user needs to setup the token.

ljharb commented 9 months ago

@LecrisUT that isn't an option, because i am the consuming user, and the user that owns the repo doesn't pay attention to it anymore - and i don't want to bother them.

aj-codecov commented 9 months ago

Hi all,

My name is AJ and I'm one of Codecov's product managers. I want to give some greater context and clarity to the above feedback.

Codecov is a tool dependent on GitHub for much of our functionality and we have worked intentionally to maintain a long and positive relationship with them. We maintain this relationship to foster outcomes that are in the best interests of Codecov users. Rate limits are something we have spoken with them about and will continue to have conversations around.

There is no benefit to us in passing friction on to our users and we do everything in our power to mitigate potential friction. In this case we made a strategic decision to enforce token usage more broadly as it's the one lever we have to alleviate rate limit issues. We knew that using a token isn’t always possible, thus our enablement of tokenless uploads on forks. This felt like the optimal middle ground to us and was done in the interest of our users first and foremost.

In the past, due to rate limiting, sometimes Codecov simply didn't work. This led to feelings of unreliability and distrust among our user base and wasn't something we could easily alleviate. We're working to provide better messaging around when an account has hit a rate limit to accompany this work as a further improvement. What this means is that you should experience the impacts of rate limiting less often, leading to a more functional coverage tool for you. If rate limiting still happens you should know it has happened when it happens. Ultimately this scenario is a constraint of how we operate and we are pursuing all available options to mitigate any impact of this reality to our end users.

If you have further questions or concerns around this change, feel free to reach out to me at anthony.fazzio@sentry.io and I'd be happy to schedule time to speak with you.

ljharb commented 9 months ago

@aj-codecov that's a great rationale to start pushing tokens - but not a rationale to disallow tokenless. Perhaps just requiring an extra config option be provided in v4 to allow tokenless when a token is unavailable?

Put another way, this changes from "sometimes rate limited, but i don't care much when that happens" to "can't use codecov at all", so I don't think it's having the intended effect.

Andrew-S-Rosen commented 9 months ago

@thomasrockhu-codecov --- I updated to v4 but am occasionally receiving codecov token errors when external contributors submit PRs to main. Actually, I think it might just be from dependabot.

Here is an example of the codecov token error. You can see my codecov action here where I do

      - name: Upload coverage to Codecov
        uses: codecov/codecov-action@v4
        env:
          CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}

The above works when I submit a PR, but the token is not registered. Is that a limitation for bots?

Andrew-S-Rosen commented 9 months ago

@aj-codecov, @thomasrockhu-codecov --- thanks for the reply. I am still getting token issues, however, when external contributors submit PRs. Here is another example. Any thoughts?

thomasrockhu-codecov commented 9 months ago

@Andrew-S-Rosen, yes you will need to add the token to the Dependabot secrets also, let me know if you need help finding that (should be in the same section as where you initially added the token)

image
Andrew-S-Rosen commented 9 months ago

@thomasrockhu-codecov thanks! I'll address that.

Strangely, the example I shared in my most recent message was actually from a (human) contributor, but I think it may be (in part) a codecov bug since it returned:

File "codecov_cli/helpers/request.py", line 133, in log_warnings_and_errors_if_any 

43NameError: name 'exit' is not defined 

44[1793] Failed to execute script 'main' due to unhandled exception!

Edit: Opening a formal issue at https://github.com/codecov/codecov-action/issues/1282

thomasrockhu-codecov commented 9 months ago

@Andrew-S-Rosen oh I see the latest one now. That is strange, @rohan-at-sentry maybe worth taking a look?

To recap: This is a tokenless example that is failing during create commit process

JoshuaKGoldberg commented 8 months ago

Given how many public repositories use the action in a tokenless way, I'm surprised at how sudden this rollout has felt. I would have thought a few proactive actions would have been done first, if possible:

Was this change communicated out to people ahead of time? The only reason I learned of this is that my repositories were no longer updating Codecov numbers (despite passing codecov-action builds).

It's of course understandable if the Codecov team doesn't have the bandwidth to work on those changes first. But letting us know early on could have saved some community pain, I think.

mwestphal commented 6 months ago

Please note codecov v3 doesn't seem to work anymore.

mwestphal commented 6 months ago

So I managed to fix this on my own CI.

Somehow, the current behavior is completely undocumented but this is pretty simple.

Actually, codecov@v4 action detect forked workflow and use tokenless automatically in the case, even if a token is specified, the codecov output will tell you:

    info - 2024-04-28 03:27:00,660 -- The PR is happening in a forked repo. Using tokenless upload.

So here is how it looks in my CI:

    - name: Upload coverage to Codecov
      uses: codecov/codecov-action@v4
      with:
        token: ${{secrets.codecov_token}}
        fail_ci_if_error: true
        verbose: true
        use_oidc: false
rohan-at-sentry commented 6 months ago

@mwestphal - thanks for writing in, this was indeed documented before at https://docs.codecov.com/docs/codecov-uploader#supporting-tokenless-uploads.

I am going to try to augment the docs with more detail, and also make this more prominent in the readme for the action

mwestphal commented 6 months ago

Indeed, thanks for pointing out the doc!

make this more prominent in the readme for the action

I think this is why I missed it. The doc you linked is massive. I should have searched there for "tokenless" but I did not. In any case, improving the action doc to cover such a classic usecase is worth it imo.

hugovk commented 6 months ago

I've started adding tokens to organisations. Unfortunately, my projects tend to be:

Is there a way to add a token to a personal account so it can be used by all repos, similar to adding an an org-level?

What are the downsides of adding tokens in some shared plaintext file?

rohan-at-sentry commented 6 months ago

@hugovk

For the following use-cases

under my personal account the only one in an org, so it's tedious to click through to add secrets

you can grab the global upload token and either

see if you can access the settings page in the codecov UI for the repo (https://app.codecov.io/gh//settings

If yes, then you can grab the token and

Re

What are the downsides of adding tokens in some shared plaintext file?

If you are doing this to ensure uploads from forks are not impacted, this should largely already be in place see above and docs.

As for risks, sharing your token means others can upload phony coverage and blow away your data.