diff --git a/models/actions/run.go b/models/actions/run.go index fe516d75d4..d8a37d0b60 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -41,27 +41,24 @@ const ( // ActionRun represents a run of a workflow file type ActionRun struct { - ID int64 - Title string - RepoID int64 `xorm:"index unique(repo_index) index(concurrency)"` - Repo *repo_model.Repository `xorm:"-"` - OwnerID int64 `xorm:"index"` - WorkflowID string `xorm:"index"` // the name of workflow file - Index int64 `xorm:"index unique(repo_index)"` // a unique number for each run of a repository - TriggerUserID int64 `xorm:"index"` - TriggerUser *user_model.User `xorm:"-"` - ScheduleID int64 - Ref string `xorm:"index"` // the commit/tag/… that caused the run - IsRefDeleted bool `xorm:"-"` - CommitSHA string - IsForkPullRequest bool // If this is triggered by a PR from a forked repository or an untrusted user, we need to check if it is approved and limit permissions when running the workflow. - NeedApproval bool // may need approval if it's a fork pull request - ApprovedBy int64 `xorm:"index"` // who approved - Event webhook_module.HookEventType // the webhook event that causes the workflow to run - EventPayload string `xorm:"LONGTEXT"` - TriggerEvent string // the trigger event defined in the `on` configuration of the triggered workflow - Status Status `xorm:"index"` - Version int `xorm:"version default 0"` // Status could be updated concomitantly, so an optimistic lock is needed + ID int64 + Title string + RepoID int64 `xorm:"index unique(repo_index) index(concurrency)"` + Repo *repo_model.Repository `xorm:"-"` + OwnerID int64 `xorm:"index"` + WorkflowID string `xorm:"index"` // the name of workflow file + Index int64 `xorm:"index unique(repo_index)"` // a unique number for each run of a repository + TriggerUserID int64 `xorm:"index"` + TriggerUser *user_model.User `xorm:"-"` + ScheduleID int64 + Ref string `xorm:"index"` // the commit/tag/… that caused the run + IsRefDeleted bool `xorm:"-"` + CommitSHA string + Event webhook_module.HookEventType // the webhook event that causes the workflow to run + EventPayload string `xorm:"LONGTEXT"` + TriggerEvent string // the trigger event defined in the `on` configuration of the triggered workflow + Status Status `xorm:"index"` + Version int `xorm:"version default 0"` // Status could be updated concomitantly, so an optimistic lock is needed // Started and Stopped is used for recording last run time, if rerun happened, they will be reset to 0 Started timeutil.TimeStamp Stopped timeutil.TimeStamp @@ -71,6 +68,13 @@ type ActionRun struct { Updated timeutil.TimeStamp `xorm:"updated"` NotifyEmail bool + // pull request trust + IsForkPullRequest bool + PullRequestPosterID int64 + PullRequestID int64 `xorm:"index"` + NeedApproval bool + ApprovedBy int64 `xorm:"index"` + ConcurrencyGroup string `xorm:"'concurrency_group' index(concurrency)"` ConcurrencyType ConcurrencyMode @@ -228,6 +232,54 @@ func clearRepoRunCountCache(repo *repo_model.Repository) { cache.Remove(actionsCountOpenCacheKey(repo.ID)) } +func condRunsThatNeedApproval(repoID, pullRequestID int64) builder.Cond { + // performance relies indexes on repo_id and pull_request_id + return builder.Eq{"repo_id": repoID, "pull_request_id": pullRequestID, "need_approval": true} +} + +func GetRunsThatNeedApprovalByRepoIDAndPullRequestID(ctx context.Context, repoID, pullRequestID int64) ([]*ActionRun, error) { + var runs []*ActionRun + if err := db.GetEngine(ctx).Where(condRunsThatNeedApproval(repoID, pullRequestID)).Find(&runs); err != nil { + return nil, err + } + return runs, nil +} + +func HasRunThatNeedApproval(ctx context.Context, repoID, pullRequestID int64) (bool, error) { + return db.GetEngine(ctx).Where(condRunsThatNeedApproval(repoID, pullRequestID)).Exist(&ActionRun{}) +} + +type ApprovalType bool + +const ( + NeedApproval = ApprovalType(true) + DoesNotNeedApproval = ApprovalType(false) + UndefinedApproval = ApprovalType(false) +) + +func UpdateRunApprovalByID(ctx context.Context, id int64, approval ApprovalType, approvedBy int64) error { + _, err := db.GetEngine(ctx).Exec("UPDATE action_run SET need_approval=?, approved_by=? WHERE id=?", bool(approval), approvedBy, id) + return err +} + +func GetRunsNotDoneByRepoIDAndPullRequestPosterID(ctx context.Context, repoID, pullRequestPosterID int64) ([]*ActionRun, error) { + var runs []*ActionRun + // performance relies on indexes on repo_id and status + if err := db.GetEngine(ctx).Where("repo_id=? AND pull_request_poster_id=?", repoID, pullRequestPosterID).And(builder.In("status", []Status{StatusUnknown, StatusWaiting, StatusRunning, StatusBlocked})).Find(&runs); err != nil { + return nil, err + } + return runs, nil +} + +func GetRunsNotDoneByRepoIDAndPullRequestID(ctx context.Context, repoID, pullRequestID int64) ([]*ActionRun, error) { + var runs []*ActionRun + // performance relies on indexes on repo_id and status + if err := db.GetEngine(ctx).Where("repo_id=? AND pull_request_id=?", repoID, pullRequestID).And(builder.In("status", []Status{StatusUnknown, StatusWaiting, StatusRunning, StatusBlocked})).Find(&runs); err != nil { + return nil, err + } + return runs, nil +} + // InsertRun inserts a run // The title will be cut off at 255 characters if it's longer than 255 characters. // We don't have to send the ActionRunNowDone notification here because there are no runs that start in a not done status. @@ -298,8 +350,10 @@ func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWork Status: status, }) } - if err := db.Insert(ctx, runJobs); err != nil { - return err + if len(runJobs) > 0 { + if err := db.Insert(ctx, runJobs); err != nil { + return err + } } // if there is a job in the waiting status, increase tasks version. diff --git a/models/actions/run_test.go b/models/actions/run_test.go index a6973bb14d..fdb5ec320d 100644 --- a/models/actions/run_test.go +++ b/models/actions/run_test.go @@ -4,6 +4,7 @@ package actions import ( + "fmt" "testing" "forgejo.org/models/db" @@ -81,3 +82,121 @@ func TestRepoNumOpenActions(t *testing.T) { assert.Equal(t, 0, RepoNumOpenActions(t.Context(), repo.ID)) }) } + +func TestActionRun_GetRunsNotDoneByRepoIDAndPullRequestPosterID(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + repoID := int64(10) + pullRequestID := int64(3) + pullRequestPosterID := int64(30) + + runDone := &ActionRun{ + RepoID: repoID, + PullRequestID: pullRequestID, + PullRequestPosterID: pullRequestPosterID, + Status: StatusSuccess, + } + require.NoError(t, InsertRun(t.Context(), runDone, nil)) + + unrelatedUser := int64(5) + runNotByPoster := &ActionRun{ + RepoID: repoID, + PullRequestID: pullRequestID, + PullRequestPosterID: unrelatedUser, + Status: StatusRunning, + } + require.NoError(t, InsertRun(t.Context(), runNotByPoster, nil)) + + unrelatedRepository := int64(6) + runNotInTheSameRepository := &ActionRun{ + RepoID: unrelatedRepository, + PullRequestID: pullRequestID, + PullRequestPosterID: pullRequestPosterID, + Status: StatusSuccess, + } + require.NoError(t, InsertRun(t.Context(), runNotInTheSameRepository, nil)) + + for _, status := range []Status{StatusUnknown, StatusWaiting, StatusRunning} { + t.Run(fmt.Sprintf("%s", status), func(t *testing.T) { + runNotDone := &ActionRun{ + RepoID: repoID, + PullRequestID: pullRequestID, + Status: status, + PullRequestPosterID: pullRequestPosterID, + } + require.NoError(t, InsertRun(t.Context(), runNotDone, nil)) + runs, err := GetRunsNotDoneByRepoIDAndPullRequestPosterID(t.Context(), repoID, pullRequestPosterID) + require.NoError(t, err) + require.Len(t, runs, 1) + run := runs[0] + assert.Equal(t, runNotDone.ID, run.ID) + assert.Equal(t, runNotDone.Status, run.Status) + unittest.AssertSuccessfulDelete(t, run) + }) + } +} + +func TestActionRun_NeedApproval(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + pullRequestPosterID := int64(4) + repoID := int64(10) + pullRequestID := int64(2) + runDoesNotNeedApproval := &ActionRun{ + RepoID: repoID, + PullRequestID: pullRequestID, + PullRequestPosterID: pullRequestPosterID, + } + require.NoError(t, InsertRun(t.Context(), runDoesNotNeedApproval, nil)) + unrelatedRepository := int64(6) + runNotInTheSameRepository := &ActionRun{ + RepoID: unrelatedRepository, + PullRequestID: pullRequestID, + PullRequestPosterID: pullRequestPosterID, + NeedApproval: true, + } + require.NoError(t, InsertRun(t.Context(), runNotInTheSameRepository, nil)) + unrelatedPullRequest := int64(3) + runNotInTheSamePullRequest := &ActionRun{ + RepoID: repoID, + PullRequestID: unrelatedPullRequest, + PullRequestPosterID: pullRequestPosterID, + NeedApproval: true, + } + require.NoError(t, InsertRun(t.Context(), runNotInTheSamePullRequest, nil)) + + t.Run("HasRunThatNeedApproval is false", func(t *testing.T) { + has, err := HasRunThatNeedApproval(t.Context(), repoID, pullRequestID) + require.NoError(t, err) + assert.False(t, has) + }) + + runNeedApproval := &ActionRun{ + RepoID: repoID, + PullRequestID: pullRequestID, + PullRequestPosterID: pullRequestPosterID, + NeedApproval: true, + } + require.NoError(t, InsertRun(t.Context(), runNeedApproval, nil)) + + t.Run("HasRunThatNeedApproval is true", func(t *testing.T) { + has, err := HasRunThatNeedApproval(t.Context(), repoID, pullRequestID) + require.NoError(t, err) + assert.True(t, has) + }) + + assertApprovalEqual := func(t *testing.T, expected, actual *ActionRun) { + t.Helper() + assert.Equal(t, expected.RepoID, actual.RepoID) + assert.Equal(t, expected.PullRequestID, actual.PullRequestID) + assert.Equal(t, expected.PullRequestPosterID, actual.PullRequestPosterID) + assert.Equal(t, expected.NeedApproval, actual.NeedApproval) + } + + t.Run("GetRunsThatNeedApproval", func(t *testing.T) { + runs, err := GetRunsThatNeedApprovalByRepoIDAndPullRequestID(t.Context(), repoID, pullRequestID) + require.NoError(t, err) + require.Len(t, runs, 1) + assertApprovalEqual(t, runNeedApproval, runs[0]) + }) +} diff --git a/models/actions/user.go b/models/actions/user.go new file mode 100644 index 0000000000..1be8606e2e --- /dev/null +++ b/models/actions/user.go @@ -0,0 +1,97 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "context" + "fmt" + "time" + + "forgejo.org/models/db" + "forgejo.org/modules/timeutil" + + "xorm.io/builder" +) + +type ActionUser struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX UNIQUE(action_user_index) REFERENCES(user, id)"` + RepoID int64 `xorm:"INDEX UNIQUE(action_user_index) REFERENCES(repository, id)"` + + TrustedWithPullRequests bool + + LastAccess timeutil.TimeStamp `xorm:"INDEX"` +} + +func init() { + db.RegisterModel(new(ActionUser)) +} + +type ErrUserNotExist struct { + UserID int64 + RepoID int64 +} + +func IsErrUserNotExist(err error) bool { + _, ok := err.(ErrUserNotExist) + return ok +} + +func (err ErrUserNotExist) Error() string { + return fmt.Sprintf("ActionUser does not exist [user_id: %d, repo_id: %d]", err.UserID, err.RepoID) +} + +func InsertActionUser(ctx context.Context, user *ActionUser) error { + user.LastAccess = timeutil.TimeStampNow() + return db.Insert(ctx, user) +} + +func DeleteActionUserByUserIDAndRepoID(ctx context.Context, userID, repoID int64) error { + _, err := db.GetEngine(ctx).Table(&ActionUser{}).Where("user_id=? AND repo_id=?", userID, repoID).Delete() + return err +} + +var updateFrequency = 24 * time.Hour + +func MaybeUpdateAccess(ctx context.Context, user *ActionUser) error { + // Keep track of the last time the record was accessed to identify which one + // are never accessed so they can be removed eventually. But only every updateFrequency + // to not stress the underlying database. + if timeutil.TimeStampNow() > user.LastAccess.AddDuration(updateFrequency) { + user.LastAccess = timeutil.TimeStampNow() + if _, err := db.GetEngine(ctx).ID(user.ID).Cols("last_access").Update(user); err != nil { + return err + } + } + + return nil +} + +func GetActionUserByUserIDAndRepoID(ctx context.Context, userID, repoID int64) (*ActionUser, error) { + user := new(ActionUser) + has, err := db.GetEngine(ctx).Where("user_id=? AND repo_id=?", userID, repoID).Get(user) + if err != nil { + return nil, err + } else if !has { + return nil, ErrUserNotExist{userID, repoID} + } + return user, nil +} + +func GetActionUserByUserIDAndRepoIDAndUpdateAccess(ctx context.Context, userID, repoID int64) (*ActionUser, error) { + user, err := GetActionUserByUserIDAndRepoID(ctx, userID, repoID) + if err != nil { + return nil, err + } + return user, MaybeUpdateAccess(ctx, user) +} + +var expire = 3 * 30 * 24 * time.Hour + +func RevokeInactiveActionUser(ctx context.Context) error { + olderThan := timeutil.TimeStampNow().AddDuration(-expire) + + _, err := db.GetEngine(ctx).Where(builder.Lt{"last_access": olderThan}).Delete(&ActionUser{}) + return err +} diff --git a/models/actions/user_test.go b/models/actions/user_test.go new file mode 100644 index 0000000000..648609a151 --- /dev/null +++ b/models/actions/user_test.go @@ -0,0 +1,107 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "testing" + "time" + + repo_model "forgejo.org/models/repo" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + "forgejo.org/modules/test" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestActionUser_CreateDelete(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + require.ErrorContains(t, InsertActionUser(t.Context(), &ActionUser{ + UserID: user.ID, + }), "FOREIGN KEY") + + require.ErrorContains(t, InsertActionUser(t.Context(), &ActionUser{ + RepoID: repo.ID, + }), "FOREIGN KEY") + + actionUser := &ActionUser{ + UserID: user.ID, + RepoID: repo.ID, + } + require.NoError(t, InsertActionUser(t.Context(), actionUser)) + assert.NotZero(t, actionUser.ID) + assert.NotZero(t, actionUser.LastAccess) + + otherUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + actionUserNotSameUser := &ActionUser{ + UserID: otherUser.ID, + RepoID: repo.ID, + } + require.NoError(t, InsertActionUser(t.Context(), actionUserNotSameUser)) + + otherRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + actionUserNotSameRepo := &ActionUser{ + UserID: user.ID, + RepoID: otherRepo.ID, + } + require.NoError(t, InsertActionUser(t.Context(), actionUserNotSameRepo)) + + unittest.AssertExistsAndLoadBean(t, &ActionUser{ID: actionUser.ID}) + require.NoError(t, DeleteActionUserByUserIDAndRepoID(t.Context(), user.ID, repo.ID)) + unittest.AssertNotExistsBean(t, &ActionUser{ID: actionUser.ID}) +} + +func TestActionUser_RevokeInactiveActionUser(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + actionUser := &ActionUser{ + UserID: user.ID, + RepoID: repo.ID, + } + require.NoError(t, InsertActionUser(t.Context(), actionUser)) + + t.Run("not revoked because it was just created", func(t *testing.T) { + unittest.AssertExistsAndLoadBean(t, &ActionUser{ID: actionUser.ID}) + require.NoError(t, RevokeInactiveActionUser(t.Context())) + unittest.AssertExistsAndLoadBean(t, &ActionUser{ID: actionUser.ID}) + }) + + // needs to be at least 1 second because unix timestamp resolution is 1 second + defer test.MockVariableValue(&expire, 1*time.Second)() + + t.Run("used not updated too frequently", func(t *testing.T) { + time.Sleep(2 * time.Second) + usedActionUser, err := GetActionUserByUserIDAndRepoIDAndUpdateAccess(t.Context(), user.ID, repo.ID) + require.NoError(t, err) + require.Equal(t, actionUser.ID, usedActionUser.ID) + assert.Equal(t, usedActionUser.LastAccess, actionUser.LastAccess) + }) + + defer test.MockVariableValue(&updateFrequency, 0)() + + t.Run("not revoked because it was recently used", func(t *testing.T) { + time.Sleep(2 * time.Second) + usedActionUser, err := GetActionUserByUserIDAndRepoIDAndUpdateAccess(t.Context(), user.ID, repo.ID) + require.NoError(t, err) + require.Equal(t, actionUser.ID, usedActionUser.ID) + assert.Greater(t, usedActionUser.LastAccess, actionUser.LastAccess) + require.NoError(t, RevokeInactiveActionUser(t.Context())) + unittest.AssertExistsAndLoadBean(t, &ActionUser{ID: actionUser.ID}) + }) + + t.Run("revoked because it was not recently used", func(t *testing.T) { + time.Sleep(2 * time.Second) + unittest.AssertExistsAndLoadBean(t, &ActionUser{ID: actionUser.ID}) + require.NoError(t, RevokeInactiveActionUser(t.Context())) + unittest.AssertNotExistsBean(t, &ActionUser{ID: actionUser.ID}) + }) +} diff --git a/models/fixtures/action_user.yml b/models/fixtures/action_user.yml new file mode 100644 index 0000000000..63b63a0e58 --- /dev/null +++ b/models/fixtures/action_user.yml @@ -0,0 +1,6 @@ +- + id: 1 + repo_id: 1 + user_id: 2 + trusted_with_pull_requests: true + last_access: 1683636528 diff --git a/models/forgejo_migrations/v14a_actions-approval-and-trust.go b/models/forgejo_migrations/v14a_actions-approval-and-trust.go new file mode 100644 index 0000000000..657d512548 --- /dev/null +++ b/models/forgejo_migrations/v14a_actions-approval-and-trust.go @@ -0,0 +1,104 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "context" + + actions_model "forgejo.org/models/actions" + "forgejo.org/models/db" + "forgejo.org/modules/log" + "forgejo.org/modules/timeutil" + + "xorm.io/xorm" +) + +func init() { + registerMigration(&Migration{ + Description: "add actions approval and trust table and fields", + Upgrade: v14ActionsApprovalAndTrust, + }) +} + +func v14ActionsApprovalAndTrust(x *xorm.Engine) error { + if err := v14ActionsApprovalAndTrustCreateTableActionUser(x); err != nil { + return err + } + if err := v14ActionsApprovalAndTrustAddActionsRunFields(x); err != nil { + return err + } + return v14ActionsApprovalAndTrustPopulateTableActionUser(x) +} + +func v14ActionsApprovalAndTrustCreateTableActionUser(x *xorm.Engine) error { + type ActionUser struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX UNIQUE(action_user_index) REFERENCES(user, id)"` + RepoID int64 `xorm:"INDEX UNIQUE(action_user_index) REFERENCES(repository, id)"` + + TrustedWithPullRequests bool + + LastAccess timeutil.TimeStamp `xorm:"INDEX"` + } + return x.Sync(new(ActionUser)) +} + +func v14ActionsApprovalAndTrustAddActionsRunFields(x *xorm.Engine) error { + type ActionRun struct { + PullRequestPosterID int64 + PullRequestID int64 `xorm:"index"` + } + _, err := x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, new(ActionRun)) + return err +} + +type v14ActionsApprovalAndTrustTrusted struct { + RepoID int64 + UserID int64 +} + +func v14ActionsApprovalAndTrustPopulateTableActionUser(x *xorm.Engine) error { + // + // Users approved once were trusted before and are trusted now. + // + // The admin will see they can revoke that trust when the user + // submits a new pull request. + // + // If the user does not submit any pull request, this trust will + // eventually be automatically revoked. + // + // The number of trusted users is assumed to be small enough to not require + // pagination, even on large instances. + // + log.Info("v14a_actions-approval-and-trust: search") + var trustedList []*v14ActionsApprovalAndTrustTrusted + if err := x.Table("`action_run`"). + Select("DISTINCT `action_run`.`repo_id`, `action_run`.`trigger_user_id` AS `user_id`"). + Join("INNER", "`repository`", "`repository`.`id` = `action_run`.`repo_id`"). + Join("INNER", "`user`", "`user`.`id` = `action_run`.`trigger_user_id`"). + Where("`action_run`.`approved_by` > 0 AND `action_run`.`trigger_user_id` > 0"). + OrderBy("`action_run`.`repo_id`, `action_run`.`trigger_user_id`"). + Find(&trustedList); err != nil { + return err + } + + log.Info("v14a_actions-approval-and-trust: start adding %d users trusted with workflow runs", len(trustedList)) + if err := db.WithTx(db.DefaultContext, func(ctx context.Context) error { + for _, trusted := range trustedList { + log.Debug("v14a_actions-approval-and-trust: repository %d trusts user %d", trusted.RepoID, trusted.UserID) + if err := actions_model.InsertActionUser(ctx, &actions_model.ActionUser{ + RepoID: trusted.RepoID, + UserID: trusted.UserID, + TrustedWithPullRequests: true, + }); err != nil { + return err + } + } + return nil + }); err != nil { + return err + } + log.Info("v14a_actions-approval-and-trust: done adding %d users", len(trustedList)) + return nil +} diff --git a/models/forgejo_migrations/v14a_actions-approval-and-trust_test.go b/models/forgejo_migrations/v14a_actions-approval-and-trust_test.go new file mode 100644 index 0000000000..c639a0d2e9 --- /dev/null +++ b/models/forgejo_migrations/v14a_actions-approval-and-trust_test.go @@ -0,0 +1,103 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "testing" + "time" + + actions_model "forgejo.org/models/actions" + "forgejo.org/models/db" + migration_tests "forgejo.org/models/gitea_migrations/test" + repo_model "forgejo.org/models/repo" + user_model "forgejo.org/models/user" + "forgejo.org/modules/timeutil" + webhook_module "forgejo.org/modules/webhook" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_v14ActionsApprovalAndTrustPopulateTableActionUser(t *testing.T) { + type ActionUser struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"INDEX UNIQUE(action_user_index) REFERENCES(user, id)"` + RepoID int64 `xorm:"INDEX UNIQUE(action_user_index) REFERENCES(repository, id)"` + + TrustedWithPullRequests bool + + LastAccess timeutil.TimeStamp `xorm:"INDEX"` + } + type ActionRun struct { + ID int64 + Title string + RepoID int64 `xorm:"index unique(repo_index) index(concurrency)"` + Repo *repo_model.Repository `xorm:"-"` + OwnerID int64 `xorm:"index"` + WorkflowID string `xorm:"index"` // the name of workflow file + Index int64 `xorm:"index unique(repo_index)"` // a unique number for each run of a repository + TriggerUserID int64 `xorm:"index"` + TriggerUser *user_model.User `xorm:"-"` + ScheduleID int64 + Ref string `xorm:"index"` // the commit/tag/… that caused the run + IsRefDeleted bool `xorm:"-"` + CommitSHA string + Event webhook_module.HookEventType // the webhook event that causes the workflow to run + EventPayload string `xorm:"LONGTEXT"` + TriggerEvent string // the trigger event defined in the `on` configuration of the triggered workflow + Status actions_model.Status `xorm:"index"` + Version int `xorm:"version default 0"` // Status could be updated concomitantly, so an optimistic lock is needed + // Started and Stopped is used for recording last run time, if rerun happened, they will be reset to 0 + Started timeutil.TimeStamp + Stopped timeutil.TimeStamp + // PreviousDuration is used for recording previous duration + PreviousDuration time.Duration + Created timeutil.TimeStamp `xorm:"created"` + Updated timeutil.TimeStamp `xorm:"updated"` + NotifyEmail bool + + // pull request trust + IsForkPullRequest bool + PullRequestPosterID int64 + PullRequestID int64 `xorm:"index"` + NeedApproval bool + ApprovedBy int64 `xorm:"index"` + + ConcurrencyGroup string `xorm:"'concurrency_group' index(concurrency)"` + ConcurrencyType actions_model.ConcurrencyMode + + PreExecutionError string `xorm:"LONGTEXT"` // used to report errors that blocked execution of a workflow + } + type Repository struct { + ID int64 `xorm:"pk autoincr"` + } + type User struct { + ID int64 `xorm:"pk autoincr"` + } + x, deferable := migration_tests.PrepareTestEnv(t, 0, new(User), new(Repository), new(ActionUser), new(ActionRun)) + defer deferable() + if x == nil || t.Failed() { + return + } + + require.NoError(t, v14ActionsApprovalAndTrustPopulateTableActionUser(x)) + + var users []*actions_model.ActionUser + require.NoError(t, db.GetEngine(t.Context()).Select("`repo_id`, `user_id`").OrderBy("`id`").Find(&users)) + // See models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/action_run.yml + assert.Equal(t, []*actions_model.ActionUser{ + { + UserID: 3, + RepoID: 15, + }, + { + UserID: 3, + RepoID: 63, + }, + { + UserID: 4, + RepoID: 63, + }, + }, users) +} diff --git a/models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/action_run.yml b/models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/action_run.yml new file mode 100644 index 0000000000..78eb18a955 --- /dev/null +++ b/models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/action_run.yml @@ -0,0 +1,151 @@ +- + id: 1000 + title: "63, 2 not trusted" + repo_id: 63 + trigger_user_id: 2 + approved_by: 0 + owner_id: 2 + workflow_id: "running.yaml" + index: 1000 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 1 # success + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + +- + id: 1001 + title: "63, 3 trusted" + repo_id: 63 + trigger_user_id: 3 + approved_by: 1 + owner_id: 2 + workflow_id: "running.yaml" + index: 1001 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 1 # success + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + +- + id: 1002 + title: "63, 3 trusted (duplicate, ignored)" + repo_id: 63 + trigger_user_id: 3 + approved_by: 1 + owner_id: 2 + workflow_id: "running.yaml" + index: 1002 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 1 # success + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + +- + id: 1003 + title: "5000, 3 trusted (non existent repo_id, ignored)" + repo_id: 5000 + trigger_user_id: 2 + approved_by: 1 + owner_id: 2 + workflow_id: "running.yaml" + index: 1003 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 1 # success + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + +- + id: 1004 + title: "63, 3000 trusted (non existent trigger_user, ignored)" + repo_id: 63 + trigger_user_id: 3000 + approved_by: 1 + owner_id: 2 + workflow_id: "running.yaml" + index: 1004 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 1 # success + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + +- + id: 1005 + title: "63, 4 trusted" + repo_id: 63 + trigger_user_id: 4 + approved_by: 1 + owner_id: 2 + workflow_id: "running.yaml" + index: 1005 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 1 # success + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + +- + id: 1006 + title: "15, 3 trusted" + repo_id: 15 + trigger_user_id: 3 + approved_by: 1 + owner_id: 2 + workflow_id: "running.yaml" + index: 1006 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 1 # success + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + +- + id: 1007 + title: "15, 0 trigger user id zero is ignored" + repo_id: 15 + trigger_user_id: 0 + approved_by: 1 + owner_id: 2 + workflow_id: "running.yaml" + index: 1007 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "workflow_dispatch" + is_fork_pull_request: 0 + status: 1 # success + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 diff --git a/models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/repository.yml b/models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/repository.yml new file mode 100644 index 0000000000..a47c261499 --- /dev/null +++ b/models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/repository.yml @@ -0,0 +1,5 @@ +- + id: 15 + +- + id: 63 diff --git a/models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/user.yml b/models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/user.yml new file mode 100644 index 0000000000..8063fa12af --- /dev/null +++ b/models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/user.yml @@ -0,0 +1,11 @@ +- # NOTE: this user (id=1) is the admin + id: 1 + +- + id: 2 + +- + id: 3 + +- + id: 4 diff --git a/models/issues/pull.go b/models/issues/pull.go index 44ead3b9f1..6f65ef3af9 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -470,6 +470,23 @@ func (pr *PullRequest) GetReviewCommentsCount(ctx context.Context) int { return int(count) } +func (pr *PullRequest) IsForkPullRequest() bool { + var isForkPullRequest bool + + switch pr.Flow { + case PullRequestFlowGithub: + isForkPullRequest = pr.IsFromFork() + case PullRequestFlowAGit: + // there is no fork concept in AGit flow, anyone with read permission can push refs/for// to the repo. + // So we must treat it as a fork pull request because it may be from an untrusted user + isForkPullRequest = true + default: + // unknown flow, treat it as it's a fork pull request + isForkPullRequest = true + } + return isForkPullRequest +} + // IsChecking returns true if this pull request is still checking conflict. func (pr *PullRequest) IsChecking() bool { return pr.Status == PullRequestStatusChecking diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go index db10386b5a..dd97ed34b2 100644 --- a/models/issues/pull_test.go +++ b/models/issues/pull_test.go @@ -466,6 +466,43 @@ func TestGetPullRequestByMergedCommit(t *testing.T) { require.ErrorAs(t, err, &issues_model.ErrPullRequestNotExist{}) } +func TestPullRequest_IsForkPullRequest(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("FlowGithub from a fork", func(t *testing.T) { + pr := &issues_model.PullRequest{ + Flow: issues_model.PullRequestFlowGithub, + HeadRepoID: 111, + BaseRepoID: 222, + } + assert.True(t, pr.IsForkPullRequest()) + }) + + t.Run("FlowGithub from the same repository", func(t *testing.T) { + pr := &issues_model.PullRequest{ + Flow: issues_model.PullRequestFlowGithub, + HeadRepoID: 111, + BaseRepoID: 111, + } + assert.False(t, pr.IsForkPullRequest()) + }) + + t.Run("PullRequestFlowAGit", func(t *testing.T) { + pr := &issues_model.PullRequest{ + Flow: issues_model.PullRequestFlowAGit, + } + assert.True(t, pr.IsForkPullRequest()) + }) + + t.Run("Other", func(t *testing.T) { + unknown := issues_model.PullRequestFlow(4854) + pr := &issues_model.PullRequest{ + Flow: unknown, + } + assert.True(t, pr.IsForkPullRequest()) + }) +} + func TestMigrate_InsertPullRequests(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) reponame := "repo1" diff --git a/services/actions/job_parser.go b/modules/actions/job_parser.go similarity index 93% rename from services/actions/job_parser.go rename to modules/actions/job_parser.go index 8c880977d1..6360902b9a 100644 --- a/services/actions/job_parser.go +++ b/modules/actions/job_parser.go @@ -9,7 +9,7 @@ import ( "code.forgejo.org/forgejo/runner/v11/act/jobparser" ) -func jobParser(workflow []byte, options ...jobparser.ParseOption) ([]*jobparser.SingleWorkflow, error) { +func JobParser(workflow []byte, options ...jobparser.ParseOption) ([]*jobparser.SingleWorkflow, error) { singleWorkflows, err := jobparser.Parse(workflow, false, options...) if err != nil { return nil, err diff --git a/services/actions/job_parser_test.go b/modules/actions/job_parser_test.go similarity index 98% rename from services/actions/job_parser_test.go rename to modules/actions/job_parser_test.go index 9c1361d74e..82bce3da13 100644 --- a/services/actions/job_parser_test.go +++ b/modules/actions/job_parser_test.go @@ -200,7 +200,7 @@ jobs: }, } { t.Run(testCase.name, func(t *testing.T) { - sw, err := jobParser([]byte(testCase.workflow)) + sw, err := JobParser([]byte(testCase.workflow)) require.NoError(t, err) for i, sw := range sw { actual, err := sw.Marshal() diff --git a/modules/actions/workflows.go b/modules/actions/workflows.go index f50e5f8289..211761e6c1 100644 --- a/modules/actions/workflows.go +++ b/modules/actions/workflows.go @@ -8,6 +8,7 @@ import ( "io" "strings" + actions_model "forgejo.org/models/actions" "forgejo.org/modules/git" "forgejo.org/modules/log" api "forgejo.org/modules/structs" @@ -25,6 +26,7 @@ type DetectedWorkflow struct { TriggerEvent *jobparser.Event Content []byte EventDetectionError error + NeedApproval actions_model.ApprovalType } func init() { diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 7d71282e40..5baf2fcfe8 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2793,8 +2793,8 @@ projects.read = Read: Access repository project boards. projects.write = Write: Create projects and columns and edit them. packages.read = Read: View and download packages assigned to the repository. packages.write = Write: Publish and delete packages assigned to the repository. -actions.read = Read: View integrated CI/CD pipelines and their logs. -actions.write = Write: Manually trigger, restart, cancel or approve pending CI/CD pipelines. +actions.read = Read: View workflow runs and their logs. +actions.write = Write: Trigger, restart, and cancel workflows. Manage trust delegation to pull requests posters. ext_issues = Access the link to an external issue tracker. The permissions are managed externally. ext_wiki = Access the link to an external wiki. The permissions are managed externally. diff --git a/options/locale_next/locale_en-US.json b/options/locale_next/locale_en-US.json index a707e564a7..edca1072a8 100644 --- a/options/locale_next/locale_en-US.json +++ b/options/locale_next/locale_en-US.json @@ -56,6 +56,19 @@ "relativetime.2months": "two months ago", "relativetime.1year": "last year", "relativetime.2years": "two years ago", + "repo.pulls.poster_manage_approval": "Manage approval", + "repo.pulls.poster_requires_approval": "Some workflows are waiting to be reviewed.", + "repo.pulls.poster_requires_approval.tooltip": "The author of this pull request is not trusted to run workflows triggered by a pull request created from a forked repository or with AGit. The workflows triggered by a `pull_request` event will not run until they are approved.", + "repo.pulls.poster_is_trusted": "The author of this pull request is always trusted to run workflows.", + "repo.pulls.poster_is_trusted.tooltip": "The author of this pull request is explicitly trusted to run workflows triggered by `pull_request` events.", + "repo.pulls.poster_trust_deny": "Deny", + "repo.pulls.poster_trust_deny.tooltip": "The workflows waiting approval will be canceled.", + "repo.pulls.poster_trust_once": "Approve once", + "repo.pulls.poster_trust_once.tooltip": "The workflows triggered by a `pull_request` event will run on this commit but will need to be approved for all future commits pushed to this pull request.", + "repo.pulls.poster_trust_always": "Approve always", + "repo.pulls.poster_trust_always.tooltip": "The workflows triggered by a `pull_request` event will run on this commit and there will be no need to approve runs from this pull request or future pull requests authored by the same user.", + "repo.pulls.poster_trust_revoke": "Revoke", + "repo.pulls.poster_trust_revoke.tooltip": "The author of this pull request will no longer be trusted to run workflows triggered by a `pull_request` event, each run will have to be manually approved.", "repo.pulls.already_merged": "Merge failed: This pull request has already been merged.", "repo.pulls.merged_title_desc": { "one": "merged %[1]d commit from %[2]s into %[3]s %[4]s", @@ -137,6 +150,7 @@ "admin.auths.allow_username_change.description": "Allow users to change their username in the profile settings", "admin.dashboard.cleanup_offline_runners": "Cleanup offline runners", "admin.dashboard.remove_resolved_reports": "Remove resolved reports", + "admin.dashboard.actions_action_user": "Revoke Forgejo Actions trust for inactive users", "admin.config.security": "Security configuration", "admin.config.global_2fa_requirement.title": "Global two-factor requirement", "admin.config.global_2fa_requirement.none": "No", diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 95f383b9a6..55999702d2 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -680,42 +680,6 @@ func Cancel(ctx *context_module.Context) { ctx.JSON(http.StatusOK, struct{}{}) } -func Approve(ctx *context_module.Context) { - runIndex := ctx.ParamsInt64("run") - - current, jobs := getRunJobs(ctx, runIndex, -1) - if ctx.Written() { - return - } - run := current.Run - doer := ctx.Doer - - if err := db.WithTx(ctx, func(ctx context.Context) error { - run.NeedApproval = false - run.ApprovedBy = doer.ID - if err := actions_service.UpdateRun(ctx, run, "need_approval", "approved_by"); err != nil { - return err - } - for _, job := range jobs { - if len(job.Needs) == 0 && job.Status.IsBlocked() { - job.Status = actions_model.StatusWaiting - _, err := actions_service.UpdateRunJob(ctx, job, nil, "status") - if err != nil { - return err - } - } - } - return nil - }); err != nil { - ctx.Error(http.StatusInternalServerError, err.Error()) - return - } - - actions_service.CreateCommitStatus(ctx, jobs...) - - ctx.JSON(http.StatusOK, struct{}{}) -} - // getRunJobs gets the jobs of runIndex, and returns jobs[jobIndex], jobs. // Any error will be written to the ctx. // It never returns a nil job of an empty jobs, if the jobIndex is out of range, it will be treated as 0. diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 9d617e0c5c..69710824a4 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -18,6 +18,7 @@ import ( "strings" "forgejo.org/models" + actions_model "forgejo.org/models/actions" activities_model "forgejo.org/models/activities" asymkey_model "forgejo.org/models/asymkey" "forgejo.org/models/db" @@ -44,6 +45,7 @@ import ( "forgejo.org/modules/util" "forgejo.org/modules/web" "forgejo.org/routers/utils" + actions_service "forgejo.org/services/actions" asymkey_service "forgejo.org/services/asymkey" "forgejo.org/services/automerge" "forgejo.org/services/context" @@ -764,6 +766,11 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C ctx.Data["IsNothingToCompare"] = true } + PrepareViewPullInfoActions(ctx, pull) + if ctx.Written() { + return nil + } + if pull.IsWorkInProgress(ctx) { ctx.Data["IsPullWorkInProgress"] = true ctx.Data["WorkInProgressPrefix"] = pull.GetWorkInProgressPrefix(ctx) @@ -1942,3 +1949,67 @@ func SetAllowEdits(ctx *context.Context) { "allow_maintainer_edit": pr.AllowMaintainerEdit, }) } + +func PrepareViewPullInfoActions(ctx *context.Context, pull *issues_model.PullRequest) { + canReadUnitActions := ctx.Repo.CanRead(unit.TypeActions) + ctx.Data["CanReadUnitActions"] = canReadUnitActions + + if !canReadUnitActions { + return + } + + PrepareViewPullInfoActionsTrust(ctx, pull) + if ctx.Written() { + return + } +} + +func PrepareViewPullInfoActionsTrust(ctx *context.Context, pull *issues_model.PullRequest) { + trusted, err := actions_service.GetPullRequestPosterIsTrustedWithActions(ctx, pull) + if err != nil { + ctx.ServerError("GetPullRequestUserIsTrustedWithActions", err) + return + } + ctx.Data["PullRequestPosterIsNotTrustedWithActions"] = trusted == actions_service.UserIsNotTrustedWithActions + ctx.Data["PullRequestPosterIsExplicitlyTrustedWithActions"] = trusted == actions_service.UserIsExplicitlyTrustedWithActions + ctx.Data["PullRequestPosterIsImplicitlyTrustedWithActions"] = trusted == actions_service.UserIsImplicitlyTrustedWithActions + + someRunsNeedApproval, err := actions_model.HasRunThatNeedApproval(ctx, pull.Issue.RepoID, pull.ID) + if err != nil { + ctx.ServerError("HasRunThatNeedApproval", err) + } + ctx.Data["SomePullRequestRunsNeedApproval"] = someRunsNeedApproval + + ctx.Data["UserCanDelegateTrustWithPullRequest"] = context.CheckRepoDelegateActionTrust(ctx) +} + +func UpdateTrustWithPullRequestActions(ctx *context.Context) { + pr, err := issues_model.GetPullRequestByIndex(ctx, ctx.Repo.Repository.ID, ctx.ParamsInt64(":index")) + if err != nil { + if issues_model.IsErrPullRequestNotExist(err) { + ctx.NotFound("GetPullRequestByIndex", err) + } else { + ctx.ServerError("GetPullRequestByIndex", err) + } + return + } + + trust := ctx.FormString("trust") + + if err := actions_service.UpdateTrustedWithPullRequest(ctx, ctx.Doer.ID, pr, actions_service.TrustUpdate(trust)); err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + + if err := pr.LoadIssue(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + + if err := pr.Issue.LoadRepo(ctx); err != nil { + ctx.Error(http.StatusInternalServerError, err.Error()) + return + } + + ctx.Redirect(fmt.Sprintf("%s#pull-request-trust-panel", pr.Issue.Link())) +} diff --git a/routers/web/web.go b/routers/web/web.go index 071da0a6a7..3358e47965 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -848,6 +848,7 @@ func registerRoutes(m *web.Route) { reqRepoProjectsWriter := context.RequireRepoWriter(unit.TypeProjects) reqRepoActionsReader := context.RequireRepoReader(unit.TypeActions) reqRepoActionsWriter := context.RequireRepoWriter(unit.TypeActions) + reqRepoDelegateActionTrust := context.RequireRepoDelegateActionTrust() reqPackageAccess := func(accessMode perm.AccessMode) func(ctx *context.Context) { return func(ctx *context.Context) { @@ -1217,6 +1218,7 @@ func registerRoutes(m *web.Route) { m.Group("/{type:issues|pulls}", func() { m.Group("/{index}", func() { m.Post("/title", repo.UpdateIssueTitle) + m.Post("/action-user-trust", reqRepoActionsReader, actions.MustEnableActions, reqRepoDelegateActionTrust, repo.UpdateTrustWithPullRequestActions) m.Post("/content", repo.UpdateIssueContent) m.Post("/deadline", web.Bind(structs.EditDeadlineOption{}), repo.UpdateIssueDeadline) m.Post("/watch", repo.IssueWatch) @@ -1460,7 +1462,6 @@ func registerRoutes(m *web.Route) { Post(web.Bind(actions.ViewRequest{}), actions.ViewPost) }) m.Post("/cancel", reqRepoActionsWriter, actions.Cancel) - m.Post("/approve", reqRepoActionsWriter, actions.Approve) m.Get("/artifacts", actions.ArtifactsView) m.Get("/artifacts/{artifact_name_or_id}", actions.ArtifactsDownloadView) m.Delete("/artifacts/{artifact_name}", reqRepoActionsWriter, actions.ArtifactsDeleteView) diff --git a/services/actions/TestActionsNotifier_IsTrusted/issue.yml b/services/actions/TestActionsNotifier_IsTrusted/issue.yml new file mode 100644 index 0000000000..11d0032926 --- /dev/null +++ b/services/actions/TestActionsNotifier_IsTrusted/issue.yml @@ -0,0 +1,16 @@ +- + id: 8000 + repo_id: 10 + index: 1000 + poster_id: 1 # admin + original_author_id: 0 + name: pr8000 + content: pull request 8000 + milestone_id: 0 + priority: 0 + is_closed: false + is_pull: true + num_comments: 0 + created_unix: 946684820 + updated_unix: 978307180 + is_locked: false diff --git a/services/actions/TestActionsNotifier_IsTrusted/pull_request.yml b/services/actions/TestActionsNotifier_IsTrusted/pull_request.yml new file mode 100644 index 0000000000..6187a7ecc2 --- /dev/null +++ b/services/actions/TestActionsNotifier_IsTrusted/pull_request.yml @@ -0,0 +1,12 @@ +- + id: 3000 + type: 0 # pull request + status: 2 # mergeable + issue_id: 8000 + index: 1000 + head_repo_id: 11 + base_repo_id: 10 + head_branch: branch3000 + base_branch: master + merge_base: 0abcb056019adb83 + has_merged: false diff --git a/services/actions/TestActionsNotifier_IsTrusted/repo_unit.yml b/services/actions/TestActionsNotifier_IsTrusted/repo_unit.yml new file mode 100644 index 0000000000..25d90c17b9 --- /dev/null +++ b/services/actions/TestActionsNotifier_IsTrusted/repo_unit.yml @@ -0,0 +1,6 @@ +- + id: 11500 + repo_id: 10 + type: 10 # TypeActions + config: "{}" + created_unix: 946684810 diff --git a/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/action_user.yml b/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/action_user.yml new file mode 100644 index 0000000000..bce18ebf96 --- /dev/null +++ b/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/action_user.yml @@ -0,0 +1,12 @@ +- + id: 10 + repo_id: 10 + user_id: 4 + trusted_with_pull_requests: true + last_access: 1683636528 +- + id: 11 + repo_id: 10 + user_id: 29 + trusted_with_pull_requests: true + last_access: 1683636528 diff --git a/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/issue.yml b/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/issue.yml new file mode 100644 index 0000000000..4eae79b5a4 --- /dev/null +++ b/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/issue.yml @@ -0,0 +1,84 @@ +- + id: 81000 + repo_id: 10 + index: 1000 + poster_id: 4 # regular user + original_author_id: 0 + name: pr1000 + content: pull request + milestone_id: 0 + priority: 0 + is_closed: false + is_pull: true + num_comments: 0 + created_unix: 946684820 + updated_unix: 978307180 + is_locked: false + +- + id: 82000 + repo_id: 11 + index: 2000 + poster_id: 2 # regular user + original_author_id: 0 + name: pr2000 + content: pull request + milestone_id: 0 + priority: 0 + is_closed: false + is_pull: true + num_comments: 0 + created_unix: 946684820 + updated_unix: 978307180 + is_locked: false + +- + id: 83000 + repo_id: 10 + index: 3000 + poster_id: 1 # admin + original_author_id: 0 + name: pr3000 + content: pull request + milestone_id: 0 + priority: 0 + is_closed: false + is_pull: true + num_comments: 0 + created_unix: 946684820 + updated_unix: 978307180 + is_locked: false + +- + id: 84000 + repo_id: 10 + index: 4000 + poster_id: 5 # regular user + original_author_id: 0 + name: pr4000 + content: pull request + milestone_id: 0 + priority: 0 + is_closed: false + is_pull: true + num_comments: 0 + created_unix: 946684820 + updated_unix: 978307180 + is_locked: false + +- + id: 85000 + repo_id: 10 + index: 5000 + poster_id: 29 # restricted user + original_author_id: 0 + name: pr5000 + content: pull request + milestone_id: 0 + priority: 0 + is_closed: false + is_pull: true + num_comments: 0 + created_unix: 946684820 + updated_unix: 978307180 + is_locked: false diff --git a/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/pull_request.yml b/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/pull_request.yml new file mode 100644 index 0000000000..c46dfc063a --- /dev/null +++ b/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/pull_request.yml @@ -0,0 +1,64 @@ +- + id: 1000 + type: 0 # pull request + status: 2 # mergeable + issue_id: 81000 + index: 1000 + head_repo_id: 11 + base_repo_id: 10 + head_branch: branch2000 + base_branch: master + merge_base: 0abcb056019adb83 + has_merged: false + +- + id: 2000 + type: 0 # pull request + status: 2 # mergeable + issue_id: 82000 + index: 2000 + head_repo_id: 11 + base_repo_id: 11 + head_branch: branch2000 + base_branch: master + merge_base: 0abcb056019adb83 + has_merged: false + +- + id: 3000 + type: 0 # pull request + status: 2 # mergeable + issue_id: 83000 + index: 3000 + head_repo_id: 11 # different from base_repo + base_repo_id: 10 + head_branch: branch3000 + base_branch: master + merge_base: 0abcb056019adb83 + has_merged: false + +- + id: 4000 + type: 0 # pull request + status: 2 # mergeable + issue_id: 84000 + index: 4000 + head_repo_id: 11 # different from base_repo + base_repo_id: 10 + head_branch: branch4000 + base_branch: master + merge_base: 0abcb056019adb83 + has_merged: false + +- + id: 5000 + type: 0 # pull request + status: 2 # mergeable + issue_id: 85000 + index: 5000 + head_repo_id: 11 # different from base_repo + base_repo_id: 10 + head_branch: branch5000 + base_branch: master + merge_base: 0abcb056019adb83 + has_merged: false diff --git a/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/repo_unit.yml b/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/repo_unit.yml new file mode 100644 index 0000000000..25d90c17b9 --- /dev/null +++ b/services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions/repo_unit.yml @@ -0,0 +1,6 @@ +- + id: 11500 + repo_id: 10 + type: 10 # TypeActions + config: "{}" + created_unix: 946684810 diff --git a/services/actions/TestActions_CancelOrApproveRun/action_run.yml b/services/actions/TestActions_CancelOrApproveRun/action_run.yml new file mode 100644 index 0000000000..b084007503 --- /dev/null +++ b/services/actions/TestActions_CancelOrApproveRun/action_run.yml @@ -0,0 +1,38 @@ +- + id: 900 + title: "run running" + repo_id: 63 + owner_id: 2 + workflow_id: "test.yaml" + index: 4 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "synchronized" + is_fork_pull_request: 0 + status: 6 # running + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 0 + approved_by: 0 + concurrency_group: abc123 +- + id: 800 + title: "run waiting because jobs need approval" + repo_id: 63 + owner_id: 2 + workflow_id: "test.yaml" + index: 5 + trigger_user_id: 2 + ref: "refs/heads/main" + commit_sha: "97f29ee599c373c729132a5c46a046978311e0ee" + trigger_event: "synchronized" + is_fork_pull_request: 0 + status: 5 # waiting + started: 1683636528 + created: 1683636108 + updated: 1683636626 + need_approval: 1 + approved_by: 0 + concurrency_group: def435 diff --git a/services/actions/TestActions_CancelOrApproveRun/action_run_job.yml b/services/actions/TestActions_CancelOrApproveRun/action_run_job.yml new file mode 100644 index 0000000000..610f34e274 --- /dev/null +++ b/services/actions/TestActions_CancelOrApproveRun/action_run_job.yml @@ -0,0 +1,42 @@ +- + id: 10900 + run_id: 900 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: job_1 + attempt: 0 + job_id: job_1 + task_id: 0 + status: 1 # success + runs_on: '["docker"]' + started: 1683636528 +- + id: 11900 + run_id: 900 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: job_2 + attempt: 0 + job_id: job_2 + task_id: 711900 + status: 6 # running + runs_on: '["docker"]' + started: 1683636528 +- + id: 10800 + run_id: 800 + repo_id: 63 + owner_id: 2 + commit_sha: 97f29ee599c373c729132a5c46a046978311e0ee + is_fork_pull_request: 0 + name: job_1 + attempt: 0 + job_id: job_1 + task_id: 0 + status: 7 # blocked + runs_on: '["docker"]' + started: 1683636528 diff --git a/services/actions/TestActions_CancelOrApproveRun/action_task.yml b/services/actions/TestActions_CancelOrApproveRun/action_task.yml new file mode 100644 index 0000000000..5aaafe97a1 --- /dev/null +++ b/services/actions/TestActions_CancelOrApproveRun/action_task.yml @@ -0,0 +1,40 @@ +- + id: 710900 + job_id: 10900 + attempt: 0 + runner_id: 1 + status: 1 # success + started: 1683636528 + stopped: 1683636626 + repo_id: 63 + owner_id: 2 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: 8d8ef48297195edcc8e22c70b3020eaa06c52976db67d39b4260c64a69a2cc1508825121b7b8394e48e00b1bf8718b2a867e + token_salt: jVuKnSPGgy + token_last_eight: eeb1a71a + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 +- + id: 711900 + job_id: 11900 + attempt: 0 + runner_id: 1 + status: 6 # running + started: 1683636528 + stopped: 1683636626 + repo_id: 63 + owner_id: 2 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + is_fork_pull_request: 0 + token_hash: 7d8ef48297195edcc8e22c70b3020eaa06c52976db67d39b4260c64a69a2cc1508825121b7b8394e48e00b1bf8718b2a867e + token_salt: jVuKnSPGgy + token_last_eight: eeb1a71a + log_filename: artifact-test2/2f/47.log + log_in_storage: 1 + log_length: 707 + log_size: 90179 + log_expired: 0 diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index 2fc89e221d..7f46784d40 100644 --- a/services/actions/notifier_helper.go +++ b/services/actions/notifier_helper.go @@ -157,12 +157,43 @@ func notify(ctx context.Context, input *notifyInput) error { return nil } - gitRepo, err := gitrepo.OpenRepository(context.Background(), input.Repo) + gitRepo, commit, ref, err := getGitRepoAndCommit(ctx, input) if err != nil { - return fmt.Errorf("git.OpenRepository: %w", err) + return err + } else if gitRepo == nil && commit == nil { + return nil } defer gitRepo.Close() + if skipWorkflows(input, commit) { + return nil + } + + if SkipPullRequestEvent(ctx, input.Event, input.Repo.ID, commit.ID.String()) { + log.Trace("repo %s with commit %s skip event %v", input.Repo.RepoPath(), commit.ID, input.Event) + return nil + } + + detectedWorkflows, schedules, err := detectWorkflows(ctx, input, gitRepo, commit, shouldDetectSchedules) + if err != nil { + return err + } + + if shouldDetectSchedules { + if err := handleSchedules(ctx, schedules, commit, input, ref.String()); err != nil { + return err + } + } + + return handleWorkflows(ctx, detectedWorkflows, commit, input, ref.String()) +} + +func getGitRepoAndCommit(_ context.Context, input *notifyInput) (*git.Repository, *git.Commit, git.RefName, error) { + gitRepo, err := gitrepo.OpenRepository(context.Background(), input.Repo) + if err != nil { + return nil, nil, "", fmt.Errorf("git.OpenRepository: %w", err) + } + ref := input.Ref if ref.BranchName() != input.Repo.DefaultBranch && actions_module.IsDefaultBranchWorkflow(input.Event) { if ref != "" { @@ -178,24 +209,20 @@ func notify(ctx context.Context, input *notifyInput) error { commitID, err := gitRepo.GetRefCommitID(ref.String()) if err != nil { - return fmt.Errorf("gitRepo.GetRefCommitID: %w", err) + gitRepo.Close() + return nil, nil, "", fmt.Errorf("gitRepo.GetRefCommitID: %w", err) } // Get the commit object for the ref commit, err := gitRepo.GetCommit(commitID) if err != nil { - return fmt.Errorf("gitRepo.GetCommit: %w", err) - } - - if skipWorkflows(input, commit) { - return nil - } - - if SkipPullRequestEvent(ctx, input.Event, input.Repo.ID, commit.ID.String()) { - log.Trace("repo %s with commit %s skip event %v", input.Repo.RepoPath(), commit.ID, input.Event) - return nil + gitRepo.Close() + return nil, nil, "", fmt.Errorf("gitRepo.GetCommit: %w", err) } + return gitRepo, commit, ref, nil +} +func detectWorkflows(ctx context.Context, input *notifyInput, gitRepo *git.Repository, commit *git.Commit, shouldDetectSchedules bool) ([]*actions_module.DetectedWorkflow, []*actions_module.DetectedWorkflow, error) { var detectedWorkflows []*actions_module.DetectedWorkflow actionsConfig := input.Repo.MustGetUnit(ctx, unit_model.TypeActions).ActionsConfig() workflows, schedules, err := actions_module.DetectWorkflows(gitRepo, commit, @@ -204,7 +231,7 @@ func notify(ctx context.Context, input *notifyInput) error { shouldDetectSchedules, ) if err != nil { - return fmt.Errorf("DetectWorkflows: %w", err) + return nil, nil, fmt.Errorf("DetectWorkflows: %w", err) } log.Trace("repo %s with commit %s event %s find %d workflows and %d schedules", @@ -215,6 +242,47 @@ func notify(ctx context.Context, input *notifyInput) error { len(schedules), ) + if input.PullRequest != nil && !actions_module.IsDefaultBranchWorkflow(input.Event) { + // detect pull_request_target workflows + baseRef := git.BranchPrefix + input.PullRequest.BaseBranch + baseCommit, err := gitRepo.GetCommit(baseRef) + if err != nil { + if prp, ok := input.Payload.(*api.PullRequestPayload); ok && errors.Is(err, util.ErrNotExist) { + // the baseBranch was deleted and the PR closed: the action can be skipped + if prp.Action == api.HookIssueClosed { + return nil, nil, nil + } + } + return nil, nil, fmt.Errorf("gitRepo.GetCommit: %w", err) + } + baseWorkflows, _, err := actions_module.DetectWorkflows(gitRepo, baseCommit, input.Event, input.Payload, false) + if err != nil { + return nil, nil, fmt.Errorf("DetectWorkflows: %w", err) + } + if len(baseWorkflows) == 0 { + log.Trace("repo %s with commit %s couldn't find pull_request_target workflows", input.Repo.RepoPath(), baseCommit.ID) + } else { + for _, wf := range baseWorkflows { + if wf.TriggerEvent.Name == actions_module.GithubEventPullRequestTarget { + detectedWorkflows = append(detectedWorkflows, wf) + } + } + } + + useHeadOrBaseCommit, pullRequestNeedApproval, err := getPullRequestCommitAndApproval(ctx, input.PullRequest, input.Doer, input.Event) + if err != nil { + return nil, nil, fmt.Errorf("getPullRequestTrust: %w", err) + } + + if useHeadOrBaseCommit == useBaseCommit { + workflows = baseWorkflows + } else if pullRequestNeedApproval { + for _, wf := range workflows { + wf.NeedApproval = pullRequestNeedApproval + } + } + } + for _, wf := range workflows { if actionsConfig.IsWorkflowDisabled(wf.EntryName) { log.Trace("repo %s has disable workflows %s", input.Repo.RepoPath(), wf.EntryName) @@ -226,41 +294,7 @@ func notify(ctx context.Context, input *notifyInput) error { } } - if input.PullRequest != nil && !actions_module.IsDefaultBranchWorkflow(input.Event) { - // detect pull_request_target workflows - baseRef := git.BranchPrefix + input.PullRequest.BaseBranch - baseCommit, err := gitRepo.GetCommit(baseRef) - if err != nil { - if prp, ok := input.Payload.(*api.PullRequestPayload); ok && errors.Is(err, util.ErrNotExist) { - // the baseBranch was deleted and the PR closed: the action can be skipped - if prp.Action == api.HookIssueClosed { - return nil - } - } - return fmt.Errorf("gitRepo.GetCommit: %w", err) - } - baseWorkflows, _, err := actions_module.DetectWorkflows(gitRepo, baseCommit, input.Event, input.Payload, false) - if err != nil { - return fmt.Errorf("DetectWorkflows: %w", err) - } - if len(baseWorkflows) == 0 { - log.Trace("repo %s with commit %s couldn't find pull_request_target workflows", input.Repo.RepoPath(), baseCommit.ID) - } else { - for _, wf := range baseWorkflows { - if wf.TriggerEvent.Name == actions_module.GithubEventPullRequestTarget { - detectedWorkflows = append(detectedWorkflows, wf) - } - } - } - } - - if shouldDetectSchedules { - if err := handleSchedules(ctx, schedules, commit, input, ref.String()); err != nil { - return err - } - } - - return handleWorkflows(ctx, detectedWorkflows, commit, input, ref.String()) + return detectedWorkflows, schedules, nil } func SkipPullRequestEvent(ctx context.Context, event webhook_module.HookEventType, repoID int64, commitSHA string) bool { @@ -321,35 +355,25 @@ func handleWorkflows( return fmt.Errorf("json.Marshal: %w", err) } - isForkPullRequest := false - if pr := input.PullRequest; pr != nil && !actions_module.IsDefaultBranchWorkflow(input.Event) { - switch pr.Flow { - case issues_model.PullRequestFlowGithub: - isForkPullRequest = pr.IsFromFork() - case issues_model.PullRequestFlowAGit: - // There is no fork concept in agit flow, anyone with read permission can push refs/for// to the repo. - // So we can treat it as a fork pull request because it may be from an untrusted user - isForkPullRequest = true - default: - // unknown flow, assume it's a fork pull request to be safe - isForkPullRequest = true - } - } - for _, dwf := range detectedWorkflows { run := &actions_model.ActionRun{ - Title: strings.SplitN(commit.CommitMessage, "\n", 2)[0], - RepoID: input.Repo.ID, - OwnerID: input.Repo.OwnerID, - WorkflowID: dwf.EntryName, - TriggerUserID: input.Doer.ID, - Ref: ref, - CommitSHA: commit.ID.String(), - IsForkPullRequest: isForkPullRequest, - Event: input.Event, - EventPayload: string(p), - TriggerEvent: dwf.TriggerEvent.Name, - Status: actions_model.StatusWaiting, + Title: strings.SplitN(commit.CommitMessage, "\n", 2)[0], + RepoID: input.Repo.ID, + OwnerID: input.Repo.OwnerID, + WorkflowID: dwf.EntryName, + TriggerUserID: input.Doer.ID, + Ref: ref, + CommitSHA: commit.ID.String(), + Event: input.Event, + EventPayload: string(p), + TriggerEvent: dwf.TriggerEvent.Name, + Status: actions_model.StatusWaiting, + } + + if !actions_module.IsDefaultBranchWorkflow(input.Event) { + if err := setRunTrustForPullRequest(ctx, run, input.PullRequest, dwf.NeedApproval); err != nil { + return fmt.Errorf("setTrustForPullRequest: %w", err) + } } workflow, err := model.ReadWorkflow(bytes.NewReader(dwf.Content), false) @@ -363,14 +387,6 @@ func handleWorkflows( } run.NotifyEmail = notifications - need, err := ifNeedApproval(ctx, run, input.Repo, input.Doer) - if err != nil { - log.Error("check if need approval for repo %d with user %d: %v", input.Repo.ID, input.Doer.ID, err) - continue - } - - run.NeedApproval = need - if err := run.LoadAttributes(ctx); err != nil { log.Error("LoadAttributes: %v", err) continue @@ -396,7 +412,7 @@ func handleWorkflows( Name: dwf.EntryName, }} } else { - jobs, err = jobParser(dwf.Content, jobparser.WithVars(vars)) + jobs, err = actions_module.JobParser(dwf.Content, jobparser.WithVars(vars)) if err != nil { log.Info("jobparser.Parse: invalid workflow, setting job status to failed: %v", err) tr := translation.NewLocale(input.Doer.Language) @@ -479,45 +495,6 @@ func notifyPackage(ctx context.Context, sender *user_model.User, pd *packages_mo Notify(ctx) } -func ifNeedApproval(ctx context.Context, run *actions_model.ActionRun, repo *repo_model.Repository, user *user_model.User) (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 - if !run.IsForkPullRequest || run.TriggerEvent == actions_module.GithubEventPullRequestTarget { - return false, nil - } - - // always need approval if the user is restricted - if user.IsRestricted { - log.Trace("need approval because user %d is restricted", user.ID) - return true, nil - } - - // don't need approval if the user can write - if perm, err := access_model.GetUserRepoPermission(ctx, repo, user); err != nil { - return false, fmt.Errorf("GetUserRepoPermission: %w", err) - } else if perm.CanWrite(unit_model.TypeActions) { - 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) - return false, nil - } - - // otherwise, need approval - log.Trace("need approval because it's the first time user %d triggered actions", user.ID) - return true, nil -} - func handleSchedules( ctx context.Context, detectedWorkflows []*actions_module.DetectedWorkflow, diff --git a/services/actions/notifier_helper_test.go b/services/actions/notifier_helper_test.go index 4a99340445..88145760d5 100644 --- a/services/actions/notifier_helper_test.go +++ b/services/actions/notifier_helper_test.go @@ -24,7 +24,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_SkipPullRequestEvent(t *testing.T) { +func TestActionsNotifier_SkipPullRequestEvent(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) repoID := int64(1) @@ -59,7 +59,7 @@ func Test_SkipPullRequestEvent(t *testing.T) { assert.True(t, SkipPullRequestEvent(db.DefaultContext, webhook_module.HookEventPullRequestSync, repoID, commitSHA)) } -func Test_IssueCommentOnForkPullRequestEvent(t *testing.T) { +func TestActionsNotifier_IssueCommentOnForkPullRequestEvent(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) @@ -103,39 +103,47 @@ func Test_IssueCommentOnForkPullRequestEvent(t *testing.T) { assert.False(t, runs[0].IsForkPullRequest) } -func Test_OpenForkPullRequestEvent(t *testing.T) { - require.NoError(t, unittest.PrepareTestDatabase()) +func testActionsNotifierPullRequest(t *testing.T, repo *repo_model.Repository, pr *issues_model.PullRequest, dw *actions_module.DetectedWorkflow, event webhook_module.HookEventType) { + t.Helper() - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) require.NoError(t, pr.LoadIssue(db.DefaultContext)) - require.True(t, pr.IsFromFork()) + testActionsNotifierPullRequestWithDoer(t, repo, pr, doer, dw, event) +} + +func testActionsNotifierPullRequestWithDoer(t *testing.T, repo *repo_model.Repository, pr *issues_model.PullRequest, doer *user_model.User, dw *actions_module.DetectedWorkflow, event webhook_module.HookEventType) { + t.Helper() commit := &git.Commit{ ID: git.MustIDFromString("0000000000000000000000000000000000000000"), CommitMessage: "test", } - detectedWorkflows := []*actions_module.DetectedWorkflow{ - { - TriggerEvent: &jobparser.Event{ - Name: "pull_request", - }, - }, + dw.EntryName = "test.yml" + dw.TriggerEvent = &jobparser.Event{ + Name: "pull_request", } + detectedWorkflows := []*actions_module.DetectedWorkflow{dw} input := ¬ifyInput{ Repo: repo, Doer: doer, - Event: webhook_module.HookEventPullRequest, + Event: event, PullRequest: pr, Payload: &api.PullRequestPayload{}, } - unittest.AssertSuccessfulDelete(t, &actions_model.ActionRun{RepoID: repo.ID}) - - err := handleWorkflows(db.DefaultContext, detectedWorkflows, commit, input, "") + err := handleWorkflows(db.DefaultContext, detectedWorkflows, commit, input, "refs/head/main") require.NoError(t, err) +} + +func TestActionsNotifier_OpenForkPullRequestEvent(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) + require.True(t, pr.IsFromFork()) + + testActionsNotifierPullRequest(t, repo, pr, &actions_module.DetectedWorkflow{}, webhook_module.HookEventPullRequest) runs, err := db.Find[actions_model.ActionRun](db.DefaultContext, actions_model.FindRunOptions{ RepoID: repo.ID, @@ -147,36 +155,16 @@ func Test_OpenForkPullRequestEvent(t *testing.T) { assert.True(t, runs[0].IsForkPullRequest) } -func TestActionsNotifierConcurrencyGroup(t *testing.T) { +func TestActionsNotifier_ConcurrencyGroup(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) - commit := &git.Commit{ - ID: git.MustIDFromString("0000000000000000000000000000000000000000"), - CommitMessage: "test", + dw := &actions_module.DetectedWorkflow{ + Content: []byte("{ on: pull_request, jobs: { j1: {} }}"), } - detectedWorkflows := []*actions_module.DetectedWorkflow{ - { - EntryName: "test.yml", - TriggerEvent: &jobparser.Event{ - Name: "pull_request", - }, - Content: []byte("{ on: pull_request, jobs: { j1: {} }}"), - }, - } - input := ¬ifyInput{ - Repo: repo, - Doer: doer, - Event: webhook_module.HookEventPullRequestSync, - PullRequest: pr, - Payload: &api.PullRequestPayload{}, - } - - err := handleWorkflows(db.DefaultContext, detectedWorkflows, commit, input, "refs/head/main") - require.NoError(t, err) + testActionsNotifierPullRequest(t, repo, pr, dw, webhook_module.HookEventPullRequestSync) runs, err := db.Find[actions_model.ActionRun](db.DefaultContext, actions_model.FindRunOptions{ RepoID: repo.ID, @@ -192,8 +180,7 @@ func TestActionsNotifierConcurrencyGroup(t *testing.T) { // Also... check if CancelPreviousWithConcurrencyGroup is invoked from handleWorkflows by firing off a second // workflow and checking that the first one gets cancelled: - err = handleWorkflows(db.DefaultContext, detectedWorkflows, commit, input, "refs/head/main") - require.NoError(t, err) + testActionsNotifierPullRequest(t, repo, pr, dw, webhook_module.HookEventPullRequestSync) runs, err = db.Find[actions_model.ActionRun](db.DefaultContext, actions_model.FindRunOptions{ RepoID: repo.ID, @@ -210,36 +197,16 @@ func TestActionsNotifierConcurrencyGroup(t *testing.T) { assert.Equal(t, actions_model.StatusCancelled, firstRun.Status) } -func TestActionsPreExecutionErrorInvalidJobs(t *testing.T) { +func TestActionsNotifier_PreExecutionErrorInvalidJobs(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) - commit := &git.Commit{ - ID: git.MustIDFromString("0000000000000000000000000000000000000000"), - CommitMessage: "test", + dw := &actions_module.DetectedWorkflow{ + Content: []byte("{ on: pull_request, jobs: 'hello, I am the jobs!' }"), } - detectedWorkflows := []*actions_module.DetectedWorkflow{ - { - EntryName: "test.yml", - TriggerEvent: &jobparser.Event{ - Name: "pull_request", - }, - Content: []byte("{ on: pull_request, jobs: 'hello, I am the jobs!' }"), - }, - } - input := ¬ifyInput{ - Repo: repo, - Doer: doer, - Event: webhook_module.HookEventPullRequestSync, - PullRequest: pr, - Payload: &api.PullRequestPayload{}, - } - - err := handleWorkflows(db.DefaultContext, detectedWorkflows, commit, input, "refs/head/main") - require.NoError(t, err) + testActionsNotifierPullRequest(t, repo, pr, dw, webhook_module.HookEventPullRequestSync) runs, err := db.Find[actions_model.ActionRun](db.DefaultContext, actions_model.FindRunOptions{ RepoID: repo.ID, @@ -252,37 +219,17 @@ func TestActionsPreExecutionErrorInvalidJobs(t *testing.T) { assert.Contains(t, createdRun.PreExecutionError, "actions.workflow.job_parsing_error%!(EXTRA *fmt.wrapError=") } -func TestActionsPreExecutionEventDetectionError(t *testing.T) { +func TestActionsNotifier_PreExecutionEventDetectionError(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) - commit := &git.Commit{ - ID: git.MustIDFromString("0000000000000000000000000000000000000000"), - CommitMessage: "test", + dw := &actions_module.DetectedWorkflow{ + Content: []byte("{ on: nothing, jobs: { j1: {} }}"), + EventDetectionError: errors.New("nothing is not a valid event"), } - detectedWorkflows := []*actions_module.DetectedWorkflow{ - { - EntryName: "test.yml", - TriggerEvent: &jobparser.Event{ - Name: "pull_request", - }, - Content: []byte("{ on: nothing, jobs: { j1: {} }}"), - EventDetectionError: errors.New("nothing is not a valid event"), - }, - } - input := ¬ifyInput{ - Repo: repo, - Doer: doer, - Event: webhook_module.HookEventPullRequestSync, - PullRequest: pr, - Payload: &api.PullRequestPayload{}, - } - - err := handleWorkflows(db.DefaultContext, detectedWorkflows, commit, input, "refs/head/main") - require.NoError(t, err) + testActionsNotifierPullRequest(t, repo, pr, dw, webhook_module.HookEventPullRequestSync) runs, err := db.Find[actions_model.ActionRun](db.DefaultContext, actions_model.FindRunOptions{ RepoID: repo.ID, @@ -294,3 +241,27 @@ func TestActionsPreExecutionEventDetectionError(t *testing.T) { assert.Equal(t, actions_model.StatusFailure, createdRun.Status) assert.Equal(t, "actions.workflow.event_detection_error%!(EXTRA *errors.errorString=nothing is not a valid event)", createdRun.PreExecutionError) } + +func TestActionsNotifier_handleWorkflows_setRunTrustForPullRequest(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) + // poster is not trusted implicitly + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3}) + + testActionsNotifierPullRequest(t, repo, pr, &actions_module.DetectedWorkflow{ + NeedApproval: true, + }, webhook_module.HookEventPullRequest) + + runs, err := db.Find[actions_model.ActionRun](db.DefaultContext, actions_model.FindRunOptions{ + RepoID: repo.ID, + }) + require.NoError(t, err) + require.Len(t, runs, 1) + + run := runs[0] + assert.True(t, run.IsForkPullRequest) + assert.Equal(t, pr.Issue.PosterID, run.PullRequestPosterID) + assert.Equal(t, pr.ID, run.PullRequestID) + assert.True(t, run.NeedApproval) +} diff --git a/services/actions/run.go b/services/actions/run.go new file mode 100644 index 0000000000..7f64e00b92 --- /dev/null +++ b/services/actions/run.go @@ -0,0 +1,70 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "context" + + actions_model "forgejo.org/models/actions" + "forgejo.org/models/db" + "forgejo.org/modules/timeutil" +) + +func CancelRun(ctx context.Context, run *actions_model.ActionRun) error { + return db.WithTx(ctx, func(ctx context.Context) error { + jobs, err := actions_model.GetRunJobsByRunID(ctx, run.ID) + if err != nil { + return err + } + for _, job := range jobs { + status := job.Status + if status.IsDone() { + continue + } + if job.TaskID == 0 { + job.Status = actions_model.StatusCancelled + job.Stopped = timeutil.TimeStampNow() + _, err := actions_model.UpdateRunJobWithoutNotification(ctx, job, nil, "status", "stopped") + if err != nil { + return err + } + continue + } + if err := StopTask(ctx, job.TaskID, actions_model.StatusCancelled); err != nil { + return err + } + } + + if run.NeedApproval { + if err := actions_model.UpdateRunApprovalByID(ctx, run.ID, actions_model.DoesNotNeedApproval, 0); err != nil { + return err + } + } + + CreateCommitStatus(ctx, jobs...) + + return nil + }) +} + +func ApproveRun(ctx context.Context, run *actions_model.ActionRun, doerID int64) error { + return db.WithTx(ctx, func(ctx context.Context) error { + jobs, err := actions_model.GetRunJobsByRunID(ctx, run.ID) + if err != nil { + return err + } + for _, job := range jobs { + if len(job.Needs) == 0 && job.Status.IsBlocked() { + job.Status = actions_model.StatusWaiting + _, err := UpdateRunJob(ctx, job, nil, "status") + if err != nil { + return err + } + } + } + CreateCommitStatus(ctx, jobs...) + + return actions_model.UpdateRunApprovalByID(ctx, run.ID, actions_model.DoesNotNeedApproval, doerID) + }) +} diff --git a/services/actions/run_test.go b/services/actions/run_test.go new file mode 100644 index 0000000000..15d64a9346 --- /dev/null +++ b/services/actions/run_test.go @@ -0,0 +1,104 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "testing" + + actions_model "forgejo.org/models/actions" + "forgejo.org/models/unittest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestActions_CancelOrApproveRun(t *testing.T) { + t.Run("run, job and task Running changes to run, job and task Cancelled", func(t *testing.T) { + defer unittest.OverrideFixtures("services/actions/TestActions_CancelOrApproveRun")() + require.NoError(t, unittest.PrepareTestDatabase()) + + taskID := int64(711900) + task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskID}) + require.Equal(t, actions_model.StatusRunning.String(), task.Status.String()) + job := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: task.JobID}) + require.Equal(t, actions_model.StatusRunning.String(), job.Status.String()) + require.Zero(t, job.Stopped) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: job.RunID}) + require.Equal(t, actions_model.StatusRunning.String(), run.Status.String()) + + require.NoError(t, CancelRun(t.Context(), run)) + + run = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: job.RunID}) + assert.Equal(t, actions_model.StatusCancelled.String(), run.Status.String()) + job = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: task.JobID}) + assert.Equal(t, actions_model.StatusCancelled.String(), job.Status.String()) + assert.NotZero(t, job.Stopped) + task = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskID}) + require.Equal(t, actions_model.StatusCancelled.String(), task.Status.String()) + }) + + t.Run("run Running, job and task Success changes to run Cancelled", func(t *testing.T) { + defer unittest.OverrideFixtures("services/actions/TestActions_CancelOrApproveRun")() + require.NoError(t, unittest.PrepareTestDatabase()) + + taskID := int64(710900) + task := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskID}) + require.Equal(t, actions_model.StatusSuccess.String(), task.Status.String()) + job := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: task.JobID}) + require.Equal(t, actions_model.StatusSuccess.String(), job.Status.String()) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: job.RunID}) + require.Equal(t, actions_model.StatusRunning.String(), run.Status.String()) + + require.NoError(t, CancelRun(t.Context(), run)) + + run = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: job.RunID}) + assert.Equal(t, actions_model.StatusCancelled.String(), run.Status.String()) + job = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: task.JobID}) + assert.Equal(t, actions_model.StatusSuccess, job.Status) + task = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionTask{ID: taskID}) + require.Equal(t, actions_model.StatusSuccess, task.Status) + }) + + t.Run("run Waiting and job Blocked for Approval changes to run and job Cancelled", func(t *testing.T) { + defer unittest.OverrideFixtures("services/actions/TestActions_CancelOrApproveRun")() + require.NoError(t, unittest.PrepareTestDatabase()) + + jobID := int64(10800) + job := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: jobID}) + require.Equal(t, actions_model.StatusBlocked.String(), job.Status.String()) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: job.RunID}) + require.Equal(t, actions_model.StatusWaiting.String(), run.Status.String()) + require.True(t, run.NeedApproval) + + require.NoError(t, CancelRun(t.Context(), run)) + + run = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: job.RunID}) + assert.Equal(t, actions_model.StatusCancelled.String(), run.Status.String()) + assert.False(t, run.NeedApproval) + job = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: jobID}) + assert.Equal(t, actions_model.StatusCancelled, job.Status) + }) + + t.Run("run Waiting and job Blocked for Approval changes to job Waiting", func(t *testing.T) { + defer unittest.OverrideFixtures("services/actions/TestActions_CancelOrApproveRun")() + require.NoError(t, unittest.PrepareTestDatabase()) + + jobID := int64(10800) + job := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: jobID}) + require.Equal(t, actions_model.StatusBlocked.String(), job.Status.String()) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: job.RunID}) + require.Equal(t, actions_model.StatusWaiting.String(), run.Status.String()) + require.True(t, run.NeedApproval) + + doerID := int64(30) + require.NoError(t, ApproveRun(t.Context(), run, doerID)) + + run = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: job.RunID}) + assert.Equal(t, actions_model.StatusWaiting.String(), run.Status.String()) + assert.False(t, run.NeedApproval) + assert.Equal(t, doerID, run.ApprovedBy) + job = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{ID: jobID}) + assert.Equal(t, actions_model.StatusWaiting, job.Status) + }) +} diff --git a/services/actions/schedule_tasks.go b/services/actions/schedule_tasks.go index e7dad2d948..dfaeaae59b 100644 --- a/services/actions/schedule_tasks.go +++ b/services/actions/schedule_tasks.go @@ -14,6 +14,7 @@ import ( "forgejo.org/models/db" repo_model "forgejo.org/models/repo" "forgejo.org/models/unit" + actions_module "forgejo.org/modules/actions" "forgejo.org/modules/log" "forgejo.org/modules/timeutil" webhook_module "forgejo.org/modules/webhook" @@ -168,7 +169,7 @@ func CreateScheduleTask(ctx context.Context, cron *actions_model.ActionSchedule) } // Parse the workflow specification from the cron schedule - workflows, err := jobParser(cron.Content, jobparser.WithVars(vars)) + workflows, err := actions_module.JobParser(cron.Content, jobparser.WithVars(vars)) if err != nil { return err } diff --git a/services/actions/trust.go b/services/actions/trust.go new file mode 100644 index 0000000000..55d97a5756 --- /dev/null +++ b/services/actions/trust.go @@ -0,0 +1,326 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "context" + "fmt" + + actions_model "forgejo.org/models/actions" + issues_model "forgejo.org/models/issues" + access_model "forgejo.org/models/perm/access" + repo_model "forgejo.org/models/repo" + unit_model "forgejo.org/models/unit" + user_model "forgejo.org/models/user" + actions_module "forgejo.org/modules/actions" + "forgejo.org/modules/log" + webhook_module "forgejo.org/modules/webhook" +) + +type TrustUpdate string + +const ( + UserTrustDenied = TrustUpdate("deny") + UserAlwaysTrusted = TrustUpdate("always") + UserTrustedOnce = TrustUpdate("once") + UserTrustRevoked = TrustUpdate("revoke") +) + +func CleanupActionUser(ctx context.Context) error { + return actions_model.RevokeInactiveActionUser(ctx) +} + +func loadPullRequestAttributes(ctx context.Context, pr *issues_model.PullRequest) error { + if err := pr.LoadIssue(ctx); err != nil { + return err + } + + return pr.Issue.LoadRepo(ctx) +} + +func getIssuePoster(ctx context.Context, issue *issues_model.Issue) (*user_model.User, error) { + if issue.Poster != nil { + return issue.Poster, nil + } + if issue.PosterID == 0 { + return nil, nil + } + + poster, err := user_model.GetPossibleUserByID(ctx, issue.PosterID) + if err != nil { + if user_model.IsErrUserNotExist(err) { + return nil, nil + } + return nil, fmt.Errorf("getIssuePoster [%d]: %w", issue.PosterID, err) + } + issue.Poster = poster + return poster, nil +} + +func mustGetIssuePoster(ctx context.Context, issue *issues_model.Issue) (*user_model.User, error) { + poster, err := getIssuePoster(ctx, issue) + if err != nil { + return nil, err + } + if poster == nil { + return nil, user_model.ErrUserNotExist{UID: issue.PosterID} + } + return poster, nil +} + +type useHeadOrBaseCommit int + +const ( + useHeadCommit = 1 << iota + useBaseCommit +) + +func getPullRequestCommitAndApproval(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, event webhook_module.HookEventType) (useHeadOrBaseCommit, actions_model.ApprovalType, error) { + if pr == nil || actions_module.IsDefaultBranchWorkflow(event) || !pr.IsForkPullRequest() { + return useHeadCommit, actions_model.DoesNotNeedApproval, nil + } + + posterTrust, err := GetPullRequestPosterIsTrustedWithActions(ctx, pr) + if err != nil { + return useHeadCommit, actions_model.UndefinedApproval, err + } + + if posterTrust.IsTrusted() { + return useHeadCommit, actions_model.DoesNotNeedApproval, nil + } + + doerTrust, err := getPullRequestUserIsTrustedWithActions(ctx, pr, doer) + if err != nil { + return useHeadCommit, actions_model.UndefinedApproval, err + } + + if doerTrust.IsTrusted() { + if event == webhook_module.HookEventPullRequestSync { + // a synchronized event action (i.e. the doer pushed a commit to the pull request) + // can run from the head + return useHeadCommit, actions_model.DoesNotNeedApproval, nil + } + // other events run from workflows found in the base, not + // from possibly modified workflows found in the head + return useBaseCommit, actions_model.DoesNotNeedApproval, nil + } + // the poster and the doer are not trusted, approval is needed + return useHeadCommit, actions_model.NeedApproval, nil +} + +// cancels or approves runs and keep track of posters that are to always be trusted +func UpdateTrustedWithPullRequest(ctx context.Context, doerID int64, pr *issues_model.PullRequest, trusted TrustUpdate) error { + if err := loadPullRequestAttributes(ctx, pr); err != nil { + return err + } + + switch trusted { + case UserAlwaysTrusted: + poster, err := mustGetIssuePoster(ctx, pr.Issue) + if err != nil { + return err + } + return AlwaysTrust(ctx, doerID, pr.Issue.RepoID, poster.ID) + case UserTrustedOnce: + return pullRequestApprove(ctx, doerID, pr.Issue.RepoID, pr.ID) + case UserTrustRevoked: + poster, err := mustGetIssuePoster(ctx, pr.Issue) + if err != nil { + return err + } + return RevokeTrust(ctx, pr.Issue.RepoID, poster.ID) + case UserTrustDenied: + return pullRequestCancel(ctx, pr.Issue.RepoID, pr.ID) + default: + return fmt.Errorf("UpdateTrustedWithPullRequest: unknown trust %v", trusted) + } +} + +func setRunTrustForPullRequest(ctx context.Context, run *actions_model.ActionRun, pr *issues_model.PullRequest, needApproval actions_model.ApprovalType) error { + if pr == nil { + return nil + } + + if err := loadPullRequestAttributes(ctx, pr); err != nil { + return err + } + + run.IsForkPullRequest = pr.IsForkPullRequest() + run.PullRequestPosterID = pr.Issue.PosterID + run.PullRequestID = pr.ID + run.NeedApproval = bool(needApproval) + + return nil +} + +type UserTrust string + +const ( + UserTrustIsNotRelevant = UserTrust("irrelevant") + UserIsNotTrustedWithActions = UserTrust("no") + UserIsExplicitlyTrustedWithActions = UserTrust("explicitly") + UserIsImplicitlyTrustedWithActions = UserTrust("implicitly") +) + +func (t UserTrust) IsTrusted() bool { + return t != UserIsNotTrustedWithActions +} + +func GetPullRequestPosterIsTrustedWithActions(ctx context.Context, pr *issues_model.PullRequest) (UserTrust, error) { + if err := loadPullRequestAttributes(ctx, pr); err != nil { + return "", err + } + poster, err := mustGetIssuePoster(ctx, pr.Issue) + if err != nil { + return UserIsNotTrustedWithActions, err + } + + return getPullRequestUserIsTrustedWithActions(ctx, pr, poster) +} + +func getPullRequestUserIsTrustedWithActions(ctx context.Context, pr *issues_model.PullRequest, user *user_model.User) (UserTrust, error) { + if err := loadPullRequestAttributes(ctx, pr); err != nil { + return "", err + } + + return userIsTrustedWithPullRequest(ctx, pr, user) +} + +func userIsTrustedWithPullRequest(ctx context.Context, pr *issues_model.PullRequest, user *user_model.User) (UserTrust, error) { + implicitlyTrusted, err := userIsImplicitlyTrustedWithPullRequest(ctx, pr, user) + if err != nil { + return "", err + } + if implicitlyTrusted { + log.Trace("%s is implicitly trusted to run actions in repository %s", user, pr.Issue.Repo) + return UserIsImplicitlyTrustedWithActions, nil + } + + explicitlyTrusted, err := userIsExplicitlyTrustedWithPullRequest(ctx, pr, user) + if err != nil { + return "", err + } + if explicitlyTrusted { + log.Trace("%s is explicitly trusted to run actions in repository %s", user, pr.Issue.Repo) + return UserIsExplicitlyTrustedWithActions, nil + } + + log.Trace("%s is not trusted to run actions in repository %s", user, pr.Issue.Repo) + return UserIsNotTrustedWithActions, nil +} + +func userIsImplicitlyTrustedWithPullRequest(ctx context.Context, pr *issues_model.PullRequest, user *user_model.User) (bool, error) { + // users that are trusted to create a pull request that is not from a fork + // are also implicitly trusted to run workflows + if !pr.IsForkPullRequest() { + log.Trace("a pull request that is not from a fork nor AGit is implicitly trusted to run actions") + return true, nil + } + + return userCanWriteActionsOnRepo(ctx, pr.Issue.Repo, user) +} + +func userCanWriteActionsOnRepo(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (bool, error) { + // users with write permission to the actions unit are trusted to + // run actions + permission, err := access_model.GetUserRepoPermission(ctx, repo, user) + if err != nil { + return false, err + } + if permission.CanWrite(unit_model.TypeActions) { + log.Trace("%s has write permissions to the Action unit on %s", user, repo) + return true, nil + } + + return false, nil +} + +func userIsExplicitlyTrustedWithPullRequest(ctx context.Context, pr *issues_model.PullRequest, user *user_model.User) (bool, error) { + // there is no need to check if the user is blocked because it is not + // allowed to create a pull request + if user.IsRestricted { + log.Trace("%v is restricted and cannot be trusted with pull requests", user) + return false, nil + } + + actionUser, err := actions_model.GetActionUserByUserIDAndRepoIDAndUpdateAccess(ctx, user.ID, pr.Issue.Repo.ID) + if err != nil { + log.Trace("%v is not explicitly trusted with pull requests on repository %v", user, pr.Issue.Repo) + if actions_model.IsErrUserNotExist(err) { + return false, nil + } + return false, err + } + + log.Trace("%v is explicitly trusted with pull requests on repository %v", user, pr.Issue.Repo) + return actionUser.TrustedWithPullRequests, nil +} + +func RevokeTrust(ctx context.Context, repoID, posterID int64) error { + if err := actions_model.DeleteActionUserByUserIDAndRepoID(ctx, posterID, repoID); err != nil { + return err + } + + runs, err := actions_model.GetRunsNotDoneByRepoIDAndPullRequestPosterID(ctx, repoID, posterID) + if err != nil { + return err + } + + for _, run := range runs { + if err := CancelRun(ctx, run); err != nil { + return err + } + } + return nil +} + +func AlwaysTrust(ctx context.Context, doerID, repoID, posterID int64) error { + if err := actions_model.InsertActionUser(ctx, &actions_model.ActionUser{ + UserID: posterID, + RepoID: repoID, + TrustedWithPullRequests: true, + }); err != nil { + return err + } + + runs, err := actions_model.GetRunsNotDoneByRepoIDAndPullRequestPosterID(ctx, repoID, posterID) + if err != nil { + return err + } + + for _, run := range runs { + if err := ApproveRun(ctx, run, doerID); err != nil { + return err + } + } + return nil +} + +func pullRequestCancel(ctx context.Context, repoID, pullRequestID int64) error { + runs, err := actions_model.GetRunsNotDoneByRepoIDAndPullRequestID(ctx, repoID, pullRequestID) + if err != nil { + return err + } + + for _, run := range runs { + if err := CancelRun(ctx, run); err != nil { + return err + } + } + return nil +} + +func pullRequestApprove(ctx context.Context, doerID, repoID, pullRequestID int64) error { + runs, err := actions_model.GetRunsThatNeedApprovalByRepoIDAndPullRequestID(ctx, repoID, pullRequestID) + if err != nil { + return err + } + + for _, run := range runs { + if err := ApproveRun(ctx, run, doerID); err != nil { + return err + } + } + return nil +} diff --git a/services/actions/trust_test.go b/services/actions/trust_test.go new file mode 100644 index 0000000000..d5aa08e35f --- /dev/null +++ b/services/actions/trust_test.go @@ -0,0 +1,377 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package actions + +import ( + "testing" + + actions_model "forgejo.org/models/actions" + issues_model "forgejo.org/models/issues" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + actions_module "forgejo.org/modules/actions" + webhook_module "forgejo.org/modules/webhook" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestActionsTrust_ChangeStatus(t *testing.T) { + require.NoError(t, unittest.PrepareTestDatabase()) + + repoID := int64(10) + pullRequestPosterID := int64(30) + + runDone := &actions_model.ActionRun{ + RepoID: repoID, + PullRequestPosterID: pullRequestPosterID, + Status: actions_model.StatusSuccess, + } + require.NoError(t, actions_model.InsertRun(t.Context(), runDone, nil)) + + runNotByPoster := &actions_model.ActionRun{ + RepoID: repoID, + PullRequestPosterID: 43243, + Status: actions_model.StatusRunning, + } + require.NoError(t, actions_model.InsertRun(t.Context(), runNotByPoster, nil)) + + runNotInTheSameRepository := &actions_model.ActionRun{ + RepoID: 5, + PullRequestPosterID: pullRequestPosterID, + Status: actions_model.StatusSuccess, + } + require.NoError(t, actions_model.InsertRun(t.Context(), runNotInTheSameRepository, nil)) + + t.Run("RevokeTrust", func(t *testing.T) { + singleWorkflows, err := actions_module.JobParser([]byte(` +jobs: + job: + runs-on: docker + steps: + - run: echo OK +`)) + require.NoError(t, err) + require.Len(t, singleWorkflows, 1) + runNotDone := &actions_model.ActionRun{ + TriggerUserID: 2, + RepoID: repoID, + Status: actions_model.StatusWaiting, + PullRequestPosterID: pullRequestPosterID, + } + require.NoError(t, actions_model.InsertRun(t.Context(), runNotDone, singleWorkflows)) + require.NoError(t, actions_model.InsertActionUser(t.Context(), &actions_model.ActionUser{ + UserID: pullRequestPosterID, + RepoID: repoID, + TrustedWithPullRequests: true, + })) + + previousCancelledCount := unittest.GetCount(t, &actions_model.ActionRun{Status: actions_model.StatusCancelled}) + _, err = actions_model.GetActionUserByUserIDAndRepoIDAndUpdateAccess(t.Context(), pullRequestPosterID, repoID) + require.NoError(t, err) + + require.NoError(t, RevokeTrust(t.Context(), repoID, pullRequestPosterID)) + + _, err = actions_model.GetActionUserByUserIDAndRepoIDAndUpdateAccess(t.Context(), pullRequestPosterID, repoID) + assert.True(t, actions_model.IsErrUserNotExist(err)) + currentCancelledCount := unittest.GetCount(t, &actions_model.ActionRun{Status: actions_model.StatusCancelled}) + assert.Equal(t, previousCancelledCount+1, currentCancelledCount) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: runNotDone.ID}) + assert.Equal(t, actions_model.StatusCancelled.String(), run.Status.String()) + }) + + createPullRequestRun := func(t *testing.T, pullRequestID, repoID int64) *actions_model.ActionRun { + t.Helper() + singleWorkflows, err := actions_module.JobParser([]byte(` +jobs: + job: + runs-on: docker + steps: + - run: echo OK +`)) + require.NoError(t, err) + require.Len(t, singleWorkflows, 1) + runNotApproved := &actions_model.ActionRun{ + TriggerUserID: 2, + RepoID: repoID, + Status: actions_model.StatusWaiting, + NeedApproval: true, + PullRequestID: pullRequestID, + PullRequestPosterID: pullRequestPosterID, + } + require.NoError(t, actions_model.InsertRun(t.Context(), runNotApproved, singleWorkflows)) + return runNotApproved + } + + t.Run("PullRequestCancel", func(t *testing.T) { + pullRequestID := int64(485) + runNotApproved := createPullRequestRun(t, pullRequestID, repoID) + + previousCancelledCount := unittest.GetCount(t, &actions_model.ActionRun{Status: actions_model.StatusCancelled}) + + require.NoError(t, pullRequestCancel(t.Context(), repoID, pullRequestID)) + + currentCancelledCount := unittest.GetCount(t, &actions_model.ActionRun{Status: actions_model.StatusCancelled}) + assert.Equal(t, previousCancelledCount+1, currentCancelledCount) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: runNotApproved.ID}) + assert.Equal(t, actions_model.StatusCancelled.String(), run.Status.String()) + }) + + t.Run("UpdateTrustedWithPullRequest deny", func(t *testing.T) { + pullRequestID := int64(485) + runNotApproved := createPullRequestRun(t, pullRequestID, repoID) + + previousCancelledCount := unittest.GetCount(t, &actions_model.ActionRun{Status: actions_model.StatusCancelled}) + + require.NoError(t, UpdateTrustedWithPullRequest(t.Context(), 0, &issues_model.PullRequest{ + ID: pullRequestID, + Issue: &issues_model.Issue{ + RepoID: repoID, + }, + }, UserTrustDenied)) + + currentCancelledCount := unittest.GetCount(t, &actions_model.ActionRun{Status: actions_model.StatusCancelled}) + assert.Equal(t, previousCancelledCount+1, currentCancelledCount) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: runNotApproved.ID}) + assert.Equal(t, actions_model.StatusCancelled.String(), run.Status.String()) + }) + + t.Run("PullRequestApprove", func(t *testing.T) { + pullRequestID := int64(534) + runNotApproved := createPullRequestRun(t, pullRequestID, repoID) + + previousWaitingCount := unittest.GetCount(t, &actions_model.ActionRunJob{Status: actions_model.StatusWaiting}) + + doerID := int64(84322) + require.NoError(t, pullRequestApprove(t.Context(), doerID, repoID, pullRequestID)) + + currentWaitingCount := unittest.GetCount(t, &actions_model.ActionRunJob{Status: actions_model.StatusWaiting}) + assert.Equal(t, previousWaitingCount+1, currentWaitingCount) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: runNotApproved.ID}) + assert.Equal(t, actions_model.StatusWaiting.String(), run.Status.String()) + assert.Equal(t, doerID, run.ApprovedBy) + assert.False(t, run.NeedApproval) + }) + + t.Run("UpdateTrustedWithPullRequest once", func(t *testing.T) { + pullRequestID := int64(534) + runNotApproved := createPullRequestRun(t, pullRequestID, repoID) + + previousWaitingCount := unittest.GetCount(t, &actions_model.ActionRunJob{Status: actions_model.StatusWaiting}) + + doerID := int64(84322) + require.NoError(t, UpdateTrustedWithPullRequest(t.Context(), doerID, &issues_model.PullRequest{ + ID: pullRequestID, + Issue: &issues_model.Issue{ + RepoID: repoID, + }, + }, UserTrustedOnce)) + + currentWaitingCount := unittest.GetCount(t, &actions_model.ActionRunJob{Status: actions_model.StatusWaiting}) + assert.Equal(t, previousWaitingCount+1, currentWaitingCount) + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: runNotApproved.ID}) + assert.Equal(t, actions_model.StatusWaiting.String(), run.Status.String()) + assert.Equal(t, doerID, run.ApprovedBy) + assert.False(t, run.NeedApproval) + }) + + t.Run("UpdateTrustedWithPullRequest always", func(t *testing.T) { + pullRequestIDs := []int64{534, 645} + var runsNotApproved []*actions_model.ActionRun + for _, pullRequestID := range pullRequestIDs { + runsNotApproved = append(runsNotApproved, createPullRequestRun(t, pullRequestID, repoID)) + } + + previousWaitingCount := unittest.GetCount(t, &actions_model.ActionRunJob{Status: actions_model.StatusWaiting}) + + doerID := int64(84322) + require.NoError(t, UpdateTrustedWithPullRequest(t.Context(), doerID, &issues_model.PullRequest{ + ID: pullRequestIDs[0], + Issue: &issues_model.Issue{ + RepoID: repoID, + PosterID: pullRequestPosterID, + }, + }, UserAlwaysTrusted)) + + currentWaitingCount := unittest.GetCount(t, &actions_model.ActionRunJob{Status: actions_model.StatusWaiting}) + assert.Equal(t, previousWaitingCount+len(pullRequestIDs), currentWaitingCount) + + for _, run := range runsNotApproved { + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run.ID}) + assert.Equal(t, actions_model.StatusWaiting.String(), run.Status.String()) + assert.Equal(t, doerID, run.ApprovedBy) + assert.False(t, run.NeedApproval) + } + }) + + t.Run("UpdateTrustedWithPullRequest revoke", func(t *testing.T) { + pullRequestIDs := []int64{748, 953} + var runsNotApproved []*actions_model.ActionRun + for _, pullRequestID := range pullRequestIDs { + runsNotApproved = append(runsNotApproved, createPullRequestRun(t, pullRequestID, repoID)) + } + + doerID := int64(84322) + require.NoError(t, UpdateTrustedWithPullRequest(t.Context(), doerID, &issues_model.PullRequest{ + ID: pullRequestIDs[0], + Issue: &issues_model.Issue{ + RepoID: repoID, + PosterID: pullRequestPosterID, + }, + }, UserTrustRevoked)) + + for _, run := range runsNotApproved { + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: run.ID}) + assert.Equal(t, actions_model.StatusCancelled.String(), run.Status.String()) + assert.False(t, run.NeedApproval) + } + }) +} + +func TestActionsTrust_GetPullRequestUserIsTrustedWithActions(t *testing.T) { + defer unittest.OverrideFixtures("services/actions/TestActionsTrust_GetPullRequestUserIsTrustedWithActions")() + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("implicitly trusted because the pull request is not from a fork", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2000}) + trust, err := GetPullRequestPosterIsTrustedWithActions(t.Context(), pr) + require.NoError(t, err) + require.False(t, pr.IsForkPullRequest()) + assert.Equal(t, UserIsImplicitlyTrustedWithActions, trust) + }) + + t.Run("implicitly trusted on a forked pull request when the poster is admin", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3000}) + trust, err := GetPullRequestPosterIsTrustedWithActions(t.Context(), pr) + require.NoError(t, err) + require.True(t, pr.IsForkPullRequest()) + require.True(t, pr.Issue.Poster.IsAdmin) + assert.Equal(t, UserIsImplicitlyTrustedWithActions, trust) + }) + + t.Run("explicitly trusted on a forked pull request when the poster was permanently approved", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1000}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) // regular user + trust, err := GetPullRequestPosterIsTrustedWithActions(t.Context(), pr) + require.NoError(t, err) + require.True(t, pr.IsForkPullRequest()) + _, err = actions_model.GetActionUserByUserIDAndRepoIDAndUpdateAccess(t.Context(), user.ID, pr.Issue.RepoID) + require.NoError(t, err) + assert.Equal(t, UserIsExplicitlyTrustedWithActions, trust) + }) + + t.Run("not trusted because on a forked pull request when the user has has no privileges", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 4000}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) // regular user + trust, err := GetPullRequestPosterIsTrustedWithActions(t.Context(), pr) + require.NoError(t, err) + assert.Equal(t, user.ID, pr.Issue.PosterID) + require.True(t, pr.IsForkPullRequest()) + assert.Equal(t, UserIsNotTrustedWithActions, trust) + }) + + t.Run("not trusted on a forked pull request because the user is restricted", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 5000}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 29}) // restricted user + trust, err := getPullRequestUserIsTrustedWithActions(t.Context(), pr, user) + require.NoError(t, err) + assert.Equal(t, user.ID, pr.Issue.PosterID) + require.True(t, pr.IsForkPullRequest()) + _, err = actions_model.GetActionUserByUserIDAndRepoIDAndUpdateAccess(t.Context(), user.ID, pr.Issue.RepoID) + require.NoError(t, err) + require.True(t, user.IsRestricted) + assert.Equal(t, UserIsNotTrustedWithActions, trust) + }) + + t.Run("approval not needed because the pr is not from a fork", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 2000}) + useCommit, approval, err := getPullRequestCommitAndApproval(t.Context(), pr, nil, webhook_module.HookEventPullRequest) + require.NoError(t, err) + assert.Equal(t, actions_model.DoesNotNeedApproval, approval) + assert.EqualValues(t, useHeadCommit, useCommit) + }) + + t.Run("approval not needed because the event is known to run out of the default branch", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3000}) + useCommit, approval, err := getPullRequestCommitAndApproval(t.Context(), pr, nil, webhook_module.HookEventPullRequestComment) + require.NoError(t, err) + assert.Equal(t, actions_model.DoesNotNeedApproval, approval) + assert.EqualValues(t, useHeadCommit, useCommit) + }) + + t.Run("approval not needed because it is not a pr", func(t *testing.T) { + useCommit, approval, err := getPullRequestCommitAndApproval(t.Context(), nil, nil, webhook_module.HookEventPullRequestComment) + require.NoError(t, err) + assert.Equal(t, actions_model.DoesNotNeedApproval, approval) + assert.EqualValues(t, useHeadCommit, useCommit) + }) + + t.Run("approval not needed for a forked pr because the poster is trusted", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 3000}) + useCommit, approval, err := getPullRequestCommitAndApproval(t.Context(), pr, nil, webhook_module.HookEventPullRequestSync) + require.NoError(t, err) + require.True(t, pr.Issue.Poster.IsAdmin) + assert.Equal(t, actions_model.DoesNotNeedApproval, approval) + assert.EqualValues(t, useHeadCommit, useCommit) + }) + + t.Run("approval needed for a forked pr because the poster and the doer are not trusted", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 4000}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) // regular user + useCommit, approval, err := getPullRequestCommitAndApproval(t.Context(), pr, doer, webhook_module.HookEventPullRequestSync) + require.NoError(t, err) + posterTrust, err := GetPullRequestPosterIsTrustedWithActions(t.Context(), pr) + require.NoError(t, err) + require.Equal(t, UserIsNotTrustedWithActions, posterTrust) + doerTrust, err := getPullRequestUserIsTrustedWithActions(t.Context(), pr, doer) + require.NoError(t, err) + require.Equal(t, UserIsNotTrustedWithActions, doerTrust) + assert.Equal(t, actions_model.NeedApproval, approval) + assert.EqualValues(t, useHeadCommit, useCommit) + }) + + t.Run("approval not needed for a forked pr because the doer is trusted and runs from the base", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 4000}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) // admin + useCommit, approval, err := getPullRequestCommitAndApproval(t.Context(), pr, doer, webhook_module.HookEventPullRequestLabel) + require.NoError(t, err) + posterTrust, err := GetPullRequestPosterIsTrustedWithActions(t.Context(), pr) + require.NoError(t, err) + require.Equal(t, UserIsNotTrustedWithActions, posterTrust) + doerTrust, err := getPullRequestUserIsTrustedWithActions(t.Context(), pr, doer) + require.NoError(t, err) + require.Equal(t, UserIsImplicitlyTrustedWithActions, doerTrust) + assert.Equal(t, actions_model.DoesNotNeedApproval, approval) + assert.EqualValues(t, useBaseCommit, useCommit) + }) + + t.Run("approval not needed for a forked pr because the doer is trusted and pushed new commits", func(t *testing.T) { + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 4000}) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) // admin + useCommit, approval, err := getPullRequestCommitAndApproval(t.Context(), pr, doer, webhook_module.HookEventPullRequestSync) + require.NoError(t, err) + posterTrust, err := GetPullRequestPosterIsTrustedWithActions(t.Context(), pr) + require.NoError(t, err) + require.Equal(t, UserIsNotTrustedWithActions, posterTrust) + doerTrust, err := getPullRequestUserIsTrustedWithActions(t.Context(), pr, doer) + require.NoError(t, err) + require.Equal(t, UserIsImplicitlyTrustedWithActions, doerTrust) + assert.Equal(t, actions_model.DoesNotNeedApproval, approval) + assert.EqualValues(t, useHeadCommit, useCommit) + }) + + t.Run("run for a pull request is set with info related to trust", func(t *testing.T) { + run := &actions_model.ActionRun{ + IsForkPullRequest: true, + } + pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 5000}) + needApproval := actions_model.NeedApproval + require.NoError(t, setRunTrustForPullRequest(t.Context(), run, nil, needApproval)) + require.NoError(t, setRunTrustForPullRequest(t.Context(), run, pr, needApproval)) + assert.True(t, run.NeedApproval) + assert.True(t, run.IsForkPullRequest) + assert.Equal(t, pr.Issue.PosterID, run.PullRequestPosterID) + assert.Equal(t, pr.ID, run.PullRequestID) + }) +} diff --git a/services/actions/workflows.go b/services/actions/workflows.go index 4c0a097452..1e4d92fa2f 100644 --- a/services/actions/workflows.go +++ b/services/actions/workflows.go @@ -155,7 +155,7 @@ func (entry *Workflow) Dispatch(ctx context.Context, inputGetter InputValueGette } } - jobs, err := jobParser(content, jobparser.WithVars(vars), jobparser.WithInputs(inputsAny)) + jobs, err := actions.JobParser(content, jobparser.WithVars(vars), jobparser.WithInputs(inputsAny)) if err != nil { return nil, nil, err } diff --git a/services/context/permission.go b/services/context/permission.go index b6af87f912..483758141c 100644 --- a/services/context/permission.go +++ b/services/context/permission.go @@ -111,6 +111,19 @@ func RequireRepoReaderOr(unitTypes ...unit.Type) func(ctx *Context) { } } +func RequireRepoDelegateActionTrust() func(ctx *Context) { + return func(ctx *Context) { + if CheckRepoDelegateActionTrust(ctx) { + return + } + ctx.NotFound(ctx.Req.URL.RequestURI(), nil) + } +} + +func CheckRepoDelegateActionTrust(ctx *Context) bool { + return ctx.Repo.IsAdmin() || (ctx.IsSigned && ctx.Doer.IsAdmin) || ctx.Repo.CanWrite(unit.TypeActions) +} + // CheckRepoScopedToken check whether personal access token has repo scope func CheckRepoScopedToken(ctx *Context, repo *repo_model.Repository, level auth_model.AccessTokenScopeLevel) { if !ctx.IsBasicAuth || ctx.Data["IsApiToken"] != true { diff --git a/services/cron/tasks_actions.go b/services/cron/tasks_actions.go index 2cd484fa69..25c16bdd8d 100644 --- a/services/cron/tasks_actions.go +++ b/services/cron/tasks_actions.go @@ -22,6 +22,7 @@ func initActionsTasks() { registerScheduleTasks() registerActionsCleanup() registerOfflineRunnersCleanup() + registerCleanupActionUser() } func registerStopZombieTasks() { @@ -95,3 +96,13 @@ func registerOfflineRunnersCleanup() { ) }) } + +func registerCleanupActionUser() { + RegisterTaskFatal("actions_action_user", &BaseConfig{ + Enabled: true, + RunAtStart: true, + Schedule: "@weekly", + }, func(ctx context.Context, _ *user_model.User, _ Config) error { + return actions_service.CleanupActionUser(ctx) + }) +} diff --git a/services/repository/delete.go b/services/repository/delete.go index a9ef4d8515..9eca085da3 100644 --- a/services/repository/delete.go +++ b/services/repository/delete.go @@ -177,6 +177,7 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID &actions_model.ActionScheduleSpec{RepoID: repoID}, &actions_model.ActionSchedule{RepoID: repoID}, &actions_model.ActionArtifact{RepoID: repoID}, + &actions_model.ActionUser{RepoID: repoID}, &repo_model.RepoArchiveDownloadCount{RepoID: repoID}, &actions_model.ActionRunnerToken{RepoID: repoID}, ); err != nil { diff --git a/services/user/block.go b/services/user/block.go index 6be8dc5f70..6182df7ca7 100644 --- a/services/user/block.go +++ b/services/user/block.go @@ -9,6 +9,7 @@ import ( "forgejo.org/models/db" repo_model "forgejo.org/models/repo" user_model "forgejo.org/models/user" + actions_service "forgejo.org/services/actions" "xorm.io/builder" ) @@ -91,5 +92,12 @@ func BlockUser(ctx context.Context, userID, blockID int64) error { return err } + err = db.Iterate(ctx, builder.Eq{"owner_id": userID}, func(ctx context.Context, repo *repo_model.Repository) error { + return actions_service.RevokeTrust(ctx, repo.ID, blockID) + }) + if err != nil { + return err + } + return committer.Commit() } diff --git a/services/user/block_test.go b/services/user/block_test.go index a2ba5d71a7..b9f23a7cda 100644 --- a/services/user/block_test.go +++ b/services/user/block_test.go @@ -7,11 +7,13 @@ import ( "testing" model "forgejo.org/models" + actions_model "forgejo.org/models/actions" "forgejo.org/models/db" issues_model "forgejo.org/models/issues" repo_model "forgejo.org/models/repo" "forgejo.org/models/unittest" user_model "forgejo.org/models/user" + actions_module "forgejo.org/modules/actions" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -110,4 +112,37 @@ func TestBlockUser(t *testing.T) { _, err = issues_model.ChangeIssueStatus(db.DefaultContext, issue, blockedUser, false) require.Error(t, err) }) + + t.Run("Pull requests actions are cancelled", func(t *testing.T) { + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2, OwnerID: doer.ID}) + blockedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 4}) + defer user_model.UnblockUser(db.DefaultContext, doer.ID, blockedUser.ID) + + pullRequestPosterID := blockedUser.ID + singleWorkflows, err := actions_module.JobParser([]byte(` +jobs: + job: + runs-on: docker + steps: + - run: echo OK +`)) + require.NoError(t, err) + require.Len(t, singleWorkflows, 1) + runWaiting := &actions_model.ActionRun{ + TriggerUserID: 2, + RepoID: repo.ID, + Status: actions_model.StatusWaiting, + PullRequestPosterID: pullRequestPosterID, + } + require.NoError(t, actions_model.InsertRun(t.Context(), runWaiting, singleWorkflows)) + + run := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: runWaiting.ID}) + require.Equal(t, actions_model.StatusWaiting.String(), run.Status.String()) + + require.NoError(t, BlockUser(db.DefaultContext, doer.ID, blockedUser.ID)) + + run = unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{ID: runWaiting.ID}) + require.Equal(t, actions_model.StatusCancelled.String(), run.Status.String()) + }) } diff --git a/services/user/delete.go b/services/user/delete.go index 2edc95ab58..0c2a0b63ed 100644 --- a/services/user/delete.go +++ b/services/user/delete.go @@ -94,6 +94,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) &pull_model.ReviewState{UserID: u.ID}, &user_model.Redirect{RedirectUserID: u.ID}, &actions_model.ActionRunner{OwnerID: u.ID}, + &actions_model.ActionUser{UserID: u.ID}, &user_model.BlockedUser{BlockID: u.ID}, &user_model.BlockedUser{UserID: u.ID}, &actions_model.ActionRunnerToken{OwnerID: u.ID}, diff --git a/templates/repo/actions/view.tmpl b/templates/repo/actions/view.tmpl index 5b8b44639e..6adc91417c 100644 --- a/templates/repo/actions/view.tmpl +++ b/templates/repo/actions/view.tmpl @@ -12,7 +12,7 @@ data-workflow-url="{{.WorkflowURL}}" data-initial-post-response="{{.InitialData}}" data-initial-artifacts-response="{{.InitialArtifactsData}}" - data-locale-approve="{{ctx.Locale.Tr "repo.diff.review.approve"}}" + data-locale-approve="{{ctx.Locale.Tr "repo.pulls.poster_manage_approval"}}" data-locale-cancel="{{ctx.Locale.Tr "cancel"}}" data-locale-rerun="{{ctx.Locale.Tr "rerun"}}" data-locale-rerun-all="{{ctx.Locale.Tr "rerun_all"}}" diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 101474d0ff..21f0d613ed 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -94,6 +94,7 @@ {{end}} + {{template "repo/pulls/trust" .}} {{template "repo/issue/view_content/update_branch_by_merge" $}} {{else if .Issue.PullRequest.IsChecking}}
@@ -187,6 +188,7 @@
{{end}} {{end}} + {{template "repo/pulls/trust" .}} {{template "repo/issue/view_content/update_branch_by_merge" $}} {{if .Issue.PullRequest.IsEmpty}}
diff --git a/templates/repo/pulls/trust.tmpl b/templates/repo/pulls/trust.tmpl new file mode 100644 index 0000000000..f715f27bbf --- /dev/null +++ b/templates/repo/pulls/trust.tmpl @@ -0,0 +1,61 @@ +{{/* +Template Attributes: +* CanReadUnitActions: true if the actions unit is active and readable +* SomePullRequestRunsNeedApproval: true if there is at least one run waiting for approval +* UserCanDelegateTrustWithPullRequest: true if the user can delegate trust in the context of pull requests +* PullRequestPosterIsNotTrustedWithActions: true if the poster of the pull request needs to be approved to run actions +* PullRequestPosterIsExplicitlyTrustedWithActions: true if the poster of the pull request is trusted to run actions (once or always) +* PullRequestPosterIsImplicitlyTrustedWithActions: true if the poster of the pull request is trusted to run actions because of elevated permissions +* Link: URL to the pull request +*/}} + +{{if .CanReadUnitActions}} + {{if and .UserCanDelegateTrustWithPullRequest .PullRequestPosterIsExplicitlyTrustedWithActions}} +
+
+
+
+ {{ctx.Locale.Tr "repo.pulls.poster_is_trusted" "https://forgejo.org/docs/latest/user/actions/security-pull-request/"}} +
+
+
+ + +
+
+
+
+ {{else if and .PullRequestPosterIsNotTrustedWithActions .SomePullRequestRunsNeedApproval}} +
+
+
+
+ {{svg "octicon-alert" 16 "text red"}} + {{ctx.Locale.Tr "repo.pulls.poster_requires_approval" "https://forgejo.org/docs/latest/user/actions/security-pull-request/"}} +
+ {{if .UserCanDelegateTrustWithPullRequest}} +
+
+
+ + +
+
+
+
+ + +
+
+
+
+ + +
+
+
+ {{end}} +
+
+ {{end}} +{{end}} diff --git a/tests/integration/actions_commit_status_test.go b/tests/integration/actions_commit_status_test.go index 05de90e5ab..6965832ee1 100644 --- a/tests/integration/actions_commit_status_test.go +++ b/tests/integration/actions_commit_status_test.go @@ -1,4 +1,4 @@ -// Copyright 20124 The Forgejo Authors. All rights reserved. +// Copyright 2024 The Forgejo Authors. All rights reserved. // SPDX-License-Identifier: MIT package integration diff --git a/tests/integration/actions_trust_test.go b/tests/integration/actions_trust_test.go new file mode 100644 index 0000000000..7a683d5625 --- /dev/null +++ b/tests/integration/actions_trust_test.go @@ -0,0 +1,355 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package integration + +import ( + "fmt" + "net/http" + "net/url" + "strings" + "testing" + "time" + + actions_model "forgejo.org/models/actions" + issues_model "forgejo.org/models/issues" + repo_model "forgejo.org/models/repo" + unit_model "forgejo.org/models/unit" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + actions_module "forgejo.org/modules/actions" + "forgejo.org/modules/git" + "forgejo.org/modules/structs" + actions_service "forgejo.org/services/actions" + pull_service "forgejo.org/services/pull" + repo_service "forgejo.org/services/repository" + files_service "forgejo.org/services/repository/files" + "forgejo.org/tests" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func actionsTrustTestClickTrustPanel(t *testing.T, session *TestSession, url, trust string) { + // an admin approves the run once + req := NewRequest(t, "GET", url) + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + htmlDoc.AssertElement(t, "#pull-request-trust-panel", true) + link, exists := htmlDoc.doc.Find("#pull-request-trust-panel-" + trust).Attr("action") + require.True(t, exists) + actualTrust, exists := htmlDoc.doc.Find(fmt.Sprintf("#pull-request-trust-panel-%s input[name='trust']", trust)).Attr("value") + require.True(t, exists) + require.Equal(t, trust, actualTrust) + req = NewRequestWithValues(t, "POST", link, map[string]string{ + "trust": trust, + }) + session.MakeRequest(t, req, http.StatusSeeOther) +} + +func actionsTrustTestAssertTrustPanelPresence(t *testing.T, session *TestSession, url string, present bool) { + t.Helper() + req := NewRequest(t, "GET", url) + resp := session.MakeRequest(t, req, http.StatusOK) + htmlDoc := NewHTMLParser(t, resp.Body) + + htmlDoc.AssertElement(t, ".error-code", false) + htmlDoc.AssertElement(t, "#pull-request-trust-panel", present) +} + +func actionsTrustTestAssertTrustPanel(t *testing.T, session *TestSession, url string) { + t.Helper() + actionsTrustTestAssertTrustPanelPresence(t, session, url, true) +} + +func actionsTrustTestAssertNoTrustPanel(t *testing.T, session *TestSession, url string) { + t.Helper() + actionsTrustTestAssertTrustPanelPresence(t, session, url, false) +} + +func actionsTrustTestCreateBaseRepo(t *testing.T, owner *user_model.User) (*repo_model.Repository, func()) { + t.Helper() + + // create the base repo + baseRepo, _, f := tests.CreateDeclarativeRepo(t, owner, "repo-pull-request", + []unit_model.Type{unit_model.TypeActions}, nil, nil, + ) + + // add workflow file to the base repo + addWorkflowToBaseResp, err := files_service.ChangeRepoFiles(git.DefaultContext, baseRepo, owner, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: ".forgejo/workflows/pr.yml", + ContentReader: strings.NewReader(` +on: + pull_request: + +jobs: + test: + runs-on: docker + steps: + - run: echo helloworld +`), + }, + }, + Message: "add workflow", + OldBranch: "main", + NewBranch: "main", + Author: &files_service.IdentityOptions{ + Name: owner.Name, + Email: owner.Email, + }, + Committer: &files_service.IdentityOptions{ + Name: owner.Name, + Email: owner.Email, + }, + Dates: &files_service.CommitDateOptions{ + Author: time.Now(), + Committer: time.Now(), + }, + }) + require.NoError(t, err) + require.NotEmpty(t, addWorkflowToBaseResp) + return baseRepo, f +} + +func actionsTrustTestRequireRun(t *testing.T, repo *repo_model.Repository, modifiedFiles *structs.FilesResponse) { + t.Helper() + + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: repo.ID, CommitSHA: modifiedFiles.Commit.SHA}) + require.Equal(t, actions_module.GithubEventPullRequest, actionRun.TriggerEvent) + require.Equal(t, actions_model.StatusWaiting.String(), actionRun.Status.String()) + unittest.BeanExists(t, &actions_model.ActionRunJob{RunID: actionRun.ID, RepoID: repo.ID}) +} + +func actionsTrustTestRepoCreateBranch(t *testing.T, doer *user_model.User, repo *repo_model.Repository) *structs.FilesResponse { + t.Helper() + + return actionsTrustTestModifyRepo(t, doer, repo, "file_in_fork.txt", "main", "fork-branch-1") +} + +func actionsTrustTestRepoModify(t *testing.T, doer *user_model.User, baseRepo, headRepo *repo_model.Repository, filename string) *structs.FilesResponse { + t.Helper() + + modified := actionsTrustTestModifyRepo(t, doer, headRepo, filename, "fork-branch-1", "fork-branch-1") + // the creation of the run is not synchronous + require.Eventually(t, func() bool { + return unittest.BeanExists(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: modified.Commit.SHA}) + }, 60*time.Second, time.Millisecond*100) + return modified +} + +func actionsTrustTestModifyRepo(t *testing.T, doer *user_model.User, repo *repo_model.Repository, filename, oldBranch, newBranch string) *structs.FilesResponse { + t.Helper() + + // add a new file to the forked repo + addFile, err := files_service.ChangeRepoFiles(git.DefaultContext, repo, doer, &files_service.ChangeRepoFilesOptions{ + Files: []*files_service.ChangeRepoFile{ + { + Operation: "create", + TreePath: filename, + ContentReader: strings.NewReader("content"), + }, + }, + Message: "add " + filename, + OldBranch: oldBranch, + NewBranch: newBranch, + Author: &files_service.IdentityOptions{ + Name: doer.Name, + Email: doer.Email, + }, + Committer: &files_service.IdentityOptions{ + Name: doer.Name, + Email: doer.Email, + }, + Dates: &files_service.CommitDateOptions{ + Author: time.Now(), + Committer: time.Now(), + }, + }) + require.NoError(t, err) + require.NotEmpty(t, addFile) + return addFile +} + +func actionsTrustTestCreatePullRequestFromForkedRepo(t *testing.T, baseUser *user_model.User, baseRepo *repo_model.Repository, headUser *user_model.User) (*repo_model.Repository, *issues_model.PullRequest, *structs.FilesResponse) { + t.Helper() + + forkRepo := func(t *testing.T, baseUser *user_model.User, baseRepo *repo_model.Repository, headUser *user_model.User) *repo_model.Repository { + t.Helper() + + // create the forked repo + forkedRepo, err := repo_service.ForkRepositoryAndUpdates(git.DefaultContext, baseUser, headUser, repo_service.ForkRepoOptions{ + BaseRepo: baseRepo, + Name: "forked-repo-pull-request", + Description: "test pull-request event", + }) + require.NoError(t, err) + require.NotEmpty(t, forkedRepo) + return forkedRepo + } + + forkedRepo := forkRepo(t, baseUser, baseRepo, headUser) + addFileToForkedResp := actionsTrustTestRepoCreateBranch(t, headUser, forkedRepo) + + // create Pull + pullIssue := &issues_model.Issue{ + RepoID: baseRepo.ID, + Title: "Test pull-request", + PosterID: headUser.ID, + Poster: headUser, + IsPull: true, + } + pullRequest := &issues_model.PullRequest{ + HeadRepoID: forkedRepo.ID, + BaseRepoID: baseRepo.ID, + HeadBranch: "fork-branch-1", + BaseBranch: "main", + HeadRepo: forkedRepo, + BaseRepo: baseRepo, + Type: issues_model.PullRequestGitea, + } + // create the pull request + err := pull_service.NewPullRequest(git.DefaultContext, baseRepo, pullIssue, nil, nil, pullRequest, nil) + require.NoError(t, err) + + actionsTrustTestRequireRun(t, baseRepo, addFileToForkedResp) + + return forkedRepo, pullRequest, addFileToForkedResp +} + +func TestActionsPullRequestTrustPanel(t *testing.T) { + onApplicationRun(t, func(t *testing.T, u *url.URL) { + ownerUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) // owner of the repo + + regularUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) // a regular user with no specific permission + regularSession := loginUser(t, regularUser.Name) + + userAdmin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) // the instance admin + adminSession := loginUser(t, userAdmin.Name) + + baseRepo, f := actionsTrustTestCreateBaseRepo(t, ownerUser) + defer f() + + forkedRepo, pullRequest, addFileToForkedResp := actionsTrustTestCreatePullRequestFromForkedRepo(t, ownerUser, baseRepo, regularUser) + pullRequestLink := pullRequest.Issue.Link() + + t.Run("Regular user sees a pending approval on a newly created pull request from a fork", func(t *testing.T) { + actionsTrustTestAssertTrustPanel(t, regularSession, pullRequestLink) + }) + + t.Run("Admin approves runs once", func(t *testing.T) { + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: addFileToForkedResp.Commit.SHA}) + + { + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: addFileToForkedResp.Commit.SHA}) + assert.True(t, actionRun.NeedApproval) + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: actionRun.ID, RepoID: baseRepo.ID}) + assert.Equal(t, actions_model.StatusBlocked.String(), actionRunJob.Status.String()) + } + + actionsTrustTestAssertTrustPanel(t, adminSession, pullRequestLink) + actionsTrustTestClickTrustPanel(t, adminSession, pullRequestLink, string(actions_service.UserTrustedOnce)) + + { + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: actionRun.ID, RepoID: baseRepo.ID}) + assert.Equal(t, actions_model.StatusWaiting.String(), actionRunJob.Status.String()) + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID}) + assert.False(t, actionRun.NeedApproval) + } + }) + + t.Run("All users sees no pending approval because it was approved once", func(t *testing.T) { + actionsTrustTestAssertNoTrustPanel(t, regularSession, pullRequestLink) + actionsTrustTestAssertNoTrustPanel(t, adminSession, pullRequestLink) + }) + + modifiedForkedResp := actionsTrustTestRepoModify(t, regularUser, baseRepo, forkedRepo, "add_file_one.txt") + + t.Run("Regular user sees a pending approval on a modified pull request from a fork (2)", func(t *testing.T) { + actionsTrustTestAssertTrustPanel(t, regularSession, pullRequestLink) + }) + + t.Run("Admin denies runs", func(t *testing.T) { + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: modifiedForkedResp.Commit.SHA}) + + { + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: modifiedForkedResp.Commit.SHA}) + assert.True(t, actionRun.NeedApproval) + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: actionRun.ID, RepoID: baseRepo.ID}) + assert.Equal(t, actions_model.StatusBlocked.String(), actionRunJob.Status.String()) + } + + actionsTrustTestAssertTrustPanel(t, adminSession, pullRequestLink) + actionsTrustTestClickTrustPanel(t, adminSession, pullRequestLink, string(actions_service.UserTrustDenied)) + + { + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: actionRun.ID, RepoID: baseRepo.ID}) + assert.Equal(t, actions_model.StatusCancelled.String(), actionRunJob.Status.String()) + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID}) + assert.False(t, actionRun.NeedApproval) + } + }) + + t.Run("All users sees no pending approval because it was denied", func(t *testing.T) { + actionsTrustTestAssertNoTrustPanel(t, regularSession, pullRequestLink) + actionsTrustTestAssertNoTrustPanel(t, adminSession, pullRequestLink) + }) + + modifiedForkedResp = actionsTrustTestRepoModify(t, regularUser, baseRepo, forkedRepo, "add_file_two.txt") + + t.Run("Regular user sees a pending approval on a modified pull request from a fork (2)", func(t *testing.T) { + actionsTrustTestAssertTrustPanel(t, regularSession, pullRequestLink) + }) + + t.Run("Admin always trusts the poster", func(t *testing.T) { + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: modifiedForkedResp.Commit.SHA}) + + { + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: modifiedForkedResp.Commit.SHA}) + assert.True(t, actionRun.NeedApproval) + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: actionRun.ID, RepoID: baseRepo.ID}) + assert.Equal(t, actions_model.StatusBlocked.String(), actionRunJob.Status.String()) + } + + actionsTrustTestAssertTrustPanel(t, adminSession, pullRequestLink) + actionsTrustTestClickTrustPanel(t, adminSession, pullRequestLink, string(actions_service.UserAlwaysTrusted)) + + { + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: actionRun.ID, RepoID: baseRepo.ID}) + assert.Equal(t, actions_model.StatusWaiting.String(), actionRunJob.Status.String()) + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID}) + assert.False(t, actionRun.NeedApproval) + } + }) + + t.Run("Regular users sees no pending approval because it was approved", func(t *testing.T) { + actionsTrustTestAssertNoTrustPanel(t, regularSession, pullRequestLink) + }) + + modifiedForkedResp = actionsTrustTestRepoModify(t, regularUser, baseRepo, forkedRepo, "add_file_three.txt") + + t.Run("No need for approval because the poster is always trusted", func(t *testing.T) { + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: modifiedForkedResp.Commit.SHA}) + assert.False(t, actionRun.NeedApproval) + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: actionRun.ID, RepoID: baseRepo.ID}) + assert.Equal(t, actions_model.StatusWaiting.String(), actionRunJob.Status.String()) + }) + + t.Run("Admin revokes the trusted poster", func(t *testing.T) { + actionsTrustTestAssertTrustPanel(t, adminSession, pullRequestLink) + actionsTrustTestClickTrustPanel(t, adminSession, pullRequestLink, string(actions_service.UserTrustRevoked)) + }) + + modifiedForkedResp = actionsTrustTestRepoModify(t, regularUser, baseRepo, forkedRepo, "add_file_four.txt") + + t.Run("There needs to be an approval again because the user is no longer trusted", func(t *testing.T) { + actionRun := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRun{RepoID: baseRepo.ID, CommitSHA: modifiedForkedResp.Commit.SHA}) + assert.True(t, actionRun.NeedApproval) + actionRunJob := unittest.AssertExistsAndLoadBean(t, &actions_model.ActionRunJob{RunID: actionRun.ID, RepoID: baseRepo.ID}) + assert.Equal(t, actions_model.StatusBlocked.String(), actionRunJob.Status.String()) + }) + }) +} diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index 5a61ef037b..b4faf9d775 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -357,11 +357,11 @@ func TestAPICron(t *testing.T) { AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) - assert.Equal(t, "29", resp.Header().Get("X-Total-Count")) + assert.Equal(t, "30", resp.Header().Get("X-Total-Count")) var crons []api.Cron DecodeJSON(t, resp, &crons) - assert.Len(t, crons, 29) + assert.Len(t, crons, 30) }) t.Run("Execute", func(t *testing.T) { diff --git a/web_src/js/components/RepoActionView.test.js b/web_src/js/components/RepoActionView.test.js index 6f575ca8da..b06751b8cf 100644 --- a/web_src/js/components/RepoActionView.test.js +++ b/web_src/js/components/RepoActionView.test.js @@ -397,6 +397,44 @@ test('historical attempt dropdown interactions', async () => { expect(window.location.href).toEqual(toAbsoluteUrl('/user1/repo2/actions/runs/123/jobs/1/attempt/2')); }); +test('run approval interaction', async () => { + const pullRequestLink = '/example-org/example-repo/pulls/456'; + const wrapper = mount(RepoActionView, { + props: { + ...defaultTestProps, + initialJobData: { + state: { + run: { + canApprove: true, + status: 'waiting', + commit: { + pusher: {}, + branch: { + link: toAbsoluteUrl(pullRequestLink), + }, + }, + }, + currentJob: { + steps: [ + { + summary: 'Test Job', + }, + ], + }, + }, + logs: { + stepsLog: [], + }, + }, + }, + }); + await flushPromises(); + const approve = wrapper.findAll('button').filter((button) => button.text() === 'Locale Approve'); + expect(approve.length).toEqual(1); + approve[0].trigger('click'); + expect(window.location.href).toEqual(toAbsoluteUrl(`${pullRequestLink}#pull-request-trust-panel`)); +}); + test('artifacts download links', async () => { Object.defineProperty(document.documentElement, 'lang', {value: 'en'}); vi.spyOn(global, 'fetch').mockImplementation((url, opts) => { diff --git a/web_src/js/components/RepoActionView.vue b/web_src/js/components/RepoActionView.vue index 3581e891fd..e8259abd86 100644 --- a/web_src/js/components/RepoActionView.vue +++ b/web_src/js/components/RepoActionView.vue @@ -223,7 +223,8 @@ export default { }, // approve a run approveRun() { - POST(`${this.run.link}/approve`); + const url = `${this.run.commit.branch.link}#pull-request-trust-panel`; + window.location.href = url; }, // show/hide the step logs for a group toggleGroupLogs(event) {