fix(security): include AWS credential vars in bedrock subprocess blocklist (#32314) - #32319
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds AWS SDK credential environment variables to Hermes’ subprocess environment blocklist (via Bedrock provider config) to prevent credential leakage, with regression tests for issue #32314.
Changes:
- Populate Bedrock (
auth_type="aws_sdk")api_key_env_varswith standard AWS credential/config env vars so they’re stripped from subprocess env. - Add regression test ensuring AWS credential env vars are removed from subprocess environment.
- Add regression test ensuring AWS SDK provider vars are included in the global provider env blocklist.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/tools/test_local_env_blocklist.py | Adds regression tests asserting AWS credential vars are stripped and included in the blocklist. |
| hermes_cli/auth.py | Updates Bedrock provider config to include AWS credential-related env vars in api_key_env_vars for blocklisting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| aws_vars = { | ||
| "AWS_BEARER_TOKEN_BEDROCK": "bedrock-bearer-secret", | ||
| "AWS_ACCESS_KEY_ID": "AKIA-fake", | ||
| "AWS_SECRET_ACCESS_KEY": "fake-secret", | ||
| "AWS_SESSION_TOKEN": "fake-session", | ||
| "AWS_PROFILE": "prod", | ||
| "AWS_ROLE_ARN": "arn:aws:iam::123:role/x", | ||
| "AWS_WEB_IDENTITY_TOKEN_FILE": "/tmp/token", | ||
| "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI": "/v2/credentials/abc", | ||
| } | ||
| result_env = _run_with_env(extra_os_env=aws_vars) | ||
|
|
||
| for var in aws_vars: | ||
| assert var not in result_env, f"{var} leaked into subprocess env" |
| "AWS_PROFILE", | ||
| "AWS_ROLE_ARN", | ||
| "AWS_WEB_IDENTITY_TOKEN_FILE", | ||
| "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI", |
| # Listed here so tools/environments/local.py's subprocess blocklist | ||
| # strips them — otherwise AWS creds leak into terminal/execute_code. | ||
| # Mirrors agent/bedrock_adapter.py::_AWS_CREDENTIAL_ENV_VARS plus the | ||
| # paired secret/session vars boto3 also reads via its default chain. | ||
| api_key_env_vars=( | ||
| "AWS_BEARER_TOKEN_BEDROCK", | ||
| "AWS_ACCESS_KEY_ID", | ||
| "AWS_SECRET_ACCESS_KEY", | ||
| "AWS_SESSION_TOKEN", | ||
| "AWS_PROFILE", | ||
| "AWS_ROLE_ARN", | ||
| "AWS_WEB_IDENTITY_TOKEN_FILE", | ||
| "AWS_CONTAINER_CREDENTIALS_RELATIVE_URI", | ||
| "AWS_CONTAINER_CREDENTIALS_FULL_URI", | ||
| "AWS_CONTAINER_AUTHORIZATION_TOKEN", | ||
| ), |
|
CI fixup in 338660514: moved the AWS credential env vars out of the bedrock |
|
This new change is wrong. The original fix was correct and the invariant is completely bogus. It must be replaced with the exact opposite assertion -- that all of the listed environment variables are present. |
3386605 to
b77ecd5
Compare
…klist (NousResearch#32314) The built-in `bedrock` provider uses `auth_type="aws_sdk"` and previously declared `api_key_env_vars=()` — empty. The subprocess sanitization in `tools/environments/local.py::_build_provider_env_blocklist()` iterates `pconfig.api_key_env_vars` to derive its blocklist, so AWS credentials (`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, `AWS_BEARER_TOKEN_BEDROCK`, `AWS_PROFILE`, etc.) were never blocked and inherited into agent-spawned terminal/execute_code/MCP subprocesses. Any subprocess that auto-discovers AWS credentials (e.g. `opencode models`) silently saw the user's full Bedrock catalog and could make Bedrock or other AWS calls the agent did not intend to expose. Populate `bedrock.api_key_env_vars` with the canonical AWS SDK credential set so the existing blocklist mechanism strips them. The set mirrors `agent/bedrock_adapter.py::_AWS_CREDENTIAL_ENV_VARS` (the runtime detector used by `resolve_aws_auth_env_var()`) plus the paired secret/session vars that boto3 reads via its default credential chain. The `_HERMES_PROVIDER_ENV_FORCE_` opt-in path still works for legitimate AWS-aware subprocesses that need the credentials. Tests: - `tests/tools/test_local_env_blocklist.py::TestProviderEnvBlocklist::test_aws_credential_vars_are_stripped` asserts the full set is removed from a LocalEnvironment subprocess env. - `tests/tools/test_local_env_blocklist.py::TestBlocklistCoverage::test_aws_sdk_provider_vars_covered` asserts the set is present in `_HERMES_PROVIDER_ENV_BLOCKLIST`. Both fail on main without the auth.py change.
Listing the AWS credential env vars on bedrock's ProviderConfig as api_key_env_vars broke the test_bedrock_has_no_api_key_env_vars invariant — bedrock uses auth_type="aws_sdk" by design, so its api_key_env_vars must stay empty. Move the AWS_BEARER_TOKEN_BEDROCK / AWS_ACCESS_KEY_ID / AWS_PROFILE / session/role/container vars into the hardcoded extras set inside _build_provider_env_blocklist() instead. Same blocklist coverage, no schema overload — the regression test for NousResearch#32314 still passes because the vars are still present in _HERMES_PROVIDER_ENV_BLOCKLIST, just sourced from the extras list rather than the registry derivation.
b77ecd5 to
c624aba
Compare
|
Superseded by #34498 (merged to main). Thanks for the early + well-tested catch on #32314 — you correctly identified that bedrock's The merged fix narrows the strip to |
What does this PR do?
The built-in
bedrockprovider usesauth_type=\"aws_sdk\"and previously declaredapi_key_env_vars=()— an empty tuple. The subprocess sanitization intools/environments/local.py::_build_provider_env_blocklist()iterates each provider'sapi_key_env_varsto derive its blocklist, so AWS credentials (AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY,AWS_BEARER_TOKEN_BEDROCK,AWS_PROFILE, …) were never blocked and inherited into agent-spawned terminal/execute_code/MCP subprocesses. Any subprocess that auto-discovers AWS credentials (e.g.opencode models) silently saw the user's full Bedrock catalog and could make Bedrock or other AWS API calls the agent did not intend to expose.This populates
bedrock.api_key_env_varswith the canonical AWS SDK credential set so the existing blocklist mechanism strips them. The set mirrorsagent/bedrock_adapter.py::_AWS_CREDENTIAL_ENV_VARS(the runtime detector used byresolve_aws_auth_env_var()) plus the paired secret/session vars boto3 reads via its default credential chain.The
_HERMES_PROVIDER_ENV_FORCE_opt-in path still works for legitimate AWS-aware subprocesses that need to inherit credentials (the standard escape hatch for the rest of the blocklist).Related Issue
Fixes #32314
Type of Change
Changes Made
How to Test
Checklist
Code
Documentation & Housekeeping
Related / Positioning
Audited siblings
Only one provider in
PROVIDER_REGISTRYdeclaresauth_type=\"aws_sdk\"today (bedrock at `hermes_cli/auth.py:450`); the other two non-api_key/non-oauth providers are `external_process` and `oauth_minimax`, which use their own credential surfaces. No widening needed.