From 9f09b89a34c59a7668b87d9a519fe3a320a7ce1f Mon Sep 17 00:00:00 2001 From: Guillaume Lours <705411+glours@users.noreply.github.com> Date: Tue, 21 Jan 2025 17:50:11 +0100 Subject: [PATCH] add warning when trying to publish env variables with OCI artifact Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com> --- cmd/compose/publish.go | 3 + docs/reference/compose_alpha_publish.md | 1 + .../docker_compose_alpha_publish.yaml | 11 +++ pkg/api/api.go | 1 + pkg/compose/publish.go | 77 ++++++++++++++----- .../publish/compose-multi-env-config.yml | 11 +++ pkg/e2e/fixtures/publish/publish.env | 1 + pkg/e2e/publish_test.go | 60 ++++++++++++++- 8 files changed, 143 insertions(+), 22 deletions(-) create mode 100644 pkg/e2e/fixtures/publish/compose-multi-env-config.yml diff --git a/cmd/compose/publish.go b/cmd/compose/publish.go index 29bc670f6e..22d5aa0941 100644 --- a/cmd/compose/publish.go +++ b/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, }) } diff --git a/docs/reference/compose_alpha_publish.md b/docs/reference/compose_alpha_publish.md index feb3942ee1..7d36553523 100644 --- a/docs/reference/compose_alpha_publish.md +++ b/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 | diff --git a/docs/reference/docker_compose_alpha_publish.yaml b/docs/reference/docker_compose_alpha_publish.yaml index 2e77acfaed..91600ac8b4 100644 --- a/docs/reference/docker_compose_alpha_publish.yaml +++ b/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 diff --git a/pkg/api/api.go b/pkg/api/api.go index a5b889ce3a..2495665a00 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -423,6 +423,7 @@ const ( type PublishOptions struct { ResolveImageDigests bool WithEnvironment bool + AssumeYes bool OCIVersion OCIVersion } diff --git a/pkg/compose/publish.go b/pkg/compose/publish.go index 5814d6df54..b5fdee6d84 100644 --- a/pkg/compose/publish.go +++ b/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 { diff --git a/pkg/e2e/fixtures/publish/compose-multi-env-config.yml b/pkg/e2e/fixtures/publish/compose-multi-env-config.yml new file mode 100644 index 0000000000..35a75eab36 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/pkg/e2e/fixtures/publish/publish.env b/pkg/e2e/fixtures/publish/publish.env index c075a74be9..7a41ee4485 100644 --- a/pkg/e2e/fixtures/publish/publish.env +++ b/pkg/e2e/fixtures/publish/publish.env @@ -1 +1,2 @@ FOO=bar +QUIX= diff --git a/pkg/e2e/publish_test.go b/pkg/e2e/publish_test.go index 2f7ad239c8..52625cfd0e 100644 --- a/pkg/e2e/publish_test.go +++ b/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()) + }) }