Files
zero/internal/config/credentials_test.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

241 lines
7.8 KiB
Go

package config
import (
"encoding/json"
"errors"
"os"
"path/filepath"
"strings"
"testing"
)
type fakeKeyGetter struct {
keys map[string]string
err error
}
func (f fakeKeyGetter) Get(provider string) (string, bool, error) {
if f.err != nil {
return "", false, f.err
}
v, ok := f.keys[provider]
return v, ok, nil
}
// fakeKeySetter records Set calls; setErr (when non-nil) makes Set fail.
type fakeKeySetter struct {
keys map[string]string
setErr error
}
func (f *fakeKeySetter) Set(provider, key string) error {
if f.setErr != nil {
return f.setErr
}
f.keys[provider] = key
return nil
}
func TestApplyStoredAPIKey(t *testing.T) {
store := fakeKeyGetter{keys: map[string]string{"openai": "sk-stored"}}
// Stored marker + empty key => filled from the store.
got := ApplyStoredAPIKey(ProviderProfile{Name: "openai", APIKeyStored: true}, store)
if got.APIKey != "sk-stored" {
t.Fatalf("expected stored key to fill empty APIKey, got %q", got.APIKey)
}
// No APIKeyStored marker => store is NOT consulted (don't reactivate a stale key).
got = ApplyStoredAPIKey(ProviderProfile{Name: "openai"}, store)
if got.APIKey != "" {
t.Fatalf("expected no load without APIKeyStored, got %q", got.APIKey)
}
// Inline key present => store is NOT consulted (inline wins).
got = ApplyStoredAPIKey(ProviderProfile{Name: "openai", APIKeyStored: true, APIKey: "sk-inline"}, store)
if got.APIKey != "sk-inline" {
t.Fatalf("inline key must win, got %q", got.APIKey)
}
// Marker set but no stored key for this provider => unchanged (empty).
got = ApplyStoredAPIKey(ProviderProfile{Name: "anthropic", APIKeyStored: true}, store)
if got.APIKey != "" {
t.Fatalf("expected no key for unstored provider, got %q", got.APIKey)
}
// Nil store => unchanged.
got = ApplyStoredAPIKey(ProviderProfile{Name: "openai", APIKeyStored: true}, nil)
if got.APIKey != "" {
t.Fatalf("nil store must leave profile unchanged, got %q", got.APIKey)
}
// Empty name => unchanged (don't query the store).
got = ApplyStoredAPIKey(ProviderProfile{APIKeyStored: true}, store)
if got.APIKey != "" {
t.Fatalf("empty name must not be filled, got %q", got.APIKey)
}
}
func TestMigratePlaintextProviderKeys(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "config.json")
cfgJSON := `{"activeProvider":"openai","providers":[` +
`{"name":"openai","apiKey":"sk-PLAINTEXT","model":"gpt"},` +
`{"name":"local","baseURL":"http://localhost"},` +
`{"name":"acme","apiKeyEnv":"ACME_KEY"}]}`
if err := os.WriteFile(path, []byte(cfgJSON), 0o600); err != nil {
t.Fatal(err)
}
store := &fakeKeySetter{keys: map[string]string{}}
n, err := MigratePlaintextProviderKeys(path, store)
if err != nil || n != 1 {
t.Fatalf("migrate = %d,%v; want 1,nil", n, err)
}
if store.keys["openai"] != "sk-PLAINTEXT" {
t.Fatalf("key not moved to store: %v", store.keys)
}
raw, _ := os.ReadFile(path)
if strings.Contains(string(raw), "sk-PLAINTEXT") {
t.Fatalf("plaintext key still in config.json:\n%s", raw)
}
if !strings.Contains(string(raw), "apiKeyStored") {
t.Fatalf("apiKeyStored marker not written:\n%s", raw)
}
// Idempotent: a second run migrates nothing.
if n2, _ := MigratePlaintextProviderKeys(path, store); n2 != 0 {
t.Fatalf("second migrate = %d, want 0", n2)
}
}
func TestMigrateLeavesKeyWhenStoreSetFails(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "config.json")
if err := os.WriteFile(path, []byte(`{"providers":[{"name":"openai","apiKey":"sk-KEEP"}]}`), 0o600); err != nil {
t.Fatal(err)
}
store := &fakeKeySetter{keys: map[string]string{}, setErr: errors.New("keychain locked")}
n, err := MigratePlaintextProviderKeys(path, store)
if err != nil || n != 0 {
t.Fatalf("migrate with failing store = %d,%v; want 0,nil", n, err)
}
raw, _ := os.ReadFile(path)
if !strings.Contains(string(raw), "sk-KEEP") {
t.Fatalf("a failed Set must not strand the key; config.json:\n%s", raw)
}
}
func TestClearProviderKeyStored(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "config.json")
if err := os.WriteFile(path, []byte(`{"providers":[{"name":"openai","apiKeyStored":true},{"name":"other","apiKeyStored":true}]}`), 0o600); err != nil {
t.Fatal(err)
}
cleared, err := ClearProviderKeyStored(path, "openai")
if err != nil || !cleared {
t.Fatalf("clear = %v,%v; want true,nil", cleared, err)
}
raw, _ := os.ReadFile(path)
// openai's marker is gone; other's remains.
var cfg FileConfig
if err := json.Unmarshal(raw, &cfg); err != nil {
t.Fatal(err)
}
for _, p := range cfg.Providers {
if p.Name == "openai" && p.APIKeyStored {
t.Fatal("openai marker should be cleared")
}
if p.Name == "other" && !p.APIKeyStored {
t.Fatal("other marker should be untouched")
}
}
// Idempotent: clearing again reports no change.
if cleared, _ := ClearProviderKeyStored(path, "openai"); cleared {
t.Fatal("second clear should report no change")
}
// Unknown provider: no change.
if cleared, _ := ClearProviderKeyStored(path, "nope"); cleared {
t.Fatal("unknown provider should report no change")
}
}
func TestProviderProfileAPIKeyStoredRoundTrips(t *testing.T) {
// The apiKeyStored marker survives JSON decode (custom UnmarshalJSON).
var p ProviderProfile
if err := p.UnmarshalJSON([]byte(`{"name":"openai","apiKeyStored":true}`)); err != nil {
t.Fatal(err)
}
if !p.APIKeyStored {
t.Fatal("expected apiKeyStored=true to decode")
}
if p.APIKey != "" {
t.Fatalf("no inline key expected, got %q", p.APIKey)
}
}
// OAuthLoginCandidates always offers the profile name, adds the catalog ID as a
// fallback ONLY when the profile has no effective own credential, and dedupes
// case-sensitively (the OAuth store is a case-sensitive map).
func TestOAuthLoginCandidates(t *testing.T) {
cases := []struct {
name string
profile ProviderProfile
want []string
}{
{
name: "renamed keyless profile falls back to catalog id",
profile: ProviderProfile{Name: "codex", CatalogID: "chatgpt"},
want: []string{"codex", "chatgpt"},
},
{
name: "case-variant name keeps distinct catalog-id candidate",
profile: ProviderProfile{Name: "ChatGPT", CatalogID: "chatgpt"},
want: []string{"ChatGPT", "chatgpt"},
},
{
name: "exact-duplicate name and catalog id collapse",
profile: ProviderProfile{Name: "chatgpt", CatalogID: "chatgpt"},
want: []string{"chatgpt"},
},
{
// A configured key must block ALL candidates (name included): a login
// under the profile's own name would otherwise erase the key too.
name: "own inline key blocks every candidate",
profile: ProviderProfile{Name: "anthropic-work", CatalogID: "anthropic", APIKey: "sk-work"},
want: nil,
},
{
name: "own auth header blocks every candidate",
profile: ProviderProfile{Name: "acme", CatalogID: "anthropic", AuthHeaderValue: "Bearer x"},
want: nil,
},
{
// APIKeyStored means key-auth even if the key isn't loaded here (e.g. a
// transiently unreadable keyring): don't silently borrow an OAuth login.
name: "stored-key profile blocks every candidate",
profile: ProviderProfile{Name: "openai-stored", CatalogID: "openai", APIKeyStored: true},
want: nil,
},
{
// APIKeyEnv is NOT a configured credential for gating: the env var may be
// unset while the profile relies on an OAuth login, so candidates stand.
name: "env-only profile still yields candidates",
profile: ProviderProfile{Name: "xai", CatalogID: "xai", APIKeyEnv: "XAI_API_KEY"},
want: []string{"xai"},
},
{
name: "empty profile yields no candidates",
profile: ProviderProfile{},
want: nil,
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
got := c.profile.OAuthLoginCandidates()
if strings.Join(got, ",") != strings.Join(c.want, ",") {
t.Fatalf("OAuthLoginCandidates() = %#v, want %#v", got, c.want)
}
})
}
}