fix(git): avoid cloning to memory and bypassing symlinking restriction BE-13115 (#2961)

.golangci.yaml

@@ -83,7 +83,7 @@ linters:
         - ruleguard
       settings:
         ruleguard:
-          rules: './analysis/ssrf.go'
+          rules: './analysis/ssrf.go,./analysis/git.go'
     forbidigo:
       forbid:
         - pattern: ^tls\.Config$
@@ -94,6 +94,8 @@ linters:
           msg: 'Not allowed because of FIPS mode'
         - pattern: ^(types\.SystemContext\.)?(DockerDaemonInsecureSkipTLSVerify|DockerInsecureSkipTLSVerify|OCIInsecureSkipTLSVerify)$
           msg: 'Not allowed because of FIPS mode'
+        - pattern: ^git\.PlainClone(Context|WithOptions)?$
+          msg: Use git.CloneContext with NewNoSymlinkFS to prevent symlink traversal attacks
       analyze-types: true
   exclusions:
     generated: lax

analysis/git.go

@@ -0,0 +1,18 @@
+//go:build ignore
+
+package gorules
+
+import "github.com/quasilyte/go-ruleguard/dsl"
+
+// inMemoryCloneWithWorktree flags git clone calls that use memory.NewStorage() as
+// the storer while also writing files to a real worktree. This holds all git objects
+// in heap for the duration of the clone, which is unbounded for user-supplied repos.
+func inMemoryCloneWithWorktree(m dsl.Matcher) {
+	m.Match(`git.CloneContext($_, memory.NewStorage(), $wt, $_)`).
+		Where(m["wt"].Text != "nil").
+		Report(`git.CloneContext with memory.NewStorage() holds all git objects in heap; use gogitfs.NewStorage with a filesystem storer instead`)
+
+	m.Match(`git.Clone(memory.NewStorage(), $wt, $_)`).
+		Where(m["wt"].Text != "nil").
+		Report(`git.Clone with memory.NewStorage() holds all git objects in heap; use gogitfs.NewStorage with a filesystem storer instead`)
+}

api/git/git.go

@@ -2,18 +2,24 @@ package git
 
 import (
 	"context"
+	"os"
+	"path/filepath"
 	"strings"
 
+	"github.com/portainer/portainer/api/filesystem"
 	gittypes "github.com/portainer/portainer/api/git/types"
 
 	"github.com/go-git/go-billy/v5"
 	"github.com/go-git/go-billy/v5/osfs"
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/config"
+	"github.com/go-git/go-git/v5/plumbing/cache"
 	"github.com/go-git/go-git/v5/plumbing/filemode"
 	"github.com/go-git/go-git/v5/plumbing/object"
+	gogitfs "github.com/go-git/go-git/v5/storage/filesystem"
 	"github.com/go-git/go-git/v5/storage/memory"
 	"github.com/pkg/errors"
+	"github.com/rs/zerolog/log"
 )
 
 // noSymlinkFS wraps a billy.Filesystem and rejects symlink creation to prevent
@@ -42,28 +48,35 @@ func NewGitClient(preserveGitDir bool) *gitClient {
 }
 
 func (c *gitClient) Download(ctx context.Context, dst string, opt *git.CloneOptions) error {
-	if c.preserveGitDirectory {
-		_, err := git.PlainCloneContext(ctx, dst, false, opt)
-		if err != nil {
-			if err.Error() == "authentication required" {
-				return gittypes.ErrAuthenticationFailure
-			}
-			return errors.Wrap(err, "failed to clone git repository")
-		}
-		return nil
+	resolved, err := filepath.EvalSymlinks(dst)
+	if err != nil && !errors.Is(err, os.ErrNotExist) {
+		return errors.Wrap(err, "failed to resolve destination path")
+	}
+	if err == nil {
+		dst = resolved
 	}
 
-	// Memory storage avoids a macOS filesystem conflict where go-git's init
-	// creates dst/.git as a directory before checkout, causing EISDIR errors
-	// that mask ErrSymlinkDetected from noSymlinkFS.
 	wt := NewNoSymlinkFS(osfs.New(dst))
-	_, err := git.CloneContext(ctx, memory.NewStorage(), wt, opt)
+	dot := osfs.New(filesystem.JoinPaths(dst, ".git"))
+	storer := gogitfs.NewStorage(dot, cache.NewObjectLRU(0))
+
+	_, err = git.CloneContext(ctx, storer, wt, opt)
 	if err != nil {
 		if err.Error() == "authentication required" {
 			return gittypes.ErrAuthenticationFailure
 		}
+
 		return errors.Wrap(err, "failed to clone git repository")
 	}
+
+	if c.preserveGitDirectory {
+		return nil
+	}
+
+	if err := os.RemoveAll(filesystem.JoinPaths(dst, ".git")); err != nil {
+		log.Error().Err(err).Msg("failed to remove .git directory")
+	}
+
 	return nil
 }
 

api/git/git_test.go

@@ -99,6 +99,19 @@ func Test_ClonePublicRepository_NoGitDirectory(t *testing.T) {
 	assert.NoDirExists(t, filesystem.JoinPaths(dir, ".git"))
 }
 
+func Test_ClonePublicRepository_NonExistentDst(t *testing.T) {
+	t.Parallel()
+	service := Service{git: NewGitClient(false)}
+	repositoryURL := setup(t)
+	referenceName := "refs/heads/main"
+
+	dir := filesystem.JoinPaths(t.TempDir(), "sub", "dir")
+	err := service.CloneRepository(t.Context(), dir, repositoryURL, referenceName, "", "", false)
+	require.NoError(t, err)
+	assert.DirExists(t, dir)
+	assert.NoDirExists(t, filesystem.JoinPaths(dir, ".git"))
+}
+
 func Test_latestCommitID(t *testing.T) {
 	t.Parallel()
 	service := Service{git: NewGitClient(true)} // no need for http client since the test access the repo via file system.
@@ -262,6 +275,7 @@ func createBareRepoWithSymlink(t *testing.T) string {
 }
 
 func Test_Download_RejectsSymlink(t *testing.T) {
+	t.Parallel()
 	client := NewGitClient(false)
 	repoURL := createBareRepoWithSymlink(t)