From bf7c63a2aeb1e665bea8001f25ba0581dd0d3663 Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Thu, 30 Oct 2025 15:40:58 +0100 Subject: [PATCH] feat: add ActionUser model & fields to ActionRun ActionUser is to keep track of pull requests posters that are permanently trusted. It has a used field to track when it was last used so records can be expired instead of accumulating forever. ActionRun has new fields to make it possible to look them up given either the pull request ID or the poster ID. --- models/actions/run.go | 94 ++++++++--- models/actions/run_test.go | 119 ++++++++++++++ models/actions/user.go | 97 +++++++++++ models/actions/user_test.go | 107 +++++++++++++ models/fixtures/action_user.yml | 6 + .../v14a_actions-approval-and-trust.go | 104 ++++++++++++ .../v14a_actions-approval-and-trust_test.go | 103 ++++++++++++ .../action_run.yml | 151 ++++++++++++++++++ .../repository.yml | 5 + .../user.yml | 11 ++ services/repository/delete.go | 1 + services/user/delete.go | 1 + 12 files changed, 778 insertions(+), 21 deletions(-) create mode 100644 models/actions/user.go create mode 100644 models/actions/user_test.go create mode 100644 models/fixtures/action_user.yml create mode 100644 models/forgejo_migrations/v14a_actions-approval-and-trust.go create mode 100644 models/forgejo_migrations/v14a_actions-approval-and-trust_test.go create mode 100644 models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/action_run.yml create mode 100644 models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/repository.yml create mode 100644 models/gitea_migrations/fixtures/Test_v14ActionsApprovalAndTrustPopulateTableActionUser/user.yml diff --git a/models/actions/run.go b/models/actions/run.go index fe516d75d4..79a6683df9 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. 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/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/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},