[3/3] setup: opt-out of check with safe.directory=*

Commit Message

Derrick Stolee April 13, 2022, 3:32 p.m. UTC

From: Derrick Stolee <[e-mail]>

With the addition of the safe.directory in 8959555ce
(setup_git_directory(): add an owner check for the top-level directory,
2022-03-02) released in v2.35.2, we are receiving feedback from a
variety of users about the feature.

Some users have a very large list of shared repositories and find it
cumbersome to add this config for every one of them.

In a more difficult case, certain workflows involve running Git commands
within containers. The container boundary prevents any global or system
config from communicating `safe.directory` values from the host into the
container. Further, the container almost always runs as a different user
than the owner of the directory in the host.

To simplify the reactions necessary for these users, extend the
definition of the safe.directory config value to include a possible '*'
value. This value implies that all directories are safe, providing a
single setting to opt-out of this protection.

Note that an empty assignment of safe.directory clears all previous
values, and this is already the case with the "if (!value || !*value)"
condition.

Signed-off-by: Derrick Stolee <[e-mail]>
---
 Documentation/config/safe.txt |  7 +++++++
 setup.c                       |  6 ++++--
 t/t0033-safe-directory.sh     | 10 ++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

Comments

Junio C Hamano April 13, 2022, 4:40 p.m. UTC | #1

"Derrick Stolee via GitGitGadget" <[e-mail]> writes:

> To simplify the reactions necessary for these users, extend the
> definition of the safe.directory config value to include a possible '*'
> value. This value implies that all directories are safe, providing a
> single setting to opt-out of this protection.

OK.  During the development of the original fix, we discussed if a
more flexible mechanism, like allowing globs, but ended up with the
simplest and easiest to explain option, with the expectation that we
may want to loosen it later as necessary.  And this is certainly
what we would have expected to add.

> Note that an empty assignment of safe.directory clears all previous
> values, and this is already the case with the "if (!value || !*value)"
> condition.

OK.

>  	if (strcmp(key, "safe.directory"))
>  		return 0;
>  
> -	if (!value || !*value)
> +	if (!value || !*value) {
>  		data->is_safe = 0;
> -	else {
> +	} else if (!strcmp(value, "*")) {
> +		data->is_safe = 1;
> +	} else {
>  		const char *interpolated = NULL;
>  
>  		if (!git_config_pathname(&interpolated, key, value) &&
> diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh

OK.

> index 6f33c0dfefa..239d93f4d21 100755
> --- a/t/t0033-safe-directory.sh
> +++ b/t/t0033-safe-directory.sh
> @@ -36,4 +36,14 @@ test_expect_success 'safe.directory matches, but is reset' '
>  	expect_rejected_dir
>  '
>  
> +test_expect_success 'safe.directory=*' '
> +	git config --global --add safe.directory "*" &&
> +	git status
> +'
> +
> +test_expect_success 'safe.directory=*, but is reset' '
> +	git config --global --add safe.directory "" &&
> +	expect_rejected_dir
> +'

Thanks.

Patch

diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt
index 63597b2df8f..6d764fe0ccf 100644
--- a/Documentation/config/safe.txt
+++ b/Documentation/config/safe.txt
@@ -19,3 +19,10 @@  line option `-c safe.directory=<path>`.
 The value of this setting is interpolated, i.e. `~/<path>` expands to a
 path relative to the home directory and `%(prefix)/<path>` expands to a
 path relative to Git's (runtime) prefix.
++
+To completely opt-out of this security check, set `safe.directory` to the
+string `*`. This will allow all repositories to be treated as if their
+directory was listed in the `safe.directory` list. If `safe.directory=*`
+is set in system config and you want to re-enable this protection, then
+initialize your list with an empty value before listing the repositories
+that you deem safe.
diff --git a/setup.c b/setup.c
index a995c359c32..a42b21307f7 100644
--- a/setup.c
+++ b/setup.c
@@ -1103,9 +1103,11 @@  static int safe_directory_cb(const char *key, const char *value, void *d)
 	if (strcmp(key, "safe.directory"))
 		return 0;
 
-	if (!value || !*value)
+	if (!value || !*value) {
 		data->is_safe = 0;
-	else {
+	} else if (!strcmp(value, "*")) {
+		data->is_safe = 1;
+	} else {
 		const char *interpolated = NULL;
 
 		if (!git_config_pathname(&interpolated, key, value) &&
diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
index 6f33c0dfefa..239d93f4d21 100755
--- a/t/t0033-safe-directory.sh
+++ b/t/t0033-safe-directory.sh
@@ -36,4 +36,14 @@  test_expect_success 'safe.directory matches, but is reset' '
 	expect_rejected_dir
 '
 
+test_expect_success 'safe.directory=*' '
+	git config --global --add safe.directory "*" &&
+	git status
+'
+
+test_expect_success 'safe.directory=*, but is reset' '
+	git config --global --add safe.directory "" &&
+	expect_rejected_dir
+'
+
 test_done