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

Merged
merged 43 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
1ac9f01
rename project settings to repo settings
anbraten Feb 7, 2024
42c8c30
update security settings
anbraten Feb 7, 2024
5fb3563
Merge remote-tracking branch 'upstream/main' into repo-settings
anbraten Sep 27, 2024
034c260
undo
anbraten Sep 27, 2024
be2dfc8
undo
anbraten Sep 27, 2024
51f98b1
next
anbraten Sep 30, 2024
2dc6684
enhance
anbraten Oct 14, 2024
a6511fb
fix
anbraten Oct 27, 2024
15fb887
rename
anbraten Oct 27, 2024
27ab0e9
rename
anbraten Oct 27, 2024
b16f2dc
add ui
anbraten Oct 27, 2024
e77d53a
Merge branch 'main' into repo-settings
anbraten Oct 27, 2024
d248001
add migration
anbraten Oct 27, 2024
b971e35
Merge branch 'repo-settings' of github.com:anbraten/woodpecker into r…
anbraten Oct 27, 2024
559d41c
fix
anbraten Oct 27, 2024
6f0d26a
enhance
anbraten Oct 28, 2024
d6390ad
Merge branch 'main' into repo-settings
anbraten Oct 28, 2024
bf80661
fallback
anbraten Oct 29, 2024
e5d4fe1
update docs
anbraten Oct 29, 2024
19eb040
Merge remote-tracking branch 'upstream/main' into repo-settings
anbraten Oct 29, 2024
de25d8e
adjust ui and update docs
anbraten Oct 29, 2024
17c7150
increase size
anbraten Oct 29, 2024
420943a
use enum
anbraten Oct 29, 2024
12688f8
address review, add additional mode
anbraten Oct 30, 2024
86b2fb4
Merge branch 'main' into repo-settings
anbraten Oct 30, 2024
3bd5876
Merge remote-tracking branch 'upstream/main' into repo-settings
anbraten Oct 30, 2024
2410934
adjust screenshot & func
anbraten Oct 30, 2024
f9c42cf
Merge branch 'repo-settings' of github.com:anbraten/woodpecker into r…
anbraten Oct 30, 2024
e7b0153
Update server/model/repo.go
anbraten Nov 1, 2024
9264813
change to approval-mode to none if not gated
anbraten Nov 1, 2024
9c7ae8c
adjust translation
anbraten Nov 1, 2024
824f8af
Merge remote-tracking branch 'upstream/main' into repo-settings
anbraten Nov 1, 2024
1a75dfc
fmt
anbraten Nov 3, 2024
7ba3f21
Merge branch 'main' into repo-settings
6543 Nov 5, 2024
9d6fb48
Merge branch 'main' into repo-settings
6543 Nov 5, 2024
dd7e50f
code-nit
6543 Nov 5, 2024
7dc42d1
move validation to where enums are declared
6543 Nov 5, 2024
24b7484
enhance migration
6543 Nov 5, 2024
93b8d77
string
6543 Nov 5, 2024
b6f73ba
Merge remote-tracking branch 'upstream/main' into repo-settings
anbraten Nov 18, 2024
f54e288
use const and back to anbratens migration
6543 Nov 18, 2024
b17ce97
address review
6543 Nov 18, 2024
099af5d
fix web lint issue
6543 Nov 18, 2024
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 }}
`
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 @@ -4905,6 +4905,9 @@ const docTemplate = `{
"forge_url": {
"type": "string"
},
"from_fork": {
"type": "boolean"
},
"id": {
"type": "integer"
},
Expand Down Expand Up @@ -5068,9 +5071,6 @@ const docTemplate = `{
"full_name": {
"type": "string"
},
"gated": {
"type": "boolean"
},
"id": {
"type": "integer"
},
Expand All @@ -5092,6 +5092,9 @@ const docTemplate = `{
"private": {
"type": "boolean"
},
"require_approval": {
"$ref": "#/definitions/model.ApprovalMode"
},
"scm": {
"$ref": "#/definitions/SCMKind"
},
Expand Down Expand Up @@ -5125,11 +5128,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 @@ -5621,6 +5628,27 @@ const docTemplate = `{
}
}
},
"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 @@ -91,6 +91,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 @@ -250,8 +251,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.Timeout != nil {
repo.Timeout = *in.Timeout
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"`
Trusted TrustedConfiguration `json:"trusted" xorm:"json '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 @@ -109,7 +130,8 @@ func (r *Repo) Update(from *Repo) {
// RepoPatch represents a repository patch object.
type RepoPatch struct {
Config *string `json:"config_file,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