feat(rust): port BaseAWSLLM auth (credential resolution + SigV4) to litellm-core as a base provider - #33888
Conversation
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
Greptile SummaryThis PR ports Python's
Confidence Score: 4/5Safe to land behind the feature flag; the auth classifier has a behavioral gap that should be fixed before the module is wired to a live route. The whole module is gated behind a default-off cargo feature and nothing is connected to a live Bedrock endpoint yet, which limits blast radius. The WebIdentity classifier silently ignores an explicitly-provided OIDC token whenever session_name is absent, routing the request through a different STS operation — a defect that will be hard to diagnose once the module is wired up. Everything else — the cache port, SigV4 golden-vector parity, IRSA short-circuit, and CI additions — looks correct and well-covered. litellm-rust/crates/core/src/providers/bedrock/aws_base.rs — the WebIdentity classification arm and the Debug derivations on credential types.
|
| Filename | Overview |
|---|---|
| litellm-rust/crates/core/src/providers/bedrock/aws_base.rs | Core Rust port of Python's BaseAWSLLM auth; contains a WebIdentity classification gap where an explicit OIDC token is silently dropped when session_name is absent, plus derived Debug on credential types exposes secrets |
| litellm-rust/crates/core/src/caching/in_memory_cache.rs | Clean 1:1 port of Python's InMemoryCache; heap-based expiration eviction, 200-entry bound, clock injection for deterministic tests, well-covered by unit tests |
| litellm-rust/crates/core/Cargo.toml | AWS SDK deps correctly pinned behind the optional bedrock-auth feature; rustls-only, no OpenSSL |
| .github/workflows/test-rust.yml | Adds Clippy and cargo-test steps for the bedrock-auth feature flag; correctly runs in addition to the baseline workspace checks |
| litellm-rust/crates/core/src/providers/bedrock/constants.rs | Litellm-namespaced AWS env-var constants; all map correctly to their Python counterparts |
Reviews (2): Last reviewed commit: "fix(rust): preserve web identity credent..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
Co-Authored-By: Ishaan Jaffer <155045088+ishaan-berri@users.noreply.github.com>
|
Addressed both review comments in Removed the unused The WebIdentity flow now forwards the STS-returned expiration into Separately restructured the cache: the inline per-module cache was removed and Bedrock now uses a shared @greptileai please re-review |
| if let (Some(token), Some(role), Some(session_name)) = ( | ||
| config.web_identity_token.clone(), | ||
| config.role_name.clone(), | ||
| config.session_name.clone(), | ||
| ) { | ||
| return AwsAuthFlow::WebIdentity { | ||
| token, | ||
| role, | ||
| session_name, | ||
| }; | ||
| } | ||
| if let Some(role) = config.role_name.clone() { | ||
| return AwsAuthFlow::AssumeRole { | ||
| role, | ||
| session_name: config.session_name.clone(), | ||
| }; |
There was a problem hiding this comment.
WebIdentity token silently dropped when
session_name is absent
The WebIdentity arm requires all three of web_identity_token, role_name, and session_name to be Some. When a caller provides web_identity_token + role_name but omits session_name (a common case when AWS_SESSION_NAME is not set), the pattern doesn't match and the code falls through to the AssumeRole arm — silently discarding the OIDC token and issuing a plain sts:AssumeRole instead of sts:AssumeRoleWithWebIdentity. The two STS operations require different IAM policies, so the wrong one may fail or succeed in surprising ways.
The AssumeRole arm already handles an absent session_name by calling default_session_name() in resolve_credentials. The same auto-generation should apply to WebIdentity — either by loosening the classifier pattern to not require session_name, or by accepting Option<String> in the WebIdentity variant and generating a default in resolve_credentials.
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
This is PR 1 of the Rust migration's auth de-risking (Vertex is the follow-up). The question it answers is whether the
boto3/botocoreauth the Python Bedrock path depends on has a maintained Rust equivalent that behaves identically, so the Rust gateway does not have to call back into Python for credentials or signing. It does: the official AWS SDK for Rust.Nothing is wired into a live route yet and the whole module is behind a default-off cargo feature, so there is no proxy endpoint to curl at this stage. The proof that matters here is that Rust and Python produce byte-identical SigV4 output, pinned by one shared golden vector that both a Rust test and a Python
botocoretest assert against, so drift on either side fails a test.Same fixed inputs on both sides (POST to a Bedrock invoke URL, body
{"input":"hello"}, regionus-east-1, servicebedrock, static creds with a session token, signing time frozen to2024-01-02T03:04:05Z); both produceRust side (byte-parity, no-session-token, classification, cache-policy, same-role comparison, no-network resolution):
Python side (recomputes the botocore signature for the identical inputs and asserts the same golden):
A live signed Bedrock call against a real account will be added as a follow-up comment on this PR, captured at the head commit.
Type
🆕 New Feature
Changes
Ports the auth surface of Python's
BaseAWSLLM(litellm/llms/bedrock/base_aws_llm.py) into the Rust workspace as a base provider atlitellm-core/src/providers/bedrock/, so the Rust Bedrock path can resolve AWS credentials and SigV4-sign a request natively instead of shelling back into Python. It is gated behind a new, default-offbedrock-authcargo feature onlitellm-core; core still builds and clippies clean with the feature off, and the AWS dependencies are optional and only pulled in when it is enabled.Placement note for reviewers: credential resolution reads env and calls STS, and signing is SDK work, which the core-purity guidance in
litellm-rust/CLAUDE.mdnormally keeps out oflitellm-core. Putting the base provider here (mirroring how Python's base provider class owns auth) is a deliberate, directed decision; the module carries a short note to that effect and the purity guidance will be reconciled separately rather than silently.Credential-flow selection is a pure function over a typed
AwsAuthConfig, with the same per-parameterAWS_*env fallback as Python, so branch order is unit-testable without the networkclassify_authreturns that tagged union;resolve_credentialsexecutes the chosen flow through the AWS SDK providers, andsign_bedrock_postsigns withaws-sigv4and normalizes the emitted header casing to match botocore (Authorization,X-Amz-Date,X-Amz-Security-Token).The port now includes the two behaviors mirrored from Python that the earlier draft had deferred. First, the process-wide IAM credential cache with matching TTLs: static access-key credentials cached for 59 minutes, the ambient default chain for 10 minutes, and the flows Python does not cache (AssumeRole, web identity, profile, explicit session-token) left uncached. Cache keys are SHA-256 of the resolved config plus flow, so no credential material appears in a key. Second, the AssumeRole "already running as this role" short-circuit: it compares the target role ARN's partition, account, and role name against the caller (via the IRSA env fast path, otherwise
sts:GetCallerIdentity) and uses ambient credentials when they already match, falling through to a normal AssumeRole when the caller-identity lookup fails.AWS SDK deps are pinned and configured rustls-only (
default-features = false,rustls,rt-tokio); no OpenSSL enters the tree.One SDK parity note:
AssumeRoleWithWebIdentityhas noExternalIdinput in the AWS API (the Python path does not pass one there either), soaws_external_idapplies only to the plain AssumeRole flow.Final Attestation
Link to Devin session: https://app.devin.ai/sessions/bdf99b89a6584cc0989b5605765aec03
Requested by: @ishaan-berri