[PATCH v3 0/5] Remove use of "whitelist"

["Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>]

The word "whitelist" has cultural implications that make its use
non-inclusive.

A previous version of this series recommended the replacement of
"allowlist", but that term is still new and not accepted by some common
dictionaries.

Instead, this version avoids the use of "whitelist" by rewording the
sentences that use it. In many cases, this improves readability since the
term is used suddenly without other context (and in some cases, is not
necessary at all).

There is one case where "whitelist" is replaced with "allow_list" but that
is because we are operating on a string list parsed from the
GIT_ALLOW_PROTOCOL environment variable.


Updates in v3
=============

 * Reorganized patches so changes to git-daemon and git-cvsserver are
   bunched together across code, documentation, and tests. Their
   documentation is more thoroughly modified to clarify the role of the
   directory arguments.
 * The changes surrounding SANITIZE=leak are delayed for Ævar's work that
   will update the documentation while changing the behavior of the test
   variables.
 * Several replacements are made to clarify meaning instead of doing a
   simple replacement. Thanks, Ævar for the suggestions.

Thanks, -Stolee

Derrick Stolee (5):
  daemon: clarify directory arguments
  git-cvsserver: clarify directory list
  git.txt: remove redundant language
  t: avoid "whitelist"
  transport.c: avoid "whitelist"

 Documentation/git-cvsserver.txt | 19 ++++++++++---------
 Documentation/git-daemon.txt    | 21 +++++++++++----------
 Documentation/git.txt           |  4 +---
 daemon.c                        |  8 ++++----
 git-cvsserver.perl              |  2 +-
 t/lib-proto-disable.sh          |  6 +++---
 t/t5812-proto-disable-http.sh   |  2 +-
 t/t5815-submodule-protos.sh     |  4 ++--
 t/t9400-git-cvsserver-server.sh |  2 +-
 t/test-lib-functions.sh         |  3 +--
 transport.c                     |  8 ++++----
 11 files changed, 39 insertions(+), 40 deletions(-)


base-commit: e4a4b31577c7419497ac30cebe30d755b97752c5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1274%2Fderrickstolee%2Fallow-deny-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1274/derrickstolee/allow-deny-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1274

Range-diff vs v2:

 1:  19632a2d245 ! 1:  d5ca7bffac0 Documentation: remove use of whitelist
     @@ Metadata
      Author: Derrick Stolee <derrickstolee@github.com>
      
       ## Commit message ##
     -    Documentation: remove use of whitelist
     +    daemon: clarify directory arguments
      
     -    The word "whitelist" has cultural implications that are not inclusive.
     -    Thankfully, it is not difficult to reword and avoid its use.
     +    The undecorated arguments to the 'git-daemon' command provide a list of
     +    directories. When at least one directory is specified, then 'git-daemon'
     +    only serves requests that are within that directory list. The boolean
     +    '--strict-paths' option makes the list more explicit in that
     +    subdirectories are no longer included.
      
     -    Remove uses of "whitelist" in the Documentation/ directory. In all
     -    cases, we can rewrite the sentences to add clarity instead of relying on
     -    the reader understanding this term.
     +    The existing documentation and error messages around this directory list
     +    refer to it and its behavior as a "whitelist". The word "whitelist" has
     +    cultural implications that are not inclusive.  Thankfully, it is not
     +    difficult to reword and avoid its use. In the process, we can define the
     +    purpose of this directory list directly.
      
     -    The most substantial change is to git-daemon.txt which had several uses,
     -    but we can refer to the input directories as a list of included
     -    directories, making the descriptions slightly simpler.
     +    In Documentation/git-daemon.txt, rewrite the OPTIONS section around the
     +    '<directory>' option. Add additional clarity to the other options that
     +    refer to these directories.
      
     -    Helped-by: Jeff King <peff@peff.net>
     -    Helped-by: Junio C Hamano <gitster@pobox.com>
     -    Signed-off-by: Derrick Stolee <derrickstolee@github.com>
     +    Some error messages can also be improved in daemon.c. The
     +    '--strict-paths' option requires '<directory>' arguments, so refer to
     +    that section of the documentation directly. A logerror() call points out
     +    that a requested directory is not in the specified directory list. We
     +    can use "list" here without any loss of information.
      
     - ## Documentation/git-cvsserver.txt ##
     -@@ Documentation/git-cvsserver.txt: circumstances, allowing easier restricted usage through git-shell.
     - 
     - GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.
     - 
     --GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The
     -+GIT_CVSSERVER_ROOT specifies a single allowed directory. The
     - repository must still be configured to allow access through
     - git-cvsserver, as described above.
     - 
     +    Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
       ## Documentation/git-daemon.txt ##
      @@ Documentation/git-daemon.txt: that service if it is enabled.
     @@ Documentation/git-daemon.txt: OPTIONS
       	"/foo/repo.git" or "/foo/repo/.git") and don't do user-relative paths.
       	'git daemon' will refuse to start when this option is enabled and no
      -	whitelist is specified.
     -+	specific directories are specified.
     ++	directory arguments are provided.
       
       --base-path=<path>::
       	Remap all the path requests as relative to the given path.
     @@ Documentation/git-daemon.txt: standard output to be sent to the requestor as an
       
       <directory>::
      -	A directory to add to the whitelist of allowed directories. Unless
     -+	A directory to add to the list of allowed directories. Unless
     - 	--strict-paths is specified this will also include subdirectories
     - 	of each named directory.
     +-	--strict-paths is specified this will also include subdirectories
     +-	of each named directory.
     ++	The remaining arguments provide a list of directories. If any
     ++	directories are specified, then the `git-daemon` process will
     ++	serve a requested directory only if it is contained in one of
     ++	these directories. If `--strict-paths` is specified, then the
     ++	requested directory must match one of these directories exactly.
       
     + SERVICES
     + --------
      @@ Documentation/git-daemon.txt: git		9418/tcp		# Git Version Control System
       
       'git daemon' as inetd server::
     @@ Documentation/git-daemon.txt: git		9418/tcp		# Git Version Control System
      -	repository under the whitelisted set of directories, /pub/foo
      -	and /pub/bar, place an entry like the following into
      -	/etc/inetd all on one line:
     -+	repository in the directory list of `/pub/foo` and `/pub/bar`,
     -+	place an entry like the following into `/etc/inetd` all on one line:
     ++	repository within `/pub/foo` or `/pub/bar`, place an entry like
     ++	the following into `/etc/inetd` all on one line:
       +
       ------------------------------------------------
       	git stream tcp nowait nobody  /usr/bin/git
      
     - ## Documentation/git.txt ##
     -@@ Documentation/git.txt: for full details.
     - 	`protocol.allow` is set to `never`, and each of the listed
     - 	protocols has `protocol.<name>.allow` set to `always`
     - 	(overriding any existing configuration). In other words, any
     --	protocol not mentioned will be disallowed (i.e., this is a
     --	whitelist, not a blacklist). See the description of
     -+	protocol not mentioned will be disallowed. See the description of
     - 	`protocol.allow` in linkgit:git-config[1] for more details.
     + ## daemon.c ##
     +@@ daemon.c: 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/.
     + 		 */
     +@@ daemon.c: 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 */
     + }
     + 
     +@@ daemon.c: 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.
     + 	 */
     + 
     +@@ daemon.c: 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 '<directory>' arguments");
       
     - `GIT_PROTOCOL_FROM_USER`::
     + 	if (base_path && !is_directory(base_path))
     + 		die("base-path '%s' does not exist or is not a directory",
 -:  ----------- > 2:  db32444b879 git-cvsserver: clarify directory list
 -:  ----------- > 3:  c1c83c4284b git.txt: remove redundant language
 2:  3c3c8c20bcb ! 4:  e08342f07d2 t/*: avoid "whitelist"
     @@ Metadata
      Author: Derrick Stolee <derrickstolee@github.com>
      
       ## Commit message ##
     -    t/*: avoid "whitelist"
     +    t: avoid "whitelist"
      
          The word "whitelist" has cultural implications that are not inclusive.
          Thankfully, it is not difficult to reword and avoid its use.
     @@ Commit message
      
          Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
     - ## t/README ##
     -@@ t/README: GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
     - test suite. Accept any boolean values that are accepted by git-config.
     - 
     - GIT_TEST_PASSING_SANITIZE_LEAK=<boolean> when compiled with
     --SANITIZE=leak will run only those tests that have whitelisted
     --themselves as passing with no memory leaks. Tests can be whitelisted
     --by setting "TEST_PASSES_SANITIZE_LEAK=true" before sourcing
     --"test-lib.sh" itself at the top of the test script. This test mode is
     --used by the "linux-leaks" CI target.
     -+SANITIZE=leak will run only those tests that have marked themselves as
     -+passing with no memory leaks by setting "TEST_PASSES_SANITIZE_LEAK=true"
     -+before sourcing "test-lib.sh" itself at the top of the test script. This
     -+test mode is used by the "linux-leaks" CI target.
     - 
     - GIT_TEST_PROTOCOL_VERSION=<n>, when set, makes 'protocol.version'
     - default to n.
     -
       ## t/lib-proto-disable.sh ##
      @@
       # Test routines for checking protocol disabling.
     @@ t/t5812-proto-disable-http.sh: test_expect_success 'create git-accessible repo'
       test_proto "smart http" http "$HTTPD_URL/smart/repo.git"
       
      -test_expect_success 'curl redirects respect whitelist' '
     -+test_expect_success 'curl redirects respect allowed protocols' '
     ++test_expect_success 'http(s) transport respects GIT_ALLOW_PROTOCOL' '
       	test_must_fail env GIT_ALLOW_PROTOCOL=http:https \
       			   GIT_SMART_HTTP=0 \
       		git clone "$HTTPD_URL/ftp-redir/repo.git" 2>stderr &&
     @@ t/t5815-submodule-protos.sh
       #!/bin/sh
       
      -test_description='test protocol whitelisting with submodules'
     -+test_description='test protocol restrictions with submodules'
     ++test_description='test protocol filtering with submodules'
       . ./test-lib.sh
       . "$TEST_DIRECTORY"/lib-proto-disable.sh
       
     @@ t/t5815-submodule-protos.sh: test_expect_success 'update of ext not allowed' '
       '
       
      -test_expect_success 'user can override whitelist' '
     -+test_expect_success 'user can override with environment variable' '
     ++test_expect_success 'user can filter protocols with GIT_ALLOW_PROTOCOL' '
       	GIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module
       '
       
      
     - ## t/t9400-git-cvsserver-server.sh ##
     -@@ t/t9400-git-cvsserver-server.sh: test_expect_success 'req_Root (export-all)' \
     -   'cat request-anonymous | git-cvsserver --export-all pserver "$WORKDIR" >log 2>&1 &&
     -    sed -ne \$p log | grep "^I LOVE YOU\$"'
     - 
     --test_expect_success 'req_Root failure (export-all w/o whitelist)' \
     -+test_expect_success 'req_Root failure (export-all w/o directory)' \
     -   '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'
     - 
     - test_expect_success 'req_Root (everything together)' \
     -
       ## t/test-lib-functions.sh ##
      @@ t/test-lib-functions.sh: test_set_prereq () {
       		# test_unset_prereq()
       		!*)
       			;;
      -		# (Temporary?) whitelist of things we can't easily
     -+		# (Temporary?) list of things we can't easily
     - 		# pretend not to support
     +-		# pretend not to support
     ++		# List of things we can't easily pretend to not support
       		SYMLINKS)
       			;;
     -
     - ## t/test-lib.sh ##
     -@@ t/test-lib.sh: then
     - 	test_done
     - fi
     - 
     --# skip non-whitelisted tests when compiled with SANITIZE=leak
     -+# skip unmarked tests when compiled with SANITIZE=leak
     - if test -n "$SANITIZE_LEAK"
     - then
     - 	if test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
     + 		# Inspecting whether GIT_TEST_FAIL_PREREQS is on
 3:  0d862cbbebe ! 5:  a4f31877b15 *: avoid "whitelist"
     @@ Metadata
      Author: Derrick Stolee <derrickstolee@github.com>
      
       ## Commit message ##
     -    *: avoid "whitelist"
     +    transport.c: avoid "whitelist"
      
          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.
     +    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.
      
          Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
     - ## daemon.c ##
     -@@ daemon.c: 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/.
     - 		 */
     -@@ daemon.c: 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 */
     - }
     - 
     -@@ daemon.c: 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.
     - 	 */
     - 
     -@@ daemon.c: 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",
     -
     - ## git-cvsserver.perl ##
     -@@ git-cvsserver.perl: $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
     -
       ## transport.c ##
      @@ transport.c: static int external_specification_len(const char *url)
       	return strchr(url, ':') - url;

-- 
gitgitgadget