fix(credentials): address PR review comments

- Guard migration with viper.InConfig() to avoid persisting env vars
- Handle os.UserHomeDir() error with TempDir fallback
- Create fallback credential directory before constructing file store
- Use viper.InConfig() in config list to detect YAML-stored keys
- Surface GetCredential errors in config list instead of silent failure
- Use filepath.Join in tests for cross-platform path safety

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>

Author: appleboy

cmd/cmd.go

@@ -41,6 +41,12 @@ var sensitiveConfigKeys = []string{"openai.api_key", "gemini.api_key"}
 // config into the secure credential store and clears them from the config file.
 func migrateCredentialsToStore() {
 	for _, key := range sensitiveConfigKeys {
+		// Only migrate values that actually exist in the config file.
+		// This prevents env vars (e.g. OPENAI_API_KEY) from being silently
+		// persisted into the credential store.
+		if !viper.InConfig(key) {
+			continue
+		}
 		val := viper.GetString(key)
 		if val == "" {
 			continue

cmd/config_list.go

@@ -77,15 +77,19 @@ var configListCmd = &cobra.Command{
 		// Add the key and value to the table
 		for _, v := range keys {
 			if slices.Contains(sensitiveConfigKeys, v) {
-				cred, _ := util.GetCredential(v)
+				cred, err := util.GetCredential(v)
+				if err != nil {
+					tbl.AddRow(v, "(error reading secure store)")
+					continue
+				}
 				switch {
 				case cred != "":
 					if util.CredStoreIsKeyring() {
 						tbl.AddRow(v, "(stored in keyring)")
 					} else {
 						tbl.AddRow(v, "(stored in secure file)")
 					}
-				case viper.GetString(v) != "":
+				case viper.InConfig(v):
 					tbl.AddRow(v, "**************** (YAML — run config set to migrate)")
 				default:
 					tbl.AddRow(v, "(not set)")

util/credstore.go

@@ -14,8 +14,18 @@ const credServiceName = "codegpt"
 var credStore *credstore.SecureStore[string]
 
 func init() {
-	home, _ := os.UserHomeDir()
-	fallbackPath := filepath.Join(home, ".config", "codegpt", ".cache", "credentials.json")
+	home, err := os.UserHomeDir()
+	var fallbackPath string
+	if err != nil || home == "" {
+		fallbackPath = filepath.Join(os.TempDir(), "codegpt", "credentials.json")
+	} else {
+		fallbackPath = filepath.Join(home, ".config", "codegpt", ".cache", "credentials.json")
+	}
+
+	// Ensure the directory for the fallback credential file exists.
+	dir := filepath.Dir(fallbackPath)
+	_ = os.MkdirAll(dir, 0o700)
+
 	keyring := credstore.NewStringKeyringStore(credServiceName)
 	file := credstore.NewStringFileStore(fallbackPath)
 	credStore = credstore.NewSecureStore(keyring, file)

util/credstore_test.go

@@ -1,6 +1,7 @@
 package util
 
 import (
+	"path/filepath"
 	"testing"
 
 	"github.com/go-authgate/sdk-go/credstore"
@@ -10,7 +11,7 @@ import (
 // This avoids touching the OS keyring in CI/CD environments.
 func newTestCredStore(t *testing.T) *credstore.SecureStore[string] {
 	t.Helper()
-	path := t.TempDir() + "/creds.json"
+	path := filepath.Join(t.TempDir(), "creds.json")
 	file := credstore.NewStringFileStore(path)
 	// Pass file as both primary and fallback so NewSecureStore always picks file.
 	return credstore.NewSecureStore[string](file, file)
@@ -63,7 +64,7 @@ func TestGetCredential_Missing(t *testing.T) {
 	original := credStore
 	defer func() { credStore = original }()
 
-	path := t.TempDir() + "/creds.json"
+	path := filepath.Join(t.TempDir(), "creds.json")
 	file := credstore.NewStringFileStore(path)
 	credStore = credstore.NewSecureStore[string](file, file)
 
@@ -81,7 +82,7 @@ func TestSetAndGetCredential(t *testing.T) {
 	original := credStore
 	defer func() { credStore = original }()
 
-	path := t.TempDir() + "/creds.json"
+	path := filepath.Join(t.TempDir(), "creds.json")
 	file := credstore.NewStringFileStore(path)
 	credStore = credstore.NewSecureStore[string](file, file)
 
@@ -103,7 +104,7 @@ func TestDeleteCredential(t *testing.T) {
 	original := credStore
 	defer func() { credStore = original }()
 
-	path := t.TempDir() + "/creds.json"
+	path := filepath.Join(t.TempDir(), "creds.json")
 	file := credstore.NewStringFileStore(path)
 	credStore = credstore.NewSecureStore[string](file, file)