| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -9,6 +9,7 @@ Security fixes for | |||
| 9 | 9 | ||
| 10 | 10 | * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-gq48-pqfc-9p58 | |
| 11 | 11 | * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-23mf-xhv8-69c2 | |
| 12 | + * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-f9j4-qggq-h239 | ||
| 12 | 13 | ||
| 13 | 14 | If you can, also try and provide feedback on the upcoming v4 branch | |
| 14 | 15 | https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1795,6 +1795,12 @@ def _call_process( | |||
| 1795 | 1795 | This allows your commands to call git more conveniently, as ``None`` is | |
| 1796 | 1796 | realized as non-existent. | |
| 1797 | 1797 | ||
| 1798 | + Positional arguments may intentionally contain command options. Higher-level | ||
| 1799 | + APIs must separate their operands with ``--`` where the Git command supports | ||
| 1800 | + it, or reject option-shaped operands where Git reparses them internally (for | ||
| 1801 | + example, ``pull`` and ``remote update``). Shell quoting cannot prevent Git | ||
| 1802 | + from interpreting a leading-dash argument as an option. | ||
| 1803 | + | ||
| 1798 | 1804 | :param kwargs: | |
| 1799 | 1805 | Contains key-values for the following: | |
| 1800 | 1806 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1132,6 +1132,7 @@ def move( | |||
| 1132 | 1132 | args = [] | |
| 1133 | 1133 | if skip_errors: | |
| 1134 | 1134 | args.append("-k") | |
| 1135 | + args.append("--") | ||
| 1135 | 1136 | ||
| 1136 | 1137 | paths = self._items_to_rela_paths(items) | |
| 1137 | 1138 | if len(paths) < 2: | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -166,7 +166,7 @@ def delete(cls, repo: "Repo", *heads: "Union[Head, str]", force: bool = False, * | |||
| 166 | 166 | flag = "-d" | |
| 167 | 167 | if force: | |
| 168 | 168 | flag = "-D" | |
| 169 | - repo.git.branch(flag, *heads) | ||
| 169 | + repo.git.branch(flag, "--", *heads) | ||
| 170 | 170 | ||
| 171 | 171 | def set_tracking_branch(self, remote_reference: Union["RemoteReference", None]) -> "Head": | |
| 172 | 172 | """Configure this branch to track the given remote reference. This will | |
@@ -241,7 +241,7 @@ def rename(self, new_path: PathLike, force: bool = False) -> "Head": | |||
| 241 | 241 | if force: | |
| 242 | 242 | flag = "-M" | |
| 243 | 243 | ||
| 244 | - self.repo.git.branch(flag, self, new_path) | ||
| 244 | + self.repo.git.branch(flag, "--", self, new_path) | ||
| 245 | 245 | self.path = "%s/%s" % (self._common_path_default, new_path) | |
| 246 | 246 | return self | |
| 247 | 247 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -61,7 +61,7 @@ def delete(cls, repo: "Repo", *refs: "RemoteReference", **kwargs: Any) -> None: | |||
| 61 | 61 | for ref in refs: | |
| 62 | 62 | cls._check_ref_name_valid(ref.path) | |
| 63 | 63 | ||
| 64 | - repo.git.branch("-d", "-r", *refs) | ||
| 64 | + repo.git.branch("-d", "-r", "--", *refs) | ||
| 65 | 65 | # The official deletion method will ignore remote symbolic refs - these are | |
| 66 | 66 | # generally ignored in the refs/ folder. We don't though and delete remainders | |
| 67 | 67 | # manually. | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -155,15 +155,15 @@ def create( | |||
| 155 | 155 | if force: | |
| 156 | 156 | kwargs["f"] = True | |
| 157 | 157 | ||
| 158 | - args = (path, reference) | ||
| 158 | + args = ("--", path, reference) | ||
| 159 | 159 | ||
| 160 | 160 | repo.git.tag(*args, **kwargs) | |
| 161 | 161 | return TagReference(repo, "%s/%s" % (cls._common_path_default, path)) | |
| 162 | 162 | ||
| 163 | 163 | @classmethod | |
| 164 | 164 | def delete(cls, repo: "Repo", *tags: "TagReference") -> None: # type: ignore[override] | |
| 165 | 165 | """Delete the given existing tag or tags.""" | |
| 166 | - repo.git.tag("-d", *tags) | ||
| 166 | + repo.git.tag("-d", "--", *tags) | ||
| 167 | 167 | ||
| 168 | 168 | ||
| 169 | 169 | # Provide an alias. | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -14,7 +14,7 @@ | |||
| 14 | 14 | from git.cmd import Git, handle_process_output | |
| 15 | 15 | from git.compat import defenc, force_text | |
| 16 | 16 | from git.config import GitConfigParser, SectionConstraint, cp | |
| 17 | - from git.exc import GitCommandError | ||
| 17 | + from git.exc import GitCommandError, UnsafeOptionError | ||
| 18 | 18 | from git.refs import Head, Reference, RemoteReference, SymbolicReference, TagReference | |
| 19 | 19 | from git.util import ( | |
| 20 | 20 | CallableRemoteProgress, | |
@@ -871,6 +871,9 @@ def update(self, **kwargs: Any) -> "Remote": | |||
| 871 | 871 | :return: | |
| 872 | 872 | self | |
| 873 | 873 | """ | |
| 874 | + # Like pull, remote update forwards operands to fetch without `--`. | ||
| 875 | + if self.name.startswith("-"): | ||
| 876 | + raise UnsafeOptionError("Remote names used by update must not start with '-'.") | ||
| 874 | 877 | scmd = "update" | |
| 875 | 878 | kwargs["insert_kwargs_after"] = scmd | |
| 876 | 879 | self.repo.git.remote(scmd, self.name, **kwargs) | |
@@ -1101,7 +1104,8 @@ def pull( | |||
| 1101 | 1104 | merge of branch with your local branch. | |
| 1102 | 1105 | ||
| 1103 | 1106 | :param refspec: | |
| 1104 | - See :meth:`fetch` method. | ||
| 1107 | + See :meth:`fetch` method. Values starting with ``-`` are rejected, | ||
| 1108 | + even when ``allow_unsafe_options`` is enabled. Pass options as keywords. | ||
| 1105 | 1109 | ||
| 1106 | 1110 | :param progress: | |
| 1107 | 1111 | See :meth:`push` method. | |
@@ -1127,6 +1131,12 @@ def pull( | |||
| 1127 | 1131 | kwargs = add_progress(kwargs, self.repo.git, progress) | |
| 1128 | 1132 | ||
| 1129 | 1133 | refspec = Git._unpack_args(refspec or []) | |
| 1134 | + # Git pull forwards these operands to fetch without preserving `--`. | ||
| 1135 | + # Reject every option-shaped operand, including with unsafe options enabled: | ||
| 1136 | + # opting into an explicit option must not turn a refspec into an option. | ||
| 1137 | + for operand in [self.name, *refspec]: | ||
| 1138 | + if operand.startswith("-"): | ||
| 1139 | + raise UnsafeOptionError("Remote names and pull refspecs must not start with '-'.") | ||
| 1130 | 1140 | if not allow_unsafe_protocols: | |
| 1131 | 1141 | for ref in refspec: | |
| 1132 | 1142 | Git.check_unsafe_protocols(ref) | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1137,7 +1137,7 @@ def ignored(self, *paths: PathLike) -> List[str]: | |||
| 1137 | 1137 | Subset of those paths which are ignored | |
| 1138 | 1138 | """ | |
| 1139 | 1139 | try: | |
| 1140 | - proc: str = self.git.check_ignore(*paths) | ||
| 1140 | + proc: str = self.git.check_ignore("--", *paths) | ||
| 1141 | 1141 | except GitCommandError as err: | |
| 1142 | 1142 | if err.status == 1: | |
| 1143 | 1143 | # If return code is 1, this means none of the items in *paths are | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,129 @@ | |||
| 1 | + """High-level operands must not become Git options.""" | ||
| 2 | + | ||
| 3 | + from unittest import mock | ||
| 4 | + | ||
| 5 | + import pytest | ||
| 6 | + | ||
| 7 | + from git import Actor, Git, GitCommandError, Head, Remote, RemoteReference, Repo, TagReference | ||
| 8 | + from git.exc import UnsafeOptionError | ||
| 9 | + | ||
| 10 | + | ||
| 11 | + @pytest.mark.parametrize("allow_unsafe_options", [False, True]) | ||
| 12 | + @pytest.mark.parametrize( | ||
| 13 | + "refspec", | ||
| 14 | + ["--upload-pack=helper", ["--upl=helper"], ["main", "--dry-run"], "-uhelper", "--arg value", "--"], | ||
| 15 | + ) | ||
| 16 | + def test_pull_rejects_option_shaped_refspec(tmp_path, refspec, allow_unsafe_options): | ||
| 17 | + repo = Repo.init(tmp_path) | ||
| 18 | + remote = Remote(repo, "origin") | ||
| 19 | + with mock.patch.object(Git, "_call_process", side_effect=AssertionError("Git must not run")) as run: | ||
| 20 | + with pytest.raises(UnsafeOptionError): | ||
| 21 | + remote.pull(refspec, allow_unsafe_options=allow_unsafe_options) | ||
| 22 | + run.assert_not_called() | ||
| 23 | + | ||
| 24 | + | ||
| 25 | + def test_pull_rejects_option_shaped_remote(tmp_path): | ||
| 26 | + repo = Repo.init(tmp_path) | ||
| 27 | + remote = Remote(repo, "--upload-pack=helper") | ||
| 28 | + with mock.patch.object(Git, "_call_process", side_effect=AssertionError("Git must not run")) as run: | ||
| 29 | + with pytest.raises(UnsafeOptionError): | ||
| 30 | + remote.pull("main") | ||
| 31 | + run.assert_not_called() | ||
| 32 | + | ||
| 33 | + | ||
| 34 | + def test_pull_preserves_operand_and_explicit_option_values(tmp_path): | ||
| 35 | + repo = Repo.init(tmp_path) | ||
| 36 | + remote = Remote(repo, "origin") | ||
| 37 | + with mock.patch.object(Git, "_call_process") as run, mock.patch.object( | ||
| 38 | + Remote, "_get_fetch_info_from_stderr", return_value=[] | ||
| 39 | + ): | ||
| 40 | + remote.pull("refs/heads/topic", upload_pack="helper with spaces", allow_unsafe_options=True) | ||
| 41 | + assert run.call_args[0] == ("pull", "--", remote, ["refs/heads/topic"]) | ||
| 42 | + assert run.call_args[1]["upload_pack"] == "helper with spaces" | ||
| 43 | + | ||
| 44 | + | ||
| 45 | + def test_delete_head_cannot_override_force(tmp_path): | ||
| 46 | + repo = Repo.init(tmp_path) | ||
| 47 | + actor = Actor("Test", "test@example.com") | ||
| 48 | + initial = repo.index.commit("initial", author=actor, committer=actor) | ||
| 49 | + branch = repo.create_head("unmerged", initial) | ||
| 50 | + branch.commit = repo.index.commit("unmerged", head=False, author=actor, committer=actor) | ||
| 51 | + with pytest.raises(GitCommandError): | ||
| 52 | + repo.delete_head("--force", branch, force=False) | ||
| 53 | + assert branch.is_valid() | ||
| 54 | + repo.delete_head(branch, force=True) | ||
| 55 | + assert not branch.is_valid() | ||
| 56 | + | ||
| 57 | + | ||
| 58 | + def test_rename_head_cannot_select_current_branch(tmp_path): | ||
| 59 | + repo = Repo.init(tmp_path) | ||
| 60 | + actor = Actor("Test", "test@example.com") | ||
| 61 | + repo.index.commit("initial", author=actor, committer=actor) | ||
| 62 | + original = repo.active_branch.name | ||
| 63 | + with pytest.raises(GitCommandError): | ||
| 64 | + Head(repo, "refs/heads/--force").rename("renamed") | ||
| 65 | + assert repo.active_branch.name == original | ||
| 66 | + | ||
| 67 | + | ||
| 68 | + def test_tag_operands_follow_option_terminator(tmp_path): | ||
| 69 | + repo = Repo.init(tmp_path) | ||
| 70 | + with mock.patch.object(Git, "_call_process") as run: | ||
| 71 | + TagReference.create(repo, "topic", "HEAD") | ||
| 72 | + assert run.call_args[0] == ("tag", "--", "topic", "HEAD") | ||
| 73 | + TagReference.delete(repo, "--list") | ||
| 74 | + assert run.call_args[0] == ("tag", "-d", "--", "--list") | ||
| 75 | + | ||
| 76 | + | ||
| 77 | + def test_remote_ref_delete_preserves_operand(tmp_path): | ||
| 78 | + repo = Repo.init(tmp_path) | ||
| 79 | + ref = RemoteReference(repo, "refs/remotes/--force") | ||
| 80 | + with mock.patch.object(Git, "_call_process") as run: | ||
| 81 | + RemoteReference.delete(repo, ref) | ||
| 82 | + assert run.call_args[0] == ("branch", "-d", "-r", "--", ref) | ||
| 83 | + | ||
| 84 | + | ||
| 85 | + def test_move_treats_option_shaped_source_as_filename(tmp_path): | ||
| 86 | + repo = Repo.init(tmp_path) | ||
| 87 | + (tmp_path / "--force").write_text("literal source") | ||
| 88 | + repo.index.add(["--force"]) | ||
| 89 | + assert repo.index.move(["--force", "destination"]) == [("--force", "destination")] | ||
| 90 | + assert (tmp_path / "destination").read_text() == "literal source" | ||
| 91 | + assert not (tmp_path / "--force").exists() | ||
| 92 | + | ||
| 93 | + | ||
| 94 | + def test_move_cannot_override_overwrite_protection(tmp_path): | ||
| 95 | + repo = Repo.init(tmp_path) | ||
| 96 | + for name in ("--force", "source", "destination"): | ||
| 97 | + (tmp_path / name).write_text(name) | ||
| 98 | + repo.index.add(["--force", "source", "destination"]) | ||
| 99 | + with pytest.raises(GitCommandError): | ||
| 100 | + repo.index.move(["--force", "source", "destination"]) | ||
| 101 | + assert (tmp_path / "source").read_text() == "source" | ||
| 102 | + assert (tmp_path / "destination").read_text() == "destination" | ||
| 103 | + repo.index.move(["source", "destination"], force=True) | ||
| 104 | + assert (tmp_path / "destination").read_text() == "source" | ||
| 105 | + | ||
| 106 | + | ||
| 107 | + def test_ignored_treats_option_shaped_path_as_filename(tmp_path): | ||
| 108 | + repo = Repo.init(tmp_path) | ||
| 109 | + (tmp_path / ".gitignore").write_text("--verbose\n--arg value\n") | ||
| 110 | + assert repo.ignored("--verbose", "--arg value") == ["--verbose", "--arg value"] | ||
| 111 | + | ||
| 112 | + | ||
| 113 | + def test_move_cannot_override_dry_run(tmp_path): | ||
| 114 | + repo = Repo.init(tmp_path) | ||
| 115 | + (tmp_path / "source").write_text("source") | ||
| 116 | + repo.index.add(["source"]) | ||
| 117 | + with pytest.raises(GitCommandError): | ||
| 118 | + repo.index.move(["--no-dry-run", "source", "destination"], dry_run=True) | ||
| 119 | + assert (tmp_path / "source").read_text() == "source" | ||
| 120 | + assert not (tmp_path / "destination").exists() | ||
| 121 | + | ||
| 122 | + | ||
| 123 | + @pytest.mark.parametrize("name", ["--prune", "--all", "--upload-pack=helper"]) | ||
| 124 | + def test_remote_update_rejects_option_shaped_name(tmp_path, name): | ||
| 125 | + repo = Repo.init(tmp_path) | ||
| 126 | + with mock.patch.object(Git, "_call_process", side_effect=AssertionError("Git must not run")) as run: | ||
| 127 | + with pytest.raises(UnsafeOptionError): | ||
| 128 | + Remote(repo, name).update() | ||
| 129 | + run.assert_not_called() | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments