From 0ab00e73066a2f0ce60d5fcd618d0ef20bec8cdc Mon Sep 17 00:00:00 2001 From: Doktor Komputer Date: Thu, 23 Jul 2026 01:39:00 +0000 Subject: [PATCH] remote: add url=/token= to fetch/push/pull, plus dest_ref for fetch Remote.fetch(), Remote.push(), and Remote.pull() could previously only operate against the remote's configured name/URL, forcing anyone who wanted to use an alternate URL (e.g. with an embedded credential) to first rewrite the remote's permanent config via set_url() -- persisting a token to .git/config as a side effect just to do a single operation. All three shell out to the underlying git transport command with the remote's name hardcoded as the target; git remote update/rename/set-url/prune etc. are not affected since those are config-only commands that don't accept an arbitrary URL to begin with. Add to Remote.fetch(), Remote.push(), and Remote.pull(): - url: use this URL for this call only. The remote's stored config is never read or written for the transport target when given; it takes precedence over the remote's name/configured url. - token: optional credential to embed into the transport target for this call only (http/https only). If url= is also given, the token is embedded into that url. If url= is omitted, the token is embedded into this remote's own *currently configured* url instead (read only, never written) -- so a caller can pass just token= on an ordinary named remote without repeating a url they already have configured, and without that token ever silently doing nothing. Never written to config or logged, and scrubbed from GitCommandError text if the call fails, so a failure can't leak the token via an exception/log message. All three methods resolve url=/token= identically via a shared Remote._resolve_transport_target() helper. Additionally, Remote.fetch() gets dest_ref (fetch-only -- push writes directly to the named destination ref regardless of url=, and pull's merge target is the checked-out branch, so neither has fetch's FETCH_HEAD-only footgun): a *bare* refspec entry (no ':dst') only updates FETCH_HEAD when fetching from an explicit url/token target, because unlike a named/configured remote, git has no remote..fetch pattern to complete a destination from. dest_ref lets a caller ask for a normal tracking ref instead: * True -> refs/remotes// * '...{branch}...' -> templated destination, applied per-entry when refspec is a list * plain string -> used verbatim (single bare entry only) Entries that already contain ':' are left untouched. Raises ValueError if given without url=/token=, or if a plain-string dest_ref is combined with more than one bare refspec entry; raises TypeError for any other dest_ref type. Existing calls (no url=/token=/dest_ref=) are unaffected on all three methods; behavior and the 'no refspec configured' assertion are unchanged for that path on fetch/pull. Adds test coverage for: one-off URL fetch/push/pull (each confirming the configured remote url/config is untouched afterwards and, for push/pull, that the operation actually took effect against the alternate target), skipping the refspec-assertion when url=/token= is given, token= rejecting non-http(s) urls (both explicit and derived-from-remote) on all three methods, token redaction in the raised GitCommandError on failure for all three, token= alone deriving and embedding into the remote's own configured url on all three methods, dest_ref=True creating a real tracking ref, a '{branch}' template applied across a list of refspecs, a plain-string dest_ref, explicit 'src:dst' refspecs being left untouched by dest_ref, and the ValueError/TypeError validation paths. --- git/remote.py | 230 +++++++++++++++++++++++++--- test/test_remote.py | 364 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 574 insertions(+), 20 deletions(-) diff --git a/git/remote.py b/git/remote.py index e2d5cbc1d..c1b290b75 100644 --- a/git/remote.py +++ b/git/remote.py @@ -10,6 +10,7 @@ import contextlib import logging import re +from urllib.parse import urlsplit, urlunsplit from git.cmd import Git, handle_process_output from git.compat import defenc, force_text @@ -1000,6 +1001,48 @@ def _assert_refspec(self) -> None: finally: config.release() + def _resolve_transport_target( + self, + url: Optional[str], + token: Optional[str], + allow_unsafe_protocols: bool, + ) -> Union[str, "Remote"]: + """Determine the one-off target to fetch/push/pull against for this call. + + Used by :meth:`fetch`, :meth:`push`, and :meth:`pull` so all three resolve + ``url``/``token`` the same way: + + - ``url`` given: use it verbatim (with ``token`` embedded into it, if given). + - ``url`` is ``None`` but ``token`` is given: read (never write) this + remote's own *currently configured* URL and embed the token into that, + so a caller can pass just ``token=`` on a normal named remote without + repeating a URL they already have configured -- still never touching + ``.git/config``. + - Neither given: return ``self`` unchanged, i.e. today's existing + behaviour (fetch/push/pull by remote name, using its configured URL as + git itself resolves it). + + :raises ValueError: + If ``token`` is given but the resolved URL is not ``http://``/``https://``. + """ + effective_url = url if url is not None else (self.url if token is not None else None) + if effective_url is None: + return self + + if not allow_unsafe_protocols: + Git.check_unsafe_protocols(effective_url) + + if token is None: + return effective_url + + parsed = urlsplit(effective_url) + if parsed.scheme not in ("http", "https"): + raise ValueError("token= is only supported with http:// or https:// urls") + netloc = f"{token}@{parsed.hostname}" + if parsed.port: + netloc += f":{parsed.port}" + return urlunsplit((parsed.scheme, netloc, parsed.path, parsed.query, parsed.fragment)) + def fetch( self, refspec: Union[str, List[str], None] = None, @@ -1008,6 +1051,9 @@ def fetch( kill_after_timeout: Union[None, float] = None, allow_unsafe_protocols: bool = False, allow_unsafe_options: bool = False, + url: Optional[str] = None, + token: Optional[str] = None, + dest_ref: Union[bool, str, None] = None, **kwargs: Any, ) -> IterableList[FetchInfo]: """Fetch the latest changes for this remote. @@ -1028,6 +1074,16 @@ def fetch( does) - supplying a list rather than a string for 'refspec' will make use of this facility. + :note: + A *bare* source name with no ``:`` (e.g. ``"main"``) only updates + ``FETCH_HEAD`` when fetching from a remote by name, because git can + complete the destination from that remote's configured fetch refspec. + When fetching from an explicit ``url`` instead (no such config exists + for an anonymous URL), a bare name updates ``FETCH_HEAD`` only and + creates or updates no local ref at all. Use ``dest_ref`` below, or + spell out ``":refs/remotes//"`` yourself, to get + a normal tracking ref out of a ``url=`` fetch. + :param progress: See the :meth:`push` method. @@ -1044,6 +1100,33 @@ def fetch( :param allow_unsafe_options: Allow unsafe options to be used, like ``--upload-pack``. + :param url: + Fetch from this URL instead of the remote's configured URL, for this + call only. The remote's stored config (``.git/config``) is never + written to or read for the transport target when this is given. + Takes precedence over the remote's name/configured url. + + :param token: + Optional credential to embed in ``url`` for this call only (e.g. a + personal access token). Only used together with ``url``, and only + for ``http://``/``https://`` URLs. Never written to config, never + logged; kept out of :class:`FetchInfo`/exception text on failure. + + :param dest_ref: + Only meaningful together with ``url``. Expands any *bare* (no ``:dst``) + entry in ``refspec`` into a full ``:`` mapping before + fetching, so the fetch updates a real local ref instead of only + ``FETCH_HEAD``. Entries that already contain a ``:`` are left untouched. + + - ``True``: derive ``dst`` as ``refs/remotes//`` + for each bare entry -- i.e. behave like a normal named-remote fetch would. + - A string containing ``{branch}``: used as a template for ``dst``, e.g. + ``"refs/remotes/upstream/{branch}"``. + - A plain string with no ``{branch}``: used verbatim as ``dst``. Only valid + when ``refspec`` is a single bare name, not a list. + - ``None`` (default): no expansion; bare names behave as plain git would + (``FETCH_HEAD`` only). + :param kwargs: Additional arguments to be passed to :manpage:`git-fetch(1)`. @@ -1055,16 +1138,41 @@ def fetch( As fetch does not provide progress information to non-ttys, we cannot make it available here unfortunately as in the :meth:`push` method. """ - if refspec is None: + if url is None and token is None and refspec is None: # No argument refspec, then ensure the repo's config has a fetch refspec. self._assert_refspec() + if dest_ref is not None and url is None and token is None: + raise ValueError("dest_ref is only meaningful together with url= or token=") + kwargs = add_progress(kwargs, self.repo.git, progress) if isinstance(refspec, list): args: Sequence[Optional[str]] = refspec else: args = [refspec] + if dest_ref is not None: + + def _expand(ref: Optional[str]) -> Optional[str]: + if not ref or ":" in ref: + return ref + if dest_ref is True: + dst = f"refs/remotes/{self.name}/{ref}" + elif isinstance(dest_ref, str) and "{branch}" in dest_ref: + dst = dest_ref.format(branch=ref) + elif isinstance(dest_ref, str): + if isinstance(refspec, list) and len(refspec) > 1: + raise ValueError( + "dest_ref as a plain string only supports a single bare refspec entry; " + "use '{branch}' as a template, or pass True, for multiple entries" + ) + dst = dest_ref + else: + raise TypeError("dest_ref must be True, a string, or None") + return f"{ref}:{dst}" + + args = [_expand(ref) for ref in args] + if not allow_unsafe_protocols: for ref in args: if ref: @@ -1076,10 +1184,28 @@ def fetch( 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 - ) - res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout) + # Determine the fetch target: an explicit `url=`, or a `token=` embedded into + # this remote's own configured url (read, never written), takes precedence + # over plain named-remote fetching. Nothing is ever written to .git/config. + fetch_target = self._resolve_transport_target(url, token, allow_unsafe_protocols) + + try: + proc = self.repo.git.fetch( + "--", + fetch_target, + *args, + as_process=True, + with_stdout=False, + universal_newlines=True, + v=verbose, + **kwargs, + ) + res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout) + except GitCommandError as err: + # Never let a token embedded in the URL leak into an exception message. + if token is not None and token in str(err): + err.args = tuple(a.replace(token, "***") if isinstance(a, str) else a for a in err.args) + raise if hasattr(self.repo.odb, "update_cache"): self.repo.odb.update_cache() return res @@ -1091,6 +1217,8 @@ def pull( kill_after_timeout: Union[None, float] = None, allow_unsafe_protocols: bool = False, allow_unsafe_options: bool = False, + url: Optional[str] = None, + token: Optional[str] = None, **kwargs: Any, ) -> IterableList[FetchInfo]: """Pull changes from the given branch, being the same as a fetch followed by a @@ -1111,13 +1239,28 @@ def pull( :param allow_unsafe_options: Allow unsafe options to be used, like ``--upload-pack``. + :param url: + Pull from this URL instead of the remote's configured URL, for this + call only. The remote's stored config (``.git/config``) is never + written to or read for the transport target when this is given. + Takes precedence over the remote's name/configured url. As with plain + ``git pull``, the fetched commit is merged into your currently checked + out branch regardless of the source, so use with the same care you + would use interactively. + + :param token: + Optional credential to embed in ``url`` for this call only (e.g. a + personal access token). Only used together with ``url``, and only + for ``http://``/``https://`` URLs. Never written to config, never + logged; kept out of exception text on failure. + :param kwargs: Additional arguments to be passed to :manpage:`git-pull(1)`. :return: Please see :meth:`fetch` method. """ - if refspec is None: + if url is None and token is None and refspec is None: # No argument refspec, then ensure the repo's config has a fetch refspec. self._assert_refspec() kwargs = add_progress(kwargs, self.repo.git, progress) @@ -1133,10 +1276,30 @@ def pull( 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 - ) - res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout) + # Same one-off-target mechanism as fetch(): an explicit `url` (optionally + # with an embedded `token`) takes precedence over the remote's configured + # name/url, and neither is ever written to .git/config. + # Same one-off-target resolution as fetch(): an explicit `url=`, or a + # `token=` embedded into this remote's own configured url (read, never + # written), takes precedence over plain named-remote pulling. + pull_target = self._resolve_transport_target(url, token, allow_unsafe_protocols) + + try: + proc = self.repo.git.pull( + "--", + pull_target, + refspec, + with_stdout=False, + as_process=True, + universal_newlines=True, + v=True, + **kwargs, + ) + res = self._get_fetch_info_from_stderr(proc, progress, kill_after_timeout=kill_after_timeout) + except GitCommandError as err: + if token is not None and token in str(err): + err.args = tuple(a.replace(token, "***") if isinstance(a, str) else a for a in err.args) + raise if hasattr(self.repo.odb, "update_cache"): self.repo.odb.update_cache() return res @@ -1148,6 +1311,8 @@ def push( kill_after_timeout: Union[None, float] = None, allow_unsafe_protocols: bool = False, allow_unsafe_options: bool = False, + url: Optional[str] = None, + token: Optional[str] = None, **kwargs: Any, ) -> PushInfoList: """Push changes from source branch in refspec to target branch in refspec. @@ -1180,6 +1345,18 @@ def push( :param allow_unsafe_options: Allow unsafe options to be used, like ``--receive-pack``. + :param url: + Push to this URL instead of the remote's configured URL, for this call + only. The remote's stored config (``.git/config``) is never written to + or read for the transport target when this is given. Takes precedence + over the remote's name/configured url. + + :param token: + Optional credential to embed in ``url`` for this call only (e.g. a + personal access token). Only used together with ``url``, and only for + ``http://``/``https://`` URLs. Never written to config, never logged; + kept out of exception text on failure. + :param kwargs: Additional arguments to be passed to :manpage:`git-push(1)`. @@ -1209,16 +1386,29 @@ def push( unsafe_options=self.unsafe_git_push_options, ) - proc = self.repo.git.push( - "--", - self, - refspec, - porcelain=True, - as_process=True, - universal_newlines=True, - kill_after_timeout=kill_after_timeout, - **kwargs, - ) + # Same one-off-target mechanism as fetch(): an explicit `url` (optionally + # with an embedded `token`) takes precedence over the remote's configured + # name/url, and neither is ever written to .git/config. + # Same one-off-target resolution as fetch(): an explicit `url=`, or a + # `token=` embedded into this remote's own configured url (read, never + # written), takes precedence over plain named-remote pushing. + push_target = self._resolve_transport_target(url, token, allow_unsafe_protocols) + + try: + proc = self.repo.git.push( + "--", + push_target, + refspec, + porcelain=True, + as_process=True, + universal_newlines=True, + kill_after_timeout=kill_after_timeout, + **kwargs, + ) + except GitCommandError as err: + if token is not None and token in str(err): + err.args = tuple(a.replace(token, "***") if isinstance(a, str) else a for a in err.args) + raise return self._get_push_info(proc, progress, kill_after_timeout=kill_after_timeout) @property diff --git a/test/test_remote.py b/test/test_remote.py index 505d283af..995195fd6 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -15,6 +15,7 @@ import pytest from git import ( + Actor, Commit, FetchInfo, GitCommandError, @@ -24,6 +25,7 @@ Remote, RemoteProgress, RemoteReference, + Repo, SymbolicReference, TagReference, ) @@ -35,6 +37,7 @@ TestBase, fixture, with_rw_and_rw_remote_repo, + with_rw_directory, with_rw_repo, ) @@ -1083,6 +1086,367 @@ def test_fetch_unsafe_branch_name(self, rw_repo, remote_repo): # Cleanup branch Head.delete(remote_repo, bad_branch_name) + @with_rw_directory + def test_fetch_with_explicit_url(self, rw_dir): + # A fetch with url= should be able to pull from a completely different + # location than the remote's configured url, and must not mutate the + # remote's stored config in doing so. + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("initial commit", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") + original_url = remote.url + + other_dir = osp.join(rw_dir, "other") + other = source.clone(other_dir) + other.index.commit("other-only commit", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + + res = remote.fetch(url=other_dir, refspec="master") + assert res + + # The configured remote url must be untouched. + assert remote.url == original_url + with remote.config_reader as cr: + assert cr.get("url") == original_url + + @with_rw_directory + def test_fetch_url_requires_refspec_assertion_skipped(self, rw_dir): + # Passing url= should not trigger the "remote has no refspec configured" + # assertion that a bare fetch() with no configured refspec would hit, + # since url= implies a one-off, out-of-band fetch. + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("initial commit", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") + with remote.config_writer as cw: + cw.remove_option("fetch") + + # Should not raise AssertionError even though 'fetch' refspec config is gone. + res = remote.fetch(url=source_dir, refspec="master") + assert res + + @with_rw_directory + def test_fetch_token_requires_http(self, rw_dir): + repo = Repo.init(rw_dir) + remote = Remote.create(repo, "origin", "https://example.invalid/repo.git") + with pytest.raises(ValueError, match="http"): + remote.fetch(url="git@example.invalid:repo.git", token="abc123", refspec="main") + + @with_rw_directory + def test_fetch_token_redacted_on_failure(self, rw_dir): + repo = Repo.init(rw_dir) + remote = Remote.create(repo, "origin", "https://example.invalid/repo.git") + token = "ghp_should_never_appear_in_errors" + with pytest.raises(GitCommandError) as excinfo: + remote.fetch( + url="https://example.invalid/nonexistent-repo.git", + token=token, + refspec="main", + kill_after_timeout=5, + ) + assert token not in str(excinfo.value) + assert "***" in str(excinfo.value) + + @with_rw_directory + def test_fetch_dest_ref_true(self, rw_dir): + # dest_ref=True should map a bare refspec name to a normal + # refs/remotes// tracking ref, instead of FETCH_HEAD only. + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("init", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") + + # Create devbranch only after cloning, so origin/devbranch does not yet + # exist in the clone -- proving the fetch below is what creates it. + dev = source.create_head("devbranch") + dev.checkout() + source.index.commit("dev commit", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + source.heads.master.checkout() + assert "origin/devbranch" not in [r.name for r in clone.refs] + + res = remote.fetch(url=source_dir, refspec="devbranch", dest_ref=True) + assert len(res) == 1 + assert res[0].ref.name == "origin/devbranch" + assert "origin/devbranch" in [r.name for r in clone.refs] + assert clone.commit("origin/devbranch").hexsha == dev.commit.hexsha + + @with_rw_directory + def test_fetch_dest_ref_template_with_list(self, rw_dir): + # A '{branch}' template in dest_ref applies per-entry to a list of refspecs. + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("init", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") + + for name in ("one", "two"): + source.create_head(name).checkout() + source.index.commit(name, author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + source.heads.master.checkout() + + res = remote.fetch( + url=source_dir, + refspec=["one", "two"], + dest_ref="refs/remotes/mirror/{branch}", + ) + assert {i.ref.name for i in res} == {"mirror/one", "mirror/two"} + assert clone.commit("mirror/one").hexsha == source.commit("one").hexsha + assert clone.commit("mirror/two").hexsha == source.commit("two").hexsha + + @with_rw_directory + def test_fetch_dest_ref_plain_string(self, rw_dir): + # A plain dest_ref string is used verbatim as the destination, only + # valid for a single bare refspec entry. + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("init", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + source.create_head("devbranch").checkout() + source.index.commit("dev", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + source.heads.master.checkout() + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") + + remote.fetch(url=source_dir, refspec="devbranch", dest_ref="refs/heads/local-copy") + assert "local-copy" in [h.name for h in clone.heads] + assert clone.commit("local-copy").hexsha == source.commit("devbranch").hexsha + + @with_rw_directory + def test_fetch_dest_ref_leaves_explicit_colon_refspec_untouched(self, rw_dir): + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("init", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") + + # Create devbranch only *after* cloning, so the clone has no pre-existing + # origin/devbranch ref from the initial clone -- otherwise the assertion + # below can't tell whether dest_ref created it or it was already there. + source.create_head("devbranch").checkout() + source.index.commit("dev", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + source.heads.master.checkout() + + # dest_ref must not touch a refspec that already has an explicit destination. + remote.fetch( + url=source_dir, + refspec="devbranch:refs/remotes/origin/explicit-devbranch", + dest_ref=True, + ) + assert "origin/explicit-devbranch" in [r.name for r in clone.refs] + # dest_ref's own naming ("origin/devbranch") must NOT have been created. + assert "origin/devbranch" not in [r.name for r in clone.refs] + + @with_rw_directory + def test_fetch_dest_ref_requires_url(self, rw_dir): + repo = Repo.init(rw_dir) + Remote.create(repo, "origin", "https://example.invalid/repo.git") + remote = repo.remote("origin") + with pytest.raises(ValueError, match="url="): + remote.fetch(refspec="devbranch", dest_ref=True) + + @with_rw_directory + def test_fetch_dest_ref_plain_string_rejects_multi_refspec_list(self, rw_dir): + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("init", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") + + with pytest.raises(ValueError, match="single bare refspec"): + remote.fetch(url=source_dir, refspec=["one", "two"], dest_ref="refs/heads/x") + + @with_rw_directory + def test_fetch_dest_ref_rejects_bad_type(self, rw_dir): + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("init", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") + + with pytest.raises(TypeError): + remote.fetch(url=source_dir, refspec="master", dest_ref=123) + + @with_rw_directory + def test_push_with_explicit_url(self, rw_dir): + # A push with url= should be able to push to a completely different + # location than the remote's configured url, and must not mutate the + # remote's stored config in doing so. + bare_dir = osp.join(rw_dir, "bare.git") + Repo.init(bare_dir, bare=True) + + work_dir = osp.join(rw_dir, "work") + work = Repo.init(work_dir) + work.index.commit("seed", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + remote = Remote.create(work, "origin", "https://example.invalid/should-not-be-used.git") + original_url = remote.url + + res = remote.push(url=bare_dir, refspec="master:refs/heads/master") + res.raise_if_error() + + assert remote.url == original_url + with remote.config_reader as cr: + assert cr.get("url") == original_url + + bare = Repo(bare_dir) + assert bare.commit("master").hexsha == work.commit("master").hexsha + + @with_rw_directory + def test_push_token_requires_http(self, rw_dir): + repo = Repo.init(rw_dir) + remote = Remote.create(repo, "origin", "https://example.invalid/repo.git") + with pytest.raises(ValueError, match="http"): + remote.push(url="git@example.invalid:repo.git", token="abc123", refspec="master") + + @with_rw_directory + def test_push_token_redacted_on_failure(self, rw_dir): + repo = Repo.init(rw_dir) + repo.index.commit("seed", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + remote = Remote.create(repo, "origin", "https://example.invalid/repo.git") + token = "ghp_should_never_appear_in_push_errors" + with pytest.raises(GitCommandError) as excinfo: + remote.push( + url="https://example.invalid/nonexistent-repo.git", + token=token, + refspec="master:refs/heads/master", + kill_after_timeout=5, + ) + assert token not in str(excinfo.value) + assert "***" in str(excinfo.value) + + @with_rw_directory + def test_pull_with_explicit_url(self, rw_dir): + # A pull with url= should be able to pull+merge from a completely + # different location than the remote's configured url, and must not + # mutate the remote's stored config in doing so. + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("c1", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + source.create_head("feature").checkout() + tip = source.index.commit("c2", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + source.heads.master.checkout() + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") + original_url = remote.url + before = clone.head.commit.hexsha + + res = remote.pull(url=source_dir, refspec="feature") + assert res + assert clone.head.commit.hexsha != before + assert clone.head.commit.hexsha == tip.hexsha + + assert remote.url == original_url + with remote.config_reader as cr: + assert cr.get("url") == original_url + + @with_rw_directory + def test_pull_token_requires_http(self, rw_dir): + repo = Repo.init(rw_dir) + remote = Remote.create(repo, "origin", "https://example.invalid/repo.git") + with pytest.raises(ValueError, match="http"): + remote.pull(url="git@example.invalid:repo.git", token="abc123", refspec="main") + + @with_rw_directory + def test_pull_token_redacted_on_failure(self, rw_dir): + repo = Repo.init(rw_dir) + remote = Remote.create(repo, "origin", "https://example.invalid/repo.git") + token = "ghp_should_never_appear_in_pull_errors" + with pytest.raises(GitCommandError) as excinfo: + remote.pull( + url="https://example.invalid/nonexistent-repo.git", + token=token, + refspec="main", + kill_after_timeout=5, + ) + assert token not in str(excinfo.value) + assert "***" in str(excinfo.value) + + @with_rw_directory + def test_fetch_token_only_derives_configured_url(self, rw_dir): + # token= with no url= should embed the token into THIS remote's own + # currently configured url (read only, never written), rather than being + # silently ignored or requiring the caller to repeat the url they + # already have configured. + repo = Repo.init(rw_dir) + remote = Remote.create(repo, "origin", "https://example.invalid/org/repo.git") + original_url = remote.url + token = "ghp_should_be_embedded_not_ignored" + + with pytest.raises(GitCommandError) as excinfo: + remote.fetch(refspec="master", token=token, kill_after_timeout=5) + cmdline = str(excinfo.value) + assert "example.invalid/org/repo.git" in cmdline + assert token not in cmdline + assert "***@example.invalid" in cmdline + # Never written to config. + assert remote.url == original_url + + @with_rw_directory + def test_push_token_only_derives_configured_url(self, rw_dir): + repo = Repo.init(rw_dir) + repo.index.commit("seed", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + remote = Remote.create(repo, "origin", "https://example.invalid/org/repo.git") + original_url = remote.url + token = "ghp_should_be_embedded_not_ignored" + + with pytest.raises(GitCommandError) as excinfo: + remote.push("master", token=token, kill_after_timeout=5) + cmdline = str(excinfo.value) + assert "example.invalid/org/repo.git" in cmdline + assert token not in cmdline + assert "***@example.invalid" in cmdline + assert remote.url == original_url + + @with_rw_directory + def test_pull_token_only_derives_configured_url(self, rw_dir): + repo = Repo.init(rw_dir) + remote = Remote.create(repo, "origin", "https://example.invalid/org/repo.git") + original_url = remote.url + token = "ghp_should_be_embedded_not_ignored" + + with pytest.raises(GitCommandError) as excinfo: + remote.pull(refspec="master", token=token, kill_after_timeout=5) + cmdline = str(excinfo.value) + assert "example.invalid/org/repo.git" in cmdline + assert token not in cmdline + assert "***@example.invalid" in cmdline + assert remote.url == original_url + + @with_rw_directory + def test_fetch_token_only_non_http_configured_url_rejected(self, rw_dir): + # If the remote's own configured url isn't http(s) (e.g. an ssh or local + # path url), token= with no url= must raise rather than silently + # embedding a token into a non-http(s) url. + source_dir = osp.join(rw_dir, "source") + source = Repo.init(source_dir) + source.index.commit("init", author=Actor("t", "t@t.com"), committer=Actor("t", "t@t.com")) + + clone_dir = osp.join(rw_dir, "clone") + clone = source.clone(clone_dir) + remote = clone.remote("origin") # configured url is a plain local path + + with pytest.raises(ValueError, match="http"): + remote.fetch(refspec="master", token="abc123") + class TestTimeouts(TestBase): @with_rw_repo("HEAD", bare=False)