Skip to content

Commit c93649f

Browse files
gitstergitster
committed
Merge branch 'jc/safe-directory' into maint-2.46
Follow-up on 2.45.1 regression fix. * jc/safe-directory: safe.directory: setting safe.directory="." allows the "current" directory safe.directory: normalize the configured path safe.directory: normalize the checked path safe.directory: preliminary clean-up
2 parents b452be0 + ee0be85 commit c93649f

File tree

2 files changed

+225
-11
lines changed

2 files changed

+225
-11
lines changed

‎setup.c

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,7 +1215,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
12151215
}
12161216

12171217
struct safe_directory_data {
1218-
const char *path;
1218+
char *path;
12191219
int is_safe;
12201220
};
12211221

@@ -1235,17 +1235,45 @@ 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-
const char *check = allowed ? allowed : value;
1239-
if (ends_with(check, "/*")) {
1240-
size_t len = strlen(check);
1241-
if (!fspathncmp(check, 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))
12421269
data->is_safe = 1;
1243-
} else if (!fspathcmp(data->path, check)) {
1270+
} else if (!fspathcmp(data->path, normalized)) {
12441271
data->is_safe = 1;
12451272
}
1246-
}
1247-
if (allowed != value)
1273+
next:
1274+
free(normalized);
12481275
free(allowed);
1276+
}
12491277
}
12501278

12511279
return 0;
@@ -1263,23 +1291,31 @@ static int ensure_valid_ownership(const char *gitfile,
12631291
const char *worktree, const char *gitdir,
12641292
struct strbuf *report)
12651293
{
1266-
struct safe_directory_data data = {
1267-
.path = worktree ? worktree : gitdir
1268-
};
1294+
struct safe_directory_data data = { 0 };
12691295

12701296
if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
12711297
(!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
12721298
(!worktree || is_path_owned_by_current_user(worktree, report)) &&
12731299
(!gitdir || is_path_owned_by_current_user(gitdir, report)))
12741300
return 1;
12751301

1302+
/*
1303+
* normalize the data.path for comparison with normalized paths
1304+
* that come from the configuration file. The path is unsafe
1305+
* if it cannot be normalized.
1306+
*/
1307+
data.path = real_pathdup(worktree ? worktree : gitdir, 0);
1308+
if (!data.path)
1309+
return 0;
1310+
12761311
/*
12771312
* data.path is the "path" that identifies the repository and it is
12781313
* constant regardless of what failed above. data.is_safe should be
12791314
* initialized to false, and might be changed by the callback.
12801315
*/
12811316
git_protected_config(safe_directory_cb, &data);
12821317

1318+
free(data.path);
12831319
return data.is_safe;
12841320
}
12851321

‎t/t0033-safe-directory.sh

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,4 +119,182 @@ test_expect_success 'local clone of unowned repo accepted in safe directory' '
119119
test_path_is_dir target
120120
'
121121

122+
test_expect_success SYMLINKS 'checked paths are normalized' '
123+
test_when_finished "rm -rf repository; rm -f repo" &&
124+
(
125+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
126+
git config --global --unset-all safe.directory
127+
) &&
128+
git init repository &&
129+
ln -s repository repo &&
130+
(
131+
cd repository &&
132+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
133+
test_commit sample
134+
) &&
135+
136+
(
137+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
138+
git config --global safe.directory "$(pwd)/repository"
139+
) &&
140+
git -C repository for-each-ref &&
141+
git -C repository/ for-each-ref &&
142+
git -C repo for-each-ref &&
143+
git -C repo/ for-each-ref &&
144+
test_must_fail git -C repository/.git for-each-ref &&
145+
test_must_fail git -C repository/.git/ for-each-ref &&
146+
test_must_fail git -C repo/.git for-each-ref &&
147+
test_must_fail git -C repo/.git/ for-each-ref
148+
'
149+
150+
test_expect_success SYMLINKS 'checked leading paths are normalized' '
151+
test_when_finished "rm -rf repository; rm -f repo" &&
152+
(
153+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
154+
git config --global --unset-all safe.directory
155+
) &&
156+
mkdir -p repository &&
157+
git init repository/s &&
158+
ln -s repository repo &&
159+
(
160+
cd repository/s &&
161+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
162+
test_commit sample
163+
) &&
164+
165+
(
166+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
167+
git config --global safe.directory "$(pwd)/repository/*"
168+
) &&
169+
git -C repository/s for-each-ref &&
170+
git -C repository/s/ for-each-ref &&
171+
git -C repo/s for-each-ref &&
172+
git -C repo/s/ for-each-ref &&
173+
git -C repository/s/.git for-each-ref &&
174+
git -C repository/s/.git/ for-each-ref &&
175+
git -C repo/s/.git for-each-ref &&
176+
git -C repo/s/.git/ for-each-ref
177+
'
178+
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+
236+
test_expect_success 'safe.directory set to a dot' '
237+
test_when_finished "rm -rf repository" &&
238+
(
239+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
240+
git config --global --unset-all safe.directory
241+
) &&
242+
mkdir -p repository/subdir &&
243+
git init repository &&
244+
(
245+
cd repository &&
246+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
247+
test_commit sample
248+
) &&
249+
250+
(
251+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
252+
git config --global safe.directory "."
253+
) &&
254+
git -C repository for-each-ref &&
255+
git -C repository/ for-each-ref &&
256+
git -C repository/.git for-each-ref &&
257+
git -C repository/.git/ for-each-ref &&
258+
259+
# What is allowed is repository/subdir but the repository
260+
# path is repository.
261+
test_must_fail git -C repository/subdir for-each-ref &&
262+
263+
# Likewise, repository .git/refs is allowed with "." but
264+
# repository/.git that is accessed is not allowed.
265+
test_must_fail git -C repository/.git/refs for-each-ref
266+
'
267+
268+
test_expect_success 'safe.directory set to asterisk' '
269+
test_when_finished "rm -rf repository" &&
270+
(
271+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
272+
git config --global --unset-all safe.directory
273+
) &&
274+
mkdir -p repository/subdir &&
275+
git init repository &&
276+
(
277+
cd repository &&
278+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
279+
test_commit sample
280+
) &&
281+
282+
(
283+
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
284+
git config --global safe.directory "*"
285+
) &&
286+
# these are trivial
287+
git -C repository for-each-ref &&
288+
git -C repository/ for-each-ref &&
289+
git -C repository/.git for-each-ref &&
290+
git -C repository/.git/ for-each-ref &&
291+
292+
# With "*", everything is allowed, and the repository is
293+
# discovered, which is different behaviour from "." above.
294+
git -C repository/subdir for-each-ref &&
295+
296+
# Likewise.
297+
git -C repository/.git/refs for-each-ref
298+
'
299+
122300
test_done

0 commit comments

Comments
 (0)