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

Guard pathspec file inputs in high-level commands · gitpython-developers/GitPython@f2550b6 · GitHub

/ GitPython Public

Commit f2550b6

Browse files
Byron
and
codex
committed
Guard pathspec file inputs in high-level commands
<!-- agent --> GHSA-hh9p-6wh2-4mfc reports that high-level rm and checkout wrappers forwarded pathspec file options without GitPython's unsafe-option policy. A regression showed that both commands surfaced multi-line pathspec data in Git errors, while reset consumed the same caller-selected file without a validation error. The audit also found that reset's positional commit could carry the option before its argument separator. Define one shared unsafe pathspec-file option list and apply it to IndexFile.remove, Head.checkout, and HEAD.reset before invoking Git. Check reset's positional commit as well as keyword options, retain the standard allow_unsafe_options escape hatch for trusted callers, and cover abbreviated long-option spellings. An audit against Git cf5497b14c5a24f10c13f7e0ee85cb95af13ea6a (v2.55.0.windows.3-16-gcf5497b14c) found pathspec-file support in add, checkout/restore, commit, reset, rm, and stash. GitPython has no arbitrary high-level option forwarding to the other commands, and git mv does not support this option. Validated with focused rejection and opt-in tests, 214 affected-module regressions, Ruff, basedpyright, and git diff --check. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
1 parent d9ddb55 commit f2550b6

6 files changed

Lines changed: 125 additions & 1 deletion

File tree

‎git/cmd.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,12 @@ class Git(metaclass=_GitMeta):
654654
"--upload-pack",
655655
]
656656

657+
unsafe_git_pathspec_from_file_options = [
658+
# Reads pathspecs from a caller-controlled file. Some commands include an
659+
# unmatched pathspec in their error output, which can disclose the file.
660+
"--pathspec-from-file",
661+
]
662+
657663
def __getstate__(self) -> Dict[str, Any]:
658664
return slots_to_dict(self, exclude=self._excluded_)
659665

‎git/index/base.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,6 +1024,7 @@ def remove(
10241024
self,
10251025
items: Union[PathLike, Sequence[Union[PathLike, Blob, BaseIndexEntry, "Submodule"]]],
10261026
working_tree: bool = False,
1027+
allow_unsafe_options: bool = False,
10271028
**kwargs: Any,
10281029
) -> List[str]:
10291030
R"""Remove the given items from the index and optionally from the working tree
@@ -1054,6 +1055,10 @@ def remove(
10541055
physically removing the respective file. This may fail if there are
10551056
uncommitted changes in it.
10561057
1058+
:param allow_unsafe_options:
1059+
Allow unsafe options such as ``--pathspec-from-file`` to be passed to
1060+
:manpage:`git-rm(1)`.
1061+
10571062
:param kwargs:
10581063
Additional keyword arguments to be passed to :manpage:`git-rm(1)`, such as
10591064
``r`` to allow recursive removal.
@@ -1065,6 +1070,11 @@ def remove(
10651070
This is interesting to know in case you have provided a directory or globs.
10661071
Paths are relative to the repository.
10671072
"""
1073+
if not allow_unsafe_options:
1074+
Git.check_unsafe_options(
1075+
options=Git._option_candidates([], kwargs),
1076+
unsafe_options=Git.unsafe_git_pathspec_from_file_options,
1077+
)
10681078
args = []
10691079
if not working_tree:
10701080
args.append("--cached")

‎git/refs/head.py‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
from typing import Any, Sequence, TYPE_CHECKING, Union
2121

22+
from git.cmd import Git
2223
from git.types import Commit_ish, PathLike
2324

2425
if TYPE_CHECKING:
@@ -62,6 +63,7 @@ def reset(
6263
index: bool = True,
6364
working_tree: bool = False,
6465
paths: Union[PathLike, Sequence[PathLike], None] = None,
66+
allow_unsafe_options: bool = False,
6567
**kwargs: Any,
6668
) -> "HEAD":
6769
"""Reset our HEAD to the given commit optionally synchronizing the index and
@@ -84,12 +86,21 @@ def reset(
8486
Single path or list of paths relative to the git root directory
8587
that are to be reset. This allows to partially reset individual files.
8688
89+
:param allow_unsafe_options:
90+
Allow unsafe options such as ``--pathspec-from-file`` to be passed to
91+
:manpage:`git-reset(1)`.
92+
8793
:param kwargs:
8894
Additional arguments passed to :manpage:`git-reset(1)`.
8995
9096
:return:
9197
self
9298
"""
99+
if not allow_unsafe_options:
100+
Git.check_unsafe_options(
101+
options=Git._option_candidates([commit], kwargs),
102+
unsafe_options=Git.unsafe_git_pathspec_from_file_options,
103+
)
93104
mode: Union[str, None]
94105
mode = "--soft"
95106
if index:
@@ -234,7 +245,12 @@ def rename(self, new_path: PathLike, force: bool = False) -> "Head":
234245
self.path = "%s/%s" % (self._common_path_default, new_path)
235246
return self
236247

237-
def checkout(self, force: bool = False, **kwargs: Any) -> Union["HEAD", "Head"]:
248+
def checkout(
249+
self,
250+
force: bool = False,
251+
allow_unsafe_options: bool = False,
252+
**kwargs: Any,
253+
) -> Union["HEAD", "Head"]:
238254
"""Check out this head by setting the HEAD to this reference, by updating the
239255
index to reflect the tree we point to and by updating the working tree to
240256
reflect the latest index.
@@ -246,6 +262,10 @@ def checkout(self, force: bool = False, **kwargs: Any) -> Union["HEAD", "Head"]:
246262
If ``False``, :exc:`~git.exc.GitCommandError` will be raised in that
247263
situation.
248264
265+
:param allow_unsafe_options:
266+
Allow unsafe options such as ``--pathspec-from-file`` to be passed to
267+
:manpage:`git-checkout(1)`.
268+
249269
:param kwargs:
250270
Additional keyword arguments to be passed to git checkout, e.g.
251271
``b="new_branch"`` to create a new branch at the given spot.
@@ -261,6 +281,11 @@ def checkout(self, force: bool = False, **kwargs: Any) -> Union["HEAD", "Head"]:
261281
the HEAD detached which is allowed and possible, but remains a special state
262282
that some tools might not be able to handle.
263283
"""
284+
if not allow_unsafe_options:
285+
Git.check_unsafe_options(
286+
options=Git._option_candidates([], kwargs),
287+
unsafe_options=Git.unsafe_git_pathspec_from_file_options,
288+
)
264289
kwargs["f"] = force
265290
if kwargs["f"] is False:
266291
kwargs.pop("f")

‎test/test_git.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,18 @@ def test_option_candidates_ignore_untransformed_kwargs(self):
215215

216216
self.assertEqual(options, ["--max-count"])
217217

218+
def test_option_candidates_include_falsey_non_boolean_values(self):
219+
kwargs = {"pathspec_from_file": 0}
220+
candidates = Git._option_candidates(kwargs=kwargs)
221+
222+
self.assertEqual(candidates, ["--pathspec-from-file"])
223+
self.assertEqual(self.git.transform_kwargs(**kwargs), ["--pathspec-from-file=0"])
224+
with self.assertRaises(UnsafeOptionError):
225+
Git.check_unsafe_options(
226+
options=candidates,
227+
unsafe_options=Git.unsafe_git_pathspec_from_file_options,
228+
)
229+
218230
def test_option_candidates_include_split_single_char_option_values(self):
219231
cases = [
220232
({"n": "--upload-pack=helper"}, ["-n", "--upload-pack=helper"], ["--upload-pack"]),

‎test/test_index.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,32 @@ def test_checkout_rejects_unsafe_prefix(self, rw_repo):
212212
rw_repo.index.checkout(prefix=f"{target}/", allow_unsafe_options=True)
213213
self.assertTrue(osp.isfile(osp.join(target, "CHANGES")))
214214

215+
@with_rw_repo("HEAD")
216+
def test_remove_rejects_pathspec_from_file(self, rw_repo):
217+
with tempfile.TemporaryDirectory() as tdir:
218+
pathspecs = Path(tdir) / "pathspecs"
219+
pathspecs.write_bytes(b"unmatched-path-one\nunmatched-path-two")
220+
for option_name in ("pathspec_from_file", "pathspec_from"):
221+
with self.assertRaises(UnsafeOptionError):
222+
rw_repo.index.remove(
223+
[],
224+
pathspec_file_nul=True,
225+
**{option_name: str(pathspecs)},
226+
)
227+
228+
@with_rw_repo("HEAD")
229+
def test_remove_allows_explicit_pathspec_from_file(self, rw_repo):
230+
with tempfile.TemporaryDirectory() as tdir:
231+
pathspecs = Path(tdir) / "pathspecs"
232+
pathspecs.write_bytes(b"CHANGES\0")
233+
removed = rw_repo.index.remove(
234+
[],
235+
pathspec_from_file=str(pathspecs),
236+
pathspec_file_nul=True,
237+
allow_unsafe_options=True,
238+
)
239+
assert "CHANGES" in removed
240+
215241
def __init__(self, *args):
216242
super().__init__(*args)
217243
self._reset_progress()

‎test/test_refs.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,51 @@ def test_head_checkout_detached_head(self, rw_repo):
262262
assert isinstance(res, SymbolicReference)
263263
assert res.name == "HEAD"
264264

265+
@with_rw_repo("HEAD")
266+
def test_head_checkout_rejects_pathspec_from_file(self, rw_repo):
267+
with tempfile.TemporaryDirectory() as tdir:
268+
pathspecs = Path(tdir) / "pathspecs"
269+
pathspecs.write_bytes(b"unmatched-path-one\nunmatched-path-two")
270+
for option_name in ("pathspec_from_file", "pathspec_from"):
271+
with self.assertRaises(UnsafeOptionError):
272+
rw_repo.active_branch.checkout(
273+
pathspec_file_nul=True,
274+
**{option_name: str(pathspecs)},
275+
)
276+
277+
@with_rw_repo("HEAD")
278+
def test_head_reset_rejects_pathspec_from_file(self, rw_repo):
279+
with tempfile.TemporaryDirectory() as tdir:
280+
pathspecs = Path(tdir) / "pathspecs"
281+
pathspecs.write_bytes(b"unmatched-path-one\nunmatched-path-two")
282+
for option_name in ("pathspec_from_file", "pathspec_from"):
283+
with self.assertRaises(UnsafeOptionError):
284+
rw_repo.head.reset(
285+
pathspec_file_nul=True,
286+
**{option_name: str(pathspecs)},
287+
)
288+
for option_name in ("--pathspec-from-file", "--pathspec-from"):
289+
with self.assertRaises(UnsafeOptionError):
290+
rw_repo.head.reset(
291+
f"{option_name}={pathspecs}",
292+
pathspec_file_nul=True,
293+
)
294+
295+
@with_rw_repo("HEAD")
296+
def test_head_commands_allow_explicit_pathspec_from_file(self, rw_repo):
297+
with tempfile.TemporaryDirectory() as tdir:
298+
pathspecs = Path(tdir) / "pathspecs"
299+
pathspecs.write_bytes(b"CHANGES\0")
300+
options = {
301+
"pathspec_from_file": str(pathspecs),
302+
"pathspec_file_nul": True,
303+
"allow_unsafe_options": True,
304+
}
305+
head = rw_repo.head
306+
branch = rw_repo.active_branch
307+
assert head.reset(**options) is head
308+
assert branch.checkout(**options) == branch
309+
265310
@with_rw_repo("0.1.6")
266311
def test_head_reset(self, rw_repo):
267312
cur_head = rw_repo.head

0 commit comments

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