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

Commit3eb7c2a

Browse files
committed
Move safer_popen from git.util to git.cmd
I had originally written it in git.util because it is used from twoseparate modules (git.cmd and git.index.fun) and is arguably thesame sort of thing as remove_password_if_present, in that bothrelate to running processes (and to security) and both are usedfrom multiple modules yet are not meant for use outside GitPython.However, all of this is also true of git.cmd.handle_process_output,which this really has more in common with: it's a utility related*only* to the use of subprocesses, while remove_password_if_presentcan be used for other sanitization. In addition, it also replacesgit.cmd.PROC_CREATIONFLAGS (also imported in git.index.fun) bypassing that to Popen on Windows (since the situations where acustom value of creationflags should be used are the same as thosewhere safer_popen should be called for its primary benefit ofavoiding an untrusted path search when creating the subprocess).safer_popen and its Windows implementation _safer_popen_windows arethus moved from git/util.py to git/cmd.py, with related changes,such as to imports, done everywhere they are needed.
1 parentc551e91 commit3eb7c2a

File tree

4 files changed

+69
-69
lines changed

4 files changed

+69
-69
lines changed

‎git/cmd.py‎

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@
2929
cygpath,
3030
expand_path,
3131
is_cygwin_git,
32+
patch_env,
3233
remove_password_if_present,
33-
safer_popen,
3434
stream_copy,
3535
)
3636

@@ -46,6 +46,7 @@
4646
Iterator,
4747
List,
4848
Mapping,
49+
Optional,
4950
Sequence,
5051
TYPE_CHECKING,
5152
TextIO,
@@ -102,7 +103,7 @@ def handle_process_output(
102103
Callable[[bytes,"Repo","DiffIndex"],None],
103104
],
104105
stderr_handler:Union[None,Callable[[AnyStr],None],Callable[[List[AnyStr]],None]],
105-
finalizer:Union[None,Callable[[Union[subprocess.Popen,"Git.AutoInterrupt"]],None]]=None,
106+
finalizer:Union[None,Callable[[Union[Popen,"Git.AutoInterrupt"]],None]]=None,
106107
decode_streams:bool=True,
107108
kill_after_timeout:Union[None,float]=None,
108109
)->None:
@@ -207,6 +208,68 @@ def pump_stream(
207208
finalizer(process)
208209

209210

211+
def_safer_popen_windows(
212+
command:Union[str,Sequence[Any]],
213+
*,
214+
shell:bool=False,
215+
env:Optional[Mapping[str,str]]=None,
216+
**kwargs:Any,
217+
)->Popen:
218+
"""Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search.
219+
220+
This avoids an untrusted search path condition where a file like ``git.exe`` in a
221+
malicious repository would be run when GitPython operates on the repository. The
222+
process using GitPython may have an untrusted repository's working tree as its
223+
current working directory. Some operations may temporarily change to that directory
224+
before running a subprocess. In addition, while by default GitPython does not run
225+
external commands with a shell, it can be made to do so, in which case the CWD of
226+
the subprocess, which GitPython usually sets to a repository working tree, can
227+
itself be searched automatically by the shell. This wrapper covers all those cases.
228+
229+
:note: This currently works by setting the ``NoDefaultCurrentDirectoryInExePath``
230+
environment variable during subprocess creation. It also takes care of passing
231+
Windows-specific process creation flags, but that is unrelated to path search.
232+
233+
:note: The current implementation contains a race condition on :attr:`os.environ`.
234+
GitPython isn't thread-safe, but a program using it on one thread should ideally
235+
be able to mutate :attr:`os.environ` on another, without unpredictable results.
236+
See comments in https://github.com/gitpython-developers/GitPython/pull/1650.
237+
"""
238+
# CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See:
239+
# https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
240+
# https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
241+
creationflags=subprocess.CREATE_NO_WINDOW|subprocess.CREATE_NEW_PROCESS_GROUP
242+
243+
# When using a shell, the shell is the direct subprocess, so the variable must be
244+
# set in its environment, to affect its search behavior. (The "1" can be any value.)
245+
ifshell:
246+
safer_env= {}ifenvisNoneelsedict(env)
247+
safer_env["NoDefaultCurrentDirectoryInExePath"]="1"
248+
else:
249+
safer_env=env
250+
251+
# When not using a shell, the current process does the search in a CreateProcessW
252+
# API call, so the variable must be set in our environment. With a shell, this is
253+
# unnecessary, in versions where https://github.com/python/cpython/issues/101283 is
254+
# patched. If not, in the rare case the ComSpec environment variable is unset, the
255+
# shell is searched for unsafely. Setting NoDefaultCurrentDirectoryInExePath in all
256+
# cases, as here, is simpler and protects against that. (The "1" can be any value.)
257+
withpatch_env("NoDefaultCurrentDirectoryInExePath","1"):
258+
returnPopen(
259+
command,
260+
shell=shell,
261+
env=safer_env,
262+
creationflags=creationflags,
263+
**kwargs,
264+
)
265+
266+
267+
ifos.name=="nt":
268+
safer_popen=_safer_popen_windows
269+
else:
270+
safer_popen=Popen
271+
272+
210273
defdashify(string:str)->str:
211274
returnstring.replace("_","-")
212275

‎git/index/fun.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,15 @@
1818
)
1919
importsubprocess
2020

21-
fromgit.cmdimporthandle_process_output
21+
fromgit.cmdimporthandle_process_output,safer_popen
2222
fromgit.compatimportdefenc,force_bytes,force_text,safe_decode
2323
fromgit.excimportHookExecutionError,UnmergedEntriesError
2424
fromgit.objects.funimport (
2525
traverse_tree_recursive,
2626
traverse_trees_recursive,
2727
tree_to_stream,
2828
)
29-
fromgit.utilimportIndexFileSHA1Writer,finalize_process,safer_popen
29+
fromgit.utilimportIndexFileSHA1Writer,finalize_process
3030
fromgitdb.baseimportIStream
3131
fromgitdb.typimportstr_tree_type
3232

‎git/util.py‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
IO,
3434
Iterator,
3535
List,
36-
Mapping,
3736
Optional,
3837
Pattern,
3938
Sequence,
@@ -536,67 +535,6 @@ def remove_password_if_present(cmdline: Sequence[str]) -> List[str]:
536535
returnnew_cmdline
537536

538537

539-
def_safer_popen_windows(
540-
command:Union[str,Sequence[Any]],
541-
*,
542-
shell:bool=False,
543-
env:Optional[Mapping[str,str]]=None,
544-
**kwargs:Any,
545-
)->subprocess.Popen:
546-
"""Call :class:`subprocess.Popen` on Windows but don't include a CWD in the search.
547-
548-
This avoids an untrusted search path condition where a file like ``git.exe`` in a
549-
malicious repository would be run when GitPython operates on the repository. The
550-
process using GitPython may have an untrusted repository's working tree as its
551-
current working directory. Some operations may temporarily change to that directory
552-
before running a subprocess. In addition, while by default GitPython does not run
553-
external commands with a shell, it can be made to do so, in which case the CWD of
554-
the subprocess, which GitPython usually sets to a repository working tree, can
555-
itself be searched automatically by the shell. This wrapper covers all those cases.
556-
557-
:note: This currently works by setting the ``NoDefaultCurrentDirectoryInExePath``
558-
environment variable during subprocess creation. It also takes care of passing
559-
Windows-specific process creation flags, but that is unrelated to path search.
560-
561-
:note: The current implementation contains a race condition on :attr:`os.environ`.
562-
GitPython isn't thread-safe, but a program using it on one thread should ideally
563-
be able to mutate :attr:`os.environ` on another, without unpredictable results.
564-
See comments in https://github.com/gitpython-developers/GitPython/pull/1650.
565-
"""
566-
# CREATE_NEW_PROCESS_GROUP is needed for some ways of killing it afterwards. See:
567-
# https://docs.python.org/3/library/subprocess.html#subprocess.Popen.send_signal
568-
# https://docs.python.org/3/library/subprocess.html#subprocess.CREATE_NEW_PROCESS_GROUP
569-
creationflags=subprocess.CREATE_NO_WINDOW|subprocess.CREATE_NEW_PROCESS_GROUP
570-
571-
# When using a shell, the shell is the direct subprocess, so the variable must be
572-
# set in its environment, to affect its search behavior. (The "1" can be any value.)
573-
ifshell:
574-
safer_env= {}ifenvisNoneelsedict(env)
575-
safer_env["NoDefaultCurrentDirectoryInExePath"]="1"
576-
else:
577-
safer_env=env
578-
579-
# When not using a shell, the current process does the search in a CreateProcessW
580-
# API call, so the variable must be set in our environment. With a shell, this is
581-
# unnecessary, in versions where https://github.com/python/cpython/issues/101283 is
582-
# patched. If not, in the rare case the ComSpec environment variable is unset, the
583-
# shell is searched for unsafely. Setting NoDefaultCurrentDirectoryInExePath in all
584-
# cases, as here, is simpler and protects against that. (The "1" can be any value.)
585-
withpatch_env("NoDefaultCurrentDirectoryInExePath","1"):
586-
returnsubprocess.Popen(
587-
command,
588-
shell=shell,
589-
env=safer_env,
590-
creationflags=creationflags,
591-
**kwargs,
592-
)
593-
594-
595-
ifos.name=="nt":
596-
safer_popen=_safer_popen_windows
597-
else:
598-
safer_popen=subprocess.Popen
599-
600538
# } END utilities
601539

602540
# { Classes

‎test/test_git.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
importddt
2626

2727
fromgitimportGit,refresh,GitCommandError,GitCommandNotFound,Repo,cmd
28-
fromgit.utilimportcwd,finalize_process,safer_popen
28+
fromgit.utilimportcwd,finalize_process
2929
fromtest.libimportTestBase,fixture_path,with_rw_directory
3030

3131

@@ -114,7 +114,6 @@ def test_it_transforms_kwargs_into_git_command_arguments(self):
114114

115115
def_do_shell_combo(self,value_in_call,value_from_class):
116116
withmock.patch.object(Git,"USE_SHELL",value_from_class):
117-
# git.cmd gets Popen via a "from" import, so patch it there.
118117
withmock.patch.object(cmd,"safer_popen",wraps=cmd.safer_popen)asmock_safer_popen:
119118
# Use a command with no arguments (besides the program name), so it runs
120119
# with or without a shell, on all OSes, with the same effect.
@@ -389,7 +388,7 @@ def test_environment(self, rw_dir):
389388
self.assertIn("FOO",str(err))
390389

391390
deftest_handle_process_output(self):
392-
fromgit.cmdimporthandle_process_output
391+
fromgit.cmdimporthandle_process_output,safer_popen
393392

394393
line_count=5002
395394
count= [None,0,0]

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp