chore(refactor): replace ifNeedApproval with trust management

What previously handled by ifNeedApproval is replaced with
two calls implemented in trust.go:

- getPullRequestCommitAndApproval
  when workflows are collected and before runs are generated
  from them, figure out if
  - they need approval
  - they should run from the base or the head
- setRunTrustForPullRequest
  when a pull request run is created from a detected workflow,
  set the information it will need for trust management

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() {

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

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

services/actions/TestActionsNotifier_IsTrusted/repo_unit.yml

@@ -0,0 +1,6 @@
+-
+  id: 11500
+  repo_id: 10
+  type: 10 # TypeActions
+  config: "{}"
+  created_unix: 946684810

services/actions/notifier_helper.go

@@ -268,6 +268,19 @@ func detectWorkflows(ctx context.Context, input *notifyInput, gitRepo *git.Repos
 				}
 			}
 		}
+
+		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 {
@@ -342,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/<target-branch>/<topic-branch> 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)
@@ -384,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
@@ -500,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,

services/actions/notifier_helper_test.go

@@ -241,3 +241,27 @@ func TestActionsNotifier_PreExecutionEventDetectionError(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)
+}