feat(llm): native AWS Bedrock Converse provider (#205)

[initializ-mk]

Summary

Adds a native bedrock LLM provider that speaks Bedrock's model-agnostic Converse API (POST /model/<id>/converse[-stream]), so any Bedrock model (Claude, Nova, Llama, Mistral, Titan) works with tool-calling through one translation — no compat proxy, no per-model wire matching. This is the #205 follow-up to the #202 auth_scheme: aws_sigv4 passthrough (which only signs an OpenAI/Anthropic-shaped request).

Minimal config — SigV4 is intrinsic, base_url derives from the region:

model:
  provider: bedrock
  name: us.amazon.nova-2-lite-v1:0   # model id or inference-profile id
  aws_region: us-east-1

What's included

• forge-core/llm/providers/bedrock.go — Chat/ChatStream/ModelID, Converse request/response translation (system blocks, toolUse/toolResult, toolConfig, inferenceConfig), region-derived base URL, model-id path percent-encoding, opt-in cachePoint prompt caching.
• bedrock_eventstream.go — hand-rolled vnd.amazon.eventstream decoder for converse-stream (stdlib only — no aws-sdk-go-v2, matching the SigV4 signer; prelude + message CRC32 validated).
• SigV4 signer fix (sigv4_transport.go) — canonical URI now double-encodes the path (signs EscapedPath(), not Path) as SigV4 mandates for every service except S3. Without it a colon in a model id 403s with SignatureDoesNotMatch. The Support Anthropic-format custom URLs (incl. AWS Bedrock) via provider: anthropic #202 passthrough paths (/v1/messages, /chat/completions) contain no reserved chars, so their signatures are byte-identical — no behavior change.
• Config & security — forge validate requires model.aws_region for provider: bedrock; the egress allowlist derives bedrock-runtime.<region>.amazonaws.com from provider + region.
• forge init — TUI wizard offers "AWS Bedrock" (prompts for region + model, no API key); non-interactive path takes --model-provider bedrock --aws-region <region>. Sourced from the shared forge-core/catalog.
• forge-ui — create-agent wizard renders an AWS region field (in place of the API-key field) for Bedrock, requires it before advancing, and the create endpoint rejects a Bedrock request missing aws_region.

Testing

• Unit tests across bedrock.go, the eventstream decoder (CRC/truncation/exception frames), the SigV4 double-encoding regression, catalog, validation, init scaffold, and the forge-ui handler.
• Verified live against real Bedrock with us.amazon.nova-2-lite-v1:0 — both non-streaming Converse and streaming (converse-stream) returned correctly.
• go build + go vet clean; forge-core, forge-cli, forge-ui test suites pass.

Scope / follow-ups

• Text + tool-calling (+ opt-in prompt caching). Image/document content blocks wait until the provider-agnostic ChatMessage body grows beyond a string.
• Credentials are env-only (AWS_ACCESS_KEY_ID / _SECRET_ACCESS_KEY / _SESSION_TOKEN); IRSA/STS resolution remains the same follow-up as Support Anthropic-format custom URLs (incl. AWS Bedrock) via provider: anthropic #202.
• forge-ui keeps its own hardcoded wizard metadata (doesn't read the catalog), so the Bedrock model list lives in two places today; unifying them is a possible cleanup.

[initializ-mk]

Review — native AWS Bedrock Converse provider (#205)

Large but well-built. I reviewed the security core (Converse translation, SigV4, the hand-rolled event-stream decoder, egress derivation); the plumbing (init/wizard/UI/catalog/validate) was traced separately. No high-severity defects — the SigV4 double-encoding fix, model-id path encoding, and the binary decoder are careful and correct. Two MEDIUMs and a LOW cluster below.

⚠️ No CI has run on this branch ("no checks reported"). The description claims local go build/vet/tests green, but that's unverified — please trigger CI before merge.

🟠 MEDIUM #1 — Event-stream decoder: unbounded allocation from an untrusted length (DoS)

bedrock_eventstream.go:70 — rest := make([]byte, totalLen-12) allocates on totalLen (a wire uint32) with no maximum-frame-size check. A 12-byte prelude with an attacker-valid CRC and totalLen ≈ 4 GB triggers a multi-GB allocation before any payload is read → memory-exhaustion DoS. Reachable via a malicious/compromised endpoint or a base_url override (or TLS MITM). AWS caps event-stream frames (~24 MB); reject totalLen above a sane bound right after the prelude-CRC check. (Everything else in the decoder is bounds-safe and CRC-correct.)

🟠 MEDIUM #2 — forge init egress derivation omits the Bedrock host → scaffolded agent blocked at forge run

forge-cli/cmd/init_egress.go (providerDomains map / deriveEgressDomains, ~lines 11-16 & 57-59) has openai/anthropic/gemini but no bedrock — and structurally can't, since the host is region-dependent. So a scaffolded Bedrock agent that also uses a channel/tool/skill/auth (→ allowlist mode) gets a forge.yaml egress allowlist without bedrock-runtime.<region>.amazonaws.com. I confirmed forge run builds its allowlist via EffectiveEgressAllowlist (platform_policy_enforce.go:176), which reads only cfg.Egress.AllowedDomains — it does not merge LLMProviderDomains (that's build-time only, called solely by egress_stage.go). So the agent's own Converse calls are blocked at local forge run. Deployed (forge build) is fine because the build stage adds the host. Fix: special-case bedrock in deriveEgressDomains to add bedrock-runtime.<opts.AWSRegion>.amazonaws.com.

🟡 LOW cluster

• Default model IDs likely uninvocable: catalog defaults to bare anthropic.claude-sonnet-4-… / claude-3-5-haiku-…, but Anthropic-on-Bedrock generally needs a cross-region inference-profile ID (us.anthropic.*); on-demand throughput returns ValidationException. The default scaffold (Sonnet 4 + us-east-1) may fail at first call — while the PR's own live test used the profile-style us.amazon.nova-2-lite-v1:0. Use us.-prefixed defaults or a doc note.
• aws_region not format-validated (validate/forge_config.go:107 checks only non-empty; TUI/handler likewise). A typo (us-east-1x) passes every gate → bad host + poisons the region-derived egress host. The catalog already defines an aws_region rule (used for the sigv4 auth field) that isn't applied to model.aws_region. Apply ^[a-z0-9-]+$.
• Empty model.name is only a warning but fatal for bedrock (forge_config.go:93) → URL path /model//converse. Make it a bedrock-specific error.
• Region requirement not at the shared choke point: scaffold()/createFunc (ui.go) bypasses collectNonInteractive; only the HTTP handler guards it. A future programmatic caller could scaffold a region-less Bedrock agent. Centralize the check in scaffold().
• TUI region enforcement untested; provider metadata drift (forge-ui hardcodes model lists vs catalog — Bedrock matches today, OpenAI already diverges; no cross-source consistency test).

✅ Verified correct

• SigV4 double-encoding fix: correct per the non-S3 canonical-URI rule; signing EscapedPath() matches AWS SDK behavior. /v1/messages & /chat/completions are byte-identical (EscapedPath == Path), so the #202 passthrough signatures are unchanged.
• Model-id path encoding: uriEscape(model, true) → %3A survives as url.RawPath, so EscapedPath() on the wire matches the double-encoded canonical URI SigV4 signs. Raw ARNs with / are explicitly out of scope (documented).
• Converse translation (system blocks, toolUse/toolResult, tool-schema default, opt-in cachePoint) mirrors the Anthropic client; response + stream parsing correct. Consecutive-role / parallel-tool handling is identical to the working Anthropic provider — not a Bedrock defect, only a shared future gap if forge ever emits parallel tool calls.
• Event-stream decoder (apart from #1): header walk validates every offset, frame-length math uses uint64 (no underflow), prelude + message CRC32 both validated, no slice panics.
• Region required at all 4 entry points (validate/init/TUI/forge-ui); no api_key demanded or stored for Bedrock; provider enum consistent; Bedrock model list matches catalog↔forge-ui; config wiring (region→client, factory routing) correct; tests + docs/CHANGELOG present.

Verdict: changes requested — the two MEDIUMs (decoder allocation bound; init egress Bedrock host) are the priorities; the LOW cluster is hardening/consistency per the standing convention that non-blocking findings are still tracked as requested changes.

[initializ-mk]

🟠 MEDIUM #1 — unbounded allocation from an untrusted length (DoS). totalLen is a wire uint32 validated only for >= 16 (line 66); there's no MAX. A 12-byte prelude with an attacker-valid prelude CRC and totalLen ≈ 4 GB reaches this make([]byte, totalLen-12) and allocates multiple GB before io.ReadFull reads any payload → memory-exhaustion DoS. Reachable from a malicious/compromised endpoint or a base_url override (the default AWS endpoint is trusted, but the client honors an operator/attacker base_url). AWS caps event-stream frames (~24 MB); add a bound like if totalLen > maxFrameSize { return error } right after the prelude-CRC check, before allocating.

[initializ-mk]

Thanks for the thorough review — all findings addressed in 7aa29d3.

🟠 MEDIUM #1 — event-stream unbounded allocation

Fixed. bedrock_eventstream.go now rejects totalLen > maxEventStreamFrame (32 MB) immediately after the prelude-CRC check, before the make([]byte, totalLen-12). Regression test TestEventStreamDecoder_RejectsOversizeFrame feeds a valid-CRC prelude claiming ~4 GB and asserts it errors without allocating.

🟠 MEDIUM #2 — forge init egress omits the Bedrock host

Fixed. deriveEgressDomains now adds bedrock-runtime.<opts.AWSRegion>.amazonaws.com for provider: bedrock (region-derived, so it can't sit in the static providerDomains map). Mirrors security.LLMProviderDomains that the build stage already applies. Test: TestDeriveEgressDomains_BedrockHost.

🟡 LOW cluster

• Uninvocable default model ids — catalog / init / forge-ui defaults are now US cross-region inference-profile ids (us. prefix); docs note the prefix is region-scoped (us./eu./apac.) and that bare ids only work for on-demand models.
• aws_region not format-validated — added ^[a-z0-9-]+$ in ValidateForgeConfig for bedrock. Test: malformed region is an error.
• Empty model.name only a warning — now a bedrock-specific error (it's the /model/<id>/converse path segment). Test: empty name is a bedrock error.
• Region check not at the choke point — enforced in scaffold() now, so the forge-ui createFunc path is covered too (not just collectNonInteractive / the HTTP handler).
• Provider-metadata drift — forge-ui now sources its Bedrock model list from the shared catalog instead of hardcoding it; TestWizardMeta_BedrockMatchesCatalog asserts they stay in sync.
• TUI region enforcement — added TestProviderStep_ApplyCarriesBedrockRegion covering the region→context wiring (the interactive enforcement also runs through the same validate/scaffold paths now tested).

CI

Re-pushed, which should trigger checks on the branch. The verification I can run locally (go build, go vet, and the forge-core / forge-cli / forge-ui suites) is green.

[initializ-mk]

Re-review — fix commit 7aa29d37 ✅ all findings resolved

Verified each fix against source. Both MEDIUMs and the full LOW cluster are addressed, with genuine tests.

Finding	Fix	Verified
🟠 MEDIUM #1 — decoder DoS	maxEventStreamFrame = 32 MB; totalLen bounded after the prelude-CRC check, before make()	✅ TestEventStreamDecoder_RejectsOversizeFrame builds a valid-CRC prelude with totalLen ≈ 4 GB and asserts a "too large" error — exercises the real allocation path
🟠 MEDIUM #2 — init egress host	deriveEgressDomains adds bedrock-runtime.<region>.amazonaws.com for a Bedrock primary	✅ + egress-host test
🟡 default model IDs	catalog / init / forge-ui now use us.* inference-profile IDs, with a region-prefix doc note	✅
🟡 aws_region format	^[a-z0-9-]+$ enforced in ValidateForgeConfig	✅ + test
🟡 empty model.name for bedrock	now a hard error (it's the /model/<id>/converse path segment)	✅ + test
🟡 shared choke point	scaffold() now enforces the region requirement (covers the forge-ui createFunc path)	✅
🟡 forge-ui drift	forge-ui sources the Bedrock list from catalog.ProviderByID instead of hardcoding	✅ + consistency test
🟡 TUI enforcement untested	provider_step_test.go added	✅

⚠️ Blocking before merge: CI still isn't running

The branch still shows "no checks reported." The description says the suites pass locally, and this fix adds solid tests — but a multi-module change to security-critical SigV4 signing and a hand-rolled binary parser should not merge without CI actually executing. Please get the workflows to run (and go green) on this branch before merging.

Minor residual (informational, non-blocking)

deriveEgressDomains (and the build-time security.LLMProviderDomains) derive the Bedrock host only for the primary model, not a Bedrock fallback — so provider: openai with a bedrock fallback would omit the fallback's Converse host from the allowlist. Narrow edge, symmetric across both egress paths, and not introduced by this PR's changes — worth a follow-up if Bedrock-as-fallback becomes a supported pattern.

Verdict: all review findings resolved. The provider is well-built — correct SigV4 double-encoding, careful (now allocation-bounded) event-stream decoder, translation faithful to the Converse API, region required and format-checked at every entry point, egress wired for both forge run and deploy. The only gate left is getting CI to run green on the branch.

[initializ-mk]

Final check — merge 472730d1d clean, CI green ✅

The conflict resolution and CI gate are both cleared.

Conflict resolution verified. The merge of origin/main had exactly one conflict: forge-core/validate/forge_config.go, where this branch's Bedrock validation met main's #464 bearer-auth_scheme changes. The resolution correctly kept both sides — aws_region required + format-checked (^[a-z0-9-]+$) + empty-model.name error, alongside main's AuthSchemeBearer additions — with no conflict markers or dropped lines. Every other Bedrock and security-core file (provider, event-stream decoder, SigV4, egress, catalog, init_egress) is byte-identical across the merge, so all the review fixes are intact.

CI now green — the blocker from my last re-review is cleared: all 10 checks pass (Build ×6, Lint, Test, Integration Tests, Doc-link), including the new oversize-frame DoS-guard test and the egress/region/validation tests.

Verdict: all review findings resolved, merge is clean, CI green — good to merge. Only mergeStateStatus: BLOCKED remains, which is branch protection expecting an approving review — and since this is your own PR, GitHub won't let me (or you) self-approve, so that needs a second reviewer's approval. Nothing left on the code side.

Recap of the full arc: two MEDIUMs (event-stream allocation bound; init-egress Bedrock host for forge run) and a LOW cluster (uninvocable default IDs → us.* profiles, aws_region format validation, empty-name-fatal error, shared scaffold() choke point, forge-ui catalog sourcing to kill drift) — all fixed with genuine tests, on a well-built provider (correct SigV4 double-encoding, careful binary decoder, faithful Converse translation).