safe.bareRepository: default to "explicit" with WITH_BREAKING_CHANGES

When an attacker can convince a user to clone a crafted repository
that contains an embedded bare repository with malicious hooks, any Git
command the user runs after entering that subdirectory will discover
the bare repository and execute the hooks. The user does not even need
to run a Git command explicitly: many shell prompts run `git status`
in the background to display branch and dirty state information, and
`git status` in turn may invoke the fsmonitor hook if so configured,
making the user vulnerable the moment they `cd` into the directory. The
`safe.bareRepository` configuration variable (introduced in 8959555cee7e
(setup_git_directory(): add an owner check for the top-level directory,
2022-03-02)) already provides protection against this attack vector by
allowing users to set it to "explicit", but the default remained "all"
for backwards compatibility.

Since Git 3.0 is the natural point to change defaults to safer
values, flip the default from "all" to "explicit" when built with
`WITH_BREAKING_CHANGES`. This means Git will refuse to work with bare
repositories that are discovered implicitly by walking up the directory
tree. Bare repositories specified via `--git-dir` or `GIT_DIR` continue
to work, and directories that look like `.git`, worktrees, or submodule
directories are unaffected (the existing `is_implicit_bare_repo()`
whitelist handles those cases).

Users who rely on implicit bare repository discovery can restore the
previous behavior by setting `safe.bareRepository=all` in their global
or system configuration.

The test for the "safe.bareRepository in the repository" scenario
needed a more involved fix: it writes a `safe.bareRepository=all`
entry into the bare repository's own config to verify that repo-local
config does not override the protected (global) setting. Previously,
`test_config -C` was used to write that entry, but its cleanup runs `git
-C <bare-repo> config --unset`, which itself fails when the default is
"explicit" and the global config has already been cleaned up. Switching
to direct git config --file access avoids going through repository
discovery entirely.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc
index af59c43f42c8e684e8c689922b77a299f0d4177e..73bb939359c72ef5d8abb6f5e8a37715dc7da31e 100644 (file)
--- a/Documentation/BreakingChanges.adoc
+++ b/Documentation/BreakingChanges.adoc
@@ -216,6 +216,30 @@ would be significant, we may decide to defer this change to a subsequent minor
 release. This evaluation will also take into account our own experience with
 how painful it is to keep Rust an optional component.
 
+* The default value of `safe.bareRepository` will change from `all` to
+  `explicit`. It is all too easy for an attacker to trick a user into cloning a
+  repository that contains an embedded bare repository with malicious hooks
+  configured. If the user enters that subdirectory and runs any Git command, Git
+  discovers the bare repository and the hooks fire. The user does not even need
+  to run a Git command explicitly: many shell prompts run `git status` in the
+  background to display branch and dirty state information, and `git status` in
+  turn may invoke the fsmonitor hook if so configured, making the user
+  vulnerable the moment they `cd` into the directory. The `safe.bareRepository`
+  configuration variable was introduced in 8959555cee (setup_git_directory():
+  add an owner check for the top-level directory, 2022-03-02) with a default of
+  `all` to preserve backwards compatibility.
++
+Changing the default to `explicit` means that Git will refuse to work with bare
+repositories that are discovered implicitly by walking up the directory tree.
+Bare repositories specified explicitly via the `--git-dir` command-line option
+or the `GIT_DIR` environment variable continue to work regardless of this
+setting. Repositories that look like a `.git` directory, a worktree, or a
+submodule directory are also unaffected.
++
+Users who rely on implicit discovery of bare repositories can restore the
+previous behavior by setting `safe.bareRepository=all` in their global or
+system configuration.
+
 === Removals
 
 * Support for grafting commits has long been superseded by git-replace(1).

diff --git a/Documentation/config/safe.adoc b/Documentation/config/safe.adoc
index 2d45c98b12d9519ac92e82fa0545045f372c8d19..5b1690aebe8f58e5af7e55cabe486e4a8219b689 100644 (file)
--- a/Documentation/config/safe.adoc
+++ b/Documentation/config/safe.adoc
@@ -2,10 +2,12 @@ safe.bareRepository::
        Specifies which bare repositories Git will work with. The currently
        supported values are:
 +
-* `all`: Git works with all bare repositories. This is the default.
+* `all`: Git works with all bare repositories. This is the default in
+  Git 2.x.
 * `explicit`: Git only works with bare repositories specified via
   the top-level `--git-dir` command-line option, or the `GIT_DIR`
-  environment variable (see linkgit:git[1]).
+  environment variable (see linkgit:git[1]). This will be the default
+  in Git 3.0.
 +
 If you do not use bare repositories in your workflow, then it may be
 beneficial to set `safe.bareRepository` to `explicit` in your global
@@ -13,6 +15,10 @@ config. This will protect you from attacks that involve cloning a
 repository that contains a bare repository and running a Git command
 within that directory.
 +
+If you use bare repositories regularly and want to preserve the current
+behavior after upgrading to Git 3.0, set `safe.bareRepository` to `all`
+in your global or system config.
++
 This config setting is only respected in protected configuration (see
 <<SCOPES>>). This prevents untrusted repositories from tampering with
 this value.

diff --git a/setup.c b/setup.c
index 7ec4427368a2a756dc7a45905209a08d12cffaba..17c0662076487e8f95111c48a4a9c96e4cc096bc 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -1485,7 +1485,11 @@ static int allowed_bare_repo_cb(const char *key, const char *value,
 
 static enum allowed_bare_repo get_allowed_bare_repo(void)
 {
+#ifdef WITH_BREAKING_CHANGES
+       enum allowed_bare_repo result = ALLOWED_BARE_REPO_EXPLICIT;
+#else
        enum allowed_bare_repo result = ALLOWED_BARE_REPO_ALL;
+#endif
        git_protected_config(allowed_bare_repo_cb, &result);
        return result;
 }

diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh
index ae7ef092abf6d9abec6aa99cb5bb362cafd091b2..1d3d19f5b476a0fcafe65740b95415f13419a244 100755 (executable)
--- a/t/t0035-safe-bare-repository.sh
+++ b/t/t0035-safe-bare-repository.sh
@@ -44,11 +44,16 @@ test_expect_success 'setup an embedded bare repo, secondary worktree and submodu
        test_path_is_dir outer-repo/.git/modules/subn
 '
 
-test_expect_success 'safe.bareRepository unset' '
+test_expect_success !WITH_BREAKING_CHANGES 'safe.bareRepository unset' '
        test_unconfig --global safe.bareRepository &&
        expect_accepted_implicit -C outer-repo/bare-repo
 '
 
+test_expect_success WITH_BREAKING_CHANGES 'safe.bareRepository unset (defaults to explicit)' '
+       test_unconfig --global safe.bareRepository &&
+       expect_rejected -C outer-repo/bare-repo
+'
+
 test_expect_success 'safe.bareRepository=all' '
        test_config_global safe.bareRepository all &&
        expect_accepted_implicit -C outer-repo/bare-repo
@@ -63,7 +68,8 @@ test_expect_success 'safe.bareRepository in the repository' '
        # safe.bareRepository must not be "explicit", otherwise
        # git config fails with "fatal: not in a git directory" (like
        # safe.directory)
-       test_config -C outer-repo/bare-repo safe.bareRepository all &&
+       test_when_finished "git config --file outer-repo/bare-repo/config --unset safe.bareRepository" &&
+       git config --file outer-repo/bare-repo/config safe.bareRepository all &&
        test_config_global safe.bareRepository explicit &&
        expect_rejected -C outer-repo/bare-repo
 '