Skip to content

Commit

Permalink
Extend approval options (#3348)
Browse files Browse the repository at this point in the history
  • Loading branch information
anbraten authored and 6543 committed Nov 18, 2024
1 parent 092ea7b commit 215beb3
Show file tree
Hide file tree
Showing 27 changed files with 432 additions and 94 deletions.
1 change: 1 addition & 0 deletions cli/repo/repo_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ Visibility: {{ .Visibility }}
Private: {{ .IsSCMPrivate }}
Trusted: {{ .IsTrusted }}
Gated: {{ .IsGated }}
Require approval for: {{ .RequireApproval }}
Clone url: {{ .Clone }}
Allow pull-requests: {{ .AllowPullRequests }}
`
28 changes: 27 additions & 1 deletion cli/repo/repo_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ var repoUpdateCmd = &cli.Command{
Name: "gated",
Usage: "repository is gated",
},
&cli.StringFlag{
Name: "require-approval",
Usage: "repository requires approval for",
},
&cli.DurationFlag{
Name: "timeout",
Usage: "repository timeout",
Expand Down Expand Up @@ -79,6 +83,7 @@ func repoUpdate(ctx context.Context, c *cli.Command) error {
timeout = c.Duration("timeout")
trusted = c.Bool("trusted")
gated = c.Bool("gated")
requireApproval = c.String("require-approval")
pipelineCounter = int(c.Int("pipeline-counter"))
unsafe = c.Bool("unsafe")
)
Expand All @@ -87,8 +92,29 @@ func repoUpdate(ctx context.Context, c *cli.Command) error {
if c.IsSet("trusted") {
patch.IsTrusted = &trusted
}
// TODO: remove isGated in next major release
if c.IsSet("gated") {
patch.IsGated = &gated
if gated {
patch.RequireApproval = &woodpecker.RequireApprovalAllEvents
} else {
patch.RequireApproval = &woodpecker.RequireApprovalNone
}
}
if c.IsSet("require-approval") {
if mode := woodpecker.ApprovalMode(requireApproval); mode.Valid() {
patch.RequireApproval = &mode
} else {
return fmt.Errorf("update approval mode failed: '%s' is no valid mode", mode)
}

// TODO: remove isGated in next major release
if requireApproval == string(woodpecker.RequireApprovalAllEvents) {
trueBool := true
patch.IsGated = &trueBool
} else if requireApproval == string(woodpecker.RequireApprovalNone) {
falseBool := false
patch.IsGated = &falseBool
}
}
if c.IsSet("timeout") {
v := int64(timeout / time.Minute)
Expand Down
34 changes: 31 additions & 3 deletions cmd/server/docs/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -4671,6 +4671,9 @@ const docTemplate = `{
"forge_url": {
"type": "string"
},
"from_fork": {
"type": "boolean"
},
"id": {
"type": "integer"
},
Expand Down Expand Up @@ -4837,9 +4840,6 @@ const docTemplate = `{
"full_name": {
"type": "string"
},
"gated": {
"type": "boolean"
},
"id": {
"type": "integer"
},
Expand All @@ -4861,6 +4861,9 @@ const docTemplate = `{
"private": {
"type": "boolean"
},
"require_approval": {
"$ref": "#/definitions/model.ApprovalMode"
},
"scm": {
"$ref": "#/definitions/SCMKind"
},
Expand Down Expand Up @@ -4894,11 +4897,15 @@ const docTemplate = `{
"type": "string"
},
"gated": {
"description": "TODO: deprecated in favor of RequireApproval =\u003e Remove in next major release",
"type": "boolean"
},
"netrc_only_trusted": {
"type": "boolean"
},
"require_approval": {
"type": "string"
},
"timeout": {
"type": "integer"
},
Expand Down Expand Up @@ -5163,6 +5170,27 @@ const docTemplate = `{
"EventManual"
]
},
"model.ApprovalMode": {
"type": "string",
"enum": [
"none",
"forks",
"pull_requests",
"all_events"
],
"x-enum-comments": {
"RequireApprovalAllEvents": "require approval for all external events",
"RequireApprovalForks": "require approval for PRs from forks (default)",
"RequireApprovalNone": "require approval for no events",
"RequireApprovalPullRequests": "require approval for all PRs"
},
"x-enum-varnames": [
"RequireApprovalNone",
"RequireApprovalForks",
"RequireApprovalPullRequests",
"RequireApprovalAllEvents"
]
},
"model.ForgeType": {
"type": "string",
"enum": [
Expand Down
5 changes: 2 additions & 3 deletions docs/docs/20-usage/75-project-settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@ Only activate this option if you trust all users who have push access to your re
Otherwise, these users will be able to steal secrets that are only available for `deploy` events.
:::

## Protected
## Require approval for

Every pipeline initiated by an webhook event needs to be approved by a project members with push permissions before being executed.
The protected option can be used as an additional review process before running potentially harmful pipelines. Especially if pipelines can be executed by third-parties through pull-requests.
To prevent malicious pipelines from extracting secrets or running harmful commands or to prevent accidental pipeline runs, you can require approval for an additional review process. Depending on the enabled option, a pipeline will be put on hold after creation and will only continue after approval. The default restrictive setting is `Approvals for forked repositories`.

## Trusted

Expand Down
Binary file modified docs/docs/20-usage/project-settings.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 15 additions & 2 deletions server/api/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ func PostRepo(c *gin.Context) {
repo.Update(from)
} else {
repo = from
repo.RequireApproval = model.RequireApprovalForks
repo.AllowPull = true
repo.AllowDeploy = false
repo.NetrcOnlyTrusted = true
Expand Down Expand Up @@ -236,8 +237,20 @@ func PatchRepo(c *gin.Context) {
if in.AllowDeploy != nil {
repo.AllowDeploy = *in.AllowDeploy
}
if in.IsGated != nil {
repo.IsGated = *in.IsGated

if in.RequireApproval != nil {
if mode := model.ApprovalMode(*in.RequireApproval); mode.Valid() {
repo.RequireApproval = mode
} else {
c.String(http.StatusBadRequest, "Invalid require-approval setting")
return
}
} else if in.IsGated != nil { // TODO: remove isGated in next major release
if *in.IsGated {
repo.RequireApproval = model.RequireApprovalAllEvents
} else {
repo.RequireApproval = model.RequireApprovalForks
}
}
if in.IsTrusted != nil {
repo.IsTrusted = *in.IsTrusted
Expand Down
1 change: 1 addition & 0 deletions server/forge/bitbucket/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ func convertPullHook(from *internal.PullRequestHook) *model.Pipeline {
Author: from.Actor.Login,
Sender: from.Actor.Login,
Timestamp: from.PullRequest.Updated.UTC().Unix(),
FromFork: from.PullRequest.Source.Repo.UUID != from.PullRequest.Dest.Repo.UUID,
}

if from.PullRequest.State == stateClosed {
Expand Down
1 change: 1 addition & 0 deletions server/forge/bitbucketdatacenter/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func convertPullRequestEvent(ev *bb.PullRequestEvent, baseURL string) *model.Pip
Ref: fmt.Sprintf("refs/pull-requests/%d/from", ev.PullRequest.ID),
ForgeURL: fmt.Sprintf("%s/projects/%s/repos/%s/commits/%s", baseURL, ev.PullRequest.Source.Repository.Project.Key, ev.PullRequest.Source.Repository.Slug, ev.PullRequest.Source.Latest),
Refspec: fmt.Sprintf("%s:%s", ev.PullRequest.Source.DisplayID, ev.PullRequest.Target.DisplayID),
FromFork: ev.PullRequest.Source.Repository.ID != ev.PullRequest.Target.Repository.ID,
}

if ev.EventKey == bb.EventKeyPullRequestMerged || ev.EventKey == bb.EventKeyPullRequestDeclined || ev.EventKey == bb.EventKeyPullRequestDeleted {
Expand Down
1 change: 1 addition & 0 deletions server/forge/forgejo/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func pipelineFromPullRequest(hook *pullRequestHook) *model.Pipeline {
hook.PullRequest.Base.Ref,
),
PullRequestLabels: convertLabels(hook.PullRequest.Labels),
FromFork: hook.PullRequest.Head.RepoID != hook.PullRequest.Base.RepoID,
}

return pipeline
Expand Down
1 change: 1 addition & 0 deletions server/forge/gitea/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ func pipelineFromPullRequest(hook *pullRequestHook) *model.Pipeline {
hook.PullRequest.Base.Ref,
),
PullRequestLabels: convertLabels(hook.PullRequest.Labels),
FromFork: hook.PullRequest.Head.RepoID != hook.PullRequest.Base.RepoID,
}

return pipeline
Expand Down
3 changes: 3 additions & 0 deletions server/forge/github/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ func parsePullHook(hook *github.PullRequestEvent, merge bool) (*github.PullReque
event = model.EventPullClosed
}

fromFork := hook.GetPullRequest().GetHead().GetRepo().GetID() != hook.GetPullRequest().GetBase().GetRepo().GetID()

pipeline := &model.Pipeline{
Event: event,
Commit: hook.GetPullRequest().GetHead().GetSHA(),
Expand All @@ -173,6 +175,7 @@ func parsePullHook(hook *github.PullRequestEvent, merge bool) (*github.PullReque
hook.GetPullRequest().GetBase().GetRef(),
),
PullRequestLabels: convertLabels(hook.GetPullRequest().Labels),
FromFork: fromFork,
}
if merge {
pipeline.Ref = fmt.Sprintf(mergeRefs, hook.GetPullRequest().GetNumber())
Expand Down
1 change: 1 addition & 0 deletions server/forge/gitlab/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func convertMergeRequestHook(hook *gitlab.MergeEvent, req *http.Request) (int, *
pipeline.Title = obj.Title
pipeline.ForgeURL = obj.URL
pipeline.PullRequestLabels = convertLabels(hook.Labels)
pipeline.FromFork = target.PathWithNamespace != source.PathWithNamespace

return obj.IID, repo, pipeline, nil
}
Expand Down
1 change: 1 addition & 0 deletions server/model/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type Pipeline struct {
AdditionalVariables map[string]string `json:"variables,omitempty" xorm:"json 'additional_variables'"`
PullRequestLabels []string `json:"pr_labels,omitempty" xorm:"json 'pr_labels'"`
IsPrerelease bool `json:"is_prerelease,omitempty" xorm:"is_prerelease"`
FromFork bool `json:"from_fork,omitempty" xorm:"from_fork"`
} // @name Pipeline

// TableName return database table name for xorm.
Expand Down
26 changes: 24 additions & 2 deletions server/model/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,27 @@ import (
"strings"
)

type ApprovalMode string

const (
RequireApprovalNone ApprovalMode = "none" // require approval for no events
RequireApprovalForks ApprovalMode = "forks" // require approval for PRs from forks (default)
RequireApprovalPullRequests ApprovalMode = "pull_requests" // require approval for all PRs
RequireApprovalAllEvents ApprovalMode = "all_events" // require approval for all external events
)

func (mode ApprovalMode) Valid() bool {
switch mode {
case RequireApprovalNone,
RequireApprovalForks,
RequireApprovalPullRequests,
RequireApprovalAllEvents:
return true
default:
return false
}
}

// Repo represents a repository.
type Repo struct {
ID int64 `json:"id,omitempty" xorm:"pk autoincr 'id'"`
Expand All @@ -42,7 +63,7 @@ type Repo struct {
Visibility RepoVisibility `json:"visibility" xorm:"varchar(10) 'visibility'"`
IsSCMPrivate bool `json:"private" xorm:"private"`
IsTrusted bool `json:"trusted" xorm:"trusted"`
IsGated bool `json:"gated" xorm:"gated"`
RequireApproval ApprovalMode `json:"require_approval" xorm:"varchar(50) require_approval"`
IsActive bool `json:"active" xorm:"active"`
AllowPull bool `json:"allow_pr" xorm:"allow_pr"`
AllowDeploy bool `json:"allow_deploy" xorm:"allow_deploy"`
Expand Down Expand Up @@ -110,7 +131,8 @@ func (r *Repo) Update(from *Repo) {
type RepoPatch struct {
Config *string `json:"config_file,omitempty"`
IsTrusted *bool `json:"trusted,omitempty"`
IsGated *bool `json:"gated,omitempty"`
IsGated *bool `json:"gated,omitempty"` // TODO: deprecated in favor of RequireApproval => Remove in next major release
RequireApproval *string `json:"require_approval,omitempty"`
Timeout *int64 `json:"timeout,omitempty"`
Visibility *string `json:"visibility,omitempty"`
AllowPull *bool `json:"allow_pr,omitempty"`
Expand Down
3 changes: 1 addition & 2 deletions server/pipeline/approve.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ import (
"go.woodpecker-ci.org/woodpecker/v2/server/store"
)

// Approve update the status to pending for a blocked pipeline because of a gated repo
// and start them afterward.
// Approve update the status to pending for a blocked pipeline so it can be executed.
func Approve(ctx context.Context, store store.Store, currentPipeline *model.Pipeline, user *model.User, repo *model.Repo) (*model.Pipeline, error) {
if currentPipeline.Status != model.StatusBlocked {
return nil, ErrBadRequest{Msg: fmt.Sprintf("cannot approve a pipeline with status %s", currentPipeline.Status)}
Expand Down
2 changes: 1 addition & 1 deletion server/pipeline/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func Create(ctx context.Context, _store store.Store, repo *model.Repo, pipeline
// update some pipeline fields
pipeline.RepoID = repo.ID
pipeline.Status = model.StatusCreated
setGatedState(repo, pipeline)
setApprovalState(repo, pipeline)
err = _store.CreatePipeline(pipeline)
if err != nil {
msg := fmt.Errorf("failed to save pipeline for %s", repo.FullName)
Expand Down
2 changes: 1 addition & 1 deletion server/pipeline/decline.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"go.woodpecker-ci.org/woodpecker/v2/server/store"
)

// Decline updates the status to declined for blocked pipelines because of a gated repo.
// Decline updates the status to declined for blocked pipelines.
func Decline(ctx context.Context, store store.Store, pipeline *model.Pipeline, user *model.User, repo *model.Repo) (*model.Pipeline, error) {
forge, err := server.Config.Services.Manager.ForgeFromRepo(repo)
if err != nil {
Expand Down
41 changes: 35 additions & 6 deletions server/pipeline/gated.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,40 @@ package pipeline

import "go.woodpecker-ci.org/woodpecker/v2/server/model"

func setGatedState(repo *model.Repo, pipeline *model.Pipeline) {
// TODO(336): extend gated feature with an allow/block List
if repo.IsGated &&
// events created by woodpecker itself should run right away
pipeline.Event != model.EventCron && pipeline.Event != model.EventManual {
pipeline.Status = model.StatusBlocked
func setApprovalState(repo *model.Repo, pipeline *model.Pipeline) {
if !needsApproval(repo, pipeline) {
return
}

// set pipeline status to blocked and require approval
pipeline.Status = model.StatusBlocked
}

func needsApproval(repo *model.Repo, pipeline *model.Pipeline) bool {
// skip events created by woodpecker itself
if pipeline.Event == model.EventCron || pipeline.Event == model.EventManual {
return false
}

// repository allows all events without approval
if repo.RequireApproval == model.RequireApprovalNone {
return false
}

// repository requires approval for pull requests from forks
if pipeline.Event == model.EventPull && pipeline.FromFork {
return true
}

// repository requires approval for pull requests
if pipeline.Event == model.EventPull && repo.RequireApproval == model.RequireApprovalPullRequests {
return true
}

// repository requires approval for all events
if repo.RequireApproval == model.RequireApprovalAllEvents {
return true
}

return false
}
Loading

0 comments on commit 215beb3

Please sign in to comment.