Skip to content
This repository was archived by the owner on Jun 13, 2025. It is now read-only.

feat: GitHub actions upload via OIDC token#177

Merged
scott-codecov merged 7 commits intomainfrom
scott/github-oidc
Oct 11, 2023
Merged

feat: GitHub actions upload via OIDC token#177
scott-codecov merged 7 commits intomainfrom
scott/github-oidc

Conversation

@scott-codecov
Copy link
Contributor

Purpose/Motivation

GitHub actions workflows have an implicitly available OIDC token that can be used to identify the repository corresponding to an upload. This alleviates the need to explicitly use a Codecov upload token in GitHub actions.

Thanks to @juho9000 for doing all the hard work here - I just added some tests.

Links to relevant tickets

Closes #90

What does this PR do?

If an upload token is a UUID then the logic remains the same as it was previously. Otherwise if the service making the upload is github-actions then we assume the token is the OIDC JWT and decode it in order to get information about the repository.

@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #177 (cc4f90d) into main (c1cd5f9) will decrease coverage by 0.02%.
The diff coverage is 88.37%.

Changes have been made to critical files, which contain lines commonly executed in production. Learn more

@@           Coverage Diff           @@
##            main    #177     +/-   ##
=======================================
- Coverage   95.53   95.51   -0.02     
=======================================
  Files        714     714             
  Lines      15551   15584     +33     
=======================================
+ Hits       14856   14884     +28     
- Misses       695     700      +5     
Flag Coverage Δ
unit 95.61% <88.37%> (-0.03%) ⬇️
unit-latest-uploader 95.61% <88.37%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
billing/helpers.py 100.00% <100.00%> (ø)
conftest.py 100.00% <100.00%> (ø)
utils/__init__.py 100.00% <100.00%> (ø)
upload/helpers.py Critical 93.39% <82.14%> (-1.71%) ⬇️

@codecov-staging
Copy link

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Files Patch % Lines
upload/helpers.py 82.14% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!.

@codecov-qa
Copy link

codecov-qa bot commented Oct 4, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (c1cd5f9) 95.63% compared to head (cc4f90d) 95.61%.

Files Patch % Lines
upload/helpers.py 82.14% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #177      +/-   ##
==========================================
- Coverage   95.63%   95.61%   -0.03%     
==========================================
  Files         599      600       +1     
  Lines       15150    15183      +33     
==========================================
+ Hits        14489    14517      +28     
- Misses        661      666       +5     
Flag Coverage Δ
unit 95.61% <88.37%> (-0.03%) ⬇️
unit-latest-uploader 95.61% <88.37%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codecov-public-qa
Copy link

codecov-public-qa bot commented Oct 5, 2023

Codecov Report

Merging #177 (cc4f90d) into main (c1cd5f9) will decrease coverage by 0.03%.
The diff coverage is 88.37%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #177      +/-   ##
==========================================
- Coverage   95.63%   95.61%   -0.03%     
==========================================
  Files         599      600       +1     
  Lines       15150    15183      +33     
==========================================
+ Hits        14489    14517      +28     
- Misses        661      666       +5     
Flag Coverage Δ
unit 95.61% <88.37%> (-0.03%) ⬇️
unit-latest-uploader 95.61% <88.37%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
billing/helpers.py 100.00% <100.00%> (ø)
conftest.py 100.00% <100.00%> (ø)
utils/__init__.py 100.00% <100.00%> (ø)
upload/helpers.py 93.39% <82.14%> (-1.71%) ⬇️

Impacted file tree graph

@webknjaz
Copy link

webknjaz commented Oct 5, 2023

Hi, I have some OIDC experience coming from the Python ecosystem / PyPI so I wanted to leave a comment regarding some misconceptions.

GitHub actions workflows have an implicitly available OIDC token

That's not exactly right. The end-users have to opt-in explicitly by tweaking their GHA job privileges. This is achieved by specifying the following:

permissions:
  id-token: write

Note that setting this globally is dangerous as it gives OIDC access to untrusted third party code. This might not be a big deal for Codecov itself but could be harmful in the context of other services that an end-user' project uses.

This is why, in the guides for PyPI we emphasize that the packages should be built in a separate low-privileges job, not in the upload job that has OIDC set:

For Codecov, though, this would mean the need to discourage people from uploading coverage from the test jobs to prevent privilege escalation through the test deps. They'd need to store coverage as GHA artifacts in the test jobs and download+upload that from an isolated one.

@scott-codecov
Copy link
Contributor Author

@webknjaz Thanks for the clarifications! Storing the coverage as a GHA artifact and then only giving our uploader access to the OIDC token makes a lot of sense. Your PyPI documentation around this looks really great and I may suggest we use some of it as inspiration for our own documentation of this feature.

@scott-codecov scott-codecov merged commit 7c02977 into main Oct 11, 2023
@scott-codecov scott-codecov deleted the scott/github-oidc branch October 11, 2023 08:25
RulaKhaled pushed a commit that referenced this pull request Oct 12, 2023
* Add Github OIDC token authentication to uploads

* Update PyJWT via pip-compile --upgrade-package

* Add GitHub OIDC test

* Remove exception from returned error message

* Fix linting

---------

Co-authored-by: Juho Majasaari <ext-juho.majasaari@elisa.fi>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants