-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
feat: Add configurable permissions for Actions automatic tokens #36173
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
base: main
Are you sure you want to change the base?
Conversation
|
@lunny @wxiaoguang Please review this |
|
Thank you for asking me to review, but I don't use Actions. You can invite the maintainers from the original issue to review. |
@silverwind Please review |
|
I review mostly frontend stuff and am not much of an actions user myself, so please be patient until someone finds time to review it properly. |
No problem |
|
By the way, I see another (older) PR: Feat/actions token permissions #36113 , it added more than 2000 lines of code. What are the differences? Which PR would win ....... @Zettat123 |
This PR doesn't fully implement the proposal in #24635. (For example, it doesn't support configuring actions access between repositories in the same organization) It seems that #36113 implemented these features, but I think its code needs improvement. |
|
@Zettat123 @silverwind Pls give me a few hours(15-20 hours) and this PR will be ready to go |
But "PR: Feat/actions token permissions #36113" came first, and it is more complete, why not respect the first author, but only review this second one? |
@wxiaoguang should i close my pr ? |
I don't know. Reviewers decide. |
I reviewed both PRs, but did not receive responses to my comments in #36113. If @Excellencedev will address the review comments, I think we should keep this PR. |
|
Imho, the only sensible thing we can do is race these 2 PRs. |
|
Adressed most your comments in my latest commit, now i just need to make sure i fully implement the proposal in #24635 |
| // DefaultTokenPermissions defines the default permissions for workflow tokens | ||
| DefaultTokenPermissions *ActionsTokenPermissions `json:"default_token_permissions,omitempty"` | ||
| // MaxTokenPermissions defines the maximum permissions (cannot be exceeded by workflow permissions keyword) | ||
| MaxTokenPermissions *ActionsTokenPermissions `json:"max_token_permissions,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't find a form on the settings page to configure MaxTokenPermissions, is it unused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've fully implemented it now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why cannot I find this anywhere in org and repository settings pages? Unresolved this myself.
|
According to the solution in #24635, I think this PR does not implement:
|
|
I found a TODO in gitea/services/context/package.go Lines 95 to 105 in f9d3983
|
eebb54f to
a5debd9
Compare
@Zettat123 fixed f8a0b25 |
|
ready for another review @Zettat123 |
|
@Zettat123 @ChristopherHX pls review this |
| TokenPermissionMode ActionsTokenPermissionMode `json:"token_permission_mode,omitempty"` | ||
| // DefaultTokenPermissions defines the specific permissions for workflow tokens when TokenPermissionMode is set to "custom" | ||
| // and no "permissions" keyword is defined in the workflow YAML. | ||
| DefaultTokenPermissions *ActionsTokenPermissions `json:"default_token_permissions,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've reviewed the code. If I understand correctly, the steps you listed are for MaxTokenPermissions, not DefaultTokenPermissions. I'm still not sure what DefaultTokenPermissions actually does. If it's a necessary field, we need a form to configure it on the UI. If not, we should remove it.
|
Based on the recent comments, it appears that the current implementation still contains logical inconsistencies and redundant code. This suggests that the PR requires a more thorough analysis of the existing code and a more comprehensive integration of the review comments provided so far. To ensure we use everyone’s time effectively, I am converting this PR back to Draft status. Since maintainer resources are limited, we expect contributors to perform a rigorous self-review before requesting a formal review. Please ensure the logic is fully closed, redundant elements are removed, and similar issues throughout the PR are addressed consistently. While I would like to help move this PR forward, I am currently tied up with several urgent tasks and do not have the bandwidth to review a PR of this size at this moment. Once you feel the PR is truly ready and you have switched the status back to "Ready for review," I welcome other reviewers to review this PR and provide their feedback. I would appreciate any comments on this. |
|
Okay understood but I hope this PR won't just amount to nothing as it's been open for almost a month with several review comments from different maintainers I will work throughout next week to make this "reviewable" an snake sure maintainers have a good time reviewing this I just hope other maintainers will have on this in the future also |
|
@Zettat123 It would help if I had a good list of main things wrong in this PR |
|
Thank you for your effort. While I’d like to provide a comprehensive list of issues, my limited bandwidth mean the following points are only representative examples, not an exhaustive list. This PR still contains several issues that should have been identified and resolved during the self-review phase. These include, but are not limited to: 1. UI Issues
I have attached screenshots below highlighting some of these UI issues. 2. Code Clarity and Intent 3. Testing Concerns
TestActionsTokenPackagePermissionfunc TestActionsTokenPackagePermission(t *testing.T) {
onGiteaRun(t, func(t *testing.T, u *url.URL) {
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
session := loginUser(t, user2.Name)
user2Token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWritePackage)
// create a new repo
apiRepo := createActionsTestRepo(t, user2Token, "actions-permission", false)
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID})
httpContext := NewAPITestContext(t, user2.Name, repo.Name, auth_model.AccessTokenScopeWriteRepository)
defer doAPIDeleteRepository(httpContext)(t)
// create a mock runner
runner := newMockRunner()
runner.registerAsRepoRunner(t, repo.OwnerName, repo.Name, "mock-runner", []string{"ubuntu-latest"}, false)
// create a package for test
packageName := fmt.Sprintf("test-pkg-%d", time.Now().UnixNano())
packageVersion := "1.0.0"
packageFileName := "package-test-1.bin"
packageUploadURL := fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", repo.OwnerName, packageName, packageVersion, packageFileName)
packageUploadReq := NewRequestWithBody(t, "PUT", packageUploadURL, bytes.NewReader([]byte("package content"))).
AddTokenAuth(user2Token)
MakeRequest(t, packageUploadReq, http.StatusCreated)
pkg := unittest.AssertExistsAndLoadBean(t, &packages_model.Package{
OwnerID: repo.OwnerID,
Type: packages_model.TypeGeneric,
Name: packageName,
})
linkReq := NewRequest(t, "POST", fmt.Sprintf("/api/v1/packages/%s/generic/%s/-/link/%s", repo.OwnerName, pkg.Name, repo.Name)).
AddTokenAuth(user2Token)
MakeRequest(t, linkReq, http.StatusCreated)
// set actions token permission to "write" on all units
req := NewRequestWithValues(t, "POST", fmt.Sprintf("/%s/%s/settings/actions/general/token_permissions", repo.OwnerName, repo.Name), map[string]string{
"token_permission_mode": "custom",
"max_code": "write",
"max_issues": "write",
"max_pull_requests": "write",
"max_wiki": "write",
"max_releases": "write",
"max_projects": "write",
"max_packages": "write",
"max_actions": "write",
})
resp := session.MakeRequest(t, req, http.StatusSeeOther)
require.Equal(t, fmt.Sprintf("/%s/%s/settings/actions/general", repo.OwnerName, repo.Name), test.RedirectURL(resp))
// create a workflow file
wfTreePath := ".gitea/workflows/test_permissions.yml"
wfFileContent := `name: Test Permissions
on:
push:
paths:
- '.gitea/workflows/test_permissions.yml'
jobs:
test-job:
runs-on: ubuntu-latest
steps:
- run: echo "write"
`
opts := getWorkflowCreateFileOptions(user2, repo.DefaultBranch, "create "+wfTreePath, wfFileContent)
createWorkflowFile(t, user2Token, user2.Name, repo.Name, wfTreePath, opts)
// fetch a task(*runnerv1.Task) and get its token
runnerTask := runner.fetchTask(t)
taskToken := runnerTask.Secrets["GITEA_TOKEN"]
require.NotEmpty(t, taskToken)
// taskToken should have "write" permission on packages
newPackageVersion := "2.0.0"
newPackageFileName := "package-test-2.bin"
newPackageUploadURL := fmt.Sprintf("/api/packages/%s/generic/%s/%s/%s", repo.OwnerName, packageName, newPackageVersion, newPackageFileName)
writeReq := NewRequestWithBody(t, "PUT", newPackageUploadURL, bytes.NewReader([]byte("new package content"))).
AddTokenAuth(taskToken)
MakeRequest(t, writeReq, http.StatusCreated)
})
}
|
|
@lunny Thanks for the review request. However, I’ve spent too much time on this PR addressing issues that should have been caught during the self-review stage. Please ensure this PR receives at least one approval from another maintainer before requesting my review again. This will help ensure the fundamental quality is met first. Thanks for understanding. |
ChristopherHX
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do another review later as my time allows.
AW While I looked at package access, I wanted that a repo can create a org/user container without explicit repo permission via private visibility if the resource does not exist yet (I am frequently using such a feature on GitHub). However I agree the linked issue does not contains such requirement anywhere.
| "actions.general.token_permissions.access_read": "Read", | ||
| "actions.general.token_permissions.access_write": "Write", | ||
| "actions.general.token_permissions.code": "Code", | ||
| "actions.general.token_permissions.code.description": "Repository contents, commits, branches, downloads, releases, and merges.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| <!-- Follow Organization Configuration --> | ||
| <div class="field"> | ||
| <div class="ui checkbox"> | ||
| <input type="checkbox" name="follow_org_config" id="follow-org-config" {{if .FollowOrgConfig}}checked{{end}} class="js-follow-org-config"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| } | ||
| } | ||
|
|
||
| if (!isSameOrg && !actionsCfg.IsCollaborativeOwner(taskRepo.OwnerID)) || !taskRepo.IsPrivate { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need another look on this, you revoke previous valid access while isSameOrg = true and orgCfg.AllowCrossRepoAccess is off
| // DefaultTokenPermissions defines the default permissions for workflow tokens | ||
| DefaultTokenPermissions *ActionsTokenPermissions `json:"default_token_permissions,omitempty"` | ||
| // MaxTokenPermissions defines the maximum permissions (cannot be exceeded by workflow permissions keyword) | ||
| MaxTokenPermissions *ActionsTokenPermissions `json:"max_token_permissions,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why cannot I find this anywhere in org and repository settings pages? Unresolved this myself.
|
I will address all issues and do self review. |
I agree. I did some testing on GitHub, and when a brand-new, non-existent package is created via GitHub Actions, it is successfully created and automatically linked to the repository running the workflow. I believe Gitea could support this feature as well. |
1b550bc to
79c38fe
Compare
|
going to work on this weekend 🎉 |





Overview
This PR introduces granular permission controls for Gitea Actions tokens (
GITEA_TOKEN), aligning Gitea's security model with GitHub Actions standards while maintaining compatibility with Gitea's unique repository unit system.It addresses the need for finer access control by allowing administrators and repository owners to define default token permissions, set maximum permission ceilings, and control cross-repository access within organizations.
Key Features
1. Granular Token Permissions
permissions:keyword in workflow and job YAML files (e.g.,contents: read,issues: write).contentsandpackages, with no access to other units.2. Organization & Repository Settings
3. Security Hardening
4. Technical Implementation
action_run_jobtable. This ensures the token's authority is deterministic throughout the job's lifecycle.contentsscope is applied first, allowing granular scopes likecodeorreleasesto override it for precise control.How to Test
go test ./services/actions/...andgo test ./models/repo/...to verify parsing logic and permission clamping.tests/integration/actions_job_token_test.gocovering:permissions:keyword evaluation.GITEA_TOKENcapabilities.Documentation
Added a PR in gitea's docs for this : https://gitea.com/gitea/docs/pulls/318
/fixes #24635
/claim #24635