fix: avoid keychain secret reads for exists checks

.codereview/agents/go/implementation-tests/prompt.md

@@ -1,4 +1,4 @@
-You are reviewing Go implementation quality and test adequacy for codereview-cli.
+You are reviewing Go implementation quality and test adequacy for this repository.
 
 Optimize for high-signal findings. Return no findings when the Go code is idiomatic enough, the changed behavior is adequately tested for its risk, or a concern would require speculation. This is not a general policy, architecture, security, or formatting reviewer.
 

credstore/filebackend_nocgo_unix.go

@@ -42,6 +42,20 @@ func (b *fileKeyringBackend) get(itemKey string) (keyringItem, error) {
 	return decodeFileKeyringItem(string(bytes), b.password)
 }
 
+func (b *fileKeyringBackend) metadata(itemKey string) (keyringItem, error) {
+	name, err := b.filename(itemKey)
+	if err != nil {
+		return keyringItem{}, err
+	}
+	if _, err := os.Stat(name); err != nil {
+		if os.IsNotExist(err) {
+			return keyringItem{}, errKeyringItemNotFound
+		}
+		return keyringItem{}, err
+	}
+	return keyringItem{key: itemKey}, nil
+}
+
 func (b *fileKeyringBackend) set(it keyringItem) error {
 	if err := b.unlock(); err != nil {
 		return err

credstore/osbackend_byteness.go

@@ -65,6 +65,30 @@ func (b bytenessBackend) get(itemKey string) (keyringItem, error) {
 	}, nil
 }
 
+func (b bytenessBackend) metadata(itemKey string) (keyringItem, error) {
+	md, err := b.kr.GetMetadata(itemKey)
+	if err != nil {
+		switch {
+		case errors.Is(err, keyring.ErrKeyNotFound):
+			return keyringItem{}, errKeyringItemNotFound
+		case errors.Is(err, keyring.ErrMetadataNotSupported), errors.Is(err, keyring.ErrMetadataNeedsCredentials):
+			return keyringItem{}, errKeyringMetadataUnsupported
+		default:
+			return keyringItem{}, err
+		}
+	}
+	if md.Item == nil {
+		return keyringItem{}, nil
+	}
+	return keyringItem{
+		key:                         md.Key,
+		label:                       md.Label,
+		description:                 md.Description,
+		keychainNotTrustApplication: md.KeychainNotTrustApplication,
+		keychainNotSynchronizable:   md.KeychainNotSynchronizable,
+	}, nil
+}
+
 func (b bytenessBackend) set(it keyringItem) error {
 	return b.kr.Set(keyring.Item{
 		Key:                         it.key,

credstore/osbackend_core.go

@@ -17,6 +17,7 @@ import (
 )
 
 var errKeyringItemNotFound = errors.New("credstore: keyring item not found")
+var errKeyringMetadataUnsupported = errors.New("credstore: keyring metadata unsupported")
 
 type promptFunc func(string) (string, error)
 
@@ -31,6 +32,7 @@ type keyringItem struct {
 
 type keyringBackend interface {
 	get(itemKey string) (keyringItem, error)
+	metadata(itemKey string) (keyringItem, error)
 	set(keyringItem) error
 	remove(itemKey string) error
 	keys() ([]string, error)
@@ -285,7 +287,13 @@ func (b *osKeyringBackend) delete(itemKey string) error {
 }
 
 func (b *osKeyringBackend) exists(itemKey string) (bool, error) {
-	if _, err := b.kr.get(itemKey); err != nil {
+	if _, err := b.kr.metadata(itemKey); err != nil {
+		if errors.Is(err, errKeyringMetadataUnsupported) {
+			_, err = b.kr.get(itemKey)
+		}
+		if err == nil {
+			return true, nil
+		}
 		if errors.Is(err, errKeyringItemNotFound) {
 			return false, nil
 		}

credstore/osbackend_test.go

@@ -170,8 +170,8 @@ func TestFileBackendPassphraseFuncError(t *testing.T) {
 // used to exercise osKeyringBackend's error mapping/wrapping arms
 // deterministically without a real OS keyring.
 type fakeKeyring struct {
-	items                           map[string]keyringItem
-	getErr, setErr, delErr, keysErr error
+	items                                        map[string]keyringItem
+	getErr, metadataErr, setErr, delErr, keysErr error
 }
 
 func newFakeKeyring() *fakeKeyring { return &fakeKeyring{items: map[string]keyringItem{}} }
@@ -186,6 +186,17 @@ func (f *fakeKeyring) get(k string) (keyringItem, error) {
 	}
 	return it, nil
 }
+func (f *fakeKeyring) metadata(k string) (keyringItem, error) {
+	if f.metadataErr != nil {
+		return keyringItem{}, f.metadataErr
+	}
+	it, ok := f.items[k]
+	if !ok {
+		return keyringItem{}, errKeyringItemNotFound
+	}
+	it.data = nil
+	return it, nil
+}
 func (f *fakeKeyring) set(it keyringItem) error {
 	if f.setErr != nil {
 		return f.setErr
@@ -248,7 +259,7 @@ func TestOSKeyringBackendErrorMapping(t *testing.T) {
 		op   string
 	}{
 		{"get", func(b *osKeyringBackend) error { _, e := b.get("p/k"); return e }, func(f *fakeKeyring) { f.getErr = sentinel }, "get"},
-		{"exists", func(b *osKeyringBackend) error { _, e := b.exists("p/k"); return e }, func(f *fakeKeyring) { f.getErr = sentinel }, "exists"},
+		{"exists", func(b *osKeyringBackend) error { _, e := b.exists("p/k"); return e }, func(f *fakeKeyring) { f.metadataErr = sentinel }, "exists"},
 		{"set", func(b *osKeyringBackend) error { return b.set("p/k", "v", true) }, func(f *fakeKeyring) { f.setErr = sentinel }, "set"},
 		// !overwrite + a non-not-found Get error hits the pre-check
 		// default arm (distinct from the overwrite=true write path above).
@@ -274,6 +285,36 @@ func TestOSKeyringBackendErrorMapping(t *testing.T) {
 	}
 }
 
+func TestOSKeyringBackendExistsUsesMetadataWhenAvailable(t *testing.T) {
+	f := newFakeKeyring()
+	f.items["p/k"] = keyringItem{key: "p/k", data: []byte("secret"), label: "label", description: "desc"}
+	f.getErr = errors.New("get should not run when metadata succeeds")
+	b := &osKeyringBackend{kr: f, backendKind: BackendKeychain}
+
+	ok, err := b.exists("p/k")
+	if err != nil {
+		t.Fatalf("exists: %v", err)
+	}
+	if !ok {
+		t.Fatal("exists = false, want true")
+	}
+}
+
+func TestOSKeyringBackendExistsFallsBackToGetWhenMetadataUnsupported(t *testing.T) {
+	f := newFakeKeyring()
+	f.items["p/k"] = keyringItem{key: "p/k", data: []byte("secret")}
+	f.metadataErr = errKeyringMetadataUnsupported
+	b := &osKeyringBackend{kr: f, backendKind: BackendFile}
+
+	ok, err := b.exists("p/k")
+	if err != nil {
+		t.Fatalf("exists: %v", err)
+	}
+	if !ok {
+		t.Fatal("exists = false, want true")
+	}
+}
+
 // TestOpenEnvSelectsFileBackend exercises the real
 // selectBackend→Open integration through os.Getenv (the file backend
 // runs in CI), proving env selection actually changes the constructed

credstore/passbackend_nocgo_unix.go

@@ -68,6 +68,13 @@ func (b *passKeyringBackend) get(itemKey string) (keyringItem, error) {
 	return decodePassItemOutput(output)
 }
 
+func (b *passKeyringBackend) metadata(itemKey string) (keyringItem, error) {
+	if err := b.checkItemExists(itemKey); err != nil {
+		return keyringItem{}, err
+	}
+	return keyringItem{key: itemKey}, nil
+}
+
 func (b *passKeyringBackend) set(it keyringItem) error {
 	bytes, err := json.Marshal(persistedKeyringItem{Key: it.key, Data: it.data})
 	if err != nil {

credstore/secretservice_core.go

@@ -80,6 +80,20 @@ func (b *secretServiceBackend) get(itemKey string) (keyringItem, error) {
 	})
 }
 
+func (b *secretServiceBackend) metadata(itemKey string) (keyringItem, error) {
+	if err := b.openCollection(); err != nil {
+		return keyringItem{}, secretServiceNotFoundError(err)
+	}
+	items, err := b.collection.searchItems(itemKey)
+	if err != nil {
+		return keyringItem{}, err
+	}
+	if len(items) == 0 {
+		return keyringItem{}, errKeyringItemNotFound
+	}
+	return keyringItem{key: itemKey}, nil
+}
+
 func (b *secretServiceBackend) set(it keyringItem) error {
 	if err := b.openSecrets(); err != nil {
 		return err