From dabcd718a41b762268b407dbbd885123308ce984 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 22 Jun 2026 09:13:30 +0000 Subject: [PATCH 1/7] fix: derive Bedrock regions on provider writes --- coderd/ai_providers.go | 33 ++++- coderd/ai_providers_test.go | 72 +++++++++++ codersdk/aiproviders.go | 16 ++- .../ProvidersPage/components/ProviderForm.tsx | 16 +-- .../components/providerFormApiMap.test.ts | 117 +----------------- .../components/providerFormApiMap.ts | 15 +-- 6 files changed, 123 insertions(+), 146 deletions(-) diff --git a/coderd/ai_providers.go b/coderd/ai_providers.go index 49e5bb8750403..d4496669f334b 100644 --- a/coderd/ai_providers.go +++ b/coderd/ai_providers.go @@ -7,6 +7,8 @@ import ( "errors" "fmt" "net/http" + "regexp" + "strings" "github.com/go-chi/chi/v5" "github.com/google/uuid" @@ -178,6 +180,8 @@ func (api *API) aiProvidersCreate(rw http.ResponseWriter, r *http.Request) { return } + req.Settings = normalizeAIProviderSettings(req.Type, req.BaseURL, req.Settings) + settings, err := encodeAIProviderSettings(req.Settings) if err != nil { api.Logger.Error(ctx, "encode AI provider settings", slog.Error(err)) @@ -320,6 +324,13 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) { if req.Settings != nil { existing = mergeAIProviderSettings(existing, *req.Settings) } + baseURL := ptr.NilToDefault(req.BaseURL, old.BaseUrl) + if req.BaseURL != nil && existing.Bedrock != nil { + if req.Settings == nil || req.Settings.Bedrock == nil || req.Settings.Bedrock.Region == "" { + existing.Bedrock.Region = "" + } + } + existing = normalizeAIProviderSettings(codersdk.AIProviderType(old.Type), baseURL, existing) // Bedrock settings are only meaningful for anthropic- or // bedrock-typed providers; rejecting the mismatch keeps a // misconfiguration from sitting silently in the encrypted @@ -354,7 +365,7 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) { Type: old.Type, DisplayName: displayName, Enabled: ptr.NilToDefault(req.Enabled, old.Enabled), - BaseUrl: ptr.NilToDefault(req.BaseURL, old.BaseUrl), + BaseUrl: baseURL, Settings: settings, // SettingsKeyID is set by the dbcrypt wrapper. SettingsKeyID: sql.NullString{}, @@ -738,6 +749,23 @@ func applyAIProviderKeyOps(ctx context.Context, tx database.Store, providerID uu // into a 400. var errAIProviderKeyUnknown = xerrors.New("api_keys references an unknown id for this provider") +var canonicalBedrockBaseURLRegex = regexp.MustCompile(`(?i)^https://bedrock-runtime\.([a-z0-9-]+)\.amazonaws\.com/?$`) + +func normalizeAIProviderSettings(providerType codersdk.AIProviderType, baseURL string, settings codersdk.AIProviderSettings) codersdk.AIProviderSettings { + if providerType == codersdk.AIProviderTypeBedrock && settings.Bedrock == nil { + settings.Bedrock = &codersdk.AIProviderBedrockSettings{} + } + if settings.Bedrock == nil || settings.Bedrock.Region != "" { + return settings + } + if match := canonicalBedrockBaseURLRegex.FindStringSubmatch(strings.TrimSpace(baseURL)); len(match) == 2 { + bedrock := *settings.Bedrock + bedrock.Region = strings.ToLower(match[1]) + settings.Bedrock = &bedrock + } + return settings +} + // encodeAIProviderSettings serializes a settings value into the // discriminated JSON form stored in ai_providers.settings. Empty // settings return an invalid sql.NullString so the row stores SQL NULL @@ -766,6 +794,9 @@ func mergeAIProviderSettings(existing, patch codersdk.AIProviderSettings) coders } merged := *patch.Bedrock if existing.Bedrock != nil { + if merged.Region == "" { + merged.Region = existing.Bedrock.Region + } if merged.AccessKey == nil { merged.AccessKey = existing.Bedrock.AccessKey } diff --git a/coderd/ai_providers_test.go b/coderd/ai_providers_test.go index b9bfd283f1c9e..90d6e7d1b2e93 100644 --- a/coderd/ai_providers_test.go +++ b/coderd/ai_providers_test.go @@ -625,6 +625,78 @@ func TestAIProvidersCRUD(t *testing.T) { require.NotContains(t, body, `"access_key"`) require.NotContains(t, body, `"access_key_secret"`) }) + + t.Run("BedrockDerivesRegionFromBaseURL", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitLong) + + //nolint:gocritic // Owner role is the audience for this endpoint. + created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: codersdk.AIProviderTypeAnthropic, + Name: "bedrock-region-derived", + Enabled: true, + BaseURL: "https://bedrock-runtime.US-EAST-1.amazonaws.com/", + Settings: codersdk.AIProviderSettings{ + Bedrock: &codersdk.AIProviderBedrockSettings{ + Model: "anthropic.claude-3-5-sonnet", + SmallFastModel: "anthropic.claude-3-5-haiku", + }, + }, + }) + require.NoError(t, err) + require.NotNil(t, created.Settings.Bedrock) + require.Equal(t, "us-east-1", created.Settings.Bedrock.Region) + + newURL := "https://bedrock-runtime.us-west-2.amazonaws.com" + updated, err := client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ + BaseURL: &newURL, + }) + require.NoError(t, err) + require.NotNil(t, updated.Settings.Bedrock) + require.Equal(t, "us-west-2", updated.Settings.Bedrock.Region) + + explicitRegion := "us-gov-west-1" + updated, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ + BaseURL: ptr.Ref("https://bedrock-runtime.us-east-2.amazonaws.com"), + Settings: &codersdk.AIProviderSettings{ + Bedrock: &codersdk.AIProviderBedrockSettings{Region: explicitRegion}, + }, + }) + require.NoError(t, err) + require.NotNil(t, updated.Settings.Bedrock) + require.Equal(t, explicitRegion, updated.Settings.Bedrock.Region) + + updated, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ + Settings: &codersdk.AIProviderSettings{ + Bedrock: &codersdk.AIProviderBedrockSettings{Model: "anthropic.claude-3-5-haiku"}, + }, + }) + require.NoError(t, err) + require.NotNil(t, updated.Settings.Bedrock) + require.Equal(t, explicitRegion, updated.Settings.Bedrock.Region) + }) + + t.Run("BedrockTypeDerivesRegionWithoutSettings", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitLong) + + //nolint:gocritic // Owner role is the audience for this endpoint. + created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: codersdk.AIProviderTypeBedrock, + Name: "bedrock-type-region", + Enabled: true, + BaseURL: "https://bedrock-runtime.us-east-2.amazonaws.com", + }) + require.NoError(t, err) + require.Equal(t, codersdk.AIProviderTypeBedrock, created.Type) + require.NotNil(t, created.Settings.Bedrock) + require.Equal(t, "us-east-2", created.Settings.Bedrock.Region) + }) + } func TestAIProvidersKeyManagement(t *testing.T) { diff --git a/codersdk/aiproviders.go b/codersdk/aiproviders.go index 7b513340bca62..9454b190453d2 100644 --- a/codersdk/aiproviders.go +++ b/codersdk/aiproviders.go @@ -234,11 +234,17 @@ func (req CreateAIProviderRequest) Validate() []ValidationError { Detail: "bedrock settings are only valid for type=anthropic or type=bedrock", }) } - if req.Type == AIProviderTypeBedrock && (req.Settings.Bedrock == nil || !req.Settings.Bedrock.IsConfigured()) { - validations = append(validations, ValidationError{ - Field: "settings", - Detail: "type=bedrock requires bedrock settings", - }) + if req.Type == AIProviderTypeBedrock { + bedrock := AIProviderBedrockSettings{} + if req.Settings.Bedrock != nil { + bedrock = *req.Settings.Bedrock + } + if !IsBedrockConfigured(req.BaseURL, bedrock) { + validations = append(validations, ValidationError{ + Field: "settings", + Detail: "type=bedrock requires bedrock settings", + }) + } } if req.Type == AIProviderTypeBedrock && len(req.APIKeys) > 0 { validations = append(validations, ValidationError{ diff --git a/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx b/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx index 99a1d24a92945..58f3d83e68fda 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx +++ b/site/src/pages/AISettingsPage/ProvidersPage/components/ProviderForm.tsx @@ -30,19 +30,10 @@ export type ProviderFormValues = { }; const HTTP_SCHEME_REGEX = /^https?:\/\//i; -const BEDROCK_CANONICAL_URL_REGEX = - /^https:\/\/bedrock-runtime\.([a-z0-9-]+)\.amazonaws\.com\/?$/i; const PROVIDER_NAME_REGEX = /^[a-z0-9]+(-[a-z0-9]+)*$/; export const SAVED_CREDENTIAL_MASK = "********"; -export const parseBedrockRegionFromBaseUrl = ( - baseUrl: string, -): string | undefined => { - const match = BEDROCK_CANONICAL_URL_REGEX.exec(baseUrl.trim()); - return match?.[1]?.toLowerCase(); -}; - const makeNameSchema = (editing: boolean) => editing ? Yup.string() @@ -160,10 +151,7 @@ const makeBedrockSchema = (editing: boolean) => displayName: makeDisplayNameSchema(editing), baseUrl: Yup.string() .url("Endpoint must be a valid URL") - .matches( - BEDROCK_CANONICAL_URL_REGEX, - "Endpoint must be a standard AWS Bedrock URL.", - ) + .matches(HTTP_SCHEME_REGEX, "Endpoint must use http or https.") .required("Endpoint is required"), apiKey: Yup.string(), model: Yup.string().required("Model is required"), @@ -461,7 +449,7 @@ export const ProviderForm: FC = ({ label="Endpoint" description={ <> - In the format of{" "} + For standard AWS endpoints, use{" "} {"https://bedrock-runtime.{region}.amazonaws.com"} diff --git a/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.test.ts b/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.test.ts index 1ae786008cc36..e4ed977f5cdbd 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.test.ts +++ b/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.test.ts @@ -6,11 +6,7 @@ import { MockAIProviderCopilot, MockAIProviderOpenAI, } from "#/testHelpers/entities"; -import { - type ProviderFormValues, - parseBedrockRegionFromBaseUrl, - SAVED_CREDENTIAL_MASK, -} from "./ProviderForm"; +import { type ProviderFormValues, SAVED_CREDENTIAL_MASK } from "./ProviderForm"; import { aiProviderToFormValues, getProviderDisplayType, @@ -65,76 +61,6 @@ const baseCopilotFormValues: ProviderFormValues = { const settings = (raw: Record): AIProvider["settings"] => raw as unknown as AIProvider["settings"]; -describe("parseBedrockRegionFromBaseUrl", () => { - it("extracts the region from a canonical AWS Bedrock URL", () => { - expect( - parseBedrockRegionFromBaseUrl( - "https://bedrock-runtime.us-east-1.amazonaws.com", - ), - ).toBe("us-east-1"); - }); - - it("accepts a trailing slash", () => { - expect( - parseBedrockRegionFromBaseUrl( - "https://bedrock-runtime.us-west-2.amazonaws.com/", - ), - ).toBe("us-west-2"); - }); - - it("lowercases the region", () => { - expect( - parseBedrockRegionFromBaseUrl( - "https://bedrock-runtime.US-EAST-1.amazonaws.com", - ), - ).toBe("us-east-1"); - }); - - it("trims surrounding whitespace before matching", () => { - expect( - parseBedrockRegionFromBaseUrl( - " https://bedrock-runtime.us-east-1.amazonaws.com ", - ), - ).toBe("us-east-1"); - }); - - it("returns undefined for a non-AWS URL", () => { - expect( - parseBedrockRegionFromBaseUrl("https://bedrock.internal.example.com"), - ).toBeUndefined(); - }); - - it("returns undefined for an empty string", () => { - expect(parseBedrockRegionFromBaseUrl("")).toBeUndefined(); - }); - - it("returns undefined for an http (non-https) URL", () => { - expect( - parseBedrockRegionFromBaseUrl( - "http://bedrock-runtime.us-east-1.amazonaws.com", - ), - ).toBeUndefined(); - }); - - it("returns undefined for a URL with a path", () => { - expect( - parseBedrockRegionFromBaseUrl( - "https://bedrock-runtime.us-east-1.amazonaws.com/v1/something", - ), - ).toBeUndefined(); - }); - - it("returns undefined for the China partition (different TLD)", () => { - // AWS China uses *.amazonaws.com.cn, which the canonical regex does - // not match by design; cn-* users get the explicit Region input. - expect( - parseBedrockRegionFromBaseUrl( - "https://bedrock-runtime.cn-north-1.amazonaws.com.cn", - ), - ).toBeUndefined(); - }); -}); - describe("isBedrockProvider", () => { it("recognises a discriminated bedrock provider", () => { expect(isBedrockProvider(MockAIProviderBedrock)).toBe(true); @@ -334,22 +260,10 @@ describe("providerFormValuesToCreate", () => { expect(req.type).toBe("anthropic"); }); - it("derives the region from a canonical AWS URL", () => { + it("leaves region derivation to the backend", () => { const req = providerFormValuesToCreate(baseBedrockFormValues); const s = req.settings as unknown as Record; expect(s._type).toBe("bedrock"); - expect(s.region).toBe("us-east-1"); - }); - - it("omits the region when the URL is non-canonical", () => { - // The form schema blocks non-canonical endpoints before submit; the - // helper itself stays strict, returning an undefined region rather - // than inventing a value. - const req = providerFormValuesToCreate({ - ...baseBedrockFormValues, - baseUrl: "https://bedrock.internal.example.com", - }); - const s = req.settings as unknown as Record; expect(s.region).toBeUndefined(); }); @@ -371,18 +285,14 @@ describe("providerFormValuesToCreate", () => { expect(s.access_key_secret).toBeUndefined(); }); - it("keeps the region so the backend recognises the Bedrock provider when access keys are omitted", () => { - // The backend treats Region as a configuration signal - // (codersdk.AIProviderBedrockSettings.IsConfigured), so omitting - // the keys must not also strip the region; otherwise the request - // would fail with "type=bedrock requires bedrock settings". + it("omits the region when access keys are omitted", () => { const req = providerFormValuesToCreate({ ...baseBedrockFormValues, accessKey: "", accessKeySecret: "", }); const s = req.settings as unknown as Record; - expect(s.region).toBe("us-east-1"); + expect(s.region).toBeUndefined(); expect(s._type).toBe("bedrock"); }); @@ -484,7 +394,7 @@ describe("providerFormValuesToUpdate", () => { }); describe("Bedrock", () => { - it("derives the region from the canonical URL", () => { + it("leaves region derivation to the backend", () => { const req = providerFormValuesToUpdate( { ...baseBedrockFormValues, @@ -495,23 +405,6 @@ describe("providerFormValuesToUpdate", () => { MockAIProviderBedrock, ); const s = req.settings as unknown as Record; - expect(s.region).toBe("us-west-2"); - }); - - it("omits the region when the URL is non-canonical", () => { - // The form schema blocks non-canonical endpoints before submit; the - // helper itself stays strict, returning an undefined region rather - // than inventing a value. - const req = providerFormValuesToUpdate( - { - ...baseBedrockFormValues, - baseUrl: "https://bedrock.internal.example.com", - accessKey: SAVED_CREDENTIAL_MASK, - accessKeySecret: SAVED_CREDENTIAL_MASK, - }, - MockAIProviderBedrock, - ); - const s = req.settings as unknown as Record; expect(s.region).toBeUndefined(); }); diff --git a/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.ts b/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.ts index 67eec7e4d913a..1df857d0fad23 100644 --- a/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.ts +++ b/site/src/pages/AISettingsPage/ProvidersPage/components/providerFormApiMap.ts @@ -7,11 +7,7 @@ import type { CreateAIProviderRequest, UpdateAIProviderRequest, } from "#/api/typesGenerated"; -import { - type ProviderFormValues, - parseBedrockRegionFromBaseUrl, - SAVED_CREDENTIAL_MASK, -} from "./ProviderForm"; +import { type ProviderFormValues, SAVED_CREDENTIAL_MASK } from "./ProviderForm"; /** Drop placeholder masks so they don't round-trip back to the API. */ const sanitizeCredential = ( @@ -104,7 +100,6 @@ export const getProviderDisplayType = ( }; const buildBedrockSettings = ( - region: string | undefined, model: string, smallFastModel: string, accessKey: string, @@ -112,7 +107,6 @@ const buildBedrockSettings = ( ): BedrockSettingsWire => ({ _type: BEDROCK_SETTINGS_TYPE, _version: BEDROCK_SETTINGS_VERSION, - ...(region ? { region } : {}), model, small_fast_model: smallFastModel, ...(accessKey ? { access_key: accessKey } : {}), @@ -134,9 +128,7 @@ export const providerFormValuesToCreate = ( }; if (values.type === "bedrock") { - const region = parseBedrockRegionFromBaseUrl(base.base_url); const settings = buildBedrockSettings( - region, values.model.trim(), values.smallFastModel.trim(), sanitizeCredential(values.accessKey), @@ -205,12 +197,7 @@ export const providerFormValuesToUpdate = ( // the user is rotating credentials. const credentialsChanged = newAccessKey !== "" && newAccessKeySecret !== ""; - // Yup blocks non-canonical Bedrock URLs upstream, so any `undefined` - // region here is a real bug that should surface, not be papered over. - const region = parseBedrockRegionFromBaseUrl(base.base_url ?? ""); - const settings = buildBedrockSettings( - region, values.model.trim(), values.smallFastModel.trim(), credentialsChanged ? newAccessKey : "", From 3fe6b31f0740435c0f155afea2259ed314b5985e Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 22 Jun 2026 09:21:23 +0000 Subject: [PATCH 2/7] style(coderd): format AI provider test --- coderd/ai_providers_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/coderd/ai_providers_test.go b/coderd/ai_providers_test.go index 90d6e7d1b2e93..55dc2de5e8de3 100644 --- a/coderd/ai_providers_test.go +++ b/coderd/ai_providers_test.go @@ -696,7 +696,6 @@ func TestAIProvidersCRUD(t *testing.T) { require.NotNil(t, created.Settings.Bedrock) require.Equal(t, "us-east-2", created.Settings.Bedrock.Region) }) - } func TestAIProvidersKeyManagement(t *testing.T) { From b581ae94dedcd463913e457c408bdd209f2f31b3 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 22 Jun 2026 09:44:38 +0000 Subject: [PATCH 3/7] test(coderd): shorten Bedrock region tests --- coderd/ai_providers_test.go | 47 +++++++++++-------------------------- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/coderd/ai_providers_test.go b/coderd/ai_providers_test.go index 55dc2de5e8de3..8a78bb2b91219 100644 --- a/coderd/ai_providers_test.go +++ b/coderd/ai_providers_test.go @@ -634,48 +634,32 @@ func TestAIProvidersCRUD(t *testing.T) { //nolint:gocritic // Owner role is the audience for this endpoint. created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ - Type: codersdk.AIProviderTypeAnthropic, - Name: "bedrock-region-derived", - Enabled: true, - BaseURL: "https://bedrock-runtime.US-EAST-1.amazonaws.com/", - Settings: codersdk.AIProviderSettings{ - Bedrock: &codersdk.AIProviderBedrockSettings{ - Model: "anthropic.claude-3-5-sonnet", - SmallFastModel: "anthropic.claude-3-5-haiku", - }, - }, + Type: codersdk.AIProviderTypeAnthropic, + Name: "bedrock-region-derived", + Enabled: true, + BaseURL: "https://bedrock-runtime.US-EAST-1.amazonaws.com/", + Settings: codersdk.AIProviderSettings{Bedrock: &codersdk.AIProviderBedrockSettings{}}, }) require.NoError(t, err) require.NotNil(t, created.Settings.Bedrock) require.Equal(t, "us-east-1", created.Settings.Bedrock.Region) - newURL := "https://bedrock-runtime.us-west-2.amazonaws.com" updated, err := client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ - BaseURL: &newURL, + BaseURL: ptr.Ref("https://bedrock-runtime.us-west-2.amazonaws.com"), }) require.NoError(t, err) require.NotNil(t, updated.Settings.Bedrock) require.Equal(t, "us-west-2", updated.Settings.Bedrock.Region) - explicitRegion := "us-gov-west-1" updated, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ BaseURL: ptr.Ref("https://bedrock-runtime.us-east-2.amazonaws.com"), Settings: &codersdk.AIProviderSettings{ - Bedrock: &codersdk.AIProviderBedrockSettings{Region: explicitRegion}, - }, - }) - require.NoError(t, err) - require.NotNil(t, updated.Settings.Bedrock) - require.Equal(t, explicitRegion, updated.Settings.Bedrock.Region) - - updated, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ - Settings: &codersdk.AIProviderSettings{ - Bedrock: &codersdk.AIProviderBedrockSettings{Model: "anthropic.claude-3-5-haiku"}, + Bedrock: &codersdk.AIProviderBedrockSettings{Region: "us-gov-west-1"}, }, }) require.NoError(t, err) require.NotNil(t, updated.Settings.Bedrock) - require.Equal(t, explicitRegion, updated.Settings.Bedrock.Region) + require.Equal(t, "us-gov-west-1", updated.Settings.Bedrock.Region) }) t.Run("BedrockTypeDerivesRegionWithoutSettings", func(t *testing.T) { @@ -1428,11 +1412,11 @@ func TestAIProvidersKeyManagement(t *testing.T) { func TestAIProviderSettingsMerge(t *testing.T) { t.Parallel() - t.Run("OmittedSecretsPreserveExisting", func(t *testing.T) { + t.Run("OmittedRegionAndSecretsPreserveExisting", func(t *testing.T) { t.Parallel() - // A PATCH that only rotates non-secret fields must keep the - // existing AccessKey and AccessKeySecret intact so the provider - // keeps authenticating after the update. + // A PATCH that omits Region, AccessKey, and AccessKeySecret must + // keep the existing values so the provider keeps authenticating + // after the update. client, db := coderdtest.NewWithDatabase(t, nil) _ = coderdtest.CreateFirstUser(t, client) ctx := testutil.Context(t, testutil.WaitLong) @@ -1456,10 +1440,7 @@ func TestAIProviderSettingsMerge(t *testing.T) { _, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ Settings: &codersdk.AIProviderSettings{ - Bedrock: &codersdk.AIProviderBedrockSettings{ - Region: "us-west-2", - Model: "anthropic.claude-3-5-haiku", - }, + Bedrock: &codersdk.AIProviderBedrockSettings{Model: "anthropic.claude-3-5-haiku"}, }, }) require.NoError(t, err) @@ -1470,7 +1451,7 @@ func TestAIProviderSettingsMerge(t *testing.T) { persisted, err := db2sdk.AIProviderSettings(row.Settings) require.NoError(t, err) require.NotNil(t, persisted.Bedrock) - require.Equal(t, "us-west-2", persisted.Bedrock.Region) + require.Equal(t, "us-east-1", persisted.Bedrock.Region) require.Equal(t, "anthropic.claude-3-5-haiku", persisted.Bedrock.Model) require.NotNil(t, persisted.Bedrock.AccessKey) require.Equal(t, "AKIA-old", *persisted.Bedrock.AccessKey) From 5a1c1c00425261a2319dc648967c97a4cfe12f52 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 22 Jun 2026 10:19:21 +0000 Subject: [PATCH 4/7] fix: reject Bedrock providers that cannot resolve a region Bedrock create validation accepted any base_url as sufficient, so a native type=bedrock provider with a non-canonical endpoint and no region or credentials could be persisted, then silently skipped at runtime where bedrockConfigFromRow requires settings.Bedrock.IsConfigured(). Validate Bedrock readiness on the write path after region derivation: canonical AWS endpoints still derive a region for free, while custom endpoints must supply a region or credentials. Updates that switch a Bedrock provider to a non-derivable endpoint without a region are rejected too. --- coderd/ai_providers.go | 25 +++++++++++++++++++++++++ coderd/ai_providers_test.go | 34 ++++++++++++++++++++++++++++++++++ codersdk/aiproviders.go | 12 ------------ 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/coderd/ai_providers.go b/coderd/ai_providers.go index d4496669f334b..d02dd97bdd517 100644 --- a/coderd/ai_providers.go +++ b/coderd/ai_providers.go @@ -181,6 +181,17 @@ func (api *API) aiProvidersCreate(rw http.ResponseWriter, r *http.Request) { } req.Settings = normalizeAIProviderSettings(req.Type, req.BaseURL, req.Settings) + if req.Type == codersdk.AIProviderTypeBedrock && + (req.Settings.Bedrock == nil || !req.Settings.Bedrock.IsConfigured()) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid AI provider request.", + Validations: []codersdk.ValidationError{{ + Field: "settings", + Detail: errAIProviderBedrockSettingsRequired.Error(), + }}, + }) + return + } settings, err := encodeAIProviderSettings(req.Settings) if err != nil { @@ -340,6 +351,10 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) { old.Type != database.AIProviderTypeBedrock { return errAIProviderBedrockTypeMismatch } + if old.Type == database.AIProviderTypeBedrock && + (existing.Bedrock == nil || !existing.Bedrock.IsConfigured()) { + return errAIProviderBedrockSettingsRequired + } settings, err := encodeAIProviderSettings(existing) if err != nil { return xerrors.Errorf("encode settings: %w", err) @@ -411,6 +426,12 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) { }) return } + if errors.Is(err, errAIProviderBedrockSettingsRequired) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: errAIProviderBedrockSettingsRequired.Error(), + }) + return + } if errors.Is(err, errAIProviderKeyUnknown) { // Use the sentinel directly so the response message does not // leak the "execute transaction:" wrapper xerrors added on the @@ -517,6 +538,10 @@ var errCopilotRejectsAPIKeys = xerrors.New("copilot providers do not accept api_ // the outer handler translates it into a 400. var errAIProviderBedrockTypeMismatch = xerrors.New("bedrock settings are only valid for type=anthropic or type=bedrock") +// errAIProviderBedrockSettingsRequired is returned when a Bedrock +// provider's settings cannot produce runtime Bedrock config. +var errAIProviderBedrockSettingsRequired = xerrors.New("type=bedrock requires a Bedrock region or credentials") + // errAIProviderInvalidName is returned from lookupAIProvider when the // idOrName parameter is neither a UUID nor a syntactically-valid name. // The handler translates this into a 400 so an integrator gets a hint diff --git a/coderd/ai_providers_test.go b/coderd/ai_providers_test.go index 8a78bb2b91219..5ec4cf4c0a248 100644 --- a/coderd/ai_providers_test.go +++ b/coderd/ai_providers_test.go @@ -680,6 +680,40 @@ func TestAIProvidersCRUD(t *testing.T) { require.NotNil(t, created.Settings.Bedrock) require.Equal(t, "us-east-2", created.Settings.Bedrock.Region) }) + + t.Run("BedrockTypeRejectsCustomBaseURLWithoutRegion", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + ctx := testutil.Context(t, testutil.WaitLong) + const required = "type=bedrock requires a Bedrock region or credentials" + + //nolint:gocritic // Owner role is the audience for this endpoint. + _, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: codersdk.AIProviderTypeBedrock, + Name: "bedrock-type-custom-no-region", + Enabled: true, + BaseURL: "https://bedrock.internal.example.com", + }) + sdkErr := requireSDKError(t, err, http.StatusBadRequest) + require.Equal(t, "Invalid AI provider request.", sdkErr.Message) + require.Contains(t, sdkErr.Validations, codersdk.ValidationError{Field: "settings", Detail: required}) + + //nolint:gocritic // Owner role is the audience for this endpoint. + created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: codersdk.AIProviderTypeBedrock, + Name: "bedrock-type-custom-update-no-region", + Enabled: true, + BaseURL: "https://bedrock-runtime.us-east-1.amazonaws.com", + }) + require.NoError(t, err) + + _, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ + BaseURL: ptr.Ref("https://bedrock.internal.example.com"), + }) + sdkErr = requireSDKError(t, err, http.StatusBadRequest) + require.Equal(t, required, sdkErr.Message) + }) } func TestAIProvidersKeyManagement(t *testing.T) { diff --git a/codersdk/aiproviders.go b/codersdk/aiproviders.go index 9454b190453d2..c0169db2ff5da 100644 --- a/codersdk/aiproviders.go +++ b/codersdk/aiproviders.go @@ -234,18 +234,6 @@ func (req CreateAIProviderRequest) Validate() []ValidationError { Detail: "bedrock settings are only valid for type=anthropic or type=bedrock", }) } - if req.Type == AIProviderTypeBedrock { - bedrock := AIProviderBedrockSettings{} - if req.Settings.Bedrock != nil { - bedrock = *req.Settings.Bedrock - } - if !IsBedrockConfigured(req.BaseURL, bedrock) { - validations = append(validations, ValidationError{ - Field: "settings", - Detail: "type=bedrock requires bedrock settings", - }) - } - } if req.Type == AIProviderTypeBedrock && len(req.APIKeys) > 0 { validations = append(validations, ValidationError{ Field: "api_keys", From 1fb3842c344cf40d8274e7e7a248c19beb406179 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 22 Jun 2026 10:50:33 +0000 Subject: [PATCH 5/7] fix(coderd): reject unconfigured Bedrock settings on any provider type The previous guard only fired for type=bedrock, but the UI creates Bedrock providers as type=anthropic with a settings.bedrock discriminator. After the frontend began accepting non-canonical endpoints, a form submit with a custom base_url and blank credentials persisted a Bedrock settings blob with no region. At runtime bedrockConfigFromRow returns nil for such a row, so the enabled provider is silently dropped or treated as a plain Anthropic provider. Gate create and update validation on settings.Bedrock != nil rather than the provider type, so any write carrying Bedrock settings must resolve to usable config after region derivation. This matches the single invariant aibridge already enforces. The two near-duplicate reject tests are merged into one table-driven subtest. --- coderd/ai_providers.go | 19 +++++---- coderd/ai_providers_test.go | 82 ++++++++++++++++++++++++++----------- 2 files changed, 69 insertions(+), 32 deletions(-) diff --git a/coderd/ai_providers.go b/coderd/ai_providers.go index d02dd97bdd517..4424cd0cab541 100644 --- a/coderd/ai_providers.go +++ b/coderd/ai_providers.go @@ -181,8 +181,11 @@ func (api *API) aiProvidersCreate(rw http.ResponseWriter, r *http.Request) { } req.Settings = normalizeAIProviderSettings(req.Type, req.BaseURL, req.Settings) - if req.Type == codersdk.AIProviderTypeBedrock && - (req.Settings.Bedrock == nil || !req.Settings.Bedrock.IsConfigured()) { + // Any provider carrying Bedrock settings must resolve to usable config. + // The UI submits Bedrock as type=anthropic with a bedrock discriminator, + // so gate on the settings blob rather than the provider type; otherwise + // aibridge silently drops the provider at runtime. + if req.Settings.Bedrock != nil && !req.Settings.Bedrock.IsConfigured() { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid AI provider request.", Validations: []codersdk.ValidationError{{ @@ -351,8 +354,10 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) { old.Type != database.AIProviderTypeBedrock { return errAIProviderBedrockTypeMismatch } - if old.Type == database.AIProviderTypeBedrock && - (existing.Bedrock == nil || !existing.Bedrock.IsConfigured()) { + // The mismatch check above guarantees a non-nil Bedrock blob only + // survives on anthropic- or bedrock-typed providers, so any such + // blob must resolve to usable config regardless of the type. + if existing.Bedrock != nil && !existing.Bedrock.IsConfigured() { return errAIProviderBedrockSettingsRequired } settings, err := encodeAIProviderSettings(existing) @@ -538,9 +543,9 @@ var errCopilotRejectsAPIKeys = xerrors.New("copilot providers do not accept api_ // the outer handler translates it into a 400. var errAIProviderBedrockTypeMismatch = xerrors.New("bedrock settings are only valid for type=anthropic or type=bedrock") -// errAIProviderBedrockSettingsRequired is returned when a Bedrock -// provider's settings cannot produce runtime Bedrock config. -var errAIProviderBedrockSettingsRequired = xerrors.New("type=bedrock requires a Bedrock region or credentials") +// errAIProviderBedrockSettingsRequired is returned when a provider's +// Bedrock settings cannot produce runtime Bedrock config. +var errAIProviderBedrockSettingsRequired = xerrors.New("Bedrock settings require a region or credentials") // errAIProviderInvalidName is returned from lookupAIProvider when the // idOrName parameter is neither a UUID nor a syntactically-valid name. diff --git a/coderd/ai_providers_test.go b/coderd/ai_providers_test.go index 5ec4cf4c0a248..f6c2a51427c97 100644 --- a/coderd/ai_providers_test.go +++ b/coderd/ai_providers_test.go @@ -681,38 +681,70 @@ func TestAIProvidersCRUD(t *testing.T) { require.Equal(t, "us-east-2", created.Settings.Bedrock.Region) }) - t.Run("BedrockTypeRejectsCustomBaseURLWithoutRegion", func(t *testing.T) { + t.Run("BedrockRejectsCustomBaseURLWithoutRegion", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) _ = coderdtest.CreateFirstUser(t, client) ctx := testutil.Context(t, testutil.WaitLong) - const required = "type=bedrock requires a Bedrock region or credentials" + const required = "Bedrock settings require a region or credentials" + + // A provider that can't resolve a region (custom endpoint, no region, + // no credentials) must be rejected, otherwise aibridge silently drops + // it at runtime. The native form sends type=bedrock while the UI sends + // type=anthropic with a bedrock settings discriminator, so both shapes + // must be guarded. + cases := []struct { + name string + providerType codersdk.AIProviderType + settings codersdk.AIProviderSettings + }{ + { + name: "bedrock-type", + providerType: codersdk.AIProviderTypeBedrock, + }, + { + name: "anthropic-type", + providerType: codersdk.AIProviderTypeAnthropic, + settings: codersdk.AIProviderSettings{Bedrock: &codersdk.AIProviderBedrockSettings{}}, + }, + } - //nolint:gocritic // Owner role is the audience for this endpoint. - _, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ - Type: codersdk.AIProviderTypeBedrock, - Name: "bedrock-type-custom-no-region", - Enabled: true, - BaseURL: "https://bedrock.internal.example.com", - }) - sdkErr := requireSDKError(t, err, http.StatusBadRequest) - require.Equal(t, "Invalid AI provider request.", sdkErr.Message) - require.Contains(t, sdkErr.Validations, codersdk.ValidationError{Field: "settings", Detail: required}) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // A custom endpoint can't derive a region, so create is rejected. + //nolint:gocritic // Owner role is the audience for this endpoint. + _, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: tc.providerType, + Name: tc.name + "-custom", + Enabled: true, + BaseURL: "https://bedrock.internal.example.com", + Settings: tc.settings, + }) + sdkErr := requireSDKError(t, err, http.StatusBadRequest) + require.Equal(t, "Invalid AI provider request.", sdkErr.Message) + require.Contains(t, sdkErr.Validations, codersdk.ValidationError{Field: "settings", Detail: required}) - //nolint:gocritic // Owner role is the audience for this endpoint. - created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ - Type: codersdk.AIProviderTypeBedrock, - Name: "bedrock-type-custom-update-no-region", - Enabled: true, - BaseURL: "https://bedrock-runtime.us-east-1.amazonaws.com", - }) - require.NoError(t, err) + // A canonical endpoint derives the region server-side, so the + // same shape succeeds without an explicit region. + //nolint:gocritic // Owner role is the audience for this endpoint. + created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: tc.providerType, + Name: tc.name + "-canonical", + Enabled: true, + BaseURL: "https://bedrock-runtime.us-east-1.amazonaws.com", + Settings: tc.settings, + }) + require.NoError(t, err) - _, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ - BaseURL: ptr.Ref("https://bedrock.internal.example.com"), - }) - sdkErr = requireSDKError(t, err, http.StatusBadRequest) - require.Equal(t, required, sdkErr.Message) + // Swapping back to a custom endpoint clears the derived region, + // so the update is rejected rather than storing a no-region row. + _, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ + BaseURL: ptr.Ref("https://bedrock.internal.example.com"), + }) + sdkErr = requireSDKError(t, err, http.StatusBadRequest) + require.Equal(t, required, sdkErr.Message) + }) + } }) } From 88aed8ecd24581b649682e879fdf8fe87bdc90fc Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Mon, 22 Jun 2026 15:13:00 +0000 Subject: [PATCH 6/7] fix(coderd): preserve Bedrock regions for custom endpoints --- coderd/ai_providers.go | 26 ++++-- coderd/ai_providers_test.go | 174 +++++++++++++++++++++--------------- 2 files changed, 121 insertions(+), 79 deletions(-) diff --git a/coderd/ai_providers.go b/coderd/ai_providers.go index 4424cd0cab541..6fdf553c9ab1a 100644 --- a/coderd/ai_providers.go +++ b/coderd/ai_providers.go @@ -180,7 +180,13 @@ func (api *API) aiProvidersCreate(rw http.ResponseWriter, r *http.Request) { return } - req.Settings = normalizeAIProviderSettings(req.Type, req.BaseURL, req.Settings) + bedrockRegionBaseURL := "" + if req.Type == codersdk.AIProviderTypeBedrock && req.Settings.Bedrock == nil { + bedrockRegionBaseURL = req.BaseURL + } else if req.Settings.Bedrock != nil && req.Settings.Bedrock.Region == "" { + bedrockRegionBaseURL = req.BaseURL + } + req.Settings = normalizeAIProviderSettings(req.Type, req.Settings, bedrockRegionBaseURL) // Any provider carrying Bedrock settings must resolve to usable config. // The UI submits Bedrock as type=anthropic with a bedrock discriminator, // so gate on the settings blob rather than the provider type; otherwise @@ -339,12 +345,14 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) { existing = mergeAIProviderSettings(existing, *req.Settings) } baseURL := ptr.NilToDefault(req.BaseURL, old.BaseUrl) - if req.BaseURL != nil && existing.Bedrock != nil { - if req.Settings == nil || req.Settings.Bedrock == nil || req.Settings.Bedrock.Region == "" { - existing.Bedrock.Region = "" - } + bedrockRegionBaseURL := "" + if codersdk.AIProviderType(old.Type) == codersdk.AIProviderTypeBedrock && existing.Bedrock == nil { + bedrockRegionBaseURL = baseURL + } else if req.BaseURL != nil && existing.Bedrock != nil && + (req.Settings == nil || req.Settings.Bedrock == nil || req.Settings.Bedrock.Region == "") { + bedrockRegionBaseURL = baseURL } - existing = normalizeAIProviderSettings(codersdk.AIProviderType(old.Type), baseURL, existing) + existing = normalizeAIProviderSettings(codersdk.AIProviderType(old.Type), existing, bedrockRegionBaseURL) // Bedrock settings are only meaningful for anthropic- or // bedrock-typed providers; rejecting the mismatch keeps a // misconfiguration from sitting silently in the encrypted @@ -781,14 +789,14 @@ var errAIProviderKeyUnknown = xerrors.New("api_keys references an unknown id for var canonicalBedrockBaseURLRegex = regexp.MustCompile(`(?i)^https://bedrock-runtime\.([a-z0-9-]+)\.amazonaws\.com/?$`) -func normalizeAIProviderSettings(providerType codersdk.AIProviderType, baseURL string, settings codersdk.AIProviderSettings) codersdk.AIProviderSettings { +func normalizeAIProviderSettings(providerType codersdk.AIProviderType, settings codersdk.AIProviderSettings, bedrockRegionBaseURL string) codersdk.AIProviderSettings { if providerType == codersdk.AIProviderTypeBedrock && settings.Bedrock == nil { settings.Bedrock = &codersdk.AIProviderBedrockSettings{} } - if settings.Bedrock == nil || settings.Bedrock.Region != "" { + if settings.Bedrock == nil || bedrockRegionBaseURL == "" { return settings } - if match := canonicalBedrockBaseURLRegex.FindStringSubmatch(strings.TrimSpace(baseURL)); len(match) == 2 { + if match := canonicalBedrockBaseURLRegex.FindStringSubmatch(strings.TrimSpace(bedrockRegionBaseURL)); len(match) == 2 { bedrock := *settings.Bedrock bedrock.Region = strings.ToLower(match[1]) settings.Bedrock = &bedrock diff --git a/coderd/ai_providers_test.go b/coderd/ai_providers_test.go index f6c2a51427c97..29135e62287c2 100644 --- a/coderd/ai_providers_test.go +++ b/coderd/ai_providers_test.go @@ -632,114 +632,148 @@ func TestAIProvidersCRUD(t *testing.T) { _ = coderdtest.CreateFirstUser(t, client) ctx := testutil.Context(t, testutil.WaitLong) - //nolint:gocritic // Owner role is the audience for this endpoint. - created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ - Type: codersdk.AIProviderTypeAnthropic, - Name: "bedrock-region-derived", - Enabled: true, - BaseURL: "https://bedrock-runtime.US-EAST-1.amazonaws.com/", - Settings: codersdk.AIProviderSettings{Bedrock: &codersdk.AIProviderBedrockSettings{}}, - }) - require.NoError(t, err) - require.NotNil(t, created.Settings.Bedrock) - require.Equal(t, "us-east-1", created.Settings.Bedrock.Region) - - updated, err := client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ - BaseURL: ptr.Ref("https://bedrock-runtime.us-west-2.amazonaws.com"), - }) - require.NoError(t, err) - require.NotNil(t, updated.Settings.Bedrock) - require.Equal(t, "us-west-2", updated.Settings.Bedrock.Region) - - updated, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ - BaseURL: ptr.Ref("https://bedrock-runtime.us-east-2.amazonaws.com"), - Settings: &codersdk.AIProviderSettings{ - Bedrock: &codersdk.AIProviderBedrockSettings{Region: "us-gov-west-1"}, + // Two request shapes resolve to Bedrock: the native type=bedrock + // form sends no settings, while the UI sends type=anthropic with a + // bedrock settings discriminator. Region derivation must behave + // identically for both. + cases := []struct { + name string + providerType codersdk.AIProviderType + createSettings codersdk.AIProviderSettings + }{ + { + name: "bedrock-type", + providerType: codersdk.AIProviderTypeBedrock, }, - }) - require.NoError(t, err) - require.NotNil(t, updated.Settings.Bedrock) - require.Equal(t, "us-gov-west-1", updated.Settings.Bedrock.Region) - }) + { + name: "anthropic-type", + providerType: codersdk.AIProviderTypeAnthropic, + createSettings: codersdk.AIProviderSettings{Bedrock: &codersdk.AIProviderBedrockSettings{}}, + }, + } - t.Run("BedrockTypeDerivesRegionWithoutSettings", func(t *testing.T) { - t.Parallel() - client := coderdtest.New(t, nil) - _ = coderdtest.CreateFirstUser(t, client) - ctx := testutil.Context(t, testutil.WaitLong) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + // A canonical endpoint derives and lower-cases the region + // server-side without an explicit region. + //nolint:gocritic // Owner role is the audience for this endpoint. + created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: tc.providerType, + Name: tc.name + "-derived", + Enabled: true, + BaseURL: "https://bedrock-runtime.US-EAST-1.amazonaws.com/", + Settings: tc.createSettings, + }) + require.NoError(t, err) + require.Equal(t, tc.providerType, created.Type) + require.NotNil(t, created.Settings.Bedrock) + require.Equal(t, "us-east-1", created.Settings.Bedrock.Region) - //nolint:gocritic // Owner role is the audience for this endpoint. - created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ - Type: codersdk.AIProviderTypeBedrock, - Name: "bedrock-type-region", - Enabled: true, - BaseURL: "https://bedrock-runtime.us-east-2.amazonaws.com", - }) - require.NoError(t, err) - require.Equal(t, codersdk.AIProviderTypeBedrock, created.Type) - require.NotNil(t, created.Settings.Bedrock) - require.Equal(t, "us-east-2", created.Settings.Bedrock.Region) + // A new canonical base URL re-derives the region. + updated, err := client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ + BaseURL: ptr.Ref("https://bedrock-runtime.us-west-2.amazonaws.com"), + }) + require.NoError(t, err) + require.NotNil(t, updated.Settings.Bedrock) + require.Equal(t, "us-west-2", updated.Settings.Bedrock.Region) + + // An explicit region wins over derivation. + updated, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ + BaseURL: ptr.Ref("https://bedrock-runtime.us-east-2.amazonaws.com"), + Settings: &codersdk.AIProviderSettings{ + Bedrock: &codersdk.AIProviderBedrockSettings{Region: "us-gov-west-1"}, + }, + }) + require.NoError(t, err) + require.NotNil(t, updated.Settings.Bedrock) + require.Equal(t, "us-gov-west-1", updated.Settings.Bedrock.Region) + + // Switching to a custom endpoint keeps the stored region + // rather than clearing it. A custom URL has nothing to + // re-derive from, so clearing it would lock the operator out + // of an existing provider. + updated, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ + BaseURL: ptr.Ref("https://bedrock.internal.example.com"), + }) + require.NoError(t, err) + require.NotNil(t, updated.Settings.Bedrock) + require.Equal(t, "us-gov-west-1", updated.Settings.Bedrock.Region) + }) + } }) - t.Run("BedrockRejectsCustomBaseURLWithoutRegion", func(t *testing.T) { + t.Run("BedrockRejectsUnresolvableRegion", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) _ = coderdtest.CreateFirstUser(t, client) ctx := testutil.Context(t, testutil.WaitLong) const required = "Bedrock settings require a region or credentials" - // A provider that can't resolve a region (custom endpoint, no region, - // no credentials) must be rejected, otherwise aibridge silently drops - // it at runtime. The native form sends type=bedrock while the UI sends - // type=anthropic with a bedrock settings discriminator, so both shapes - // must be guarded. + // A provider that can't resolve a region (custom endpoint, no + // region, no credentials) must be rejected, otherwise aibridge + // silently drops it at runtime. Both request shapes must be + // guarded: the native type=bedrock form sends no settings, while + // the UI sends type=anthropic with a bedrock settings + // discriminator. cases := []struct { - name string - providerType codersdk.AIProviderType - settings codersdk.AIProviderSettings + name string + providerType codersdk.AIProviderType + createSettings codersdk.AIProviderSettings }{ { name: "bedrock-type", providerType: codersdk.AIProviderTypeBedrock, }, { - name: "anthropic-type", - providerType: codersdk.AIProviderTypeAnthropic, - settings: codersdk.AIProviderSettings{Bedrock: &codersdk.AIProviderBedrockSettings{}}, + name: "anthropic-type", + providerType: codersdk.AIProviderTypeAnthropic, + createSettings: codersdk.AIProviderSettings{Bedrock: &codersdk.AIProviderBedrockSettings{}}, }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - // A custom endpoint can't derive a region, so create is rejected. + // Create against a custom endpoint with no region or + // credentials is rejected. //nolint:gocritic // Owner role is the audience for this endpoint. _, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ Type: tc.providerType, Name: tc.name + "-custom", Enabled: true, BaseURL: "https://bedrock.internal.example.com", - Settings: tc.settings, + Settings: tc.createSettings, }) sdkErr := requireSDKError(t, err, http.StatusBadRequest) require.Equal(t, "Invalid AI provider request.", sdkErr.Message) require.Contains(t, sdkErr.Validations, codersdk.ValidationError{Field: "settings", Detail: required}) - // A canonical endpoint derives the region server-side, so the - // same shape succeeds without an explicit region. + // Credentials alone make a custom endpoint usable (SigV4 + // authenticates with static credentials), so create succeeds. //nolint:gocritic // Owner role is the audience for this endpoint. - created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ - Type: tc.providerType, - Name: tc.name + "-canonical", - Enabled: true, - BaseURL: "https://bedrock-runtime.us-east-1.amazonaws.com", - Settings: tc.settings, + withCreds, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + Type: tc.providerType, + Name: tc.name + "-creds", + Enabled: true, + BaseURL: "https://bedrock.internal.example.com", + Settings: codersdk.AIProviderSettings{ + Bedrock: &codersdk.AIProviderBedrockSettings{ + AccessKey: ptr.Ref("AKIA-fixture"), //nolint:gosec // test fixture, not a real credential + AccessKeySecret: ptr.Ref("secret-fixture"), //nolint:gosec // test fixture, not a real credential + }, + }, }) require.NoError(t, err) - // Swapping back to a custom endpoint clears the derived region, - // so the update is rejected rather than storing a no-region row. - _, err = client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ - BaseURL: ptr.Ref("https://bedrock.internal.example.com"), + // Clearing those credentials leaves no region and no + // credentials behind a custom endpoint, so the update is + // rejected rather than persisting an unresolvable provider. + _, err = client.UpdateAIProvider(ctx, withCreds.Name, codersdk.UpdateAIProviderRequest{ + Settings: &codersdk.AIProviderSettings{ + Bedrock: &codersdk.AIProviderBedrockSettings{ + AccessKey: ptr.Ref(""), + AccessKeySecret: ptr.Ref(""), + }, + }, }) sdkErr = requireSDKError(t, err, http.StatusBadRequest) require.Equal(t, required, sdkErr.Message) From cd05cbdf0acd874d26fedbcaa6f15cc6b171f32e Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 23 Jun 2026 04:39:39 +0000 Subject: [PATCH 7/7] fix: allow base URL-only Bedrock providers --- cli/aibridged.go | 8 +- cli/aibridged_internal_test.go | 144 ++++++++++++++++++++++++--------- coderd/ai_providers.go | 4 +- coderd/ai_providers_migrate.go | 11 ++- coderd/ai_providers_test.go | 49 +++++------ 5 files changed, 140 insertions(+), 76 deletions(-) diff --git a/cli/aibridged.go b/cli/aibridged.go index e838c1db5ab0a..2234fb6164650 100644 --- a/cli/aibridged.go +++ b/cli/aibridged.go @@ -327,16 +327,14 @@ func buildAIProviderKeyPool(providerName string, keys []database.AIProviderKey, } // bedrockConfigFromRow returns nil when the settings have no Bedrock -// discriminator or when the Bedrock fields are not actually configured. -// The provider row's BaseUrl is the generic upstream endpoint and is -// always non-empty, so it cannot serve as a Bedrock detection signal; -// gate on the settings blob alone via [codersdk.AIProviderBedrockSettings.IsConfigured]. +// discriminator or when neither the row BaseUrl nor the Bedrock fields +// identify usable Bedrock config. func bedrockConfigFromRow(row database.AIProvider, settings codersdk.AIProviderSettings) *aibridge.AWSBedrockConfig { if settings.Bedrock == nil { return nil } bedrockSettings := *settings.Bedrock - if !bedrockSettings.IsConfigured() { + if !codersdk.IsBedrockConfigured(row.BaseUrl, bedrockSettings) { return nil } accessKey := ptr.NilToEmpty(bedrockSettings.AccessKey) diff --git a/cli/aibridged_internal_test.go b/cli/aibridged_internal_test.go index dee00f841f2d8..63351a208aaad 100644 --- a/cli/aibridged_internal_test.go +++ b/cli/aibridged_internal_test.go @@ -159,20 +159,44 @@ func TestBuildProviders(t *testing.T) { t.Run("LegacyBedrockWithoutAnthropicKey", func(t *testing.T) { t.Parallel() - // Bedrock credentials alone should be enough to create an - // Anthropic provider. No CODER_AIBRIDGE_ANTHROPIC_KEY needed. - cfg := codersdk.AIBridgeConfig{} - cfg.LegacyBedrock.Region = serpent.String("us-west-2") - cfg.LegacyBedrock.AccessKey = serpent.String("AKID") - cfg.LegacyBedrock.AccessKeySecret = serpent.String("secret") - providers, err := buildFromEnv(t, cfg) - require.NoError(t, err) - require.Len(t, providers, 1) + // Bedrock config alone should be enough to create an Anthropic + // provider. No CODER_AIBRIDGE_ANTHROPIC_KEY needed. + tests := []struct { + name string + configure func(*codersdk.AIBridgeConfig) + }{ + { + name: "region-and-static-credentials", + configure: func(cfg *codersdk.AIBridgeConfig) { + cfg.LegacyBedrock.Region = serpent.String("us-west-2") + cfg.LegacyBedrock.AccessKey = serpent.String("AKID") + cfg.LegacyBedrock.AccessKeySecret = serpent.String("secret") + }, + }, + { + name: "base-url-only", + configure: func(cfg *codersdk.AIBridgeConfig) { + cfg.LegacyBedrock.BaseURL = serpent.String("https://bedrock.internal.example.com") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cfg := codersdk.AIBridgeConfig{} + tt.configure(&cfg) - p := providers[0] - assert.Equal(t, aibridge.ProviderAnthropic, p.Type()) - assert.Equal(t, aibridge.ProviderAnthropic, p.Name()) + providers, err := buildFromEnv(t, cfg) + require.NoError(t, err) + require.Len(t, providers, 1) + + p := providers[0] + assert.Equal(t, aibridge.ProviderAnthropic, p.Type()) + assert.Equal(t, aibridge.ProviderAnthropic, p.Name()) + }) + } }) t.Run("UnknownType", func(t *testing.T) { @@ -256,45 +280,85 @@ func TestBuildProviders(t *testing.T) { assert.Nil(t, bedrockConfigFromRow(row, codersdk.AIProviderSettings{})) }) - t.Run("BedrockSettingsPresent", func(t *testing.T) { + t.Run("BedrockSettingsConfigured", func(t *testing.T) { t.Parallel() accessKey := "AKID" secret := "secret" model := "anthropic.claude-3-5-sonnet-20241022-v2:0" smallModel := "anthropic.claude-3-5-haiku-20241022-v1:0" - row := database.AIProvider{ - Type: database.AIProviderTypeAnthropic, - Name: "anthropic-bedrock", - BaseUrl: "https://bedrock-runtime.us-west-2.amazonaws.com/", - } - settings := codersdk.AIProviderSettings{ - Bedrock: &codersdk.AIProviderBedrockSettings{ - Region: "us-west-2", - AccessKey: &accessKey, - AccessKeySecret: &secret, - Model: model, - SmallFastModel: smallModel, + tests := []struct { + name string + row database.AIProvider + settings codersdk.AIProviderSettings + expectedBaseURL string + expectedRegion string + expectedAccessKey string + expectedAccessSecret string + expectedModel string + expectedSmallFastModel string + }{ + { + name: "region-and-static-credentials", + row: database.AIProvider{ + Type: database.AIProviderTypeAnthropic, + Name: "anthropic-bedrock", + BaseUrl: "https://bedrock-runtime.us-west-2.amazonaws.com/", + }, + settings: codersdk.AIProviderSettings{ + Bedrock: &codersdk.AIProviderBedrockSettings{ + Region: "us-west-2", + AccessKey: &accessKey, + AccessKeySecret: &secret, + Model: model, + SmallFastModel: smallModel, + }, + }, + expectedBaseURL: "https://bedrock-runtime.us-west-2.amazonaws.com/", + expectedRegion: "us-west-2", + expectedAccessKey: accessKey, + expectedAccessSecret: secret, + expectedModel: model, + expectedSmallFastModel: smallModel, }, + { + name: "base-url-only", + row: database.AIProvider{ + Type: database.AIProviderTypeAnthropic, + Name: "anthropic-bedrock-base-url", + BaseUrl: "https://bedrock.internal.example.com/", + }, + settings: codersdk.AIProviderSettings{ + Bedrock: &codersdk.AIProviderBedrockSettings{ + Model: model, + SmallFastModel: smallModel, + }, + }, + expectedBaseURL: "https://bedrock.internal.example.com/", + expectedModel: model, + expectedSmallFastModel: smallModel, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := bedrockConfigFromRow(tt.row, tt.settings) + require.NotNil(t, got) + assert.Equal(t, tt.expectedBaseURL, got.BaseURL) + assert.Equal(t, tt.expectedRegion, got.Region) + assert.Equal(t, tt.expectedAccessKey, got.AccessKey) + assert.Equal(t, tt.expectedAccessSecret, got.AccessKeySecret) + assert.Equal(t, tt.expectedModel, got.Model) + assert.Equal(t, tt.expectedSmallFastModel, got.SmallFastModel) + }) } - got := bedrockConfigFromRow(row, settings) - require.NotNil(t, got) - assert.Equal(t, row.BaseUrl, got.BaseURL) - assert.Equal(t, "us-west-2", got.Region) - assert.Equal(t, accessKey, got.AccessKey) - assert.Equal(t, secret, got.AccessKeySecret) - assert.Equal(t, model, got.Model) - assert.Equal(t, smallModel, got.SmallFastModel) }) - t.Run("BedrockSettingsEmpty", func(t *testing.T) { + t.Run("BedrockSettingsUnconfigured", func(t *testing.T) { t.Parallel() - // A non-nil but zero-valued Bedrock settings blob should not - // produce a Bedrock config; the provider's generic BaseUrl is - // not a Bedrock detection signal. row := database.AIProvider{ - Type: database.AIProviderTypeAnthropic, - Name: "anthropic-empty-bedrock", - BaseUrl: "https://api.anthropic.com/", + Type: database.AIProviderTypeAnthropic, + Name: "anthropic-empty-bedrock", } settings := codersdk.AIProviderSettings{ Bedrock: &codersdk.AIProviderBedrockSettings{}, diff --git a/coderd/ai_providers.go b/coderd/ai_providers.go index 6fdf553c9ab1a..da942347ee71d 100644 --- a/coderd/ai_providers.go +++ b/coderd/ai_providers.go @@ -191,7 +191,7 @@ func (api *API) aiProvidersCreate(rw http.ResponseWriter, r *http.Request) { // The UI submits Bedrock as type=anthropic with a bedrock discriminator, // so gate on the settings blob rather than the provider type; otherwise // aibridge silently drops the provider at runtime. - if req.Settings.Bedrock != nil && !req.Settings.Bedrock.IsConfigured() { + if req.Settings.Bedrock != nil && !codersdk.IsBedrockConfigured(req.BaseURL, *req.Settings.Bedrock) { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid AI provider request.", Validations: []codersdk.ValidationError{{ @@ -365,7 +365,7 @@ func (api *API) aiProvidersUpdate(rw http.ResponseWriter, r *http.Request) { // The mismatch check above guarantees a non-nil Bedrock blob only // survives on anthropic- or bedrock-typed providers, so any such // blob must resolve to usable config regardless of the type. - if existing.Bedrock != nil && !existing.Bedrock.IsConfigured() { + if existing.Bedrock != nil && !codersdk.IsBedrockConfigured(baseURL, *existing.Bedrock) { return errAIProviderBedrockSettingsRequired } settings, err := encodeAIProviderSettings(existing) diff --git a/coderd/ai_providers_migrate.go b/coderd/ai_providers_migrate.go index 7ec6a997b0dfe..47ac4843eedd1 100644 --- a/coderd/ai_providers_migrate.go +++ b/coderd/ai_providers_migrate.go @@ -319,9 +319,8 @@ func providersFromEnv(ctx context.Context, cfg codersdk.AIBridgeConfig, logger s // Legacy Anthropic + Bedrock. Anthropic is enabled if either an // Anthropic key OR any Bedrock setting is explicitly configured. - // Detection goes through AIProviderBedrockSettings.IsConfigured() - // so the legacy and indexed paths agree on what counts as a - // Bedrock provider. + // Detection goes through IsBedrockConfigured so the legacy and + // indexed paths agree on what counts as a Bedrock provider. bedrock := codersdk.NewAIProviderBedrockSettings( cfg.LegacyBedrock.Region.String(), cfg.LegacyBedrock.AccessKey.String(), @@ -385,9 +384,9 @@ func providersFromEnv(ctx context.Context, cfg codersdk.AIBridgeConfig, logger s dp.BaseURL = p.BaseURL // Bedrock fields apply to Anthropic and the dedicated Bedrock - // type. Detection goes through - // AIProviderBedrockSettings.IsConfigured() so the legacy and - // indexed paths agree on what counts as a Bedrock provider. + // type. Detection goes through IsBedrockConfigured so the + // legacy and indexed paths agree on what counts as a Bedrock + // provider. isBedrock := false if dp.Type == database.AIProviderTypeAnthropic || dp.Type == database.AIProviderTypeBedrock { var accessKey, accessKeySecret string diff --git a/coderd/ai_providers_test.go b/coderd/ai_providers_test.go index 29135e62287c2..81e88a16b7a5f 100644 --- a/coderd/ai_providers_test.go +++ b/coderd/ai_providers_test.go @@ -702,19 +702,16 @@ func TestAIProvidersCRUD(t *testing.T) { } }) - t.Run("BedrockRejectsUnresolvableRegion", func(t *testing.T) { + t.Run("BedrockAllowsBaseURLOnlyConfig", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, nil) _ = coderdtest.CreateFirstUser(t, client) ctx := testutil.Context(t, testutil.WaitLong) - const required = "Bedrock settings require a region or credentials" - - // A provider that can't resolve a region (custom endpoint, no - // region, no credentials) must be rejected, otherwise aibridge - // silently drops it at runtime. Both request shapes must be - // guarded: the native type=bedrock form sends no settings, while - // the UI sends type=anthropic with a bedrock settings - // discriminator. + + // A custom endpoint alone is usable when AWS credentials come from + // the default credential chain. Both request shapes must accept it: + // the native type=bedrock form sends no settings, while the UI sends + // type=anthropic with a bedrock settings discriminator. cases := []struct { name string providerType codersdk.AIProviderType @@ -733,22 +730,30 @@ func TestAIProvidersCRUD(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - // Create against a custom endpoint with no region or - // credentials is rejected. //nolint:gocritic // Owner role is the audience for this endpoint. - _, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ + created, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ Type: tc.providerType, Name: tc.name + "-custom", Enabled: true, BaseURL: "https://bedrock.internal.example.com", Settings: tc.createSettings, }) - sdkErr := requireSDKError(t, err, http.StatusBadRequest) - require.Equal(t, "Invalid AI provider request.", sdkErr.Message) - require.Contains(t, sdkErr.Validations, codersdk.ValidationError{Field: "settings", Detail: required}) + require.NoError(t, err) + require.NotNil(t, created.Settings.Bedrock) + require.Empty(t, created.Settings.Bedrock.Region) + + // An unrelated PATCH must validate against the stored base URL + // rather than rejecting the empty region and static credentials. + updated, err := client.UpdateAIProvider(ctx, created.Name, codersdk.UpdateAIProviderRequest{ + Enabled: ptr.Ref(false), + }) + require.NoError(t, err) + require.False(t, updated.Enabled) + require.NotNil(t, updated.Settings.Bedrock) + require.Empty(t, updated.Settings.Bedrock.Region) - // Credentials alone make a custom endpoint usable (SigV4 - // authenticates with static credentials), so create succeeds. + // Clearing static credentials is also valid when a custom base + // URL remains, because AWS can resolve ambient credentials. //nolint:gocritic // Owner role is the audience for this endpoint. withCreds, err := client.CreateAIProvider(ctx, codersdk.CreateAIProviderRequest{ Type: tc.providerType, @@ -764,10 +769,7 @@ func TestAIProvidersCRUD(t *testing.T) { }) require.NoError(t, err) - // Clearing those credentials leaves no region and no - // credentials behind a custom endpoint, so the update is - // rejected rather than persisting an unresolvable provider. - _, err = client.UpdateAIProvider(ctx, withCreds.Name, codersdk.UpdateAIProviderRequest{ + cleared, err := client.UpdateAIProvider(ctx, withCreds.Name, codersdk.UpdateAIProviderRequest{ Settings: &codersdk.AIProviderSettings{ Bedrock: &codersdk.AIProviderBedrockSettings{ AccessKey: ptr.Ref(""), @@ -775,8 +777,9 @@ func TestAIProvidersCRUD(t *testing.T) { }, }, }) - sdkErr = requireSDKError(t, err, http.StatusBadRequest) - require.Equal(t, required, sdkErr.Message) + require.NoError(t, err) + require.NotNil(t, cleared.Settings.Bedrock) + require.Empty(t, cleared.Settings.Bedrock.Region) }) } })