Skip to content

Commit 806ac91

Browse files
gloursglours
committed
add warning when trying to publish env variables with OCI artifact
Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com>
1 parent 1c073c0 commit 806ac91

File tree

8 files changed

+143
-22
lines changed

8 files changed

+143
-22
lines changed

‎cmd/compose/publish.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type publishOptions struct {
3030
resolveImageDigests bool
3131
ociVersion string
3232
withEnvironment bool
33+
assumeYes bool
3334
}
3435

3536
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
4849
flags.BoolVar(&opts.resolveImageDigests, "resolve-image-digests", false, "Pin image tags to digests")
4950
flags.StringVar(&opts.ociVersion, "oci-version", "", "OCI image/artifact specification version (automatically determined by default)")
5051
flags.BoolVar(&opts.withEnvironment, "with-env", false, "Include environment variables in the published OCI artifact")
52+
flags.BoolVarP(&opts.assumeYes, "y", "y", false, `Assume "yes" as answer to all prompts`)
5153

5254
return cmd
5355
}
@@ -62,5 +64,6 @@ func runPublish(ctx context.Context, dockerCli command.Cli, backend api.Service,
6264
ResolveImageDigests: opts.resolveImageDigests,
6365
OCIVersion: api.OCIVersion(opts.ociVersion),
6466
WithEnvironment: opts.withEnvironment,
67+
AssumeYes: opts.assumeYes,
6568
})
6669
}

‎docs/reference/compose_alpha_publish.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Publish compose application
1111
| `--oci-version` | `string` | | OCI image/artifact specification version (automatically determined by default) |
1212
| `--resolve-image-digests` | `bool` | | Pin image tags to digests |
1313
| `--with-env` | `bool` | | Include environment variables in the published OCI artifact |
14+
| `-y`, `--y` | `bool` | | Assume "yes" as answer to all prompts |
1415

1516

1617
<!---MARKER_GEN_END-->

‎docs/reference/docker_compose_alpha_publish.yaml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,17 @@ options:
3535
experimentalcli: false
3636
kubernetes: false
3737
swarm: false
38+
- option: "y"
39+
shorthand: "y"
40+
value_type: bool
41+
default_value: "false"
42+
description: Assume "yes" as answer to all prompts
43+
deprecated: false
44+
hidden: false
45+
experimental: false
46+
experimentalcli: false
47+
kubernetes: false
48+
swarm: false
3849
inherited_options:
3950
- option: dry-run
4051
value_type: bool

‎pkg/api/api.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,7 @@ const (
423423
type PublishOptions struct {
424424
ResolveImageDigests bool
425425
WithEnvironment bool
426+
AssumeYes bool
426427

427428
OCIVersion OCIVersion
428429
}

‎pkg/compose/publish.go‎

Lines changed: 58 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@ import (
2424
"github.com/compose-spec/compose-go/v2/types"
2525
"github.com/distribution/reference"
2626
"github.com/docker/buildx/util/imagetools"
27+
"github.com/docker/cli/cli/command"
2728
"github.com/docker/compose/v2/internal/ocipush"
2829
"github.com/docker/compose/v2/pkg/api"
2930
"github.com/docker/compose/v2/pkg/progress"
31+
"github.com/docker/compose/v2/pkg/prompt"
3032
)
3133

3234
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
3638
}
3739

3840
func (s *composeService) publish(ctx context.Context, project *types.Project, repository string, options api.PublishOptions) error {
39-
err := preChecks(project, options)
41+
accept, err := s.preChecks(project, options)
4042
if err != nil {
4143
return err
4244
}
45+
if !accept {
46+
return nil
47+
}
4348
err = s.Push(ctx, project, api.PushOptions{IgnoreFailures: true, ImageMandatory: true})
4449
if err != nil {
4550
return err
@@ -130,31 +135,65 @@ func (s *composeService) generateImageDigestsOverride(ctx context.Context, proje
130135
return override.MarshalYAML()
131136
}
132137

133-
func preChecks(project *types.Project, options api.PublishOptions) error {
134-
if !options.WithEnvironment {
135-
for _, service := range project.Services {
136-
if len(service.EnvFiles) > 0 {
137-
return fmt.Errorf("service %q has env_file declared. To avoid leaking sensitive data, "+
138-
"you must either explicitly allow the sending of environment variables by using the --with-env flag,"+
139-
" or remove sensitive data from your Compose configuration", service.Name)
140-
}
141-
if len(service.Environment) > 0 {
142-
return fmt.Errorf("service %q has environment variable(s) declared. To avoid leaking sensitive data, "+
143-
"you must either explicitly allow the sending of environment variables by using the --with-env flag,"+
144-
" or remove sensitive data from your Compose configuration", service.Name)
138+
func (s *composeService) preChecks(project *types.Project, options api.PublishOptions) (bool, error) {
139+
envVariables, err := s.checkEnvironmentVariables(project, options)
140+
if err != nil {
141+
return false, err
142+
}
143+
if !options.AssumeYes && len(envVariables) > 0 {
144+
fmt.Println("you are about to publish environment variables within your OCI artifact.\n" +
145+
"please double check that you are not leaking sensitive data")
146+
for key, val := range envVariables {
147+
_, _ = fmt.Fprintln(s.dockerCli.Out(), "Service/Config ", key)
148+
for k, v := range val {
149+
_, _ = fmt.Fprintf(s.dockerCli.Out(), "%s=%v\n", k, *v)
145150
}
146151
}
152+
return acceptPublishEnvVariables(s.dockerCli)
153+
}
154+
return true, nil
155+
}
156+
157+
func (s *composeService) checkEnvironmentVariables(project *types.Project, options api.PublishOptions) (map[string]types.MappingWithEquals, error) {
158+
envVarList := map[string]types.MappingWithEquals{}
159+
errorList := map[string][]string{}
160+
161+
for _, service := range project.Services {
162+
if len(service.EnvFiles) > 0 {
163+
errorList[service.Name] = append(errorList[service.Name], fmt.Sprintf("service %q has env_file declared.", service.Name))
164+
}
165+
if len(service.Environment) > 0 {
166+
errorList[service.Name] = append(errorList[service.Name], fmt.Sprintf("service %q has environment variable(s) declared.", service.Name))
167+
envVarList[service.Name] = service.Environment
168+
}
169+
}
147170

148-
for _, config := range project.Configs {
149-
if config.Environment != "" {
150-
return fmt.Errorf("config %q is declare as an environment variable. To avoid leaking sensitive data, "+
151-
"you must either explicitly allow the sending of environment variables by using the --with-env flag,"+
152-
" or remove sensitive data from your Compose configuration", config.Name)
171+
for _, config := range project.Configs {
172+
if config.Environment != "" {
173+
errorList[config.Name] = append(errorList[config.Name], fmt.Sprintf("config %q is declare as an environment variable.", config.Name))
174+
envVarList[config.Name] = types.NewMappingWithEquals([]string{fmt.Sprintf("%s=%s", config.Name, config.Environment)})
175+
}
176+
}
177+
178+
if !options.WithEnvironment && len(errorList) > 0 {
179+
errorMsgSuffix := "To avoid leaking sensitive data, you must either explicitly allow the sending of environment variables by using the --with-env flag,\n" +
180+
"or remove sensitive data from your Compose configuration"
181+
errorMsg := ""
182+
for _, errors := range errorList {
183+
for _, err := range errors {
184+
errorMsg += fmt.Sprintf("%s\n", err)
153185
}
154186
}
187+
return nil, fmt.Errorf("%s%s", errorMsg, errorMsgSuffix)
188+
155189
}
190+
return envVarList, nil
191+
}
156192

157-
return nil
193+
func acceptPublishEnvVariables(cli command.Cli) (bool, error) {
194+
msg := "Are you ok to publish these environment variables? [y/N]: "
195+
confirm, err := prompt.NewPrompt(cli.In(), cli.Out()).Confirm(msg, false)
196+
return confirm, err
158197
}
159198

160199
func envFileLayers(project *types.Project) []ocipush.Pushable {

‎pkg/e2e/fixtures/publish/compose-multi-env-config.yml‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
services:
2+
serviceA:
3+
image: "alpine:3.12"
4+
environment:
5+
- "FOO=bar"
6+
serviceB:
7+
image: "alpine:3.12"
8+
env_file:
9+
- publish.env
10+
environment:
11+
- "BAR=baz"

‎pkg/e2e/fixtures/publish/publish.env‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,2 @@
11
FOO=bar
2+
QUIX=

‎pkg/e2e/publish_test.go‎

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,26 +31,80 @@ func TestPublishChecks(t *testing.T) {
3131
t.Run("publish error environment", func(t *testing.T) {
3232
res := c.RunDockerComposeCmdNoCheck(t, "-f", "./fixtures/publish/compose-environment.yml",
3333
"-p", projectName, "alpha", "publish", "test/test")
34-
res.Assert(t, icmd.Expected{ExitCode: 1, Err: `service "serviceA" has environment variable(s) declared. To avoid leaking sensitive data,`})
34+
res.Assert(t, icmd.Expected{ExitCode: 1, Err: `service "serviceA" has environment variable(s) declared.
35+
To avoid leaking sensitive data,`})
3536
})
3637

3738
t.Run("publish error env_file", func(t *testing.T) {
3839
res := c.RunDockerComposeCmdNoCheck(t, "-f", "./fixtures/publish/compose-env-file.yml",
3940
"-p", projectName, "alpha", "publish", "test/test")
40-
res.Assert(t, icmd.Expected{ExitCode: 1, Err: `service "serviceA" has env_file declared. To avoid leaking sensitive data,`})
41+
res.Assert(t, icmd.Expected{ExitCode: 1, Err: `service "serviceA" has env_file declared.
42+
service "serviceA" has environment variable(s) declared.
43+
To avoid leaking sensitive data,`})
44+
})
45+
46+
t.Run("publish multiple errors env_file and environment", func(t *testing.T) {
47+
res := c.RunDockerComposeCmdNoCheck(t, "-f", "./fixtures/publish/compose-multi-env-config.yml",
48+
"-p", projectName, "alpha", "publish", "test/test")
49+
// we don't in which order the services will be loaded, so we can't predict the order of the error messages
50+
assert.Assert(t, strings.Contains(res.Combined(), `service "serviceB" has env_file declared.`), res.Combined())
51+
assert.Assert(t, strings.Contains(res.Combined(), `service "serviceB" has environment variable(s) declared.`), res.Combined())
52+
assert.Assert(t, strings.Contains(res.Combined(), `service "serviceA" has environment variable(s) declared.`), res.Combined())
53+
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,
54+
or remove sensitive data from your Compose configuration
55+
`), res.Combined())
4156
})
4257

4358
t.Run("publish success environment", func(t *testing.T) {
4459
res := c.RunDockerComposeCmd(t, "-f", "./fixtures/publish/compose-environment.yml",
45-
"-p", projectName, "alpha", "publish", "test/test", "--with-env", "--dry-run")
60+
"-p", projectName, "alpha", "publish", "test/test", "--with-env", "-y", "--dry-run")
4661
assert.Assert(t, strings.Contains(res.Combined(), "test/test publishing"), res.Combined())
4762
assert.Assert(t, strings.Contains(res.Combined(), "test/test published"), res.Combined())
4863
})
4964

5065
t.Run("publish success env_file", func(t *testing.T) {
5166
res := c.RunDockerComposeCmd(t, "-f", "./fixtures/publish/compose-env-file.yml",
67+
"-p", projectName, "alpha", "publish", "test/test", "--with-env", "-y", "--dry-run")
68+
assert.Assert(t, strings.Contains(res.Combined(), "test/test publishing"), res.Combined())
69+
assert.Assert(t, strings.Contains(res.Combined(), "test/test published"), res.Combined())
70+
})
71+
72+
t.Run("publish approve validation message", func(t *testing.T) {
73+
cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/publish/compose-env-file.yml",
5274
"-p", projectName, "alpha", "publish", "test/test", "--with-env", "--dry-run")
75+
cmd.Stdin = strings.NewReader("y\n")
76+
res := icmd.RunCmd(cmd)
77+
res.Assert(t, icmd.Expected{ExitCode: 0})
78+
assert.Assert(t, strings.Contains(res.Combined(), "Are you ok to publish these environment variables? [y/N]:"), res.Combined())
5379
assert.Assert(t, strings.Contains(res.Combined(), "test/test publishing"), res.Combined())
5480
assert.Assert(t, strings.Contains(res.Combined(), "test/test published"), res.Combined())
5581
})
82+
83+
t.Run("publish refuse validation message", func(t *testing.T) {
84+
cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/publish/compose-env-file.yml",
85+
"-p", projectName, "alpha", "publish", "test/test", "--with-env", "--dry-run")
86+
cmd.Stdin = strings.NewReader("n\n")
87+
res := icmd.RunCmd(cmd)
88+
res.Assert(t, icmd.Expected{ExitCode: 0})
89+
assert.Assert(t, strings.Contains(res.Combined(), "Are you ok to publish these environment variables? [y/N]:"), res.Combined())
90+
assert.Assert(t, !strings.Contains(res.Combined(), "test/test publishing"), res.Combined())
91+
assert.Assert(t, !strings.Contains(res.Combined(), "test/test published"), res.Combined())
92+
})
93+
94+
t.Run("publish list env variables", func(t *testing.T) {
95+
cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/publish/compose-multi-env-config.yml",
96+
"-p", projectName, "alpha", "publish", "test/test", "--with-env", "--dry-run")
97+
cmd.Stdin = strings.NewReader("n\n")
98+
res := icmd.RunCmd(cmd)
99+
res.Assert(t, icmd.Expected{ExitCode: 0})
100+
assert.Assert(t, strings.Contains(res.Combined(), `you are about to publish environment variables within your OCI artifact.
101+
please double check that you are not leaking sensitive data`), res.Combined())
102+
assert.Assert(t, strings.Contains(res.Combined(), `Service/Config serviceA
103+
FOO=bar`), res.Combined())
104+
assert.Assert(t, strings.Contains(res.Combined(), `Service/Config serviceB`), res.Combined())
105+
// we don't know in which order the env variables will be loaded
106+
assert.Assert(t, strings.Contains(res.Combined(), `FOO=bar`), res.Combined())
107+
assert.Assert(t, strings.Contains(res.Combined(), `BAR=baz`), res.Combined())
108+
assert.Assert(t, strings.Contains(res.Combined(), `QUIX=`), res.Combined())
109+
})
56110
}

0 commit comments

Comments
 (0)