diff --git a/.github/workflows/alpine-test.yml b/.github/workflows/alpine-test.yml index b7de7482e..a5b63f423 100644 --- a/.github/workflows/alpine-test.yml +++ b/.github/workflows/alpine-test.yml @@ -1,6 +1,10 @@ name: test-alpine -on: [push, pull_request, workflow_dispatch] +on: + push: + branches: [main] + pull_request: + workflow_dispatch: permissions: contents: read @@ -26,7 +30,7 @@ jobs: adduser runner docker shell: sh -exo pipefail {0} # Run this as root, not the "runner" user. - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: fetch-depth: 0 @@ -61,6 +65,27 @@ jobs: . .venv/bin/activate pip install '.[test]' + - name: Show POSIX file ownership + run: | + ls -ld -- \ + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "${HOME:?HOME is not set}/.gitconfig" \ + 2>&1 || true + + - name: Show safe.directory entries + # `actions/checkout`'s safe.directory add is only durable for the + # checkout itself (it writes under a throwaway HOME override and + # then discards it), so by the time this step runs the runner + # user's `~/.gitconfig` has no entries -- and the Alpine container + # chowns the workspace to runner:docker to match the test user, so + # git accepts the ownership without one. Expected: `(none)`. + run: git config --global --get-all safe.directory || echo "(none)" + - name: Show version and platform information run: | . .venv/bin/activate diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index e243416a8..1ad1a612c 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -13,6 +13,7 @@ name: "CodeQL" on: push: + branches: [main] pull_request: schedule: - cron: '27 10 * * 3' @@ -47,7 +48,7 @@ jobs: # your codebase is analyzed, see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages steps: - name: Checkout repository - uses: actions/checkout@v6 + uses: actions/checkout@v7 # Add any setup steps before running the `github/codeql-action/init` action. # This includes steps like installing compilers or runtimes (`actions/setup-node` diff --git a/.github/workflows/cygwin-test.yml b/.github/workflows/cygwin-test.yml index 327e1f10c..6f9f347a9 100644 --- a/.github/workflows/cygwin-test.yml +++ b/.github/workflows/cygwin-test.yml @@ -1,6 +1,10 @@ name: test-cygwin -on: [push, pull_request, workflow_dispatch] +on: + push: + branches: [main] + pull_request: + workflow_dispatch: permissions: contents: read @@ -34,7 +38,7 @@ jobs: git config --global core.autocrlf false # Affects the non-Cygwin git. shell: pwsh # Do this outside Cygwin, to affect actions/checkout. - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: fetch-depth: 0 @@ -53,6 +57,8 @@ jobs: run: | git config --global --add safe.directory "$(pwd)" git config --global --add safe.directory "$(pwd)/.git" + git config --global --add safe.directory "$(pwd)/git/ext/gitdb" + git config --global --add safe.directory "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" git config --global core.autocrlf false - name: Prepare this repo for tests @@ -80,6 +86,58 @@ jobs: run: | pip install '.[test]' + - name: Show POSIX file ownership + # Cygwin's `ls -ld` reports the NTFS Owner SID via Cygwin's SID-to-uid + # mapping (well-known SIDs by their RID, machine-local accounts by + # 0x30000+RID). That mapping is what Cygwin git's + # `is_path_owned_by_current_user` reduces to, so this is the view that + # determines whether `safe.directory` is consulted. + run: | + ls -ld -- \ + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/.git/modules/gitdb" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "$(pwd)/.git/modules/gitdb/modules/smmap" \ + "${HOME:?HOME is not set}/.gitconfig" \ + 2>&1 || true + + - name: Show NTFS file ownership + # Authoritative NTFS Owner via Get-Acl, with no Cygwin SID-to-uid layer + # in between -- useful for confirming what the Cygwin view reports as + # "Administrators" is the BUILTIN\Administrators SID (S-1-5-32-544). + shell: pwsh + run: | + $paths = @( + "$pwd", + "$pwd\.git", + "$pwd\git\ext\gitdb", + "$pwd\git\ext\gitdb\.git", + "$pwd\.git\modules\gitdb", + "$pwd\git\ext\gitdb\gitdb\ext\smmap", + "$pwd\git\ext\gitdb\gitdb\ext\smmap\.git", + "$pwd\.git\modules\gitdb\modules\smmap", + "$env:USERPROFILE\.gitconfig" + ) + foreach ($p in $paths) { + if (Test-Path -LiteralPath $p) { + try { + $owner = (Get-Acl -LiteralPath $p).Owner + } catch { + $owner = "ERROR: $($_.Exception.Message)" + } + "{0,-44} {1}" -f $owner, $p + } else { + "(missing: $p)" + } + } + + - name: Show safe.directory entries + run: git config --global --get-all safe.directory + - name: Show version and platform information run: | uname -a diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index e32e946c8..79d786669 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,6 +1,10 @@ name: Lint -on: [push, pull_request, workflow_dispatch] +on: + push: + branches: [main] + pull_request: + workflow_dispatch: permissions: contents: read @@ -10,7 +14,7 @@ jobs: runs-on: ubuntu-slim steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - uses: actions/setup-python@v6 with: diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 874e18a8f..fe3e26236 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -3,7 +3,11 @@ name: Python package -on: [push, pull_request, workflow_dispatch] +on: + push: + branches: [main] + pull_request: + workflow_dispatch: permissions: contents: read @@ -50,7 +54,7 @@ jobs: shell: bash --noprofile --norc -exo pipefail {0} steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: fetch-depth: 0 @@ -87,6 +91,62 @@ jobs: run: | pip install '.[test]' + - name: Show POSIX file ownership + # Linux and macOS only. On Windows, Git Bash's `ls -ld` reports a + # uniform uid+gid for every path regardless of NTFS Owner (MSYS2's + # SID-to-uid mapping doesn't have Cygwin's fidelity), so it would + # not be informative here. The NTFS Owner check below covers Windows. + if: matrix.os-type != 'windows' + run: | + ls -ld -- \ + "$(pwd)" \ + "$(pwd)/.git" \ + "$(pwd)/git/ext/gitdb" \ + "$(pwd)/git/ext/gitdb/.git" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap" \ + "$(pwd)/git/ext/gitdb/gitdb/ext/smmap/.git" \ + "${HOME:?HOME is not set}/.gitconfig" \ + 2>&1 || true + + - name: Show NTFS file ownership + # Windows only. Reads NTFS Owner directly via Get-Acl, which is the + # authoritative view for Windows-side ownership questions; the POSIX + # view via Git Bash's MSYS2 layer is not a reliable proxy here. + if: matrix.os-type == 'windows' + shell: pwsh + run: | + $paths = @( + "$pwd", + "$pwd\.git", + "$pwd\git\ext\gitdb", + "$pwd\git\ext\gitdb\.git", + "$pwd\git\ext\gitdb\gitdb\ext\smmap", + "$pwd\git\ext\gitdb\gitdb\ext\smmap\.git", + "$env:USERPROFILE\.gitconfig" + ) + foreach ($p in $paths) { + if (Test-Path -LiteralPath $p) { + try { + $owner = (Get-Acl -LiteralPath $p).Owner + } catch { + $owner = "ERROR: $($_.Exception.Message)" + } + "{0,-44} {1}" -f $owner, $p + } else { + "(missing: $p)" + } + } + + - name: Show safe.directory entries + # `actions/checkout`'s safe.directory add is only durable for the + # checkout itself (it writes under a throwaway HOME override and + # then discards it), so by the time this step runs the runner + # user's `~/.gitconfig` has no entries -- and git accepts the + # workspace's ownership anyway: Git for Windows via its + # Admins-group exemption on the windows matrix; on Linux/macOS + # the workspace is owned by the test user. Expected: `(none)`. + run: git config --global --get-all safe.directory || echo "(none)" + - name: Show version and platform information run: | uname -a diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f3ab67035..a13e85d40 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,7 +7,7 @@ repos: exclude: ^test/fixtures/ - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.15.12 + rev: v0.15.20 hooks: - id: ruff-check args: ["--fix"] diff --git a/AUTHORS b/AUTHORS index 15333e1e5..95205f439 100644 --- a/AUTHORS +++ b/AUTHORS @@ -57,5 +57,6 @@ Contributors are: -Jonas Scharpf -Gordon Marx -Enji Cooper +-Harshita Yadav Portions derived from other open source works and are clearly marked. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8536d7f73..60e34a651 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,6 +9,37 @@ The following is a short step-by-step rundown of what one typically would do to - Feel free to add yourself to AUTHORS file. - Create a pull request. +## Quality expectations + +Contributions must be made with care and meet the quality bar of the surrounding code. +That means a change should not leave GitPython worse than it was before: it should be +readable, maintainable, tested where practical, documented and consistent with the +existing style and behavior. + +A contribution that works only narrowly but lowers the quality of the +codebase may be declined. The maintainers may not always be able to provide +detailed feedback. + +## AI-assisted contributions + +If AI edits files for you, disclose it in the pull request description and commit +metadata. Prefer making the agent identity part of the commit, for example by using +an AI author such as `$agent $version ` or a co-author via +a `Co-authored-by: ` trailer. + +Agents operating through a person's GitHub account must identify themselves. For +example, comments posted by an agent should say so directly with phrases like +`AI agent on behalf of : ...`. + +Fully AI-generated comments on pull requests or issues must also be disclosed. +Undisclosed AI-generated comments may lead to the pull request or issue being closed. + +AI-assisted proofreading or wording polish does not need disclosure, but it is still +courteous to mention it when the AI materially influenced the final text. + +Automated or "full-auto" AI contributions without a human responsible for reviewing +and standing behind the work may be closed. + ## Fuzzing Test Specific Documentation For details related to contributing to the fuzzing test suite and OSS-Fuzz integration, please diff --git a/VERSION b/VERSION index 0bc461141..2c5a9fd4a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.50 +3.1.52 diff --git a/doc/source/changes.rst b/doc/source/changes.rst index b5152b3c5..2bfe6dd17 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,6 +2,32 @@ Changelog ========= +3.1.52 +====== + +A security fix for +https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-rwj8-pgh3-r573. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.1.52 + +3.1.51 +====== + +This is primarily a security release. It prevents additional argument-injection +paths that could allow execution of arbitrary commands or writing to arbitrary +files through unsafe Git options (GHSA-956x-8gvw-wg5v), and closes bypasses of +the existing protections using abbreviated long options or joined short options +(GHSA-2f96-g7mh-g2hx). + +The release also improves support for relative worktree paths and diffs against +the empty tree, preserves stderr from failed diff processes, supports relative +configuration include paths, and includes assorted documentation, typing, test, +and CI improvements. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.1.51 + 3.1.50 ====== diff --git a/git/cmd.py b/git/cmd.py index 096900819..150c9d16f 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -649,6 +649,11 @@ class Git(metaclass=_GitMeta): re_unsafe_protocol = re.compile(r"(.+)::.+") + unsafe_git_ls_remote_options = [ + # This option allows arbitrary command execution in git-ls-remote. + "--upload-pack", + ] + def __getstate__(self) -> Dict[str, Any]: return slots_to_dict(self, exclude=self._excluded_) @@ -898,29 +903,34 @@ def is_cygwin(cls) -> bool: @overload @classmethod - def polish_url(cls, url: str, is_cygwin: Literal[False] = ...) -> str: ... + def polish_url(cls, url: str, is_cygwin: Literal[False] = ..., expand_vars: bool = ...) -> str: ... @overload @classmethod - def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> str: ... + def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None, expand_vars: bool = True) -> str: ... @classmethod - def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> PathLike: + def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None, expand_vars: bool = True) -> PathLike: """Remove any backslashes from URLs to be written in config files. Windows might create config files containing paths with backslashes, but git stops liking them as it will escape the backslashes. Hence we undo the escaping just to be sure. + + :param expand_vars: + Expand environment variables and an initial ``~``. Disable this for values + obtained from an untrusted source, such as remote URLs. """ if is_cygwin is None: is_cygwin = cls.is_cygwin() if is_cygwin: - url = cygpath(url) + url = cygpath(url, expand_vars=expand_vars) else: - url = os.path.expandvars(url) - if url.startswith("~"): - url = os.path.expanduser(url) + if expand_vars: + url = os.path.expandvars(url) + if url.startswith("~"): + url = os.path.expanduser(url) url = url.replace("\\\\", "\\").replace("\\", "/") return url @@ -961,17 +971,80 @@ def _canonicalize_option_name(cls, option: str) -> str: @classmethod def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) -> None: - """Check for unsafe options. - - Some options that are passed to ``git `` can be used to execute - arbitrary commands. These are blocked by default. + """Raise :class:`~git.exc.UnsafeOptionError` for blocked option spellings. + + In addition to exact matches, this rejects abbreviated long options accepted + by Git (for example, ``--upl`` for ``--upload-pack``) and unsafe short options + whose values are joined to the same token, including after clusterable flags + (for example, ``-uVALUE`` and ``-fuVALUE``). + + A list containing only bare names is treated as normalized keyword arguments, + so multi-character names such as ``upload_p`` are checked as long-option + abbreviations. If any item starts with ``-``, the list is treated as tokenized + command-line input: bare items can be option values and are not checked as + abbreviations. Thus ``["--origin", "upload"]`` is allowed. Single-dash options + use short-option parsing rather than broad prefix matching, preserving safe + attached values such as ``-oupstream`` and ``-bcurrent``. + + Some options passed to ``git `` can execute arbitrary commands and + are therefore blocked by default unless the caller explicitly allows them. """ # Options can be of the form `foo`, `--foo`, `--foo bar`, or `--foo=bar`. + # Git accepts any unambiguous prefix of a long option, so an abbreviated + # spelling such as `--upl` for `--upload-pack` must be rejected too. An + # option is unsafe if its canonical name is a prefix of any blocked + # option's canonical name. Only long options and multi-character kwargs + # can be abbreviations; single-character short options remain exact-match + # only. canonical_unsafe_options = {cls._canonicalize_option_name(option): option for option in unsafe_options} + unsafe_short_options = { + canonical: option + for canonical, option in canonical_unsafe_options.items() + if option.startswith("-") and not option.startswith("--") and len(canonical) == 1 + } + # These value-less Git flags can be clustered before another short option + # (for example, ``-fuVALUE``). Stop at any other character because it may + # begin an attached value, as ``o`` does in the safe option ``-oupstream``. + clusterable_short_options = frozenset("46flnqsv") + options_are_kwargs = all(not option.startswith("-") for option in options) for option in options: - unsafe_option = canonical_unsafe_options.get(cls._canonicalize_option_name(option)) + candidate = cls._canonicalize_option_name(option) + if not candidate: + continue + unsafe_option = canonical_unsafe_options.get(candidate) if unsafe_option is not None: raise UnsafeOptionError(f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it.") + option_token = option.split("=", 1)[0].split(None, 1)[0] + if option_token.startswith("-") and not option_token.startswith("--"): + for option_char in option_token[1:]: + unsafe_option = unsafe_short_options.get(option_char) + if unsafe_option is not None: + raise UnsafeOptionError( + f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it." + ) + if option_char not in clusterable_short_options: + break + if not (option.startswith("--") or (options_are_kwargs and len(candidate) > 1)): + continue + for canonical, unsafe_option in canonical_unsafe_options.items(): + if canonical.startswith(candidate): + raise UnsafeOptionError( + f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it." + ) + + @classmethod + def _option_candidates(cls, args: Sequence[Any] = (), kwargs: Optional[Mapping[str, Any]] = None) -> List[str]: + """Collect possible option spellings before command-line transformation.""" + options = [ + option for option in cls._unpack_args([arg for arg in args if arg is not None]) if option.startswith("-") + ] + if kwargs: + for key, value in kwargs.items(): + values = value if isinstance(value, (list, tuple)) else (value,) + if any(value is True or (value is not False and value is not None) for value in values): + key = str(key) + options.append(f"-{key}" if len(key) == 1 else f"--{dashify(key)}") + return options AutoInterrupt: TypeAlias = _AutoInterrupt @@ -1030,6 +1103,22 @@ def set_persistent_git_options(self, **kwargs: Any) -> None: self._persistent_git_options = self.transform_kwargs(split_single_char_options=True, **kwargs) + def ls_remote( + self, + *args: Any, + allow_unsafe_options: bool = False, + **kwargs: Any, + ) -> Union[str, bytes, Tuple[int, Union[str, bytes], str], "Git.AutoInterrupt"]: + """List references in a remote repository. + + :param allow_unsafe_options: + Allow unsafe options, like ``--upload-pack``. + """ + if not allow_unsafe_options: + candidate_options = self._option_candidates(args, kwargs) + Git.check_unsafe_options(options=candidate_options, unsafe_options=self.unsafe_git_ls_remote_options) + return self._call_process("ls_remote", *args, **kwargs) + @property def working_dir(self) -> Union[None, PathLike]: """:return: Git directory we are working on""" @@ -1131,9 +1220,28 @@ def execute( information (stdout). :param command: - The command argument list to execute. - It should be a sequence of program arguments, or a string. The - program to execute is the first item in the args sequence or string. + The command to execute. A sequence of program arguments is recommended. + A string is also accepted, but its meaning is strongly platform-dependent. + + By default, a shell is not used. On Unix-like systems, a string is the whole + program name (so ``"git log -n 1"`` raises :class:`GitCommandNotFound`). On + Windows, the program parses the arguments itself, so multi-word strings can + work but are not portable. + + Avoid ``shell=True`` (and :attr:`Git.USE_SHELL`): this runs the command in + a shell, which is generally unsafe. The shell interprets metacharacters + such as ``;``, ``|``, ``&``, ``$(...)``, ``$VAR``, ``%VAR%``, and ``^`` + (depending on the platform) as syntax. Any untrusted text in the command + can then execute arbitrary OS commands. See :attr:`Git.USE_SHELL`. + + Producing a sequence automatically by :func:`shlex.split` and passing it + as the command is far safer than ``shell=True``. But :func:`shlex.split` + parses POSIX shell syntax on all systems, and the result is still unsafe + for anything but *fixed, fully trusted* strings. Do not use it on strings + built by interpolating values: whitespace or quoting in an untrusted value + can still inject arguments. For input derived in any way from untrusted + data, build the argument sequence yourself, while ensuring each argument + is fully sanitized. :param istream: Standard input filehandle passed to :class:`subprocess.Popen`. @@ -1201,6 +1309,11 @@ def execute( needed (nor useful) to work around any known operating system specific issues. + On Unix-like systems, when migrating away from passing string commands with + ``shell=True``, :func:`shlex.split` may serve as a transitional step in rare + cases, with extreme care. (Drop ``shell=True`` and pass the resulting + sequence as the command.) See the `command` parameter above on the risks. + :param env: A dictionary of environment variables to be passed to :class:`subprocess.Popen`. @@ -1512,7 +1625,7 @@ def transform_kwargs(self, split_single_char_options: bool = True, **kwargs: Any return args @classmethod - def _unpack_args(cls, arg_list: Sequence[str]) -> List[str]: + def _unpack_args(cls, arg_list: Sequence[Any]) -> List[str]: outlist = [] if isinstance(arg_list, (list, tuple)): for arg in arg_list: @@ -1588,7 +1701,7 @@ def _call_process( turns into:: - git rev-list --max-count=10 --header=master + git rev-list --max-count=10 --header master :return: Same as :meth:`execute`. If no args are given, used :meth:`execute`'s diff --git a/git/config.py b/git/config.py index 82747eadd..64f501424 100644 --- a/git/config.py +++ b/git/config.py @@ -634,6 +634,8 @@ def read(self) -> None: # type: ignore[override] files_to_read = list(self._file_or_files) # END ensure we have a copy of the paths to handle + files_to_read = [osp.abspath(path) if isinstance(path, (str, os.PathLike)) else path for path in files_to_read] + seen = set(files_to_read) num_read_include_files = 0 while files_to_read: diff --git a/git/diff.py b/git/diff.py index 23cb5675e..5af53e556 100644 --- a/git/diff.py +++ b/git/diff.py @@ -3,7 +3,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ -__all__ = ["DiffConstants", "NULL_TREE", "INDEX", "Diffable", "DiffIndex", "Diff"] +__all__ = ["DiffConstants", "NULL_TREE", "NULL_TREE_SHA", "INDEX", "Diffable", "DiffIndex", "Diff"] import enum import re @@ -84,6 +84,9 @@ class DiffConstants(enum.Enum): :const:`git.NULL_TREE` and :const:`Diffable.NULL_TREE`. """ +NULL_TREE_SHA = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" +"""SHA of Git's canonical empty tree object.""" + INDEX: Literal[DiffConstants.INDEX] = DiffConstants.INDEX """Stand-in indicating you want to diff against the index. @@ -599,7 +602,14 @@ def _index_from_patch_format(cls, repo: "Repo", proc: Union["Popen", "Git.AutoIn # FIXME: Here SLURPING raw, need to re-phrase header-regexes linewise. text_list: List[bytes] = [] - handle_process_output(proc, text_list.append, None, finalize_process, decode_streams=False) + stderr_list: List[bytes] = [] + + def finalize_process_with_stderr(proc: Union["Popen", "Git.AutoInterrupt"]) -> None: + finalize_process(proc, stderr=b"".join(stderr_list)) + + handle_process_output( + proc, text_list.append, stderr_list.append, finalize_process_with_stderr, decode_streams=False + ) # For now, we have to bake the stream. text = b"".join(text_list) @@ -765,11 +775,16 @@ def _index_from_raw_format(cls, repo: "Repo", proc: "Popen") -> "DiffIndex[Diff] # :100644 100644 687099101... 37c5e30c8... M .gitignore index: "DiffIndex" = DiffIndex() + stderr_list: List[bytes] = [] + + def finalize_process_with_stderr(proc: Union["Popen", "Git.AutoInterrupt"]) -> None: + finalize_process(proc, stderr=b"".join(stderr_list)) + handle_process_output( proc, lambda byt: cls._handle_diff_line(byt, repo, index), - None, - finalize_process, + stderr_list.append, + finalize_process_with_stderr, decode_streams=False, ) diff --git a/git/index/base.py b/git/index/base.py index 2276343f2..f03b452dc 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -1480,12 +1480,11 @@ def reset( return self - # FIXME: This is documented to accept the same parameters as Diffable.diff, but this - # does not handle NULL_TREE for `other`. (The suppressed mypy error is about this.) def diff( self, - other: Union[ # type: ignore[override] + other: Union[ Literal[git_diff.DiffConstants.INDEX], + Literal[git_diff.DiffConstants.NULL_TREE], "Tree", "Commit", str, @@ -1512,6 +1511,44 @@ def diff( if other is self.INDEX: return git_diff.DiffIndex() + if other == git_diff.NULL_TREE or other == git_diff.NULL_TREE_SHA: + args: List[Union[PathLike, str]] = [ + "--cached", + git_diff.NULL_TREE_SHA, + "--abbrev=40", + "--full-index", + ] + + if not any(x in kwargs for x in ("find_renames", "no_renames", "M")): + args.append("-M") + + if create_patch: + args.append("-p") + args.append("--no-ext-diff") + else: + args.append("--raw") + args.append("-z") + + args.append("--no-color") + + if paths is not None and not isinstance(paths, (tuple, list)): + paths = [paths] + + if paths: + args.append("--") + args.extend(paths) + + kwargs["as_process"] = True + proc = self.repo.git.diff(*args, **kwargs) + + diff_method = ( + git_diff.Diff._index_from_patch_format if create_patch else git_diff.Diff._index_from_raw_format + ) + index = diff_method(self.repo, proc) + + proc.wait() + return index + # Index against anything but None is a reverse diff with the respective item. # Handle existing -R flags properly. # Transform strings to the object so that we can call diff on it. diff --git a/git/objects/commit.py b/git/objects/commit.py index da7677ee0..1d8e8f071 100644 --- a/git/objects/commit.py +++ b/git/objects/commit.py @@ -86,6 +86,12 @@ class Commit(base.Object, TraversableIterableObj, Diffable, Serializable): # INVARIANTS default_encoding = "UTF-8" + # Options to :manpage:`git-rev-list(1)` that can overwrite files. + unsafe_git_rev_options = [ + "--output", + "-o", + ] + type: Literal["commit"] = "commit" __slots__ = ( @@ -302,6 +308,7 @@ def iter_items( repo: "Repo", rev: Union[str, "Commit", "SymbolicReference"], paths: Union[PathLike, Sequence[PathLike]] = "", + allow_unsafe_options: bool = False, **kwargs: Any, ) -> Iterator["Commit"]: R"""Find all commits matching the given criteria. @@ -330,6 +337,11 @@ def iter_items( raise ValueError("--pretty cannot be used as parsing expects single sha's only") # END handle pretty + if not allow_unsafe_options: + Git.check_unsafe_options( + options=Git._option_candidates([rev], kwargs), unsafe_options=cls.unsafe_git_rev_options + ) + # Use -- in all cases, to prevent possibility of ambiguous arguments. # See https://github.com/gitpython-developers/GitPython/issues/264. @@ -374,6 +386,11 @@ def stats(self) -> Stats: """Create a git stat from changes between this commit and its first parent or from all changes done if this is the very first commit. + :note: + If this commit is at the boundary of a shallow clone, this will + raise :exc:`~git.exc.GitCommandError`, since the parent object + was never fetched and only exists as a reference on this commit. + :return: :class:`Stats` """ diff --git a/git/remote.py b/git/remote.py index 20e42b412..0c3dbfe15 100644 --- a/git/remote.py +++ b/git/remote.py @@ -517,6 +517,9 @@ def iter_items(cls, repo: "Repo", *args: Any, **kwargs: Any) -> NoReturn: # -> raise NotImplementedError +Progress = Union[RemoteProgress, "UpdateProgress", Callable[..., RemoteProgress], None] + + class Remote(LazyMixin, IterableObj): """Provides easy read and write access to a git remote. @@ -872,7 +875,7 @@ def update(self, **kwargs: Any) -> "Remote": def _get_fetch_info_from_stderr( self, proc: "Git.AutoInterrupt", - progress: Union[Callable[..., Any], RemoteProgress, None], + progress: Progress, kill_after_timeout: Union[None, float] = None, ) -> IterableList["FetchInfo"]: progress = to_progress_instance(progress) @@ -1000,7 +1003,7 @@ def _assert_refspec(self) -> None: def fetch( self, refspec: Union[str, List[str], None] = None, - progress: Union[RemoteProgress, None, "UpdateProgress"] = None, + progress: Progress = None, verbose: bool = True, kill_after_timeout: Union[None, float] = None, allow_unsafe_protocols: bool = False, @@ -1068,7 +1071,10 @@ def fetch( Git.check_unsafe_protocols(ref) if not allow_unsafe_options: - Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_fetch_options) + Git.check_unsafe_options( + options=Git._option_candidates([], kwargs), + unsafe_options=self.unsafe_git_fetch_options, + ) proc = self.repo.git.fetch( "--", self, *args, as_process=True, with_stdout=False, universal_newlines=True, v=verbose, **kwargs @@ -1081,7 +1087,7 @@ def fetch( def pull( self, refspec: Union[str, List[str], None] = None, - progress: Union[RemoteProgress, "UpdateProgress", None] = None, + progress: Progress = None, kill_after_timeout: Union[None, float] = None, allow_unsafe_protocols: bool = False, allow_unsafe_options: bool = False, @@ -1122,7 +1128,10 @@ def pull( Git.check_unsafe_protocols(ref) if not allow_unsafe_options: - Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_pull_options) + Git.check_unsafe_options( + options=Git._option_candidates([], kwargs), + unsafe_options=self.unsafe_git_pull_options, + ) proc = self.repo.git.pull( "--", self, refspec, with_stdout=False, as_process=True, universal_newlines=True, v=True, **kwargs @@ -1135,7 +1144,7 @@ def pull( def push( self, refspec: Union[str, List[str], None] = None, - progress: Union[RemoteProgress, "UpdateProgress", Callable[..., RemoteProgress], None] = None, + progress: Progress = None, kill_after_timeout: Union[None, float] = None, allow_unsafe_protocols: bool = False, allow_unsafe_options: bool = False, @@ -1195,7 +1204,10 @@ def push( Git.check_unsafe_protocols(ref) if not allow_unsafe_options: - Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=self.unsafe_git_push_options) + Git.check_unsafe_options( + options=Git._option_candidates([], kwargs), + unsafe_options=self.unsafe_git_push_options, + ) proc = self.repo.git.push( "--", diff --git a/git/repo/base.py b/git/repo/base.py index 7579e326f..e478396b2 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -161,6 +161,20 @@ class Repo: https://git-scm.com/docs/git-clone#Documentation/git-clone.txt---configltkeygtltvaluegt """ + unsafe_git_archive_options = [ + # Allows arbitrary command execution through the remote git-upload-archive command. + "--exec", + # Writes output to a caller-controlled filesystem path. + "--output", + "-o", + ] + + unsafe_git_revision_options = [ + # This option allows output to be written to arbitrary files before revision parsing. + "--output", + "-o", + ] + # Invariants config_level: ConfigLevels_Tup = ("system", "user", "global", "repository") """Represents the configuration level of a configuration file.""" @@ -295,7 +309,8 @@ def __init__( sm_gitpath = find_worktree_git_dir(dotgit) if sm_gitpath is not None: - git_dir = expand_path(sm_gitpath, expand_vars) + # worktrees can use relative paths as of Git 2.48, so we join to curpath + git_dir = osp.normpath(osp.join(curpath, sm_gitpath)) self._working_tree_dir = curpath break @@ -774,6 +789,7 @@ def iter_commits( self, rev: Union[str, Commit, "SymbolicReference", None] = None, paths: Union[PathLike, Sequence[PathLike]] = "", + allow_unsafe_options: bool = False, **kwargs: Any, ) -> Iterator[Commit]: """An iterator of :class:`~git.objects.commit.Commit` objects representing the @@ -791,6 +807,9 @@ def iter_commits( Arguments to be passed to :manpage:`git-rev-list(1)`. Common ones are ``max_count`` and ``skip``. + :param allow_unsafe_options: + Allow unsafe options in the revision argument, like ``--output``. + :note: To receive only commits between two named revisions, use the ``"revA...revB"`` revision specifier. @@ -801,7 +820,18 @@ def iter_commits( if rev is None: rev = self.head.commit - return Commit.iter_items(self, rev, paths, **kwargs) + if not allow_unsafe_options: + Git.check_unsafe_options( + options=Git._option_candidates([rev], kwargs), unsafe_options=self.unsafe_git_revision_options + ) + + return Commit.iter_items( + self, + rev, + paths, + allow_unsafe_options=allow_unsafe_options, + **kwargs, + ) def merge_base(self, *rev: TBD, **kwargs: Any) -> List[Commit]: R"""Find the closest common ancestor for the given revision @@ -1078,7 +1108,9 @@ def active_branch(self) -> Head: ) return active_branch - def blame_incremental(self, rev: str | HEAD | None, file: str, **kwargs: Any) -> Iterator["BlameEntry"]: + def blame_incremental( + self, rev: str | HEAD | None, file: str, allow_unsafe_options: bool = False, **kwargs: Any + ) -> Iterator["BlameEntry"]: """Iterator for blame information for the given file at the given revision. Unlike :meth:`blame`, this does not return the actual file's contents, only a @@ -1089,6 +1121,9 @@ def blame_incremental(self, rev: str | HEAD | None, file: str, **kwargs: Any) -> uncommitted changes. Otherwise, anything successfully parsed by :manpage:`git-rev-parse(1)` is a valid option. + :param allow_unsafe_options: + Allow unsafe options in revision argument, like ``--output``. + :return: Lazy iterator of :class:`BlameEntry` tuples, where the commit indicates the commit to blame for the line, and range indicates a span of line numbers in @@ -1097,6 +1132,10 @@ def blame_incremental(self, rev: str | HEAD | None, file: str, **kwargs: Any) -> If you combine all line number ranges outputted by this command, you should get a continuous range spanning all line numbers in the file. """ + if not allow_unsafe_options: + Git.check_unsafe_options( + options=Git._option_candidates([rev], kwargs), unsafe_options=self.unsafe_git_revision_options + ) data: bytes = self.git.blame(rev, "--", file, p=True, incremental=True, stdout_as_string=False, **kwargs) commits: Dict[bytes, Commit] = {} @@ -1175,7 +1214,8 @@ def blame( rev: Union[str, HEAD, None], file: str, incremental: bool = False, - rev_opts: Optional[List[str]] = None, + rev_opts: Optional[Sequence[str]] = None, + allow_unsafe_options: bool = False, **kwargs: Any, ) -> List[List[Commit | List[str | bytes] | None]] | Iterator[BlameEntry] | None: """The blame information for the given file at the given revision. @@ -1185,6 +1225,9 @@ def blame( uncommitted changes. Otherwise, anything successfully parsed by :manpage:`git-rev-parse(1)` is a valid option. + :param allow_unsafe_options: + Allow unsafe options in revision argument, like ``--output``. + :return: list: [git.Commit, list: []] @@ -1194,9 +1237,14 @@ def blame( appearance. """ if incremental: - return self.blame_incremental(rev, file, **kwargs) - rev_opts = rev_opts or [] - data: bytes = self.git.blame(rev, *rev_opts, "--", file, p=True, stdout_as_string=False, **kwargs) + return self.blame_incremental(rev, file, allow_unsafe_options=allow_unsafe_options, **kwargs) + rev_opts_list = list(rev_opts or []) + if not allow_unsafe_options: + Git.check_unsafe_options( + options=Git._option_candidates([rev, rev_opts_list], kwargs), + unsafe_options=self.unsafe_git_revision_options, + ) + data: bytes = self.git.blame(rev, *rev_opts_list, "--", file, p=True, stdout_as_string=False, **kwargs) commits: Dict[str, Commit] = {} blames: List[List[Commit | List[str | bytes] | None]] = [] @@ -1404,17 +1452,21 @@ def _clone( if multi_options: multi = shlex.split(" ".join(multi_options)) + clone_url = Git.polish_url(url, expand_vars=False) if not allow_unsafe_protocols: - Git.check_unsafe_protocols(url) + Git.check_unsafe_protocols(clone_url) if not allow_unsafe_options: - Git.check_unsafe_options(options=list(kwargs.keys()), unsafe_options=cls.unsafe_git_clone_options) + Git.check_unsafe_options( + options=Git._option_candidates([], kwargs), + unsafe_options=cls.unsafe_git_clone_options, + ) if not allow_unsafe_options and multi: Git.check_unsafe_options(options=multi, unsafe_options=cls.unsafe_git_clone_options) proc = git.clone( multi, "--", - Git.polish_url(url), + clone_url, clone_path, with_extended_output=True, as_process=True, @@ -1454,7 +1506,7 @@ def _clone( # escape the backslashes. Hence we undo the escaping just to be sure. if repo.remotes: with repo.remotes[0].config_writer as writer: - writer.set_value("url", Git.polish_url(repo.remotes[0].url)) + writer.set_value("url", Git.polish_url(repo.remotes[0].url, expand_vars=False)) # END handle remote repo return repo @@ -1582,6 +1634,8 @@ def archive( ostream: Union[TextIO, BinaryIO], treeish: Optional[str] = None, prefix: Optional[str] = None, + allow_unsafe_options: bool = False, + allow_unsafe_protocols: bool = False, **kwargs: Any, ) -> Repo: """Archive the tree at the given revision. @@ -1604,6 +1658,12 @@ def archive( repository-relative path to a directory or file to place into the archive, or a list or tuple of multiple paths. + :param allow_unsafe_options: + Allow unsafe options, like ``--exec`` or ``--output``. + + :param allow_unsafe_protocols: + Allow unsafe protocols to be used in ``remote``, like ``ext``. + :raise git.exc.GitCommandError: If something went wrong. @@ -1614,6 +1674,14 @@ def archive( treeish = self.head.commit if prefix and "prefix" not in kwargs: kwargs["prefix"] = prefix + remote = kwargs.get("remote") + if not allow_unsafe_protocols and remote is not None: + Git.check_unsafe_protocols(str(remote)) + if not allow_unsafe_options: + Git.check_unsafe_options( + options=Git._option_candidates([], kwargs), + unsafe_options=self.unsafe_git_archive_options, + ) kwargs["output_stream"] = ostream path = kwargs.pop("path", []) path = cast(Union[PathLike, List[PathLike], Tuple[PathLike, ...]], path) diff --git a/git/util.py b/git/util.py index 712fabe85..11c1a0b15 100644 --- a/git/util.py +++ b/git/util.py @@ -382,13 +382,13 @@ def is_exec(fpath: str) -> bool: return progs -def _cygexpath(drive: Optional[str], path: str) -> str: +def _cygexpath(drive: Optional[str], path: str, expand_vars: bool = True) -> str: if osp.isabs(path) and not drive: # Invoked from `cygpath()` directly with `D:Apps\123`? # It's an error, leave it alone just slashes) p = path # convert to str if AnyPath given else: - p = path and osp.normpath(osp.expandvars(osp.expanduser(path))) + p = path and osp.normpath(osp.expandvars(osp.expanduser(path)) if expand_vars else path) if osp.isabs(p): if drive: # Confusing, maybe a remote system should expand vars. @@ -416,7 +416,7 @@ def _cygexpath(drive: Optional[str], path: str) -> str: ) -def cygpath(path: str) -> str: +def cygpath(path: str, expand_vars: bool = True) -> str: """Use :meth:`git.cmd.Git.polish_url` instead, that works on any environment.""" path = os.fspath(path) # Ensure is str and not AnyPath. # Fix to use Paths when 3.5 dropped. Or to be just str if only for URLs? @@ -424,12 +424,15 @@ def cygpath(path: str) -> str: for regex, parser, recurse in _cygpath_parsers: match = regex.match(path) if match: - path = parser(*match.groups()) + if parser is _cygexpath: + path = parser(*match.groups(), expand_vars=expand_vars) + else: + path = parser(*match.groups()) if recurse: - path = cygpath(path) + path = cygpath(path, expand_vars=expand_vars) break else: - path = _cygexpath(None, path) + path = _cygexpath(None, path, expand_vars=expand_vars) return path diff --git a/init-tests-after-clone.sh b/init-tests-after-clone.sh index bfada01b0..a88f983fc 100755 --- a/init-tests-after-clone.sh +++ b/init-tests-after-clone.sh @@ -40,6 +40,11 @@ fi git tag __testing_point__ # The tests need a branch called master. +# +# If master is locally absent but more than one remote has it, checkout fails +# by default even if all remotes agree, and we fall back to creating it at +# HEAD. The reflog we populate below then traces HEAD's history rather than +# a remote master's, but master is reset to __testing_point__ either way. git checkout master -- || git checkout -b master # The tests need a reflog history on the master branch. diff --git a/test/lib/helper.py b/test/lib/helper.py index 6a8b714e6..1c110e103 100644 --- a/test/lib/helper.py +++ b/test/lib/helper.py @@ -18,6 +18,7 @@ "skipIf", "GIT_REPO", "GIT_DAEMON_PORT", + "xfail_if_raises", ] import contextlib @@ -35,8 +36,10 @@ import time import unittest import venv +from typing import Union, Type, Tuple import gitdb +import pytest from git.util import rmtree, cwd @@ -87,7 +90,7 @@ def __init__(self, input_string): self.stdout = io.BytesIO(input_string) self.stderr = io.BytesIO() - def wait(self): + def wait(self, stderr=None): return 0 poll = wait @@ -465,3 +468,27 @@ def _executable(self, basename): if osp.isfile(path) or osp.islink(path): return path raise RuntimeError(f"no regular file or symlink {path!r}") + + +@contextlib.contextmanager +def xfail_if_raises( + condition: bool, + *, + raises: Union[Type[BaseException], Tuple[Type[BaseException], ...]], + reason: str = "", + strict: bool = False, +): + """Approximates the behavior of @pytest.mark.xfail(..., raises=...) as a context + manager that can be used within a test, such as when the condition is complex or has + side effects + + One difference is it will not report XPASS if the test passes, but setting `strict` + simulates it by raising an exception""" + try: + yield + except raises: + if condition: + pytest.xfail(reason) + raise + if strict and condition: + pytest.fail("[XPASS(strict)] " + reason) diff --git a/test/test_clone.py b/test/test_clone.py index 653d50aa3..5fd59b3a3 100644 --- a/test/test_clone.py +++ b/test/test_clone.py @@ -7,8 +7,9 @@ import sys import tempfile from unittest import skip +from unittest import mock -from git import GitCommandError, Repo +from git import Git, GitCommandError, Repo from git.exc import UnsafeOptionError, UnsafeProtocolError from test.lib import TestBase, with_rw_directory, with_rw_repo, PathLikeMock @@ -117,9 +118,16 @@ def test_clone_unsafe_options(self, rw_repo): tmp_file = tmp_dir / "pwn" unsafe_options = [ f"--upload-pack='touch {tmp_file}'", + f"--upl='touch {tmp_file}'", f"-u 'touch {tmp_file}'", + f"-utouch {tmp_file}; false", + f"-futouch${{IFS}}{tmp_file}; false", + f"-qutouch${{IFS}}{tmp_file}; false", "--config=protocol.ext.allow=always", + "--conf=protocol.ext.allow=always", "-c protocol.ext.allow=always", + "-cprotocol.ext.allow=always", + "-vcprotocol.ext.allow=always", ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): @@ -129,8 +137,10 @@ def test_clone_unsafe_options(self, rw_repo): unsafe_options = [ {"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}, + {"upl": f"touch {tmp_file}"}, {"u": f"touch {tmp_file}"}, {"config": "protocol.ext.allow=always"}, + {"conf": "protocol.ext.allow=always"}, {"c": "protocol.ext.allow=always"}, ] for unsafe_option in unsafe_options: @@ -138,6 +148,31 @@ def test_clone_unsafe_options(self, rw_repo): rw_repo.clone(tmp_dir, **unsafe_option) assert not tmp_file.exists() + @with_rw_repo("HEAD") + def test_clone_unsafe_options_abbreviated(self, rw_repo): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = pathlib.Path(tdir) + tmp_file = tmp_dir / "pwn" + unsafe_options = [ + f"--upl='touch {tmp_file}'", + f"--upload-pac='touch {tmp_file}'", + "--conf=protocol.ext.allow=always", + ] + for unsafe_option in unsafe_options: + with self.assertRaises(UnsafeOptionError): + rw_repo.clone(tmp_dir, multi_options=[unsafe_option]) + assert not tmp_file.exists() + + unsafe_kwargs = [ + {"upl": f"touch {tmp_file}"}, + {"upload_pac": f"touch {tmp_file}"}, + {"conf": "protocol.ext.allow=always"}, + ] + for unsafe_option in unsafe_kwargs: + with self.assertRaises(UnsafeOptionError): + rw_repo.clone(tmp_dir, **unsafe_option) + assert not tmp_file.exists() + @with_rw_repo("HEAD") def test_clone_unsafe_options_are_checked_after_splitting_multi_options(self, rw_repo): with tempfile.TemporaryDirectory() as tdir: @@ -191,7 +226,9 @@ def test_clone_safe_options(self, rw_repo): options = [ "--depth=1", "--single-branch", + "--origin upload", "-q", + "-oupstream", ] for option in options: destination = tmp_dir / option @@ -207,8 +244,13 @@ def test_clone_from_unsafe_options(self, rw_repo): unsafe_options = [ f"--upload-pack='touch {tmp_file}'", f"-u 'touch {tmp_file}'", + f"-utouch {tmp_file}; false", + f"-futouch${{IFS}}{tmp_file}; false", + f"-qutouch${{IFS}}{tmp_file}; false", "--config=protocol.ext.allow=always", "-c protocol.ext.allow=always", + "-cprotocol.ext.allow=always", + "-vcprotocol.ext.allow=always", ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): @@ -305,6 +347,44 @@ def test_clone_from_unsafe_protocol(self): Repo.clone_from(url, tmp_dir / "repo") assert not tmp_file.exists() + def test_clone_from_does_not_expand_environment_variables_in_url(self): + urls = [ + "https://example.com/$GITPYTHON_TEST_SECRET/repo.git", + "https://example.com/${GITPYTHON_TEST_SECRET}/repo.git", + "https://example.com/%GITPYTHON_TEST_SECRET%/repo.git", + ] + with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}): + for url in urls: + with mock.patch.object(Git, "_call_process", side_effect=RuntimeError) as call_process: + with self.assertRaises(RuntimeError): + Repo.clone_from(url, "unused") + + assert call_process.call_args[0][3] == url + + @with_rw_directory + def test_clone_from_does_not_expand_environment_variables_in_stored_url(self, rw_dir): + url = pathlib.Path(rw_dir) / "$GITPYTHON_TEST_SECRET" / "source" + Git().init(url) + + with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}): + cloned = Repo.clone_from(url, pathlib.Path(rw_dir) / "clone") + + assert cloned.remotes.origin.url == Git.polish_url(str(url), expand_vars=False) + + def test_clone_from_checks_polished_url_for_unsafe_protocol(self): + with mock.patch.object(Git, "polish_url", return_value="ext::command"): + with mock.patch.object(Git, "_call_process") as call_process: + with self.assertRaises(UnsafeProtocolError): + Repo.clone_from("$GITPYTHON_TEST_URL", "unused") + + call_process.assert_not_called() + + def test_polish_url_does_not_expand_environment_variables_for_cygwin(self): + urls = ["$GITPYTHON_TEST_SECRET/repo", "user@example.com:$GITPYTHON_TEST_SECRET/repo"] + with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}): + for url in urls: + assert Git.polish_url(url, is_cygwin=True, expand_vars=False) == url + def test_clone_from_unsafe_protocol_allowed(self): with tempfile.TemporaryDirectory() as tdir: tmp_dir = pathlib.Path(tdir) diff --git a/test/test_commit.py b/test/test_commit.py index b56ad3a18..74b7078f5 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -6,6 +6,7 @@ import copy from datetime import datetime from io import BytesIO +import tempfile import os.path as osp import re import sys @@ -15,6 +16,7 @@ from gitdb import IStream from git import Actor, Commit, Repo +from git.exc import UnsafeOptionError from git.objects.util import tzoffset, utc from git.repo.fun import touch @@ -288,6 +290,17 @@ def test_iter_items(self): # pretty not allowed. self.assertRaises(ValueError, Commit.iter_items, self.rorepo, "master", pretty="raw") + def test_iter_items_rejects_unsafe_revision(self): + with tempfile.TemporaryDirectory() as tdir: + marker = osp.join(tdir, "pwn") + self.assertRaises(UnsafeOptionError, Commit.iter_items, self.rorepo, f"--output={marker}") + + def test_iter_items_rejects_unsafe_options(self): + with tempfile.TemporaryDirectory() as tdir: + marker = osp.join(tdir, "pwn") + with self.assertRaises(UnsafeOptionError): + list(Commit.iter_items(self.rorepo, "HEAD", output=marker)) + def test_rev_list_bisect_all(self): """ 'git rev-list --bisect-all' returns additional information diff --git a/test/test_config.py b/test/test_config.py index 3ddaf0a4b..c4e39db48 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -310,6 +310,23 @@ def check_test_value(cr, value): with GitConfigParser(fpa, read_only=True) as cr: check_test_value(cr, tv) + @with_rw_directory + def test_config_relative_path_include(self, rw_dir): + included_path = osp.join(rw_dir, "included") + with GitConfigParser(included_path, read_only=False) as cw: + cw.set_value("included", "value", "included") + + config_path = osp.join(rw_dir, "config") + with GitConfigParser(config_path, read_only=False) as cw: + cw.set_value("include", "path", "included") + + if osp.splitdrive(config_path)[0] != osp.splitdrive(os.getcwd())[0]: + pytest.skip("The temporary directory and checkout are on different drives") + + relative_config_path = osp.relpath(config_path) + with GitConfigParser(relative_config_path, read_only=True) as cr: + assert cr.get_value("included", "value") == "included" + @with_rw_directory def test_multiple_include_paths_with_same_key(self, rw_dir): """Test that multiple 'path' entries under [include] are all respected. diff --git a/test/test_docs.py b/test/test_docs.py index cc0bbf26a..c3426a807 100644 --- a/test/test_docs.py +++ b/test/test_docs.py @@ -6,9 +6,6 @@ import gc import os import os.path -import sys - -import pytest from test.lib import TestBase from test.lib.helper import with_rw_directory @@ -478,11 +475,6 @@ def test_references_and_objects(self, rw_dir): repo.git.clear_cache() - @pytest.mark.xfail( - sys.platform == "cygwin", - reason="Cygwin GitPython can't find SHA for submodule", - raises=ValueError, - ) def test_submodules(self): # [1-test_submodules] repo = self.rorepo diff --git a/test/test_fixture_health.py b/test/test_fixture_health.py new file mode 100644 index 000000000..b18d5e8f9 --- /dev/null +++ b/test/test_fixture_health.py @@ -0,0 +1,131 @@ +# This module is part of GitPython and is released under the +# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ + +"""Verify that fixture directories are usable by git. + +If a fixture directory is missing, isn't an initialized git repository, +or is rejected by git for "dubious ownership", dependent tests +elsewhere in the suite fail in opaque ways. The checks here name the +preconditions directly so a misconfigured environment is recognizable +from the test output rather than from a cascade of unrelated-seeming +failures. + +These tests do not exercise GitPython's production code. They verify +the conditions under which production code is exercised are valid. +""" + +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent + +# Directories git must trust for the test suite to operate normally. The +# current set is the GitPython working tree plus the working trees of its +# gitdb submodule and the smmap submodule nested inside gitdb. New entries +# should be added here whenever the test suite gains a dependency on git +# accepting another directory. +FIXTURE_DIRS = [ + pytest.param(REPO_ROOT, id="repo_root"), + pytest.param(REPO_ROOT / "git" / "ext" / "gitdb", id="gitdb"), + pytest.param( + REPO_ROOT / "git" / "ext" / "gitdb" / "gitdb" / "ext" / "smmap", + id="smmap", + ), +] + +# Submodule working trees that must be present and initialized for the +# test suite to operate normally: gitdb at `git/ext/gitdb`, and smmap +# nested inside gitdb at `git/ext/gitdb/gitdb/ext/smmap`. The paths +# below are anchored at REPO_ROOT (the GitPython source tree), not at +# any rorepo redirection target. +SUBMODULE_DIRS = [ + pytest.param(REPO_ROOT / "git" / "ext" / "gitdb", id="gitdb"), + pytest.param( + REPO_ROOT / "git" / "ext" / "gitdb" / "gitdb" / "ext" / "smmap", + id="smmap", + ), +] + + +@pytest.mark.parametrize("fixture_dir", FIXTURE_DIRS) +def test_fixture_dir_is_trusted_by_git(fixture_dir: Path) -> None: + """git accepts ``fixture_dir`` as its own repository owned by a trusted user. + + Run ``git -C rev-parse --show-toplevel`` and assert it + succeeds and reports ``fixture_dir`` itself as the toplevel. Failure + typically means the directory's on-disk ownership doesn't match the + running user and the CI workflow's ``safe.directory`` list is missing + an entry that would override the check. + """ + if not fixture_dir.exists(): + pytest.skip(f"{fixture_dir} not present (run `git submodule update --init --recursive` from the repo root)") + if not (fixture_dir / ".git").exists(): + pytest.skip( + f"{fixture_dir} has no .git marker " + "(submodule not initialized; run " + "`git submodule update --init --recursive` from the repo root)" + ) + try: + result = subprocess.run( + ["git", "-C", str(fixture_dir), "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + check=False, + ) + except FileNotFoundError: + pytest.skip("git is not installed or not on PATH") + assert result.returncode == 0, ( + f"git refuses to operate in {fixture_dir}.\n" + f"stderr: {result.stderr.strip()}\n" + "The directory's owner doesn't match the running user and no " + "`safe.directory` entry overrides the check. On CI, the " + "workflow's `safe.directory` list typically needs an entry for " + "this path. Locally, this is unexpected and usually indicates " + "an ownership problem worth investigating." + ) + reported = Path(result.stdout.strip()) + assert reported.samefile(fixture_dir), ( + f"git reports the toplevel as {reported}, " + f"not as {fixture_dir} itself. " + "This usually means the directory is not an initialized git " + "repository (its `.git` marker may be stale or pointing elsewhere)." + ) + + +@pytest.mark.parametrize("submodule_dir", SUBMODULE_DIRS) +def test_required_submodule_is_initialized(submodule_dir: Path) -> None: + """The submodule's working tree is present and initialized. + + Failure means the source tree is a git clone but the submodule's + working tree hasn't been populated. Skipped when the source tree + itself isn't a git clone (e.g. an extracted release tarball), since + ``git submodule update`` cannot operate there; setups that handle + submodules in a separately-prepared tree (via + ``GIT_PYTHON_TEST_GIT_REPO_BASE``) are exempted from this check. + """ + if not (REPO_ROOT / ".git").exists(): + pytest.skip( + "Source tree is not a git clone (no .git in REPO_ROOT); submodules " + "cannot be initialized via `git submodule update` here. Setups " + "that prepare submodules in a separately-pointed tree (via " + "GIT_PYTHON_TEST_GIT_REPO_BASE) are exempted from this check." + ) + # The assertion messages below recommend `git submodule update --init + # --recursive` rather than `init-tests-after-clone.sh`, even though the + # latter is the documented entry point for first-time test setup. Two + # reasons: the script performs `git reset --hard` operations that can + # destroy local work, and #1713 showed the script itself can carry + # submodule-init regressions, in which case recommending it would lead + # developers in a circle. The direct git command is a safe minimal fix + # for this test's specific failure mode and bypasses any such regression. + assert submodule_dir.is_dir(), ( + f"Submodule working tree missing: {submodule_dir}.\n" + "Run `git submodule update --init --recursive` from the repo root." + ) + assert (submodule_dir / ".git").exists(), ( + f"Submodule directory exists but has no .git marker: {submodule_dir}.\n" + "The submodule hasn't been initialized. " + "Run `git submodule update --init --recursive` from the repo root." + ) diff --git a/test/test_git.py b/test/test_git.py index 24b60af9d..e95992ba8 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -164,12 +164,56 @@ def test_check_unsafe_options_normalizes_kwargs(self): (["c"], ["-c"]), (["--upload-pack=/tmp/helper"], ["--upload-pack"]), (["--config core.filemode=false"], ["--config"]), + (["--upl=/tmp/helper"], ["--upload-pack"]), + (["conf"], ["--config"]), + (["--out=/tmp/output"], ["--output"]), ] for options, unsafe_options in cases: with self.assertRaises(UnsafeOptionError): Git.check_unsafe_options(options=options, unsafe_options=unsafe_options) + def test_check_unsafe_options_does_not_treat_short_options_as_abbreviations(self): + unsafe_options = ["--upload-pack"] + + Git.check_unsafe_options(options=["-u", "u", "-upl"], unsafe_options=unsafe_options) + with self.assertRaises(UnsafeOptionError): + Git.check_unsafe_options(options=["--u"], unsafe_options=unsafe_options) + + def test_check_unsafe_options_does_not_treat_option_values_as_abbreviations(self): + Git.check_unsafe_options(options=["--branch", "conf"], unsafe_options=["--config"]) + + def test_check_unsafe_options_rejects_joined_unsafe_short_options(self): + cases = [ + (["-utouch /tmp/pwn"], ["-u"]), + (["-futouch /tmp/pwn"], ["-u"]), + (["-qutouch${IFS}/tmp/pwn"], ["-u"]), + (["-cprotocol.ext.allow=always"], ["-c"]), + (["-vcprotocol.ext.allow=always"], ["-c"]), + ] + + for options, unsafe_options in cases: + with self.assertRaises(UnsafeOptionError): + Git.check_unsafe_options(options=options, unsafe_options=unsafe_options) + + def test_check_unsafe_options_allows_attached_safe_short_option_values(self): + unsafe_options = ["--upload-pack", "-u", "--config", "-c"] + + Git.check_unsafe_options(options=["-oupstream", "-bcurrent"], unsafe_options=unsafe_options) + + def test_option_candidates_ignore_untransformed_kwargs(self): + options = Git._option_candidates( + kwargs={ + "output": None, + "upload_pack": False, + "exec": [], + "config": [None, False], + "max_count": 1, + } + ) + + self.assertEqual(options, ["--max-count"]) + _shell_cases = ( # value_in_call, value_from_class, expected_popen_arg (None, False, False), diff --git a/test/test_index.py b/test/test_index.py index f8280450a..3be750dbb 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -23,7 +23,8 @@ import ddt import pytest -from git import BlobFilter, Diff, Git, IndexFile, Object, Repo, Tree +from git import BlobFilter, Diff, Git, IndexFile, NULL_TREE, Object, Repo, Tree +from git.diff import NULL_TREE_SHA from git.exc import ( CheckoutError, GitCommandError, @@ -38,6 +39,7 @@ from git.util import Actor, cwd, hex_to_bin, rmtree from test.lib import TestBase, VirtualEnvironment, fixture, fixture_path, with_rw_directory, with_rw_repo, PathLikeMock +from test.lib.helper import xfail_if_raises HOOKS_SHEBANG = "#!/usr/bin/env sh\n" @@ -554,6 +556,39 @@ def test_index_file_diffing(self, rw_repo): rval = index.checkout("lib") assert len(list(rval)) > 1 + @with_rw_directory + def test_index_file_diff_null_tree_with_initial_index(self, rw_dir): + repo = Repo.init(rw_dir) + filename = ".gitkeep" + file_path = osp.join(repo.working_tree_dir, filename) + with open(file_path, "w") as fp: + fp.write("# Initial file\n") + + index = repo.index + index.add([filename]) + index.write() + + index = IndexFile(repo) + self.assertEqual(len(index.diff(None)), 0) + + diff = index.diff(NULL_TREE) + self.assertEqual(len(diff), 1) + self.assertEqual(diff[0].change_type, "A") + assert diff[0].new_file + self.assertEqual(diff[0].b_path, filename) + + self.assertEqual(len(index.diff(NULL_TREE, paths=filename)), 1) + self.assertEqual(len(index.diff(NULL_TREE_SHA, paths=filename)), 1) + self.assertEqual(len(index.diff(NULL_TREE, paths="missing")), 0) + + patch = index.diff(NULL_TREE, create_patch=True) + self.assertEqual(len(patch), 1) + self.assertIn(b"+# Initial file", patch[0].diff) + + with self.assertRaises(GitCommandError) as exc_info: + index.diff(NULL_TREE, bogus_option=True) + self.assertIn("usage: git diff", exc_info.exception.stderr) + def _count_existing(self, repo, files): """Return count of files that actually exist in the repository directory.""" existing = 0 @@ -565,369 +600,369 @@ def _count_existing(self, repo, files): # END num existing helper - @pytest.mark.xfail( - sys.platform == "win32" and (Git().config("core.symlinks") == "true" or _windows_supports_symlinks()), - reason="Assumes symlinks are not created on Windows and opens a symlink to a nonexistent target.", - raises=(FileNotFoundError, GitCommandError), - ) @with_rw_repo("0.1.6") def test_index_mutation(self, rw_repo): - index = rw_repo.index - num_entries = len(index.entries) - cur_head = rw_repo.head - - uname = "Thomas Müller" - umail = "sd@company.com" - with rw_repo.config_writer() as writer: - writer.set_value("user", "name", uname) - writer.set_value("user", "email", umail) - self.assertEqual(writer.get_value("user", "name"), uname) - - # Remove all of the files, provide a wild mix of paths, BaseIndexEntries, - # IndexEntries. - def mixed_iterator(): - count = 0 - for entry in index.entries.values(): - type_id = count % 5 - if type_id == 0: # path (str) - yield entry.path - elif type_id == 1: # path (PathLike) - yield Path(entry.path) - elif type_id == 2: # path mock (PathLike) - yield PathLikeMock(entry.path) - elif type_id == 3: # path mock in a blob - yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) - elif type_id == 4: # blob - yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) - elif type_id == 5: # BaseIndexEntry - yield BaseIndexEntry(entry[:4]) - elif type_id == 6: # IndexEntry - yield entry - else: - raise AssertionError("Invalid Type") - count += 1 - # END for each entry - - # END mixed iterator - deleted_files = index.remove(mixed_iterator(), working_tree=False) - assert deleted_files - self.assertEqual(self._count_existing(rw_repo, deleted_files), len(deleted_files)) - self.assertEqual(len(index.entries), 0) - - # Reset the index to undo our changes. - index.reset() - self.assertEqual(len(index.entries), num_entries) - - # Remove with working copy. - deleted_files = index.remove(mixed_iterator(), working_tree=True) - assert deleted_files - self.assertEqual(self._count_existing(rw_repo, deleted_files), 0) - - # Reset everything. - index.reset(working_tree=True) - self.assertEqual(self._count_existing(rw_repo, deleted_files), len(deleted_files)) - - # Invalid type. - self.assertRaises(TypeError, index.remove, [1]) - - # Absolute path. - deleted_files = index.remove([osp.join(rw_repo.working_tree_dir, "lib")], r=True) - assert len(deleted_files) > 1 - self.assertRaises(ValueError, index.remove, ["/doesnt/exists"]) - - # TEST COMMITTING - # Commit changed index. - cur_commit = cur_head.commit - commit_message = "commit default head by Frèderic Çaufl€" - - new_commit = index.commit(commit_message, head=False) - assert cur_commit != new_commit - self.assertEqual(new_commit.author.name, uname) - self.assertEqual(new_commit.author.email, umail) - self.assertEqual(new_commit.committer.name, uname) - self.assertEqual(new_commit.committer.email, umail) - self.assertEqual(new_commit.message, commit_message) - self.assertEqual(new_commit.parents[0], cur_commit) - self.assertEqual(len(new_commit.parents), 1) - self.assertEqual(cur_head.commit, cur_commit) - - # Commit with other actor. - cur_commit = cur_head.commit - - my_author = Actor("Frèderic Çaufl€", "author@example.com") - my_committer = Actor("Committing Frèderic Çaufl€", "committer@example.com") - commit_actor = index.commit(commit_message, author=my_author, committer=my_committer) - assert cur_commit != commit_actor - self.assertEqual(commit_actor.author.name, "Frèderic Çaufl€") - self.assertEqual(commit_actor.author.email, "author@example.com") - self.assertEqual(commit_actor.committer.name, "Committing Frèderic Çaufl€") - self.assertEqual(commit_actor.committer.email, "committer@example.com") - self.assertEqual(commit_actor.message, commit_message) - self.assertEqual(commit_actor.parents[0], cur_commit) - self.assertEqual(len(new_commit.parents), 1) - self.assertEqual(cur_head.commit, commit_actor) - self.assertEqual(cur_head.log()[-1].actor, my_committer) - - # Commit with author_date and commit_date. - cur_commit = cur_head.commit - commit_message = "commit with dates by Avinash Sajjanshetty" - - new_commit = index.commit( - commit_message, - author_date="2006-04-07T22:13:13", - commit_date="2005-04-07T22:13:13", - ) - assert cur_commit != new_commit - print(new_commit.authored_date, new_commit.committed_date) - self.assertEqual(new_commit.message, commit_message) - self.assertEqual(new_commit.authored_date, 1144447993) - self.assertEqual(new_commit.committed_date, 1112911993) - - # Same index, no parents. - commit_message = "index without parents" - commit_no_parents = index.commit(commit_message, parent_commits=[], head=True) - self.assertEqual(commit_no_parents.message, commit_message) - self.assertEqual(len(commit_no_parents.parents), 0) - self.assertEqual(cur_head.commit, commit_no_parents) - - # same index, multiple parents. - commit_message = "Index with multiple parents\n commit with another line" - commit_multi_parent = index.commit(commit_message, parent_commits=(commit_no_parents, new_commit)) - self.assertEqual(commit_multi_parent.message, commit_message) - self.assertEqual(len(commit_multi_parent.parents), 2) - self.assertEqual(commit_multi_parent.parents[0], commit_no_parents) - self.assertEqual(commit_multi_parent.parents[1], new_commit) - self.assertEqual(cur_head.commit, commit_multi_parent) - - # Re-add all files in lib. - # Get the lib folder back on disk, but get an index without it. - index.reset(new_commit.parents[0], working_tree=True).reset(new_commit, working_tree=False) - lib_file_path = osp.join("lib", "git", "__init__.py") - assert (lib_file_path, 0) not in index.entries - assert osp.isfile(osp.join(rw_repo.working_tree_dir, lib_file_path)) - - # Directory. - entries = index.add(["lib"], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - assert len(entries) > 1 - - # Glob. - entries = index.reset(new_commit).add([osp.join("lib", "git", "*.py")], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - self.assertEqual(len(entries), 14) - - # Same file. - entries = index.reset(new_commit).add( - [osp.join(rw_repo.working_tree_dir, "lib", "git", "head.py")] * 2, - fprogress=self._fprogress_add, - ) - self._assert_entries(entries) - self.assertEqual(entries[0].mode & 0o644, 0o644) - # Would fail, test is too primitive to handle this case. - # self._assert_fprogress(entries) - self._reset_progress() - self.assertEqual(len(entries), 2) - - # Missing path. - self.assertRaises(OSError, index.reset(new_commit).add, ["doesnt/exist/must/raise"]) - - # Blob from older revision overrides current index revision. - old_blob = new_commit.parents[0].tree.blobs[0] - entries = index.reset(new_commit).add([old_blob], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - self.assertEqual(index.entries[(old_blob.path, 0)].hexsha, old_blob.hexsha) - self.assertEqual(len(entries), 1) - - # Mode 0 not allowed. - null_hex_sha = Diff.NULL_HEX_SHA - null_bin_sha = b"\0" * 20 - self.assertRaises( - ValueError, - index.reset(new_commit).add, - [BaseIndexEntry((0, null_bin_sha, 0, "doesntmatter"))], - ) - - # Add new file. - new_file_relapath = "my_new_file" - self._make_file(new_file_relapath, "hello world", rw_repo) - entries = index.reset(new_commit).add( - [BaseIndexEntry((0o10644, null_bin_sha, 0, new_file_relapath))], - fprogress=self._fprogress_add, - ) - self._assert_entries(entries) - self._assert_fprogress(entries) - self.assertEqual(len(entries), 1) - self.assertNotEqual(entries[0].hexsha, null_hex_sha) - - # Add symlink. - if sys.platform != "win32": - for target in ("/etc/nonexisting", "/etc/passwd", "/etc"): - basename = "my_real_symlink" - - link_file = osp.join(rw_repo.working_tree_dir, basename) - os.symlink(target, link_file) - entries = index.reset(new_commit).add([link_file], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - self.assertEqual(len(entries), 1) - self.assertTrue(S_ISLNK(entries[0].mode)) - self.assertTrue(S_ISLNK(index.entries[index.entry_key("my_real_symlink", 0)].mode)) - - # We expect only the target to be written. - self.assertEqual( - index.repo.odb.stream(entries[0].binsha).read().decode("ascii"), - target, - ) - - os.remove(link_file) - # END for each target - # END real symlink test - - # Add fake symlink and assure it checks out as a symlink. - fake_symlink_relapath = "my_fake_symlink" - link_target = "/etc/that" - fake_symlink_path = self._make_file(fake_symlink_relapath, link_target, rw_repo) - fake_entry = BaseIndexEntry((0o120000, null_bin_sha, 0, fake_symlink_relapath)) - entries = index.reset(new_commit).add([fake_entry], fprogress=self._fprogress_add) - self._assert_entries(entries) - self._assert_fprogress(entries) - assert entries[0].hexsha != null_hex_sha - self.assertEqual(len(entries), 1) - self.assertTrue(S_ISLNK(entries[0].mode)) - - # Check that this also works with an alternate method. - full_index_entry = IndexEntry.from_base(BaseIndexEntry((0o120000, entries[0].binsha, 0, entries[0].path))) - entry_key = index.entry_key(full_index_entry) - index.reset(new_commit) - - assert entry_key not in index.entries - index.entries[entry_key] = full_index_entry - index.write() - index.update() # Force reread of entries. - new_entry = index.entries[entry_key] - assert S_ISLNK(new_entry.mode) + with xfail_if_raises( + sys.platform == "win32" and (Git().config("core.symlinks") == "true" or _windows_supports_symlinks()), + raises=(FileNotFoundError, GitCommandError), + reason="Assumes symlinks are not created on Windows and opens a symlink to a nonexistent target.", + ): + index = rw_repo.index + num_entries = len(index.entries) + cur_head = rw_repo.head + + uname = "Thomas Müller" + umail = "sd@company.com" + with rw_repo.config_writer() as writer: + writer.set_value("user", "name", uname) + writer.set_value("user", "email", umail) + self.assertEqual(writer.get_value("user", "name"), uname) + + # Remove all of the files, provide a wild mix of paths, BaseIndexEntries, + # IndexEntries. + def mixed_iterator(): + count = 0 + for entry in index.entries.values(): + type_id = count % 5 + if type_id == 0: # path (str) + yield entry.path + elif type_id == 1: # path (PathLike) + yield Path(entry.path) + elif type_id == 2: # path mock (PathLike) + yield PathLikeMock(entry.path) + elif type_id == 3: # path mock in a blob + yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) + elif type_id == 4: # blob + yield Blob(rw_repo, entry.binsha, entry.mode, entry.path) + elif type_id == 5: # BaseIndexEntry + yield BaseIndexEntry(entry[:4]) + elif type_id == 6: # IndexEntry + yield entry + else: + raise AssertionError("Invalid Type") + count += 1 + # END for each entry + + # END mixed iterator + deleted_files = index.remove(mixed_iterator(), working_tree=False) + assert deleted_files + self.assertEqual(self._count_existing(rw_repo, deleted_files), len(deleted_files)) + self.assertEqual(len(index.entries), 0) + + # Reset the index to undo our changes. + index.reset() + self.assertEqual(len(index.entries), num_entries) + + # Remove with working copy. + deleted_files = index.remove(mixed_iterator(), working_tree=True) + assert deleted_files + self.assertEqual(self._count_existing(rw_repo, deleted_files), 0) + + # Reset everything. + index.reset(working_tree=True) + self.assertEqual(self._count_existing(rw_repo, deleted_files), len(deleted_files)) + + # Invalid type. + self.assertRaises(TypeError, index.remove, [1]) + + # Absolute path. + deleted_files = index.remove([osp.join(rw_repo.working_tree_dir, "lib")], r=True) + assert len(deleted_files) > 1 + self.assertRaises(ValueError, index.remove, ["/doesnt/exists"]) + + # TEST COMMITTING + # Commit changed index. + cur_commit = cur_head.commit + commit_message = "commit default head by Frèderic Çaufl€" + + new_commit = index.commit(commit_message, head=False) + assert cur_commit != new_commit + self.assertEqual(new_commit.author.name, uname) + self.assertEqual(new_commit.author.email, umail) + self.assertEqual(new_commit.committer.name, uname) + self.assertEqual(new_commit.committer.email, umail) + self.assertEqual(new_commit.message, commit_message) + self.assertEqual(new_commit.parents[0], cur_commit) + self.assertEqual(len(new_commit.parents), 1) + self.assertEqual(cur_head.commit, cur_commit) + + # Commit with other actor. + cur_commit = cur_head.commit + + my_author = Actor("Frèderic Çaufl€", "author@example.com") + my_committer = Actor("Committing Frèderic Çaufl€", "committer@example.com") + commit_actor = index.commit(commit_message, author=my_author, committer=my_committer) + assert cur_commit != commit_actor + self.assertEqual(commit_actor.author.name, "Frèderic Çaufl€") + self.assertEqual(commit_actor.author.email, "author@example.com") + self.assertEqual(commit_actor.committer.name, "Committing Frèderic Çaufl€") + self.assertEqual(commit_actor.committer.email, "committer@example.com") + self.assertEqual(commit_actor.message, commit_message) + self.assertEqual(commit_actor.parents[0], cur_commit) + self.assertEqual(len(new_commit.parents), 1) + self.assertEqual(cur_head.commit, commit_actor) + self.assertEqual(cur_head.log()[-1].actor, my_committer) + + # Commit with author_date and commit_date. + cur_commit = cur_head.commit + commit_message = "commit with dates by Avinash Sajjanshetty" + + new_commit = index.commit( + commit_message, + author_date="2006-04-07T22:13:13", + commit_date="2005-04-07T22:13:13", + ) + assert cur_commit != new_commit + print(new_commit.authored_date, new_commit.committed_date) + self.assertEqual(new_commit.message, commit_message) + self.assertEqual(new_commit.authored_date, 1144447993) + self.assertEqual(new_commit.committed_date, 1112911993) + + # Same index, no parents. + commit_message = "index without parents" + commit_no_parents = index.commit(commit_message, parent_commits=[], head=True) + self.assertEqual(commit_no_parents.message, commit_message) + self.assertEqual(len(commit_no_parents.parents), 0) + self.assertEqual(cur_head.commit, commit_no_parents) + + # same index, multiple parents. + commit_message = "Index with multiple parents\n commit with another line" + commit_multi_parent = index.commit(commit_message, parent_commits=(commit_no_parents, new_commit)) + self.assertEqual(commit_multi_parent.message, commit_message) + self.assertEqual(len(commit_multi_parent.parents), 2) + self.assertEqual(commit_multi_parent.parents[0], commit_no_parents) + self.assertEqual(commit_multi_parent.parents[1], new_commit) + self.assertEqual(cur_head.commit, commit_multi_parent) + + # Re-add all files in lib. + # Get the lib folder back on disk, but get an index without it. + index.reset(new_commit.parents[0], working_tree=True).reset(new_commit, working_tree=False) + lib_file_path = osp.join("lib", "git", "__init__.py") + assert (lib_file_path, 0) not in index.entries + assert osp.isfile(osp.join(rw_repo.working_tree_dir, lib_file_path)) + + # Directory. + entries = index.add(["lib"], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + assert len(entries) > 1 + + # Glob. + entries = index.reset(new_commit).add([osp.join("lib", "git", "*.py")], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + self.assertEqual(len(entries), 14) + + # Same file. + entries = index.reset(new_commit).add( + [osp.join(rw_repo.working_tree_dir, "lib", "git", "head.py")] * 2, + fprogress=self._fprogress_add, + ) + self._assert_entries(entries) + self.assertEqual(entries[0].mode & 0o644, 0o644) + # Would fail, test is too primitive to handle this case. + # self._assert_fprogress(entries) + self._reset_progress() + self.assertEqual(len(entries), 2) + + # Missing path. + self.assertRaises(OSError, index.reset(new_commit).add, ["doesnt/exist/must/raise"]) + + # Blob from older revision overrides current index revision. + old_blob = new_commit.parents[0].tree.blobs[0] + entries = index.reset(new_commit).add([old_blob], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + self.assertEqual(index.entries[(old_blob.path, 0)].hexsha, old_blob.hexsha) + self.assertEqual(len(entries), 1) + + # Mode 0 not allowed. + null_hex_sha = Diff.NULL_HEX_SHA + null_bin_sha = b"\0" * 20 + self.assertRaises( + ValueError, + index.reset(new_commit).add, + [BaseIndexEntry((0, null_bin_sha, 0, "doesntmatter"))], + ) - # A tree created from this should contain the symlink. - tree = index.write_tree() - assert fake_symlink_relapath in tree - index.write() # Flush our changes for the checkout. - - # Check out the fake link, should be a link then. - assert not S_ISLNK(os.stat(fake_symlink_path)[ST_MODE]) - os.remove(fake_symlink_path) - index.checkout(fake_symlink_path) - - # On Windows, we currently assume we will never get symlinks. - if sys.platform == "win32": - # Symlinks should contain the link as text (which is what a - # symlink actually is). - with open(fake_symlink_path, "rt") as fd: - self.assertEqual(fd.read(), link_target) - else: - self.assertTrue(S_ISLNK(os.lstat(fake_symlink_path)[ST_MODE])) - - # TEST RENAMING - def assert_mv_rval(rval): - for source, dest in rval: - assert not osp.exists(source) and osp.exists(dest) - # END for each renamed item - - # END move assertion utility - - self.assertRaises(ValueError, index.move, ["just_one_path"]) - # Try to move a file onto an existing file. - files = ["AUTHORS", "LICENSE"] - self.assertRaises(GitCommandError, index.move, files) - - # Again, with force. - assert_mv_rval(index.move(files, f=True)) - - # Move files into a directory - dry run. - paths = ["LICENSE", "VERSION", "doc"] - rval = index.move(paths, dry_run=True) - self.assertEqual(len(rval), 2) - assert osp.exists(paths[0]) - - # Again, no dry run. - rval = index.move(paths) - assert_mv_rval(rval) - - # Move dir into dir. - rval = index.move(["doc", "test"]) - assert_mv_rval(rval) - - # TEST PATH REWRITING - ###################### - count = [0] - - def rewriter(entry): - rval = str(count[0]) - count[0] += 1 - return rval - - # END rewriter - - def make_paths(): - """Help out the test by yielding two existing paths and one new path.""" - yield "CHANGES" - yield "ez_setup.py" - yield index.entries[index.entry_key("README", 0)] - yield index.entries[index.entry_key(".gitignore", 0)] - - for fid in range(3): - fname = "newfile%i" % fid - with open(fname, "wb") as fd: - fd.write(b"abcd") - yield Blob(rw_repo, Blob.NULL_BIN_SHA, 0o100644, fname) - # END for each new file - - # END path producer - paths = list(make_paths()) - self._assert_entries(index.add(paths, path_rewriter=rewriter)) - - for filenum in range(len(paths)): - assert index.entry_key(str(filenum), 0) in index.entries - - # TEST RESET ON PATHS - ###################### - arela = "aa" - brela = "bb" - afile = self._make_file(arela, "adata", rw_repo) - bfile = self._make_file(brela, "bdata", rw_repo) - akey = index.entry_key(arela, 0) - bkey = index.entry_key(brela, 0) - keys = (akey, bkey) - absfiles = (afile, bfile) - files = (arela, brela) - - for fkey in keys: - assert fkey not in index.entries - - index.add(files, write=True) - nc = index.commit("2 files committed", head=False) - - for fkey in keys: - assert fkey in index.entries - - # Just the index. - index.reset(paths=(arela, afile)) - assert akey not in index.entries - assert bkey in index.entries - - # Now with working tree - files on disk as well as entries must be recreated. - rw_repo.head.commit = nc - for absfile in absfiles: - os.remove(absfile) - - index.reset(working_tree=True, paths=files) - - for fkey in keys: - assert fkey in index.entries - for absfile in absfiles: - assert osp.isfile(absfile) + # Add new file. + new_file_relapath = "my_new_file" + self._make_file(new_file_relapath, "hello world", rw_repo) + entries = index.reset(new_commit).add( + [BaseIndexEntry((0o10644, null_bin_sha, 0, new_file_relapath))], + fprogress=self._fprogress_add, + ) + self._assert_entries(entries) + self._assert_fprogress(entries) + self.assertEqual(len(entries), 1) + self.assertNotEqual(entries[0].hexsha, null_hex_sha) + + # Add symlink. + if sys.platform != "win32": + for target in ("/etc/nonexisting", "/etc/passwd", "/etc"): + basename = "my_real_symlink" + + link_file = osp.join(rw_repo.working_tree_dir, basename) + os.symlink(target, link_file) + entries = index.reset(new_commit).add([link_file], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + self.assertEqual(len(entries), 1) + self.assertTrue(S_ISLNK(entries[0].mode)) + self.assertTrue(S_ISLNK(index.entries[index.entry_key("my_real_symlink", 0)].mode)) + + # We expect only the target to be written. + self.assertEqual( + index.repo.odb.stream(entries[0].binsha).read().decode("ascii"), + target, + ) + + os.remove(link_file) + # END for each target + # END real symlink test + + # Add fake symlink and assure it checks out as a symlink. + fake_symlink_relapath = "my_fake_symlink" + link_target = "/etc/that" + fake_symlink_path = self._make_file(fake_symlink_relapath, link_target, rw_repo) + fake_entry = BaseIndexEntry((0o120000, null_bin_sha, 0, fake_symlink_relapath)) + entries = index.reset(new_commit).add([fake_entry], fprogress=self._fprogress_add) + self._assert_entries(entries) + self._assert_fprogress(entries) + assert entries[0].hexsha != null_hex_sha + self.assertEqual(len(entries), 1) + self.assertTrue(S_ISLNK(entries[0].mode)) + + # Check that this also works with an alternate method. + full_index_entry = IndexEntry.from_base(BaseIndexEntry((0o120000, entries[0].binsha, 0, entries[0].path))) + entry_key = index.entry_key(full_index_entry) + index.reset(new_commit) + + assert entry_key not in index.entries + index.entries[entry_key] = full_index_entry + index.write() + index.update() # Force reread of entries. + new_entry = index.entries[entry_key] + assert S_ISLNK(new_entry.mode) + + # A tree created from this should contain the symlink. + tree = index.write_tree() + assert fake_symlink_relapath in tree + index.write() # Flush our changes for the checkout. + + # Check out the fake link, should be a link then. + assert not S_ISLNK(os.stat(fake_symlink_path)[ST_MODE]) + os.remove(fake_symlink_path) + index.checkout(fake_symlink_path) + + # On Windows, we currently assume we will never get symlinks. + if sys.platform == "win32": + # Symlinks should contain the link as text (which is what a + # symlink actually is). + with open(fake_symlink_path, "rt") as fd: + self.assertEqual(fd.read(), link_target) + else: + self.assertTrue(S_ISLNK(os.lstat(fake_symlink_path)[ST_MODE])) + + # TEST RENAMING + def assert_mv_rval(rval): + for source, dest in rval: + assert not osp.exists(source) and osp.exists(dest) + # END for each renamed item + + # END move assertion utility + + self.assertRaises(ValueError, index.move, ["just_one_path"]) + # Try to move a file onto an existing file. + files = ["AUTHORS", "LICENSE"] + self.assertRaises(GitCommandError, index.move, files) + + # Again, with force. + assert_mv_rval(index.move(files, f=True)) + + # Move files into a directory - dry run. + paths = ["LICENSE", "VERSION", "doc"] + rval = index.move(paths, dry_run=True) + self.assertEqual(len(rval), 2) + assert osp.exists(paths[0]) + + # Again, no dry run. + rval = index.move(paths) + assert_mv_rval(rval) + + # Move dir into dir. + rval = index.move(["doc", "test"]) + assert_mv_rval(rval) + + # TEST PATH REWRITING + ###################### + count = [0] + + def rewriter(entry): + rval = str(count[0]) + count[0] += 1 + return rval + + # END rewriter + + def make_paths(): + """Help out the test by yielding two existing paths and one new path.""" + yield "CHANGES" + yield "ez_setup.py" + yield index.entries[index.entry_key("README", 0)] + yield index.entries[index.entry_key(".gitignore", 0)] + + for fid in range(3): + fname = "newfile%i" % fid + with open(fname, "wb") as fd: + fd.write(b"abcd") + yield Blob(rw_repo, Blob.NULL_BIN_SHA, 0o100644, fname) + # END for each new file + + # END path producer + paths = list(make_paths()) + self._assert_entries(index.add(paths, path_rewriter=rewriter)) + + for filenum in range(len(paths)): + assert index.entry_key(str(filenum), 0) in index.entries + + # TEST RESET ON PATHS + ###################### + arela = "aa" + brela = "bb" + afile = self._make_file(arela, "adata", rw_repo) + bfile = self._make_file(brela, "bdata", rw_repo) + akey = index.entry_key(arela, 0) + bkey = index.entry_key(brela, 0) + keys = (akey, bkey) + absfiles = (afile, bfile) + files = (arela, brela) + + for fkey in keys: + assert fkey not in index.entries + + index.add(files, write=True) + nc = index.commit("2 files committed", head=False) + + for fkey in keys: + assert fkey in index.entries + + # Just the index. + index.reset(paths=(arela, afile)) + assert akey not in index.entries + assert bkey in index.entries + + # Now with working tree - files on disk as well as entries must be recreated. + rw_repo.head.commit = nc + for absfile in absfiles: + os.remove(absfile) + + index.reset(working_tree=True, paths=files) + + for fkey in keys: + assert fkey in index.entries + for absfile in absfiles: + assert osp.isfile(absfile) @with_rw_repo("HEAD") def test_compare_write_tree(self, rw_repo): diff --git a/test/test_remote.py b/test/test_remote.py index 1c627127a..64c41290e 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -9,7 +9,7 @@ import random import sys import tempfile -from unittest import skipIf +from unittest import mock, skipIf import pytest @@ -687,7 +687,12 @@ def test_multiple_urls(self, rw_repo): def test_fetch_error(self): rem = self.rorepo.remote("origin") - with self.assertRaisesRegex(GitCommandError, "[Cc]ouldn't find remote ref __BAD_REF__"): + msg = ( + r"[Cc]ouldn't find remote ref __BAD_REF__|" + r"could not read Username|" + r"expected flush after ref listing" + ) + with self.assertRaisesRegex(GitCommandError, msg): rem.fetch("__BAD_REF__") @with_rw_repo("0.1.6", bare=False) @@ -827,7 +832,11 @@ def test_fetch_unsafe_options(self, rw_repo): remote = rw_repo.remote("origin") tmp_dir = Path(tdir) tmp_file = tmp_dir / "pwn" - unsafe_options = [{"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}] + unsafe_options = [ + {"upload-pack": f"touch {tmp_file}"}, + {"upload_pack": f"touch {tmp_file}"}, + {"upload_p": f"touch {tmp_file}"}, + ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): remote.fetch(**unsafe_option) @@ -895,7 +904,11 @@ def test_pull_unsafe_options(self, rw_repo): remote = rw_repo.remote("origin") tmp_dir = Path(tdir) tmp_file = tmp_dir / "pwn" - unsafe_options = [{"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}] + unsafe_options = [ + {"upload-pack": f"touch {tmp_file}"}, + {"upload_pack": f"touch {tmp_file}"}, + {"upload_p": f"touch {tmp_file}"}, + ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): remote.pull(**unsafe_option) @@ -966,7 +979,9 @@ def test_push_unsafe_options(self, rw_repo): unsafe_options = [ {"receive-pack": f"touch {tmp_file}"}, {"receive_pack": f"touch {tmp_file}"}, + {"receive_p": f"touch {tmp_file}"}, {"exec": f"touch {tmp_file}"}, + {"exe": f"touch {tmp_file}"}, ] for unsafe_option in unsafe_options: assert not tmp_file.exists() @@ -1002,6 +1017,47 @@ def test_push_unsafe_options_allowed(self, rw_repo): assert tmp_file.exists() tmp_file.unlink() + @with_rw_repo("HEAD") + def test_ls_remote_unsafe_options(self, rw_repo): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = Path(tdir) + tmp_file = tmp_dir / "pwn" + unsafe_options = [ + {"upload-pack": f"touch {tmp_file}"}, + {"upload_pack": f"touch {tmp_file}"}, + {"upl": f"touch {tmp_file}"}, + ] + for unsafe_option in unsafe_options: + with self.assertRaises(UnsafeOptionError): + rw_repo.git.ls_remote(".", **unsafe_option) + with self.assertRaises(UnsafeOptionError): + rw_repo.git.ls_remote([f"--upload-pack={tmp_file}"], ".") + with self.assertRaises(UnsafeOptionError): + rw_repo.git.ls_remote([f"--upl={tmp_file}"], ".") + with self.assertRaises(UnsafeOptionError): + rw_repo.git.ls_remote(f"--upload-pack={tmp_file}", ".") + with self.assertRaises(UnsafeOptionError): + rw_repo.git.ls_remote(f"--upl={tmp_file}", ".") + with self.assertRaises(UnsafeOptionError): + rw_repo.git.ls_remote("--upload-pack", "touch", ".") + with self.assertRaises(UnsafeOptionError): + rw_repo.git.ls_remote("--refs", ".", upl=f"touch {tmp_file}") + + def test_ls_remote_allows_operand_named_like_unsafe_option(self): + with mock.patch.object(Git, "_call_process") as git: + Git().ls_remote("upload-pack") + git.assert_called_once() + + @with_rw_repo("HEAD") + def test_ls_remote_unsafe_options_allowed(self, rw_repo): + with tempfile.TemporaryDirectory() as tdir: + tmp_dir = Path(tdir) + tmp_file = tmp_dir / "pwn" + unsafe_options = [{"upload-pack": f"touch {tmp_file}"}] + for unsafe_option in unsafe_options: + with self.assertRaises(GitCommandError): + rw_repo.git.ls_remote(".", **unsafe_option, allow_unsafe_options=True) + @with_rw_and_rw_remote_repo("0.1.6") def test_fetch_unsafe_branch_name(self, rw_repo, remote_repo): # Create branch with a name containing a NBSP diff --git a/test/test_repo.py b/test/test_repo.py index fae3dc0b9..cfad73489 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -37,6 +37,8 @@ Submodule, Tree, ) +from git.exc import UnsafeOptionError +from git.exc import UnsafeProtocolError from git.exc import BadObject from git.repo.fun import touch from git.util import bin_to_hex, cwd, cygpath, join_path_native, rmfile, rmtree @@ -422,6 +424,54 @@ def test_archive(self): assert stream.tell() os.remove(stream.name) # Do it this way so we can inspect the file on failure. + def test_archive_rejects_unsafe_options(self): + with tempfile.TemporaryDirectory() as tdir: + output_marker = osp.join(tdir, "pwn") + with self.assertRaises(UnsafeOptionError): + self.rorepo.archive(io.BytesIO(), "0.1.6", exec=f"touch {output_marker}") + assert not osp.exists(output_marker) + with self.assertRaises(UnsafeOptionError): + self.rorepo.archive(io.BytesIO(), "0.1.6", output=output_marker) + assert not osp.exists(output_marker) + + def test_archive_rejects_unsafe_remote_protocol(self): + with tempfile.TemporaryDirectory() as tdir: + output_marker = osp.join(tdir, "pwn") + with self.assertRaises(UnsafeProtocolError): + self.rorepo.archive(io.BytesIO(), "HEAD", remote=f"ext::sh -c touch% {output_marker}") + assert not osp.exists(output_marker) + + def test_archive_preserves_positional_allow_unsafe_options(self): + with mock.patch.object(Git, "_call_process") as git: + self.rorepo.archive(io.BytesIO(), "HEAD", None, True, exec="git-upload-archive") + git.assert_called_once() + + def test_archive_accepts_stringifiable_remote(self): + class StringifiableRemote: + def __str__(self): + return "origin" + + with mock.patch.object(Git, "_call_process") as git: + self.rorepo.archive(io.BytesIO(), "HEAD", remote=StringifiableRemote()) + git.assert_called_once() + + def test_archive_rejects_unsafe_falsey_remote_protocol(self): + class FalseyRemote: + def __bool__(self): + return False + + def __str__(self): + return "ext::sh -c true" + + with self.assertRaises(UnsafeProtocolError): + self.rorepo.archive(io.BytesIO(), "HEAD", remote=FalseyRemote()) + + def test_iter_commits_rejects_unsafe_revision(self): + with tempfile.TemporaryDirectory() as tdir: + target = osp.join(tdir, "pwn") + with self.assertRaises(UnsafeOptionError): + list(self.rorepo.iter_commits(f"--output={target}", max_count=1)) + @mock.patch.object(Git, "_call_process") def test_should_display_blame_information(self, git): git.return_value = fixture("blame") @@ -471,6 +521,27 @@ def test_blame_real(self): assert c, "Should have executed at least one blame command" assert nml, "There should at least be one blame commit that contains multiple lines" + def test_blame_rejects_unsafe_revision(self): + with tempfile.TemporaryDirectory() as tdir: + output_marker = osp.join(tdir, "pwn") + with self.assertRaises(UnsafeOptionError): + self.rorepo.blame(f"--output={output_marker}", "README.md") + assert not osp.exists(output_marker) + + def test_blame_rejects_unsafe_options(self): + with tempfile.TemporaryDirectory() as tdir: + output_marker = osp.join(tdir, "pwn") + with self.assertRaises(UnsafeOptionError): + self.rorepo.blame("HEAD", "README.md", output=output_marker) + assert not osp.exists(output_marker) + + def test_blame_rejects_unsafe_rev_opts(self): + with tempfile.TemporaryDirectory() as tdir: + output_marker = osp.join(tdir, "pwn") + with self.assertRaises(UnsafeOptionError): + self.rorepo.blame("HEAD", "README.md", rev_opts=(f"--output={output_marker}",)) + assert not osp.exists(output_marker) + @mock.patch.object(Git, "_call_process") def test_blame_incremental(self, git): # Loop over two fixtures, create a test fixture for 2.11.1+ syntax. @@ -877,11 +948,6 @@ def test_repo_odbtype(self): target_type = GitCmdObjectDB self.assertIsInstance(self.rorepo.odb, target_type) - @pytest.mark.xfail( - sys.platform == "cygwin", - reason="Cygwin GitPython can't find submodule SHA", - raises=ValueError, - ) def test_submodules(self): self.assertEqual(len(self.rorepo.submodules), 1) # non-recursive self.assertGreaterEqual(len(list(self.rorepo.iter_submodules())), 2) @@ -1094,7 +1160,7 @@ def test_is_valid_object(self): self.assertFalse(repo.is_valid_object(tag_sha, "commit")) @with_rw_directory - def test_git_work_tree_dotgit(self, rw_dir): + def test_git_work_tree_dotgit(self, rw_dir, use_relative_paths=False): """Check that we find .git as a worktree file and find the worktree based on it.""" git = Git(rw_dir) @@ -1106,7 +1172,11 @@ def test_git_work_tree_dotgit(self, rw_dir): worktree_path = join_path_native(rw_dir, "worktree_repo") if Git.is_cygwin(): worktree_path = cygpath(worktree_path) - rw_master.git.worktree("add", worktree_path, branch.name) + wt_add_kwargs = {"insert_kwargs_after": "add"} + # relative worktree paths introduced in git 2.48.0 + if use_relative_paths and git.version_info[:3] >= (2, 48, 0): + wt_add_kwargs["relative_paths"] = True + rw_master.git.worktree("add", worktree_path, branch.name, **wt_add_kwargs) # This ensures that we can read the repo's gitdir correctly. repo = Repo(worktree_path) @@ -1124,6 +1194,15 @@ def test_git_work_tree_dotgit(self, rw_dir): self.assertIsInstance(repo.heads["aaaaaaaa"], Head) + def test_git_work_tree_dotgit_relative(self): + """Check that we find .git as a worktree file containing a relative path + and find the worktree based on it.""" + if Git().version_info[:3] < (2, 48, 0): + pytest.skip("relative worktree feature unsupported, needs git 2.48.0 or later") + # this class inherits from TestCase so we can't use pytest.mark.parametrize on + # test_git_work_tree_dotgit; delegate instead + self.test_git_work_tree_dotgit(use_relative_paths=True) + @with_rw_directory def test_git_work_tree_env(self, rw_dir): """Check that we yield to GIT_WORK_TREE.""" diff --git a/test/test_submodule.py b/test/test_submodule.py index 63bb007de..778d22e3f 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -480,11 +480,6 @@ def test_base_rw(self, rwrepo): def test_base_bare(self, rwrepo): self._do_base_tests(rwrepo) - @pytest.mark.xfail( - sys.platform == "cygwin", - reason="Cygwin GitPython can't find submodule SHA", - raises=ValueError, - ) @pytest.mark.xfail( HIDE_WINDOWS_KNOWN_ERRORS, reason=( @@ -513,9 +508,9 @@ def test_root_module(self, rwrepo): with rm.config_writer(): pass - # Deep traversal gitdb / async. + # Deep traversal yields gitdb and its nested smmap. rsmsp = [sm.path for sm in rm.traverse()] - assert len(rsmsp) >= 2 # gitdb and async [and smmap], async being a child of gitdb. + assert rsmsp == ["git/ext/gitdb", "gitdb/ext/smmap"] # Cannot set the parent commit as root module's path didn't exist. self.assertRaises(ValueError, rm.set_parent_commit, "HEAD")