Skip to content

Commit

Permalink
feat(job_attachment): reject files on non-Windows systems that do not…
Browse files Browse the repository at this point in the history
… support O_NOFOLLOW (#242)

Signed-off-by: Gahyun Suh <[email protected]>
  • Loading branch information
gahyusuh authored Mar 25, 2024
1 parent 9de687e commit 9e23b81
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
14 changes: 12 additions & 2 deletions src/deadline/job_attachments/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,10 +450,20 @@ def _open_non_symlink_file_binary(
# Make sure the file isn’t following a symlink to a different path.
if hasattr(os, "O_NOFOLLOW"):
open_flags |= os.O_NOFOLLOW
elif sys.platform != "win32" and not os.path.islink(path):
# We are on a non-Windows system that does not support O_NOFOLLOW. When we encounter
# symbolic link, we cannot guarantee security here, so log a warning and reject the file.
logger.warning(
f"Job Attachments does not support files referenced by symbolic links on this system ({sys.platform}). "
"Please refrain from using symbolic links in Job Attachment asset roots and use real files instead. "
f"The following file will be skipped: {path}."
)
yield None

fd = os.open(path, open_flags)
if sys.platform == "win32":
# On Windows, check the file handle with GetFinalPathNameByHandle
# Windows does not support O_NOFOLLOW. So, check the file handle with GetFinalPathNameByHandle
# to verify it is actually pointing to the path that we verified to be safe to open.
if not self._is_path_win32_final_path_of_file_descriptor(path, fd):
# ELOOP is the error code that open with NOFOLLOW will return
# if the path is a symlink. We raise the same error here for
Expand All @@ -466,7 +476,7 @@ def _open_non_symlink_file_binary(
with os.fdopen(fd, "rb", closefd=False) as file_obj:
yield file_obj
except OSError as e:
logger.warning(f"Failed to open file {path}: {e}")
logger.warning(f"Failed to open file. The following file will be skipped: {path}: {e}")
yield None
finally:
if fd is not None:
Expand Down
5 changes: 4 additions & 1 deletion test/unit/deadline_job_attachments/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -2420,7 +2420,10 @@ def test_open_non_symlink_file_binary_posix_fail(self, tmp_path: Path, caplog):
with a3_asset_uploader._open_non_symlink_file_binary(str(symlink_path)) as file_obj:
# THEN
assert file_obj is None
assert f"Failed to open file {symlink_path}" in caplog.text
assert (
f"Failed to open file. The following file will be skipped: {symlink_path}"
in caplog.text
)
if hasattr(os, "O_NOFOLLOW") is False:
# Windows or other platforms that don't support O_NOFOLLOW
assert "Mismatch between path and its final path" in caplog.text
Expand Down

0 comments on commit 9e23b81

Please sign in to comment.