mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-08 01:09:46 -10:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
02313be3c8 | ||
|
|
10ef5f55d9 |
No files matched your search
+5
-2
@@ -43,8 +43,11 @@ func runACP(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int
|
||||
conn := acp.NewConn(deps.stdin, stdout)
|
||||
acp.NewAgent(conn, acp.Deps{
|
||||
ResolveConfig: deps.resolveConfig,
|
||||
NewProvider: deps.newProvider,
|
||||
RunAgent: agent.Run,
|
||||
// deps.newProvider is wrapped in fillAppDeps to apply the stored API key,
|
||||
// so ACP is authenticated for apiKeyStored profiles like every other
|
||||
// surface — no ACP-specific credential handling needed.
|
||||
NewProvider: deps.newProvider,
|
||||
RunAgent: agent.Run,
|
||||
// Build the SCOPED registry + sandbox engine per workspace, exactly like the
|
||||
// exec surface, so ACP shell/file tools are confined — never run unconfined.
|
||||
BuildWorkspace: func(workspaceRoot string, resolved config.ResolvedConfig) (*tools.Registry, *sandbox.Engine, error) {
|
||||
|
||||
+68
-16
@@ -43,12 +43,17 @@ import (
|
||||
var version = "dev"
|
||||
|
||||
type appDeps struct {
|
||||
getwd func() (string, error)
|
||||
stdin io.Reader
|
||||
userConfigPath func() (string, error)
|
||||
resolveConfig func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error)
|
||||
resolveMCPConfig func(workspaceRoot string) (config.MCPConfig, error)
|
||||
newProvider func(config.ProviderProfile) (zeroruntime.Provider, error)
|
||||
getwd func() (string, error)
|
||||
stdin io.Reader
|
||||
userConfigPath func() (string, error)
|
||||
resolveConfig func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error)
|
||||
resolveMCPConfig func(workspaceRoot string) (config.MCPConfig, error)
|
||||
newProvider func(config.ProviderProfile) (zeroruntime.Provider, error)
|
||||
// exportActiveProvider pins spawned children to the run's provider (production:
|
||||
// config.SetActiveProviderEnv, set in defaultAppDeps — deliberately NOT filled
|
||||
// by fillAppDeps, so tests never mutate the process environment unless they
|
||||
// inject it). nil ⇒ no export.
|
||||
exportActiveProvider func(providerName string)
|
||||
probeProviderHealth func(context.Context, providerhealth.Options) providerhealth.Result
|
||||
detectLocalRuntimes func(context.Context, provideronboarding.LocalDetectOptions) []provideronboarding.DetectedLocalRuntime
|
||||
newSessionStore func() *sessions.Store
|
||||
@@ -99,9 +104,10 @@ func Run(args []string, stdout io.Writer, stderr io.Writer) int {
|
||||
|
||||
func defaultAppDeps() appDeps {
|
||||
return appDeps{
|
||||
getwd: os.Getwd,
|
||||
stdin: os.Stdin,
|
||||
userConfigPath: config.DefaultUserConfigPath,
|
||||
getwd: os.Getwd,
|
||||
stdin: os.Stdin,
|
||||
userConfigPath: config.DefaultUserConfigPath,
|
||||
exportActiveProvider: config.SetActiveProviderEnv,
|
||||
resolveConfig: func(workspaceRoot string, overrides config.Overrides) (config.ResolvedConfig, error) {
|
||||
options, err := config.DefaultResolveOptions(workspaceRoot)
|
||||
if err != nil {
|
||||
@@ -118,9 +124,14 @@ func defaultAppDeps() appDeps {
|
||||
return config.ResolveMCP(options)
|
||||
},
|
||||
newProvider: func(profile config.ProviderProfile) (zeroruntime.Provider, error) {
|
||||
// Resolve the OAuth login ONCE: the bearer resolver and the login key it
|
||||
// bound must describe the same login (the key is passed on to the Codex
|
||||
// account-header resolver so it never re-selects independently).
|
||||
resolver, loginKey := oauthLoginForProfile(profile)
|
||||
return providers.New(profile, providers.Options{
|
||||
UserAgent: userAgent(),
|
||||
OAuthResolver: oauthResolverForProfile(profile),
|
||||
OAuthResolver: resolver,
|
||||
OAuthLoginKey: loginKey,
|
||||
})
|
||||
},
|
||||
probeProviderHealth: providerhealth.Probe,
|
||||
@@ -493,6 +504,17 @@ func fillAppDeps(deps appDeps) appDeps {
|
||||
if deps.now == nil {
|
||||
deps.now = defaults.now
|
||||
}
|
||||
// Wrap newProvider ONCE, after all defaults are filled, so every surface that
|
||||
// builds the runtime provider gets the credential-store key applied. The
|
||||
// resolver is pure and leaves apiKeyStored profiles keyless; an unwrapped
|
||||
// build sends unauthenticated requests. config.ApplyStoredAPIKey is a no-op
|
||||
// when the key is already set or the profile isn't stored-key, so this is
|
||||
// safe and idempotent for every profile kind and every injected newProvider.
|
||||
baseNewProvider := deps.newProvider
|
||||
userConfigPath := deps.userConfigPath
|
||||
deps.newProvider = func(profile config.ProviderProfile) (zeroruntime.Provider, error) {
|
||||
return baseNewProvider(applyStoredProviderKeyAt(profile, userConfigPath))
|
||||
}
|
||||
return deps
|
||||
}
|
||||
|
||||
@@ -720,20 +742,50 @@ func tuiSandboxSetupCommand(backend sandbox.Backend, deps appDeps) func(context.
|
||||
}
|
||||
}
|
||||
|
||||
// buildProvider constructs the run's provider at STARTUP — it is called only from
|
||||
// the two launch paths (interactive TUI and headless exec), never from mid-run
|
||||
// rebuilds (escalation, wizard, ACP go through deps.newProvider directly).
|
||||
func buildProvider(resolved config.ResolvedConfig, deps appDeps) (zeroruntime.Provider, error) {
|
||||
if !config.HasProviderProfile(resolved.Provider) {
|
||||
return nil, nil
|
||||
}
|
||||
// Load the API key from the encrypted credential store when it isn't already
|
||||
// resolved from inline config or an env var. This keeps the resolver pure (no
|
||||
// I/O) and is a no-op for env/inline-key providers and when nothing is stored.
|
||||
profile := resolved.Provider
|
||||
if path, perr := deps.userConfigPath(); perr == nil {
|
||||
// deps.newProvider is wrapped in fillAppDeps to apply the stored key, so this
|
||||
// (and every other newProvider caller) needs no per-site key handling.
|
||||
provider, err := deps.newProvider(resolved.Provider)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
// Pin spawned children (sub-agents / swarm members inherit the environment) to
|
||||
// THIS run's provider from launch, not only after an in-session switch. Without
|
||||
// the launch-time export a child re-resolves config.json at spawn time, so a
|
||||
// provider switch persisted by ANOTHER zero process mid-session would silently
|
||||
// move new children onto a different provider (and different credentials) than
|
||||
// the parent is running. Runtime switches (/model, /provider, wizard,
|
||||
// onboarding) re-export on commit, keeping the pin current. Injected via deps
|
||||
// (nil in tests) because it mutates the process environment.
|
||||
if deps.exportActiveProvider != nil {
|
||||
deps.exportActiveProvider(resolved.Provider.Name)
|
||||
}
|
||||
return provider, nil
|
||||
}
|
||||
|
||||
// applyStoredProviderKeyAt loads the API key from the encrypted credential store
|
||||
// when the resolver left it empty (the resolver is pure — no I/O — so an
|
||||
// apiKeyStored profile reaches this layer keyless). No-op for inline/env-key
|
||||
// providers and when nothing is stored. This is applied once, centrally, by the
|
||||
// deps.newProvider wrapper in fillAppDeps so EVERY surface that builds a runtime
|
||||
// provider (headless exec, the ACP builder, exec's mid-run escalation switcher,
|
||||
// and any future caller) is covered without a per-site invariant to forget.
|
||||
func applyStoredProviderKeyAt(profile config.ProviderProfile, userConfigPath func() (string, error)) config.ProviderProfile {
|
||||
if userConfigPath == nil {
|
||||
return profile
|
||||
}
|
||||
if path, perr := userConfigPath(); perr == nil {
|
||||
if store, err := config.ProviderKeyStoreAt(filepath.Dir(path)); err == nil {
|
||||
profile = config.ApplyStoredAPIKey(profile, store)
|
||||
}
|
||||
}
|
||||
return deps.newProvider(profile)
|
||||
return profile
|
||||
}
|
||||
|
||||
func newCoreRegistry(workspaceRoot string) *tools.Registry {
|
||||
|
||||
@@ -0,0 +1,180 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"errors"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/config"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
// seedStoredProviderKey writes key into a credential store rooted beside the
|
||||
// returned config path, and returns deps fields wired at that path — the same
|
||||
// layout applyStoredProviderKey resolves through in production.
|
||||
func seedStoredProviderKey(t *testing.T, providerName, key string) (userConfigPath string) {
|
||||
t.Helper()
|
||||
t.Setenv("ZERO_CRED_STORAGE", "encrypted-file") // never touch the real OS keychain in tests
|
||||
dir := t.TempDir()
|
||||
store, err := config.ProviderKeyStoreAt(dir)
|
||||
if err != nil {
|
||||
t.Fatalf("ProviderKeyStoreAt: %v", err)
|
||||
}
|
||||
if err := store.Set(providerName, key); err != nil {
|
||||
t.Fatalf("seed stored key: %v", err)
|
||||
}
|
||||
return filepath.Join(dir, "config.json")
|
||||
}
|
||||
|
||||
// applyStoredProviderKeyAt must fill APIKey from the credential store for an
|
||||
// apiKeyStored profile — the single central helper the newProvider wrapper uses.
|
||||
func TestApplyStoredProviderKeyAtFillsFromCredstore(t *testing.T) {
|
||||
configPath := seedStoredProviderKey(t, "echo", "sk-stored-test")
|
||||
ucp := func() (string, error) { return configPath, nil }
|
||||
|
||||
stored := applyStoredProviderKeyAt(config.ProviderProfile{Name: "echo", APIKeyStored: true}, ucp)
|
||||
if stored.APIKey != "sk-stored-test" {
|
||||
t.Fatalf("APIKey = %q, want the stored key", stored.APIKey)
|
||||
}
|
||||
|
||||
// A profile that did NOT opt into stored-key auth must be left unchanged —
|
||||
// a stale store entry must not silently reactivate credentials.
|
||||
plain := applyStoredProviderKeyAt(config.ProviderProfile{Name: "echo"}, ucp)
|
||||
if plain.APIKey != "" {
|
||||
t.Fatalf("non-stored profile APIKey = %q, want empty", plain.APIKey)
|
||||
}
|
||||
}
|
||||
|
||||
// fillAppDeps wraps newProvider so EVERY surface that builds a runtime provider
|
||||
// (buildProvider, the ACP builder — which is just deps.newProvider — and exec's
|
||||
// escalation switcher) gets the stored key applied. Passing the raw resolved
|
||||
// profile previously sent unauthenticated requests for apiKeyStored profiles,
|
||||
// the default onboarding outcome. Testing the wrap covers all those surfaces.
|
||||
func TestFillAppDepsWrapsNewProviderWithStoredKey(t *testing.T) {
|
||||
configPath := seedStoredProviderKey(t, "echo", "sk-stored-wrap")
|
||||
|
||||
var captured config.ProviderProfile
|
||||
deps := fillAppDeps(appDeps{
|
||||
userConfigPath: func() (string, error) { return configPath, nil },
|
||||
newProvider: func(profile config.ProviderProfile) (zeroruntime.Provider, error) {
|
||||
captured = profile
|
||||
return nil, nil // only the captured profile matters here
|
||||
},
|
||||
})
|
||||
|
||||
if _, err := deps.newProvider(config.ProviderProfile{Name: "echo", APIKeyStored: true}); err != nil {
|
||||
t.Fatalf("wrapped newProvider: %v", err)
|
||||
}
|
||||
if captured.APIKey != "sk-stored-wrap" {
|
||||
t.Fatalf("wrapped newProvider built with APIKey = %q, want the stored key (unauthenticated regression)", captured.APIKey)
|
||||
}
|
||||
}
|
||||
|
||||
// buildProvider (the TUI/exec STARTUP construction site) must export
|
||||
// ZERO_PROVIDER so children spawned at any point in the run are pinned to the
|
||||
// parent's provider from launch — not only after an in-session switch. Without
|
||||
// this, a provider switch persisted by another zero process mid-session moves
|
||||
// new children onto a different provider (and credentials) than the parent.
|
||||
func TestBuildProviderExportsActiveProviderEnv(t *testing.T) {
|
||||
t.Setenv(config.ActiveProviderEnv, "stale-from-elsewhere")
|
||||
// The exporter is injected (defaultAppDeps wires config.SetActiveProviderEnv;
|
||||
// fillAppDeps deliberately leaves it nil so ordinary tests never mutate the
|
||||
// process env). Inject the real one here to assert the actual env effect.
|
||||
deps := appDeps{
|
||||
exportActiveProvider: config.SetActiveProviderEnv,
|
||||
newProvider: func(config.ProviderProfile) (zeroruntime.Provider, error) {
|
||||
return nil, nil
|
||||
},
|
||||
}
|
||||
|
||||
resolved := config.ResolvedConfig{Provider: config.ProviderProfile{Name: "pinned", ProviderKind: config.ProviderKindAnthropic, Model: "m"}}
|
||||
if _, err := buildProvider(resolved, deps); err != nil {
|
||||
t.Fatalf("buildProvider: %v", err)
|
||||
}
|
||||
if got := os.Getenv(config.ActiveProviderEnv); got != "pinned" {
|
||||
t.Fatalf("%s = %q after startup build, want %q (children would spawn on the stale provider)", config.ActiveProviderEnv, got, "pinned")
|
||||
}
|
||||
|
||||
// A FAILED build must not move the pin: the process never committed to the
|
||||
// new provider, so children must keep resolving whatever was in effect.
|
||||
t.Setenv(config.ActiveProviderEnv, "still-current")
|
||||
failing := appDeps{
|
||||
exportActiveProvider: config.SetActiveProviderEnv,
|
||||
newProvider: func(config.ProviderProfile) (zeroruntime.Provider, error) {
|
||||
return nil, errors.New("boom")
|
||||
},
|
||||
}
|
||||
if _, err := buildProvider(resolved, failing); err == nil {
|
||||
t.Fatal("expected build error")
|
||||
}
|
||||
if got := os.Getenv(config.ActiveProviderEnv); got != "still-current" {
|
||||
t.Fatalf("%s = %q after failed build, want it untouched", config.ActiveProviderEnv, got)
|
||||
}
|
||||
|
||||
// No provider profile at all → no provider, no pin change.
|
||||
if _, err := buildProvider(config.ResolvedConfig{}, deps); err != nil {
|
||||
t.Fatalf("buildProvider(empty): %v", err)
|
||||
}
|
||||
if got := os.Getenv(config.ActiveProviderEnv); got != "still-current" {
|
||||
t.Fatalf("%s = %q after profile-less build, want it untouched", config.ActiveProviderEnv, got)
|
||||
}
|
||||
}
|
||||
|
||||
// A mid-run --allow-escalation model switch previously rebuilt the provider
|
||||
// from the PURE resolved profile (the stored key lives only in buildProvider's
|
||||
// local copy), so the escalated provider was keyless and the next completion
|
||||
// call 401'd mid-run. Every profile handed to newProvider on this path must
|
||||
// carry the stored key.
|
||||
func TestRunExecEscalationSwitchKeepsStoredKey(t *testing.T) {
|
||||
dataHome := t.TempDir()
|
||||
t.Setenv("XDG_DATA_HOME", dataHome)
|
||||
cwd := t.TempDir()
|
||||
|
||||
if _, ok := mustUpgradeTarget(t, "claude-haiku-4.5"); !ok {
|
||||
t.Skip("registry has no upgrade target for claude-haiku-4.5")
|
||||
}
|
||||
|
||||
configPath := seedStoredProviderKey(t, "echo", "sk-stored-escalation")
|
||||
|
||||
var builtProfiles []config.ProviderProfile
|
||||
var stdout, stderr bytes.Buffer
|
||||
exitCode := runWithDeps([]string{
|
||||
"exec",
|
||||
"--allow-escalation",
|
||||
"--model", "claude-haiku-4.5",
|
||||
"--init-session-id", "escalation_stored_key_run",
|
||||
"escalate please",
|
||||
}, &stdout, &stderr, appDeps{
|
||||
getwd: func() (string, error) { return cwd, nil },
|
||||
userConfigPath: func() (string, error) { return configPath, nil },
|
||||
resolveConfig: func(_ string, overrides config.Overrides) (config.ResolvedConfig, error) {
|
||||
model := "claude-haiku-4.5"
|
||||
if overrides.Provider.Model != "" {
|
||||
model = overrides.Provider.Model
|
||||
}
|
||||
cfg := execResolvedConfig()
|
||||
cfg.Provider.ProviderKind = config.ProviderKindAnthropic
|
||||
cfg.Provider.Model = model
|
||||
cfg.Provider.APIKeyStored = true // key lives ONLY in the credstore
|
||||
cfg.MaxTurns = 3
|
||||
return cfg, nil
|
||||
},
|
||||
newProvider: func(profile config.ProviderProfile) (zeroruntime.Provider, error) {
|
||||
builtProfiles = append(builtProfiles, profile)
|
||||
return &usageEmittingEscalatingProvider{escalate: len(builtProfiles) == 1}, nil
|
||||
},
|
||||
})
|
||||
if exitCode != exitSuccess {
|
||||
t.Fatalf("exit = %d, want success; stderr=%s", exitCode, stderr.String())
|
||||
}
|
||||
if len(builtProfiles) < 2 {
|
||||
t.Fatalf("want at least 2 provider builds (initial + escalated), got %d", len(builtProfiles))
|
||||
}
|
||||
for i, profile := range builtProfiles {
|
||||
if profile.APIKey != "sk-stored-escalation" {
|
||||
t.Fatalf("provider build %d APIKey = %q, want the stored key (escalated build went out keyless)", i, profile.APIKey)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -365,6 +365,9 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in
|
||||
var modelSwitcher func(context.Context, string) (agent.Provider, error)
|
||||
if options.allowEscalation {
|
||||
modelSwitcher = func(_ context.Context, modelID string) (agent.Provider, error) {
|
||||
// deps.newProvider is wrapped (fillAppDeps) to apply the stored key, so
|
||||
// the escalated provider is authenticated even though resolved.Provider
|
||||
// is the pure profile — no per-site key handling here.
|
||||
switchedProfile := resolved.Provider
|
||||
switchedProfile.Model = modelID
|
||||
switchedProvider, err := deps.newProvider(switchedProfile)
|
||||
|
||||
@@ -24,6 +24,7 @@ func TestProviderHasOAuthLogin(t *testing.T) {
|
||||
|
||||
func TestSetupRequiredRecognizesOAuthLogin(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "tok.json")
|
||||
t.Setenv("ZERO_OAUTH_STORAGE", "file") // an inherited "keyring" would ignore the temp path and hit the OS keychain
|
||||
t.Setenv("ZERO_OAUTH_TOKENS_PATH", path)
|
||||
store, err := oauth.NewStore(oauth.StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
@@ -45,3 +46,88 @@ func TestSetupRequiredRecognizesOAuthLogin(t *testing.T) {
|
||||
t.Fatal("a keyless provider with no OAuth login must require onboarding")
|
||||
}
|
||||
}
|
||||
|
||||
// A profile renamed away from its catalog ID must still get a runtime OAuth
|
||||
// resolver when the login is stored under the catalog name — and a CASE-VARIANT
|
||||
// profile name must not drop the catalog-ID candidate (the store is
|
||||
// case-sensitive; a case-insensitive dedupe would swallow it).
|
||||
func TestOAuthResolverForProfileFallsBackToCatalogID(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "tok.json")
|
||||
t.Setenv("ZERO_OAUTH_STORAGE", "file") // an inherited "keyring" would ignore the temp path and hit the OS keychain
|
||||
t.Setenv("ZERO_OAUTH_TOKENS_PATH", path)
|
||||
store, err := oauth.NewStore(oauth.StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewStore: %v", err)
|
||||
}
|
||||
if err := store.Save(oauth.ProviderKey("chatgpt"), oauth.Token{AccessToken: "tok", ExpiresAt: time.Now().Add(time.Hour)}); err != nil {
|
||||
t.Fatalf("seed token: %v", err)
|
||||
}
|
||||
|
||||
// A renamed profile resolves the catalog-ID login, and the returned key MUST be
|
||||
// that login's key — the same key the Codex account-header resolver reads from,
|
||||
// so bearer and account can never come from different logins.
|
||||
if resolver, key := oauthLoginForProfile(config.ProviderProfile{Name: "codex", CatalogID: "chatgpt"}); resolver == nil {
|
||||
t.Fatal("renamed profile with a catalog-ID login must get a resolver (unauthenticated-children regression)")
|
||||
} else if key != oauth.ProviderKey("chatgpt") {
|
||||
t.Fatalf("bound key = %q, want the chatgpt login key (bearer/account must share it)", key)
|
||||
}
|
||||
// Name differs from catalog ID only in CASE: the exact-case store key is
|
||||
// "provider:chatgpt", so "chatgpt" must survive as a distinct candidate.
|
||||
if resolver, key := oauthLoginForProfile(config.ProviderProfile{Name: "ChatGPT", CatalogID: "chatgpt"}); resolver == nil {
|
||||
t.Fatal("case-variant profile name must still resolve the catalog-ID login (case-insensitive dedupe regression)")
|
||||
} else if key != oauth.ProviderKey("chatgpt") {
|
||||
t.Fatalf("case-variant bound key = %q, want the chatgpt login key", key)
|
||||
}
|
||||
if resolver, _ := oauthLoginForProfile(config.ProviderProfile{Name: "chatgpt", CatalogID: "chatgpt"}); resolver == nil {
|
||||
t.Fatal("catalog-named profile must get a resolver")
|
||||
}
|
||||
if resolver, key := oauthLoginForProfile(config.ProviderProfile{Name: "openai", CatalogID: "openai"}); resolver != nil || key != "" {
|
||||
t.Fatalf("a profile with no stored login must get no resolver and an empty key, got key=%q", key)
|
||||
}
|
||||
}
|
||||
|
||||
// The catalog-ID fallback must NOT reach a sibling profile's OAuth login when
|
||||
// this profile has its own key — withBearer would erase the key and silently
|
||||
// bill the model call under the sibling's account (a real cross-profile
|
||||
// override the fallback introduced). A profile WITH its own resolved key gets no
|
||||
// resolver even when a catalog-shared login exists.
|
||||
func TestOAuthResolverForProfileDoesNotOverrideExplicitKey(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "tok.json")
|
||||
t.Setenv("ZERO_OAUTH_STORAGE", "file") // an inherited "keyring" would ignore the temp path and hit the OS keychain
|
||||
t.Setenv("ZERO_OAUTH_TOKENS_PATH", path)
|
||||
store, err := oauth.NewStore(oauth.StoreOptions{FilePath: path})
|
||||
if err != nil {
|
||||
t.Fatalf("NewStore: %v", err)
|
||||
}
|
||||
// A personal login stored under the catalog name.
|
||||
if err := store.Save(oauth.ProviderKey("anthropic"), oauth.Token{AccessToken: "personal-tok", ExpiresAt: time.Now().Add(time.Hour)}); err != nil {
|
||||
t.Fatalf("seed token: %v", err)
|
||||
}
|
||||
|
||||
// Work profile: same catalog, its OWN resolved API key, deliberately NOT the
|
||||
// personal OAuth account. It must keep using its key (no resolver attached).
|
||||
work := config.ProviderProfile{Name: "anthropic-work", CatalogID: "anthropic", APIKey: "sk-work-key"}
|
||||
if resolver, key := oauthLoginForProfile(work); resolver != nil || key != "" {
|
||||
t.Fatalf("a profile with its own API key must not get a catalog-shared OAuth resolver (cross-profile override), got key=%q", key)
|
||||
}
|
||||
// A raw auth-header credential must be protected the same way.
|
||||
header := config.ProviderProfile{Name: "anthropic-hdr", CatalogID: "anthropic", AuthHeaderValue: "Bearer byo"}
|
||||
if resolver, _ := oauthLoginForProfile(header); resolver != nil {
|
||||
t.Fatal("a profile with its own auth header must not get a catalog-shared OAuth resolver")
|
||||
}
|
||||
// A same-NAME login must not override an explicit key either (the name
|
||||
// candidate is gated on the profile being keyless, not just the catalog one).
|
||||
if err := store.Save(oauth.ProviderKey("anthropic-work"), oauth.Token{AccessToken: "name-tok", ExpiresAt: time.Now().Add(time.Hour)}); err != nil {
|
||||
t.Fatalf("seed name token: %v", err)
|
||||
}
|
||||
if resolver, _ := oauthLoginForProfile(work); resolver != nil {
|
||||
t.Fatal("a profile with its own API key must not get an OAuth resolver even from a same-name login")
|
||||
}
|
||||
// Sanity: a keyless sibling on the same catalog DOES resolve the shared login.
|
||||
keyless := config.ProviderProfile{Name: "anthropic-oauth", CatalogID: "anthropic"}
|
||||
if resolver, key := oauthLoginForProfile(keyless); resolver == nil {
|
||||
t.Fatal("a keyless profile should still resolve the catalog-shared login")
|
||||
} else if key != oauth.ProviderKey("anthropic") {
|
||||
t.Fatalf("keyless sibling bound key = %q, want the catalog login key", key)
|
||||
}
|
||||
}
|
||||
@@ -4,7 +4,6 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/Gitlawb/zero/internal/config"
|
||||
@@ -12,33 +11,36 @@ import (
|
||||
"github.com/Gitlawb/zero/internal/providers/providerio"
|
||||
)
|
||||
|
||||
// providerOAuthName resolves the OAuth login name for a provider profile. It is
|
||||
// the name a user logs in under (`zero auth login <name>`) whose token then
|
||||
// authenticates this provider's model calls. For now it is the profile name;
|
||||
// the provider presets map well-known kinds (openai/anthropic) to canonical
|
||||
// names on top of this.
|
||||
func providerOAuthName(profile config.ProviderProfile) string {
|
||||
return strings.TrimSpace(profile.Name)
|
||||
}
|
||||
|
||||
// oauthResolverForProfile returns a TokenResolver that authenticates a provider's
|
||||
// model calls with the user's OAuth login, or nil when no login exists for this
|
||||
// provider. Returning nil for the no-login case keeps API-key users free of any
|
||||
// per-request store lookups: the resolver is only attached once a login is
|
||||
// present at construction time.
|
||||
func oauthResolverForProfile(profile config.ProviderProfile) providerio.TokenResolver {
|
||||
name := providerOAuthName(profile)
|
||||
if name == "" {
|
||||
return nil
|
||||
// oauthLoginForProfile resolves the user's OAuth login for a provider ONCE and
|
||||
// returns both a TokenResolver that authenticates model calls with it and the
|
||||
// credential-store key it bound to. It returns (nil, "") when no login exists —
|
||||
// keeping API-key users free of any per-request store lookups, since the resolver
|
||||
// is only attached when a login is present at construction time.
|
||||
//
|
||||
// The returned key is the single source of truth for "which login is this
|
||||
// provider using": callers pass it to providers.Options.OAuthLoginKey so the
|
||||
// Codex chatgpt-account-id header reads its account from the exact login that
|
||||
// issued the bearer token, instead of doing a second, independent lookup that
|
||||
// could select a different login (a backend-rejected mismatch).
|
||||
//
|
||||
// Candidate login names (profile name, then a catalog-ID fallback, both gated on
|
||||
// the profile having no own configured credential) come from the shared
|
||||
// ProviderProfile.OAuthLoginCandidates so the runtime resolver, the Codex account
|
||||
// resolver, and the onboarding presence check never diverge.
|
||||
func oauthLoginForProfile(profile config.ProviderProfile) (providerio.TokenResolver, string) {
|
||||
candidates := profile.OAuthLoginCandidates()
|
||||
if len(candidates) == 0 {
|
||||
return nil, ""
|
||||
}
|
||||
store, err := oauth.NewStore(oauth.StoreOptions{})
|
||||
if err != nil {
|
||||
return nil
|
||||
return nil, ""
|
||||
}
|
||||
key := oauth.ProviderKey(name)
|
||||
if _, ok, loadErr := store.Load(key); loadErr != nil || !ok {
|
||||
// No login (or an unreadable/invalid key) → API-key auth, no resolver.
|
||||
return nil
|
||||
_, key, ok := oauth.FirstStored(store, candidates)
|
||||
if !ok {
|
||||
// No login under any candidate (or unreadable/invalid keys) → API-key
|
||||
// auth, no resolver.
|
||||
return nil, ""
|
||||
}
|
||||
manager, err := oauth.NewManager(oauth.ManagerOptions{
|
||||
Store: store,
|
||||
@@ -48,9 +50,9 @@ func oauthResolverForProfile(profile config.ProviderProfile) providerio.TokenRes
|
||||
AllowPresets: true,
|
||||
})
|
||||
if err != nil {
|
||||
return nil
|
||||
return nil, ""
|
||||
}
|
||||
return func(ctx context.Context, forceRefresh bool) (string, string, bool, error) {
|
||||
resolver := func(ctx context.Context, forceRefresh bool) (string, string, bool, error) {
|
||||
var token string
|
||||
var rerr error
|
||||
if forceRefresh {
|
||||
@@ -67,4 +69,5 @@ func oauthResolverForProfile(profile config.ProviderProfile) providerio.TokenRes
|
||||
}
|
||||
return "Authorization", "Bearer " + token, true, nil
|
||||
}
|
||||
return resolver, key
|
||||
}
|
||||
@@ -488,7 +488,7 @@ func validateProviderRuntimeReady(profile config.ProviderProfile) error {
|
||||
}
|
||||
|
||||
func providerProfileHasCredential(profile config.ProviderProfile) bool {
|
||||
return strings.TrimSpace(profile.APIKey) != "" || profile.APIKeyStored || strings.TrimSpace(profile.AuthHeaderValue) != ""
|
||||
return profile.HasConfiguredCredential()
|
||||
}
|
||||
|
||||
func providerKindForDescriptor(descriptor providercatalog.Descriptor) config.ProviderKind {
|
||||
|
||||
@@ -314,10 +314,14 @@ func setupRequired(resolved config.ResolvedConfig) bool {
|
||||
}
|
||||
|
||||
// providerHasOAuthLogin reports whether a stored OAuth login exists for the
|
||||
// provider, keyed by its profile name or catalog id.
|
||||
// provider. It uses the same ProviderProfile.OAuthLoginCandidates the runtime
|
||||
// resolver does, so the "authenticated in the UI" gate and the "authenticated at
|
||||
// runtime" resolver can never diverge. This is only consulted for a profile with
|
||||
// no usable API-key credential (setupRequired returns early otherwise), where
|
||||
// the candidate set is permissive (profile name + catalog ID).
|
||||
func providerHasOAuthLogin(profile config.ProviderProfile, oauthLogins map[string]bool) bool {
|
||||
for _, name := range []string{strings.TrimSpace(profile.Name), strings.TrimSpace(profile.CatalogID)} {
|
||||
if name != "" && oauthLogins[name] {
|
||||
for _, name := range profile.OAuthLoginCandidates() {
|
||||
if oauthLogins[name] {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
@@ -155,6 +155,63 @@ func MigratePlaintextProviderKeys(path string, store APIKeySetter) (int, error)
|
||||
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
|
||||
|
||||
@@ -172,3 +172,69 @@ func TestProviderProfileAPIKeyStoredRoundTrips(t *testing.T) {
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
+39
-21
@@ -1,11 +1,11 @@
|
||||
package mcp
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
@@ -393,28 +393,43 @@ func TestConnectRejectsUnsupportedTransport(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestClientRequestWaitsForMatchingResponseID(t *testing.T) {
|
||||
var incoming bytes.Buffer
|
||||
incomingWriter := newMessageWriter(&incoming)
|
||||
if err := incomingWriter.write(rpcMessage{Method: "notifications/progress"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := incomingWriter.write(rpcMessage{
|
||||
ID: 99,
|
||||
Error: &rpcError{Code: -32000, Message: "wrong response"},
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := incomingWriter.write(rpcMessage{
|
||||
ID: 1,
|
||||
Result: mustRaw(map[string]any{"value": "matched"}),
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Pipes, not a pre-filled bytes.Buffer: the fake server responds only AFTER
|
||||
// reading the outgoing request, matching production ordering. With responses
|
||||
// pre-loaded, the client's reader goroutine could consume the matching
|
||||
// response — and hit EOF — before request() registered its pending id,
|
||||
// flaking as "request() error = EOF" on fast runners.
|
||||
serverReader, clientWriter := io.Pipe() // client → server
|
||||
clientReader, serverWriter := io.Pipe() // server → client
|
||||
t.Cleanup(func() {
|
||||
_ = clientWriter.Close()
|
||||
_ = serverWriter.Close()
|
||||
})
|
||||
|
||||
serverErr := make(chan error, 1)
|
||||
go func() {
|
||||
requests := newMessageReader(serverReader)
|
||||
request, err := requests.read()
|
||||
if err != nil {
|
||||
serverErr <- err
|
||||
return
|
||||
}
|
||||
responses := newMessageWriter(serverWriter)
|
||||
for _, message := range []rpcMessage{
|
||||
{Method: "notifications/progress"},
|
||||
{ID: 99, Error: &rpcError{Code: -32000, Message: "wrong response"}},
|
||||
{ID: request.ID, Result: mustRaw(map[string]any{"value": "matched"})},
|
||||
} {
|
||||
if err := responses.write(message); err != nil {
|
||||
serverErr <- err
|
||||
return
|
||||
}
|
||||
}
|
||||
serverErr <- nil
|
||||
}()
|
||||
|
||||
var outgoing bytes.Buffer
|
||||
client := &Client{
|
||||
reader: newMessageReader(&incoming),
|
||||
writer: newMessageWriter(&outgoing),
|
||||
reader: newMessageReader(clientReader),
|
||||
writer: newMessageWriter(clientWriter),
|
||||
nextID: 1,
|
||||
}
|
||||
var result struct {
|
||||
@@ -426,6 +441,9 @@ func TestClientRequestWaitsForMatchingResponseID(t *testing.T) {
|
||||
if result.Value != "matched" {
|
||||
t.Fatalf("result.Value = %q, want matched response", result.Value)
|
||||
}
|
||||
if err := <-serverErr; err != nil {
|
||||
t.Fatalf("fake server error: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
type errorCloser struct {
|
||||
|
||||
@@ -41,6 +41,26 @@ func ValidateKey(key string) error {
|
||||
// ProviderKey builds the store key for a provider login.
|
||||
func ProviderKey(name string) string { return KeyPrefixProvider + name }
|
||||
|
||||
// FirstStored returns the token and its ProviderKey for the FIRST candidate name
|
||||
// that has a token in the store, with ok=false when none do. Callers pass
|
||||
// ProviderProfile.OAuthLoginCandidates() so that everything derived from a login
|
||||
// — the bearer token AND any header claim like chatgpt-account-id — comes from
|
||||
// the SAME login; selecting independently per consumer could otherwise pair a
|
||||
// bearer from one login with an account header from another. A load error on a
|
||||
// candidate is treated as a miss (skip to the next), never a hard failure.
|
||||
func FirstStored(store *Store, candidates []string) (Token, string, bool) {
|
||||
if store == nil {
|
||||
return Token{}, "", false
|
||||
}
|
||||
for _, name := range candidates {
|
||||
key := ProviderKey(name)
|
||||
if token, ok, err := store.Load(key); err == nil && ok {
|
||||
return token, key, true
|
||||
}
|
||||
}
|
||||
return Token{}, "", false
|
||||
}
|
||||
|
||||
// Status is a redaction-safe summary of a stored token (no secret material).
|
||||
type Status struct {
|
||||
Key string `json:"key"`
|
||||
|
||||
@@ -26,6 +26,13 @@ type Options struct {
|
||||
// OAuth bearer token (preferred over the API key). nil => API-key auth only.
|
||||
// Applied to the OpenAI and Anthropic providers.
|
||||
OAuthResolver providerio.TokenResolver
|
||||
// OAuthLoginKey is the credential-store key OAuthResolver bound to (empty when
|
||||
// there is no OAuth login). It is the SAME selection the bearer resolver made,
|
||||
// resolved ONCE by the caller so the Codex chatgpt-account-id header reads its
|
||||
// account from the exact login that issued the bearer token — never a second,
|
||||
// independent lookup that could pick a different login (a backend-rejected
|
||||
// mismatch).
|
||||
OAuthLoginKey string
|
||||
}
|
||||
|
||||
// New creates a runtime provider for a resolved provider profile.
|
||||
@@ -251,14 +258,22 @@ func isCodexCatalog(profile config.ProviderProfile, _ resolvedProfile) bool {
|
||||
|
||||
// newCodexProvider builds a Codex-flavored openai provider for the chatgpt
|
||||
// catalog. The Codex headers (`originator`, `chatgpt-account-id`) are
|
||||
// injected by the CodexProvider wrapper. The `chatgpt-account-id` is
|
||||
// resolved dynamically from the stored OAuth token's Account field on
|
||||
// every request so a refresh that rotates the token (and its account
|
||||
// claim) takes effect immediately — a static AccountID captured at
|
||||
// construction would go stale on the first refresh.
|
||||
// injected by the CodexProvider wrapper. The `chatgpt-account-id` is read from
|
||||
// the stored OAuth token's Account field on every request so a refresh that
|
||||
// rotates the token (and its account claim) takes effect immediately — a static
|
||||
// AccountID captured at construction would go stale on the first refresh.
|
||||
//
|
||||
// The *login* the header reads from is options.OAuthLoginKey — the SAME key the
|
||||
// bearer resolver bound, resolved ONCE by the caller. The bearer resolver
|
||||
// refreshes the token in place under that key; reading the account from the same
|
||||
// key keeps header and token on one login for the provider's life. Resolving the
|
||||
// key independently here (a second oauth.FirstStored) could, after a transient
|
||||
// per-candidate load error or a mid-session `zero auth login`, pick a different
|
||||
// login than the bearer — a mismatch the backend rejects.
|
||||
func newCodexProvider(profile config.ProviderProfile, resolved resolvedProfile, options Options) (zeroruntime.Provider, error) {
|
||||
accountKey := options.OAuthLoginKey
|
||||
resolver := openai.CodexAccountResolver(func(ctx context.Context) (string, bool, error) {
|
||||
account := codexAccountFromStore(profile.Name)
|
||||
account := codexAccountForKey(accountKey)
|
||||
if account == "" {
|
||||
return "", false, nil
|
||||
}
|
||||
@@ -288,17 +303,20 @@ func newCodexProvider(profile config.ProviderProfile, resolved resolvedProfile,
|
||||
})
|
||||
}
|
||||
|
||||
// codexAccountFromStore reads the chatgpt_account_id from the stored OAuth
|
||||
// token for the given provider name. It is called per-request (via the
|
||||
// resolver) so a refresh that rotates the token takes effect immediately
|
||||
// without restarting the agent. Returns "" when no token is stored (the
|
||||
// Codex provider then omits the header and the user sees a clear 401).
|
||||
func codexAccountFromStore(providerName string) string {
|
||||
// codexAccountForKey reads the chatgpt_account_id from the token currently
|
||||
// stored under key. Called per-request (via the resolver) so a refresh that
|
||||
// rotates the token — saved back in place under the same key — takes effect
|
||||
// immediately without restarting the agent. Returns "" when key is empty (no
|
||||
// OAuth login), or the token is absent or carries no account claim.
|
||||
func codexAccountForKey(key string) string {
|
||||
if key == "" {
|
||||
return ""
|
||||
}
|
||||
store, err := oauth.NewStore(oauth.StoreOptions{})
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
token, ok, err := store.Load(oauth.ProviderKey(providerName))
|
||||
token, ok, err := store.Load(key)
|
||||
if err != nil || !ok {
|
||||
return ""
|
||||
}
|
||||
|
||||
@@ -269,6 +269,7 @@ func TestNewRoutesChatGPTCatalogToCodexProvider(t *testing.T) {
|
||||
// OAuth token and the "want empty chatgpt-account-id" assertion fails locally
|
||||
// (it still passes in CI, where no login is stored). Mirrors the isolation in
|
||||
// TestNewRoutesChatGPTCatalogWithStoredAccountID.
|
||||
t.Setenv("ZERO_OAUTH_STORAGE", "file")
|
||||
t.Setenv("ZERO_OAUTH_TOKENS_PATH", t.TempDir()+"/tokens.json")
|
||||
|
||||
transport := &captureTransport{
|
||||
@@ -322,11 +323,10 @@ func TestNewRoutesChatGPTCatalogToCodexProvider(t *testing.T) {
|
||||
|
||||
func TestNewRoutesChatGPTCatalogWithStoredAccountID(t *testing.T) {
|
||||
// The factory reads the stored OAuth token's Account field for the
|
||||
// chatgpt-account-id header. Seed a token in a temp store and point
|
||||
// ZERO_OAUTH_TOKENS_PATH at it so the factory picks it up.
|
||||
dir := t.TempDir()
|
||||
t.Setenv("ZERO_OAUTH_TOKENS_PATH", dir+"/tokens.json")
|
||||
store, err := newOAuthStoreForTest()
|
||||
// chatgpt-account-id header, from the login key the CALLER supplies in
|
||||
// Options.OAuthLoginKey (the same key the bearer resolver bound). Seed a
|
||||
// token in an isolated temp store, then pass that key.
|
||||
store, err := newOAuthStoreForTest(t)
|
||||
if err != nil {
|
||||
t.Fatalf("NewStore: %v", err)
|
||||
}
|
||||
@@ -344,8 +344,9 @@ func TestNewRoutesChatGPTCatalogWithStoredAccountID(t *testing.T) {
|
||||
CatalogID: "chatgpt",
|
||||
Model: "gpt-5",
|
||||
}, Options{
|
||||
HTTPClient: &http.Client{Transport: transport},
|
||||
UserAgent: "zero-factory-test",
|
||||
HTTPClient: &http.Client{Transport: transport},
|
||||
UserAgent: "zero-factory-test",
|
||||
OAuthLoginKey: oauth.ProviderKey("chatgpt"),
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("New() error = %v", err)
|
||||
@@ -407,10 +408,61 @@ func (transport *captureTransport) body() io.Reader {
|
||||
return strings.NewReader(transport.requestBody)
|
||||
}
|
||||
|
||||
// newOAuthStoreForTest builds a Store pointed at the current
|
||||
// ZERO_OAUTH_TOKENS_PATH (set by the caller). It exists so the chatgpt
|
||||
// factory tests can seed a token without copying the path-handling dance
|
||||
// from internal/cli.
|
||||
func newOAuthStoreForTest() (*oauth.Store, error) {
|
||||
// newOAuthStoreForTest pins the OAuth token store to a plain temp FILE and
|
||||
// returns a Store on it. Pinning ZERO_OAUTH_STORAGE matters as much as the
|
||||
// path: an inherited "keyring" value would send NewStore to the OS keychain
|
||||
// and ignore ZERO_OAUTH_TOKENS_PATH entirely, making the test read/write the
|
||||
// developer's real logins. Exists so the chatgpt factory tests can seed a
|
||||
// token without copying the path-handling dance from internal/cli.
|
||||
func newOAuthStoreForTest(t *testing.T) (*oauth.Store, error) {
|
||||
t.Helper()
|
||||
t.Setenv("ZERO_OAUTH_STORAGE", "file")
|
||||
t.Setenv("ZERO_OAUTH_TOKENS_PATH", t.TempDir()+"/tokens.json")
|
||||
return oauth.NewStore(oauth.StoreOptions{})
|
||||
}
|
||||
|
||||
// codexAccountForKey reads the chatgpt-account-id from the token stored under a
|
||||
// FIXED key — the key the caller (cli.oauthLoginForProfile) already bound for the
|
||||
// bearer token, passed through providers.Options.OAuthLoginKey. The account is
|
||||
// therefore always read from the same login that issued the bearer (no second,
|
||||
// independent selection). It re-reads per call, so an in-place token refresh (new
|
||||
// account claim under the SAME key) is picked up; an empty key (no OAuth login)
|
||||
// or a missing/account-less token yields "".
|
||||
func TestCodexAccountForKey(t *testing.T) {
|
||||
store, err := newOAuthStoreForTest(t)
|
||||
if err != nil {
|
||||
t.Fatalf("NewStore: %v", err)
|
||||
}
|
||||
if err := store.Save(oauth.ProviderKey("chatgpt"), oauth.Token{AccessToken: "tok-1", Account: "acc-catalog-7"}); err != nil {
|
||||
t.Fatalf("Save chatgpt: %v", err)
|
||||
}
|
||||
// A different login WITHOUT an account claim, to prove the read stays on the
|
||||
// requested key and never crosses to another login's account.
|
||||
if err := store.Save(oauth.ProviderKey("codex"), oauth.Token{AccessToken: "tok-codex"}); err != nil {
|
||||
t.Fatalf("Save codex: %v", err)
|
||||
}
|
||||
|
||||
if got := codexAccountForKey(oauth.ProviderKey("chatgpt")); got != "acc-catalog-7" {
|
||||
t.Fatalf("account = %q, want acc-catalog-7", got)
|
||||
}
|
||||
// The bound key's token has no account → "", NOT the other login's account.
|
||||
if got := codexAccountForKey(oauth.ProviderKey("codex")); got != "" {
|
||||
t.Fatalf("account = %q, want empty (must not cross to another login's account)", got)
|
||||
}
|
||||
// Empty key (no OAuth login) → header omitted.
|
||||
if got := codexAccountForKey(""); got != "" {
|
||||
t.Fatalf("account for empty key = %q, want empty", got)
|
||||
}
|
||||
// Unknown key → "".
|
||||
if got := codexAccountForKey(oauth.ProviderKey("nope")); got != "" {
|
||||
t.Fatalf("account for unknown key = %q, want empty", got)
|
||||
}
|
||||
|
||||
// An in-place refresh under the bound key IS reflected per request.
|
||||
if err := store.Save(oauth.ProviderKey("chatgpt"), oauth.Token{AccessToken: "tok-refreshed", Account: "acc-rotated"}); err != nil {
|
||||
t.Fatalf("refresh chatgpt: %v", err)
|
||||
}
|
||||
if got := codexAccountForKey(oauth.ProviderKey("chatgpt")); got != "acc-rotated" {
|
||||
t.Fatalf("account = %q, want acc-rotated (in-place refresh must be picked up)", got)
|
||||
}
|
||||
}
|
||||
@@ -467,7 +467,7 @@ func providerNextActionLines(profile config.ProviderProfile, snapshot zerocomman
|
||||
}
|
||||
|
||||
func providerProfileHasCredential(profile config.ProviderProfile) bool {
|
||||
return strings.TrimSpace(profile.APIKey) != "" || profile.APIKeyStored || strings.TrimSpace(profile.AuthHeaderValue) != ""
|
||||
return profile.HasConfiguredCredential()
|
||||
}
|
||||
|
||||
func providerCredentialRequired(profile config.ProviderProfile, providerKind string) bool {
|
||||
|
||||
@@ -755,6 +755,15 @@ func (m model) completeSetup() (tea.Model, tea.Cmd) {
|
||||
m.providerProfile = result.Provider
|
||||
m.providerName = result.Provider.Name
|
||||
m.modelName = result.Provider.Model
|
||||
// Export ZERO_PROVIDER alongside the committed profile fields (and the
|
||||
// config setupSave already persisted as active). Unlike command_center's
|
||||
// switch — which commits everything only after a successful build — setup
|
||||
// commits config + profile unconditionally here, so the env must match
|
||||
// them unconditionally too: applyEnv makes ZERO_PROVIDER WIN over config,
|
||||
// so a stale value from an earlier /model switch left in place would send
|
||||
// spawned children to the OLD provider even though config now names the
|
||||
// new one. That is the exact D3 gap this closes.
|
||||
config.SetActiveProviderEnv(result.Provider.Name)
|
||||
if m.newProvider != nil {
|
||||
if provider, providerErr := m.newProvider(result.Provider); providerErr == nil {
|
||||
m.provider = provider
|
||||
|
||||
@@ -3,6 +3,7 @@ package tui
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -1535,3 +1536,42 @@ func absInt(value int) int {
|
||||
}
|
||||
return value
|
||||
}
|
||||
|
||||
// Completing setup switches the live provider, so it must export ZERO_PROVIDER
|
||||
// exactly like the /model, /provider, and wizard switch paths — a stale value
|
||||
// from an earlier switch would otherwise win over config in every spawned
|
||||
// child (applyEnv) and pin specialists/swarm members to the OLD provider's
|
||||
// credentials.
|
||||
func TestCompleteSetupExportsActiveProviderEnv(t *testing.T) {
|
||||
t.Setenv(config.ActiveProviderEnv, "stale-previous-provider")
|
||||
m := newModel(context.Background(), Options{
|
||||
Setup: SetupOptions{
|
||||
Visible: true,
|
||||
Providers: []SetupProviderOption{
|
||||
{ID: "openai", Name: "OpenAI", DefaultModel: "gpt-4.1", EnvVar: "OPENAI_API_KEY", RequiresAuth: true},
|
||||
},
|
||||
Save: func(selection SetupSelection) (SetupResult, error) {
|
||||
return SetupResult{
|
||||
Provider: config.ProviderProfile{
|
||||
Name: selection.CatalogID,
|
||||
CatalogID: selection.CatalogID,
|
||||
Model: selection.Model,
|
||||
},
|
||||
}, nil
|
||||
},
|
||||
},
|
||||
})
|
||||
m.width = 100
|
||||
m.height = 30
|
||||
m.setup.stage = setupStageReady
|
||||
|
||||
updated, _ := m.completeSetup()
|
||||
next := updated.(model)
|
||||
|
||||
if next.providerName == "" {
|
||||
t.Fatal("setup completion should have set a provider name")
|
||||
}
|
||||
if got := os.Getenv(config.ActiveProviderEnv); got != next.providerName {
|
||||
t.Fatalf("%s = %q after setup save, want %q (children would spawn on the stale provider)", config.ActiveProviderEnv, got, next.providerName)
|
||||
}
|
||||
}
|
||||
@@ -20,6 +20,7 @@ import (
|
||||
"github.com/Gitlawb/zero/internal/providercatalog"
|
||||
"github.com/Gitlawb/zero/internal/provideroauth"
|
||||
"github.com/Gitlawb/zero/internal/redaction"
|
||||
"github.com/Gitlawb/zero/internal/zeroruntime"
|
||||
)
|
||||
|
||||
// providerWizardOAuthMsg carries the result of an in-wizard browser OAuth login.
|
||||
@@ -887,13 +888,20 @@ func (m model) applyProviderWizard() (model, tea.Cmd) {
|
||||
modelChoice := wizard.currentModel()
|
||||
profile := providerWizardProfile(provider, modelChoice.ID, wizard.apiKey, wizard.baseURL, wizard.profileName)
|
||||
runtimeProfile := providerWizardRuntimeProfile(profile)
|
||||
|
||||
// Build and persist into LOCALS first, committing live state only once BOTH
|
||||
// succeed. A persist failure (read-only config, disk full) must not leave the
|
||||
// chat running on the new provider while the status line, m.providerProfile,
|
||||
// and the ZERO_PROVIDER export (which pins spawned children) still point at
|
||||
// the old one — parent and children would silently run on different providers.
|
||||
var nextProvider zeroruntime.Provider
|
||||
if m.newProvider != nil {
|
||||
nextProvider, err := m.newProvider(runtimeProfile)
|
||||
built, err := m.newProvider(runtimeProfile)
|
||||
if err != nil {
|
||||
wizard.err = redaction.RedactString(err.Error(), redaction.Options{ExtraSecretValues: []string{profile.APIKey, runtimeProfile.APIKey}})
|
||||
return m, nil
|
||||
}
|
||||
m.provider = nextProvider
|
||||
nextProvider = built
|
||||
}
|
||||
if strings.TrimSpace(m.userConfigPath) != "" {
|
||||
// Capture flip: move the freshly entered key into the encrypted credential
|
||||
@@ -903,12 +911,24 @@ func (m model) applyProviderWizard() (model, tea.Cmd) {
|
||||
profile = config.SecureProviderProfile(profile, m.userConfigPath)
|
||||
if _, err := config.UpsertProvider(m.userConfigPath, profile, true); err != nil {
|
||||
wizard.err = redaction.RedactString(err.Error(), redaction.Options{ExtraSecretValues: []string{secret, profile.APIKey}})
|
||||
return m, nil
|
||||
return m, nil // nothing committed to live state yet
|
||||
}
|
||||
}
|
||||
|
||||
// Both succeeded — commit the live provider, profile, model, and the child
|
||||
// env export together, so they can never disagree.
|
||||
if nextProvider != nil {
|
||||
m.provider = nextProvider
|
||||
}
|
||||
m.providerProfile = profile
|
||||
m.providerName = profile.Name
|
||||
m.modelName = profile.Model
|
||||
// Keep sub-agent child processes on the same provider we just switched to —
|
||||
// same as the /model and /provider switch paths (command_center.go). Without
|
||||
// this, a ZERO_PROVIDER exported by an earlier switch stays pointing at the
|
||||
// OLD provider and wins over config in every spawned child (applyEnv), so
|
||||
// specialists/swarm members run on the wrong provider's credentials.
|
||||
config.SetActiveProviderEnv(profile.Name)
|
||||
m.providerWizard = nil
|
||||
return m, nil
|
||||
}
|
||||
|
||||
@@ -1189,3 +1189,104 @@ func containsString(values []string, want string) bool {
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// Applying the wizard switches the live provider, so it must export
|
||||
// ZERO_PROVIDER exactly like the /model and /provider switch paths — a stale
|
||||
// value from an earlier switch would otherwise win over config in every
|
||||
// spawned child (applyEnv) and pin specialists/swarm members to the OLD
|
||||
// provider's credentials.
|
||||
func TestApplyProviderWizardExportsActiveProviderEnv(t *testing.T) {
|
||||
t.Setenv(config.ActiveProviderEnv, "stale-previous-provider")
|
||||
m := newModel(context.Background(), Options{})
|
||||
// Isolate the SUCCESS path's persist to a temp config (the default empty
|
||||
// path would skip persist; a future default must never reach the real user
|
||||
// config), and stub the build so the full commit sequence runs.
|
||||
m.userConfigPath = filepath.Join(t.TempDir(), "config.json")
|
||||
m.newProvider = func(config.ProviderProfile) (zeroruntime.Provider, error) {
|
||||
return &fakeProvider{}, nil
|
||||
}
|
||||
m.providerWizard = &providerWizardState{
|
||||
step: providerWizardStepModel,
|
||||
profileName: "acme-wizard",
|
||||
providers: []providercatalog.Descriptor{{
|
||||
ID: "openrouter",
|
||||
Name: "OpenRouter",
|
||||
Transport: providercatalog.TransportOpenAICompatible,
|
||||
DefaultBaseURL: "https://openrouter.ai/api/v1",
|
||||
DefaultModel: "openai/gpt-4.1",
|
||||
AuthEnvVars: []string{"OPENROUTER_API_KEY"},
|
||||
RequiresAuth: true,
|
||||
SupportedAPIFormats: []providercatalog.APIFormat{providercatalog.APIFormatOpenAIChatCompletions},
|
||||
}},
|
||||
models: []providerWizardModel{{ID: "openai/gpt-4.1", Description: "GPT-4.1"}},
|
||||
}
|
||||
|
||||
updated, _ := m.applyProviderWizard()
|
||||
next := updated
|
||||
|
||||
if next.providerName == "" {
|
||||
t.Fatal("wizard apply should have set a provider name")
|
||||
}
|
||||
if got := os.Getenv(config.ActiveProviderEnv); got != next.providerName {
|
||||
t.Fatalf("%s = %q after wizard apply, want %q (children would spawn on the stale provider)", config.ActiveProviderEnv, got, next.providerName)
|
||||
}
|
||||
}
|
||||
|
||||
// On a config PERSIST failure, applyProviderWizard must leave live state fully
|
||||
// unchanged — the chat must NOT already be running on the new provider while the
|
||||
// status line and the ZERO_PROVIDER export (which pins spawned children) still
|
||||
// point at the old one. Build and persist are staged into locals; nothing is
|
||||
// committed unless both succeed.
|
||||
func TestApplyProviderWizardPersistFailureLeavesLiveStateUnchanged(t *testing.T) {
|
||||
t.Setenv("ZERO_CRED_STORAGE", "encrypted-file") // never touch the real OS keychain: apiKey is secured before the persist fails
|
||||
t.Setenv(config.ActiveProviderEnv, "old-provider")
|
||||
|
||||
// A config path whose parent is a regular FILE, so writeConfigFile's MkdirAll
|
||||
// fails and UpsertProvider returns an error.
|
||||
blocker := filepath.Join(t.TempDir(), "not-a-dir")
|
||||
if err := os.WriteFile(blocker, []byte("x"), 0o600); err != nil {
|
||||
t.Fatalf("seed blocker file: %v", err)
|
||||
}
|
||||
brokenConfigPath := filepath.Join(blocker, "config.json")
|
||||
|
||||
oldProvider := &fakeProvider{}
|
||||
newProvider := &fakeProvider{}
|
||||
m := newModel(context.Background(), Options{})
|
||||
m.provider = oldProvider
|
||||
m.providerProfile = config.ProviderProfile{Name: "old-provider"}
|
||||
m.providerName = "old-provider"
|
||||
m.userConfigPath = brokenConfigPath
|
||||
m.newProvider = func(config.ProviderProfile) (zeroruntime.Provider, error) { return newProvider, nil }
|
||||
m.providerWizard = &providerWizardState{
|
||||
step: providerWizardStepModel,
|
||||
profileName: "acme-new",
|
||||
providers: []providercatalog.Descriptor{{
|
||||
ID: "openrouter",
|
||||
Name: "OpenRouter",
|
||||
Transport: providercatalog.TransportOpenAICompatible,
|
||||
DefaultBaseURL: "https://openrouter.ai/api/v1",
|
||||
DefaultModel: "openai/gpt-4.1",
|
||||
AuthEnvVars: []string{"OPENROUTER_API_KEY"},
|
||||
RequiresAuth: true,
|
||||
SupportedAPIFormats: []providercatalog.APIFormat{providercatalog.APIFormatOpenAIChatCompletions},
|
||||
}},
|
||||
models: []providerWizardModel{{ID: "openai/gpt-4.1", Description: "GPT-4.1"}},
|
||||
apiKey: "sk-new",
|
||||
}
|
||||
|
||||
updated, _ := m.applyProviderWizard()
|
||||
next := updated
|
||||
|
||||
if next.providerWizard == nil || next.providerWizard.err == "" {
|
||||
t.Fatal("a persist failure must surface a wizard error and keep the wizard open")
|
||||
}
|
||||
if next.provider != oldProvider {
|
||||
t.Fatal("live provider must NOT be swapped when the persist fails")
|
||||
}
|
||||
if next.providerName != "old-provider" {
|
||||
t.Fatalf("providerName = %q, want it unchanged on persist failure", next.providerName)
|
||||
}
|
||||
if got := os.Getenv(config.ActiveProviderEnv); got != "old-provider" {
|
||||
t.Fatalf("%s = %q, want it unchanged on persist failure (children would diverge from parent)", config.ActiveProviderEnv, got)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user