Skip to content

Commit

Permalink
Allow PR secrets to be used on close (woodpecker-ci#3084)
Browse files Browse the repository at this point in the history
closes woodpecker-ci#3071

1. If a secret can be used on PRs, it can also be used on PR close.
2. If no events are set, disallow access to secret. This was different
before, secrets without any event set were allowed for all events.
3. Compare strings instead of patterns.

---------

Co-authored-by: 6543 <6543@obermui.de>
  • Loading branch information
2 people authored and fernandrone committed Feb 1, 2024
1 parent ee6c74c commit 5bb38b1
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 16 deletions.
13 changes: 10 additions & 3 deletions server/model/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package model
import (
"errors"
"fmt"
"path/filepath"
"regexp"
"sort"
)
Expand Down Expand Up @@ -104,15 +103,23 @@ func (s Secret) IsRepository() bool {
}

// Match returns true if an image and event match the restricted list.
// Note that EventPullClosed are treated as EventPull.
func (s *Secret) Match(event WebhookEvent) bool {
// if there is no filter set secret matches all webhook events
if len(s.Events) == 0 {
return true
}
for _, pattern := range s.Events {
if match, _ := filepath.Match(string(pattern), string(event)); match {
// tread all pull events the same way
if event == EventPullClosed {
event = EventPull
}
// one match is enough
for _, e := range s.Events {
if e == event {
return true
}
}
// a filter is set but the webhook did not match it
return false
}

Expand Down
55 changes: 42 additions & 13 deletions server/model/secret_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,52 @@ import (
"testing"

"github.com/franela/goblin"
"github.com/stretchr/testify/assert"
)

func TestSecret(t *testing.T) {
func TestSecretMatch(t *testing.T) {
tcl := []*struct {
name string
secret Secret
event WebhookEvent
match bool
}{
{
name: "should match event",
secret: Secret{Events: []WebhookEvent{"pull_request"}},
event: EventPull,
match: true,
},
{
name: "should not match event",
secret: Secret{Events: []WebhookEvent{"pull_request"}},
event: EventPush,
match: false,
},
{
name: "should match when no event filters defined",
secret: Secret{},
event: EventPull,
match: true,
},
{
name: "pull close should match pull",
secret: Secret{Events: []WebhookEvent{"pull_request"}},
event: EventPullClosed,
match: true,
},
}

for _, tc := range tcl {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.match, tc.secret.Match(tc.event))
})
}
}

func TestSecretValidate(t *testing.T) {
g := goblin.Goblin(t)
g.Describe("Secret", func() {
g.It("should match event", func() {
secret := Secret{Events: []WebhookEvent{"pull_request"}}
g.Assert(secret.Match("pull_request")).IsTrue()
})
g.It("should not match event", func() {
secret := Secret{Events: []WebhookEvent{"pull_request"}}
g.Assert(secret.Match("push")).IsFalse()
})
g.It("should match when no event filters defined", func() {
secret := Secret{}
g.Assert(secret.Match("pull_request")).IsTrue()
})
g.It("should pass validation", func() {
secret := Secret{
Name: "secretname",
Expand Down

0 comments on commit 5bb38b1

Please sign in to comment.