feat(providers): add AWS Bedrock as a built-in provider with native SigV4 auth

[cru-Luis-Rodriguez]

Closes #659. Adds bedrock as a built-in provider with native SigV4 auth, taking option (1) direct dependency as you decided in that issue.

Two commits: the provider itself, then the config/CLI surface it turned out to need. The second one exists because registering a provider was not sufficient to make it configurable — details under Q1–Q3 below.

Sample config

{
  "provider": "bedrock",
  "model": "us.anthropic.claude-sonnet-4-6",
  "providers": {
    "bedrock": {
      "aws_region": "us-west-2",
      "aws_profile": "example-profile"
    }
  }
}

Both AWS fields are optional; without them the standard credential chain decides, as with any other AWS tool. There is no api_key line, and none is accepted as a substitute for a signature.

Answers to the five questions from #659

1. Provider entry shape. api_key is optional and ignored, not forbidden. Rejecting a stray key would punish someone who pasted one out of habit, and it cannot leak into a request: the client deletes Authorization and X-Api-Key before the signing middleware runs. The resolver's gate is now apiKey == "" && !(isPreset && preset.AmbientAuth), so every key-based provider keeps its requirement — TestNonAmbientProviderStillRequiresAPIKey pins that.

aws_region and aws_profile are real fields on the provider entry, JSON-serializable under providers.bedrock, not read purely from the env chain. Pinning them is what makes a run reproducible without exporting AWS_PROFILE first, which matters most on CI runners that carry a different default.

While wiring that up I found a bug worth fixing here rather than later: the fields were readable by the resolver but absent from the ProviderEntry struct the CLI marshals. Config is unmarshalled into that struct and written back on every config command, so a hand-written aws_region was silently deleted the first time the user ran ocr config model or ocr config set — no error, and nothing to explain why the next review reached a different region. TestConfigRoundTripKeepsAWSSettings is the regression test.

2. Interactive wizard. Keyed off AmbientAuth, never off a provider name. For an ambient provider the model step is the last one — the wizard confirms there instead of advancing to an API-key prompt that has to be left blank, which reads as a step the user failed to complete. Before this, bedrock was unreachable through ocr config provider entirely: applyOfficialProviderConfig rejected the empty key. That gate is now a named checkAPIKeyRequirement so the ambient case is explicit and testable, and apiKeyStepCanConfirm also accepts an empty value for an ambient provider, which is the state reachable when an existing config is edited.

Yes, it still runs the connection test at the end — that is exactly where an expired SSO session surfaces, and the cheapest moment to catch it.

What is not in this PR: collecting region and profile as two optional inputs in that step. It needs a real design decision about the wizard's shape (placeholders showing what the AWS chain currently resolves, so blank reads as "inherit us-west-2" rather than "unset"), and I would rather agree on it with you than guess. Until then the wizard configures bedrock and leaves both to the AWS chain, and ocr config set covers pinning them. Happy to add it as a follow-up commit here.

3. Non-interactive config. Yes, both need to be settable, and they now are:

ocr config set providers.bedrock.aws_region us-west-2
ocr config set providers.bedrock.aws_profile example-profile

One applyProviderField case covers providers.* and custom_providers.*, since setProviderValue and setCustomProviderField both funnel there. Values are trimmed; whitespace inside one is rejected. Region names are not validated against a fixed list — AWS adds regions faster than an embedded list stays correct, and a wrong region already fails at request time with a clear message (see Q5). Setting either field on a provider that authenticates by api_key is an error rather than stored dead config, which catches providers.anthropic.aws_region typos; for a custom provider it is accepted once the entry declares the bedrock protocol.

The unknown-config-key message is pinned byte-for-byte by an existing test, so it is updated for the two new fields and for anthropic-bedrock as a protocol value.

4. Model list semantics. The preset ships a Models list, but it should be read as a starting point, not a closed set, and free-form entry has to keep working — the existing custom-model path in the model picker already provides it. Three reasons Bedrock cannot be validated like a hosted API:

• Inference profile IDs differ per account and per region.
• An application inference profile ARN is account-specific and long, and is the right value when usage has to be attributed for cost allocation.
• Suffix conventions vary per family, so IDs cannot be derived. us.anthropic.claude-sonnet-5 is correct while us.anthropic.claude-sonnet-5-v1:0 is rejected outright. The listed IDs are taken verbatim from aws bedrock list-inference-profiles on a live account rather than inferred, and global.* variants are listed beside us.* since either is a valid routing target.

So an ID missing from the preset must not be rejected. That is now implemented rather than merely argued: preset.Models still gates a --model override for key-based providers — a typo against a hosted API is worth catching locally — but not for an ambient-auth preset, where the list is a picker for ocr config model and nothing more. Before that change, --model with an application inference profile ARN failed locally with "is not available for provider", which contradicted this very section.

5. Connection-test messaging. Bedrock's own wording sends people after the wrong problem, so failures are classified before falling through to a generic error. Real output from this branch:

$ ocr llm test
Source: provider:bedrock
Region: us-east-1
Profile: example-profile
Model:  claude-sonnet-5
✓ Connection test successful

The URL line is replaced by region and profile, because bedrock has no configured URL — the region decides the host, and a request that reached the wrong one otherwise fails as though the model ID were malformed.

Failure	What the user sees
Unresolvable profile / no AWS config	"bedrock uses the standard AWS credential chain — set AWS_PROFILE, or run aws sso login --profile NAME"
Expired session (ExpiredToken, refresh failure)	Names the profile in the aws sso login suggestion
AccessDeniedException: You don't have access to the model…	Model access is granted per account and per region in the console; an IAM policy alone does not enable it
AccessDeniedException naming bedrock:InvokeModel	"credentials resolved, so this is an authorization gap" — the IAM side
Invalid model identifier	Points at aws bedrock list-inference-profiles --region <r> and calls out the -v1:0 suffix trap
Anything else (ValidationException on max_tokens, a reset, a throttle)	Keeps the service's own wording, with the region and profile appended
Invalid API Key format	Explains that no api_key applies to bedrock and that a bearer token reached the request; if AWS_BEARER_TOKEN_BEDROCK is set, names that variable specifically

Two notes on that last row. It is checked first, because it occurs with otherwise-valid credentials and a later "expired"/"denied" branch would mislabel it. And the SDK reports the pre-middleware /v1/messages path in its error text, which makes correct path rewriting look broken — the surrounding message now says which region and profile were actually used, so the URL is not the only context available.

Every other protocol shares this client type, so the translation is gated on the bedrock flag and returns other errors untouched (TestExplainErrorLeavesNonBedrockErrorsAlone).

On the SDK bug

Per your note, the workaround stays in this patch: bedrock.WithConfig prefers bearer auth whenever cfg.BearerAuthTokenProvider is non-nil, and LoadDefaultConfig populates it from the SSO token cache — so an SSO-authenticated caller silently sends its OIDC access token and gets 403 Invalid API Key format. The provider is therefore cleared before WithConfig runs.

There is a second, smaller SDK problem worth knowing about, which I hit while reviewing my own patch. WithConfig's doc comment says the environment variable takes precedence:

Authentication is determined as follows: if the AWS_BEARER_TOKEN_BEDROCK environment variable is set, it is used for bearer token authentication. Otherwise, if cfg.BearerAuthTokenProvider is set, it is used.

The code does the opposite — it consults the variable only if cfg.BearerAuthTokenProvider == nil. My first version of this patch trusted the comment and cleared the provider only when the variable was unset, which meant an SSO user who deliberately set a Bedrock API key still sent the SSO token. Clearing unconditionally is what actually delivers the documented precedence, so that is what this does. Glad to open the anthropic-sdk-go issue for both points separately; say the word if you would rather this patch shrink once they land.

bedrock.WithLoadDefaultConfig also panics when AWS config cannot be loaded, so the config is loaded directly and the failure deferred to the first request as a sentence naming the likely fix — a CLI should not answer an expired session with a stack trace.

Verification

• Full suite and go vet pass; gofmt clean.
• New tests: protocol registration, preset shape, resolution with no api_key, AWS settings reaching the client, the api_key requirement surviving for non-ambient providers, deferred-panic behaviour, config round-trip preserving the AWS fields, config set accept/reject/trim cases, the wizard skipping the key step, and each error classification.
• End-to-end against a live Bedrock account: reviews complete using SigV4 credentials from an SSO profile with no AWS variables in the environment; a -v1:0 model ID and an unresolvable profile each produce their intended message rather than a bare 400 or 403.

Rebased on current main, so it carries the finalizeResolvedEndpoint refactor and the per-run provider/model overrides from #687 — the resolver change is down to one completeness condition.

[cru-Luis-Rodriguez]

Pushed a third commit (0da3406) after reviewing the first two adversarially. Four defects, each verified by execution or against SDK source rather than inferred — flagging them here so the delta is visible rather than buried in a force-push. The description above is updated to match.

1. AWS_BEARER_TOKEN_BEDROCK was unreachable for exactly the users it should serve. I had cleared cfg.BearerAuthTokenProvider only when the variable was unset, trusting WithConfig's doc comment that the variable takes precedence. The code does the opposite — it reads the variable only if cfg.BearerAuthTokenProvider == nil. So an SSO profile plus a deliberately set Bedrock API key still sent the SSO OIDC token, and the error message then blamed a token that never left the machine. Cleared unconditionally now, which is what actually delivers the documented precedence. Details in the SDK section of the description.

2. A model the account has not enabled was reported as an IAM problem. Bedrock returns AccessDeniedException for both, but You don't have access to the model with the specified model ID is fixed by enabling model access in the console, per account and per region — no IAM policy provides it. The specific wording is now matched ahead of the generic code; my clause for it had been stranded in an unreachable branch.

3. A bare ValidationException match over-triggered. Input is too long for requested model was answered with "go list your inference profiles". Only the model-identifier wording is matched now; everything else keeps the service's own message. Same treatment for the credential-expiry arm, which had matched a bare expired and so claimed x509: certificate has expired was an SSO problem.

4. --model rejected identifiers absent from the preset list, which contradicted the Q4 answer in this very description. A preset list cannot be an allowlist for Bedrock: IDs are account- and region-scoped, and an application inference profile ARN can never appear in a list compiled upstream. The list no longer gates an override for an ambient-auth provider; key-based providers keep the check.

Also dropped a cfg.URL normalization block that could not have had any effect (WithConfig is appended last and installs its own base URL), pinned AWS_CONFIG_FILE in the one test that was reading the developer's real ~/.aws/config, and fixed a column alignment in ocr llm test.

New tests cover each: the two AccessDeniedException shapes with their verbatim service wording, a request-shape ValidationException falling through to the generic message, the TLS-certificate case, and --model accepting an ARN for bedrock while still rejecting a typo for a key-based provider. Suite, go vet and gofmt clean; ocr llm test re-verified against a live account, and an identifier the preset does not list now reaches Bedrock and gets Bedrock's own verdict instead of a local rejection.

[github-actions]

🔍 OpenCodeReview found 3 issue(s) in this PR.

• ✅ Successfully posted inline: 2 comment(s)
• ❌ Failed to post inline: 1 comment(s)

---

[documentation · low]

📄 internal/llm/protocol.go (L60-L61)

⚠️ GitHub could not post this as an inline comment: Unprocessable Entity: "Line could not be resolved" - https://docs.github.com/rest/pulls/reviews#create-a-review-for-a-pull-request

The comment says "accepts the three canonical protocol names" but there are now four supported protocols (including ProtocolAnthropicBedrock). Update the comment to reflect the new count.

💡 Suggested Change

Before:

// ValidateProtocol accepts the three canonical protocol names and rejects
// everything else.

After:

// ValidateProtocol accepts the four canonical protocol names and rejects
// everything else.

[github-actions]

[bug · medium]
Bug: For preset providers, this function only checks the static preset.AmbientAuth flag and ignores any user-level protocol override in entry.Protocol. Since the resolver (see tryProviderConfig in resolver.go lines 363-369) allows entry.Protocol to override the preset's default protocol, a user could set protocol: openai on the bedrock preset and aws_region/aws_profile would still pass validation here — creating dead config that reads as applied but has no effect at runtime.

The fix should determine the effective protocol by checking entry.Protocol first (when non-empty) before falling back to the preset default, similar to how the resolver does it.

Suggestion:

Suggested change

	func providerAcceptsAWSSettings(providerName string, entry *ProviderEntry) bool {
	if preset, isPreset := llm.LookupProvider(providerName); isPreset {
	return preset.AmbientAuth
	}
	return llm.NormalizeProtocol(entry.Protocol) == llm.ProtocolAnthropicBedrock
	}
	func providerAcceptsAWSSettings(providerName string, entry *ProviderEntry) bool {
	if preset, isPreset := llm.LookupProvider(providerName); isPreset {
	// Honor user-level protocol override: if the entry explicitly sets a
	// non-Bedrock protocol, AWS settings are meaningless even though the
	// preset defaults to ambient auth.
	if entry.Protocol != "" {
	return llm.NormalizeProtocol(entry.Protocol) == llm.ProtocolAnthropicBedrock
	}
	return preset.AmbientAuth
	}
	return llm.NormalizeProtocol(entry.Protocol) == llm.ProtocolAnthropicBedrock
	}

[cru-Luis-Rodriguez]

Confirmed and fixed in 68d250c — thank you, this was a real hole and the diagnosis was exact.

I took it a step further than the config layer, because the same preset-only assumption existed on the resolver side and was the more serious half. tryProviderConfig skipped the api_key requirement on isPreset && preset.AmbientAuth, evaluated before the entry's protocol override was read. So providers.bedrock.protocol = openai resolved with no key and no URL (the bedrock preset has no BaseURL), producing an endpoint that spoke a token protocol while carrying nothing to authenticate with — reported as configured, failing only at request time.

Ambient auth is now derived once, after the override is applied:

ambientAuth := protocol == ProtocolAnthropicBedrock ||
    (isPreset && preset.AmbientAuth && entry.Protocol == "")

That value gates the api_key requirement, the resolved endpoint's AmbientAuth, and the --model allowlist relaxation, which had the same assumption. It also makes the reverse case coherent: an entry that explicitly selects anthropic-bedrock signs its requests whatever preset it sits under, so it no longer demands a key it would ignore.

providerAcceptsAWSSettings follows the shape you suggested — the entry's protocol decides whenever it sets one, with the preset's flag answering only when the entry is silent — so the two layers now agree on what "ambient" means. Tests added for both: an overridden entry that must require a key again, and aws_region rejected on that entry then accepted once the protocol is set back.

[github-actions]

[bug · medium]
Matching on "ValidationException" alone is too broad. AWS Bedrock returns ValidationException for many reasons unrelated to model IDs — e.g., malformed request body, invalid parameter values, content exceeding size limits, or missing required fields. When any of those occur, the user will see guidance to run aws bedrock list-inference-profiles, which is misleading and won't fix the actual problem.

Consider narrowing this match to only catch model-related validation errors, for example by requiring both "ValidationException" and a model-related keyword, or by removing "ValidationException" from this case entirely and relying on the more specific "model identifier is invalid" and "don't have access to the model" patterns.

[cru-Luis-Rodriguez]

Agreed, and already fixed — this review ran against fdd1ad2, one commit behind. 0da3406 narrowed exactly this, for the same reason you give:

• The bare ValidationException arm is gone. Only model identifier is invalid (and an inference-profile-not-found shape) map to the list-inference-profiles guidance; a request-shape rejection such as Input is too long for requested model now falls through to the generic wrapper, which preserves the service's own message.
• The credential arm no longer matches a bare expired, which had been claiming x509: certificate has expired was an expired SSO session.
• Separately, AccessDeniedException had two meanings collapsed into one message. You don't have access to the model with the specified model ID needs model access enabled per account and per region in the console, which no IAM policy provides, so it is now matched ahead of the generic AccessDenied code and says so.

Tests cover each with the verbatim service wording rather than paraphrases.

[cru-Luis-Rodriguez]

Addressed all three findings from the OCR run above in 68d250c, with replies in the two inline threads.

The third one could not be posted inline ("Line could not be resolved"), so for the record: it was right. ValidateProtocol said "the three canonical protocol names" while accepting four. Fixed, and the internal/llm package comment now lists anthropic-bedrock among the supported protocols too — it had the same omission, one the bot did not flag.

Worth noting on the second finding (ValidationException too broad): that run reviewed fdd1ad2, one commit behind, and 0da3406 had already narrowed it. So the finding was accurate against the commit it saw. The first finding — ambient auth read off the preset while the protocol can be overridden per entry — was live and is the substantive fix here; it also existed on the resolver side, where it let an overridden entry resolve with no credentials at all.

Suite, go vet and gofmt clean; ocr llm test re-verified against a live Bedrock account after the change.

[lizhengfeng101]

CI / test (pull_request)
CI / test (pull_request)Failing after 39s

@cru-Luis-Rodriguez

[cru-Luis-Rodriguez]

Thanks for the ping — fixed in 273357a.

The failure was the govulncheck step, not a test. GO-2026-5764 affects github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.3, and the trace runs through this PR's own new dependency (llm.init → bedrock.init → eventstream.init), so it's on me to clear it.

aws-sdk-go-v2/config is the only direct AWS import here, so bumping it from v1.27.27 to v1.32.34 pulls the tree forward and resolves eventstream to v1.7.16, past the v1.7.8 fix. smithy-go moves to v1.27.6 as its runtime companion; internal/ini drops out and internal/v4a / service/signin come in, both internal restructuring within aws-sdk-go-v2 itself. No non-AWS dependency moves — the diff is go.mod and go.sum only.

On why the core module jumps so many minors: there's no supported way to move eventstream past v1.7.8 while holding the 2024-era pins, since MVS resolves it from the direct dependency. This is the minimal coherent bump.

Verified locally against the CI configuration (go1.26.5): go build ./..., gofmt -s, go vet, and go mod tidy are all clean, go test -race -count=1 ./... passes across all packages, and govulncheck ./... reports 0 affecting vulnerabilities.

One note for a separate PR: govulncheck still reports GO-2026-5970 (golang.org/x/text v0.37.0) and GO-2026-5942 (golang.org/x/net v0.55.0) as imported-but-not-called. Neither affects the exit code and both pre-date this PR, so I left them out to keep this diff scoped to the AWS tree — happy to open a follow-up if you'd like them bumped.

[lizhengfeng101]

@cru-Luis-Rodriguez CI is failing due to missing SPDX license headers on the two new test files:

cmd/opencodereview/bedrock_config_test.go (missing SPDX identifier)
internal/llm/bedrock_test.go (missing SPDX identifier)

The merge conflict has been resolved on our side. Could you rebase on main and run make license-add to fix the headers?