@matheusbsbs / matheus-bagliano / commits / eb3c027e17

apply: don't use core.sharedRepository to create working tree files

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@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Matheus Tavares committed Dec 1, 2020 at 20:45 UTC eb3c027e1779d0d1b5bd3b4e96a108461c9759a5

5 files changed +47 -6

apply.c

+1 -1

@@ -4409,7 +4409,7 @@ static int create_one_file(struct apply_state *state,

4409 return 0;

4410

4411 if (errno == ENOENT) {

4412 - if (safe_create_leading_directories(path))

4412 + if (safe_create_leading_directories_no_share(path))

4413 return 0;

4414 res = try_create_file(state, path, mode, buf, size);

4415 if (res < 0)

cache.h

+6 -1

@@ -1255,7 +1255,11 @@ int adjust_shared_perm(const char *path);

1255 * safe_create_leading_directories() temporarily changes path while it

1256 * is working but restores it before returning.

1257 * safe_create_leading_directories_const() doesn't modify path, even

1258 - * temporarily.

1258 + * temporarily. Both these variants adjust the permissions of the

1259 + * created directories to honor core.sharedRepository, so they are best

1260 + * suited for files inside the git dir. For working tree files, use

1261 + * safe_create_leading_directories_no_share() instead, as it ignores

1262 + * the core.sharedRepository setting.

1263 */

1264 enum scld_error {

1265 SCLD_OK = 0,

@@ -1266,6 +1270,7 @@ enum scld_error {

1270 };

1271 enum scld_error safe_create_leading_directories(char *path);

1272 enum scld_error safe_create_leading_directories_const(const char *path);

1273 +enum scld_error safe_create_leading_directories_no_share(char *path);

1274

1275 /*

1276 * Callback function for raceproof_create_file(). This function is

sha1-file.c

+12 -2

@@ -291,7 +291,7 @@ int mkdir_in_gitdir(const char *path)

291 return adjust_shared_perm(path);

292 }

293

294 -enum scld_error safe_create_leading_directories(char *path)

294 +static enum scld_error safe_create_leading_directories_1(char *path, int share)

295 {

296 char *next_component = path + offset_1st_component(path);

297 enum scld_error ret = SCLD_OK;

@@ -337,7 +337,7 @@ enum scld_error safe_create_leading_directories(char *path)

337 ret = SCLD_VANISHED;

338 else

339 ret = SCLD_FAILED;

340 - } else if (adjust_shared_perm(path)) {

340 + } else if (share && adjust_shared_perm(path)) {

341 ret = SCLD_PERMS;

342 }

343 *slash = slash_character;

@@ -345,6 +345,16 @@ enum scld_error safe_create_leading_directories(char *path)

345 return ret;

346 }

347

348 +enum scld_error safe_create_leading_directories(char *path)

349 +{

350 + return safe_create_leading_directories_1(path, 1);

351 +}

352 +

353 +enum scld_error safe_create_leading_directories_no_share(char *path)

354 +{

355 + return safe_create_leading_directories_1(path, 0);

356 +}

357 +

358 enum scld_error safe_create_leading_directories_const(const char *path)

359 {

360 int save_errno;

t/t4129-apply-samemode.sh

+26

@@ -73,4 +73,30 @@ test_expect_success FILEMODE 'bogus mode is rejected' '

73 test_i18ngrep "invalid mode" err

74 '

76 +test_expect_success POSIXPERM 'do not use core.sharedRepository for working tree files' '

77 + git reset --hard &&

78 + test_config core.sharedRepository 0666 &&

79 + (

80 + # Remove a default ACL if possible.

81 + (setfacl -k newdir 2>/dev/null || true) &&

82 + umask 0077 &&

83 +

84 + # Test both files (f1) and leading dirs (d)

85 + mkdir d &&

86 + touch f1 d/f2 &&

87 + git add f1 d/f2 &&

88 + git diff --staged >patch-f1-and-f2.txt &&

89 +

90 + rm -rf d f1 &&

91 + git apply patch-f1-and-f2.txt &&

92 +

93 + echo "-rw-------" >f1_mode.expected &&

94 + echo "drwx------" >d_mode.expected &&

95 + test_modebits f1 >f1_mode.actual &&

96 + test_modebits d >d_mode.actual &&

97 + test_cmp f1_mode.expected f1_mode.actual &&

98 + test_cmp d_mode.expected d_mode.actual

99 + )

100 +'

101 +

102 test_done

t/test-lib-functions.sh

+2 -2

@@ -367,9 +367,9 @@ test_chmod () {

367 git update-index --add "--chmod=$@"

368 }

369

370 -# Get the modebits from a file.

370 +# Get the modebits from a file or directory.

371 test_modebits () {

372 - ls -l "$1" | sed -e 's|^$..........$.*|\1|'

372 + ls -ld "$1" | sed -e 's|^$..........$.*|\1|'

373 }

374

375 # Unset a configuration variable, but don't fail if it doesn't exist.