cherry-pick: reject bare str at file-input sinks (#27762) onto litellm_1.84.0rc2#27794
Conversation
…#27762) Cherry-pick of #27762 onto litellm_1.84.0rc2. * chore: reject bare str at file-input sinks to prevent local-file read (#27667) * fix: use os.PathLike in ocr sink and check truthy reasoningSummary for bridge - ocr/main.py: widen Path check to os.PathLike for consistency with other sinks - main.py: bridge condition checks truthiness of reasoning_summary, not just None * fix: remove unused pathlib.Path import in ocr/main.py Co-authored-by: yuneng-jiang <yuneng@berri.ai> Co-authored-by: ryan-crabbe-berri <ryan@berri.ai> Co-authored-by: stuxf <70670632+stuxf@users.noreply.github.com>
|
|
Greptile SummaryThis cherry-pick closes an arbitrary local file read vulnerability at four file-input sinks (
Confidence Score: 4/5Safe to merge — the LFI fix is correct and well-tested across all four sinks. The one inconsistency (silent swallow in calculate_request_duration) does not reintroduce the vulnerability. The calculate_request_duration function wraps its entire body in try/except Exception: return None, so the new raise ValueError for bare str is caught and swallowed — callers get None instead of the documented migration error. The security objective is still met (no file is opened), but the function's stated contract and its comment do not match the actual behavior for this input type. litellm/litellm_core_utils/audio_utils/utils.py — the calculate_request_duration bare-str guard is silently swallowed by the outer except block.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/audio_utils/utils.py | Rejects bare str at process_audio_file and calculate_request_duration; PathLike still accepted. ValueError in calculate_request_duration is inside a broad try/except and will be silently swallowed. |
| litellm/litellm_core_utils/prompt_templates/common_utils.py | Cleanly splits str/PathLike branches in extract_file_data; ValueError propagates correctly to callers. |
| litellm/ocr/main.py | OCR convert_file_document_to_url_document now rejects bare str with a clear error; PathLike branch unchanged and correct. |
| tests/test_litellm/litellm_core_utils/prompt_templates/test_litellm_core_utils_prompt_templates_common_utils.py | Adds TestExtractFileDataBareStr class with rejection and acceptance cases; no real network calls; coverage is additive. |
| tests/test_litellm/litellm_core_utils/test_audio_utils.py | Existing path tests updated to use pathlib.Path; new test_process_bare_str_path_rejected added; coverage maintained. |
| tests/test_litellm/llms/vertex_ai/gemini/test_vertex_ai_gemini_transformation.py | String-path tests updated to pathlib.Path equivalents; assertions unchanged — no regression in coverage. |
| tests/test_litellm/ocr/test_ocr_file_input.py | OCR tests updated to use pathlib.Path; new test_should_reject_bare_str_path added to confirm the new guard. |
Reviews (1): Last reviewed commit: "chore: reject bare str at file-input sin..." | Re-trigger Greptile
| elif isinstance(file, str): | ||
| # Bare strings are rejected — see extract_file_data. | ||
| raise ValueError( | ||
| "calculate_request_duration does not accept bare str inputs. " | ||
| "Pass bytes, an open file handle, a (filename, content) " | ||
| "tuple, or a pathlib.Path." | ||
| ) |
There was a problem hiding this comment.
ValueError silently swallowed by outer
except Exception
The raise ValueError on bare str input is inside the try: block that spans lines 247–303 and ends with except Exception: return None. So calculate_request_duration("/etc/passwd") returns None instead of surfacing the migration message — the stated intent ("Bare strings are rejected") is not met from a caller's perspective. The security goal is still achieved (no file is opened), but developers passing a wrong type will silently get None with no guidance.
Either re-raise ValueError specifically (except Exception as e: if isinstance(e, ValueError): raise; return None) or move the guard check before the outer try block so it always propagates.
Relevant issues
Cherry-picks #27762 onto
litellm_1.84.0rc2so the RC matches staging.Pre-Submission checklist
Type
🐛 Bug Fix
Changes
Summary
Bring #27762 onto
litellm_1.84.0rc2. The patch rejects barestrat file-input sinks (extract_file_data,audio_utils.process_audio_file,audio_utils.calculate_request_duration, OCR_extract_file_metadata) so that attacker-controlled values arriving through the proxy can't be opened as local file paths.pathlib.Pathis still accepted for SDK ergonomics (HTTP form values can't fabricate aPath); barestrraisesValueErrorwith a migration message.Cherry-pick details
0deffd3618636fb92981c316c6186e9e48aa83ff(squash-merge of chore: reject bare str at file-input sinks to prevent local-file read #27762 onlitellm_internal_staging)tests/test_litellm/litellm_core_utils/prompt_templates/test_litellm_core_utils_prompt_templates_common_utils.py: the RC branch is missing several unrelatedupdate_messages_with_model_file_ids_*tests that landed on staging between RC cut and chore: reject bare str at file-input sinks to prevent local-file read #27762. Resolved by appending only theTestExtractFileDataBareStrclass (the actual contribution of chore: reject bare str at file-input sinks to prevent local-file read #27762) at the end of the existing file. Net diff per file matches the source commit exactly (185 insertions, 65 deletions, 7 files).Local verification
A preexisting failure in
test_audio_utils.py::TestCalculateRequestDuration::test_bytesio_at_end_positionreproduces on unmodifiedorigin/litellm_1.84.0rc2and is not introduced by this cherry-pick.