Skip to content

contain gemini batch-job results file within results_folder - #71832

Open
Samin061 wants to merge 2 commits into
apache:mainfrom
Samin061:gemini-batch-results-path
Open

contain gemini batch-job results file within results_folder#71832
Samin061 wants to merge 2 commits into
apache:mainfrom
Samin061:gemini-batch-results-path

Conversation

@Samin061

Copy link
Copy Markdown
Contributor

Path traversal via unsanitized Gemini batch-job display_name

GenAIGeminiCreateBatchJobOperator and GenAIGeminiCreateEmbeddingsBatchJobOperator name the local results file after job.display_name, a field returned in the batch-job metadata. job.name is already sanitized against / at that spot, but display_name (used first) is not, so a .. in it lands the .jsonl outside results_folder on the worker. The check now resolves the join and refuses anything escaping results_folder, applied at both sites, matching the sync_to_local_dir guards already in the GCS and S3 hooks.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

@Samin061
Samin061 requested a review from shahar1 as a code owner August 19, 2026 10:34
@boring-cyborg boring-cyborg Bot added area:providers provider:google Google (including GCP) related issues labels Aug 19, 2026
@fat-catTW

Copy link
Copy Markdown
Contributor

Thanks for the fix!

Could we use Path.resolve() for the containment check here, matching the existing GCS/S3 sync_to_local_dir guards? abspath() catches .. traversal, but it does not resolve symlinks.
For example, if results_folder/link is a symlink to a directory outside results_folder, a display_name like link/evil would still pass the commonpath() check while writing outside the intended folder.

Using Path(results_folder).resolve() and checking the resolved target path is relative to that resolved base would make the containment guarantee stronger and align this with the existing guards.

@Samin061

Copy link
Copy Markdown
Contributor Author

Good point on the symlink gap. Switched the check to Path(results_folder).resolve() and resolved_target.is_relative_to(base), so a symlinked subdir pointing outside now gets rejected too. That matches the GCS/S3 sync_to_local_dir guards. Tests still pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:google Google (including GCP) related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants