Skip to content

Commit 15372b1

Browse files
fallintoplacefallintoplace
committed
fix: harden local directory copy against symlink swaps
1 parent 656baf8 commit 15372b1

2 files changed

Lines changed: 417 additions & 61 deletions

File tree

‎src/agents/sandbox/entries/artifacts.py‎

Lines changed: 217 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,10 @@ def _list_local_dir_files_from_dir_fd(
397397
try:
398398
child_fd = os.open(entry.name, dir_flags, dir_fd=dir_fd)
399399
child_stat = os.fstat(child_fd)
400-
if not stat.S_ISDIR(child_stat.st_mode):
400+
if not stat.S_ISDIR(child_stat.st_mode) or not os.path.samestat(
401+
entry_stat,
402+
child_stat,
403+
):
401404
raise LocalDirReadError(
402405
src=src_root,
403406
context={
@@ -499,48 +502,25 @@ def _open_local_dir_file_for_copy(
499502
dir_fds.append(current_fd)
500503
for part in rel_child.parts[:-1]:
501504
current_rel = current_rel / part if current_rel.parts else Path(part)
502-
try:
503-
next_fd = os.open(part, dir_flags, dir_fd=current_fd)
504-
except OSError as e:
505-
raise self._local_dir_open_error(
506-
src_root=src_root,
507-
parent_fd=current_fd,
508-
entry_name=part,
509-
rel_child=current_rel,
510-
expect_dir=True,
511-
error=e,
512-
) from e
513-
next_stat = os.fstat(next_fd)
514-
if not stat.S_ISDIR(next_stat.st_mode):
515-
raise LocalDirReadError(
516-
src=src_root,
517-
context={
518-
"reason": "path_changed_during_copy",
519-
"child": rel_child.as_posix(),
520-
},
521-
)
522-
dir_fds.append(next_fd)
523-
current_fd = next_fd
524-
525-
try:
526-
leaf_fd = os.open(rel_child.name, file_flags, dir_fd=current_fd)
527-
except OSError as e:
528-
raise self._local_dir_open_error(
505+
next_fd = self._open_local_dir_entry_checked(
529506
src_root=src_root,
530507
parent_fd=current_fd,
531-
entry_name=rel_child.name,
532-
rel_child=rel_child,
533-
expect_dir=False,
534-
error=e,
535-
) from e
536-
leaf_stat = os.fstat(leaf_fd)
537-
if not stat.S_ISREG(leaf_stat.st_mode):
538-
os.close(leaf_fd)
539-
raise LocalDirReadError(
540-
src=src_root,
541-
context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()},
508+
entry_name=part,
509+
rel_child=current_rel,
510+
expect_dir=True,
511+
flags=dir_flags,
542512
)
543-
return leaf_fd
513+
dir_fds.append(next_fd)
514+
current_fd = next_fd
515+
516+
return self._open_local_dir_entry_checked(
517+
src_root=src_root,
518+
parent_fd=current_fd,
519+
entry_name=rel_child.name,
520+
rel_child=rel_child,
521+
expect_dir=False,
522+
flags=file_flags,
523+
)
544524
except FileNotFoundError:
545525
raise LocalDirReadError(
546526
src=src_root,
@@ -583,30 +563,23 @@ def _open_local_dir_src_root_fd(
583563
parts = self.src.parts
584564

585565
try:
586-
current_fd = os.open(current_path, dir_flags)
566+
current_fd = self._open_local_dir_path_checked(
567+
src_root=src_root,
568+
path=current_path,
569+
rel_child=Path(self._local_dir_source_child_label(base_dir, current_path)),
570+
flags=dir_flags,
571+
)
587572
dir_fds.append(current_fd)
588573
for part in parts:
589574
current_rel = current_rel / part if current_rel.parts else Path(part)
590-
try:
591-
next_fd = os.open(part, dir_flags, dir_fd=current_fd)
592-
except OSError as e:
593-
raise self._local_dir_open_error(
594-
src_root=src_root,
595-
parent_fd=current_fd,
596-
entry_name=part,
597-
rel_child=current_rel,
598-
expect_dir=True,
599-
error=e,
600-
) from e
601-
next_stat = os.fstat(next_fd)
602-
if not stat.S_ISDIR(next_stat.st_mode):
603-
raise LocalDirReadError(
604-
src=src_root,
605-
context={
606-
"reason": "path_changed_during_copy",
607-
"child": current_rel.as_posix(),
608-
},
609-
)
575+
next_fd = self._open_local_dir_entry_checked(
576+
src_root=src_root,
577+
parent_fd=current_fd,
578+
entry_name=part,
579+
rel_child=current_rel,
580+
expect_dir=True,
581+
flags=dir_flags,
582+
)
610583
dir_fds.append(next_fd)
611584
current_fd = next_fd
612585
return dir_fds.pop()
@@ -620,6 +593,189 @@ def _open_local_dir_src_root_fd(
620593
for dir_fd in reversed(dir_fds):
621594
os.close(dir_fd)
622595

596+
def _open_local_dir_path_checked(
597+
self,
598+
*,
599+
src_root: Path,
600+
path: Path,
601+
rel_child: Path,
602+
flags: int,
603+
) -> int:
604+
path_stat = self._stat_local_dir_path(
605+
src_root=src_root,
606+
path=path,
607+
rel_child=rel_child,
608+
)
609+
try:
610+
path_fd = os.open(path, flags)
611+
except OSError as e:
612+
raise self._local_dir_path_open_error(
613+
src_root=src_root,
614+
path=path,
615+
rel_child=rel_child,
616+
error=e,
617+
) from e
618+
619+
try:
620+
opened_stat = os.fstat(path_fd)
621+
if not stat.S_ISDIR(opened_stat.st_mode) or not os.path.samestat(
622+
path_stat,
623+
opened_stat,
624+
):
625+
raise LocalDirReadError(
626+
src=src_root,
627+
context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()},
628+
)
629+
return path_fd
630+
except Exception:
631+
os.close(path_fd)
632+
raise
633+
634+
def _stat_local_dir_path(
635+
self,
636+
*,
637+
src_root: Path,
638+
path: Path,
639+
rel_child: Path,
640+
) -> os.stat_result:
641+
try:
642+
path_stat = path.stat(follow_symlinks=False)
643+
except FileNotFoundError:
644+
raise LocalDirReadError(
645+
src=src_root,
646+
context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()},
647+
) from None
648+
except OSError as e:
649+
raise LocalDirReadError(src=src_root, cause=e) from e
650+
651+
if stat.S_ISLNK(path_stat.st_mode):
652+
raise LocalDirReadError(
653+
src=src_root,
654+
context={"reason": "symlink_not_supported", "child": rel_child.as_posix()},
655+
)
656+
if not stat.S_ISDIR(path_stat.st_mode):
657+
raise LocalDirReadError(
658+
src=src_root,
659+
context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()},
660+
)
661+
return path_stat
662+
663+
def _open_local_dir_entry_checked(
664+
self,
665+
*,
666+
src_root: Path,
667+
parent_fd: int,
668+
entry_name: str,
669+
rel_child: Path,
670+
expect_dir: bool,
671+
flags: int,
672+
) -> int:
673+
entry_stat = self._stat_local_dir_entry(
674+
src_root=src_root,
675+
parent_fd=parent_fd,
676+
entry_name=entry_name,
677+
rel_child=rel_child,
678+
expect_dir=expect_dir,
679+
)
680+
try:
681+
entry_fd = os.open(entry_name, flags, dir_fd=parent_fd)
682+
except OSError as e:
683+
raise self._local_dir_open_error(
684+
src_root=src_root,
685+
parent_fd=parent_fd,
686+
entry_name=entry_name,
687+
rel_child=rel_child,
688+
expect_dir=expect_dir,
689+
error=e,
690+
) from e
691+
692+
try:
693+
opened_stat = os.fstat(entry_fd)
694+
if not self._local_dir_entry_stat_matches(
695+
opened_stat,
696+
expect_dir=expect_dir,
697+
) or not os.path.samestat(entry_stat, opened_stat):
698+
raise LocalDirReadError(
699+
src=src_root,
700+
context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()},
701+
)
702+
return entry_fd
703+
except Exception:
704+
os.close(entry_fd)
705+
raise
706+
707+
def _stat_local_dir_entry(
708+
self,
709+
*,
710+
src_root: Path,
711+
parent_fd: int,
712+
entry_name: str,
713+
rel_child: Path,
714+
expect_dir: bool,
715+
) -> os.stat_result:
716+
try:
717+
entry_stat = os.stat(entry_name, dir_fd=parent_fd, follow_symlinks=False)
718+
except FileNotFoundError:
719+
raise LocalDirReadError(
720+
src=src_root,
721+
context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()},
722+
) from None
723+
except OSError as e:
724+
raise LocalDirReadError(src=src_root, cause=e) from e
725+
726+
if stat.S_ISLNK(entry_stat.st_mode):
727+
raise LocalDirReadError(
728+
src=src_root,
729+
context={"reason": "symlink_not_supported", "child": rel_child.as_posix()},
730+
)
731+
if not self._local_dir_entry_stat_matches(entry_stat, expect_dir=expect_dir):
732+
raise LocalDirReadError(
733+
src=src_root,
734+
context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()},
735+
)
736+
return entry_stat
737+
738+
@staticmethod
739+
def _local_dir_entry_stat_matches(entry_stat: os.stat_result, *, expect_dir: bool) -> bool:
740+
if expect_dir:
741+
return stat.S_ISDIR(entry_stat.st_mode)
742+
return stat.S_ISREG(entry_stat.st_mode)
743+
744+
def _local_dir_path_open_error(
745+
self,
746+
*,
747+
src_root: Path,
748+
path: Path,
749+
rel_child: Path,
750+
error: OSError,
751+
) -> LocalDirReadError:
752+
try:
753+
path_stat = path.stat(follow_symlinks=False)
754+
except FileNotFoundError:
755+
return LocalDirReadError(
756+
src=src_root,
757+
context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()},
758+
)
759+
except OSError:
760+
path_stat = None
761+
762+
if path_stat is not None and stat.S_ISLNK(path_stat.st_mode):
763+
return LocalDirReadError(
764+
src=src_root,
765+
context={"reason": "symlink_not_supported", "child": rel_child.as_posix()},
766+
)
767+
if path_stat is not None and not stat.S_ISDIR(path_stat.st_mode):
768+
return LocalDirReadError(
769+
src=src_root,
770+
context={"reason": "path_changed_during_copy", "child": rel_child.as_posix()},
771+
)
772+
if error.errno == errno.ELOOP:
773+
return LocalDirReadError(
774+
src=src_root,
775+
context={"reason": "symlink_not_supported", "child": rel_child.as_posix()},
776+
)
777+
return LocalDirReadError(src=src_root, cause=error)
778+
623779
def _local_dir_open_error(
624780
self,
625781
*,

0 commit comments

Comments
 (0)