Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

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

Sign up
Appearance settings

Commitd1297f6

Browse files
authored
Merge pull request#1198 from RyaxTech/replace-password-in-uri-by-stars
Replace password in URI by stars if present to avoid leaking secrets in logs
2 parentsd906f31 +d283c83 commitd1297f6

File tree

5 files changed

+81
-14
lines changed

5 files changed

+81
-14
lines changed

‎git/cmd.py

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
is_win,
3030
)
3131
fromgit.excimportCommandError
32-
fromgit.utilimportis_cygwin_git,cygpath,expand_path
32+
fromgit.utilimportis_cygwin_git,cygpath,expand_path,remove_password_if_present
3333

3434
from .excimport (
3535
GitCommandError,
@@ -83,8 +83,8 @@ def pump_stream(cmdline, name, stream, is_decode, handler):
8383
line=line.decode(defenc)
8484
handler(line)
8585
exceptExceptionasex:
86-
log.error("Pumping %r of cmd(%s) failed due to: %r",name,cmdline,ex)
87-
raiseCommandError(['<%s-pump>'%name]+cmdline,ex)fromex
86+
log.error("Pumping %r of cmd(%s) failed due to: %r",name,remove_password_if_present(cmdline),ex)
87+
raiseCommandError(['<%s-pump>'%name]+remove_password_if_present(cmdline),ex)fromex
8888
finally:
8989
stream.close()
9090

@@ -406,7 +406,7 @@ def read_all_from_possibly_closed_stream(stream):
406406
ifstatus!=0:
407407
errstr=read_all_from_possibly_closed_stream(self.proc.stderr)
408408
log.debug('AutoInterrupt wait stderr: %r'% (errstr,))
409-
raiseGitCommandError(self.args,status,errstr)
409+
raiseGitCommandError(remove_password_if_present(self.args),status,errstr)
410410
# END status handling
411411
returnstatus
412412
# END auto interrupt
@@ -683,8 +683,10 @@ def execute(self, command,
683683
:note:
684684
If you add additional keyword arguments to the signature of this method,
685685
you must update the execute_kwargs tuple housed in this module."""
686+
# Remove password for the command if present
687+
redacted_command=remove_password_if_present(command)
686688
ifself.GIT_PYTHON_TRACEand (self.GIT_PYTHON_TRACE!='full'oras_process):
687-
log.info(' '.join(command))
689+
log.info(' '.join(redacted_command))
688690

689691
# Allow the user to have the command executed in their working dir.
690692
cwd=self._working_diroros.getcwd()
@@ -705,7 +707,7 @@ def execute(self, command,
705707
ifis_win:
706708
cmd_not_found_exception=OSError
707709
ifkill_after_timeout:
708-
raiseGitCommandError(command,'"kill_after_timeout" feature is not supported on Windows.')
710+
raiseGitCommandError(redacted_command,'"kill_after_timeout" feature is not supported on Windows.')
709711
else:
710712
ifsys.version_info[0]>2:
711713
cmd_not_found_exception=FileNotFoundError# NOQA # exists, flake8 unknown @UndefinedVariable
@@ -720,7 +722,7 @@ def execute(self, command,
720722
ifistream:
721723
istream_ok="<valid stream>"
722724
log.debug("Popen(%s, cwd=%s, universal_newlines=%s, shell=%s, istream=%s)",
723-
command,cwd,universal_newlines,shell,istream_ok)
725+
redacted_command,cwd,universal_newlines,shell,istream_ok)
724726
try:
725727
proc=Popen(command,
726728
env=env,
@@ -736,7 +738,7 @@ def execute(self, command,
736738
**subprocess_kwargs
737739
)
738740
exceptcmd_not_found_exceptionaserr:
739-
raiseGitCommandNotFound(command,err)fromerr
741+
raiseGitCommandNotFound(redacted_command,err)fromerr
740742

741743
ifas_process:
742744
returnself.AutoInterrupt(proc,command)
@@ -786,7 +788,7 @@ def _kill_process(pid):
786788
watchdog.cancel()
787789
ifkill_check.isSet():
788790
stderr_value= ('Timeout: the command "%s" did not complete in %d '
789-
'secs.'% (" ".join(command),kill_after_timeout))
791+
'secs.'% (" ".join(redacted_command),kill_after_timeout))
790792
ifnotuniversal_newlines:
791793
stderr_value=stderr_value.encode(defenc)
792794
# strip trailing "\n"
@@ -810,7 +812,7 @@ def _kill_process(pid):
810812
proc.stderr.close()
811813

812814
ifself.GIT_PYTHON_TRACE=='full':
813-
cmdstr=" ".join(command)
815+
cmdstr=" ".join(redacted_command)
814816

815817
defas_text(stdout_value):
816818
returnnotoutput_streamandsafe_decode(stdout_value)or'<OUTPUT_STREAM>'
@@ -826,7 +828,7 @@ def as_text(stdout_value):
826828
# END handle debug printing
827829

828830
ifwith_exceptionsandstatus!=0:
829-
raiseGitCommandError(command,status,stderr_value,stdout_value)
831+
raiseGitCommandError(redacted_command,status,stderr_value,stdout_value)
830832

831833
ifisinstance(stdout_value,bytes)andstdout_as_string:# could also be output_stream
832834
stdout_value=safe_decode(stdout_value)

‎git/repo/base.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
fromgit.objectsimportSubmodule,RootModule,Commit
2626
fromgit.refsimportHEAD,Head,Reference,TagReference
2727
fromgit.remoteimportRemote,add_progress,to_progress_instance
28-
fromgit.utilimportActor,finalize_process,decygpath,hex_to_bin,expand_path
28+
fromgit.utilimportActor,finalize_process,decygpath,hex_to_bin,expand_path,remove_password_if_present
2929
importos.pathasosp
3030

3131
from .funimportrev_parse,is_git_dir,find_submodule_git_dir,touch,find_worktree_git_dir
@@ -1018,7 +1018,10 @@ def _clone(cls, git: 'Git', url: PathLike, path: PathLike, odb_default_type: Typ
10181018
finalize_process,decode_streams=False)
10191019
else:
10201020
(stdout,stderr)=proc.communicate()
1021-
log.debug("Cmd(%s)'s unused stdout: %s",getattr(proc,'args',''),stdout)
1021+
cmdline=getattr(proc,'args','')
1022+
cmdline=remove_password_if_present(cmdline)
1023+
1024+
log.debug("Cmd(%s)'s unused stdout: %s",cmdline,stdout)
10221025
finalize_process(proc,stderr=stderr)
10231026

10241027
# our git command could have a different working dir than our actual

‎git/util.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
fromsysimportmaxsize
1818
importtime
1919
fromunittestimportSkipTest
20+
fromurllib.parseimporturlsplit,urlunsplit
2021

2122
# typing ---------------------------------------------------------
2223

@@ -362,6 +363,34 @@ def expand_path(p: PathLike, expand_vars: bool = True) -> Optional[PathLike]:
362363
exceptException:
363364
returnNone
364365

366+
367+
defremove_password_if_present(cmdline):
368+
"""
369+
Parse any command line argument and if on of the element is an URL with a
370+
password, replace it by stars (in-place).
371+
372+
If nothing found just returns the command line as-is.
373+
374+
This should be used for every log line that print a command line.
375+
"""
376+
new_cmdline= []
377+
forindex,to_parseinenumerate(cmdline):
378+
new_cmdline.append(to_parse)
379+
try:
380+
url=urlsplit(to_parse)
381+
# Remove password from the URL if present
382+
ifurl.passwordisNone:
383+
continue
384+
385+
edited_url=url._replace(
386+
netloc=url.netloc.replace(url.password,"*****"))
387+
new_cmdline[index]=urlunsplit(edited_url)
388+
exceptValueError:
389+
# This is not a valid URL
390+
continue
391+
returnnew_cmdline
392+
393+
365394
#} END utilities
366395

367396
#{ Classes

‎test/test_repo.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,21 @@ def test_clone_from_with_path_contains_unicode(self):
238238
exceptUnicodeEncodeError:
239239
self.fail('Raised UnicodeEncodeError')
240240

241+
@with_rw_directory
242+
deftest_leaking_password_in_clone_logs(self,rw_dir):
243+
password="fakepassword1234"
244+
try:
245+
Repo.clone_from(
246+
url="https://fakeuser:{}@fakerepo.example.com/testrepo".format(
247+
password),
248+
to_path=rw_dir)
249+
exceptGitCommandErroraserr:
250+
assertpasswordnotinstr(err),"The error message '%s' should not contain the password"%err
251+
# Working example from a blank private project
252+
Repo.clone_from(
253+
url="https://gitlab+deploy-token-392045:mLWhVus7bjLsy8xj8q2V@gitlab.com/mercierm/test_git_python",
254+
to_path=rw_dir)
255+
241256
@with_rw_repo('HEAD')
242257
deftest_max_chunk_size(self,repo):
243258
classTestOutputStream(TestBase):

‎test/test_util.py

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@
3030
Actor,
3131
IterableList,
3232
cygpath,
33-
decygpath
33+
decygpath,
34+
remove_password_if_present,
3435
)
3536

3637

@@ -322,3 +323,20 @@ def test_pickle_tzoffset(self):
322323
t2=pickle.loads(pickle.dumps(t1))
323324
self.assertEqual(t1._offset,t2._offset)
324325
self.assertEqual(t1._name,t2._name)
326+
327+
deftest_remove_password_from_command_line(self):
328+
password="fakepassword1234"
329+
url_with_pass="https://fakeuser:{}@fakerepo.example.com/testrepo".format(password)
330+
url_without_pass="https://fakerepo.example.com/testrepo"
331+
332+
cmd_1= ["git","clone","-v",url_with_pass]
333+
cmd_2= ["git","clone","-v",url_without_pass]
334+
cmd_3= ["no","url","in","this","one"]
335+
336+
redacted_cmd_1=remove_password_if_present(cmd_1)
337+
assertpasswordnotin" ".join(redacted_cmd_1)
338+
# Check that we use a copy
339+
assertcmd_1isnotredacted_cmd_1
340+
assertpasswordin" ".join(cmd_1)
341+
assertcmd_2==remove_password_if_present(cmd_2)
342+
assertcmd_3==remove_password_if_present(cmd_3)

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp