commit a2c162ebaea11f9bd1d46c4cdf5b8f488ebaf58f
from: Omar Polo <op@omarpolo.com>
via: Thomas Adam <thomas@xteddy.org>
date: Sun Oct 30 08:25:55 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 - 0df6b4df0fdcddf91ef84e4af4e0a6603b87c6f3
commit + a2c162ebaea11f9bd1d46c4cdf5b8f488ebaf58f
blob - 96dcf72a253bd57b62dcd7c865069573c7f70be7
blob + c0fc6a3072c20b18fa7a9f239b1869878c328691
--- lib/patch.c
+++ lib/patch.c
@@ -85,6 +85,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)
@@ -914,7 +924,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 - 26c789d75c4c6e26b8f5d2f0584e1cfaed88bbb6
blob + 593c8dec12555e8bcd41aa811af48a649efb6851
--- lib/worktree.c
+++ lib/worktree.c
@@ -61,6 +61,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)
@@ -701,7 +703,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;
 	}
@@ -774,7 +776,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;
 	}
@@ -1122,7 +1124,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 */
@@ -1386,9 +1398,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;
@@ -1401,7 +1415,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);
@@ -1423,15 +1437,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) {
@@ -4588,7 +4602,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 - 8e515695e66fe8502ee5328569beffeec45ae61c
blob + df1931610d6b5246e4ce7cfe6ddc2cfc5f3297b6
--- 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 - 2c58e6d9d1f0ef383bd21393bfb3bf7f37dc4279
blob + a5f38f86a9c1e1366cdab61194d81fc23aa6163c
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -2164,7 +2164,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
@@ -2187,3 +2243,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 <<EOF >$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 <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 - e6f0068db0c7a000201f333c5968512f1d4ebd87
blob + be35f77809d804c49d6d9222b8479d500a1df7a8
--- regress/cmdline/rebase.sh
+++ regress/cmdline/rebase.sh
@@ -1792,7 +1792,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
@@ -1812,3 +1856,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 - 0ef1763cc888e8f8cc05d0bb0ae84332c052625f
blob + 299d87314c0270628f8e1c8ececbfa4a336ab7d8
--- 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 - 78efb6baf1ef1a675d0eda18bf4d4f2156f1814d
blob + 857244844b7df7f9a6d779630dcca80db3f645db
--- 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