commit b2b3fce13e4eca588bb28a869b07f0063568b505 from: Omar Polo date: Sat Oct 29 18:22:42 2022 UTC respect umask when creating or changing files and directories This behaviour is already documented in got-worktree(5) but wasn't actually implemented. ok stsp@ commit - ed0cf971e59aa78ca1de9e9a4365ec7025dac004 commit + b2b3fce13e4eca588bb28a869b07f0063568b505 blob - 14024d2a98a433e4feb490708b08e72e42f96f75 blob + 8647dea45a83ad12f49ae63960dd903bd55fde98 --- lib/patch.c +++ lib/patch.c @@ -87,6 +87,16 @@ struct patch_args { void *progress_arg; struct got_patch_hunk_head *head; }; + +static mode_t +apply_umask(mode_t mode) +{ + mode_t um; + + um = umask(000); + umask(um); + return mode & ~um; +} static const struct got_error * send_patch(struct imsgbuf *ibuf, int fd) @@ -916,7 +926,7 @@ apply_patch(int *overlapcnt, struct got_worktree *work goto done; } - if (fchmod(outfd, mode) == -1) { + if (fchmod(outfd, apply_umask(mode)) == -1) { err = got_error_from_errno2("chmod", tmppath); goto done; } blob - d0e5af7c33c6ca8a4ba2e9dc621a494ed26dda57 blob + 438973a5f89e071064f01cb39023043b4337946a --- lib/worktree.c +++ lib/worktree.c @@ -63,6 +63,8 @@ #define GOT_MERGE_LABEL_MERGED "merged change" #define GOT_MERGE_LABEL_BASE "3-way merge base" + +static mode_t apply_umask(mode_t); static const struct got_error * create_meta_file(const char *path_got, const char *name, const char *content) @@ -703,7 +705,7 @@ merge_file(int *local_changes_subsumed, struct got_wor goto done; } - if (fchmod(fileno(f_merged), st_mode) != 0) { + if (fchmod(fileno(f_merged), apply_umask(st_mode)) != 0) { err = got_error_from_errno2("fchmod", merged_path); goto done; } @@ -776,7 +778,7 @@ install_symlink_conflict(const char *deriv_target, if (err) goto done; - if (fchmod(fileno(f), GOT_DEFAULT_FILE_MODE) == -1) { + if (fchmod(fileno(f), apply_umask(GOT_DEFAULT_FILE_MODE)) == -1) { err = got_error_from_errno2("fchmod", path); goto done; } @@ -1124,7 +1126,17 @@ get_ondisk_perms(int executable, mode_t st_mode) return st_mode | xbits; } - return (st_mode & ~(S_IXUSR | S_IXGRP | S_IXOTH)); + return st_mode; +} + +static mode_t +apply_umask(mode_t mode) +{ + mode_t um; + + um = umask(000); + umask(um); + return mode & ~um; } /* forward declaration */ @@ -1388,9 +1400,11 @@ install_blob(struct got_worktree *worktree, const char size_t len, hdrlen; int update = 0; char *tmppath = NULL; + mode_t mode; + mode = get_ondisk_perms(te_mode & S_IXUSR, GOT_DEFAULT_FILE_MODE); fd = open(ondisk_path, O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | - O_CLOEXEC, GOT_DEFAULT_FILE_MODE); + O_CLOEXEC, mode); if (fd == -1) { if (errno == ENOENT) { char *parent; @@ -1403,7 +1417,7 @@ install_blob(struct got_worktree *worktree, const char return err; fd = open(ondisk_path, O_RDWR | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC, - GOT_DEFAULT_FILE_MODE); + mode); if (fd == -1) return got_error_from_errno2("open", ondisk_path); @@ -1425,15 +1439,15 @@ install_blob(struct got_worktree *worktree, const char if (err) goto done; update = 1; + + if (fchmod(fd, apply_umask(mode)) == -1) { + err = got_error_from_errno2("fchmod", + tmppath); + goto done; + } } } else return got_error_from_errno2("open", ondisk_path); - } - - if (fchmod(fd, get_ondisk_perms(te_mode & S_IXUSR, st_mode)) == -1) { - err = got_error_from_errno2("fchmod", - update ? tmppath : ondisk_path); - goto done; } if (progress_cb) { @@ -4590,7 +4604,10 @@ create_patched_content(char **path_outfile, int revers goto done; if (!S_ISLNK(sb2.st_mode)) { - if (fchmod(fileno(outfile), sb2.st_mode) == -1) { + mode_t mode; + + mode = apply_umask(sb2.st_mode); + if (fchmod(fileno(outfile), mode) == -1) { err = got_error_from_errno2("fchmod", path2); goto done; } blob - 71210cda27e4286934db7aa6882ed109ca1ad397 blob + 02acb5b14f9b116de16f3ff0fe380929aada9a65 --- regress/cmdline/backout.sh +++ regress/cmdline/backout.sh @@ -208,7 +208,42 @@ test_backout_next_commit() { test_done "$testroot" "$ret" } +test_backout_umask() { + local testroot=`test_init backout_umask` + + got checkout "$testroot/repo" "$testroot/wt" >/dev/null + echo "edit alpha" >$testroot/wt/alpha + (cd "$testroot/wt" && got commit -m 'edit alpha') >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + local commit=`git_show_head "$testroot/repo"` + + (cd "$testroot/wt" && got update) >/dev/null + + # using a subshell to avoid clobbering global umask + (umask 077 && cd "$testroot/wt" && got backout $commit) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + if ! ls -l "$testroot/wt/alpha" | grep -q ^-rw-------; then + echo "alpha is not 0600 after backout" >&2 + ls -l "$testroot/wt/alpha" >&2 + test_done "$testroot" $ret + return 1 + fi + + test_done "$testroot" 0 +} + test_parseargs "$@" run_test test_backout_basic run_test test_backout_edits_for_file_since_deleted run_test test_backout_next_commit +run_test test_backout_umask blob - fa55d96f05c0ea98d084505524dc36353abc0120 blob + 3ceb6774f7bce8a411b960566268aea4c45554cc --- regress/cmdline/checkout.sh +++ regress/cmdline/checkout.sh @@ -852,6 +852,41 @@ test_checkout_quiet() { diff -u $testroot/content.expected $testroot/content fi test_done "$testroot" "$ret" +} + +test_checkout_umask() { + local testroot=`test_init checkout_umask` + + # using a subshell to avoid clobbering global umask + (umask 044 && got checkout "$testroot/repo" "$testroot/wt") \ + >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + for f in alpha beta epsilon/zeta gamma/delta; do + ls -l "$testroot/wt/$f" | grep -q ^-rw------- + if [ $? -ne 0 ]; then + echo "$f is not 0600 after checkout" >&2 + ls -l "$testroot/wt/$f" >&2 + test_done "$testroot" 1 + return 1 + fi + done + + for d in epsilon gamma; do + ls -ld "$testroot/wt/$d" | grep -q ^drwx--x--x + if [ $? -ne 0 ]; then + echo "$d is not 711 after checkout" >&2 + ls -ld "$testroot/wt/$d" >&2 + test_done "$testroot" 1 + return 1 + fi + done + + test_done "$testroot" 0 } test_parseargs "$@" @@ -868,3 +903,4 @@ run_test test_checkout_symlink run_test test_checkout_symlink_relative_wtpath run_test test_checkout_repo_with_unknown_extension run_test test_checkout_quiet +run_test test_checkout_umask blob - ed1cc69eb0e97b9e276519a28388cd22e2ec82dc blob + 464cd2159b38557b4fa61c2ac402ba344ba16d43 --- regress/cmdline/cherrypick.sh +++ regress/cmdline/cherrypick.sh @@ -1690,6 +1690,39 @@ test_cherrypick_binary_file() { return 1 fi test_done "$testroot" "0" +} + +test_cherrypick_umask() { + local testroot=`test_init cherrypick_umask` + + got checkout $testroot/repo $testroot/wt >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + (cd "$testroot/wt" && got branch newbranch) >/dev/null + echo "modified alpha on branch" > $testroot/wt/alpha + (cd "$testroot/wt" && got commit -m 'edit alpha') >/dev/null + (cd "$testroot/wt" && got update -b master) >/dev/null + + # using a subshell to avoid clobbering global umask + (umask 077 && cd "$testroot/wt" && got cherrypick newbranch) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + if ! ls -l "$testroot/wt/alpha" | grep -q ^-rw-------; then + echo "alpha is not 0600 after cherrypick!" >&2 + ls -l "$testroot/wt/alpha" >&2 + test_done "$testroot" $ret + return 1 + fi + + test_done "$testroot" 0 } test_parseargs "$@" @@ -1709,3 +1742,4 @@ run_test test_cherrypick_unrelated_changes run_test test_cherrypick_same_branch run_test test_cherrypick_dot_on_a_line_by_itself run_test test_cherrypick_binary_file +run_test test_cherrypick_umask blob - 1264347441d2f3f6859d520ab1230f6161a9d666 blob + ed77df82520d57a35def63dbe04ca0b4cc145444 --- regress/cmdline/histedit.sh +++ regress/cmdline/histedit.sh @@ -2156,7 +2156,63 @@ test_histedit_resets_committer() { fi test_done "$testroot" "$ret" } + +test_histedit_umask() { + local testroot=`test_init histedit_umask` + local orig_commit=`git_show_head "$testroot/repo"` + + got checkout "$testroot/repo" "$testroot/wt" >/dev/null + + echo "modified alpha" > $testroot/wt/alpha + (cd "$testroot/wt" && got commit -m 'edit #1') >/dev/null + local commit1=`git_show_head "$testroot/repo"` + + echo "modified again" > $testroot/wt/alpha + (cd "$testroot/wt" && got commit -m 'edit #2') >/dev/null + local commit2=`git_show_head "$testroot/repo"` + + echo "modified again!" > $testroot/wt/alpha + echo "modify beta too!" > $testroot/wt/beta + (cd "$testroot/wt" && got commit -m 'edit #3') >/dev/null + local commit3=`git_show_head "$testroot/repo"` + (cd "$testroot/wt" && got update -c "$orig_commit") >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + echo "update to $orig_commit failed!" >&2 + test_done "$testroot" 1 + return 1 + fi + + echo fold $commit1 >$testroot/histedit-script + echo fold $commit2 >>$testroot/histedit-script + echo pick $commit3 >>$testroot/histedit-script + echo mesg folding changes >>$testroot/histedit-script + + # using a subshell to avoid clobbering global umask + (umask 077 && cd "$testroot/wt" && \ + got histedit -F "$testroot/histedit-script") >/dev/null + ret=$? + + if [ $ret -ne 0 ]; then + echo "histedit operation failed" >&2 + test_done "$testroot" $ret + return 1 + fi + + for f in alpha beta; do + ls -l "$testroot/wt/$f" | grep -q ^-rw------- + if [ $? -ne 0 ]; then + echo "$f is not 0600 after histedi" >&2 + ls -l "$testroot/wt/$f" >&2 + test_done "$testroot" 1 + return 1 + fi + done + + test_done "$testroot" 0 +} + test_parseargs "$@" run_test test_histedit_no_op run_test test_histedit_swap @@ -2179,3 +2235,4 @@ run_test test_histedit_edit_only run_test test_histedit_prepend_line run_test test_histedit_mesg_invalid run_test test_histedit_resets_committer +run_test test_histedit_umask blob - 3f0578e460b5ab26c3e92ecb652e7a6e21f4cb65 blob + 43fc62a9959f02a646f8a0777654f8bce35c3878 --- regress/cmdline/merge.sh +++ regress/cmdline/merge.sh @@ -1392,6 +1392,37 @@ test_merge_interrupt() { diff -u $testroot/stdout.expected $testroot/stdout fi test_done "$testroot" "$ret" +} + +test_merge_umask() { + local testroot=`test_init merge_umask` + + (cd $testroot/repo && git checkout -q -b newbranch) + echo "modified alpha on branch" >$testroot/repo/alpha + git_commit "$testroot/repo" -m "committing alpha on newbranch" + echo "modified delta on branch" >$testroot/repo/gamma/delta + git_commit "$testroot/repo" -m "committing delta on newbranch" + + # diverge from newbranch + (cd "$testroot/repo" && git checkout -q master) + echo "modified beta on master" >$testroot/repo/beta + git_commit "$testroot/repo" -m "committing zeto no master" + + got checkout "$testroot/repo" "$testroot/wt" >/dev/null + + # using a subshell to avoid clobbering global umask + (umask 077 && cd "$testroot/wt" && got merge newbranch) >/dev/null + + for f in alpha gamma/delta; do + ls -l "$testroot/wt/$f" | grep -q ^-rw------- + if [ $? -ne 0 ]; then + echo "$f is not 0600 after merge" >&2 + ls -l "$testroot/wt/$f" >&2 + test_done "$testroot" 1 + fi + done + + test_done "$testroot" 0 } test_parseargs "$@" @@ -1404,3 +1435,4 @@ run_test test_merge_missing_file run_test test_merge_no_op run_test test_merge_imported_branch run_test test_merge_interrupt +run_test test_merge_umask blob - f8f949b792887413b26e30cab123e9e1c57ae123 blob + af677fff9e318f5b61edce4c73ec63709b02fd53 --- regress/cmdline/patch.sh +++ regress/cmdline/patch.sh @@ -1848,6 +1848,37 @@ EOF fi test_done $testroot $ret +} + +test_patch_umask() { + local testroot=`test_init patch_umask` + + got checkout "$testroot/repo" "$testroot/wt" >/dev/null + + cat <$testroot/wt/patch +--- alpha ++++ alpha +@@ -1 +1 @@ +-alpha ++modified alpha +EOF + + # using a subshell to avoid clobbering global umask + (umask 077 && cd "$testroot/wt" && got patch /dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + if ! ls -l "$testroot/wt/alpha" | grep -q ^-rw-------; then + echo "alpha is not 0600 after patch" >&2 + ls -l "$testroot/wt/alpha" >&2 + test_done "$testroot" 1 + return 1 + fi + + test_done "$testroot" 0 } test_parseargs "$@" @@ -1878,3 +1909,4 @@ run_test test_patch_merge_unknown_blob run_test test_patch_merge_reverse run_test test_patch_newfile_xbit_got_diff run_test test_patch_newfile_xbit_git_diff +run_test test_patch_umask blob - 62ae3b151c0c8c8e867585314a8e3bc15764c831 blob + f809879812d293f5b44a4ad0e1a299f1384dc027 --- regress/cmdline/rebase.sh +++ regress/cmdline/rebase.sh @@ -1789,7 +1789,51 @@ test_rebase_nonbranch() { fi test_done "$testroot" "$ret" } + +test_rebase_umask() { + local testroot=`test_init rebase_umask` + local commit0=`git_show_head "$testroot/repo"` + + got checkout "$testroot/repo" "$testroot/wt" >/dev/null + (cd "$testroot/wt" && got branch newbranch) >/dev/null + + echo "modified alpha on branch" >$testroot/wt/alpha + (cd "$testroot/wt" && got commit -m 'modified alpha on newbranch') \ + >/dev/null + + (cd "$testroot/wt" && got update -b master) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + echo "got update failed!" >&2 + test_done "$testroot" $ret + return 1 + fi + echo "modified beta on master" >$testroot/wt/beta + (cd "$testroot/wt" && got commit -m 'modified beta on master') \ + >/dev/null + (cd "$testroot/wt" && got update) >/dev/null + + # using a subshell to avoid clobbering global umask + (umask 077 && cd "$testroot/wt" && got rebase newbranch) >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + echo "got rebase failed" >&2 + test_done "$testroot" $ret + return 1 + fi + + ls -l "$testroot/wt/alpha" | grep -q ^-rw------- + if [ $? -ne 0 ]; then + echo "alpha is not 0600 after rebase" >&2 + ls -l "$testroot/wt/alpha" >&2 + test_done "$testroot" 1 + return 1 + fi + + test_done "$testroot" 0 +} + test_parseargs "$@" run_test test_rebase_basic run_test test_rebase_ancestry_check @@ -1809,3 +1853,4 @@ run_test test_rebase_rm_add_rm_file run_test test_rebase_resets_committer run_test test_rebase_no_author_info run_test test_rebase_nonbranch +run_test test_rebase_umask blob - a565acdd0271008c6ea82910d668b93ed096e382 blob + 3d38c4d175937ca52bef1b76659276b50d0cf91e --- regress/cmdline/revert.sh +++ regress/cmdline/revert.sh @@ -1488,7 +1488,31 @@ EOF fi test_done "$testroot" "$ret" } + +test_revert_umask() { + local testroot=`test_init revert_umask` + + got checkout "$testroot/repo" "$testroot/wt" >/dev/null + echo "edit alpha" > $testroot/wt/alpha + # using a subshell to avoid clobbering global umask + (umask 077 && cd "$testroot/wt" && got revert alpha) \ + >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + if ! ls -l "$testroot/wt/alpha" | grep -q ^-rw-------; then + echo "alpha is not 0600 after revert" >&2 + ls -l "$testroot/wt/alpha" >&2 + test_done "$testroot" 1 + return 1 + fi + test_done "$testroot" 0 +} + test_parseargs "$@" run_test test_revert_basic run_test test_revert_rm @@ -1506,3 +1530,4 @@ run_test test_revert_added_subtree run_test test_revert_deleted_subtree run_test test_revert_symlink run_test test_revert_patch_symlink +run_test test_revert_umask blob - fce60c7f277d948ec481a744ed23e1b55ec128ed blob + afb98ebb0b2de07c90dfa887d9d20fe0cb5f0db4 --- regress/cmdline/update.sh +++ regress/cmdline/update.sh @@ -3009,7 +3009,71 @@ test_update_binary_file() { fi test_done "$testroot" "0" } + +test_update_umask() { + local testroot=`test_init update_binary_file` + + got checkout "$testroot/repo" "$testroot/wt" >/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" "$ret" + return 1 + fi + + rm "$testroot/wt/alpha" + + # using a subshell to avoid clobbering global umask + (umask 022 && cd "$testroot/wt" && got update alpha) \ + >/dev/null 2>/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + if ! ls -l "$testroot/wt/alpha" | grep -q ^-rw-r--r--; then + echo "alpha is not 0644" >&2 + test_done "$testroot" 1 + return 1 + fi + + rm "$testroot/wt/alpha" + + # using a subshell to avoid clobbering global umask + (umask 044 && cd "$testroot/wt" && got update alpha) \ + >/dev/null 2>/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + if ! ls -l "$testroot/wt/alpha" | grep -q ^-rw-------; then + echo "alpha is not 0600" >&2 + test_done "$testroot" 1 + return 1 + fi + + rm "$testroot/wt/alpha" + + # using a subshell to avoid clobbering global umask + (umask 222 && cd "$testroot/wt" && got update alpha) \ + >/dev/null 2>/dev/null + ret=$? + if [ $ret -ne 0 ]; then + test_done "$testroot" $ret + return 1 + fi + + if ! ls -l "$testroot/wt/alpha" | grep -q ^-r--r--r--; then + echo "alpha is not 0444" >&2 + test_done "$testroot" 1 + return 1; + fi + + test_done "$testroot" 0 +} + test_parseargs "$@" run_test test_update_basic run_test test_update_adds_file @@ -3054,3 +3118,4 @@ run_test test_update_file_skipped_due_to_conflict run_test test_update_file_skipped_due_to_obstruction run_test test_update_quiet run_test test_update_binary_file +run_test test_update_umask