Files
zero/internal/config/credentials.go
KRATOS 6e0a665118 fix(auth): propagate credentials to every provider-build surface and pin children to the live provider (#366)
* fix(auth): propagate credentials to every provider-build surface and pin children to the live provider

Sub-agents, swarm members, ACP sessions, and mid-run escalation switches
could build providers without the credentials the parent was running with.
This consolidates credential resolution so every surface gets the same auth:

- Wrap deps.newProvider once in fillAppDeps to fold the stored API key in
  centrally; ACP, exec escalation, and buildProvider no longer patch it
  per call site (an apiKeyStored profile previously went out keyless on
  those paths and 401'd).
- Export ZERO_PROVIDER at startup (buildProvider) and keep exporting on
  every runtime switch (wizard, onboarding, /model, /provider), so spawned
  children resolve the parent's live provider instead of whatever another
  process last persisted to config.json. Injected via appDeps so tests
  never mutate the process environment.
- Share one OAuth login selection end to end: ProviderProfile.
  OAuthLoginCandidates (name, then catalog-ID fallback; nothing for a
  profile with its own configured credential) + oauth.FirstStored resolve
  the login ONCE, and the bound store key is threaded through
  providers.Options.OAuthLoginKey so the Codex account-id header always
  reads the same login that issued the bearer token. A renamed profile
  (name != catalog ID) now finds its login; an explicit API key is never
  silently overridden by a same-name or catalog-shared OAuth login.
- Export ProviderProfile.HasConfiguredCredential as the single "profile is
  key-authed" predicate (cli/tui/config previously had three copies).
- applyProviderWizard commits live state (provider, profile, name, env
  export) only after BOTH the build and the config persist succeed, so a
  persist failure no longer leaves the chat running on a provider the
  status line and children disagree with.

Covered by red-green tests for each path: stored-key wrap, escalation
switch, catalog-ID fallback (incl. case-variant names), explicit-key
protection, same-login account selection, wizard persist-failure rollback,
onboarding/startup env export.

* test: isolate OAuth/config test storage and fix the flaky MCP response-id test

Review + CI fixes, all test-only:

- factory/cli OAuth tests: pin ZERO_OAUTH_STORAGE=file next to the temp
  ZERO_OAUTH_TOKENS_PATH — an inherited "keyring" value sends NewStore to
  the OS keychain and ignores the temp path entirely, so the tests could
  read (or clobber) the developer's real logins. newOAuthStoreForTest now
  owns both pins so a future call site can't forget them.
- TestApplyProviderWizardExportsActiveProviderEnv: point m.userConfigPath
  at a temp config and stub newProvider, so the SUCCESS path exercises the
  full build+persist+commit sequence in isolation (the default empty path
  silently skipped persist; a changed default must never reach the real
  user config).
- TestClientRequestWaitsForMatchingResponseID (ubuntu smoke flake): the
  responses were pre-loaded into a bytes.Buffer, so the client's reader
  goroutine could consume the matching response — and hit EOF — before
  request() registered its pending id, failing with "request() error =
  EOF" on fast runners. The fake server now sits on pipes and responds
  only after reading the outgoing request, matching production ordering;
  survives -race -count=50.
2026-07-02 12:21:27 +05:30

235 lines
8.8 KiB
Go

package config
import (
"encoding/json"
"fmt"
"os"
"path/filepath"
"strings"
"github.com/Gitlawb/zero/internal/credstore"
)
// ProviderKeyStoreAt opens the encrypted credential store whose file backend lives
// in dir. The backend resolves keyring-first, then encrypted-file, with a plaintext
// opt-out via ZERO_CRED_STORAGE. The dir parameter exists so tests can point the file
// backend at a temp directory; production always uses the user config directory
// (ProviderKeyStore) because provider API keys are user-scoped by design — they are
// only ever captured under the user config, never project config (a cloned repo must
// not carry keys), so runtime lookups deliberately use the user store regardless of
// where a provider profile was resolved from.
func ProviderKeyStoreAt(dir string) (*credstore.Store, error) {
return credstore.New(credstore.Options{Dir: dir})
}
// ProviderKeyStore opens the credential store beside the default user config.
func ProviderKeyStore() (*credstore.Store, error) {
configPath, err := DefaultUserConfigPath()
if err != nil {
return nil, err
}
return ProviderKeyStoreAt(filepath.Dir(configPath))
}
// APIKeyGetter is the read surface of the credential store; *credstore.Store
// satisfies it. Kept here so callers can inject a fake in tests.
type APIKeyGetter interface {
Get(provider string) (string, bool, error)
}
// APIKeySetter is the write surface of the credential store.
type APIKeySetter interface {
Set(provider, key string) error
}
// SecureProviderProfile moves an inline APIKey on the profile into the credential
// store co-located with configPath, returning a profile with APIKeyStored set and
// APIKey cleared so the secret is never written to config.json. On any store error
// (e.g. no keyring) it returns the profile unchanged — the inline key persists and
// the startup migration retries later — so capturing a provider never fails on a
// flaky credential backend.
func SecureProviderProfile(profile ProviderProfile, configPath string) ProviderProfile {
if strings.TrimSpace(profile.APIKey) == "" || strings.TrimSpace(profile.Name) == "" {
return profile
}
store, err := ProviderKeyStoreAt(filepath.Dir(configPath))
if err != nil {
return profile
}
if err := store.Set(profile.Name, profile.APIKey); err != nil {
return profile
}
secured := profile
secured.APIKey = ""
secured.APIKeyStored = true
return secured
}
// ForgetProviderKey removes a provider's stored API key from the credential store,
// reporting whether one existed. Used by the lifecycle "remove key" / auth logout.
func ForgetProviderKey(provider string) (bool, error) {
store, err := ProviderKeyStore()
if err != nil {
return false, err
}
return store.Delete(provider)
}
// ClearProviderKeyStored unsets the APIKeyStored marker for a provider in the
// config at path, so credential checks no longer claim a stored key after one is
// removed. No-op when path/provider is empty, the config is absent, or the marker
// is already unset.
func ClearProviderKeyStored(path, provider string) (bool, error) {
path = strings.TrimSpace(path)
provider = strings.TrimSpace(provider)
if path == "" || provider == "" {
return false, nil
}
data, err := os.ReadFile(path)
if err != nil {
if os.IsNotExist(err) {
return false, nil
}
return false, fmt.Errorf("read config %s: %w", path, err)
}
var cfg FileConfig
if err := json.Unmarshal(data, &cfg); err != nil {
return false, fmt.Errorf("invalid config JSON %s: %w", path, err)
}
changed := false
for index := range cfg.Providers {
if strings.EqualFold(strings.TrimSpace(cfg.Providers[index].Name), provider) && cfg.Providers[index].APIKeyStored {
cfg.Providers[index].APIKeyStored = false
changed = true
}
}
if !changed {
return false, nil
}
return true, writeConfigFile(path, cfg)
}
// MigratePlaintextProviderKeys moves any inline plaintext API key in the config at
// path into the credential store, marking the profile APIKeyStored and stripping
// the inline secret — but ONLY after the store write succeeds, so a failed Set
// leaves the plaintext key in place and never strands a credential. Returns how
// many keys were migrated; a no-op (0, nil) when path is empty/absent or nothing
// needs migrating. Safe to run on every startup (idempotent).
func MigratePlaintextProviderKeys(path string, store APIKeySetter) (int, error) {
path = strings.TrimSpace(path)
if path == "" || store == nil {
return 0, nil
}
data, err := os.ReadFile(path)
if err != nil {
if os.IsNotExist(err) {
return 0, nil
}
return 0, fmt.Errorf("read config %s: %w", path, err)
}
var cfg FileConfig
if err := json.Unmarshal(data, &cfg); err != nil {
return 0, fmt.Errorf("invalid config JSON %s: %w", path, err)
}
migrated := 0
for index := range cfg.Providers {
profile := &cfg.Providers[index]
key := strings.TrimSpace(profile.APIKey)
if key == "" || strings.TrimSpace(profile.Name) == "" {
continue
}
if err := store.Set(profile.Name, key); err != nil {
// Leave the plaintext key untouched; a failed Set must not strand it.
continue
}
profile.APIKey = ""
profile.APIKeyStored = true
migrated++
}
if migrated == 0 {
return 0, nil
}
if err := writeConfigFile(path, cfg); err != nil {
return migrated, fmt.Errorf("rewrite config after migration %s: %w", path, err)
}
return migrated, nil
}
// HasConfiguredCredential reports whether the profile is set up to authenticate
// with its own API key (inline, stored, or a raw auth header). It is the single
// definition of "this profile is key-authed", shared by OAuthLoginCandidates and
// the cli/tui setup surfaces so they never disagree about whether a profile is
// keyed. APIKeyEnv is deliberately NOT included: an env var may be unset at
// runtime while the profile actually relies on an OAuth login, so an
// env-configured-but-empty profile must still be allowed to resolve a token.
func (profile ProviderProfile) HasConfiguredCredential() bool {
return strings.TrimSpace(profile.APIKey) != "" ||
profile.APIKeyStored ||
strings.TrimSpace(profile.AuthHeaderValue) != ""
}
// OAuthLoginCandidates returns the login names to try, in order, when resolving
// this profile's OAuth token — used by the runtime bearer resolver, the Codex
// account-header resolver, and the onboarding presence check so all three agree.
//
// It returns NO candidates for a profile that carries its own configured
// credential: attaching an OAuth resolver to such a profile makes withBearer
// erase the key the instant a token resolves, silently routing/billing the call
// through the OAuth account instead of the intended key (the cross-profile
// override the review caught — and, via the profile name, a same-name login
// overriding an explicit key). A stored-key profile that reaches this keyless
// (e.g. a transiently unreadable keyring) is likewise treated as key-auth, so it
// fails with a clear missing-credential error rather than silently borrowing an
// unrelated OAuth login. APIKeyEnv is intentionally not part of that gate (an
// env var may be unset while the profile relies on OAuth).
//
// For a keyless profile the profile name comes first (a login under your own
// profile name is unambiguously yours), then the catalog ID as a FALLBACK (so a
// profile renamed away from its catalog ID — e.g. {name:"codex",
// catalogID:"chatgpt"} — still finds a `zero auth login chatgpt` token).
// Candidates are trimmed, blank-skipped, and de-duplicated CASE-SENSITIVELY — the
// OAuth token store is a case-sensitive map, so "ChatGPT" and "chatgpt" are
// distinct keys and both must survive.
func (profile ProviderProfile) OAuthLoginCandidates() []string {
if profile.HasConfiguredCredential() {
return nil
}
var names []string
add := func(s string) {
s = strings.TrimSpace(s)
if s == "" {
return
}
for _, existing := range names {
if existing == s { // case-sensitive: the store lookup is case-sensitive
return
}
}
names = append(names, s)
}
add(profile.Name)
add(profile.CatalogID)
return names
}
// ApplyStoredAPIKey fills profile.APIKey from the credential store when it is not
// already resolved — an inline config key or a resolved APIKeyEnv always wins, so
// this only supplies a key for providers whose secret lives in the store. A nil
// store, a miss, or a store error leaves the profile unchanged.
func ApplyStoredAPIKey(profile ProviderProfile, store APIKeyGetter) ProviderProfile {
// Only load for profiles that opted into stored-key auth (APIKeyStored). Without
// this gate a stale keyring/file entry could silently reactivate credentials for a
// profile that no longer uses the store.
if store == nil || !profile.APIKeyStored || strings.TrimSpace(profile.APIKey) != "" {
return profile
}
name := strings.TrimSpace(profile.Name)
if name == "" {
return profile
}
if key, ok, err := store.Get(name); err == nil && ok && strings.TrimSpace(key) != "" {
profile.APIKey = key
}
return profile
}