feat(credentials): add secure credential storage for API keys by appleboy · Pull Request #263 · appleboy/CodeGPT

appleboy · 2026-03-11T04:26:39Z

Summary

Integrates github.com/go-authgate/sdk-go v0.2.0 credstore package to store openai.api_key and gemini.api_key in the OS keyring (macOS Keychain / Linux Secret Service / Windows Credential Manager)
Automatically falls back to a file-based store (~/.config/codegpt/.cache/credentials.json) when the OS keyring is unavailable (CI/CD, headless servers)
Auto-migrates existing plaintext YAML values to the secure store on first run
config list now shows the storage status of sensitive keys: (stored in keyring), (stored in secure file), or a migration prompt if still in YAML

Changes

File	Change
`go.mod` / `go.sum`	Add `github.com/go-authgate/sdk-go v0.2.0` and `github.com/zalando/go-keyring`
`util/credstore.go`	New — thin wrapper (`GetCredential`, `SetCredential`, `DeleteCredential`, `CredStoreIsKeyring`)
`util/credstore_test.go`	New — 6 unit tests using file-based store (no real keyring needed in CI)
`cmd/cmd.go`	Add `sensitiveConfigKeys`, `migrateCredentialsToStore()`, call migration after `viper.ReadInConfig()`
`cmd/config_set.go`	Route `openai.api_key` / `gemini.api_key` to credstore instead of YAML
`cmd/provider.go`	Add `getAPIKey()` helper; use it in `NewOpenAI`, `NewGemini`, `NewAnthropic`
`cmd/config_list.go`	Show credstore status for sensitive keys

Test plan

go test ./... passes
make lint reports 0 issues
codegpt config set openai.api_key sk-xxx stores key in keyring, not YAML
grep api_key ~/.config/codegpt/.codegpt.yaml returns empty / absent
codegpt config list shows (stored in keyring) or (stored in secure file)
Existing OPENAI_API_KEY env var still works as fallback
On headless/CI server (no keyring), falls back to file store transparently

🤖 Generated with Claude Code

- Introduce a secure credential store with OS keyring support and file fallback for managing API keys - Migrate sensitive API keys out of plaintext config files into the secure store during config initialization - Route config set operations for sensitive keys to the secure store and ensure they are removed from YAML - Enhance config list output to reflect where sensitive credentials are stored or if they remain unmigrated - Update all provider initializations to retrieve API keys from the secure store with fallback to existing config sources - Add comprehensive tests covering secure credential storage, retrieval, deletion, and missing-key behavior - Add and update dependencies required for secure credential storage and platform support Signed-off-by: Bo-Yi Wu <appleboy.tw@gmail.com>

Copilot

Pull request overview

Adds secure credential storage for provider API keys by integrating github.com/go-authgate/sdk-go/credstore, with automatic migration from plaintext YAML and UI/status updates in config commands.

Changes:

Introduce util/credstore wrapper + unit tests for file-backed storage.
Auto-migrate openai.api_key / gemini.api_key out of YAML into the secure store; route config set for those keys into credstore.
Update provider initialization and config list to retrieve/show sensitive keys via credstore first.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`util/credstore.go`	Adds singleton credstore wrapper with keyring + file fallback.
`util/credstore_test.go`	Adds unit tests validating file-backed credstore and wrapper behavior.
`cmd/cmd.go`	Adds sensitive key list and auto-migration after config load.
`cmd/config_set.go`	Stores sensitive keys in credstore instead of YAML.
`cmd/config_list.go`	Shows status for sensitive keys (keyring/file/YAML prompt).
`cmd/provider.go`	Adds helper to fetch API keys from credstore then viper fallback.
`go.mod` / `go.sum`	Adds credstore/keyring dependencies and bumps `x/sys`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-11T04:31:43Z

cmd/cmd.go

+	for _, key := range sensitiveConfigKeys {
+		val := viper.GetString(key)
+		if val == "" {
+			continue
+		}
+		// Check if already in credstore; skip if already migrated.
+		existing, err := util.GetCredential(key)
+		if err != nil || existing != "" {
+			continue
+		}
+		if err := util.SetCredential(key, val); err != nil {
+			fmt.Fprintf(os.Stderr, "warning: could not migrate %s to secure store: %v\n", key, err)
+			continue
+		}


migrateCredentialsToStore() calls viper.GetString(key), which includes values from env vars due to viper.AutomaticEnv(). This means running any command with OPENAI_API_KEY/GEMINI_API_KEY set can silently persist that secret into the credential store and also write to the config file (side-effect in CI/headless too). Migration should only occur for values actually present in the config file (e.g., gate on viper.InConfig(key) or read the YAML directly) and avoid writing when the value source is env/flags.

Copilot · 2026-03-11T04:31:44Z

util/credstore.go

+
+func init() {
+	home, _ := os.UserHomeDir()
+	fallbackPath := filepath.Join(home, ".config", "codegpt", ".cache", "credentials.json")


The fallback credential file path points under ~/.config/codegpt/.cache/, but this init() doesn’t create that directory. If the OS keyring is unavailable and the directory doesn’t already exist, file-backed Save/Load can fail. Consider ensuring filepath.Dir(fallbackPath) exists (MkdirAll with restrictive perms) before constructing the file store.

Suggested change

fallbackPath := filepath.Join(home, ".config", "codegpt", ".cache", "credentials.json")

fallbackPath := filepath.Join(home, ".config", "codegpt", ".cache", "credentials.json")

// Ensure the directory for the fallback credential file exists, with

// restrictive permissions, before constructing the file-backed store.

dir := filepath.Dir(fallbackPath)

_ = os.MkdirAll(dir, 0o700)

Copilot · 2026-03-11T04:31:44Z

util/credstore.go

+	home, _ := os.UserHomeDir()
+	fallbackPath := filepath.Join(home, ".config", "codegpt", ".cache", "credentials.json")


os.UserHomeDir() errors are ignored here, which can yield an empty/relative fallbackPath and make credential storage behave unpredictably. This should handle the error explicitly (and either fail fast, or fall back to a known safe directory) so callers can surface a useful error instead of silently misconfiguring the store.

Suggested change

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")

}

Copilot · 2026-03-11T04:31:44Z

cmd/config_list.go

+					} else {
+						tbl.AddRow(v, "(stored in secure file)")
+					}
+				case viper.GetString(v) != "":


config list uses viper.GetString(v) to decide whether a sensitive key is still stored in YAML, but viper.GetString will also return env var/flag values. With OPENAI_API_KEY set, this will incorrectly show the "(YAML — run config set to migrate)" message even though nothing is in the YAML file. Use a config-file-only check (e.g., viper.InConfig(v)) for this branch so the status reflects actual storage.

Suggested change

case viper.GetString(v) != "":

case viper.InConfig(v):

Copilot · 2026-03-11T04:31:44Z

cmd/config_list.go

+			if slices.Contains(sensitiveConfigKeys, v) {
+				cred, _ := util.GetCredential(v)
+				switch {
+				case cred != "":
+					if util.CredStoreIsKeyring() {
+						tbl.AddRow(v, "(stored in keyring)")
+					} else {


util.GetCredential(v) errors are ignored here. If the credential store is misconfigured/unavailable, the command will silently fall through and potentially present a misleading status. Consider handling the error explicitly (e.g., show an "error reading secure store" status) so users can troubleshoot.

Copilot · 2026-03-11T04:31:44Z

util/credstore_test.go

+	path := t.TempDir() + "/creds.json"
+	file := credstore.NewStringFileStore(path)


The test builds paths via string concatenation with "/creds.json". Using filepath.Join(t.TempDir(), "creds.json") avoids relying on path separators and matches the rest of the codebase’s filepath usage.

- 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>

Copilot AI review requested due to automatic review settings March 11, 2026 04:26

Copilot started reviewing on behalf of appleboy March 11, 2026 04:27 View session

Copilot AI reviewed Mar 11, 2026

View reviewed changes

appleboy merged commit cba837a into main Mar 11, 2026
27 checks passed

appleboy deleted the credstore-secure-api-key branch March 11, 2026 05:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(credentials): add secure credential storage for API keys#263

feat(credentials): add secure credential storage for API keys#263
appleboy merged 2 commits intomainfrom
credstore-secure-api-key

appleboy commented Mar 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Copilot AI Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-	fallbackPath := filepath.Join(home, ".config", "codegpt", ".cache", "credentials.json")
+	fallbackPath := filepath.Join(home, ".config", "codegpt", ".cache", "credentials.json")
+	// Ensure the directory for the fallback credential file exists, with
+	// restrictive permissions, before constructing the file-backed store.
+	dir := filepath.Dir(fallbackPath)
+	_ = os.MkdirAll(dir, 0o700)

		home, _ := os.UserHomeDir()
		fallbackPath := filepath.Join(home, ".config", "codegpt", ".cache", "credentials.json")

-	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")
+	}

		path := t.TempDir() + "/creds.json"
		file := credstore.NewStringFileStore(path)

Uh oh!

Conversation

appleboy commented Mar 11, 2026

Summary

Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants