pass: return correct error, and ignore empty stores on list

commit 2fc2313 changed the errors returned by the pass credentials-helper to use a errCredentialsNotFound. This error string is used in the client to distinguish a "not found" error from other errors. (see [client.Get][1]). However, there were additional second code-paths that returned a custom error, which would not be detected as a "not found" error, resulting in an error when logging out; Removing login credentials for https://index.docker.io/v1/ WARNING: could not erase credentials: https://index.docker.io/v1/: error erasing credentials - err: exit status 1, out: `error getting credentials - err: exit status 1, out: `no usernames for https://index.docker.io/v1/`` This patch: - updates Pass.Get() to return a errCredentialsNotFound if no credentials were found - updates Pass.List() to not return an error if any of the domains had no credentials stored. [1]: https://github.com/docker/docker-credential-helpers/blob/73b9e5d51f8dc9f598e08a0f2171c5d5a828e76b/client/client.go#L51-L55 Signed-off-by: Sebastiaan van Stijn <[email protected]>
docker · May 9, 2024 · 85a678b · 85a678b
1 parent 73b9e5d
commit 85a678b
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 2 deletions.
diff --git a/pass/pass.go b/pass/pass.go
@@ -158,7 +158,7 @@ func (p Pass) Get(serverURL string) (string, string, error) {
 	}
 
 	if len(usernames) < 1 {
-		return "", "", fmt.Errorf("no usernames for %s", serverURL)
+		return "", "", credentials.NewErrCredentialsNotFound()
 	}
 
 	actual := strings.TrimSuffix(usernames[0].Name(), ".gpg")
@@ -191,7 +191,7 @@ func (p Pass) List() (map[string]string, error) {
 		}
 
 		if len(usernames) < 1 {
-			return nil, fmt.Errorf("no usernames for %s", serverURL)
+			continue
 		}
 
 		resp[string(serverURL)] = strings.TrimSuffix(usernames[0].Name(), ".gpg")

diff --git a/pass/pass_test.go b/pass/pass_test.go
@@ -3,6 +3,9 @@
 package pass
 
 import (
+	"encoding/base64"
+	"os"
+	"path"
 	"strings"
 	"testing"
 
@@ -116,6 +119,63 @@ func TestPassHelperList(t *testing.T) {
 	}
 }
 
+// TestPassHelperListWithEmptyServer verifies that empty directories
+// (servers without credentials) are ignored, but still returns credentials
+// for other servers.
+func TestPassHelperListWithEmptyServer(t *testing.T) {
+	helper := Pass{}
+	if err := helper.checkInitialized(); err != nil {
+		t.Error(err)
+	}
+
+	creds := []*credentials.Credentials{
+		{
+			ServerURL: "https://myreqistry.example.com:2375/v1",
+			Username:  "foo",
+			Secret:    "isthebestmeshuggahalbum",
+		},
+		{
+			ServerURL: "https://index.example.com/v1//access-token",
+		},
+	}
+
+	t.Cleanup(func() {
+		for _, cred := range creds {
+			_ = helper.Delete(cred.ServerURL)
+		}
+	})
+
+	for _, cred := range creds {
+		if cred.Username != "" {
+			if err := helper.Add(cred); err != nil {
+				t.Error(err)
+			}
+		} else {
+			// No credentials; create an empty directory for this server.
+			serverURL := base64.URLEncoding.EncodeToString([]byte(cred.ServerURL))
+			p := path.Join(getPassDir(), PASS_FOLDER, serverURL)
+			if err := os.Mkdir(p, 0o755); err != nil {
+				t.Error(err)
+			}
+		}
+	}
+
+	credsList, err := helper.List()
+	if err != nil {
+		t.Error(err)
+	}
+	if len(credsList) == 0 {
+		t.Error("expected credentials to be returned, but got none")
+	}
+	for _, cred := range creds {
+		if cred.Username != "" {
+			if username := credsList[cred.ServerURL]; username != cred.Username {
+				t.Errorf("expected username %q, actual: %q", cred.Username, username)
+			}
+		}
+	}
+}
+
 func TestMissingCred(t *testing.T) {
 	helper := Pass{}
 	if _, _, err := helper.Get("garbage"); !credentials.IsErrCredentialsNotFound(err) {