From da7b81d4079dca56ee7e9485f2ae287fafc249ab Mon Sep 17 00:00:00 2001 From: Norbert Szulc Date: Sun, 1 Nov 2026 07:27:12 +0100 Subject: [PATCH] feat: add commitstatus access token scope (#40144) Add `read:commitstatus` and `write:commitstatus` token scopes, so CI and other integrations can report commit statuses without a token that grants full repository access. - The commit status endpoints (`/statuses/{sha}`, `/commits/{ref}/status` and `/commits/{ref}/statuses`) accept the new scopes, and the repository scopes keep working as they include them - Creating a status still requires code write permission on the repository, the scope only narrows what the token may do - `public-only` tokens still cannot reach private repositories - `PathGroup` middlewares now run only for a matched path, so unrelated methods and paths that fall back to the group are not rejected by its scope check Fixes: https://github.com/go-gitea/gitea/issues/32625 Ref: https://github.com/go-gitea/gitea/pull/35383 Docs: https://gitea.com/gitea/docs/pulls/622 Signed-off-by: Norbert Szulc Co-authored-by: Aiden Scandella Co-authored-by: silverwind Co-authored-by: wxiaoguang --- models/auth/access_token_scope.go | 17 ++++++++-- models/auth/access_token_scope_test.go | 4 ++- modules/structs/user_app.go | 2 +- modules/web/router.go | 4 +-- modules/web/router_path.go | 15 ++++----- modules/web/router_test.go | 6 ++-- options/locale/locale_en-US.json | 2 +- routers/api/v1/api.go | 30 +++++++++++------- templates/swagger/v1-openapi3.generated.json | 1 + templates/swagger/v1-swagger.generated.json | 1 + tests/integration/repo_commits_test.go | 33 ++++++++++++++++++++ 11 files changed, 87 insertions(+), 28 deletions(-) diff --git a/models/auth/access_token_scope.go b/models/auth/access_token_scope.go index 1585bcbcd06..83bf8a01b5e 100644 --- a/models/auth/access_token_scope.go +++ b/models/auth/access_token_scope.go @@ -24,6 +24,7 @@ const ( AccessTokenScopeCategoryIssue AccessTokenScopeCategoryRepository AccessTokenScopeCategoryUser + AccessTokenScopeCategoryCommitStatus ) // AllAccessTokenScopeCategories contains all access token scope categories @@ -37,6 +38,7 @@ var AllAccessTokenScopeCategories = []AccessTokenScopeCategory{ AccessTokenScopeCategoryIssue, AccessTokenScopeCategoryRepository, AccessTokenScopeCategoryUser, + AccessTokenScopeCategoryCommitStatus, } // AccessTokenScopeLevel represents the access levels without a given scope category @@ -83,6 +85,9 @@ const ( AccessTokenScopeReadUser AccessTokenScope = "read:user" AccessTokenScopeWriteUser AccessTokenScope = "write:user" + + AccessTokenScopeReadCommitStatus AccessTokenScope = "read:commitstatus" + AccessTokenScopeWriteCommitStatus AccessTokenScope = "write:commitstatus" ) // accessTokenScopeBitmap represents a bitmap of access token scopes. @@ -119,12 +124,15 @@ const ( accessTokenScopeReadIssueBits accessTokenScopeBitmap = 1 << iota accessTokenScopeWriteIssueBits accessTokenScopeBitmap = 1< 64 scopes, // refactoring the whole implementation in this file (and only this file) is needed. @@ -143,6 +151,7 @@ var allAccessTokenScopes = []AccessTokenScope{ AccessTokenScopeWriteIssue, AccessTokenScopeReadIssue, AccessTokenScopeWriteRepository, AccessTokenScopeReadRepository, AccessTokenScopeWriteUser, AccessTokenScopeReadUser, + AccessTokenScopeWriteCommitStatus, AccessTokenScopeReadCommitStatus, } // allAccessTokenScopeBits contains all access token scopes. @@ -168,6 +177,8 @@ var allAccessTokenScopeBits = map[AccessTokenScope]accessTokenScopeBitmap{ AccessTokenScopeWriteRepository: accessTokenScopeWriteRepositoryBits, AccessTokenScopeReadUser: accessTokenScopeReadUserBits, AccessTokenScopeWriteUser: accessTokenScopeWriteUserBits, + AccessTokenScopeReadCommitStatus: accessTokenScopeReadCommitStatusBits, + AccessTokenScopeWriteCommitStatus: accessTokenScopeWriteCommitStatusBits, } // readAccessTokenScopes maps a scope category to the read permission scope @@ -182,6 +193,7 @@ var accessTokenScopes = map[AccessTokenScopeLevel]map[AccessTokenScopeCategory]A AccessTokenScopeCategoryIssue: AccessTokenScopeReadIssue, AccessTokenScopeCategoryRepository: AccessTokenScopeReadRepository, AccessTokenScopeCategoryUser: AccessTokenScopeReadUser, + AccessTokenScopeCategoryCommitStatus: AccessTokenScopeReadCommitStatus, }, Write: { AccessTokenScopeCategoryActivityPub: AccessTokenScopeWriteActivityPub, @@ -193,6 +205,7 @@ var accessTokenScopes = map[AccessTokenScopeLevel]map[AccessTokenScopeCategory]A AccessTokenScopeCategoryIssue: AccessTokenScopeWriteIssue, AccessTokenScopeCategoryRepository: AccessTokenScopeWriteRepository, AccessTokenScopeCategoryUser: AccessTokenScopeWriteUser, + AccessTokenScopeCategoryCommitStatus: AccessTokenScopeWriteCommitStatus, }, } diff --git a/models/auth/access_token_scope_test.go b/models/auth/access_token_scope_test.go index 9bf49a6b215..ed43c639566 100644 --- a/models/auth/access_token_scope_test.go +++ b/models/auth/access_token_scope_test.go @@ -17,7 +17,7 @@ type scopeTestNormalize struct { } func TestAccessTokenScope_Normalize(t *testing.T) { - assert.Equal(t, []string{"activitypub", "admin", "issue", "misc", "notification", "organization", "package", "repository", "user"}, GetAccessTokenCategories()) + assert.Equal(t, []string{"activitypub", "admin", "commitstatus", "issue", "misc", "notification", "organization", "package", "repository", "user"}, GetAccessTokenCategories()) assert.False(t, AccessTokenScopeNone.HasPermissionScope()) tests := []scopeTestNormalize{ {"", "", nil}, @@ -30,6 +30,8 @@ func TestAccessTokenScope_Normalize(t *testing.T) { {"all", "all", nil}, {"write:activitypub,write:admin,write:misc,write:notification,write:organization,write:package,write:issue,write:repository,write:user", "all", nil}, {"write:activitypub,write:admin,write:misc,write:notification,write:organization,write:package,write:issue,write:repository,write:user,public-only", "public-only,all", nil}, + {"write:repository,write:commitstatus", "write:repository", nil}, + {"read:repository,write:commitstatus", "read:repository,write:commitstatus", nil}, } for _, scope := range GetAccessTokenCategories() { diff --git a/modules/structs/user_app.go b/modules/structs/user_app.go index eb894a8a99b..029ca8a1a93 100644 --- a/modules/structs/user_app.go +++ b/modules/structs/user_app.go @@ -35,7 +35,7 @@ type AccessTokenList []*AccessToken type CreateAccessTokenOption struct { // required: true Name string `json:"name" binding:"Required"` - // example: ["all", "read:activitypub","read:issue", "write:misc", "read:notification", "read:organization", "read:package", "read:repository", "read:user"] + // example: ["all", "read:activitypub", "write:commitstatus", "read:issue", "write:misc", "read:notification", "read:organization", "read:package", "read:repository", "read:user"] Scopes []string `json:"scopes"` } diff --git a/modules/web/router.go b/modules/web/router.go index d1384028390..2ddfbdaec67 100644 --- a/modules/web/router.go +++ b/modules/web/router.go @@ -313,7 +313,7 @@ func (r *Router) Combo(pattern string, h ...any) *Combo { // It is only designed to resolve some special cases which chi router can't handle. // For most cases, it shouldn't be used because it needs to iterate all rules to find the matched one (inefficient). func (r *Router) PathGroup(pattern string, fn func(g *RouterPathGroup), h ...any) { - g := &RouterPathGroup{r: r, pathParam: "*"} + g := &RouterPathGroup{r: r, pathParam: "*", middlewares: h} // run h only on a match, unmatched methods and paths also fall back here fn(g) - r.Any(pattern, append(h, g.ServeHTTP)...) + r.Any(pattern, g.ServeHTTP) } diff --git a/modules/web/router_path.go b/modules/web/router_path.go index 46cc0d7c0a9..2624aef6a28 100644 --- a/modules/web/router_path.go +++ b/modules/web/router_path.go @@ -17,10 +17,11 @@ import ( ) type RouterPathGroup struct { - r *Router - pathParam string - matchers []*routerPathMatcher - unescape bool + r *Router + pathParam string + matchers []*routerPathMatcher + unescape bool + middlewares []any } func (g *RouterPathGroup) ServeHTTP(resp http.ResponseWriter, req *http.Request) { @@ -55,7 +56,7 @@ func (g *RouterPathGroup) MatchPath(methods, pattern string, h ...any) { } func (g *RouterPathGroup) MatchPattern(methods string, pattern *RouterPathGroupPattern, h ...any) { - g.matchers = append(g.matchers, newRouterPathMatcher(methods, pattern, h...)) + g.matchers = append(g.matchers, newRouterPathMatcher(methods, pattern, g.middlewares, h...)) } func (g *RouterPathGroup) UseUnescapedPath() { @@ -127,8 +128,8 @@ func isValidMethod(name string) bool { return false } -func newRouterPathMatcher(methods string, patternRegexp *RouterPathGroupPattern, h ...any) *routerPathMatcher { - middlewares, handlerFunc, hasPreMiddlewares := wrapMiddlewareAndHandler(nil, patternRegexp.middlewares, h) +func newRouterPathMatcher(methods string, patternRegexp *RouterPathGroupPattern, uses []any, h ...any) *routerPathMatcher { + middlewares, handlerFunc, hasPreMiddlewares := wrapMiddlewareAndHandler(uses, patternRegexp.middlewares, h) if hasPreMiddlewares { panic("pre-middlewares are not supported in router path matcher") } diff --git a/modules/web/router_test.go b/modules/web/router_test.go index c1453217232..88d4ce1dc4d 100644 --- a/modules/web/router_test.go +++ b/modules/web/router_test.go @@ -102,7 +102,7 @@ func TestPathProcessor(t *testing.T) { testProcess := func(pattern, uri string, expectedPathParams map[string]string) { chiCtx := chi.NewRouteContext() chiCtx.RouteMethod = "GET" - p := newRouterPathMatcher("GET", patternRegexp(pattern), http.NotFound) + p := newRouterPathMatcher("GET", patternRegexp(pattern), nil, http.NotFound) shouldProcess := expectedPathParams != nil if unescape { uri, _ = url.PathUnescape(uri) @@ -236,12 +236,12 @@ func TestRouter(t *testing.T) { testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1/d2/000", resultStruct{ method: "GET", pathParams: map[string]string{"reponame": "the-repo", "username": "the-user", "*": "d1/d2/000"}, - handlerMarks: []string{"s1", "not-found:/api/v1"}, + handlerMarks: []string{"not-found:/api/v1"}, }) testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1/d2/fn?stop=s1", resultStruct{ method: "GET", - pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1/d2/fn"}, + pathParams: map[string]string{"*": "d1/d2/fn", "dir": "d1/d2", "file": "fn", "reponame": "the-repo", "username": "the-user"}, handlerMarks: []string{"s1"}, }) diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 53153e51b19..902534b3065 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -853,7 +853,7 @@ "settings.permission_anonymous_read": "Anonymous Read", "settings.permission_everyone_read": "Everyone Read", "settings.permission_everyone_write": "Everyone Write", - "settings.access_token_desc": "Selected token permissions limit authorization only to the corresponding API routes. Read the documentation for more information.", + "settings.access_token_desc": "Selected token permissions limit authorization only to the corresponding API routes. Repository permissions include commit status permissions. Read the documentation for more information.", "settings.at_least_one_permission": "You must select at least one permission to create a token", "settings.permissions_list": "Permissions:", "settings.manage_oauth2_applications": "Manage OAuth2 Applications", diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index f876f91f8fc..0a364b27c4f 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1489,23 +1489,13 @@ func Routes() *web.Router { }) m.Get("/{base}/*", repo.GetPullRequestByBaseHead) }, mustAllowPulls, reqRepoReader(unit.TypeCode), context.ReferencesGitRepo()) - m.Group("/statuses", func() { // "/statuses/{sha}" only accepts commit ID - m.Combo("/{sha}").Get(repo.GetCommitStatuses). - Post(reqToken(), reqRepoWriter(unit.TypeCode), bind(api.CreateStatusOption{}), repo.NewCommitStatus) - }, reqRepoReader(unit.TypeCode)) m.Group("/commits", func() { m.Group("", func() { m.Get("", repo.GetAllCommits) m.Get("/{sha}", repo.GetSingleCommit) // GitHub-compatible endpoint m.Get("/{sha}.{diffType:diff|patch}", repo.DownloadCommitDiffOrPatch) }, context.ReferencesGitRepo(true)) - m.PathGroup("/*", func(g *web.RouterPathGroup) { - // Mis-configured reverse proxy might decode the `%2F` to slash ahead, so we need to support both formats (escaped, unescaped) here. - // It also matches GitHub's behavior - g.MatchPath("GET", "//status", repo.GetCombinedCommitStatusByRef) - g.MatchPath("GET", "//statuses", repo.GetCommitStatusesByRef) - g.MatchPath("GET", "//pull", repo.GetCommitPullRequest) - }) + m.Get("/{sha}/pull", repo.GetCommitPullRequest) }, reqRepoReader(unit.TypeCode)) m.Group("/git", func() { m.Group("/commits", func() { @@ -1566,6 +1556,24 @@ func Routes() *web.Router { }, repoAssignment()) }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository)) + m.Group("/repos/{username}/{reponame}", func() { // outside "/repos" so tokens need only the commitstatus scope + middlewares := []any{ + tokenRequiresScopes(auth_model.AccessTokenScopeCategoryCommitStatus), + repoAssignment(), reqRepoReader(unit.TypeCode), + } + // "/.../statuses/{sha}" only accepts commit ID + m.Combo("/statuses/{sha}", middlewares...). + Get(repo.GetCommitStatuses). + Post(reqToken(), reqRepoWriter(unit.TypeCode), bind(api.CreateStatusOption{}), repo.NewCommitStatus) + + // Mis-configured reverse proxy might decode the `%2F` to slash ahead, so we need to support both formats (escaped, unescaped) here. + // It also matches GitHub's behavior + m.PathGroup("/commits/*", func(g *web.RouterPathGroup) { + g.MatchPath("GET", "//status", repo.GetCombinedCommitStatusByRef) + g.MatchPath("GET", "//statuses", repo.GetCommitStatusesByRef) + }, middlewares...) + }) + // Artifacts direct download endpoint authenticates via signed url // it is protected by the "sig" parameter (to help to access private repo), so no need to use other middlewares m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw) diff --git a/templates/swagger/v1-openapi3.generated.json b/templates/swagger/v1-openapi3.generated.json index 7502806197d..2d5376621da 100644 --- a/templates/swagger/v1-openapi3.generated.json +++ b/templates/swagger/v1-openapi3.generated.json @@ -3663,6 +3663,7 @@ "example": [ "all", "read:activitypub", + "write:commitstatus", "read:issue", "write:misc", "read:notification", diff --git a/templates/swagger/v1-swagger.generated.json b/templates/swagger/v1-swagger.generated.json index 84231c3d429..8e2c5b4d1a5 100644 --- a/templates/swagger/v1-swagger.generated.json +++ b/templates/swagger/v1-swagger.generated.json @@ -26124,6 +26124,7 @@ "example": [ "all", "read:activitypub", + "write:commitstatus", "read:issue", "write:misc", "read:notification", diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index f95fc7978b6..a54557bbd01 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -204,6 +204,39 @@ func TestRepoCommitsWithStatus(t *testing.T) { assert.NotEmpty(t, statuses) assert.NotEmpty(t, status.Statuses) }) + t.Run("TokenScopes", func(t *testing.T) { + commitIDs := map[string]string{"repo1": "65f1bf27bc3bf70f64657658635e66094edbcb4d", "repo2": "1032bbf17fbc0d9c95bb5418dabe8f8c99278700"} + for _, test := range []struct { + name string + userID int64 + reponame string + scope auth_model.AccessTokenScope + getCode, postCode int + }{ + {"WriteCommitStatus", 2, "repo1", auth_model.AccessTokenScopeWriteCommitStatus, http.StatusOK, http.StatusCreated}, + {"ReadCommitStatus", 2, "repo1", auth_model.AccessTokenScopeReadCommitStatus, http.StatusOK, http.StatusForbidden}, + {"WriteRepository", 2, "repo1", auth_model.AccessTokenScopeWriteRepository, http.StatusOK, http.StatusCreated}, + {"ReadRepository", 2, "repo1", auth_model.AccessTokenScopeReadRepository, http.StatusOK, http.StatusForbidden}, + {"UnrelatedScope", 2, "repo1", auth_model.AccessTokenScopeWriteIssue, http.StatusForbidden, http.StatusForbidden}, + {"NonWriter", 4, "repo1", auth_model.AccessTokenScopeWriteCommitStatus, http.StatusOK, http.StatusForbidden}, + {"PublicOnlyPrivateRepo", 2, "repo2", "public-only,write:commitstatus", http.StatusNotFound, http.StatusNotFound}, + } { + t.Run(test.name, func(t *testing.T) { + token := &auth_model.AccessToken{UID: test.userID, Name: test.name, Scope: test.scope} + require.NoError(t, auth_model.NewAccessToken(t.Context(), token)) + repoLink, commitID := "/api/v1/repos/user2/"+test.reponame, commitIDs[test.reponame] + for _, link := range []string{"/statuses/" + commitID, "/commits/master/status", "/commits/master/statuses"} { + MakeRequest(t, NewRequest(t, "GET", repoLink+link).AddTokenAuth(token.Token), test.getCode) + } + MakeRequest(t, NewRequestWithJSON(t, "POST", repoLink+"/statuses/"+commitID, api.CreateStatusOption{State: commitstatus.CommitStatusSuccess}).AddTokenAuth(token.Token), test.postCode) + }) + } + t.Run("UnmatchedCommitPathSkipsCommitStatusScope", func(t *testing.T) { + token := &auth_model.AccessToken{UID: 2, Name: "UnmatchedCommitPath", Scope: auth_model.AccessTokenScopeWriteIssue} + require.NoError(t, auth_model.NewAccessToken(t.Context(), token)) + MakeRequest(t, NewRequest(t, "POST", "/api/v1/repos/user2/repo1/commits/"+commitIDs["repo1"]+"/pull").AddTokenAuth(token.Token), http.StatusNotFound) + }) + }) } func TestRepoCommitsStatus(t *testing.T) {