diff --git a/coderd/exp_chats.go b/coderd/exp_chats.go index acc815e54d..180867a1fe 100644 --- a/coderd/exp_chats.go +++ b/coderd/exp_chats.go @@ -5165,6 +5165,7 @@ func (api *API) createChatProvider(rw http.ResponseWriter, r *http.Request) { } if err := validateChatProviderCentralAPIKey( + provider, centralAPIKeyEnabled, api.hasEffectiveCentralProviderAPIKey(ctx, database.ChatProvider{ Provider: provider, @@ -5326,6 +5327,7 @@ func (api *API) updateChatProvider(rw http.ResponseWriter, r *http.Request) { } if err := validateChatProviderCentralAPIKey( + existing.Provider, centralAPIKeyEnabled, api.hasEffectiveCentralProviderAPIKey(ctx, database.ChatProvider{ ID: existing.ID, @@ -5462,7 +5464,7 @@ func (api *API) listUserChatProviderConfigs(rw http.ResponseWriter, r *http.Requ hasUserAPIKey := hasUserAPIKeyByProviderID[provider.ID] hasCentralAPIKeyFallback := provider.Enabled && provider.AllowCentralApiKeyFallback && - api.hasEffectiveCentralProviderAPIKey(ctx, provider, uuid.Nil) + api.hasEffectiveCentralProviderCredentials(ctx, provider, uuid.Nil) resp = append( resp, convertUserChatProviderConfig( @@ -5548,7 +5550,7 @@ func (api *API) upsertUserChatProviderKey(rw http.ResponseWriter, r *http.Reques hasCentralAPIKeyFallback := provider.Enabled && provider.AllowCentralApiKeyFallback && - api.hasEffectiveCentralProviderAPIKey(ctx, provider, uuid.Nil) + api.hasEffectiveCentralProviderCredentials(ctx, provider, uuid.Nil) httpapi.Write( ctx, rw, @@ -6395,15 +6397,17 @@ func validateChatProviderCredentialPolicy( //nolint:revive // This helper validates central-key requirements. func validateChatProviderCentralAPIKey( + provider string, centralEnabled bool, hasCentralAPIKey bool, ) error { - if centralEnabled && !hasCentralAPIKey { - return xerrors.New( - "API key is required when central API key is enabled.", - ) + if !centralEnabled || hasCentralAPIKey { + return nil } - return nil + if chatprovider.ProviderAllowsAmbientCredentials(provider) { + return nil + } + return xerrors.New("API key is required when central API key is enabled.") } // ChatProviderAPIKeysFromDeploymentValues returns deployment-backed chat @@ -6421,6 +6425,18 @@ func (api *API) hasEffectiveProviderAPIKey(ctx context.Context, provider databas return api.hasEffectiveCentralProviderAPIKey(ctx, provider, uuid.Nil) } +func (api *API) hasEffectiveCentralProviderCredentials( + ctx context.Context, + provider database.ChatProvider, + excludeProviderID uuid.UUID, +) bool { + if api.hasEffectiveCentralProviderAPIKey(ctx, provider, excludeProviderID) { + return true + } + return provider.CentralApiKeyEnabled && + chatprovider.ProviderAllowsAmbientCredentials(provider.Provider) +} + func (api *API) hasEffectiveCentralProviderAPIKey( ctx context.Context, provider database.ChatProvider, diff --git a/coderd/exp_chats_test.go b/coderd/exp_chats_test.go index eb1c0e4feb..75b1af4306 100644 --- a/coderd/exp_chats_test.go +++ b/coderd/exp_chats_test.go @@ -46,7 +46,10 @@ import ( "github.com/coder/websocket/wsjson" ) -const chatProviderAPIKeySizeLimit = 10240 +const ( + chatProviderAPIKeySizeLimit = 10240 + missingCentralKeyMessage = "API key is required when central API key is enabled." +) func chatDeploymentValues(t testing.TB) *codersdk.DeploymentValues { t.Helper() @@ -2098,6 +2101,100 @@ func TestCreateChatProvider(t *testing.T) { require.Equal(t, codersdk.ChatProviderConfigSourceDatabase, provider.Source) }) + t.Run("AllowsBedrockWithCentralAPIKeyEnabledWithoutStoredKey", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + client := newChatClient(t) + _ = coderdtest.CreateFirstUser(t, client.Client) + + provider, err := client.CreateChatProvider(ctx, codersdk.CreateChatProviderConfigRequest{ + Provider: "bedrock", + DisplayName: "AWS Bedrock", + CentralAPIKeyEnabled: ptr.Ref(true), + }) + require.NoError(t, err) + require.NotEqual(t, uuid.Nil, provider.ID) + require.Equal(t, "bedrock", provider.Provider) + require.Equal(t, "AWS Bedrock", provider.DisplayName) + require.True(t, provider.Enabled) + require.False(t, provider.HasAPIKey) + require.True(t, provider.CentralAPIKeyEnabled) + require.Equal(t, codersdk.ChatProviderConfigSourceDatabase, provider.Source) + + providers, err := client.ListChatProviders(ctx) + require.NoError(t, err) + for _, listed := range providers { + if listed.Provider == "bedrock" { + require.False(t, listed.HasAPIKey) + return + } + } + t.Fatal("bedrock provider not found") + }) + + t.Run("ReportsBedrockAmbientFallbackForUserConfigs", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + client := newChatClient(t) + _ = coderdtest.CreateFirstUser(t, client.Client) + + provider, err := client.CreateChatProvider(ctx, codersdk.CreateChatProviderConfigRequest{ + Provider: "bedrock", + DisplayName: "AWS Bedrock Fallback", + CentralAPIKeyEnabled: ptr.Ref(true), + AllowUserAPIKey: ptr.Ref(true), + AllowCentralAPIKeyFallback: ptr.Ref(true), + }) + require.NoError(t, err) + require.False(t, provider.HasAPIKey) + + configs, err := client.ListUserChatProviderConfigs(ctx) + require.NoError(t, err) + require.Len(t, configs, 1) + require.Equal(t, provider.ID, configs[0].ProviderID) + require.Equal(t, provider.Provider, configs[0].Provider) + require.False(t, configs[0].HasUserAPIKey) + require.True(t, configs[0].HasCentralAPIKeyFallback) + }) + + t.Run("AllowsBedrockWithExplicitAPIKey", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + client := newChatClient(t) + _ = coderdtest.CreateFirstUser(t, client.Client) + + provider, err := client.CreateChatProvider(ctx, codersdk.CreateChatProviderConfigRequest{ + Provider: "bedrock", + DisplayName: "AWS Bedrock Token", + APIKey: "bedrock-bearer-token", + CentralAPIKeyEnabled: ptr.Ref(true), + }) + require.NoError(t, err) + require.Equal(t, "bedrock", provider.Provider) + require.Equal(t, "AWS Bedrock Token", provider.DisplayName) + require.True(t, provider.HasAPIKey) + require.True(t, provider.CentralAPIKeyEnabled) + }) + + t.Run("RejectsMissingCentralAPIKeyForNonBedrock", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + client := newChatClient(t) + _ = coderdtest.CreateFirstUser(t, client.Client) + + _, err := client.CreateChatProvider(ctx, codersdk.CreateChatProviderConfigRequest{ + Provider: "openai", + DisplayName: "OpenAI", + CentralAPIKeyEnabled: ptr.Ref(true), + }) + sdkErr := requireSDKError(t, err, http.StatusBadRequest) + require.Equal(t, missingCentralKeyMessage, sdkErr.Message) + }) + t.Run("InvalidProvider", func(t *testing.T) { t.Parallel() @@ -2199,7 +2296,7 @@ func TestCreateChatProvider(t *testing.T) { Provider: "openai", }) sdkErr := requireSDKError(t, err, http.StatusBadRequest) - require.Equal(t, "API key is required when central API key is enabled.", sdkErr.Message) + require.Equal(t, missingCentralKeyMessage, sdkErr.Message) }) t.Run("RejectsInvalidPolicyTuple", func(t *testing.T) { @@ -2310,6 +2407,34 @@ func TestUpdateChatProvider(t *testing.T) { require.Equal(t, baseURL, updated.BaseURL) }) + t.Run("AllowsClearingBedrockAPIKeyWithCentralAPIKeyEnabled", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + client := newChatClient(t) + _ = coderdtest.CreateFirstUser(t, client.Client) + + provider, err := client.CreateChatProvider(ctx, codersdk.CreateChatProviderConfigRequest{ + Provider: "bedrock", + DisplayName: "AWS Bedrock", + APIKey: "bedrock-bearer-token", + CentralAPIKeyEnabled: ptr.Ref(true), + }) + require.NoError(t, err) + require.True(t, provider.HasAPIKey) + require.True(t, provider.CentralAPIKeyEnabled) + + updated, err := client.UpdateChatProvider(ctx, provider.ID, codersdk.UpdateChatProviderConfigRequest{ + APIKey: ptr.Ref(""), + CentralAPIKeyEnabled: ptr.Ref(true), + }) + require.NoError(t, err) + require.Equal(t, provider.ID, updated.ID) + require.Equal(t, "bedrock", updated.Provider) + require.False(t, updated.HasAPIKey) + require.True(t, updated.CentralAPIKeyEnabled) + }) + t.Run("NotFound", func(t *testing.T) { t.Parallel() @@ -2408,7 +2533,7 @@ func TestUpdateChatProvider(t *testing.T) { CentralAPIKeyEnabled: ptr.Ref(true), }) sdkErr := requireSDKError(t, err, http.StatusBadRequest) - require.Equal(t, "API key is required when central API key is enabled.", sdkErr.Message) + require.Equal(t, missingCentralKeyMessage, sdkErr.Message) }) t.Run("RejectsClearingLastCentralKey", func(t *testing.T) { @@ -2428,7 +2553,7 @@ func TestUpdateChatProvider(t *testing.T) { APIKey: ptr.Ref(""), }) sdkErr := requireSDKError(t, err, http.StatusBadRequest) - require.Equal(t, "API key is required when central API key is enabled.", sdkErr.Message) + require.Equal(t, missingCentralKeyMessage, sdkErr.Message) }) t.Run("RejectsEnablingCentralKeyWithoutKey", func(t *testing.T) { @@ -2449,7 +2574,7 @@ func TestUpdateChatProvider(t *testing.T) { CentralAPIKeyEnabled: ptr.Ref(true), }) sdkErr := requireSDKError(t, err, http.StatusBadRequest) - require.Equal(t, "API key is required when central API key is enabled.", sdkErr.Message) + require.Equal(t, missingCentralKeyMessage, sdkErr.Message) }) t.Run("RejectsInvalidPolicyTuple", func(t *testing.T) { diff --git a/coderd/x/chatd/chatprovider/chatprovider.go b/coderd/x/chatd/chatprovider/chatprovider.go index 94a8ca2f31..72e8ca21aa 100644 --- a/coderd/x/chatd/chatprovider/chatprovider.go +++ b/coderd/x/chatd/chatprovider/chatprovider.go @@ -74,6 +74,13 @@ func ProviderDisplayName(provider string) string { return normalized } +// ProviderAllowsAmbientCredentials reports whether provider can use +// ambient credentials from the Coder server instead of an explicit +// API key. +func ProviderAllowsAmbientCredentials(provider string) bool { + return NormalizeProvider(provider) == fantasybedrock.Name +} + // ProviderAPIKeys contains API keys for provider calls. type ProviderAPIKeys struct { OpenAI string @@ -136,6 +143,17 @@ func (k ProviderAPIKeys) APIKey(provider string) string { } } +// HasProvider reports whether a provider has an explicit resolved entry +// in the provider key map, even when the resolved key is empty. +func (k ProviderAPIKeys) HasProvider(provider string) bool { + normalized := NormalizeProvider(provider) + if normalized == "" || k.ByProvider == nil { + return false + } + _, ok := k.ByProvider[normalized] + return ok +} + // BaseURL returns the configured base URL for a provider. func (k ProviderAPIKeys) BaseURL(provider string) string { normalized := NormalizeProvider(provider) @@ -295,6 +313,15 @@ func ResolveUserProviderKeys( } else { resolved.UnavailableReason = codersdk.ChatModelProviderUnavailableReasonUserAPIKeyRequired } + case normalizedProvider == fantasybedrock.Name && provider.CentralAPIKeyEnabled: + // Bedrock can use ambient AWS credentials from the Coder server + // without an explicit key, but only when the credential policy + // allows central credentials to satisfy the request. + if !provider.AllowUserAPIKey || provider.AllowCentralAPIKeyFallback { + resolved.Available = true + } else { + resolved.UnavailableReason = codersdk.ChatModelProviderUnavailableReasonUserAPIKeyRequired + } case provider.AllowUserAPIKey && provider.AllowCentralAPIKeyFallback && provider.CentralAPIKeyEnabled: // When users can add their own key, a missing central fallback key is // still something the user can remedy. @@ -305,14 +332,18 @@ func ResolveUserProviderKeys( resolved.UnavailableReason = codersdk.ChatModelProviderUnavailableMissingAPIKey } - setResolvedProviderAPIKey(&merged, normalizedProvider, chosenKey) + setResolvedProviderAPIKey(&merged, normalizedProvider, chosenKey, resolved) availabilityByProvider[normalizedProvider] = resolved } return merged, availabilityByProvider } -func setResolvedProviderAPIKey(keys *ProviderAPIKeys, provider string, apiKey string) { +// setResolvedProviderAPIKey keeps ByProvider presence aligned with +// resolved provider availability. An empty value means ambient +// credentials may satisfy the provider. An absent entry means the +// provider is not resolvable. +func setResolvedProviderAPIKey(keys *ProviderAPIKeys, provider string, apiKey string, availability ProviderAvailability) { normalizedProvider := NormalizeProvider(provider) if normalizedProvider == "" { return @@ -329,7 +360,7 @@ func setResolvedProviderAPIKey(keys *ProviderAPIKeys, provider string, apiKey st case fantasyanthropic.Name: keys.Anthropic = trimmedKey } - if trimmedKey != "" { + if trimmedKey != "" || (availability.Available && ProviderAllowsAmbientCredentials(normalizedProvider)) { keys.ByProvider[normalizedProvider] = trimmedKey } } @@ -1132,7 +1163,8 @@ func ModelFromConfig( } apiKey := providerKeys.APIKey(provider) - if apiKey == "" { + if apiKey == "" && + !(ProviderAllowsAmbientCredentials(provider) && providerKeys.HasProvider(provider)) { return nil, missingProviderAPIKeyError(provider) } baseURL := providerKeys.BaseURL(provider) @@ -1173,12 +1205,17 @@ func ModelFromConfig( providerClient, err = fantasyazure.New(azureOpts...) case fantasybedrock.Name: bedrockOpts := []fantasybedrock.Option{ - fantasybedrock.WithAPIKey(apiKey), fantasybedrock.WithUserAgent(userAgent), } + if apiKey != "" { + bedrockOpts = append(bedrockOpts, fantasybedrock.WithAPIKey(apiKey)) + } if len(extraHeaders) > 0 { bedrockOpts = append(bedrockOpts, fantasybedrock.WithHeaders(extraHeaders)) } + if baseURL != "" { + bedrockOpts = append(bedrockOpts, fantasybedrock.WithBaseURL(baseURL)) + } if httpClient != nil { bedrockOpts = append(bedrockOpts, fantasybedrock.WithHTTPClient(httpClient)) } @@ -1260,7 +1297,7 @@ func ModelFromConfig( return nil, xerrors.Errorf("unsupported model provider %q", provider) } if err != nil { - return nil, xerrors.Errorf("create %s provider: %w", provider, err) + return nil, providerCreationError(provider, err) } model, err := providerClient.LanguageModel(context.Background(), modelID) @@ -1270,14 +1307,19 @@ func ModelFromConfig( return model, nil } +func providerCreationError(provider string, err error) error { + return xerrors.Errorf("create %s provider: %w", provider, err) +} + +// Providers that allow ambient credentials, such as Bedrock, bypass +// this helper only after ResolveUserProviderKeys marks them +// available. func missingProviderAPIKeyError(provider string) error { switch provider { case fantasyanthropic.Name: return xerrors.New("ANTHROPIC_API_KEY is not set") case fantasyazure.Name: return xerrors.New("AZURE_OPENAI_API_KEY is not set") - case fantasybedrock.Name: - return xerrors.New("BEDROCK_API_KEY is not set") case fantasygoogle.Name: return xerrors.New("GOOGLE_API_KEY is not set") case fantasyopenai.Name: diff --git a/coderd/x/chatd/chatprovider/chatprovider_test.go b/coderd/x/chatd/chatprovider/chatprovider_test.go index 5fa63a19ec..acb0dc9ea1 100644 --- a/coderd/x/chatd/chatprovider/chatprovider_test.go +++ b/coderd/x/chatd/chatprovider/chatprovider_test.go @@ -1,11 +1,14 @@ package chatprovider_test import ( + "encoding/json" "net/http" + "net/http/httptest" "testing" "charm.land/fantasy" fantasyanthropic "charm.land/fantasy/providers/anthropic" + fantasybedrock "charm.land/fantasy/providers/bedrock" fantasyopenai "charm.land/fantasy/providers/openai" fantasyopenrouter "charm.land/fantasy/providers/openrouter" fantasyvercel "charm.land/fantasy/providers/vercel" @@ -44,6 +47,7 @@ func TestResolveUserProviderKeys(t *testing.T) { openAIProviderID := uuid.MustParse("00000000-0000-0000-0000-000000000001") anthropicProviderID := uuid.MustParse("00000000-0000-0000-0000-000000000002") + bedrockProviderID := uuid.MustParse("00000000-0000-0000-0000-000000000003") tests := []struct { name string @@ -52,6 +56,7 @@ func TestResolveUserProviderKeys(t *testing.T) { userKeys []chatprovider.UserProviderKey wantAvailability map[string]chatprovider.ProviderAvailability wantKeys map[string]string + wantKeyPresence map[string]bool }{ { name: "CentralOnlyKeyPresent", @@ -72,6 +77,74 @@ func TestResolveUserProviderKeys(t *testing.T) { wantKeys: map[string]string{ fantasyopenai.Name: "", }, + wantKeyPresence: map[string]bool{ + fantasyopenai.Name: false, + }, + }, + { + name: "BedrockCentralOnlyAmbientCredentialsEnabled", + providers: []chatprovider.ConfiguredProvider{configuredProvider(bedrockProviderID, fantasybedrock.Name, true, "", false, false)}, + wantAvailability: map[string]chatprovider.ProviderAvailability{ + fantasybedrock.Name: {Available: true}, + }, + wantKeys: map[string]string{ + fantasybedrock.Name: "", + }, + wantKeyPresence: map[string]bool{ + fantasybedrock.Name: true, + }, + }, + { + name: "BedrockFallbackAmbientCredentialsEnabled", + providers: []chatprovider.ConfiguredProvider{configuredProvider(bedrockProviderID, fantasybedrock.Name, true, "", true, true)}, + wantAvailability: map[string]chatprovider.ProviderAvailability{ + fantasybedrock.Name: {Available: true}, + }, + wantKeys: map[string]string{ + fantasybedrock.Name: "", + }, + wantKeyPresence: map[string]bool{ + fantasybedrock.Name: true, + }, + }, + { + name: "BedrockUserKeyRequiredWithoutFallback", + providers: []chatprovider.ConfiguredProvider{configuredProvider(bedrockProviderID, fantasybedrock.Name, true, "", true, false)}, + wantAvailability: map[string]chatprovider.ProviderAvailability{ + fantasybedrock.Name: {Available: false, UnavailableReason: codersdk.ChatModelProviderUnavailableReasonUserAPIKeyRequired}, + }, + wantKeys: map[string]string{ + fantasybedrock.Name: "", + }, + wantKeyPresence: map[string]bool{ + fantasybedrock.Name: false, + }, + }, + { + name: "BedrockCentralDisabledMissingAPIKey", + providers: []chatprovider.ConfiguredProvider{configuredProvider(bedrockProviderID, fantasybedrock.Name, false, "", false, false)}, + wantAvailability: map[string]chatprovider.ProviderAvailability{ + fantasybedrock.Name: {Available: false, UnavailableReason: codersdk.ChatModelProviderUnavailableMissingAPIKey}, + }, + wantKeys: map[string]string{ + fantasybedrock.Name: "", + }, + wantKeyPresence: map[string]bool{ + fantasybedrock.Name: false, + }, + }, + { + name: "BedrockCentralStoredKeyPresent", + providers: []chatprovider.ConfiguredProvider{configuredProvider(bedrockProviderID, fantasybedrock.Name, true, "bedrock-token", false, false)}, + wantAvailability: map[string]chatprovider.ProviderAvailability{ + fantasybedrock.Name: {Available: true}, + }, + wantKeys: map[string]string{ + fantasybedrock.Name: "bedrock-token", + }, + wantKeyPresence: map[string]bool{ + fantasybedrock.Name: true, + }, }, { name: "UserOnlyUserHasKey", @@ -177,6 +250,14 @@ func TestResolveUserProviderKeys(t *testing.T) { require.Equal(t, wantAvailability, gotAvailability) require.Equal(t, tt.wantKeys[provider], keys.APIKey(provider)) } + for provider, wantPresent := range tt.wantKeyPresence { + gotKey, ok := keys.ByProvider[provider] + require.Equal(t, wantPresent, ok, "unexpected key presence for provider %q", provider) + require.Equal(t, wantPresent, keys.HasProvider(provider), "unexpected HasProvider result for provider %q", provider) + if wantPresent { + require.Equal(t, tt.wantKeys[provider], gotKey) + } + } }) } } @@ -742,6 +823,181 @@ func TestCoderHeaders(t *testing.T) { }) } +func TestModelFromConfig_Bedrock(t *testing.T) { + t.Parallel() + + const modelID = "us.anthropic.claude-sonnet-4-20250514-v1:0" + + // This verifies the policy gate that permits an empty Bedrock key. + // End-to-end ambient credential auth would need a real AWS + // environment or a more complete mock, which is outside this scope. + t.Run("AllowsEmptyAPIKeyForAmbientCredentials", func(t *testing.T) { + t.Parallel() + + model, err := chatprovider.ModelFromConfig( + fantasybedrock.Name, + modelID, + chatprovider.ProviderAPIKeys{ + ByProvider: map[string]string{ + fantasybedrock.Name: "", + }, + }, + chatprovider.UserAgent(), + nil, + nil, + ) + require.NoError(t, err) + require.NotNil(t, model) + require.Equal(t, fantasybedrock.Name, model.Provider()) + }) + + t.Run("RequiresResolvedProviderForAmbientCredentials", func(t *testing.T) { + t.Parallel() + + model, err := chatprovider.ModelFromConfig( + fantasybedrock.Name, + modelID, + chatprovider.ProviderAPIKeys{}, + chatprovider.UserAgent(), + nil, + nil, + ) + require.Nil(t, model) + require.EqualError(t, err, "API key for provider \"bedrock\" is not set") + }) + + t.Run("ForwardsBaseURLAndExplicitAPIKey", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + + type requestCapture struct { + Path string + Authorization string + UserAgent string + } + + requests := make(chan requestCapture, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests <- requestCapture{ + Path: r.URL.Path, + Authorization: r.Header.Get("Authorization"), + UserAgent: r.Header.Get("User-Agent"), + } + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(bedrockNonStreamingResponse()) + })) + defer server.Close() + + model, err := chatprovider.ModelFromConfig( + fantasybedrock.Name, + modelID, + chatprovider.ProviderAPIKeys{ + ByProvider: map[string]string{ + fantasybedrock.Name: "test-key", + }, + BaseURLByProvider: map[string]string{ + fantasybedrock.Name: server.URL, + }, + }, + chatprovider.UserAgent(), + nil, + nil, + ) + require.NoError(t, err) + require.NotNil(t, model) + + _, err = model.Generate(ctx, fantasy.Call{ + Prompt: []fantasy.Message{ + { + Role: fantasy.MessageRoleUser, + Content: []fantasy.MessagePart{ + fantasy.TextPart{Text: "hello"}, + }, + }, + }, + }) + require.NoError(t, err) + + got := testutil.TryReceive(ctx, t, requests) + require.Equal(t, "/model/"+modelID+"/invoke", got.Path) + require.Equal(t, "Bearer test-key", got.Authorization) + require.Equal(t, chatprovider.UserAgent(), got.UserAgent) + }) + + t.Run("NonBedrockStillRequiresAPIKey", func(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + provider string + model string + wantErr string + }{ + { + name: "OpenAI", + provider: fantasyopenai.Name, + model: "gpt-4", + wantErr: "OPENAI_API_KEY is not set", + }, + { + name: "Anthropic", + provider: fantasyanthropic.Name, + model: "claude-sonnet-4-20250514", + wantErr: "ANTHROPIC_API_KEY is not set", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + model, err := chatprovider.ModelFromConfig( + tt.provider, + tt.model, + chatprovider.ProviderAPIKeys{}, + chatprovider.UserAgent(), + nil, + nil, + ) + require.Nil(t, model) + require.EqualError(t, err, tt.wantErr) + }) + } + }) +} + +func bedrockNonStreamingResponse() map[string]any { + return map[string]any{ + "id": "msg_01Test", + "type": "message", + "role": "assistant", + "model": "claude-sonnet-4-20250514", + "content": []any{ + map[string]any{ + "type": "text", + "text": "Hi there", + }, + }, + "stop_reason": "end_turn", + "stop_sequence": "", + "usage": map[string]any{ + "cache_creation": map[string]any{ + "ephemeral_1h_input_tokens": 0, + "ephemeral_5m_input_tokens": 0, + }, + "cache_creation_input_tokens": 0, + "cache_read_input_tokens": 0, + "input_tokens": 5, + "output_tokens": 2, + "server_tool_use": map[string]any{ + "web_search_requests": 0, + }, + "service_tier": "standard", + }, + } +} + // TestModelFromConfig_ExtraHeaders verifies that extra headers passed // to ModelFromConfig are sent on outgoing LLM API requests. Only the // OpenAI and Anthropic providers are tested end-to-end because the diff --git a/docs/ai-coder/agents/models.md b/docs/ai-coder/agents/models.md index 7f37657fb6..2036f0b068 100644 --- a/docs/ai-coder/agents/models.md +++ b/docs/ai-coder/agents/models.md @@ -1,29 +1,29 @@ # Models Administrators configure LLM providers and models from the Coder dashboard. -Providers, models, and API keys are deployment-wide settings managed by -platform teams. Developers select from the set of models that an administrator -has enabled. +Providers, models, and centrally managed credentials are deployment-wide +settings managed by platform teams. Developers select from the set of models +that an administrator has enabled. Optionally, administrators can allow developers to supply their own API keys for specific providers. See [User API keys](#user-api-keys-byok) below. ## Providers -Each LLM provider has a type, an API key, and an optional base URL override. +Each LLM provider has a type, a credential configuration, and an optional base URL override. Coder supports the following provider types: -| Provider | Description | -|-------------------|------------------------------------------| -| Anthropic | Claude models via Anthropic API | -| OpenAI | GPT and o-series models via OpenAI API | -| Google | Gemini models via Google AI API | -| Azure OpenAI | OpenAI models hosted on Azure | -| AWS Bedrock | Models available through AWS Bedrock | -| OpenAI Compatible | Any endpoint implementing the OpenAI API | -| OpenRouter | Multi-model routing via OpenRouter | -| Vercel AI Gateway | Models via Vercel AI SDK | +| Provider | Description | +|-------------------|------------------------------------------------------------------| +| Anthropic | Claude models via Anthropic API | +| OpenAI | GPT and o-series models via OpenAI API | +| Google | Gemini models via Google AI API | +| Azure OpenAI | OpenAI models hosted on Azure | +| AWS Bedrock | Models via AWS Bedrock (bearer token or ambient AWS credentials) | +| OpenAI Compatible | Any endpoint implementing the OpenAI API | +| OpenRouter | Multi-model routing via OpenRouter | +| Vercel AI Gateway | Models via Vercel AI SDK | The **OpenAI Compatible** type is a catch-all for any service that exposes an OpenAI-compatible chat completions endpoint. Use it to connect to self-hosted @@ -35,7 +35,7 @@ models, internal gateways, or third-party proxies like LiteLLM. 1. Click **Admin** in the top bar to open the configuration dialog. 1. Select the **Providers** tab. 1. Click the provider you want to configure. -1. Enter the **API key** for the provider. +1. Enter the **API key** for the provider, if required. 1. Optionally set a **Base URL** to override the default endpoint. This is useful for enterprise proxies, regional endpoints, or self-hosted models. 1. Click **Save**. @@ -47,28 +47,58 @@ status. Screenshot of the add provider form -Adding a provider requires an API key. The base URL is optional. +Adding a provider usually requires an API key. AWS Bedrock can also use +ambient AWS credentials. The base URL is optional. -### Provider API keys and security +## Configuring AWS Bedrock -Provider API keys are stored encrypted in the Coder database. They are never -exposed to workspaces, developers, or the browser after initial entry. The -dashboard shows only whether a key is set, not the key itself. +AWS Bedrock supports two credential modes for Agents providers: + +- **Bearer token mode**: Enter a Bedrock-compatible bearer token in the + **API key** field when you add the provider. +- **Ambient AWS credentials mode**: Leave the **API key** field empty. The + Coder server resolves credentials from the standard AWS SDK credential chain, + including IAM instance roles and `AWS_ACCESS_KEY_ID` / + `AWS_SECRET_ACCESS_KEY` environment variables. + +Region comes from the standard AWS SDK configuration. In most deployments, set +`AWS_REGION` on the Coder server. Bearer token mode falls back to `us-east-1` +when no region is configured. Ambient credentials require a region from the +standard AWS SDK chain, for example `AWS_REGION`. + +The **Base URL** field overrides the Bedrock runtime endpoint. Use it for +custom endpoints or VPC endpoints. + +> [!NOTE] +> Agents Bedrock provider configuration is separate from AI Gateway Bedrock +> flags (`CODER_AIBRIDGE_BEDROCK_*`). AI Gateway and Agents use independent +> credential paths. + +## Provider credentials and security + +Provider API keys entered in the dashboard are stored encrypted in the Coder +database. They are never exposed to workspaces, developers, or the browser +after initial entry. The dashboard shows only whether a key is set, not the +key itself. + +When a provider uses ambient credentials, Coder resolves them from the server +environment at request time instead of storing a secret in the database. Because the agent loop runs in the control plane, workspaces never need direct access to LLM providers. See [Architecture](./architecture.md#no-api-keys-in-workspaces) for details on this security model. -### Key policy +## Key policy -Each provider has three policy flags that control how API keys are sourced: +Each provider has three policy flags that control how provider credentials are +sourced: -| Setting | Default | Description | -|-------------------------|---------|-----------------------------------------------------------------------------------------------------| -| Central API key | On | The provider uses a deployment-managed API key entered by an administrator. | -| Allow user API keys | Off | Developers may supply their own API key for this provider. | -| Central key as fallback | Off | When user keys are allowed, fall back to the central key if a developer has not set a personal key. | +| Setting | Default | Description | +|-------------------------|---------|--------------------------------------------------------------------------------------------------------------------------| +| Central API key | On | The provider uses deployment-managed credentials configured by an administrator. For most providers, this is an API key. | +| Allow user API keys | Off | Developers may supply their own API key for this provider. | +| Central key as fallback | Off | When user keys are allowed, fall back to deployment-managed credentials if a developer has not set a personal key. | At least one credential source must be enabled. These settings appear in the provider configuration form under **Key policy**. @@ -87,10 +117,11 @@ to a given developer: | On | On | On | No | Uses central key | When a developer's personal key is present, it always takes precedence over -the central key. When user keys are required and fallback is disabled, -the provider is unavailable to developers who have not saved a personal key — -even if a central key exists. This is intentional: it enforces that each -developer authenticates with their own credentials. +deployment-managed credentials. When user keys are required and fallback is +disabled, the provider is unavailable to developers who have not saved a +personal key, even if deployment-managed credentials exist. This is +intentional: it enforces that each developer authenticates with their own +credentials. ## Models @@ -196,14 +227,15 @@ fields appear dynamically in the admin UI when you select a provider. > [!NOTE] > Azure OpenAI uses the same options as OpenAI. AWS Bedrock uses the same -> options as Anthropic. +> model configuration options as Anthropic (thinking budget, reasoning +> effort). ## How developers select models Developers see a model selector dropdown when starting or continuing a chat on the Agents page. The selector shows only models from providers that have valid -API keys configured. Models are grouped by provider if multiple providers are -active. +credentials configured. Models are grouped by provider if multiple providers +are active. The model selector uses the following precedence to pre-select a model: @@ -232,17 +264,17 @@ developers can supply their own API key from the Agents settings page. 1. Enter your API key and click **Save**. Personal API keys are encrypted at rest using the same database encryption -as deployment-managed keys. The dashboard never displays a saved key — only -whether one is set. +used for deployment-managed provider secrets. The dashboard never displays a +saved key, only whether one is set. ### How key selection works -When you start a chat, the control plane resolves which API key to use for -each provider: +When you start a chat, the control plane resolves which credential source to +use for each provider: 1. If you have a personal key for the provider, it is used. 1. If you do not have a personal key and central key fallback is enabled, - the deployment-managed key is used. + deployment-managed credentials are used. 1. If you do not have a personal key and fallback is disabled, the provider is unavailable to you. Models from that provider will not appear in the model selector. @@ -251,8 +283,8 @@ each provider: Click **Remove** on the provider card in the API Keys settings tab. If central key fallback is enabled, subsequent requests will use the shared -deployment key. If fallback is disabled, the provider becomes unavailable -until you add a new personal key. +deployment-managed credentials. If fallback is disabled, the provider becomes +unavailable until you add a new personal key. ## Using an LLM proxy diff --git a/docs/ai-coder/ai-gateway/setup.md b/docs/ai-coder/ai-gateway/setup.md index 06a0e2637a..0df93376d6 100644 --- a/docs/ai-coder/ai-gateway/setup.md +++ b/docs/ai-coder/ai-gateway/setup.md @@ -61,6 +61,10 @@ If both are set, `CODER_AIBRIDGE_BEDROCK_BASE_URL` takes precedence. - `CODER_AIBRIDGE_BEDROCK_MODEL` or `--aibridge-bedrock-model` - `CODER_AIBRIDGE_BEDROCK_SMALL_FAST_MODEL` or `--aibridge-bedrock-small-fast-model` +> [!NOTE] +> These Bedrock settings configure AI Gateway only. To configure Bedrock as an +> Agents provider, see [Configuring AWS Bedrock](../agents/models.md#configuring-aws-bedrock). + **Optional:** - `CODER_AIBRIDGE_BEDROCK_ACCESS_KEY` or `--aibridge-bedrock-access-key` diff --git a/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ChatModelAdminPanel.stories.tsx b/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ChatModelAdminPanel.stories.tsx index a0c28b4807..61f6cca95e 100644 --- a/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ChatModelAdminPanel.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ChatModelAdminPanel.stories.tsx @@ -813,6 +813,158 @@ export const ProviderInvalidCredentialState: Story = { }, }; +export const ProviderFormBedrockAmbientCredentials: Story = { + args: { + section: "providers" as ChatModelAdminSection, + providerConfigsData: [ + createProviderConfig({ + id: nilProviderConfigID, + provider: "bedrock", + display_name: "AWS Bedrock", + source: "supported", + enabled: false, + }), + ], + modelCatalogData: { providers: [] }, + }, + play: async ({ canvasElement, args }) => { + const body = within(canvasElement.ownerDocument.body); + await userEvent.click( + await body.findByRole("button", { name: /AWS Bedrock/i }), + ); + + const apiKeyInput = await body.findByLabelText(/^API Key$/i); + const createButton = body.getByRole("button", { + name: "Create provider config", + }); + + await expect(apiKeyInput).not.toBeRequired(); + await expect(apiKeyInput).toHaveAttribute( + "placeholder", + "Enter bearer token", + ); + await expect( + body.findByText( + "Bearer token for Bedrock authentication. Leave empty to use ambient AWS credentials.", + ), + ).resolves.toBeInTheDocument(); + await expect( + body.findByText( + /Overrides the Bedrock runtime endpoint\.\s+Set AWS_REGION on\s+the Coder server to select the target region\./i, + ), + ).resolves.toBeInTheDocument(); + await expect(createButton).toBeEnabled(); + + await userEvent.click(createButton); + await waitFor(() => { + expect(args.onCreateProvider).toHaveBeenCalledTimes(1); + }); + const createProviderMock = args.onCreateProvider as ReturnType; + const createRequest = createProviderMock.mock.calls[0][0] as Record< + string, + unknown + >; + expect(createRequest).toMatchObject({ + provider: "bedrock", + central_api_key_enabled: true, + allow_user_api_key: false, + allow_central_api_key_fallback: false, + }); + expect(createRequest).not.toHaveProperty("api_key"); + }, +}; + +export const ProviderFormBedrockBearerToken: Story = { + args: { + section: "providers" as ChatModelAdminSection, + providerConfigsData: [ + createProviderConfig({ + id: "provider-bedrock-bearer", + provider: "bedrock", + display_name: "AWS Bedrock", + has_api_key: true, + central_api_key_enabled: true, + allow_user_api_key: false, + allow_central_api_key_fallback: false, + }), + ], + modelCatalogData: { providers: [] }, + }, + play: async ({ canvasElement, args }) => { + const body = within(canvasElement.ownerDocument.body); + await userEvent.click( + await body.findByRole("button", { name: /AWS Bedrock/i }), + ); + + const apiKeyInput = await body.findByLabelText(/^API Key$/i); + const saveButton = body.getByRole("button", { name: "Save changes" }); + + await expect(apiKeyInput).not.toBeRequired(); + await expect(apiKeyInput).toHaveValue("••••••••••••••••"); + + await userEvent.click(apiKeyInput); + await userEvent.type(apiKeyInput, "bedrock-bearer-token"); + await waitFor(() => { + expect(saveButton).toBeEnabled(); + }); + await userEvent.click(saveButton); + + await waitFor(() => { + expect(args.onUpdateProvider).toHaveBeenCalledTimes(1); + }); + expect(args.onUpdateProvider).toHaveBeenCalledWith( + "provider-bedrock-bearer", + expect.objectContaining({ api_key: "bedrock-bearer-token" }), + ); + }, +}; + +export const ProviderFormBedrockClearBearerToken: Story = { + args: { + section: "providers" as ChatModelAdminSection, + providerConfigsData: [ + createProviderConfig({ + id: "provider-bedrock-clear", + provider: "bedrock", + display_name: "AWS Bedrock", + has_api_key: true, + central_api_key_enabled: true, + allow_user_api_key: false, + allow_central_api_key_fallback: false, + }), + ], + modelCatalogData: { providers: [] }, + }, + play: async ({ canvasElement, args }) => { + const body = within(canvasElement.ownerDocument.body); + await userEvent.click( + await body.findByRole("button", { name: /AWS Bedrock/i }), + ); + + const apiKeyInput = await body.findByLabelText(/^API Key$/i); + const clearStoredTokenButton = body.getByRole("button", { + name: /Clear stored token/i, + }); + const saveButton = body.getByRole("button", { name: "Save changes" }); + + await expect(apiKeyInput).toHaveValue("••••••••••••••••"); + await userEvent.click(clearStoredTokenButton); + await waitFor(() => { + expect(apiKeyInput).toHaveValue(""); + expect(saveButton).toBeEnabled(); + }); + await userEvent.click(saveButton); + + await waitFor(() => { + expect(args.onUpdateProvider).toHaveBeenCalledTimes(1); + }); + expect(args.onUpdateProvider).toHaveBeenCalledWith( + "provider-bedrock-clear", + expect.objectContaining({ api_key: "" }), + ); + }, +}; + const openAddModelForm = async ( body: ReturnType, providerLabel: string, diff --git a/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ChatModelAdminPanel.tsx b/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ChatModelAdminPanel.tsx index 67cb16e0b3..49e170611d 100644 --- a/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ChatModelAdminPanel.tsx +++ b/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ChatModelAdminPanel.tsx @@ -156,6 +156,9 @@ const useProviderStates = ( const label = readOptionalString(providerConfigEntry?.display_name) ?? formatProviderLabel(provider); + const hasBedrockAmbientCredentials = + provider === "bedrock" && + providerConfig?.central_api_key_enabled === true; const modelConfigsForProvider = modelConfigsByProvider.get(provider) ?? []; const isCatalogEnvPreset = !providerConfig && @@ -173,7 +176,7 @@ const useProviderStates = ( hasManagedAPIKey, hasCatalogAPIKey, hasEffectiveAPIKey: providerConfigEntry - ? hasProviderEntryAPIKey + ? hasProviderEntryAPIKey || hasBedrockAmbientCredentials : hasManagedAPIKey || hasCatalogAPIKey, isEnvPreset, baseURL: getProviderBaseURL(providerConfigEntry), diff --git a/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ProviderForm.tsx b/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ProviderForm.tsx index ccc4635575..ae11125844 100644 --- a/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ProviderForm.tsx +++ b/site/src/pages/AgentsPage/components/ChatModelAdminPanel/ProviderForm.tsx @@ -88,6 +88,7 @@ export const ProviderForm: FC = ({ providerState.hasManagedAPIKey ? API_KEY_PLACEHOLDER : "", ); const [apiKeyTouched, setApiKeyTouched] = useState(false); + const [apiKeyModified, setApiKeyModified] = useState(false); const [baseURLValue, setBaseURLValue] = useState(initialValues.baseURL); const [centralAPIKeyEnabled, setCentralAPIKeyEnabled] = useState( initialValues.centralAPIKeyEnabled, @@ -100,6 +101,7 @@ export const ProviderForm: FC = ({ ); const [confirmingDelete, setConfirmingDelete] = useState(false); + const isBedrockProvider = provider === "bedrock"; const isAPIKeyEnvManaged = isEnvPreset && !providerConfig; const shouldShowAPIKeyField = centralAPIKeyEnabled; const shouldShowFallbackToggle = centralAPIKeyEnabled && allowUserAPIKey; @@ -109,30 +111,56 @@ export const ProviderForm: FC = ({ initialValues.allowCentralAPIKeyFallback; const effectiveFallback = shouldShowFallbackToggle && allowCentralAPIKeyFallback; - // Require a key whenever central-key usage is enabled and there is no - // stored deployment key yet. This covers both create and update flows, - // including toggling central-key usage on for an existing provider. + // Most providers require a stored deployment key whenever central-key + // usage is enabled and there is no saved key yet. Bedrock can also use + // ambient AWS credentials from the Coder server, so its API key stays + // optional. const requiresAPIKey = !isAPIKeyEnvManaged && + !isBedrockProvider && centralAPIKeyEnabled && !providerState.hasManagedAPIKey; const effectiveApiKey = apiKeyTouched && apiKey !== API_KEY_PLACEHOLDER ? apiKey.trim() : ""; + const hasTypedAPIKey = effectiveApiKey.length > 0; + // Clearing a saved Bedrock bearer token switches the provider back + // to ambient AWS credentials, so updates must send an explicit + // empty string. + const isClearingBedrockAPIKey = + isBedrockProvider && + providerState.hasManagedAPIKey && + apiKeyModified && + effectiveApiKey === ""; + const hasPendingAPIKeyChange = + (centralAPIKeyEnabled && hasTypedAPIKey) || isClearingBedrockAPIKey; + const shouldCreateAPIKey = centralAPIKeyEnabled && hasTypedAPIKey; const hasCredentialSource = centralAPIKeyEnabled || allowUserAPIKey; + const apiKeyDescription = isBedrockProvider + ? "Bearer token for Bedrock authentication. Leave empty to use ambient AWS credentials." + : "Secret key used to authenticate requests to this provider."; + const baseURLDescription = isBedrockProvider + ? "Optional. Overrides the Bedrock runtime endpoint. Set AWS_REGION on the Coder server to select the target region." + : "Custom endpoint for this provider. Leave empty to use the default."; + const apiKeyPlaceholder = isBedrockProvider ? "Enter bearer token" : "sk-..."; const deleteProviderDescription = normalizedProviderConfig?.allow_user_api_key ? "Are you sure you want to delete this provider? Any personal API " + "keys that users have saved for this provider will also be " + "permanently deleted. This action is irreversible." : "Are you sure you want to delete this provider? This action is irreversible."; + // New Bedrock providers can be saved immediately with ambient AWS + // credentials, even before any fields differ from their defaults. + const hasNewBedrockAmbientConfiguration = + isBedrockProvider && !providerConfig && centralAPIKeyEnabled; const isDirty = displayName.trim() !== initialValues.displayName || - effectiveApiKey !== "" || + hasPendingAPIKeyChange || baseURLValue.trim() !== initialValues.baseURL.trim() || centralAPIKeyEnabled !== initialValues.centralAPIKeyEnabled || allowUserAPIKey !== initialValues.allowUserAPIKey || - effectiveFallback !== effectiveInitialFallback; + effectiveFallback !== effectiveInitialFallback || + hasNewBedrockAmbientConfiguration; const canSave = !providerConfigsUnavailable && @@ -140,7 +168,7 @@ export const ProviderForm: FC = ({ !isAPIKeyEnvManaged && isDirty && hasCredentialSource && - (!requiresAPIKey || effectiveApiKey); + (!requiresAPIKey || hasTypedAPIKey); const handleSubmit = async (event: FormEvent) => { event.preventDefault(); @@ -153,7 +181,7 @@ export const ProviderForm: FC = ({ return; } - if (requiresAPIKey && !effectiveApiKey) { + if (requiresAPIKey && !hasTypedAPIKey) { return; } @@ -168,8 +196,7 @@ export const ProviderForm: FC = ({ ...(trimmedDisplayName !== currentDisplayName && { display_name: trimmedDisplayName, }), - ...(centralAPIKeyEnabled && - effectiveApiKey && { api_key: effectiveApiKey }), + ...(hasPendingAPIKeyChange && { api_key: effectiveApiKey }), ...(trimmedBaseURL !== currentBaseURL && { base_url: trimmedBaseURL, }), @@ -198,7 +225,7 @@ export const ProviderForm: FC = ({ } else { const req: TypesGen.CreateChatProviderConfigRequest = { provider, - ...(centralAPIKeyEnabled && { api_key: effectiveApiKey }), + ...(shouldCreateAPIKey && { api_key: effectiveApiKey }), central_api_key_enabled: centralAPIKeyEnabled, allow_user_api_key: allowUserAPIKey, allow_central_api_key_fallback: effectiveFallback, @@ -218,6 +245,7 @@ export const ProviderForm: FC = ({ } setApiKeyTouched(false); + setApiKeyModified(false); setApiKey(API_KEY_PLACEHOLDER); }; @@ -280,36 +308,57 @@ export const ProviderForm: FC = ({ label="API Key" htmlFor={apiKeyInputId} required={requiresAPIKey} - description="Secret key used to authenticate requests to this provider." + description={apiKeyDescription} > - { - setApiKey(event.target.value); - setApiKeyTouched(true); - }} - disabled={isDisabled} - /> +
+ { + setApiKey(event.target.value); + setApiKeyTouched(true); + setApiKeyModified(true); + }} + disabled={isDisabled} + /> + {isBedrockProvider && + providerState.hasManagedAPIKey && + !isDisabled && + (!apiKeyModified || apiKey !== "") && ( +
+ +
+ )} +
)}