From a9fd97d7fea621915938df7edcb205c3633f49b2 Mon Sep 17 00:00:00 2001
From: Tom Chauveau <tom.chauveau@epitech.eu>
Date: Tue, 31 Aug 2021 13:04:16 +0200
Subject: [PATCH] Handle secrets in DockerLogin operation

Before, secret was a plain text string, but it could lead to security issue
so we are now handling secrets as `dagger.#Secret` or string.
I've add a new struct SecretStore that expose the inputStore to easily
retrieve secret value.

Signed-off-by: Tom Chauveau <tom.chauveau@epitech.eu>
---
 client/client.go          | 18 +++++++++---------
 environment/pipeline.go   | 24 +++++++++++++++++++++---
 solver/secretsprovider.go | 15 +++++++++++++--
 solver/solver.go          | 18 +++++++++++-------
 stdlib/dagger/op/op.cue   |  2 +-
 5 files changed, 55 insertions(+), 22 deletions(-)

diff --git a/client/client.go b/client/client.go
index 84ea75e5..e4e339a8 100644
--- a/client/client.go
+++ b/client/client.go
@@ -128,15 +128,15 @@ func (c *Client) buildfn(ctx context.Context, st *state.State, env *environment.
 	// buildkit auth provider (registry)
 	auth := solver.NewRegistryAuthProvider()
 
-	// secrets
-	secrets := solver.NewSecretsProvider(st)
+	// session (secrets & store)
+	secretsStore := solver.NewSecretsStoreProvider(st)
 
 	// Setup solve options
 	opts := bk.SolveOpt{
 		LocalDirs: localdirs,
 		Session: []session.Attachable{
 			auth,
-			secrets,
+			secretsStore.Secrets,
 			solver.NewDockerSocketProvider(),
 		},
 		CacheExports: c.cfg.CacheExports,
@@ -171,12 +171,12 @@ func (c *Client) buildfn(ctx context.Context, st *state.State, env *environment.
 
 	resp, err := c.c.Build(ctx, opts, "", func(ctx context.Context, gw bkgw.Client) (*bkgw.Result, error) {
 		s := solver.New(solver.Opts{
-			Control: c.c,
-			Gateway: gw,
-			Events:  eventsCh,
-			Auth:    auth,
-			Secrets: secrets,
-			NoCache: c.cfg.NoCache,
+			Control:      c.c,
+			Gateway:      gw,
+			Events:       eventsCh,
+			Auth:         auth,
+			SecretsStore: secretsStore,
+			NoCache:      c.cfg.NoCache,
 		})
 
 		// Close events channel
diff --git a/environment/pipeline.go b/environment/pipeline.go
index 0d9dbb92..fab37512 100644
--- a/environment/pipeline.go
+++ b/environment/pipeline.go
@@ -651,9 +651,27 @@ func (p *Pipeline) DockerLogin(ctx context.Context, op *compiler.Value, st llb.S
 		return st, err
 	}
 
-	secret, err := op.Lookup("secret").String()
+	// Inject secret as plain text or retrieve string
+	// FIXME If we could create secret directly in `cue`, we could clean up
+	//  that condition
+	// But currently it's not possible because ECR secret's is a string
+	// so we need to handle both option (string & secrets)
+	secretValue, err := op.Lookup("secret").String()
 	if err != nil {
-		return st, err
+		// Retrieve secret
+		if secret := op.Lookup("secret"); secret.Exists() {
+			id, err := getSecretID(secret)
+			if err != nil {
+				return st, err
+			}
+			secretBytes, err := p.s.GetOptions().SecretsStore.Store.GetSecret(ctx, id)
+			if err != nil {
+				return st, err
+			}
+			secretValue = string(secretBytes)
+		} else {
+			return st, err
+		}
 	}
 
 	target, err := op.Lookup("target").String()
@@ -661,7 +679,7 @@ func (p *Pipeline) DockerLogin(ctx context.Context, op *compiler.Value, st llb.S
 		return st, err
 	}
 
-	p.s.AddCredentials(target, username, secret)
+	p.s.AddCredentials(target, username, secretValue)
 	log.
 		Ctx(ctx).
 		Debug().
diff --git a/solver/secretsprovider.go b/solver/secretsprovider.go
index 67f8436e..f36a159d 100644
--- a/solver/secretsprovider.go
+++ b/solver/secretsprovider.go
@@ -11,8 +11,19 @@ import (
 	"go.dagger.io/dagger/state"
 )
 
-func NewSecretsProvider(st *state.State) session.Attachable {
-	return secretsprovider.NewSecretProvider(&inputStore{st})
+type SecretsStore struct {
+	Secrets session.Attachable
+	Store   *inputStore
+}
+
+func NewSecretsStoreProvider(st *state.State) SecretsStore {
+	store := &inputStore{st}
+
+	return SecretsStore{
+		Secrets: secretsprovider.NewSecretProvider(store),
+		Store:   store,
+	}
+
 }
 
 type inputStore struct {
diff --git a/solver/solver.go b/solver/solver.go
index d167675a..05f890ae 100644
--- a/solver/solver.go
+++ b/solver/solver.go
@@ -26,12 +26,12 @@ type Solver struct {
 }
 
 type Opts struct {
-	Control *bk.Client
-	Gateway bkgw.Client
-	Events  chan *bk.SolveStatus
-	Auth    *RegistryAuthProvider
-	Secrets session.Attachable
-	NoCache bool
+	Control      *bk.Client
+	Gateway      bkgw.Client
+	Events       chan *bk.SolveStatus
+	Auth         *RegistryAuthProvider
+	SecretsStore SecretsStore
+	NoCache      bool
 }
 
 func New(opts Opts) Solver {
@@ -61,6 +61,10 @@ func invalidateCache(def *llb.Definition) error {
 	return nil
 }
 
+func (s Solver) GetOptions() Opts {
+	return s.opts
+}
+
 func (s Solver) NoCache() bool {
 	return s.opts.NoCache
 }
@@ -189,7 +193,7 @@ func (s Solver) Export(ctx context.Context, st llb.State, img *dockerfile2llb.Im
 		Exports: []bk.ExportEntry{output},
 		Session: []session.Attachable{
 			s.opts.Auth,
-			s.opts.Secrets,
+			s.opts.SecretsStore.Secrets,
 			NewDockerSocketProvider(),
 		},
 	}
diff --git a/stdlib/dagger/op/op.cue b/stdlib/dagger/op/op.cue
index 81adc6e5..a7671bbc 100644
--- a/stdlib/dagger/op/op.cue
+++ b/stdlib/dagger/op/op.cue
@@ -68,7 +68,7 @@ package op
 	target:   string
 	username: string
 	// FIXME: should be a #Secret (circular import)
-	secret: string | bytes
+	secret: _ @dagger(secret)
 }
 
 #FetchContainer: {