FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Merge pull request #2172 from gitpython-developers/fix-clone-expandvars · gitpython-developers/GitPython@c65dfb3 · GitHub

/ GitPython Public

Commit c65dfb3

Browse files
Byron
authored
Merge pull request #2172 from gitpython-developers/fix-clone-expandvars
fix: preserve literal clone URLs
2 parents 7a46dfc + 8ac5a30 commit c65dfb3

4 files changed

Lines changed: 65 additions & 17 deletions

File tree

‎git/cmd.py‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -903,29 +903,34 @@ def is_cygwin(cls) -> bool:
903903

904904
@overload
905905
@classmethod
906-
def polish_url(cls, url: str, is_cygwin: Literal[False] = ...) -> str: ...
906+
def polish_url(cls, url: str, is_cygwin: Literal[False] = ..., expand_vars: bool = ...) -> str: ...
907907

908908
@overload
909909
@classmethod
910-
def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> str: ...
910+
def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None, expand_vars: bool = True) -> str: ...
911911

912912
@classmethod
913-
def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None) -> PathLike:
913+
def polish_url(cls, url: str, is_cygwin: Union[None, bool] = None, expand_vars: bool = True) -> PathLike:
914914
"""Remove any backslashes from URLs to be written in config files.
915915
916916
Windows might create config files containing paths with backslashes, but git
917917
stops liking them as it will escape the backslashes. Hence we undo the escaping
918918
just to be sure.
919+
920+
:param expand_vars:
921+
Expand environment variables and an initial ``~``. Disable this for values
922+
obtained from an untrusted source, such as remote URLs.
919923
"""
920924
if is_cygwin is None:
921925
is_cygwin = cls.is_cygwin()
922926

923927
if is_cygwin:
924-
url = cygpath(url)
928+
url = cygpath(url, expand_vars=expand_vars)
925929
else:
926-
url = os.path.expandvars(url)
927-
if url.startswith("~"):
928-
url = os.path.expanduser(url)
930+
if expand_vars:
931+
url = os.path.expandvars(url)
932+
if url.startswith("~"):
933+
url = os.path.expanduser(url)
929934
url = url.replace("\\\\", "\\").replace("\\", "/")
930935
return url
931936

‎git/repo/base.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1452,8 +1452,9 @@ def _clone(
14521452
if multi_options:
14531453
multi = shlex.split(" ".join(multi_options))
14541454

1455+
clone_url = Git.polish_url(url, expand_vars=False)
14551456
if not allow_unsafe_protocols:
1456-
Git.check_unsafe_protocols(url)
1457+
Git.check_unsafe_protocols(clone_url)
14571458
if not allow_unsafe_options:
14581459
Git.check_unsafe_options(
14591460
options=Git._option_candidates([], kwargs),
@@ -1465,7 +1466,7 @@ def _clone(
14651466
proc = git.clone(
14661467
multi,
14671468
"--",
1468-
Git.polish_url(url),
1469+
clone_url,
14691470
clone_path,
14701471
with_extended_output=True,
14711472
as_process=True,
@@ -1505,7 +1506,7 @@ def _clone(
15051506
# escape the backslashes. Hence we undo the escaping just to be sure.
15061507
if repo.remotes:
15071508
with repo.remotes[0].config_writer as writer:
1508-
writer.set_value("url", Git.polish_url(repo.remotes[0].url))
1509+
writer.set_value("url", Git.polish_url(repo.remotes[0].url, expand_vars=False))
15091510
# END handle remote repo
15101511
return repo
15111512

‎git/util.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -382,13 +382,13 @@ def is_exec(fpath: str) -> bool:
382382
return progs
383383

384384

385-
def _cygexpath(drive: Optional[str], path: str) -> str:
385+
def _cygexpath(drive: Optional[str], path: str, expand_vars: bool = True) -> str:
386386
if osp.isabs(path) and not drive:
387387
# Invoked from `cygpath()` directly with `D:Apps\123`?
388388
# It's an error, leave it alone just slashes)
389389
p = path # convert to str if AnyPath given
390390
else:
391-
p = path and osp.normpath(osp.expandvars(osp.expanduser(path)))
391+
p = path and osp.normpath(osp.expandvars(osp.expanduser(path)) if expand_vars else path)
392392
if osp.isabs(p):
393393
if drive:
394394
# Confusing, maybe a remote system should expand vars.
@@ -416,20 +416,23 @@ def _cygexpath(drive: Optional[str], path: str) -> str:
416416
)
417417

418418

419-
def cygpath(path: str) -> str:
419+
def cygpath(path: str, expand_vars: bool = True) -> str:
420420
"""Use :meth:`git.cmd.Git.polish_url` instead, that works on any environment."""
421421
path = os.fspath(path) # Ensure is str and not AnyPath.
422422
# Fix to use Paths when 3.5 dropped. Or to be just str if only for URLs?
423423
if not path.startswith(("/cygdrive", "//", "/proc/cygdrive")):
424424
for regex, parser, recurse in _cygpath_parsers:
425425
match = regex.match(path)
426426
if match:
427-
path = parser(*match.groups())
427+
if parser is _cygexpath:
428+
path = parser(*match.groups(), expand_vars=expand_vars)
429+
else:
430+
path = parser(*match.groups())
428431
if recurse:
429-
path = cygpath(path)
432+
path = cygpath(path, expand_vars=expand_vars)
430433
break
431434
else:
432-
path = _cygexpath(None, path)
435+
path = _cygexpath(None, path, expand_vars=expand_vars)
433436

434437
return path
435438

‎test/test_clone.py‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77
import sys
88
import tempfile
99
from unittest import skip
10+
from unittest import mock
1011

11-
from git import GitCommandError, Repo
12+
from git import Git, GitCommandError, Repo
1213
from git.exc import UnsafeOptionError, UnsafeProtocolError
1314

1415
from test.lib import TestBase, with_rw_directory, with_rw_repo, PathLikeMock
@@ -346,6 +347,44 @@ def test_clone_from_unsafe_protocol(self):
346347
Repo.clone_from(url, tmp_dir / "repo")
347348
assert not tmp_file.exists()
348349

350+
def test_clone_from_does_not_expand_environment_variables_in_url(self):
351+
urls = [
352+
"https://example.com/$GITPYTHON_TEST_SECRET/repo.git",
353+
"https://example.com/${GITPYTHON_TEST_SECRET}/repo.git",
354+
"https://example.com/%GITPYTHON_TEST_SECRET%/repo.git",
355+
]
356+
with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}):
357+
for url in urls:
358+
with mock.patch.object(Git, "_call_process", side_effect=RuntimeError) as call_process:
359+
with self.assertRaises(RuntimeError):
360+
Repo.clone_from(url, "unused")
361+
362+
assert call_process.call_args[0][3] == url
363+
364+
@with_rw_directory
365+
def test_clone_from_does_not_expand_environment_variables_in_stored_url(self, rw_dir):
366+
url = pathlib.Path(rw_dir) / "$GITPYTHON_TEST_SECRET" / "source"
367+
Git().init(url)
368+
369+
with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}):
370+
cloned = Repo.clone_from(url, pathlib.Path(rw_dir) / "clone")
371+
372+
assert cloned.remotes.origin.url == Git.polish_url(str(url), expand_vars=False)
373+
374+
def test_clone_from_checks_polished_url_for_unsafe_protocol(self):
375+
with mock.patch.object(Git, "polish_url", return_value="ext::command"):
376+
with mock.patch.object(Git, "_call_process") as call_process:
377+
with self.assertRaises(UnsafeProtocolError):
378+
Repo.clone_from("$GITPYTHON_TEST_URL", "unused")
379+
380+
call_process.assert_not_called()
381+
382+
def test_polish_url_does_not_expand_environment_variables_for_cygwin(self):
383+
urls = ["$GITPYTHON_TEST_SECRET/repo", "user@example.com:$GITPYTHON_TEST_SECRET/repo"]
384+
with mock.patch.dict(os.environ, {"GITPYTHON_TEST_SECRET": "sensitive-value"}):
385+
for url in urls:
386+
assert Git.polish_url(url, is_cygwin=True, expand_vars=False) == url
387+
349388
def test_clone_from_unsafe_protocol_allowed(self):
350389
with tempfile.TemporaryDirectory() as tdir:
351390
tmp_dir = pathlib.Path(tdir)

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL