diff --git a/cmd/compose/exec.go b/cmd/compose/exec.go index e68cd0f173..88b6d20a9b 100644 --- a/cmd/compose/exec.go +++ b/cmd/compose/exec.go @@ -82,7 +82,7 @@ func execCommand(p *ProjectOptions, dockerCli command.Cli, backend api.Compose) runCmd.Flags().IntVar(&opts.index, "index", 0, "Index of the container if service has multiple replicas") runCmd.Flags().BoolVarP(&opts.privileged, "privileged", "", false, "Give extended privileges to the process") runCmd.Flags().StringVarP(&opts.user, "user", "u", "", "Run the command as this user") - runCmd.Flags().BoolVarP(&opts.noTty, "no-tty", "T", !dockerCli.Out().IsTerminal(), "Disable pseudo-TTY allocation. By default `docker compose exec` allocates a TTY.") + runCmd.Flags().BoolVarP(&opts.noTty, "no-tty", "T", !dockerCli.Out().IsTerminal(), "Disable pseudo-TTY allocation. By default 'docker compose exec' allocates a TTY.") runCmd.Flags().StringVarP(&opts.workingDir, "workdir", "w", "", "Path to workdir directory for this command") runCmd.Flags().BoolVarP(&opts.interactive, "interactive", "i", true, "Keep STDIN open even if not attached") diff --git a/cmd/formatter/shortcut_windows.go b/cmd/formatter/shortcut_windows.go index c642d069a4..1efa14cc2d 100644 --- a/cmd/formatter/shortcut_windows.go +++ b/cmd/formatter/shortcut_windows.go @@ -22,4 +22,4 @@ package formatter func handleCtrlZ() { // Windows doesn't support SIGSTOP/SIGCONT signals // Ctrl+Z behavior is handled differently by the Windows terminal -} \ No newline at end of file +} diff --git a/docs/reference/compose_exec.md b/docs/reference/compose_exec.md index ab3bbbe352..312219e731 100644 --- a/docs/reference/compose_exec.md +++ b/docs/reference/compose_exec.md @@ -20,7 +20,7 @@ a script. | `--dry-run` | `bool` | | Execute command in dry run mode | | `-e`, `--env` | `stringArray` | | Set environment variables | | `--index` | `int` | `0` | Index of the container if service has multiple replicas | -| `-T`, `--no-tty` | `bool` | `true` | Disable pseudo-TTY allocation. By default `docker compose exec` allocates a TTY. | +| `-T`, `--no-tty` | `bool` | `true` | Disable pseudo-TTY allocation. By default 'docker compose exec' allocates a TTY. | | `--privileged` | `bool` | | Give extended privileges to the process | | `-u`, `--user` | `string` | | Run the command as this user | | `-w`, `--workdir` | `string` | | Path to workdir directory for this command | diff --git a/docs/reference/docker_compose_exec.yaml b/docs/reference/docker_compose_exec.yaml index 749682c96f..66ecfddab8 100644 --- a/docs/reference/docker_compose_exec.yaml +++ b/docs/reference/docker_compose_exec.yaml @@ -63,7 +63,7 @@ options: value_type: bool default_value: "true" description: | - Disable pseudo-TTY allocation. By default `docker compose exec` allocates a TTY. + Disable pseudo-TTY allocation. By default 'docker compose exec' allocates a TTY. deprecated: false hidden: false experimental: false diff --git a/go.mod b/go.mod index 1386dcc462..fa33020b23 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/Microsoft/go-winio v0.6.2 github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d github.com/buger/goterm v1.0.4 - github.com/compose-spec/compose-go/v2 v2.9.0 + github.com/compose-spec/compose-go/v2 v2.9.1 github.com/containerd/containerd/v2 v2.1.4 github.com/containerd/errdefs v1.0.0 github.com/containerd/platforms v1.0.0-rc.1 diff --git a/go.sum b/go.sum index 807b1e602e..f4f31c259b 100644 --- a/go.sum +++ b/go.sum @@ -78,8 +78,8 @@ github.com/cloudflare/cfssl v0.0.0-20180223231731-4e2dcbde5004 h1:lkAMpLVBDaj17e github.com/cloudflare/cfssl v0.0.0-20180223231731-4e2dcbde5004/go.mod h1:yMWuSON2oQp+43nFtAV/uvKQIFpSPerB57DCt9t8sSA= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUoc7Ik9EfrFqcylYqgPZ9ANSbTAntnE= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4= -github.com/compose-spec/compose-go/v2 v2.9.0 h1:UHSv/QHlo6QJtrT4igF1rdORgIUhDo1gWuyJUoiNNIM= -github.com/compose-spec/compose-go/v2 v2.9.0/go.mod h1:Oky9AZGTRB4E+0VbTPZTUu4Kp+oEMMuwZXZtPPVT1iE= +github.com/compose-spec/compose-go/v2 v2.9.1 h1:8UwI+ujNU+9Ffkf/YgAm/qM9/eU7Jn8nHzWG721W4rs= +github.com/compose-spec/compose-go/v2 v2.9.1/go.mod h1:Oky9AZGTRB4E+0VbTPZTUu4Kp+oEMMuwZXZtPPVT1iE= github.com/containerd/cgroups/v3 v3.0.5 h1:44na7Ud+VwyE7LIoJ8JTNQOa549a8543BmzaJHo6Bzo= github.com/containerd/cgroups/v3 v3.0.5/go.mod h1:SA5DLYnXO8pTGYiAHXz94qvLQTKfVM5GEVisn4jpins= github.com/containerd/console v1.0.5 h1:R0ymNeydRqH2DmakFNdmjR2k0t7UPuiOV/N/27/qqsc= diff --git a/internal/oci/resolver.go b/internal/oci/resolver.go index 0f44f94700..94d31421ea 100644 --- a/internal/oci/resolver.go +++ b/internal/oci/resolver.go @@ -20,6 +20,7 @@ import ( "context" "io" "net/url" + "os" "strings" "github.com/containerd/containerd/v2/core/remotes" @@ -50,6 +51,11 @@ func NewResolver(config *configfile.ConfigFile) remotes.Resolver { return auth.Username, auth.Password, nil }), )), + docker.WithPlainHTTP(func(s string) (bool, error) { + // Used for testing **only** + _, b := os.LookupEnv("__TEST__INSECURE__REGISTRY__") + return b, nil + }), ), }) } diff --git a/pkg/compose/apiSocket.go b/pkg/compose/apiSocket.go index 4bd2aabead..1c347528e5 100644 --- a/pkg/compose/apiSocket.go +++ b/pkg/compose/apiSocket.go @@ -45,7 +45,7 @@ func (s *composeService) useAPISocket(project *types.Project) (*types.Project, e return nil, errors.New("use_api_socket can't be used with a Windows Docker Engine") } - creds, err := s.dockerCli.ConfigFile().GetAllCredentials() + creds, err := s.configFile().GetAllCredentials() if err != nil { return nil, fmt.Errorf("resolving credentials failed: %w", err) } diff --git a/pkg/compose/commit.go b/pkg/compose/commit.go index 85d19df7c7..d466679108 100644 --- a/pkg/compose/commit.go +++ b/pkg/compose/commit.go @@ -40,7 +40,7 @@ func (s *composeService) commit(ctx context.Context, projectName string, options return err } - clnt := s.dockerCli.Client() + clnt := s.apiClient() w := progress.ContextWriter(ctx) diff --git a/pkg/compose/compose.go b/pkg/compose/compose.go index 52f2e1b509..0ab377db68 100644 --- a/pkg/compose/compose.go +++ b/pkg/compose/compose.go @@ -37,6 +37,7 @@ import ( "github.com/docker/docker/api/types/volume" "github.com/docker/docker/client" "github.com/jonboulle/clockwork" + "github.com/sirupsen/logrus" "github.com/docker/compose/v2/pkg/api" ) @@ -63,6 +64,13 @@ func NewComposeService(dockerCli command.Cli, options ...Option) api.Compose { for _, option := range options { option(s) } + if s.prompt == nil { + s.prompt = func(message string, defaultValue bool) (bool, error) { + fmt.Println(message) + logrus.Warning("Compose is running without a 'prompt' component to interact with user") + return defaultValue, nil + } + } return s } @@ -92,7 +100,7 @@ type composeService struct { func (s *composeService) Close() error { var errs []error if s.dockerCli != nil { - errs = append(errs, s.dockerCli.Client().Close()) + errs = append(errs, s.apiClient().Close()) } return errors.Join(errs...) } @@ -323,7 +331,7 @@ var runtimeVersion runtimeVersionCache func (s *composeService) RuntimeVersion(ctx context.Context) (string, error) { runtimeVersion.once.Do(func() { - version, err := s.dockerCli.Client().ServerVersion(ctx) + version, err := s.apiClient().ServerVersion(ctx) if err != nil { runtimeVersion.err = err } diff --git a/pkg/compose/export.go b/pkg/compose/export.go index 183b3eee46..73161cf5f9 100644 --- a/pkg/compose/export.go +++ b/pkg/compose/export.go @@ -50,7 +50,7 @@ func (s *composeService) export(ctx context.Context, projectName string, options return fmt.Errorf("failed to export container: %w", err) } - clnt := s.dockerCli.Client() + clnt := s.apiClient() w := progress.ContextWriter(ctx) diff --git a/pkg/compose/publish.go b/pkg/compose/publish.go index b2a9127ae6..ad57d830d4 100644 --- a/pkg/compose/publish.go +++ b/pkg/compose/publish.go @@ -90,8 +90,7 @@ func (s *composeService) publish(ctx context.Context, project *types.Project, re return err } - config := s.dockerCli.ConfigFile() - resolver := oci.NewResolver(config) + resolver := oci.NewResolver(s.configFile()) descriptor, err := oci.PushManifest(ctx, resolver, named, layers, options.OCIVersion) if err != nil { diff --git a/pkg/compose/publish_test.go b/pkg/compose/publish_test.go index 8d2c30c767..e8901c8260 100644 --- a/pkg/compose/publish_test.go +++ b/pkg/compose/publish_test.go @@ -77,7 +77,8 @@ services: MediaType: "application/vnd.docker.compose.file+yaml", Annotations: map[string]string{ "com.docker.compose.file": "compose.yaml", - "com.docker.compose.version": internal.Version}, + "com.docker.compose.version": internal.Version, + }, }, { MediaType: "application/vnd.docker.compose.file+yaml", @@ -98,5 +99,4 @@ services: assert.DeepEqual(t, expectedLayers, layers, cmp.FilterPath(func(path cmp.Path) bool { return !slices.Contains([]string{".Data", ".Digest", ".Size"}, path.String()) }, cmp.Ignore())) - } diff --git a/pkg/compose/pull.go b/pkg/compose/pull.go index 6dfb07bc27..a5cb2a2fb4 100644 --- a/pkg/compose/pull.go +++ b/pkg/compose/pull.go @@ -116,7 +116,7 @@ func (s *composeService) pull(ctx context.Context, project *types.Project, opts idx := i eg.Go(func() error { - _, err := s.pullServiceImage(ctx, service, s.configFile(), w, opts.Quiet, project.Environment["DOCKER_DEFAULT_PLATFORM"]) + _, err := s.pullServiceImage(ctx, service, w, opts.Quiet, project.Environment["DOCKER_DEFAULT_PLATFORM"]) if err != nil { pullErrors[idx] = err if service.Build != nil { @@ -177,9 +177,7 @@ func getUnwrappedErrorMessage(err error) string { return err.Error() } -func (s *composeService) pullServiceImage(ctx context.Context, service types.ServiceConfig, - configFile driver.Auth, w progress.Writer, quietPull bool, defaultPlatform string, -) (string, error) { +func (s *composeService) pullServiceImage(ctx context.Context, service types.ServiceConfig, w progress.Writer, quietPull bool, defaultPlatform string) (string, error) { w.Event(progress.Event{ ID: service.Name, Status: progress.Working, @@ -190,7 +188,7 @@ func (s *composeService) pullServiceImage(ctx context.Context, service types.Ser return "", err } - encodedAuth, err := encodedAuth(ref, configFile) + encodedAuth, err := encodedAuth(ref, s.configFile()) if err != nil { return "", err } @@ -330,7 +328,7 @@ func (s *composeService) pullRequiredImages(ctx context.Context, project *types. var mutex sync.Mutex for name, service := range needPull { eg.Go(func() error { - id, err := s.pullServiceImage(ctx, service, s.configFile(), w, quietPull, project.Environment["DOCKER_DEFAULT_PLATFORM"]) + id, err := s.pullServiceImage(ctx, service, w, quietPull, project.Environment["DOCKER_DEFAULT_PLATFORM"]) mutex.Lock() defer mutex.Unlock() pulledImages[name] = api.ImageSummary{ diff --git a/pkg/compose/push.go b/pkg/compose/push.go index 8d8b3691b0..8ae66bc50e 100644 --- a/pkg/compose/push.go +++ b/pkg/compose/push.go @@ -27,7 +27,6 @@ import ( "github.com/compose-spec/compose-go/v2/types" "github.com/distribution/reference" - "github.com/docker/buildx/driver" "github.com/docker/docker/api/types/image" "github.com/docker/docker/pkg/jsonmessage" "golang.org/x/sync/errgroup" @@ -70,7 +69,7 @@ func (s *composeService) push(ctx context.Context, project *types.Project, optio for _, tag := range tags { eg.Go(func() error { - err := s.pushServiceImage(ctx, tag, s.configFile(), w, options.Quiet) + err := s.pushServiceImage(ctx, tag, w, options.Quiet) if err != nil { if !options.IgnoreFailures { return err @@ -84,13 +83,13 @@ func (s *composeService) push(ctx context.Context, project *types.Project, optio return eg.Wait() } -func (s *composeService) pushServiceImage(ctx context.Context, tag string, configFile driver.Auth, w progress.Writer, quietPush bool) error { +func (s *composeService) pushServiceImage(ctx context.Context, tag string, w progress.Writer, quietPush bool) error { ref, err := reference.ParseNormalizedNamed(tag) if err != nil { return err } - authConfig, err := configFile.GetAuthConfig(registry.GetAuthConfigKey(reference.Domain(ref))) + authConfig, err := s.configFile().GetAuthConfig(registry.GetAuthConfigKey(reference.Domain(ref))) if err != nil { return err } diff --git a/pkg/compose/restart.go b/pkg/compose/restart.go index 4de9622310..c8714be775 100644 --- a/pkg/compose/restart.go +++ b/pkg/compose/restart.go @@ -34,7 +34,7 @@ func (s *composeService) Restart(ctx context.Context, projectName string, option }, s.stdinfo(), "Restarting") } -func (s *composeService) restart(ctx context.Context, projectName string, options api.RestartOptions) error { +func (s *composeService) restart(ctx context.Context, projectName string, options api.RestartOptions) error { //nolint:gocyclo containers, err := s.getContainers(ctx, projectName, oneOffExclude, true) if err != nil { return err @@ -86,6 +86,13 @@ func (s *composeService) restart(ctx context.Context, projectName string, option eg, ctx := errgroup.WithContext(ctx) for _, ctr := range containers.filter(isService(service)) { eg.Go(func() error { + def := project.Services[service] + for _, hook := range def.PreStop { + err = s.runHook(ctx, ctr, def, hook, nil) + if err != nil { + return err + } + } eventName := getContainerProgressName(ctr) w.Event(progress.RestartingEvent(eventName)) timeout := utils.DurationSecondToInt(options.Timeout) @@ -94,6 +101,12 @@ func (s *composeService) restart(ctx context.Context, projectName string, option return err } w.Event(progress.StartedEvent(eventName)) + for _, hook := range def.PostStart { + err = s.runHook(ctx, ctr, def, hook, nil) + if err != nil { + return err + } + } return nil }) } diff --git a/pkg/compose/run.go b/pkg/compose/run.go index 0e454e6a2f..9fafec94b5 100644 --- a/pkg/compose/run.go +++ b/pkg/compose/run.go @@ -97,7 +97,9 @@ func (s *composeService) prepareRun(ctx context.Context, project *types.Project, Add(api.SlugLabel, slug). Add(api.OneoffLabel, "True") - if err := s.ensureImagesExists(ctx, project, opts.Build, opts.QuietPull); err != nil { // all dependencies already checked, but might miss service img + // Only ensure image exists for the target service, dependencies were already handled by startDependencies + buildOpts := prepareBuildOptions(opts) + if err := s.ensureImagesExists(ctx, project, buildOpts, opts.QuietPull); err != nil { // all dependencies already checked, but might miss service img return "", err } @@ -147,6 +149,16 @@ func (s *composeService) prepareRun(ctx context.Context, project *types.Project, return created.ID, err } +func prepareBuildOptions(opts api.RunOptions) *api.BuildOptions { + if opts.Build == nil { + return nil + } + // Create a copy of build options and restrict to only the target service + buildOptsCopy := *opts.Build + buildOptsCopy.Services = []string{opts.Service} + return &buildOptsCopy +} + func applyRunOptions(project *types.Project, service *types.ServiceConfig, opts api.RunOptions) { service.Tty = opts.Tty service.StdinOpen = opts.Interactive diff --git a/pkg/compose/transform/replace.go b/pkg/compose/transform/replace.go index a33959fd2f..8fdaf60b9c 100644 --- a/pkg/compose/transform/replace.go +++ b/pkg/compose/transform/replace.go @@ -104,7 +104,6 @@ func ReplaceEnvFile(in []byte, service string, i int, value string) ([]byte, err } else { return replace(in, envFile.Line, envFile.Column, value), nil } - } func getMapping(root *yaml.Node, key string) (*yaml.Node, error) { diff --git a/pkg/compose/wait.go b/pkg/compose/wait.go index e7628abd21..6cf88b0b3a 100644 --- a/pkg/compose/wait.go +++ b/pkg/compose/wait.go @@ -38,7 +38,7 @@ func (s *composeService) Wait(ctx context.Context, projectName string, options a for _, ctr := range containers { eg.Go(func() error { var err error - resultC, errC := s.dockerCli.Client().ContainerWait(waitCtx, ctr.ID, "") + resultC, errC := s.apiClient().ContainerWait(waitCtx, ctr.ID, "") select { case result := <-resultC: diff --git a/pkg/e2e/compose_run_build_once_test.go b/pkg/e2e/compose_run_build_once_test.go new file mode 100644 index 0000000000..9ec809f44c --- /dev/null +++ b/pkg/e2e/compose_run_build_once_test.go @@ -0,0 +1,103 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package e2e + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "strings" + "testing" + + "gotest.tools/v3/assert" + "gotest.tools/v3/icmd" +) + +// TestRunBuildOnce tests that services with pull_policy: build are only built once +// when using 'docker compose run', even when they are dependencies. +// This addresses a bug where dependencies were built twice: once in startDependencies +// and once in ensureImagesExists. +func TestRunBuildOnce(t *testing.T) { + c := NewCLI(t) + + t.Run("dependency with pull_policy build is built only once", func(t *testing.T) { + projectName := randomProjectName("build-once") + res := c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once.yaml", "down", "--rmi", "local", "--remove-orphans") + res.Assert(t, icmd.Success) + + res = c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once.yaml", "run", "--build", "--rm", "curl") + res.Assert(t, icmd.Success) + + // Count how many times nginx was built by looking for its unique RUN command output + nginxBuilds := strings.Count(res.Combined(), "Building nginx at") + + // nginx should build exactly once, not twice + assert.Equal(t, nginxBuilds, 1, "nginx dependency should build once, but built %d times", nginxBuilds) + assert.Assert(t, strings.Contains(res.Combined(), "curl service")) + + c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once.yaml", "down", "--remove-orphans") + }) + + t.Run("nested dependencies build only once each", func(t *testing.T) { + projectName := randomProjectName("build-nested") + _ = c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once-nested.yaml", "down", "--rmi", "local", "--remove-orphans", "-v") + + res := c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once-nested.yaml", "--verbose", "run", "--build", "--rm", "app") + res.Assert(t, icmd.Success) + + output := res.Combined() + + dbBuildMarker := fmt.Sprintf("naming to docker.io/library/%s-db", projectName) + apiBuildMarker := fmt.Sprintf("naming to docker.io/library/%s-api", projectName) + appBuildMarker := fmt.Sprintf("naming to docker.io/library/%s-app", projectName) + + dbBuilds := strings.Count(output, dbBuildMarker) + apiBuilds := strings.Count(output, apiBuildMarker) + appBuilds := strings.Count(output, appBuildMarker) + + assert.Equal(t, dbBuilds, 1, "db should build once, built %d times\nOutput:\n%s", dbBuilds, output) + assert.Equal(t, apiBuilds, 1, "api should build once, built %d times\nOutput:\n%s", apiBuilds, output) + assert.Equal(t, appBuilds, 1, "app should build once, built %d times\nOutput:\n%s", appBuilds, output) + assert.Assert(t, strings.Contains(output, "App running")) + + c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once-nested.yaml", "down", "--rmi", "local", "--remove-orphans", "-v") + }) + + t.Run("service with no dependencies builds once", func(t *testing.T) { + projectName := randomProjectName("build-simple") + res := c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once-no-deps.yaml", "down", "--rmi", "local", "--remove-orphans") + res.Assert(t, icmd.Success) + + res = c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once-no-deps.yaml", "run", "--build", "--rm", "simple") + res.Assert(t, icmd.Success) + + // Should build exactly once + simpleBuilds := strings.Count(res.Combined(), "Simple service built at") + assert.Equal(t, simpleBuilds, 1, "simple should build once, built %d times", simpleBuilds) + assert.Assert(t, strings.Contains(res.Combined(), "Simple service")) + + c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once-no-deps.yaml", "down", "--remove-orphans") + }) +} + +// randomProjectName generates a unique project name for parallel test execution +// Format: prefix-<8 random hex chars> (e.g., "build-once-3f4a9b2c") +func randomProjectName(prefix string) string { + b := make([]byte, 4) // 4 bytes = 8 hex chars + rand.Read(b) //nolint:errcheck + return fmt.Sprintf("%s-%s", prefix, hex.EncodeToString(b)) +} diff --git a/pkg/e2e/fixtures/publish/oci/compose-override.yaml b/pkg/e2e/fixtures/publish/oci/compose-override.yaml new file mode 100644 index 0000000000..c8947e610a --- /dev/null +++ b/pkg/e2e/fixtures/publish/oci/compose-override.yaml @@ -0,0 +1,3 @@ +services: + app: + env_file: test.env \ No newline at end of file diff --git a/pkg/e2e/fixtures/publish/oci/compose.yaml b/pkg/e2e/fixtures/publish/oci/compose.yaml new file mode 100644 index 0000000000..369094bec1 --- /dev/null +++ b/pkg/e2e/fixtures/publish/oci/compose.yaml @@ -0,0 +1,5 @@ +services: + app: + extends: + file: extends.yaml + service: test \ No newline at end of file diff --git a/pkg/e2e/fixtures/publish/oci/extends.yaml b/pkg/e2e/fixtures/publish/oci/extends.yaml new file mode 100644 index 0000000000..9184173d08 --- /dev/null +++ b/pkg/e2e/fixtures/publish/oci/extends.yaml @@ -0,0 +1,3 @@ +services: + test: + image: alpine \ No newline at end of file diff --git a/pkg/e2e/fixtures/publish/oci/test.env b/pkg/e2e/fixtures/publish/oci/test.env new file mode 100644 index 0000000000..6e1f61b59e --- /dev/null +++ b/pkg/e2e/fixtures/publish/oci/test.env @@ -0,0 +1 @@ +HELLO=WORLD \ No newline at end of file diff --git a/pkg/e2e/fixtures/run-test/build-once-nested.yaml b/pkg/e2e/fixtures/run-test/build-once-nested.yaml new file mode 100644 index 0000000000..4972db5a7b --- /dev/null +++ b/pkg/e2e/fixtures/run-test/build-once-nested.yaml @@ -0,0 +1,32 @@ +services: + # Database service with build + db: + pull_policy: build + build: + dockerfile_inline: | + FROM alpine + RUN echo "DB built at $(date)" > /db-build.txt + CMD sleep 3600 + + # API service that depends on db + api: + pull_policy: build + build: + dockerfile_inline: | + FROM alpine + RUN echo "API built at $(date)" > /api-build.txt + CMD sleep 3600 + depends_on: + - db + + # App service that depends on api (which depends on db) + app: + pull_policy: build + build: + dockerfile_inline: | + FROM alpine + RUN echo "App built at $(date)" > /app-build.txt + CMD echo "App running" + depends_on: + - api + diff --git a/pkg/e2e/fixtures/run-test/build-once-no-deps.yaml b/pkg/e2e/fixtures/run-test/build-once-no-deps.yaml new file mode 100644 index 0000000000..bf53d95167 --- /dev/null +++ b/pkg/e2e/fixtures/run-test/build-once-no-deps.yaml @@ -0,0 +1,10 @@ +services: + # Simple service with no dependencies + simple: + pull_policy: build + build: + dockerfile_inline: | + FROM alpine + RUN echo "Simple service built at $(date)" > /build.txt + CMD echo "Simple service" + diff --git a/pkg/e2e/fixtures/run-test/build-once.yaml b/pkg/e2e/fixtures/run-test/build-once.yaml new file mode 100644 index 0000000000..1d86f875fc --- /dev/null +++ b/pkg/e2e/fixtures/run-test/build-once.yaml @@ -0,0 +1,18 @@ +services: + # Service with pull_policy: build to ensure it always rebuilds + # This is the key to testing the bug - without the fix, this would build twice + nginx: + pull_policy: build + build: + dockerfile_inline: | + FROM alpine + RUN echo "Building nginx at $(date)" > /build-time.txt + CMD sleep 3600 + + # Service that depends on nginx + curl: + image: alpine + depends_on: + - nginx + command: echo "curl service" + diff --git a/pkg/e2e/publish_test.go b/pkg/e2e/publish_test.go index 5b768d45c9..33413d528d 100644 --- a/pkg/e2e/publish_test.go +++ b/pkg/e2e/publish_test.go @@ -17,6 +17,7 @@ package e2e import ( + "fmt" "strings" "testing" @@ -173,3 +174,43 @@ FOO=bar`), out) assert.Assert(t, strings.Contains(output, "Private Key\n\"\": -----BEGIN DSA PRIVATE KEY-----\nwxyz+ABC=\n-----END DSA PRIVATE KEY-----"), output) }) } + +func TestPublish(t *testing.T) { + c := NewParallelCLI(t) + const projectName = "compose-e2e-publish" + const registryName = projectName + "-registry" + c.RunDockerCmd(t, "run", "--name", registryName, "-P", "-d", "registry:3") + port := c.RunDockerCmd(t, "inspect", "--format", `{{ (index (index .NetworkSettings.Ports "5000/tcp") 0).HostPort }}`, registryName).Stdout() + registry := "localhost:" + strings.TrimSpace(port) + t.Cleanup(func() { + c.RunDockerCmd(t, "rm", "--force", registryName) + }) + + cmd := c.NewDockerComposeCmd(t, "-f", "./fixtures/publish/oci/compose.yaml", "-f", "./fixtures/publish/oci/compose-override.yaml", + "-p", projectName, "publish", "--with-env", "--yes", registry+"/test:test") + icmd.RunCmd(cmd, func(cmd *icmd.Cmd) { + cmd.Env = append(cmd.Env, "__TEST__INSECURE__REGISTRY__=true") + }).Assert(t, icmd.Expected{ExitCode: 0}) + + // docker exec -it compose-e2e-publish-registry tree /var/lib/registry/docker/registry/v2/ + + cmd = c.NewDockerComposeCmd(t, "--verbose", "--project-name=oci", "-f", fmt.Sprintf("oci://%s/test:test", registry), "config") + res := icmd.RunCmd(cmd, func(cmd *icmd.Cmd) { + cmd.Env = append(cmd.Env, + "XDG_CACHE_HOME="+t.TempDir(), + "__TEST__INSECURE__REGISTRY__=true") + }) + res.Assert(t, icmd.Expected{ExitCode: 0}) + assert.Equal(t, res.Stdout(), `name: oci +services: + app: + environment: + HELLO: WORLD + image: alpine + networks: + default: null +networks: + default: + name: oci_default +`) +} diff --git a/pkg/remote/git.go b/pkg/remote/git.go index 5d85345ea9..3e01acd585 100644 --- a/pkg/remote/git.go +++ b/pkg/remote/git.go @@ -26,6 +26,7 @@ import ( "path/filepath" "regexp" "strconv" + "strings" "github.com/compose-spec/compose-go/v2/cli" "github.com/compose-spec/compose-go/v2/loader" @@ -113,6 +114,9 @@ func (g gitRemoteLoader) Load(ctx context.Context, path string) (string, error) g.known[path] = local } if ref.SubDir != "" { + if err := validateGitSubDir(local, ref.SubDir); err != nil { + return "", err + } local = filepath.Join(local, ref.SubDir) } stat, err := os.Stat(local) @@ -129,6 +133,41 @@ func (g gitRemoteLoader) Dir(path string) string { return g.known[path] } +// validateGitSubDir ensures a subdirectory path is contained within the base directory +// and doesn't escape via path traversal. Unlike validatePathInBase for OCI artifacts, +// this allows nested directories but prevents traversal outside the base. +func validateGitSubDir(base, subDir string) error { + cleanSubDir := filepath.Clean(subDir) + + if filepath.IsAbs(cleanSubDir) { + return fmt.Errorf("git subdirectory must be relative, got: %s", subDir) + } + + if cleanSubDir == ".." || strings.HasPrefix(cleanSubDir, "../") || strings.HasPrefix(cleanSubDir, "..\\") { + return fmt.Errorf("git subdirectory path traversal detected: %s", subDir) + } + + if len(cleanSubDir) >= 2 && cleanSubDir[1] == ':' { + return fmt.Errorf("git subdirectory must be relative, got: %s", subDir) + } + + targetPath := filepath.Join(base, cleanSubDir) + cleanBase := filepath.Clean(base) + cleanTarget := filepath.Clean(targetPath) + + // Ensure the target starts with the base path + relPath, err := filepath.Rel(cleanBase, cleanTarget) + if err != nil { + return fmt.Errorf("invalid git subdirectory path: %w", err) + } + + if relPath == ".." || strings.HasPrefix(relPath, "../") || strings.HasPrefix(relPath, "..\\") { + return fmt.Errorf("git subdirectory escapes base directory: %s", subDir) + } + + return nil +} + func (g gitRemoteLoader) resolveGitRef(ctx context.Context, path string, ref *gitutil.GitRef) error { if !commitSHA.MatchString(ref.Ref) { cmd := exec.CommandContext(ctx, "git", "ls-remote", "--exit-code", ref.Remote, ref.Ref) diff --git a/pkg/remote/git_test.go b/pkg/remote/git_test.go new file mode 100644 index 0000000000..f78bcc25db --- /dev/null +++ b/pkg/remote/git_test.go @@ -0,0 +1,175 @@ +/* + Copyright 2020 Docker Compose CLI authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package remote + +import ( + "testing" + + "gotest.tools/v3/assert" +) + +func TestValidateGitSubDir(t *testing.T) { + base := "/tmp/cache/compose/abc123def456" + + tests := []struct { + name string + subDir string + wantErr bool + }{ + { + name: "valid simple directory", + subDir: "examples", + wantErr: false, + }, + { + name: "valid nested directory", + subDir: "examples/nginx", + wantErr: false, + }, + { + name: "valid deeply nested directory", + subDir: "examples/web/frontend/config", + wantErr: false, + }, + { + name: "valid current directory", + subDir: ".", + wantErr: false, + }, + { + name: "valid directory with redundant separators", + subDir: "examples//nginx", + wantErr: false, + }, + { + name: "valid directory with dots in name", + subDir: "examples/nginx.conf.d", + wantErr: false, + }, + { + name: "path traversal - parent directory", + subDir: "..", + wantErr: true, + }, + { + name: "path traversal - multiple parent directories", + subDir: "../../../etc/passwd", + wantErr: true, + }, + { + name: "path traversal - deeply nested escape", + subDir: "../../../../../../../tmp/pwned", + wantErr: true, + }, + { + name: "path traversal - mixed with valid path", + subDir: "examples/../../etc/passwd", + wantErr: true, + }, + { + name: "path traversal - at the end", + subDir: "examples/..", + wantErr: false, // This resolves to "." which is the current directory, safe + }, + { + name: "path traversal - in the middle", + subDir: "examples/../../../etc/passwd", + wantErr: true, + }, + { + name: "path traversal - windows style", + subDir: "..\\..\\..\\windows\\system32", + wantErr: true, + }, + { + name: "absolute unix path", + subDir: "/etc/passwd", + wantErr: true, + }, + { + name: "absolute windows path", + subDir: "C:\\windows\\system32\\config\\sam", + wantErr: true, + }, + { + name: "absolute path with home directory", + subDir: "/home/user/.ssh/id_rsa", + wantErr: true, + }, + { + name: "normalized path that would escape", + subDir: "./../../etc/passwd", + wantErr: true, + }, + { + name: "directory name with three dots", + subDir: ".../config", + wantErr: false, + }, + { + name: "directory name with four dots", + subDir: "..../config", + wantErr: false, + }, + { + name: "directory name with five dots", + subDir: "...../etc/passwd", + wantErr: false, // ".....'' is a valid directory name, not path traversal + }, + { + name: "directory name starting with two dots and letter", + subDir: "..foo/bar", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateGitSubDir(base, tt.subDir) + if (err != nil) != tt.wantErr { + t.Errorf("validateGitSubDir(%q, %q) error = %v, wantErr %v", + base, tt.subDir, err, tt.wantErr) + } + }) + } +} + +// TestValidateGitSubDirSecurityScenarios tests specific security scenarios +func TestValidateGitSubDirSecurityScenarios(t *testing.T) { + base := "/var/cache/docker-compose/git/1234567890abcdef" + + // Test the exact vulnerability scenario from the issue + t.Run("CVE scenario - /tmp traversal", func(t *testing.T) { + maliciousPath := "../../../../../../../tmp/pwned" + err := validateGitSubDir(base, maliciousPath) + assert.ErrorContains(t, err, "path traversal") + }) + + // Test variations of the attack + t.Run("CVE scenario - /etc traversal", func(t *testing.T) { + maliciousPath := "../../../../../../../../etc/passwd" + err := validateGitSubDir(base, maliciousPath) + assert.ErrorContains(t, err, "path traversal") + }) + + // Test that legitimate nested paths still work + t.Run("legitimate nested path", func(t *testing.T) { + validPath := "examples/docker-compose/nginx/config" + err := validateGitSubDir(base, validPath) + assert.NilError(t, err) + }) +} diff --git a/pkg/remote/oci.go b/pkg/remote/oci.go index 7e25543e1d..1055993da9 100644 --- a/pkg/remote/oci.go +++ b/pkg/remote/oci.go @@ -179,7 +179,7 @@ func (g ociRemoteLoader) Dir(path string) string { return g.known[path] } -func (g ociRemoteLoader) pullComposeFiles(ctx context.Context, local string, manifest spec.Manifest, ref reference.Named, resolver remotes.Resolver) error { //nolint:gocyclo +func (g ociRemoteLoader) pullComposeFiles(ctx context.Context, local string, manifest spec.Manifest, ref reference.Named, resolver remotes.Resolver) error { err := os.MkdirAll(local, 0o700) if err != nil { return err @@ -223,7 +223,7 @@ func writeComposeFile(layer spec.Descriptor, i int, local string, content []byte return err } } - f, err := os.Create(filepath.Join(local, file)) + f, err := os.OpenFile(filepath.Join(local, file), os.O_RDWR|os.O_CREATE|os.O_APPEND, 0o600) if err != nil { return err } diff --git a/pkg/remote/oci_test.go b/pkg/remote/oci_test.go index 02ae63a876..28a4dbd484 100644 --- a/pkg/remote/oci_test.go +++ b/pkg/remote/oci_test.go @@ -19,6 +19,9 @@ package remote import ( "path/filepath" "testing" + + spec "github.com/opencontainers/image-spec/specs-go/v1" + "gotest.tools/v3/assert" ) func TestValidatePathInBase(t *testing.T) { @@ -84,11 +87,6 @@ func TestValidatePathInBase(t *testing.T) { unsafePath: "..", wantErr: true, }, - { - name: "current directory reference", - unsafePath: "./file.yaml", - wantErr: false, // ./ resolves to base dir - }, { name: "mixed separators", unsafePath: "config/sub\\file.yaml", @@ -104,11 +102,6 @@ func TestValidatePathInBase(t *testing.T) { unsafePath: "file-name_v1.2.3.yaml", wantErr: false, }, - { - name: "single parent then back", - unsafePath: "../compose/file.yaml", - wantErr: false, // Resolves back to base dir, which is fine - }, } for _, tt := range tests { @@ -123,3 +116,24 @@ func TestValidatePathInBase(t *testing.T) { }) } } + +func TestWriteComposeFileWithExtendsPathTraversal(t *testing.T) { + tmpDir := t.TempDir() + + // Create a layer with com.docker.compose.extends=true and a path traversal attempt + layer := spec.Descriptor{ + MediaType: "application/vnd.docker.compose.file.v1+yaml", + Digest: "sha256:test123", + Size: 100, + Annotations: map[string]string{ + "com.docker.compose.extends": "true", + "com.docker.compose.file": "../other", + }, + } + + content := []byte("services:\n test:\n image: nginx\n") + + // writeComposeFile should return an error due to path traversal + err := writeComposeFile(layer, 0, tmpDir, content) + assert.Error(t, err, "invalid OCI artifact") +}