From 8fef3caad862b4215ea6f2aecf64b33e6859df60 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Tue, 21 Apr 2026 09:56:59 -0700 Subject: [PATCH 1/3] Fix desktop secret CLI output --- app/redis_tls_auth_test.go | 4 ++-- app/secrets/plugins/keychain/plugin.go | 10 ++++++++- app/secrets/plugins/keychain/plugin_test.go | 22 +++++++++++++++++-- app/secrets/plugins/secretservice/plugin.go | 10 ++++++++- .../plugins/secretservice/plugin_test.go | 22 +++++++++++++++++-- 5 files changed, 60 insertions(+), 8 deletions(-) diff --git a/app/redis_tls_auth_test.go b/app/redis_tls_auth_test.go index bb1a919..ec5060b 100644 --- a/app/redis_tls_auth_test.go +++ b/app/redis_tls_auth_test.go @@ -147,7 +147,7 @@ func TestRateLimiterRedisAuthUsername(t *testing.T) { } func TestRateLimiterRedisTLSAuthRequiresVerification(t *testing.T) { - key, _ := rsa.GenerateKey(rand.Reader, 1024) + key, _ := rsa.GenerateKey(rand.Reader, 2048) tmpl := &x509.Certificate{ SerialNumber: big.NewInt(1), Subject: pkix.Name{CommonName: "srv"}, @@ -201,7 +201,7 @@ func TestRateLimiterRedisTLSAuthRequiresVerification(t *testing.T) { } func TestRateLimiterRedisTLSWithCA(t *testing.T) { - key, _ := rsa.GenerateKey(rand.Reader, 1024) + key, _ := rsa.GenerateKey(rand.Reader, 2048) tmpl := &x509.Certificate{ SerialNumber: big.NewInt(1), Subject: pkix.Name{CommonName: "srv"}, diff --git a/app/secrets/plugins/keychain/plugin.go b/app/secrets/plugins/keychain/plugin.go index 29ef216..438f1a2 100644 --- a/app/secrets/plugins/keychain/plugin.go +++ b/app/secrets/plugins/keychain/plugin.go @@ -47,7 +47,7 @@ func (keychainPlugin) Load(ctx context.Context, id string) (string, error) { return "", fmt.Errorf("keychain lookup failed: %w", err) } - return string(out), nil + return trimCommandLineTerminator(out), nil } func parseKeychainID(id string) (service, account string, err error) { @@ -65,4 +65,12 @@ func parseKeychainID(id string) (service, account string, err error) { return service, account, nil } +func trimCommandLineTerminator(out []byte) string { + secret := string(out) + if strings.HasSuffix(secret, "\r\n") { + return strings.TrimSuffix(secret, "\r\n") + } + return strings.TrimSuffix(secret, "\n") +} + func init() { secrets.Register(keychainPlugin{}) } diff --git a/app/secrets/plugins/keychain/plugin_test.go b/app/secrets/plugins/keychain/plugin_test.go index 249ad37..9511378 100644 --- a/app/secrets/plugins/keychain/plugin_test.go +++ b/app/secrets/plugins/keychain/plugin_test.go @@ -23,7 +23,7 @@ func TestKeychainPluginLoad(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != "super-secret\n" { + if got != "super-secret" { t.Fatalf("expected exact secret bytes, got %q", got) } @@ -46,11 +46,29 @@ func TestKeychainPluginLoadPreservesWhitespace(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != " secret with spaces \n" { + if got != " secret with spaces " { t.Fatalf("expected exact secret bytes, got %q", got) } } +func TestKeychainPluginLoadTrimsCRLFCommandTerminator(t *testing.T) { + old := execSecurityCommand + t.Cleanup(func() { execSecurityCommand = old }) + + execSecurityCommand = func(ctx context.Context, args ...string) ([]byte, error) { + return []byte("secret\r\n"), nil + } + + p := keychainPlugin{} + got, err := p.Load(context.Background(), "svc") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "secret" { + t.Fatalf("expected command terminator to be trimmed, got %q", got) + } +} + func TestKeychainPluginLoadServiceOnly(t *testing.T) { old := execSecurityCommand t.Cleanup(func() { execSecurityCommand = old }) diff --git a/app/secrets/plugins/secretservice/plugin.go b/app/secrets/plugins/secretservice/plugin.go index 8763f77..b279152 100644 --- a/app/secrets/plugins/secretservice/plugin.go +++ b/app/secrets/plugins/secretservice/plugin.go @@ -35,7 +35,7 @@ func (secretServicePlugin) Load(ctx context.Context, id string) (string, error) if err != nil { return "", fmt.Errorf("secretservice lookup failed: %w", err) } - return string(out), nil + return trimCommandLineTerminator(out), nil } func parseSecretServiceAttrs(id string) ([][2]string, error) { @@ -60,4 +60,12 @@ func parseSecretServiceAttrs(id string) ([][2]string, error) { return attrs, nil } +func trimCommandLineTerminator(out []byte) string { + secret := string(out) + if strings.HasSuffix(secret, "\r\n") { + return strings.TrimSuffix(secret, "\r\n") + } + return strings.TrimSuffix(secret, "\n") +} + func init() { secrets.Register(secretServicePlugin{}) } diff --git a/app/secrets/plugins/secretservice/plugin_test.go b/app/secrets/plugins/secretservice/plugin_test.go index f6514b0..c630c82 100644 --- a/app/secrets/plugins/secretservice/plugin_test.go +++ b/app/secrets/plugins/secretservice/plugin_test.go @@ -22,7 +22,7 @@ func TestSecretServicePluginLoad(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != "secret\n" { + if got != "secret" { t.Fatalf("expected exact secret bytes, got %q", got) } @@ -45,11 +45,29 @@ func TestSecretServicePluginLoadPreservesWhitespace(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != " secret with spaces \n" { + if got != " secret with spaces " { t.Fatalf("expected exact secret bytes, got %q", got) } } +func TestSecretServicePluginLoadTrimsCRLFCommandTerminator(t *testing.T) { + old := execSecretTool + t.Cleanup(func() { execSecretTool = old }) + + execSecretTool = func(ctx context.Context, args ...string) ([]byte, error) { + return []byte("secret\r\n"), nil + } + + p := secretServicePlugin{} + got, err := p.Load(context.Background(), "service=slack") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "secret" { + t.Fatalf("expected command terminator to be trimmed, got %q", got) + } +} + func TestSecretServicePluginLoadInvalidID(t *testing.T) { p := secretServicePlugin{} if _, err := p.Load(context.Background(), "bad"); err == nil { From 01af0657197c295d911b7746fe8bb73e72230081 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Tue, 21 Apr 2026 10:18:15 -0700 Subject: [PATCH 2/3] Preserve secretservice output bytes --- app/secrets/plugins/secretservice/plugin.go | 10 +--------- app/secrets/plugins/secretservice/plugin_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/app/secrets/plugins/secretservice/plugin.go b/app/secrets/plugins/secretservice/plugin.go index b279152..8763f77 100644 --- a/app/secrets/plugins/secretservice/plugin.go +++ b/app/secrets/plugins/secretservice/plugin.go @@ -35,7 +35,7 @@ func (secretServicePlugin) Load(ctx context.Context, id string) (string, error) if err != nil { return "", fmt.Errorf("secretservice lookup failed: %w", err) } - return trimCommandLineTerminator(out), nil + return string(out), nil } func parseSecretServiceAttrs(id string) ([][2]string, error) { @@ -60,12 +60,4 @@ func parseSecretServiceAttrs(id string) ([][2]string, error) { return attrs, nil } -func trimCommandLineTerminator(out []byte) string { - secret := string(out) - if strings.HasSuffix(secret, "\r\n") { - return strings.TrimSuffix(secret, "\r\n") - } - return strings.TrimSuffix(secret, "\n") -} - func init() { secrets.Register(secretServicePlugin{}) } diff --git a/app/secrets/plugins/secretservice/plugin_test.go b/app/secrets/plugins/secretservice/plugin_test.go index c630c82..23b9c6a 100644 --- a/app/secrets/plugins/secretservice/plugin_test.go +++ b/app/secrets/plugins/secretservice/plugin_test.go @@ -22,7 +22,7 @@ func TestSecretServicePluginLoad(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != "secret" { + if got != "secret\n" { t.Fatalf("expected exact secret bytes, got %q", got) } @@ -45,12 +45,12 @@ func TestSecretServicePluginLoadPreservesWhitespace(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != " secret with spaces " { + if got != " secret with spaces \n" { t.Fatalf("expected exact secret bytes, got %q", got) } } -func TestSecretServicePluginLoadTrimsCRLFCommandTerminator(t *testing.T) { +func TestSecretServicePluginLoadPreservesCRLFTrailingBytes(t *testing.T) { old := execSecretTool t.Cleanup(func() { execSecretTool = old }) @@ -63,8 +63,8 @@ func TestSecretServicePluginLoadTrimsCRLFCommandTerminator(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != "secret" { - t.Fatalf("expected command terminator to be trimmed, got %q", got) + if got != "secret\r\n" { + t.Fatalf("expected exact secret bytes, got %q", got) } } From 54ffbf43d725d229f5f1afba701cdbad6a6e0496 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Tue, 21 Apr 2026 10:26:24 -0700 Subject: [PATCH 3/3] Preserve keychain trailing carriage returns --- app/secrets/plugins/keychain/plugin.go | 6 +----- app/secrets/plugins/keychain/plugin_test.go | 6 +++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/app/secrets/plugins/keychain/plugin.go b/app/secrets/plugins/keychain/plugin.go index 438f1a2..16b86ac 100644 --- a/app/secrets/plugins/keychain/plugin.go +++ b/app/secrets/plugins/keychain/plugin.go @@ -66,11 +66,7 @@ func parseKeychainID(id string) (service, account string, err error) { } func trimCommandLineTerminator(out []byte) string { - secret := string(out) - if strings.HasSuffix(secret, "\r\n") { - return strings.TrimSuffix(secret, "\r\n") - } - return strings.TrimSuffix(secret, "\n") + return strings.TrimSuffix(string(out), "\n") } func init() { secrets.Register(keychainPlugin{}) } diff --git a/app/secrets/plugins/keychain/plugin_test.go b/app/secrets/plugins/keychain/plugin_test.go index 9511378..1771a26 100644 --- a/app/secrets/plugins/keychain/plugin_test.go +++ b/app/secrets/plugins/keychain/plugin_test.go @@ -51,7 +51,7 @@ func TestKeychainPluginLoadPreservesWhitespace(t *testing.T) { } } -func TestKeychainPluginLoadTrimsCRLFCommandTerminator(t *testing.T) { +func TestKeychainPluginLoadPreservesTrailingCRBeforeCommandLF(t *testing.T) { old := execSecurityCommand t.Cleanup(func() { execSecurityCommand = old }) @@ -64,8 +64,8 @@ func TestKeychainPluginLoadTrimsCRLFCommandTerminator(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != "secret" { - t.Fatalf("expected command terminator to be trimmed, got %q", got) + if got != "secret\r" { + t.Fatalf("expected trailing carriage return to be preserved, got %q", got) } }