feat: add IAM role authentication for AWS Bedrock - #268
Conversation
|
Caution Review failedThe pull request is closed. Summary by CodeRabbit
WalkthroughSigning for Bedrock requests was refactored to be context-aware and use the AWS default credential chain when explicit keys are absent; call sites (including streaming) now pass ctx into signing. Documentation and UI were updated to require/handle region and to document IAM role vs explicit credential usage. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant BedrockProvider
participant signAWSRequest
participant AWSConfig
participant Creds
participant SigV4
participant BedrockAPI
Client->>BedrockProvider: Invoke request with ctx, req
BedrockProvider->>signAWSRequest: signAWSRequest(ctx, req, keys, region, service)
alt Explicit keys provided
signAWSRequest->>AWSConfig: Load config with static credentials
else No explicit keys
signAWSRequest->>AWSConfig: LoadDefaultConfig(ctx) (default credential chain)
end
AWSConfig->>Creds: Credentials.Retrieve(ctx)
signAWSRequest->>SigV4: SignHTTP(ctx, req, creds, region, service)
BedrockProvider->>BedrockAPI: Send signed request (or stream)
BedrockAPI-->>BedrockProvider: Response / stream events
BedrockProvider-->>Client: Return response/stream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (9)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
e8185ed to
3e7b965
Compare
d286583 to
bf775a7
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (7)
ui/components/config/provider-form.tsx (3)
809-816: Associate label with input and fix placeholder naming
- Add htmlFor/id for a11y.
- Use the standard env var name AWS_ACCESS_KEY_ID in the placeholder.
Apply this diff:
-<label className="text-sm font-medium">Access Key</label> -<Input - placeholder="your-aws-access-key or env.AWS_ACCESS_KEY" - value={key.bedrock_key_config?.access_key || ''} - onChange={(e) => updateKeyBedrockConfig(index, 'access_key', e.target.value)} - className="transition-all duration-200 ease-in-out" -/> +{(() => { + const id = `bedrock-access-key-${index}` + return ( + <> + <label className="text-sm font-medium" htmlFor={id}>Access Key</label> + <Input + id={id} + placeholder="your-aws-access-key-id or env.AWS_ACCESS_KEY_ID" + value={key.bedrock_key_config?.access_key || ''} + onChange={(e) => updateKeyBedrockConfig(index, 'access_key', e.target.value)} + className="transition-all duration-200 ease-in-out" + /> + </> + ) +})()}
818-825: Mark Secret Key as password and fix a11y and placeholder
- Use input type="password" for secrets.
- Add htmlFor/id for a11y.
- Placeholder should be AWS_SECRET_ACCESS_KEY.
Apply this diff:
-<label className="text-sm font-medium">Secret Key</label> -<Input - placeholder="your-aws-secret-key or env.AWS_SECRET_KEY" - value={key.bedrock_key_config?.secret_key || ''} - onChange={(e) => updateKeyBedrockConfig(index, 'secret_key', e.target.value)} - className="transition-all duration-200 ease-in-out" -/> +{(() => { + const id = `bedrock-secret-key-${index}` + return ( + <> + <label className="text-sm font-medium" htmlFor={id}>Secret Key</label> + <Input + id={id} + type="password" + placeholder="your-aws-secret-access-key or env.AWS_SECRET_ACCESS_KEY" + value={key.bedrock_key_config?.secret_key || ''} + onChange={(e) => updateKeyBedrockConfig(index, 'secret_key', e.target.value)} + className="transition-all duration-200 ease-in-out" + /> + </> + ) +})()}
70-77: Default us-east-1 region is reasonable, but consider surfacing ARN input (optional)You default Region and include arn in data shape, but there’s no UI field to set ARN, which is needed for inference profiles. Either:
- Add an optional ARN input when Bedrock is selected, or
- Drop arn from defaults to avoid hinting at a field users can’t set.
Also applies to: 358-366, 451-459
docs/usage/http-transport/configuration/providers.md (1)
149-153: Use the standard AWS env var namesReplace AWS_ACCESS_KEY with AWS_ACCESS_KEY_ID for consistency with AWS tooling.
Apply this diff:
- "access_key": "env.AWS_ACCESS_KEY", + "access_key": "env.AWS_ACCESS_KEY_ID",core/providers/bedrock.go (3)
231-235: Compile error: cannot range over integer when pre-warming poolfor range over an int won’t compile. Use a counted for loop.
Apply this diff:
- // Pre-warm response pools - for range config.ConcurrencyAndBufferSize.Concurrency { - bedrockChatResponsePool.Put(&BedrockChatResponse{}) - - } + // Pre-warm response pools + for i := 0; i < config.ConcurrencyAndBufferSize.Concurrency; i++ { + bedrockChatResponsePool.Put(&BedrockChatResponse{}) + }
925-934: Consider consistent path escaping for model identifiersYou PathEscape model for embeddings but not for chat/converse paths unless ARN/deployment is used. For consistency and safety (colons, etc.), consider always escaping the model segment.
Example:
- When building path := fmt.Sprintf("%s/converse", model), wrap model with url.PathEscape(model).
Also applies to: 1158-1160, 1217-1219
1046-1113: Update AWS request signing: remove hard-coded Accept, set per-route, and cache config
- Remove the line
fromreq.Header.Set("Accept", "application/json")signAWSRequestin core/providers/bedrock.go.- In
(*BedrockProvider).completeRequest(core/providers/bedrock.go around line 253), after creating the HTTP request and before callingsignAWSRequest, add:req.Header.Set("Accept", "application/json")- In
(*BedrockProvider).ChatCompletionStream(core/providers/bedrock.go around line 1270), after creating the HTTP request and before signing, add:req.Header.Set("Accept", "application/vnd.amazon.eventstream")- To improve performance, cache the loaded
aws.Configper region instead of callingconfig.LoadDefaultConfigon every request.- SigV4 service name “bedrock” is correct for Amazon Bedrock Runtime signing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
core/providers/bedrock.go(5 hunks)docs/usage/http-transport/configuration/providers.md(5 hunks)docs/usage/key-management.md(2 hunks)docs/usage/providers.md(1 hunks)ui/components/config/provider-form.tsx(9 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
📚 Learning: 2025-06-04T09:07:20.867Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Applied to files:
docs/usage/http-transport/configuration/providers.mddocs/usage/providers.mddocs/usage/key-management.mdcore/providers/bedrock.goui/components/config/provider-form.tsx
📚 Learning: 2025-07-08T18:21:31.772Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/README.md:34-39
Timestamp: 2025-07-08T18:21:31.772Z
Learning: In the Bifrost Docker container (transports/Dockerfile), the entrypoint is configured to use `/app/data` as the default app directory, so Docker volume mounts to `/app/data` work automatically without needing to specify the `-app-dir` flag in the docker run command. This is different from the Go binary usage where `-app-dir` needs to be explicitly specified.
Applied to files:
docs/usage/http-transport/configuration/providers.md
🧬 Code Graph Analysis (2)
core/providers/bedrock.go (4)
core/schemas/bifrost.go (2)
BifrostError(681-690)Bedrock(44-44)ui/lib/types/logs.ts (1)
BifrostError(194-200)ui/lib/types/config.ts (1)
BedrockKeyConfig(33-40)core/schemas/account.go (1)
BedrockKeyConfig(36-43)
ui/components/config/provider-form.tsx (3)
ui/lib/utils/validation.ts (1)
isValidDeployments(236-263)ui/components/ui/alert.tsx (3)
Alert(41-41)AlertTitle(41-41)AlertDescription(41-41)ui/components/ui/input.tsx (1)
Input(7-22)
🪛 LanguageTool
docs/usage/http-transport/configuration/providers.md
[grammar] ~131-~131: Use correct spacing
Context: ...ential configuration and IAM role-based authentication. #### Explicit Credentials Configuration ``...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~133-~133: Use correct spacing
Context: ...ntication. #### Explicit Credentials Configuration json { "providers": { "bedrock": { "keys": [ { "models": [ "anthropic.claude-v2:1", "mistral.mixtral-8x7b-instruct-v0:1", "mistral.mistral-large-2402-v1:0", "anthropic.claude-3-sonnet-20240229-v1:0" ], "weight": 1.0, "bedrock_key_config": { "access_key": "env.AWS_ACCESS_KEY", "secret_key": "env.AWS_SECRET_ACCESS_KEY", "session_token": "env.AWS_SESSION_TOKEN", "region": "us-east-1" } } ], "network_config": { "default_request_timeout_in_seconds": 30, "max_retries": 1, "retry_backoff_initial_ms": 100, "retry_backoff_max_ms": 2000 }, "concurrency_and_buffer_size": { "concurrency": 3, "buffer_size": 10 } } } } #### IAM Role Authentication (Recommended) ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~171-~171: Use correct spacing
Context: ...} } ``` #### IAM Role Authentication (Recommended) For production environments, use IAM rol...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~173-~173: There might be a mistake here.
Context: ...role authentication instead of explicit credentials: json { "providers": { "bedrock": { "keys": [ { "models": [ "anthropic.claude-v2:1", "mistral.mixtral-8x7b-instruct-v0:1", "mistral.mistral-large-2402-v1:0", "anthropic.claude-3-sonnet-20240229-v1:0" ], "weight": 1.0, "bedrock_key_config": { "region": "us-east-1" // No access_key or secret_key - uses IAM role automatically } } ], "network_config": { "default_request_timeout_in_seconds": 30, "max_retries": 1, "retry_backoff_initial_ms": 100, "retry_backoff_max_ms": 2000 }, "concurrency_and_buffer_size": { "concurrency": 3, "buffer_size": 10 } } } } **IAM R...
(QB_NEW_EN_OTHER)
[grammar] ~209-~209: There might be a mistake here.
Context: ... 10 } } } } ``` IAM Role Setup: 1. EC2/ECS/Lambda: Attach IAM role with ...
(QB_NEW_EN_OTHER)
[grammar] ~211-~211: There might be a mistake here.
Context: .../Lambda**: Attach IAM role with Bedrock permissions 2. Local Development: Use AWS CLI configu...
(QB_NEW_EN_OTHER)
[grammar] ~212-~212: There might be a mistake here.
Context: ... CLI configured profiles or environment variables 3. Container Deployments: Use IAM roles f...
(QB_NEW_EN_OTHER)
[grammar] ~213-~213: Use proper capitalization
Context: ...AM roles for service accounts (IRSA) on EKS Required IAM Policy: ```json { "Ver...
(QB_NEW_EN_OTHER_ERROR_IDS_6)
[grammar] ~215-~215: There might be a mistake here.
Context: ... accounts (IRSA) on EKS Required IAM Policy: json { "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Action": [ "bedrock:InvokeModel", "bedrock:InvokeModelWithResponseStream" ], "Resource": "arn:aws:bedrock:*::foundation-model/*" } ] } ### Azure OpenAI ```json...
(QB_NEW_EN_OTHER)
🪛 markdownlint-cli2 (0.17.2)
docs/usage/providers.md
315-315: Inline HTML
Element: details
(MD033, no-inline-html)
316-316: Inline HTML
Element: summary
(MD033, no-inline-html)
316-316: Inline HTML
Element: strong
(MD033, no-inline-html)
450-450: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
459-459: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
467-467: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 Biome (2.1.2)
ui/components/config/provider-form.tsx
[error] 809-809: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 818-818: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 836-836: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (5)
docs/usage/key-management.md (1)
648-675: IAM Role default config looks good; keep pre-validation minimalDefaulting BedrockKeyConfig with empty AccessKey/SecretKey and a Region for IAM role auth aligns with the code and the team’s preference to let AWS validate credentials. LGTM.
ui/components/config/provider-form.tsx (2)
182-205: Validation rules for IAM role vs. explicit creds are correct
- Allowing both Access/Secret to be empty (IAM) or both present (explicit) and requiring Region is the right balance.
- This stays consistent with the preference to let AWS surface auth errors.
620-629: Nice UX touch: inline IAM Role guidanceThe inline alert clarifies IAM role usage and avoids misconfiguration. Looks good.
core/providers/bedrock.go (2)
295-298: IAM role fallback wired correctly into request path — LGTMSwitching to signAWSRequest with ctx and default chain fallback is correct and aligns with IAM role support. Good use of context.
1335-1338: Streaming path uses central signer correctlySigning stream requests via signAWSRequest with ctx + default chain fallback is correct.
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (5)
docs/usage/providers.md (1)
260-275: Fix invalid JSON: trailing comma breaks the configuration example.The bedrock provider JSON has a trailing comma after the keys array (Line 274). This renders the snippet invalid JSON and will confuse users copy-pasting it.
Apply this diff to remove the trailing comma:
] - ], + ] } } }docs/usage/http-transport/configuration/providers.md (1)
149-153: Use standard AWS env var name for Access Key.The example uses env.AWS_ACCESS_KEY, which is non-standard and inconsistent with AWS conventions and other examples in the repo. Prefer env.AWS_ACCESS_KEY_ID.
Apply this diff:
- "access_key": "env.AWS_ACCESS_KEY", + "access_key": "env.AWS_ACCESS_KEY_ID", "secret_key": "env.AWS_SECRET_ACCESS_KEY",core/providers/bedrock.go (1)
1046-1113: Robust SigV4: default credential chain when keys are absent; minor optimization possible.The implementation correctly:
- Sets Content-Type/Accept
- Computes payload hash
- Loads AWS config with region
- Uses default chain if keys are empty; otherwise explicit static provider
- Retrieves credentials and signs via v4 signer
Optional improvements:
- Avoid reloading AWS config on every request. Consider caching aws.Config per region/provider instance, or injecting a CredentialsProvider derived from the key to reduce per-request overhead.
- If you expect large bodies in future, avoid full body read by streaming or ensuring the payload hash is computed without duplicating large buffers (not critical for current JSON bodies).
ui/components/config/provider-form.tsx (2)
809-843: Fix a11y: associate labels with inputs and correct AWS env var placeholders.Labels aren’t associated with inputs (lint/a11y/noLabelWithoutControl). Also, the placeholders use non-standard AWS env var names; use AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY for consistency.
Apply this diff:
-<label className="text-sm font-medium">Access Key</label> +<label className="text-sm font-medium" htmlFor={`bedrock-access-key-${index}`}>Access Key</label> <Input - placeholder="your-aws-access-key or env.AWS_ACCESS_KEY" + placeholder="your-aws-access-key or env.AWS_ACCESS_KEY_ID" + id={`bedrock-access-key-${index}`} value={key.bedrock_key_config?.access_key || ''} onChange={(e) => updateKeyBedrockConfig(index, 'access_key', e.target.value)} className="transition-all duration-200 ease-in-out" /> -<label className="text-sm font-medium">Secret Key</label> +<label className="text-sm font-medium" htmlFor={`bedrock-secret-key-${index}`}>Secret Key</label> <Input - placeholder="your-aws-secret-key or env.AWS_SECRET_KEY" + placeholder="your-aws-secret-key or env.AWS_SECRET_ACCESS_KEY" + id={`bedrock-secret-key-${index}`} value={key.bedrock_key_config?.secret_key || ''} onChange={(e) => updateKeyBedrockConfig(index, 'secret_key', e.target.value)} className="transition-all duration-200 ease-in-out" /> -<label className="text-sm font-medium">Region (Required)</label> +<label className="text-sm font-medium" htmlFor={`bedrock-region-${index}`}>Region (Required)</label> <Input placeholder="us-east-1 or env.AWS_REGION" + id={`bedrock-region-${index}`} value={key.bedrock_key_config?.region || ''} onChange={(e) => updateKeyBedrockConfig(index, 'region', e.target.value)} className="transition-all duration-200 ease-in-out" />
848-876: Use Bedrock-specific placeholders for Deployments to reduce confusion.The current placeholder shows OpenAI models (“gpt-4”, “gpt-3.5-turbo”). For Bedrock, prefer examples like anthropic.claude-3-5-sonnet-20241022-v2:0 mapped to an inference profile.
Example placeholder:
{"anthropic.claude-3-5-sonnet-20241022-v2:0": "my-inference-profile-id"}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
core/providers/bedrock.go(5 hunks)docs/usage/http-transport/configuration/providers.md(5 hunks)docs/usage/key-management.md(2 hunks)docs/usage/providers.md(1 hunks)ui/components/config/provider-form.tsx(9 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
📚 Learning: 2025-06-04T09:07:20.867Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Applied to files:
docs/usage/providers.mddocs/usage/key-management.mdcore/providers/bedrock.godocs/usage/http-transport/configuration/providers.mdui/components/config/provider-form.tsx
📚 Learning: 2025-07-08T18:21:31.772Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/README.md:34-39
Timestamp: 2025-07-08T18:21:31.772Z
Learning: In the Bifrost Docker container (transports/Dockerfile), the entrypoint is configured to use `/app/data` as the default app directory, so Docker volume mounts to `/app/data` work automatically without needing to specify the `-app-dir` flag in the docker run command. This is different from the Go binary usage where `-app-dir` needs to be explicitly specified.
Applied to files:
docs/usage/http-transport/configuration/providers.md
🧬 Code Graph Analysis (2)
core/providers/bedrock.go (2)
core/schemas/bifrost.go (2)
BifrostError(681-690)Bedrock(44-44)core/schemas/account.go (1)
BedrockKeyConfig(36-43)
ui/components/config/provider-form.tsx (3)
ui/lib/utils/validation.ts (1)
isValidDeployments(236-263)ui/components/ui/alert.tsx (3)
Alert(41-41)AlertTitle(41-41)AlertDescription(41-41)ui/components/ui/input.tsx (1)
Input(7-22)
🪛 LanguageTool
docs/usage/providers.md
[grammar] ~318-~318: Use correct spacing
Context: ... role-based authentication for enhanced security. #### **Explicit Credentials (Traditional Method...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~320-~320: Use correct spacing
Context: ...### Explicit Credentials (Traditional Method) Go Package: ```go func (a *MyAccount)...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~322-~322: There might be a mistake here.
Context: ...redentials (Traditional Method)** Go Package: go func (a *MyAccount) GetKeysForProvider(ctx *context.Context, provider schemas.ModelProvider) ([]schemas.Key, error) { if provider == schemas.Bedrock { return []schemas.Key{ { Models: []string{"anthropic.claude-3-5-sonnet-20241022-v2:0"}, Weight: 1.0, BedrockKeyConfig: &schemas.BedrockKeyConfig{ AccessKey: os.Getenv("AWS_ACCESS_KEY_ID"), SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), SessionToken: os.Getenv("AWS_SESSION_TOKEN"), // Optional Region: "us-east-1", }, }, }, nil } return nil, fmt.Errorf("provider not configured") } HTTP Transport: ...
(QB_NEW_EN_OTHER)
[grammar] ~344-~344: There might be a mistake here.
Context: ...provider not configured") } **HTTP Transport:** json { "providers": { "bedrock": { "keys": [ { "models": ["anthropic.claude-3-5-sonnet-20241022-v2:0"], "weight": 1.0, "bedrock_key_config": { "access_key": "env.AWS_ACCESS_KEY_ID", "secret_key": "env.AWS_SECRET_ACCESS_KEY", "session_token": "env.AWS_SESSION_TOKEN", "region": "us-east-1" } } ] } } } ``` #### **IAM Role Authentic...
(QB_NEW_EN_OTHER)
[grammar] ~367-~367: Use correct spacing
Context: ...AM Role Authentication (Recommended for Production)** For enhanced security, Bifrost supports ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~369-~369: Use correct spacing
Context: ...ased authentication when running in AWS environments. Go Package: ```go func (a *MyAccount)...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~371-~371: There might be a mistake here.
Context: ...when running in AWS environments. Go Package: go func (a *MyAccount) GetKeysForProvider(ctx *context.Context, provider schemas.ModelProvider) ([]schemas.Key, error) { if provider == schemas.Bedrock { return []schemas.Key{ { Models: []string{"anthropic.claude-3-5-sonnet-20241022-v2:0"}, Weight: 1.0, BedrockKeyConfig: &schemas.BedrockKeyConfig{ // Leave AccessKey and SecretKey empty for IAM role authentication AccessKey: "", SecretKey: "", Region: "us-east-1", }, }, }, nil } return nil, fmt.Errorf("provider not configured") } HTTP Transport: ...
(QB_NEW_EN_OTHER)
[grammar] ~393-~393: There might be a mistake here.
Context: ...provider not configured") } **HTTP Transport:** json { "providers": { "bedrock": { "keys": [ { "models": ["anthropic.claude-3-5-sonnet-20241022-v2:0"], "weight": 1.0, "bedrock_key_config": { "region": "us-east-1" // No access_key or secret_key - uses IAM role } } ] } } } ``` #### **IAM Role Authenticat...
(QB_NEW_EN_OTHER)
[grammar] ~414-~414: Use correct spacing
Context: ...} } ``` #### IAM Role Authentication Environments IAM role authentication automatically wo...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~416-~416: There might be a mistake here.
Context: ...cation automatically works in these AWS environments: - 🟢 EC2 Instances - Instance profiles ...
(QB_NEW_EN_OTHER)
[grammar] ~418-~418: There might be a problem here.
Context: ...in these AWS environments: - 🟢 EC2 Instances - Instance profiles with attached IAM roles - **🟢...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~419-~419: There might be a mistake here.
Context: ...s with attached IAM roles - 🟢 Lambda Functions - Execution role credentials - **🟢 ECS T...
(QB_NEW_EN_OTHER)
[typographical] ~419-~419: To join two clauses or set off examples, consider using an em dash.
Context: ...d IAM roles - 🟢 Lambda Functions - Execution role credentials - 🟢 ECS Tasks - T...
(QB_NEW_EN_DASH_RULE_EM)
[grammar] ~420-~420: There might be a mistake here.
Context: ...- Execution role credentials - 🟢 ECS Tasks - Task role credentials - 🟢 EKS Pods ...
(QB_NEW_EN_OTHER)
[typographical] ~420-~420: To join two clauses or set off examples, consider using an em dash.
Context: ...n role credentials - 🟢 ECS Tasks - Task role credentials - 🟢 EKS Pods - IA...
(QB_NEW_EN_DASH_RULE_EM)
[grammar] ~421-~421: There might be a problem here.
Context: ...ks** - Task role credentials - 🟢 EKS Pods - IAM roles for service accounts (IRSA) - **🟢 AWS ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~422-~422: There might be a problem here.
Context: ... for service accounts (IRSA) - 🟢 AWS CodeBuild - Service role credentials - 🟢 On-Premises - IAM ...
(QB_NEW_EN_MERGED_MATCH)
[typographical] ~423-~423: To join two clauses or set off examples, consider using an em dash.
Context: ...** - Service role credentials - 🟢 On-Premises - IAM Roles Anywhere for hybrid envir...
(QB_NEW_EN_DASH_RULE_EM)
[grammar] ~423-~423: There might be a mistake here.
Context: ...vice role credentials - 🟢 On-Premises - IAM Roles Anywhere for hybrid environments #### ...
(QB_NEW_EN_OTHER)
[grammar] ~423-~423: Use correct spacing
Context: ...mises** - IAM Roles Anywhere for hybrid environments #### Required IAM Permissions Your IAM rol...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~425-~425: Use correct spacing
Context: ...ybrid environments #### Required IAM Permissions Your IAM role must have the following pe...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~427-~427: There might be a mistake here.
Context: ... Your IAM role must have the following permissions: json { "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Action": [ "bedrock:InvokeModel", "bedrock:InvokeModelWithResponseStream" ], "Resource": [ "arn:aws:bedrock:*::foundation-model/*" ] } ] } #### Setup Examples **E...
(QB_NEW_EN_OTHER)
[grammar] ~447-~447: Use correct spacing
Context: ..." ] } ] } #### **Setup Examples** **EC2 Instance Setup:**bash # 1. Creat...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~449-~449: There might be a mistake here.
Context: ...#### Setup Examples EC2 Instance Setup: bash # 1. Create IAM role with Bedrock permissions # 2. Attach role to EC2 instance # 3. Configure Bifrost with empty credentials export AWS_REGION=us-east-1 # No need to set AWS_ACCESS_KEY_ID or AWS_SECRET_ACCESS_KEY Docker on EC2: ```bash docker ...
(QB_NEW_EN_OTHER)
[grammar] ~458-~458: There might be a mistake here.
Context: ... AWS_SECRET_ACCESS_KEY **Docker on EC2:**bash docker run -p 8080:8080 \ -e AWS_REGION=us-east-1 \ -v $(pwd)/config.json:/app/config/config.json \ maximhq/bifrost **Lambda Function:**javascript ...
(QB_NEW_EN_OTHER)
[grammar] ~466-~466: There might be a mistake here.
Context: ....json \ maximhq/bifrost **Lambda Function:**javascript // Lambda execution role automatically provides credentials // No additional configuration needed </details> <details> <summary><strong>Azure OpenAI Configuration</strong></summary> **Go Package:** go func (a *...
(QB_NEW_EN_OTHER)
🪛 markdownlint-cli2 (0.17.2)
docs/usage/providers.md
315-315: Inline HTML
Element: details
(MD033, no-inline-html)
316-316: Inline HTML
Element: summary
(MD033, no-inline-html)
316-316: Inline HTML
Element: strong
(MD033, no-inline-html)
450-450: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
459-459: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
467-467: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 Biome (2.1.2)
ui/components/config/provider-form.tsx
[error] 809-809: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 818-818: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 836-836: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (5)
docs/usage/key-management.md (1)
648-658: LGTM: Clear IAM-role default path for Bedrock.Defaulting AccessKey/SecretKey to empty and setting Region explicitly is consistent with the new IAM role support and the project’s preference to let AWS handle validation. No changes requested.
core/providers/bedrock.go (2)
295-298: LGTM: Context-aware signing call with IAM-role fallback.Switching to signAWSRequest(ctx, ...) here correctly enables IAM role-based auth when keys are empty and ensures cancellation/timeouts propagate.
1335-1338: LGTM: Streaming path aligned with new signing flow.Streaming requests now use the same context-aware signer with IAM-role support.
ui/components/config/provider-form.tsx (2)
620-629: Nice UX: clear IAM role guidance inline.The alert concisely explains the IAM role path and aligns with the new backend behavior. Good addition.
181-205: Validation rule is reasonable; note prior preference to defer to AWS for empty credentials.You enforce "both empty or both provided" for Access/Secret keys. This prevents a class of user errors, but be aware of the team’s past preference to let AWS perform credential validation. Keeping this in UI is acceptable; just ensure it doesn’t block legitimate flows (e.g., keys fully omitted for IAM, or both present for explicit credentials).
We can regression-test:
- IAM role path: ensure Region is required and keys empty pass validation.
- Explicit path: both keys provided pass, and mismatched pairs fail with the intended message.
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (6)
docs/usage/http-transport/configuration/providers.md (1)
148-154: Use the correct AWS env var names in explicit credentialsUse AWS_ACCESS_KEY_ID (not AWS_ACCESS_KEY).
"bedrock_key_config": { - "access_key": "env.AWS_ACCESS_KEY", + "access_key": "env.AWS_ACCESS_KEY_ID", "secret_key": "env.AWS_SECRET_ACCESS_KEY", "session_token": "env.AWS_SESSION_TOKEN", "region": "us-east-1" }ui/components/config/provider-form.tsx (3)
70-77: Avoid sending empty arn: it breaks backend logicSetting arn: '' will decode in Go as a non-nil pointer to "", and backend checks only for ARN != nil (Line 929 in core/providers/bedrock.go). This will incorrectly alter the request path.
Remove the arn field from default configs so it’s omitted unless the user provides a real value.
} else if (providerName === 'bedrock') { baseKey.bedrock_key_config = { access_key: '', secret_key: '', session_token: '', region: 'us-east-1', - arn: '', deployments: {}, } }Apply a similar change in addKey (Lines 358-365).
451-459: Ensure arn is omitted when empty during updatesIf the user clears the ARN field, keep it undefined so the backend sees nil, not an empty string pointer.
if (!keyToUpdate.bedrock_key_config) { keyToUpdate.bedrock_key_config = { access_key: '', secret_key: '', session_token: '', region: 'us-east-1', - arn: '', deployments: {}, } } - keyToUpdate.bedrock_key_config = { - ...keyToUpdate.bedrock_key_config, - [field]: value, - } + if (field === 'arn' && typeof value === 'string' && value.trim() === '') { + const { arn, ...rest } = keyToUpdate.bedrock_key_config + keyToUpdate.bedrock_key_config = rest + } else { + keyToUpdate.bedrock_key_config = { + ...keyToUpdate.bedrock_key_config, + [field]: value, + } + }Also applies to: 462-465
809-815: A11y: associate labels with inputs; fix env var placeholders
- Add htmlFor/id pairs to satisfy noLabelWithoutControl.
- Correct placeholder env names to AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY.
- <label className="text-sm font-medium">Access Key</label> + <label className="text-sm font-medium" htmlFor={`bedrock-access-key-${index}`}>Access Key</label> <Input - placeholder="your-aws-access-key or env.AWS_ACCESS_KEY" + placeholder="your-aws-access-key-id or env.AWS_ACCESS_KEY_ID" + id={`bedrock-access-key-${index}`} value={key.bedrock_key_config?.access_key || ''} onChange={(e) => updateKeyBedrockConfig(index, 'access_key', e.target.value)} className="transition-all duration-200 ease-in-out" /> ... - <label className="text-sm font-medium">Secret Key</label> + <label className="text-sm font-medium" htmlFor={`bedrock-secret-key-${index}`}>Secret Key</label> <Input - placeholder="your-aws-secret-key or env.AWS_SECRET_KEY" + placeholder="your-aws-secret-access-key or env.AWS_SECRET_ACCESS_KEY" + id={`bedrock-secret-key-${index}`} value={key.bedrock_key_config?.secret_key || ''} onChange={(e) => updateKeyBedrockConfig(index, 'secret_key', e.target.value)} className="transition-all duration-200 ease-in-out" /> ... - <label className="text-sm font-medium">Region (Required)</label> + <label className="text-sm font-medium" htmlFor={`bedrock-region-${index}`}>Region (Required)</label> <Input placeholder="us-east-1 or env.AWS_REGION" + id={`bedrock-region-${index}`} value={key.bedrock_key_config?.region || ''} onChange={(e) => updateKeyBedrockConfig(index, 'region', e.target.value)} className="transition-all duration-200 ease-in-out" />Also applies to: 818-824, 836-843
core/providers/bedrock.go (2)
1046-1050: Do not set Accept in the signer; it breaks streamingSigners should be transport-agnostic. Hardcoding Accept: application/json causes the streaming API to misbehave.
func signAWSRequest(ctx context.Context, req *http.Request, accessKey, secretKey string, sessionToken *string, region, service string) *schemas.BifrostError { // Set required headers before signing - req.Header.Set("Content-Type", "application/json") - req.Header.Set("Accept", "application/json") + req.Header.Set("Content-Type", "application/json") @@ - if err := signer.SignHTTP(ctx, creds, req, bodyHash, service, region, time.Now()); err != nil { + if err := signer.SignHTTP(ctx, creds, req, bodyHash, service, region, time.Now()); err != nil { return newBifrostOperationError("failed to sign request", err, schemas.Bedrock) }Follow-up: Accept should be set by the caller:
- application/json for non-streaming
- application/vnd.amazon.eventstream for streaming
Also applies to: 1108-1109
1046-1096: Optional: avoid re-loading AWS config on every requestconfig.LoadDefaultConfig(ctx, ...) is relatively heavy. Consider caching aws.Config per (region, credential-mode) and reusing the signer for better throughput.
If you want, I can sketch a small LRU keyed by {region, explicitCredsHash} with a refresh TTL to reuse cfg and signer.
♻️ Duplicate comments (1)
ui/components/config/provider-form.tsx (1)
358-365: Repeat: remove default arn to avoid sending empty valueSame rationale as above; omit arn unless provided.
} else if (selectedProvider === 'bedrock') { newKey.bedrock_key_config = { access_key: '', secret_key: '', session_token: '', region: 'us-east-1', - arn: '', deployments: {}, } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
core/providers/bedrock.go(5 hunks)docs/usage/http-transport/configuration/providers.md(5 hunks)docs/usage/key-management.md(2 hunks)docs/usage/providers.md(1 hunks)ui/components/config/provider-form.tsx(9 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
📚 Learning: 2025-06-04T09:07:20.867Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Applied to files:
docs/usage/key-management.mddocs/usage/providers.mddocs/usage/http-transport/configuration/providers.mdui/components/config/provider-form.tsxcore/providers/bedrock.go
📚 Learning: 2025-07-08T18:21:31.772Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/README.md:34-39
Timestamp: 2025-07-08T18:21:31.772Z
Learning: In the Bifrost Docker container (transports/Dockerfile), the entrypoint is configured to use `/app/data` as the default app directory, so Docker volume mounts to `/app/data` work automatically without needing to specify the `-app-dir` flag in the docker run command. This is different from the Go binary usage where `-app-dir` needs to be explicitly specified.
Applied to files:
docs/usage/http-transport/configuration/providers.md
🧬 Code Graph Analysis (1)
core/providers/bedrock.go (2)
core/schemas/bifrost.go (2)
BifrostError(681-690)Bedrock(44-44)core/schemas/account.go (1)
BedrockKeyConfig(36-43)
🪛 LanguageTool
docs/usage/providers.md
[grammar] ~318-~318: Use correct spacing
Context: ... role-based authentication for enhanced security. #### **Explicit Credentials (Traditional Method...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~320-~320: Use correct spacing
Context: ...### Explicit Credentials (Traditional Method) Go Package: ```go func (a *MyAccount)...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~322-~322: There might be a mistake here.
Context: ...redentials (Traditional Method)** Go Package: go func (a *MyAccount) GetKeysForProvider(ctx *context.Context, provider schemas.ModelProvider) ([]schemas.Key, error) { if provider == schemas.Bedrock { return []schemas.Key{ { Models: []string{"anthropic.claude-3-5-sonnet-20241022-v2:0"}, Weight: 1.0, BedrockKeyConfig: &schemas.BedrockKeyConfig{ AccessKey: os.Getenv("AWS_ACCESS_KEY_ID"), SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), SessionToken: os.Getenv("AWS_SESSION_TOKEN"), // Optional Region: "us-east-1", }, }, }, nil } return nil, fmt.Errorf("provider not configured") } HTTP Transport: ...
(QB_NEW_EN_OTHER)
[grammar] ~344-~344: There might be a mistake here.
Context: ...provider not configured") } **HTTP Transport:** json { "providers": { "bedrock": { "keys": [ { "models": ["anthropic.claude-3-5-sonnet-20241022-v2:0"], "weight": 1.0, "bedrock_key_config": { "access_key": "env.AWS_ACCESS_KEY_ID", "secret_key": "env.AWS_SECRET_ACCESS_KEY", "session_token": "env.AWS_SESSION_TOKEN", "region": "us-east-1" } } ] } } } ``` #### **IAM Role Authentic...
(QB_NEW_EN_OTHER)
[grammar] ~367-~367: Use correct spacing
Context: ...AM Role Authentication (Recommended for Production)** For enhanced security, Bifrost supports ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~369-~369: Use correct spacing
Context: ...ased authentication when running in AWS environments. Go Package: ```go func (a *MyAccount)...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~371-~371: There might be a mistake here.
Context: ...when running in AWS environments. Go Package: go func (a *MyAccount) GetKeysForProvider(ctx *context.Context, provider schemas.ModelProvider) ([]schemas.Key, error) { if provider == schemas.Bedrock { return []schemas.Key{ { Models: []string{"anthropic.claude-3-5-sonnet-20241022-v2:0"}, Weight: 1.0, BedrockKeyConfig: &schemas.BedrockKeyConfig{ // Leave AccessKey and SecretKey empty for IAM role authentication AccessKey: "", SecretKey: "", Region: "us-east-1", }, }, }, nil } return nil, fmt.Errorf("provider not configured") } HTTP Transport: ...
(QB_NEW_EN_OTHER)
[grammar] ~393-~393: There might be a mistake here.
Context: ...provider not configured") } **HTTP Transport:** json { "providers": { "bedrock": { "keys": [ { "models": ["anthropic.claude-3-5-sonnet-20241022-v2:0"], "weight": 1.0, "bedrock_key_config": { "region": "us-east-1" // No access_key or secret_key - uses IAM role } } ] } } } ``` #### **IAM Role Authenticat...
(QB_NEW_EN_OTHER)
[grammar] ~414-~414: Use correct spacing
Context: ...} } ``` #### IAM Role Authentication Environments IAM role authentication automatically wo...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~416-~416: There might be a mistake here.
Context: ...cation automatically works in these AWS environments: - 🟢 EC2 Instances - Instance profiles ...
(QB_NEW_EN_OTHER)
[grammar] ~418-~418: There might be a problem here.
Context: ...in these AWS environments: - 🟢 EC2 Instances - Instance profiles with attached IAM roles - **🟢...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~419-~419: There might be a mistake here.
Context: ...s with attached IAM roles - 🟢 Lambda Functions - Execution role credentials - **🟢 ECS T...
(QB_NEW_EN_OTHER)
[typographical] ~419-~419: To join two clauses or set off examples, consider using an em dash.
Context: ...d IAM roles - 🟢 Lambda Functions - Execution role credentials - 🟢 ECS Tasks - T...
(QB_NEW_EN_DASH_RULE_EM)
[grammar] ~420-~420: There might be a mistake here.
Context: ...- Execution role credentials - 🟢 ECS Tasks - Task role credentials - 🟢 EKS Pods ...
(QB_NEW_EN_OTHER)
[typographical] ~420-~420: To join two clauses or set off examples, consider using an em dash.
Context: ...n role credentials - 🟢 ECS Tasks - Task role credentials - 🟢 EKS Pods - IA...
(QB_NEW_EN_DASH_RULE_EM)
[grammar] ~421-~421: There might be a problem here.
Context: ...ks** - Task role credentials - 🟢 EKS Pods - IAM roles for service accounts (IRSA) - **🟢 AWS ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~422-~422: There might be a problem here.
Context: ... for service accounts (IRSA) - 🟢 AWS CodeBuild - Service role credentials - 🟢 On-Premises - IAM ...
(QB_NEW_EN_MERGED_MATCH)
[typographical] ~423-~423: To join two clauses or set off examples, consider using an em dash.
Context: ...** - Service role credentials - 🟢 On-Premises - IAM Roles Anywhere for hybrid envir...
(QB_NEW_EN_DASH_RULE_EM)
[grammar] ~423-~423: There might be a mistake here.
Context: ...vice role credentials - 🟢 On-Premises - IAM Roles Anywhere for hybrid environments #### ...
(QB_NEW_EN_OTHER)
[grammar] ~423-~423: Use correct spacing
Context: ...mises** - IAM Roles Anywhere for hybrid environments #### Required IAM Permissions Your IAM rol...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~425-~425: Use correct spacing
Context: ...ybrid environments #### Required IAM Permissions Your IAM role must have the following pe...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~427-~427: There might be a mistake here.
Context: ... Your IAM role must have the following permissions: json { "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Action": [ "bedrock:InvokeModel", "bedrock:InvokeModelWithResponseStream" ], "Resource": [ "arn:aws:bedrock:*::foundation-model/*" ] } ] } #### Setup Examples **E...
(QB_NEW_EN_OTHER)
[grammar] ~447-~447: Use correct spacing
Context: ..." ] } ] } #### **Setup Examples** **EC2 Instance Setup:**bash # 1. Creat...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~449-~449: There might be a mistake here.
Context: ...#### Setup Examples EC2 Instance Setup: bash # 1. Create IAM role with Bedrock permissions # 2. Attach role to EC2 instance # 3. Configure Bifrost with empty credentials export AWS_REGION=us-east-1 # No need to set AWS_ACCESS_KEY_ID or AWS_SECRET_ACCESS_KEY Docker on EC2: ```bash docker ...
(QB_NEW_EN_OTHER)
[grammar] ~458-~458: There might be a mistake here.
Context: ... AWS_SECRET_ACCESS_KEY **Docker on EC2:**bash docker run -p 8080:8080 \ -e AWS_REGION=us-east-1 \ -v $(pwd)/config.json:/app/config/config.json \ maximhq/bifrost **Lambda Function:**javascript ...
(QB_NEW_EN_OTHER)
[grammar] ~466-~466: There might be a mistake here.
Context: ....json \ maximhq/bifrost **Lambda Function:**javascript // Lambda execution role automatically provides credentials // No additional configuration needed </details> <details> <summary><strong>Azure OpenAI Configuration</strong></summary> **Go Package:** go func (a *...
(QB_NEW_EN_OTHER)
docs/usage/http-transport/configuration/providers.md
[grammar] ~131-~131: Use correct spacing
Context: ...ential configuration and IAM role-based authentication. #### Explicit Credentials Configuration ``...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~133-~133: Use correct spacing
Context: ...ntication. #### Explicit Credentials Configuration json { "providers": { "bedrock": { "keys": [ { "models": [ "anthropic.claude-v2:1", "mistral.mixtral-8x7b-instruct-v0:1", "mistral.mistral-large-2402-v1:0", "anthropic.claude-3-sonnet-20240229-v1:0" ], "weight": 1.0, "bedrock_key_config": { "access_key": "env.AWS_ACCESS_KEY", "secret_key": "env.AWS_SECRET_ACCESS_KEY", "session_token": "env.AWS_SESSION_TOKEN", "region": "us-east-1" } } ], "network_config": { "default_request_timeout_in_seconds": 30, "max_retries": 1, "retry_backoff_initial_ms": 100, "retry_backoff_max_ms": 2000 }, "concurrency_and_buffer_size": { "concurrency": 3, "buffer_size": 10 } } } } #### IAM Role Authentication (Recommended) ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~171-~171: Use correct spacing
Context: ...} } ``` #### IAM Role Authentication (Recommended) For production environments, use IAM rol...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~173-~173: There might be a mistake here.
Context: ...role authentication instead of explicit credentials: json { "providers": { "bedrock": { "keys": [ { "models": [ "anthropic.claude-v2:1", "mistral.mixtral-8x7b-instruct-v0:1", "mistral.mistral-large-2402-v1:0", "anthropic.claude-3-sonnet-20240229-v1:0" ], "weight": 1.0, "bedrock_key_config": { "region": "us-east-1" // No access_key or secret_key - uses IAM role automatically } } ], "network_config": { "default_request_timeout_in_seconds": 30, "max_retries": 1, "retry_backoff_initial_ms": 100, "retry_backoff_max_ms": 2000 }, "concurrency_and_buffer_size": { "concurrency": 3, "buffer_size": 10 } } } } **IAM R...
(QB_NEW_EN_OTHER)
[grammar] ~209-~209: There might be a mistake here.
Context: ... 10 } } } } ``` IAM Role Setup: 1. EC2/ECS/Lambda: Attach IAM role with ...
(QB_NEW_EN_OTHER)
[grammar] ~211-~211: There might be a mistake here.
Context: .../Lambda**: Attach IAM role with Bedrock permissions 2. Local Development: Use AWS CLI configu...
(QB_NEW_EN_OTHER)
[grammar] ~212-~212: There might be a mistake here.
Context: ... CLI configured profiles or environment variables 3. Container Deployments: Use IAM roles f...
(QB_NEW_EN_OTHER)
[grammar] ~213-~213: Use proper capitalization
Context: ...AM roles for service accounts (IRSA) on EKS Required IAM Policy: ```json { "Ver...
(QB_NEW_EN_OTHER_ERROR_IDS_6)
[grammar] ~215-~215: There might be a mistake here.
Context: ... accounts (IRSA) on EKS Required IAM Policy: json { "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Action": [ "bedrock:InvokeModel", "bedrock:InvokeModelWithResponseStream" ], "Resource": "arn:aws:bedrock:*::foundation-model/*" } ] } ### Azure OpenAI ```json...
(QB_NEW_EN_OTHER)
🪛 markdownlint-cli2 (0.17.2)
docs/usage/providers.md
315-315: Inline HTML
Element: details
(MD033, no-inline-html)
316-316: Inline HTML
Element: summary
(MD033, no-inline-html)
316-316: Inline HTML
Element: strong
(MD033, no-inline-html)
450-450: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
459-459: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
467-467: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 Biome (2.1.2)
ui/components/config/provider-form.tsx
[error] 809-809: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 818-818: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 836-836: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (3)
ui/components/config/provider-form.tsx (2)
621-629: Nice UX: IAM role guidance inlineThe alert explaining the IAM role path is helpful and aligns with backend behavior.
36-36: Verified: AlertTitle export existsConfirmed that
AlertTitleis defined (line 25) and exported (line 41) inui/alert.tsx. No changes are required inui/components/config/provider-form.tsx.docs/usage/providers.md (1)
430-446: IAM policy snippet is goodThe required permissions look correct for invoke and streaming across foundation model resources.
52fba51 to
9a3c96e
Compare
3e7b965 to
1a9c8c7
Compare
9a3c96e to
cceefd0
Compare
1a9c8c7 to
122ad1e
Compare
cceefd0 to
593af3c
Compare
122ad1e to
45da154
Compare
The base branch was changed.
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (7)
docs/usage/providers.md (4)
179-183: Bedrock Go example misuses Key.Value and has wrong env var; use BedrockKeyConfig insteadFor Bedrock, credentials belong in BedrockKeyConfig, not in Key.Value. Also fix the env var typo.
- { - Value: os.Getenv("AWS_ACCESS_KEY_ID_ID"), - Models: []string{"anthropic.claude-3-5-sonnet-20241022-v2:0"}, - Weight: 1.0, - }, + { + Models: []string{"anthropic.claude-3-5-sonnet-20241022-v2:0"}, + Weight: 1.0, + BedrockKeyConfig: &schemas.BedrockKeyConfig{ + AccessKey: os.Getenv("AWS_ACCESS_KEY_ID"), + SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), + SessionToken: os.Getenv("AWS_SESSION_TOKEN"), // Optional + Region: StrPtr("us-east-1"), + }, + },
266-271: Fix env var and remove incorrect ARN in HTTP JSON example (Bedrock)
- access_key should be env.AWS_ACCESS_KEY_ID
- arn here suggests an IAM Role ARN, which is invalid for bedrock_key_config.arn
- "access_key": "env.AWS_ACCESS_KEY_ID_ID", + "access_key": "env.AWS_ACCESS_KEY_ID", "secret_key": "env.AWS_SECRET_ACCESS_KEY", "session_token": "env.AWS_SESSION_TOKEN", "region": "us-east-1", - "arn": "arn:aws:iam::123456789012:role/BedrockRole" + // Optional: if using Bedrock inference profiles, set to your Bedrock inference-profile ARN base, e.g.: + // "arn": "arn:aws:bedrock:us-east-1:123456789012:inference-profile"
541-545: Vertex Go example: use Region field (not Location) to match schemas.VertexKeyConfigPer project convention, VertexKeyConfig uses Region (and in JSON: "region").
- VertexKeyConfig: &schemas.VertexKeyConfig{ - ProjectID: "your-project-id", - Location: "us-central1", - AuthCredentials: os.Getenv("VERTEX_AUTH_CREDENTIALS"), // Or read from file - }, + VertexKeyConfig: &schemas.VertexKeyConfig{ + ProjectID: "your-project-id", + Region: "us-central1", + AuthCredentials: os.Getenv("VERTEX_AUTH_CREDENTIALS"), // Or read from file + },
171-183: Fix AWS_ACCESS_KEY_ID typo across docs and code examplesMultiple instances of the incorrect environment variable name
AWS_ACCESS_KEY_ID_IDwere found. Please update all occurrences toAWS_ACCESS_KEY_ID.Affected locations:
- tests/core-providers/README.md:64
- transports/config.example.json:81
- docs/usage/providers.md:179, 266, 332, 355, 455
- docs/usage/key-management.md:668, 734
- docs/usage/go-package/account.md:100
You can apply a bulk replacement, for example:
sed -i 's/AWS_ACCESS_KEY_ID_ID/AWS_ACCESS_KEY_ID/g' <file>docs/usage/http-transport/configuration/providers.md (1)
505-509: Invalid JSON: missing comma after weightAdd a trailing comma after "weight": 1.0 in the Bedrock block.
- "weight": 1.0 + "weight": 1.0, "bedrock_key_config": {ui/components/config/provider-form.tsx (2)
817-833: Improve a11y and secrecy: associate labels and mask sensitive inputs
- Add htmlFor/id to labels/inputs (a11y)
- Mask Secret Key as password
- <label className="text-sm font-medium">Access Key</label> - <Input - placeholder="your-aws-access-key or env.AWS_ACCESS_KEY_ID" - value={key.bedrock_key_config?.access_key || ''} - onChange={(e) => updateKeyBedrockConfig(index, 'access_key', e.target.value)} - className="transition-all duration-200 ease-in-out" - /> + {(() => { + const id = `bedrock-access-key-${index}` + return ( + <> + <label className="text-sm font-medium" htmlFor={id}>Access Key</label> + <Input + id={id} + placeholder="your-aws-access-key or env.AWS_ACCESS_KEY_ID" + value={key.bedrock_key_config?.access_key || ''} + onChange={(e) => updateKeyBedrockConfig(index, 'access_key', e.target.value)} + className="transition-all duration-200 ease-in-out" + /> + </> + ) + })()} ... - <label className="text-sm font-medium">Secret Key</label> - <Input - placeholder="your-aws-secret-key or env.AWS_SECRET_ACCESS_KEY" - value={key.bedrock_key_config?.secret_key || ''} - onChange={(e) => updateKeyBedrockConfig(index, 'secret_key', e.target.value)} - className="transition-all duration-200 ease-in-out" - /> + {(() => { + const id = `bedrock-secret-key-${index}` + return ( + <> + <label className="text-sm font-medium" htmlFor={id}>Secret Key</label> + <Input + id={id} + type="password" + placeholder="your-aws-secret-key or env.AWS_SECRET_ACCESS_KEY" + value={key.bedrock_key_config?.secret_key || ''} + onChange={(e) => updateKeyBedrockConfig(index, 'secret_key', e.target.value)} + className="transition-all duration-200 ease-in-out" + /> + </> + ) + })()}
835-841: Mask session token and associate label/inputSession tokens are sensitive; mask them and add htmlFor/id.
- <label className="text-sm font-medium">Session Token (Optional)</label> - <Input - placeholder="your-aws-session-token or env.AWS_SESSION_TOKEN" - value={key.bedrock_key_config?.session_token || ''} - onChange={(e) => updateKeyBedrockConfig(index, 'session_token', e.target.value)} - className="transition-all duration-200 ease-in-out" - /> + {(() => { + const id = `bedrock-session-token-${index}` + return ( + <> + <label className="text-sm font-medium" htmlFor={id}>Session Token (Optional)</label> + <Input + id={id} + type="password" + placeholder="your-aws-session-token or env.AWS_SESSION_TOKEN" + value={key.bedrock_key_config?.session_token || ''} + onChange={(e) => updateKeyBedrockConfig(index, 'session_token', e.target.value)} + className="transition-all duration-200 ease-in-out" + /> + </> + ) + })()}
♻️ Duplicate comments (12)
core/providers/bedrock.go (3)
1046-1050: Remove hard-coded Accept header from signerThe signer should not set Accept. Different endpoints require different Accepts (JSON vs EventStream). Let callers decide.
Apply this diff:
func signAWSRequest(ctx context.Context, req *http.Request, accessKey, secretKey string, sessionToken *string, region, service string) *schemas.BifrostError { // Set required headers before signing req.Header.Set("Content-Type", "application/json") - req.Header.Set("Accept", "application/json")
295-297: Do not set Accept in the signer; set Accept at call-site for non-streaming requestsHardcoding Accept in the signer breaks streaming (which needs eventstream). Set Accept here to JSON and remove it from signAWSRequest.
Apply this diff:
// Set any extra headers from network config setExtraHeadersHTTP(req, provider.networkConfig.ExtraHeaders, nil) - // Sign the request using either explicit credentials or IAM role authentication + // Non-streaming: Accept JSON responses (must be set before signing) + req.Header.Set("Accept", "application/json") + // Sign the request using either explicit credentials or IAM role authentication if err := signAWSRequest(ctx, req, config.AccessKey, config.SecretKey, config.SessionToken, region, "bedrock"); err != nil { return nil, err }
1335-1337: Streaming must set Accept to AWS EventStream before signingBedrock streaming uses AWS EventStream; Accept must be application/vnd.amazon.eventstream and included in the signature.
Apply this diff:
// Set any extra headers from network config setExtraHeadersHTTP(req, provider.networkConfig.ExtraHeaders, nil) - // Sign the request using either explicit credentials or IAM role authentication + // Streaming: Accept AWS EventStream frames (must be set before signing) + req.Header.Set("Accept", "application/vnd.amazon.eventstream") + // Sign the request using either explicit credentials or IAM role authentication if signErr := signAWSRequest(ctx, req, key.BedrockKeyConfig.AccessKey, key.BedrockKeyConfig.SecretKey, key.BedrockKeyConfig.SessionToken, region, "bedrock"); signErr != nil { return nil, signErr }docs/usage/key-management.md (3)
722-724: JSON code block contains comments; switch fence to jsonc or remove commentsThe snippet includes // comments but is fenced as JSON. Use jsonc or remove the comments to keep examples copy-pastable.
653-657: *Go example: Region must be a pointer (string), not a string literalschemas.BedrockKeyConfig.Region is a *string. Using a string literal will not compile.
Apply this diff:
- BedrockKeyConfig: &schemas.BedrockKeyConfig{ - AccessKey: "", // Empty for IAM role authentication - SecretKey: "", // Empty for IAM role authentication - Region: "us-east-1", - }, + BedrockKeyConfig: &schemas.BedrockKeyConfig{ + AccessKey: "", // Empty for IAM role authentication + SecretKey: "", // Empty for IAM role authentication + Region: StrPtr("us-east-1"), + },If StrPtr is not available, use a local variable:
region := "us-east-1"; Region: ®ion
734-739: Incorrect ARN example for bedrock_key_config; remove or replace with a Bedrock inference profile ARNbedrock_key_config.arn is used for Bedrock inference profiles (arn:aws:bedrock:...:inference-profile/...). An IAM Role ARN is invalid here.
Apply this diff to remove the misleading line:
- // "arn": "your-arn"Optionally replace with a correct example and a short note:
"arn": "arn:aws:bedrock:us-east-1:123456789012:inference-profile",docs/usage/providers.md (4)
315-365: Go example (explicit credentials): Region pointer fixRegion must be *string.
- BedrockKeyConfig: &schemas.BedrockKeyConfig{ - AccessKey: os.Getenv("AWS_ACCESS_KEY_ID_ID"), - SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), - SessionToken: os.Getenv("AWS_SESSION_TOKEN"), // Optional - Region: "us-east-1", - }, + BedrockKeyConfig: &schemas.BedrockKeyConfig{ + AccessKey: os.Getenv("AWS_ACCESS_KEY_ID"), + SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), + SessionToken: os.Getenv("AWS_SESSION_TOKEN"), // Optional + Region: StrPtr("us-east-1"), + },
374-391: Go example (IAM role): Region pointer fixUse a string pointer for Region.
- BedrockKeyConfig: &schemas.BedrockKeyConfig{ - // Leave AccessKey and SecretKey empty for IAM role authentication - AccessKey: "", - SecretKey: "", - Region: "us-east-1", - }, + BedrockKeyConfig: &schemas.BedrockKeyConfig{ + // Leave AccessKey and SecretKey empty for IAM role authentication + AccessKey: "", + SecretKey: "", + Region: StrPtr("us-east-1"), + },
395-407: JSON contains a comment; use jsonc or remove the commentSwitch the fence to jsonc to keep illustrative comments valid.
449-456: Markdown lint: add blank lines around fenced code blocks (MD031)Insert a blank line before and after each fenced block in “Setup Examples”.
-**EC2 Instance Setup:** -```bash +**EC2 Instance Setup:** + +```bash ... -``` +``` -**Docker on EC2:** -```bash +**Docker on EC2:** + +```bash ... -``` +``` -**Lambda Function:** -```javascript +**Lambda Function:** + +```javascript ... -``` +```Also applies to: 458-464, 466-470
docs/usage/http-transport/configuration/providers.md (1)
175-207: JSON contains comments; mark as jsonc or remove commentsThe IAM Role example embeds inline comments; switch the fence to jsonc to keep examples valid.
ui/components/config/provider-form.tsx (1)
844-850: Add htmlFor/id to Region label and input for a11yAlso matches linter recommendation.
- <label className="text-sm font-medium">Region (Required)</label> - <Input - placeholder="us-east-1 or env.AWS_REGION" - value={key.bedrock_key_config?.region || ''} - onChange={(e) => updateKeyBedrockConfig(index, 'region', e.target.value)} - className="transition-all duration-200 ease-in-out" - /> + {(() => { + const id = `bedrock-region-${index}` + return ( + <> + <label className="text-sm font-medium" htmlFor={id}>Region (Required)</label> + <Input + id={id} + placeholder="us-east-1 or env.AWS_REGION" + value={key.bedrock_key_config?.region || ''} + onChange={(e) => updateKeyBedrockConfig(index, 'region', e.target.value)} + className="transition-all duration-200 ease-in-out" + /> + </> + ) + })()}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
core/providers/bedrock.go(5 hunks)docs/usage/go-package/account.md(2 hunks)docs/usage/http-transport/configuration/providers.md(6 hunks)docs/usage/http-transport/openapi.json(1 hunks)docs/usage/key-management.md(2 hunks)docs/usage/providers.md(3 hunks)tests/core-providers/README.md(1 hunks)transports/config.example.json(1 hunks)ui/components/config/provider-form.tsx(8 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/providers/anthropic.go:358-388
Timestamp: 2025-06-04T05:37:59.699Z
Learning: User Pratham-Mishra04 prefers not to extract small code duplications (around 2 lines) into helper functions, considering the overhead not worth it for such minor repetition.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#267
File: core/providers/cerebras.go:25-31
Timestamp: 2025-08-13T07:56:05.626Z
Learning: Pratham-Mishra04 prefers temporary solutions that prevent correctness issues (like commenting out problematic defer statements) when planning to implement better long-term solutions (like reference counting) in the future, accepting trade-offs like temporary memory leaks for safety.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#102
File: README.md:62-66
Timestamp: 2025-06-19T17:03:03.639Z
Learning: Pratham-Mishra04 prefers using the implicit 'latest' tag for the maximhq/bifrost Docker image rather than pinning to specific versions.
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#143
File: core/mcp.go:155-196
Timestamp: 2025-07-08T15:33:47.698Z
Learning: Pratham-Mishra04 prefers not to add explanatory comments for obvious code patterns, such as the unlock/lock strategy around network I/O operations, considering them self-explanatory to experienced developers.
📚 Learning: 2025-06-04T09:07:20.867Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#54
File: core/providers/bedrock.go:241-252
Timestamp: 2025-06-04T09:07:20.867Z
Learning: In the Bifrost codebase, when working with AWS Bedrock provider authentication, the preference is to let AWS handle access key validation naturally rather than adding preemptive checks for empty/blank access keys. This allows AWS to provide its own authentication error messages which can be more informative than custom validation errors.
Applied to files:
docs/usage/go-package/account.mdcore/providers/bedrock.godocs/usage/key-management.mddocs/usage/http-transport/configuration/providers.mddocs/usage/providers.mdui/components/config/provider-form.tsx
📚 Learning: 2025-07-16T07:13:29.496Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#162
File: transports/bifrost-http/integrations/genai/types.go:0-0
Timestamp: 2025-07-16T07:13:29.496Z
Learning: Pratham-Mishra04 prefers to avoid redundant error handling across architectural layers in the Bifrost streaming implementation. When error handling (such as timeouts, context cancellation, and JSON marshaling failures) is already handled at the provider level, they prefer not to duplicate this logic at the transport integration layer to keep the code simple and avoid unnecessary complexity.
Applied to files:
core/providers/bedrock.go
📚 Learning: 2025-07-18T07:51:10.781Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#170
File: docs/usage/http-transport/configuration/providers.md:192-202
Timestamp: 2025-07-18T07:51:10.781Z
Learning: In Bifrost's VertexKeyConfig struct, the field for specifying the Google Cloud region is called "Region" (corresponding to "region" in JSON configuration), not "location". This is the correct field name to use in vertex_key_config examples.
Applied to files:
docs/usage/key-management.mddocs/usage/providers.md
📚 Learning: 2025-06-19T09:06:25.750Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#97
File: transports/Dockerfile:37-42
Timestamp: 2025-06-19T09:06:25.750Z
Learning: In Docker configurations for this project, plugin-specific environment variables (like MAXIM_LOG_REPO_ID) should not be included in the Dockerfile's ENV section. The architectural goal is to keep Docker images plugin-agnostic and externalize all plugin configuration to runtime via docker run -e flags, rather than baking plugin config into the image.
Applied to files:
docs/usage/http-transport/configuration/providers.md
📚 Learning: 2025-07-08T18:21:31.772Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#149
File: transports/README.md:34-39
Timestamp: 2025-07-08T18:21:31.772Z
Learning: In the Bifrost Docker container (transports/Dockerfile), the entrypoint is configured to use `/app/data` as the default app directory, so Docker volume mounts to `/app/data` work automatically without needing to specify the `-app-dir` flag in the docker run command. This is different from the Go binary usage where `-app-dir` needs to be explicitly specified.
Applied to files:
docs/usage/http-transport/configuration/providers.md
📚 Learning: 2025-06-04T04:58:12.239Z
Learnt from: Pratham-Mishra04
PR: maximhq/bifrost#55
File: core/tests/e2e_tool_test.go:29-30
Timestamp: 2025-06-04T04:58:12.239Z
Learning: In the Bifrost project, environment variables should be used only for secrets (like API keys), not for general configuration. Test parameters like provider and model can be hardcoded at the start of test files for predictability and consistency.
Applied to files:
docs/usage/http-transport/configuration/providers.md
🧬 Code Graph Analysis (2)
core/providers/bedrock.go (2)
core/schemas/bifrost.go (2)
BifrostError(681-690)Bedrock(44-44)core/schemas/account.go (1)
BedrockKeyConfig(36-43)
ui/components/config/provider-form.tsx (3)
ui/lib/utils/validation.ts (1)
isValidDeployments(236-263)ui/components/ui/alert.tsx (3)
Alert(41-41)AlertTitle(41-41)AlertDescription(41-41)ui/components/ui/input.tsx (1)
Input(7-22)
🪛 LanguageTool
docs/usage/http-transport/configuration/providers.md
[grammar] ~131-~131: Use correct spacing
Context: ...ential configuration and IAM role-based authentication. #### Explicit Credentials Configuration ``...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~133-~133: Use correct spacing
Context: ...ntication. #### Explicit Credentials Configuration json { "providers": { "bedrock": { "keys": [ { "models": [ "anthropic.claude-v2:1", "mistral.mixtral-8x7b-instruct-v0:1", "mistral.mistral-large-2402-v1:0", "anthropic.claude-3-sonnet-20240229-v1:0" ], "weight": 1.0, "bedrock_key_config": { "access_key": "env.AWS_ACCESS_KEY_ID", "secret_key": "env.AWS_SECRET_ACCESS_KEY", "session_token": "env.AWS_SESSION_TOKEN", "region": "us-east-1" } } ], "network_config": { "default_request_timeout_in_seconds": 30, "max_retries": 1, "retry_backoff_initial_ms": 100, "retry_backoff_max_ms": 2000 }, "concurrency_and_buffer_size": { "concurrency": 3, "buffer_size": 10 } } } } #### IAM Role Authentication (Recommended) ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~171-~171: Use correct spacing
Context: ...} } ``` #### IAM Role Authentication (Recommended) For production environments, use IAM rol...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~173-~173: There might be a mistake here.
Context: ...role authentication instead of explicit credentials: json { "providers": { "bedrock": { "keys": [ { "models": [ "anthropic.claude-v2:1", "mistral.mixtral-8x7b-instruct-v0:1", "mistral.mistral-large-2402-v1:0", "anthropic.claude-3-sonnet-20240229-v1:0" ], "weight": 1.0, "bedrock_key_config": { "region": "us-east-1" // No access_key or secret_key - uses IAM role automatically } } ], "network_config": { "default_request_timeout_in_seconds": 30, "max_retries": 1, "retry_backoff_initial_ms": 100, "retry_backoff_max_ms": 2000 }, "concurrency_and_buffer_size": { "concurrency": 3, "buffer_size": 10 } } } } **IAM R...
(QB_NEW_EN_OTHER)
[grammar] ~209-~209: There might be a mistake here.
Context: ... 10 } } } } ``` IAM Role Setup: 1. EC2/ECS/Lambda: Attach IAM role with ...
(QB_NEW_EN_OTHER)
[grammar] ~211-~211: There might be a mistake here.
Context: .../Lambda**: Attach IAM role with Bedrock permissions 2. Local Development: Use AWS CLI configu...
(QB_NEW_EN_OTHER)
[grammar] ~212-~212: There might be a mistake here.
Context: ... CLI configured profiles or environment variables 3. Container Deployments: Use IAM roles f...
(QB_NEW_EN_OTHER)
[grammar] ~213-~213: Use proper capitalization
Context: ...AM roles for service accounts (IRSA) on EKS Required IAM Policy: ```json { "Ver...
(QB_NEW_EN_OTHER_ERROR_IDS_6)
[grammar] ~215-~215: There might be a mistake here.
Context: ... accounts (IRSA) on EKS Required IAM Policy: json { "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Action": [ "bedrock:InvokeModel", "bedrock:InvokeModelWithResponseStream" ], "Resource": "arn:aws:bedrock:*::foundation-model/*" } ] } ### Azure OpenAI ```json...
(QB_NEW_EN_OTHER)
🪛 markdownlint-cli2 (0.17.2)
docs/usage/providers.md
315-315: Inline HTML
Element: details
(MD033, no-inline-html)
316-316: Inline HTML
Element: summary
(MD033, no-inline-html)
316-316: Inline HTML
Element: strong
(MD033, no-inline-html)
450-450: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
459-459: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
467-467: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 Biome (2.1.2)
ui/components/config/provider-form.tsx
[error] 817-817: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 826-826: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 844-844: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Graphite / mergeability_check
- GitHub Check: Graphite / mergeability_check
🔇 Additional comments (9)
core/providers/bedrock.go (2)
1069-1093: Credential resolution logic LGTM (explicit creds or default chain)Good use of aws-sdk-go-v2 default credential chain when keys are absent, and explicit credentials when provided. This aligns with IAM role support and keeps AWS error semantics.
1102-1109: Ctx-aware credential retrieval and signing LGTMUsing ctx in Retrieve and SignHTTP is correct and enables proper cancellation/timeouts during auth/signing.
docs/usage/http-transport/openapi.json (1)
2783-2789: LGTM: correct Bedrock env var examplesExamples use env.AWS_ACCESS_KEY_ID and env.AWS_SECRET_ACCESS_KEY, which are the expected names. Ensure other docs stay consistent.
docs/usage/http-transport/configuration/providers.md (3)
149-153: Good fix: standardize on AWS_ACCESS_KEY_ID for explicit credentialsUsing AWS_ACCESS_KEY_ID aligns with AWS SDK conventions.
549-554: Env var section looks correct for AWS BedrockStandard names and optionality are documented accurately.
573-591: Docker examples: env var names are correct and consistentUsing AWS_ACCESS_KEY_ID and AWS_REGION is consistent with the rest of the docs.
Also applies to: 597-602
ui/components/config/provider-form.tsx (3)
182-201: Bedrock validation: solid handling of IAM-role vs explicit-keys and session token
- Enforces “both provided or both empty” for AK/SK
- Blocks session token when using IAM roles
628-637: Nice UX: clear IAM role auth guidance inlineThe alert makes the IAM path discoverable and reduces misconfiguration.
70-77: Default Bedrock region initialized consistentlyDefaulting to us-east-1 reduces user friction and aligns with docs/UI changes.
Also applies to: 366-373, 459-467
| if os.Getenv("AWS_ACCESS_KEY_ID_ID") != "" { | ||
| providers = append(providers, schemas.Bedrock) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Do not gate Bedrock on access key presence; support IAM role (region-only) too
Gating on AWS_ACCESS_KEY_ID_ID both uses a typo and blocks IAM role setups (no keys). Gate on either keys present or region set, or include Bedrock unconditionally.
Apply this diff:
- if os.Getenv("AWS_ACCESS_KEY_ID_ID") != "" {
- providers = append(providers, schemas.Bedrock)
- }
+ // Bedrock: enable when explicit keys are set OR when running with IAM role (region configured)
+ if (os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") != "") || os.Getenv("AWS_REGION") != "" {
+ providers = append(providers, schemas.Bedrock)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if os.Getenv("AWS_ACCESS_KEY_ID_ID") != "" { | |
| providers = append(providers, schemas.Bedrock) | |
| } | |
| // Bedrock: enable when explicit keys are set OR when running with IAM role (region configured) | |
| if (os.Getenv("AWS_ACCESS_KEY_ID") != "" && os.Getenv("AWS_SECRET_ACCESS_KEY") != "") || os.Getenv("AWS_REGION") != "" { | |
| providers = append(providers, schemas.Bedrock) | |
| } |
🤖 Prompt for AI Agents
In docs/usage/go-package/account.md around lines 100 to 102, the code
incorrectly gates adding the Bedrock provider on the misspelled environment
variable AWS_ACCESS_KEY_ID_ID and thus also blocks IAM-role (region-only)
setups; change the check to look for the correct AWS_ACCESS_KEY_ID (and
AWS_SECRET_ACCESS_KEY) and/or the region so Bedrock is enabled when either
explicit keys are present or a region is configured (or alternatively enable
Bedrock unconditionally). Update the conditional to (a) fix the typo to
AWS_ACCESS_KEY_ID, (b) check for AWS_SECRET_ACCESS_KEY as well, and (c) allow
Bedrock if either keys are present OR AWS_REGION (or equivalent region var) is
set; ensure the logic reflects this OR condition and append providers.Bedrock
accordingly.
| AccessKey: os.Getenv("AWS_ACCESS_KEY_ID"), | ||
| SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), | ||
| SessionToken: bifrost.Ptr(os.Getenv("AWS_SESSION_TOKEN")), | ||
| Region: bifrost.Ptr("us-east-1"), | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Respect AWS_REGION in examples (fallback to us-east-1)
Use AWS_REGION when provided; fallback to us-east-1 otherwise. Keeps docs aligned with IAM role workflows.
Apply this diff:
- Region: bifrost.Ptr("us-east-1"),
+ Region: func() *string { r := os.Getenv("AWS_REGION"); if r == "" { r = "us-east-1" }; return bifrost.Ptr(r) }(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| AccessKey: os.Getenv("AWS_ACCESS_KEY_ID"), | |
| SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), | |
| SessionToken: bifrost.Ptr(os.Getenv("AWS_SESSION_TOKEN")), | |
| Region: bifrost.Ptr("us-east-1"), | |
| }, | |
| AccessKey: os.Getenv("AWS_ACCESS_KEY_ID"), | |
| SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), | |
| SessionToken: bifrost.Ptr(os.Getenv("AWS_SESSION_TOKEN")), | |
| Region: func() *string { r := os.Getenv("AWS_REGION"); if r == "" { r = "us-east-1" }; return bifrost.Ptr(r) }(), |
🤖 Prompt for AI Agents
In docs/usage/go-package/account.md around lines 149 to 153, the example
hardcodes Region to "us-east-1"; update it to read AWS_REGION from the
environment and fall back to "us-east-1" when AWS_REGION is empty. Modify the
snippet to obtain region := os.Getenv("AWS_REGION"); if region == "" set region
= "us-east-1"; then pass Region: bifrost.Ptr(region) so examples respect
AWS_REGION while keeping the same default.
| AccessKey: os.Getenv("AWS_ACCESS_KEY_ID_ID"), | ||
| SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), | ||
| Region: "us-east-1", | ||
| }, |
There was a problem hiding this comment.
Fix env var typo and Region pointer in explicit-credentials Go example
- Typo: AWS_ACCESS_KEY_ID_ID → AWS_ACCESS_KEY_ID
- Region must be *string
- BedrockKeyConfig: &schemas.BedrockKeyConfig{
- AccessKey: os.Getenv("AWS_ACCESS_KEY_ID_ID"),
- SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"),
- Region: "us-east-1",
- },
+ BedrockKeyConfig: &schemas.BedrockKeyConfig{
+ AccessKey: os.Getenv("AWS_ACCESS_KEY_ID"),
+ SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"),
+ Region: StrPtr("us-east-1"),
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| AccessKey: os.Getenv("AWS_ACCESS_KEY_ID_ID"), | |
| SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), | |
| Region: "us-east-1", | |
| }, | |
| BedrockKeyConfig: &schemas.BedrockKeyConfig{ | |
| AccessKey: os.Getenv("AWS_ACCESS_KEY_ID"), | |
| SecretKey: os.Getenv("AWS_SECRET_ACCESS_KEY"), | |
| Region: StrPtr("us-east-1"), | |
| }, |
🤖 Prompt for AI Agents
In docs/usage/key-management.md around lines 668 to 671, the Go example has a
typo and wrong type for Region: change os.Getenv("AWS_ACCESS_KEY_ID_ID") to
os.Getenv("AWS_ACCESS_KEY_ID") and make Region a *string (e.g., assign region :=
"us-east-1" and set Region: ®ion or use a pointer expression) so the field
receives a *string instead of a plain string.
| // "access_key": "env.AWS_ACCESS_KEY_ID_ID", | ||
| // "secret_key": "env.AWS_SECRET_ACCESS_KEY", | ||
| // "session_token": "env.AWS_SESSION_TOKEN", |
There was a problem hiding this comment.
Env var typo in JSON example: AWS_ACCESS_KEY_ID_ID → AWS_ACCESS_KEY_ID
This typo breaks explicit-credentials setups.
- // "access_key": "env.AWS_ACCESS_KEY_ID_ID",
+ // "access_key": "env.AWS_ACCESS_KEY_ID",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // "access_key": "env.AWS_ACCESS_KEY_ID_ID", | |
| // "secret_key": "env.AWS_SECRET_ACCESS_KEY", | |
| // "session_token": "env.AWS_SESSION_TOKEN", | |
| // "access_key": "env.AWS_ACCESS_KEY_ID", | |
| // "secret_key": "env.AWS_SECRET_ACCESS_KEY", | |
| // "session_token": "env.AWS_SESSION_TOKEN", |
🤖 Prompt for AI Agents
In docs/usage/key-management.md around lines 734 to 736, the JSON example
contains a typo in the environment variable name ("AWS_ACCESS_KEY_ID_ID");
change it to the correct "AWS_ACCESS_KEY_ID" so explicit-credentials setups work
correctly and update any matching examples to use the proper env var name.
| export AWS_ACCESS_KEY_ID_ID="your-aws-access-key" | ||
| export AWS_SECRET_ACCESS_KEY="your-aws-secret-key" | ||
| export AWS_REGION="us-east-1" | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix env var typo and document IAM role path
AWS_ACCESS_KEY_ID_ID is a typo. Use AWS_ACCESS_KEY_ID. Also clarify that IAM role auth requires only AWS_REGION (no keys).
Apply this diff:
-export AWS_ACCESS_KEY_ID_ID="your-aws-access-key"
-export AWS_SECRET_ACCESS_KEY="your-aws-secret-key"
-export AWS_REGION="us-east-1"
+export AWS_ACCESS_KEY_ID="your-aws-access-key"
+export AWS_SECRET_ACCESS_KEY="your-aws-secret-key"
+export AWS_REGION="us-east-1"
+# If using IAM Role on AWS (recommended), omit AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY; only set AWS_REGION.Run to find other occurrences to correct:
🏁 Script executed:
#!/bin/bash
# Find inconsistent variable name across repo
rg -n --no-heading 'AWS_ACCESS_KEY_ID_ID|env\.AWS_ACCESS_KEY_ID_ID'Length of output: 955
Fix AWS_ACCESS_KEY_ID_ID typo across repository and clarify IAM role usage
The incorrect AWS_ACCESS_KEY_ID_ID appears in multiple places—replace it with AWS_ACCESS_KEY_ID and update docs to note that IAM Role–based auth only requires AWS_REGION.
Files to update:
- transports/config.example.json (line 81)
- tests/core-providers/README.md (line 64)
- docs/usage/providers.md (lines 179, 266, 332, 355, 455)
- docs/usage/key-management.md (lines 668, 734)
- docs/usage/go-package/account.md (line 100)
Apply replacements, for example:
--- a/tests/core-providers/README.md
+++ b/tests/core-providers/README.md
@@ -64,3 +64,5 @@ export AWS_SECRET_ACCESS_KEY="your-aws-secret-key"
export AWS_REGION="us-east-1"
-# If using IAM Role on AWS (recommended), omit AWS_ACCESS_KEY_ID_ID and AWS_SECRET_ACCESS_KEY
+# If using IAM Role on AWS (recommended), omit AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY; only set AWS_REGION.And similarly in JSON and Go files:
-transports/config.example.json: "access_key": "env.AWS_ACCESS_KEY_ID_ID",
+transports/config.example.json: "access_key": "env.AWS_ACCESS_KEY_ID",To bulk-fix:
rg -l 'AWS_ACCESS_KEY_ID_ID' | xargs sed -i 's/AWS_ACCESS_KEY_ID_ID/AWS_ACCESS_KEY_ID/g'| "access_key": "env.AWS_ACCESS_KEY_ID_ID", | ||
| "secret_key": "env.AWS_SECRET_ACCESS_KEY", | ||
| "session_token": "env.AWS_SESSION_TOKEN", | ||
| "deployments": { | ||
| "gpt-4o": "gpt-4o-deployment-id" | ||
| }, | ||
| "arn": "your-arn", | ||
| "region": "us-east-1" | ||
| } |
There was a problem hiding this comment.
Use correct env var for access_key and avoid typos
Replace env.AWS_ACCESS_KEY_ID_ID with env.AWS_ACCESS_KEY_ID. Leaving keys empty will trigger IAM role via default chain; consider noting that in comments near this example elsewhere.
Apply this diff:
- "access_key": "env.AWS_ACCESS_KEY_ID_ID",
+ "access_key": "env.AWS_ACCESS_KEY_ID",
"secret_key": "env.AWS_SECRET_ACCESS_KEY",
"session_token": "env.AWS_SESSION_TOKEN",
"deployments": {
"gpt-4o": "gpt-4o-deployment-id"
},
"arn": "your-arn",
"region": "us-east-1"🤖 Prompt for AI Agents
In transports/config.example.json around lines 81 to 89, fix the typo in the
access key and improve the example: replace "env.AWS_ACCESS_KEY_ID_ID" with
"env.AWS_ACCESS_KEY_ID" and ensure the other AWS env keys remain correctly
named; additionally add a short comment near this example stating that leaving
access_key/secret_key/session_token empty will allow AWS SDK to use the default
credential chain / IAM role, so users know it's intentional.
## Add IAM role authentication support for AWS Bedrock This PR enhances AWS Bedrock integration by adding support for IAM role-based authentication, allowing Bifrost to use the AWS credentials chain when running in AWS environments (EC2, Lambda, ECS, EKS) without requiring explicit access keys. ## Changes - Modified `signAWSRequest` to support IAM role authentication when access keys are not provided - Updated error handling in Bedrock provider to use IAM roles when explicit credentials are missing - Added context propagation to AWS credential retrieval functions - Enhanced documentation with IAM role setup instructions and examples - Added UI support for IAM role configuration with helpful guidance ## Type of change - [x] Feature - [x] Documentation ## Affected areas - [x] Core (Go) - [x] Providers/Integrations - [x] UI (Next.js) - [x] Docs ## How to test Test with IAM role authentication: ```sh # When running on EC2/ECS/Lambda with IAM role export AWS_REGION=us-east-1 # No need to set AWS_ACCESS_KEY_ID or AWS_SECRET_ACCESS_KEY # Run Bifrost go run main.go ``` Test with explicit credentials: ```sh # Set credentials explicitly export AWS_ACCESS_KEY_ID=your-access-key export AWS_SECRET_ACCESS_KEY=your-secret-key export AWS_REGION=us-east-1 # Run Bifrost go run main.go ``` ## Security considerations This PR improves security by: - Supporting IAM role-based authentication (recommended AWS best practice) - Eliminating the need to store AWS credentials in environment variables or config files - Allowing for fine-grained IAM policies to control Bedrock access ## Checklist - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI)
2 participants
Add IAM role authentication support for AWS Bedrock
This PR enhances AWS Bedrock integration by adding support for IAM role-based authentication, allowing Bifrost to use the AWS credentials chain when running in AWS environments (EC2, Lambda, ECS, EKS) without requiring explicit access keys.
Changes
signAWSRequestto support IAM role authentication when access keys are not providedType of change
Affected areas
How to test
Test with IAM role authentication:
Test with explicit credentials:
Security considerations
This PR improves security by:
Checklist