diff --git a/internal/auth/token_store.go b/internal/auth/token_store.go index 80883a649..7e52f6704 100644 --- a/internal/auth/token_store.go +++ b/internal/auth/token_store.go @@ -39,8 +39,8 @@ func MaskToken(token string) string { // GetStoredToken reads the stored UAT for a given (appId, userOpenId) pair. func GetStoredToken(appId, userOpenId string) *StoredUAToken { - jsonStr := keychain.Get(keychain.LarkCliService, accountKey(appId, userOpenId)) - if jsonStr == "" { + jsonStr, err := keychain.Get(keychain.LarkCliService, accountKey(appId, userOpenId)) + if err != nil || jsonStr == "" { return nil } var token StoredUAToken diff --git a/internal/core/config.go b/internal/core/config.go index ced1e27b4..18a2aa4e0 100644 --- a/internal/core/config.go +++ b/internal/core/config.go @@ -5,11 +5,13 @@ package core import ( "encoding/json" + "errors" "fmt" "os" "path/filepath" "github.com/larksuite/cli/internal/keychain" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" ) @@ -113,6 +115,12 @@ func RequireConfig(kc keychain.KeychainAccess) (*CliConfig, error) { app := raw.Apps[0] secret, err := ResolveSecretInput(app.AppSecret, kc) if err != nil { + // If the error comes from the keychain, it will already be wrapped as an ExitError. + // For other errors (e.g. file read errors, unknown sources), wrap them as ConfigError. + var exitErr *output.ExitError + if errors.As(err, &exitErr) { + return nil, exitErr + } return nil, &ConfigError{Code: 2, Type: "config", Message: err.Error()} } cfg := &CliConfig{ diff --git a/internal/keychain/default.go b/internal/keychain/default.go index 5d9e3d104..59d99ebf5 100644 --- a/internal/keychain/default.go +++ b/internal/keychain/default.go @@ -3,17 +3,12 @@ package keychain -import "fmt" - -// defaultKeychain implements KeychainAccess using the real platform keychain. +// defaultKeychain is the default implementation of KeychainAccess +// that uses the package-level functions. type defaultKeychain struct{} func (d *defaultKeychain) Get(service, account string) (string, error) { - val := Get(service, account) - if val == "" { - return "", fmt.Errorf("keychain entry not found: %s/%s", service, account) - } - return val, nil + return Get(service, account) } func (d *defaultKeychain) Set(service, account, value string) error { diff --git a/internal/keychain/keychain.go b/internal/keychain/keychain.go index c225db8b3..a5dc74b57 100644 --- a/internal/keychain/keychain.go +++ b/internal/keychain/keychain.go @@ -5,6 +5,15 @@ // macOS uses the system Keychain; Linux uses AES-256-GCM encrypted files; Windows uses DPAPI + registry. package keychain +import ( + "errors" + "fmt" + + "github.com/larksuite/cli/internal/output" +) + +var errNotInitialized = errors.New("keychain not initialized") + const ( // LarkCliService is the unified keychain service name for all secrets // (both AppSecret and UAT). Entries are distinguished by account key format: @@ -13,6 +22,22 @@ const ( LarkCliService = "lark-cli" ) +// wrapError is a helper to wrap underlying errors into output.ExitError. +// It formats the error message and provides a hint for troubleshooting keychain access issues. +func wrapError(op string, err error) error { + if err == nil { + return nil + } + msg := fmt.Sprintf("keychain %s failed: %v", op, err) + hint := "Check if the OS keychain/credential manager is locked or accessible. If running inside a sandbox or CI environment, please ensure the process has the necessary permissions to access the keychain." + + if errors.Is(err, errNotInitialized) { + hint = "The keychain master key may have been cleaned up or deleted. Please reconfigure the CLI by running `lark-cli config init`." + } + + return output.ErrWithHint(output.ExitAPI, "config", msg, hint) +} + // KeychainAccess abstracts keychain Get/Set/Remove for dependency injection. // Used by AppSecret operations (ForStorage, ResolveSecretInput, RemoveSecretStore). // UAT operations in token_store.go use the package-level Get/Set/Remove directly. @@ -24,16 +49,17 @@ type KeychainAccess interface { // Get retrieves a value from the keychain. // Returns empty string if the entry does not exist. -func Get(service, account string) string { - return platformGet(service, account) +func Get(service, account string) (string, error) { + val, err := platformGet(service, account) + return val, wrapError("Get", err) } // Set stores a value in the keychain, overwriting any existing entry. func Set(service, account, data string) error { - return platformSet(service, account, data) + return wrapError("Set", platformSet(service, account, data)) } // Remove deletes an entry from the keychain. No error if not found. func Remove(service, account string) error { - return platformRemove(service, account) + return wrapError("Remove", platformRemove(service, account)) } diff --git a/internal/keychain/keychain_darwin.go b/internal/keychain/keychain_darwin.go index fe71583d6..a49633f7f 100644 --- a/internal/keychain/keychain_darwin.go +++ b/internal/keychain/keychain_darwin.go @@ -11,6 +11,7 @@ import ( "crypto/cipher" "crypto/rand" "encoding/base64" + "errors" "os" "path/filepath" "regexp" @@ -36,11 +37,14 @@ func StorageDir(service string) string { var safeFileNameRe = regexp.MustCompile(`[^a-zA-Z0-9._-]`) +// safeFileName sanitizes an account name to be used as a safe file name. func safeFileName(account string) string { return safeFileNameRe.ReplaceAllString(account, "_") + ".enc" } -func getMasterKey(service string) ([]byte, error) { +// getMasterKey retrieves the master key from the system keychain. +// If allowCreate is true, it generates and stores a new master key if one doesn't exist. +func getMasterKey(service string, allowCreate bool) ([]byte, error) { ctx, cancel := context.WithTimeout(context.Background(), keychainTimeout) defer cancel() @@ -59,28 +63,48 @@ func getMasterKey(service string) ([]byte, error) { resCh <- result{key: key, err: nil} return } + // Key is found but invalid or corrupted + resCh <- result{key: nil, err: errors.New("keychain is corrupted")} + return + } else if !errors.Is(err, keyring.ErrNotFound) { + // Not ErrNotFound, which means access was denied or blocked by the system + resCh <- result{key: nil, err: errors.New("keychain access blocked")} + return + } + + // If ErrNotFound, check if we are allowed to create a new key + if !allowCreate { + // Creation not allowed (e.g., during Get operation), return error + resCh <- result{key: nil, err: errNotInitialized} + return } - // Generate new master key if not found or invalid + // It's the first time and creation is allowed (Set operation), generate a new key key := make([]byte, masterKeyBytes) if _, randErr := rand.Read(key); randErr != nil { resCh <- result{key: nil, err: randErr} return } - encodedKey = base64.StdEncoding.EncodeToString(key) - setErr := keyring.Set(service, "master.key", encodedKey) - resCh <- result{key: key, err: setErr} + encodedKeyStr := base64.StdEncoding.EncodeToString(key) + setErr := keyring.Set(service, "master.key", encodedKeyStr) + if setErr != nil { + resCh <- result{key: nil, err: setErr} + return + } + resCh <- result{key: key, err: nil} }() select { case res := <-resCh: return res.key, res.err case <-ctx.Done(): - return nil, ctx.Err() + // Timeout is usually caused by ignored/blocked permission prompts + return nil, errors.New("keychain access blocked") } } +// encryptData encrypts data using AES-GCM. func encryptData(plaintext string, key []byte) ([]byte, error) { block, err := aes.NewCipher(key) if err != nil { @@ -103,6 +127,7 @@ func encryptData(plaintext string, key []byte) ([]byte, error) { return result, nil } +// decryptData decrypts data using AES-GCM. func decryptData(data []byte, key []byte) (string, error) { if len(data) < ivBytes+tagBytes { return "", os.ErrInvalid @@ -125,24 +150,30 @@ func decryptData(data []byte, key []byte) (string, error) { return string(plaintext), nil } -func platformGet(service, account string) string { - key, err := getMasterKey(service) +// platformGet retrieves a value from the macOS keychain. +func platformGet(service, account string) (string, error) { + path := filepath.Join(StorageDir(service), safeFileName(account)) + data, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return "", nil + } if err != nil { - return "" + return "", err } - data, err := os.ReadFile(filepath.Join(StorageDir(service), safeFileName(account))) + key, err := getMasterKey(service, false) if err != nil { - return "" + return "", err } plaintext, err := decryptData(data, key) if err != nil { - return "" + return "", err } - return plaintext + return plaintext, nil } +// platformSet stores a value in the macOS keychain. func platformSet(service, account, data string) error { - key, err := getMasterKey(service) + key, err := getMasterKey(service, true) if err != nil { return err } @@ -170,6 +201,7 @@ func platformSet(service, account, data string) error { return nil } +// platformRemove deletes a value from the macOS keychain. func platformRemove(service, account string) error { err := os.Remove(filepath.Join(StorageDir(service), safeFileName(account))) if err != nil && !os.IsNotExist(err) { diff --git a/internal/keychain/keychain_other.go b/internal/keychain/keychain_other.go index 631a9fb0b..55192d46b 100644 --- a/internal/keychain/keychain_other.go +++ b/internal/keychain/keychain_other.go @@ -9,6 +9,7 @@ import ( "crypto/aes" "crypto/cipher" "crypto/rand" + "errors" "fmt" "os" "path/filepath" @@ -21,8 +22,7 @@ const masterKeyBytes = 32 const ivBytes = 12 const tagBytes = 16 -// StorageDir returns the storage directory for a given service name. -// Each service gets its own directory for physical isolation. +// StorageDir returns the directory where encrypted files are stored. func StorageDir(service string) string { home, err := os.UserHomeDir() if err != nil || home == "" { @@ -36,11 +36,14 @@ func StorageDir(service string) string { var safeFileNameRe = regexp.MustCompile(`[^a-zA-Z0-9._-]`) +// safeFileName sanitizes an account name to be used as a safe file name. func safeFileName(account string) string { return safeFileNameRe.ReplaceAllString(account, "_") + ".enc" } -func getMasterKey(service string) ([]byte, error) { +// getMasterKey retrieves the master key from the file system. +// If allowCreate is true, it generates and stores a new master key if one doesn't exist. +func getMasterKey(service string, allowCreate bool) ([]byte, error) { dir := StorageDir(service) keyPath := filepath.Join(dir, "master.key") @@ -48,6 +51,18 @@ func getMasterKey(service string) ([]byte, error) { if err == nil && len(key) == masterKeyBytes { return key, nil } + if err == nil && len(key) != masterKeyBytes { + // Key file exists but is corrupted + return nil, errors.New("keychain is corrupted") + } + if err != nil && !errors.Is(err, os.ErrNotExist) { + // Real I/O error (permission denied, etc.) - propagate it + return nil, err + } + + if !allowCreate { + return nil, errNotInitialized + } if err := os.MkdirAll(dir, 0700); err != nil { return nil, err @@ -78,6 +93,7 @@ func getMasterKey(service string) ([]byte, error) { return key, nil } +// encryptData encrypts data using AES-GCM. func encryptData(plaintext string, key []byte) ([]byte, error) { block, err := aes.NewCipher(key) if err != nil { @@ -100,6 +116,7 @@ func encryptData(plaintext string, key []byte) ([]byte, error) { return result, nil } +// decryptData decrypts data using AES-GCM. func decryptData(data []byte, key []byte) (string, error) { if len(data) < ivBytes+tagBytes { return "", os.ErrInvalid @@ -122,24 +139,30 @@ func decryptData(data []byte, key []byte) (string, error) { return string(plaintext), nil } -func platformGet(service, account string) string { - key, err := getMasterKey(service) +// platformGet retrieves a value from the file system. +func platformGet(service, account string) (string, error) { + path := filepath.Join(StorageDir(service), safeFileName(account)) + data, err := os.ReadFile(path) + if errors.Is(err, os.ErrNotExist) { + return "", nil + } if err != nil { - return "" + return "", err } - data, err := os.ReadFile(filepath.Join(StorageDir(service), safeFileName(account))) + key, err := getMasterKey(service, false) if err != nil { - return "" + return "", err } plaintext, err := decryptData(data, key) if err != nil { - return "" + return "", err } - return plaintext + return plaintext, nil } +// platformSet stores a value in the file system. func platformSet(service, account, data string) error { - key, err := getMasterKey(service) + key, err := getMasterKey(service, true) if err != nil { return err } @@ -167,6 +190,7 @@ func platformSet(service, account, data string) error { return nil } +// platformRemove deletes a value from the file system. func platformRemove(service, account string) error { err := os.Remove(filepath.Join(StorageDir(service), safeFileName(account))) if err != nil && !os.IsNotExist(err) { diff --git a/internal/keychain/keychain_windows.go b/internal/keychain/keychain_windows.go index 8830e8acf..f0e3f9f84 100644 --- a/internal/keychain/keychain_windows.go +++ b/internal/keychain/keychain_windows.go @@ -22,12 +22,14 @@ import ( const regRootPath = `Software\LarkCli\keychain` +// registryPathForService returns the registry path for a given service. func registryPathForService(service string) string { return regRootPath + `\` + safeRegistryComponent(service) } var safeRegRe = regexp.MustCompile(`[^a-zA-Z0-9._-]`) +// safeRegistryComponent sanitizes a string to be used as a registry key component. func safeRegistryComponent(s string) string { // Registry key path uses '\\' separators; avoid accidental nesting and odd chars. s = strings.ReplaceAll(s, "\\", "_") @@ -39,6 +41,7 @@ func valueNameForAccount(account string) string { return base64.RawURLEncoding.EncodeToString([]byte(account)) } +// dpapiEntropy generates entropy for DPAPI encryption based on the service and account names. func dpapiEntropy(service, account string) *windows.DataBlob { // Bind ciphertext to (service, account) to reduce swap/replay risks. // Note: empty entropy is allowed, but we intentionally use deterministic entropy. @@ -49,6 +52,7 @@ func dpapiEntropy(service, account string) *windows.DataBlob { return &windows.DataBlob{Size: uint32(len(data)), Data: &data[0]} } +// dpapiProtect encrypts data using Windows DPAPI. func dpapiProtect(plaintext []byte, entropy *windows.DataBlob) ([]byte, error) { var in windows.DataBlob if len(plaintext) > 0 { @@ -70,6 +74,7 @@ func dpapiProtect(plaintext []byte, entropy *windows.DataBlob) ([]byte, error) { return res, nil } +// dpapiUnprotect decrypts data using Windows DPAPI. func dpapiUnprotect(ciphertext []byte, entropy *windows.DataBlob) ([]byte, error) { var in windows.DataBlob if len(ciphertext) > 0 { @@ -91,6 +96,7 @@ func dpapiUnprotect(ciphertext []byte, entropy *windows.DataBlob) ([]byte, error return res, nil } +// freeDataBlob frees the memory allocated for a DataBlob. func freeDataBlob(b *windows.DataBlob) { if b == nil || b.Data == nil { return @@ -101,11 +107,16 @@ func freeDataBlob(b *windows.DataBlob) { b.Size = 0 } -func platformGet(service, account string) string { - v, _ := registryGet(service, account) - return v +// platformGet retrieves a value from the Windows registry. +func platformGet(service, account string) (string, error) { + v, ok := registryGet(service, account) + if !ok { + return "", nil + } + return v, nil } +// platformSet stores a value in the Windows registry. func platformSet(service, account, data string) error { entropy := dpapiEntropy(service, account) protected, err := dpapiProtect([]byte(data), entropy) @@ -115,10 +126,12 @@ func platformSet(service, account, data string) error { return registrySet(service, account, protected) } +// platformRemove deletes a value from the Windows registry. func platformRemove(service, account string) error { return registryRemove(service, account) } +// registryGet retrieves a string value from the registry under the given service and account. func registryGet(service, account string) (string, bool) { keyPath := registryPathForService(service) k, err := registry.OpenKey(registry.CURRENT_USER, keyPath, registry.QUERY_VALUE) @@ -143,6 +156,7 @@ func registryGet(service, account string) (string, bool) { return string(plain), true } +// registrySet stores a string value in the registry under the given service and account. func registrySet(service, account string, protected []byte) error { keyPath := registryPathForService(service) k, _, err := registry.CreateKey(registry.CURRENT_USER, keyPath, registry.SET_VALUE) @@ -158,6 +172,7 @@ func registrySet(service, account string, protected []byte) error { return nil } +// registryRemove deletes a value from the registry under the given service and account. func registryRemove(service, account string) error { keyPath := registryPathForService(service) k, err := registry.OpenKey(registry.CURRENT_USER, keyPath, registry.SET_VALUE)