From: Junio C Hamano <gitster@pobox.com>
To: Stefan Metzmacher <metze@samba.org>
Cc: git@vger.kernel.org, Derrick Stolee <stolee@gmail.com>
Subject: Re: safe.directory wildcards
Date: Wed, 29 May 2024 09:02:16 -0700 [thread overview]
Message-ID: <xmqqplt4zjw7.fsf@gitster.g> (raw)
In-Reply-To: <715163c3-8d59-46ef-81bf-1dda10e6570c@samba.org> (Stefan Metzmacher's message of "Wed, 29 May 2024 11:08:50 +0200")
Stefan Metzmacher <metze@samba.org> writes:
> given the recent importance of safe.directory, it would be great to
> have something like '/data/git/*' to be supported, not just a single '*'
> for a server that serves a lot of public git repositories owned by different owners.
Interesting.
The original commit that introduced the '*' opt-out, 0f85c4a3
(setup: opt-out of check with safe.directory=*, 2022-04-13), was
done to specifically help those who have a large list of shared
repositories. We could have moved all the way to allow globs back
then, and the possibility certainly was brought up.
https://lore.kernel.org/git/xmqqk0bt9bsb.fsf@gitster.g/
But the loosening was done in a context of "brown paper bag fix"
so it is very much understandable that we did the simplest and most
obvious thing to avoid making silly mistakes in a haste.
I am reluctant to use wildmatch() but I would expect that in
practice "leading path matches" (in other words, "everything under
this directory is OK") is sufficient, perhaps?
------- >8 ------------- >8 ------------- >8 -------
Subject: safe.directory: allow "lead/ing/path/*" match
When safe.directory was introduced in v2.30.3 timeframe, 8959555c
(setup_git_directory(): add an owner check for the top-level
directory, 2022-03-02), it only allowed specific opt-out
directories. Immediately after an embargoed release that included
the change, 0f85c4a3 (setup: opt-out of check with safe.directory=*,
2022-04-13) was done as a response to loosen the check so that a
single '*' can be used to say "I trust all repositories" for folks
who host too many repositories to list individually.
Let's further allow people to say "everything under this hierarchy
is deemed safe" by specifying such a leading directory with "/*"
appended to it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config/safe.txt | 3 ++-
setup.c | 24 +++++++++++++++++-------
t/t0033-safe-directory.sh | 15 +++++++++++++++
3 files changed, 34 insertions(+), 8 deletions(-)
diff --git c/Documentation/config/safe.txt w/Documentation/config/safe.txt
index 577df40223..2d45c98b12 100644
--- c/Documentation/config/safe.txt
+++ w/Documentation/config/safe.txt
@@ -44,7 +44,8 @@ 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.
+that you deem safe. Giving a directory with `/*` appended to it will
+allow access to all repositories under the named directory.
+
As explained, Git only allows you to access repositories owned by
yourself, i.e. the user who is running Git, by default. When Git
diff --git c/setup.c w/setup.c
index 9247cded6a..83c4ea44cd 100644
--- c/setup.c
+++ w/setup.c
@@ -1177,13 +1177,23 @@ static int safe_directory_cb(const char *key, const char *value,
} else if (!strcmp(value, "*")) {
data->is_safe = 1;
} else {
- const char *interpolated = NULL;
-
- if (!git_config_pathname(&interpolated, key, value) &&
- !fspathcmp(data->path, interpolated ? interpolated : value))
- data->is_safe = 1;
-
- free((char *)interpolated);
+ const char *allowed = NULL;
+
+ if (!git_config_pathname(&allowed, key, value)) {
+ size_t len;
+
+ if (!allowed)
+ allowed = value;
+ if (ends_with(allowed, "/*")) {
+ len = strlen(allowed);
+ if (!fspathncmp(allowed, data->path, len - 1))
+ data->is_safe = 1;
+ } else if (!fspathcmp(data->path, allowed)) {
+ data->is_safe = 1;
+ }
+ }
+ if (allowed != value)
+ free((char *)allowed);
}
return 0;
diff --git c/t/t0033-safe-directory.sh w/t/t0033-safe-directory.sh
index 11c3e8f28e..5fe61f1291 100755
--- c/t/t0033-safe-directory.sh
+++ w/t/t0033-safe-directory.sh
@@ -71,7 +71,22 @@ test_expect_success 'safe.directory=*, but is reset' '
expect_rejected_dir
'
+test_expect_success 'safe.directory with matching glob' '
+ git config --global --unset-all safe.directory &&
+ p=$(pwd) &&
+ git config --global safe.directory "${p%/*}/*" &&
+ git status
+'
+
+test_expect_success 'safe.directory with unmatching glob' '
+ git config --global --unset-all safe.directory &&
+ p=$(pwd) &&
+ git config --global safe.directory "${p%/*}no/*" &&
+ expect_rejected_dir
+'
+
test_expect_success 'safe.directory in included file' '
+ git config --global --unset-all safe.directory &&
cat >gitconfig-include <<-EOF &&
[safe]
directory = "$(pwd)"
next prev parent reply other threads:[~2024-05-29 16:02 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-29 9:08 safe.directory wildcards Stefan Metzmacher
2024-05-29 16:02 ` Junio C Hamano [this message]
2024-05-29 16:35 ` Konstantin Ryabitsev
2024-05-29 18:15 ` Stefan Metzmacher
2024-05-31 8:18 ` Patrick Steinhardt
2024-05-31 14:35 ` Chris Torek
2024-05-31 14:49 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqplt4zjw7.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=metze@samba.org \
--cc=stolee@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).