feat: upsert credential support

plugins/pass/teststore/teststore.go

@@ -33,6 +33,7 @@ type MockStore struct {
 	errDelete error
 	errGet    error
 	errFilter error
+	errUpsert error
 	store     map[store.ID]store.Secret
 }
 
@@ -74,6 +75,12 @@ func WithStoreDeleteErr(err error) Option {
 	}
 }
 
+func WithStoreUpsertErr(err error) Option {
+	return func(m *MockStore) {
+		m.errUpsert = err
+	}
+}
+
 func WithStore(store map[store.ID]store.Secret) Option {
 	return func(m *MockStore) {
 		m.store = store
@@ -127,6 +134,17 @@ func (m *MockStore) Save(_ context.Context, id store.ID, secret store.Secret) er
 	return nil
 }
 
+func (m *MockStore) Upsert(_ context.Context, id store.ID, secret store.Secret) error {
+	m.lock.Lock()
+	defer m.lock.Unlock()
+	if m.errUpsert != nil {
+		return m.errUpsert
+	}
+
+	m.store[id] = secret
+	return nil
+}
+
 func (m *MockStore) Filter(_ context.Context, pattern store.Pattern) (map[store.ID]store.Secret, error) {
 	m.lock.Lock()
 	defer m.lock.Unlock()

store/keychain/keychain_darwin.go

@@ -19,6 +19,7 @@ import (
 	"errors"
 	"fmt"
 	"maps"
+	"sync"
 
 	kc "github.com/docker/secrets-engine/store/keychain/internal/go-keychain"
 
@@ -31,6 +32,7 @@ var (
 )
 
 type keychainStore[T store.Secret] struct {
+	mu                        sync.Mutex
 	serviceGroup              string
 	serviceName               string
 	factory                   store.Factory[T]
@@ -198,6 +200,21 @@ func (k *keychainStore[T]) Save(_ context.Context, id store.ID, secret store.Sec
 	return mapKeychainError(kc.AddItem(item))
 }
 
+// Upsert atomically replaces a credential in the macOS Keychain.
+//
+// The macOS Keychain does not allow overwriting an existing item via AddItem,
+// so Upsert holds a mutex and performs a Delete followed by a Save to ensure
+// no concurrent Upsert can interleave between the two operations.
+func (k *keychainStore[T]) Upsert(ctx context.Context, id store.ID, secret store.Secret) error {
+	k.mu.Lock()
+	defer k.mu.Unlock()
+
+	if err := k.Delete(ctx, id); err != nil {
+		return err
+	}
+	return k.Save(ctx, id, secret)
+}
+
 func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (map[store.ID]store.Secret, error) {
 	// Note: Filter on macOS cannot filter by generic attributes and thus we
 	// cannot split the ID and store it in the keychain as parts for later

store/keychain/keychain_darwin_test.go

@@ -25,6 +25,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	kc "github.com/docker/secrets-engine/store/keychain/internal/go-keychain"
+
 	"github.com/docker/secrets-engine/store"
 	"github.com/docker/secrets-engine/store/mocks"
 )
@@ -150,6 +152,82 @@ func TestMacosKeychain(t *testing.T) {
 	})
 }
 
+func TestUpsert(t *testing.T) {
+	var (
+		serviceName  = uuid.NewString()
+		serviceGroup = "com.test.testing"
+	)
+	ks := keychainStore[*mocks.MockCredential]{
+		serviceGroup: serviceGroup,
+		serviceName:  serviceName,
+		factory: func(_ context.Context, _ store.ID) *mocks.MockCredential {
+			return &mocks.MockCredential{}
+		},
+	}
+
+	t.Run("inserts when credential does not exist", func(t *testing.T) {
+		id := store.MustParseID(serviceGroup + "/" + serviceName + "/" + uuid.NewString())
+		t.Cleanup(func() {
+			assert.NoError(t, ks.Delete(t.Context(), id))
+		})
+
+		secret := &mocks.MockCredential{
+			Username: "alice",
+			Password: "alice-password",
+		}
+		require.NoError(t, ks.Upsert(t.Context(), id, secret))
+
+		got, err := ks.Get(t.Context(), id)
+		require.NoError(t, err)
+		actual := got.(*mocks.MockCredential)
+		actual.Attributes = nil
+		assert.Equal(t, secret.Username, actual.Username)
+		assert.Equal(t, secret.Password, actual.Password)
+	})
+
+	t.Run("overwrites an existing credential", func(t *testing.T) {
+		id := store.MustParseID(serviceGroup + "/" + serviceName + "/" + uuid.NewString())
+		t.Cleanup(func() {
+			assert.NoError(t, ks.Delete(t.Context(), id))
+		})
+
+		original := &mocks.MockCredential{
+			Username: "bob",
+			Password: "original-password",
+		}
+		require.NoError(t, ks.Save(t.Context(), id, original))
+
+		updated := &mocks.MockCredential{
+			Username: "bob",
+			Password: "updated-password",
+		}
+		require.NoError(t, ks.Upsert(t.Context(), id, updated))
+
+		got, err := ks.Get(t.Context(), id)
+		require.NoError(t, err)
+		actual := got.(*mocks.MockCredential)
+		actual.Attributes = nil
+		assert.Equal(t, updated.Username, actual.Username)
+		assert.Equal(t, updated.Password, actual.Password)
+	})
+
+	t.Run("save returns duplicate item error when credential already exists", func(t *testing.T) {
+		id := store.MustParseID(serviceGroup + "/" + serviceName + "/" + uuid.NewString())
+		t.Cleanup(func() {
+			assert.NoError(t, ks.Delete(t.Context(), id))
+		})
+
+		secret := &mocks.MockCredential{
+			Username: "charlie",
+			Password: "charlie-password",
+		}
+		require.NoError(t, ks.Save(t.Context(), id, secret))
+
+		err := ks.Save(t.Context(), id, secret)
+		require.ErrorIs(t, err, kc.ErrorDuplicateItem)
+	})
+}
+
 func TestConvertAttributes(t *testing.T) {
 	t.Run("can convert attributes map into map of any", func(t *testing.T) {
 		attributes := map[string]any{

store/keychain/keychain_linux.go

@@ -339,6 +339,10 @@ func (k *keychainStore[T]) Save(_ context.Context, id store.ID, secret store.Sec
 	return nil
 }
 
+func (k *keychainStore[T]) Upsert(ctx context.Context, id store.ID, secret store.Secret) error {
+	return k.Save(ctx, id, secret)
+}
+
 //gocyclo:ignore
 func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (map[store.ID]store.Secret, error) {
 	service, err := kc.NewService()

store/keychain/keychain_test.go

@@ -366,6 +366,52 @@ func TestKeychain(t *testing.T) {
 		require.ErrorContains(t, err, "i am failing on purpose")
 	})
 
+	t.Run("upsert inserts when credential does not exist", func(t *testing.T) {
+		ks := setupKeychain(t, nil)
+		id := store.MustParseID("com.test.test/test/upsert-insert")
+		creds := &mocks.MockCredential{
+			Username: "alice",
+			Password: "alice-password",
+		}
+		t.Cleanup(func() {
+			require.NoError(t, ks.Delete(context.Background(), id))
+		})
+		require.NoError(t, ks.Upsert(t.Context(), id, creds))
+
+		secret, err := ks.Get(t.Context(), id)
+		require.NoError(t, err)
+		actual := secret.(*mocks.MockCredential)
+		actual.Attributes = nil
+		assert.Equal(t, creds.Username, actual.Username)
+		assert.Equal(t, creds.Password, actual.Password)
+	})
+
+	t.Run("upsert overwrites an existing credential", func(t *testing.T) {
+		ks := setupKeychain(t, nil)
+		id := store.MustParseID("com.test.test/test/upsert-overwrite")
+		original := &mocks.MockCredential{
+			Username: "bob",
+			Password: "original-password",
+		}
+		t.Cleanup(func() {
+			require.NoError(t, ks.Delete(context.Background(), id))
+		})
+		require.NoError(t, ks.Save(t.Context(), id, original))
+
+		updated := &mocks.MockCredential{
+			Username: "bob",
+			Password: "updated-password",
+		}
+		require.NoError(t, ks.Upsert(t.Context(), id, updated))
+
+		secret, err := ks.Get(t.Context(), id)
+		require.NoError(t, err)
+		actual := secret.(*mocks.MockCredential)
+		actual.Attributes = nil
+		assert.Equal(t, updated.Username, actual.Username)
+		assert.Equal(t, updated.Password, actual.Password)
+	})
+
 	t.Run("set metadata error on getAllMetadata", func(t *testing.T) {
 		kc := setupKeychain(t, func(_ context.Context, _ store.ID) store.Secret {
 			return &mustUnmarshalError{}

store/keychain/keychain_windows.go

@@ -336,6 +336,10 @@ func (k *keychainStore[T]) Save(_ context.Context, id store.ID, secret store.Sec
 	return mapWindowsCredentialError(g.Write())
 }
 
+func (k *keychainStore[T]) Upsert(ctx context.Context, id store.ID, secret store.Secret) error {
+	return k.Save(ctx, id, secret)
+}
+
 func (k *keychainStore[T]) Filter(ctx context.Context, pattern store.Pattern) (map[store.ID]store.Secret, error) {
 	// Note: there is no notion of a filter on Windows inside the wincred API.
 	// It has no way to even filter on known attributes.

store/mocks/mock_store.go

@@ -70,6 +70,10 @@ func (m *MockStore) Save(_ context.Context, id store.ID, secret store.Secret) er
 	return nil
 }
 
+func (m *MockStore) Upsert(ctx context.Context, id store.ID, secret store.Secret) error {
+	return m.Save(ctx, id, secret)
+}
+
 func (m *MockStore) Filter(_ context.Context, pattern store.Pattern) (map[store.ID]store.Secret, error) {
 	m.lock.Lock()
 	defer m.lock.Unlock()

store/posixage/store.go

@@ -376,6 +376,10 @@ func (f *fileStore[T]) Save(ctx context.Context, id store.ID, s store.Secret) er
 	return secretfile.Persist(id, f.filesystem, metadata, secrets)
 }
 
+func (f *fileStore[T]) Upsert(ctx context.Context, id store.ID, s store.Secret) error {
+	return f.Save(ctx, id, s)
+}
+
 type config struct {
 	logger                    logging.Logger
 	registeredDecryptionFunc  []promptCaller

store/posixage/store_test.go

@@ -656,6 +656,80 @@ func TestPOSIXAge(t *testing.T) {
 		assert.Equal(t, secret, storeSecret)
 	})
 
+	t.Run("upsert inserts when credential does not exist", func(t *testing.T) {
+		root, err := os.OpenRoot(t.TempDir())
+		require.NoError(t, err)
+		t.Cleanup(func() {
+			assert.NoError(t, root.Close())
+		})
+
+		masterKey := uuid.NewString()
+		s, err := New(root,
+			func(_ context.Context, _ store.ID) *mocks.MockCredential {
+				return &mocks.MockCredential{}
+			},
+			WithLogger(&testLogger{t}),
+			WithEncryptionCallbackFunc[EncryptionPassword](func(_ context.Context) ([]byte, error) {
+				return []byte(masterKey), nil
+			}),
+			WithDecryptionCallbackFunc[DecryptionPassword](func(_ context.Context) ([]byte, error) {
+				return []byte(masterKey), nil
+			}),
+		)
+		require.NoError(t, err)
+
+		secret := &mocks.MockCredential{
+			Username: uuid.NewString(),
+			Password: uuid.NewString(),
+		}
+		id := secrets.MustParseID("test/something/" + uuid.NewString())
+		require.NoError(t, s.Upsert(t.Context(), id, secret))
+
+		storeSecret, err := s.Get(t.Context(), id)
+		require.NoError(t, err)
+		assert.EqualValues(t, secret, storeSecret)
+	})
+
+	t.Run("upsert overwrites an existing credential", func(t *testing.T) {
+		root, err := os.OpenRoot(t.TempDir())
+		require.NoError(t, err)
+		t.Cleanup(func() {
+			assert.NoError(t, root.Close())
+		})
+
+		masterKey := uuid.NewString()
+		s, err := New(root,
+			func(_ context.Context, _ store.ID) *mocks.MockCredential {
+				return &mocks.MockCredential{}
+			},
+			WithLogger(&testLogger{t}),
+			WithEncryptionCallbackFunc[EncryptionPassword](func(_ context.Context) ([]byte, error) {
+				return []byte(masterKey), nil
+			}),
+			WithDecryptionCallbackFunc[DecryptionPassword](func(_ context.Context) ([]byte, error) {
+				return []byte(masterKey), nil
+			}),
+		)
+		require.NoError(t, err)
+
+		original := &mocks.MockCredential{
+			Username: uuid.NewString(),
+			Password: uuid.NewString(),
+		}
+		id := secrets.MustParseID("test/something/" + uuid.NewString())
+		require.NoError(t, s.Save(t.Context(), id, original))
+
+		updated := &mocks.MockCredential{
+			Username: uuid.NewString(),
+			Password: uuid.NewString(),
+		}
+		require.NoError(t, s.Upsert(t.Context(), id, updated))
+
+		storeSecret, err := s.Get(t.Context(), id)
+		require.NoError(t, err)
+		assert.EqualValues(t, updated, storeSecret)
+	})
+
 	t.Run("an error on encryption callbackFunc is propagated on save", func(t *testing.T) {
 		root, err := os.OpenRoot(t.TempDir())
 		require.NoError(t, err)

store/store.go

@@ -84,6 +84,11 @@ type Store interface {
 	GetAllMetadata(ctx context.Context) (map[ID]Secret, error)
 	// Save persists credentials from the store.
 	Save(ctx context.Context, id ID, secret Secret) error
+	// Upsert atomically replaces an existing credential or inserts a new one.
+	// On stores that do not support overwriting (e.g. macOS Keychain), it
+	// deletes the existing credential and then saves the new one under a mutex
+	// to ensure the two operations are not interleaved.
+	Upsert(ctx context.Context, id ID, secret Secret) error
 	// Filter returns a map of secrets based on a [Pattern].
 	//
 	// Secrets returned will have both [Secret.SetMetadata] and [Secret.Unmarshal]