From 6782d0a37d1f29c691d99b7c66f3e4799edb56eb Mon Sep 17 00:00:00 2001 From: bircni Date: Sat, 31 Oct 2026 13:03:59 +0100 Subject: [PATCH] fix: harden context user access scope (#39945) API tokens and Actions task tokens could see data beyond their scope through user-scoped API routes and web profile pages. - Actions task tokens authenticate as a shared system user with a negative ID, which org, project and SSH/GPG key filters treated as "no owner filter". These filters now check `!= 0`, so system users own nothing. The `/api/v1/user` routes except `GET /api/v1/user` only accept individual and bot accounts, and repo search no longer skips access checks for the owner. - `/{username}` only accepts tokens for RSS/Atom feeds. HTML profile and org pages render as anonymous for token requests. - User and org feeds require `read:user` or `read:organization` from tokens, the same way repo feeds require `read:repository`. Co-authored-by: silverwind Co-authored-by: wxiaoguang --- models/asymkey/gpg_key.go | 2 +- models/asymkey/ssh_key.go | 2 +- models/organization/org_list.go | 2 +- models/project/project.go | 2 +- models/repo/repo_list.go | 3 +- routers/api/v1/api.go | 16 ++++-- routers/api/v1/user/repo.go | 1 + routers/web/feed/profile.go | 6 ++ routers/web/web.go | 7 ++- .../api_actions_permission_test.go | 12 ++++ tests/integration/feed_user_test.go | 57 ++++++++++--------- 11 files changed, 71 insertions(+), 39 deletions(-) diff --git a/models/asymkey/gpg_key.go b/models/asymkey/gpg_key.go index 64685e4cc24..99914a7bc24 100644 --- a/models/asymkey/gpg_key.go +++ b/models/asymkey/gpg_key.go @@ -85,7 +85,7 @@ func (opts FindGPGKeyOptions) ToConds() builder.Cond { cond = cond.And(builder.Eq{"primary_key_id": ""}) } - if opts.OwnerID > 0 { + if opts.OwnerID != 0 { cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) } if opts.KeyID != "" { diff --git a/models/asymkey/ssh_key.go b/models/asymkey/ssh_key.go index a84ba7736ca..3d628f8d10b 100644 --- a/models/asymkey/ssh_key.go +++ b/models/asymkey/ssh_key.go @@ -224,7 +224,7 @@ func (opts FindPublicKeyOptions) ToOrders() string { func (opts FindPublicKeyOptions) ToConds() builder.Cond { cond := builder.NewCond() - if opts.OwnerID > 0 { + if opts.OwnerID != 0 { cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) } if opts.Fingerprint != "" { diff --git a/models/organization/org_list.go b/models/organization/org_list.go index 9a64bbd4192..508205b72a2 100644 --- a/models/organization/org_list.go +++ b/models/organization/org_list.go @@ -64,7 +64,7 @@ func queryUserOrgIDs(userID int64, includePrivate bool) *builder.Builder { func (opts FindOrgOptions) ToConds() builder.Cond { var cond builder.Cond = builder.Eq{"`user`.`type`": user_model.UserTypeOrganization} - if opts.UserID > 0 { + if opts.UserID != 0 { cond = cond.And(builder.In("`user`.`id`", queryUserOrgIDs(opts.UserID, opts.IncludeVisibility == structs.VisibleTypePrivate))) } // public=0, limited=1, private=2 diff --git a/models/project/project.go b/models/project/project.go index d57df23dbbb..ad09ba6eed9 100644 --- a/models/project/project.go +++ b/models/project/project.go @@ -225,7 +225,7 @@ func (opts SearchOptions) ToConds() builder.Cond { if opts.Type > 0 { cond = cond.And(builder.Eq{"type": opts.Type}) } - if opts.OwnerID > 0 { + if opts.OwnerID != 0 { cond = cond.And(builder.Eq{"owner_id": opts.OwnerID}) } diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index c053e60d4cd..2515012551f 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -369,8 +369,7 @@ func SearchRepositoryCondition(opts SearchRepoOptions) builder.Cond { cond := builder.NewCond() if opts.Private { - if opts.Actor != nil && !opts.Actor.IsAdmin && opts.Actor.ID != opts.OwnerID { - // OK we're in the context of a User + if opts.Actor != nil && !opts.Actor.IsAdmin { cond = cond.And(AccessibleRepositoryCondition(opts.Actor, unit.TypeInvalid)) } } else { diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 0d5b5271b4b..f876f91f8fc 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -256,12 +256,18 @@ func rejectPublicOnly() func(ctx *context.APIContext) { } } -func contextAuthenticatedUser() func(ctx *context.APIContext) { +func authenticatedUserAsContextUser(allowSystemUser bool) func(ctx *context.APIContext) { return func(ctx *context.APIContext) { if !user_model.DoerTokenAllowsOwner(ctx.Doer, ctx.Doer) { ctx.APIError(http.StatusForbidden, "this endpoint is not available for public-only tokens") return } + isIndividualOrBot := ctx.Doer.IsIndividual() || ctx.Doer.IsTypeBot() + isSystem := ctx.Doer.ID < 0 + if !isIndividualOrBot || isSystem && !allowSystemUser { + ctx.APIError(http.StatusForbidden, "this endpoint is only available for individual user or bot accounts") + return + } ctx.ContextUser = ctx.Doer } } @@ -1097,8 +1103,11 @@ func Routes() *web.Router { }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser), reqToken()) // Users (requires user scope) + contextRealDoerUser := authenticatedUserAsContextUser(false) + contextRealOrSystemUser := authenticatedUserAsContextUser(true) + m.Get("/user", tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser), reqToken(), contextRealOrSystemUser, user.GetAuthenticatedUser) m.Group("/user", func() { - m.Get("", user.GetAuthenticatedUser) + m.Get("/orgs", reqToken(), tokenRequiresScopes(auth_model.AccessTokenScopeCategoryOrganization), org.ListMyOrgs) m.Group("/settings", func() { m.Get("", user.GetUserSettings) m.Patch("", bind(api.UserSettingsOptions{}), user.UpdateUserSettings) @@ -1218,7 +1227,7 @@ func Routes() *web.Router { m.Delete("", user.UnblockUser) }, context.UserAssignmentAPI()) }, rejectPublicOnly()) - }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser), reqToken(), contextAuthenticatedUser()) + }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser), reqToken(), contextRealDoerUser) // Repositories (requires repo scope, org scope) m.Post("/org/{org}/repos", @@ -1719,7 +1728,6 @@ func Routes() *web.Router { }, reqToken(), tokenRequiresScopes(auth_model.AccessTokenScopeCategoryPackage), context.UserAssignmentAPI(), context.PackageAssignmentAPI(), reqPackageAccess(perm.AccessModeRead)) // Organizations - m.Get("/user/orgs", reqToken(), tokenRequiresScopes(auth_model.AccessTokenScopeCategoryUser, auth_model.AccessTokenScopeCategoryOrganization), org.ListMyOrgs) m.Group("/users/{username}/orgs", func() { m.Get("", reqToken(), org.ListUserOrgs) m.Get("/{org}/permissions", reqToken(), org.GetUserOrgsPermissions) diff --git a/routers/api/v1/user/repo.go b/routers/api/v1/user/repo.go index 0f0389d371d..8ace0a1c00b 100644 --- a/routers/api/v1/user/repo.go +++ b/routers/api/v1/user/repo.go @@ -126,6 +126,7 @@ func ListMyRepos(ctx *context.APIContext) { permission, err := access_model.GetDoerRepoPermission(ctx, repo, ctx.Doer) if err != nil { ctx.APIErrorInternal(err) + return } results[i] = convert.ToRepo(ctx, repo, permission) } diff --git a/routers/web/feed/profile.go b/routers/web/feed/profile.go index 201649ab2d6..af0987dad70 100644 --- a/routers/web/feed/profile.go +++ b/routers/web/feed/profile.go @@ -7,10 +7,12 @@ import ( "time" activities_model "gitea.dev/models/activities" + auth_model "gitea.dev/models/auth" "gitea.dev/models/organization" "gitea.dev/models/renderhelper" "gitea.dev/modules/markup/markdown" "gitea.dev/modules/setting" + "gitea.dev/modules/util" "gitea.dev/services/context" feed_service "gitea.dev/services/feed" @@ -33,6 +35,10 @@ func showUserFeed(ctx *context.Context, formatType string) { ctx.NotFound(nil) return } + context.CheckTokenScopes(ctx, util.Iif(ctx.ContextUser.IsOrganization(), auth_model.AccessTokenScopeReadOrganization, auth_model.AccessTokenScopeReadUser)) + if ctx.Written() { + return + } includePrivate := ctx.IsSigned && (ctx.Doer.IsAdmin || ctx.Doer.ID == ctx.ContextUser.ID) if ctx.IsSigned && ctx.ContextUser.IsOrganization() && !includePrivate { diff --git a/routers/web/web.go b/routers/web/web.go index 8a07fd4b26b..33fc62a2f56 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -917,10 +917,11 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { m.Get("/-/ownername.png/{username}", user.AvatarByUsernameSize) m.Get("/-/ownername.keys/{username}", user.ShowSSHKeys) m.Get("/-/ownername.gpg/{username}", user.ShowGPGKeys) - m.Get("/-/ownername.rss/{username}", feed.ShowUserFeedRSS) - m.Get("/-/ownername.atom/{username}", feed.ShowUserFeedAtom) + // tokens only authenticate feeds, the other variants render data outside any token scope + m.Get("/-/ownername.rss/{username}", webAuth.AllowBasic, feed.ShowUserFeedRSS) + m.Get("/-/ownername.atom/{username}", webAuth.AllowBasic, feed.ShowUserFeedAtom) m.Get("/{username}", user.OwnerProfile) - }, webAuth.AllowBasic, optSignIn, context.UserAssignmentWeb(), context.ReqContextUserVisibleToDoer()) + }, optSignIn, context.UserAssignmentWeb(), context.ReqContextUserVisibleToDoer()) m.Post("/{username}", reqSignIn, context.UserAssignmentWeb(), context.ReqContextUserVisibleToDoer(), user.ActionUserFollow) diff --git a/tests/integration/api_actions_permission_test.go b/tests/integration/api_actions_permission_test.go index 90908662966..65413ea0677 100644 --- a/tests/integration/api_actions_permission_test.go +++ b/tests/integration/api_actions_permission_test.go @@ -24,6 +24,17 @@ func testActionUserSignIn(t *testing.T) { assert.Equal(t, "gitea-actions", u.UserName) } +func testActionUserOwnsNothing(t *testing.T) { + const token = "8061e833a55f6fc0157c98b883e91fcfeeb1a71a" + for _, link := range []string{"/api/v1/user/repos", "/api/v1/user/orgs", "/api/v1/user/actions/runs"} { + MakeRequest(t, NewRequest(t, "GET", link).AddTokenAuth(token), http.StatusForbidden) + } + for _, link := range []string{"/api/v1/users/gitea-actions/orgs", "/api/v1/users/gitea-actions/keys"} { + assert.JSONEq(t, "[]", MakeRequest(t, NewRequest(t, "GET", link).AddTokenAuth(token), http.StatusOK).Body.String(), link) + } + assert.NotContains(t, MakeRequest(t, NewRequest(t, "GET", "/api/v1/repos/search?uid=-2").AddTokenAuth(token), http.StatusOK).Body.String(), `"private":true`) +} + func testActionUserAccessPublicRepo(t *testing.T) { req := NewRequestf(t, "GET", "/api/v1/repos/user2/repo1/raw/README.md"). AddTokenAuth("8061e833a55f6fc0157c98b883e91fcfeeb1a71a") @@ -48,6 +59,7 @@ func TestActionUserAccessPermission(t *testing.T) { defer tests.PrepareTestEnv(t)() t.Run("ActionUserSignIn", testActionUserSignIn) + t.Run("ActionUserOwnsNothing", testActionUserOwnsNothing) t.Run("ActionUserAccessPublicRepo", testActionUserAccessPublicRepo) t.Run("ActionUserNoAccessOtherPrivateRepo", testActionUserNoAccessOtherPrivateRepo) } diff --git a/tests/integration/feed_user_test.go b/tests/integration/feed_user_test.go index 280b21ffd6e..5a5b952b45c 100644 --- a/tests/integration/feed_user_test.go +++ b/tests/integration/feed_user_test.go @@ -6,6 +6,7 @@ package integration import ( "encoding/xml" "net/http" + "strings" "testing" auth_model "gitea.dev/models/auth" @@ -38,6 +39,36 @@ type RSS struct { func TestFeedUser(t *testing.T) { defer tests.PrepareTestEnv(t)() + resp := MakeRequest(t, NewRequest(t, "GET", "/user2.atom"), http.StatusOK) + assert.Contains(t, resp.Body.String(), `