Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extend approval options (#3348) #4429

Merged
merged 19 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 }}
`
33 changes: 30 additions & 3 deletions cli/repo/repo_update.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,12 @@ var repoUpdateCmd = &cli.Command{
Usage: "repository is trusted",
},
&cli.BoolFlag{
Name: "gated",
Usage: "repository is gated",
Name: "gated", // TODO: remove in next major release
Usage: "[deprecated] repository is gated",
},
&cli.StringFlag{
Name: "require-approval",
Usage: "repository requires approval for",
},
&cli.DurationFlag{
Name: "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,30 @@ 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
fmt.Print("[WARNING] The 'gated' flag was deprecated, use 'require-approval' instead.")
if gated {
6543 marked this conversation as resolved.
Show resolved Hide resolved
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
37 changes: 34 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: 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,30 @@ const docTemplate = `{
"EventManual"
]
},
"model.ApprovalMode": {
"type": "string",
"enum": [
"old_not_gated",
"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",
"RequireApprovalOldNotGated": "require approval for no events (deprecated is gated) // TODO: remove it in next major",
"RequireApprovalPullRequests": "require approval for all PRs"
},
"x-enum-varnames": [
"RequireApprovalOldNotGated",
"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
anbraten marked this conversation as resolved.
Show resolved Hide resolved

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 `All pull requests`.

## 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.
1 change: 1 addition & 0 deletions docs/docs/91-migrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Some versions need some changes to the server configuration or the pipeline conf

## `next`

- Deprecated `gated` repo settings option, use `require-approval`
- Deprecated `steps.[name].group` in favor of `steps.[name].depends_on` (see [workflow syntax](./20-usage/20-workflow-syntax.md#depends_on) to learn how to set dependencies)
- Removed `WOODPECKER_ROOT_PATH` and `WOODPECKER_ROOT_URL` config variables. Use `WOODPECKER_HOST` with a path instead
- Pipelines without a config file will now be skipped instead of failing
Expand Down
23 changes: 20 additions & 3 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.RequireApprovalPullRequests
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.RequireApprovalOldNotGated
}
}
if in.IsTrusted != nil {
repo.IsTrusted = *in.IsTrusted
Expand Down Expand Up @@ -319,7 +332,11 @@ func LookupRepo(c *gin.Context) {
// @Param Authorization header string true "Insert your personal access token" default(Bearer <personal access token>)
// @Param repo_id path int true "the repository id"
func GetRepo(c *gin.Context) {
c.JSON(http.StatusOK, session.Repo(c))
repo := session.Repo(c)
if repo.RequireApproval == model.RequireApprovalOldNotGated {
repo.RequireApproval = model.RequireApprovalNone // TODO: remove in next major release
}
c.JSON(http.StatusOK, repo)
}

// GetRepoPermissions
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
27 changes: 25 additions & 2 deletions server/model/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,28 @@ import (
"strings"
)

type ApprovalMode string

const (
RequireApprovalOldNotGated ApprovalMode = "old_not_gated" // require approval for no events (deprecated is gated) // TODO: remove it in next major
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 +64,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 +132,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"`
RequireApproval *string `json:"require_approval,omitempty"`
IsGated *bool `json:"gated,omitempty"` // TODO: remove in next major release
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
46 changes: 40 additions & 6 deletions server/pipeline/gated.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,45 @@ 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
}

// TODO: remove this option in next major release
if repo.RequireApproval == model.RequireApprovalOldNotGated {
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