Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Dec 17, 2025

Related GitHub Issue

Addresses: #9481

Description

This PR attempts to address Issue #9481 by fixing the backend ARN validation in src/api/providers/bedrock.ts to support AWS GovCloud (aws-us-gov) and AWS China (aws-cn) region ARNs.

While PR #10110 fixed the webview validation, there was still a strict regex in the backend parseArn() method that only accepted arn:aws: prefix. This fix updates the regex to also accept:

  • arn:aws-us-gov: (AWS GovCloud)
  • arn:aws-cn: (AWS China)

Changes

  • Updated the ARN regex in parseArn() from:

    /^arn:aws:(?:bedrock|sagemaker):...$/

    to:

    /^arn:(?:aws|aws-us-gov|aws-cn):(?:bedrock|sagemaker):...$/
  • Added comprehensive tests for:

    • AWS GovCloud ARN parsing
    • AWS China ARN parsing
    • GovCloud custom ARN handler construction
    • China region custom ARN handler construction
    • Region mismatch detection in GovCloud ARNs

Test Procedure

  1. Run tests: cd src && npx vitest run api/providers/__tests__/bedrock.spec.ts

All 54 tests pass, including 6 new tests for GovCloud/China ARN support.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes.
  • Documentation Impact: I have considered if my changes require documentation updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

  • No documentation updates are required.

Additional Notes

Feedback and guidance are welcome.


Important

Updates parseArn() in bedrock.ts to support AWS GovCloud and China ARNs, with tests added for validation.

  • Behavior:
    • Updates ARN regex in parseArn() in bedrock.ts to support arn:aws-us-gov: and arn:aws-cn:.
    • Handles region mismatch by logging a warning and using the ARN's region.
  • Tests:
    • Adds tests in bedrock.spec.ts for AWS GovCloud and China ARN parsing.
    • Tests custom ARN handling and region mismatch detection for GovCloud and China.
  • Misc:
    • Updates docstring in bedrock.ts to reflect new ARN regex behavior.

This description was created by Ellipsis for d61e4b1. You can customize this summary. It will automatically update as commits are pushed.

@roomote
Copy link
Contributor Author

roomote bot commented Dec 17, 2025

Rooviewer Clock   See task on Roo Cloud

Review complete. No issues found.

The latest change simplifies the ARN regex to accept any partition ([^:]+) instead of explicitly listing aws, aws-us-gov, and aws-cn. This is a good improvement that makes the code more future-proof while maintaining correct ARN validation.

Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Dec 17, 2025
@mrubens
Copy link
Collaborator

mrubens commented Dec 17, 2025

@roomote can you just allow anything for the partition in that regex? We don't actually capture it, right?

@roomote
Copy link
Contributor Author

roomote bot commented Dec 17, 2025

Fixaroo Clock   See task on Roo Cloud

Simplified the ARN regex to accept any partition (using [^:]+ instead of explicitly listing partitions). All 54 tests pass.

View commit | Revert commit

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 17, 2025
@mrubens mrubens marked this pull request as ready for review December 17, 2025 19:50
@mrubens mrubens requested review from cte and jr as code owners December 17, 2025 19:50
@mrubens mrubens merged commit eac0d62 into main Dec 17, 2025
19 checks passed
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Dec 17, 2025
@mrubens mrubens deleted the fix/bedrock-govcloud-arn-validation branch December 17, 2025 19:50
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Dec 17, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Dec 17, 2025
@dosubot dosubot bot added the bug Something isn't working label Dec 17, 2025
@roomote
Copy link
Contributor Author

roomote bot commented Dec 17, 2025

Rooviewer Clock   See task on Roo Cloud

Review complete. No issues found.

The changes correctly update the ARN regex from explicitly matching arn:aws: to using arn:[^:]+: which accepts any AWS partition (aws, aws-us-gov, aws-cn, or future partitions). This is a good improvement that makes the code more future-proof while maintaining correct ARN validation. Comprehensive tests were added for GovCloud and China region support.

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

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

Labels

bug Something isn't working Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants