Skip to content

Commit b83ce1b

Browse files
committed
Mark unsafe-options "allowed" tests xfail on Windows
The tests of unsafe options are among those introduced originally in gitpython-developers#1521. They are regression tests for gitpython-developers#1515 (CVE-2022-24439). The unsafe options tests are paired: a test for the usual, default behavior of forbidding the option, and a test for the behavior when the option is explicitly allowed. In each such pair, both tests use a payload that is intended to produce the side effect of a file of a specific name being created in a temporary directory. All the tests work on Unix-like systems. On Windows, the tests of the *allowed* cases are broken, and this commit marks them xfail. However, this has implications for the tests of the default, secure behavior, because until the "allowed" versions work on Windows, it will be unclear if either are using a payload that is effective and that corresponds to the way its effect is examined. What *seems* to happen is this: The "\" characters in the path are treated as shell escape characters rather than literally, with the effect of disappearing in most paths since most letters lack special meaning when escaped. Also, "touch" is not a native Windows command, and the "touch" command provided by Git for Windows is linked against MSYS2 libraries, causing it to map (some?) occurrences of ":" in filenames to a separate code point in the Private Use Area of the Basic Multilingual Plane. The result is a path with no directory separators or drive letter. It denotes a file of an unintended name in the current directory, which is never the intended location. The current directory depends on GitPython implementation details, but at present it's the top-level directory of the rw_repo working tree. A new unstaged file, named like "C\357\200\272UsersekAppDataLocalTemptmpc7x4xik5pwn", can be observed there (this is how "git status" will format the name). Fortunately, this and all related tests are working on other OSes, and the affected code under test does not appear highly dependent on OS. So the fix is *probably* fully working on Windows as well.
1 parent 5e4b899 commit b83ce1b

File tree

2 files changed

+45
-0
lines changed

2 files changed

+45
-0
lines changed

test/test_remote.py

+27
Original file line numberDiff line numberDiff line change
@@ -831,6 +831,15 @@ def test_fetch_unsafe_options(self, rw_repo):
831831
remote.fetch(**unsafe_option)
832832
assert not tmp_file.exists()
833833

834+
@pytest.mark.xfail(
835+
os.name == "nt",
836+
reason=(
837+
"File not created. A separate Windows command may be needed. This and the "
838+
"currently passing test test_fetch_unsafe_options must be adjusted in the "
839+
"same way. Until then, test_fetch_unsafe_options is unreliable on Windows."
840+
),
841+
raises=AssertionError,
842+
)
834843
@with_rw_repo("HEAD")
835844
def test_fetch_unsafe_options_allowed(self, rw_repo):
836845
with tempfile.TemporaryDirectory() as tdir:
@@ -890,6 +899,15 @@ def test_pull_unsafe_options(self, rw_repo):
890899
remote.pull(**unsafe_option)
891900
assert not tmp_file.exists()
892901

902+
@pytest.mark.xfail(
903+
os.name == "nt",
904+
reason=(
905+
"File not created. A separate Windows command may be needed. This and the "
906+
"currently passing test test_pull_unsafe_options must be adjusted in the "
907+
"same way. Until then, test_pull_unsafe_options is unreliable on Windows."
908+
),
909+
raises=AssertionError,
910+
)
893911
@with_rw_repo("HEAD")
894912
def test_pull_unsafe_options_allowed(self, rw_repo):
895913
with tempfile.TemporaryDirectory() as tdir:
@@ -955,6 +973,15 @@ def test_push_unsafe_options(self, rw_repo):
955973
remote.push(**unsafe_option)
956974
assert not tmp_file.exists()
957975

976+
@pytest.mark.xfail(
977+
os.name == "nt",
978+
reason=(
979+
"File not created. A separate Windows command may be needed. This and the "
980+
"currently passing test test_push_unsafe_options must be adjusted in the "
981+
"same way. Until then, test_push_unsafe_options is unreliable on Windows."
982+
),
983+
raises=AssertionError,
984+
)
958985
@with_rw_repo("HEAD")
959986
def test_push_unsafe_options_allowed(self, rw_repo):
960987
with tempfile.TemporaryDirectory() as tdir:

test/test_repo.py

+18
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,15 @@ def test_clone_unsafe_options(self, rw_repo):
294294
rw_repo.clone(tmp_dir, **unsafe_option)
295295
assert not tmp_file.exists()
296296

297+
@pytest.mark.xfail(
298+
os.name == "nt",
299+
reason=(
300+
"File not created. A separate Windows command may be needed. This and the "
301+
"currently passing test test_clone_unsafe_options must be adjusted in the "
302+
"same way. Until then, test_clone_unsafe_options is unreliable on Windows."
303+
),
304+
raises=AssertionError,
305+
)
297306
@with_rw_repo("HEAD")
298307
def test_clone_unsafe_options_allowed(self, rw_repo):
299308
with tempfile.TemporaryDirectory() as tdir:
@@ -364,6 +373,15 @@ def test_clone_from_unsafe_options(self, rw_repo):
364373
Repo.clone_from(rw_repo.working_dir, tmp_dir, **unsafe_option)
365374
assert not tmp_file.exists()
366375

376+
@pytest.mark.xfail(
377+
os.name == "nt",
378+
reason=(
379+
"File not created. A separate Windows command may be needed. This and the "
380+
"currently passing test test_clone_from_unsafe_options must be adjusted in the "
381+
"same way. Until then, test_clone_from_unsafe_options is unreliable on Windows."
382+
),
383+
raises=AssertionError,
384+
)
367385
@with_rw_repo("HEAD")
368386
def test_clone_from_unsafe_options_allowed(self, rw_repo):
369387
with tempfile.TemporaryDirectory() as tdir:

0 commit comments

Comments
 (0)