Require approval to run actions for fork pull request (go-gitea#22803)

Currently, Gitea will run actions automatically which are triggered by
fork pull request. It's a security risk, people can create a PR and
modify the workflow yamls to execute a malicious script.

So we should require approval for first-time contributors, which is the
default strategy of a public repo on GitHub, see [Approving workflow
runs from public
forks](https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks).

Current strategy:

- don't need approval if it's not a fork PR;
- always need approval if the user is restricted;
- don't need approval if the user can write;
- don't need approval if the user has been approved before;
- otherwise, need approval.

https://user-images.githubusercontent.com/9418365/217207121-badf50a8-826c-4425-bef1-d82d1979bc81.mov

GitHub has an option for that, you can see that at
`/<owner>/<repo>/settings/actions`, and we can support that later.

<img width="835" alt="image"
src="https://user-images.githubusercontent.com/9418365/217199990-2967e68b-e693-4e59-8186-ab33a1314a16.png">

---------

Co-authored-by: Lunny Xiao <[email protected]>

Author: wolfogre

models/actions/run.go

@@ -32,11 +32,13 @@ type ActionRun struct {
 	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
-	TriggerUser       *user_model.User `xorm:"-"`
+	TriggerUserID     int64                  `xorm:"index"`
+	TriggerUser       *user_model.User       `xorm:"-"`
 	Ref               string
 	CommitSHA         string
 	IsForkPullRequest bool
+	NeedApproval      bool  // may need approval if it's a fork pull request
+	ApprovedBy        int64 `xorm:"index"` // who approved
 	Event             webhook_module.HookEventType
 	EventPayload      string `xorm:"LONGTEXT"`
 	Status            Status `xorm:"index"`
@@ -164,10 +166,6 @@ func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWork
 	}
 	run.Index = index
 
-	if run.Status.IsUnknown() {
-		run.Status = StatusWaiting
-	}
-
 	if err := db.Insert(ctx, run); err != nil {
 		return err
 	}
@@ -191,7 +189,7 @@ func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWork
 		job.EraseNeeds()
 		payload, _ := v.Marshal()
 		status := StatusWaiting
-		if len(needs) > 0 {
+		if len(needs) > 0 || run.NeedApproval {
 			status = StatusBlocked
 		}
 		runJobs = append(runJobs, &ActionRunJob{

models/actions/run_list.go

@@ -68,6 +68,8 @@ type FindRunOptions struct {
 	OwnerID          int64
 	IsClosed         util.OptionalBool
 	WorkflowFileName string
+	TriggerUserID    int64
+	Approved         bool // not util.OptionalBool, it works only when it's true
 }
 
 func (opts FindRunOptions) toConds() builder.Cond {
@@ -89,6 +91,12 @@ func (opts FindRunOptions) toConds() builder.Cond {
 	if opts.WorkflowFileName != "" {
 		cond = cond.And(builder.Eq{"workflow_id": opts.WorkflowFileName})
 	}
+	if opts.TriggerUserID > 0 {
+		cond = cond.And(builder.Eq{"trigger_user_id": opts.TriggerUserID})
+	}
+	if opts.Approved {
+		cond = cond.And(builder.Gt{"approved_by": 0})
+	}
 	return cond
 }
 

models/actions/status.go

@@ -82,6 +82,10 @@ func (s Status) IsRunning() bool {
 	return s == StatusRunning
 }
 
+func (s Status) IsBlocked() bool {
+	return s == StatusBlocked
+}
+
 // In returns whether s is one of the given statuses
 func (s Status) In(statuses ...Status) bool {
 	for _, v := range statuses {

models/migrations/migrations.go

@@ -19,6 +19,7 @@ import (
 	"code.gitea.io/gitea/models/migrations/v1_17"
 	"code.gitea.io/gitea/models/migrations/v1_18"
 	"code.gitea.io/gitea/models/migrations/v1_19"
+	"code.gitea.io/gitea/models/migrations/v1_20"
 	"code.gitea.io/gitea/models/migrations/v1_6"
 	"code.gitea.io/gitea/models/migrations/v1_7"
 	"code.gitea.io/gitea/models/migrations/v1_8"
@@ -463,6 +464,9 @@ var migrations = []Migration{
 	NewMigration("Add exclusive label", v1_19.AddExclusiveLabel),
 
 	// Gitea 1.19.0 ends at v244
+
+	// v244 -> v245
+	NewMigration("Add NeedApproval to actions tables", v1_20.AddNeedApprovalToActionRun),
 }
 
 // GetCurrentDBVersion returns the current db version

models/migrations/v1_20/v244.go

@@ -0,0 +1,22 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_20 //nolint
+
+import (
+	"xorm.io/xorm"
+)
+
+func AddNeedApprovalToActionRun(x *xorm.Engine) error {
+	/*
+		New index: TriggerUserID
+		New fields: NeedApproval, ApprovedBy
+	*/
+	type ActionRun struct {
+		TriggerUserID int64 `xorm:"index"`
+		NeedApproval  bool  // may need approval if it's a fork pull request
+		ApprovedBy    int64 `xorm:"index"` // who approved
+	}
+
+	return x.Sync(new(ActionRun))
+}

options/locale/locale_en-US.ini

@@ -3346,3 +3346,5 @@ runs.open_tab = %d Open
 runs.closed_tab = %d Closed
 runs.commit = Commit
 runs.pushed_by = Pushed by
+
+need_approval_desc = Need approval to run workflows for fork pull request.

routers/web/repo/actions/view.go

@@ -49,11 +49,12 @@ type ViewRequest struct {
 type ViewResponse struct {
 	State struct {
 		Run struct {
-			Link      string     `json:"link"`
-			Title     string     `json:"title"`
-			CanCancel bool       `json:"canCancel"`
-			Done      bool       `json:"done"`
-			Jobs      []*ViewJob `json:"jobs"`
+			Link       string     `json:"link"`
+			Title      string     `json:"title"`
+			CanCancel  bool       `json:"canCancel"`
+			CanApprove bool       `json:"canApprove"` // the run needs an approval and the doer has permission to approve
+			Done       bool       `json:"done"`
+			Jobs       []*ViewJob `json:"jobs"`
 		} `json:"run"`
 		CurrentJob struct {
 			Title  string         `json:"title"`
@@ -107,6 +108,7 @@ func ViewPost(ctx *context_module.Context) {
 	resp.State.Run.Title = run.Title
 	resp.State.Run.Link = run.Link()
 	resp.State.Run.CanCancel = !run.Status.IsDone() && ctx.Repo.CanWrite(unit.TypeActions)
+	resp.State.Run.CanApprove = run.NeedApproval && ctx.Repo.CanWrite(unit.TypeActions)
 	resp.State.Run.Done = run.Status.IsDone()
 	resp.State.Run.Jobs = make([]*ViewJob, 0, len(jobs)) // marshal to '[]' instead fo 'null' in json
 	for _, v := range jobs {
@@ -135,6 +137,9 @@ func ViewPost(ctx *context_module.Context) {
 
 	resp.State.CurrentJob.Title = current.Name
 	resp.State.CurrentJob.Detail = current.Status.LocaleString(ctx.Locale)
+	if run.NeedApproval {
+		resp.State.CurrentJob.Detail = ctx.Locale.Tr("actions.need_approval_desc")
+	}
 	resp.State.CurrentJob.Steps = make([]*ViewJobStep, 0) // marshal to '[]' instead fo 'null' in json
 	resp.Logs.StepsLog = make([]*ViewStepLog, 0)          // marshal to '[]' instead fo 'null' in json
 	if task != nil {
@@ -261,6 +266,40 @@ 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_model.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_model.UpdateRunJob(ctx, job, nil, "status")
+				if err != nil {
+					return err
+				}
+			}
+		}
+		return nil
+	}); err != nil {
+		ctx.Error(http.StatusInternalServerError, err.Error())
+		return
+	}
+
+	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.

routers/web/web.go

@@ -1286,6 +1286,7 @@ func RegisterRoutes(m *web.Route) {
 					m.Post("/rerun", reqRepoActionsWriter, actions.Rerun)
 				})
 				m.Post("/cancel", reqRepoActionsWriter, actions.Cancel)
+				m.Post("/approve", reqRepoActionsWriter, actions.Approve)
 			})
 		}, reqRepoActionsReader, actions.MustEnableActions)
 

services/actions/notifier_helper.go

@@ -153,7 +153,7 @@ func notify(ctx context.Context, input *notifyInput) error {
 	}
 
 	for id, content := range workflows {
-		run := actions_model.ActionRun{
+		run := &actions_model.ActionRun{
 			Title:             strings.SplitN(commit.CommitMessage, "\n", 2)[0],
 			RepoID:            input.Repo.ID,
 			OwnerID:           input.Repo.OwnerID,
@@ -166,12 +166,19 @@ func notify(ctx context.Context, input *notifyInput) error {
 			EventPayload:      string(p),
 			Status:            actions_model.StatusWaiting,
 		}
+		if need, err := ifNeedApproval(ctx, run, input.Repo, input.Doer); err != nil {
+			log.Error("check if need approval for repo %d with user %d: %v", input.Repo.ID, input.Doer.ID, err)
+			continue
+		} else {
+			run.NeedApproval = need
+		}
+
 		jobs, err := jobparser.Parse(content)
 		if err != nil {
 			log.Error("jobparser.Parse: %v", err)
 			continue
 		}
-		if err := actions_model.InsertRun(ctx, &run, jobs); err != nil {
+		if err := actions_model.InsertRun(ctx, run, jobs); err != nil {
 			log.Error("InsertRun: %v", err)
 			continue
 		}
@@ -234,3 +241,40 @@ 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) {
+	// don't need approval if it's not a fork PR
+	if !run.IsForkPullRequest {
+		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 := actions_model.CountRuns(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
+}

web_src/js/components/RepoActionView.vue

@@ -3,7 +3,10 @@
     <div class="action-view-header">
       <div class="action-info-summary">
         {{ run.title }}
-        <button class="run_cancel" @click="cancelRun()" v-if="run.canCancel">
+        <button class="run_approve" @click="approveRun()" v-if="run.canApprove">
+          <i class="play circle outline icon"/>
+        </button>
+        <button class="run_cancel" @click="cancelRun()" v-else-if="run.canCancel">
           <i class="stop circle outline icon"/>
         </button>
       </div>
@@ -97,6 +100,7 @@ const sfc = {
         link: '',
         title: '',
         canCancel: false,
+        canApprove: false,
         done: false,
         jobs: [
           // {
@@ -173,6 +177,10 @@ const sfc = {
     cancelRun() {
       this.fetchPost(`${this.run.link}/cancel`);
     },
+    // approve a run
+    approveRun() {
+      this.fetchPost(`${this.run.link}/approve`);
+    },
 
     createLogLine(line) {
       const div = document.createElement('div');
@@ -303,7 +311,15 @@ export function initRepositoryActionView() {
     cursor: pointer;
     transition:transform 0.2s;
   };
-  .run_cancel:hover{
+  .run_approve {
+    border: none;
+    color: var(--color-green);
+    background-color: transparent;
+    outline: none;
+    cursor: pointer;
+    transition:transform 0.2s;
+  };
+  .run_cancel:hover, .run_approve:hover {
     transform:scale(130%);
   };
 }