During a conversation with Theo at g2k19 I somehow became convinced
that Got should avoid calling unveil before launching an editor.

The code I had written initially didn't make this assumption. So I
already knew at the time that running an editor would work just fine
with unveil already set up. But I was cautioned about future changes
to how unveil would operate.
Either the design of unveil wasn't quite finalized yet at that point,
or I misunderstood what Theo was trying to get across (no surprise,
that happens quite often ;)

Either way, this removes all the lies about unveil from got/got.c.
The issue persists in cvg/cvg.c but that can be fixed later (cvg has
no regression tests yet).

Tests are passing and some manual testing suggests interactive use
of the editor still works as expected.

ok?

-----------------------------------------------
 
 call unveil earlier in 'got import'
 
 We now know that unveil(2) will never traverse exec.
 No need to wait with unveil until the editor has been run.
 
diff 87890bc26c1c6958bd64bb9d46fbc29ba6a92d95 b01d8dca1ccae2fac6fc8a6b5ab03d47d0cb9e2e
commit - 87890bc26c1c6958bd64bb9d46fbc29ba6a92d95
commit + b01d8dca1ccae2fac6fc8a6b5ab03d47d0cb9e2e
blob - eb068e10c4f52020498b05f1af70fa1d2f6d5ee8
blob + be364f33ae558187761d281438ee798b41d5c990
--- got/got.c
+++ got/got.c
@@ -834,6 +834,29 @@ cmd_import(int argc, char *argv[])
 	if (error)
 		goto done;
 
+	path_dir = realpath(argv[0], NULL);
+	if (path_dir == NULL) {
+		error = got_error_from_errno2("realpath", argv[0]);
+		goto done;
+	}
+	got_path_strip_trailing_slashes(path_dir);
+
+	error = get_editor(&editor);
+	if (error)
+		goto done;
+
+	if (unveil(path_dir, "r") != 0) {
+		error = got_error_from_errno2("unveil", path_dir);
+		goto done;
+	}
+	if (unveil(editor, "x") != 0) {
+		error = got_error_from_errno2("unveil", editor);
+		goto done;
+	}
+	error = apply_unveil(got_repo_get_path(repo), 0, NULL);
+	if (error)
+		goto done;
+
 	error = get_author(&author, repo, NULL);
 	if (error)
 		return error;
@@ -872,21 +895,7 @@ cmd_import(int argc, char *argv[])
 		goto done;
 	}
 
-	path_dir = realpath(argv[0], NULL);
-	if (path_dir == NULL) {
-		error = got_error_from_errno2("realpath", argv[0]);
-		goto done;
-	}
-	got_path_strip_trailing_slashes(path_dir);
-
-	/*
-	 * unveil(2) traverses exec(2); if an editor is used we have
-	 * to apply unveil after the log message has been written.
-	 */
 	if (logmsg == NULL || *logmsg == '\0') {
-		error = get_editor(&editor);
-		if (error)
-			goto done;
 		free(logmsg);
 		error = collect_import_msg(&logmsg, &logmsg_path, editor,
 		    path_dir, refname);
@@ -898,20 +907,6 @@ cmd_import(int argc, char *argv[])
 		}
 	}
 
-	if (unveil(path_dir, "r") != 0) {
-		error = got_error_from_errno2("unveil", path_dir);
-		if (logmsg_path)
-			preserve_logmsg = 1;
-		goto done;
-	}
-
-	error = apply_unveil(got_repo_get_path(repo), 0, NULL);
-	if (error) {
-		if (logmsg_path)
-			preserve_logmsg = 1;
-		goto done;
-	}
-
 	error = got_repo_import(&new_commit_id, path_dir, logmsg,
 	    author, &ignores, repo, import_progress, NULL);
 	if (error) {

-----------------------------------------------
 
 call unveil earlier in 'got commit'
 
 We now know that unveil(2) will never traverse exec.
 No need to wait with unveil until the editor has been run.
 
diff b01d8dca1ccae2fac6fc8a6b5ab03d47d0cb9e2e e9005b2dca2feab760702c966d0c9b8ee754a693
commit - b01d8dca1ccae2fac6fc8a6b5ab03d47d0cb9e2e
commit + e9005b2dca2feab760702c966d0c9b8ee754a693
blob - be364f33ae558187761d281438ee798b41d5c990
blob + a3d939c4058c0a946db7bdf78bfdf2c4cb46dffe
--- got/got.c
+++ got/got.c
@@ -9150,10 +9150,6 @@ done:
 
 	if (fd != -1 && close(fd) == -1 && err == NULL)
 		err = got_error_from_errno2("close", a->logmsg_path);
-
-	/* Editor is done; we can now apply unveil(2) */
-	if (err == NULL)
-		err = apply_unveil(a->repo_path, 0, a->worktree_path);
 	if (err) {
 		free(*logmsg);
 		*logmsg = NULL;
@@ -9423,15 +9419,18 @@ cmd_commit(int argc, char *argv[])
 	if (author == NULL)
 		author = committer;
 
-	/*
-	 * unveil(2) traverses exec(2); if an editor is used we have
-	 * to apply unveil after the log message has been written.
-	 */
-	if (logmsg == NULL || strlen(logmsg) == 0)
+	if (logmsg == NULL || strlen(logmsg) == 0) {
 		error = get_editor(&editor);
-	else
-		error = apply_unveil(got_repo_get_path(repo), 0,
-		    got_worktree_get_root_path(worktree));
+		if (error)
+			goto done;
+		if (unveil(editor, "x") != 0) {
+			error = got_error_from_errno2("unveil", editor);
+			goto done;
+		}
+	}
+
+	error = apply_unveil(got_repo_get_path(repo), 0,
+	    got_worktree_get_root_path(worktree));
 	if (error)
 		goto done;
 

-----------------------------------------------
 
 call unveil earlier in 'got histedit'
 
 We now know that unveil(2) will never traverse exec.
 No need to wait with unveil until the editor has been run.
 
diff e9005b2dca2feab760702c966d0c9b8ee754a693 d84895356b2dbc532cec559f78c7cc9012ed2c6d
commit - e9005b2dca2feab760702c966d0c9b8ee754a693
commit + d84895356b2dbc532cec559f78c7cc9012ed2c6d
blob - a3d939c4058c0a946db7bdf78bfdf2c4cb46dffe
blob + 1894bb3e86cc2dba58804185cb071b246b95f9f5
--- got/got.c
+++ got/got.c
@@ -11975,10 +11975,10 @@ get_folded_commits(struct got_histedit_list_entry *hle
 
 static const struct got_error *
 histedit_edit_logmsg(struct got_histedit_list_entry *hle,
-    struct got_repository *repo)
+    const char *editor, struct got_repository *repo)
 {
 	char *logmsg_path = NULL, *id_str = NULL, *orig_logmsg = NULL;
-	char *logmsg = NULL, *new_msg = NULL, *editor = NULL;
+	char *logmsg = NULL, *new_msg = NULL;
 	const struct got_error *err = NULL;
 	struct got_commit_object *commit = NULL;
 	int logmsg_len;
@@ -12041,10 +12041,6 @@ histedit_edit_logmsg(struct got_histedit_list_entry *h
 	}
 	fd = -1;
 
-	err = get_editor(&editor);
-	if (err)
-		goto done;
-
 	err = edit_logmsg(&hle->logmsg, editor, logmsg_path, logmsg,
 	    logmsg_len, 0);
 	if (err) {
@@ -12063,7 +12059,6 @@ done:
 	free(logmsg_path);
 	free(logmsg);
 	free(orig_logmsg);
-	free(editor);
 	if (commit)
 		got_object_commit_close(commit);
 	return err;
@@ -12204,19 +12199,14 @@ histedit_check_script(struct got_histedit_list *histed
 
 static const struct got_error *
 histedit_run_editor(struct got_histedit_list *histedit_cmds,
-    const char *path, struct got_object_id_queue *commits,
-    struct got_repository *repo)
+    const char *editor, const char *path,
+    struct got_object_id_queue *commits, struct got_repository *repo)
 {
 	const struct got_error *err = NULL;
 	struct stat st, st2;
 	struct timespec timeout;
-	char *editor;
 	FILE *f = NULL;
 
-	err = get_editor(&editor);
-	if (err)
-		return err;
-
 	if (stat(path, &st) == -1) {
 		err = got_error_from_errno2("stat", path);
 		goto done;
@@ -12256,20 +12246,19 @@ histedit_run_editor(struct got_histedit_list *histedit
 done:
 	if (f && fclose(f) == EOF && err == NULL)
 		err = got_error_from_errno("fclose");
-	free(editor);
 	return err;
 }
 
 static const struct got_error *
 histedit_edit_list_retry(struct got_histedit_list *, const struct got_error *,
-    struct got_object_id_queue *, const char *, const char *,
+    struct got_object_id_queue *, const char *, const char *, const char *,
     struct got_repository *);
 
 static const struct got_error *
 histedit_edit_script(struct got_histedit_list *histedit_cmds,
     struct got_object_id_queue *commits, const char *branch_name,
     int edit_logmsg_only, int fold_only, int drop_only, int edit_only,
-    struct got_repository *repo)
+    const char *editor, struct got_repository *repo)
 {
 	const struct got_error *err;
 	FILE *f = NULL;
@@ -12297,13 +12286,14 @@ histedit_edit_script(struct got_histedit_list *histedi
 			goto done;
 		}
 		f = NULL;
-		err = histedit_run_editor(histedit_cmds, path, commits, repo);
+		err = histedit_run_editor(histedit_cmds, editor, path,
+		    commits, repo);
 		if (err) {
 			if (err->code != GOT_ERR_HISTEDIT_SYNTAX &&
 			    err->code != GOT_ERR_HISTEDIT_CMD)
 				goto done;
 			err = histedit_edit_list_retry(histedit_cmds, err,
-			    commits, path, branch_name, repo);
+			    commits, editor, path, branch_name, repo);
 		}
 	}
 done:
@@ -12380,7 +12370,8 @@ done:
 static const struct got_error *
 histedit_edit_list_retry(struct got_histedit_list *histedit_cmds,
     const struct got_error *edit_err, struct got_object_id_queue *commits,
-    const char *path, const char *branch_name, struct got_repository *repo)
+    const char *editor, const char *path, const char *branch_name,
+    struct got_repository *repo)
 {
 	const struct got_error *err = NULL, *prev_err = edit_err;
 	int resp = ' ';
@@ -12393,8 +12384,8 @@ histedit_edit_list_retry(struct got_histedit_list *his
 			resp = getchar();
 		if (resp == 'c') {
 			histedit_free_list(histedit_cmds);
-			err = histedit_run_editor(histedit_cmds, path, commits,
-			    repo);
+			err = histedit_run_editor(histedit_cmds, editor, path,
+			    commits, repo);
 			if (err) {
 				if (err->code != GOT_ERR_HISTEDIT_SYNTAX &&
 				    err->code != GOT_ERR_HISTEDIT_CMD)
@@ -12407,7 +12398,7 @@ histedit_edit_list_retry(struct got_histedit_list *his
 		} else if (resp == 'r') {
 			histedit_free_list(histedit_cmds);
 			err = histedit_edit_script(histedit_cmds,
-			    commits, branch_name, 0, 0, 0, 0, repo);
+			    commits, branch_name, 0, 0, 0, 0, editor, repo);
 			if (err) {
 				if (err->code != GOT_ERR_HISTEDIT_SYNTAX &&
 				    err->code != GOT_ERR_HISTEDIT_CMD)
@@ -12497,7 +12488,8 @@ static const struct got_error *
 histedit_commit(struct got_pathlist_head *merged_paths,
     struct got_worktree *worktree, struct got_fileindex *fileindex,
     struct got_reference *tmp_branch, struct got_histedit_list_entry *hle,
-    const char *committer, int allow_conflict, struct got_repository *repo)
+    const char *committer, int allow_conflict, const char *editor,
+    struct got_repository *repo)
 {
 	const struct got_error *err;
 	struct got_commit_object *commit;
@@ -12505,7 +12497,7 @@ histedit_commit(struct got_pathlist_head *merged_paths
 
 	if ((hle->cmd->code == GOT_HISTEDIT_EDIT || get_folded_commits(hle))
 	    && hle->logmsg == NULL) {
-		err = histedit_edit_logmsg(hle, repo);
+		err = histedit_edit_logmsg(hle, editor, repo);
 		if (err)
 			return err;
 	}
@@ -12603,6 +12595,7 @@ cmd_histedit(int argc, char *argv[])
 	int drop_only = 0, edit_logmsg_only = 0, fold_only = 0, edit_only = 0;
 	int allow_conflict = 0, list_backups = 0, delete_backups = 0;
 	const char *edit_script_path = NULL;
+	char *editor = NULL;
 	struct got_object_id_queue commits;
 	struct got_pathlist_head merged_paths;
 	const struct got_object_id_queue *parent_ids;
@@ -12758,15 +12751,6 @@ cmd_histedit(int argc, char *argv[])
 	else if (argc != 0)
 		usage_histedit();
 
-	/*
-	 * This command cannot apply unveil(2) in all cases because the
-	 * user may choose to run an editor to edit the histedit script
-	 * and to edit individual commit log messages.
-	 * unveil(2) traverses exec(2); if an editor is used we have to
-	 * apply unveil after edit script and log messages have been written.
-	 * XXX TODO: Make use of unveil(2) where possible.
-	 */
-
 	cwd = getcwd(NULL, 0);
 	if (cwd == NULL) {
 		error = got_error_from_errno("getcwd");
@@ -12804,16 +12788,34 @@ cmd_histedit(int argc, char *argv[])
 		    GOT_WORKTREE_HISTEDIT_BACKUP_REF_PREFIX,
 		    argc == 1 ? argv[0] : NULL, delete_backups, repo);
 		goto done; /* nothing else to do */
+	} else {
+		error = get_gitconfig_path(&gitconfig_path);
+		if (error)
+			goto done;
+		error = got_repo_open(&repo, got_worktree_get_repo_path(worktree),
+		    gitconfig_path, pack_fds);
+		if (error != NULL)
+			goto done;
+		error = get_editor(&editor);
+		if (error)
+			goto done;
+		if (unveil(editor, "x") != 0) {
+			error = got_error_from_errno2("unveil", editor);
+			goto done;
+		}
+		if (edit_script_path) {
+			if (unveil(edit_script_path, "r") != 0) {
+				error = got_error_from_errno2("unveil",
+				    edit_script_path);
+				goto done;
+			}
+		}
+		error = apply_unveil(got_repo_get_path(repo), 0,
+		    got_worktree_get_root_path(worktree));
+		if (error)
+			goto done;
 	}
 
-	error = get_gitconfig_path(&gitconfig_path);
-	if (error)
-		goto done;
-	error = got_repo_open(&repo, got_worktree_get_repo_path(worktree),
-	    gitconfig_path, pack_fds);
-	if (error != NULL)
-		goto done;
-
 	if (worktree != NULL && !list_backups && !delete_backups) {
 		error = worktree_has_logmsg_ref("histedit", worktree, repo);
 		if (error)
@@ -13009,7 +13011,7 @@ cmd_histedit(int argc, char *argv[])
 				branch_name += 11;
 			error = histedit_edit_script(&histedit_cmds, &commits,
 			    branch_name, edit_logmsg_only, fold_only,
-			    drop_only, edit_only, repo);
+			    drop_only, edit_only, editor, repo);
 			if (error) {
 				got_worktree_histedit_abort(worktree, fileindex,
 				    repo, branch, base_commit_id,
@@ -13071,7 +13073,7 @@ cmd_histedit(int argc, char *argv[])
 				if (have_changes) {
 					error = histedit_commit(NULL, worktree,
 					    fileindex, tmp_branch, hle,
-					    committer, allow_conflict, repo);
+					    committer, allow_conflict, editor, repo);
 					if (error)
 						goto done;
 				} else {
@@ -13142,13 +13144,13 @@ cmd_histedit(int argc, char *argv[])
 				goto done;
 			continue;
 		} else if (hle->cmd->code == GOT_HISTEDIT_MESG) {
-			error = histedit_edit_logmsg(hle, repo);
+			error = histedit_edit_logmsg(hle, editor, repo);
 			if (error)
 				goto done;
 		}
 
 		error = histedit_commit(&merged_paths, worktree, fileindex,
-		    tmp_branch, hle, committer, allow_conflict, repo);
+		    tmp_branch, hle, committer, allow_conflict, editor, repo);
 		got_pathlist_free(&merged_paths, GOT_PATHLIST_FREE_PATH);
 		if (error)
 			goto done;
@@ -13183,6 +13185,7 @@ cmd_histedit(int argc, char *argv[])
 		    branch, repo);
 done:
 	free(cwd);
+	free(editor);
 	free(committer);
 	free(gitconfig_path);
 	got_object_id_queue_free(&commits);

-----------------------------------------------
 
 call unveil earlier in 'got tag'
 
 We now know that unveil(2) will never traverse exec.
 No need to wait with unveil until the editor has been run.
 
diff d84895356b2dbc532cec559f78c7cc9012ed2c6d a9f25ba432efbc09d7f521f9a95bee860d168b54
commit - d84895356b2dbc532cec559f78c7cc9012ed2c6d
commit + a9f25ba432efbc09d7f521f9a95bee860d168b54
blob - 1894bb3e86cc2dba58804185cb071b246b95f9f5
blob + 0685c98825a1a25a87beda8b3db3f24546a7fab3
--- got/got.c
+++ got/got.c
@@ -7547,11 +7547,10 @@ done:
 
 static const struct got_error *
 get_tag_message(char **tagmsg, char **tagmsg_path, const char *commit_id_str,
-    const char *tag_name, const char *repo_path)
+    const char *tag_name, const char *editor, const char *repo_path)
 {
 	const struct got_error *err = NULL;
 	char *template = NULL, *initial_content = NULL;
-	char *editor = NULL;
 	int initial_content_len;
 	int fd = -1;
 
@@ -7582,15 +7581,11 @@ get_tag_message(char **tagmsg, char **tagmsg_path, con
 	}
 	fd = -1;
 
-	err = get_editor(&editor);
-	if (err)
-		goto done;
 	err = edit_logmsg(tagmsg, editor, *tagmsg_path, initial_content,
 	    initial_content_len, 1);
 done:
 	free(initial_content);
 	free(template);
-	free(editor);
 
 	if (fd != -1 && close(fd) == -1 && err == NULL)
 		err = got_error_from_errno2("close", *tagmsg_path);
@@ -7605,7 +7600,7 @@ done:
 static const struct got_error *
 add_tag(struct got_repository *repo, const char *tagger,
     const char *tag_name, const char *commit_arg, const char *tagmsg_arg,
-    const char *signer_id, int verbosity)
+    const char *signer_id, const char *editor, int verbosity)
 {
 	const struct got_error *err = NULL;
 	struct got_object_id *commit_id = NULL, *tag_id = NULL;
@@ -7654,20 +7649,13 @@ add_tag(struct got_repository *repo, const char *tagge
 
 	if (tagmsg_arg == NULL) {
 		err = get_tag_message(&tagmsg, &tagmsg_path, commit_id_str,
-		    tag_name, got_repo_get_path(repo));
+		    tag_name, editor, got_repo_get_path(repo));
 		if (err) {
 			if (err->code != GOT_ERR_COMMIT_MSG_EMPTY &&
 			    tagmsg_path != NULL)
 				preserve_tagmsg = 1;
 			goto done;
 		}
-		/* Editor is done; we can now apply unveil(2) */
-		err = got_sigs_apply_unveil();
-		if (err)
-			goto done;
-		err = apply_unveil(got_repo_get_path(repo), 0, NULL);
-		if (err)
-			goto done;
 	}
 
 	err = got_object_tag_create(&tag_id, tag_name, commit_id,
@@ -7726,7 +7714,7 @@ cmd_tag(int argc, char *argv[])
 	struct got_worktree *worktree = NULL;
 	char *cwd = NULL, *repo_path = NULL, *commit_id_str = NULL;
 	char *gitconfig_path = NULL, *tagger = NULL, *keyword_idstr = NULL;
-	char *allowed_signers = NULL, *revoked_signers = NULL;
+	char *allowed_signers = NULL, *revoked_signers = NULL, *editor = NULL;
 	const char *signer_id = NULL;
 	const char *tag_name = NULL, *commit_id_arg = NULL, *tagmsg = NULL;
 	int ch, do_list = 0, verify_tags = 0, verbosity = 0;
@@ -7885,16 +7873,23 @@ cmd_tag(int argc, char *argv[])
 		if (signer_id == NULL)
 			signer_id = get_signer_id(repo, worktree);
 
-		if (tagmsg) {
-			if (signer_id) {
-				error = got_sigs_apply_unveil();
-				if (error)
-					goto done;
+		if (tagmsg == NULL) {
+			error = get_editor(&editor);
+			if (error)
+				goto done;
+			if (unveil(editor, "x") != 0) {
+				error = got_error_from_errno2("unveil", editor);
+				goto done;
 			}
-			error = apply_unveil(got_repo_get_path(repo), 0, NULL);
+		}
+		if (signer_id) {
+			error = got_sigs_apply_unveil();
 			if (error)
 				goto done;
 		}
+		error = apply_unveil(got_repo_get_path(repo), 0, NULL);
+		if (error)
+			goto done;
 
 		if (commit_id_arg == NULL) {
 			struct got_reference *head_ref;
@@ -7928,7 +7923,7 @@ cmd_tag(int argc, char *argv[])
 
 		error = add_tag(repo, tagger, tag_name,
 		    commit_id_str ? commit_id_str : commit_id_arg, tagmsg,
-		    signer_id, verbosity);
+		    signer_id, editor, verbosity);
 	}
 done:
 	if (repo) {
@@ -7945,6 +7940,7 @@ done:
 			error = pack_err;
 	}
 	free(cwd);
+	free(editor);
 	free(repo_path);
 	free(gitconfig_path);
 	free(commit_id_str);