From 6de3d71ab6c829b4eb504305abacd6242fca5b6d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= <pawel.gronowski@docker.com>
Date: Thu, 28 Nov 2024 12:45:19 +0100
Subject: [PATCH] cli-plugins: Fix searching inaccessible directories
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fix a case where one inaccessible plugin search path stops the whole
search and prevents latter paths from being scanned.

Remove a preliminary `Stat` call that verifies whether path is an actual
directory and is accessible.
It's unneeded and doesn't actually check whether the directory can be
listed or not.
`os.ReadDir` will fail in such case anyway, so just attempt to do that
and ignore any encountered error, instead of erroring out the whole
plugin candidate listing.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
---
 cli-plugins/manager/manager.go      | 10 +++-------
 cli-plugins/manager/manager_test.go | 24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/cli-plugins/manager/manager.go b/cli-plugins/manager/manager.go
index f9229c5257..ef4b489606 100644
--- a/cli-plugins/manager/manager.go
+++ b/cli-plugins/manager/manager.go
@@ -77,8 +77,10 @@ func getPluginDirs(cfg *configfile.ConfigFile) ([]string, error) {
 
 func addPluginCandidatesFromDir(res map[string][]string, d string) error {
 	dentries, err := os.ReadDir(d)
+	// Silently ignore any directories which we cannot list (e.g. due to
+	// permissions or anything else) or which is not a directory
 	if err != nil {
-		return err
+		return nil
 	}
 	for _, dentry := range dentries {
 		switch dentry.Type() & os.ModeType {
@@ -106,12 +108,6 @@ func addPluginCandidatesFromDir(res map[string][]string, d string) error {
 func listPluginCandidates(dirs []string) (map[string][]string, error) {
 	result := make(map[string][]string)
 	for _, d := range dirs {
-		// Silently ignore any directories which we cannot
-		// Stat (e.g. due to permissions or anything else) or
-		// which is not a directory.
-		if fi, err := os.Stat(d); err != nil || !fi.IsDir() {
-			continue
-		}
 		if err := addPluginCandidatesFromDir(result, d); err != nil {
 			// Silently ignore paths which don't exist.
 			if os.IsNotExist(err) {
diff --git a/cli-plugins/manager/manager_test.go b/cli-plugins/manager/manager_test.go
index 1583420a48..42945cc4e3 100644
--- a/cli-plugins/manager/manager_test.go
+++ b/cli-plugins/manager/manager_test.go
@@ -82,6 +82,30 @@ func TestListPluginCandidates(t *testing.T) {
 	assert.DeepEqual(t, candidates, exp)
 }
 
+// Regression test for https://github.com/docker/cli/issues/5643.
+// Check that inaccessible directories that come before accessible ones are ignored
+// and do not prevent the latter from being processed.
+func TestListPluginCandidatesInaccesibleDir(t *testing.T) {
+	dir := fs.NewDir(t, t.Name(),
+		fs.WithDir("no-perm", fs.WithMode(0)),
+		fs.WithDir("plugins",
+			fs.WithFile("docker-buildx", ""),
+		),
+	)
+	defer dir.Remove()
+
+	candidates, err := listPluginCandidates([]string{
+		dir.Join("no-perm"),
+		dir.Join("plugins"),
+	})
+	assert.NilError(t, err)
+	assert.DeepEqual(t, candidates, map[string][]string{
+		"buildx": {
+			dir.Join("plugins", "docker-buildx"),
+		},
+	})
+}
+
 func TestGetPlugin(t *testing.T) {
 	dir := fs.NewDir(t, t.Name(),
 		fs.WithFile("docker-bbb", `