Fix tokenless uploads by hardcoding token...

[A5rocks]

This is recommended by codecov themselves: https://community.codecov.com/t/upload-issues-unable-to-locate-build-via-github-actions-api/3954

While this sucks, I don't think there's a better alternative. I believe codecov operates the following way for "tokenless" uploads:

• the action notifies the core service that it's done
• the core service uses github's API to verify that the action is actually who it claims to be

Now, you may go "hey! tokenless? sounds like pypi's tokenless publishing! how's pypi doing it but not codecov?!?!?"

Basically, pypi uses the id-token permission to do some fancy OpenIDConnect nonsense. codecov's action could require this permission, but then it would only work for PRs not from forks: the maximum permission level for PRs from forks for id-token is read, whereas you need write permissions to actually do the nonsense.

A similar restriction accompanies secrets, meaning that I need to hardcode this for PRs from forks to work.

---

tl;dr what codecov offers is the best way they can offer tokenless publishing, but it runs into github rate limits. we don't want to have flakey codecov like that, meaning that we gotta use the token to tell codecov "yes I am who I claim to be". this token needs to be hardcoded as:

Due to the dangers inherent to automatic processing of PRs, GitHub’s standard pull_request workflow trigger by default prevents write permissions and secrets access to the target repository.

https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

[codecov]

Codecov Report

Merging #2672 (30235b8) into master (3146638) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2672   +/-   ##
=======================================
  Coverage   98.11%   98.11%           
=======================================
  Files         118      118           
  Lines       16539    16539           
  Branches     3003     3003           
=======================================
  Hits        16227    16227           
  Misses        254      254           
  Partials       58       58           

[A5rocks]

Hi @webknjaz, requested your review because you both:

• know trio's CI
• know pypi's tokenless publishing

Feel free to correct my initial comment if I'm wrong! (I'd love to be incorrect!)

[webknjaz]

@A5rocks from the link you provided:

A malicious actor would be able to upload incorrect or misleading coverage reports to a specific repository if they have access to your upload token, but would not be able to pull down source code or make any code changes.

So with that in mind, I'd suggest using repository secrets to store an actual token instead of displaying it publicly.

Here, it'd look like

Suggested change

	token: 87cefb17-c44b-4f2f-8b30-1fff5769ce46
	token: ${{ secrets.CODECOV_TOKEN }}

and it'd be sanitized in the logs, addressing said problem.

Though, somebody should probably revoke/reset the token exposed here. I suppose only @njsmith can add repo secrets?

[webknjaz]

I was a bit too fast to comment. Your assessment seems correct. I wonder, though, if the secrets declared in environments are accessible from PRs… I didn't find any docs explicitly describing this case, and I haven't experimented with this either.

One thought I have on this, though, is that what Hynek advocates for in his article https://hynek.me/articles/ditch-codecov-python/ could be useful here too. Maybe, if we merged all the reports on the GHA side and made just a single upload to Codecov, it could help with the rate limit just enough for this CI to have the best of two worlds: stability + ability to view coverage on Codecov's web UI.

That said, I'm going to approve this PR as it does seem to improve the overall state of things and the above suggestion could be implemented in a follow-up, no need to block this effort for that to happen.

[A5rocks]

I wonder, though, if the secrets declared in environments are accessible from PRs… I didn't find any docs explicitly describing this case, and I haven't experimented with this either.

Hm. For what it's worth we already had the secret hardcoded (for builds.sr.ht) so this just makes it more visible -- which is why I initially felt it was OK to hardcode it like this without asking for permission first!

The merging stuff in the CI sounds like it would work, especially given we already run a single job at the end but may require some experimentation (I find github actions changes unintuitive at best and would need to try a few possibilities!)