8000 Fix #1284: strip usernames from URLs as well as passwords · akkv04/GitPython@85fe273 · GitHub
[go: up one dir, main page]
Skip to content
Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 85fe273

Browse files
glennmatthewsglennmatthews
committed
Fix gitpython-developers#1284: strip usernames from URLs as well as passwords
1 parent d5cee4a commit 85fe273

File tree

4 files changed

+46
-20
lines changed

4 files changed

+46
-20
lines changed

‎git/exc.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from gitdb.exc import BadName # NOQA @UnusedWildImport skipcq: PYL-W0401, PYL-W0614
99
from gitdb.exc import * # NOQA @UnusedWildImport skipcq: PYL-W0401, PYL-W0614
1010
from git.compat import safe_decode
11+
from git.util import remove_password_if_present
1112

1213
# typing ----------------------------------------------------
1314

@@ -54,7 +55,7 @@ def __init__(self, command: Union[List[str], Tuple[str, ...], str],
5455
stdout: Union[bytes, str, None] = None) -> None:
5556
if not isinstance(command, (tuple, list)):
5657
command = command.split()
57-
self.command = command
58+
self.command = remove_password_if_present(command)
5859
self.status = status
5960
if status:
6061
if isinstance(status, Exception):
@@ -66,8 +67,8 @@ def __init__(self, command: Union[List[str], Tuple[str, ...], str],
6667
s = safe_decode(str(status))
6768
status = "'%s'" % s if isinstance(status, str) else s
6869

69-
self._cmd = safe_decode(command[0])
70-
self._cmdline = ' '.join(safe_decode(i) for i in command)
70+
self._cmd = safe_decode(self.command[0])
71+
self._cmdline = ' '.join(safe_decode(i) for i in self.command)
7172
self._cause = status and " due to: %s" % status or "!"
7273
stdout_decode = safe_decode(stdout)
7374
stderr_decode = safe_decode(stderr)

‎git/util.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
# the BSD License: http://www.opensource.org/licenses/bsd-license.php
66

77
from abc import abstractmethod
8-
from .exc import InvalidGitRepositoryError
98
import os.path as osp
109
from .compat import is_win
1110
import contextlib
@@ -94,6 +93,8 @@ def unbare_repo(func: Callable[..., T]) -> Callable[..., T]:
9493
"""Methods with this decorator raise InvalidGitRepositoryError if they
9594
encounter a bare repository"""
9695

96+
from .exc import InvalidGitRepositoryError
97+
9798
@wraps(func)
9899
def wrapper(self: 'Remote', *args: Any, **kwargs: Any) -> T:
99100
if self.repo.bare:
@@ -412,24 +413,29 @@ def expand_path(p: Union[None, PathLike], expand_vars: bool = True) -> Optional[
412413
def remove_password_if_present(cmdline: Sequence[str]) -> List[str]:
413414
"""
414415
Parse any command line argument and if on of the element is an URL with a
415-
password, replace it by stars (in-place).
416+
username and/or password, replace them by stars (in-place).
416417
417418
If nothing found just returns the command line as-is.
418419
419-
This should be used for every log line that print a command line.
420+
This should be used for every log line that print a command line, as well as
421+
exception messages.
420422
"""
421423
new_cmdline = []
422424
for index, to_parse in enumerate(cmdline):
423425
new_cmdline.append(to_parse)
424426
try:
425427
url = urlsplit(to_parse)
426428
# Remove password from the URL if present
427-
if url.password is None:
429+
if url.password is None and url.username is None:
428430
continue
429431

430-
edited_url = url._replace(
431-
netloc=url.netloc.replace(url.password, "*****"))
432-
new_cmdline[index] = urlunsplit(edited_url)
432+
if url.password is not None:
433+
url = url._replace(
434+
netloc=url.netloc.replace(url.password, "*****"))
435+
if url.username is not None:
436+
url = url._replace(
437+
netloc=url.netloc.replace(url.username, "*****"))
438+
new_cmdline[index] = urlunsplit(url)
433439
except ValueError:
434440
# This is not a valid URL
435441
continue

‎test/test_exc.py

57AE
Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
HookExecutionError,
2323
RepositoryDirtyError,
2424
)
25+
from git.util import remove_password_if_present
2526
from test.lib import TestBase
2627

2728
import itertools as itt
@@ -34,6 +35,7 @@
3435
('cmd', 'ελληνικα', 'args'),
3536
('θνιψοδε', 'κι', 'αλλα', 'strange', 'args'),
3637
('θνιψοδε', 'κι', 'αλλα', 'non-unicode', 'args'),
38+
('git', 'clone', '-v', 'https://fakeuser:fakepassword1234@fakerepo.example.com/testrepo'),
3739
)
3840
_causes_n_substrings = (
3941
(None, None), # noqa: E241 @IgnorePep8
@@ -81,7 +83,7 @@ def test_CommandError_unicode(self, case):
8183
self.assertIsNotNone(c._msg)
8284
self.assertIn(' cmdline: ', s)
8385

84-
for a in argv:
86+
for a in remove_password_if_present(argv):
8587
self.assertIn(a, s)
8688

8789
if not cause:
@@ -137,14 +139,15 @@ def test_GitCommandNotFound(self, init_args):
137139
@ddt.data(
138140
(['cmd1'], None),
139141
(['cmd1'], "some cause"),
140-
(['cmd1'], Exception()),
142+
(['cmd1', 'https://fakeuser@fakerepo.example.com/testrepo'], Exception()),
141143
)
142144
def test_GitCommandError(self, init_args):
143145
argv, cause = init_args
144146
c = GitCommandError(argv, cause)
145147
s = str(c)
146148

147-
self.assertIn(argv[0], s)
149+
for arg in remove_password_if_present(argv):
150+
self.assertIn(arg, s)
148151
if cause:
149152
self.assertIn(' failed due to: ', s)
150153
self.assertIn(str(cause), s)

‎test/test_util.py

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -343,18 +343,34 @@ def test_pickle_tzoffset(self):
343343
self.assertEqual(t1._name, t2._name)
344344

345345
def test_remove_password_from_command_line(self):
346+
username = "fakeuser"
346347
password = "fakepassword1234"
347-
url_with_pass = "https://fakeuser:{}@fakerepo.example.com/testrepo".format(password)
348-
url_without_pass = "https://fakerepo.example.com/testrepo"
348+
url_with_user_and_pass = "https://{}:{}@fakerepo.example.com/testrepo".format(username, password)
349+
url_with_user = "https://{}@fakerepo.example.com/testrepo".format(username)
350+
url_with_pass = "https://:{}@fakerepo.example.com/testrepo".format(password)
351+
url_without_user_or_pass = "https://fakerepo.example.com/testrepo"
349352

350-
cmd_1 = ["git", "clone", "-v", url_with_pass]
351-
cmd_2 = ["git", "clone", "-v", url_without_pass]
352-
cmd_3 = ["no", "url", "in", "this", "one"]
353+
cmd_1 = ["git", "clone", "-v", url_with_user_and_pass]
354+
cmd_2 = ["git", "clone", "-v", url_with_user]
355+
cmd_3 = ["git", "clone", "-v", url_with_pass]
356+
cmd_4 = ["git", "clone", "-v", url_without_user_or_pass]
357+
cmd_5 = ["no", "url", "in", "this", "one"]
353358

354359
redacted_cmd_1 = remove_password_if_present(cmd_1)
360+
assert username not in " ".join(redacted_cmd_1)
355361
assert password not in " ".join(redacted_cmd_1)
356362
# Check that we use a copy
357363
assert cmd_1 is not redacted_cmd_1
364+
assert username in " ".join(cmd_1)
358365
assert password in " ".join(cmd_1)
359-
assert cmd_2 == remove_password_if_present(cmd_2)
360-
assert cmd_3 == remove_password_if_present(cmd_3)
366+
367+
redacted_cmd_2 = remove_password_if_present(cmd_2)
368+
assert username not in " ".join(redacted_cmd_2)
369+
assert password not in " ".join(redacted_cmd_2)
370+
371+
redacted_cmd_3 = remove_password_if_present(cmd_3)
372+
assert username not in " ".join(redacted_cmd_3)
373+
assert password not in " ".join(redacted_cmd_3)
374+
375+
assert cmd_4 == remove_password_if_present(cmd_4)
376+
assert cmd_5 == remove_password_if_present(cmd_5)

0 commit comments

Comments
 (0)
0