-
Notifications
You must be signed in to change notification settings - Fork 206
feat(cli): add AWS Bedrock as a supported provider #1015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,16 @@ | ||
| import { getApiKeyFromEnv } from "../provider.js" | ||
| import { getApiKeyFromEnv, getProviderSettings, providerRequiresApiKey } from "../provider.js" | ||
|
|
||
| // Bedrock-relevant AWS environment variables. Cleared before each test so the | ||
| // suite is hermetic regardless of the host machine's ambient AWS configuration. | ||
| const AWS_ENV_KEYS = [ | ||
| "AWS_REGION", | ||
| "AWS_DEFAULT_REGION", | ||
| "AWS_PROFILE", | ||
| "AWS_ACCESS_KEY_ID", | ||
| "AWS_SECRET_ACCESS_KEY", | ||
| "AWS_SESSION_TOKEN", | ||
| "AWS_BEDROCK_API_KEY", | ||
| ] | ||
|
|
||
| describe("getApiKeyFromEnv", () => { | ||
| const originalEnv = process.env | ||
|
|
@@ -27,8 +39,190 @@ describe("getApiKeyFromEnv", () => { | |
| expect(getApiKeyFromEnv("openai-native")).toBe("test-openai-key") | ||
| }) | ||
|
|
||
| it("should return API key from environment variable for bedrock", () => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor hermiticity note: the |
||
| process.env.AWS_BEDROCK_API_KEY = "test-bedrock-key" | ||
| expect(getApiKeyFromEnv("bedrock")).toBe("test-bedrock-key") | ||
| }) | ||
|
|
||
| it("should return undefined when API key is not set", () => { | ||
| delete process.env.ANTHROPIC_API_KEY | ||
| expect(getApiKeyFromEnv("anthropic")).toBeUndefined() | ||
| }) | ||
| }) | ||
|
|
||
| describe("providerRequiresApiKey", () => { | ||
| it("returns false for bedrock (AWS credential chain / profile / direct creds are valid)", () => { | ||
| expect(providerRequiresApiKey("bedrock")).toBe(false) | ||
| }) | ||
|
|
||
| it.each(["anthropic", "openai-native", "gemini", "openrouter", "vercel-ai-gateway"] as const)( | ||
| "returns true for '%s'", | ||
| (provider) => { | ||
| expect(providerRequiresApiKey(provider)).toBe(true) | ||
| }, | ||
| ) | ||
| }) | ||
|
|
||
| describe("getProviderSettings", () => { | ||
| const originalEnv = process.env | ||
|
|
||
| beforeEach(() => { | ||
| process.env = { ...originalEnv } | ||
| for (const key of AWS_ENV_KEYS) { | ||
| delete process.env[key] | ||
| } | ||
| }) | ||
|
|
||
| afterEach(() => { | ||
| process.env = originalEnv | ||
| }) | ||
|
|
||
| it("sets apiProvider for the selected provider", () => { | ||
| const config = getProviderSettings("anthropic", undefined, undefined) | ||
| expect(config.apiProvider).toBe("anthropic") | ||
| }) | ||
|
|
||
| describe("bedrock authentication modes", () => { | ||
| it("mode 1: bearer token / API key sets awsUseApiKey and awsApiKey", () => { | ||
| const config = getProviderSettings("bedrock", "bearer-token-123", undefined) | ||
|
|
||
| expect(config.apiProvider).toBe("bedrock") | ||
| expect(config.awsUseApiKey).toBe(true) | ||
| expect(config.awsApiKey).toBe("bearer-token-123") | ||
| // API-key mode must not enable profile or direct-credential fields. | ||
| expect(config.awsUseProfile).toBeUndefined() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These suppression assertions use
|
||
| expect(config.awsProfile).toBeUndefined() | ||
| expect(config.awsAccessKey).toBeUndefined() | ||
| expect(config.awsSecretKey).toBeUndefined() | ||
| }) | ||
|
|
||
| it("mode 2: AWS_PROFILE sets awsUseProfile and awsProfile", () => { | ||
| process.env.AWS_PROFILE = "my-sso-profile" | ||
|
|
||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
|
|
||
| expect(config.awsUseProfile).toBe(true) | ||
| expect(config.awsProfile).toBe("my-sso-profile") | ||
| expect(config.awsUseApiKey).toBeUndefined() | ||
| expect(config.awsApiKey).toBeUndefined() | ||
| expect(config.awsAccessKey).toBeUndefined() | ||
| }) | ||
|
|
||
| it("mode 3: direct credentials map AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY", () => { | ||
| process.env.AWS_ACCESS_KEY_ID = "AKIAEXAMPLE" | ||
| process.env.AWS_SECRET_ACCESS_KEY = "secret-example" | ||
|
|
||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
|
|
||
| expect(config.awsAccessKey).toBe("AKIAEXAMPLE") | ||
| expect(config.awsSecretKey).toBe("secret-example") | ||
| expect(config.awsSessionToken).toBeUndefined() | ||
| expect(config.awsUseApiKey).toBeUndefined() | ||
| expect(config.awsUseProfile).toBeUndefined() | ||
| }) | ||
|
|
||
| it("mode 3: direct credentials include AWS_SESSION_TOKEN when present", () => { | ||
| process.env.AWS_ACCESS_KEY_ID = "AKIAEXAMPLE" | ||
| process.env.AWS_SECRET_ACCESS_KEY = "secret-example" | ||
| process.env.AWS_SESSION_TOKEN = "session-token-example" | ||
|
|
||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
|
|
||
| expect(config.awsAccessKey).toBe("AKIAEXAMPLE") | ||
| expect(config.awsSecretKey).toBe("secret-example") | ||
| expect(config.awsSessionToken).toBe("session-token-example") | ||
| }) | ||
|
|
||
| it("mode 4: default credential chain — no creds set does NOT throw and sets no explicit auth fields", () => { | ||
| expect(() => getProviderSettings("bedrock", undefined, undefined)).not.toThrow() | ||
|
|
||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
|
|
||
| // Region is always resolved, but none of the explicit auth modes engage, | ||
| // leaving the AWS SDK to resolve credentials via its default chain. | ||
| expect(config.awsUseApiKey).toBeUndefined() | ||
| expect(config.awsApiKey).toBeUndefined() | ||
| expect(config.awsUseProfile).toBeUndefined() | ||
| expect(config.awsProfile).toBeUndefined() | ||
| expect(config.awsAccessKey).toBeUndefined() | ||
| expect(config.awsSecretKey).toBeUndefined() | ||
| }) | ||
|
|
||
| it("prioritises API key over AWS_PROFILE and direct credentials", () => { | ||
| process.env.AWS_PROFILE = "my-profile" | ||
| process.env.AWS_ACCESS_KEY_ID = "AKIAEXAMPLE" | ||
| process.env.AWS_SECRET_ACCESS_KEY = "secret-example" | ||
|
|
||
| const config = getProviderSettings("bedrock", "bearer-token-123", undefined) | ||
|
|
||
| expect(config.awsUseApiKey).toBe(true) | ||
| expect(config.awsApiKey).toBe("bearer-token-123") | ||
| expect(config.awsUseProfile).toBeUndefined() | ||
| expect(config.awsAccessKey).toBeUndefined() | ||
| }) | ||
|
|
||
| it("prioritises AWS_PROFILE over direct credentials when no API key is present", () => { | ||
| process.env.AWS_PROFILE = "my-profile" | ||
| process.env.AWS_ACCESS_KEY_ID = "AKIAEXAMPLE" | ||
| process.env.AWS_SECRET_ACCESS_KEY = "secret-example" | ||
|
|
||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
|
|
||
| expect(config.awsUseProfile).toBe(true) | ||
| expect(config.awsProfile).toBe("my-profile") | ||
| expect(config.awsAccessKey).toBeUndefined() | ||
| expect(config.awsSecretKey).toBeUndefined() | ||
| }) | ||
| }) | ||
|
|
||
| describe("bedrock region resolution", () => { | ||
| it("defaults awsRegion to us-east-1 when no region env is set", () => { | ||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
| expect(config.awsRegion).toBe("us-east-1") | ||
| }) | ||
|
|
||
| it("uses AWS_REGION when set", () => { | ||
| process.env.AWS_REGION = "eu-west-1" | ||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
| expect(config.awsRegion).toBe("eu-west-1") | ||
| }) | ||
|
|
||
| it("falls back to AWS_DEFAULT_REGION when AWS_REGION is unset", () => { | ||
| process.env.AWS_DEFAULT_REGION = "ap-southeast-2" | ||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
| expect(config.awsRegion).toBe("ap-southeast-2") | ||
| }) | ||
|
|
||
| it("prefers AWS_REGION over AWS_DEFAULT_REGION", () => { | ||
| process.env.AWS_REGION = "eu-west-1" | ||
| process.env.AWS_DEFAULT_REGION = "ap-southeast-2" | ||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
| expect(config.awsRegion).toBe("eu-west-1") | ||
| }) | ||
| }) | ||
|
|
||
| describe("bedrock cross-region inference auto-enable", () => { | ||
| it.each([ | ||
| "us.anthropic.claude-3-5-sonnet-20241022-v2:0", | ||
| "eu.anthropic.claude-3-5-sonnet", | ||
| "apac.amazon.nova-pro", | ||
| ])("enables awsUseCrossRegionInference for regional-prefixed model '%s'", (model) => { | ||
| const config = getProviderSettings("bedrock", undefined, model) | ||
| expect(config.apiModelId).toBe(model) | ||
| expect(config.awsUseCrossRegionInference).toBe(true) | ||
| }) | ||
|
|
||
| it("does NOT enable cross-region inference for a non-prefixed model", () => { | ||
| const model = "anthropic.claude-3-5-sonnet-20241022-v2:0" | ||
| const config = getProviderSettings("bedrock", undefined, model) | ||
| expect(config.apiModelId).toBe(model) | ||
| expect(config.awsUseCrossRegionInference).toBeUndefined() | ||
| }) | ||
|
|
||
| it("does not set apiModelId or cross-region flag when no model is given", () => { | ||
| const config = getProviderSettings("bedrock", undefined, undefined) | ||
| expect(config.apiModelId).toBeUndefined() | ||
| expect(config.awsUseCrossRegionInference).toBeUndefined() | ||
| }) | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,6 +4,7 @@ import type { SupportedProvider } from "@/types/index.js" | |||||
|
|
||||||
| const envVarMap: Record<SupportedProvider, string> = { | ||||||
| anthropic: "ANTHROPIC_API_KEY", | ||||||
| bedrock: "AWS_BEDROCK_API_KEY", | ||||||
| "openai-native": "OPENAI_API_KEY", | ||||||
| gemini: "GOOGLE_API_KEY", | ||||||
| openrouter: "OPENROUTER_API_KEY", | ||||||
|
|
@@ -19,6 +20,40 @@ export function getApiKeyFromEnv(provider: SupportedProvider): string | undefine | |||||
| return process.env[envVar] | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Whether a provider requires an explicit API key before a task can run. | ||||||
| * | ||||||
| * Every provider except Bedrock authenticates solely via an API key | ||||||
| * (`--api-key` or its provider-specific environment variable). Bedrock is the | ||||||
| * exception: in addition to a bearer token / API key, it can authenticate via | ||||||
| * an AWS profile, direct AWS access/secret keys, OR the AWS SDK default | ||||||
| * credential chain (IMDS / EC2 instance profile, ECS task role, IRSA / | ||||||
| * web-identity, SSO, shared-config default). Those chain-based sources set none | ||||||
| * of our recognised environment variables, so Bedrock must never be hard-failed | ||||||
| * for a "missing" API key — the downstream `AwsBedrockHandler` resolves the | ||||||
| * credentials and surfaces a real error only if resolution actually fails. | ||||||
| */ | ||||||
| export function providerRequiresApiKey(provider: SupportedProvider): boolean { | ||||||
| return provider !== "bedrock" | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Build the provider-specific `RooCodeSettings` used to configure the extension | ||||||
| * host for a CLI run. | ||||||
| * | ||||||
| * The Bedrock case supports four authentication modes, resolved in priority | ||||||
| * order: | ||||||
| * 1. Bearer token / API key — `--api-key` or `AWS_BEDROCK_API_KEY` | ||||||
| * (`awsUseApiKey` + `awsApiKey`). | ||||||
| * 2. AWS profile — `AWS_PROFILE` (`awsUseProfile` + `awsProfile`). | ||||||
| * 3. Direct credentials — `AWS_ACCESS_KEY_ID` + `AWS_SECRET_ACCESS_KEY` | ||||||
| * (+ optional `AWS_SESSION_TOKEN`). | ||||||
| * 4. Default credential chain — none of the above set; the AWS SDK resolves | ||||||
| * credentials (IMDS, ECS task role, IRSA, SSO, shared-config default). | ||||||
| * The region is resolved from `AWS_REGION` / `AWS_DEFAULT_REGION` (defaulting to | ||||||
| * `us-east-1`), and cross-region inference is auto-enabled for model IDs that | ||||||
| * carry a regional prefix (`us.` / `eu.` / `apac.`). | ||||||
| */ | ||||||
| export function getProviderSettings( | ||||||
| provider: SupportedProvider, | ||||||
| apiKey: string | undefined, | ||||||
|
|
@@ -31,6 +66,36 @@ export function getProviderSettings( | |||||
| if (apiKey) config.apiKey = apiKey | ||||||
| if (model) config.apiModelId = model | ||||||
| break | ||||||
| case "bedrock": | ||||||
| config.awsRegion = process.env.AWS_REGION || process.env.AWS_DEFAULT_REGION || "us-east-1" | ||||||
| if (model) { | ||||||
| config.apiModelId = model | ||||||
| // Auto-enable cross-region inference when model ID has a regional prefix | ||||||
| // (e.g. "us.", "eu.", "apac.") — these are cross-region inference profiles | ||||||
| // that require awsUseCrossRegionInference to be set. | ||||||
| if (/^(us|eu|apac)\./.test(model)) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This regex only covers Would it make sense to derive this from the existing mapping rather than maintaining a parallel regex?
Suggested change
|
||||||
| config.awsUseCrossRegionInference = true | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (apiKey) { | ||||||
| // Bearer token / API key mode (LiteLLM proxy, Bedrock gateway) | ||||||
| config.awsUseApiKey = true | ||||||
| config.awsApiKey = apiKey | ||||||
| } else if (process.env.AWS_PROFILE) { | ||||||
| // Profile-based auth | ||||||
| config.awsUseProfile = true | ||||||
| config.awsProfile = process.env.AWS_PROFILE | ||||||
| } else if (process.env.AWS_ACCESS_KEY_ID && process.env.AWS_SECRET_ACCESS_KEY) { | ||||||
| // Direct credentials from env | ||||||
| config.awsAccessKey = process.env.AWS_ACCESS_KEY_ID | ||||||
| config.awsSecretKey = process.env.AWS_SECRET_ACCESS_KEY | ||||||
| if (process.env.AWS_SESSION_TOKEN) { | ||||||
| config.awsSessionToken = process.env.AWS_SESSION_TOKEN | ||||||
| } | ||||||
| } | ||||||
| // else: fall through to default credential chain (SDK handles IMDS, ECS task role, etc.) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If a user switches auth modes between CLI runs — e.g. from Would explicitly clearing the non-winning flags fix this? |
||||||
| break | ||||||
| case "openai-native": | ||||||
| if (apiKey) config.openAiNativeApiKey = apiKey | ||||||
| if (model) config.apiModelId = model | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a test covering this new branch?
run.test.tscurrently only tests--prompt-filefile I/O — nothing exercises theproviderRequiresApiKeygate. It would be good to confirm that (a) a non-Bedrock provider without an API key still exits with 1, and (b) a Bedrock provider without any key or AWS env vars proceeds past this point.