From: Stefan Sperling Subject: Re: prevent 'got merge' from modifying arbitrary references To: Omar Polo Cc: gameoftrees@openbsd.org Date: Wed, 21 Jun 2023 17:36:20 +0200 On Wed, Jun 21, 2023 at 01:54:02PM +0200, Omar Polo wrote: > I'm in doubt however for the regress, shouldn't merge.sh be more > accurate than fetch.sh? Ideally one should run all the regresses, but > I can see myself limiting to only the subset of the ones directly > impacted when hacking on something, and only later running the full > blown regress suite. Sure, the tests can be moved to merge.sh. See below. ----------------------------------------------- prevent 'got merge' from creating commits on branches outside of "refs/heads/" diff b73055ebbea59457c1aef2cf7c473ea2898b434b 1334230721068ac62f5ea69c359962cdedb4df60 commit - b73055ebbea59457c1aef2cf7c473ea2898b434b commit + 1334230721068ac62f5ea69c359962cdedb4df60 blob - 8122fe9273cc37091dc13c83016ef03a2a436be5 blob + b6da5695eaab874ad02b4ba5ad0801b9748b60fb --- got/got.1 +++ got/got.1 @@ -2837,7 +2837,10 @@ The tip commit of the work tree's current branch, whic .Cm got import . .Pp Merge commits are commits based on multiple parent commits. -The tip commit of the work tree's current branch, which must be set with +The tip commit of the work tree's current branch, which must be +must be in the +.Dq refs/heads/ +reference namespace and must be set with .Cm got update -b before starting the .Cm merge @@ -2882,6 +2885,13 @@ If the work tree is not yet fully updated to the tip c .Pp .Cm got merge will refuse to run if certain preconditions are not met. +If the work tree's current branch is not in the +.Dq refs/heads/ +reference namespace then the work tree must first be switched to a +branch in the +.Dq refs/heads/ +namespace with +.Cm got update -b . If the work tree is not yet fully updated to the tip commit of its branch, then the work tree must first be updated with .Cm got update . blob - ea8a08baed68affba73cafa577c3125e9756466a blob + 4c6ca7ff2f76eaa431b055ab0a6912772b492b38 --- got/got.c +++ got/got.c @@ -13264,6 +13264,16 @@ cmd_merge(int argc, char *argv[]) goto done; /* nothing else to do */ } + if (strncmp(got_worktree_get_head_ref_name(worktree), + "refs/heads/", 11) != 0) { + error = got_error_fmt(GOT_ERR_COMMIT_BRANCH, + "work tree's current branch %s is outside the " + "\"refs/heads/\" reference namespace; " + "update -b required", + got_worktree_get_head_ref_name(worktree)); + goto done; + } + error = get_author(&author, repo, worktree); if (error) goto done; blob - 92b590aa2ef2958738fb2d4e8419cb879fc9bf5c blob + 0531f8b1f57c1f50521b6ec8c350bb9da1bebaf4 --- regress/cmdline/merge.sh +++ regress/cmdline/merge.sh @@ -1692,6 +1692,171 @@ test_parseargs "$@" test_done "$testroot" "$ret" } +test_merge_fetched_branch() { + local testroot=`test_init merge_fetched_branch` + local testurl=ssh://127.0.0.1/$testroot + local commit_id=`git_show_head $testroot/repo` + + got clone -q $testurl/repo $testroot/repo-clone + ret=$? + if [ $ret -ne 0 ]; then + echo "got clone command failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + echo "modified alpha" > $testroot/repo/alpha + git_commit $testroot/repo -m "modified alpha" + local commit_id2=`git_show_head $testroot/repo` + + got fetch -q -r $testroot/repo-clone > $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + echo "got fetch command failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + echo -n > $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + got ref -l -r $testroot/repo-clone > $testroot/stdout + + echo "HEAD: refs/heads/master" > $testroot/stdout.expected + echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected + echo "refs/remotes/origin/HEAD: refs/remotes/origin/master" \ + >> $testroot/stdout.expected + echo "refs/remotes/origin/master: $commit_id2" \ + >> $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + got checkout $testroot/repo-clone $testroot/wt > /dev/null + + echo "modified beta" > $testroot/wt/beta + (cd $testroot/wt && got commit -m "modified beta" > /dev/null) + local commit_id3=`git_show_head $testroot/repo-clone` + + (cd $testroot/wt && got update > /dev/null) + (cd $testroot/wt && got merge origin/master > $testroot/stdout) + local merge_commit_id=`git_show_head $testroot/repo-clone` + + cat > $testroot/stdout.expected <&2 + test_done "$testroot" "$ret" + return 1 + fi + + echo "modified alpha" > $testroot/repo/alpha + git_commit $testroot/repo -m "modified alpha" + local commit_id2=`git_show_head $testroot/repo` + + got fetch -q -r $testroot/repo-clone > $testroot/stdout + ret=$? + if [ $ret -ne 0 ]; then + echo "got fetch command failed unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + + echo -n > $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + got ref -l -r $testroot/repo-clone > $testroot/stdout + + echo "HEAD: refs/heads/master" > $testroot/stdout.expected + echo "refs/heads/master: $commit_id" >> $testroot/stdout.expected + echo "refs/remotes/origin/HEAD: refs/remotes/origin/master" \ + >> $testroot/stdout.expected + echo "refs/remotes/origin/master: $commit_id2" \ + >> $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + got checkout $testroot/repo-clone $testroot/wt > /dev/null + + echo "modified beta" > $testroot/wt/beta + (cd $testroot/wt && got commit -m "modified beta" > /dev/null) + local commit_id3=`git_show_head $testroot/repo-clone` + + (cd $testroot/wt && got update -b origin/master > /dev/null) + (cd $testroot/wt && got merge master > \ + $testroot/stdout 2> $testroot/stderr) + local merge_commit_id=`git_show_head $testroot/repo-clone` + + echo -n > $testroot/stdout.expected + + cmp -s $testroot/stdout $testroot/stdout.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stdout.expected $testroot/stdout + test_done "$testroot" "$ret" + return 1 + fi + + echo -n "got: work tree's current branch refs/remotes/origin/master " \ + > $testroot/stderr.expected + echo -n 'is outside the "refs/heads/" reference namespace; ' \ + >> $testroot/stderr.expected + echo -n "update -b required: will not commit to a branch " \ + >> $testroot/stderr.expected + echo 'outside the "refs/heads/" reference namespace' \ + >> $testroot/stderr.expected + + cmp -s $testroot/stderr $testroot/stderr.expected + ret=$? + if [ $ret -ne 0 ]; then + diff -u $testroot/stderr.expected $testroot/stderr + fi + test_done "$testroot" "$ret" +} + test_parseargs "$@" run_test test_merge_basic run_test test_merge_forward @@ -1707,3 +1872,5 @@ run_test test_merge_gitconfig_author run_test test_merge_interrupt run_test test_merge_umask run_test test_merge_gitconfig_author +run_test test_merge_fetched_branch +run_test test_merge_fetched_branch_remote