From 419935b9f9d4164ef85556ff7968f69cb5ed74f6 Mon Sep 17 00:00:00 2001 From: Duc Nguyen <87276974+itsddvn@users.noreply.github.com> Date: Sun, 1 Mar 2026 15:36:13 +0700 Subject: [PATCH] fix: validate provider API keys during bootstrap using auth-required endpoints (#22) * fix(channels): start outbound dispatcher before channel check StartAll() returned early when no channels existed at boot, skipping the dispatchOutbound goroutine. Channels loaded later via Reload() assumed the dispatcher was running, causing outbound messages (agent responses) to never reach Telegram. Move dispatcher startup before the empty-channel early return so dynamically loaded channels always have a running consumer. * feat(ui): add LLM provider warning on overview page and ignore plans dir Show alert when no providers configured or all disabled, linking to provider settings. Add plans/ to .gitignore. * feat(onboard): add provider connectivity verification and placeholder seeding - Add onboard_verify.go: verify API keys via POST to chat/completions endpoint (401/403 = fatal, 400/422 = key valid, 5xx = warn) - Verify all configured providers before seeding in auto-onboard - Seed disabled placeholder providers (OpenRouter, Synthetic, AliCloud API/Sub) for UI discoverability after managed data seeding * refactor: reuse provider layer for bootstrap key verification - Replace raw HTTP requests with provider.Chat() calls, matching the existing verify pattern in internal/http/provider_verify.go - Respect config base URL overrides (GOCLAW_ANTHROPIC_BASE_URL, etc.) - Only block bootstrap if the primary provider key is invalid; secondary provider failures are logged as warnings - Add friendlyProviderError for human-readable error extraction Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: ntduc Co-authored-by: viettranx Co-authored-by: Claude Opus 4.6 --- cmd/onboard_auto.go | 9 +++ cmd/onboard_managed.go | 55 +++++++++++++ cmd/onboard_verify.go | 173 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 237 insertions(+) create mode 100644 cmd/onboard_verify.go diff --git a/cmd/onboard_auto.go b/cmd/onboard_auto.go index ef3a3462..4245bf8b 100644 --- a/cmd/onboard_auto.go +++ b/cmd/onboard_auto.go @@ -140,6 +140,15 @@ func runAutoOnboard(cfgPath string) bool { m.Close() } + // Verify provider connectivity for all configured providers before seeding. + // Only the primary provider's auth failure blocks bootstrap. + fmt.Println(" Verifying provider connectivity...") + if fatalErrors := verifyAllProviders(cfg, provider); len(fatalErrors) > 0 { + slog.Error("auto-onboard: primary provider verification failed", "errors", fatalErrors) + fmt.Printf(" Provider verification FAILED: primary provider %q has invalid API key\n", provider) + return false + } + // Seed default data (non-fatal if already exists) fmt.Print(" Seeding default agent/provider...") if err := seedManagedData(cfg.Database.PostgresDSN, cfg); err != nil { diff --git a/cmd/onboard_managed.go b/cmd/onboard_managed.go index 80f4b8c6..d6f8f059 100644 --- a/cmd/onboard_managed.go +++ b/cmd/onboard_managed.go @@ -145,6 +145,9 @@ func seedManagedData(dsn string, cfg *config.Config) error { // 7. Seed config_secrets from env vars seedConfigSecrets(ctx, stores, cfg) + // 8. Seed placeholder providers for UI discoverability + seedDefaultPlaceholders(ctx, stores) + return nil } @@ -281,6 +284,58 @@ func seedConfigSecrets(ctx context.Context, stores *store.Stores, cfg *config.Co } } +// defaultPlaceholderProviders defines disabled placeholder providers seeded for +// UI discoverability. Users can later enable and configure them via the dashboard. +var defaultPlaceholderProviders = []store.LLMProviderData{ + {Name: "openrouter", DisplayName: "OpenRouter", ProviderType: store.ProviderOpenRouter, APIBase: "https://openrouter.ai/api/v1", Enabled: false}, + {Name: "synthetic", DisplayName: "Synthetic", ProviderType: store.ProviderOpenAICompat, APIBase: "https://api.synthetic.new/openai/v1", Enabled: false}, + {Name: "alicloud-api", DisplayName: "AliCloud API", ProviderType: store.ProviderDashScope, APIBase: "https://dashscope-intl.aliyuncs.com/compatible-mode/v1", Enabled: false}, + {Name: "alicloud-sub", DisplayName: "AliCloud Sub", ProviderType: store.ProviderBailian, APIBase: "https://coding-intl.dashscope.aliyuncs.com/v1", Enabled: false}, +} + +// seedDefaultPlaceholders inserts disabled placeholder providers so they +// appear in the UI for easy configuration. Idempotent: UNIQUE(name) constraint +// skips duplicates, and providers whose api_base already exists are skipped +// to avoid overwriting user-configured entries. +func seedDefaultPlaceholders(ctx context.Context, stores *store.Stores) { + if stores.Providers == nil { + return + } + + // Build a set of existing api_base values to avoid seeding a placeholder + // when a user-configured provider already uses that base URL. + existing, err := stores.Providers.ListProviders(ctx) + if err != nil { + slog.Debug("seedDefaultPlaceholders: list providers failed", "error", err) + return + } + existingBases := make(map[string]bool, len(existing)) + for _, p := range existing { + if p.APIBase != "" { + existingBases[p.APIBase] = true + } + } + + seeded := 0 + for _, ph := range defaultPlaceholderProviders { + // Skip if a provider with this api_base already exists + if ph.APIBase != "" && existingBases[ph.APIBase] { + continue + } + + p := ph // copy + if err := stores.Providers.CreateProvider(ctx, &p); err != nil { + slog.Debug("seed placeholder skipped (may already exist)", "name", ph.Name, "error", err) + continue + } + seeded++ + } + + if seeded > 0 { + fmt.Printf(" Seeded %d placeholder provider(s)\n", seeded) + } +} + // resolveProviderAPIKey extracts the API key for a provider from the config. func resolveProviderAPIKey(cfg *config.Config, providerName string) string { switch providerName { diff --git a/cmd/onboard_verify.go b/cmd/onboard_verify.go new file mode 100644 index 00000000..e7b1fc7d --- /dev/null +++ b/cmd/onboard_verify.go @@ -0,0 +1,173 @@ +package cmd + +import ( + "context" + "errors" + "fmt" + "log/slog" + "strings" + "time" + + "github.com/nextlevelbuilder/goclaw/internal/config" + "github.com/nextlevelbuilder/goclaw/internal/providers" +) + +// providerVerifyError holds the result of a provider connectivity probe. +type providerVerifyError struct { + fatal bool // true = bad credentials, block startup + message string // human-readable description +} + +func (e *providerVerifyError) Error() string { return e.message } + +// newProviderForVerify instantiates a temporary provider for key verification. +// Mirrors the logic in registerProviders (gateway_providers.go) so auth headers, +// base URL overrides, and custom chat paths are handled correctly. +func newProviderForVerify(cfg *config.Config, name string) providers.Provider { + apiKey := resolveProviderAPIKey(cfg, name) + apiBase := resolveProviderAPIBase(name) + + switch name { + case "anthropic": + if cfg.Providers.Anthropic.APIBase != "" { + apiBase = cfg.Providers.Anthropic.APIBase + } + return providers.NewAnthropicProvider(apiKey, providers.WithAnthropicBaseURL(apiBase)) + case "dashscope": + if cfg.Providers.DashScope.APIBase != "" { + apiBase = cfg.Providers.DashScope.APIBase + } + return providers.NewDashScopeProvider(apiKey, apiBase, "") + case "minimax": + return providers.NewOpenAIProvider(name, apiKey, apiBase, "").WithChatPath("/text/chatcompletion_v2") + case "openai": + if cfg.Providers.OpenAI.APIBase != "" { + apiBase = cfg.Providers.OpenAI.APIBase + } + return providers.NewOpenAIProvider(name, apiKey, apiBase, "") + case "bailian": + if cfg.Providers.Bailian.APIBase != "" { + apiBase = cfg.Providers.Bailian.APIBase + } + return providers.NewOpenAIProvider(name, apiKey, apiBase, "") + default: + return providers.NewOpenAIProvider(name, apiKey, apiBase, "") + } +} + +// verifyProviderConnectivity checks whether a provider's API key is valid by +// sending a minimal Chat request through the provider layer. This reuses the +// same auth headers, base URLs, and HTTP client as the real provider calls. +// +// Strategy: provider.Chat() with message "hi", max_tokens=1. +// - 401/403 HTTPError → invalid API key (fatal) +// - Any other error → non-fatal warning (transient/config issue) +// - Success → key is valid +func verifyProviderConnectivity(cfg *config.Config, providerName string) *providerVerifyError { + apiKey := resolveProviderAPIKey(cfg, providerName) + if apiKey == "" { + return nil + } + + apiBase := resolveProviderAPIBase(providerName) + if apiBase == "" { + return nil // custom/unknown provider — skip + } + + prov := newProviderForVerify(cfg, providerName) + + model := "" + if pi, ok := providerMap[providerName]; ok { + model = pi.modelHint + } + + ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() + + _, err := prov.Chat(ctx, providers.ChatRequest{ + Messages: []providers.Message{{Role: "user", Content: "hi"}}, + Model: model, + Options: map[string]interface{}{"max_tokens": 1}, + }) + if err != nil { + var httpErr *providers.HTTPError + if errors.As(err, &httpErr) && (httpErr.Status == 401 || httpErr.Status == 403) { + return &providerVerifyError{ + fatal: true, + message: fmt.Sprintf("%s returned %d — invalid API key", providerName, httpErr.Status), + } + } + // Non-auth errors: transient network issue, bad model, rate limit, etc. + return &providerVerifyError{ + fatal: false, + message: fmt.Sprintf("%s: %s", providerName, friendlyProviderError(err)), + } + } + + return nil +} + +// verifyAllProviders checks connectivity for every provider that has an API key. +// Only the primary provider's auth failure is fatal (blocks bootstrap). +// Secondary provider failures are logged as warnings. +func verifyAllProviders(cfg *config.Config, primaryProvider string) []string { + var fatalErrors []string + + for _, name := range providerPriority { + apiKey := resolveProviderAPIKey(cfg, name) + if apiKey == "" { + continue + } + + verr := verifyProviderConnectivity(cfg, name) + if verr == nil { + slog.Info("provider connectivity verified", "provider", name) + fmt.Printf(" %s: OK\n", name) + continue + } + + if verr.fatal && name == primaryProvider { + slog.Error("primary provider key invalid", "provider", name, "error", verr.message) + fmt.Printf(" %s: FAILED — %s\n", name, verr.message) + fatalErrors = append(fatalErrors, fmt.Sprintf("%s: %s", name, verr.message)) + } else if verr.fatal { + slog.Warn("secondary provider key invalid (continuing)", "provider", name, "error", verr.message) + fmt.Printf(" %s: WARNING — %s (non-primary, skipping)\n", name, verr.message) + } else { + slog.Warn("provider connectivity warning", "provider", name, "warning", verr.message) + fmt.Printf(" %s: WARNING — %s\n", name, verr.message) + } + } + + return fatalErrors +} + +// friendlyProviderError extracts a human-readable message from provider errors. +func friendlyProviderError(err error) string { + msg := err.Error() + + // Try to extract "message" field from embedded JSON error blobs. + if idx := strings.Index(msg, `"message"`); idx >= 0 { + rest := msg[idx:] + if start := strings.Index(rest, `:`); start >= 0 { + rest = strings.TrimLeft(rest[start+1:], " ") + if len(rest) > 0 && rest[0] == '"' { + rest = rest[1:] + if end := strings.Index(rest, `"`); end >= 0 && rest[:end] != "" { + return rest[:end] + } + } + } + } + + // Strip "HTTP NNN: provider: " prefix. + if idx := strings.LastIndex(msg, ": "); idx >= 0 && idx < len(msg)-2 { + suffix := msg[idx+2:] + if strings.HasPrefix(suffix, "{") { + return "request rejected by provider" + } + return suffix + } + + return msg +}