Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion litellm/llms/bedrock/base_aws_llm.py
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ def _auth_with_web_identity_token(
"RoleSessionName": aws_session_name,
"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"}}}]}',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 UserAgent condition silently removed

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 No tests added for this fix

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)

}

# Add ExternalId parameter if provided
Expand Down
Loading