Regarding CodeCov and its checkered history

[mapkon]

Context

Recently, we integrated Codecov into the saml2aws project, after stellar work by @gliptak - these are the PRs in which it was merged:

• Add Codecov to GHA #988
• Correct Codecov badge in README #989
• Ignore (generated) coverage.xml #991

That said, Codecov has had a checkered history with regards to supply chain attacks, especially if it has write permissions to the a given repository (but I would imagine that read permissions are a valid attack vector as well).

The more interesting supply chain attack is documented here

Discussion

Given that saml2aws operates in the gray area of security/identity enablement, a good number of folks in the community are not comfortable with having Codecov on the repository, especially given that it is running via github actions. To be fair, the project has had a stringent approach to adding dependencies or libraries, only adding those that are vetted to the dep list - so it is only right that another external dependency is subject to the same scrutiny.

I personally see the need for Codecov. It allows us to visualize test coverage, and also allows the project to quickly identify which areas of the code base need attention as pertains to testing. The ability of having those metrics after each build or PR submission is invaluable, given that the feedback can be used in PR reviews to gauge the suitability of contributions.

On the flip-side, it is possible that enabling codecov introduces an attack vector, which could ideally upstream to all the users of the library. This is a risk the project is not willing to take (at least not lightly) since the repercussions would be devastating and would almost spell the end of this project.

Conclusio

Given the above, and also the experience of supply chain attacks, should we still go ahead and enable codecov or should we revert those PRs, so as to achieve a "better security posture"?

[gliptak]

thank you for the links @mapkon supply chain compromises have wide impacts. Github itself could be (similarly) compromised

https://about.codecov.io/security-update/

reading the details, my understanding is that you would have to specifically inject GITHUB_TOKEN in the step running the above compromised script for the exploit to work like https://github.com/Versent/saml2aws/blob/master/.github/workflows/release.yml#L40

which we are not doing/required to do for our Codecov upload https://github.com/Versent/saml2aws/blob/master/.github/workflows/go.yml#L34

the current setup (mostly) works without enabling Codecov GitHub App Integration

[briantist]

One thing to keep in mind is that only certain GHA jobs have access to sensitive information like repository/organization secrets, or write access. Jobs that use the workflow_dispatch, push, and pull_request_target triggers for example.

Jobs that use the pull_request trigger depend on where they came from: if the PR is from a fork, then the workflows only have read access to the various permission scopes (so they can't write to the repo, can't access secrets). This makes sense because otherwise someone without access to the repository could gain it by modifying the workflow in their PR to grant them access or make changes. But if the PR comes from a branch on the repo itself, then it has the same access as a push job.

So the exploit as described above would likely not have been very useful in PRs that come from forks, but since codecov is usually used in push workflows as well, it would have been able to get at a lot of information, including an overprivileged GitHub token.

For reference (see Assigning permissions to jobs), it's the contents scope on a token that allows access to secrets, but only contents: write (which also allows for pushing commits) gives access to secrets, while contents: read does not.

This is a public repo, but in the exploit, private repo contents were exposed this way by way of exfiltrating a token with read access to the repository.

---

One way to mitigate some of these concerns is the have the jobs that generate coverage reports upload them as build artifacts, and then perform the upload to codecov in a separate job, which doesn't have write access to anything; it can also be denied read access to everything except the actions scope, so it would not even be able to read the repository's contents.

The actions/upload-artifact and /download-artifact are used to handle the artifacts between jobs, and the permissions key in the workflow allows for setting different permissions on different jobs. By having the upload job add needs: [build] it will not run until the build job has completed and uploaded its artifact.

That keeps the codecov component away from the jobs that have access to the code, so even if the case of a private repo, an exploit would not have even read access to the contents.

[briantist]

Here's a PR for that: #998
Having some issues with it which I'll describe in the PR comments.

[mapkon]

You beautiful soul. I will take a look and merge

[mapkon]

The PR has been merged, I suppose that concludes this discussion: #998

[wolfeidau]

Bit late to the party, but thanks a lot for addressing concerns, and uplifting the security of this feature!

Given the mitigations I am also 👍🏻