From 14a860bb990ce82962779f4c0b17f81b491eafdf Mon Sep 17 00:00:00 2001 From: Suleiman Dibirov Date: Thu, 30 Oct 2025 07:55:31 +0200 Subject: [PATCH 1/2] fix(git): Add validation for Git subdirectory paths to prevent traversal Signed-off-by: Suleiman Dibirov --- pkg/remote/git.go | 39 +++++++++ pkg/remote/git_test.go | 175 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+) create mode 100644 pkg/remote/git_test.go diff --git a/pkg/remote/git.go b/pkg/remote/git.go index 5d85345ea99..3e01acd5852 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 00000000000..f78bcc25dbb --- /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) + }) +} From 00824b700aae25cf0b066a2174a4f00e95412456 Mon Sep 17 00:00:00 2001 From: Suleiman Dibirov Date: Thu, 30 Oct 2025 09:05:09 +0200 Subject: [PATCH 2/2] fix compose_run_build_once_test.go Signed-off-by: Suleiman Dibirov --- pkg/e2e/compose_run_build_once_test.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/pkg/e2e/compose_run_build_once_test.go b/pkg/e2e/compose_run_build_once_test.go index c891ca0f9b9..a78440986b8 100644 --- a/pkg/e2e/compose_run_build_once_test.go +++ b/pkg/e2e/compose_run_build_once_test.go @@ -17,6 +17,9 @@ package e2e import ( + "crypto/rand" + "encoding/hex" + "fmt" "strings" "testing" @@ -32,11 +35,11 @@ func TestRunBuildOnce(t *testing.T) { c := NewParallelCLI(t) t.Run("dependency with pull_policy build is built only once", func(t *testing.T) { - projectName := "e2e-run-build-once-single" + 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", "--rm", "curl") + 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 @@ -50,11 +53,11 @@ func TestRunBuildOnce(t *testing.T) { }) t.Run("nested dependencies build only once each", func(t *testing.T) { - projectName := "e2e-run-build-once-nested" + projectName := randomProjectName("build-nested") res := c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once-nested.yaml", "down", "--rmi", "local", "--remove-orphans") res.Assert(t, icmd.Success) - res = c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once-nested.yaml", "run", "--rm", "app") + res = c.RunDockerComposeCmd(t, "-p", projectName, "-f", "./fixtures/run-test/build-once-nested.yaml", "run", "--build", "--rm", "app") res.Assert(t, icmd.Success) output := res.Combined() @@ -73,11 +76,11 @@ func TestRunBuildOnce(t *testing.T) { }) t.Run("service with no dependencies builds once", func(t *testing.T) { - projectName := "e2e-run-build-once-no-deps" + 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", "--rm", "simple") + 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 @@ -88,3 +91,11 @@ func TestRunBuildOnce(t *testing.T) { 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)) +}