feat(agents): test bedrock connection#2687
Conversation
|
✅ No security or compliance issues detected. Reviewed everything up to 524ed65. Security Overview
Detected Code Changes
|
f91f191 to
72eaed5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72eaed5667
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
3 issues found across 9 files
Confidence score: 3/5
- There is moderate merge risk because
tracecat/agent/catalog/router.pyintroduces a concrete permission mismatch: requiringagent:updatefor pre-save Bedrock testing can block users who only have create rights from validating before creating entries. tracecat/agent/catalog/bedrock_validation.pyshould normalize HTTPX/network failures intoBedrockVerificationError; otherwise timeout/connection errors may surface as untyped exceptions and create inconsistent error handling.frontend/src/components/organization/org-settings-agent.tsxuses a global last-viewed workspace cookie in the test payload, which can send the wrong workspace context and trigger false 404 verification failures across org/workspace boundaries.- Pay close attention to
tracecat/agent/catalog/router.py,tracecat/agent/catalog/bedrock_validation.py,frontend/src/components/organization/org-settings-agent.tsx- permission gating, exception wrapping, and workspace scoping can all directly affect Bedrock verification behavior.
You’re at about 97% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tracecat/agent/catalog/bedrock_validation.py">
<violation number="1" location="tracecat/agent/catalog/bedrock_validation.py:211">
P2: Wrap HTTPX request failures so network/timeout errors are returned as BedrockVerificationError instead of leaking as untyped exceptions.</violation>
</file>
<file name="frontend/src/components/organization/org-settings-agent.tsx">
<violation number="1" location="frontend/src/components/organization/org-settings-agent.tsx:356">
P2: Using a global last-viewed workspace cookie directly in the Bedrock test payload can cause false 404 verification failures when the cookie points to a stale or other-org workspace.</violation>
</file>
<file name="tracecat/agent/catalog/router.py">
<violation number="1" location="tracecat/agent/catalog/router.py:105">
P2: The new Bedrock pre-save test route requires `agent:update`, which can block users who have create permission but not update permission from validating before creating a catalog entry.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6510b6f5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d837164118
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
d837164 to
eb6da60
Compare
eb6da60 to
832ac68
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 832ac68168
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
832ac68 to
524ed65
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 524ed65fc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| inference_profile_id: z.string().trim().optional(), | ||
| model_id: z.string().trim().optional(), | ||
| use_converse: z.boolean().default(false), | ||
| use_converse: z.boolean().default(true), |
There was a problem hiding this comment.
is making this default intentional?
| """Resolve configured Bedrock credentials into an explicit auth shape.""" | ||
| if credentials.get("AWS_ROLE_ARN"): | ||
| try: | ||
| return await asyncio.to_thread(_assume_bedrock_role, credentials) |
There was a problem hiding this comment.
nit: could use aioboto3 instead
| bedrock = session.client("bedrock", region_name=region) | ||
| runtime = session.client("bedrock-runtime", region_name=region) | ||
| if is_inference_profile: | ||
| details = _check_profile_response( | ||
| bedrock.get_inference_profile(inferenceProfileIdentifier=target_id) | ||
| ) | ||
| else: | ||
| details = _check_model_availability_response( | ||
| bedrock.get_foundation_model_availability(modelId=target_id) | ||
| ) |
| async def _verify_with_api_key( | ||
| *, | ||
| api_key: str, | ||
| region: str, | ||
| target_id: str, | ||
| is_inference_profile: bool, | ||
| use_converse: bool, | ||
| ) -> dict[str, Any]: | ||
| encoded_target = quote(target_id, safe="") | ||
| if is_inference_profile: | ||
| details = _check_profile_response( | ||
| await _request_bedrock_api_key( | ||
| method="GET", | ||
| url=f"https://bedrock.{region}.amazonaws.com/inference-profiles/{encoded_target}", | ||
| api_key=api_key, | ||
| ) | ||
| ) | ||
| else: | ||
| details = _check_model_availability_response( | ||
| await _request_bedrock_api_key( | ||
| method="GET", | ||
| url=f"https://bedrock.{region}.amazonaws.com/foundation-model-availability/{encoded_target}", | ||
| api_key=api_key, | ||
| ) | ||
| ) | ||
|
|
||
| if not use_converse: | ||
| raise BedrockVerificationError( | ||
| "Runtime verification requires the Bedrock Converse API. Enable Use Converse API and try again." | ||
| ) | ||
|
|
||
| await _request_bedrock_api_key( | ||
| method="POST", | ||
| url=f"https://bedrock-runtime.{region}.amazonaws.com/model/{encoded_target}/converse", | ||
| api_key=api_key, | ||
| json={ | ||
| "messages": _PING_MESSAGES, | ||
| "inferenceConfig": _PING_INFERENCE_CONFIG, | ||
| }, | ||
| ) | ||
| return details | {"runtime_test": "converse"} |
There was a problem hiding this comment.
we recently updated boto3, are there any sdk methods that can replace these?
|
|
||
| try: | ||
| return await asyncio.to_thread( | ||
| _verify_with_boto3, |
| params: BedrockCatalogTest = Body(...), | ||
| ) -> BedrockCatalogTestResponse: | ||
| """Verify an unsaved Bedrock catalog target.""" | ||
| assert role.organization_id is not None # OrgUserRole guarantees this |
There was a problem hiding this comment.
throw http exception instead
| credentials = await self.get_provider_credentials(provider) | ||
| if credentials is None: | ||
| return None | ||
| credentials = self._with_bedrock_organization_external_id( |
There was a problem hiding this comment.
why can't this live inside _augment_runtime_provider_credentials? i don't think the service needs to leak bedrock concepts outside of the provider-specific metch-case
Summary by cubic
Add a Bedrock connection test so users can verify a model or inference profile before saving. Defaults to the Converse API and auto-injects org-scoped AWS External IDs for Bedrock role assumptions.
/organization/agent-catalog/bedrock/testverifies unsaved targets with control-plane checks (profile ACTIVE or model available/authorized) and a Converse runtime ping. Supports AWS role (server builds External ID from org ID when missing), static keys, orAWS_BEARER_TOKEN_BEDROCK. RequiresAWS_REGION, returns non-sensitive details, and handles missing org credentials gracefully.use_conversenow defaults to true across create/update/test and the form. Server injects org-scoped External ID for Bedrock role credentials across provider, catalog, and runtime; workspace-scoped External ID is used only for workspace runtime. Error messages reference “AWS External ID.”Written for commit 524ed65. Summary will update on new commits. Review in cubic