Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/auth/token_store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Comment thread
JackZhao10086 marked this conversation as resolved.
var token StoredUAToken
Expand Down
8 changes: 8 additions & 0 deletions internal/core/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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{
Expand Down
11 changes: 3 additions & 8 deletions internal/keychain/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Comment thread
JackZhao10086 marked this conversation as resolved.

func (d *defaultKeychain) Set(service, account, value string) error {
Expand Down
34 changes: 30 additions & 4 deletions internal/keychain/keychain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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.
Expand All @@ -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))
}
60 changes: 46 additions & 14 deletions internal/keychain/keychain_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"crypto/cipher"
"crypto/rand"
"encoding/base64"
"errors"
"os"
"path/filepath"
"regexp"
Expand All @@ -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()

Expand All @@ -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
Comment thread
greptile-apps[bot] marked this conversation as resolved.
}

// 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}
Comment thread
JackZhao10086 marked this conversation as resolved.
}()

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 {
Expand All @@ -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
Expand All @@ -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
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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
}
Expand Down Expand Up @@ -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) {
Expand Down
46 changes: 35 additions & 11 deletions internal/keychain/keychain_other.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"crypto/aes"
"crypto/cipher"
"crypto/rand"
"errors"
"fmt"
"os"
"path/filepath"
Expand All @@ -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 == "" {
Expand All @@ -36,18 +36,33 @@ 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")

key, err := os.ReadFile(keyPath)
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
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if err := os.MkdirAll(dir, 0700); err != nil {
return nil, err
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down
Loading
Loading