fix: macOS keychain item metadata defaults

credstore/filebackend_compat_test.go

@@ -97,6 +97,41 @@ func TestFileBackendEncodingMatchesByteNessShape(t *testing.T) {
 	}
 }
 
+func TestFileBackendEncodingPreservesMetadataFields(t *testing.T) {
+	passphrase := "metadata-" + "passphrase"
+	token, err := encodeFileKeyringItem(keyringItem{
+		key:                         "default/tok",
+		data:                        []byte("shape-value"),
+		label:                       "codereview default/tok",
+		description:                 "Credential for codereview default/tok",
+		keychainNotTrustApplication: true,
+		keychainNotSynchronizable:   true,
+	}, passphrase)
+	if err != nil {
+		t.Fatalf("encodeFileKeyringItem: %v", err)
+	}
+	payload, _, err := jose.Decode(token, passphrase)
+	if err != nil {
+		t.Fatalf("jose.Decode: %v", err)
+	}
+	var got persistedKeyringItem
+	if err := json.Unmarshal([]byte(payload), &got); err != nil {
+		t.Fatalf("json.Unmarshal payload: %v", err)
+	}
+	if got.Label != "codereview default/tok" {
+		t.Fatalf("Label = %q, want %q", got.Label, "codereview default/tok")
+	}
+	if got.Description != "Credential for codereview default/tok" {
+		t.Fatalf("Description = %q, want %q", got.Description, "Credential for codereview default/tok")
+	}
+	if !got.KeychainNotTrustApplication {
+		t.Fatal("KeychainNotTrustApplication = false, want true")
+	}
+	if !got.KeychainNotSynchronizable {
+		t.Fatal("KeychainNotSynchronizable = false, want true")
+	}
+}
+
 func writeRawFileBackendItem(t *testing.T, base, service, encodedName, token string) {
 	t.Helper()
 	dir := filepath.Join(base, service, "keyring")

credstore/filebackend_format.go

@@ -18,7 +18,14 @@ type persistedKeyringItem struct {
 }
 
 func encodeFileKeyringItem(it keyringItem, password string) (string, error) {
-	bytes, err := json.Marshal(persistedKeyringItem{Key: it.key, Data: it.data})
+	bytes, err := json.Marshal(persistedKeyringItem{
+		Key:                         it.key,
+		Data:                        it.data,
+		Label:                       it.label,
+		Description:                 it.description,
+		KeychainNotTrustApplication: it.keychainNotTrustApplication,
+		KeychainNotSynchronizable:   it.keychainNotSynchronizable,
+	})
 	if err != nil {
 		return "", err
 	}

credstore/osbackend_byteness.go

@@ -11,12 +11,13 @@ import (
 
 func openKeyringBackend(kind Backend, cfg backendConfig) (keyringBackend, error) {
 	kcfg := keyring.Config{
-		ServiceName:     cfg.serviceName,
-		AllowedBackends: []keyring.BackendType{keyring.BackendType(cfg.allowedBackend)},
-		FileDir:         cfg.fileDir,
-		PassDir:         cfg.passDir,
-		PassCmd:         cfg.passCmd,
-		PassPrefix:      cfg.passPrefix,
+		ServiceName:              cfg.serviceName,
+		AllowedBackends:          []keyring.BackendType{keyring.BackendType(cfg.allowedBackend)},
+		KeychainTrustApplication: cfg.keychainTrustApplication,
+		FileDir:                  cfg.fileDir,
+		PassDir:                  cfg.passDir,
+		PassCmd:                  cfg.passCmd,
+		PassPrefix:               cfg.passPrefix,
 	}
 	if cfg.filePasswordFunc != nil {
 		kcfg.FilePasswordFunc = keyring.PromptFunc(cfg.filePasswordFunc)
@@ -40,11 +41,25 @@ func (b bytenessBackend) get(itemKey string) (keyringItem, error) {
 		}
 		return keyringItem{}, err
 	}
-	return keyringItem{key: it.Key, data: it.Data}, nil
+	return keyringItem{
+		key:                         it.Key,
+		data:                        it.Data,
+		label:                       it.Label,
+		description:                 it.Description,
+		keychainNotTrustApplication: it.KeychainNotTrustApplication,
+		keychainNotSynchronizable:   it.KeychainNotSynchronizable,
+	}, nil
 }
 
 func (b bytenessBackend) set(it keyringItem) error {
-	return b.kr.Set(keyring.Item{Key: it.key, Data: it.data})
+	return b.kr.Set(keyring.Item{
+		Key:                         it.key,
+		Data:                        it.data,
+		Label:                       it.label,
+		Description:                 it.description,
+		KeychainNotTrustApplication: it.keychainNotTrustApplication,
+		KeychainNotSynchronizable:   it.keychainNotSynchronizable,
+	})
 }
 
 func (b bytenessBackend) remove(itemKey string) error {

credstore/osbackend_byteness_test.go

@@ -0,0 +1,63 @@
+//go:build cgo || windows
+
+package credstore
+
+import (
+	"testing"
+
+	"github.com/byteness/keyring"
+)
+
+type captureBytenessKeyring struct {
+	setItem keyring.Item
+}
+
+func (c *captureBytenessKeyring) Get(string) (keyring.Item, error) {
+	return keyring.Item{}, keyring.ErrKeyNotFound
+}
+
+func (c *captureBytenessKeyring) GetMetadata(string) (keyring.Metadata, error) {
+	return keyring.Metadata{}, keyring.ErrMetadataNotSupported
+}
+
+func (c *captureBytenessKeyring) Set(item keyring.Item) error {
+	c.setItem = item
+	return nil
+}
+
+func (c *captureBytenessKeyring) Remove(string) error { return nil }
+
+func (c *captureBytenessKeyring) Keys() ([]string, error) { return nil, nil }
+
+func TestBytenessBackendSetPassesThroughMetadata(t *testing.T) {
+	kr := &captureBytenessKeyring{}
+	be := bytenessBackend{kr: kr}
+
+	item := keyringItem{
+		key:                         "default/git_token",
+		data:                        []byte("secret"),
+		label:                       "codereview default/git_token",
+		description:                 "Credential for codereview default/git_token",
+		keychainNotTrustApplication: true,
+		keychainNotSynchronizable:   true,
+	}
+	if err := be.set(item); err != nil {
+		t.Fatalf("set: %v", err)
+	}
+
+	if kr.setItem.Key != item.key || string(kr.setItem.Data) != "secret" {
+		t.Fatalf("stored item = (%q,%q), want (%q,%q)", kr.setItem.Key, string(kr.setItem.Data), item.key, "secret")
+	}
+	if kr.setItem.Label != item.label {
+		t.Fatalf("Label = %q, want %q", kr.setItem.Label, item.label)
+	}
+	if kr.setItem.Description != item.description {
+		t.Fatalf("Description = %q, want %q", kr.setItem.Description, item.description)
+	}
+	if !kr.setItem.KeychainNotTrustApplication {
+		t.Fatal("KeychainNotTrustApplication = false, want true")
+	}
+	if !kr.setItem.KeychainNotSynchronizable {
+		t.Fatal("KeychainNotSynchronizable = false, want true")
+	}
+}

credstore/osbackend_core.go

@@ -20,8 +20,12 @@ var errKeyringItemNotFound = errors.New("credstore: keyring item not found")
 type promptFunc func(string) (string, error)
 
 type keyringItem struct {
-	key  string
-	data []byte
+	key                         string
+	data                        []byte
+	label                       string
+	description                 string
+	keychainNotTrustApplication bool
+	keychainNotSynchronizable   bool
 }
 
 type keyringBackend interface {
@@ -32,13 +36,14 @@ type keyringBackend interface {
 }
 
 type backendConfig struct {
-	serviceName      string
-	allowedBackend   Backend
-	fileDir          string
-	filePasswordFunc promptFunc
-	passDir          string
-	passCmd          string
-	passPrefix       string
+	serviceName              string
+	allowedBackend           Backend
+	keychainTrustApplication bool
+	fileDir                  string
+	filePasswordFunc         promptFunc
+	passDir                  string
+	passCmd                  string
+	passPrefix               string
 }
 
 // osKeyringBackend adapts a platform keyring implementation to the
@@ -47,6 +52,7 @@ type backendConfig struct {
 type osKeyringBackend struct {
 	kr          keyringBackend
 	backendKind Backend
+	service     string
 }
 
 // openOSBackend opens exactly the selected backend. The opener is
@@ -64,7 +70,7 @@ func openOSBackend(kind Backend, service string, opts *Options, getenv func(stri
 	if err != nil {
 		return nil, fmt.Errorf("credstore: opening %s backend for service %q: %w", kind, service, err)
 	}
-	return &osKeyringBackend{kr: kr, backendKind: kind}, nil
+	return &osKeyringBackend{kr: kr, backendKind: kind, service: service}, nil
 }
 
 // preflightOSBackend runs cheap, actionable pre-construction checks for
@@ -118,7 +124,12 @@ func buildKeyringConfig(kind Backend, service string, opts *Options, getenv func
 		// CLI gets its own subtree.
 		cfg.passPrefix = service
 	case BackendKeychain, BackendWinCred, BackendSecretService:
-		// No additional construction — these use ServiceName directly.
+		// All three use ServiceName directly. On macOS, trust the current
+		// signed application by default so newly-written items do not show
+		// the generic "allow access?" dialog for the same binary.
+		if kind == BackendKeychain {
+			cfg.keychainTrustApplication = true
+		}
 	case BackendMemory:
 		// BackendMemory short-circuits in store.Open before reaching
 		// openOSBackend; this arm exists only to keep the exhaustive
@@ -127,6 +138,23 @@ func buildKeyringConfig(kind Backend, service string, opts *Options, getenv func
 	return cfg, nil
 }
 
+func keyringItemForWrite(service, itemKey string, value []byte) keyringItem {
+	return keyringItem{
+		key:         itemKey,
+		data:        value,
+		label:       keyringItemLabel(service, itemKey),
+		description: keyringItemDescription(service, itemKey),
+	}
+}
+
+func keyringItemLabel(service, itemKey string) string {
+	return service + " " + itemKey
+}
+
+func keyringItemDescription(service, itemKey string) string {
+	return "Credential for " + service + " " + itemKey
+}
+
 // fileKeyringDir is the encrypted-file backend location and the test
 // isolation seam. XDG Base Directory: $XDG_DATA_HOME/<service>/keyring,
 // else $HOME/.local/share/<service>/keyring. Tests set XDG_DATA_HOME to
@@ -202,7 +230,7 @@ func (b *osKeyringBackend) set(itemKey, value string, overwrite bool) error {
 			return fmt.Errorf("credstore: %s set %q (overwrite pre-check): %w", b.backendKind, itemKey, err)
 		}
 	}
-	if err := b.kr.set(keyringItem{key: itemKey, data: []byte(value)}); err != nil {
+	if err := b.kr.set(keyringItemForWrite(b.service, itemKey, []byte(value))); err != nil {
 		return fmt.Errorf("credstore: %s set %q: %w", b.backendKind, itemKey, err)
 	}
 	return nil

credstore/osbackend_test.go

@@ -388,6 +388,66 @@ func TestBuildKeyringConfig_PassSetsPrefixToService(t *testing.T) {
 			t.Errorf("fileDir is empty; expected file backend to set it")
 		}
 	})
+
+	t.Run("keychain: trust current application by default", func(t *testing.T) {
+		cfg, err := buildKeyringConfig(BackendKeychain, "codereview", &Options{}, emptyEnv)
+		if err != nil {
+			t.Fatalf("buildKeyringConfig keychain: %v", err)
+		}
+		if !cfg.keychainTrustApplication {
+			t.Fatal("keychainTrustApplication = false, want true")
+		}
+	})
+
+	t.Run("wincred: do not set keychain trust flag", func(t *testing.T) {
+		cfg, err := buildKeyringConfig(BackendWinCred, "codereview", &Options{}, emptyEnv)
+		if err != nil {
+			t.Fatalf("buildKeyringConfig wincred: %v", err)
+		}
+		if cfg.keychainTrustApplication {
+			t.Fatal("keychainTrustApplication = true, want false")
+		}
+	})
+}
+
+func TestKeyringItemForWriteBuildsInspectableMetadata(t *testing.T) {
+	got := keyringItemForWrite("codereview", "default/git_token", []byte("secret"))
+
+	if got.key != "default/git_token" || string(got.data) != "secret" {
+		t.Fatalf("item = (%q,%q), want (default/git_token,secret)", got.key, string(got.data))
+	}
+	if got.label != "codereview default/git_token" {
+		t.Fatalf("label = %q, want %q", got.label, "codereview default/git_token")
+	}
+	if got.description != "Credential for codereview default/git_token" {
+		t.Fatalf("description = %q, want %q", got.description, "Credential for codereview default/git_token")
+	}
+	if got.keychainNotTrustApplication {
+		t.Fatal("keychainNotTrustApplication = true, want false")
+	}
+	if got.keychainNotSynchronizable {
+		t.Fatal("keychainNotSynchronizable = true, want false")
+	}
+}
+
+func TestOSKeyringBackendSetStampsMetadataBeforeWrite(t *testing.T) {
+	f := newFakeKeyring()
+	b := &osKeyringBackend{kr: f, backendKind: BackendKeychain, service: "codereview"}
+
+	if err := b.set("default/git_token", "secret", true); err != nil {
+		t.Fatalf("set: %v", err)
+	}
+
+	got, ok := f.items["default/git_token"]
+	if !ok {
+		t.Fatal("backend write did not reach fake keyring")
+	}
+	if got.label != "codereview default/git_token" {
+		t.Fatalf("label = %q, want %q", got.label, "codereview default/git_token")
+	}
+	if got.description != "Credential for codereview default/git_token" {
+		t.Fatalf("description = %q, want %q", got.description, "Credential for codereview default/git_token")
+	}
 }
 
 // TestOpenOSBackend_PassOnWindows_FailsGracefully pins the Windows

docs/working-with-secrets.md

@@ -131,6 +131,14 @@ Within a bundle, individual secrets are addressed by well-known keys defined per
 
 So `slack-chat-api/work-account` with secret `bot_token` becomes `ServiceName="slack-chat-api"`, `Item.Key="work-account/bot_token"`. This makes `Keyring.Keys()` enumerable per-service for `config show` and `config clear`, and gives a deterministic, inspectable layout when a user opens Keychain Access / Credential Manager / `secret-tool`.
 
+On macOS, new or overwritten Keychain generic-password items MUST also carry a
+deterministic user-facing label and description derived from the same mapping:
+`Label = "<service> <profile>/<key>"` and `Description = "Credential for
+<service> <profile>/<key>"`. New or overwritten items trust the calling signed
+application by default (`KeychainTrustApplication` / implied self). Existing
+items are not migrated in place: if an older item has blank metadata or older
+ACL state, it keeps that state until the CLI rewrites or overwrites it.
+
 Because `/` is structural in this mapping, **`/` is forbidden inside any segment.** Allowed characters within `service`, `profile`, and `key` are `[A-Za-z0-9_-]`. The shared package rejects anything else at write time with a clear error. CLIs that need a richer identifier (e.g. an email address as a profile) must escape it; the shared package provides helpers.
 
 **Multi-key-per-ref is supported by all three platform backends.** A `Keyring` opened against one `ServiceName` holds any number of `Item`s with distinct `Item.Key` values. On macOS this is one Keychain service with multiple account names; on Windows, one target-name prefix with multiple targets; on Linux Secret Service, one collection with multiple items distinguished by attributes. So `slack-chat-api/work-account` legitimately holds both `work-account/bot_token` and `work-account/user_token` as separate, independently-readable entries.