Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add artifacts v4 jwt to job message and accept it #28885

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

ChristopherHX
Copy link
Contributor

@ChristopherHX ChristopherHX commented Jan 21, 2024

This change allows act_runner / actions_runner to use jwt tokens for ACTIONS_RUNTIME_TOKEN that are compatible with actions/upload-artifact@v4.

The official Artifact actions are now validating and extracting the jwt claim scp to get the runid and jobid, the old artifact backend also needs to accept the same token jwt.


Related to #28853

I'm not familar with the auth system, maybe you know how to improve this

I have tested

  • the jwt token is a valid token for artifact uploading
  • the jwt token can be parsed by actions/upload-artifact@v4 and passes their scp claim validation

Next steps would be a new artifacts@v4 backend.

I'm linking the act_runner change soonish.
act_runner change to make the change effective and use jwt tokens https://gitea.com/gitea/act_runner/pulls/471

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 21, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 21, 2024
@github-actions github-actions bot added the modifies/api This PR adds API routes or modifies them label Jan 21, 2024
@lunny lunny requested a review from fuxiaohei January 22, 2024 01:39
@lunny lunny added this to the 1.22.0 milestone Jan 22, 2024
@lunny lunny added the type/enhancement An improvement of existing functionality label Jan 22, 2024
@lunny lunny requested a review from wolfogre January 22, 2024 01:42
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 22, 2024
@ChristopherHX
Copy link
Contributor Author

Do we want to pass the jwt token only via secrets as GITEA_RUNTIME_TOKEN?, my own runner implementation would leak the jwt in case an old version of it would be used after updating Gitea.

old act_runner itself discards such fields by default, due to a manual mapping to a strong typed struct.

...this is only one possible way to pass the jwt.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 2, 2024
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 2, 2024
@lunny lunny added the topic/gitea-actions related to the actions of Gitea label Feb 2, 2024
@lunny lunny enabled auto-merge (squash) February 2, 2024 14:11
@lunny lunny merged commit a9bc590 into go-gitea:main Feb 2, 2024
25 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 2, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 4, 2024
* giteaofficial/main:
  Add `must-change-password` cli parameter (go-gitea#27626)
  Include username in email headers (go-gitea#28981)
  Update tool dependencies (go-gitea#29030)
  Add artifacts v4 jwt to job message and accept it (go-gitea#28885)
  Pass es2020 to esbuild-loader as well (go-gitea#29027)
  Fix default avatar image size in PR diff page (go-gitea#28971)
  Update JS and PY dependencies, build for `es2020` browsers (go-gitea#28977)
silverwind pushed a commit to silverwind/gitea that referenced this pull request Feb 20, 2024
This change allows act_runner / actions_runner to use jwt tokens for
`ACTIONS_RUNTIME_TOKEN` that are compatible with
actions/upload-artifact@v4.

The official Artifact actions are now validating and extracting the jwt
claim scp to get the runid and jobid, the old artifact backend also
needs to accept the same token jwt.

---
Related to go-gitea#28853

I'm not familar with the auth system, maybe you know how to improve this

I have tested
- the jwt token is a valid token for artifact uploading
- the jwt token can be parsed by actions/upload-artifact@v4 and passes
their scp claim validation

Next steps would be a new artifacts@v4 backend.

~~I'm linking the act_runner change soonish.~~
act_runner change to make the change effective and use jwt tokens
<https://gitea.com/gitea/act_runner/pulls/471>
wustus pushed a commit to wustus/act_runner-dind that referenced this pull request Feb 21, 2024
Needs go-gitea/gitea#28885 to provide jwt if sent by server

Could fix #459, but that has not been verified.

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Reviewed-on: https://gitea.com/gitea/act_runner/pulls/471
Reviewed-by: delvh <dev.lh@web.de>
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: Christopher Homberger <christopher.homberger@web.de>
Co-committed-by: Christopher Homberger <christopher.homberger@web.de>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/gitea-actions related to the actions of Gitea type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants