commit 35213c7c838a48142d398147b54bb9938af8cab0 from: Stefan Sperling date: Thu Jul 23 14:22:39 2020 UTC forbid bad symlinks; add -S option to 'got commit' and 'got stage' to allow them commit - 3c1ec782ab68abf6084ad937bc2a3e4a9c8fe049 commit + 35213c7c838a48142d398147b54bb9938af8cab0 blob - ca0ab90afd597806d087a341444532719ec5a156 blob + 1bd20230e857c83cb4e2cbd62cec36b1a83b09ab --- got/got.1 +++ got/got.1 @@ -1169,7 +1169,7 @@ is a directory. .It Cm rv Short alias for .Cm revert . -.It Cm commit Oo Fl m Ar message Oc Op Ar path ... +.It Cm commit Oo Fl m Ar message Oc Oo Fl S Oc Op Ar path ... Create a new commit in the repository from changes in a work tree and use this commit as the new base commit for the work tree. If no @@ -1227,6 +1227,15 @@ Without the option, .Cm got commit opens a temporary file in an editor where a log message can be written. +.It Fl S +Allow symbolic links which point somewhere outside of the path space +managed by +.Nm . +As a precaution, +when such a symbolic link gets installed in a work tree +.Nm +may decide to represent the symbolic link as a regular file which contains +the link's target path, rather than creating an actual symbolic link. .El .Pp .Cm got commit @@ -1637,7 +1646,7 @@ or reverted with .It Cm ig Short alias for .Cm integrate . -.It Cm stage Oo Fl l Oc Oo Fl p Oc Oo Fl F Ar response-script Oc Op Ar path ... +.It Cm stage Oo Fl l Oc Oo Fl p Oc Oo Fl F Ar response-script Oc Oo Fl S Oc Op Ar path ... Stage local changes for inclusion in the next commit. If no .Ar path @@ -1713,6 +1722,15 @@ and responses line-by-line from the specified .Ar response-script file instead of prompting interactively. +.It Fl S +Allow symbolic links which point somewhere outside of the path space +managed by +.Nm . +As a precaution, +when such a symbolic link gets installed in a work tree +.Nm +may decide to represent the symbolic link as a regular file which contains +the link's target path, rather than creating an actual symbolic link. .El .Pp .Cm got stage blob - 8d5e78e2dc5a7a5aabb308726b8319e56d4a3ab9 blob + fb2d55929f89f48f959e0ddfdc2fa639fe2c64d0 --- got/got.c +++ got/got.c @@ -6366,7 +6366,7 @@ done: __dead static void usage_commit(void) { - fprintf(stderr, "usage: %s commit [-m msg] [path ...]\n", + fprintf(stderr, "usage: %s commit [-m msg] [-S] [path ...]\n", getprogname()); exit(1); } @@ -6453,16 +6453,20 @@ cmd_commit(int argc, char *argv[]) struct collect_commit_logmsg_arg cl_arg; char *gitconfig_path = NULL, *editor = NULL, *author = NULL; int ch, rebase_in_progress, histedit_in_progress, preserve_logmsg = 0; + int allow_bad_symlinks = 0; struct got_pathlist_head paths; TAILQ_INIT(&paths); cl_arg.logmsg_path = NULL; - while ((ch = getopt(argc, argv, "m:")) != -1) { + while ((ch = getopt(argc, argv, "m:S")) != -1) { switch (ch) { case 'm': logmsg = optarg; break; + case 'S': + allow_bad_symlinks = 1; + break; default: usage_commit(); /* NOTREACHED */ @@ -6543,7 +6547,8 @@ cmd_commit(int argc, char *argv[]) } cl_arg.repo_path = got_repo_get_path(repo); error = got_worktree_commit(&id, worktree, &paths, author, NULL, - collect_commit_logmsg, &cl_arg, print_status, NULL, repo); + allow_bad_symlinks, collect_commit_logmsg, &cl_arg, + print_status, NULL, repo); if (error) { if (error->code != GOT_ERR_COMMIT_MSG_EMPTY && cl_arg.logmsg_path != NULL) @@ -8760,7 +8765,7 @@ __dead static void usage_stage(void) { fprintf(stderr, "usage: %s stage [-l] | [-p] [-F response-script] " - "[file-path ...]\n", + "[-S] [file-path ...]\n", getprogname()); exit(1); } @@ -8801,14 +8806,14 @@ cmd_stage(int argc, char *argv[]) char *cwd = NULL; struct got_pathlist_head paths; struct got_pathlist_entry *pe; - int ch, list_stage = 0, pflag = 0; + int ch, list_stage = 0, pflag = 0, allow_bad_symlinks = 0; FILE *patch_script_file = NULL; const char *patch_script_path = NULL; struct choose_patch_arg cpa; TAILQ_INIT(&paths); - while ((ch = getopt(argc, argv, "lpF:")) != -1) { + while ((ch = getopt(argc, argv, "lpF:S")) != -1) { switch (ch) { case 'l': list_stage = 1; @@ -8819,6 +8824,9 @@ cmd_stage(int argc, char *argv[]) case 'F': patch_script_path = optarg; break; + case 'S': + allow_bad_symlinks = 1; + break; default: usage_stage(); /* NOTREACHED */ @@ -8881,7 +8889,8 @@ cmd_stage(int argc, char *argv[]) cpa.action = "stage"; error = got_worktree_stage(worktree, &paths, pflag ? NULL : print_status, NULL, - pflag ? choose_patch : NULL, &cpa, repo); + pflag ? choose_patch : NULL, &cpa, + allow_bad_symlinks, repo); } done: if (patch_script_file && fclose(patch_script_file) == EOF && blob - 1921d0479484792497498d1b7aedaad701aadc51 blob + b3805d39c06efa63932243754b6f446fa0218eda --- include/got_error.h +++ include/got_error.h @@ -143,6 +143,7 @@ #define GOT_ERR_TREE_ENTRY_TYPE 126 #define GOT_ERR_PARSE_Y_YY 127 #define GOT_ERR_NO_CONFIG_FILE 128 +#define GOT_ERR_BAD_SYMLINK 129 static const struct got_error { int code; @@ -292,6 +293,8 @@ static const struct got_error { { GOT_ERR_TREE_ENTRY_TYPE, "unexpected tree entry type" }, { GOT_ERR_PARSE_Y_YY, "yyerror error" }, { GOT_ERR_NO_CONFIG_FILE, "configuration file doesn't exit" }, + { GOT_ERR_BAD_SYMLINK, "symbolic link points outside of paths under " + "version control" }, }; /* blob - 0dd838e3937672354147a6c4cddeaadaa34de9c5 blob + 277c66b0096dcee60d7ae5305877516328376f34 --- include/got_worktree.h +++ include/got_worktree.h @@ -223,10 +223,13 @@ typedef const struct got_error *(*got_worktree_commit_ * current base commit. * An author and a non-empty log message must be specified. * The name of the committer is optional (may be NULL). + * If a path to be committed contains a symlink which points outside + * of the path space under version control, raise an error unless + * committing of such paths is being forced by the caller. */ const struct got_error *got_worktree_commit(struct got_object_id **, struct got_worktree *, struct got_pathlist_head *, const char *, - const char *, got_worktree_commit_msg_cb, void *, + const char *, int, got_worktree_commit_msg_cb, void *, got_worktree_status_cb, void *, struct got_repository *); /* Get the path of a commitable worktree item. */ @@ -424,10 +427,13 @@ const struct got_error *got_worktree_integrate_abort(s * Stage the specified paths for commit. * If the patch callback is not NULL, call it to select patch hunks for * staging. Otherwise, stage the full file content found at each path. -*/ + * If a path being staged contains a symlink which points outside + * of the path space under version control, raise an error unless + * staging of such paths is being forced by the caller. + */ const struct got_error *got_worktree_stage(struct got_worktree *, struct got_pathlist_head *, got_worktree_status_cb, void *, - got_worktree_patch_cb, void *, struct got_repository *); + got_worktree_patch_cb, void *, int, struct got_repository *); /* * Merge staged changes for the specified paths back into the work tree blob - f912c374a4ff37850113d396d182a47f7d72ef55 blob + bf1f2beb4941056d469b27d8a85aa3aa02dd903a --- lib/worktree.c +++ lib/worktree.c @@ -4533,6 +4533,7 @@ struct collect_commitables_arg { struct got_worktree *worktree; struct got_fileindex *fileindex; int have_staged_files; + int allow_bad_symlinks; }; static const struct got_error * @@ -4630,6 +4631,30 @@ collect_commitables(void *arg, unsigned char status, goto done; } + if (S_ISLNK(ct->mode) && staged_status == GOT_STATUS_NO_CHANGE && + status == GOT_STATUS_ADD && !a->allow_bad_symlinks) { + int is_bad_symlink; + char target_path[PATH_MAX]; + ssize_t target_len; + target_len = readlink(ct->ondisk_path, target_path, + sizeof(target_path)); + if (target_len == -1) { + err = got_error_from_errno2("readlink", + ct->ondisk_path); + goto done; + } + err = is_bad_symlink_target(&is_bad_symlink, target_path, + target_len, ct->ondisk_path, a->worktree->root_path); + if (err) + goto done; + if (is_bad_symlink) { + err = got_error_path(ct->ondisk_path, + GOT_ERR_BAD_SYMLINK); + goto done; + } + } + + ct->status = status; ct->staged_status = staged_status; ct->blob_id = NULL; /* will be filled in when blob gets created */ @@ -5523,7 +5548,7 @@ check_non_staged_files(struct got_fileindex *fileindex const struct got_error * got_worktree_commit(struct got_object_id **new_commit_id, struct got_worktree *worktree, struct got_pathlist_head *paths, - const char *author, const char *committer, + const char *author, const char *committer, int allow_bad_symlinks, got_worktree_commit_msg_cb commit_msg_cb, void *commit_arg, got_worktree_status_cb status_cb, void *status_arg, struct got_repository *repo) @@ -5573,6 +5598,7 @@ got_worktree_commit(struct got_object_id **new_commit_ cc_arg.fileindex = fileindex; cc_arg.repo = repo; cc_arg.have_staged_files = have_staged_files; + cc_arg.allow_bad_symlinks = allow_bad_symlinks; TAILQ_FOREACH(pe, paths, entry) { err = worktree_status(worktree, pe->path, fileindex, repo, collect_commitables, &cc_arg, NULL, NULL, 0, 0); @@ -7110,6 +7136,7 @@ struct stage_path_arg { got_worktree_patch_cb patch_cb; void *patch_arg; int staged_something; + int allow_bad_symlinks; }; static const struct got_error * @@ -7175,8 +7202,34 @@ stage_path(void *arg, unsigned char status, stage = GOT_FILEIDX_STAGE_MODIFY; got_fileindex_entry_stage_set(ie, stage); if (S_ISLNK(sb.st_mode)) { - got_fileindex_entry_staged_filetype_set(ie, - GOT_FILEIDX_MODE_SYMLINK); + int is_bad_symlink = 0; + if (!a->allow_bad_symlinks) { + char target_path[PATH_MAX]; + ssize_t target_len; + target_len = readlink(ondisk_path, target_path, + sizeof(target_path)); + if (target_len == -1) { + err = got_error_from_errno2("readlink", + ondisk_path); + break; + } + err = is_bad_symlink_target(&is_bad_symlink, + target_path, target_len, ondisk_path, + a->worktree->root_path); + if (err) + break; + if (is_bad_symlink) { + err = got_error_path(ondisk_path, + GOT_ERR_BAD_SYMLINK); + break; + } + } + if (is_bad_symlink) + got_fileindex_entry_staged_filetype_set(ie, + GOT_FILEIDX_MODE_BAD_SYMLINK); + else + got_fileindex_entry_staged_filetype_set(ie, + GOT_FILEIDX_MODE_SYMLINK); } else { got_fileindex_entry_staged_filetype_set(ie, GOT_FILEIDX_MODE_REGULAR_FILE); @@ -7239,7 +7292,7 @@ got_worktree_stage(struct got_worktree *worktree, struct got_pathlist_head *paths, got_worktree_status_cb status_cb, void *status_arg, got_worktree_patch_cb patch_cb, void *patch_arg, - struct got_repository *repo) + int allow_bad_symlinks, struct got_repository *repo) { const struct got_error *err = NULL, *sync_err, *unlockerr; struct got_pathlist_entry *pe; @@ -7290,6 +7343,7 @@ got_worktree_stage(struct got_worktree *worktree, spa.status_cb = status_cb; spa.status_arg = status_arg; spa.staged_something = 0; + spa.allow_bad_symlinks = allow_bad_symlinks; TAILQ_FOREACH(pe, paths, entry) { err = worktree_status(worktree, pe->path, fileindex, repo, stage_path, &spa, NULL, NULL, 0, 0); blob - 55663aa76dce7c64b71dc3d173d4daefb0d66870 blob + eb45b1bbd4a5793290cac2e8dbc68456aab8018e --- regress/cmdline/cherrypick.sh +++ regress/cmdline/cherrypick.sh @@ -509,7 +509,7 @@ function test_cherrypick_symlink_conflicts { # added symlink to file A vs added symlink to file B (cd $testroot/wt && ln -sf beta new.link) (cd $testroot/wt && got add new.link > /dev/null) - (cd $testroot/wt && got commit -m "change on symlinks on branch foo" \ + (cd $testroot/wt && got commit -S -m "change symlinks on foo" \ > /dev/null) (cd $testroot/wt && got update >/dev/null) blob - f6bf52a3c60f518baf559ef3112a4086136ee414 blob + 1f478a3523850ec49f239dc826069cc14c97b220 --- regress/cmdline/commit.sh +++ regress/cmdline/commit.sh @@ -992,8 +992,28 @@ function test_commit_symlink { (cd $testroot/wt && got add alpha.link epsilon.link passwd.link \ epsilon/beta.link nonexistent.link > /dev/null) - (cd $testroot/wt && got commit -m 'test commit_symlink' > $testroot/stdout) + (cd $testroot/wt && got commit -m 'test commit_symlink' \ + > $testroot/stdout 2> $testroot/stderr) + ret="$?" + if [ "$ret" == "0" ]; then + echo "got commit succeeded unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + echo -n "got: $testroot/wt/passwd.link: " > $testroot/stderr.expected + echo "symbolic link points outside of paths under version control" \ + >> $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + (cd $testroot/wt && got commit -S -m 'test commit_symlink' \ + > $testroot/stdout) + local head_rev=`git_show_head $testroot/repo` echo "A alpha.link" > $testroot/stdout.expected echo "A epsilon.link" >> $testroot/stdout.expected @@ -1037,15 +1057,37 @@ function test_commit_symlink { rm $testroot/wt/epsilon/beta.link echo "this is a regular file" > $testroot/wt/epsilon/beta.link (cd $testroot/wt && ln -sf .got/bar dotgotbar.link) + (cd $testroot/wt && got add dotgotbar.link > /dev/null) (cd $testroot/wt && got rm nonexistent.link > /dev/null) (cd $testroot/wt && ln -sf gamma/delta zeta.link) (cd $testroot/wt && ln -sf alpha new.link) (cd $testroot/wt && got add new.link > /dev/null) - (cd $testroot/wt && got commit -m 'test commit_symlink' > $testroot/stdout) + (cd $testroot/wt && got commit -m 'test commit_symlink' \ + > $testroot/stdout 2> $testroot/stderr) + ret="$?" + if [ "$ret" == "0" ]; then + echo "got commit succeeded unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + echo -n "got: $testroot/wt/dotgotbar.link: " > $testroot/stderr.expected + echo "symbolic link points outside of paths under version control" \ + >> $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + (cd $testroot/wt && got commit -S -m 'test commit_symlink' \ + > $testroot/stdout) + local head_rev=`git_show_head $testroot/repo` - echo "A new.link" > $testroot/stdout.expected + echo "A dotgotbar.link" > $testroot/stdout.expected + echo "A new.link" >> $testroot/stdout.expected echo "M alpha.link" >> $testroot/stdout.expected echo "M epsilon/beta.link" >> $testroot/stdout.expected echo "M epsilon.link" >> $testroot/stdout.expected @@ -1065,6 +1107,7 @@ function test_commit_symlink { alpha alpha.link@ -> beta beta +dotgotbar.link@ -> .got/bar epsilon/ epsilon/beta.link epsilon/zeta @@ -1096,7 +1139,8 @@ function test_commit_fix_bad_symlink { (cd $testroot/wt && ln -s /etc/passwd passwd.link) (cd $testroot/wt && got add passwd.link > /dev/null) - (cd $testroot/wt && got commit -m 'commit bad symlink' > $testroot/stdout) + (cd $testroot/wt && got commit -S -m 'commit bad symlink' \ + > $testroot/stdout) if [ -h $testroot/wt/passwd.link ]; then echo "passwd.link is a symlink but should be a regular file" >&2 blob - 937cb0bffe198932b2348672a1f25bf74819b672 blob + 90792acb612758516f6519ea08af7cfa75605260 --- regress/cmdline/stage.sh +++ regress/cmdline/stage.sh @@ -2383,7 +2383,25 @@ function test_stage_symlink { (cd $testroot/wt && ln -sf gamma/delta zeta.link) (cd $testroot/wt && got add zeta.link > /dev/null) - (cd $testroot/wt && got stage > $testroot/stdout) + (cd $testroot/wt && got stage > $testroot/stdout 2> $testroot/stderr) + ret="$?" + if [ "$ret" == "0" ]; then + echo "got stage succeeded unexpectedly" >&2 + test_done "$testroot" "$ret" + return 1 + fi + echo -n "got: $testroot/wt/dotgotbar.link: " > $testroot/stderr.expected + echo "symbolic link points outside of paths under version control" \ + >> $testroot/stderr.expected + cmp -s $testroot/stderr.expected $testroot/stderr + ret="$?" + if [ "$ret" != "0" ]; then + diff -u $testroot/stderr.expected $testroot/stderr + test_done "$testroot" "$ret" + return 1 + fi + + (cd $testroot/wt && got stage -S > $testroot/stdout) cat > $testroot/stdout.expected < /dev/null) - (cd $testroot/wt && got stage > /dev/null) + (cd $testroot/wt && got stage -S > /dev/null) (cd $testroot/wt && got status > $testroot/stdout) cat > $testroot/stdout.expected <