core.sharedRepository defines which permissions Git should set when creating files in $GIT_DIR, so that the repository may be shared with other users. But (in its current form) the setting shouldn't affect how files are created in the working tree. This is not respected by apply and am (which uses apply), when creating leading directories: $ cat d.patch diff --git a/d/f b/d/f new file mode 100644 index 0000000..e69de29 Apply without the setting: $ umask 0077 $ git apply d.patch $ ls -ld d drwx------ Apply with the setting: $ umask 0077 $ git -c core.sharedRepository=0770 apply d.patch $ ls -ld d drwxrws--- Only the leading directories are affected. That's because they are created with safe_create_leading_directories(), which calls adjust_shared_perm() to set the directories' permissions based on core.sharedRepository. To fix that, let's introduce a variant of this function that ignores the setting, and use it in apply. Also add a regression test and a note in the function documentation about the use of each variant according to the destination (working tree or git dir). Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx> --- apply.c | 2 +- cache.h | 7 ++++++- sha1-file.c | 14 ++++++++++++-- t/t4129-apply-samemode.sh | 26 ++++++++++++++++++++++++++ t/test-lib-functions.sh | 4 ++-- 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/apply.c b/apply.c index 359ceb632c..4a4e9a0158 100644 --- a/apply.c +++ b/apply.c @@ -4409,7 +4409,7 @@ static int create_one_file(struct apply_state *state, return 0; if (errno == ENOENT) { - if (safe_create_leading_directories(path)) + if (safe_create_leading_directories_no_share(path)) return 0; res = try_create_file(state, path, mode, buf, size); if (res < 0) diff --git a/cache.h b/cache.h index e986cf4ea9..8d279bc110 100644 --- a/cache.h +++ b/cache.h @@ -1255,7 +1255,11 @@ int adjust_shared_perm(const char *path); * safe_create_leading_directories() temporarily changes path while it * is working but restores it before returning. * safe_create_leading_directories_const() doesn't modify path, even - * temporarily. + * temporarily. Both these variants adjust the permissions of the + * created directories to honor core.sharedRepository, so they are best + * suited for files inside the git dir. For working tree files, use + * safe_create_leading_directories_no_share() instead, as it ignores + * the core.sharedRepository setting. */ enum scld_error { SCLD_OK = 0, @@ -1266,6 +1270,7 @@ enum scld_error { }; enum scld_error safe_create_leading_directories(char *path); enum scld_error safe_create_leading_directories_const(const char *path); +enum scld_error safe_create_leading_directories_no_share(char *path); /* * Callback function for raceproof_create_file(). This function is diff --git a/sha1-file.c b/sha1-file.c index dd65bd5c68..c3c49d2fa5 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -291,7 +291,7 @@ int mkdir_in_gitdir(const char *path) return adjust_shared_perm(path); } -enum scld_error safe_create_leading_directories(char *path) +static enum scld_error safe_create_leading_directories_1(char *path, int share) { char *next_component = path + offset_1st_component(path); enum scld_error ret = SCLD_OK; @@ -337,7 +337,7 @@ enum scld_error safe_create_leading_directories(char *path) ret = SCLD_VANISHED; else ret = SCLD_FAILED; - } else if (adjust_shared_perm(path)) { + } else if (share && adjust_shared_perm(path)) { ret = SCLD_PERMS; } *slash = slash_character; @@ -345,6 +345,16 @@ enum scld_error safe_create_leading_directories(char *path) return ret; } +enum scld_error safe_create_leading_directories(char *path) +{ + return safe_create_leading_directories_1(path, 1); +} + +enum scld_error safe_create_leading_directories_no_share(char *path) +{ + return safe_create_leading_directories_1(path, 0); +} + enum scld_error safe_create_leading_directories_const(const char *path) { int save_errno; diff --git a/t/t4129-apply-samemode.sh b/t/t4129-apply-samemode.sh index 5cdd76dfa7..41818d8315 100755 --- a/t/t4129-apply-samemode.sh +++ b/t/t4129-apply-samemode.sh @@ -73,4 +73,30 @@ test_expect_success FILEMODE 'bogus mode is rejected' ' test_i18ngrep "invalid mode" err ' +test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree files' ' + git reset --hard && + test_config core.sharedRepository 0666 && + ( + # Remove a default ACL if possible. + (setfacl -k newdir 2>/dev/null || true) && + umask 0077 && + + # Test both files (f1) and leading dirs (d) + mkdir d && + touch f1 d/f2 && + git add f1 d/f2 && + git diff --staged >patch-f1-and-f2.txt && + + rm -rf d f1 && + git apply patch-f1-and-f2.txt && + + echo "-rw-------" >f1_mode.expected && + echo "drwx------" >d_mode.expected && + test_modebits f1 >f1_mode.actual && + test_modebits d >d_mode.actual && + test_cmp f1_mode.expected f1_mode.actual && + test_cmp d_mode.expected d_mode.actual + ) +' + test_done diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 7ba3011b90..0f7f247159 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -367,9 +367,9 @@ test_chmod () { git update-index --add "--chmod=$@" } -# Get the modebits from a file. +# Get the modebits from a file or directory. test_modebits () { - ls -l "$1" | sed -e 's|^\(..........\).*|\1|' + ls -ld "$1" | sed -e 's|^\(..........\).*|\1|' } # Unset a configuration variable, but don't fail if it doesn't exist. -- 2.29.2