Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions apps/cli/src/commands/cli/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { JsonEventEmitter } from "@/agent/json-event-emitter.js"

import { loadSettings } from "@/lib/storage/index.js"
import { readWorkspaceTaskSessions, resolveWorkspaceResumeSessionId } from "@/lib/task-history/index.js"
import { getEnvVarName, getApiKeyFromEnv } from "@/lib/utils/provider.js"
import { getEnvVarName, getApiKeyFromEnv, providerRequiresApiKey } from "@/lib/utils/provider.js"
import { validateTerminalShellPath } from "@/lib/utils/shell.js"
import { getDefaultExtensionPath } from "@/lib/utils/extension.js"
import { isValidSessionId } from "@/lib/utils/session-id.js"
Expand Down Expand Up @@ -189,10 +189,17 @@ export async function run(promptArg: string | undefined, flagOptions: FlagOption

extensionHostOptions.apiKey = flagOptions.apiKey || getApiKeyFromEnv(extensionHostOptions.provider)

if (!extensionHostOptions.apiKey) {
// Every provider except Bedrock requires an explicit API key up front. Bedrock
// is intentionally exempt: beyond a bearer token / API key, it can authenticate
// via AWS_PROFILE, direct AWS access/secret keys, or the AWS SDK default
// credential chain (IMDS / EC2 instance profile, ECS task role, IRSA, SSO,
// shared-config default). Those chain-based sources set none of our recognised
// environment variables, so we must not hard-fail here — we let execution
// continue and allow the downstream AwsBedrockHandler to resolve credentials and
// surface a real error only if resolution actually fails.
if (!extensionHostOptions.apiKey && providerRequiresApiKey(extensionHostOptions.provider)) {

Copy link
Copy Markdown
Contributor

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.ts currently only tests --prompt-file file I/O — nothing exercises the providerRequiresApiKey gate. 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.

console.error(`[CLI] Error: No API key provided. Use --api-key or set the appropriate environment variable.`)
console.error(`[CLI] For ${extensionHostOptions.provider}, set ${getEnvVarName(extensionHostOptions.provider)}`)

process.exit(1)
}

Expand Down
3 changes: 2 additions & 1 deletion apps/cli/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Command } from "commander"

import { DEFAULT_FLAGS } from "@/types/constants.js"
import { supportedProviders } from "@/types/index.js"
import { VERSION } from "@/lib/utils/version.js"
import { run, logout, status, listCommands, listModes, listModels, listSessions, upgrade } from "@/commands/index.js"

Expand Down Expand Up @@ -35,7 +36,7 @@ program
.option("-d, --debug", "Enable debug output (includes detailed debug information)", false)
.option("-a, --require-approval", "Require manual approval for actions", false)
.option("-k, --api-key <key>", "API key for the LLM provider")
.option("--provider <provider>", "API provider (anthropic, openai-native, gemini, openrouter, etc.)")
.option("--provider <provider>", `API provider (${supportedProviders.join(", ")})`)
.option("-m, --model <model>", "Model to use", DEFAULT_FLAGS.model)
.option("--mode <mode>", "Mode to start in (code, architect, ask, debug, etc.)", DEFAULT_FLAGS.mode)
.option("--terminal-shell <path>", "Absolute path to shell executable for inline terminal commands")
Expand Down
196 changes: 195 additions & 1 deletion apps/cli/src/lib/utils/__tests__/provider.test.ts
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
Expand Down Expand Up @@ -27,8 +39,190 @@ describe("getApiKeyFromEnv", () => {
expect(getApiKeyFromEnv("openai-native")).toBe("test-openai-key")
})

it("should return API key from environment variable for bedrock", () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor hermiticity note: the getApiKeyFromEnv describe block (above this test) only does process.env = { ...originalEnv } without scrubbing AWS keys — so on a machine with AWS_BEDROCK_API_KEY exported in the shell, future "returns undefined" assertions for bedrock could be non-deterministic. The getProviderSettings block below correctly scrubs AWS_ENV_KEYS in its beforeEach; worth applying the same pattern here.

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()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These suppression assertions use .toBeUndefined(), which passes whether the key is absent from the object or explicitly set to undefined — both look the same in JS. A mutation that writes config.awsUseProfile = undefined instead of leaving the key unset would pass all of these.

expect(config).not.toHaveProperty('awsUseProfile') asserts the key is genuinely absent.

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()
})
})
})
65 changes: 65 additions & 0 deletions apps/cli/src/lib/utils/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
Expand All @@ -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)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regex only covers us., eu., and apac. prefixes, but AWS_INFERENCE_PROFILE_MAPPING in packages/types/src/providers/bedrock.ts defines 8 prefixes: au., jp., ug., us., eu., apac., ca., sa.. A model like au.anthropic.claude-3-5-sonnet-20241022-v2:0 will silently get awsUseCrossRegionInference = false, causing a ResourceNotFoundException with no indication of why.

Would it make sense to derive this from the existing mapping rather than maintaining a parallel regex?

Suggested change
if (/^(us|eu|apac)\./.test(model)) {
if (AWS_INFERENCE_PROFILE_MAPPING.some(([, prefix]) => model.startsWith(prefix))) {

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.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a user switches auth modes between CLI runs — e.g. from AWS_PROFILE in run 1 to direct AWS_ACCESS_KEY_ID credentials in run 2 — awsUseProfile=true can persist in ~/.vscode-mock/global-storage/global-state.json from run 1. The Bedrock handler checks awsUseProfile before awsAccessKey (bedrock.ts line 282), so run 2 silently authenticates with the stale profile instead of the new direct credentials.

Would explicitly clearing the non-winning flags fix this?

break
case "openai-native":
if (apiKey) config.openAiNativeApiKey = apiKey
if (model) config.apiModelId = model
Expand Down
1 change: 1 addition & 0 deletions apps/cli/src/types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { OutputFormat } from "./json-events.js"

export const supportedProviders = [
"anthropic",
"bedrock",
"openai-native",
"gemini",
"openrouter",
Expand Down
Loading