Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions pkg/e2e/compose_run_build_once_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
package e2e

import (
"crypto/rand"
"encoding/hex"
"fmt"
"strings"
"testing"

Expand All @@ -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
Expand All @@ -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()
Expand All @@ -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
Expand All @@ -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))
}
39 changes: 39 additions & 0 deletions pkg/remote/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
175 changes: 175 additions & 0 deletions pkg/remote/git_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
Loading