feat: add AWS Bedrock API support#260
Conversation
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| agent_env = dict(agent_env or {}) | ||
| explicit_agent_env_keys = set(agent_env) | ||
| auto_inherit_env(agent_env) | ||
| auto_inherit_env(agent_env, source_env=load_dotenv_env()) |
There was a problem hiding this comment.
🔴 resolve_agent_env no longer inherits API keys from os.environ, only from .env file
The call auto_inherit_env(agent_env, source_env=load_dotenv_env()) at src/benchflow/_agent_env.py:281 replaces the prior auto_inherit_env(agent_env) which read from os.environ. When no .env file exists, load_dotenv_env() returns {} (an empty dict), which is not None. Inside auto_inherit_env, the guard source = source_env if source_env is not None else os.environ (src/benchflow/_agent_env.py:101) therefore uses the empty dict instead of falling back to os.environ. This means API keys set via shell environment variables (e.g. export ANTHROPIC_API_KEY=sk-test) are silently ignored — breaking the established workflow for all users who don't have a .env file. The CLI passes only explicit --agent-env flags as agent_env (src/benchflow/cli/main.py:129-135), so there is no other path that compensates. The existing auto_inherit_env unit tests (tests/test_resolve_env_helpers.py:37-41) call the function without source_env, so they exercise the os.environ fallback that production code no longer uses.
Prompt for agents
The problem is in resolve_agent_env at src/benchflow/_agent_env.py:281. The call auto_inherit_env(agent_env, source_env=load_dotenv_env()) passes the .env file contents as source_env, completely replacing os.environ. When no .env file exists, load_dotenv_env() returns {} (empty dict, not None), so the os.environ fallback in auto_inherit_env never activates.
The fix should merge both sources: .env values should act as a fallback layer on top of os.environ (or vice versa, depending on desired precedence). One approach: build a merged source dict that combines os.environ with .env overrides, e.g.:
dotenv = load_dotenv_env()
merged_source = {**os.environ, **dotenv} # .env overrides os.environ
auto_inherit_env(agent_env, source_env=merged_source)
Alternatively, call auto_inherit_env twice (once with .env, once with os.environ) using setdefault semantics to get the right precedence. The key invariant to preserve: users who export API keys in their shell must still have them inherited into agent_env.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is a valid finding. On main, auto_inherit_env(agent_env) reads from os.environ. This PR changes it to auto_inherit_env(agent_env, source_env=load_dotenv_env()) which only reads from the .env file.
This was an intentional change from the original PR author (commit f18f719 — "fix: prefer explicit and .env auth vars over host env for Bedrock runs"), but it does break the established workflow for users who export API keys in their shell without a .env file.
A simple fix would be to merge both sources as suggested:
dotenv = load_dotenv_env()
merged_source = {**os.environ, **dotenv} # .env overrides os.environ
auto_inherit_env(agent_env, source_env=merged_source)@xdotli — should I apply this fix, or was the .env-only behavior intentional for Bedrock?
- Restore check_subscription_auth() fallback in resolve_agent_env before raising ValueError (inadvertently removed during Bedrock merge) - Add ty: ignore[unresolved-import] for optional boto3 import - Update test_no_auth_file_raises match pattern for new error message - Fix ruff format issues
41bd7f9 to
cc2b584
Compare
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| BEDROCK_PROXY_BIND_HOST = "0.0.0.0" |
There was a problem hiding this comment.
🔴 Bedrock proxy binds to 0.0.0.0 without inbound authentication, exposing AWS credentials to the network
The Bedrock proxy server binds to 0.0.0.0 (all network interfaces) at _provider_runtime.py:15 and the proxy has no authentication on the inbound side (bedrock_proxy.py:170-245). Any device on the same network can send requests to the proxy's port and they will be forwarded to AWS Bedrock using the user's AWS_BEARER_TOKEN_BEDROCK credentials. This is especially concerning because the proxy is started automatically when using an aws-bedrock/ model prefix.
How the proxy is reached without auth
The proxy binds on BEDROCK_PROXY_BIND_HOST = "0.0.0.0" at _provider_runtime.py:111. The _handle_connection method at bedrock_proxy.py:170 processes all inbound requests with no authentication — no token check, no IP allowlist. The routes at /v1/messages, /v1/responses, and /model/*/invoke* all proxy through to AWS Bedrock using the bearer token from _bedrock_auth_headers at bedrock_proxy.py:337-349.
Prompt for agents
The Bedrock proxy server binds to 0.0.0.0 (BEDROCK_PROXY_BIND_HOST at _provider_runtime.py:15), making it accessible from any network interface. This proxy forwards requests to AWS Bedrock using the user's AWS_BEARER_TOKEN_BEDROCK with no inbound authentication.
The bind address should be restricted. For Docker environments, consider binding to the Docker bridge network interface instead of all interfaces. For non-Docker environments, 127.0.0.1 is already used as the URL host but the server still binds on 0.0.0.0.
Possible approaches:
1. Bind to 127.0.0.1 by default. For Docker on Linux where host.docker.internal maps to the bridge gateway (not loopback), bind to the Docker bridge interface IP (e.g. 172.17.0.1) which can be detected at runtime.
2. Add a randomly-generated auth token that is set as a header by the agent and validated by the proxy.
3. At minimum, document the security implications and let users opt into 0.0.0.0 explicitly.
Files involved: src/benchflow/_provider_runtime.py (BEDROCK_PROXY_BIND_HOST constant, ensure_bedrock_proxy_runtime), src/benchflow/providers/bedrock_proxy.py (BedrockProxyServer._handle_connection).
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Summary
Add initial AWS Bedrock API support to BenchFlow. Rebased clean version of #247.
This PR introduces a host-side Bedrock proxy and wires
aws-bedrock/...models into the existing provider/agent flow so BenchFlow can route supported ACP agents through Bedrock instead of direct Anthropic endpoints. Current focus is the Sonnet 4.5 Claude path.What Changed
Provider and Agent Plumbing
aws-bedrockas a registered provider withauth_type=\"aws\"BENCHFLOW_PROVIDER_*flowHost-Side Bedrock Proxy
invoke,invoke-with-response-stream,count-tokens)Trial Integration
aws-bedrock/models; stop during cleanupFixes during merge with main
check_subscription_auth()fallback that was inadvertently dropped — standard Claude/Codex/Gemini subscription auth works again# ty: ignore[unresolved-import]for optionalboto3import to passty checkTests — 8 new/updated test files covering provider registry, env resolution, ACP model selection, Bedrock proxy HTTP behavior, runtime startup, and trial-side proxy integration.
Review & Testing Checklist for Human
aws-bedrock/us.anthropic.claude-sonnet-4-5modelAWS_BEARER_TOKEN_BEDROCKandAWS_REGIONare properly validated and propagated.claude/.credentials.jsonpresent)boto3optional dependency installs correctly withuv sync --extra bedrockNotes
ty checkfailure from the original PR is fixed with an inline suppression for the optionalboto3importLink to Devin session: https://app.devin.ai/sessions/35319f5359ec4bb3851a8ce65dcc7620
Requested by: @xdotli