fix: various security fixes (#38406) (#38426)

Backport #38406 by @bircni

Addresses a batch of privately reported security issues, grouped by
area:

- **SSRF** - migration PR-patch/asset fetches, OAuth2 avatar & OpenID
discovery, pull-mirror URL re-validation, and the outbound proxy path.
- **Access-token scope** - prevent scope escalation on token creation;
keep public-only tokens confined (feeds, packages, Actions listings,
star/watch lists, limited/private owners).
- **Access control / disclosure** - go-get default-branch leak, webhook
authorization-header leak, watch clearing on private transitions,
label/attachment scoping.
- **Denial of service** - input bounds for npm dist-tags, Debian control
files, Arch file lists, and SSH keys.

### 📌 Attention for site admins

Not breaking - existing configs keep working - but two changes are worth
a look:

- **New SSRF protection** Outbound requests (migrations, OAuth2 avatars,
OpenID discovery, pull mirrors, proxy path) are now validated against
the allow/block host lists. If your instance legitimately reaches
internal hosts, you may need to add them to
`[security].ALLOWED_HOST_LIST` (and the relevant `ALLOW_LOCALNETWORKS`
settings).
- **Deprecation** `[webhook].ALLOWED_HOST_LIST` is deprecated and will
be removed in a future release. Use `[security].ALLOWED_HOST_LIST`
instead; the old key still works for now.

Co-authored-by: bircni <bircni@icloud.com>
Co-authored-by: TheFox0x7 <thefox0x7@gmail.com>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: Zettat123 <zettat123@gmail.com>
This commit is contained in:
Giteabot
2026-07-12 10:41:58 -07:00
committed by GitHub
parent acbc75e2bd
commit de4b8277e9
93 changed files with 1715 additions and 138 deletions
+7
View File
@@ -99,6 +99,10 @@ type FindRunJobOptions struct {
UpdatedBefore timeutil.TimeStamp
ConcurrencyGroup string
OrderBy db.SearchOrderBy
// AccessibleRepoIDsSubQuery, when non-nil, restricts results to the repo IDs selected by the
// subquery (the caller's accessible repos). A nil value means no restriction. Using a subquery
// instead of a materialized ID slice avoids exceeding DB parameter limits for large owners.
AccessibleRepoIDsSubQuery *builder.Builder
}
var JobOrderByMap = map[string]map[string]db.SearchOrderBy{
@@ -132,6 +136,9 @@ func (opts FindRunJobOptions) ToConds() builder.Cond {
}
cond = cond.And(builder.Eq{"`action_run_job`.concurrency_group": opts.ConcurrencyGroup})
}
if opts.AccessibleRepoIDsSubQuery != nil {
cond = cond.And(builder.In("`action_run_job`.repo_id", opts.AccessibleRepoIDsSubQuery))
}
return cond
}
+7
View File
@@ -70,6 +70,10 @@ type FindRunOptions struct {
Status []Status
ConcurrencyGroup string
CommitSHA string
// AccessibleRepoIDsSubQuery, when non-nil, restricts results to the repo IDs selected by the
// subquery (the caller's accessible repos). A nil value means no restriction. Using a subquery
// instead of a materialized ID slice avoids exceeding DB parameter limits for large owners.
AccessibleRepoIDsSubQuery *builder.Builder
}
func (opts FindRunOptions) ToConds() builder.Cond {
@@ -101,6 +105,9 @@ func (opts FindRunOptions) ToConds() builder.Cond {
if opts.CommitSHA != "" {
cond = cond.And(builder.Eq{"`action_run`.commit_sha": opts.CommitSHA})
}
if opts.AccessibleRepoIDsSubQuery != nil {
cond = cond.And(builder.In("`action_run`.repo_id", opts.AccessibleRepoIDsSubQuery))
}
return cond
}
+19 -1
View File
@@ -36,7 +36,18 @@ import (
const ssh2keyStart = "---- BEGIN SSH2 PUBLIC KEY ----"
const (
// the longest RSA key ssh-keygen allows to generate is 16384 bits (2048 bytes), we still relax the limit a little here
maxKeyBinaryBytes = 4096
maxKeyContentBase64Bytes = maxKeyBinaryBytes * 4 / 3
maxKeyContentExtraBytes = 4 * 1024 // header, footer, comment
maxKeyContentBytes = maxKeyContentBase64Bytes + maxKeyContentExtraBytes
)
func extractTypeFromBase64Key(key string) (string, error) {
if len(key) > maxKeyContentBase64Bytes {
return "", util.NewInvalidArgumentErrorf("SSH public key base64 is too long")
}
b, err := base64.StdEncoding.DecodeString(key)
if err != nil || len(b) < 4 {
return "", fmt.Errorf("invalid key format: %w", err)
@@ -52,6 +63,10 @@ func extractTypeFromBase64Key(key string) (string, error) {
// parseKeyString parses any key string in OpenSSH or SSH2 format to clean OpenSSH string (RFC4253).
func parseKeyString(content string) (string, error) {
if len(content) > maxKeyContentBytes {
return "", util.NewInvalidArgumentErrorf("SSH public key content is too long")
}
// remove whitespace at start and end
content = strings.TrimSpace(content)
@@ -63,6 +78,8 @@ func parseKeyString(content string) (string, error) {
// Transform all legal line endings to a single "\n".
content = strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(content)
var b strings.Builder
b.Grow(len(content))
lines := strings.Split(content, "\n")
continuationLine := false
@@ -74,9 +91,10 @@ func parseKeyString(content string) (string, error) {
if continuationLine || strings.ContainsAny(line, ":-") {
continuationLine = strings.HasSuffix(line, "\\")
} else {
keyContent += line
b.WriteString(line)
}
}
keyContent = b.String()
t, err := extractTypeFromBase64Key(keyContent)
if err != nil {
+11 -1
View File
@@ -473,10 +473,20 @@ func runErr(t *testing.T, stdin []byte, args ...string) {
}
}
func Test_PublicKeysAreExternallyManaged(t *testing.T) {
func TestPublicKeysAreExternallyManaged(t *testing.T) {
key1 := unittest.AssertExistsAndLoadBean(t, &PublicKey{ID: 1})
externals, err := PublicKeysAreExternallyManaged(t.Context(), []*PublicKey{key1})
assert.NoError(t, err)
assert.Len(t, externals, 1)
assert.False(t, externals[0])
}
// TestCheckPublicKeyStringOversized tests if oversized SSH2 public key strings are rejected before triggering costly operations.
func TestCheckPublicKeyStringOversized(t *testing.T) {
_, err := parseKeyString(strings.Repeat("a", maxKeyContentBytes+1))
assert.ErrorContains(t, err, "SSH public key content is too long")
content := "---- BEGIN SSH2 PUBLIC KEY ----\n" + strings.Repeat("a", maxKeyContentBase64Bytes+1) + "\n--- END SSH2 PUBLIC KEY ----"
_, err = parseKeyString(content)
assert.ErrorContains(t, err, "SSH public key base64 is too long")
}
+30
View File
@@ -304,6 +304,36 @@ func (s AccessTokenScope) PublicOnly() (bool, error) {
return bitmap.hasScope(AccessTokenScopePublicOnly)
}
// CanCreateChildScope reports whether a request authenticated by this (parent) scope may mint a token
// carrying the child scope. It rejects any grantable scope the parent does not hold, closing the
// scope-escalation path. public-only is a restriction rather than a grantable permission, so it is
// ignored here (a child may always be public-only); EnforcePublicOnlyFrom handles carrying it down.
func (s AccessTokenScope) CanCreateChildScope(child AccessTokenScope) (bool, error) {
requested := child.StringSlice()
scopes := make([]AccessTokenScope, 0, len(requested))
for _, sc := range requested {
childScope := AccessTokenScope(sc)
if childScope == AccessTokenScopePublicOnly {
continue
}
scopes = append(scopes, childScope)
}
return s.HasScope(scopes...)
}
// EnforcePublicOnlyFrom adds the public-only restriction to s when the authorizing parent scope is
// public-only, so a public-only token cannot mint a child token that drops the restriction.
func (s AccessTokenScope) EnforcePublicOnlyFrom(parent AccessTokenScope) (AccessTokenScope, error) {
publicOnly, err := parent.PublicOnly()
if err != nil {
return "", err
}
if !publicOnly {
return s, nil
}
return AccessTokenScope(string(s) + "," + string(AccessTokenScopePublicOnly)).Normalize()
}
// HasScope returns true if the string has the given scope
func (s AccessTokenScope) HasScope(scopes ...AccessTokenScope) (bool, error) {
bitmap, err := s.parse()
+23
View File
@@ -89,3 +89,26 @@ func TestAccessTokenScope_HasScope(t *testing.T) {
})
}
}
func TestAccessTokenScope_EnforcePublicOnlyFrom(t *testing.T) {
tests := []struct {
in AccessTokenScope
parent AccessTokenScope
out AccessTokenScope
}{
// public-only parent forces the restriction onto the minted scope
{"write:user", "write:user,public-only", "public-only,write:user"},
// already public-only stays public-only
{"public-only,read:user", "public-only", "public-only,read:user"},
// non-public-only parent leaves the scope untouched
{"write:user", "write:user", "write:user"},
{"all", "all", "all"},
}
for _, test := range tests {
t.Run(string(test.parent)+"->"+string(test.in), func(t *testing.T) {
got, err := test.in.EnforcePublicOnlyFrom(test.parent)
assert.NoError(t, err)
assert.Equal(t, test.out, got)
})
}
}
+7
View File
@@ -627,11 +627,18 @@ func UpdateCommentAttachments(ctx context.Context, c *Comment, uuids []string) e
return nil
}
return db.WithTx(ctx, func(ctx context.Context) error {
issue, err := GetIssueByID(ctx, c.IssueID)
if err != nil {
return err
}
attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids)
if err != nil {
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err)
}
for i := range attachments {
if err := validateAttachmentForIssue(ctx, issue, attachments[i]); err != nil {
return err
}
attachments[i].IssueID = c.IssueID
attachments[i].CommentID = c.ID
if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil {
+3 -1
View File
@@ -62,8 +62,10 @@ func Test_UpdateCommentAttachment(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 1})
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: comment.IssueID})
attachment := repo_model.Attachment{
Name: "test.txt",
RepoID: issue.RepoID, // must match the comment's repo, else the cross-repo guard rejects it
Name: "test.txt",
}
assert.NoError(t, db.Insert(t.Context(), &attachment))
+32
View File
@@ -263,14 +263,46 @@ func AddDeletePRBranchComment(ctx context.Context, doer *user_model.User, repo *
return err
}
// validateAttachmentForIssue rejects a foreign or already-linked attachment before it is linked to
// issue: a known UUID could otherwise re-link (and expose) another repo's private attachment. A
// legacy attachment predating repo_id-on-upload is adopted into the issue's repo.
func validateAttachmentForIssue(ctx context.Context, issue *Issue, attachment *repo_model.Attachment) error {
if attachment.RepoID == 0 && attachment.CreatedUnix < repo_model.LegacyAttachmentMissingRepoIDCutoff {
attachment.RepoID = issue.RepoID
if err := repo_model.UpdateAttachmentByUUID(ctx, attachment, "repo_id"); err != nil {
return fmt.Errorf("update attachment repo_id [id: %d]: %w", attachment.ID, err)
}
}
if attachment.RepoID != issue.RepoID {
return util.NewPermissionDeniedErrorf("attachment belongs to a different repository")
}
if attachment.IssueID != 0 && attachment.IssueID != issue.ID {
return util.NewPermissionDeniedErrorf("attachment is already linked to another issue")
}
if attachment.ReleaseID != 0 {
return util.NewPermissionDeniedErrorf("attachment is already linked to a release")
}
return nil
}
// UpdateIssueAttachments update attachments by UUIDs for the issue
func UpdateIssueAttachments(ctx context.Context, issueID int64, uuids []string) (err error) {
if len(uuids) == 0 {
return nil
}
return db.WithTx(ctx, func(ctx context.Context) error {
issue, err := GetIssueByID(ctx, issueID)
if err != nil {
return err
}
attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids)
if err != nil {
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err)
}
for i := range attachments {
if err := validateAttachmentForIssue(ctx, issue, attachments[i]); err != nil {
return err
}
attachments[i].IssueID = issueID
if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil {
return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err)
+33
View File
@@ -0,0 +1,33 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package issues_test
import (
"testing"
issues_model "gitea.dev/models/issues"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unittest"
"gitea.dev/modules/util"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestUpdateIssueAttachmentsCrossRepo(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
// attachment id 2 belongs to repo 2 / issue 4; issue 1 lives in repo 1
issue1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
foreign := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 2})
require.NotEqual(t, issue1.RepoID, foreign.RepoID)
// re-linking a foreign repo's attachment by UUID must be rejected
err := issues_model.UpdateIssueAttachments(t.Context(), issue1.ID, []string{foreign.UUID})
assert.ErrorIs(t, err, util.ErrPermissionDenied)
// the foreign attachment must be left untouched
reloaded := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 2})
assert.Equal(t, foreign.IssueID, reloaded.IssueID)
}
+13 -6
View File
@@ -6,6 +6,7 @@ package issues
import (
"context"
"errors"
"fmt"
"slices"
"strconv"
@@ -27,12 +28,6 @@ type ErrRepoLabelNotExist struct {
RepoID int64
}
// IsErrRepoLabelNotExist checks if an error is a RepoErrLabelNotExist.
func IsErrRepoLabelNotExist(err error) bool {
_, ok := err.(ErrRepoLabelNotExist)
return ok
}
func (err ErrRepoLabelNotExist) Error() string {
return fmt.Sprintf("label does not exist [label_id: %d, repo_id: %d]", err.LabelID, err.RepoID)
}
@@ -312,6 +307,18 @@ func GetLabelInRepoByName(ctx context.Context, repoID int64, labelName string) (
return l, nil
}
// GetLabelInRepoOrOrgByID returns the label with labelID scoped to the repo, falling back to the
// repo's owning organization when ownerIsOrg is set. It returns ErrRepoLabelNotExist /
// ErrOrgLabelNotExist when the label is in neither scope, so a foreign-but-existing label ID is
// indistinguishable from a nonexistent one (no cross-repo enumeration oracle).
func GetLabelInRepoOrOrgByID(ctx context.Context, repoID, ownerID int64, ownerIsOrg bool, labelID int64) (*Label, error) {
label, err := GetLabelInRepoByID(ctx, repoID, labelID)
if err != nil && errors.Is(err, util.ErrNotExist) && ownerIsOrg {
return GetLabelInOrgByID(ctx, ownerID, labelID)
}
return label, err
}
// GetLabelInRepoByID returns a label by ID in given repository.
func GetLabelInRepoByID(ctx context.Context, repoID, labelID int64) (*Label, error) {
if labelID <= 0 || repoID <= 0 {
+5 -4
View File
@@ -12,6 +12,7 @@ import (
"gitea.dev/models/unittest"
user_model "gitea.dev/models/user"
"gitea.dev/modules/timeutil"
"gitea.dev/modules/util"
"github.com/stretchr/testify/assert"
)
@@ -94,10 +95,10 @@ func TestGetLabelInRepoByName(t *testing.T) {
assert.Equal(t, "label1", label.Name)
_, err = issues_model.GetLabelInRepoByName(t.Context(), 1, "")
assert.True(t, issues_model.IsErrRepoLabelNotExist(err))
assert.ErrorIs(t, err, util.ErrNotExist)
_, err = issues_model.GetLabelInRepoByName(t.Context(), unittest.NonexistentID, "nonexistent")
assert.True(t, issues_model.IsErrRepoLabelNotExist(err))
assert.ErrorIs(t, err, util.ErrNotExist)
}
func TestGetLabelInRepoByNames(t *testing.T) {
@@ -131,10 +132,10 @@ func TestGetLabelInRepoByID(t *testing.T) {
assert.EqualValues(t, 1, label.ID)
_, err = issues_model.GetLabelInRepoByID(t.Context(), 1, -1)
assert.True(t, issues_model.IsErrRepoLabelNotExist(err))
assert.ErrorIs(t, err, util.ErrNotExist)
_, err = issues_model.GetLabelInRepoByID(t.Context(), unittest.NonexistentID, unittest.NonexistentID)
assert.True(t, issues_model.IsErrRepoLabelNotExist(err))
assert.ErrorIs(t, err, util.ErrNotExist)
}
func TestGetLabelsInRepoByIDs(t *testing.T) {
+44 -4
View File
@@ -310,11 +310,17 @@ func userOrgTeamRepoBuilder(userID int64) *builder.Builder {
}
// userOrgTeamUnitRepoBuilder returns repo ids where user's teams can access the special unit.
// A team grants the unit either through an explicit team_unit row (access_mode > none) or by being an
// admin/owner team (team.authorize >= admin), which grants every unit regardless of team_unit rows —
// mirroring the HasAdminAccess() short-circuit in access.GetIndividualUserRepoPermission.
func userOrgTeamUnitRepoBuilder(userID int64, unitType unit.Type) *builder.Builder {
return userOrgTeamRepoBuilder(userID).
Join("INNER", "team_unit", "`team_unit`.team_id = `team_repo`.team_id").
Where(builder.Eq{"`team_unit`.`type`": unitType}).
And(builder.Gt{"`team_unit`.`access_mode`": int(perm.AccessModeNone)})
Join("INNER", "team", "`team`.id = `team_repo`.team_id").
Join("LEFT", "team_unit", builder.Expr("`team_unit`.team_id = `team_repo`.team_id AND `team_unit`.`type` = ?", unitType)).
Where(builder.Or(
builder.Gte{"`team`.authorize": int(perm.AccessModeAdmin)},
builder.Gt{"`team_unit`.`access_mode`": int(perm.AccessModeNone)},
))
}
// userOrgTeamUnitRepoCond returns a condition to select repo ids where user's teams can access the special unit.
@@ -326,7 +332,7 @@ func userOrgTeamUnitRepoCond(idStr string, userID int64, unitType unit.Type) bui
func UserOrgUnitRepoCond(idStr string, userID, orgID int64, unitType unit.Type) builder.Cond {
return builder.In(idStr,
userOrgTeamUnitRepoBuilder(userID, unitType).
And(builder.Eq{"`team_unit`.org_id": orgID}),
And(builder.Eq{"`team`.org_id": orgID}),
)
}
@@ -755,6 +761,40 @@ func FindUserCodeAccessibleOwnerRepoIDs(ctx context.Context, ownerID int64, user
))
}
// PublicRepoUnderPublicOwnerCond restricts to public repos whose owner is publicly visible: the
// "genuinely public" set a public-only token or an anonymous caller may see (a public repo under a
// limited/private owner is not publicly reachable and must be excluded).
func PublicRepoUnderPublicOwnerCond() builder.Cond {
return builder.And(
builder.Eq{"`repository`.is_private": false},
builder.In("`repository`.owner_id", builder.Select("id").From("`user`").Where(builder.Eq{"visibility": structs.VisibleTypePublic})),
)
}
// UserActionsAccessibleOwnerRepoCond selects the repos owned by ownerID whose Actions `user` may read.
// It is used to list an org/user's Actions runs and jobs (see the callers in routers/api/v1/shared).
// - owner_id = ownerID: only that owner's repos.
// - AccessibleRepositoryCondition(user, TypeActions): only repos whose Actions the user can read
// (admin/owner teams are handled inside it; a site admin is not, callers must skip the filter for one).
// - publicOnly (a public-only token): additionally limit to public repos under a public owner.
func UserActionsAccessibleOwnerRepoCond(ownerID int64, user *user_model.User, publicOnly bool) builder.Cond {
cond := builder.NewCond().And(
builder.Eq{"`repository`.owner_id": ownerID},
AccessibleRepositoryCondition(user, unit.TypeActions),
)
if publicOnly {
cond = cond.And(PublicRepoUnderPublicOwnerCond())
}
return cond
}
// FindUserActionsAccessibleOwnerRepoIDsSubQuery returns a subquery selecting the repository IDs the user
// can see for the given owner. Callers embed it in an `IN (...)` condition so that a large owner does not
// materialize every repo ID into the SQL statement, which could exceed database parameter limits.
func FindUserActionsAccessibleOwnerRepoIDsSubQuery(ownerID int64, user *user_model.User, publicOnly bool) *builder.Builder {
return builder.Select("id").From("repository").Where(UserActionsAccessibleOwnerRepoCond(ownerID, user, publicOnly))
}
// GetUserRepositories returns a list of repositories of given user.
func GetUserRepositories(ctx context.Context, opts SearchRepoOptions) (RepositoryList, int64, error) {
if len(opts.OrderBy) == 0 {
+48
View File
@@ -9,6 +9,7 @@ import (
"gitea.dev/models/db"
repo_model "gitea.dev/models/repo"
"gitea.dev/models/unit"
"gitea.dev/models/unittest"
user_model "gitea.dev/models/user"
"gitea.dev/modules/optional"
@@ -466,3 +467,50 @@ func TestSearchRepositoryByTopicName(t *testing.T) {
})
}
}
func TestFindUserActionsAccessibleOwnerRepoIDs(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
// user2 is on org3's owner team, so it can access org3's private repo3 (which has the actions unit)
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
// org3 is a public org owning repo3 (private) and repo32 (public), both with the actions unit
const orgID = 3
all, err := repo_model.SearchRepositoryIDsByCondition(t.Context(), repo_model.UserActionsAccessibleOwnerRepoCond(orgID, user, false))
require.NoError(t, err)
assert.Contains(t, all, int64(3), "without public-only the private repo's actions are listed")
publicOnly, err := repo_model.SearchRepositoryIDsByCondition(t.Context(), repo_model.UserActionsAccessibleOwnerRepoCond(orgID, user, true))
require.NoError(t, err)
assert.NotContains(t, publicOnly, int64(3), "a public-only token must not list a private repo's actions")
assert.Contains(t, publicOnly, int64(32), "a public repo under a public owner stays listed")
}
// TestUserOrgUnitRepoCondTeamAuthorize pins the team.authorize behavior of userOrgTeamUnitRepoBuilder
// (exercised through UserOrgUnitRepoCond): an admin/owner team grants every unit even without an explicit
// team_unit row, while a non-admin team only grants a unit it has an explicit row for. This guards both
// directions — hiding repos from admin-team members, and over-broadening a plain team's access.
func TestUserOrgUnitRepoCondTeamAuthorize(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
accessibleRepoIDs := func(userID, orgID int64, unitType unit.Type) []int64 {
ids, err := repo_model.SearchRepositoryIDsByCondition(t.Context(),
repo_model.UserOrgUnitRepoCond("`repository`.id", userID, orgID, unitType))
require.NoError(t, err)
return ids
}
// Case A: user18 is only on org17's owner team (team5, authorize=owner), linked to the private repo24
// but with no Actions team_unit row. The owner authorize must still grant it, mirroring the runtime
// HasAdminAccess() short-circuit in access.GetIndividualUserRepoPermission.
assert.Contains(t, accessibleRepoIDs(18, 17, unit.TypeActions), int64(24),
"an owner team grants a unit it has no explicit team_unit row for")
// Cases B and C share one subject so the team_unit row is the only difference: user4 is only on org3's
// write team (team2, authorize=write, non-admin), linked to the private repo3. team2 has an explicit
// Projects row but none for Actions.
assert.Contains(t, accessibleRepoIDs(4, 3, unit.TypeProjects), int64(3),
"a non-admin team grants a unit it has an explicit team_unit row for")
assert.NotContains(t, accessibleRepoIDs(4, 3, unit.TypeActions), int64(3),
"a non-admin team must NOT grant a unit it has no team_unit row for")
}
+4 -6
View File
@@ -46,9 +46,8 @@ func (opts *StarredReposOptions) ToConds() builder.Cond {
// only include private repos the actor can still access, so metadata does not leak after access revocation
cond = cond.And(AccessibleRepositoryCondition(opts.Actor, unit.TypeInvalid))
} else {
cond = cond.And(builder.Eq{
"repository.is_private": false,
})
// a public repo under a limited/private owner is not publicly reachable, so exclude it too
cond = cond.And(PublicRepoUnderPublicOwnerCond())
}
return cond
}
@@ -96,9 +95,8 @@ func (opts *WatchedReposOptions) ToConds() builder.Cond {
// only include private repos the actor can still access, so metadata does not leak after access revocation
cond = cond.And(AccessibleRepositoryCondition(opts.Actor, unit.TypeInvalid))
} else {
cond = cond.And(builder.Eq{
"repository.is_private": false,
})
// a public repo under a limited/private owner is not publicly reachable, so exclude it too
cond = cond.And(PublicRepoUnderPublicOwnerCond())
}
return cond.And(builder.Neq{
"watch.mode": WatchModeDont,
+37
View File
@@ -84,3 +84,40 @@ func testUserRepoGetIssuePostersWithSearch(t *testing.T) {
require.Len(t, users, 1)
assert.Equal(t, "user2", users[0].Name)
}
func TestStarredWatchedReposExcludeNonPublicOwners(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
const viewerID = 2
// repo1: public repo under a public owner; repo38: public repo under a limited org (not publicly reachable)
const publicOwnerRepo, limitedOwnerRepo = 1, 38
require.NoError(t, db.Insert(t.Context(), &repo_model.Star{UID: viewerID, RepoID: publicOwnerRepo}))
require.NoError(t, db.Insert(t.Context(), &repo_model.Star{UID: viewerID, RepoID: limitedOwnerRepo}))
require.NoError(t, db.Insert(t.Context(), &repo_model.Watch{UserID: viewerID, RepoID: publicOwnerRepo, Mode: repo_model.WatchModeNormal}))
require.NoError(t, db.Insert(t.Context(), &repo_model.Watch{UserID: viewerID, RepoID: limitedOwnerRepo, Mode: repo_model.WatchModeNormal}))
listOpts := db.ListOptions{Page: 1, PageSize: 50}
starred, err := repo_model.GetStarredRepos(t.Context(), &repo_model.StarredReposOptions{
ListOptions: listOpts, StarrerID: viewerID, IncludePrivate: false,
})
require.NoError(t, err)
assert.NotContains(t, repoIDs(starred), int64(limitedOwnerRepo), "a public repo under a limited owner must be hidden from a public star listing")
assert.Contains(t, repoIDs(starred), int64(publicOwnerRepo), "a public repo under a public owner stays visible")
watched, _, err := repo_model.GetWatchedRepos(t.Context(), &repo_model.WatchedReposOptions{
ListOptions: listOpts, WatcherID: viewerID, IncludePrivate: false,
})
require.NoError(t, err)
assert.NotContains(t, repoIDs(watched), int64(limitedOwnerRepo), "a public repo under a limited owner must be hidden from a public watch listing")
assert.Contains(t, repoIDs(watched), int64(publicOwnerRepo), "a public repo under a public owner stays visible")
}
func repoIDs(repos []*repo_model.Repository) []int64 {
ids := make([]int64, len(repos))
for i, r := range repos {
ids[i] = r.ID
}
return ids
}