| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent cc02e45 commit 1a2117c
3 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -962,6 +962,14 @@ def check_unsafe_protocols(cls, url: str) -> None: | |||
| 962 | 962 | f"The `{protocol}` protocol looks suspicious, use `allow_unsafe_protocols=True` to allow it." | |
| 963 | 963 | ) | |
| 964 | 964 | ||
| 965 | + def _check_unsafe_protocols_in_args(self, args: Sequence[Any], kwargs: Mapping[str, Any]) -> None: | ||
| 966 | + """Check positional operands and standalone values in rendered command options. | ||
| 967 | + | ||
| 968 | + A short flag's split value can become the repository operand before ``--``. | ||
| 969 | + """ | ||
| 970 | + for arg in self._unpack_args(args) + self.transform_kwargs(**kwargs): | ||
| 971 | + self.check_unsafe_protocols(arg) | ||
| 972 | + | ||
| 965 | 973 | @classmethod | |
| 966 | 974 | def _canonicalize_option_name(cls, option: str) -> str: | |
| 967 | 975 | """Return the option name used for unsafe-option checks. | |
@@ -1148,12 +1156,7 @@ def ls_remote( | |||
| 1148 | 1156 | candidate_options = self._option_candidates(args, kwargs) | |
| 1149 | 1157 | Git.check_unsafe_options(options=candidate_options, unsafe_options=self.unsafe_git_ls_remote_options) | |
| 1150 | 1158 | if not allow_unsafe_protocols: | |
| 1151 | - protocol_args = list(args) | ||
| 1152 | - if kwargs.get("split_single_char_options", True): | ||
| 1153 | - # Split short-option values can become the URL after parsing earlier options. | ||
| 1154 | - protocol_args.extend(value for key, value in kwargs.items() if len(key) == 1) | ||
| 1155 | - for arg in self._unpack_args(protocol_args): | ||
| 1156 | - self.check_unsafe_protocols(arg) | ||
| 1159 | + self._check_unsafe_protocols_in_args(args, kwargs) | ||
| 1157 | 1160 | return self._call_process("ls_remote", *args, **kwargs) | |
| 1158 | 1161 | ||
| 1159 | 1162 | @property | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1073,9 +1073,7 @@ def fetch( | |||
| 1073 | 1073 | args = [refspec] | |
| 1074 | 1074 | ||
| 1075 | 1075 | if not allow_unsafe_protocols: | |
| 1076 | - for ref in args: | ||
| 1077 | - if ref: | ||
| 1078 | - Git.check_unsafe_protocols(ref) | ||
| 1076 | + self.repo.git._check_unsafe_protocols_in_args([self, *args], kwargs) | ||
| 1079 | 1077 | ||
| 1080 | 1078 | if not allow_unsafe_options: | |
| 1081 | 1079 | Git.check_unsafe_options( | |
@@ -1084,7 +1082,7 @@ def fetch( | |||
| 1084 | 1082 | ) | |
| 1085 | 1083 | ||
| 1086 | 1084 | proc = self.repo.git.fetch( | |
| 1087 | - "--", self, *args, as_process=True, with_stdout=False, universal_newlines=True, v=verbose, **kwargs | ||
| 1085 | + "--", self, *args, as_process=True, with_stdout=False, universal_newlines=True, v=bool(verbose), **kwargs | ||
| 1088 | 1086 | ) | |
| 1089 | 1087 | res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout) | |
| 1090 | 1088 | if hasattr(self.repo.odb, "update_cache"): | |
@@ -1138,8 +1136,7 @@ def pull( | |||
| 1138 | 1136 | if operand.startswith("-"): | |
| 1139 | 1137 | raise UnsafeOptionError("Remote names and pull refspecs must not start with '-'.") | |
| 1140 | 1138 | if not allow_unsafe_protocols: | |
| 1141 | - for ref in refspec: | ||
| 1142 | - Git.check_unsafe_protocols(ref) | ||
| 1139 | + self.repo.git._check_unsafe_protocols_in_args([self, *refspec], kwargs) | ||
| 1143 | 1140 | ||
| 1144 | 1141 | if not allow_unsafe_options: | |
| 1145 | 1142 | Git.check_unsafe_options( | |
@@ -1214,8 +1211,7 @@ def push( | |||
| 1214 | 1211 | ||
| 1215 | 1212 | refspec = Git._unpack_args(refspec or []) | |
| 1216 | 1213 | if not allow_unsafe_protocols: | |
| 1217 | - for ref in refspec: | ||
| 1218 | - Git.check_unsafe_protocols(ref) | ||
| 1214 | + self.repo.git._check_unsafe_protocols_in_args([self, *refspec], kwargs) | ||
| 1219 | 1215 | ||
| 1220 | 1216 | if not allow_unsafe_options: | |
| 1221 | 1217 | Git.check_unsafe_options( | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -5,7 +5,7 @@ | |||
| 5 | 5 | ||
| 6 | 6 | import pytest | |
| 7 | 7 | ||
| 8 | - from git import Actor, Git, GitCommandError, Repo | ||
| 8 | + from git import Actor, Git, GitCommandError, Remote, Repo | ||
| 9 | 9 | from git.exc import UnsafeOptionError, UnsafeProtocolError | |
| 10 | 10 | ||
| 11 | 11 | ||
@@ -103,6 +103,89 @@ def test_ls_remote_unsafe_opt_ins_are_independent(): | |||
| 103 | 103 | run.assert_called_once_with([Git.GIT_PYTHON_GIT_EXECUTABLE, "ls-remote", "--upload-pack=helper", "ext::helper"]) | |
| 104 | 104 | ||
| 105 | 105 | ||
| 106 | + @pytest.mark.parametrize("method", ["fetch", "pull", "push"]) | ||
| 107 | + @pytest.mark.parametrize("allow_unsafe_options", [False, True]) | ||
| 108 | + @pytest.mark.parametrize( | ||
| 109 | + "name, kwargs", | ||
| 110 | + [ | ||
| 111 | + ("origin", {"q": "ext::helper"}), | ||
| 112 | + ("origin", {"q": "ext://helper"}), | ||
| 113 | + ("origin", {"q": [True, "ext::helper"]}), | ||
| 114 | + ("origin", {"q": (None, False, True, "custom::address")}), | ||
| 115 | + ("origin", {"-": "ext::helper"}), | ||
| 116 | + ("ext::helper", {}), | ||
| 117 | + ("ext://helper", {}), | ||
| 118 | + ], | ||
| 119 | + ) | ||
| 120 | + def test_remote_protocol_guards_check_rendered_operands(tmp_path, method, allow_unsafe_options, name, kwargs): | ||
| 121 | + with Repo.init(tmp_path) as repo: | ||
| 122 | + remote = Remote(repo, name) | ||
| 123 | + with mock.patch.object(Git, "execute", side_effect=AssertionError("Git must not run")) as run: | ||
| 124 | + with pytest.raises(UnsafeProtocolError): | ||
| 125 | + getattr(remote, method)("HEAD", allow_unsafe_options=allow_unsafe_options, **kwargs) | ||
| 126 | + run.assert_not_called() | ||
| 127 | + | ||
| 128 | + | ||
| 129 | + @pytest.mark.parametrize("method", ["fetch", "pull", "push"]) | ||
| 130 | + @pytest.mark.parametrize( | ||
| 131 | + "kwargs, token", | ||
| 132 | + [ | ||
| 133 | + ({"q": True}, "-q"), | ||
| 134 | + ({"q": [None, False, True]}, "-q"), | ||
| 135 | + ({"q": "https://[::1]/repo.git"}, "https://[::1]/repo.git"), | ||
| 136 | + ({"o": "ext::literal", "split_single_char_options": False}, "-oext::literal"), | ||
| 137 | + ({"server_option": "ext::literal"}, "--server-option=ext::literal"), | ||
| 138 | + ], | ||
| 139 | + ) | ||
| 140 | + def test_remote_protocol_guards_preserve_safe_rendered_arguments(tmp_path, method, kwargs, token): | ||
| 141 | + with Repo.init(tmp_path) as repo: | ||
| 142 | + remote = Remote(repo, "origin") | ||
| 143 | + error = GitCommandError("captured command", 128) | ||
| 144 | + with mock.patch.object(Git, "execute", side_effect=error) as run: | ||
| 145 | + with pytest.raises(GitCommandError) as raised: | ||
| 146 | + getattr(remote, method)("HEAD", **kwargs) | ||
| 147 | + assert raised.value is error | ||
| 148 | + run.assert_called_once() | ||
| 149 | + argv = run.call_args[0][0] | ||
| 150 | + assert token in argv | ||
| 151 | + assert argv[-3:] == ["--", "origin", "HEAD"] | ||
| 152 | + | ||
| 153 | + | ||
| 154 | + @pytest.mark.parametrize("method", ["fetch", "pull", "push"]) | ||
| 155 | + def test_remote_protocol_and_option_opt_ins_are_independent(tmp_path, method): | ||
| 156 | + with Repo.init(tmp_path) as repo: | ||
| 157 | + remote = Remote(repo, "origin") | ||
| 158 | + kwargs = {"q": "ext::helper"} | ||
| 159 | + option = "receive_pack" if method == "push" else "upload_pack" | ||
| 160 | + error = GitCommandError("captured command", 128) | ||
| 161 | + with mock.patch.object(Git, "execute", side_effect=error) as run: | ||
| 162 | + with pytest.raises(UnsafeOptionError): | ||
| 163 | + getattr(remote, method)("HEAD", allow_unsafe_protocols=True, **kwargs, **{option: "helper"}) | ||
| 164 | + run.assert_not_called() | ||
| 165 | + | ||
| 166 | + with pytest.raises(GitCommandError) as raised: | ||
| 167 | + getattr(remote, method)("HEAD", allow_unsafe_protocols=True, **kwargs) | ||
| 168 | + assert raised.value is error | ||
| 169 | + run.assert_called_once() | ||
| 170 | + argv = run.call_args[0][0] | ||
| 171 | + assert argv[argv.index("-q") + 1] == "ext::helper" | ||
| 172 | + assert argv[-3:] == ["--", "origin", "HEAD"] | ||
| 173 | + assert not any(arg.startswith("--allow-unsafe") for arg in argv) | ||
| 174 | + | ||
| 175 | + | ||
| 176 | + @pytest.mark.parametrize("verbose", ["ext::helper", "--upload-pack=helper"]) | ||
| 177 | + def test_fetch_verbose_cannot_introduce_operands_or_options(tmp_path, verbose): | ||
| 178 | + with Repo.init(tmp_path) as repo: | ||
| 179 | + remote = Remote(repo, "origin") | ||
| 180 | + error = GitCommandError("captured command", 128) | ||
| 181 | + with mock.patch.object(Git, "execute", side_effect=error) as run: | ||
| 182 | + with pytest.raises(GitCommandError) as raised: | ||
| 183 | + remote.fetch("HEAD", verbose=verbose) | ||
| 184 | + assert raised.value is error | ||
| 185 | + run.assert_called_once() | ||
| 186 | + assert run.call_args[0][0] == [Git.GIT_PYTHON_GIT_EXECUTABLE, "fetch", "-v", "--", "origin", "HEAD"] | ||
| 187 | + | ||
| 188 | + | ||
| 106 | 189 | @pytest.mark.parametrize("allow_unsafe_options", [False, True]) | |
| 107 | 190 | @pytest.mark.parametrize( | |
| 108 | 191 | "revs, kwargs", | |
| Back | FazBrowse Home | New Git URL |
0 commit comments