feat(context): make keychain migration opt-in via context fix

Stop migrating plaintext credentials on config load to preserve existing
workflows. New logins still store secrets in the keychain when available,
and users can run tcloud context fix --migrate-credentials to migrate.

Author: thojkooi

README.md

@@ -46,6 +46,30 @@ users:
         token: <PAT>
 ```
 
+## Config file permissions
+
+The CLI stores settings in `~/.tcloud`. On load, the CLI warns if that file is world-readable. Fix permissions with:
+
+```bash
+tcloud context fix
+```
+
+This restricts `~/.tcloud` to owner-only access (`0600`).
+
+### Migrating credentials to the keychain
+
+New logins store secrets in the OS credential store (macOS Keychain, Windows Credential Manager, or Linux Secret Service) when available. Existing configs with plaintext tokens in `~/.tcloud` are left unchanged until manually running the `fix` command.
+
+To move existing plaintext credentials into the keychain:
+
+```bash
+tcloud context fix --migrate-credentials
+```
+
+Use `tcloud context view` to inspect your config with secrets redacted.
+
+Control credential storage with `THALASSA_CREDENTIAL_STORE` (`auto`, `keychain`, or `file`).
+
 ## Development
 
 ### Prerequisites

cmd/context/fix.go

@@ -9,13 +9,23 @@ import (
 	"github.com/thalassa-cloud/cli/internal/config/securefile"
 )
 
+var migrateCredentials bool
+
 var fixCmd = &cobra.Command{
-	Use:     "fix",
-	Short:   "Fix config file security issues",
-	Long:    "Fix security issues in the CLI config file, such as overly permissive file permissions.",
-	Example: `  tcloud context fix`,
-	Args:    cobra.NoArgs,
+	Use:   "fix",
+	Short: "Fix config file security issues",
+	Long:  "Fix security issues in the CLI config file, such as overly permissive file permissions or migrating credentials to the keychain.",
+	Example: `  tcloud context fix
+  tcloud context fix --migrate-credentials`,
+	Args: cobra.NoArgs,
 	RunE: func(cmd *cobra.Command, args []string) error {
+		if migrateCredentials {
+			if err := contextstate.MigrateCredentialsToKeychain(); err != nil {
+				return err
+			}
+			fmt.Println("Migrated credentials from the config file to the keychain.")
+		}
+
 		filename := contextstate.ConfigFilename()
 		mode, permissive, err := securefile.CheckPermissions(filename)
 		if err != nil {
@@ -36,4 +46,5 @@ var fixCmd = &cobra.Command{
 
 func init() {
 	ContextCmd.AddCommand(fixCmd)
+	fixCmd.Flags().BoolVar(&migrateCredentials, "migrate-credentials", false, "move plaintext credentials from the config file into the keychain")
 }

internal/config/contextstate/config.go

@@ -92,6 +92,17 @@ func (c *configFileContextManager) Load() error {
 	return nil
 }
 
+func (c *configFileContextManager) MigrateCredentialsToKeychain() error {
+	migrated, err := c.migratePlaintextCredentialsToKeychain()
+	if err != nil {
+		return err
+	}
+	if !migrated {
+		return nil
+	}
+	return c.Save()
+}
+
 // AddOrMergeContext adds or merges the given context.
 func (c *configFileContextManager) AddOrMergeContext(context Context) error {
 	c.setUser(context.Users)
@@ -173,24 +184,13 @@ func (c *configFileContextManager) RemoveContextServer(name string) error {
 	return c.Save()
 }
 
-func (c *configFileContextManager) syncCredentialsAfterLoad() (bool, error) {
+func (c *configFileContextManager) syncCredentialsAfterLoad() (bool, error) { //nolint:unparam
 	preference := credentials.PreferredStore()
-	migrated := false
 
 	for i := range c.config.Users {
 		user := &c.config.Users[i]
-		if user.User.HasSecrets() && credentials.UseKeychain(preference, user.CredentialStore) {
-			store := credentials.ResolveStore(credentials.StoreKeychain)
-			if err := store.Set(user.Name, user.User.Secrets()); err != nil {
-				if preference == credentials.StoreKeychain {
-					return false, fmt.Errorf("store credentials in keychain: %w", err)
-				}
-				user.CredentialStore = credentials.StoreFile
-				continue
-			}
-			user.CredentialStore = credentials.StoreKeychain
-			migrated = true
-			continue
+		if user.CredentialStore == "" && user.User.HasSecrets() {
+			user.CredentialStore = credentials.StoreFile
 		}
 
 		if user.CredentialStore != credentials.StoreKeychain {
@@ -211,6 +211,30 @@ func (c *configFileContextManager) syncCredentialsAfterLoad() (bool, error) {
 		user.User.ApplySecrets(secrets)
 	}
 
+	return false, nil
+}
+
+func (c *configFileContextManager) migratePlaintextCredentialsToKeychain() (bool, error) {
+	store := credentials.ResolveStore(credentials.StoreKeychain)
+	if !store.Available() {
+		return false, fmt.Errorf("keychain credential store is not available")
+	}
+
+	migrated := false
+	for i := range c.config.Users {
+		user := &c.config.Users[i]
+		if user.CredentialStore == credentials.StoreKeychain {
+			continue
+		}
+		if !user.User.HasSecrets() {
+			continue
+		}
+		if err := store.Set(user.Name, user.User.Secrets()); err != nil {
+			return false, fmt.Errorf("store credentials for user %q in keychain: %w", user.Name, err)
+		}
+		user.CredentialStore = credentials.StoreKeychain
+		migrated = true
+	}
 	return migrated, nil
 }
 

internal/config/contextstate/config_test.go

@@ -14,10 +14,9 @@ import (
 	"github.com/thalassa-cloud/cli/internal/credentials"
 )
 
-func TestConfigManagerMigratesPlaintextCredentialsToKeychain(t *testing.T) {
+func TestConfigManagerMigrateCredentialsToKeychain(t *testing.T) {
 	keyring.MockInit()
 	t.Cleanup(credentials.ResetKeyring)
-	t.Setenv(credentials.ThalassaCredentialStoreEnvVar, credentials.StoreKeychain)
 
 	dir := t.TempDir()
 	path := filepath.Join(dir, ".tcloud")
@@ -42,11 +41,17 @@ users:
 	manager := contextstate.NewConfigFileContextManager(path)
 	require.NoError(t, manager.Load())
 
+	onDisk, err := os.ReadFile(path)
+	require.NoError(t, err)
+	assert.Contains(t, string(onDisk), "tc_pat_secret")
+
+	require.NoError(t, manager.MigrateCredentialsToKeychain())
+
 	ctx, err := manager.Get()
 	require.NoError(t, err)
 	assert.Equal(t, "tc_pat_secret", ctx.Users.User.Token)
 
-	onDisk, err := os.ReadFile(path)
+	onDisk, err = os.ReadFile(path)
 	require.NoError(t, err)
 	assert.NotContains(t, string(onDisk), "tc_pat_secret")
 
@@ -74,6 +79,37 @@ users:
 	assert.Equal(t, "tc_pat_secret", got.Token)
 }
 
+func TestConfigManagerLoadDoesNotMigratePlaintextCredentials(t *testing.T) {
+	keyring.MockInit()
+	t.Cleanup(credentials.ResetKeyring)
+
+	dir := t.TempDir()
+	path := filepath.Join(dir, ".tcloud")
+	require.NoError(t, os.WriteFile(path, []byte(`configVersion: v1
+current-context: default
+contexts:
+  - name: default
+    context:
+      api: api.thalassa.cloud
+      user: default
+servers:
+  - name: api.thalassa.cloud
+    api:
+      server: https://api.thalassa.cloud
+users:
+  - name: default
+    user:
+      token: tc_pat_secret
+`), 0o644))
+
+	manager := contextstate.NewConfigFileContextManager(path)
+	require.NoError(t, manager.Load())
+
+	onDisk, err := os.ReadFile(path)
+	require.NoError(t, err)
+	assert.Contains(t, string(onDisk), "tc_pat_secret")
+}
+
 func TestConfigManagerSanitizedConfigRedactsSecrets(t *testing.T) {
 	manager := contextstate.NewConfigFileContextManager(filepath.Join(t.TempDir(), ".tcloud"))
 	require.NoError(t, manager.AddOrMergeContext(contextstate.Context{

internal/config/contextstate/credentials.go

@@ -0,0 +1,19 @@
+package contextstate
+
+import "github.com/thalassa-cloud/cli/internal/credentials"
+
+// ApplyPreferredCredentialStoreForNewLogin marks where freshly authenticated credentials should be stored.
+func ApplyPreferredCredentialStoreForNewLogin(user *Users) {
+	switch credentials.PreferredStore() {
+	case credentials.StoreKeychain:
+		user.CredentialStore = credentials.StoreKeychain
+	case credentials.StoreFile:
+		user.CredentialStore = credentials.StoreFile
+	default:
+		if credentials.ResolveStore(credentials.StoreKeychain).Available() {
+			user.CredentialStore = credentials.StoreKeychain
+		} else {
+			user.CredentialStore = credentials.StoreFile
+		}
+	}
+}

internal/config/contextstate/credentials_test.go

@@ -0,0 +1,29 @@
+package contextstate
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/zalando/go-keyring"
+
+	"github.com/thalassa-cloud/cli/internal/credentials"
+)
+
+func TestApplyPreferredCredentialStoreForNewLoginUsesKeychainWhenAvailable(t *testing.T) {
+	keyring.MockInit()
+	t.Cleanup(credentials.ResetKeyring)
+
+	user := Users{Name: "default"}
+	ApplyPreferredCredentialStoreForNewLogin(&user)
+	assert.Equal(t, credentials.StoreKeychain, user.CredentialStore)
+}
+
+func TestApplyPreferredCredentialStoreForNewLoginRespectsFilePreference(t *testing.T) {
+	keyring.MockInit()
+	t.Cleanup(credentials.ResetKeyring)
+	t.Setenv(credentials.ThalassaCredentialStoreEnvVar, credentials.StoreFile)
+
+	user := Users{Name: "default"}
+	ApplyPreferredCredentialStoreForNewLogin(&user)
+	assert.Equal(t, credentials.StoreFile, user.CredentialStore)
+}

internal/config/contextstate/login.go

@@ -13,6 +13,7 @@ func Login(ctx context.Context, token string) error {
 		return err
 	}
 	context.Users.User.Token = token
+	ApplyPreferredCredentialStoreForNewLogin(&context.Users)
 	if err := CombineConfigContext(context); err != nil {
 		return err
 	}
@@ -36,6 +37,7 @@ func LoginWithAPIEndpointOidc(ctx context.Context, clientID, clientSecret, apiEn
 	context.Users.User.AccessToken = ""
 
 	context.Servers.API.Server = u.String()
+	ApplyPreferredCredentialStoreForNewLogin(&context.Users)
 	if err := CombineConfigContext(context); err != nil {
 		return err
 	}
@@ -58,6 +60,7 @@ func LoginWithAccessToken(ctx context.Context, accessToken, apiEndpoint string)
 	context.Users.User.ClientID = ""
 	context.Users.User.ClientSecret = ""
 	context.Servers.API.Server = u.String()
+	ApplyPreferredCredentialStoreForNewLogin(&context.Users)
 	if err := CombineConfigContext(context); err != nil {
 		return err
 	}
@@ -81,6 +84,7 @@ func LoginWithAPIEndpoint(ctx context.Context, token, apiEndpoint string) error
 	context.Users.User.AccessToken = ""
 
 	context.Servers.API.Server = u.String()
+	ApplyPreferredCredentialStoreForNewLogin(&context.Users)
 	if err := CombineConfigContext(context); err != nil {
 		return err
 	}

internal/config/contextstate/manager.go

@@ -93,6 +93,9 @@ type ConfigManager interface {
 
 	// FixPermissions restricts the config file to owner-only access.
 	FixPermissions() error
+
+	// MigrateCredentialsToKeychain moves plaintext credentials from the config file into the keychain.
+	MigrateCredentialsToKeychain() error
 }
 
 func Init() {
@@ -138,6 +141,10 @@ func FixConfigPermissions() error {
 	return globalConfigManager.FixPermissions()
 }
 
+func MigrateCredentialsToKeychain() error {
+	return globalConfigManager.MigrateCredentialsToKeychain()
+}
+
 func GetContextConfiguration() (Context, error) {
 	return globalConfigManager.Get()
 }

internal/config/contextstate/oidc_browser.go

@@ -69,6 +69,7 @@ func loginWithBrowserTokens(tokens browseroidc.TokenPair, apiEndpoint string) er
 
 	currentContext.Users.User.SetBrowserTokens(tokens.AccessToken, tokens.RefreshToken, tokens.Expiry)
 	currentContext.Servers.API.Server = u.String()
+	ApplyPreferredCredentialStoreForNewLogin(&currentContext.Users)
 	if err := CombineConfigContext(currentContext); err != nil {
 		return err
 	}

internal/credentials/credentials.go

@@ -84,7 +84,7 @@ func UseKeychain(preference, userStore string) bool {
 		if userStore == StoreFile {
 			return false
 		}
-		return keyringStore{}.Available()
+		return false
 	}
 }