Skip to content

fix(cdk): honor Bedrock geography in MicroVM grants (#955) - #960

Open
proton0210 wants to merge 2 commits into
aws-samples:mainfrom
proton0210:fix/955-microvm-bedrock-geo
Open

proton0210 wants to merge 2 commits into
aws-samples:mainfrom
proton0210:fix/955-microvm-bedrock-geo

Conversation

@proton0210

@proton0210 proton0210 commented Oct 18, 2026 •

Copy link
Copy Markdown

Lambda MicroVM execution roles grant us.* inference profiles even when the deployment invokes global.* or eu.*. Normal task calls use the per-task SessionRole, whose grants already follow bedrockGeoRegion. The mismatch causes AccessDenied when the credentials helper falls back to the compute role and that fallback persists beyond a retry. Resolve bedrockGeoRegion with the shared resolver and build each profile ID with inferenceProfileId, restoring the documented ambient-credential fallback.

Area

  • cdk — infrastructure, handlers, constructs
  • agent — Python runtime / Docker image
  • cli — bgagent client
  • docs — guides or design sources (docs/guides/, docs/design/)
  • tooling — root mise.toml, scripts, CI workflows

Related

Fixes #955. Implements the scope proposed in the issue comment.

Changes

  • Use the shared geography resolver and inference-profile helper for the MicroVM execution-role grants.

  • Add synthesis regressions for global, eu, explicit us, and omitted context. Assert the account/Region-scoped profile ARNs, bare foundation-model ARNs, unchanged invoke actions and resource count, and absence of stale us.* profiles for non-US geographies.

  • Update both shared-model documentation comments to describe all three grant sites, including the MicroVM execution role.

Validation

  • Focused construct and shared-model suites: 120 tests pass.
  • Regression check against the original construct: global and eu fail as expected; explicit and default us pass.
  • CDK TypeScript compilation and ESLint pass.
  • mise run build: agent quality (1,800 tests), CLI (1,079 tests), docs, drift checks, CDK compilation/lint, and bundled synthesis pass. CDK reports 5,767 passing tests and one failure in the existing ECS network-extraction comparison (test/stacks/network.test.ts:307), where Docker asset hashes differ between two templates. After the full build finishes, an isolated rerun of that ECS group passes all 10 tests. The full parallel build is therefore not reported as green.

Independent reviewer validation confirms the fix across eight geography configurations and three backends, including live sandbox checks of the fallback permissions. The reviewer also reproduced the ECS asset-hash failure on pristine main and traced it to .jest-cache inside the Docker build context. These independent results are separate from the local validation above.

Three audit passes covered configuration propagation, synthesized IAM scope/default behavior, and the final diff and regression evidence. No live AWS deployment was performed.

Approval status

Draft while #955 awaits a permitted maintainer's approved label and assignment under ADR-003.

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

Resolve the configured geography so the fallback compute-role grants match the inference profiles delivered to the agent. Cover global, eu, explicit us, and omitted context in synthesis regressions.

Fixes aws-samples#955

Co-Authored-By: Codex <codex@openai.com>
@laithalsaadoon

Copy link
Copy Markdown
Contributor

We measured this PR at aa271e10 against main (6dad935f). The fix is correct and complete for the MicroVM backend. Two notes: the impact is narrower than #955 states, and the network.test.ts:307 failure in your validation is pre-existing on main.

What we checked

  • Your tests: lambda-microvm-compute.test.ts passes 81/81 on the PR head. With only your test file copied onto main's source, the global and eu cells fail and us/unset pass, matching your regression check.
  • Every geography and backend: we synthesized AgentStack for 3 backends (AgentCore default, ecs, lambda-microvm) and 8 bedrockGeoRegion values (unset, us, global, eu, apac, au, jp, us-gov) on both trees, and collected every bedrock:InvokeModel* resource per role, including AWS::IAM::ManagedPolicy. On main the MicroVM execution role grants us. profiles in all 8 cells. On the PR head its inference-profile set equals the SessionRole's in all 8, its foundation-model resources are unchanged, and every other role in all 24 cells is identical to main.
  • Sweep: no other us. literal builds a grant in cdk/src. After this change all three grant sites (AgentCore runtime, ECS task role, MicroVM execution role) resolve the geography.
  • Live, in a sandbox account, with four throwaway roles carrying the synthesized statements and converse on Haiku 5.5: main's MicroVM statement is denied on global. and allowed on us.; the PR's statement and the SessionRole's statement are allowed on global.; a no-grant control is denied on both.

Impact

On MicroVM, Claude Code signs Bedrock calls with whatever agent/src/bedrock_creds_helper.py returns (awsCredentialExport in agent/managed-settings.json). On the normal path that is the per-task SessionRole, and its grant already follows bedrockGeoRegion (cdk/src/stacks/agent.ts:775-812). The compute-role grant is used only when the helper falls back to ambient credentials.

We ran the shipped helper with the pinned CLI (2.1.301 from claude-agent-sdk==0.2.168), ambient credentials from a role carrying main's MicroVM statement, and ANTHROPIC_MODEL=global.anthropic.claude-haiku-5-5:

Cell Helper returned Result
attribution file names the SessionRole SessionRole credentials ok
attribution file names a role the compute role cannot assume compute-role credentials, no Expiration 403, is_error: true
same, with this PR's statement on the compute role compute-role credentials ok
helper falls back on call 1, succeeds on call 2 call 2 came 2.3 s after call 1 ok (control without the failure: 1 call)

So on main, a MicroVM task on global (the shipped cdk.json) fails only when the fallback lasts longer than one retry. Two cases do that: the agent could not write the attribution file (agent/src/runner.py:101 logs a WARN and continues), or the SessionRole assume keeps failing, which agent/src/aws_session.py also treats as fatal for tenant-data access. After a single transient STS failure, Claude Code ran awsCredentialExport again and the request succeeded. This is a regression of the fail-open that lambda-microvm-compute.ts:1196-1200 documents. It does not make every task fail at turn 0. The fix is still right, and the priority is the maintainers' call.

network.test.ts:307

This failure is pre-existing on main and unrelated to this PR. Jest's .jest-cache sits inside the agent image's repo-root Docker build context, so a cold-cache full run changes the image asset hash between the two synths that test compares. We reproduced it on pristine main in 3 of 3 cold runs on 2026-10-08, and .dockerignore has not changed since. It passes alone, as you found. Commit 75cfb090 on #165 adds **/.jest-cache/ to .dockerignore and is unmerged.

Doc lines

cdk/src/constructs/bedrock-models.ts:106 and :141 both say "Both grant sites"; after this PR there are three.

— drafted by Bonk

Address review feedback on aws-samples#960. Refs aws-samples#955.

Co-Authored-By: Codex <codex@openai.com>
@proton0210

Copy link
Copy Markdown
Author

@laithalsaadoon Thanks for the detailed validation. Addressed the documentation feedback in df5fb39: both comments in bedrock-models.ts now describe all three grant sites, with the MicroVM execution role named explicitly.

I also corrected the PR description to scope the impact to persistent ambient-credential fallback and linked your findings about the pre-existing ECS asset-hash test failure. The issue body remains for its author/maintainers to update.

Validation for this follow-up: CDK ESLint and git diff --check pass. The commit changes comments only; the existing implementation and regression tests are unchanged.

Could you please take another look at the updated PR? It remains a draft pending the maintainer approval required for #955.

@proton0210
proton0210 marked this pull request as ready for review October 18, 2026 06:24
@proton0210
proton0210 requested review from a team and backgroundagents as code owners October 18, 2026 06:24
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@6dad935). Learn more about missing BASE report.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #960   +/-   ##
=======================================
  Coverage        ?   92.60%           
=======================================
  Files           ?      353           
  Lines           ?   101994           
  Branches        ?    11620           
=======================================
  Hits            ?    94453           
  Misses          ?     7541           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(cdk): lambda-microvm grants hardcode the us. inference-profile prefix instead of bedrockGeoRegion

3 participants