Skip to content

Commit dc0edbb

Browse files
committed
safe.directory: normalize the configured path
The pathname of a repository comes from getcwd() and it could be a path aliased via symbolic links, e.g., the real directory may be /home/u/repository but a symbolic link /home/u/repo may point at it, and the clone request may come as "git clone file:///home/u/repo/" A request to check if /home/u/repository is safe would be rejected if the safe.directory configuration allows /home/u/repo/ but not its alias /home/u/repository/. Normalize the paths configured for the safe.directory configuration variable before comparing them with the path being checked. Two and a half things to note, compared to the previous step to normalize the actual path of the suspected repository, are: - A configured safe.directory may be coming from .gitignore in the home directory that may be shared across machines. The path meant to match with an entry may not necessarily exist on all of such machines, so not being able to convert them to real path on this machine is *not* a condition that is worthy of warning. Hence, we ignore a path that cannot be converted to a real path. - A configured safe.directory is essentially a random string that user throws at us, written completely unrelated to the directory the current process happens to be in. Hence it makes little sense to give a non-absolute path. Hence we ignore any non-absolute paths, except for ".". - The safe.directory set to "." was once advertised on the list as a valid workaround for the regression caused by the overly tight safe.directory check introduced in 2.45.1; we treat it to mean "if we are at the top level of a repository, it is OK". (cf. <[email protected]>). Suggested-by: Phillip Wood <[email protected]> Helped-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 7f547c9 commit dc0edbb

File tree

2 files changed

+91
-4
lines changed

2 files changed

+91
-4
lines changed

setup.c

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,13 +1235,43 @@ static int safe_directory_cb(const char *key, const char *value,
12351235
char *allowed = NULL;
12361236

12371237
if (!git_config_pathname(&allowed, key, value)) {
1238-
if (ends_with(allowed, "/*")) {
1239-
size_t len = strlen(allowed);
1240-
if (!fspathncmp(allowed, data->path, len - 1))
1238+
char *normalized = NULL;
1239+
1240+
/*
1241+
* Setting safe.directory to a non-absolute path
1242+
* makes little sense---it won't be relative to
1243+
* the configuration file the item is defined in.
1244+
* Except for ".", which means "if we are at the top
1245+
* level of a repository, then it is OK", which is
1246+
* slightly tighter than "*" that allows discovery.
1247+
*/
1248+
if (!is_absolute_path(allowed) && strcmp(allowed, ".")) {
1249+
warning(_("safe.directory '%s' not absolute"),
1250+
allowed);
1251+
goto next;
1252+
}
1253+
1254+
/*
1255+
* A .gitconfig in $HOME may be shared across
1256+
* different machines and safe.directory entries
1257+
* may or may not exist as paths on all of these
1258+
* machines. In other words, it is not a warning
1259+
* worthy event when there is no such path on this
1260+
* machine---the entry may be useful elsewhere.
1261+
*/
1262+
normalized = real_pathdup(allowed, 0);
1263+
if (!normalized)
1264+
goto next;
1265+
1266+
if (ends_with(normalized, "/*")) {
1267+
size_t len = strlen(normalized);
1268+
if (!fspathncmp(normalized, data->path, len - 1))
12411269
data->is_safe = 1;
1242-
} else if (!fspathcmp(data->path, allowed)) {
1270+
} else if (!fspathcmp(data->path, normalized)) {
12431271
data->is_safe = 1;
12441272
}
1273+
next:
1274+
free(normalized);
12451275
free(allowed);
12461276
}
12471277
}

t/t0033-safe-directory.sh

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,61 @@ test_expect_success SYMLINKS 'checked leading paths are normalized' '
176176
git -C repo/s/.git/ for-each-ref
177177
'
178178

179+
test_expect_success SYMLINKS 'configured paths are normalized' '
180+
test_when_finished "rm -rf repository; rm -f repo" &&
181+
(
182+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
183+
git config --global --unset-all safe.directory
184+
) &&
185+
git init repository &&
186+
ln -s repository repo &&
187+
(
188+
cd repository &&
189+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
190+
test_commit sample
191+
) &&
192+
193+
(
194+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
195+
git config --global safe.directory "$(pwd)/repo"
196+
) &&
197+
git -C repository for-each-ref &&
198+
git -C repository/ for-each-ref &&
199+
git -C repo for-each-ref &&
200+
git -C repo/ for-each-ref &&
201+
test_must_fail git -C repository/.git for-each-ref &&
202+
test_must_fail git -C repository/.git/ for-each-ref &&
203+
test_must_fail git -C repo/.git for-each-ref &&
204+
test_must_fail git -C repo/.git/ for-each-ref
205+
'
206+
207+
test_expect_success SYMLINKS 'configured leading paths are normalized' '
208+
test_when_finished "rm -rf repository; rm -f repo" &&
209+
(
210+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
211+
git config --global --unset-all safe.directory
212+
) &&
213+
mkdir -p repository &&
214+
git init repository/s &&
215+
ln -s repository repo &&
216+
(
217+
cd repository/s &&
218+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
219+
test_commit sample
220+
) &&
221+
222+
(
223+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
224+
git config --global safe.directory "$(pwd)/repo/*"
225+
) &&
226+
git -C repository/s for-each-ref &&
227+
git -C repository/s/ for-each-ref &&
228+
git -C repository/s/.git for-each-ref &&
229+
git -C repository/s/.git/ for-each-ref &&
230+
git -C repo/s for-each-ref &&
231+
git -C repo/s/ for-each-ref &&
232+
git -C repo/s/.git for-each-ref &&
233+
git -C repo/s/.git/ for-each-ref
234+
'
235+
179236
test_done

0 commit comments

Comments
 (0)