remote: add url=/token= to fetch/push/pull, plus dest_ref for fetch#2182
Closed
cumulus13 wants to merge 1 commit into
Closed
remote: add url=/token= to fetch/push/pull, plus dest_ref for fetch#2182cumulus13 wants to merge 1 commit into
cumulus13 wants to merge 1 commit into
Conversation
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.<name>.fetch pattern to complete a destination from.
dest_ref lets a caller ask for a normal tracking ref instead:
* True -> refs/remotes/<this-remote-name>/<branch>
* '...{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.
cumulus13
force-pushed
the
feature/remote-fetch-url-token
branch
from
July 23, 2026 20:54
908a6c9 to
0ab00e7
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends git.remote.Remote transport methods to support one-off targets without mutating a remote’s persistent .git/config, enabling temporary url= and optional token= (credential-in-URL) for fetch(), push(), and pull(), plus a dest_ref helper for fetch() to avoid FETCH_HEAD-only updates when fetching from an anonymous URL.
Changes:
- Add
_resolve_transport_target()and threadurl=/token=throughRemote.fetch(),Remote.pull(), andRemote.push(). - Add
dest_refexpansion logic inRemote.fetch()to map bare refspecs to tracking refs when using one-off targets. - Add comprehensive tests covering explicit URL usage, token validation/redaction behavior, and
dest_refbehaviors and validation.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
git/remote.py |
Adds one-off transport target resolution (url/token) for fetch/pull/push and adds dest_ref expansion for fetch. |
test/test_remote.py |
Adds test coverage for one-off URL operations, token validation/redaction, and dest_ref behaviors. |
Comments suppressed due to low confidence (4)
git/remote.py:1255
- The
tokenparameter docs say it is “Only used together with url”, butpull()supportstoken=withouturl=via_resolve_transport_target. Please align the docstring with the implemented behavior.
: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.
git/remote.py:1358
- The
tokenparameter docs say it is “Only used together with url”, butpush()supportstoken=withouturl=via_resolve_transport_target. Please align the docstring with the implemented behavior.
: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.
git/remote.py:1302
- The token redaction here only rewrites
err.args, butGitCommandError.__str__useserr.command/err._cmdlineand includes capturedstderr/stdout. If git prints the credentialed URL in stderr, the token can leak. Scrub the rendered fields directly (as infetch()).
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
git/remote.py:1411
- The token redaction here only rewrites
err.args, butGitCommandError.__str__useserr.command/err._cmdlineand includes capturedstderr/stdout. If git prints the credentialed URL in stderr, the token can leak. Scrub the rendered fields directly (as infetch()).
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
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1039
to
+1044
| 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)) |
Comment on lines
+1164
to
+1168
| 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" | ||
| ) |
Comment on lines
+1109
to
+1113
| :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. |
Comment on lines
+1204
to
+1208
| 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 |
Member
|
Thanks for contributing! However, given the size of the PR and what seems to me like a mix of unrelated features (like Thanks for your understanding. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 embedded into url's authority component for this call only (http/https only). 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.
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 anonymous url, 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:
when refspec is a list
Entries that already contain ':' are left untouched. Raises
ValueError if given without url=, 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= is given, token= rejecting non-http(s) urls on all three methods, token redaction in the raised GitCommandError on failure for all three, 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.