diff options
author | Jason Song | 2023-02-24 15:58:49 +0800 |
---|---|---|
committer | GitHub | 2023-02-24 15:58:49 +0800 |
commit | edf98a2dc30956c8e04b778bb7f1ce55c14ba963 (patch) | |
tree | 2d8b7708f76a36c27700ab128082bdb976a0753e /services | |
parent | a6175b01d92a75dcbadbbfc6782a486636fd62a2 (diff) |
Require approval to run actions for fork pull request (#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 <xiaolunwen@gmail.com>
Diffstat (limited to 'services')
-rw-r--r-- | services/actions/notifier_helper.go | 48 |
1 files changed, 46 insertions, 2 deletions
diff --git a/services/actions/notifier_helper.go b/services/actions/notifier_helper.go index df67d2fa1..ef63b8cf9 100644 --- a/services/actions/notifier_helper.go +++ b/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 +} |