safe.directory: normalize the checked 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/repo is safe would be rejected if the safe.directory configuration allows /home/u/repository/ but not its alias /home/u/repo/. Normalize the path being checked before comparing with safe.directory value(s). Suggested-by: Phillip Wood <phillip.wood123@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed Jul 30, 2024 at 11:43 UTC 7f547c99a627bca120bf44abf3dd95c8837dfdfa
2 files changed +69 -4
setup.c
+12 -4
@@ -1215,7 +1215,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
1215 }
1216
1217 struct safe_directory_data {
1218 - const char *path;
1218 + char *path;
1219 int is_safe;
1220 };
1221
@@ -1261,9 +1261,7 @@ static int ensure_valid_ownership(const char *gitfile,
1261 const char *worktree, const char *gitdir,
1262 struct strbuf *report)
1263 {
1264 - struct safe_directory_data data = {
1265 - .path = worktree ? worktree : gitdir
1266 - };
1264 + struct safe_directory_data data = { 0 };
1265
1266 if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
1267 (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
@@ -1271,6 +1269,15 @@ static int ensure_valid_ownership(const char *gitfile,
1269 (!gitdir || is_path_owned_by_current_user(gitdir, report)))
1270 return 1;
1271
1272 + /*
1273 + * normalize the data.path for comparison with normalized paths
1274 + * that come from the configuration file. The path is unsafe
1275 + * if it cannot be normalized.
1276 + */
1277 + data.path = real_pathdup(worktree ? worktree : gitdir, 0);
1278 + if (!data.path)
1279 + return 0;
1280 +
1281 /*
1282 * data.path is the "path" that identifies the repository and it is
1283 * constant regardless of what failed above. data.is_safe should be
@@ -1278,6 +1285,7 @@ static int ensure_valid_ownership(const char *gitfile,
1285 */
1286 git_protected_config(safe_directory_cb, &data);
1287
1288 + free(data.path);
1289 return data.is_safe;
1290 }
1291
t/t0033-safe-directory.sh
+57
@@ -119,4 +119,61 @@ test_expect_success 'local clone of unowned repo accepted in safe directory' '
119 test_path_is_dir target
120 '
121
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_done