Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR modifies the hardcoded inline session policy passed to Key changes:
Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| litellm/llms/bedrock/base_aws_llm.py | Single-line change to the AWS STS AssumeRoleWithWebIdentity inline session policy: adds 3 Bedrock guardrail actions and silently removes the aws:UserAgent condition — the removal is undocumented and weakens the credential scope; no tests are included. |
Sequence Diagram
sequenceDiagram
participant LiteLLM
participant STS as AWS STS
participant Bedrock as AWS Bedrock
LiteLLM->>STS: AssumeRoleWithWebIdentity(RoleArn, WebIdentityToken, Policy)
Note over STS: Inline session policy now allows:<br/>bedrock:InvokeModel<br/>bedrock:InvokeModelWithResponseStream<br/>bedrock:ApplyGuardrail (NEW)<br/>bedrock:GetGuardrail (NEW)<br/>bedrock:ListGuardrails (NEW)<br/>Condition: aws:SecureTransport=true<br/>(UserAgent restriction REMOVED)
STS-->>LiteLLM: Temporary Credentials (AccessKeyId, SecretKey, SessionToken)
LiteLLM->>Bedrock: bedrock:InvokeModel / bedrock:InvokeModelWithResponseStream
LiteLLM->>Bedrock: bedrock:ApplyGuardrail / bedrock:GetGuardrail / bedrock:ListGuardrails
Last reviewed commit: "iam policy fix"
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "WebIdentityToken": oidc_token, | ||
| "DurationSeconds": 3600, | ||
| "Policy": '{"Version":"2012-10-17","Statement":[{"Sid":"BedrockLiteLLM","Effect":"Allow","Action":["bedrock:InvokeModel","bedrock:InvokeModelWithResponseStream"],"Resource":"*","Condition":{"Bool":{"aws:SecureTransport":"true"},"StringLike":{"aws:UserAgent":"litellm/*"}}}]}', | ||
| "Policy": '{"Version":"2012-10-17","Statement":[{"Sid":"BedrockLiteLLM","Effect":"Allow","Action":["bedrock:InvokeModel","bedrock:InvokeModelWithResponseStream","bedrock:ApplyGuardrail","bedrock:GetGuardrail","bedrock:ListGuardrails"],"Resource":"*","Condition":{"Bool":{"aws:SecureTransport":"true"}}}]}', |
There was a problem hiding this comment.
The original policy included "StringLike":{"aws:UserAgent":"litellm/*"} as an additional session-policy guard, ensuring the scoped temporary credentials could only be exercised by requests that identify themselves as litellm/*. This PR removes that restriction without any explanation.
Even though user-agent values can be spoofed, the removal still weakens the security posture of the session policy by allowing any caller (e.g. AWS CLI, other SDKs, scripts) to make bedrock:Invoke* and guardrail calls with these temporary credentials. If the motivation was that guardrail SDK calls fail the user-agent check, the fix should be to also match the guardrail SDK's user-agent (e.g. "StringLike":{"aws:UserAgent":["litellm/*","boto3/*"]}) rather than dropping the condition entirely.
Could you confirm whether removing this condition is intentional and, if so, document the reason here?
| "WebIdentityToken": oidc_token, | ||
| "DurationSeconds": 3600, | ||
| "Policy": '{"Version":"2012-10-17","Statement":[{"Sid":"BedrockLiteLLM","Effect":"Allow","Action":["bedrock:InvokeModel","bedrock:InvokeModelWithResponseStream"],"Resource":"*","Condition":{"Bool":{"aws:SecureTransport":"true"},"StringLike":{"aws:UserAgent":"litellm/*"}}}]}', | ||
| "Policy": '{"Version":"2012-10-17","Statement":[{"Sid":"BedrockLiteLLM","Effect":"Allow","Action":["bedrock:InvokeModel","bedrock:InvokeModelWithResponseStream","bedrock:ApplyGuardrail","bedrock:GetGuardrail","bedrock:ListGuardrails"],"Resource":"*","Condition":{"Bool":{"aws:SecureTransport":"true"}}}]}', |
There was a problem hiding this comment.
The pre-submission checklist and repository policy (rule 47800116) explicitly require at least one test to be included with any bug fix. This PR makes a security-sensitive change to an IAM inline session policy — the scope of affected actions and conditions — but contains no tests verifying:
- That the new guardrail actions (
bedrock:ApplyGuardrail,bedrock:GetGuardrail,bedrock:ListGuardrails) are present in the generated policy. - That the
"Bool":{"aws:SecureTransport":"true"}condition is preserved. - That the policy JSON remains syntactically valid and within the AWS packed-policy size limit.
Please add at least one unit test (mocked) under tests/test_litellm/ that validates the shape of the assume_role_params["Policy"] produced by this code path.
Rule Used: What: Ensure that any PR claiming to fix an issue ... (source)
1 participant
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes