Merge branch 'main' into lunny/fix_38016

This commit is contained in:
Lunny Xiao committed 2026-06-08 18:41:52 -07:00
commit e0e5d6e5eb
134 files changed
+3527 -1508

No files matched your search

+4
View File
@@ -232,6 +232,10 @@ func DeleteRun(ctx context.Context, run *actions_model.ActionRun) error {
RepoID: repoID,
RunID: run.ID,
})
recordsToDelete = append(recordsToDelete, &actions_model.ActionRunJobSummary{
RepoID: repoID,
RunID: run.ID,
})
if err := db.WithTx(ctx, func(ctx context.Context) error {
// TODO: Deleting task records could break current ephemeral runner implementation. This is a temporary workaround suggested by ChristopherHX.
+27 -12
View File
@@ -399,6 +399,24 @@ func notifyPackage(ctx context.Context, sender *user_model.User, pd *packages_mo
}
func ifNeedApproval(ctx context.Context, run *actions_model.ActionRun, repo *repo_model.Repository, user *user_model.User) (bool, error) {
canWrite := func(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (bool, error) {
perm, err := access_model.GetDoerRepoPermission(ctx, repo, user)
if err != nil {
return false, err
}
return perm.CanWrite(unit_model.TypeActions), nil
}
return ifNeedApprovalWith(ctx, run, repo, user, canWrite, issues_model.HasMergedPullRequestInRepo)
}
func ifNeedApprovalWith(
ctx context.Context,
run *actions_model.ActionRun,
repo *repo_model.Repository,
user *user_model.User,
canWriteActions func(context.Context, *repo_model.Repository, *user_model.User) (bool, error),
hasMergedPR func(context.Context, int64, int64) (bool, error),
) (bool, error) {
// 1. don't need approval if it's not a fork PR
// 2. don't need approval if the event is `pull_request_target` since the workflow will run in the context of base branch
// see https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks#about-workflow-runs-from-public-forks
@@ -413,27 +431,24 @@ func ifNeedApproval(ctx context.Context, run *actions_model.ActionRun, repo *rep
}
// don't need approval if the user can write
if perm, err := access_model.GetDoerRepoPermission(ctx, repo, user); err != nil {
if ok, err := canWriteActions(ctx, repo, user); err != nil {
return false, fmt.Errorf("GetDoerRepoPermission: %w", err)
} else if perm.CanWrite(unit_model.TypeActions) {
} else if ok {
log.Trace("do not need approval because user %d can write", user.ID)
return false, nil
}
// don't need approval if the user has been approved before
if count, err := db.Count[actions_model.ActionRun](ctx, actions_model.FindRunOptions{
RepoID: repo.ID,
TriggerUserID: user.ID,
Approved: true,
}); err != nil {
return false, fmt.Errorf("CountRuns: %w", err)
} else if count > 0 {
log.Trace("do not need approval because user %d has been approved before", user.ID)
// trust the user only after a merged PR — matching GitHub Actions. Approving one
// fork PR's run must not implicitly trust later fork PRs that replace the workflow.
if merged, err := hasMergedPR(ctx, repo.ID, user.ID); err != nil {
return false, fmt.Errorf("HasMergedPullRequestInRepo: %w", err)
} else if merged {
log.Trace("do not need approval because user %d has a merged pull request in repo %d", user.ID, repo.ID)
return false, nil
}
// otherwise, need approval
log.Trace("need approval because it's the first time user %d triggered actions", user.ID)
log.Trace("need approval because user %d has no merged pull request in repo %d", user.ID, repo.ID)
return true, nil
}
+102
View File
@@ -0,0 +1,102 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package actions
import (
"context"
"errors"
"testing"
actions_model "gitea.dev/models/actions"
repo_model "gitea.dev/models/repo"
user_model "gitea.dev/models/user"
actions_module "gitea.dev/modules/actions"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestIfNeedApproval(t *testing.T) {
alwaysWrite := func(_ context.Context, _ *repo_model.Repository, _ *user_model.User) (bool, error) {
return true, nil
}
neverWrite := func(_ context.Context, _ *repo_model.Repository, _ *user_model.User) (bool, error) {
return false, nil
}
hasMerged := func(_ context.Context, _, _ int64) (bool, error) { return true, nil }
noMerged := func(_ context.Context, _, _ int64) (bool, error) { return false, nil }
errPerm := errors.New("perm error")
errMerge := errors.New("merge error")
forkRun := &actions_model.ActionRun{IsForkPullRequest: true, TriggerEvent: actions_module.GithubEventPullRequest}
nonForkRun := &actions_model.ActionRun{IsForkPullRequest: false, TriggerEvent: actions_module.GithubEventPullRequest}
prTargetRun := &actions_model.ActionRun{IsForkPullRequest: true, TriggerEvent: actions_module.GithubEventPullRequestTarget}
repo := &repo_model.Repository{ID: 1}
normalUser := &user_model.User{ID: 10}
restrictedUser := &user_model.User{ID: 11, IsRestricted: true}
t.Run("not a fork PR never needs approval", func(t *testing.T) {
need, err := ifNeedApprovalWith(t.Context(), nonForkRun, repo, normalUser, alwaysWrite, hasMerged)
require.NoError(t, err)
assert.False(t, need)
})
t.Run("pull_request_target never needs approval even when fork", func(t *testing.T) {
need, err := ifNeedApprovalWith(t.Context(), prTargetRun, repo, normalUser, alwaysWrite, hasMerged)
require.NoError(t, err)
assert.False(t, need)
})
t.Run("restricted user always needs approval", func(t *testing.T) {
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, restrictedUser, alwaysWrite, hasMerged)
require.NoError(t, err)
assert.True(t, need)
})
t.Run("fork PR with write permission does not need approval", func(t *testing.T) {
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, alwaysWrite, noMerged)
require.NoError(t, err)
assert.False(t, need)
})
t.Run("fork PR with merged PR but no write permission does not need approval", func(t *testing.T) {
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, neverWrite, hasMerged)
require.NoError(t, err)
assert.False(t, need)
})
t.Run("fork PR with no write and no merged PR needs approval", func(t *testing.T) {
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, neverWrite, noMerged)
require.NoError(t, err)
assert.True(t, need)
})
t.Run("canWriteActions error is propagated", func(t *testing.T) {
failWrite := func(_ context.Context, _ *repo_model.Repository, _ *user_model.User) (bool, error) {
return false, errPerm
}
_, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, failWrite, noMerged)
require.ErrorIs(t, err, errPerm)
})
t.Run("hasMergedPR error is propagated", func(t *testing.T) {
failMerge := func(_ context.Context, _, _ int64) (bool, error) { return false, errMerge }
_, err := ifNeedApprovalWith(t.Context(), forkRun, repo, normalUser, neverWrite, failMerge)
require.ErrorIs(t, err, errMerge)
})
t.Run("restricted user skips permission check entirely", func(t *testing.T) {
// The perm and merge functions must not be called for a restricted user.
called := false
trackWrite := func(_ context.Context, _ *repo_model.Repository, _ *user_model.User) (bool, error) {
called = true
return true, nil
}
need, err := ifNeedApprovalWith(t.Context(), forkRun, repo, restrictedUser, trackWrite, noMerged)
require.NoError(t, err)
assert.True(t, need)
assert.False(t, called, "permission check must not run for restricted user")
})
}
+23 -3
View File
@@ -6,6 +6,7 @@ package actions
import (
"context"
"fmt"
"strings"
actions_model "gitea.dev/models/actions"
"gitea.dev/models/db"
@@ -15,7 +16,9 @@ import (
"gitea.dev/modules/actions/jobparser"
"gitea.dev/modules/container"
"gitea.dev/modules/gitrepo"
"gitea.dev/modules/httplib"
"gitea.dev/modules/json"
"gitea.dev/modules/setting"
api "gitea.dev/modules/structs"
"gitea.dev/modules/util"
"gitea.dev/services/convert"
@@ -149,10 +152,10 @@ func expandReusableWorkflowCaller(ctx context.Context, run *actions_model.Action
return fmt.Errorf("parse caller job %d: %w", caller.ID, err)
}
// 3. Load called-workflow source.
ref, err := jobparser.ParseUses(parsedJob.Uses)
// 3. Resolve `uses` and load called-workflow source.
ref, err := ResolveUses(ctx, parsedJob.Uses)
if err != nil {
return fmt.Errorf("parse uses %q: %w", parsedJob.Uses, err)
return fmt.Errorf("resolve uses %q: %w", parsedJob.Uses, err)
}
content, contentSourceRepoID, contentSourceCommitSHA, err := loadReusableWorkflowSource(ctx, run, caller, ref)
if err != nil {
@@ -340,3 +343,20 @@ func insertCallerChildren(ctx context.Context, run *actions_model.ActionRun, att
}
return nil
}
// ResolveUses normalizes and parses a reusable workflow `uses:` value.
// It first rewrites an absolute URL pointing to this instance into the cross-repo form (rejecting external URLs),
// then validates the syntax via jobparser.ParseUses.
func ResolveUses(ctx context.Context, uses string) (*jobparser.UsesRef, error) {
// Rewrite a local-instance URL to the equivalent cross-repo form "owner/repo/.gitea/workflows/file.yml@ref".
if strings.HasPrefix(uses, "http://") || strings.HasPrefix(uses, "https://") {
// ParseGiteaSiteURL returns nil for URLs that do not belong to this instance.
gsu := httplib.ParseGiteaSiteURL(ctx, uses)
if gsu == nil {
return nil, fmt.Errorf("unsupported reusable workflow URL %q: an absolute URL must point to this Gitea instance (%s)", uses, setting.AppURL)
}
// RoutePath is the instance-relative path (AppSubURL already stripped), e.g. "/owner/repo/.gitea/workflows/file.yml@ref".
uses = strings.TrimPrefix(gsu.RoutePath, "/")
}
return jobparser.ParseUses(uses)
}
@@ -10,6 +10,9 @@ import (
actions_model "gitea.dev/models/actions"
"gitea.dev/models/db"
"gitea.dev/models/unittest"
"gitea.dev/modules/actions/jobparser"
"gitea.dev/modules/setting"
"gitea.dev/modules/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -132,3 +135,44 @@ func buildCallerChain(t *testing.T, callerUses ...string) []*actions_model.Actio
}
return jobs
}
func TestResolveUses(t *testing.T) {
defer test.MockVariableValue(&setting.AppURL, "https://gitea.example.com/sub/")()
defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
ctx := t.Context()
t.Run("LocalForms", func(t *testing.T) {
// Same-repo and cross-repo forms are not URLs and are parsed as-is.
ref, err := ResolveUses(ctx, "./.gitea/workflows/build.yml")
require.NoError(t, err)
assert.Equal(t, jobparser.UsesRef{Kind: jobparser.UsesKindLocalSameRepo, Path: ".gitea/workflows/build.yml"}, *ref)
ref, err = ResolveUses(ctx, "owner/repo/.gitea/workflows/build.yml@v1")
require.NoError(t, err)
assert.Equal(t, jobparser.UsesRef{Kind: jobparser.UsesKindLocalCrossRepo, Owner: "owner", Repo: "repo", Path: ".gitea/workflows/build.yml", Ref: "v1"}, *ref)
})
t.Run("LocalInstanceURL", func(t *testing.T) {
// An absolute URL on this instance (incl. AppSubURL) resolves to the equivalent cross-repo ref.
ref, err := ResolveUses(ctx, "https://gitea.example.com/sub/owner/repo/.gitea/workflows/ci.yml@refs/heads/main")
require.NoError(t, err)
assert.Equal(t, jobparser.UsesRef{Kind: jobparser.UsesKindLocalCrossRepo, Owner: "owner", Repo: "repo", Path: ".gitea/workflows/ci.yml", Ref: "refs/heads/main"}, *ref)
})
t.Run("InvalidSyntax", func(t *testing.T) {
for _, in := range []string{
"owner/.gitea/workflows/foo.yml", // missing repo segment
"owner/repo/.gitea/workflows/foo.yml", // missing @ref
"https://gitea.example.com/sub/repo/.gitea/workflows/ci.yml@refs/heads/main", // local absolute URL but missing owner
"not a valid uses at all",
} {
_, err := ResolveUses(ctx, in)
require.Error(t, err, "in = %s", in)
}
})
t.Run("ForeignURL", func(t *testing.T) {
_, err := ResolveUses(ctx, "https://other.gitea-example.com/owner/repo/.gitea/workflows/ci.yaml@v1")
assert.ErrorContains(t, err, "must point to this Gitea instance")
})
}
+4 -20
View File
@@ -138,26 +138,10 @@ func (ctx *APIContext) apiErrorInternal(skip int, err error) {
}
// APIErrorNotFound handles 404s for APIContext
// String will replace message, errors will be added to a slice
func (ctx *APIContext) APIErrorNotFound(objs ...any) {
var message string
var errs []string
for _, obj := range objs {
// Ignore nil
if obj == nil {
continue
}
if err, ok := obj.(error); ok {
errs = append(errs, err.Error())
} else {
message = obj.(string)
}
}
ctx.JSON(http.StatusNotFound, map[string]any{
"message": util.IfZero(message, "not found"), // do not use locale in API
"url": setting.API.SwaggerURL,
"errors": errs,
func (ctx *APIContext) APIErrorNotFound(msg ...string) {
ctx.JSON(http.StatusNotFound, APIError{
Message: util.OptionalArg(msg, "not found"),
URL: setting.API.SwaggerURL,
})
}
+10 -9
View File
@@ -9,6 +9,7 @@ import (
asymkey_model "gitea.dev/models/asymkey"
"gitea.dev/models/db"
git_model "gitea.dev/models/git"
"gitea.dev/models/gituser"
repo_model "gitea.dev/models/repo"
user_model "gitea.dev/models/user"
"gitea.dev/modules/container"
@@ -17,14 +18,14 @@ import (
)
// ParseCommitsWithSignature checks if signaute of commits are corresponding to users gpg keys.
func ParseCommitsWithSignature(ctx context.Context, repo *repo_model.Repository, oldCommits []*user_model.UserCommit, repoTrustModel repo_model.TrustModelType) ([]*asymkey_model.SignCommit, error) {
func ParseCommitsWithSignature(ctx context.Context, repo *repo_model.Repository, oldCommits []*gituser.UserCommit, repoTrustModel repo_model.TrustModelType) ([]*asymkey_model.SignCommit, error) {
newCommits := make([]*asymkey_model.SignCommit, 0, len(oldCommits))
keyMap := map[string]bool{}
emails := make(container.Set[string])
for _, c := range oldCommits {
if c.Committer != nil {
emails.Add(c.Committer.Email)
if c.GitCommit.Committer != nil {
emails.Add(c.GitCommit.Committer.Email)
}
}
@@ -34,10 +35,10 @@ func ParseCommitsWithSignature(ctx context.Context, repo *repo_model.Repository,
}
for _, c := range oldCommits {
committerUser := emailUsers.GetByEmail(c.Committer.Email) // FIXME: why ValidateCommitsWithEmails uses "Author", but ParseCommitsWithSignature uses "Committer"?
committerUser := emailUsers.GetByEmail(c.GitCommit.Committer.Email) // FIXME: why GetUserCommitsByGitCommits uses "Author", but ParseCommitsWithSignature uses "Committer"?
signCommit := &asymkey_model.SignCommit{
UserCommit: c,
Verification: asymkey_service.ParseCommitWithSignatureCommitter(ctx, c.Commit, committerUser),
Verification: asymkey_service.ParseCommitWithSignatureCommitter(ctx, c.GitCommit, committerUser),
}
isOwnerMemberCollaborator := func(user *user_model.User) (bool, error) {
@@ -52,15 +53,15 @@ func ParseCommitsWithSignature(ctx context.Context, repo *repo_model.Repository,
}
// ConvertFromGitCommit converts git commits into SignCommitWithStatuses
func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo_model.Repository) ([]*git_model.SignCommitWithStatuses, error) {
validatedCommits, err := user_model.ValidateCommitsWithEmails(ctx, commits)
func ConvertFromGitCommit(ctx context.Context, commits []*git.Commit, repo *repo_model.Repository, currentRef git.RefName) ([]*git_model.SignCommitWithStatuses, error) {
userCommits, err := gituser.GetUserCommitsByGitCommits(ctx, commits, repo.Link(), currentRef)
if err != nil {
return nil, err
}
signedCommits, err := ParseCommitsWithSignature(
ctx,
repo,
validatedCommits,
userCommits,
repo.GetTrustModel(),
)
if err != nil {
@@ -77,7 +78,7 @@ func ParseCommitsWithStatus(ctx context.Context, oldCommits []*asymkey_model.Sig
commit := &git_model.SignCommitWithStatuses{
SignCommit: c,
}
statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commit.ID.String(), db.ListOptionsAll)
statuses, err := git_model.GetLatestCommitStatus(ctx, repo.ID, commit.GitCommit.ID.String(), db.ListOptionsAll)
if err != nil {
return nil, err
}
+1 -1
View File
@@ -184,7 +184,7 @@ func LoadCommentPushCommits(ctx context.Context, c *issues_model.Comment) error
}
defer closer.Close()
c.Commits, err = git_service.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo)
c.Commits, err = git_service.ConvertFromGitCommit(ctx, gitRepo.GetCommitsFromIDs(data.CommitIDs), c.Issue.Repo, "") // no current ref sub path for PR commit list
if err != nil {
log.Debug("ConvertFromGitCommit: %v", err) // no need to show 500 error to end user when the commit does not exist
} else {
+1 -3
View File
@@ -65,9 +65,7 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error {
}
if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() {
// add trailer
message = AddCommitMessageTailer(message, "Co-authored-by", sig.String())
message = AddCommitMessageTailer(message, "Co-committed-by", sig.String()) // FIXME: this one should be removed, it is not really used or widely used
message = AddCommitMessageTailer(message, git.CoAuthoredByTrailer, sig.String())
}
cmdCommit := gitcmd.NewCommand("commit").
AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email).
+1 -1
View File
@@ -917,7 +917,7 @@ func GetSquashMergeCommitMessages(ctx context.Context, pr *issues_model.PullRequ
}
for _, author := range authors {
stringBuilder.WriteString("Co-authored-by: ")
stringBuilder.WriteString(git.CoAuthoredByTrailer + ": ")
stringBuilder.WriteString(author)
stringBuilder.WriteRune('\n')
}
+1
View File
@@ -177,6 +177,7 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams
&actions_model.ActionScheduleSpec{RepoID: repoID},
&actions_model.ActionSchedule{RepoID: repoID},
&actions_model.ActionArtifact{RepoID: repoID},
&actions_model.ActionRunJobSummary{RepoID: repoID},
&actions_model.ActionRunnerToken{RepoID: repoID},
&issues_model.IssuePin{RepoID: repoID},
); err != nil {
+1 -6
View File
@@ -300,12 +300,7 @@ func (t *TemporaryUploadRepository) CommitTree(ctx context.Context, opts *Commit
cmdCommitTree.AddOptionFormat("-S%s", key.KeyID)
if t.repo.GetTrustModel() == repo_model.CommitterTrustModel || t.repo.GetTrustModel() == repo_model.CollaboratorCommitterTrustModel {
if committerSig.Name != authorSig.Name || committerSig.Email != authorSig.Email {
// Add trailers
_, _ = messageBytes.WriteString("\n")
_, _ = messageBytes.WriteString("Co-authored-by: ")
_, _ = messageBytes.WriteString(committerSig.String())
_, _ = messageBytes.WriteString("\n")
_, _ = messageBytes.WriteString("Co-committed-by: ")
_, _ = messageBytes.WriteString("\n" + git.CoAuthoredByTrailer + ": ")
_, _ = messageBytes.WriteString(committerSig.String())
_, _ = messageBytes.WriteString("\n")
}
+34 -21
View File
@@ -13,8 +13,10 @@ import (
asymkey_model "gitea.dev/models/asymkey"
"gitea.dev/models/db"
git_model "gitea.dev/models/git"
"gitea.dev/models/gituser"
repo_model "gitea.dev/models/repo"
user_model "gitea.dev/models/user"
"gitea.dev/modules/container"
"gitea.dev/modules/git"
"gitea.dev/modules/log"
asymkey_service "gitea.dev/services/asymkey"
@@ -93,9 +95,7 @@ func (graph *Graph) AddCommit(row, column int, flowID int64, data []byte) error
// before finally retrieving the latest status
func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_model.Repository, gitRepo *git.Repository) error {
var err error
var ok bool
emails := map[string]*user_model.User{}
emailSet := make(container.Set[string])
keyMap := map[string]bool{}
for _, c := range graph.Commits {
@@ -106,14 +106,26 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_
if err != nil {
return fmt.Errorf("GetCommit: %s Error: %w", c.Rev, err)
}
if c.Commit.Author != nil {
email := c.Commit.Author.Email
if c.User, ok = emails[email]; !ok {
c.User, _ = user_model.GetUserByEmail(ctx, email)
emails[email] = c.User
}
emailSet.Add(c.Commit.Author.Email)
}
for _, sig := range c.Commit.AllParticipantIdentities() {
emailSet.Add(sig.Email)
}
}
emailUserMap, err := user_model.GetUsersByEmails(ctx, emailSet.Values())
if err != nil {
log.Error("GetUsersByEmails: %v", err)
}
for _, c := range graph.Commits {
if c.Commit == nil {
continue
}
c.User = emailUserMap.GetByEmail(c.Commit.Author.Email)
c.AvatarStackData = gituser.BuildAvatarStackData(ctx, c.Commit.AllParticipantIdentities(), emailUserMap)
c.Verification = asymkey_service.ParseCommitWithSignature(ctx, c.Commit)
@@ -246,18 +258,19 @@ func newRefsFromRefNames(refNames []byte) []git.Reference {
// Commit represents a commit at coordinate X, Y with the data
type Commit struct {
Commit *git.Commit
User *user_model.User
Verification *asymkey_model.CommitVerification
Status *git_model.CommitStatus
Flow int64
Row int
Column int
Refs []git.Reference
Rev string
Date time.Time
ShortRev string
Subject string
Commit *git.Commit
User *user_model.User // author
AvatarStackData *gituser.AvatarStackData
Verification *asymkey_model.CommitVerification
Status *git_model.CommitStatus
Flow int64
Row int
Column int
Refs []git.Reference
Rev string
Date time.Time // author date from "%ad"
ShortRev string
Subject string
}
// OnlyRelation returns whether this a relation only commit
+2 -2
View File
@@ -7,7 +7,6 @@ package wiki
import (
"context"
"fmt"
"os"
"gitea.dev/models/db"
repo_model "gitea.dev/models/repo"
@@ -21,6 +20,7 @@ import (
"gitea.dev/modules/graceful"
"gitea.dev/modules/log"
repo_module "gitea.dev/modules/repository"
"gitea.dev/modules/util"
asymkey_service "gitea.dev/services/asymkey"
repo_service "gitea.dev/services/repository"
)
@@ -304,7 +304,7 @@ func DeleteWikiPage(ctx context.Context, doer *user_model.User, repo *repo_model
return err
}
} else {
return os.ErrNotExist
return util.ErrNotExist
}
// FIXME: The wiki doesn't have lfs support at present - if this changes need to check attributes here