feat: secure passwords (#193) (#194)

Drew Smirnoff created

* feat: OS keyring to store passwords

* fix: add migration tool to the config

Change summary

config/config.go      | 90 ++++++++++++++++++++++++++++++++++++++++-----
config/config_test.go |  8 ++++
go.mod                |  4 ++
go.sum                |  8 ++++
4 files changed, 100 insertions(+), 10 deletions(-)

Detailed changes

config/config.go 🔗

@@ -6,14 +6,17 @@ import (
 	"path/filepath"
 
 	"github.com/google/uuid"
+	"github.com/zalando/go-keyring"
 )
 
+const keyringServiceName = "matcha-email-client"
+
 // Account stores the configuration for a single email account.
 type Account struct {
 	ID              string `json:"id"`
 	Name            string `json:"name"`
 	Email           string `json:"email"`
-	Password        string `json:"password"`
+	Password        string `json:"-"`                // "-" prevents the password from being saved to config.json
 	ServiceProvider string `json:"service_provider"` // "gmail", "icloud", or "custom"
 	// FetchEmail is the single email address for which messages should be fetched.
 	// If empty, it will default to `Email` when accounts are added.
@@ -108,8 +111,17 @@ func configFile() (string, error) {
 	return filepath.Join(dir, "config.json"), nil
 }
 
-// SaveConfig saves the given configuration to the config file.
+// SaveConfig saves the given configuration to the config file and passwords to the keyring.
 func SaveConfig(config *Config) error {
+	// Save passwords to the OS keyring before writing the JSON file
+	for _, acc := range config.Accounts {
+		if acc.Password != "" {
+			// We ignore the error here because some environments (like headless CI)
+			// might not have a keyring service, but we still want to save the rest of the config.
+			_ = keyring.Set(keyringServiceName, acc.Email, acc.Password)
+		}
+	}
+
 	path, err := configFile()
 	if err != nil {
 		return err
@@ -124,7 +136,8 @@ func SaveConfig(config *Config) error {
 	return os.WriteFile(path, data, 0600)
 }
 
-// LoadConfig loads the configuration from the config file.
+// LoadConfig loads the configuration from the config file and passwords from the keyring.
+// It automatically migrates plain-text passwords to the OS keyring if they exist.
 func LoadConfig() (*Config, error) {
 	path, err := configFile()
 	if err != nil {
@@ -134,12 +147,31 @@ func LoadConfig() (*Config, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	var config Config
-	if err := json.Unmarshal(data, &config); err != nil {
-		// Try to load legacy single-account config
+	var needsMigration bool
+
+	type rawAccount struct {
+		ID              string `json:"id"`
+		Name            string `json:"name"`
+		Email           string `json:"email"`
+		Password        string `json:"password,omitempty"`
+		ServiceProvider string `json:"service_provider"`
+		FetchEmail      string `json:"fetch_email,omitempty"`
+		IMAPServer      string `json:"imap_server,omitempty"`
+		IMAPPort        int    `json:"imap_port,omitempty"`
+		SMTPServer      string `json:"smtp_server,omitempty"`
+		SMTPPort        int    `json:"smtp_port,omitempty"`
+	}
+	type diskConfig struct {
+		Accounts      []rawAccount `json:"accounts"`
+		DisableImages bool         `json:"disable_images,omitempty"`
+	}
+
+	var raw diskConfig
+	if err := json.Unmarshal(data, &raw); err != nil {
 		var legacyConfig legacyConfigFormat
 		if legacyErr := json.Unmarshal(data, &legacyConfig); legacyErr == nil && legacyConfig.Email != "" {
-			// Convert legacy config to new format
 			config = Config{
 				Accounts: []Account{
 					{
@@ -148,12 +180,11 @@ func LoadConfig() (*Config, error) {
 						Email:           legacyConfig.Email,
 						Password:        legacyConfig.Password,
 						ServiceProvider: legacyConfig.ServiceProvider,
-						// Default FetchEmail to the legacy Email value
-						FetchEmail: legacyConfig.Email,
+						FetchEmail:      legacyConfig.Email,
 					},
 				},
 			}
-			// Save the migrated config
+			// SaveConfig automatically pushes the password to the keyring and strips it from JSON
 			if saveErr := SaveConfig(&config); saveErr != nil {
 				return nil, saveErr
 			}
@@ -161,6 +192,42 @@ func LoadConfig() (*Config, error) {
 		}
 		return nil, err
 	}
+
+	config.DisableImages = raw.DisableImages
+	for _, rawAcc := range raw.Accounts {
+		acc := Account{
+			ID:              rawAcc.ID,
+			Name:            rawAcc.Name,
+			Email:           rawAcc.Email,
+			ServiceProvider: rawAcc.ServiceProvider,
+			FetchEmail:      rawAcc.FetchEmail,
+			IMAPServer:      rawAcc.IMAPServer,
+			IMAPPort:        rawAcc.IMAPPort,
+			SMTPServer:      rawAcc.SMTPServer,
+			SMTPPort:        rawAcc.SMTPPort,
+		}
+
+		if rawAcc.Password != "" {
+			// Found a plain-text password! Move it to the OS Keyring.
+			_ = keyring.Set(keyringServiceName, rawAcc.Email, rawAcc.Password)
+			acc.Password = rawAcc.Password
+			needsMigration = true
+		} else {
+			// No plaintext password in JSON, fetch from Keyring as normal.
+			if pwd, err := keyring.Get(keyringServiceName, acc.Email); err == nil {
+				acc.Password = pwd
+			}
+		}
+
+		config.Accounts = append(config.Accounts, acc)
+	}
+
+	if needsMigration {
+		if saveErr := SaveConfig(&config); saveErr != nil {
+			return nil, saveErr
+		}
+	}
+
 	return &config, nil
 }
 
@@ -184,10 +251,13 @@ func (c *Config) AddAccount(account Account) {
 	c.Accounts = append(c.Accounts, account)
 }
 
-// RemoveAccount removes an account by its ID.
+// RemoveAccount removes an account by its ID and deletes its password from the keyring.
 func (c *Config) RemoveAccount(id string) bool {
 	for i, acc := range c.Accounts {
 		if acc.ID == id {
+			// Delete password from OS Keyring when account is removed
+			_ = keyring.Delete(keyringServiceName, acc.Email)
+
 			c.Accounts = append(c.Accounts[:i], c.Accounts[i+1:]...)
 			return true
 		}

config/config_test.go 🔗

@@ -3,10 +3,15 @@ package config
 import (
 	"reflect"
 	"testing"
+
+	"github.com/zalando/go-keyring"
 )
 
 // TestSaveAndLoadConfig verifies that the config can be saved to and loaded from a file correctly.
 func TestSaveAndLoadConfig(t *testing.T) {
+	// Use an in-memory mock keyring so tests do not interact with the host OS keyring
+	keyring.MockInit()
+
 	// Create a temporary directory for the test to avoid interfering with actual user config.
 	tempDir := t.TempDir()
 
@@ -107,6 +112,9 @@ func TestAccountGetSMTPServer(t *testing.T) {
 
 // TestConfigAddRemoveAccount tests adding and removing accounts from config.
 func TestConfigAddRemoveAccount(t *testing.T) {
+	// Use an in-memory mock keyring to test the deletion step cleanly
+	keyring.MockInit()
+
 	cfg := &Config{}
 
 	// Add an account

go.mod 🔗

@@ -16,6 +16,7 @@ require (
 )
 
 require (
+	al.essio.dev/pkg/shellescape v1.5.1 // indirect
 	github.com/andybalholm/cascadia v1.3.3 // indirect
 	github.com/atotto/clipboard v0.1.4 // indirect
 	github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
@@ -26,8 +27,10 @@ require (
 	github.com/clipperhouse/displaywidth v0.9.0 // indirect
 	github.com/clipperhouse/stringish v0.1.1 // indirect
 	github.com/clipperhouse/uax29/v2 v2.5.0 // indirect
+	github.com/danieljoos/wincred v1.2.2 // indirect
 	github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 // indirect
 	github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect
+	github.com/godbus/dbus/v5 v5.1.0 // indirect
 	github.com/lucasb-eyer/go-colorful v1.3.0 // indirect
 	github.com/mattn/go-isatty v0.0.20 // indirect
 	github.com/mattn/go-localereader v0.0.1 // indirect
@@ -38,5 +41,6 @@ require (
 	github.com/rivo/uniseg v0.4.7 // indirect
 	github.com/sahilm/fuzzy v0.1.1 // indirect
 	github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect
+	github.com/zalando/go-keyring v0.2.6 // indirect
 	golang.org/x/net v0.47.0 // indirect
 )

go.sum 🔗

@@ -1,3 +1,5 @@
+al.essio.dev/pkg/shellescape v1.5.1 h1:86HrALUujYS/h+GtqoB26SBEdkWfmMI6FubjXlsXyho=
+al.essio.dev/pkg/shellescape v1.5.1/go.mod h1:6sIqp7X2P6mThCQ7twERpZTuigpr6KbZWtls1U8I890=
 github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ=
 github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE=
 github.com/PuerkitoBio/goquery v1.11.0 h1:jZ7pwMQXIITcUXNH83LLk+txlaEy6NVOfTuP43xxfqw=
@@ -32,6 +34,8 @@ github.com/clipperhouse/stringish v0.1.1 h1:+NSqMOr3GR6k1FdRhhnXrLfztGzuG+VuFDfa
 github.com/clipperhouse/stringish v0.1.1/go.mod h1:v/WhFtE1q0ovMta2+m+UbpZ+2/HEXNWYXQgCt4hdOzA=
 github.com/clipperhouse/uax29/v2 v2.5.0 h1:x7T0T4eTHDONxFJsL94uKNKPHrclyFI0lm7+w94cO8U=
 github.com/clipperhouse/uax29/v2 v2.5.0/go.mod h1:Wn1g7MK6OoeDT0vL+Q0SQLDz/KpfsVRgg6W7ihQeh4g=
+github.com/danieljoos/wincred v1.2.2 h1:774zMFJrqaeYCK2W57BgAem/MLi6mtSE47MB6BOJ0i0=
+github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7nd/ogr0Uh8=
 github.com/emersion/go-imap v1.2.1 h1:+s9ZjMEjOB8NzZMVTM3cCenz2JrQIGGo5j1df19WjTA=
 github.com/emersion/go-imap v1.2.1/go.mod h1:Qlx1FSx2FTxjnjWpIlVNEuX+ylerZQNFE5NsmKFSejY=
 github.com/emersion/go-message v0.15.0/go.mod h1:wQUEfE+38+7EW8p8aZ96ptg6bAb1iwdgej19uXASlE4=
@@ -43,6 +47,8 @@ github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6/go.mod h1:iL2twTe
 github.com/emersion/go-textwrapper v0.0.0-20200911093747-65d896831594/go.mod h1:aqO8z8wPrjkscevZJFVE1wXJrLpC5LtJG7fqLOsPb2U=
 github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6baUTXGLOoWe4PQhGxaX0KpnayAqC48p4=
 github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM=
+github.com/godbus/dbus/v5 v5.1.0 h1:4KLkAxT3aOY8Li4FRJe/KvhoNFFxo0m6fNuFUO8QJUk=
+github.com/godbus/dbus/v5 v5.1.0/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
 github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
@@ -71,6 +77,8 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJu
 github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
 github.com/yuin/goldmark v1.7.16 h1:n+CJdUxaFMiDUNnWC3dMWCIQJSkxH4uz3ZwQBkAlVNE=
 github.com/yuin/goldmark v1.7.16/go.mod h1:ip/1k0VRfGynBgxOz0yCqHrbZXhcjxyuS66Brc7iBKg=
+github.com/zalando/go-keyring v0.2.6 h1:r7Yc3+H+Ux0+M72zacZoItR3UDxeWfKTcabvkI8ua9s=
+github.com/zalando/go-keyring v0.2.6/go.mod h1:2TCrxYrbUNYfNS/Kgy/LSrkSQzZ5UPVH85RwfczwvcI=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
 golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliYc=