Skip to content

Commit

Permalink
Merge pull request #25 from grafana/julienduchesne/disallow-forks
Browse files Browse the repository at this point in the history 
feat: Disallow forks from getting secrets
  • Loading branch information
TP Honey authored Apr 25, 2023
2 parents 2f6ccb8 + 7cf2a7c commit 6496086
Show file tree
Hide file tree
Showing 4 changed files with 169 additions and 16 deletions.
9 changes: 7 additions & 2 deletions main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ import (
"github.com/drone/drone-go/plugin/secret"
"github.com/drone/drone-vault/plugin"
"github.com/drone/drone-vault/plugin/token"
"github.com/drone/drone-vault/plugin/token/kubernetes"
"github.com/drone/drone-vault/plugin/token/approle"
"github.com/drone/drone-vault/plugin/token/kubernetes"

"github.com/hashicorp/vault/api"
"github.com/kelseyhightower/envconfig"
Expand All @@ -40,6 +40,7 @@ type config struct {
Address string `envconfig:"DRONE_BIND"`
Debug bool `envconfig:"DRONE_DEBUG"`
Secret string `envconfig:"DRONE_SECRET"`
DisallowForks bool `envconfig:"DRONE_DISALLOW_FORKS"`
VaultAddr string `envconfig:"VAULT_ADDR"`
VaultRenew time.Duration `envconfig:"VAULT_TOKEN_RENEWAL"`
VaultTTL time.Duration `envconfig:"VAULT_TOKEN_TTL"`
Expand Down Expand Up @@ -80,9 +81,13 @@ func main() {
// global context
ctx := context.Background()

if spec.DisallowForks {
logrus.Info("globally disallowing secrets in forks")
}

http.Handle("/", secret.Handler(
spec.Secret,
plugin.New(client),
plugin.New(client, spec.DisallowForks),
logrus.StandardLogger(),
))

Expand Down
38 changes: 31 additions & 7 deletions plugin/plugin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,33 @@ import (

// New returns a new secret plugin that sources secrets
// from the AWS secrets manager.
func New(client *api.Client) secret.Plugin {
func New(client *api.Client, disallowForks bool) secret.Plugin {
return &plugin{
client: client,
client: client,
disallowForks: disallowForks,
}
}

type plugin struct {
client *api.Client
client *api.Client
disallowForks bool
}

func (p *plugin) Find(ctx context.Context, req *secret.Request) (*drone.Secret, error) {
// The Fork attribute will be empty on a branch build (e.g. master).
// Branch builds cannot be from a fork.
isFork := req.Build.Fork != "" && req.Build.Fork != req.Repo.Slug
forkRepo := ""
if isFork {
forkRepo = req.Build.Fork
}

logEvent := logrus.WithFields(logrus.Fields{
"event": req.Build.Event,
"repo": req.Repo.Slug,
"ref": req.Build.Ref,
"secret": req.Path,
"fork": forkRepo,
})

path := req.Path
Expand All @@ -52,7 +63,7 @@ func (p *plugin) Find(ctx context.Context, req *secret.Request) (*drone.Secret,
return nil, errors.New("secret key not found")
}

// the user can filter out requets based on event type
// the user can filter out requests based on event type
// using the X-Drone-Events secret key. Check for this
// user-defined filter logic.
events := extractEvents(params)
Expand All @@ -62,7 +73,7 @@ func (p *plugin) Find(ctx context.Context, req *secret.Request) (*drone.Secret,
return nil, errors.New(msg)
}

// the user can filter out requets based on repository
// the user can filter out requests based on repository
// using the X-Drone-Repos secret key. Check for this
// user-defined filter logic.
repos := extractRepos(params)
Expand All @@ -72,7 +83,7 @@ func (p *plugin) Find(ctx context.Context, req *secret.Request) (*drone.Secret,
return nil, errors.New(msg)
}

// the user can filter out requets based on repository
// the user can filter out requests based on repository
// branch using the X-Drone-Branches secret key. Check
// for this user-defined filter logic.
branches := extractBranches(params)
Expand All @@ -82,13 +93,26 @@ func (p *plugin) Find(ctx context.Context, req *secret.Request) (*drone.Secret,
return nil, errors.New(msg)
}

// the user can disallow fork builds using the
// X-Drone-Disallow-Forks secret key. Check for this
// user-defined filter logic.
disallowForks := p.disallowForks
if secretSetting := extractDisallowForks(params); secretSetting != nil {
disallowForks = *secretSetting
}
if disallowForks && isFork {
msg := "access denied: forks are not allowed"
logEvent.WithField("disallow_forks", disallowForks).Debug(msg)
return nil, errors.New(msg)
}

logEvent.Debug("secret matched and returned")

return &drone.Secret{
Name: name,
Data: value,
Pull: true, // always true. use X-Drone-Events to prevent pull requests.
Fork: true, // always true. use X-Drone-Events to prevent pull requests.
Fork: true, // always true. use X-Drone-Disallow-Forks to prevent secrets from forks.
}, nil
}

Expand Down
121 changes: 115 additions & 6 deletions plugin/plugin_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package plugin

import (
"context"
"encoding/json"
"io/ioutil"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -50,7 +51,7 @@ func TestPlugin(t *testing.T) {
Slug: "octocat/hello-world",
},
}
plugin := New(client)
plugin := New(client, false)
got, err := plugin.Find(noContext, req)
if err != nil {
t.Error(err)
Expand Down Expand Up @@ -92,7 +93,7 @@ func TestPlugin_FilterBranches(t *testing.T) {
Slug: "octocat/hello-world",
},
}
plugin := New(client)
plugin := New(client, false)
_, err := plugin.Find(noContext, req)
if err == nil {
t.Errorf("Expect error")
Expand Down Expand Up @@ -126,7 +127,7 @@ func TestPlugin_FilterRepo(t *testing.T) {
Slug: "spaceghost/hello-world",
},
}
plugin := New(client)
plugin := New(client, false)
_, err := plugin.Find(noContext, req)
if err == nil {
t.Errorf("Expect error")
Expand Down Expand Up @@ -160,7 +161,7 @@ func TestPlugin_FilterEvent(t *testing.T) {
Slug: "octocat/hello-world",
},
}
plugin := New(client)
plugin := New(client, false)
_, err := plugin.Find(noContext, req)
if err == nil {
t.Errorf("Expect error")
Expand All @@ -171,6 +172,114 @@ func TestPlugin_FilterEvent(t *testing.T) {
}
}

func TestPlugin_FilterForks(t *testing.T) {
cases := []struct {
name string
disallowForksSecretSetting string
disallowForksGlobalSetting bool
expectedError string
isFork bool
}{
{
name: "disallow forks, secret setting is true",
disallowForksSecretSetting: "true",
disallowForksGlobalSetting: false,
expectedError: "access denied: forks are not allowed",
isFork: true,
},
{
name: "disallow forks, global setting is true",
disallowForksSecretSetting: "",
disallowForksGlobalSetting: true,
expectedError: "access denied: forks are not allowed",
isFork: true,
},
{
name: "disallow forks, secret setting is false",
disallowForksSecretSetting: "false",
disallowForksGlobalSetting: false,
expectedError: "",
isFork: true,
},
{
name: "disallow forks, secret setting is not set",
disallowForksSecretSetting: "",
disallowForksGlobalSetting: false,
expectedError: "",
isFork: true,
},
{
name: "disallow forks, secret setting enabled but not a fork",
disallowForksSecretSetting: "true",
disallowForksGlobalSetting: false,
expectedError: "",
isFork: false,
},
{
name: "disallow forks, global setting enabled but not a fork",
disallowForksSecretSetting: "",
disallowForksGlobalSetting: true,
expectedError: "",
isFork: false,
},
{
name: "allow forks, secret setting is false (override global setting)",
disallowForksSecretSetting: "false",
disallowForksGlobalSetting: true,
expectedError: "",
isFork: true,
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
payloadBefore, _ := ioutil.ReadFile("testdata/secret.json")
payload := make(map[string]interface{})
json.Unmarshal(payloadBefore, &payload)
if c.disallowForksSecretSetting != "" {
data := payload["data"].(map[string]interface{})
data["X-Drone-Disallow-Forks"] = c.disallowForksSecretSetting
payload["data"] = data
}
payloadAfter, _ := json.Marshal(payload)
w.Write(payloadAfter)
}))
defer ts.Close()

client, _ := api.NewClient(&api.Config{
Address: ts.URL,
MaxRetries: 1,
})

req := &secret.Request{
Path: "secret/docker",
Name: "username",
Build: drone.Build{
Event: "push",
Target: "master",
},
Repo: drone.Repo{
Slug: "octocat/hello-world",
},
}
if c.isFork {
req.Build.Fork = "spaceghost/hello-world"
}

plugin := New(client, c.disallowForksGlobalSetting)
gotErr := ""
if _, err := plugin.Find(noContext, req); err != nil {
gotErr = err.Error()
}

if gotErr != c.expectedError {
t.Errorf("Want error %q, got %q", c.expectedError, gotErr)
}
})
}
}

func TestPlugin_NotFound(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
out, _ := ioutil.ReadFile("testdata/not_found.json")
Expand All @@ -195,7 +304,7 @@ func TestPlugin_NotFound(t *testing.T) {
Slug: "octocat/hello-world",
},
}
plugin := New(client)
plugin := New(client, false)
_, err := plugin.Find(noContext, req)
if err == nil {
t.Errorf("Expect error")
Expand Down Expand Up @@ -231,7 +340,7 @@ func TestPlugin_KeyNotFound(t *testing.T) {
Slug: "octocat/hello-world",
},
}
plugin := New(client)
plugin := New(client, false)
_, err := plugin.Find(noContext, req)
if err == nil {
t.Errorf("Expect error")
Expand Down
17 changes: 16 additions & 1 deletion plugin/util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

package plugin

import "strings"
import (
"strconv"
"strings"
)

// helper function extracts the branch filters from the
// secret payload in key value format.
Expand Down Expand Up @@ -39,6 +42,18 @@ func extractEvents(params map[string]string) []string {
return nil
}

// helper function extracts the fork filter from the
// secret payload in key value format.
func extractDisallowForks(params map[string]string) *bool {
for key, value := range params {
if strings.EqualFold(key, "X-Drone-Disallow-Forks") {
v, _ := strconv.ParseBool(value)
return &v // Allow non-truthy or non-falsey values are false
}
}
return nil
}

func parseCommaSeparated(s string) []string {
parts := strings.Split(s, ",")
if len(parts) == 1 && parts[0] == "" {
Expand Down

0 comments on commit 6496086

Please sign in to comment.