Merge pull request #1858 from donny-dont/feature/secret-concealer

Feature/secret concealer

agent/agent.go

@@ -22,18 +22,17 @@ type Logger interface {
 }
 
 type Agent struct {
-	Update         UpdateFunc
-	Logger         LoggerFunc
-	Engine         build.Engine
-	Timeout        time.Duration
-	Platform       string
-	Namespace      string
-	Disable        []string
-	Escalate       []string
-	Netrc          []string
-	Local          string
-	Pull           bool
-	ConcealSecrets bool
+	Update    UpdateFunc
+	Logger    LoggerFunc
+	Engine    build.Engine
+	Timeout   time.Duration
+	Platform  string
+	Namespace string
+	Disable   []string
+	Escalate  []string
+	Netrc     []string
+	Local     string
+	Pull      bool
 }
 
 func (a *Agent) Poll() error {
@@ -189,7 +188,7 @@ func (a *Agent) exec(spec *yaml.Config, payload *model.Work, cancel <-chan bool)
 		return err
 	}
 
-	secretsReplacer := newSecretsReplacer(payload.Secrets)
+	replacer := NewSecretReplacer(payload.Secrets)
 	timeout := time.After(time.Duration(payload.Repo.Timeout) * time.Minute)
 
 	for {
@@ -229,25 +228,12 @@ func (a *Agent) exec(spec *yaml.Config, payload *model.Work, cancel <-chan bool)
 				pipeline.Exec()
 			}
 		case line := <-pipeline.Pipe():
-			// FIXME(vaijab): avoid checking a.ConcealSecrets is true everytime new line is received
-			if a.ConcealSecrets {
-				line.Out = secretsReplacer.Replace(line.Out)
-			}
+			line.Out = replacer.Replace(line.Out)
 			a.Logger(line)
 		}
 	}
 }
 
-// newSecretsReplacer takes []*model.Secret as secrets and returns a list of
-// secret value, "*****" pairs.
-func newSecretsReplacer(secrets []*model.Secret) *strings.Replacer {
-	var r []string
-	for _, s := range secrets {
-		r = append(r, s.Value, "*****")
-	}
-	return strings.NewReplacer(r...)
-}
-
 func toEnv(w *model.Work) map[string]string {
 	envs := map[string]string{
 		"CI":                         "drone",

agent/agent_test.go

@@ -1,23 +0,0 @@
-package agent
-
-import "testing"
-import "github.com/drone/drone/model"
-
-func Test_newSecretsReplacer(t *testing.T) {
-	secrets := []*model.Secret{
-		{Name: "SECRET",
-			Value:  "secret_value",
-			Images: []string{"*"},
-			Events: []string{"*"},
-		},
-	}
-
-	text := "This is SECRET: secret_value"
-	expected := "This is SECRET: *****"
-	secretsReplacer := newSecretsReplacer(secrets)
-	result := secretsReplacer.Replace(text)
-
-	if result != expected {
-		t.Errorf("Wanted %q, got %q.", expected, result)
-	}
-}

agent/secret.go

@@ -0,0 +1,46 @@
+package agent
+
+import (
+	"strings"
+
+	"github.com/drone/drone/model"
+)
+
+// SecretReplacer hides secrets from being exposed by the build output.
+type SecretReplacer interface {
+	// Replace conceals instances of secrets found in s.
+	Replace(s string) string
+}
+
+// NewSecretReplacer creates a SecretReplacer based on whether any value in
+// secrets requests it be hidden.
+func NewSecretReplacer(secrets []*model.Secret) SecretReplacer {
+	var r []string
+	for _, s := range secrets {
+		if s.Conceal {
+			r = append(r, s.Value, "*****")
+		}
+	}
+
+	if len(r) == 0 {
+		return &noopReplacer{}
+	}
+
+	return &secretReplacer{
+		replacer: strings.NewReplacer(r...),
+	}
+}
+
+type noopReplacer struct{}
+
+func (*noopReplacer) Replace(s string) string {
+	return s
+}
+
+type secretReplacer struct {
+	replacer *strings.Replacer
+}
+
+func (r *secretReplacer) Replace(s string) string {
+	return r.replacer.Replace(s)
+}

agent/secret_test.go

@@ -0,0 +1,39 @@
+package agent
+
+import (
+	"testing"
+
+	"github.com/drone/drone/model"
+	"github.com/franela/goblin"
+)
+
+const testString = "This is SECRET: secret_value"
+
+func TestSecret(t *testing.T) {
+	g := goblin.Goblin(t)
+	g.Describe("SecretReplacer", func() {
+		g.It("Should conceal secret", func() {
+			secrets := []*model.Secret{
+				{
+					Name:    "SECRET",
+					Value:   "secret_value",
+					Conceal: true,
+				},
+			}
+			r := NewSecretReplacer(secrets)
+			g.Assert(r.Replace(testString)).Equal("This is SECRET: *****")
+		})
+
+		g.It("Should not conceal secret", func() {
+			secrets := []*model.Secret{
+				{
+					Name:    "SECRET",
+					Value:   "secret_value",
+					Conceal: false,
+				},
+			}
+			r := NewSecretReplacer(secrets)
+			g.Assert(r.Replace(testString)).Equal(testString)
+		})
+	})
+}

drone/agent/agent.go

@@ -75,11 +75,6 @@ var AgentCmd = cli.Command{
 			Name:   "drone-secret",
 			Usage:  "drone agent secret",
 		},
-		cli.BoolFlag{
-			Name:   "conceal-secrets",
-			Usage:  "conceal secrets from build logs",
-			EnvVar: "DRONE_CONCEAL_SECRETS",
-		},
 		cli.DurationFlag{
 			EnvVar: "DRONE_BACKOFF",
 			Name:   "backoff",
@@ -191,13 +186,12 @@ func start(c *cli.Context) {
 			drone:  client,
 			docker: docker,
 			config: config{
-				platform:       c.String("docker-os") + "/" + c.String("docker-arch"),
-				timeout:        c.Duration("timeout"),
-				namespace:      c.String("namespace"),
-				privileged:     c.StringSlice("privileged"),
-				pull:           c.BoolT("pull"),
-				logs:           int64(c.Int("max-log-size")) * 1000000,
-				concealSecrets: c.Bool("conceal-secrets"),
+				platform:   c.String("docker-os") + "/" + c.String("docker-arch"),
+				timeout:    c.Duration("timeout"),
+				namespace:  c.String("namespace"),
+				privileged: c.StringSlice("privileged"),
+				pull:       c.BoolT("pull"),
+				logs:       int64(c.Int("max-log-size")) * 1000000,
 			},
 		}
 

drone/agent/exec.go

@@ -13,13 +13,12 @@ import (
 )
 
 type config struct {
-	platform       string
-	namespace      string
-	privileged     []string
-	pull           bool
-	logs           int64
-	timeout        time.Duration
-	concealSecrets bool
+	platform   string
+	namespace  string
+	privileged []string
+	pull       bool
+	logs       int64
+	timeout    time.Duration
 }
 
 type pipeline struct {
@@ -41,15 +40,14 @@ func (r *pipeline) run(w *model.Work) {
 	engine := docker.NewClient(r.docker)
 
 	a := agent.Agent{
-		Update:         agent.NewClientUpdater(r.drone),
-		Logger:         agent.NewClientLogger(r.drone, w.Job.ID, r.config.logs),
-		Engine:         engine,
-		Timeout:        r.config.timeout,
-		Platform:       r.config.platform,
-		Namespace:      r.config.namespace,
-		Escalate:       r.config.privileged,
-		Pull:           r.config.pull,
-		ConcealSecrets: r.config.concealSecrets,
+		Update:    agent.NewClientUpdater(r.drone),
+		Logger:    agent.NewClientLogger(r.drone, w.Job.ID, r.config.logs),
+		Engine:    engine,
+		Timeout:   r.config.timeout,
+		Platform:  r.config.platform,
+		Namespace: r.config.namespace,
+		Escalate:  r.config.privileged,
+		Pull:      r.config.pull,
 	}
 
 	cancelFunc := func(m *stomp.Message) {

drone/exec.go

@@ -48,11 +48,6 @@ var execCmd = cli.Command{
 			Usage:  "build secrets file in KEY=VALUE format",
 			EnvVar: "DRONE_SECRETS_FILE",
 		},
-		cli.BoolFlag{
-			Name:   "conceal-secrets",
-			Usage:  "conceal secrets from build logs",
-			EnvVar: "DRONE_CONCEAL_SECRETS",
-		},
 		cli.StringSliceFlag{
 			Name:   "matrix",
 			Usage:  "build matrix in KEY=VALUE format",
@@ -331,18 +326,17 @@ func exec(c *cli.Context) error {
 	}
 
 	a := agent.Agent{
-		Update:         agent.NoopUpdateFunc,
-		Logger:         agent.TermLoggerFunc,
-		Engine:         engine,
-		Timeout:        c.Duration("timeout.inactivity"),
-		Platform:       "linux/amd64",
-		Namespace:      c.String("namespace"),
-		Disable:        c.StringSlice("plugin"),
-		Escalate:       c.StringSlice("privileged"),
-		Netrc:          []string{},
-		Local:          dir,
-		Pull:           c.Bool("pull"),
-		ConcealSecrets: c.Bool("conceal-secrets"),
+		Update:    agent.NoopUpdateFunc,
+		Logger:    agent.TermLoggerFunc,
+		Engine:    engine,
+		Timeout:   c.Duration("timeout.inactivity"),
+		Platform:  "linux/amd64",
+		Namespace: c.String("namespace"),
+		Disable:   c.StringSlice("plugin"),
+		Escalate:  c.StringSlice("privileged"),
+		Netrc:     []string{},
+		Local:     dir,
+		Pull:      c.Bool("pull"),
 	}
 
 	payload := &model.Work{