From 240e7d932cc7de328781e1c58412061e0cc12496 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 14:43:55 -0700 Subject: [PATCH 01/12] Add keychain, secretservice, and wincred secret plugins --- app/secrets/plugins/keychain/plugin.go | 62 +++++++++++++ app/secrets/plugins/keychain/plugin_test.go | 92 +++++++++++++++++++ app/secrets/plugins/plugins.go | 3 + app/secrets/plugins/secretservice/plugin.go | 63 +++++++++++++ .../plugins/secretservice/plugin_test.go | 61 ++++++++++++ app/secrets/plugins/wincred/load_other.go | 9 ++ app/secrets/plugins/wincred/load_windows.go | 84 +++++++++++++++++ app/secrets/plugins/wincred/plugin.go | 19 ++++ app/secrets/plugins/wincred/plugin_test.go | 14 +++ docs/secret-backends.md | 6 ++ 10 files changed, 413 insertions(+) create mode 100644 app/secrets/plugins/keychain/plugin.go create mode 100644 app/secrets/plugins/keychain/plugin_test.go create mode 100644 app/secrets/plugins/secretservice/plugin.go create mode 100644 app/secrets/plugins/secretservice/plugin_test.go create mode 100644 app/secrets/plugins/wincred/load_other.go create mode 100644 app/secrets/plugins/wincred/load_windows.go create mode 100644 app/secrets/plugins/wincred/plugin.go create mode 100644 app/secrets/plugins/wincred/plugin_test.go diff --git a/app/secrets/plugins/keychain/plugin.go b/app/secrets/plugins/keychain/plugin.go new file mode 100644 index 0000000..6ae3c80 --- /dev/null +++ b/app/secrets/plugins/keychain/plugin.go @@ -0,0 +1,62 @@ +package plugins + +import ( + "context" + "errors" + "fmt" + "os/exec" + "strings" + + "github.com/winhowes/AuthTranslator/app/secrets" +) + +// keychainPlugin loads secrets from the macOS Keychain via the security CLI. +// +// Expected id formats: +// - "service" +// - "service#account" +type keychainPlugin struct{} + +var execSecurityCommand = func(ctx context.Context, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "security", args...) + return cmd.Output() +} + +func (keychainPlugin) Prefix() string { return "keychain" } + +func (keychainPlugin) Load(ctx context.Context, id string) (string, error) { + service, account := parseKeychainID(id) + if service == "" { + return "", fmt.Errorf("keychain service is required") + } + + args := []string{"find-generic-password", "-w", "-s", service} + if account != "" { + args = append(args, "-a", account) + } + + out, err := execSecurityCommand(ctx, args...) + if err != nil { + var ee *exec.ExitError + if errors.As(err, &ee) { + stderr := strings.TrimSpace(string(ee.Stderr)) + if stderr != "" { + return "", fmt.Errorf("keychain lookup failed: %s", stderr) + } + } + return "", fmt.Errorf("keychain lookup failed: %w", err) + } + + return strings.TrimSpace(string(out)), nil +} + +func parseKeychainID(id string) (service, account string) { + parts := strings.SplitN(id, "#", 2) + service = strings.TrimSpace(parts[0]) + if len(parts) == 2 { + account = strings.TrimSpace(parts[1]) + } + return service, account +} + +func init() { secrets.Register(keychainPlugin{}) } diff --git a/app/secrets/plugins/keychain/plugin_test.go b/app/secrets/plugins/keychain/plugin_test.go new file mode 100644 index 0000000..2173659 --- /dev/null +++ b/app/secrets/plugins/keychain/plugin_test.go @@ -0,0 +1,92 @@ +package plugins + +import ( + "context" + "os/exec" + "reflect" + "testing" +) + +func TestKeychainPluginLoad(t *testing.T) { + old := execSecurityCommand + t.Cleanup(func() { execSecurityCommand = old }) + + var gotArgs []string + execSecurityCommand = func(ctx context.Context, args ...string) ([]byte, error) { + gotArgs = append([]string{}, args...) + return []byte("super-secret\n"), nil + } + + p := keychainPlugin{} + got, err := p.Load(context.Background(), "slack#bot") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "super-secret" { + t.Fatalf("expected trimmed secret, got %q", got) + } + + wantArgs := []string{"find-generic-password", "-w", "-s", "slack", "-a", "bot"} + if !reflect.DeepEqual(gotArgs, wantArgs) { + t.Fatalf("args = %v, want %v", gotArgs, wantArgs) + } +} + +func TestKeychainPluginLoadServiceOnly(t *testing.T) { + old := execSecurityCommand + t.Cleanup(func() { execSecurityCommand = old }) + + var gotArgs []string + execSecurityCommand = func(ctx context.Context, args ...string) ([]byte, error) { + gotArgs = append([]string{}, args...) + return []byte("token"), nil + } + + p := keychainPlugin{} + if _, err := p.Load(context.Background(), "slack"); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + wantArgs := []string{"find-generic-password", "-w", "-s", "slack"} + if !reflect.DeepEqual(gotArgs, wantArgs) { + t.Fatalf("args = %v, want %v", gotArgs, wantArgs) + } +} + +func TestKeychainPluginLoadMissingService(t *testing.T) { + p := keychainPlugin{} + if _, err := p.Load(context.Background(), " "); err == nil { + t.Fatal("expected validation error") + } +} + +func TestKeychainPluginLoadExitError(t *testing.T) { + old := execSecurityCommand + t.Cleanup(func() { execSecurityCommand = old }) + + execSecurityCommand = func(ctx context.Context, args ...string) ([]byte, error) { + return nil, &exec.ExitError{Stderr: []byte("item not found")} + } + + p := keychainPlugin{} + if _, err := p.Load(context.Background(), "missing"); err == nil { + t.Fatal("expected lookup error") + } +} + +func TestParseKeychainID(t *testing.T) { + service, account := parseKeychainID("svc#acc") + if service != "svc" || account != "acc" { + t.Fatalf("unexpected parse result: %q %q", service, account) + } + + service, account = parseKeychainID("svc-only") + if service != "svc-only" || account != "" { + t.Fatalf("unexpected parse result: %q %q", service, account) + } + + service, account = parseKeychainID(" svc # acc ") + if service != "svc" || account != "acc" { + t.Fatalf("unexpected trimmed parse result: %q %q", service, account) + } +} diff --git a/app/secrets/plugins/plugins.go b/app/secrets/plugins/plugins.go index a0a100e..269c5bf 100644 --- a/app/secrets/plugins/plugins.go +++ b/app/secrets/plugins/plugins.go @@ -8,5 +8,8 @@ import ( _ "github.com/winhowes/AuthTranslator/app/secrets/plugins/file" _ "github.com/winhowes/AuthTranslator/app/secrets/plugins/gcp" _ "github.com/winhowes/AuthTranslator/app/secrets/plugins/k8s" + _ "github.com/winhowes/AuthTranslator/app/secrets/plugins/keychain" + _ "github.com/winhowes/AuthTranslator/app/secrets/plugins/secretservice" _ "github.com/winhowes/AuthTranslator/app/secrets/plugins/vault" + _ "github.com/winhowes/AuthTranslator/app/secrets/plugins/wincred" ) diff --git a/app/secrets/plugins/secretservice/plugin.go b/app/secrets/plugins/secretservice/plugin.go new file mode 100644 index 0000000..43d0611 --- /dev/null +++ b/app/secrets/plugins/secretservice/plugin.go @@ -0,0 +1,63 @@ +package plugins + +import ( + "context" + "fmt" + "os/exec" + "strings" + + "github.com/winhowes/AuthTranslator/app/secrets" +) + +// secretServicePlugin reads secrets from Linux Secret Service using secret-tool. +// id must be comma-separated key/value pairs, e.g. "service=slack,user=bot". +type secretServicePlugin struct{} + +var execSecretTool = func(ctx context.Context, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "secret-tool", args...) + return cmd.Output() +} + +func (secretServicePlugin) Prefix() string { return "secretservice" } + +func (secretServicePlugin) Load(ctx context.Context, id string) (string, error) { + attrs, err := parseSecretServiceAttrs(id) + if err != nil { + return "", err + } + + args := []string{"lookup"} + for _, attr := range attrs { + args = append(args, attr[0], attr[1]) + } + + out, err := execSecretTool(ctx, args...) + if err != nil { + return "", fmt.Errorf("secretservice lookup failed: %w", err) + } + return strings.TrimSpace(string(out)), nil +} + +func parseSecretServiceAttrs(id string) ([][2]string, error) { + id = strings.TrimSpace(id) + if id == "" { + return nil, fmt.Errorf("secretservice attributes are required") + } + parts := strings.Split(id, ",") + attrs := make([][2]string, 0, len(parts)) + for _, part := range parts { + kv := strings.SplitN(strings.TrimSpace(part), "=", 2) + if len(kv) != 2 { + return nil, fmt.Errorf("invalid secretservice attribute %q", part) + } + k := strings.TrimSpace(kv[0]) + v := strings.TrimSpace(kv[1]) + if k == "" || v == "" { + return nil, fmt.Errorf("invalid secretservice attribute %q", part) + } + attrs = append(attrs, [2]string{k, v}) + } + return attrs, nil +} + +func init() { secrets.Register(secretServicePlugin{}) } diff --git a/app/secrets/plugins/secretservice/plugin_test.go b/app/secrets/plugins/secretservice/plugin_test.go new file mode 100644 index 0000000..4488fac --- /dev/null +++ b/app/secrets/plugins/secretservice/plugin_test.go @@ -0,0 +1,61 @@ +package plugins + +import ( + "context" + "reflect" + "testing" +) + +func TestSecretServicePluginLoad(t *testing.T) { + old := execSecretTool + t.Cleanup(func() { execSecretTool = old }) + + var gotArgs []string + execSecretTool = func(ctx context.Context, args ...string) ([]byte, error) { + gotArgs = append([]string{}, args...) + return []byte("secret\n"), nil + } + + p := secretServicePlugin{} + got, err := p.Load(context.Background(), "service=slack,user=bot") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "secret" { + t.Fatalf("expected secret, got %q", got) + } + + wantArgs := []string{"lookup", "service", "slack", "user", "bot"} + if !reflect.DeepEqual(gotArgs, wantArgs) { + t.Fatalf("args = %v, want %v", gotArgs, wantArgs) + } +} + +func TestSecretServicePluginLoadInvalidID(t *testing.T) { + p := secretServicePlugin{} + if _, err := p.Load(context.Background(), "bad"); err == nil { + t.Fatal("expected parse error") + } +} + +func TestParseSecretServiceAttrs(t *testing.T) { + attrs, err := parseSecretServiceAttrs("service=slack,user=bot") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := [][2]string{{"service", "slack"}, {"user", "bot"}} + if !reflect.DeepEqual(attrs, want) { + t.Fatalf("attrs = %v, want %v", attrs, want) + } +} + +func TestParseSecretServiceAttrsErrors(t *testing.T) { + cases := []string{"", "missingequals", "=value", "key="} + for _, tc := range cases { + t.Run(tc, func(t *testing.T) { + if _, err := parseSecretServiceAttrs(tc); err == nil { + t.Fatalf("expected error for %q", tc) + } + }) + } +} diff --git a/app/secrets/plugins/wincred/load_other.go b/app/secrets/plugins/wincred/load_other.go new file mode 100644 index 0000000..672b568 --- /dev/null +++ b/app/secrets/plugins/wincred/load_other.go @@ -0,0 +1,9 @@ +//go:build !windows + +package plugins + +import "fmt" + +func loadWindowsCredential(id string) (string, error) { + return "", fmt.Errorf("wincred plugin is only supported on windows") +} diff --git a/app/secrets/plugins/wincred/load_windows.go b/app/secrets/plugins/wincred/load_windows.go new file mode 100644 index 0000000..480e054 --- /dev/null +++ b/app/secrets/plugins/wincred/load_windows.go @@ -0,0 +1,84 @@ +//go:build windows + +package plugins + +import ( + "fmt" + "syscall" + "unicode/utf16" + "unsafe" +) + +const credTypeGeneric = 1 + +type credential struct { + Flags uint32 + Type uint32 + TargetName *uint16 + Comment *uint16 + LastWritten syscall.Filetime + CredentialBlobSize uint32 + CredentialBlob *byte + Persist uint32 + AttributeCount uint32 + Attributes uintptr + TargetAlias *uint16 + UserName *uint16 +} + +var ( + advapi32 = syscall.NewLazyDLL("advapi32.dll") + procReadW = advapi32.NewProc("CredReadW") + procFree = advapi32.NewProc("CredFree") +) + +func loadWindowsCredential(id string) (string, error) { + target, err := syscall.UTF16PtrFromString(id) + if err != nil { + return "", err + } + + var credPtr uintptr + r1, _, callErr := procReadW.Call( + uintptr(unsafe.Pointer(target)), + uintptr(credTypeGeneric), + 0, + uintptr(unsafe.Pointer(&credPtr)), + ) + if r1 == 0 { + if callErr != nil && callErr != syscall.Errno(0) { + return "", fmt.Errorf("credread failed: %w", callErr) + } + return "", fmt.Errorf("credread failed") + } + defer procFree.Call(credPtr) + + cred := (*credential)(unsafe.Pointer(credPtr)) + if cred.CredentialBlob == nil || cred.CredentialBlobSize == 0 { + return "", fmt.Errorf("credential has no secret data") + } + + blob := unsafe.Slice(cred.CredentialBlob, cred.CredentialBlobSize) + return decodeCredentialBlob(blob), nil +} + +func decodeCredentialBlob(blob []byte) string { + if len(blob)%2 == 0 { + u16 := make([]uint16, 0, len(blob)/2) + looksUTF16 := true + for i := 0; i < len(blob); i += 2 { + v := uint16(blob[i]) | uint16(blob[i+1])<<8 + u16 = append(u16, v) + if blob[i+1] != 0 { + looksUTF16 = false + } + } + if looksUTF16 { + for len(u16) > 0 && u16[len(u16)-1] == 0 { + u16 = u16[:len(u16)-1] + } + return string(utf16.Decode(u16)) + } + } + return string(blob) +} diff --git a/app/secrets/plugins/wincred/plugin.go b/app/secrets/plugins/wincred/plugin.go new file mode 100644 index 0000000..249015b --- /dev/null +++ b/app/secrets/plugins/wincred/plugin.go @@ -0,0 +1,19 @@ +package plugins + +import ( + "context" + + "github.com/winhowes/AuthTranslator/app/secrets" +) + +// winCredPlugin loads generic credentials from Windows Credential Manager. +// id should be the credential TargetName. +type winCredPlugin struct{} + +func (winCredPlugin) Prefix() string { return "wincred" } + +func (winCredPlugin) Load(ctx context.Context, id string) (string, error) { + return loadWindowsCredential(id) +} + +func init() { secrets.Register(winCredPlugin{}) } diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go new file mode 100644 index 0000000..a2b54ed --- /dev/null +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -0,0 +1,14 @@ +package plugins + +import ( + "context" + "testing" +) + +func TestWinCredPluginLoad(t *testing.T) { + p := winCredPlugin{} + _, err := p.Load(context.Background(), "my-target") + if err == nil { + t.Fatal("expected wincred loader error on non-windows test environment") + } +} diff --git a/docs/secret-backends.md b/docs/secret-backends.md index e2dbf3b..ed86ffc 100644 --- a/docs/secret-backends.md +++ b/docs/secret-backends.md @@ -27,6 +27,9 @@ outgoing_auth: | `aws` | `aws:Ci0KU29tZUNpcGhlcnRleHQ=` | AES‑GCM encrypted values decrypted using `AWS_KMS_KEY`. | | `azure` | `azure:https://kv-name.vault.azure.net/secrets/secret-name` | AKS or VM SS with **Managed Identity**. | | `vault` | `vault:secret/data/slack` | Self‑hosted **HashiCorp Vault** cluster. | +| `keychain` | `keychain:github-cli#octocat` | macOS hosts with secrets in Keychain (`service#account`). | +| `secretservice` | `secretservice:service=slack,user=bot` | Linux desktops/servers with D-Bus Secret Service (`secret-tool`). | +| `wincred` | `wincred:github-cli` | Windows hosts using Credential Manager generic credentials. | | `dangerousLiteral` | `dangerousLiteral:__PLACEHOLDER__` | Rare cases where you need a literal sentinel string. | ### Literal placeholders (`dangerousLiteral:`) @@ -63,6 +66,9 @@ Some schemes rely on environment variables for authentication or decryption: | `azure` | `AZURE_TENANT_ID`, `AZURE_CLIENT_ID`, `AZURE_CLIENT_SECRET` | Credentials for fetching `azure:` secrets from Key Vault. | `azure:https://kv-name.vault.azure.net/secrets/token` | | `gcp` | _none_ | Uses the GCP metadata service when resolving `gcp:` secrets. | `gcp:projects/p/locations/l/keyRings/r/cryptoKeys/k:cipher` | | `vault` | `VAULT_ADDR`, `VAULT_TOKEN` | Fetches secrets from HashiCorp Vault via its HTTP API. | `vault:secret/data/api` reads from Vault | +| `keychain` | _none_ | Uses the macOS `security` CLI and current keychain access permissions. | `keychain:service#account` | +| `secretservice` | _none_ | Uses Linux `secret-tool` to query attributes like `service=...`. | `secretservice:service=slack,user=bot` | +| `wincred` | _none_ | Reads generic credentials by target name from Windows Credential Manager. | `wincred:github-cli` | | `dangerousLiteral` | _none_ | Value is stored directly in config; no external dependencies. | `dangerousLiteral:__PLACEHOLDER__` | For `file:` URIs that use the `:KEY` suffix, AuthTranslator treats the file as a simple `KEY=value` list: From 64e3c2139a72d6519e74e4e7156a163f369a5ab5 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 15:00:52 -0700 Subject: [PATCH 02/12] Fix wincred UTF-16 decoding for non-ASCII secrets --- app/secrets/plugins/wincred/decode.go | 51 +++++++++++++++++++++ app/secrets/plugins/wincred/load_windows.go | 22 --------- app/secrets/plugins/wincred/plugin_test.go | 42 +++++++++++++++++ 3 files changed, 93 insertions(+), 22 deletions(-) create mode 100644 app/secrets/plugins/wincred/decode.go diff --git a/app/secrets/plugins/wincred/decode.go b/app/secrets/plugins/wincred/decode.go new file mode 100644 index 0000000..2d54469 --- /dev/null +++ b/app/secrets/plugins/wincred/decode.go @@ -0,0 +1,51 @@ +package plugins + +import ( + "encoding/binary" + "unicode/utf16" +) + +func decodeCredentialBlob(blob []byte) string { + if len(blob)%2 != 0 { + return string(blob) + } + + u16 := make([]uint16, len(blob)/2) + for i := 0; i < len(u16); i++ { + u16[i] = binary.LittleEndian.Uint16(blob[i*2:]) + } + + for len(u16) > 0 && u16[len(u16)-1] == 0 { + u16 = u16[:len(u16)-1] + } + if len(u16) == 0 { + return "" + } + + if !isValidUTF16(u16) { + return string(blob) + } + + return string(utf16.Decode(u16)) +} + +func isValidUTF16(u16 []uint16) bool { + for i := 0; i < len(u16); i++ { + v := u16[i] + if v >= 0xD800 && v <= 0xDBFF { + if i+1 >= len(u16) { + return false + } + next := u16[i+1] + if next < 0xDC00 || next > 0xDFFF { + return false + } + i++ + continue + } + if v >= 0xDC00 && v <= 0xDFFF { + return false + } + } + return true +} diff --git a/app/secrets/plugins/wincred/load_windows.go b/app/secrets/plugins/wincred/load_windows.go index 480e054..292af08 100644 --- a/app/secrets/plugins/wincred/load_windows.go +++ b/app/secrets/plugins/wincred/load_windows.go @@ -5,7 +5,6 @@ package plugins import ( "fmt" "syscall" - "unicode/utf16" "unsafe" ) @@ -61,24 +60,3 @@ func loadWindowsCredential(id string) (string, error) { blob := unsafe.Slice(cred.CredentialBlob, cred.CredentialBlobSize) return decodeCredentialBlob(blob), nil } - -func decodeCredentialBlob(blob []byte) string { - if len(blob)%2 == 0 { - u16 := make([]uint16, 0, len(blob)/2) - looksUTF16 := true - for i := 0; i < len(blob); i += 2 { - v := uint16(blob[i]) | uint16(blob[i+1])<<8 - u16 = append(u16, v) - if blob[i+1] != 0 { - looksUTF16 = false - } - } - if looksUTF16 { - for len(u16) > 0 && u16[len(u16)-1] == 0 { - u16 = u16[:len(u16)-1] - } - return string(utf16.Decode(u16)) - } - } - return string(blob) -} diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index a2b54ed..bd3a9a5 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -2,7 +2,9 @@ package plugins import ( "context" + "encoding/binary" "testing" + "unicode/utf16" ) func TestWinCredPluginLoad(t *testing.T) { @@ -12,3 +14,43 @@ func TestWinCredPluginLoad(t *testing.T) { t.Fatal("expected wincred loader error on non-windows test environment") } } + +func TestDecodeCredentialBlobUTF16ASCII(t *testing.T) { + blob := encodeUTF16LE("secret") + if got := decodeCredentialBlob(blob); got != "secret" { + t.Fatalf("decodeCredentialBlob() = %q, want %q", got, "secret") + } +} + +func TestDecodeCredentialBlobUTF16Unicode(t *testing.T) { + want := "päss-東京-🔐" + blob := encodeUTF16LE(want) + if got := decodeCredentialBlob(blob); got != want { + t.Fatalf("decodeCredentialBlob() = %q, want %q", got, want) + } +} + +func TestDecodeCredentialBlobInvalidUTF16FallsBackToBytes(t *testing.T) { + blob := []byte{0x00, 0xD8, 0x41, 0x00} // lone high surrogate + 'A' + if got := decodeCredentialBlob(blob); got != string(blob) { + t.Fatalf("decodeCredentialBlob() = %q, want byte fallback", got) + } +} + +func TestDecodeCredentialBlobOddLengthFallsBackToBytes(t *testing.T) { + blob := []byte{0x61, 0x62, 0x63} + if got := decodeCredentialBlob(blob); got != string(blob) { + t.Fatalf("decodeCredentialBlob() = %q, want byte fallback", got) + } +} + +func encodeUTF16LE(s string) []byte { + u16 := utf16.Encode([]rune(s)) + blob := make([]byte, (len(u16)+1)*2) + for i, v := range u16 { + binary.LittleEndian.PutUint16(blob[i*2:], v) + } + // include a trailing UTF-16 NUL as Windows APIs commonly do. + binary.LittleEndian.PutUint16(blob[len(u16)*2:], 0) + return blob +} From 677b5414a28d8633f4fec388e63dacba5c30d71e Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 15:12:28 -0700 Subject: [PATCH 03/12] Harden wincred decoding heuristics and edge-case tests --- app/secrets/plugins/wincred/decode.go | 30 +++++++++++++++++++--- app/secrets/plugins/wincred/plugin_test.go | 20 ++++++++++++--- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/app/secrets/plugins/wincred/decode.go b/app/secrets/plugins/wincred/decode.go index 2d54469..ac0b621 100644 --- a/app/secrets/plugins/wincred/decode.go +++ b/app/secrets/plugins/wincred/decode.go @@ -1,13 +1,35 @@ package plugins import ( + "bytes" "encoding/binary" "unicode/utf16" + "unicode/utf8" ) +// decodeCredentialBlob converts Windows credential blobs into a string. +// +// CredMan generic credentials are often stored as UTF-16LE text, but callers +// can write arbitrary bytes. We therefore: +// - prefer UTF-8 when bytes are valid UTF-8 text without embedded NULs, +// - otherwise decode valid UTF-16LE, +// - and finally fall back to raw bytes with trailing NULs removed. func decodeCredentialBlob(blob []byte) string { + trimmed := bytes.TrimRight(blob, "\x00") + if utf8.Valid(trimmed) && !bytes.Contains(trimmed, []byte{0x00}) { + return string(trimmed) + } + + if s, ok := decodeUTF16LEBlob(blob); ok { + return s + } + + return string(trimmed) +} + +func decodeUTF16LEBlob(blob []byte) (string, bool) { if len(blob)%2 != 0 { - return string(blob) + return "", false } u16 := make([]uint16, len(blob)/2) @@ -19,14 +41,14 @@ func decodeCredentialBlob(blob []byte) string { u16 = u16[:len(u16)-1] } if len(u16) == 0 { - return "" + return "", true } if !isValidUTF16(u16) { - return string(blob) + return "", false } - return string(utf16.Decode(u16)) + return string(utf16.Decode(u16)), true } func isValidUTF16(u16 []uint16) bool { diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index bd3a9a5..8635169 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -32,15 +32,29 @@ func TestDecodeCredentialBlobUTF16Unicode(t *testing.T) { func TestDecodeCredentialBlobInvalidUTF16FallsBackToBytes(t *testing.T) { blob := []byte{0x00, 0xD8, 0x41, 0x00} // lone high surrogate + 'A' - if got := decodeCredentialBlob(blob); got != string(blob) { + if got := decodeCredentialBlob(blob); got != string(blob[:3]) { t.Fatalf("decodeCredentialBlob() = %q, want byte fallback", got) } } func TestDecodeCredentialBlobOddLengthFallsBackToBytes(t *testing.T) { blob := []byte{0x61, 0x62, 0x63} - if got := decodeCredentialBlob(blob); got != string(blob) { - t.Fatalf("decodeCredentialBlob() = %q, want byte fallback", got) + if got := decodeCredentialBlob(blob); got != "abc" { + t.Fatalf("decodeCredentialBlob() = %q, want %q", got, "abc") + } +} + +func TestDecodeCredentialBlobUTF8WithTrailingNull(t *testing.T) { + blob := append([]byte("päss-東京"), 0x00, 0x00) + if got := decodeCredentialBlob(blob); got != "päss-東京" { + t.Fatalf("decodeCredentialBlob() = %q, want %q", got, "päss-東京") + } +} + +func TestDecodeUTF16LEBlobInvalid(t *testing.T) { + blob := []byte{0x00, 0xDC} // lone low surrogate + if _, ok := decodeUTF16LEBlob(blob); ok { + t.Fatal("expected invalid UTF-16 blob") } } From 6945605f0338d5f5f00ae2fb4cde50e2846c1e86 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 15:12:44 -0700 Subject: [PATCH 04/12] Increase secret plugin test coverage to 100 percent --- app/secrets/plugins/keychain/plugin_test.go | 29 +++++++++++++++ .../plugins/secretservice/plugin_test.go | 15 ++++++++ app/secrets/plugins/wincred/plugin_test.go | 37 +++++++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/app/secrets/plugins/keychain/plugin_test.go b/app/secrets/plugins/keychain/plugin_test.go index 2173659..f483fa7 100644 --- a/app/secrets/plugins/keychain/plugin_test.go +++ b/app/secrets/plugins/keychain/plugin_test.go @@ -2,6 +2,7 @@ package plugins import ( "context" + "errors" "os/exec" "reflect" "testing" @@ -74,6 +75,34 @@ func TestKeychainPluginLoadExitError(t *testing.T) { } } +func TestKeychainPluginLoadExitErrorNoStderr(t *testing.T) { + old := execSecurityCommand + t.Cleanup(func() { execSecurityCommand = old }) + + execSecurityCommand = func(ctx context.Context, args ...string) ([]byte, error) { + return nil, &exec.ExitError{} + } + + p := keychainPlugin{} + if _, err := p.Load(context.Background(), "missing"); err == nil { + t.Fatal("expected lookup error") + } +} + +func TestKeychainPluginLoadCommandError(t *testing.T) { + old := execSecurityCommand + t.Cleanup(func() { execSecurityCommand = old }) + + execSecurityCommand = func(ctx context.Context, args ...string) ([]byte, error) { + return nil, errors.New("command missing") + } + + p := keychainPlugin{} + if _, err := p.Load(context.Background(), "service"); err == nil { + t.Fatal("expected lookup error") + } +} + func TestParseKeychainID(t *testing.T) { service, account := parseKeychainID("svc#acc") if service != "svc" || account != "acc" { diff --git a/app/secrets/plugins/secretservice/plugin_test.go b/app/secrets/plugins/secretservice/plugin_test.go index 4488fac..5d6f572 100644 --- a/app/secrets/plugins/secretservice/plugin_test.go +++ b/app/secrets/plugins/secretservice/plugin_test.go @@ -2,6 +2,7 @@ package plugins import ( "context" + "errors" "reflect" "testing" ) @@ -38,6 +39,20 @@ func TestSecretServicePluginLoadInvalidID(t *testing.T) { } } +func TestSecretServicePluginLoadCommandError(t *testing.T) { + old := execSecretTool + t.Cleanup(func() { execSecretTool = old }) + + execSecretTool = func(ctx context.Context, args ...string) ([]byte, error) { + return nil, errors.New("secret-tool failed") + } + + p := secretServicePlugin{} + if _, err := p.Load(context.Background(), "service=slack"); err == nil { + t.Fatal("expected command error") + } +} + func TestParseSecretServiceAttrs(t *testing.T) { attrs, err := parseSecretServiceAttrs("service=slack,user=bot") if err != nil { diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index 8635169..4f441fb 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -58,6 +58,43 @@ func TestDecodeUTF16LEBlobInvalid(t *testing.T) { } } +func TestDecodeUTF16LEBlobEmptyAfterTrim(t *testing.T) { + blob := []byte{0x00, 0x00} + got, ok := decodeUTF16LEBlob(blob) + if !ok || got != "" { + t.Fatalf("decodeUTF16LEBlob() = (%q,%v), want (\"\",true)", got, ok) + } +} + +func TestDecodeUTF16LEBlobValidSurrogatePair(t *testing.T) { + blob := encodeUTF16LE("🔐") + got, ok := decodeUTF16LEBlob(blob) + if !ok || got != "🔐" { + t.Fatalf("decodeUTF16LEBlob() = (%q,%v), want (\"🔐\",true)", got, ok) + } +} + +func TestDecodeUTF16LEBlobInvalidHighSurrogatePairing(t *testing.T) { + blob := []byte{0x00, 0xD8, 0x41, 0x00} + if _, ok := decodeUTF16LEBlob(blob); ok { + t.Fatal("expected invalid surrogate pairing") + } +} + +func TestDecodeUTF16LEBlobLoneHighSurrogate(t *testing.T) { + blob := []byte{0x00, 0xD8} + if _, ok := decodeUTF16LEBlob(blob); ok { + t.Fatal("expected lone high surrogate to be invalid") + } +} + +func TestDecodeUTF16LEBlobOddLength(t *testing.T) { + blob := []byte{0x41, 0x00, 0x42} + if _, ok := decodeUTF16LEBlob(blob); ok { + t.Fatal("expected odd-length blob to be invalid UTF-16") + } +} + func encodeUTF16LE(s string) []byte { u16 := utf16.Encode([]rune(s)) blob := make([]byte, (len(u16)+1)*2) From 8d31849ea61bb88ed269ae13ccc023b9a41ecbf9 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 15:38:24 -0700 Subject: [PATCH 05/12] Preserve secret bytes and harden wincred generic blob decoding --- app/secrets/plugins/keychain/plugin.go | 2 +- app/secrets/plugins/keychain/plugin_test.go | 22 +++++++- app/secrets/plugins/secretservice/plugin.go | 2 +- .../plugins/secretservice/plugin_test.go | 22 +++++++- app/secrets/plugins/wincred/decode.go | 56 +++++++++++++++---- app/secrets/plugins/wincred/plugin_test.go | 31 +++++++++- 6 files changed, 116 insertions(+), 19 deletions(-) diff --git a/app/secrets/plugins/keychain/plugin.go b/app/secrets/plugins/keychain/plugin.go index 6ae3c80..c5c175b 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 strings.TrimSpace(string(out)), nil + return string(out), nil } func parseKeychainID(id string) (service, account string) { diff --git a/app/secrets/plugins/keychain/plugin_test.go b/app/secrets/plugins/keychain/plugin_test.go index f483fa7..31aff5a 100644 --- a/app/secrets/plugins/keychain/plugin_test.go +++ b/app/secrets/plugins/keychain/plugin_test.go @@ -23,8 +23,8 @@ func TestKeychainPluginLoad(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != "super-secret" { - t.Fatalf("expected trimmed secret, got %q", got) + if got != "super-secret\n" { + t.Fatalf("expected exact secret bytes, got %q", got) } wantArgs := []string{"find-generic-password", "-w", "-s", "slack", "-a", "bot"} @@ -33,6 +33,24 @@ func TestKeychainPluginLoad(t *testing.T) { } } +func TestKeychainPluginLoadPreservesWhitespace(t *testing.T) { + old := execSecurityCommand + t.Cleanup(func() { execSecurityCommand = old }) + + execSecurityCommand = func(ctx context.Context, args ...string) ([]byte, error) { + return []byte(" secret with spaces \n"), nil + } + + p := keychainPlugin{} + got, err := p.Load(context.Background(), "svc") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != " secret with spaces \n" { + t.Fatalf("expected exact secret bytes, 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 43d0611..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 strings.TrimSpace(string(out)), nil + return string(out), nil } func parseSecretServiceAttrs(id string) ([][2]string, error) { diff --git a/app/secrets/plugins/secretservice/plugin_test.go b/app/secrets/plugins/secretservice/plugin_test.go index 5d6f572..4d30c10 100644 --- a/app/secrets/plugins/secretservice/plugin_test.go +++ b/app/secrets/plugins/secretservice/plugin_test.go @@ -22,8 +22,8 @@ func TestSecretServicePluginLoad(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got != "secret" { - t.Fatalf("expected secret, got %q", got) + if got != "secret\n" { + t.Fatalf("expected exact secret bytes, got %q", got) } wantArgs := []string{"lookup", "service", "slack", "user", "bot"} @@ -32,6 +32,24 @@ func TestSecretServicePluginLoad(t *testing.T) { } } +func TestSecretServicePluginLoadPreservesWhitespace(t *testing.T) { + old := execSecretTool + t.Cleanup(func() { execSecretTool = old }) + + execSecretTool = func(ctx context.Context, args ...string) ([]byte, error) { + return []byte(" secret with spaces \n"), nil + } + + p := secretServicePlugin{} + got, err := p.Load(context.Background(), "service=slack") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != " secret with spaces \n" { + t.Fatalf("expected exact secret bytes, got %q", got) + } +} + func TestSecretServicePluginLoadInvalidID(t *testing.T) { p := secretServicePlugin{} if _, err := p.Load(context.Background(), "bad"); err == nil { diff --git a/app/secrets/plugins/wincred/decode.go b/app/secrets/plugins/wincred/decode.go index ac0b621..6c800f3 100644 --- a/app/secrets/plugins/wincred/decode.go +++ b/app/secrets/plugins/wincred/decode.go @@ -7,24 +7,56 @@ import ( "unicode/utf8" ) -// decodeCredentialBlob converts Windows credential blobs into a string. +// decodeCredentialBlob converts a CredentialBlob to text conservatively. // -// CredMan generic credentials are often stored as UTF-16LE text, but callers -// can write arbitrary bytes. We therefore: -// - prefer UTF-8 when bytes are valid UTF-8 text without embedded NULs, -// - otherwise decode valid UTF-16LE, -// - and finally fall back to raw bytes with trailing NULs removed. +// For CRED_TYPE_GENERIC, CredentialBlob is application-defined bytes. To avoid +// corrupting non-UTF16 blobs, we only decode as UTF-16LE when the payload looks +// like UTF-16LE (BOM or enough NUL-byte structure). Otherwise we keep bytes as-is. func decodeCredentialBlob(blob []byte) string { - trimmed := bytes.TrimRight(blob, "\x00") - if utf8.Valid(trimmed) && !bytes.Contains(trimmed, []byte{0x00}) { - return string(trimmed) + trimmedNUL := bytes.TrimRight(blob, "\x00") + if utf8.Valid(trimmedNUL) && !bytes.Contains(trimmedNUL, []byte{0x00}) { + return string(trimmedNUL) } - if s, ok := decodeUTF16LEBlob(blob); ok { - return s + if looksLikeUTF16LE(blob) { + if s, ok := decodeUTF16LEBlob(blob); ok { + return s + } + } + + return string(blob) +} + +func looksLikeUTF16LE(blob []byte) bool { + if len(blob) < 2 || len(blob)%2 != 0 { + return false + } + + // UTF-16LE BOM. + if len(blob) >= 2 && blob[0] == 0xFF && blob[1] == 0xFE { + return true + } + + // Heuristic: UTF-16 text often contains many zero bytes in one parity (ASCII + // in UTF-16LE has zeros at odd indices) or explicit NUL terminators. + oddZeros, evenZeros := 0, 0 + for i, b := range blob { + if b != 0 { + continue + } + if i%2 == 0 { + evenZeros++ + } else { + oddZeros++ + } + } + + threshold := len(blob) / 4 + if threshold == 0 { + threshold = 1 } - return string(trimmed) + return oddZeros >= threshold || evenZeros >= threshold } func decodeUTF16LEBlob(blob []byte) (string, bool) { diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index 4f441fb..e82fb8b 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -32,7 +32,7 @@ func TestDecodeCredentialBlobUTF16Unicode(t *testing.T) { func TestDecodeCredentialBlobInvalidUTF16FallsBackToBytes(t *testing.T) { blob := []byte{0x00, 0xD8, 0x41, 0x00} // lone high surrogate + 'A' - if got := decodeCredentialBlob(blob); got != string(blob[:3]) { + if got := decodeCredentialBlob(blob); got != string(blob) { t.Fatalf("decodeCredentialBlob() = %q, want byte fallback", got) } } @@ -44,6 +44,35 @@ func TestDecodeCredentialBlobOddLengthFallsBackToBytes(t *testing.T) { } } +func TestDecodeCredentialBlobEvenLengthNonUTF16Binary(t *testing.T) { + blob := []byte{0x80, 0x81, 0x82, 0x83} + if got := decodeCredentialBlob(blob); got != string(blob) { + t.Fatalf("decodeCredentialBlob() = %q, want raw bytes", got) + } +} + +func TestLooksLikeUTF16LE(t *testing.T) { + tests := []struct { + name string + blob []byte + want bool + }{ + {name: "odd length", blob: []byte{0x41}, want: false}, + {name: "len2 threshold path", blob: []byte{0x00, 0x00}, want: true}, + {name: "bom", blob: []byte{0xFF, 0xFE, 0x41, 0x00}, want: true}, + {name: "ascii utf16 style", blob: []byte{0x41, 0x00, 0x42, 0x00}, want: true}, + {name: "binary without nul pattern", blob: []byte{0x80, 0x81, 0x82, 0x83}, want: false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if got := looksLikeUTF16LE(tc.blob); got != tc.want { + t.Fatalf("looksLikeUTF16LE(%v) = %v, want %v", tc.blob, got, tc.want) + } + }) + } +} + func TestDecodeCredentialBlobUTF8WithTrailingNull(t *testing.T) { blob := append([]byte("päss-東京"), 0x00, 0x00) if got := decodeCredentialBlob(blob); got != "päss-東京" { From e3c1d5e8db749b43dd45827e2c1d6f688bebb669 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 15:45:06 -0700 Subject: [PATCH 06/12] Make go test -cover reach 100 for new secret plugins --- app/secrets/plugins/keychain/plugin_test.go | 9 +++++++++ app/secrets/plugins/secretservice/plugin_test.go | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/app/secrets/plugins/keychain/plugin_test.go b/app/secrets/plugins/keychain/plugin_test.go index 31aff5a..31843b0 100644 --- a/app/secrets/plugins/keychain/plugin_test.go +++ b/app/secrets/plugins/keychain/plugin_test.go @@ -121,6 +121,15 @@ func TestKeychainPluginLoadCommandError(t *testing.T) { } } +func TestExecSecurityCommandDefault(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + if _, err := execSecurityCommand(ctx, "find-generic-password", "-w", "-s", "unused"); err == nil { + t.Fatal("expected error from canceled context") + } +} + func TestParseKeychainID(t *testing.T) { service, account := parseKeychainID("svc#acc") if service != "svc" || account != "acc" { diff --git a/app/secrets/plugins/secretservice/plugin_test.go b/app/secrets/plugins/secretservice/plugin_test.go index 4d30c10..f6514b0 100644 --- a/app/secrets/plugins/secretservice/plugin_test.go +++ b/app/secrets/plugins/secretservice/plugin_test.go @@ -82,6 +82,15 @@ func TestParseSecretServiceAttrs(t *testing.T) { } } +func TestExecSecretToolDefault(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + if _, err := execSecretTool(ctx, "lookup", "service", "unused"); err == nil { + t.Fatal("expected error from canceled context") + } +} + func TestParseSecretServiceAttrsErrors(t *testing.T) { cases := []string{"", "missingequals", "=value", "key="} for _, tc := range cases { From b497e152fe8b97dbca2545cd64d95282b2292010 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 15:47:30 -0700 Subject: [PATCH 07/12] Strip UTF-16 BOM when decoding wincred blobs --- app/secrets/plugins/wincred/decode.go | 3 +++ app/secrets/plugins/wincred/plugin_test.go | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/app/secrets/plugins/wincred/decode.go b/app/secrets/plugins/wincred/decode.go index 6c800f3..413ff37 100644 --- a/app/secrets/plugins/wincred/decode.go +++ b/app/secrets/plugins/wincred/decode.go @@ -72,6 +72,9 @@ func decodeUTF16LEBlob(blob []byte) (string, bool) { for len(u16) > 0 && u16[len(u16)-1] == 0 { u16 = u16[:len(u16)-1] } + if len(u16) > 0 && u16[0] == 0xFEFF { + u16 = u16[1:] + } if len(u16) == 0 { return "", true } diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index e82fb8b..203c0fb 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -22,6 +22,13 @@ func TestDecodeCredentialBlobUTF16ASCII(t *testing.T) { } } +func TestDecodeCredentialBlobUTF16WithBOM(t *testing.T) { + blob := append([]byte{0xFF, 0xFE}, encodeUTF16LE("secret")...) + if got := decodeCredentialBlob(blob); got != "secret" { + t.Fatalf("decodeCredentialBlob() = %q, want %q", got, "secret") + } +} + func TestDecodeCredentialBlobUTF16Unicode(t *testing.T) { want := "päss-東京-🔐" blob := encodeUTF16LE(want) From 851665206844847913d6b546ad7417d54ba6779b Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 16:08:45 -0700 Subject: [PATCH 08/12] Tighten wincred UTF-16 detection for generic blobs --- app/secrets/plugins/wincred/decode.go | 40 +++++++++++++++++----- app/secrets/plugins/wincred/plugin_test.go | 22 +++++++++++- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/app/secrets/plugins/wincred/decode.go b/app/secrets/plugins/wincred/decode.go index 413ff37..6692bd6 100644 --- a/app/secrets/plugins/wincred/decode.go +++ b/app/secrets/plugins/wincred/decode.go @@ -3,6 +3,7 @@ package plugins import ( "bytes" "encoding/binary" + "unicode" "unicode/utf16" "unicode/utf8" ) @@ -11,7 +12,7 @@ import ( // // For CRED_TYPE_GENERIC, CredentialBlob is application-defined bytes. To avoid // corrupting non-UTF16 blobs, we only decode as UTF-16LE when the payload looks -// like UTF-16LE (BOM or enough NUL-byte structure). Otherwise we keep bytes as-is. +// like UTF-16LE. Otherwise we keep bytes as-is. func decodeCredentialBlob(blob []byte) string { trimmedNUL := bytes.TrimRight(blob, "\x00") if utf8.Valid(trimmedNUL) && !bytes.Contains(trimmedNUL, []byte{0x00}) { @@ -33,13 +34,14 @@ func looksLikeUTF16LE(blob []byte) bool { } // UTF-16LE BOM. - if len(blob) >= 2 && blob[0] == 0xFF && blob[1] == 0xFE { + if blob[0] == 0xFF && blob[1] == 0xFE { return true } - // Heuristic: UTF-16 text often contains many zero bytes in one parity (ASCII - // in UTF-16LE has zeros at odd indices) or explicit NUL terminators. + // Strong structural signal: one parity is almost all zero bytes (ASCII-like + // UTF-16LE has zeros at odd indices). oddZeros, evenZeros := 0, 0 + paritySlots := len(blob) / 2 for i, b := range blob { if b != 0 { continue @@ -50,13 +52,35 @@ func looksLikeUTF16LE(blob []byte) bool { oddZeros++ } } + if oddZeros == paritySlots || evenZeros == paritySlots { + return true + } - threshold := len(blob) / 4 - if threshold == 0 { - threshold = 1 + // Also accept a UTF-16-style null-terminated payload when it decodes cleanly + // to plausible text. + if len(blob) >= 4 && blob[len(blob)-2] == 0 && blob[len(blob)-1] == 0 { + s, ok := decodeUTF16LEBlob(blob) + if ok && isProbablyText(s) { + return true + } } - return oddZeros >= threshold || evenZeros >= threshold + return false +} + +func isProbablyText(s string) bool { + if s == "" { + return true + } + for _, r := range s { + if r == '\n' || r == '\r' || r == '\t' { + continue + } + if unicode.IsControl(r) { + return false + } + } + return true } func decodeUTF16LEBlob(blob []byte) (string, bool) { diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index 203c0fb..d39d598 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -37,6 +37,13 @@ func TestDecodeCredentialBlobUTF16Unicode(t *testing.T) { } } +func TestDecodeCredentialBlobShortEvenWithSingleZeroFallsBack(t *testing.T) { + blob := []byte{0x00, 0xAB, 0xCD, 0xEF} + if got := decodeCredentialBlob(blob); got != string(blob) { + t.Fatalf("decodeCredentialBlob() = %q, want raw bytes", got) + } +} + func TestDecodeCredentialBlobInvalidUTF16FallsBackToBytes(t *testing.T) { blob := []byte{0x00, 0xD8, 0x41, 0x00} // lone high surrogate + 'A' if got := decodeCredentialBlob(blob); got != string(blob) { @@ -65,9 +72,10 @@ func TestLooksLikeUTF16LE(t *testing.T) { want bool }{ {name: "odd length", blob: []byte{0x41}, want: false}, - {name: "len2 threshold path", blob: []byte{0x00, 0x00}, want: true}, {name: "bom", blob: []byte{0xFF, 0xFE, 0x41, 0x00}, want: true}, {name: "ascii utf16 style", blob: []byte{0x41, 0x00, 0x42, 0x00}, want: true}, + {name: "short even one zero", blob: []byte{0x00, 0xAB, 0xCD, 0xEF}, want: false}, + {name: "null terminated plausible utf16", blob: []byte{0x71, 0x67, 0xAC, 0x4E, 0x00, 0x00}, want: true}, {name: "binary without nul pattern", blob: []byte{0x80, 0x81, 0x82, 0x83}, want: false}, } @@ -87,6 +95,18 @@ func TestDecodeCredentialBlobUTF8WithTrailingNull(t *testing.T) { } } +func TestIsProbablyText(t *testing.T) { + if !isProbablyText("hello東京\n\r\t") { + t.Fatal("expected printable text to be accepted") + } + if !isProbablyText("") { + t.Fatal("expected empty string to be accepted") + } + if isProbablyText("bad\x01text") { + t.Fatal("expected control-character text to be rejected") + } +} + func TestDecodeUTF16LEBlobInvalid(t *testing.T) { blob := []byte{0x00, 0xDC} // lone low surrogate if _, ok := decodeUTF16LEBlob(blob); ok { From 302b40bf2ac74cfa7b122f5446ffa638b96aec29 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 18:33:37 -0700 Subject: [PATCH 09/12] Replace wincred auto-heuristics with explicit decode modes --- app/secrets/plugins/wincred/decode.go | 86 +++---------- app/secrets/plugins/wincred/load_other.go | 2 +- app/secrets/plugins/wincred/load_windows.go | 6 +- app/secrets/plugins/wincred/plugin.go | 33 ++++- app/secrets/plugins/wincred/plugin_test.go | 135 +++++++++++--------- docs/secret-backends.md | 4 +- 6 files changed, 126 insertions(+), 140 deletions(-) diff --git a/app/secrets/plugins/wincred/decode.go b/app/secrets/plugins/wincred/decode.go index 6692bd6..d010d1c 100644 --- a/app/secrets/plugins/wincred/decode.go +++ b/app/secrets/plugins/wincred/decode.go @@ -1,86 +1,30 @@ package plugins import ( - "bytes" "encoding/binary" - "unicode" + "fmt" "unicode/utf16" "unicode/utf8" ) -// decodeCredentialBlob converts a CredentialBlob to text conservatively. -// -// For CRED_TYPE_GENERIC, CredentialBlob is application-defined bytes. To avoid -// corrupting non-UTF16 blobs, we only decode as UTF-16LE when the payload looks -// like UTF-16LE. Otherwise we keep bytes as-is. -func decodeCredentialBlob(blob []byte) string { - trimmedNUL := bytes.TrimRight(blob, "\x00") - if utf8.Valid(trimmedNUL) && !bytes.Contains(trimmedNUL, []byte{0x00}) { - return string(trimmedNUL) - } - - if looksLikeUTF16LE(blob) { - if s, ok := decodeUTF16LEBlob(blob); ok { - return s - } - } - - return string(blob) -} - -func looksLikeUTF16LE(blob []byte) bool { - if len(blob) < 2 || len(blob)%2 != 0 { - return false - } - - // UTF-16LE BOM. - if blob[0] == 0xFF && blob[1] == 0xFE { - return true - } - - // Strong structural signal: one parity is almost all zero bytes (ASCII-like - // UTF-16LE has zeros at odd indices). - oddZeros, evenZeros := 0, 0 - paritySlots := len(blob) / 2 - for i, b := range blob { - if b != 0 { - continue - } - if i%2 == 0 { - evenZeros++ - } else { - oddZeros++ +func decodeCredentialBlob(blob []byte, mode string) (string, error) { + switch mode { + case "raw": + return string(blob), nil + case "utf8": + if !utf8.Valid(blob) { + return "", fmt.Errorf("credential blob is not valid utf-8") } - } - if oddZeros == paritySlots || evenZeros == paritySlots { - return true - } - - // Also accept a UTF-16-style null-terminated payload when it decodes cleanly - // to plausible text. - if len(blob) >= 4 && blob[len(blob)-2] == 0 && blob[len(blob)-1] == 0 { + return string(blob), nil + case "utf16le": s, ok := decodeUTF16LEBlob(blob) - if ok && isProbablyText(s) { - return true + if !ok { + return "", fmt.Errorf("credential blob is not valid utf-16le") } + return s, nil + default: + return "", fmt.Errorf("unsupported decode mode %q", mode) } - - return false -} - -func isProbablyText(s string) bool { - if s == "" { - return true - } - for _, r := range s { - if r == '\n' || r == '\r' || r == '\t' { - continue - } - if unicode.IsControl(r) { - return false - } - } - return true } func decodeUTF16LEBlob(blob []byte) (string, bool) { diff --git a/app/secrets/plugins/wincred/load_other.go b/app/secrets/plugins/wincred/load_other.go index 672b568..6b8b832 100644 --- a/app/secrets/plugins/wincred/load_other.go +++ b/app/secrets/plugins/wincred/load_other.go @@ -4,6 +4,6 @@ package plugins import "fmt" -func loadWindowsCredential(id string) (string, error) { +func loadWindowsCredential(targetName, mode string) (string, error) { return "", fmt.Errorf("wincred plugin is only supported on windows") } diff --git a/app/secrets/plugins/wincred/load_windows.go b/app/secrets/plugins/wincred/load_windows.go index 292af08..2686523 100644 --- a/app/secrets/plugins/wincred/load_windows.go +++ b/app/secrets/plugins/wincred/load_windows.go @@ -31,8 +31,8 @@ var ( procFree = advapi32.NewProc("CredFree") ) -func loadWindowsCredential(id string) (string, error) { - target, err := syscall.UTF16PtrFromString(id) +func loadWindowsCredential(targetName, mode string) (string, error) { + target, err := syscall.UTF16PtrFromString(targetName) if err != nil { return "", err } @@ -58,5 +58,5 @@ func loadWindowsCredential(id string) (string, error) { } blob := unsafe.Slice(cred.CredentialBlob, cred.CredentialBlobSize) - return decodeCredentialBlob(blob), nil + return decodeCredentialBlob(blob, mode) } diff --git a/app/secrets/plugins/wincred/plugin.go b/app/secrets/plugins/wincred/plugin.go index 249015b..97e842a 100644 --- a/app/secrets/plugins/wincred/plugin.go +++ b/app/secrets/plugins/wincred/plugin.go @@ -2,18 +2,47 @@ package plugins import ( "context" + "fmt" + "strings" "github.com/winhowes/AuthTranslator/app/secrets" ) // winCredPlugin loads generic credentials from Windows Credential Manager. -// id should be the credential TargetName. +// id format: +// - "target" (raw bytes) +// - "target#utf8" +// - "target#utf16le" type winCredPlugin struct{} func (winCredPlugin) Prefix() string { return "wincred" } func (winCredPlugin) Load(ctx context.Context, id string) (string, error) { - return loadWindowsCredential(id) + target, mode, err := parseWinCredID(id) + if err != nil { + return "", err + } + return loadWindowsCredential(target, mode) +} + +func parseWinCredID(id string) (target, mode string, err error) { + parts := strings.SplitN(strings.TrimSpace(id), "#", 2) + target = strings.TrimSpace(parts[0]) + if target == "" { + return "", "", fmt.Errorf("wincred target is required") + } + + mode = "raw" + if len(parts) == 2 { + mode = strings.ToLower(strings.TrimSpace(parts[1])) + } + + switch mode { + case "raw", "utf8", "utf16le": + return target, mode, nil + default: + return "", "", fmt.Errorf("unsupported wincred decode mode %q", mode) + } } func init() { secrets.Register(winCredPlugin{}) } diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index d39d598..764debb 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -15,95 +15,108 @@ func TestWinCredPluginLoad(t *testing.T) { } } -func TestDecodeCredentialBlobUTF16ASCII(t *testing.T) { - blob := encodeUTF16LE("secret") - if got := decodeCredentialBlob(blob); got != "secret" { - t.Fatalf("decodeCredentialBlob() = %q, want %q", got, "secret") +func TestWinCredPluginLoadInvalidID(t *testing.T) { + p := winCredPlugin{} + if _, err := p.Load(context.Background(), "#utf8"); err == nil { + t.Fatal("expected parse error") } } -func TestDecodeCredentialBlobUTF16WithBOM(t *testing.T) { - blob := append([]byte{0xFF, 0xFE}, encodeUTF16LE("secret")...) - if got := decodeCredentialBlob(blob); got != "secret" { - t.Fatalf("decodeCredentialBlob() = %q, want %q", got, "secret") +func TestParseWinCredID(t *testing.T) { + tests := []struct { + name string + input string + wantTgt string + wantMode string + wantError bool + }{ + {name: "default raw", input: "target", wantTgt: "target", wantMode: "raw"}, + {name: "utf8 mode", input: "target#utf8", wantTgt: "target", wantMode: "utf8"}, + {name: "utf16 mode", input: "target#utf16le", wantTgt: "target", wantMode: "utf16le"}, + {name: "invalid mode", input: "target#auto", wantError: true}, + {name: "missing target", input: " #utf8", wantError: true}, } -} -func TestDecodeCredentialBlobUTF16Unicode(t *testing.T) { - want := "päss-東京-🔐" - blob := encodeUTF16LE(want) - if got := decodeCredentialBlob(blob); got != want { - t.Fatalf("decodeCredentialBlob() = %q, want %q", got, want) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + target, mode, err := parseWinCredID(tc.input) + if tc.wantError { + if err == nil { + t.Fatal("expected error") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target != tc.wantTgt || mode != tc.wantMode { + t.Fatalf("parseWinCredID(%q) = (%q,%q), want (%q,%q)", tc.input, target, mode, tc.wantTgt, tc.wantMode) + } + }) } } -func TestDecodeCredentialBlobShortEvenWithSingleZeroFallsBack(t *testing.T) { +func TestDecodeCredentialBlobRaw(t *testing.T) { blob := []byte{0x00, 0xAB, 0xCD, 0xEF} - if got := decodeCredentialBlob(blob); got != string(blob) { - t.Fatalf("decodeCredentialBlob() = %q, want raw bytes", got) + got, err := decodeCredentialBlob(blob, "raw") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != string(blob) { + t.Fatalf("decodeCredentialBlob(raw) = %q, want raw bytes", got) } } -func TestDecodeCredentialBlobInvalidUTF16FallsBackToBytes(t *testing.T) { - blob := []byte{0x00, 0xD8, 0x41, 0x00} // lone high surrogate + 'A' - if got := decodeCredentialBlob(blob); got != string(blob) { - t.Fatalf("decodeCredentialBlob() = %q, want byte fallback", got) +func TestDecodeCredentialBlobUTF8(t *testing.T) { + blob := []byte("päss-東京") + got, err := decodeCredentialBlob(blob, "utf8") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "päss-東京" { + t.Fatalf("decodeCredentialBlob(utf8) = %q", got) } } -func TestDecodeCredentialBlobOddLengthFallsBackToBytes(t *testing.T) { - blob := []byte{0x61, 0x62, 0x63} - if got := decodeCredentialBlob(blob); got != "abc" { - t.Fatalf("decodeCredentialBlob() = %q, want %q", got, "abc") +func TestDecodeCredentialBlobUTF8Invalid(t *testing.T) { + blob := []byte{0xff, 0xfe} + if _, err := decodeCredentialBlob(blob, "utf8"); err == nil { + t.Fatal("expected utf8 decode error") } } -func TestDecodeCredentialBlobEvenLengthNonUTF16Binary(t *testing.T) { - blob := []byte{0x80, 0x81, 0x82, 0x83} - if got := decodeCredentialBlob(blob); got != string(blob) { - t.Fatalf("decodeCredentialBlob() = %q, want raw bytes", got) +func TestDecodeCredentialBlobUTF16ASCII(t *testing.T) { + blob := encodeUTF16LE("secret") + got, err := decodeCredentialBlob(blob, "utf16le") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "secret" { + t.Fatalf("decodeCredentialBlob(utf16le) = %q, want %q", got, "secret") } } -func TestLooksLikeUTF16LE(t *testing.T) { - tests := []struct { - name string - blob []byte - want bool - }{ - {name: "odd length", blob: []byte{0x41}, want: false}, - {name: "bom", blob: []byte{0xFF, 0xFE, 0x41, 0x00}, want: true}, - {name: "ascii utf16 style", blob: []byte{0x41, 0x00, 0x42, 0x00}, want: true}, - {name: "short even one zero", blob: []byte{0x00, 0xAB, 0xCD, 0xEF}, want: false}, - {name: "null terminated plausible utf16", blob: []byte{0x71, 0x67, 0xAC, 0x4E, 0x00, 0x00}, want: true}, - {name: "binary without nul pattern", blob: []byte{0x80, 0x81, 0x82, 0x83}, want: false}, +func TestDecodeCredentialBlobUTF16WithBOM(t *testing.T) { + blob := append([]byte{0xFF, 0xFE}, encodeUTF16LE("secret")...) + got, err := decodeCredentialBlob(blob, "utf16le") + if err != nil { + t.Fatalf("unexpected error: %v", err) } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - if got := looksLikeUTF16LE(tc.blob); got != tc.want { - t.Fatalf("looksLikeUTF16LE(%v) = %v, want %v", tc.blob, got, tc.want) - } - }) + if got != "secret" { + t.Fatalf("decodeCredentialBlob(utf16le) = %q, want %q", got, "secret") } } -func TestDecodeCredentialBlobUTF8WithTrailingNull(t *testing.T) { - blob := append([]byte("päss-東京"), 0x00, 0x00) - if got := decodeCredentialBlob(blob); got != "päss-東京" { - t.Fatalf("decodeCredentialBlob() = %q, want %q", got, "päss-東京") +func TestDecodeCredentialBlobUTF16Invalid(t *testing.T) { + blob := []byte{0x00, 0xD8, 0x41, 0x00} + if _, err := decodeCredentialBlob(blob, "utf16le"); err == nil { + t.Fatal("expected utf16 decode error") } } -func TestIsProbablyText(t *testing.T) { - if !isProbablyText("hello東京\n\r\t") { - t.Fatal("expected printable text to be accepted") - } - if !isProbablyText("") { - t.Fatal("expected empty string to be accepted") - } - if isProbablyText("bad\x01text") { - t.Fatal("expected control-character text to be rejected") +func TestDecodeCredentialBlobUnsupportedMode(t *testing.T) { + if _, err := decodeCredentialBlob([]byte("x"), "bogus"); err == nil { + t.Fatal("expected unsupported mode error") } } diff --git a/docs/secret-backends.md b/docs/secret-backends.md index ed86ffc..2803057 100644 --- a/docs/secret-backends.md +++ b/docs/secret-backends.md @@ -29,7 +29,7 @@ outgoing_auth: | `vault` | `vault:secret/data/slack` | Self‑hosted **HashiCorp Vault** cluster. | | `keychain` | `keychain:github-cli#octocat` | macOS hosts with secrets in Keychain (`service#account`). | | `secretservice` | `secretservice:service=slack,user=bot` | Linux desktops/servers with D-Bus Secret Service (`secret-tool`). | -| `wincred` | `wincred:github-cli` | Windows hosts using Credential Manager generic credentials. | +| `wincred` | `wincred:github-cli#utf16le` | Windows hosts using Credential Manager generic credentials. Use `#raw` (default), `#utf8`, or `#utf16le`. | | `dangerousLiteral` | `dangerousLiteral:__PLACEHOLDER__` | Rare cases where you need a literal sentinel string. | ### Literal placeholders (`dangerousLiteral:`) @@ -68,7 +68,7 @@ Some schemes rely on environment variables for authentication or decryption: | `vault` | `VAULT_ADDR`, `VAULT_TOKEN` | Fetches secrets from HashiCorp Vault via its HTTP API. | `vault:secret/data/api` reads from Vault | | `keychain` | _none_ | Uses the macOS `security` CLI and current keychain access permissions. | `keychain:service#account` | | `secretservice` | _none_ | Uses Linux `secret-tool` to query attributes like `service=...`. | `secretservice:service=slack,user=bot` | -| `wincred` | _none_ | Reads generic credentials by target name from Windows Credential Manager. | `wincred:github-cli` | +| `wincred` | _none_ | Reads generic credentials by target name from Windows Credential Manager. | `wincred:github-cli#raw` | | `dangerousLiteral` | _none_ | Value is stored directly in config; no external dependencies. | `dangerousLiteral:__PLACEHOLDER__` | For `file:` URIs that use the `:KEY` suffix, AuthTranslator treats the file as a simple `KEY=value` list: From 13c2db701d6a6cd4544aaff4a5cfaf96de98e093 Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 18:33:46 -0700 Subject: [PATCH 10/12] Add non-terminated UTF-16LE wincred decode regression test --- app/secrets/plugins/wincred/plugin_test.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index 764debb..095dba8 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -107,6 +107,17 @@ func TestDecodeCredentialBlobUTF16WithBOM(t *testing.T) { } } +func TestDecodeCredentialBlobUTF16UnicodeNoTerminator(t *testing.T) { + blob := encodeUTF16LENoTerminator("東京🔐") + got, err := decodeCredentialBlob(blob, "utf16le") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "東京🔐" { + t.Fatalf("decodeCredentialBlob(utf16le) = %q, want %q", got, "東京🔐") + } +} + func TestDecodeCredentialBlobUTF16Invalid(t *testing.T) { blob := []byte{0x00, 0xD8, 0x41, 0x00} if _, err := decodeCredentialBlob(blob, "utf16le"); err == nil { @@ -164,6 +175,15 @@ func TestDecodeUTF16LEBlobOddLength(t *testing.T) { } } +func encodeUTF16LENoTerminator(s string) []byte { + u16 := utf16.Encode([]rune(s)) + blob := make([]byte, len(u16)*2) + for i, v := range u16 { + binary.LittleEndian.PutUint16(blob[i*2:], v) + } + return blob +} + func encodeUTF16LE(s string) []byte { u16 := utf16.Encode([]rune(s)) blob := make([]byte, (len(u16)+1)*2) From 6b9b77cd2d29b16d038f6a4846bd797bdc6ea67f Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 19:23:59 -0700 Subject: [PATCH 11/12] Validate keychain account suffix and stub wincred loader in tests --- app/secrets/plugins/keychain/plugin.go | 16 ++++++++----- app/secrets/plugins/keychain/plugin_test.go | 21 ++++++++++++++--- app/secrets/plugins/wincred/plugin.go | 4 +++- app/secrets/plugins/wincred/plugin_test.go | 25 ++++++++++++++++++--- 4 files changed, 54 insertions(+), 12 deletions(-) diff --git a/app/secrets/plugins/keychain/plugin.go b/app/secrets/plugins/keychain/plugin.go index c5c175b..29ef216 100644 --- a/app/secrets/plugins/keychain/plugin.go +++ b/app/secrets/plugins/keychain/plugin.go @@ -25,9 +25,9 @@ var execSecurityCommand = func(ctx context.Context, args ...string) ([]byte, err func (keychainPlugin) Prefix() string { return "keychain" } func (keychainPlugin) Load(ctx context.Context, id string) (string, error) { - service, account := parseKeychainID(id) - if service == "" { - return "", fmt.Errorf("keychain service is required") + service, account, err := parseKeychainID(id) + if err != nil { + return "", err } args := []string{"find-generic-password", "-w", "-s", service} @@ -50,13 +50,19 @@ func (keychainPlugin) Load(ctx context.Context, id string) (string, error) { return string(out), nil } -func parseKeychainID(id string) (service, account string) { +func parseKeychainID(id string) (service, account string, err error) { parts := strings.SplitN(id, "#", 2) service = strings.TrimSpace(parts[0]) + if service == "" { + return "", "", fmt.Errorf("keychain service is required") + } if len(parts) == 2 { account = strings.TrimSpace(parts[1]) + if account == "" { + return "", "", fmt.Errorf("keychain account is required when using service#account format") + } } - return service, account + return service, account, nil } func init() { secrets.Register(keychainPlugin{}) } diff --git a/app/secrets/plugins/keychain/plugin_test.go b/app/secrets/plugins/keychain/plugin_test.go index 31843b0..249ad37 100644 --- a/app/secrets/plugins/keychain/plugin_test.go +++ b/app/secrets/plugins/keychain/plugin_test.go @@ -131,18 +131,33 @@ func TestExecSecurityCommandDefault(t *testing.T) { } func TestParseKeychainID(t *testing.T) { - service, account := parseKeychainID("svc#acc") + service, account, err := parseKeychainID("svc#acc") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if service != "svc" || account != "acc" { t.Fatalf("unexpected parse result: %q %q", service, account) } - service, account = parseKeychainID("svc-only") + service, account, err = parseKeychainID("svc-only") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if service != "svc-only" || account != "" { t.Fatalf("unexpected parse result: %q %q", service, account) } - service, account = parseKeychainID(" svc # acc ") + service, account, err = parseKeychainID(" svc # acc ") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if service != "svc" || account != "acc" { t.Fatalf("unexpected trimmed parse result: %q %q", service, account) } } + +func TestParseKeychainIDMissingAccount(t *testing.T) { + if _, _, err := parseKeychainID("svc#"); err == nil { + t.Fatal("expected error for empty account") + } +} diff --git a/app/secrets/plugins/wincred/plugin.go b/app/secrets/plugins/wincred/plugin.go index 97e842a..33b62de 100644 --- a/app/secrets/plugins/wincred/plugin.go +++ b/app/secrets/plugins/wincred/plugin.go @@ -15,6 +15,8 @@ import ( // - "target#utf16le" type winCredPlugin struct{} +var winCredLoader = loadWindowsCredential + func (winCredPlugin) Prefix() string { return "wincred" } func (winCredPlugin) Load(ctx context.Context, id string) (string, error) { @@ -22,7 +24,7 @@ func (winCredPlugin) Load(ctx context.Context, id string) (string, error) { if err != nil { return "", err } - return loadWindowsCredential(target, mode) + return winCredLoader(target, mode) } func parseWinCredID(id string) (target, mode string, err error) { diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index 095dba8..74be0fa 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -8,10 +8,29 @@ import ( ) func TestWinCredPluginLoad(t *testing.T) { + old := winCredLoader + t.Cleanup(func() { winCredLoader = old }) + + winCredLoader = func(targetName, mode string) (string, error) { + if targetName != "my-target" || mode != "raw" { + t.Fatalf("unexpected loader args: %q %q", targetName, mode) + } + return "loaded-secret", nil + } + p := winCredPlugin{} - _, err := p.Load(context.Background(), "my-target") - if err == nil { - t.Fatal("expected wincred loader error on non-windows test environment") + got, err := p.Load(context.Background(), "my-target") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "loaded-secret" { + t.Fatalf("expected loaded secret, got %q", got) + } +} + +func TestLoadWindowsCredentialUnsupported(t *testing.T) { + if _, err := loadWindowsCredential("target", "raw"); err == nil { + t.Fatal("expected unsupported-platform error") } } From 0318d5aaa01bb76b7c35c7106f77faa0c95c0f9a Mon Sep 17 00:00:00 2001 From: Winston Howes Date: Mon, 20 Apr 2026 19:54:39 -0700 Subject: [PATCH 12/12] Skip unsupported-path wincred test on Windows --- app/secrets/plugins/wincred/plugin_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/secrets/plugins/wincred/plugin_test.go b/app/secrets/plugins/wincred/plugin_test.go index 74be0fa..fa6d80b 100644 --- a/app/secrets/plugins/wincred/plugin_test.go +++ b/app/secrets/plugins/wincred/plugin_test.go @@ -3,6 +3,7 @@ package plugins import ( "context" "encoding/binary" + "runtime" "testing" "unicode/utf16" ) @@ -29,6 +30,9 @@ func TestWinCredPluginLoad(t *testing.T) { } func TestLoadWindowsCredentialUnsupported(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("non-windows unsupported-path test") + } if _, err := loadWindowsCredential("target", "raw"); err == nil { t.Fatal("expected unsupported-platform error") }