From ef1d286596068f7249909b0453b9e31e244d4060 Mon Sep 17 00:00:00 2001
From: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Date: Sun, 31 Dec 2023 21:43:24 +0100
Subject: [PATCH] Allow PR secrets to be used on close (#3084)

closes https://github.com/woodpecker-ci/woodpecker/issues/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>
---
 server/model/secret.go      | 13 +++++++--
 server/model/secret_test.go | 55 ++++++++++++++++++++++++++++---------
 2 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/server/model/secret.go b/server/model/secret.go
index 7dbb087e9..0594c1d91 100644
--- a/server/model/secret.go
+++ b/server/model/secret.go
@@ -18,7 +18,6 @@ package model
 import (
 	"errors"
 	"fmt"
-	"path/filepath"
 	"regexp"
 	"sort"
 )
@@ -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
 }
 
diff --git a/server/model/secret_test.go b/server/model/secret_test.go
index b226b669c..01d242823 100644
--- a/server/model/secret_test.go
+++ b/server/model/secret_test.go
@@ -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",