Skip to content

Commit 2f5c8f8

Browse files
jscudcopybara-github
authored andcommitted
fix: added safety checks in download_from_gcs
PiperOrigin-RevId: 934043631
1 parent 97cc5bd commit 2f5c8f8

2 files changed

Lines changed: 59 additions & 7 deletions

File tree

google/cloud/aiplatform/utils/gcs_utils.py

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,43 @@ def download_file_from_gcs(
414414
source_blob.download_to_filename(filename=destination_file_path)
415415

416416

417+
def _is_path_subdirectory(destination_path: str, blob_path: str) -> bool:
418+
"""Checks that the path provided by the blob would produce a subdirectory of the destination path.
419+
420+
Args:
421+
destination_path (str):
422+
Required. The destination directory's path to use as a foundation.
423+
blob_path (str):
424+
Required. The file sub path provided by the blob.
425+
426+
Returns:
427+
bool: True if the blob path is a subdirectory of the destination path, False otherwise.
428+
"""
429+
blob_path_obj = pathlib.Path(blob_path)
430+
destination_path_obj = pathlib.Path(destination_path)
431+
if any((part == ".." or ":" in part) for part in blob_path_obj.parts):
432+
_logger.warning(
433+
"The specified prefix '%s' contains '..' or ':', "
434+
"which is not supported. Skipping blob '%s'.",
435+
destination_path,
436+
blob_path,
437+
)
438+
return False
439+
resolved_path = (destination_path_obj / blob_path_obj).resolve()
440+
resolved_destination_path = destination_path_obj.resolve()
441+
if os.path.commonpath([str(resolved_destination_path), str(resolved_path)]) != str(
442+
resolved_destination_path
443+
):
444+
_logger.warning(
445+
"The specified prefix '%s' is not a subdirectory "
446+
"of the destination path '%s'.",
447+
blob_path,
448+
destination_path,
449+
)
450+
return False
451+
return True
452+
453+
417454
def download_from_gcs(
418455
source_uri: str,
419456
destination_path: str,
@@ -452,13 +489,14 @@ def download_from_gcs(
452489
# These files ends with '/', and we'll skip them.
453490
if not blob.name.endswith("/"):
454491
rel_path = os.path.relpath(blob.name, prefix)
455-
filename = (
456-
destination_path
457-
if rel_path == "."
458-
else os.path.join(destination_path, rel_path)
459-
)
460-
os.makedirs(os.path.dirname(filename), exist_ok=True)
461-
blob.download_to_filename(filename=filename)
492+
if _is_path_subdirectory(destination_path, rel_path):
493+
filename = (
494+
destination_path
495+
if rel_path == "."
496+
else os.path.join(destination_path, rel_path)
497+
)
498+
os.makedirs(os.path.dirname(filename), exist_ok=True)
499+
blob.download_to_filename(filename=filename)
462500

463501

464502
def _upload_pandas_df_to_gcs(

tests/unit/aiplatform/test_utils.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,20 @@ def test_download_from_gcs_invalid_source_uri(self):
658658
):
659659
gcs_utils.download_from_gcs(source_uri, destination_path)
660660

661+
def test_is_path_subdirectory(self):
662+
assert gcs_utils._is_path_subdirectory(
663+
destination_path="test_dir", blob_path="test_dir/test_file"
664+
)
665+
assert gcs_utils._is_path_subdirectory(
666+
destination_path="test_dir", blob_path="test_file"
667+
)
668+
assert not gcs_utils._is_path_subdirectory(
669+
destination_path="test_dir", blob_path="test_dir/../test_file"
670+
)
671+
assert not gcs_utils._is_path_subdirectory(
672+
destination_path="test_dir", blob_path="test_dir/c:file"
673+
)
674+
661675
def test_validate_gcs_path(self):
662676
test_valid_path = "gs://test_valid_path"
663677
gcs_utils.validate_gcs_path(test_valid_path)

0 commit comments

Comments
 (0)