FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(index): confine staging reads to worktree files · gitpython-developers/GitPython@f2dfa9f · GitHub

  1. gitpython-developers
  2. GitPython

Repository navigation

Commit f2dfa9f

Browse files
codex
authored and
Byron
committed
fix(index): confine staging reads to worktree files
<!-- Byron --> rubber-stamp, after checking diff quickly. <!-- agent --> `IndexFile.add()` could read through intermediate directory symlinks and store files outside the working tree. Null-SHA `Blob` and `BaseIndexEntry` inputs reached the same reader, while unsafe entry and rewritten paths could enter the in-memory index with `write=False` (finding 21501). Validate supplied object paths and all final entry paths with the shared repository path checker. Confine filesystem inputs in `_store_path()` and reject intermediate symlinks or redirected directories before opening files. Preserve final symlinks as links, including when rewriting paths, and reject special files. Use `O_NOFOLLOW` and `O_NONBLOCK` where available and check the opened regular file with `fstat()`. Skip Git metadata during directory expansion and stage directory symlinks themselves. Validate complete relative directory paths so nested POSIX colon names are not mistaken for drive-prefixed paths. Parent-directory checks handle the existing filesystem layout; they do not make staging atomic against concurrent directory replacement. The open flags protect final-component substitution on supporting platforms. Tests cover path/glob/object inputs, inside and outside symlink targets, rewriters, in-memory index validation, metadata exclusion, directory symlinks, special files, and nested colon names. Validation: 129 combined tree/index tests passed, with two platform skips; Ruff checks and formatting checks passed.
1 parent d1576c4 commit f2dfa9f

2 files changed

Lines changed: 150 additions & 8 deletions

File tree

‎git/index/base.py‎

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
from io import BytesIO
1515
import os
1616
import os.path as osp
17-
from stat import S_ISLNK
17+
from stat import S_ISLNK, S_ISREG
1818
import subprocess
1919
import sys
2020
import tempfile
@@ -36,6 +36,7 @@
3636
file_contents_ro,
3737
_is_path_rooted,
3838
_to_relative_path,
39+
_validate_repo_path,
3940
to_native_path_linux,
4041
unbare_repo,
4142
to_bin_sha,
@@ -476,7 +477,17 @@ def raise_exc(e: Exception) -> NoReturn:
476477
continue
477478
# END glob handling
478479
try:
479-
for root, _dirs, files in os.walk(abs_path, onerror=raise_exc):
480+
for root, dirs, files in os.walk(abs_path, onerror=raise_exc):
481+
for dirname in dirs[:]:
482+
directory = osp.join(root, dirname)
483+
try:
484+
_validate_repo_path(to_native_path_linux(osp.relpath(directory, r)))
485+
except ValueError:
486+
dirs.remove(dirname)
487+
continue
488+
if osp.islink(directory):
489+
dirs.remove(dirname)
490+
yield osp.relpath(directory, r)
480491
for rela_file in files:
481492
# Add relative paths only.
482493
yield osp.join(root.replace(rs, ""), rela_file)
@@ -717,6 +728,8 @@ def _preprocess_add_items(
717728
else:
718729
raise TypeError("Invalid Type: %r" % item)
719730
# END for each item
731+
for entry in entries:
732+
_validate_repo_path(entry.path)
720733
return paths, entries
721734

722735
def _store_path(self, filepath: PathLike, fprogress: Callable) -> BaseIndexEntry:
@@ -726,20 +739,43 @@ def _store_path(self, filepath: PathLike, fprogress: Callable) -> BaseIndexEntry
726739
This needs the :func:`~git.index.util.git_working_dir` decorator active!
727740
This must be ensured in the calling code.
728741
"""
729-
st = os.lstat(filepath) # Handles non-symlinks as well.
730-
742+
filepath = self._to_relative_path(filepath)
743+
_validate_repo_path(filepath)
744+
parent = osp.realpath(self.repo.working_dir)
745+
for component in os.fspath(filepath).split("/")[:-1]:
746+
parent = osp.join(parent, component)
747+
if osp.islink(parent) or osp.normcase(osp.realpath(parent)) != osp.normcase(osp.abspath(parent)):
748+
raise ValueError("Cannot stage a path beyond a symbolic link: %r" % filepath)
749+
st = os.lstat(filepath)
750+
if not S_ISLNK(st.st_mode) and not S_ISREG(st.st_mode):
751+
raise ValueError("Can only stage a regular file or symbolic link: %r" % filepath)
752+
753+
stream_size = st.st_size
731754
if S_ISLNK(st.st_mode):
732755
# readlink is a string, but we need bytes.
756+
target = force_bytes(os.readlink(filepath), encoding=defenc)
757+
stream_size = len(target)
758+
733759
def open_stream() -> BinaryIO:
734-
return BytesIO(force_bytes(os.readlink(filepath), encoding=defenc))
760+
return BytesIO(target)
735761
else:
736762

737763
def open_stream() -> BinaryIO:
738-
return open(filepath, "rb")
764+
# Do not follow a final symlink or block on a FIFO substituted
765+
# between lstat and open on platforms supporting these flags.
766+
def opener(path: str, flags: int) -> int:
767+
return os.open(path, flags | getattr(os, "O_NOFOLLOW", 0) | getattr(os, "O_NONBLOCK", 0))
768+
769+
return open(filepath, "rb", opener=opener)
739770

740771
with open_stream() as stream:
772+
if not S_ISLNK(st.st_mode):
773+
st = os.fstat(stream.fileno())
774+
if not S_ISREG(st.st_mode):
775+
raise ValueError("Can only stage a regular file: %r" % filepath)
776+
stream_size = st.st_size
741777
fprogress(filepath, False, filepath)
742-
istream = self.repo.odb.store(IStream(Blob.type, st.st_size, stream))
778+
istream = self.repo.odb.store(IStream(Blob.type, stream_size, stream))
743779
fprogress(filepath, True, filepath)
744780
return BaseIndexEntry(
745781
(
@@ -777,7 +813,7 @@ def _entries_for_paths(
777813
blob = Blob(
778814
self.repo,
779815
Blob.NULL_BIN_SHA,
780-
stat_mode_to_index_mode(os.stat(abspath).st_mode),
816+
stat_mode_to_index_mode(os.lstat(abspath).st_mode),
781817
to_native_path_linux(gitrelative_path),
782818
)
783819
# TODO: variable undefined
@@ -989,6 +1025,8 @@ def handle_null_entries(self: "IndexFile") -> None:
9891025

9901026
# FINALIZE
9911027
# Add the new entries to this instance.
1028+
for entry in entries_added:
1029+
_validate_repo_path(entry.path)
9921030
for entry in entries_added:
9931031
self.entries[(entry.path, 0)] = IndexEntry.from_base(entry)
9941032

‎test/test_index.py‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1180,6 +1180,110 @@ def test_add_a_file_with_wildcard_chars(self, rw_dir):
11801180
r.index.add([fp])
11811181
r.index.commit("Added [.exe")
11821182

1183+
@ddt.data(*product(("path", "glob", "blob", "entry"), (False, True)))
1184+
@ddt.unpack
1185+
@with_rw_directory
1186+
def test_staging_never_reads_through_a_directory_symlink(self, rw_dir, kind, outside):
1187+
tmp_path = Path(rw_dir)
1188+
with Repo.init(tmp_path / "repo") as repo:
1189+
root = tmp_path / "repo"
1190+
target = (tmp_path if outside else root) / "target"
1191+
target.mkdir()
1192+
(target / "secret").write_text("private data")
1193+
try:
1194+
(root / "link").symlink_to(target, target_is_directory=True)
1195+
except OSError:
1196+
pytest.skip("Symlinks unavailable")
1197+
item = "link/secret"
1198+
if kind == "glob":
1199+
item = "link/*"
1200+
elif kind == "blob":
1201+
item = Blob(repo, Blob.NULL_BIN_SHA, 0o100644, item)
1202+
elif kind == "entry":
1203+
item = BaseIndexEntry((0o100644, Blob.NULL_BIN_SHA, 0, item))
1204+
with mock.patch.object(repo.odb, "store", wraps=repo.odb.store) as store:
1205+
with pytest.raises(ValueError, match="symbolic link"):
1206+
repo.index.add([item], write=False)
1207+
store.assert_not_called()
1208+
1209+
@ddt.data("blob", "entry", "rewriter")
1210+
@with_rw_directory
1211+
def test_staging_rejects_unsafe_object_paths_even_without_writing(self, rw_dir, kind):
1212+
with Repo.init(rw_dir) as repo:
1213+
entry = BaseIndexEntry((0o100644, b"a" * 20, 0, "../outside"))
1214+
item = Blob(repo, entry.binsha, entry.mode, entry.path) if kind == "blob" else entry
1215+
kwargs = {}
1216+
if kind == "rewriter":
1217+
item = BaseIndexEntry((0o100644, b"a" * 20, 0, "safe"))
1218+
kwargs["path_rewriter"] = lambda entry: "../outside"
1219+
index = repo.index
1220+
with pytest.raises(ValueError):
1221+
index.add([item], write=False, **kwargs)
1222+
assert not index.entries
1223+
1224+
@with_rw_directory
1225+
def test_staging_root_preserves_symlinks_and_skips_git_metadata(self, rw_dir):
1226+
tmp_path = Path(rw_dir)
1227+
root = tmp_path / "repo"
1228+
(tmp_path / "outside").mkdir()
1229+
with Repo.init(root) as repo:
1230+
(root / "file").write_text("contents")
1231+
try:
1232+
(root / "link").symlink_to("../outside", target_is_directory=True)
1233+
except OSError:
1234+
pytest.skip("Symlinks unavailable")
1235+
entries = repo.index.add(["."])
1236+
assert {entry.path for entry in entries} == {"file", "link"}
1237+
link = repo.index.entries[("link", 0)]
1238+
assert link.mode == 0o120000
1239+
assert repo.odb.stream(link.binsha).read() == os.fsencode(os.readlink(root / "link"))
1240+
1241+
@with_rw_directory
1242+
def test_staging_symlink_measures_encoded_target_instead_of_stat_size(self, rw_dir):
1243+
tmp_path = Path(rw_dir)
1244+
with Repo.init(tmp_path) as repo:
1245+
link = tmp_path / "link"
1246+
try:
1247+
link.symlink_to("../café", target_is_directory=True)
1248+
except OSError:
1249+
pytest.skip("Symlinks unavailable")
1250+
target = os.fsencode(os.readlink(link))
1251+
original_lstat = os.lstat
1252+
1253+
def zero_size_for_symlinks(*args, **kwargs):
1254+
result = original_lstat(*args, **kwargs)
1255+
if S_ISLNK(result.st_mode):
1256+
fields = list(result)
1257+
fields[6] = 0
1258+
return os.stat_result(fields)
1259+
return result
1260+
1261+
with mock.patch("os.lstat", side_effect=zero_size_for_symlinks):
1262+
(entry,) = repo.index.add(["link"])
1263+
assert entry.mode == 0o120000
1264+
assert repo.odb.stream(entry.binsha).read() == target
1265+
1266+
@pytest.mark.skipif(os.name == "nt", reason="Colons are not valid Windows filenames")
1267+
@with_rw_directory
1268+
def test_staging_nested_colon_directory(self, rw_dir):
1269+
tmp_path = Path(rw_dir)
1270+
with Repo.init(tmp_path) as repo:
1271+
directory = tmp_path / "nested" / "a:b"
1272+
directory.mkdir(parents=True)
1273+
(directory / "file").write_text("contents")
1274+
assert [entry.path for entry in repo.index.add(["nested"])] == ["nested/a:b/file"]
1275+
assert repo.index.write_tree()["nested/a:b/file"].data_stream.read() == b"contents"
1276+
1277+
@pytest.mark.skipif(not hasattr(os, "mkfifo"), reason="FIFOs unavailable")
1278+
@with_rw_directory
1279+
def test_staging_special_files_fails_before_opening(self, rw_dir):
1280+
tmp_path = Path(rw_dir)
1281+
with Repo.init(tmp_path) as repo:
1282+
os.mkfifo(tmp_path / "fifo")
1283+
with mock.patch("builtins.open", side_effect=AssertionError("must not open a FIFO")):
1284+
with pytest.raises(ValueError, match="regular file"):
1285+
repo.index.add(["fifo"], write=False)
1286+
11831287
def test__to_relative_path_at_root(self):
11841288
root = osp.abspath(os.sep)
11851289

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL