fix(files): constrain cloud storage file paths (VERIA-45, VERIA-59)

[stuxf]

Summary

Tightens the cloud-storage-backed file and logging paths for Vertex AI, Bedrock, and GCS logging.

• Adds shared cloud object validation/sanitization for managed GCS/S3 paths.
• Vertex AI and Bedrock file uploads now use server-generated managed object names, encode storage URL components, and keep bucket selection on the server side.
• File retrieve/content/delete paths now validate that cloud file IDs resolve inside the configured bucket/prefix and a LiteLLM-managed object prefix before server credentials are used.
• GCS logging now URL-encodes object names, treats gcs_log_id as a sanitized hint instead of an exact path, and does not derive GCS bucket/service-account callback config from request kwargs or metadata.

Behavior changes

• Normal upload -> returned file ID -> retrieve/content/delete flows remain supported.
• Direct cloud object IDs outside the configured bucket and LiteLLM-managed prefixes are rejected.
• Client requests can no longer choose GCS logging storage destinations through callback params.
• Pre-existing raw filename object IDs outside LiteLLM-managed prefixes should be re-uploaded through the normal file flow.

Test plan

• uv run pytest tests/test_litellm/llms/vertex_ai/files tests/test_litellm/llms/bedrock/files tests/test_litellm/integrations/gcs_bucket tests/test_litellm/litellm_core_utils/test_initialize_dynamic_callback_params.py -q — 82 pass
• uv run ruff check litellm/litellm_core_utils/cloud_storage_security.py litellm/llms/vertex_ai/files litellm/llms/bedrock/files litellm/integrations/gcs_bucket litellm/litellm_core_utils/initialize_dynamic_callback_params.py tests/test_litellm/llms/vertex_ai/files tests/test_litellm/llms/bedrock/files tests/test_litellm/integrations/gcs_bucket tests/test_litellm/litellm_core_utils/test_initialize_dynamic_callback_params.py
• uv run black --check on touched files

Type

🐛 Bug Fix
✅ Test

[codecov]

Codecov Report

❌ Patch coverage is 79.39698% with 41 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...tellm/litellm_core_utils/cloud_storage_security.py	79.04%	22 Missing ⚠️
litellm/llms/bedrock/files/transformation.py	60.00%	6 Missing ⚠️
litellm/llms/vertex_ai/files/transformation.py	75.00%	6 Missing ⚠️
litellm/integrations/gcs_bucket/gcs_bucket_base.py	66.66%	2 Missing ⚠️
...m_core_utils/initialize_dynamic_callback_params.py	60.00%	2 Missing ⚠️
litellm/llms/bedrock/files/handler.py	90.00%	2 Missing ⚠️
litellm/integrations/gcs_bucket/gcs_bucket.py	80.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[greptile-apps]

Greptile Summary

This PR hardens cloud storage paths for Vertex AI, Bedrock, and GCS logging by introducing a shared cloud_storage_security module with bucket/prefix validation, managed-prefix enforcement, and centralized URL encoding. Trusted server credentials are now propagated via an immutable MappingProxyType snapshot (_litellm_internal_model_credentials), and client-supplied GCS bucket/service-account callback params are fully blocked.

Confidence Score: 5/5

PR is safe to merge; only P2 findings remain after previous review threads addressed the critical issues.

No new P0/P1 issues found. The two remaining comments are P2 operability suggestions (global env var escape hatch for allow_legacy_cloud_file_ids and the corresponding missing import os). All previous P0/P1 concerns raised in earlier threads are addressed by the new MappingProxyType credential snapshot, the litellm_params threading fix in the Vertex handler, and the _get_configured_s3_bucket_name helper reading from trusted credentials first.

litellm/litellm_core_utils/cloud_storage_security.py — operability gap for global legacy-ID opt-in

Important Files Changed

Filename	Overview
litellm/litellm_core_utils/cloud_storage_security.py	New shared security module: bucket/object validation, trusted-credential helpers, and URL-encoding utilities. Well-structured with MappingProxyType enforcement and idempotent encoding via unquote→quote pattern.
litellm/llms/bedrock/files/handler.py	Adds bucket-validation and managed-prefix enforcement for Bedrock S3 file retrieval. _get_configured_s3_bucket_name correctly reads from the MappingProxyType snapshot first; called with litellm_params_dict (passed as optional_params) from main.py.
litellm/llms/vertex_ai/files/handler.py	Threads litellm_params through afile_content and _extract_bucket_and_object_from_file_id, fixing the legacy-flag silent-ignore noted in previous threads. Service-account config now pinned from gcs_logging_config.
litellm/llms/vertex_ai/files/transformation.py	Enforces managed-prefix validation in _parse_gcs_uri and uses server-generated object names for uploads. Removes the litellm_metadata.gcs_bucket_name override path. Bucket sourced via _get_configured_bucket_name helper only.
litellm/llms/bedrock/files/transformation.py	Uses build_managed_cloud_object_name for upload paths, URL-encodes S3 object keys, and decodes keys extracted from HTTPS response URLs before constructing s3:// URIs. Correct use of safe="/" for S3 path-style URLs.
litellm/integrations/gcs_bucket/gcs_bucket_base.py	Centralizes URL encoding of GCS object names in download/delete/upload methods via encode_gcs_object_name_for_url. Delegates bucket/prefix splitting to split_configured_cloud_bucket_name.
litellm/files/main.py	Adds _add_trusted_model_credentials_to_litellm_params helper and calls it in retrieve, delete, and content paths so that _litellm_internal_model_credentials flows through to provider validation.
litellm/proxy/openai_files_endpoints/common_utils.py	Adds include_internal_credentials flag to prepare_data_with_credentials; creates an immutable MappingProxyType snapshot of server credentials for security-sensitive code paths.

_{Reviews (6): Last reviewed commit: "trust only server legacy file flag" | Re-trigger Greptile}

[stuxf]

@greptileai

[stuxf]

@greptileai

[stuxf]

Pushing back on both P1 findings — both safeguards are already in the PR:

P1 #1 (asymmetric bucket lookup): The asymmetry between handler and upload path is intentional defense-in-depth. The handler explicitly refuses to trust litellm_params.s3_bucket_name because that key can flow in from a request body. The existing test test_should_not_trust_request_s3_bucket_name_for_expected_bucket (test_bedrock_files_handler.py:126) asserts exactly this — it pins env-var resolution as winning over a request-supplied litellm_params.s3_bucket_name. The proxy YAML path goes through prepare_data_with_credentials(..., include_internal_credentials=True) at files_endpoints.py:777, which populates _litellm_internal_model_credentials — the trusted form the handler reads from. So proxy-config users are not broken on retrieval; the handler reads from the trusted dict.

P1 #2 (no feature flag for managed-prefix enforcement): The PR ships an opt-in flag for exactly this — should_allow_legacy_cloud_file_ids in cloud_storage_security.py:113, threaded through both vertex_ai/files/handler.py:138 and bedrock/files/handler.py:144. Operators with pre-existing non-managed file IDs can flip allow_legacy_cloud_file_ids to keep old retrievals working while the security model stays default-deny.

@greptileai

[greptile-apps]

Both pushbacks check out against the code:

P1 #1 — acknowledged. _get_configured_s3_bucket_name in bedrock/files/handler.py correctly reads only from _litellm_internal_model_credentials (a MappingProxyType) or the env var — litellm_params.s3_bucket_name is never trusted. The asymmetry is intentional and the code supports it.

P1 #2 — acknowledged. should_allow_legacy_cloud_file_ids exists at cloud_storage_security.py:113 and is wired through the Bedrock handler via should_allow_legacy_cloud_file_ids(optional_params). Fair point.

One small inconsistency worth noting: the Vertex AI handler calls should_allow_legacy_cloud_file_ids() without arguments in _extract_bucket_and_object_from_file_id, meaning it can only read from the LITELLM_ALLOW_LEGACY_CLOUD_FILE_IDS env var — the per-request allow_legacy_cloud_file_ids in litellm_params is silently ignored for Vertex. Bedrock correctly passes optional_params. Whether that asymmetry is intentional (env-var-only gate for Vertex legacy IDs) or an oversight is worth confirming, but it's not a security regression.

Withdrawing both P1 flags.

[greptile-apps]

Backwards-incompatible gcs_log_id behavior change without a feature flag

Previously gcs_log_id in metadata was used as the exact GCS object path. This PR changes it to a sanitized hint incorporated into a new randomized path ({date}/custom-{uuid}-{safe_hint}). Any deployment that reads GCS log objects by a predictable gcs_log_id-derived path (e.g., external tooling, dashboards, or audit pipelines that construct GCS paths from known gcs_log_id values) will silently stop finding those objects after this change.

Per the project's backwards-compatibility rule, breaking behavior changes should be gated behind a server-side flag (e.g. litellm.enforce_safe_gcs_log_paths or an env var), so existing deployments can opt in on their own schedule. The analogous file retrieval path has allow_legacy_cloud_file_ids for exactly this reason.

Rule Used: What: avoid backwards-incompatible changes without... (source)

[stuxf]

@greptileai

[stuxf]

Update on the open findings:

• Vertex allow_legacy_cloud_file_ids not forwarded in the download path: addressed in e7c23fc041 — the call was removed entirely (Vertex now always validates against the managed prefix; the legacy escape hatch only applies to upload-side parsing where it's meaningful). Bedrock retains the per-config flag.

• gcs_bucket_name / gcs_path_service_account remain in _supported_callback_params: intentional. Removing them would break callers that legitimately set them via callback registration. The new _request_blocked_callback_params guard is the right layer to deny request-context overrides; the supported set defines what config may set.

• Backwards-incompat managed-prefix enforcement / gcs_log_id naming: these are the security fix. The PR ships an opt-in escape hatch (allow_legacy_cloud_file_ids for file IDs); the gcs_log_id sanitize-then-uuid path is the prevention for path-injection in log destinations. Adding a flag that re-enables raw gcs_log_id values would re-open the vector. External tooling that builds GCS paths from gcs_log_id values needs to be updated — same shape as the upload path's managed-prefix migration.

All findings here are about the strictness of the deny path, not the deny path being incorrect. Holding the line; the maintainer can decide if a feature flag is warranted for the gcs_log_id case.

@greptileai

[stuxf]

@greptileai

[greptile-apps]

The resolved findings are accepted — the reasoning for all three stands up:

• Vertex legacy flag in download path: removing the handler path entirely rather than threading litellm_params through is cleaner; the managed-prefix check is now the sole gate, which is correct.
• _supported_callback_params overlap: the two-list architecture (_supported_callback_params for config-level registration, _request_blocked_callback_params for the deny guard) is sound. A comment explaining the distinction would help future readers, but the logic is correct.

---

On the one remaining open thread — gcs_log_id backwards-incompatible rename:

The security argument is valid for raw/unsanitized values. But there's a narrower framing that avoids re-opening the injection vector: the current code calls sanitize_cloud_object_component on the value before using it. If that sanitized value equals the original (i.e., the hint was already clean), using {date}/{safe_log_id} directly is no more dangerous than the current path — the traversal characters are already stripped by sanitize_cloud_object_component regardless.

A flag like LITELLM_ALLOW_PREDICTABLE_GCS_LOG_IDS that switches between:

• {date}/custom-{uuid}-{safe_log_id} (default, current behavior)
• {date}/{safe_log_id} (opt-in, still sanitized)

...would restore predictable naming for deployments using safe log IDs without re-enabling path injection, because safe_log_id is the already-sanitized value in both branches.

That said — the developer has explicitly flagged this for the maintainer to decide, and that's the right call. If the maintainer is comfortable with the migration cost for external tooling, the strict path is defensible. If a flag is added, the above scoping (re-enable predictable naming over sanitized value, not raw value) is the safe boundary.

Maintaining the finding as unresolved pending maintainer decision.

[stuxf]

@greptileai