Allow listing of directories

With depth limiter, but might have problems with scaling on very big
directories.

Part of: https://gitlab.com/gitlab-org/gitaly/issues/954

7 files changed, 213 insertions, 11 deletions

diff --git a/changelogs/unreleased/zj-list-directories.yml b/changelogs/unreleased/zj-list-directories.yml
new file mode 100644
index 000000000..34864c938
--- /dev/null
+++ b/changelogs/unreleased/zj-list-directories.yml
@@ -0,0 +1,5 @@
+---
+title: Server implementation ListDirectories
+merge_request: 868
+author:
+type: added
diff --git a/internal/helper/repo.go b/internal/helper/repo.go
index a7348075a..cccc9d7d0 100644
--- a/internal/helper/repo.go
+++ b/internal/helper/repo.go
@@ -3,7 +3,6 @@ package helper
 import (
 	"os"
 	"path"
-	"strings"
 
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 
@@ -53,11 +52,7 @@ func GetPath(repo *pb.Repository) (string, error) {
 		return "", err
 	}
 
-	// Disallow directory traversal for security
-	separator := string(os.PathSeparator)
-	if strings.HasPrefix(relativePath, ".."+separator) ||
-		strings.Contains(relativePath, separator+".."+separator) ||
-		strings.HasSuffix(relativePath, separator+"..") {
+	if ContainsPathTraversal(relativePath) {
 		return "", status.Errorf(codes.InvalidArgument, "GetRepoPath: relative path can't contain directory traversal")
 	}
 
diff --git a/internal/helper/security.go b/internal/helper/security.go
new file mode 100644
index 000000000..5742ed4ec
--- /dev/null
+++ b/internal/helper/security.go
@@ -0,0 +1,19 @@
+package helper
+
+import (
+	"os"
+	"strings"
+)
+
+// ContainsPathTraversal checks if the path contains any traversal
+func ContainsPathTraversal(path string) bool {
+	// Disallow directory traversal for security
+	separator := string(os.PathSeparator)
+	if strings.HasPrefix(path, ".."+separator) ||
+		strings.Contains(path, separator+".."+separator) ||
+		strings.HasSuffix(path, separator+"..") {
+		return true
+	}
+
+	return false
+}
diff --git a/internal/helper/security_test.go b/internal/helper/security_test.go
new file mode 100644
index 000000000..cd31d9f73
--- /dev/null
+++ b/internal/helper/security_test.go
@@ -0,0 +1,24 @@
+package helper
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestContainsPathTraversal(t *testing.T) {
+	testCases := []struct {
+		path              string
+		containsTraversal bool
+	}{
+		{"../parent", true},
+		{"subdir/../../parent", true},
+		{"subdir/..", true},
+		{"subdir", false},
+		{"./subdir", false},
+	}
+
+	for _, tc := range testCases {
+		assert.Equal(t, tc.containsTraversal, ContainsPathTraversal(tc.path))
+	}
+}
diff --git a/internal/service/storage/listdirectories.go b/internal/service/storage/listdirectories.go
new file mode 100644
index 000000000..6e51e2390
--- /dev/null
+++ b/internal/service/storage/listdirectories.go
@@ -0,0 +1,76 @@
+package storage
+
+import (
+	"os"
+	"path"
+
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+// This function won't scale, as it walks the dir structure in lexical order,
+// which means that it first will
+func (s *server) ListDirectories(req *pb.ListDirectoriesRequest, stream pb.StorageService_ListDirectoriesServer) error {
+	if helper.ContainsPathTraversal(req.GetPath()) {
+		return status.Error(codes.InvalidArgument, "ListDirectories: path traversal is not allowed")
+	}
+
+	storageDir, err := helper.GetStorageByName(req.StorageName)
+	if err != nil {
+		return status.Errorf(codes.InvalidArgument, "storage lookup failed: %v", err)
+	}
+
+	fullPath := path.Join(storageDir, req.GetPath())
+	fi, err := os.Stat(fullPath)
+	if os.IsNotExist(err) || !fi.IsDir() {
+		return status.Errorf(codes.NotFound, "ListDirectories: %v", err)
+	}
+
+	maxDepth := dirDepth(storageDir) + req.GetDepth()
+	for _, dir := range directoriesInPath(fullPath) {
+		stream.Send(&pb.ListDirectoriesResponse{Paths: recursiveDirs(dir, maxDepth)})
+	}
+
+	return nil
+}
+
+// Depends on the fact that input is always a path to a dir, not a file
+func dirDepth(dir string) uint32 {
+	dirs, _ := path.Split(dir)
+
+	return uint32(len(dirs) + 1)
+}
+
+func recursiveDirs(dir string, maxDepth uint32) []string {
+	var queue, dirs []string
+
+	for len(queue) > 0 {
+		dir := queue[0]
+		queue = queue[1:]
+
+		subDirs := directoriesInPath(dir)
+
+		if dirDepth(dir)+1 <= maxDepth {
+			queue = append(queue, subDirs...)
+		}
+
+		dirs = append(dirs, subDirs...)
+	}
+
+	return dirs
+}
+
+func directoriesInPath(path string) []string {
+	fi, err := os.Open(path)
+	if os.IsNotExist(err) || err != nil {
+		return []string{}
+	}
+
+	// Readdirnames returns an empty slice if an error occurs. Given we can't
+	// recover, we ignore possible errors here
+	dirs, _ := fi.Readdirnames(0)
+
+	return dirs
+}
diff --git a/internal/service/storage/listdirectories_test.go b/internal/service/storage/listdirectories_test.go
new file mode 100644
index 000000000..c4178087b
--- /dev/null
+++ b/internal/service/storage/listdirectories_test.go
@@ -0,0 +1,88 @@
+package storage
+
+import (
+	"io"
+	"os"
+	"os/exec"
+	"path"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+)
+
+func TestListDirectories(t *testing.T) {
+	testDir := path.Join(testStorage.Path, t.Name())
+	require.NoError(t, os.MkdirAll(path.Dir(testDir), 0755))
+
+	repoPaths := []string{
+		"foo/bar1.git",
+		"foo/bar2.git",
+		"baz/foo/qux3.git",
+		"baz/foo/bar1.git",
+	}
+
+	for _, p := range repoPaths {
+		fullPath := path.Join(testStorage.Path, p)
+		require.NoError(t, os.MkdirAll(fullPath, 0755))
+		require.NoError(t, exec.Command("git", "init", "--bare", fullPath).Run())
+	}
+	defer os.RemoveAll(testStorage.Path)
+
+	server, socketPath := runStorageServer(t)
+	defer server.Stop()
+
+	client, conn := newStorageClient(t, socketPath)
+	defer conn.Close()
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	testCases := []struct {
+		path   string
+		depth  uint32
+		subset []string
+	}{
+		{
+			path:   "",
+			depth:  1,
+			subset: []string{"foo", "baz"},
+		},
+		{
+			path:   "",
+			depth:  0,
+			subset: []string{"foo", "baz"},
+		},
+		{
+			path:   "foo",
+			depth:  0,
+			subset: []string{"bar1.git", "foo/bar2.git"},
+		},
+		{
+			path:   "",
+			depth:  5,
+			subset: repoPaths,
+		},
+	}
+
+	for _, tc := range testCases {
+		stream, err := client.ListDirectories(ctx, &pb.ListDirectoriesRequest{StorageName: testStorage.Name, Path: tc.path, Depth: tc.depth})
+
+		var dirs []string
+		for {
+			resp, err := stream.Recv()
+			if err == io.EOF {
+				break
+			}
+
+			require.NoError(t, err)
+			dirs = append(dirs, resp.GetPaths()...)
+		}
+
+		require.NoError(t, err)
+		assert.NotContains(t, dirs, "HEAD")
+		assert.Subset(t, tc.subset, dirs)
+	}
+}
diff --git a/internal/service/storage/server.go b/internal/service/storage/server.go
index 80a068890..76d051a6a 100644
--- a/internal/service/storage/server.go
+++ b/internal/service/storage/server.go
@@ -2,7 +2,6 @@ package storage
 
 import (
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 )
 
 type server struct{}
@@ -11,7 +10,3 @@ type server struct{}
 func NewServer() pb.StorageServiceServer {
 	return &server{}
 }
-
-func (*server) ListDirectories(*pb.ListDirectoriesRequest, pb.StorageService_ListDirectoriesServer) error {
-	return helper.Unimplemented
-}