From 2b87a705b223d0eb60b811fbdf0af3722a58d4e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hermann?= Date: Tue, 17 Mar 2026 21:34:49 +0100 Subject: [PATCH 1/2] refactor: share writable native secret backends --- cmd/cloudstic/cmd_store.go | 54 +++--- cmd/cloudstic/cmd_store_test.go | 135 ++++++++++----- cmd/cloudstic/secret_store_darwin.go | 63 ------- cmd/cloudstic/secret_store_darwin_test.go | 162 ------------------ cmd/cloudstic/secret_store_stub.go | 16 -- internal/secretref/keychain_backend.go | 68 +++++++- internal/secretref/keychain_backend_darwin.go | 53 ++++++ .../secretref/keychain_backend_darwin_test.go | 42 +++++ internal/secretref/keychain_backend_stub.go | 8 + internal/secretref/keychain_backend_test.go | 43 +++++ internal/secretref/secretref.go | 91 +++++++++- internal/secretref/secretref_test.go | 45 +++++ 12 files changed, 463 insertions(+), 317 deletions(-) delete mode 100644 cmd/cloudstic/secret_store_darwin.go delete mode 100644 cmd/cloudstic/secret_store_darwin_test.go delete mode 100644 cmd/cloudstic/secret_store_stub.go diff --git a/cmd/cloudstic/cmd_store.go b/cmd/cloudstic/cmd_store.go index 522f0ff..4dc695d 100644 --- a/cmd/cloudstic/cmd_store.go +++ b/cmd/cloudstic/cmd_store.go @@ -7,7 +7,6 @@ import ( "fmt" "io" "os" - "runtime" "strings" cloudstic "github.com/cloudstic/cli" @@ -510,7 +509,6 @@ func configureStoreEncryptionSelection( func (r *runner) promptSecretReference(storeName, secretLabel, defaultEnvName, defaultAccount string) (string, error) { return promptSecretReferenceWithFns( - runtime.GOOS, storeName, secretLabel, defaultEnvName, @@ -519,29 +517,27 @@ func (r *runner) promptSecretReference(storeName, secretLabel, defaultEnvName, d r.promptLine, r.promptSecret, os.LookupEnv, - nativeSecretExists, - saveSecretToNativeStore, + profileSecretResolver, ) } func promptSecretReferenceWithFns( - goos, storeName, secretLabel, defaultEnvName, defaultAccount string, + storeName, secretLabel, defaultEnvName, defaultAccount string, promptSelect func(string, []string) (string, error), promptLine func(string, string) (string, error), promptSecret func(string) (string, error), lookupEnv func(string) (string, bool), - nativeSecretExists func(context.Context, string, string) (bool, error), - writeNativeSecret func(context.Context, string, string, string) error, + resolver *secretref.Resolver, ) (string, error) { - keychainRef := func() (string, error) { - service := "cloudstic/store/" + storeName - account := defaultAccount - exists, err := nativeSecretExists(context.Background(), service, account) + writableBackends := resolver.WritableBackends() + nativeRef := func(backend secretref.WritableBackend) (string, error) { + ref := backend.DefaultRef(storeName, defaultAccount) + exists, err := resolver.Exists(context.Background(), ref) if err != nil { return "", err } if exists { - return "keychain://" + service + "/" + account, nil + return ref, nil } secretValue, err := promptSecret("Secret value") if err != nil { @@ -550,22 +546,29 @@ func promptSecretReferenceWithFns( if secretValue == "" { return "", fmt.Errorf("secret value cannot be empty") } - if err := writeNativeSecret(context.Background(), service, account, secretValue); err != nil { + if err := resolver.Store(context.Background(), ref, secretValue); err != nil { return "", err } - return "keychain://" + service + "/" + account, nil + return ref, nil } - if goos == "darwin" { + if len(writableBackends) > 0 { + options := []string{"Environment variable (env://)"} + backendByOption := map[string]secretref.WritableBackend{} + for _, backend := range writableBackends { + option := fmt.Sprintf("%s (%s://)", backend.DisplayName(), backend.Scheme()) + options = append(options, option) + backendByOption[option] = backend + } picked, err := promptSelect( fmt.Sprintf("Where should %s be stored?", secretLabel), - []string{"Environment variable (env://)", "macOS Keychain (keychain://)"}, + options, ) if err != nil { return "", err } - if strings.HasPrefix(picked, "macOS Keychain") { - return keychainRef() + if backend, ok := backendByOption[picked]; ok { + return nativeRef(backend) } } @@ -573,16 +576,23 @@ func promptSecretReferenceWithFns( if err != nil { return "", err } - if _, ok := lookupEnv(envName); !ok && goos == "darwin" { + if _, ok := lookupEnv(envName); !ok && len(writableBackends) > 0 { + options := []string{"Keep environment variable reference (env://)"} + backendByOption := map[string]secretref.WritableBackend{} + for _, backend := range writableBackends { + option := fmt.Sprintf("Store in %s instead (%s://)", backend.DisplayName(), backend.Scheme()) + options = append(options, option) + backendByOption[option] = backend + } picked, err := promptSelect( fmt.Sprintf("Environment variable %q is not set in this shell", envName), - []string{"Keep environment variable reference (env://)", "Store in macOS Keychain instead (keychain://)"}, + options, ) if err != nil { return "", err } - if strings.HasPrefix(picked, "Store in macOS Keychain") { - return keychainRef() + if backend, ok := backendByOption[picked]; ok { + return nativeRef(backend) } } return envRef(envName), nil diff --git a/cmd/cloudstic/cmd_store_test.go b/cmd/cloudstic/cmd_store_test.go index 43fa1b2..eb0c9e1 100644 --- a/cmd/cloudstic/cmd_store_test.go +++ b/cmd/cloudstic/cmd_store_test.go @@ -10,9 +10,35 @@ import ( "testing" cloudstic "github.com/cloudstic/cli" + "github.com/cloudstic/cli/internal/secretref" "github.com/cloudstic/cli/pkg/keychain" ) +type writableBackendStub struct { + scheme string + displayName string + defaultRef string + exists func(context.Context, secretref.Ref) (bool, error) + store func(context.Context, secretref.Ref, string) error +} + +func (b writableBackendStub) Resolve(context.Context, secretref.Ref) (string, error) { return "", nil } +func (b writableBackendStub) Scheme() string { return b.scheme } +func (b writableBackendStub) DisplayName() string { return b.displayName } +func (b writableBackendStub) DefaultRef(string, string) string { return b.defaultRef } +func (b writableBackendStub) Exists(ctx context.Context, ref secretref.Ref) (bool, error) { + if b.exists == nil { + return false, nil + } + return b.exists(ctx, ref) +} +func (b writableBackendStub) Store(ctx context.Context, ref secretref.Ref, value string) error { + if b.store == nil { + return nil + } + return b.store(ctx, ref, value) +} + func TestRunStoreNewAndListAndShow(t *testing.T) { tmpDir := t.TempDir() profilesPath := filepath.Join(tmpDir, "profiles.yaml") @@ -753,8 +779,25 @@ func TestRunStoreNew_WithSecretRefFlags(t *testing.T) { } func TestPromptSecretReferenceWithFns_DarwinKeychain(t *testing.T) { + resolver := secretref.NewResolver(map[string]secretref.Backend{ + "keychain": writableBackendStub{ + scheme: "keychain", + displayName: "macOS Keychain", + defaultRef: "keychain://cloudstic/store/prod-store/password", + exists: func(context.Context, secretref.Ref) (bool, error) { return false, nil }, + store: func(_ context.Context, ref secretref.Ref, value string) error { + if ref.Raw != "keychain://cloudstic/store/prod-store/password" { + t.Fatalf("ref=%q", ref.Raw) + } + if value != "super-secret" { + t.Fatalf("value=%q", value) + } + return nil + }, + }, + }) + gotRef, err := promptSecretReferenceWithFns( - "darwin", "prod-store", "repository password", "CLOUDSTIC_PASSWORD", @@ -763,19 +806,7 @@ func TestPromptSecretReferenceWithFns_DarwinKeychain(t *testing.T) { func(label, def string) (string, error) { return def, nil }, func(_ string) (string, error) { return "super-secret", nil }, func(string) (string, bool) { return "", false }, - func(context.Context, string, string) (bool, error) { return false, nil }, - func(_ context.Context, service, account, value string) error { - if service != "cloudstic/store/prod-store" { - t.Fatalf("service=%q", service) - } - if account != "password" { - t.Fatalf("account=%q", account) - } - if value != "super-secret" { - t.Fatalf("value=%q", value) - } - return nil - }, + resolver, ) if err != nil { t.Fatalf("promptSecretReferenceWithFns: %v", err) @@ -786,8 +817,8 @@ func TestPromptSecretReferenceWithFns_DarwinKeychain(t *testing.T) { } func TestPromptSecretReferenceWithFns_EnvFallback(t *testing.T) { + resolver := secretref.NewResolver(nil) gotRef, err := promptSecretReferenceWithFns( - "darwin", "prod-store", "repository password", "CLOUDSTIC_PASSWORD", @@ -804,14 +835,7 @@ func TestPromptSecretReferenceWithFns_EnvFallback(t *testing.T) { return "", nil }, func(string) (string, bool) { return "", true }, - func(context.Context, string, string) (bool, error) { - t.Fatal("nativeSecretExists should not be called") - return false, nil - }, - func(context.Context, string, string, string) error { - t.Fatal("writeNativeSecret should not be called") - return nil - }, + resolver, ) if err != nil { t.Fatalf("promptSecretReferenceWithFns: %v", err) @@ -822,8 +846,16 @@ func TestPromptSecretReferenceWithFns_EnvFallback(t *testing.T) { } func TestPromptSecretReferenceWithFns_KeychainWriteError(t *testing.T) { + resolver := secretref.NewResolver(map[string]secretref.Backend{ + "keychain": writableBackendStub{ + scheme: "keychain", + displayName: "macOS Keychain", + defaultRef: "keychain://cloudstic/store/prod-store/password", + exists: func(context.Context, secretref.Ref) (bool, error) { return false, nil }, + store: func(context.Context, secretref.Ref, string) error { return errors.New("write failed") }, + }, + }) _, err := promptSecretReferenceWithFns( - "darwin", "prod-store", "repository password", "CLOUDSTIC_PASSWORD", @@ -832,8 +864,7 @@ func TestPromptSecretReferenceWithFns_KeychainWriteError(t *testing.T) { func(_ string, def string) (string, error) { return def, nil }, func(_ string) (string, error) { return "secret", nil }, func(string) (string, bool) { return "", false }, - func(context.Context, string, string) (bool, error) { return false, nil }, - func(context.Context, string, string, string) error { return errors.New("write failed") }, + resolver, ) if err == nil { t.Fatal("expected error") @@ -844,8 +875,10 @@ func TestPromptSecretReferenceWithFns_KeychainWriteError(t *testing.T) { } func TestPromptSecretReferenceWithFns_EmptySecret(t *testing.T) { + resolver := secretref.NewResolver(map[string]secretref.Backend{ + "keychain": writableBackendStub{scheme: "keychain", displayName: "macOS Keychain", defaultRef: "keychain://cloudstic/store/prod-store/password"}, + }) _, err := promptSecretReferenceWithFns( - "darwin", "prod-store", "repository password", "CLOUDSTIC_PASSWORD", @@ -854,8 +887,7 @@ func TestPromptSecretReferenceWithFns_EmptySecret(t *testing.T) { func(_ string, def string) (string, error) { return def, nil }, func(_ string) (string, error) { return "", nil }, func(string) (string, bool) { return "", false }, - func(context.Context, string, string) (bool, error) { return false, nil }, - func(context.Context, string, string, string) error { return nil }, + resolver, ) if err == nil { t.Fatal("expected error") @@ -866,8 +898,24 @@ func TestPromptSecretReferenceWithFns_EmptySecret(t *testing.T) { } func TestPromptSecretReferenceWithFns_DarwinKeychainAdoptsExisting(t *testing.T) { + resolver := secretref.NewResolver(map[string]secretref.Backend{ + "keychain": writableBackendStub{ + scheme: "keychain", + displayName: "macOS Keychain", + defaultRef: "keychain://cloudstic/store/prod-store/password", + exists: func(_ context.Context, ref secretref.Ref) (bool, error) { + if ref.Raw != "keychain://cloudstic/store/prod-store/password" { + t.Fatalf("ref=%q", ref.Raw) + } + return true, nil + }, + store: func(context.Context, secretref.Ref, string) error { + t.Fatal("store should not be called when secret exists") + return nil + }, + }, + }) gotRef, err := promptSecretReferenceWithFns( - "darwin", "prod-store", "repository password", "CLOUDSTIC_PASSWORD", @@ -879,19 +927,7 @@ func TestPromptSecretReferenceWithFns_DarwinKeychainAdoptsExisting(t *testing.T) return "", nil }, func(string) (string, bool) { return "", false }, - func(_ context.Context, service, account string) (bool, error) { - if service != "cloudstic/store/prod-store" { - t.Fatalf("service=%q", service) - } - if account != "password" { - t.Fatalf("account=%q", account) - } - return true, nil - }, - func(context.Context, string, string, string) error { - t.Fatal("writeNativeSecret should not be called when key exists") - return nil - }, + resolver, ) if err != nil { t.Fatalf("promptSecretReferenceWithFns: %v", err) @@ -903,8 +939,16 @@ func TestPromptSecretReferenceWithFns_DarwinKeychainAdoptsExisting(t *testing.T) func TestPromptSecretReferenceWithFns_DarwinEnvUnsetSwitchesToKeychain(t *testing.T) { selectCall := 0 + resolver := secretref.NewResolver(map[string]secretref.Backend{ + "keychain": writableBackendStub{ + scheme: "keychain", + displayName: "macOS Keychain", + defaultRef: "keychain://cloudstic/store/prod-store/password", + exists: func(context.Context, secretref.Ref) (bool, error) { return false, nil }, + store: func(context.Context, secretref.Ref, string) error { return nil }, + }, + }) gotRef, err := promptSecretReferenceWithFns( - "darwin", "prod-store", "repository password", "CLOUDSTIC_PASSWORD", @@ -924,8 +968,7 @@ func TestPromptSecretReferenceWithFns_DarwinEnvUnsetSwitchesToKeychain(t *testin }, func(_ string) (string, error) { return "secret-value", nil }, func(string) (string, bool) { return "", false }, - func(context.Context, string, string) (bool, error) { return false, nil }, - func(context.Context, string, string, string) error { return nil }, + resolver, ) if err != nil { t.Fatalf("promptSecretReferenceWithFns: %v", err) diff --git a/cmd/cloudstic/secret_store_darwin.go b/cmd/cloudstic/secret_store_darwin.go deleted file mode 100644 index 8293953..0000000 --- a/cmd/cloudstic/secret_store_darwin.go +++ /dev/null @@ -1,63 +0,0 @@ -//go:build darwin && cgo - -package main - -import ( - "context" - "fmt" - - "github.com/keybase/go-keychain" -) - -var ( - keychainAddItem = keychain.AddItem - keychainUpdateItem = keychain.UpdateItem - keychainGetGenericPassword = keychain.GetGenericPassword - keychainGenericPasswordKind = keychain.SecClassGenericPassword -) - -func saveSecretToNativeStore(ctx context.Context, service, account, value string) error { - _ = ctx - - item := keychain.NewItem() - item.SetSecClass(keychainGenericPasswordKind) - item.SetService(service) - item.SetAccount(account) - item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly) - item.SetData([]byte(value)) - - if err := keychainAddItem(item); err != nil { - if err == keychain.ErrorDuplicateItem { - query := keychain.NewItem() - query.SetSecClass(keychainGenericPasswordKind) - query.SetService(service) - query.SetAccount(account) - - update := keychain.NewItem() - update.SetData([]byte(value)) - - if updateErr := keychainUpdateItem(query, update); updateErr != nil { - return fmt.Errorf("save secret in macOS keychain failed: %v", updateErr) - } - return nil - } - return fmt.Errorf("save secret in macOS keychain failed: %v", err) - } - return nil -} - -func nativeSecretExists(ctx context.Context, service, account string) (bool, error) { - _ = ctx - - data, err := keychainGetGenericPassword(service, account, "", "") - if err != nil { - if err == keychain.ErrorItemNotFound { - return false, nil - } - return false, fmt.Errorf("check secret in macOS keychain failed: %v", err) - } - if data == nil { - return false, nil - } - return true, nil -} diff --git a/cmd/cloudstic/secret_store_darwin_test.go b/cmd/cloudstic/secret_store_darwin_test.go deleted file mode 100644 index b133466..0000000 --- a/cmd/cloudstic/secret_store_darwin_test.go +++ /dev/null @@ -1,162 +0,0 @@ -//go:build darwin && cgo - -package main - -import ( - "context" - "strings" - "testing" - - "github.com/keybase/go-keychain" -) - -func TestSaveSecretToNativeStore_Success(t *testing.T) { - origAdd := keychainAddItem - origUpdate := keychainUpdateItem - defer func() { - keychainAddItem = origAdd - keychainUpdateItem = origUpdate - }() - - addCalled := false - updateCalled := false - keychainAddItem = func(item keychain.Item) error { - addCalled = true - return nil - } - keychainUpdateItem = func(_, _ keychain.Item) error { - updateCalled = true - return nil - } - - err := saveSecretToNativeStore(context.Background(), "cloudstic/store/prod", "password", "super-secret") - if err != nil { - t.Fatalf("saveSecretToNativeStore: %v", err) - } - if !addCalled { - t.Fatal("expected add to be called") - } - if updateCalled { - t.Fatal("did not expect update path on successful add") - } -} - -func TestSaveSecretToNativeStore_Failure(t *testing.T) { - origAdd := keychainAddItem - defer func() { keychainAddItem = origAdd }() - - keychainAddItem = func(keychain.Item) error { - return keychain.ErrorNotAvailable - } - - err := saveSecretToNativeStore(context.Background(), "svc", "acct", "secret") - if err == nil { - t.Fatal("expected error") - } - if !strings.Contains(err.Error(), "save secret in macOS keychain failed") { - t.Fatalf("unexpected error: %v", err) - } - if !strings.Contains(err.Error(), "-25291") { - t.Fatalf("expected keychain status code in output: %v", err) - } -} - -func TestSaveSecretToNativeStore_DuplicateUpdates(t *testing.T) { - origAdd := keychainAddItem - origUpdate := keychainUpdateItem - defer func() { - keychainAddItem = origAdd - keychainUpdateItem = origUpdate - }() - - keychainAddItem = func(keychain.Item) error { - return keychain.ErrorDuplicateItem - } - - updated := false - keychainUpdateItem = func(_, _ keychain.Item) error { - updated = true - return nil - } - - if err := saveSecretToNativeStore(context.Background(), "svc", "acct", "new-secret"); err != nil { - t.Fatalf("saveSecretToNativeStore: %v", err) - } - if !updated { - t.Fatal("expected duplicate path to call update") - } -} - -func TestNativeSecretExists_Success(t *testing.T) { - origGet := keychainGetGenericPassword - defer func() { keychainGetGenericPassword = origGet }() - - keychainGetGenericPassword = func(service, account, label, accessGroup string) ([]byte, error) { - if service != "cloudstic/store/prod" || account != "password" || label != "" || accessGroup != "" { - return nil, keychain.ErrorParam - } - return []byte("secret"), nil - } - - exists, err := nativeSecretExists(context.Background(), "cloudstic/store/prod", "password") - if err != nil { - t.Fatalf("nativeSecretExists: %v", err) - } - if !exists { - t.Fatal("expected exists=true") - } -} - -func TestNativeSecretExists_NotFound(t *testing.T) { - origGet := keychainGetGenericPassword - defer func() { keychainGetGenericPassword = origGet }() - - keychainGetGenericPassword = func(service, account, label, accessGroup string) ([]byte, error) { - return nil, keychain.ErrorItemNotFound - } - - exists, err := nativeSecretExists(context.Background(), "svc", "acct") - if err != nil { - t.Fatalf("nativeSecretExists: %v", err) - } - if exists { - t.Fatal("expected exists=false") - } -} - -func TestNativeSecretExists_NotFoundNilData(t *testing.T) { - origGet := keychainGetGenericPassword - defer func() { keychainGetGenericPassword = origGet }() - - keychainGetGenericPassword = func(service, account, label, accessGroup string) ([]byte, error) { - return nil, nil - } - - exists, err := nativeSecretExists(context.Background(), "svc", "acct") - if err != nil { - t.Fatalf("nativeSecretExists: %v", err) - } - if exists { - t.Fatal("expected exists=false for nil data result") - } -} - -func TestNativeSecretExists_OtherError(t *testing.T) { - origGet := keychainGetGenericPassword - defer func() { keychainGetGenericPassword = origGet }() - - keychainGetGenericPassword = func(service, account, label, accessGroup string) ([]byte, error) { - return nil, keychain.ErrorNotAvailable - } - - exists, err := nativeSecretExists(context.Background(), "svc", "acct") - if err == nil { - t.Fatal("expected error") - } - if exists { - t.Fatal("expected exists=false") - } - if !strings.Contains(err.Error(), "check secret in macOS keychain failed") { - t.Fatalf("unexpected error: %v", err) - } -} diff --git a/cmd/cloudstic/secret_store_stub.go b/cmd/cloudstic/secret_store_stub.go deleted file mode 100644 index 9ab1ee4..0000000 --- a/cmd/cloudstic/secret_store_stub.go +++ /dev/null @@ -1,16 +0,0 @@ -//go:build !darwin || (darwin && !cgo) - -package main - -import ( - "context" - "errors" -) - -func saveSecretToNativeStore(_ context.Context, _, _, _ string) error { - return errors.New("native secret write not supported on this platform") -} - -func nativeSecretExists(_ context.Context, _, _ string) (bool, error) { - return false, nil -} diff --git a/internal/secretref/keychain_backend.go b/internal/secretref/keychain_backend.go index 04a9115..9558b1d 100644 --- a/internal/secretref/keychain_backend.go +++ b/internal/secretref/keychain_backend.go @@ -13,22 +13,40 @@ var ( ) type keychainLookupFunc func(ctx context.Context, service, account string) (string, error) +type keychainExistsFunc func(ctx context.Context, service, account string) (bool, error) +type keychainStoreFunc func(ctx context.Context, service, account, value string) error // KeychainBackend resolves keychain://service/account references. type KeychainBackend struct { lookup keychainLookupFunc + exists keychainExistsFunc + store keychainStoreFunc } // NewKeychainBackend creates a keychain backend for the current platform. func NewKeychainBackend() *KeychainBackend { - return &KeychainBackend{lookup: defaultKeychainLookup} + return &KeychainBackend{ + lookup: defaultKeychainLookup, + exists: defaultKeychainExists, + store: defaultKeychainStore, + } } -func newKeychainBackendWithLookup(lookup keychainLookupFunc) *KeychainBackend { +func newKeychainBackendWithFns(lookup keychainLookupFunc, exists keychainExistsFunc, store keychainStoreFunc) *KeychainBackend { if lookup == nil { lookup = defaultKeychainLookup } - return &KeychainBackend{lookup: lookup} + if exists == nil { + exists = defaultKeychainExists + } + if store == nil { + store = defaultKeychainStore + } + return &KeychainBackend{lookup: lookup, exists: exists, store: store} +} + +func newKeychainBackendWithLookup(lookup keychainLookupFunc) *KeychainBackend { + return newKeychainBackendWithFns(lookup, nil, nil) } func parseKeychainPath(path string) (service string, account string, err error) { @@ -69,3 +87,47 @@ func (b *KeychainBackend) Resolve(ctx context.Context, ref Ref) (string, error) return value, nil } + +func (b *KeychainBackend) Scheme() string { return "keychain" } + +func (b *KeychainBackend) DisplayName() string { return "macOS Keychain" } + +func (b *KeychainBackend) DefaultRef(storeName, account string) string { + service := "cloudstic/store/" + storeName + return "keychain://" + service + "/" + account +} + +func (b *KeychainBackend) Exists(ctx context.Context, ref Ref) (bool, error) { + service, account, err := parseKeychainPath(ref.Path) + if err != nil { + return false, errorf(KindInvalidRef, ref.Raw, err.Error(), nil) + } + + exists, err := b.exists(ctx, service, account) + if err != nil { + switch { + case errors.Is(err, errKeychainUnavailable): + return false, errorf(KindBackendUnavailable, ref.Raw, err.Error(), err) + default: + return false, errorf(KindBackendUnavailable, ref.Raw, err.Error(), err) + } + } + return exists, nil +} + +func (b *KeychainBackend) Store(ctx context.Context, ref Ref, value string) error { + service, account, err := parseKeychainPath(ref.Path) + if err != nil { + return errorf(KindInvalidRef, ref.Raw, err.Error(), nil) + } + + if err := b.store(ctx, service, account, value); err != nil { + switch { + case errors.Is(err, errKeychainUnavailable): + return errorf(KindBackendUnavailable, ref.Raw, err.Error(), err) + default: + return errorf(KindBackendUnavailable, ref.Raw, err.Error(), err) + } + } + return nil +} diff --git a/internal/secretref/keychain_backend_darwin.go b/internal/secretref/keychain_backend_darwin.go index 2927a07..5808ec1 100644 --- a/internal/secretref/keychain_backend_darwin.go +++ b/internal/secretref/keychain_backend_darwin.go @@ -4,6 +4,7 @@ package secretref import ( "context" + "errors" "fmt" "strings" @@ -11,6 +12,8 @@ import ( ) var keychainGetGenericPasswordDarwin = keychain.GetGenericPassword +var keychainAddItemDarwin = keychain.AddItem +var keychainUpdateItemDarwin = keychain.UpdateItem func defaultKeychainLookup(ctx context.Context, service, account string) (string, error) { _ = ctx @@ -31,3 +34,53 @@ func defaultKeychainLookup(ctx context.Context, service, account string) (string } return strings.TrimRight(string(out), "\r\n"), nil } + +func defaultKeychainExists(ctx context.Context, service, account string) (bool, error) { + _, err := defaultKeychainLookup(ctx, service, account) + if err != nil { + switch { + case errors.Is(err, errKeychainNotFound): + return false, nil + default: + return false, err + } + } + return true, nil +} + +func defaultKeychainStore(_ context.Context, service, account, value string) error { + item := keychain.NewItem() + item.SetSecClass(keychain.SecClassGenericPassword) + item.SetService(service) + item.SetAccount(account) + item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly) + item.SetData([]byte(value)) + + if err := keychainAddItemDarwin(item); err != nil { + if err == keychain.ErrorDuplicateItem { + query := keychain.NewItem() + query.SetSecClass(keychain.SecClassGenericPassword) + query.SetService(service) + query.SetAccount(account) + + update := keychain.NewItem() + update.SetData([]byte(value)) + + if updateErr := keychainUpdateItemDarwin(query, update); updateErr != nil { + return mapKeychainStoreError(updateErr) + } + return nil + } + return mapKeychainStoreError(err) + } + return nil +} + +func mapKeychainStoreError(err error) error { + switch err { + case keychain.ErrorInteractionNotAllowed, keychain.ErrorNotAvailable, keychain.ErrorNoSuchKeychain: + return fmt.Errorf("%w: keychain locked or unavailable in this session", errKeychainUnavailable) + default: + return fmt.Errorf("keychain write failed: %w", err) + } +} diff --git a/internal/secretref/keychain_backend_darwin_test.go b/internal/secretref/keychain_backend_darwin_test.go index 37ac6ae..007278b 100644 --- a/internal/secretref/keychain_backend_darwin_test.go +++ b/internal/secretref/keychain_backend_darwin_test.go @@ -37,3 +37,45 @@ func TestDefaultKeychainLookup_MapsInteractionNotAllowed(t *testing.T) { t.Fatalf("expected errKeychainUnavailable, got %v", err) } } + +func TestDefaultKeychainStore_DuplicateUpdates(t *testing.T) { + origAdd := keychainAddItemDarwin + origUpdate := keychainUpdateItemDarwin + defer func() { + keychainAddItemDarwin = origAdd + keychainUpdateItemDarwin = origUpdate + }() + + keychainAddItemDarwin = func(keychain.Item) error { + return keychain.ErrorDuplicateItem + } + updated := false + keychainUpdateItemDarwin = func(_, _ keychain.Item) error { + updated = true + return nil + } + + if err := defaultKeychainStore(context.Background(), "svc", "acct", "secret"); err != nil { + t.Fatalf("defaultKeychainStore: %v", err) + } + if !updated { + t.Fatal("expected update on duplicate item") + } +} + +func TestDefaultKeychainExists_NotFound(t *testing.T) { + orig := keychainGetGenericPasswordDarwin + defer func() { keychainGetGenericPasswordDarwin = orig }() + + keychainGetGenericPasswordDarwin = func(service, account, label, accessGroup string) ([]byte, error) { + return nil, keychain.ErrorItemNotFound + } + + exists, err := defaultKeychainExists(context.Background(), "svc", "acct") + if err != nil { + t.Fatalf("defaultKeychainExists: %v", err) + } + if exists { + t.Fatal("expected exists=false") + } +} diff --git a/internal/secretref/keychain_backend_stub.go b/internal/secretref/keychain_backend_stub.go index 77c2625..d98ebe0 100644 --- a/internal/secretref/keychain_backend_stub.go +++ b/internal/secretref/keychain_backend_stub.go @@ -10,3 +10,11 @@ import ( func defaultKeychainLookup(_ context.Context, _, _ string) (string, error) { return "", fmt.Errorf("%w: keychain backend is only available on macOS", errKeychainUnavailable) } + +func defaultKeychainExists(_ context.Context, _, _ string) (bool, error) { + return false, fmt.Errorf("%w: keychain backend is only available on macOS", errKeychainUnavailable) +} + +func defaultKeychainStore(_ context.Context, _, _, _ string) error { + return fmt.Errorf("%w: keychain backend is only available on macOS", errKeychainUnavailable) +} diff --git a/internal/secretref/keychain_backend_test.go b/internal/secretref/keychain_backend_test.go index 4014e91..313d6ec 100644 --- a/internal/secretref/keychain_backend_test.go +++ b/internal/secretref/keychain_backend_test.go @@ -105,3 +105,46 @@ func TestKeychainBackend_ResolveErrors(t *testing.T) { }) } } + +func TestKeychainBackend_DefaultRef(t *testing.T) { + b := NewKeychainBackend() + if got := b.DefaultRef("prod", "password"); got != "keychain://cloudstic/store/prod/password" { + t.Fatalf("DefaultRef() = %q", got) + } +} + +func TestKeychainBackend_Exists(t *testing.T) { + b := newKeychainBackendWithFns( + func(context.Context, string, string) (string, error) { return "", nil }, + func(_ context.Context, service, account string) (bool, error) { + if service != "cloudstic/store/prod" || account != "password" { + t.Fatalf("unexpected args %q/%q", service, account) + } + return true, nil + }, + nil, + ) + exists, err := b.Exists(context.Background(), Ref{Raw: "keychain://cloudstic/store/prod/password", Scheme: "keychain", Path: "cloudstic/store/prod/password"}) + if err != nil { + t.Fatalf("Exists: %v", err) + } + if !exists { + t.Fatal("expected exists=true") + } +} + +func TestKeychainBackend_Store(t *testing.T) { + b := newKeychainBackendWithFns( + func(context.Context, string, string) (string, error) { return "", nil }, + nil, + func(_ context.Context, service, account, value string) error { + if service != "cloudstic/store/prod" || account != "password" || value != "secret" { + t.Fatalf("unexpected args %q/%q/%q", service, account, value) + } + return nil + }, + ) + if err := b.Store(context.Background(), Ref{Raw: "keychain://cloudstic/store/prod/password", Scheme: "keychain", Path: "cloudstic/store/prod/password"}, "secret"); err != nil { + t.Fatalf("Store: %v", err) + } +} diff --git a/internal/secretref/secretref.go b/internal/secretref/secretref.go index 6d6b64b..3ae735c 100644 --- a/internal/secretref/secretref.go +++ b/internal/secretref/secretref.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "regexp" + "slices" "strings" ) @@ -85,6 +86,17 @@ type Backend interface { Resolve(ctx context.Context, ref Ref) (string, error) } +// WritableBackend extends a backend with native-store write and existence checks +// for interactive CLI flows. +type WritableBackend interface { + Backend + Scheme() string + DisplayName() string + DefaultRef(storeName, account string) string + Exists(ctx context.Context, ref Ref) (bool, error) + Store(ctx context.Context, ref Ref, value string) error +} + // Resolver routes secret references by scheme. type Resolver struct { backends map[string]Backend @@ -111,14 +123,10 @@ func NewDefaultResolver() *Resolver { // Resolve parses and resolves a secret reference. func (r *Resolver) Resolve(ctx context.Context, raw string) (string, error) { - parsed, err := Parse(raw) + parsed, backend, err := r.lookupBackend(raw) if err != nil { return "", err } - backend, ok := r.backends[parsed.Scheme] - if !ok { - return "", errorf(KindBackendUnavailable, parsed.Raw, fmt.Sprintf("no backend registered for scheme %q", parsed.Scheme), nil) - } value, err := backend.Resolve(ctx, parsed) if err != nil { @@ -131,6 +139,79 @@ func (r *Resolver) Resolve(ctx context.Context, raw string) (string, error) { return value, nil } +// Exists reports whether a writable secret reference already exists. +func (r *Resolver) Exists(ctx context.Context, raw string) (bool, error) { + parsed, writable, err := r.lookupWritableBackend(raw) + if err != nil { + return false, err + } + + exists, err := writable.Exists(ctx, parsed) + if err != nil { + var refErr *Error + if errors.As(err, &refErr) { + return false, err + } + return false, errorf(KindBackendUnavailable, parsed.Raw, err.Error(), err) + } + return exists, nil +} + +// Store writes a secret value through a writable backend. +func (r *Resolver) Store(ctx context.Context, raw, value string) error { + parsed, writable, err := r.lookupWritableBackend(raw) + if err != nil { + return err + } + + if err := writable.Store(ctx, parsed, value); err != nil { + var refErr *Error + if errors.As(err, &refErr) { + return err + } + return errorf(KindBackendUnavailable, parsed.Raw, err.Error(), err) + } + return nil +} + +// WritableBackends returns registered backends that support interactive writes. +func (r *Resolver) WritableBackends() []WritableBackend { + backends := make([]WritableBackend, 0, len(r.backends)) + for _, backend := range r.backends { + if writable, ok := backend.(WritableBackend); ok { + backends = append(backends, writable) + } + } + slices.SortFunc(backends, func(a, b WritableBackend) int { + return strings.Compare(a.Scheme(), b.Scheme()) + }) + return backends +} + +func (r *Resolver) lookupBackend(raw string) (Ref, Backend, error) { + parsed, err := Parse(raw) + if err != nil { + return Ref{}, nil, err + } + backend, ok := r.backends[parsed.Scheme] + if !ok { + return Ref{}, nil, errorf(KindBackendUnavailable, parsed.Raw, fmt.Sprintf("no backend registered for scheme %q", parsed.Scheme), nil) + } + return parsed, backend, nil +} + +func (r *Resolver) lookupWritableBackend(raw string) (Ref, WritableBackend, error) { + parsed, backend, err := r.lookupBackend(raw) + if err != nil { + return Ref{}, nil, err + } + writable, ok := backend.(WritableBackend) + if !ok { + return Ref{}, nil, errorf(KindBackendUnavailable, parsed.Raw, fmt.Sprintf("scheme %q does not support writing secrets", parsed.Scheme), nil) + } + return parsed, writable, nil +} + type EnvLookup func(string) (string, bool) // EnvBackend resolves env://VAR references. diff --git a/internal/secretref/secretref_test.go b/internal/secretref/secretref_test.go index a7a37a9..926844c 100644 --- a/internal/secretref/secretref_test.go +++ b/internal/secretref/secretref_test.go @@ -3,6 +3,7 @@ package secretref import ( "context" "errors" + "fmt" "testing" ) @@ -98,3 +99,47 @@ func TestResolver_Errors(t *testing.T) { }) } } + +func TestResolver_WritableBackendsAndStore(t *testing.T) { + b := &testWritableBackend{scheme: "native", displayName: "Native", defaultRef: "native://cloudstic/store/prod/password"} + resolver := NewResolver(map[string]Backend{"native": b}) + + backends := resolver.WritableBackends() + if len(backends) != 1 || backends[0].Scheme() != "native" { + t.Fatalf("WritableBackends() = %#v", backends) + } + if err := resolver.Store(context.Background(), "native://cloudstic/store/prod/password", "secret"); err != nil { + t.Fatalf("Store: %v", err) + } + if !b.stored["native://cloudstic/store/prod/password"] { + t.Fatal("expected backend store to be called") + } + if exists, err := resolver.Exists(context.Background(), "native://cloudstic/store/prod/password"); err != nil || !exists { + t.Fatalf("Exists() = %v, %v", exists, err) + } +} + +type testWritableBackend struct { + scheme string + displayName string + defaultRef string + stored map[string]bool +} + +func (b *testWritableBackend) Resolve(context.Context, Ref) (string, error) { return "", nil } +func (b *testWritableBackend) Scheme() string { return b.scheme } +func (b *testWritableBackend) DisplayName() string { return b.displayName } +func (b *testWritableBackend) DefaultRef(string, string) string { return b.defaultRef } +func (b *testWritableBackend) Exists(_ context.Context, ref Ref) (bool, error) { + return b.stored[ref.Raw], nil +} +func (b *testWritableBackend) Store(_ context.Context, ref Ref, value string) error { + if value == "" { + return fmt.Errorf("empty") + } + if b.stored == nil { + b.stored = map[string]bool{} + } + b.stored[ref.Raw] = true + return nil +} From 3c65699620a6e22ba8a4e1fe1d1e0f576314af62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hermann?= Date: Tue, 17 Mar 2026 21:40:49 +0100 Subject: [PATCH 2/2] fix: hide unsupported writable secret backends --- cmd/cloudstic/cmd_store_test.go | 1 + internal/secretref/keychain_backend.go | 2 ++ internal/secretref/keychain_backend_darwin.go | 2 ++ internal/secretref/keychain_backend_stub.go | 2 ++ internal/secretref/secretref.go | 5 ++++- internal/secretref/secretref_test.go | 1 + 6 files changed, 12 insertions(+), 1 deletion(-) diff --git a/cmd/cloudstic/cmd_store_test.go b/cmd/cloudstic/cmd_store_test.go index eb0c9e1..e5e5792 100644 --- a/cmd/cloudstic/cmd_store_test.go +++ b/cmd/cloudstic/cmd_store_test.go @@ -25,6 +25,7 @@ type writableBackendStub struct { func (b writableBackendStub) Resolve(context.Context, secretref.Ref) (string, error) { return "", nil } func (b writableBackendStub) Scheme() string { return b.scheme } func (b writableBackendStub) DisplayName() string { return b.displayName } +func (b writableBackendStub) WriteSupported() bool { return true } func (b writableBackendStub) DefaultRef(string, string) string { return b.defaultRef } func (b writableBackendStub) Exists(ctx context.Context, ref secretref.Ref) (bool, error) { if b.exists == nil { diff --git a/internal/secretref/keychain_backend.go b/internal/secretref/keychain_backend.go index 9558b1d..2be9504 100644 --- a/internal/secretref/keychain_backend.go +++ b/internal/secretref/keychain_backend.go @@ -92,6 +92,8 @@ func (b *KeychainBackend) Scheme() string { return "keychain" } func (b *KeychainBackend) DisplayName() string { return "macOS Keychain" } +func (b *KeychainBackend) WriteSupported() bool { return defaultKeychainWriteSupported() } + func (b *KeychainBackend) DefaultRef(storeName, account string) string { service := "cloudstic/store/" + storeName return "keychain://" + service + "/" + account diff --git a/internal/secretref/keychain_backend_darwin.go b/internal/secretref/keychain_backend_darwin.go index 5808ec1..d1fba78 100644 --- a/internal/secretref/keychain_backend_darwin.go +++ b/internal/secretref/keychain_backend_darwin.go @@ -15,6 +15,8 @@ var keychainGetGenericPasswordDarwin = keychain.GetGenericPassword var keychainAddItemDarwin = keychain.AddItem var keychainUpdateItemDarwin = keychain.UpdateItem +func defaultKeychainWriteSupported() bool { return true } + func defaultKeychainLookup(ctx context.Context, service, account string) (string, error) { _ = ctx diff --git a/internal/secretref/keychain_backend_stub.go b/internal/secretref/keychain_backend_stub.go index d98ebe0..40daf9b 100644 --- a/internal/secretref/keychain_backend_stub.go +++ b/internal/secretref/keychain_backend_stub.go @@ -7,6 +7,8 @@ import ( "fmt" ) +func defaultKeychainWriteSupported() bool { return false } + func defaultKeychainLookup(_ context.Context, _, _ string) (string, error) { return "", fmt.Errorf("%w: keychain backend is only available on macOS", errKeychainUnavailable) } diff --git a/internal/secretref/secretref.go b/internal/secretref/secretref.go index 3ae735c..9d9abf4 100644 --- a/internal/secretref/secretref.go +++ b/internal/secretref/secretref.go @@ -92,6 +92,7 @@ type WritableBackend interface { Backend Scheme() string DisplayName() string + WriteSupported() bool DefaultRef(storeName, account string) string Exists(ctx context.Context, ref Ref) (bool, error) Store(ctx context.Context, ref Ref, value string) error @@ -179,7 +180,9 @@ func (r *Resolver) WritableBackends() []WritableBackend { backends := make([]WritableBackend, 0, len(r.backends)) for _, backend := range r.backends { if writable, ok := backend.(WritableBackend); ok { - backends = append(backends, writable) + if writable.WriteSupported() { + backends = append(backends, writable) + } } } slices.SortFunc(backends, func(a, b WritableBackend) int { diff --git a/internal/secretref/secretref_test.go b/internal/secretref/secretref_test.go index 926844c..cef1c78 100644 --- a/internal/secretref/secretref_test.go +++ b/internal/secretref/secretref_test.go @@ -129,6 +129,7 @@ type testWritableBackend struct { func (b *testWritableBackend) Resolve(context.Context, Ref) (string, error) { return "", nil } func (b *testWritableBackend) Scheme() string { return b.scheme } func (b *testWritableBackend) DisplayName() string { return b.displayName } +func (b *testWritableBackend) WriteSupported() bool { return true } func (b *testWritableBackend) DefaultRef(string, string) string { return b.defaultRef } func (b *testWritableBackend) Exists(_ context.Context, ref Ref) (bool, error) { return b.stored[ref.Raw], nil