From 5f6e6e10fa620689dfc6cd68ab40df388a80b7e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hermann?= Date: Mon, 16 Mar 2026 10:20:16 +0100 Subject: [PATCH] refactor: use native macOS keychain API for secret refs --- cmd/cloudstic/cmd_profile.go | 7 + cmd/cloudstic/cmd_store.go | 84 ++++++++++-- cmd/cloudstic/cmd_store_test.go | 10 +- cmd/cloudstic/completion.go | 21 ++- cmd/cloudstic/secret_store_darwin.go | 68 +++++---- cmd/cloudstic/secret_store_darwin_test.go | 129 ++++++++++++------ cmd/cloudstic/usage.go | 8 ++ go.mod | 1 + go.sum | 2 + internal/secretref/keychain_backend_darwin.go | 34 ++--- .../secretref/keychain_backend_darwin_test.go | 39 ++++++ ...eychain_backend_integration_darwin_test.go | 31 +++-- rfcs/0006-direct-to-filesystem-restore.md | 2 +- ...011-profile-credential-storage-backends.md | 65 ++++++++- 14 files changed, 380 insertions(+), 121 deletions(-) create mode 100644 internal/secretref/keychain_backend_darwin_test.go diff --git a/cmd/cloudstic/cmd_profile.go b/cmd/cloudstic/cmd_profile.go index f5773cc..c2e4ed9 100644 --- a/cmd/cloudstic/cmd_profile.go +++ b/cmd/cloudstic/cmd_profile.go @@ -385,6 +385,13 @@ func (r *runner) runProfileNew() int { if !storeHasExplicitEncryption(s) { r.promptEncryptionConfig(cfg, a.storeRef, a.profilesFile) } + if err := r.checkOrInitStore(cfg, a.storeRef, a.profilesFile, checkOrInitOptions{ + allowMissingSecrets: true, + warnOnMissingSecrets: true, + offerInit: true, + }); err != nil { + _, _ = fmt.Fprintf(r.errOut, "%v\n", err) + } } requiredProvider := profileProviderFromSource(a.source) diff --git a/cmd/cloudstic/cmd_store.go b/cmd/cloudstic/cmd_store.go index a1adb09..7b7fca1 100644 --- a/cmd/cloudstic/cmd_store.go +++ b/cmd/cloudstic/cmd_store.go @@ -21,7 +21,7 @@ func (r *runner) runStore() int { if len(os.Args) < 3 { _, _ = fmt.Fprintln(r.errOut, "Usage: cloudstic store [options]") _, _ = fmt.Fprintln(r.errOut, "") - _, _ = fmt.Fprintln(r.errOut, "Available subcommands: list, show, new, verify") + _, _ = fmt.Fprintln(r.errOut, "Available subcommands: list, show, new, verify, init") return 1 } @@ -34,6 +34,8 @@ func (r *runner) runStore() int { return r.runStoreNew() case "verify": return r.runStoreVerify() + case "init": + return r.runStoreInit() default: return r.fail("Unknown store subcommand: %s", os.Args[2]) } @@ -315,7 +317,11 @@ func (r *runner) runStoreNew() int { if forcePromptEncryption || !storeHasExplicitEncryption(s) { r.promptEncryptionConfig(cfg, *name, *profilesFile) } - if err := r.checkOrInitStore(cfg, *name, *profilesFile, true, !existedBefore, true); err != nil { + if err := r.checkOrInitStore(cfg, *name, *profilesFile, checkOrInitOptions{ + allowMissingSecrets: true, + warnOnMissingSecrets: !existedBefore, + offerInit: true, + }); err != nil { _, _ = fmt.Fprintf(r.errOut, "%v\n", err) } } @@ -364,18 +370,74 @@ func (r *runner) runStoreVerify() int { if _, ok := cfg.Stores[name]; !ok { return r.fail("Unknown store %q", name) } - if err := r.checkOrInitStore(cfg, name, *profilesFile, false, true, false); err != nil { + if err := r.checkOrInitStore(cfg, name, *profilesFile, checkOrInitOptions{ + warnOnMissingSecrets: true, + }); err != nil { return r.fail("%v", err) } return 0 } -func (r *runner) checkOrInitStore(cfg *cloudstic.ProfilesConfig, storeName, profilesFile string, allowMissingSecrets, warnOnMissingSecrets, offerInit bool) error { +func (r *runner) runStoreInit() int { + fs := flag.NewFlagSet("store init", flag.ExitOnError) + profilesFile := fs.String("profiles-file", envDefault("CLOUDSTIC_PROFILES_FILE", defaultProfilesPathFallback()), "Path to profiles YAML file") + yes := fs.Bool("yes", false, "Initialize without confirmation prompt") + _ = fs.Parse(reorderArgs(fs, os.Args[3:])) + if fs.NArg() > 1 { + return r.fail("usage: cloudstic store init [-profiles-file ] [-yes] ") + } + + name := "" + if fs.NArg() == 1 { + name = fs.Arg(0) + } + + cfg, err := cloudstic.LoadProfilesFile(*profilesFile) + if err != nil { + return r.fail("Failed to load profiles: %v", err) + } + if len(cfg.Stores) == 0 { + return r.fail("No stores configured") + } + + if name == "" { + if !r.canPrompt() { + return r.fail("usage: cloudstic store init [-profiles-file ] [-yes] ") + } + names := sortedKeys(cfg.Stores) + picked, pickErr := r.promptSelect("Select store", names) + if pickErr != nil { + return r.fail("Failed to select store: %v", pickErr) + } + name = picked + } + + if _, ok := cfg.Stores[name]; !ok { + return r.fail("Unknown store %q", name) + } + if err := r.checkOrInitStore(cfg, name, *profilesFile, checkOrInitOptions{ + warnOnMissingSecrets: true, + offerInit: true, + assumeYes: *yes, + }); err != nil { + return r.fail("%v", err) + } + return 0 +} + +type checkOrInitOptions struct { + allowMissingSecrets bool + warnOnMissingSecrets bool + offerInit bool + assumeYes bool +} + +func (r *runner) checkOrInitStore(cfg *cloudstic.ProfilesConfig, storeName, profilesFile string, opts checkOrInitOptions) error { s := cfg.Stores[storeName] g, err := globalFlagsFromProfileStore(s) if err != nil { - if allowMissingSecrets && isSecretNotFoundError(err) { - if warnOnMissingSecrets { + if opts.allowMissingSecrets && isSecretNotFoundError(err) { + if opts.warnOnMissingSecrets { _, _ = fmt.Fprintf(r.errOut, "Store credentials are configured but not currently available: %v\n", err) _, _ = fmt.Fprintf(r.errOut, "Set required secrets and run: cloudstic store verify %s\n", storeName) } @@ -409,12 +471,14 @@ func (r *runner) checkOrInitStore(cfg *cloudstic.ProfilesConfig, storeName, prof } _, _ = fmt.Fprintln(r.out, "Store is accessible but not yet initialized.") - if !offerInit { + if !opts.offerInit { return nil } - yes, promptErr := r.promptConfirm("Initialize it now?", true) - if promptErr != nil || !yes { - return nil + if !opts.assumeYes { + yes, promptErr := r.promptConfirm("Initialize it now?", true) + if promptErr != nil || !yes { + return nil + } } // Check if the store has encryption config. diff --git a/cmd/cloudstic/cmd_store_test.go b/cmd/cloudstic/cmd_store_test.go index e40bb82..2cb545b 100644 --- a/cmd/cloudstic/cmd_store_test.go +++ b/cmd/cloudstic/cmd_store_test.go @@ -305,7 +305,7 @@ func TestCheckOrInitStore_AlreadyInitialized(t *testing.T) { var out strings.Builder var errOut strings.Builder r := &runner{out: &out, errOut: &errOut} - if err := r.checkOrInitStore(cfg, "test", profilesPath, false, true, true); err != nil { + if err := r.checkOrInitStore(cfg, "test", profilesPath, checkOrInitOptions{warnOnMissingSecrets: true, offerInit: true}); err != nil { t.Fatalf("checkOrInitStore: %v", err) } @@ -343,7 +343,7 @@ func TestCheckOrInitStore_InitializedEncrypted_ValidCredentials(t *testing.T) { var out strings.Builder var errOut strings.Builder r := &runner{out: &out, errOut: &errOut} - if err := r.checkOrInitStore(cfg, "test", "profiles.yaml", false, true, true); err != nil { + if err := r.checkOrInitStore(cfg, "test", "profiles.yaml", checkOrInitOptions{warnOnMissingSecrets: true, offerInit: true}); err != nil { t.Fatalf("checkOrInitStore: %v", err) } if !strings.Contains(out.String(), "Repository is encrypted; verifying configured credentials") { @@ -381,7 +381,7 @@ func TestCheckOrInitStore_InitializedEncrypted_InvalidCredentials(t *testing.T) }} r := &runner{out: &strings.Builder{}, errOut: &strings.Builder{}} - err = r.checkOrInitStore(cfg, "test", "profiles.yaml", false, true, true) + err = r.checkOrInitStore(cfg, "test", "profiles.yaml", checkOrInitOptions{warnOnMissingSecrets: true, offerInit: true}) if err == nil { t.Fatal("expected error") } @@ -930,7 +930,7 @@ func TestCheckOrInitStore_MissingSecretAllowed(t *testing.T) { var errOut strings.Builder r := &runner{out: &out, errOut: &errOut} - if err := r.checkOrInitStore(cfg, "test", "profiles.yaml", true, true, true); err != nil { + if err := r.checkOrInitStore(cfg, "test", "profiles.yaml", checkOrInitOptions{allowMissingSecrets: true, warnOnMissingSecrets: true, offerInit: true}); err != nil { t.Fatalf("checkOrInitStore: %v", err) } if !strings.Contains(errOut.String(), "cloudstic store verify test") { @@ -949,7 +949,7 @@ func TestCheckOrInitStore_MissingSecretAllowedSilent(t *testing.T) { var errOut strings.Builder r := &runner{out: &out, errOut: &errOut} - if err := r.checkOrInitStore(cfg, "test", "profiles.yaml", true, false, true); err != nil { + if err := r.checkOrInitStore(cfg, "test", "profiles.yaml", checkOrInitOptions{allowMissingSecrets: true, offerInit: true}); err != nil { t.Fatalf("checkOrInitStore: %v", err) } if errOut.String() != "" { diff --git a/cmd/cloudstic/completion.go b/cmd/cloudstic/completion.go index 33a7dfa..6bb7dbc 100644 --- a/cmd/cloudstic/completion.go +++ b/cmd/cloudstic/completion.go @@ -171,7 +171,7 @@ _cloudstic() { esac done if [[ -z "$store_sub" ]]; then - COMPREPLY=($(compgen -W "list show new verify" -- "$cur")) + COMPREPLY=($(compgen -W "list show new verify init" -- "$cur")) return fi case "$store_sub" in @@ -181,6 +181,8 @@ _cloudstic() { cmd_flags="-profiles-file" ;; verify) cmd_flags="-profiles-file" ;; + init) + cmd_flags="-profiles-file -yes" ;; new) cmd_flags="-profiles-file -name -uri -s3-region -s3-profile -s3-endpoint -s3-access-key -s3-secret-key -s3-access-key-secret -s3-secret-key-secret -s3-access-key-env -s3-secret-key-env -s3-profile-env -store-sftp-password -store-sftp-key -store-sftp-password-secret -store-sftp-key-secret -store-sftp-password-env -store-sftp-key-env -password-secret -encryption-key-secret -recovery-key-secret -password-env -encryption-key-env -recovery-key-env -kms-key-arn -kms-region -kms-endpoint" ;; *) @@ -426,6 +428,7 @@ _cloudstic() { 'show:Show one store and its configuration' 'new:Create or update a store entry' 'verify:Verify store credentials and connectivity' + 'init:Initialize a store by reference' ) local store_sub local -i si=$((i+1)) @@ -450,6 +453,9 @@ _cloudstic() { verify) _arguments '-profiles-file[Path to profiles YAML file]:path:_files' ':store name:' ;; + init) + _arguments '-profiles-file[Path to profiles YAML file]:path:_files' '-yes[Initialize without confirmation prompt]' ':store name:' + ;; new) _arguments \ '-profiles-file[Path to profiles YAML file]:path:_files' \ @@ -667,6 +673,19 @@ complete -c cloudstic -n '__fish_seen_subcommand_from profile; and __fish_seen_s complete -c cloudstic -n '__fish_seen_subcommand_from profile; and __fish_seen_subcommand_from new' -l onedrive-client-id -x -d 'OneDrive OAuth client ID' complete -c cloudstic -n '__fish_seen_subcommand_from profile; and __fish_seen_subcommand_from new' -l onedrive-token-file -r -F -d 'OneDrive OAuth token file' +# store subcommands +complete -c cloudstic -n '__fish_seen_subcommand_from store; and not __fish_seen_subcommand_from list show new verify init' -a list -d 'List configured stores' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and not __fish_seen_subcommand_from list show new verify init' -a show -d 'Show one store and its configuration' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and not __fish_seen_subcommand_from list show new verify init' -a new -d 'Create or update a store entry' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and not __fish_seen_subcommand_from list show new verify init' -a verify -d 'Verify store credentials and connectivity' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and not __fish_seen_subcommand_from list show new verify init' -a init -d 'Initialize a store by reference' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and __fish_seen_subcommand_from list' -l profiles-file -r -F -d 'Path to profiles YAML file' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and __fish_seen_subcommand_from show' -l profiles-file -r -F -d 'Path to profiles YAML file' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and __fish_seen_subcommand_from new' -l profiles-file -r -F -d 'Path to profiles YAML file' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and __fish_seen_subcommand_from verify' -l profiles-file -r -F -d 'Path to profiles YAML file' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and __fish_seen_subcommand_from init' -l profiles-file -r -F -d 'Path to profiles YAML file' +complete -c cloudstic -n '__fish_seen_subcommand_from store; and __fish_seen_subcommand_from init' -l yes -d 'Initialize without confirmation prompt' + # auth subcommands complete -c cloudstic -n '__fish_seen_subcommand_from auth; and not __fish_seen_subcommand_from list show new login' -a list -d 'List auth entries from profiles.yaml' complete -c cloudstic -n '__fish_seen_subcommand_from auth; and not __fish_seen_subcommand_from list show new login' -a show -d 'Show one auth entry' diff --git a/cmd/cloudstic/secret_store_darwin.go b/cmd/cloudstic/secret_store_darwin.go index 748e980..1aceb2b 100644 --- a/cmd/cloudstic/secret_store_darwin.go +++ b/cmd/cloudstic/secret_store_darwin.go @@ -3,47 +3,61 @@ package main import ( - "bytes" "context" - "errors" "fmt" - "io" - "os/exec" - "strings" + + "github.com/keybase/go-keychain" ) -var execCommandContext = exec.CommandContext +var ( + keychainAddItem = keychain.AddItem + keychainUpdateItem = keychain.UpdateItem + keychainGetGenericPassword = keychain.GetGenericPassword + keychainGenericPasswordKind = keychain.SecClassGenericPassword +) func saveSecretToNativeStore(ctx context.Context, service, account, value string) error { - cmd := execCommandContext(ctx, "security", "add-generic-password", "-U", "-s", service, "-a", account, "-w") - cmd.Stdin = strings.NewReader(value + "\n" + value + "\n") - var stderr bytes.Buffer - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - msg := strings.TrimSpace(stderr.String()) - if msg == "" { - msg = err.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: %s", msg) + return fmt.Errorf("save secret in macOS keychain failed: %v", err) } return nil } func nativeSecretExists(ctx context.Context, service, account string) (bool, error) { - cmd := execCommandContext(ctx, "security", "find-generic-password", "-s", service, "-a", account) - cmd.Stdout = io.Discard - var stderr bytes.Buffer - cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { - var exitErr *exec.ExitError - if errors.As(err, &exitErr) && exitErr.ExitCode() == 44 { + _ = ctx + + data, err := keychainGetGenericPassword(service, account, "", "") + if err != nil { + if err == keychain.ErrorItemNotFound { return false, nil } - msg := strings.TrimSpace(stderr.String()) - if msg == "" { - msg = err.Error() - } - return false, fmt.Errorf("check secret in macOS keychain failed: %s", msg) + 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 index 96c8499..6d661f1 100644 --- a/cmd/cloudstic/secret_store_darwin_test.go +++ b/cmd/cloudstic/secret_store_darwin_test.go @@ -4,43 +4,49 @@ package main import ( "context" - "os/exec" - "reflect" "strings" "testing" + + "github.com/keybase/go-keychain" ) func TestSaveSecretToNativeStore_Success(t *testing.T) { - orig := execCommandContext - defer func() { execCommandContext = orig }() + origAdd := keychainAddItem + origUpdate := keychainUpdateItem + defer func() { + keychainAddItem = origAdd + keychainUpdateItem = origUpdate + }() - var gotName string - var gotArgs []string - execCommandContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { - gotName = name - gotArgs = append([]string{}, args...) - return exec.CommandContext(ctx, "sh", "-c", "exit 0") + 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 gotName != "security" { - t.Fatalf("command name=%q want security", gotName) + if !addCalled { + t.Fatal("expected add to be called") } - wantArgs := []string{"add-generic-password", "-U", "-s", "cloudstic/store/prod", "-a", "password", "-w"} - if !reflect.DeepEqual(gotArgs, wantArgs) { - t.Fatalf("command args=%v want=%v", gotArgs, wantArgs) + if updateCalled { + t.Fatal("did not expect update path on successful add") } } func TestSaveSecretToNativeStore_Failure(t *testing.T) { - orig := execCommandContext - defer func() { execCommandContext = orig }() + origAdd := keychainAddItem + defer func() { keychainAddItem = origAdd }() - execCommandContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { - return exec.CommandContext(ctx, "sh", "-c", "echo keychain failed 1>&2; exit 1") + keychainAddItem = func(keychain.Item) error { + return keychain.ErrorNotAvailable } err := saveSecretToNativeStore(context.Background(), "svc", "acct", "secret") @@ -50,21 +56,46 @@ func TestSaveSecretToNativeStore_Failure(t *testing.T) { if !strings.Contains(err.Error(), "save secret in macOS keychain failed") { t.Fatalf("unexpected error: %v", err) } - if !strings.Contains(err.Error(), "keychain failed") { - t.Fatalf("expected stderr in 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) { - orig := execCommandContext - defer func() { execCommandContext = orig }() + origGet := keychainGetGenericPassword + defer func() { keychainGetGenericPassword = origGet }() - var gotName string - var gotArgs []string - execCommandContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { - gotName = name - gotArgs = append([]string{}, args...) - return exec.CommandContext(ctx, "sh", "-c", "exit 0") + 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") @@ -74,21 +105,14 @@ func TestNativeSecretExists_Success(t *testing.T) { if !exists { t.Fatal("expected exists=true") } - if gotName != "security" { - t.Fatalf("command name=%q want security", gotName) - } - wantArgs := []string{"find-generic-password", "-s", "cloudstic/store/prod", "-a", "password"} - if !reflect.DeepEqual(gotArgs, wantArgs) { - t.Fatalf("command args=%v want=%v", gotArgs, wantArgs) - } } func TestNativeSecretExists_NotFound(t *testing.T) { - orig := execCommandContext - defer func() { execCommandContext = orig }() + origGet := keychainGetGenericPassword + defer func() { keychainGetGenericPassword = origGet }() - execCommandContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { - return exec.CommandContext(ctx, "sh", "-c", "exit 44") + keychainGetGenericPassword = func(service, account, label, accessGroup string) ([]byte, error) { + return nil, keychain.ErrorItemNotFound } exists, err := nativeSecretExists(context.Background(), "svc", "acct") @@ -100,12 +124,29 @@ func TestNativeSecretExists_NotFound(t *testing.T) { } } +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) { - orig := execCommandContext - defer func() { execCommandContext = orig }() + origGet := keychainGetGenericPassword + defer func() { keychainGetGenericPassword = origGet }() - execCommandContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { - return exec.CommandContext(ctx, "sh", "-c", "echo boom 1>&2; exit 2") + keychainGetGenericPassword = func(service, account, label, accessGroup string) ([]byte, error) { + return nil, keychain.ErrorNotAvailable } exists, err := nativeSecretExists(context.Background(), "svc", "acct") diff --git a/cmd/cloudstic/usage.go b/cmd/cloudstic/usage.go index 7f1e768..90d22f8 100644 --- a/cmd/cloudstic/usage.go +++ b/cmd/cloudstic/usage.go @@ -164,6 +164,14 @@ func printUsage() { t.Note(" Resolve store credentials and verify connectivity.") t.Blank() + t.Command("store init", "") + t.Flags([][2]string{ + {"-profiles-file ", ui.Env("Path to profiles YAML file", "CLOUDSTIC_PROFILES_FILE")}, + {"-yes", "Initialize without confirmation prompt"}, + }) + t.Note(" Initialize a configured store by reference from profiles.yaml.") + t.Blank() + t.Command("store new", "") t.Flags([][2]string{ {"-name ", "Store reference name"}, diff --git a/go.mod b/go.mod index bf6d712..104a5d0 100644 --- a/go.mod +++ b/go.mod @@ -74,6 +74,7 @@ require ( github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect + github.com/keybase/go-keychain v0.0.1 // indirect github.com/kr/fs v0.1.0 // indirect github.com/lufia/plan9stats v0.0.0-20211012122336-39d0f177ccd0 // indirect github.com/magiconair/properties v1.8.10 // indirect diff --git a/go.sum b/go.sum index 0edf256..3eb5e62 100644 --- a/go.sum +++ b/go.sum @@ -128,6 +128,8 @@ github.com/jotfs/fastcdc-go v0.2.0 h1:WHYIGk3k9NumGWfp4YMsemEcx/s4JKpGAa6tpCpHJO github.com/jotfs/fastcdc-go v0.2.0/go.mod h1:PGFBIloiASFbiKnkCd/hmHXxngxYDYtisyurJ/zyDNM= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/keybase/go-keychain v0.0.1 h1:way+bWYa6lDppZoZcgMbYsvC7GxljxrskdNInRtuthU= +github.com/keybase/go-keychain v0.0.1/go.mod h1:PdEILRW3i9D8JcdM+FmY6RwkHGnhHxXwkPPMeUgOK1k= github.com/klauspost/compress v1.18.0 h1:c/Cqfb0r+Yi+JtIEq73FWXVkRonBlf0CRNYc8Zttxdo= github.com/klauspost/compress v1.18.0/go.mod h1:2Pp+KzxcywXVXMr50+X0Q/Lsb43OQHYWRCY2AiWywWQ= github.com/klauspost/cpuid/v2 v2.2.6 h1:ndNyv040zDGIDh8thGkXYjnFtiN02M1PVVF+JE/48xc= diff --git a/internal/secretref/keychain_backend_darwin.go b/internal/secretref/keychain_backend_darwin.go index f2f09eb..97a0695 100644 --- a/internal/secretref/keychain_backend_darwin.go +++ b/internal/secretref/keychain_backend_darwin.go @@ -3,35 +3,31 @@ package secretref import ( - "bytes" "context" - "errors" "fmt" - "os/exec" "strings" + + "github.com/keybase/go-keychain" ) +var keychainGetGenericPasswordDarwin = keychain.GetGenericPassword + func defaultKeychainLookup(ctx context.Context, service, account string) (string, error) { - cmd := exec.CommandContext(ctx, "security", "find-generic-password", "-s", service, "-a", account, "-w") - var stderr bytes.Buffer - cmd.Stderr = &stderr - out, err := cmd.Output() + _ = ctx + + out, err := keychainGetGenericPasswordDarwin(service, account, "", "") if err != nil { - if errors.Is(err, exec.ErrNotFound) { - return "", fmt.Errorf("%w: macOS security tool not found", errKeychainUnavailable) - } - msg := strings.TrimSpace(stderr.String()) - if msg == "" { - msg = strings.TrimSpace(err.Error()) - } - lower := strings.ToLower(msg) - if strings.Contains(lower, "could not be found") || strings.Contains(lower, "item not found") { + switch err { + case keychain.ErrorItemNotFound: return "", errKeychainNotFound - } - if strings.Contains(lower, "user interaction is not allowed") || strings.Contains(lower, "interaction not allowed") { + case keychain.ErrorInteractionNotAllowed, keychain.ErrorNotAvailable, keychain.ErrorNoSuchKeychain: return "", fmt.Errorf("%w: keychain locked or unavailable in this session", errKeychainUnavailable) + default: + return "", fmt.Errorf("keychain lookup failed: %w", err) } - return "", fmt.Errorf("security find-generic-password failed: %s", msg) + } + if out == nil { + return "", errKeychainNotFound } return strings.TrimRight(string(out), "\r\n"), nil } diff --git a/internal/secretref/keychain_backend_darwin_test.go b/internal/secretref/keychain_backend_darwin_test.go new file mode 100644 index 0000000..46c7ff9 --- /dev/null +++ b/internal/secretref/keychain_backend_darwin_test.go @@ -0,0 +1,39 @@ +//go:build darwin + +package secretref + +import ( + "context" + "errors" + "testing" + + "github.com/keybase/go-keychain" +) + +func TestDefaultKeychainLookup_NotFoundOnNilData(t *testing.T) { + orig := keychainGetGenericPasswordDarwin + defer func() { keychainGetGenericPasswordDarwin = orig }() + + keychainGetGenericPasswordDarwin = func(service, account, label, accessGroup string) ([]byte, error) { + return nil, nil + } + + _, err := defaultKeychainLookup(context.Background(), "svc", "acct") + if !errors.Is(err, errKeychainNotFound) { + t.Fatalf("expected errKeychainNotFound, got %v", err) + } +} + +func TestDefaultKeychainLookup_MapsInteractionNotAllowed(t *testing.T) { + orig := keychainGetGenericPasswordDarwin + defer func() { keychainGetGenericPasswordDarwin = orig }() + + keychainGetGenericPasswordDarwin = func(service, account, label, accessGroup string) ([]byte, error) { + return nil, keychain.ErrorInteractionNotAllowed + } + + _, err := defaultKeychainLookup(context.Background(), "svc", "acct") + if !errors.Is(err, errKeychainUnavailable) { + t.Fatalf("expected errKeychainUnavailable, got %v", err) + } +} diff --git a/internal/secretref/keychain_backend_integration_darwin_test.go b/internal/secretref/keychain_backend_integration_darwin_test.go index 0d35860..39a9b32 100644 --- a/internal/secretref/keychain_backend_integration_darwin_test.go +++ b/internal/secretref/keychain_backend_integration_darwin_test.go @@ -6,11 +6,11 @@ import ( "context" "errors" "os" - "os/exec" "strconv" - "strings" "testing" "time" + + "github.com/keybase/go-keychain" ) func TestKeychainBackend_Integration(t *testing.T) { @@ -22,17 +22,28 @@ func TestKeychainBackend_Integration(t *testing.T) { account := "cloudstic-test-account" secret := "cloudstic-test-secret" - add := exec.Command("security", "add-generic-password", "-U", "-s", service, "-a", account, "-w", secret) - if out, err := add.CombinedOutput(); err != nil { - msg := strings.ToLower(strings.TrimSpace(string(out))) - if strings.Contains(msg, "user interaction is not allowed") || strings.Contains(msg, "interaction not allowed") || strings.Contains(msg, "not allowed") { - t.Skipf("keychain unavailable in this session: %s", strings.TrimSpace(string(out))) + item := keychain.NewGenericPassword(service, account, "", []byte(secret), "") + item.SetAccessible(keychain.AccessibleWhenUnlockedThisDeviceOnly) + if err := keychain.AddItem(item); err != nil { + if err == keychain.ErrorInteractionNotAllowed || err == keychain.ErrorNotAvailable || err == keychain.ErrorNoSuchKeychain { + t.Skipf("keychain unavailable in this session: %v", err) + } + if err == keychain.ErrorDuplicateItem { + query := keychain.NewItem() + query.SetSecClass(keychain.SecClassGenericPassword) + query.SetService(service) + query.SetAccount(account) + update := keychain.NewItem() + update.SetData([]byte(secret)) + if updateErr := keychain.UpdateItem(query, update); updateErr != nil { + t.Fatalf("update duplicate keychain item: %v", updateErr) + } + } else { + t.Fatalf("add keychain item failed: %v", err) } - t.Fatalf("add-generic-password failed: %v\n%s", err, out) } t.Cleanup(func() { - del := exec.Command("security", "delete-generic-password", "-s", service, "-a", account) - _, _ = del.CombinedOutput() + _ = keychain.DeleteGenericPasswordItem(service, account) }) b := NewKeychainBackend() diff --git a/rfcs/0006-direct-to-filesystem-restore.md b/rfcs/0006-direct-to-filesystem-restore.md index 2a69b1f..675c758 100644 --- a/rfcs/0006-direct-to-filesystem-restore.md +++ b/rfcs/0006-direct-to-filesystem-restore.md @@ -1,6 +1,6 @@ # RFC 0006: Direct-to-Filesystem Restore -* **Status:** Proposed +* **Status:** In Progress * **Date:** 2026-03-08 * **Affects:** `internal/engine/restore.go`, `cmd/cloudstic/cmd_restore.go`, `client.go`, `cmd/cloudstic/client_iface.go` * **Depends on:** RFC 0004 (Extended File Attributes) diff --git a/rfcs/0011-profile-credential-storage-backends.md b/rfcs/0011-profile-credential-storage-backends.md index b501e70..99c21a2 100644 --- a/rfcs/0011-profile-credential-storage-backends.md +++ b/rfcs/0011-profile-credential-storage-backends.md @@ -1,6 +1,6 @@ # RFC 0011: Profile Credential Storage Backends -- **Status:** Draft +- **Status:** In Progress - **Date:** 2026-03-15 - **Affects:** `internal/engine/profiles`, `cmd/cloudstic/{backup,store,profile,auth}`, docs @@ -21,6 +21,15 @@ new `*_secret` reference model that can resolve secrets from: The primary objective is to improve secret-at-rest protection for interactive users while preserving automation and headless workflows. +This RFC improves operational security and ergonomics, but it does not claim +protection against a fully compromised local user session. + +Current implementation status (as of this RFC revision): + +- `env://` is implemented. +- `keychain://` on macOS is implemented. +- `wincred://` and `secret-service://` are planned but not yet implemented. + ## Context Current profile secret handling (post RFC 0010): @@ -43,6 +52,8 @@ This is a good baseline, but gaps remain: - Keep current env-var-based workflows working unchanged. - Fail safely in headless/missing-native-backend environments. - Ensure CLI output never prints resolved secret values. +- Make backend guarantees explicit so docs do not overstate app-only or + hardware-bound protection. ## Non-goals @@ -50,6 +61,9 @@ This is a good baseline, but gaps remain: - No mandatory migration of existing `profiles.yaml` files. - No full redesign of repository key slot encryption (`pkg/keychain`). - No immediate migration of OAuth token JSON blobs to native stores. +- No promise that retrievable local secrets remain safe if an attacker controls + the user's session. +- No Secure Enclave non-exportable-key design in this RFC (potential follow-up). ## Proposal @@ -146,6 +160,22 @@ Rationale: - `store new` interactive mode may offer to save secrets to native backends and write the resulting `*_secret` reference. +### 6. Threat model and limits + +This RFC targets protection against accidental disclosure and weak secret +handling patterns (for example plaintext in config files, ad-hoc shell scripts, +or overuse of long-lived environment variables). + +Out of scope for this RFC: + +- A fully compromised local machine or user session. +- Guarantees that a retrievable secret can only ever be used by one app + regardless of runtime compromise. + +If the process can retrieve plaintext, a sufficiently privileged local attacker +can generally coerce or inspect that process. Native stores still improve +default posture, but they are not equivalent to non-exportable hardware keys. + Potential follow-up commands (not required in initial implementation): - `cloudstic secret set` @@ -161,6 +191,20 @@ Potential follow-up commands (not required in initial implementation): - If Keychain is unavailable/locked in non-interactive contexts, return a clear error and suggest `env://` fallback. +Current model in this RFC is retrievable secrets (`keychain://service/account`). +It does not imply Secure Enclave non-exportable keys, and does not assume +app-scoped ACL enforcement for the Cloudstic CLI process. + +Implementation note: + +- The macOS backend uses native Security framework calls via + `github.com/keybase/go-keychain` (not shelling out to the `security` CLI). +- Stored items use `AccessibleWhenUnlockedThisDeviceOnly` in current + implementation. + +Terminology note: on macOS, "hardware-backed" typically refers to Secure +Enclave integration, not TPM in the Windows/Linux sense. + ### Windows - Backend: Credential Manager (implementation may use DPAPI-backed storage). @@ -199,6 +243,15 @@ password_secret: keychain://cloudstic/store/prod/repo-password - Error messages should name fields and references, but not secret content. - Native backend implementations should avoid caching plaintext in long-lived global state. +- Documentation must not overclaim backend properties (for example, "app-only" + or "hardware-bound") unless those properties are explicitly implemented. + +Capability framing for this RFC: + +- `env://` and `keychain://` currently represent retrievable-secret flows. +- Secure Enclave non-exportable key operations are a possible future extension, + likely via a separate key-operation reference model rather than plaintext + `Resolve(...)`. ## Testing strategy @@ -211,10 +264,12 @@ password_secret: keychain://cloudstic/store/prod/repo-password ## Rollout plan -1. Add schema fields and resolver abstraction with `env://` support. -2. Wire resolver into backup/store profile resolution paths. +1. Add schema fields and resolver abstraction with `env://` support. (done) +2. Wire resolver into backup/store profile resolution paths. (done) 3. Add native providers incrementally per OS. -4. Update docs and examples to prefer `*_secret` references. + - macOS `keychain://` done + - Windows/Linux pending +4. Update docs and examples to prefer `*_secret` references. (in progress) 5. Consider deprecation warnings for `*_env` in a later RFC after adoption data. ## Open questions @@ -224,3 +279,5 @@ password_secret: keychain://cloudstic/store/prod/repo-password normalized under `native://` with OS-specific dispatch? - Should OAuth token files get a separate `token_secret` model later, or remain file-based with stronger path management? +- Should a follow-up RFC define a non-exportable key model (for example + signing/unwrapping) distinct from plaintext secret references?