Skip to content

safe mode to disable executing any external programs except git #2029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 98 additions & 1 deletion git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
CommandError,
GitCommandError,
GitCommandNotFound,
UnsafeExecutionError,
UnsafeOptionError,
UnsafeProtocolError,
)
Expand Down Expand Up @@ -398,6 +399,7 @@ class Git(metaclass=_GitMeta):

__slots__ = (
"_working_dir",
"_safe",
"cat_file_all",
"cat_file_header",
"_version_info",
Expand Down Expand Up @@ -944,17 +946,56 @@ def __del__(self) -> None:
self._stream.read(bytes_left + 1)
# END handle incomplete read

def __init__(self, working_dir: Union[None, PathLike] = None) -> None:
def __init__(self, working_dir: Union[None, PathLike] = None, safe: bool = False) -> None:
"""Initialize this instance with:
:param working_dir:
Git directory we should work in. If ``None``, we always work in the current
directory as returned by :func:`os.getcwd`.
This is meant to be the working tree directory if available, or the
``.git`` directory in case of bare repositories.
:param safe:
Lock down the configuration to make it as safe as possible
when working with publicly accessible, untrusted
repositories. This disables all known options that can run
external programs and limits networking to the HTTP protocol
via ``https://`` URLs. This might not cover Git config
options that were added since this was implemented, or
options that have unknown exploit vectors. It is a best
effort defense rather than an exhaustive protection measure.
In order to make this more likely to work with submodules,
some attempts are made to rewrite remote URLs to ``https://``
using `insteadOf` in the config. This might not work on all
projects, so submodules should always use ``https://`` URLs.
:envvar:`GIT_TERMINAL_PROMPT` is set to `false` and these
environment variables are forced to `/bin/true`:
:envvar:`GIT_ASKPASS`, :envvar:`GIT_EDITOR`,
:envvar:`GIT_PAGER`, :envvar:`GIT_SSH`,
:envvar:`GIT_SSH_COMMAND`, and :envvar:`SSH_ASKPASS`.
Git config options are supplied via the command line to set
up key parts of safe mode.
- Direct options for executing external commands are set to ``/bin/true``:
``core.askpass``, ``core.sshCommand`` and ``credential.helper``.
- External password prompts are disabled by skipping authentication using
``http.emptyAuth=true``.
- Any use of an fsmonitor daemon is disabled using ``core.fsmonitor=false``.
- Hook scripts are disabled using ``core.hooksPath=/dev/null``.
It was not possible to cover all config items that might execute an external
command, for example, ``receive.procReceiveRefs``,
``uploadpack.packObjectsHook`` and ``remote.<name>.vcs``.
"""
super().__init__()
self._working_dir = expand_path(working_dir)
self._safe = safe
self._git_options: Union[List[str], Tuple[str, ...]] = ()
self._persistent_git_options: List[str] = []

Expand Down Expand Up @@ -1201,6 +1242,8 @@ def execute(
:raise git.exc.GitCommandError:
:raise git.exc.UnsafeExecutionError:
:note:
If you add additional keyword arguments to the signature of this method, you
must update the ``execute_kwargs`` variable housed in this module.
Expand All @@ -1210,6 +1253,51 @@ def execute(
if self.GIT_PYTHON_TRACE and (self.GIT_PYTHON_TRACE != "full" or as_process):
_logger.info(" ".join(redacted_command))

if self._safe:
if isinstance(command, str) or command[0] != self.GIT_PYTHON_GIT_EXECUTABLE:
Copy link
Member

@EliahKagan EliahKagan Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To give clearer error messages, I recommend moving this check of the command type and value below the if shell check, and also splitting this command check into two checks with two separate messages.

The reason I recommend doing this after the if shell check is that, if a shell would (be attempted to) be used, then the condition here could come out True even for git commands. Technically the message is correct in that case, in that the shell is separate from the git executable, but I don't think users would understand what was going on.

The reasons I recommend splitting this into separate if isinstance(command, str) and if command[0] != self.GIT_PYTHON_GIT_EXECUTABLE checks, with two separate exception messages, are:

  • On Windows, passing a string like "git version" is unambiguously an attempt to run git with the argument version. So in that case the message would be incorrect (and very confusing, if someone is using GitPython in a Windows-only application and writing command arguments as strings).
  • It's possible for the message to be incorrect even outside Windows, because someone could pass "git" by itself (or whatever other value Git.GIT_PYTHON_GIT_EXECUTABLE has been set to), either by accident or intentionally in order to see the summary of subcommands it outputs.
  • On any system, even if the message is correct, I think it's useful to distinguish the problem of passing a string rather than a sequence from the problem of passing a sequence that attempts to execute something that isn't a Git command.

raise UnsafeExecutionError(
redacted_command,
f"Only {self.GIT_PYTHON_GIT_EXECUTABLE} can be executed when in safe mode.",
)
if shell:
raise UnsafeExecutionError(
redacted_command,
"Command cannot be executed in a shell when in safe mode.",
)
Comment on lines +1262 to +1266
Copy link
Member

@EliahKagan EliahKagan Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This blocks using a shell when the reason the shell would be used is that a shell=True argument was passed explicitly. But it fails to block using a shell when the reason is instead that Git.USE_SHELL has been set to True. See the first bullet point in #2029 (comment).

We can observe our own direct subprocess invocations (provided that they are done from Python and in the usual manner). Setting it up:

import sys
import git

def popen_hook(event, args):
    if event == "subprocess.Popen":
        print(event, args)

sys.addaudithook(popen_hook)

g = git.Git(".", safe=True)

Due to the Git instance being created with safe=True, this rightly declines to execute anything:

g.execute(["git", "version"], shell=True)  # Blocked as of 07a3889.

But this executes a shell, even though it should also decline for the same reason, in the same way, and preferably through the same logic automatically:

git.Git.USE_SHELL = True
g.execute(["git", "version"])  # NOT blocked as of 07a3889.

(This example is written in such a way that it demonstrates the problem on any platform, not just Windows, though exactly what unintended git … command is run in the shell differs across systems.)

The cause of the problem is that, at this point in the code, a None value of shell has not yet been replaced by the value of the USE_SHELL class attribute. The if shell logic should appear after this if shell is None check, instead of before:

GitPython/git/cmd.py

Lines 1343 to 1349 in 07a3889

if shell is None:
# Get the value of USE_SHELL with no deprecation warning. Do this without
# warnings.catch_warnings, to avoid a race condition with application code
# configuring warnings. The value could be looked up in type(self).__dict__
# or Git.__dict__, but those can break under some circumstances. This works
# the same as self.USE_SHELL in more situations; see Git.__getattribute__.
shell = super().__getattribute__("USE_SHELL")

Although one option would be to move the new if self._safe code lower, that code for checking if shell is None (and resetting shell to the value of the USE_SHELL class attribute) could probably instead just be moved higher.

config_args = [
"-c",
"core.askpass=/bin/true",
"-c",
"core.fsmonitor=false",
"-c",
"core.hooksPath=/dev/null",
"-c",
"core.sshCommand=/bin/true",
"-c",
"credential.helper=/bin/true",
"-c",
"http.emptyAuth=true",
"-c",
"protocol.allow=never",
"-c",
"protocol.https.allow=always",
"-c",
"url.https://bitbucket.org/[email protected]:",
"-c",
"url.https://codeberg.org/[email protected]:",
"-c",
"url.https://github.com/[email protected]:",
"-c",
"url.https://gitlab.com/[email protected]:",
"-c",
"url.https://.insteadOf=git://",
"-c",
"url.https://.insteadOf=http://",
"-c",
"url.https://.insteadOf=ssh://",
]
command = [command.pop(0)] + config_args + command

# Allow the user to have the command executed in their working dir.
try:
cwd = self._working_dir or os.getcwd() # type: Union[None, str]
Expand All @@ -1227,6 +1315,15 @@ def execute(
# just to be sure.
env["LANGUAGE"] = "C"
env["LC_ALL"] = "C"
# Globally disable things that can execute commands, including password prompts.
if self._safe:
env["GIT_ASKPASS"] = "/bin/true"
env["GIT_EDITOR"] = "/bin/true"
env["GIT_PAGER"] = "/bin/true"
env["GIT_SSH"] = "/bin/true"
env["GIT_SSH_COMMAND"] = "/bin/true"
env["GIT_TERMINAL_PROMPT"] = "false"
env["SSH_ASKPASS"] = "/bin/true"
env.update(self._environment)
if inline_env is not None:
env.update(inline_env)
Expand Down
4 changes: 4 additions & 0 deletions git/exc.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ def __init__(
super().__init__(command, status, stderr, stdout)


class UnsafeExecutionError(CommandError):
"""Thrown if anything but git is executed when in safe mode."""


class CheckoutError(GitError):
"""Thrown if a file could not be checked out from the index as it contained
changes.
Expand Down
139 changes: 134 additions & 5 deletions git/repo/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ class Repo:
git_dir: PathLike
"""The ``.git`` repository directory."""

safe: None
"""Whether this is operating using restricted protocol and execution access."""

_common_dir: PathLike = ""

# Precompiled regex
Expand Down Expand Up @@ -175,6 +178,7 @@ def __init__(
odbt: Type[LooseObjectDB] = GitCmdObjectDB,
search_parent_directories: bool = False,
expand_vars: bool = True,
safe: bool = False,
) -> None:
R"""Create a new :class:`Repo` instance.

Expand Down Expand Up @@ -204,6 +208,44 @@ def __init__(
Please note that this was the default behaviour in older versions of
GitPython, which is considered a bug though.

:param safe:
Lock down the configuration to make it as safe as possible
when working with publicly accessible, untrusted
repositories. This disables all known options that can run
external programs and limits networking to the HTTP protocol
via ``https://`` URLs. This might not cover Git config
options that were added since this was implemented, or
options that have unknown exploit vectors. It is a best
effort defense rather than an exhaustive protection measure.

In order to make this more likely to work with submodules,
some attempts are made to rewrite remote URLs to ``https://``
using `insteadOf` in the config. This might not work on all
projects, so submodules should always use ``https://`` URLs.

:envvar:`GIT_TERMINAL_PROMPT` is set to `false` and these
environment variables are forced to `/bin/true`:
:envvar:`GIT_ASKPASS`, :envvar:`GIT_EDITOR`,
:envvar:`GIT_PAGER`, :envvar:`GIT_SSH`,
:envvar:`GIT_SSH_COMMAND`, and :envvar:`SSH_ASKPASS`.

Git config options are supplied via the command line to set
up key parts of safe mode.

- Direct options for executing external commands are set to ``/bin/true``:
``core.askpass``, ``core.sshCommand`` and ``credential.helper``.

- External password prompts are disabled by skipping authentication using
``http.emptyAuth=true``.

- Any use of an fsmonitor daemon is disabled using ``core.fsmonitor=false``.

- Hook scripts are disabled using ``core.hooksPath=/dev/null``.

It was not possible to cover all config items that might execute an external
command, for example, ``receive.procReceiveRefs``,
``uploadpack.packObjectsHook`` and ``remote.<name>.vcs``.

:raise git.exc.InvalidGitRepositoryError:

:raise git.exc.NoSuchPathError:
Expand Down Expand Up @@ -235,6 +277,8 @@ def __init__(
if not os.path.exists(epath):
raise NoSuchPathError(epath)

self.safe = safe

# Walk up the path to find the `.git` dir.
curpath = epath
git_dir = None
Expand Down Expand Up @@ -309,7 +353,7 @@ def __init__(
# END working dir handling

self.working_dir: PathLike = self._working_tree_dir or self.common_dir
self.git = self.GitCommandWrapperType(self.working_dir)
self.git = self.GitCommandWrapperType(self.working_dir, safe)

# Special handling, in special times.
rootpath = osp.join(self.common_dir, "objects")
Expand Down Expand Up @@ -1305,6 +1349,7 @@ def init(
mkdir: bool = True,
odbt: Type[GitCmdObjectDB] = GitCmdObjectDB,
expand_vars: bool = True,
safe: bool = False,
**kwargs: Any,
) -> "Repo":
"""Initialize a git repository at the given path if specified.
Expand All @@ -1329,6 +1374,44 @@ def init(
information disclosure, allowing attackers to access the contents of
environment variables.

:param safe:
Lock down the configuration to make it as safe as possible
when working with publicly accessible, untrusted
repositories. This disables all known options that can run
external programs and limits networking to the HTTP protocol
via ``https://`` URLs. This might not cover Git config
options that were added since this was implemented, or
options that have unknown exploit vectors. It is a best
effort defense rather than an exhaustive protection measure.

In order to make this more likely to work with submodules,
some attempts are made to rewrite remote URLs to ``https://``
using `insteadOf` in the config. This might not work on all
projects, so submodules should always use ``https://`` URLs.

:envvar:`GIT_TERMINAL_PROMPT` is set to `false` and these
environment variables are forced to `/bin/true`:
:envvar:`GIT_ASKPASS`, :envvar:`GIT_EDITOR`,
:envvar:`GIT_PAGER`, :envvar:`GIT_SSH`,
:envvar:`GIT_SSH_COMMAND`, and :envvar:`SSH_ASKPASS`.

Git config options are supplied via the command line to set
up key parts of safe mode.

- Direct options for executing external commands are set to ``/bin/true``:
``core.askpass``, ``core.sshCommand`` and ``credential.helper``.

- External password prompts are disabled by skipping authentication using
``http.emptyAuth=true``.

- Any use of an fsmonitor daemon is disabled using ``core.fsmonitor=false``.

- Hook scripts are disabled using ``core.hooksPath=/dev/null``.

It was not possible to cover all config items that might execute an external
command, for example, ``receive.procReceiveRefs``,
``uploadpack.packObjectsHook`` and ``remote.<name>.vcs``.

:param kwargs:
Keyword arguments serving as additional options to the
:manpage:`git-init(1)` command.
Expand All @@ -1342,9 +1425,9 @@ def init(
os.makedirs(path, 0o755)

# git command automatically chdir into the directory
git = cls.GitCommandWrapperType(path)
git = cls.GitCommandWrapperType(path, safe)
git.init(**kwargs)
return cls(path, odbt=odbt)
return cls(path, odbt=odbt, safe=safe)

@classmethod
def _clone(
Expand All @@ -1357,6 +1440,7 @@ def _clone(
multi_options: Optional[List[str]] = None,
allow_unsafe_protocols: bool = False,
allow_unsafe_options: bool = False,
safe: Union[bool, None] = None,
**kwargs: Any,
) -> "Repo":
odbt = kwargs.pop("odbt", odb_default_type)
Expand Down Expand Up @@ -1418,7 +1502,11 @@ def _clone(
if not osp.isabs(path):
path = osp.join(git._working_dir, path) if git._working_dir is not None else path

repo = cls(path, odbt=odbt)
# if safe is not explicitly defined, then the new Repo instance should inherit the safe value
if safe is None:
safe = git._safe

repo = cls(path, odbt=odbt, safe=safe)

# Retain env values that were passed to _clone().
repo.git.update_environment(**git.environment())
Expand Down Expand Up @@ -1501,6 +1589,7 @@ def clone_from(
multi_options: Optional[List[str]] = None,
allow_unsafe_protocols: bool = False,
allow_unsafe_options: bool = False,
safe: bool = False,
**kwargs: Any,
) -> "Repo":
"""Create a clone from the given URL.
Expand Down Expand Up @@ -1531,13 +1620,52 @@ def clone_from(
:param allow_unsafe_options:
Allow unsafe options to be used, like ``--upload-pack``.

:param safe:
Lock down the configuration to make it as safe as possible
when working with publicly accessible, untrusted
repositories. This disables all known options that can run
external programs and limits networking to the HTTP protocol
via ``https://`` URLs. This might not cover Git config
options that were added since this was implemented, or
options that have unknown exploit vectors. It is a best
effort defense rather than an exhaustive protection measure.

In order to make this more likely to work with submodules,
some attempts are made to rewrite remote URLs to ``https://``
using `insteadOf` in the config. This might not work on all
projects, so submodules should always use ``https://`` URLs.

:envvar:`GIT_TERMINAL_PROMPT` is set to `false` and these
environment variables are forced to `/bin/true`:
:envvar:`GIT_ASKPASS`, :envvar:`GIT_EDITOR`,
:envvar:`GIT_PAGER`, :envvar:`GIT_SSH`,
:envvar:`GIT_SSH_COMMAND`, and :envvar:`SSH_ASKPASS`.

Git config options are supplied via the command line to set
up key parts of safe mode.

- Direct options for executing external commands are set to ``/bin/true``:
``core.askpass``, ``core.sshCommand`` and ``credential.helper``.

- External password prompts are disabled by skipping authentication using
``http.emptyAuth=true``.

- Any use of an fsmonitor daemon is disabled using ``core.fsmonitor=false``.

- Hook scripts are disabled using ``core.hooksPath=/dev/null``.

It was not possible to cover all config items that might execute an external
command, for example, ``receive.procReceiveRefs``,
``uploadpack.packObjectsHook`` and ``remote.<name>.vcs``.

:param kwargs:
See the :meth:`clone` method.

:return:
:class:`Repo` instance pointing to the cloned directory.

"""
git = cls.GitCommandWrapperType(os.getcwd())
git = cls.GitCommandWrapperType(os.getcwd(), safe)
if env is not None:
git.update_environment(**env)
return cls._clone(
Expand All @@ -1549,6 +1677,7 @@ def clone_from(
multi_options,
allow_unsafe_protocols=allow_unsafe_protocols,
allow_unsafe_options=allow_unsafe_options,
safe=safe,
**kwargs,
)

Expand Down
Loading