Skip to content

Commit

Permalink
add warning when trying to publish env variables with OCI artifact
Browse files Browse the repository at this point in the history
Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com>
  • Loading branch information
glours committed Jan 30, 2025
1 parent 8402888 commit 9f09b89
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 22 deletions.
3 changes: 3 additions & 0 deletions cmd/compose/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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 change: 1 addition & 0 deletions docs/reference/compose_alpha_publish.md
Original file line number Diff line number Diff line change
Expand Up @@ -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-->
Expand Down
11 changes: 11 additions & 0 deletions docs/reference/docker_compose_alpha_publish.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions pkg/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,7 @@ const (
type PublishOptions struct {
ResolveImageDigests bool
WithEnvironment bool
AssumeYes bool

OCIVersion OCIVersion
}
Expand Down
77 changes: 58 additions & 19 deletions pkg/compose/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
11 changes: 11 additions & 0 deletions pkg/e2e/fixtures/publish/compose-multi-env-config.yml
Original file line number Diff line number Diff line change
@@ -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 change: 1 addition & 0 deletions pkg/e2e/fixtures/publish/publish.env
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
FOO=bar
QUIX=
60 changes: 57 additions & 3 deletions pkg/e2e/publish_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
}

0 comments on commit 9f09b89

Please sign in to comment.