From 270d5c4ba467bd93d2c190362c68af2f74a97d1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hermann?= Date: Mon, 16 Mar 2026 09:21:21 +0100 Subject: [PATCH 1/2] fix: address follow-up concerns from PR110 --- cmd/cloudstic/interactive.go | 2 +- cmd/cloudstic/secret_store_darwin.go | 17 ++++++++++++++-- cmd/cloudstic/secret_store_darwin_test.go | 24 +++++++++++++++++++++-- cmd/cloudstic/usage.go | 16 +++++++-------- docs/encryption.md | 5 +++-- docs/user-guide.md | 5 +++-- 6 files changed, 52 insertions(+), 17 deletions(-) diff --git a/cmd/cloudstic/interactive.go b/cmd/cloudstic/interactive.go index 527bbe6..fd0552f 100644 --- a/cmd/cloudstic/interactive.go +++ b/cmd/cloudstic/interactive.go @@ -83,5 +83,5 @@ func (r *runner) promptSecret(label string) (string, error) { if err != nil { return "", err } - return strings.TrimSpace(string(b)), nil + return strings.TrimRight(string(b), "\r\n"), nil } diff --git a/cmd/cloudstic/secret_store_darwin.go b/cmd/cloudstic/secret_store_darwin.go index 6125152..f7106e2 100644 --- a/cmd/cloudstic/secret_store_darwin.go +++ b/cmd/cloudstic/secret_store_darwin.go @@ -5,7 +5,9 @@ package main import ( "bytes" "context" + "errors" "fmt" + "io" "os/exec" "strings" ) @@ -27,9 +29,20 @@ func saveSecretToNativeStore(ctx context.Context, service, account, value string } func nativeSecretExists(ctx context.Context, service, account string) (bool, error) { - cmd := execCommandContext(ctx, "security", "find-generic-password", "-s", service, "-a", account, "-w") + 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 { - return false, nil + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 44 { + 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 true, nil } diff --git a/cmd/cloudstic/secret_store_darwin_test.go b/cmd/cloudstic/secret_store_darwin_test.go index 5449117..395abc6 100644 --- a/cmd/cloudstic/secret_store_darwin_test.go +++ b/cmd/cloudstic/secret_store_darwin_test.go @@ -77,7 +77,7 @@ func TestNativeSecretExists_Success(t *testing.T) { if gotName != "security" { t.Fatalf("command name=%q want security", gotName) } - wantArgs := []string{"find-generic-password", "-s", "cloudstic/store/prod", "-a", "password", "-w"} + 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) } @@ -88,7 +88,7 @@ func TestNativeSecretExists_NotFound(t *testing.T) { defer func() { execCommandContext = orig }() execCommandContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { - return exec.CommandContext(ctx, "sh", "-c", "exit 1") + return exec.CommandContext(ctx, "sh", "-c", "exit 44") } exists, err := nativeSecretExists(context.Background(), "svc", "acct") @@ -99,3 +99,23 @@ func TestNativeSecretExists_NotFound(t *testing.T) { t.Fatal("expected exists=false") } } + +func TestNativeSecretExists_OtherError(t *testing.T) { + orig := execCommandContext + defer func() { execCommandContext = orig }() + + execCommandContext = func(ctx context.Context, name string, args ...string) *exec.Cmd { + return exec.CommandContext(ctx, "sh", "-c", "echo boom 1>&2; exit 2") + } + + 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/usage.go b/cmd/cloudstic/usage.go index 60a7e00..7f1e768 100644 --- a/cmd/cloudstic/usage.go +++ b/cmd/cloudstic/usage.go @@ -173,20 +173,20 @@ func printUsage() { {"-s3-endpoint ", "S3-compatible endpoint URL"}, {"-s3-access-key ", "S3 static access key"}, {"-s3-secret-key ", "S3 static secret key"}, - {"-s3-access-key-secret ", "Secret reference for S3 access key (env://, keychain://, wincred://, secret-service://)"}, - {"-s3-secret-key-secret ", "Secret reference for S3 secret key (env://, keychain://, wincred://, secret-service://)"}, + {"-s3-access-key-secret ", "Secret reference for S3 access key (env://, keychain://)"}, + {"-s3-secret-key-secret ", "Secret reference for S3 secret key (env://, keychain://)"}, {"-s3-access-key-env ", "Env var name for S3 access key"}, {"-s3-secret-key-env ", "Env var name for S3 secret key"}, {"-s3-profile-env ", "Env var name for AWS profile"}, {"-store-sftp-password ", "SFTP password"}, {"-store-sftp-key ", "Path to SFTP private key"}, - {"-store-sftp-password-secret ", "Secret reference for SFTP password (env://, keychain://, wincred://, secret-service://)"}, - {"-store-sftp-key-secret ", "Secret reference for SFTP key path (env://, keychain://, wincred://, secret-service://)"}, + {"-store-sftp-password-secret ", "Secret reference for SFTP password (env://, keychain://)"}, + {"-store-sftp-key-secret ", "Secret reference for SFTP key path (env://, keychain://)"}, {"-store-sftp-password-env ", "Env var name for SFTP password"}, {"-store-sftp-key-env ", "Env var name for SFTP key path"}, - {"-password-secret ", "Secret reference for repository password (env://, keychain://, wincred://, secret-service://)"}, - {"-encryption-key-secret ", "Secret reference for platform key (env://, keychain://, wincred://, secret-service://)"}, - {"-recovery-key-secret ", "Secret reference for recovery key mnemonic (env://, keychain://, wincred://, secret-service://)"}, + {"-password-secret ", "Secret reference for repository password (env://, keychain://)"}, + {"-encryption-key-secret ", "Secret reference for platform key (env://, keychain://)"}, + {"-recovery-key-secret ", "Secret reference for recovery key mnemonic (env://, keychain://)"}, {"-password-env ", "Env var name for repository password"}, {"-encryption-key-env ", "Env var name for platform key (hex)"}, {"-recovery-key-env ", "Env var name for recovery key mnemonic"}, @@ -197,7 +197,7 @@ func printUsage() { }) t.Note(" Create or update a store entry in profiles.yaml.", " Prefer secret refs: -password-secret / -encryption-key-secret / -recovery-key-secret.", - " Legacy -*-env flags are still supported and auto-converted to env:// refs on write.", + " Legacy *-env flags are still supported and auto-converted to env:// refs on write.", " KMS settings are stored directly (ARN is not a secret).", ) t.Blank() diff --git a/docs/encryption.md b/docs/encryption.md index 7f076b1..c85a886 100644 --- a/docs/encryption.md +++ b/docs/encryption.md @@ -273,8 +273,9 @@ reference schemes: - `env://VAR_NAME` - `keychain://service/account` (macOS) -- `wincred://target` (Windows) -- `secret-service://collection/item` (Linux) + +`wincred://...` (Windows) and `secret-service://...` (Linux) are planned but not +yet available in the default CLI resolver. Examples: diff --git a/docs/user-guide.md b/docs/user-guide.md index 42161eb..61fe2df 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -567,8 +567,9 @@ Prefer `*_secret` fields and secret refs in `profiles.yaml`. Supported schemes: - `env://VAR_NAME` - `keychain://service/account` (macOS) -- `wincred://target` (Windows) -- `secret-service://collection/item` (Linux) + +`wincred://...` (Windows) and `secret-service://...` (Linux) are planned but not +yet available in the default CLI resolver. Legacy `*_env` fields are still read for backward compatibility, but new writes should use `*_secret`. From e1945cee699753fa587c389dc5d6bf44820c14b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Hermann?= Date: Mon, 16 Mar 2026 09:27:30 +0100 Subject: [PATCH 2/2] fix: harden keychain handling and resolver docs --- cmd/cloudstic/secret_store_darwin.go | 3 ++- cmd/cloudstic/secret_store_darwin_test.go | 2 +- internal/secretref/keychain_backend_darwin.go | 2 +- .../keychain_backend_integration_darwin_test.go | 10 ++++++++++ internal/secretref/secretref.go | 2 +- 5 files changed, 15 insertions(+), 4 deletions(-) diff --git a/cmd/cloudstic/secret_store_darwin.go b/cmd/cloudstic/secret_store_darwin.go index f7106e2..748e980 100644 --- a/cmd/cloudstic/secret_store_darwin.go +++ b/cmd/cloudstic/secret_store_darwin.go @@ -15,7 +15,8 @@ import ( var execCommandContext = exec.CommandContext func saveSecretToNativeStore(ctx context.Context, service, account, value string) error { - cmd := execCommandContext(ctx, "security", "add-generic-password", "-U", "-s", service, "-a", account, "-w", value) + 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 { diff --git a/cmd/cloudstic/secret_store_darwin_test.go b/cmd/cloudstic/secret_store_darwin_test.go index 395abc6..96c8499 100644 --- a/cmd/cloudstic/secret_store_darwin_test.go +++ b/cmd/cloudstic/secret_store_darwin_test.go @@ -29,7 +29,7 @@ func TestSaveSecretToNativeStore_Success(t *testing.T) { if gotName != "security" { t.Fatalf("command name=%q want security", gotName) } - wantArgs := []string{"add-generic-password", "-U", "-s", "cloudstic/store/prod", "-a", "password", "-w", "super-secret"} + 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) } diff --git a/internal/secretref/keychain_backend_darwin.go b/internal/secretref/keychain_backend_darwin.go index 07493e0..f2f09eb 100644 --- a/internal/secretref/keychain_backend_darwin.go +++ b/internal/secretref/keychain_backend_darwin.go @@ -33,5 +33,5 @@ func defaultKeychainLookup(ctx context.Context, service, account string) (string } return "", fmt.Errorf("security find-generic-password failed: %s", msg) } - return strings.TrimSpace(string(out)), nil + return strings.TrimRight(string(out), "\r\n"), nil } diff --git a/internal/secretref/keychain_backend_integration_darwin_test.go b/internal/secretref/keychain_backend_integration_darwin_test.go index b83b3a3..0d35860 100644 --- a/internal/secretref/keychain_backend_integration_darwin_test.go +++ b/internal/secretref/keychain_backend_integration_darwin_test.go @@ -4,9 +4,11 @@ package secretref import ( "context" + "errors" "os" "os/exec" "strconv" + "strings" "testing" "time" ) @@ -22,6 +24,10 @@ func TestKeychainBackend_Integration(t *testing.T) { 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))) + } t.Fatalf("add-generic-password failed: %v\n%s", err, out) } t.Cleanup(func() { @@ -36,6 +42,10 @@ func TestKeychainBackend_Integration(t *testing.T) { Path: service + "/" + account, }) if err != nil { + var refErr *Error + if errors.As(err, &refErr) && refErr.Kind == KindBackendUnavailable { + t.Skipf("keychain backend unavailable: %v", err) + } t.Fatalf("Resolve: %v", err) } if got != secret { diff --git a/internal/secretref/secretref.go b/internal/secretref/secretref.go index fc5ebdc..0162ad5 100644 --- a/internal/secretref/secretref.go +++ b/internal/secretref/secretref.go @@ -99,7 +99,7 @@ func NewResolver(backends map[string]Backend) *Resolver { return r } -// NewDefaultResolver builds the baseline resolver with env:// support. +// NewDefaultResolver builds the baseline resolver with env:// and keychain:// support. func NewDefaultResolver() *Resolver { return NewResolver(map[string]Backend{ "env": NewEnvBackend(nil),