Skip to content

Commit 1ecfad6

Browse files
committed
git: Exit clearly when in a "dubious" directory
I hit a _very_ hard to diagnose issue today after switching part of a CI environment. It turns out that due to non-privileged container magic the CI step was not running as the user that cloned the repo. This manifested as the sdist generated during this step just not having a few extra data files; but the build still passed. So several steps later, suddenly files have just vanished from the sdist, which of course is not something that is checked for and so testing blows up in very strange ways. After I understood what was going on, there is a _tiny_ little message hidden amongst the logs that gives a hint of what's going on. ... writing requirements to src/proj.egg-info/requires.txt writing top-level names to src/proj.egg-info/top_level.txt listing git files failed - pretending there aren't any reading manifest file 'src/proj.egg-info/SOURCES.txt' writing manifest file 'src/proj.egg-info/SOURCES.txt' ... What is actually happening is that if you run git you get "git status fatal: detected dubious ownership in repository at '/..proj'". This is the well-known CVE-2022-24765 issue where trusting a `.git` config dir from another user causes problems. In 6a3bb96 all the calls in setuptools_scm/git.py were updated to use `--git-dir` directly -- git will not complain if you have told it to explicitly trust the config dir like this. However, there are calls in _file_finders/git.py to find the top-level that are not using this. It silently (modulo an easily missed log) skips adding files when this occurs. I can not see that this would ever be the behaviour you would want. If it had of exploded telling me the git call failed, it would have short-cut all of the problem finding. This adds an explicit match on the static part of this git failure message and raises a SystemExit if it hits. A test case that mocks such a situation is added. Closes: #784 Signed-off-by: Ian Wienand <[email protected]>
1 parent e56b78f commit 1ecfad6

File tree

3 files changed

+49
-0
lines changed

3 files changed

+49
-0
lines changed

docs/usage.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,6 +622,9 @@ would be required when not using `setuptools-scm`.
622622
-**Check**: Is setuptools-scm installed in your build environment?
623623
-**Check**: Are you in a valid SCM repository?
624624

625+
**Problem: Error about dubious ownership**
626+
- CVE-2022-24765 identified security issues with git trusting repositories owned by another user. If the file-finder detects that git is unable to list files because it is operating in a git directory owned by another user, it will raise an error. Since this was not the default behaviour of `setuptools_scm` previously, you can override this by setting `SETUPTOOLS_SCM_IGNORE_DUBIOUS_OWNER`, but be warned git will not be listing files correctly with this flag set.
627+
625628
### Timestamps for Local Development Versions
626629

627630
!!! info "Improved Timestamp Behavior"

src/setuptools_scm/_file_finders/git.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ def _git_toplevel(path: str) -> str | None:
2222
cwd = os.path.abspath(path or ".")
2323
res = _run(["git", "rev-parse", "HEAD"], cwd=cwd)
2424
if res.returncode:
25+
# This catches you being in a git directory, but the
26+
# permissions being incorrect. With modern contanizered
27+
# CI environments you can easily end up in a cloned repo
28+
# with incorrect permissions and we don't want to silently
29+
# ignore files.
30+
if "--add safe.directory" in res.stderr and not os.environ.get(
31+
"SETUPTOOLS_SCM_IGNORE_DUBIOUS_OWNER"
32+
):
33+
log.error(res.stderr)
34+
raise SystemExit(
35+
"git introspection failed: {}".format(res.stderr.split("\n")[0])
36+
)
2537
# BAIL if there is no commit
2638
log.error("listing git files failed - pretending there aren't any")
2739
return None

testing/test_git.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import shutil
66
import subprocess
77
import sys
8+
import textwrap
89

910
from datetime import date
1011
from datetime import datetime
@@ -19,6 +20,7 @@
1920
import pytest
2021

2122
import setuptools_scm._file_finders
23+
import setuptools_scm._file_finders.git
2224

2325
from setuptools_scm import Configuration
2426
from setuptools_scm import NonNormalizedVersion
@@ -861,3 +863,35 @@ def test_git_no_commits_uses_fallback_version(wd: WorkDir) -> None:
861863
assert str(version_no_fallback.tag) == "0.0"
862864
assert version_no_fallback.distance == 0
863865
assert version_no_fallback.dirty is True
866+
867+
868+
@pytest.mark.issue("https://github.com/pypa/setuptools-scm/issues/784")
869+
def test_dubious_dir(
870+
wd: WorkDir, caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch
871+
) -> None:
872+
"""Test that we exit clearly if we are in a unsafe directory"""
873+
wd.commit_testfile()
874+
git_wd = git.GitWorkdir(wd.cwd)
875+
876+
def _run(*args, **kwargs) -> CompletedProcess: # type: ignore[no-untyped-def]
877+
"""Fake "git rev-parse HEAD" to fail as if you do not own the git repo"""
878+
stderr = textwrap.dedent(f"""
879+
fatal: detected dubious ownership in repository at '{git_wd}'
880+
To add an exception for this directory, call:
881+
882+
git config --global --add safe.directory /this/is/a/fake/path
883+
""")
884+
orig_run = run
885+
if args[0] == ["git", "rev-parse", "HEAD"]:
886+
return CompletedProcess(
887+
args=[], stdout="%cI", stderr=stderr, returncode=128
888+
)
889+
return orig_run(*args, **kwargs)
890+
891+
monkeypatch.setattr(setuptools_scm._file_finders.git, "_run", _run)
892+
with pytest.raises(SystemExit):
893+
git_find_files(str(wd.cwd))
894+
895+
assert "fatal: detected dubious ownership in repository" in " ".join(
896+
caplog.messages
897+
)

0 commit comments

Comments
 (0)