From 75919594afc57ef9e766f6b46413f8122b34af2d Mon Sep 17 00:00:00 2001 From: Lorris Saint-Genez Date: Thu, 4 Jun 2026 10:48:14 -0700 Subject: [PATCH] feat(config): add state.toml + keychain store and migrate config.toml Co-authored-by: Cursor --- .golangci.yml | 8 ++ pkg/cmd/root/root.go | 8 ++ pkg/config/config.go | 8 +- pkg/config/migrate.go | 196 ++++++++++++++++++++++++++ pkg/config/migrate_test.go | 242 ++++++++++++++++++++++++++++++++ pkg/config/profile.go | 61 +++++++- pkg/config/profile_test.go | 164 ++++++++++++++++++++++ pkg/config/state/secret.go | 53 +++++++ pkg/config/state/secret_test.go | 69 +++++++++ pkg/config/state/state.go | 212 ++++++++++++++++++++++++++++ pkg/config/state/state_test.go | 160 +++++++++++++++++++++ 11 files changed, 1179 insertions(+), 2 deletions(-) create mode 100644 pkg/config/migrate.go create mode 100644 pkg/config/migrate_test.go create mode 100644 pkg/config/state/secret.go create mode 100644 pkg/config/state/secret_test.go create mode 100644 pkg/config/state/state.go create mode 100644 pkg/config/state/state_test.go diff --git a/.golangci.yml b/.golangci.yml index 79ad127c..3c597d74 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,3 +14,11 @@ issues: linters: - stylecheck text: ST1005 + # secret.go defines the keychain service name and the secret-kind + # identifiers (e.g. "api_key", "crawler_api_key") used as keychain account + # suffixes. These are field names, not embedded credentials, so gosec's + # G101 hardcoded-credential heuristic is a false positive here. + - path: pkg/config/state/secret.go + linters: + - gosec + text: G101 diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 399535e1..7c3c6e05 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -84,6 +84,10 @@ func NewRootCmd(f *cmdutil.Factory) *cobra.Command { cmd.PersistentFlags(). StringVarP(&f.Config.Profile().Name, "profile", "p", "", "The profile to use") _ = cmd.RegisterFlagCompletionFunc("profile", cmdutil.ConfiguredProfilesCompletionFunc(f)) + _ = cmd.PersistentFlags().MarkDeprecated( + "profile", + "use `algolia auth login` and `algolia application select` instead; `--profile` still resolves stored aliases until next major version", + ) cmd.PersistentFlags(). StringVarP(&f.Config.Profile().ApplicationID, "application-id", "", "", "The application ID") @@ -131,6 +135,10 @@ func Execute() exitCode { cmdFactory := factory.New(version.Version, &cfg) stderr := cmdFactory.IOStreams.ErrOut + // One-time migration of the legacy config.toml into state.toml + the OS + // keychain. No-op once state.toml exists; never fatal. + cfg.MigrateIfNeeded(stderr) + // Set up the update notifier. updateMessageChan := make(chan *update.ReleaseInfo) go func() { diff --git a/pkg/config/config.go b/pkg/config/config.go index d25be6d5..28dbe4ea 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -7,10 +7,12 @@ import ( "path/filepath" "github.com/BurntSushi/toml" - "github.com/algolia/cli/pkg/utils" "github.com/mitchellh/go-homedir" log "github.com/sirupsen/logrus" "github.com/spf13/viper" + + "github.com/algolia/cli/pkg/config/state" + "github.com/algolia/cli/pkg/utils" ) type IConfig interface { @@ -43,6 +45,10 @@ type Config struct { // InitConfig reads in profiles file and ENV variables if set. func (c *Config) InitConfig() { + // state.toml is the source of truth for credential resolution; config.toml + // is only read as a legacy fallback (removed in CLI v2.0). + c.CurrentProfile.statePath = state.DefaultPath() + if c.File != "" { viper.SetConfigFile(c.File) } else { diff --git a/pkg/config/migrate.go b/pkg/config/migrate.go new file mode 100644 index 00000000..5bccafc5 --- /dev/null +++ b/pkg/config/migrate.go @@ -0,0 +1,196 @@ +package config + +import ( + "fmt" + "io" + "os" + "sort" + + "github.com/spf13/viper" + + "github.com/algolia/cli/pkg/config/state" +) + +// legacyProfile is a flattened view of a single profile read from config.toml, +// including the crawler fields that are not part of the Profile struct. +type legacyProfile struct { + Name string + ApplicationID string + APIKey string + AdminAPIKey string + SearchHosts []string + Default bool + CrawlerUserID string + CrawlerAPIKey string +} + +// MigrateIfNeeded performs the one-time migration of config.toml into +// state.toml (non-secrets) + the OS keychain (secrets). It runs at most once: +// the presence of state.toml marks the migration as done. Migration is +// best-effort and never fatal — on any failure config.toml is left untouched so +// it remains usable as a read-only fallback (until removal in CLI v2.0). +// +// Skip rules: +// - profiles with an empty api_key are skipped (logged with a next step); +// - profiles sharing an application_id keep the one marked default = true, +// the others are logged as conflicts and skipped; +// - admin_api_key is never migrated; a one-line notice points the user at +// ALGOLIA_ADMIN_API_KEY or the --api-key flag. +func (c *Config) MigrateIfNeeded(stderr io.Writer) { + statePath := c.CurrentProfile.statePath + if statePath == "" { + statePath = state.DefaultPath() + } + + // Already migrated: state.toml exists. + if _, err := os.Stat(statePath); err == nil { + return + } + + configFile := c.File + if configFile == "" { + return + } + if _, err := os.Stat(configFile); err != nil { + return // nothing legacy to migrate (fresh install) + } + + legacy, err := readLegacyProfiles(configFile) + if err != nil { + fmt.Fprintf(stderr, "Could not read %s for migration: %s\n", configFile, err) + return + } + if len(legacy) == 0 { + return + } + + st := migrateProfiles(legacy, stderr) + + if err := st.Save(statePath); err != nil { + fmt.Fprintf(stderr, "Could not write %s during migration: %s\n", statePath, err) + return + } +} + +// migrateProfiles builds the new State from legacy profiles, storing secrets in +// the keychain and logging skipped profiles and the admin-key notice. +func migrateProfiles(legacy []legacyProfile, stderr io.Writer) *state.State { + // Default profiles first, then alphabetical, for deterministic conflict + // resolution (the default among a shared application_id wins). + sort.SliceStable(legacy, func(i, j int) bool { + if legacy[i].Default != legacy[j].Default { + return legacy[i].Default + } + return legacy[i].Name < legacy[j].Name + }) + + st := state.New() + claimedBy := map[string]string{} // application_id -> alias that claimed it + adminNoticeShown := false + + for _, p := range legacy { + if p.AdminAPIKey != "" && !adminNoticeShown { + fmt.Fprintf( + stderr, + "Note: admin API keys are not migrated. Set ALGOLIA_ADMIN_API_KEY or pass --api-key when a command needs one.\n", + ) + adminNoticeShown = true + } + + if p.APIKey == "" { + fmt.Fprintf( + stderr, + "Skipped profile %q during migration: no API key stored. Run `algolia application select` to configure it.\n", + p.Name, + ) + continue + } + if p.ApplicationID == "" { + fmt.Fprintf( + stderr, + "Skipped profile %q during migration: no application ID.\n", + p.Name, + ) + continue + } + if owner, taken := claimedBy[p.ApplicationID]; taken { + fmt.Fprintf( + stderr, + "Skipped profile %q during migration: application %s is already configured by profile %q.\n", + p.Name, + p.ApplicationID, + owner, + ) + continue + } + + st.SetApp(&state.ApplicationState{ + ApplicationID: p.ApplicationID, + Alias: p.Name, + SearchHosts: p.SearchHosts, + CrawlerUserID: p.CrawlerUserID, + }) + claimedBy[p.ApplicationID] = p.Name + + if err := state.SetSecret(p.ApplicationID, state.SecretAPIKey, p.APIKey); err != nil { + fmt.Fprintf(stderr, "Could not store API key for profile %q: %s\n", p.Name, err) + } + if p.CrawlerAPIKey != "" { + if err := state.SetSecret(p.ApplicationID, state.SecretCrawlerAPIKey, p.CrawlerAPIKey); err != nil { + fmt.Fprintf( + stderr, + "Could not store crawler API key for profile %q: %s\n", + p.Name, + err, + ) + } + } + + if p.Default && st.CurrentApplicationID == "" { + st.SetCurrent(p.ApplicationID) + } + } + + // No explicit default but a single application migrated: make it current + // so commands work without --profile. + if st.CurrentApplicationID == "" && len(claimedBy) == 1 { + for appID := range claimedBy { + st.SetCurrent(appID) + } + } + + return st +} + +// readLegacyProfiles reads config.toml from a dedicated viper instance (so it +// does not depend on or mutate global viper state) and returns the profiles it +// contains, including their crawler credentials. +func readLegacyProfiles(configFile string) ([]legacyProfile, error) { + v := viper.New() + v.SetConfigType("toml") + v.SetConfigFile(configFile) + if err := v.ReadInConfig(); err != nil { + return nil, err + } + + settings := v.AllSettings() + profiles := make([]legacyProfile, 0, len(settings)) + for name := range settings { + var p Profile + if err := v.UnmarshalKey(name, &p); err != nil { + return nil, err + } + profiles = append(profiles, legacyProfile{ + Name: name, + ApplicationID: p.ApplicationID, + APIKey: p.APIKey, + AdminAPIKey: p.AdminAPIKey, + SearchHosts: p.SearchHosts, + Default: p.Default, + CrawlerUserID: v.GetString(name + ".crawler_user_id"), + CrawlerAPIKey: v.GetString(name + ".crawler_api_key"), + }) + } + + return profiles, nil +} diff --git a/pkg/config/migrate_test.go b/pkg/config/migrate_test.go new file mode 100644 index 00000000..2635898d --- /dev/null +++ b/pkg/config/migrate_test.go @@ -0,0 +1,242 @@ +package config + +import ( + "bytes" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" + + "github.com/algolia/cli/pkg/config/state" +) + +// newMigrateConfig writes config.toml with the given content and returns a +// Config wired to a temp config.toml + state.toml plus the state path. +func newMigrateConfig(t *testing.T, configTOML string) (*Config, string) { + t.Helper() + dir := t.TempDir() + configPath := filepath.Join(dir, "config.toml") + statePath := filepath.Join(dir, "state.toml") + + if configTOML != "" { + require.NoError(t, os.WriteFile(configPath, []byte(configTOML), 0o600)) + } + + c := &Config{File: configPath} + c.CurrentProfile.statePath = statePath + return c, statePath +} + +func TestMigrateIfNeeded_HappyPath(t *testing.T) { + keyring.MockInit() + + c, statePath := newMigrateConfig(t, ` +[prod] +application_id = "APP_PROD" +api_key = "key-prod" +default = true + +[staging] +application_id = "APP_STAGING" +api_key = "key-staging" +search_hosts = ["host1", "host2"] +`) + + var stderr bytes.Buffer + c.MigrateIfNeeded(&stderr) + + st, err := state.Load(statePath) + require.NoError(t, err) + + assert.Equal(t, "APP_PROD", st.CurrentApplicationID, "default profile becomes current") + + prod := st.App("APP_PROD") + require.NotNil(t, prod) + assert.Equal(t, "prod", prod.Alias) + + staging := st.App("APP_STAGING") + require.NotNil(t, staging) + assert.Equal(t, "staging", staging.Alias) + assert.Equal(t, []string{"host1", "host2"}, staging.SearchHosts) + + // Secrets land in the keychain, not in state.toml. + prodKey, err := state.GetSecret("APP_PROD", state.SecretAPIKey) + require.NoError(t, err) + assert.Equal(t, "key-prod", prodKey) + stagingKey, err := state.GetSecret("APP_STAGING", state.SecretAPIKey) + require.NoError(t, err) + assert.Equal(t, "key-staging", stagingKey) + + raw, err := os.ReadFile(statePath) + require.NoError(t, err) + assert.NotContains(t, string(raw), "key-prod", "API keys must never be written to state.toml") +} + +func TestMigrateIfNeeded_SkipsEmptyAPIKey(t *testing.T) { + keyring.MockInit() + + c, statePath := newMigrateConfig(t, ` +[good] +application_id = "APP_GOOD" +api_key = "key-good" +default = true + +[empty] +application_id = "APP_EMPTY" +`) + + var stderr bytes.Buffer + c.MigrateIfNeeded(&stderr) + + st, err := state.Load(statePath) + require.NoError(t, err) + assert.NotNil(t, st.App("APP_GOOD")) + assert.Nil(t, st.App("APP_EMPTY"), "profile without API key is skipped") + assert.Contains(t, stderr.String(), `Skipped profile "empty"`) + assert.Contains(t, stderr.String(), "algolia application select") +} + +func TestMigrateIfNeeded_ConflictKeepsDefault(t *testing.T) { + keyring.MockInit() + + c, statePath := newMigrateConfig(t, ` +[secondary] +application_id = "SHARED_APP" +api_key = "key-secondary" + +[primary] +application_id = "SHARED_APP" +api_key = "key-primary" +default = true +`) + + var stderr bytes.Buffer + c.MigrateIfNeeded(&stderr) + + st, err := state.Load(statePath) + require.NoError(t, err) + + app := st.App("SHARED_APP") + require.NotNil(t, app) + assert.Equal(t, "primary", app.Alias, "the default profile wins the shared application_id") + assert.Equal(t, "SHARED_APP", st.CurrentApplicationID) + + key, err := state.GetSecret("SHARED_APP", state.SecretAPIKey) + require.NoError(t, err) + assert.Equal(t, "key-primary", key) + + assert.Contains(t, stderr.String(), `Skipped profile "secondary"`) + assert.Contains(t, stderr.String(), "already configured by profile") +} + +func TestMigrateIfNeeded_AdminKeyNoticeAndNotMigrated(t *testing.T) { + keyring.MockInit() + + c, statePath := newMigrateConfig(t, ` +[legacy] +application_id = "APP_LEGACY" +admin_api_key = "admin-secret" +`) + + var stderr bytes.Buffer + c.MigrateIfNeeded(&stderr) + + assert.Contains(t, stderr.String(), "ALGOLIA_ADMIN_API_KEY") + + // admin_api_key alone (no api_key) means the profile is skipped entirely. + st, err := state.Load(statePath) + require.NoError(t, err) + assert.Nil(t, st.App("APP_LEGACY")) + + stored, err := state.GetSecret("APP_LEGACY", state.SecretAPIKey) + require.NoError(t, err) + assert.Empty(t, stored, "admin keys are never stored") +} + +func TestMigrateIfNeeded_CrawlerCredentials(t *testing.T) { + keyring.MockInit() + + c, statePath := newMigrateConfig(t, ` +[prod] +application_id = "APP_PROD" +api_key = "key-prod" +crawler_user_id = "crawler-user" +crawler_api_key = "crawler-secret" +default = true +`) + + c.MigrateIfNeeded(&bytes.Buffer{}) + + st, err := state.Load(statePath) + require.NoError(t, err) + + app := st.App("APP_PROD") + require.NotNil(t, app) + assert.Equal(t, "crawler-user", app.CrawlerUserID, "crawler user ID is a non-secret in state.toml") + + crawlerKey, err := state.GetSecret("APP_PROD", state.SecretCrawlerAPIKey) + require.NoError(t, err) + assert.Equal(t, "crawler-secret", crawlerKey) + + raw, err := os.ReadFile(statePath) + require.NoError(t, err) + assert.NotContains(t, string(raw), "crawler-secret") +} + +func TestMigrateIfNeeded_SingleProfileWithoutDefaultBecomesCurrent(t *testing.T) { + keyring.MockInit() + + c, statePath := newMigrateConfig(t, ` +[only] +application_id = "APP_ONLY" +api_key = "key-only" +`) + + c.MigrateIfNeeded(&bytes.Buffer{}) + + st, err := state.Load(statePath) + require.NoError(t, err) + assert.Equal(t, "APP_ONLY", st.CurrentApplicationID) +} + +func TestMigrateIfNeeded_IdempotentWhenStateExists(t *testing.T) { + keyring.MockInit() + + c, statePath := newMigrateConfig(t, ` +[prod] +application_id = "APP_PROD" +api_key = "key-prod" +default = true +`) + + // Pre-existing state.toml means the migration is already done. + existing := state.New() + existing.SetApp(&state.ApplicationState{ApplicationID: "EXISTING", Alias: "existing"}) + existing.SetCurrent("EXISTING") + require.NoError(t, existing.Save(statePath)) + + var stderr bytes.Buffer + c.MigrateIfNeeded(&stderr) + + st, err := state.Load(statePath) + require.NoError(t, err) + assert.Equal(t, "EXISTING", st.CurrentApplicationID, "existing state.toml is left untouched") + assert.Nil(t, st.App("APP_PROD")) + assert.Empty(t, stderr.String()) +} + +func TestMigrateIfNeeded_NoConfigFileIsNoop(t *testing.T) { + keyring.MockInit() + + c, statePath := newMigrateConfig(t, "") // no config.toml written + + var stderr bytes.Buffer + c.MigrateIfNeeded(&stderr) + + _, err := os.Stat(statePath) + assert.True(t, os.IsNotExist(err), "no state.toml is created without a legacy config") + assert.Empty(t, stderr.String()) +} diff --git a/pkg/config/profile.go b/pkg/config/profile.go index ffa94df8..a0bd8d57 100644 --- a/pkg/config/profile.go +++ b/pkg/config/profile.go @@ -5,8 +5,10 @@ import ( "path/filepath" "strings" - "github.com/algolia/cli/pkg/utils" "github.com/spf13/viper" + + "github.com/algolia/cli/pkg/config/state" + "github.com/algolia/cli/pkg/utils" ) // DefaultSearchHosts can be set at build time via ldflags, e.g. @@ -22,6 +24,32 @@ type Profile struct { SearchHosts []string `mapstructure:"search_hosts"` Default bool `mapstructure:"default"` + + // statePath points at state.toml; it is set by Config.InitConfig. When + // empty (e.g. in unit tests) credential resolution falls back to the + // legacy config.toml behavior. + statePath string +} + +// loadState resolves the application state that applies to this profile from +// state.toml. The deprecated `--profile ` flag is resolved against the +// stored alias; when the name is not a known alias the current application is +// used instead for consistency. Returns nil when there is no state.toml or no +// applicable application entry. +func (p *Profile) loadState() *state.ApplicationState { + if p.statePath == "" { + return nil + } + s, err := state.Load(p.statePath) + if err != nil { + return nil + } + if p.Name != "" { + if app := s.AppByAlias(p.Name); app != nil { + return app + } + } + return s.ResolveCurrent() } func (p *Profile) GetFieldName(field string) string { @@ -46,6 +74,11 @@ func (p *Profile) GetApplicationID() (string, error) { return p.ApplicationID, nil } + if app := p.loadState(); app != nil && app.ApplicationID != "" { + return app.ApplicationID, nil + } + + // Legacy fallback: config.toml (read-only, scheduled for removal in v2.0). if p.Name == "" { p.LoadDefault() } @@ -69,6 +102,14 @@ func (p *Profile) GetAPIKey() (string, error) { return p.APIKey, nil } + if app := p.loadState(); app != nil && app.ApplicationID != "" { + if key, err := state.GetSecret(app.ApplicationID, state.SecretAPIKey); err == nil && + key != "" { + return key, nil + } + } + + // Legacy fallback: config.toml (read-only, scheduled for removal in v2.0). if p.Name == "" { p.LoadDefault() } @@ -117,6 +158,11 @@ func (p *Profile) GetSearchHosts() []string { return p.SearchHosts } + if app := p.loadState(); app != nil && len(app.SearchHosts) > 0 { + return app.SearchHosts + } + + // Legacy fallback: config.toml (read-only, scheduled for removal in v2.0). if p.Name == "" { p.LoadDefault() } @@ -141,6 +187,11 @@ func (p *Profile) GetCrawlerUserID() (string, error) { return os.Getenv("ALGOLIA_CRAWLER_USER_ID"), nil } + if app := p.loadState(); app != nil && app.CrawlerUserID != "" { + return app.CrawlerUserID, nil + } + + // Legacy fallback: config.toml (read-only, scheduled for removal in v2.0). if p.Name == "" { p.LoadDefault() } @@ -161,6 +212,14 @@ func (p *Profile) GetCrawlerAPIKey() (string, error) { return os.Getenv("ALGOLIA_CRAWLER_API_KEY"), nil } + if app := p.loadState(); app != nil && app.ApplicationID != "" { + if key, err := state.GetSecret(app.ApplicationID, state.SecretCrawlerAPIKey); err == nil && + key != "" { + return key, nil + } + } + + // Legacy fallback: config.toml (read-only, scheduled for removal in v2.0). if p.Name == "" { p.LoadDefault() } diff --git a/pkg/config/profile_test.go b/pkg/config/profile_test.go index 05da8655..18cbaeb0 100644 --- a/pkg/config/profile_test.go +++ b/pkg/config/profile_test.go @@ -1,12 +1,16 @@ package config import ( + "os" "path/filepath" "testing" "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" + + "github.com/algolia/cli/pkg/config/state" ) func initTestViper(configFile string) { @@ -16,6 +20,166 @@ func initTestViper(configFile string) { _ = viper.ReadInConfig() } +// seedState writes a state.toml with the given applications and current app, +// and returns its path. The keychain must be mocked separately. +func seedState(t *testing.T, current string, apps ...*state.ApplicationState) string { + t.Helper() + statePath := filepath.Join(t.TempDir(), "state.toml") + st := state.New() + for _, app := range apps { + st.SetApp(app) + } + if current != "" { + st.SetCurrent(current) + } + require.NoError(t, st.Save(statePath)) + return statePath +} + +func TestProfile_ResolvesFromStateAndKeychain(t *testing.T) { + keyring.MockInit() + viper.Reset() + + statePath := seedState(t, "APP_STATE", &state.ApplicationState{ + ApplicationID: "APP_STATE", + Alias: "prod", + CrawlerUserID: "crawler-user", + SearchHosts: []string{"h1", "h2"}, + }) + require.NoError(t, state.SetSecret("APP_STATE", state.SecretAPIKey, "key-state")) + require.NoError(t, state.SetSecret("APP_STATE", state.SecretCrawlerAPIKey, "crawler-key")) + + p := &Profile{statePath: statePath} + + appID, err := p.GetApplicationID() + require.NoError(t, err) + assert.Equal(t, "APP_STATE", appID) + + apiKey, err := p.GetAPIKey() + require.NoError(t, err) + assert.Equal(t, "key-state", apiKey) + + assert.Equal(t, []string{"h1", "h2"}, p.GetSearchHosts()) + + uid, err := p.GetCrawlerUserID() + require.NoError(t, err) + assert.Equal(t, "crawler-user", uid) + + crawlerKey, err := p.GetCrawlerAPIKey() + require.NoError(t, err) + assert.Equal(t, "crawler-key", crawlerKey) +} + +func TestProfile_EnvWinsOverState(t *testing.T) { + keyring.MockInit() + viper.Reset() + + statePath := seedState(t, "APP_STATE", &state.ApplicationState{ + ApplicationID: "APP_STATE", + Alias: "prod", + }) + require.NoError(t, state.SetSecret("APP_STATE", state.SecretAPIKey, "key-state")) + + t.Setenv("ALGOLIA_APPLICATION_ID", "ENV_APP") + t.Setenv("ALGOLIA_API_KEY", "env-key") + + p := &Profile{statePath: statePath} + + appID, err := p.GetApplicationID() + require.NoError(t, err) + assert.Equal(t, "ENV_APP", appID) + + apiKey, err := p.GetAPIKey() + require.NoError(t, err) + assert.Equal(t, "env-key", apiKey) +} + +func TestProfile_FlagWinsOverState(t *testing.T) { + keyring.MockInit() + viper.Reset() + + statePath := seedState(t, "APP_STATE", &state.ApplicationState{ + ApplicationID: "APP_STATE", + Alias: "prod", + }) + require.NoError(t, state.SetSecret("APP_STATE", state.SecretAPIKey, "key-state")) + + p := &Profile{statePath: statePath, ApplicationID: "FLAG_APP", APIKey: "flag-key"} + + appID, err := p.GetApplicationID() + require.NoError(t, err) + assert.Equal(t, "FLAG_APP", appID) + + apiKey, err := p.GetAPIKey() + require.NoError(t, err) + assert.Equal(t, "flag-key", apiKey) +} + +func TestProfile_ProfileFlagResolvedViaAlias(t *testing.T) { + keyring.MockInit() + viper.Reset() + + statePath := seedState(t, "APP_A", + &state.ApplicationState{ApplicationID: "APP_A", Alias: "a"}, + &state.ApplicationState{ApplicationID: "APP_B", Alias: "b"}, + ) + require.NoError(t, state.SetSecret("APP_A", state.SecretAPIKey, "key-a")) + require.NoError(t, state.SetSecret("APP_B", state.SecretAPIKey, "key-b")) + + // `--profile b` resolves against the stored alias, not the current app. + p := &Profile{statePath: statePath, Name: "b"} + + appID, err := p.GetApplicationID() + require.NoError(t, err) + assert.Equal(t, "APP_B", appID) + + apiKey, err := p.GetAPIKey() + require.NoError(t, err) + assert.Equal(t, "key-b", apiKey) +} + +func TestProfile_UnknownProfileFlagFallsBackToCurrent(t *testing.T) { + keyring.MockInit() + viper.Reset() + + statePath := seedState(t, "APP_A", + &state.ApplicationState{ApplicationID: "APP_A", Alias: "a"}, + ) + require.NoError(t, state.SetSecret("APP_A", state.SecretAPIKey, "key-a")) + + // `--profile does-not-exist` is not a known alias: resolve to current. + p := &Profile{statePath: statePath, Name: "does-not-exist"} + + appID, err := p.GetApplicationID() + require.NoError(t, err) + assert.Equal(t, "APP_A", appID) +} + +func TestProfile_FallsBackToConfigTomlWhenNoState(t *testing.T) { + keyring.MockInit() + + dir := t.TempDir() + configPath := filepath.Join(dir, "config.toml") + require.NoError(t, os.WriteFile(configPath, []byte(` +[prod] +application_id = "APP_CFG" +api_key = "key-cfg" +default = true +`), 0o600)) + initTestViper(configPath) + + // state.toml does not exist on disk → fall back to config.toml. + p := &Profile{statePath: filepath.Join(dir, "state.toml")} + + appID, err := p.GetApplicationID() + require.NoError(t, err) + assert.Equal(t, "APP_CFG", appID) + + apiKey, err := p.GetAPIKey() + require.NoError(t, err) + assert.Equal(t, "key-cfg", apiKey) +} + func TestAddProfile_PreservesExistingProfiles(t *testing.T) { configFile := filepath.Join(t.TempDir(), "config.toml") diff --git a/pkg/config/state/secret.go b/pkg/config/state/secret.go new file mode 100644 index 00000000..cb656e35 --- /dev/null +++ b/pkg/config/state/secret.go @@ -0,0 +1,53 @@ +package state + +import ( + "errors" + + "github.com/zalando/go-keyring" +) + +// secretService is the keychain service under which per-application secrets are +// stored. It matches the service used for the OAuth token (pkg/auth) but the +// account names are namespaced per application and secret kind, so there is no +// collision. +const secretService = "algolia-cli" + +// Secret kinds stored in the OS keychain, namespaced per application. +const ( + SecretAPIKey = "api_key" + SecretCrawlerAPIKey = "crawler_api_key" +) + +// secretAccount builds the keychain account name for an application secret, +// e.g. "ABCDEF1234:api_key". +func secretAccount(appID, kind string) string { + return appID + ":" + kind +} + +// GetSecret reads a secret of the given kind for appID from the OS keychain. +// It returns an empty string and no error when the secret is not set. +func GetSecret(appID, kind string) (string, error) { + value, err := keyring.Get(secretService, secretAccount(appID, kind)) + if err != nil { + if errors.Is(err, keyring.ErrNotFound) { + return "", nil + } + return "", err + } + return value, nil +} + +// SetSecret stores a secret of the given kind for appID in the OS keychain. +func SetSecret(appID, kind, value string) error { + return keyring.Set(secretService, secretAccount(appID, kind), value) +} + +// DeleteSecret removes a secret of the given kind for appID from the OS +// keychain. Removing a missing secret is not an error. +func DeleteSecret(appID, kind string) error { + err := keyring.Delete(secretService, secretAccount(appID, kind)) + if err != nil && !errors.Is(err, keyring.ErrNotFound) { + return err + } + return nil +} diff --git a/pkg/config/state/secret_test.go b/pkg/config/state/secret_test.go new file mode 100644 index 00000000..f10aec03 --- /dev/null +++ b/pkg/config/state/secret_test.go @@ -0,0 +1,69 @@ +package state + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/go-keyring" +) + +func TestSecrets_SetGetDelete(t *testing.T) { + keyring.MockInit() + + require.NoError(t, SetSecret("APP1", SecretAPIKey, "secret-key")) + + got, err := GetSecret("APP1", SecretAPIKey) + require.NoError(t, err) + assert.Equal(t, "secret-key", got) + + require.NoError(t, DeleteSecret("APP1", SecretAPIKey)) + + got, err = GetSecret("APP1", SecretAPIKey) + require.NoError(t, err) + assert.Empty(t, got, "deleted secret reads back empty") +} + +func TestGetSecret_MissingReturnsEmpty(t *testing.T) { + keyring.MockInit() + + got, err := GetSecret("UNKNOWN", SecretAPIKey) + require.NoError(t, err) + assert.Empty(t, got) +} + +func TestDeleteSecret_MissingIsNoError(t *testing.T) { + keyring.MockInit() + require.NoError(t, DeleteSecret("UNKNOWN", SecretCrawlerAPIKey)) +} + +func TestSecrets_NamespacedPerAppAndKind(t *testing.T) { + keyring.MockInit() + + require.NoError(t, SetSecret("APP1", SecretAPIKey, "app1-api")) + require.NoError(t, SetSecret("APP1", SecretCrawlerAPIKey, "app1-crawler")) + require.NoError(t, SetSecret("APP2", SecretAPIKey, "app2-api")) + + api1, err := GetSecret("APP1", SecretAPIKey) + require.NoError(t, err) + assert.Equal(t, "app1-api", api1) + + crawler1, err := GetSecret("APP1", SecretCrawlerAPIKey) + require.NoError(t, err) + assert.Equal(t, "app1-crawler", crawler1) + + api2, err := GetSecret("APP2", SecretAPIKey) + require.NoError(t, err) + assert.Equal(t, "app2-api", api2) + + // Deleting one secret must not affect the others. + require.NoError(t, DeleteSecret("APP1", SecretAPIKey)) + + crawler1, err = GetSecret("APP1", SecretCrawlerAPIKey) + require.NoError(t, err) + assert.Equal(t, "app1-crawler", crawler1) + + api2, err = GetSecret("APP2", SecretAPIKey) + require.NoError(t, err) + assert.Equal(t, "app2-api", api2) +} diff --git a/pkg/config/state/state.go b/pkg/config/state/state.go new file mode 100644 index 00000000..ad812017 --- /dev/null +++ b/pkg/config/state/state.go @@ -0,0 +1,212 @@ +// Package state stores the CLI's non-secret application state in state.toml, +// alongside the secrets kept in the OS keychain (see secret.go). It is the +// source of truth introduced to replace the legacy config.toml; config.toml is +// only read as a migration/fallback source and is scheduled for removal in +// CLI v2.0. +package state + +import ( + "bytes" + "errors" + "os" + "path/filepath" + + "github.com/BurntSushi/toml" + "github.com/mitchellh/go-homedir" +) + +// ApplicationState holds the non-secret state for a single Algolia +// application. Secrets (api_key, crawler_api_key) live in the OS keychain and +// are intentionally absent here. +type ApplicationState struct { + // ApplicationID is the Algolia application ID and the map key under + // [applications.]. + ApplicationID string `toml:"application_id"` + // Alias preserves the legacy profile name so that `--profile ` + // keeps working until CLI v2.0. + Alias string `toml:"alias,omitempty"` + // APIKeyUUID identifies the API key stored in the keychain, so it can be + // rotated/revoked without re-reading the secret. + APIKeyUUID string `toml:"api_key_uuid,omitempty"` + + // The following are lazily fetched when needed and cached here. + ApplicationName string `toml:"application_name,omitempty"` + CrawlerUserID string `toml:"crawler_user_id,omitempty"` + Region string `toml:"region,omitempty"` + SearchHosts []string `toml:"search_hosts,omitempty"` +} + +// State is the on-disk representation of state.toml. +type State struct { + // CurrentApplicationID points at the active application in Applications. + CurrentApplicationID string `toml:"current_application_id,omitempty"` + // Applications is keyed by application ID. + Applications map[string]*ApplicationState `toml:"applications,omitempty"` +} + +// New returns an empty, ready-to-use State. +func New() *State { + return &State{Applications: map[string]*ApplicationState{}} +} + +// DefaultPath returns the default location of state.toml, mirroring the config +// folder resolution used for config.toml (XDG_CONFIG_HOME, then ~/.config). +func DefaultPath() string { + base := os.Getenv("XDG_CONFIG_HOME") + if base == "" { + home, err := homedir.Dir() + if err != nil { + // Last resort: relative path in the working directory. + return filepath.Join("algolia", "state.toml") + } + base = filepath.Join(home, ".config") + } + return filepath.Join(base, "algolia", "state.toml") +} + +// Load reads state.toml from path. A missing file yields an empty State and no +// error, so callers can treat "not migrated yet" the same as "empty state". +func Load(path string) (*State, error) { + s := New() + if path == "" { + return s, nil + } + + data, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return s, nil + } + return nil, err + } + + if _, err := toml.Decode(string(data), s); err != nil { + return nil, err + } + if s.Applications == nil { + s.Applications = map[string]*ApplicationState{} + } + return s, nil +} + +// Save writes state.toml atomically with 0600 permissions, creating parent +// directories as needed. It writes to a temp file in the destination directory +// and renames it into place to avoid leaving a half-written file. +func (s *State) Save(path string) error { + if path == "" { + return errors.New("state: empty path") + } + + dir := filepath.Dir(path) + if err := os.MkdirAll(dir, 0o700); err != nil { + return err + } + + var buf bytes.Buffer + if err := toml.NewEncoder(&buf).Encode(s); err != nil { + return err + } + + tmp, err := os.CreateTemp(dir, ".state-*.toml") + if err != nil { + return err + } + tmpName := tmp.Name() + defer os.Remove(tmpName) + + if err := tmp.Chmod(0o600); err != nil { + _ = tmp.Close() + return err + } + if _, err := tmp.Write(buf.Bytes()); err != nil { + _ = tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + + return os.Rename(tmpName, path) +} + +// ResolveCurrent returns the current application's state, or nil when no +// current application is set or it is missing from Applications. +func (s *State) ResolveCurrent() *ApplicationState { + if s == nil || s.CurrentApplicationID == "" { + return nil + } + return s.App(s.CurrentApplicationID) +} + +// SetCurrent marks appID as the current application, creating its entry when +// missing. +func (s *State) SetCurrent(appID string) { + if appID == "" { + return + } + s.ensureApp(appID) + s.CurrentApplicationID = appID +} + +// App returns the application state for appID, or nil when absent. +func (s *State) App(appID string) *ApplicationState { + if s == nil || s.Applications == nil { + return nil + } + return s.Applications[appID] +} + +// AppByAlias returns the application state whose alias matches, or nil. It is +// used to resolve the deprecated `--profile ` flag. +func (s *State) AppByAlias(alias string) *ApplicationState { + if s == nil || alias == "" { + return nil + } + for _, app := range s.Applications { + if app.Alias == alias { + return app + } + } + return nil +} + +// SetApp inserts or replaces the state for an application (keyed by its ID). +func (s *State) SetApp(app *ApplicationState) { + if app == nil || app.ApplicationID == "" { + return + } + if s.Applications == nil { + s.Applications = map[string]*ApplicationState{} + } + s.Applications[app.ApplicationID] = app +} + +// PutAPIKeyUUID records the keychain API key UUID for appID, creating the +// entry when missing. +func (s *State) PutAPIKeyUUID(appID, uuid string) { + if appID == "" { + return + } + s.ensureApp(appID).APIKeyUUID = uuid +} + +// APIKeyUUID returns the stored API key UUID for appID, or "". +func (s *State) APIKeyUUID(appID string) string { + if app := s.App(appID); app != nil { + return app.APIKeyUUID + } + return "" +} + +// ensureApp returns the existing entry for appID or creates a new one. +func (s *State) ensureApp(appID string) *ApplicationState { + if s.Applications == nil { + s.Applications = map[string]*ApplicationState{} + } + app, ok := s.Applications[appID] + if !ok { + app = &ApplicationState{ApplicationID: appID} + s.Applications[appID] = app + } + return app +} diff --git a/pkg/config/state/state_test.go b/pkg/config/state/state_test.go new file mode 100644 index 00000000..4ebcc925 --- /dev/null +++ b/pkg/config/state/state_test.go @@ -0,0 +1,160 @@ +package state + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLoad_MissingFileReturnsEmptyState(t *testing.T) { + s, err := Load(filepath.Join(t.TempDir(), "does-not-exist.toml")) + require.NoError(t, err) + require.NotNil(t, s) + assert.Empty(t, s.CurrentApplicationID) + assert.Empty(t, s.Applications) +} + +func TestLoad_EmptyPathReturnsEmptyState(t *testing.T) { + s, err := Load("") + require.NoError(t, err) + require.NotNil(t, s) + assert.NotNil(t, s.Applications) +} + +func TestSaveLoad_RoundTrip(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + + s := New() + s.SetApp(&ApplicationState{ + ApplicationID: "APP1", + Alias: "prod", + APIKeyUUID: "uuid-1", + ApplicationName: "Production", + CrawlerUserID: "crawler-1", + Region: "us", + SearchHosts: []string{"host1", "host2"}, + }) + s.SetCurrent("APP1") + require.NoError(t, s.Save(path)) + + got, err := Load(path) + require.NoError(t, err) + assert.Equal(t, "APP1", got.CurrentApplicationID) + + app := got.App("APP1") + require.NotNil(t, app) + assert.Equal(t, "APP1", app.ApplicationID) + assert.Equal(t, "prod", app.Alias) + assert.Equal(t, "uuid-1", app.APIKeyUUID) + assert.Equal(t, "Production", app.ApplicationName) + assert.Equal(t, "crawler-1", app.CrawlerUserID) + assert.Equal(t, "us", app.Region) + assert.Equal(t, []string{"host1", "host2"}, app.SearchHosts) +} + +func TestSave_FilePermissionsAre0600(t *testing.T) { + path := filepath.Join(t.TempDir(), "state.toml") + s := New() + s.SetCurrent("APP1") + require.NoError(t, s.Save(path)) + + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o600), info.Mode().Perm()) +} + +func TestSave_CreatesParentDirectories(t *testing.T) { + path := filepath.Join(t.TempDir(), "nested", "deeper", "state.toml") + s := New() + s.SetCurrent("APP1") + require.NoError(t, s.Save(path)) + + _, err := os.Stat(path) + require.NoError(t, err) +} + +func TestSave_OverwritesAtomically(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "state.toml") + + first := New() + first.SetApp(&ApplicationState{ApplicationID: "APP1", Alias: "one"}) + first.SetCurrent("APP1") + require.NoError(t, first.Save(path)) + + second := New() + second.SetApp(&ApplicationState{ApplicationID: "APP2", Alias: "two"}) + second.SetCurrent("APP2") + require.NoError(t, second.Save(path)) + + got, err := Load(path) + require.NoError(t, err) + assert.Equal(t, "APP2", got.CurrentApplicationID) + assert.Nil(t, got.App("APP1")) + assert.NotNil(t, got.App("APP2")) + + // No leftover temp files. + entries, err := os.ReadDir(dir) + require.NoError(t, err) + assert.Len(t, entries, 1) +} + +func TestSave_EmptyPathErrors(t *testing.T) { + require.Error(t, New().Save("")) +} + +func TestResolveCurrent(t *testing.T) { + s := New() + assert.Nil(t, s.ResolveCurrent(), "no current set") + + s.SetApp(&ApplicationState{ApplicationID: "APP1", Alias: "a"}) + s.CurrentApplicationID = "MISSING" + assert.Nil(t, s.ResolveCurrent(), "current points at missing app") + + s.SetCurrent("APP1") + require.NotNil(t, s.ResolveCurrent()) + assert.Equal(t, "APP1", s.ResolveCurrent().ApplicationID) +} + +func TestSetCurrent_CreatesEntry(t *testing.T) { + s := New() + s.SetCurrent("APP1") + assert.Equal(t, "APP1", s.CurrentApplicationID) + require.NotNil(t, s.App("APP1")) + assert.Equal(t, "APP1", s.App("APP1").ApplicationID) +} + +func TestAppByAlias(t *testing.T) { + s := New() + s.SetApp(&ApplicationState{ApplicationID: "APP1", Alias: "prod"}) + s.SetApp(&ApplicationState{ApplicationID: "APP2", Alias: "staging"}) + + require.NotNil(t, s.AppByAlias("staging")) + assert.Equal(t, "APP2", s.AppByAlias("staging").ApplicationID) + assert.Nil(t, s.AppByAlias("unknown")) + assert.Nil(t, s.AppByAlias(""), "empty alias must never match") +} + +func TestAPIKeyUUID(t *testing.T) { + s := New() + assert.Empty(t, s.APIKeyUUID("APP1")) + + s.PutAPIKeyUUID("APP1", "uuid-123") + assert.Equal(t, "uuid-123", s.APIKeyUUID("APP1")) + + // PutAPIKeyUUID must not clobber other fields on an existing app. + s.SetApp(&ApplicationState{ApplicationID: "APP2", Alias: "two"}) + s.PutAPIKeyUUID("APP2", "uuid-456") + assert.Equal(t, "two", s.App("APP2").Alias) + assert.Equal(t, "uuid-456", s.App("APP2").APIKeyUUID) +} + +func TestSetApp_IgnoresInvalid(t *testing.T) { + s := New() + s.SetApp(nil) + s.SetApp(&ApplicationState{ApplicationID: ""}) + assert.Empty(t, s.Applications) +}