Browse Source

add warning when trying to publish env variables with OCI artifact

Signed-off-by: Guillaume Lours <[email protected]>
Guillaume Lours 10 months ago
parent
commit
806ac91cf6

+ 3 - 0
cmd/compose/publish.go

@@ -30,6 +30,7 @@ type publishOptions struct {
 	resolveImageDigests bool
 	ociVersion          string
 	withEnvironment     bool
+	assumeYes           bool
 }
 
 func publishCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Service) *cobra.Command {
@@ -48,6 +49,7 @@ func publishCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Servic
 	flags.BoolVar(&opts.resolveImageDigests, "resolve-image-digests", false, "Pin image tags to digests")
 	flags.StringVar(&opts.ociVersion, "oci-version", "", "OCI image/artifact specification version (automatically determined by default)")
 	flags.BoolVar(&opts.withEnvironment, "with-env", false, "Include environment variables in the published OCI artifact")
+	flags.BoolVarP(&opts.assumeYes, "y", "y", false, `Assume "yes" as answer to all prompts`)
 
 	return cmd
 }
@@ -62,5 +64,6 @@ func runPublish(ctx context.Context, dockerCli command.Cli, backend api.Service,
 		ResolveImageDigests: opts.resolveImageDigests,
 		OCIVersion:          api.OCIVersion(opts.ociVersion),
 		WithEnvironment:     opts.withEnvironment,
+		AssumeYes:           opts.assumeYes,
 	})
 }

+ 1 - 0
docs/reference/compose_alpha_publish.md

@@ -11,6 +11,7 @@ Publish compose application
 | `--oci-version`           | `string` |         | OCI image/artifact specification version (automatically determined by default) |
 | `--resolve-image-digests` | `bool`   |         | Pin image tags to digests                                                      |
 | `--with-env`              | `bool`   |         | Include environment variables in the published OCI artifact                    |
+| `-y`, `--y`               | `bool`   |         | Assume "yes" as answer to all prompts                                          |
 
 
 <!---MARKER_GEN_END-->

+ 11 - 0
docs/reference/docker_compose_alpha_publish.yaml

@@ -35,6 +35,17 @@ options:
       experimentalcli: false
       kubernetes: false
       swarm: false
+    - option: "y"
+      shorthand: "y"
+      value_type: bool
+      default_value: "false"
+      description: Assume "yes" as answer to all prompts
+      deprecated: false
+      hidden: false
+      experimental: false
+      experimentalcli: false
+      kubernetes: false
+      swarm: false
 inherited_options:
     - option: dry-run
       value_type: bool

+ 1 - 0
pkg/api/api.go

@@ -423,6 +423,7 @@ const (
 type PublishOptions struct {
 	ResolveImageDigests bool
 	WithEnvironment     bool
+	AssumeYes           bool
 
 	OCIVersion OCIVersion
 }

+ 58 - 19
pkg/compose/publish.go

@@ -24,9 +24,11 @@ import (
 	"github.com/compose-spec/compose-go/v2/types"
 	"github.com/distribution/reference"
 	"github.com/docker/buildx/util/imagetools"
+	"github.com/docker/cli/cli/command"
 	"github.com/docker/compose/v2/internal/ocipush"
 	"github.com/docker/compose/v2/pkg/api"
 	"github.com/docker/compose/v2/pkg/progress"
+	"github.com/docker/compose/v2/pkg/prompt"
 )
 
 func (s *composeService) Publish(ctx context.Context, project *types.Project, repository string, options api.PublishOptions) error {
@@ -36,10 +38,13 @@ func (s *composeService) Publish(ctx context.Context, project *types.Project, re
 }
 
 func (s *composeService) publish(ctx context.Context, project *types.Project, repository string, options api.PublishOptions) error {
-	err := preChecks(project, options)
+	accept, err := s.preChecks(project, options)
 	if err != nil {
 		return err
 	}
+	if !accept {
+		return nil
+	}
 	err = s.Push(ctx, project, api.PushOptions{IgnoreFailures: true, ImageMandatory: true})
 	if err != nil {
 		return err
@@ -130,31 +135,65 @@ func (s *composeService) generateImageDigestsOverride(ctx context.Context, proje
 	return override.MarshalYAML()
 }
 
-func preChecks(project *types.Project, options api.PublishOptions) error {
-	if !options.WithEnvironment {
-		for _, service := range project.Services {
-			if len(service.EnvFiles) > 0 {
-				return fmt.Errorf("service %q has env_file declared. To avoid leaking sensitive data, "+
-					"you must either explicitly allow the sending of environment variables by using the --with-env flag,"+
-					" or remove sensitive data from your Compose configuration", service.Name)
-			}
-			if len(service.Environment) > 0 {
-				return fmt.Errorf("service %q has environment variable(s) declared. To avoid leaking sensitive data, "+
-					"you must either explicitly allow the sending of environment variables by using the --with-env flag,"+
-					" or remove sensitive data from your Compose configuration", service.Name)
+func (s *composeService) preChecks(project *types.Project, options api.PublishOptions) (bool, error) {
+	envVariables, err := s.checkEnvironmentVariables(project, options)
+	if err != nil {
+		return false, err
+	}
+	if !options.AssumeYes && len(envVariables) > 0 {
+		fmt.Println("you are about to publish environment variables within your OCI artifact.\n" +
+			"please double check that you are not leaking sensitive data")
+		for key, val := range envVariables {
+			_, _ = fmt.Fprintln(s.dockerCli.Out(), "Service/Config ", key)
+			for k, v := range val {
+				_, _ = fmt.Fprintf(s.dockerCli.Out(), "%s=%v\n", k, *v)
 			}
 		}
+		return acceptPublishEnvVariables(s.dockerCli)
+	}
+	return true, nil
+}
+
+func (s *composeService) checkEnvironmentVariables(project *types.Project, options api.PublishOptions) (map[string]types.MappingWithEquals, error) {
+	envVarList := map[string]types.MappingWithEquals{}
+	errorList := map[string][]string{}
+
+	for _, service := range project.Services {
+		if len(service.EnvFiles) > 0 {
+			errorList[service.Name] = append(errorList[service.Name], fmt.Sprintf("service %q has env_file declared.", service.Name))
+		}
+		if len(service.Environment) > 0 {
+			errorList[service.Name] = append(errorList[service.Name], fmt.Sprintf("service %q has environment variable(s) declared.", service.Name))
+			envVarList[service.Name] = service.Environment
+		}
+	}
 
-		for _, config := range project.Configs {
-			if config.Environment != "" {
-				return fmt.Errorf("config %q is declare as an environment variable. To avoid leaking sensitive data, "+
-					"you must either explicitly allow the sending of environment variables by using the --with-env flag,"+
-					" or remove sensitive data from your Compose configuration", config.Name)
+	for _, config := range project.Configs {
+		if config.Environment != "" {
+			errorList[config.Name] = append(errorList[config.Name], fmt.Sprintf("config %q is declare as an environment variable.", config.Name))
+			envVarList[config.Name] = types.NewMappingWithEquals([]string{fmt.Sprintf("%s=%s", config.Name, config.Environment)})
+		}
+	}
+
+	if !options.WithEnvironment && len(errorList) > 0 {
+		errorMsgSuffix := "To avoid leaking sensitive data, you must either explicitly allow the sending of environment variables by using the --with-env flag,\n" +
+			"or remove sensitive data from your Compose configuration"
+		errorMsg := ""
+		for _, errors := range errorList {
+			for _, err := range errors {
+				errorMsg += fmt.Sprintf("%s\n", err)
 			}
 		}
+		return nil, fmt.Errorf("%s%s", errorMsg, errorMsgSuffix)
+
 	}
+	return envVarList, nil
+}
 
-	return nil
+func acceptPublishEnvVariables(cli command.Cli) (bool, error) {
+	msg := "Are you ok to publish these environment variables? [y/N]: "
+	confirm, err := prompt.NewPrompt(cli.In(), cli.Out()).Confirm(msg, false)
+	return confirm, err
 }
 
 func envFileLayers(project *types.Project) []ocipush.Pushable {

+ 11 - 0
pkg/e2e/fixtures/publish/compose-multi-env-config.yml

@@ -0,0 +1,11 @@
+services:
+  serviceA:
+    image: "alpine:3.12"
+    environment:
+      - "FOO=bar"
+  serviceB:
+    image: "alpine:3.12"
+    env_file:
+      - publish.env
+    environment:
+      - "BAR=baz"

+ 1 - 0
pkg/e2e/fixtures/publish/publish.env

@@ -1 +1,2 @@
 FOO=bar
+QUIX=

+ 57 - 3
pkg/e2e/publish_test.go

@@ -31,26 +31,80 @@ func TestPublishChecks(t *testing.T) {
 	t.Run("publish error environment", func(t *testing.T) {
 		res := c.RunDockerComposeCmdNoCheck(t, "-f", "./fixtures/publish/compose-environment.yml",
 			"-p", projectName, "alpha", "publish", "test/test")
-		res.Assert(t, icmd.Expected{ExitCode: 1, Err: `service "serviceA" has environment variable(s) declared. To avoid leaking sensitive data,`})
+		res.Assert(t, icmd.Expected{ExitCode: 1, Err: `service "serviceA" has environment variable(s) declared.
+To avoid leaking sensitive data,`})
 	})
 
 	t.Run("publish error env_file", func(t *testing.T) {
 		res := c.RunDockerComposeCmdNoCheck(t, "-f", "./fixtures/publish/compose-env-file.yml",
 			"-p", projectName, "alpha", "publish", "test/test")
-		res.Assert(t, icmd.Expected{ExitCode: 1, Err: `service "serviceA" has env_file declared. To avoid leaking sensitive data,`})
+		res.Assert(t, icmd.Expected{ExitCode: 1, Err: `service "serviceA" has env_file declared.
+service "serviceA" has environment variable(s) declared.
+To avoid leaking sensitive data,`})
+	})
+
+	t.Run("publish multiple errors env_file and environment", func(t *testing.T) {
+		res := c.RunDockerComposeCmdNoCheck(t, "-f", "./fixtures/publish/compose-multi-env-config.yml",
+			"-p", projectName, "alpha", "publish", "test/test")
+		// we don't in which order the services will be loaded, so we can't predict the order of the error messages
+		assert.Assert(t, strings.Contains(res.Combined(), `service "serviceB" has env_file declared.`), res.Combined())
+		assert.Assert(t, strings.Contains(res.Combined(), `service "serviceB" has environment variable(s) declared.`), res.Combined())
+		assert.Assert(t, strings.Contains(res.Combined(), `service "serviceA" has environment variable(s) declared.`), res.Combined())
+		assert.Assert(t, strings.Contains(res.Combined(), `To avoid leaking sensitive data, you must either explicitly allow the sending of environment variables by using the --with-env flag,
+or remove sensitive data from your Compose configuration
+`), res.Combined())
 	})
 
 	t.Run("publish success environment", func(t *testing.T) {
 		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/publish/compose-environment.yml",
-			"-p", projectName, "alpha", "publish", "test/test", "--with-env", "--dry-run")
+			"-p", projectName, "alpha", "publish", "test/test", "--with-env", "-y", "--dry-run")
 		assert.Assert(t, strings.Contains(res.Combined(), "test/test publishing"), res.Combined())
 		assert.Assert(t, strings.Contains(res.Combined(), "test/test published"), res.Combined())
 	})
 
 	t.Run("publish success env_file", func(t *testing.T) {
 		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/publish/compose-env-file.yml",
+			"-p", projectName, "alpha", "publish", "test/test", "--with-env", "-y", "--dry-run")
+		assert.Assert(t, strings.Contains(res.Combined(), "test/test publishing"), res.Combined())
+		assert.Assert(t, strings.Contains(res.Combined(), "test/test published"), res.Combined())
+	})
+
+	t.Run("publish approve validation message", func(t *testing.T) {
+		cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/publish/compose-env-file.yml",
 			"-p", projectName, "alpha", "publish", "test/test", "--with-env", "--dry-run")
+		cmd.Stdin = strings.NewReader("y\n")
+		res := icmd.RunCmd(cmd)
+		res.Assert(t, icmd.Expected{ExitCode: 0})
+		assert.Assert(t, strings.Contains(res.Combined(), "Are you ok to publish these environment variables? [y/N]:"), res.Combined())
 		assert.Assert(t, strings.Contains(res.Combined(), "test/test publishing"), res.Combined())
 		assert.Assert(t, strings.Contains(res.Combined(), "test/test published"), res.Combined())
 	})
+
+	t.Run("publish refuse validation message", func(t *testing.T) {
+		cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/publish/compose-env-file.yml",
+			"-p", projectName, "alpha", "publish", "test/test", "--with-env", "--dry-run")
+		cmd.Stdin = strings.NewReader("n\n")
+		res := icmd.RunCmd(cmd)
+		res.Assert(t, icmd.Expected{ExitCode: 0})
+		assert.Assert(t, strings.Contains(res.Combined(), "Are you ok to publish these environment variables? [y/N]:"), res.Combined())
+		assert.Assert(t, !strings.Contains(res.Combined(), "test/test publishing"), res.Combined())
+		assert.Assert(t, !strings.Contains(res.Combined(), "test/test published"), res.Combined())
+	})
+
+	t.Run("publish list env variables", func(t *testing.T) {
+		cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/publish/compose-multi-env-config.yml",
+			"-p", projectName, "alpha", "publish", "test/test", "--with-env", "--dry-run")
+		cmd.Stdin = strings.NewReader("n\n")
+		res := icmd.RunCmd(cmd)
+		res.Assert(t, icmd.Expected{ExitCode: 0})
+		assert.Assert(t, strings.Contains(res.Combined(), `you are about to publish environment variables within your OCI artifact.
+please double check that you are not leaking sensitive data`), res.Combined())
+		assert.Assert(t, strings.Contains(res.Combined(), `Service/Config  serviceA
+FOO=bar`), res.Combined())
+		assert.Assert(t, strings.Contains(res.Combined(), `Service/Config  serviceB`), res.Combined())
+		// we don't know in which order the env variables will be loaded
+		assert.Assert(t, strings.Contains(res.Combined(), `FOO=bar`), res.Combined())
+		assert.Assert(t, strings.Contains(res.Combined(), `BAR=baz`), res.Combined())
+		assert.Assert(t, strings.Contains(res.Combined(), `QUIX=`), res.Combined())
+	})
 }