Skip to content

Commit 56cb39b

Browse files
Merge pull request #3791: Various fixes around safe.directory
The first three commits are rebased versions of those in gitgitgadget#1215. These allow the following: 1. Fix `git config --global foo.bar <path>` from allowing the `<path>`. As a bonus, users with a config value starting with `/` will not get a warning about "old-style" paths needing a "`%(prefix)/`". 2. When in WSL, the path starts with `/` so it needs to be interpolated properly. Update the warning to include `%(prefix)/` to get the right value for WSL users. (This is specifically for using Git for Windows from Git Bash, but in a WSL directory.) 3. When using WSL, the ownership check fails and reports an error message. This is noisy, and happens even if the user has marked the path with `safe.directory`. Remove that error message.
2 parents 3f8d163 + 109ae35 commit 56cb39b

File tree

4 files changed

+75
-7
lines changed

4 files changed

+75
-7
lines changed

Documentation/config/safe.txt

+7
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,10 @@ Unix' simpler permission model, it can be a bit tricky to figure out why
2525
a directory is considered unsafe. To help with this, Git will provide
2626
more detailed information when the environment variable
2727
`GIT_TEST_DEBUG_UNSAFE_DIRECTORIES` is set to `true`.
28+
+
29+
To completely opt-out of this security check, set `safe.directory` to the
30+
string `*`. This will allow all repositories to be treated as if their
31+
directory was listed in the `safe.directory` list. If `safe.directory=*`
32+
is set in system config and you want to re-enable this protection, then
33+
initialize your list with an empty value before listing the repositories
34+
that you deem safe.

compat/mingw.c

+1-3
Original file line numberDiff line numberDiff line change
@@ -3509,9 +3509,7 @@ int is_path_owned_by_current_sid(const char *path)
35093509
DACL_SECURITY_INFORMATION,
35103510
&sid, NULL, NULL, NULL, &descriptor);
35113511

3512-
if (err != ERROR_SUCCESS)
3513-
error(_("failed to get owner for '%s' (%ld)"), path, err);
3514-
else if (sid && IsValidSid(sid)) {
3512+
if (err == ERROR_SUCCESS && sid && IsValidSid(sid)) {
35153513
/* Now, verify that the SID matches the current user's */
35163514
static PSID current_user_sid;
35173515
BOOL is_member;

setup.c

+18-4
Original file line numberDiff line numberDiff line change
@@ -1100,9 +1100,14 @@ static int safe_directory_cb(const char *key, const char *value, void *d)
11001100
{
11011101
struct safe_directory_data *data = d;
11021102

1103-
if (!value || !*value)
1103+
if (strcmp(key, "safe.directory"))
1104+
return 0;
1105+
1106+
if (!value || !*value) {
11041107
data->is_safe = 0;
1105-
else {
1108+
} else if (!strcmp(value, "*")) {
1109+
data->is_safe = 1;
1110+
} else {
11061111
const char *interpolated = NULL;
11071112

11081113
if (!git_config_pathname(&interpolated, key, value) &&
@@ -1119,7 +1124,8 @@ static int ensure_valid_ownership(const char *path)
11191124
{
11201125
struct safe_directory_data data = { .path = path };
11211126

1122-
if (is_path_owned_by_current_user(path))
1127+
if (is_path_owned_by_current_user(path) &&
1128+
!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0))
11231129
return 1;
11241130

11251131
read_very_early_config(safe_directory_cb, &data);
@@ -1368,9 +1374,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
13681374
break;
13691375
case GIT_DIR_INVALID_OWNERSHIP:
13701376
if (!nongit_ok) {
1377+
struct strbuf prequoted = STRBUF_INIT;
13711378
struct strbuf quoted = STRBUF_INIT;
13721379

1373-
sq_quote_buf_pretty(&quoted, dir.buf);
1380+
#ifdef __MINGW32__
1381+
if (dir.buf[0] == '/')
1382+
strbuf_addstr(&prequoted, "%(prefix)/");
1383+
#endif
1384+
1385+
strbuf_add(&prequoted, dir.buf, dir.len);
1386+
sq_quote_buf_pretty(&quoted, prequoted.buf);
1387+
13741388
die(_("unsafe repository ('%s' is owned by someone else)\n"
13751389
"To add an exception for this directory, call:\n"
13761390
"\n"

t/t0033-safe-directory.sh

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
#!/bin/sh
2+
3+
test_description='verify safe.directory checks'
4+
5+
. ./test-lib.sh
6+
7+
GIT_TEST_ASSUME_DIFFERENT_OWNER=1
8+
export GIT_TEST_ASSUME_DIFFERENT_OWNER
9+
10+
expect_rejected_dir () {
11+
test_must_fail git status 2>err &&
12+
grep "safe.directory" err
13+
}
14+
15+
test_expect_success 'safe.directory is not set' '
16+
expect_rejected_dir
17+
'
18+
19+
test_expect_success 'safe.directory does not match' '
20+
git config --global safe.directory bogus &&
21+
expect_rejected_dir
22+
'
23+
24+
test_expect_success 'path exist as different key' '
25+
git config --global foo.bar "$(pwd)" &&
26+
expect_rejected_dir
27+
'
28+
29+
test_expect_success 'safe.directory matches' '
30+
git config --global --add safe.directory "$(pwd)" &&
31+
git status
32+
'
33+
34+
test_expect_success 'safe.directory matches, but is reset' '
35+
git config --global --add safe.directory "" &&
36+
expect_rejected_dir
37+
'
38+
39+
test_expect_success 'safe.directory=*' '
40+
git config --global --add safe.directory "*" &&
41+
git status
42+
'
43+
44+
test_expect_success 'safe.directory=*, but is reset' '
45+
git config --global --add safe.directory "" &&
46+
expect_rejected_dir
47+
'
48+
49+
test_done

0 commit comments

Comments
 (0)