diff mbox series

[v2,3/3] *: avoid "whitelist"

Message ID 0d862cbbebe1a9f47f72255217faf734b1db4055.1657852722.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Remove use of "whitelist" | expand

Commit Message

Derrick Stolee July 15, 2022, 2:38 a.m. UTC
From: Derrick Stolee <derrickstolee@github.com>

The word "whitelist" has cultural implications that are not inclusive.
Thankfully, it is not difficult to reword and avoid its use.

A previous change already modified the documentation for git-cvsserver
and git-daemon to refer to a directory list. It is simple to update the
comments and error messages here to refer to that directory list.

In the case of transport.c, the GIT_ALLOW_PROTOCOL environment variable
was referred to as a "whitelist", but the word "allow" is already part
of the variable. Replace "whitelist" with "allow_list" in these cases to
demonstrate that we are processing a list of allowed protocols.

After this change, the only remaining uses of "whitelist" and its
companion "blacklist" are in release notes for older versions of Git and
in the sha1dc project, which is maintained independently.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 daemon.c           | 8 ++++----
 git-cvsserver.perl | 2 +-
 transport.c        | 8 ++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

Comments

Ævar Arnfjörð Bjarmason July 15, 2022, 11:19 a.m. UTC | #1
On Fri, Jul 15 2022, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>
> [...]
> -	logerror("'%s': not in whitelist", path);
> +	logerror("'%s': not in directory list", path);
>  	return NULL;		/* Fallthrough. Deny by default */
>  }
>  	if (strict_paths && (!ok_paths || !*ok_paths))
> -		die("option --strict-paths requires a whitelist");
> +		die("option --strict-paths requires a directory list");
>  
>  	if (base_path && !is_directory(base_path))
>  		die("base-path '%s' does not exist or is not a directory",

The former here is worse than before, which relates to the latter.

Put yourself in the shoes of a user who gets this error message. You
then do "man git-daemon", search for "whitelist", and the only thing
you'll find are references to --strict-paths and --interpolated-path,
which puts you on the right path.

IOW I don't care about the term "whitelist" here, I care that it it has
(goes and counts...) 5 occurances in the manpage, directorty has 16.

And SYNOPSIS of "git-daemon" says "...[<directory...]", so that
hypothehtical user is probably wondering e.g. if the directory they
provided on argv is bad. It's not just 5 v.s. 16, it's that we've
introduced jargon we only used for that feature, but now we use a vague
general term.

So we need to tell the user *what directory*. That's obviously easy, we
should refer back to the option that brought us here.

So per [1] I really welcome any improvements to the docs, but I was also
expecting to find various issues related to the origin of this series
blindly replacing things :( Please look a bit more at the bigger picture
beyond the specific hunks being changed, I might have missed some
similar issues on this read-through...

1. https://lore.kernel.org/git/220715.86sfn2zlkm.gmgdl@evledraar.gmail.com/
diff mbox series

Patch

diff --git a/daemon.c b/daemon.c
index 58f1077885c..e0706efc652 100644
--- a/daemon.c
+++ b/daemon.c
@@ -279,7 +279,7 @@  static const char *path_ok(const char *directory, struct hostinfo *hi)
 		/* The validation is done on the paths after enter_repo
 		 * appends optional {.git,.git/.git} and friends, but
 		 * it does not use getcwd().  So if your /pub is
-		 * a symlink to /mnt/pub, you can whitelist /pub and
+		 * a symlink to /mnt/pub, you can include /pub and
 		 * do not have to say /mnt/pub.
 		 * Do not say /pub/.
 		 */
@@ -298,7 +298,7 @@  static const char *path_ok(const char *directory, struct hostinfo *hi)
 			return path;
 	}
 
-	logerror("'%s': not in whitelist", path);
+	logerror("'%s': not in directory list", path);
 	return NULL;		/* Fallthrough. Deny by default */
 }
 
@@ -403,7 +403,7 @@  static int run_service(const char *dir, struct daemon_service *service,
 	 * a "git-daemon-export-ok" flag that says that the other side
 	 * is ok with us doing this.
 	 *
-	 * path_ok() uses enter_repo() and does whitelist checking.
+	 * path_ok() uses enter_repo() and checks for included directories.
 	 * We only need to make sure the repository is exported.
 	 */
 
@@ -1444,7 +1444,7 @@  int cmd_main(int argc, const char **argv)
 		cred = prepare_credentials(user_name, group_name);
 
 	if (strict_paths && (!ok_paths || !*ok_paths))
-		die("option --strict-paths requires a whitelist");
+		die("option --strict-paths requires a directory list");
 
 	if (base_path && !is_directory(base_path))
 		die("base-path '%s' does not exist or is not a directory",
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 4c8118010a8..ec64f06af7c 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -152,7 +152,7 @@  $state->{allowed_roots} = [ @ARGV ];
 
 # don't export the whole system unless the users requests it
 if ($state->{'export-all'} && !@{$state->{allowed_roots}}) {
-    die "--export-all can only be used together with an explicit whitelist\n";
+    die "--export-all can only be used together with an explicit directory list\n";
 }
 
 # Environment handling for running under git-shell
diff --git a/transport.c b/transport.c
index 52db7a3cb09..b51e991e443 100644
--- a/transport.c
+++ b/transport.c
@@ -940,7 +940,7 @@  static int external_specification_len(const char *url)
 	return strchr(url, ':') - url;
 }
 
-static const struct string_list *protocol_whitelist(void)
+static const struct string_list *protocol_allow_list(void)
 {
 	static int enabled = -1;
 	static struct string_list allowed = STRING_LIST_INIT_DUP;
@@ -1020,9 +1020,9 @@  static enum protocol_allow_config get_protocol_config(const char *type)
 
 int is_transport_allowed(const char *type, int from_user)
 {
-	const struct string_list *whitelist = protocol_whitelist();
-	if (whitelist)
-		return string_list_has_string(whitelist, type);
+	const struct string_list *allow_list = protocol_allow_list();
+	if (allow_list)
+		return string_list_has_string(allow_list, type);
 
 	switch (get_protocol_config(type)) {
 	case PROTOCOL_ALLOW_ALWAYS: