fixup! Add first cut of git-credential-fdoss

Do not return (get) or delete (erase) secrets unless the attributes
provided match exactly. It is possible we match a subset (if a secret is
stored with a path but you try to delete it without a path, for
example), so ensure we have an exact match before returning or deleting
it.

Change the "expectSecret" to "expectedPassword" in Delete and only
compare the stored secret up to the first newline. We should not need to
match on other volatile fields (expiry, refresh token).

Do not require host AND path when validating "erase". That was silly.
Validation for "erase" now matches the other operations.

dbus.go

@@ -1,8 +1,10 @@
+//nolint:err113 // dynamic errors in main are OK
 package main
 
 import (
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/godbus/dbus/v5"
 )
@@ -85,10 +87,11 @@ func (ss *SecretService) Close() error {
 // The attributes are a set of arbitrary name/value strings that were provided
 // when the secret was stored.
 //
-// If more that one unlocked secret matches the attributes, a diagnostic error
-// will be printed on stderr and the first matching secret will be returned. It
+// Only secrets that match on all attributes and have no extra attributes are
+// considered. If there are multiple exact matches, the first is returned. It
 // is not clear what the ordering of the secrets is, so the "first" secret may
-// be arbitrary.
+// be arbitrary. However, it should not be possible to have multiple secrets
+// with the same attribues so this should not happen.
 //
 // Currently only unlocked secrets can be returned. If only a locked secret
 // matches the attributes, a diagnostic error will be printed to stderr and no
@@ -108,20 +111,29 @@ func (ss *SecretService) Get(attrs map[string]string) (string, error) {
 		return "", err
 	}
 
-	switch {
-	case len(unlocked) > 1:
-		fmt.Fprintln(os.Stderr, "Warning: Got more than one secret back. Using only the first")
-		fallthrough
-	case len(unlocked) > 0:
-		secret, err := ss.getSecret(unlocked[0])
+	// Find the first item with an exact attribute match. Sometimes
+	// attrs may be a subset of attributes that have been stored (e.g.
+	// may not contain a path), and we want to skip those. We return
+	// the secret of the first one found that matches.
+	for _, item := range unlocked {
+		ok, err := ss.attrsMatch(attrs, item)
+		if err != nil {
+			// We could continue to the next item but errors
+			// should not happen here, so lets surface them early.
+			return "", err
+		}
+		if !ok {
+			continue
+		}
+		secret, err := ss.getSecret(item)
 		if err != nil {
 			return "", err
 		}
 		return string(secret.Secret), nil
-	case len(locked) > 0:
+	}
+
+	if len(locked) > 0 {
 		fmt.Fprintln(os.Stderr, "TODO: Found locked secret. Sorry, can't unlock yet")
-	default:
-		// secret not found. return nothing
 	}
 
 	return "", nil
@@ -152,15 +164,17 @@ func (ss *SecretService) Store(label string, attrs map[string]string, secret str
 	return nil
 }
 
-// Delete removes a secret matching the given attributes. If expectedSecret is
-// not empty, then the secret matching the attributes will only be removed if
-// the value of the secret stored matches expectedSecret. If expectedSecret is
-// empty, then the secret will be removed if it just matches the attributes.
+// Delete removes a secret matching the given attributes. If expectedPassword
+// is not empty, then the secret matching the attributes will only be removed
+// if the password in the value of the secret stored matches expectedPassword.
+// If expectedPassword is empty, then the secret will be removed if it just
+// matches the attributes.
 //
-// If more that one unlocked secret matches the attributes, a diagnostic error
-// will be printed on stderr and the first matching secret will be deleted. It
+// Only secrets that match on all attributes and have no extra attributes are
+// considered. If there are multiple exact matches, the first is returned. It
 // is not clear what the ordering of the secrets is, so the "first" secret may
-// be arbitrary.
+// be arbitrary. However, it should not be possible to have multiple secrets
+// with the same attribues so this should not happen.
 //
 // Currently only unlocked secrets can be deleted. If only a locked secret
 // matches the attributes, a diagnostic error will be printed to stderr and no
@@ -169,31 +183,49 @@ func (ss *SecretService) Store(label string, attrs map[string]string, secret str
 // If an error looking up the items occurs, an error returning the secret for
 // the selected item occurs, or the secret cannot be deleted, an error is
 // returned.
-func (ss *SecretService) Delete(attrs map[string]string, expectedSecret string) error {
+func (ss *SecretService) Delete(attrs map[string]string, expectedPassword string) error {
 	unlocked, locked, err := ss.search(attrs)
 	if err != nil {
 		return err
 	}
 
+	// Find the first item with an exact attribute match. Sometimes
+	// attrs may be a subset of attributes that have been stored (e.g.
+	// may not contain a path), and we want to skip those. Ensure that
+	// expectedSecret matches the stored secret value
+	// the secret of the first one found that matches.
 	var itemPath dbus.ObjectPath
-	switch {
-	case len(unlocked) > 1:
-		fmt.Fprintln(os.Stderr, "Warning: Got more than one secret back. Erasing only the first")
-		fallthrough
-	case expectedSecret != "" && len(unlocked) > 0:
-		// We will only erase the secret when presented with a password if the password
-		// stored in the secret matches that password.
-		secret, err := ss.getSecret(unlocked[0])
+	for _, item := range unlocked {
+		ok, err := ss.attrsMatch(attrs, item)
 		if err != nil {
+			// We could continue to the next item but errors
+			// should not happen here, so lets surface them early.
 			return err
 		}
-		if string(secret.Secret) == expectedSecret {
-			itemPath = unlocked[0]
+		if !ok {
+			continue
 		}
-	case len(locked) > 0:
-		fmt.Fprintln(os.Stderr, "TODO: Found locked secret. Sorry, can't unlock for erase yet")
+		// We will only erase the secret when presented with a password if the password
+		// stored in the secret matches that password. A secret can contain multiple
+		// fields separated by newlines. The password is the part before the first
+		// newline if there is one at all.
+		if expectedPassword != "" {
+			secret, err := ss.getSecret(item)
+			if err != nil {
+				return err
+			}
+			password, _, _ := strings.Cut(string(secret.Secret), "\n")
+			if password != expectedPassword {
+				continue
+			}
+		}
+		itemPath = item
+		break
 	}
 
+	if !itemPath.IsValid() && len(locked) > 0 {
+		fmt.Fprintln(os.Stderr, "TODO: Found locked secret. Sorry, can't unlock for erase yet")
+	}
 	if !itemPath.IsValid() {
 		return nil
 	}
@@ -229,3 +261,31 @@ func (ss *SecretService) getSecret(itemPath dbus.ObjectPath) (secret Secret, err
 	err = call.Store(&secret)
 	return
 }
+
+// attrsMatch returns true if the given items have exactly the given
+// attributes. If the item has extra or fewer attributes, or any values are
+// different, false is returned. If the attributes of the item could be
+// retrieved an error is returned.
+func (ss *SecretService) attrsMatch(attrs map[string]string, itemPath dbus.ObjectPath) (bool, error) {
+	item := ss.conn.Object("org.freedesktop.secrets", itemPath)
+	prop, err := item.GetProperty("org.freedesktop.Secret.Item.Attributes")
+	if err != nil {
+		return false, err
+	}
+
+	itemAttrs, ok := prop.Value().(map[string]string)
+	if !ok {
+		return false, fmt.Errorf("item attributes property is not a map: %v", itemPath)
+	}
+
+	if len(itemAttrs) != len(attrs) {
+		return false, nil
+	}
+	for k, v1 := range attrs {
+		v2, ok := itemAttrs[k]
+		if !ok || v1 != v2 {
+			return false, nil
+		}
+	}
+	return true, nil
+}

main.go

@@ -169,8 +169,7 @@ func (cmd *CmdStore) Run(gc *GitCredential, ss *SecretService) error {
 func (cmd *CmdErase) AfterApply(gc *GitCredential) error {
 	cmd.validate(gc.Protocol != "", "protocol")
 	cmd.validate(gc.Username != "", "username")
-	cmd.validate(gc.Host != "", "host")
-	cmd.validate(gc.Path != "", "path")
+	cmd.validate(gc.Host != "" || gc.Path != "", "host or path")
 	return cmd.err
 }
 
@@ -182,7 +181,7 @@ func (cmd *CmdErase) AfterApply(gc *GitCredential) error {
 //
 // [gitcredentials]: https://git-scm.com/docs/gitcredentials
 func (cmd *CmdErase) Run(gc *GitCredential, ss *SecretService) error {
-	return ss.Delete(makeAttrs(gc), formatSecretVal(gc))
+	return ss.Delete(makeAttrs(gc), gc.Password)
 }
 
 // makeLabel returns a string describing the given GitCredential, used as a