mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
Merge pull request #42312 from BerriAI/litellm_bedrock_batch_s3_creds_leak
Register s3_access_key_id, s3_secret_access_key and s3_encryption_key_id as LiteLLM-owned batch params so they are no longer forwarded to Bedrock as additionalModelRequestFields (which 400s ordinary chat on a batch-configured deployment), keep them on CredentialLiteLLMParams so the batch/file paths still receive them, and redact the S3 credential key names in debug logs. Resolves LIT-8290 Co-authored-by: yucheng <yucheng@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
commit
e0e5b7924a
10 changed files with 107 additions and 1 deletions
|
|
@ -65,7 +65,7 @@ def _build_secret_patterns() -> "re.Pattern[str]":
|
|||
# private_key with PEM-aware value capture
|
||||
r"""private_key['\"]?\s*[:=]\s*['\"]?(?:-----BEGIN[A-Z \-]*PRIVATE KEY-----[\s\S]*?-----END[A-Z \-]*PRIVATE KEY-----|[^\s,'\"})\]{}>]+)""",
|
||||
r"(?:master_key|xai_key|database_url|db_url|connection_string|"
|
||||
r"aws_secret_access_key|aws_session_token|aws_access_key_id|"
|
||||
r"aws_secret_access_key|aws_session_token|aws_access_key_id|s3_secret_access_key|s3_access_key_id|"
|
||||
r"signing_key|encryption_key|"
|
||||
r"auth_token|access_token|refresh_token|"
|
||||
r"slack_webhook_url|webhook_url|"
|
||||
|
|
|
|||
|
|
@ -305,6 +305,8 @@ class CredentialLiteLLMParams(BaseModel):
|
|||
s3_bucket_name: str | None = None
|
||||
s3_endpoint_url: str | None = None
|
||||
s3_region_name: str | None = None
|
||||
s3_access_key_id: str | None = None
|
||||
s3_secret_access_key: str | None = None
|
||||
s3_encryption_key_id: str | None = None
|
||||
s3_bucket_owner: str | None = None
|
||||
aws_batch_role_arn: str | None = None
|
||||
|
|
|
|||
|
|
@ -3840,6 +3840,9 @@ bedrock_batch_litellm_params: Final = (
|
|||
"s3_endpoint_url",
|
||||
"s3_output_bucket_name",
|
||||
"s3_bucket_owner",
|
||||
"s3_access_key_id",
|
||||
"s3_secret_access_key",
|
||||
"s3_encryption_key_id",
|
||||
"bedrock_tags",
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@
|
|||
- {id: llm.chat_completions.bedrock_converse.thinking.nonstream.works, module: llm, tier: P1, subject_endpoint: chat_completions, route: bedrock_converse, capability: thinking, streaming: nonstream, assertions: [works], source: "model_prices json", rationale: "Anthropic thinking on Bedrock"}
|
||||
- {id: llm.chat_completions.bedrock_converse.response_headers.nonstream.works, module: llm, tier: P0, subject_endpoint: chat_completions, route: bedrock_converse, capability: response_headers, streaming: nonstream, assertions: [works], source: "llms/bedrock/chat/converse_handler.py:248", rationale: "Bedrock request ids must surface as llm_provider-* response headers on /chat/completions so callers can correlate calls with AWS-side logs (#37003)", fail_before_fix: proven}
|
||||
- {id: llm.chat_completions.bedrock_converse.response_headers.stream.works, module: llm, tier: P0, subject_endpoint: chat_completions, route: bedrock_converse, capability: response_headers, streaming: stream, assertions: [works], source: "llms/bedrock/chat/converse_handler.py:154", rationale: "The llm_provider-* headers must also surface on streaming /chat/completions, where CustomStreamWrapper carries them instead of the nonstream setter"}
|
||||
- {id: llm.chat_completions.bedrock_converse.batch_deployment.nonstream.works, module: llm, tier: P0, subject_endpoint: chat_completions, route: bedrock_converse, capability: batch_deployment, streaming: nonstream, assertions: [works], source: "types/utils.py bedrock_batch_litellm_params", rationale: "A deployment carrying the documented batch-only S3 keys (s3_access_key_id, s3_secret_access_key, s3_encryption_key_id) must still serve ordinary chat; unregistered keys fall into optional_params and are forwarded as additionalModelRequestFields, which Bedrock 400s and which puts the S3 secret in the request body and debug log (LIT-8290)", fail_before_fix: proven}
|
||||
- {id: llm.chat_completions.bedrock_invoke.basic.nonstream.works, module: llm, tier: P0, subject_endpoint: chat_completions, route: bedrock_invoke, capability: basic, streaming: nonstream, assertions: [works], source: "proxy_server.py:8455", rationale: "Regional inference-profile ids (us.anthropic.*) over the invoke route, the deployment shape behind a customer timeout report on v1.90.0"}
|
||||
- {id: llm.chat_completions.bedrock_invoke.basic.stream.works, module: llm, tier: P0, subject_endpoint: chat_completions, route: bedrock_invoke, capability: basic, streaming: stream, assertions: [works], source: "proxy_server.py:8455", rationale: "Streaming with regional inference-profile ids over the invoke route"}
|
||||
- {id: llm.chat_completions.vertex.basic.nonstream.works, module: llm, tier: P0, subject_endpoint: chat_completions, route: vertex, capability: basic, streaming: nonstream, assertions: [works], source: "proxy_server.py:8455", rationale: "P0 route; Vertex AI"}
|
||||
|
|
|
|||
|
|
@ -63,6 +63,7 @@ LlmRoute = Literal[
|
|||
LlmCapability = Literal[
|
||||
"assume_role",
|
||||
"basic",
|
||||
"batch_deployment",
|
||||
"count_tokens",
|
||||
"govcloud_partition",
|
||||
"input_validation",
|
||||
|
|
|
|||
|
|
@ -131,6 +131,50 @@ class TestBedrockResponseHeaders:
|
|||
_assert_request_id_header(result)
|
||||
|
||||
|
||||
def _register_bedrock_batch_deployment(client: PassthroughClient, resources: ResourceManager) -> str:
|
||||
model = f"e2e-bedrock-batch-chat-{unique_marker()}"
|
||||
model_id = client.proxy.create_model(
|
||||
model,
|
||||
LiteLLMParamsBody(
|
||||
model=CONVERSE_REGIONAL_BACKEND,
|
||||
aws_access_key_id="os.environ/AWS_ACCESS_KEY_ID",
|
||||
aws_secret_access_key="os.environ/AWS_SECRET_ACCESS_KEY",
|
||||
aws_region_name="os.environ/AWS_REGION",
|
||||
s3_bucket_name="os.environ/AWS_BATCH_S3_BUCKET",
|
||||
s3_access_key_id="os.environ/AWS_ACCESS_KEY_ID",
|
||||
s3_secret_access_key="os.environ/AWS_SECRET_ACCESS_KEY",
|
||||
s3_encryption_key_id=f"alias/e2e-unused-{unique_marker()}",
|
||||
aws_batch_role_arn="os.environ/AWS_BATCH_ROLE_ARN",
|
||||
),
|
||||
)
|
||||
resources.defer(lambda: client.proxy.delete_model(model_id))
|
||||
return model
|
||||
|
||||
|
||||
class TestBedrockBatchDeploymentServesChat:
|
||||
@pytest.mark.covers(
|
||||
"llm.chat_completions.bedrock_converse.batch_deployment.nonstream.works",
|
||||
exercised_on=[],
|
||||
)
|
||||
def test_batch_s3_keys_do_not_break_chat(
|
||||
self, client: PassthroughClient, resources: ResourceManager
|
||||
) -> None:
|
||||
model = _register_bedrock_batch_deployment(client, resources)
|
||||
key = resources.key()
|
||||
|
||||
result = client.proxy.transport.send(
|
||||
"/chat/completions",
|
||||
headers=client.proxy.transport.bearer(key),
|
||||
json=ChatBody(model=model, messages=_prompt(), max_tokens=64),
|
||||
)
|
||||
|
||||
assert result.ok, (
|
||||
f"chat on a batch-configured deployment failed: {result.status_code} {result.body[:300]}; "
|
||||
"batch-only S3 keys were forwarded to Bedrock as additionalModelRequestFields"
|
||||
)
|
||||
_assert_completion(ChatResponse.model_validate_json(result.body))
|
||||
|
||||
|
||||
class TestBedrockInvokeRegionalModelIds:
|
||||
@pytest.mark.covers("llm.chat_completions.bedrock_invoke.basic.nonstream.works", exercised_on=[])
|
||||
def test_invoke_regional_id_completes(
|
||||
|
|
|
|||
|
|
@ -1011,6 +1011,7 @@ class LiteLLMParamsBody(BaseModel):
|
|||
s3_region_name: str | None = None
|
||||
s3_access_key_id: str | None = None
|
||||
s3_secret_access_key: str | None = None
|
||||
s3_encryption_key_id: str | None = None
|
||||
aws_batch_role_arn: str | None = None
|
||||
aws_role_name: str | None = None
|
||||
aws_session_name: str | None = None
|
||||
|
|
|
|||
|
|
@ -629,6 +629,25 @@ def test_aws_credential_redaction_catches_quoted_values():
|
|||
assert redact_string(safe) == safe
|
||||
|
||||
|
||||
def test_bedrock_batch_s3_credential_redaction_in_deployment_dump():
|
||||
"""The router logs each deployment's litellm_params at DEBUG. A Bedrock batch
|
||||
deployment carries s3_secret_access_key there, which the aws_* key-name rule
|
||||
did not cover, so the S3 secret was printed verbatim (LIT-8290)."""
|
||||
cases = (
|
||||
"{'s3_secret_access_key': 'wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY'}",
|
||||
"s3_secret_access_key=wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY",
|
||||
"{'s3_access_key_id': 'not-an-akia-shaped-value'}",
|
||||
)
|
||||
for secret_line in cases:
|
||||
result = redact_string(secret_line)
|
||||
assert "REDACTED" in result, f"S3 credential redaction missed: {secret_line!r}"
|
||||
assert "wJalrXUtnFEMIK7MDENGbPxRfiCYEXAMPLEKEY" not in result
|
||||
assert "not-an-akia-shaped-value" not in result
|
||||
|
||||
safe = "'s3_bucket_name': 'my-batch-bucket'"
|
||||
assert redact_string(safe) == safe
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"extra",
|
||||
(
|
||||
|
|
|
|||
|
|
@ -4682,6 +4682,33 @@ def test_bedrock_batch_params_never_reach_the_provider():
|
|||
)
|
||||
|
||||
|
||||
def test_documented_batch_s3_credentials_never_reach_the_provider():
|
||||
"""The Bedrock batch docs tell users to put s3_access_key_id, s3_secret_access_key
|
||||
and s3_encryption_key_id on the deployment. Left unregistered they are swept into
|
||||
additionalModelRequestFields, Bedrock 400s ordinary chat on that deployment with
|
||||
`s3_secret_access_key: Extra inputs are not permitted`, and the S3 secret is sent
|
||||
to the provider and printed in the debug log (LIT-8290).
|
||||
"""
|
||||
configured = {
|
||||
"s3_access_key_id": "configured-access-key-id",
|
||||
"s3_secret_access_key": "configured-secret-access-key",
|
||||
"s3_encryption_key_id": "arn:aws:kms:us-east-1:000000000000:key/configured",
|
||||
}
|
||||
kwargs = {"a_real_provider_specific_param": 1, **configured}
|
||||
|
||||
non_default = get_non_default_completion_params(dict(kwargs))
|
||||
|
||||
assert non_default == {"a_real_provider_specific_param": 1}, (
|
||||
"documented batch S3 credentials leaked into the provider params: "
|
||||
f"{sorted(set(non_default) - {'a_real_provider_specific_param'})}"
|
||||
)
|
||||
|
||||
batch_params = dict(GenericLiteLLMParams(**kwargs))
|
||||
assert {field: batch_params.get(field) for field in configured} == configured, (
|
||||
"registering these must not strip them from the batch path"
|
||||
)
|
||||
|
||||
|
||||
def test_client_side_timeout_marker_never_reaches_the_provider():
|
||||
"""The proxy stamps kwargs["client_side_timeout"] = True whenever a request carries
|
||||
a caller-supplied timeout (body timeout / request_timeout / stream_timeout or the
|
||||
|
|
|
|||
8
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
8
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
|
|
@ -31245,6 +31245,8 @@ export interface components {
|
|||
regional_processing_uplift_multiplier_us?: number | null;
|
||||
/** Rpm */
|
||||
rpm?: number | null;
|
||||
/** S3 Access Key Id */
|
||||
s3_access_key_id?: string | null;
|
||||
/** S3 Bucket Name */
|
||||
s3_bucket_name?: string | null;
|
||||
/** S3 Bucket Owner */
|
||||
|
|
@ -31257,6 +31259,8 @@ export interface components {
|
|||
s3_output_bucket_name?: string | null;
|
||||
/** S3 Region Name */
|
||||
s3_region_name?: string | null;
|
||||
/** S3 Secret Access Key */
|
||||
s3_secret_access_key?: string | null;
|
||||
/** Search Context Cost Per Query */
|
||||
search_context_cost_per_query?: {
|
||||
[key: string]: unknown;
|
||||
|
|
@ -42045,6 +42049,8 @@ export interface components {
|
|||
regional_processing_uplift_multiplier_us?: number | null;
|
||||
/** Rpm */
|
||||
rpm?: number | null;
|
||||
/** S3 Access Key Id */
|
||||
s3_access_key_id?: string | null;
|
||||
/** S3 Bucket Name */
|
||||
s3_bucket_name?: string | null;
|
||||
/** S3 Bucket Owner */
|
||||
|
|
@ -42057,6 +42063,8 @@ export interface components {
|
|||
s3_output_bucket_name?: string | null;
|
||||
/** S3 Region Name */
|
||||
s3_region_name?: string | null;
|
||||
/** S3 Secret Access Key */
|
||||
s3_secret_access_key?: string | null;
|
||||
/** Search Context Cost Per Query */
|
||||
search_context_cost_per_query?: {
|
||||
[key: string]: unknown;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue