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>

diff --git a/setup.c b/setup.c
index 3177d010d1e0826304e121e0acc3507cf5e14fa6..54cce7219b9567139f6eb4a8791b6958fe79e0d9 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -1215,7 +1215,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
 }
 
 struct safe_directory_data {
-       const char *path;
+       char *path;
        int is_safe;
 };
 
@@ -1261,9 +1261,7 @@ static int ensure_valid_ownership(const char *gitfile,
                                  const char *worktree, const char *gitdir,
                                  struct strbuf *report)
 {
-       struct safe_directory_data data = {
-               .path = worktree ? worktree : gitdir
-       };
+       struct safe_directory_data data = { 0 };
 
        if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
            (!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
@@ -1271,6 +1269,15 @@ static int ensure_valid_ownership(const char *gitfile,
            (!gitdir || is_path_owned_by_current_user(gitdir, report)))
                return 1;
 
+       /*
+        * normalize the data.path for comparison with normalized paths
+        * that come from the configuration file.  The path is unsafe
+        * if it cannot be normalized.
+        */
+       data.path = real_pathdup(worktree ? worktree : gitdir, 0);
+       if (!data.path)
+               return 0;
+
        /*
         * data.path is the "path" that identifies the repository and it is
         * constant regardless of what failed above. data.is_safe should be
@@ -1278,6 +1285,7 @@ static int ensure_valid_ownership(const char *gitfile,
         */
        git_protected_config(safe_directory_cb, &data);
 
+       free(data.path);
        return data.is_safe;
 }
 

diff --git a/t/t0033-safe-directory.sh b/t/t0033-safe-directory.sh
index 5fe61f129129efce671b1c3889a92a59af9be4da..07ac0f9a0163232c24c3d98c4990ecdba848c9fc 100755 (executable)
--- a/t/t0033-safe-directory.sh
+++ b/t/t0033-safe-directory.sh
@@ -119,4 +119,61 @@ test_expect_success 'local clone of unowned repo accepted in safe directory' '
        test_path_is_dir target
 '
 
+test_expect_success SYMLINKS 'checked paths are normalized' '
+       test_when_finished "rm -rf repository; rm -f repo" &&
+       (
+               sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+               git config --global --unset-all safe.directory
+       ) &&
+       git init repository &&
+       ln -s repository repo &&
+       (
+               cd repository &&
+               sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+               test_commit sample
+       ) &&
+
+       (
+               sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+               git config --global safe.directory "$(pwd)/repository"
+       ) &&
+       git -C repository for-each-ref &&
+       git -C repository/ for-each-ref &&
+       git -C repo for-each-ref &&
+       git -C repo/ for-each-ref &&
+       test_must_fail git -C repository/.git for-each-ref &&
+       test_must_fail git -C repository/.git/ for-each-ref &&
+       test_must_fail git -C repo/.git for-each-ref &&
+       test_must_fail git -C repo/.git/ for-each-ref
+'
+
+test_expect_success SYMLINKS 'checked leading paths are normalized' '
+       test_when_finished "rm -rf repository; rm -f repo" &&
+       (
+               sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+               git config --global --unset-all safe.directory
+       ) &&
+       mkdir -p repository &&
+       git init repository/s &&
+       ln -s repository repo &&
+       (
+               cd repository/s &&
+               sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+               test_commit sample
+       ) &&
+
+       (
+               sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
+               git config --global safe.directory "$(pwd)/repository/*"
+       ) &&
+       git -C repository/s for-each-ref &&
+       git -C repository/s/ for-each-ref &&
+       git -C repo/s for-each-ref &&
+       git -C repo/s/ for-each-ref &&
+       git -C repository/s/.git for-each-ref &&
+       git -C repository/s/.git/ for-each-ref &&
+       git -C repo/s/.git for-each-ref &&
+       git -C repo/s/.git/ for-each-ref
+'
+
 test_done