[PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[Derrick Stolee via GitGitGadget]

The terms "allowlist" and "denylist" are self-defining. One "allows" things
while the other "denies" things.

These are better terms over "whitelist" and "blacklist" which require prior
knowledge of the terms or cultural expectations around what each color
"means".

This series replaces (almost) all uses of these terms with allowlist and
denylist. The only exceptions are in release notes for older Git versions.

There is no meaningful functional change, although one logging message in
daemon.c is changed and I'm unfamiliar with exactly how that might be
consumed.

Some recommend using "blocklist", but I personally prefer "denylist". To me,
"blocking" something seems permanent. "Denying" something seems ephemeral
and related to a specific request being denied due to some (possibly
mutable) state. I'm open to suggestions here. There are many fewer
replacements needed in this case.

I did not make any change to our CodingGuidelines. Hopefully having clear
usage throughout the codebase will be enough to promote using consistent
terminology.

Thanks, -Stolee

Derrick Stolee (3):
  Documentation: use allowlist and denylist
  t/*: use allowlist
  *: use allowlist and denylist

 Documentation/git-cvsserver.txt |  2 +-
 Documentation/git-daemon.txt    | 10 +++++-----
 Documentation/git.txt           |  2 +-
 daemon.c                        |  8 ++++----
 git-cvsserver.perl              |  2 +-
 sha1dc/sha1.c                   | 12 ++++++------
 t/README                        |  4 ++--
 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         |  2 +-
 t/test-lib.sh                   |  2 +-
 transport.c                     |  8 ++++----
 14 files changed, 33 insertions(+), 33 deletions(-)


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

[PATCH 1/3] Documentation: use allowlist and denylist

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

Using "allowlist" and "denylist" is a more precise definition of the
functionality they provide. The previous color-based words assume
cultural interpretation to provide the meaning.

Focus on replacements in the Documentation/ directory since these are
not functional uses.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-cvsserver.txt |  2 +-
 Documentation/git-daemon.txt    | 10 +++++-----
 Documentation/git.txt           |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 4dc57ed2547..40ea1f3af8e 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -313,7 +313,7 @@ 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-directory allowlist. The
 repository must still be configured to allow access through
 git-cvsserver, as described above.
 
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index fdc28c041c7..ff74a90aead 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -33,7 +33,7 @@ It verifies that the directory has the magic file "git-daemon-export-ok", and
 it will refuse to export any Git directory that hasn't explicitly been marked
 for export this way (unless the `--export-all` parameter is specified). If you
 pass some directory paths as 'git daemon' arguments, you can further restrict
-the offers to a whitelist comprising of those.
+the offers to a allowlist comprising of those.
 
 By default, only `upload-pack` service is enabled, which serves
 'git fetch-pack' and 'git ls-remote' clients, which are invoked
@@ -50,7 +50,7 @@ OPTIONS
 	Match paths exactly (i.e. don't allow "/foo/repo" when the real path is
 	"/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.
+	allowlist is specified.
 
 --base-path=<path>::
 	Remap all the path requests as relative to the given path.
@@ -73,7 +73,7 @@ OPTIONS
 	%IP for the server's IP address, %P for the port number,
 	and %D for the absolute path of the named repository.
 	After interpolation, the path is validated against the directory
-	whitelist.
+	allowlist.
 
 --export-all::
 	Allow pulling from all directories that look like Git repositories
@@ -218,7 +218,7 @@ standard output to be sent to the requestor as an error message when
 it declines the service.
 
 <directory>::
-	A directory to add to the whitelist of allowed directories. Unless
+	A directory to add to the allowlist of allowed directories. Unless
 	--strict-paths is specified this will also include subdirectories
 	of each named directory.
 
@@ -264,7 +264,7 @@ git		9418/tcp		# Git Version Control System
 
 'git daemon' as inetd server::
 	To set up 'git daemon' as an inetd service that handles any
-	repository under the whitelisted set of directories, /pub/foo
+	repository under the allowlisted set of directories, /pub/foo
 	and /pub/bar, place an entry like the following into
 	/etc/inetd all on one line:
 +
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 302607a4967..384718ee677 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -887,7 +887,7 @@ for full details.
 	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
+	allowlist, not a denylist). See the description of
 	`protocol.allow` in linkgit:git-config[1] for more details.
 
 `GIT_PROTOCOL_FROM_USER`::
-- 
gitgitgadget

Re: [PATCH 1/3] Documentation: use allowlist and denylist

[Jeff King]

On Wed, Jul 13, 2022 at 01:20:48PM +0000, Derrick Stolee via GitGitGadget wrote:

> Using "allowlist" and "denylist" is a more precise definition of the
> functionality they provide. The previous color-based words assume
> cultural interpretation to provide the meaning.
> 
> Focus on replacements in the Documentation/ directory since these are
> not functional uses.

Thanks, the direction looks reasonable to me. I knew at least about the
one for protocol.*, which I think I introduced, and had been meaning to
grep for others.

I think you need some grammatical fixups, though. E.g.:

> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> index fdc28c041c7..ff74a90aead 100644
> --- a/Documentation/git-daemon.txt
> +++ b/Documentation/git-daemon.txt
> @@ -33,7 +33,7 @@ It verifies that the directory has the magic file "git-daemon-export-ok", and
>  it will refuse to export any Git directory that hasn't explicitly been marked
>  for export this way (unless the `--export-all` parameter is specified). If you
>  pass some directory paths as 'git daemon' arguments, you can further restrict
> -the offers to a whitelist comprising of those.
> +the offers to a allowlist comprising of those.

You'd want s/a/an/ here.

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 302607a4967..384718ee677 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -887,7 +887,7 @@ for full details.
>  	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
> +	allowlist, not a denylist). See the description of
>  	`protocol.allow` in linkgit:git-config[1] for more details.

Ditto here.

>  'git daemon' as inetd server::
>  	To set up 'git daemon' as an inetd service that handles any
> -	repository under the whitelisted set of directories, /pub/foo
> +	repository under the allowlisted set of directories, /pub/foo
>  	and /pub/bar, place an entry like the following into
>  	/etc/inetd all on one line:

This one is more gut feeling.  Somehow "allowlisted" as an adjective
seems more awkward than "whitelisted". Probably because I've just seen
"whitelisted" so many more times. Or maybe it just crosses my personal
line of too many syllables. ;)

I don't know if there's an easy way around it. I don't have a suggestion
that's better than "allowlist" for the general term, and we want to use
the terms consistently. You could probably write it as:

  ...any repository under the set of directories in the allowlist...

but I'm sure somebody probably likes that less. :) So I register it only
as a suggestion, not a request for a change.

-Peff

Re: [PATCH 1/3] Documentation: use allowlist and denylist

[Derrick Stolee]

On 7/13/2022 11:21 AM, Jeff King wrote:
> On Wed, Jul 13, 2022 at 01:20:48PM +0000, Derrick Stolee via GitGitGadget wrote:
> 
>> Using "allowlist" and "denylist" is a more precise definition of the
>> functionality they provide. The previous color-based words assume
>> cultural interpretation to provide the meaning.
>>
>> Focus on replacements in the Documentation/ directory since these are
>> not functional uses.
> 
> Thanks, the direction looks reasonable to me. I knew at least about the
> one for protocol.*, which I think I introduced, and had been meaning to
> grep for others.
> 
> I think you need some grammatical fixups, though. E.g.:
> 
>> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
>> index fdc28c041c7..ff74a90aead 100644
>> --- a/Documentation/git-daemon.txt
>> +++ b/Documentation/git-daemon.txt
>> @@ -33,7 +33,7 @@ It verifies that the directory has the magic file "git-daemon-export-ok", and
>>  it will refuse to export any Git directory that hasn't explicitly been marked
>>  for export this way (unless the `--export-all` parameter is specified). If you
>>  pass some directory paths as 'git daemon' arguments, you can further restrict
>> -the offers to a whitelist comprising of those.
>> +the offers to a allowlist comprising of those.
> 
> You'd want s/a/an/ here.

Thank you for the careful attention to detail. I should have
been more careful when switching from a consonant to a vowel.
>>  'git daemon' as inetd server::
>>  	To set up 'git daemon' as an inetd service that handles any
>> -	repository under the whitelisted set of directories, /pub/foo
>> +	repository under the allowlisted set of directories, /pub/foo
>>  	and /pub/bar, place an entry like the following into
>>  	/etc/inetd all on one line:
> 
> This one is more gut feeling.  Somehow "allowlisted" as an adjective
> seems more awkward than "whitelisted". Probably because I've just seen
> "whitelisted" so many more times. Or maybe it just crosses my personal
> line of too many syllables. ;)
> 
> I don't know if there's an easy way around it. I don't have a suggestion
> that's better than "allowlist" for the general term, and we want to use
> the terms consistently. You could probably write it as:
> 
>   ...any repository under the set of directories in the allowlist...
> 
> but I'm sure somebody probably likes that less. :) So I register it only
> as a suggestion, not a request for a change.

I think that is the natural way to reword here, but I'll take some
time to think of alternatives before sending v2.

Thanks,
-Stolee

Re: [PATCH 1/3] Documentation: use allowlist and denylist

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> @@ -313,7 +313,7 @@ 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-directory allowlist. The

Nowhere before this documentation there is mention of white/allow.
They consistently say "list of allowed directories".

It may probably make the resulting text much easier to read if we
avoid the non-word "allowlist", which was recently invented only to
avoid using the word "whitelist".

    GIT_CVSSERVER_ROOT specifies a single allowed directory.

IOW, the original did not even have to say "whitelist".

> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> index fdc28c041c7..ff74a90aead 100644
> --- a/Documentation/git-daemon.txt
> +++ b/Documentation/git-daemon.txt
> @@ -33,7 +33,7 @@ It verifies that the directory has the magic file "git-daemon-export-ok", and
>  it will refuse to export any Git directory that hasn't explicitly been marked
>  for export this way (unless the `--export-all` parameter is specified). If you
>  pass some directory paths as 'git daemon' arguments, you can further restrict
> -the offers to a whitelist comprising of those.
> +the offers to a allowlist comprising of those.

"a -> an"; but I think the same suggestion as cvs-server equally
applies here.

    -for export this way (unless the `--export-all` parameter is specified). If you
    -pass some directory paths as 'git daemon' arguments, you can further restrict
    -the offers to a whitelist comprising of those.
    +for export this way (unless the `--export-all` parameter is specified).
    +You can furtherrestrict the offers by passing the list of allowed directories
    +as 'git daemon' arguments.

would be much easier to understand, with or without s/white/allow/.

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 302607a4967..384718ee677 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -887,7 +887,7 @@ for full details.
>  	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
> +	allowlist, not a denylist). See the description of

As I always tell contributors in my review, whenever we see "In
other words" and parenthesized follow-up explanation, we should take
it as an admission by the author that whatever the author wrote
before it is badly written and should have been expressed in a more
clear way.

    In other words, only the protocols listed on this variable are
    allowed and all others are disallowed.  See the description of
    ...

without "(i.e....)" would be shorter and easier to read, I would
think.

[PATCH 2/3] t/*: use allowlist

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

Using "allowlist" is a more precise definition of the functionality
provided. The previous color-based word assume cultural interpretation
to provide the meaning.

Focus on changes in the test scripts, since most of the changes are in
comments and test names. The one exception is the renamed test_allowlist
helper.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/README                        | 4 ++--
 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         | 2 +-
 t/test-lib.sh                   | 2 +-
 7 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/t/README b/t/README
index 309a31133c6..0c388a952f9 100644
--- a/t/README
+++ b/t/README
@@ -367,8 +367,8 @@ 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
+SANITIZE=leak will run only those tests that have allowlisted
+themselves as passing with no memory leaks. Tests can be allowlisted
 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.
diff --git a/t/lib-proto-disable.sh b/t/lib-proto-disable.sh
index 83babe57d95..6f2de57ef29 100644
--- a/t/lib-proto-disable.sh
+++ b/t/lib-proto-disable.sh
@@ -1,7 +1,7 @@
 # Test routines for checking protocol disabling.
 
-# Test clone/fetch/push with GIT_ALLOW_PROTOCOL whitelist
-test_whitelist () {
+# Test clone/fetch/push with GIT_ALLOW_PROTOCOL allowlist
+test_allowlist () {
 	desc=$1
 	proto=$2
 	url=$3
@@ -183,7 +183,7 @@ test_config () {
 #   $2 - machine-readable name of the protocol
 #   $3 - the URL to try cloning
 test_proto () {
-	test_whitelist "$@"
+	test_allowlist "$@"
 
 	test_config "$@"
 }
diff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh
index af8772fadaa..d6f9cd67588 100755
--- a/t/t5812-proto-disable-http.sh
+++ b/t/t5812-proto-disable-http.sh
@@ -16,7 +16,7 @@ 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 allowlist' '
 	test_must_fail env GIT_ALLOW_PROTOCOL=http:https \
 			   GIT_SMART_HTTP=0 \
 		git clone "$HTTPD_URL/ftp-redir/repo.git" 2>stderr &&
diff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh
index 06f55a1b8a0..eecc401831f 100755
--- a/t/t5815-submodule-protos.sh
+++ b/t/t5815-submodule-protos.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='test protocol whitelisting with submodules'
+test_description='test protocol allowlisting with submodules'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-proto-disable.sh
 
@@ -36,7 +36,7 @@ test_expect_success 'update of ext not allowed' '
 	test_must_fail git -C dst submodule update ext-module
 '
 
-test_expect_success 'user can override whitelist' '
+test_expect_success 'user can override allowlist' '
 	GIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module
 '
 
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 210ddf09e30..03962d598f5 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -221,7 +221,7 @@ 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 allowlist)' \
   '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'
 
 test_expect_success 'req_Root (everything together)' \
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 6da7273f1d5..5351bbd83b9 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -651,7 +651,7 @@ test_set_prereq () {
 		# test_unset_prereq()
 		!*)
 			;;
-		# (Temporary?) whitelist of things we can't easily
+		# (Temporary?) allowlist of things we can't easily
 		# pretend not to support
 		SYMLINKS)
 			;;
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 55857af601b..6f5fccccb75 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1408,7 +1408,7 @@ then
 	test_done
 fi
 
-# skip non-whitelisted tests when compiled with SANITIZE=leak
+# skip non-allowlisted tests when compiled with SANITIZE=leak
 if test -n "$SANITIZE_LEAK"
 then
 	if test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
-- 
gitgitgadget

[PATCH 3/3] *: use allowlist and denylist

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

Using "allowlist" and "denylist" is a more precise definition of the
functionality they provide. The previous color-based words assume
cultural interpretation to provide the meaning.

These changes to the Git codebase are mostly cosmetic. Several comments
are updated. The renamed protocol_allowlist() method is local to
transport.c so does not update any header file API definition. There are
some untranslated error messages that are reworded, so this _might_
affect error parsers. However, two of the three error messages are
around option parsing, so they are "fast failures". The one perhaps
meaningful change is the logerror() in daemon.c.

After this change, the only remaining uses of the previous words are
in release notes for older versions of Git.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 daemon.c           |  8 ++++----
 git-cvsserver.perl |  2 +-
 sha1dc/sha1.c      | 12 ++++++------
 transport.c        |  8 ++++----
 4 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/daemon.c b/daemon.c
index 58f1077885c..ed7c53b1110 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 allowlist /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 allowlist", 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 does allowlist checking.
 	 * 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 allowlist");
 
 	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..7d13b0a5ac1 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 allowlist\n";
 }
 
 # Environment handling for running under git-shell
diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c
index dede2cbddf9..b4a5f23c1ec 100644
--- a/sha1dc/sha1.c
+++ b/sha1dc/sha1.c
@@ -86,30 +86,30 @@
        defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \
        defined(__sparc))
 /*
- * Should define Big Endian for a whitelist of known processors. See
+ * Should define Big Endian for a allowlist of known processors. See
  * https://sourceforge.net/p/predef/wiki/Endianness/ and
  * http://www.oracle.com/technetwork/server-storage/solaris/portingtosolaris-138514.html
  */
 #define SHA1DC_BIGENDIAN
 
-/* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> */
+/* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> */
 #elif (defined(_AIX) || defined(__hpux))
 
 /*
- * Defines Big Endian on a whitelist of OSs that are known to be Big
+ * Defines Big Endian on a allowlist of OSs that are known to be Big
  * Endian-only. See
  * https://lore.kernel.org/git/93056823-2740-d072-1ebd-46b440b33d7e@felt.demon.nl/
  */
 #define SHA1DC_BIGENDIAN
 
-/* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> or <os whitelist> */
+/* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> or <os allowlist> */
 #elif defined(SHA1DC_ON_INTEL_LIKE_PROCESSOR)
 /*
  * As a last resort before we do anything else we're not 100% sure
- * about below, we blacklist specific processors here. We could add
+ * about below, we denylist specific processors here. We could add
  * more, see e.g. https://wiki.debian.org/ArchitectureSpecificsMemo
  */
-#else /* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> or <os whitelist> or <processor blacklist> */
+#else /* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> or <os allowlist> or <processor denylist> */
 
 /* We do nothing more here for now */
 /*#error "Uncomment this to see if you fall through all the detection"*/
diff --git a/transport.c b/transport.c
index 52db7a3cb09..321bbe382cc 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_allowlist(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 *allowlist = protocol_allowlist();
+	if (allowlist)
+		return string_list_has_string(allowlist, type);
 
 	switch (get_protocol_config(type)) {
 	case PROTOCOL_ALLOW_ALWAYS:
-- 
gitgitgadget

Re: [PATCH 3/3] *: use allowlist and denylist

[Johannes Schindelin]

Hi Stolee,

On Wed, 13 Jul 2022, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>
>
> Using "allowlist" and "denylist" is a more precise definition of the
> functionality they provide. The previous color-based words assume
> cultural interpretation to provide the meaning.
>
> These changes to the Git codebase are mostly cosmetic. Several comments
> are updated. The renamed protocol_allowlist() method is local to
> transport.c so does not update any header file API definition. There are
> some untranslated error messages that are reworded, so this _might_
> affect error parsers. However, two of the three error messages are
> around option parsing, so they are "fast failures". The one perhaps
> meaningful change is the logerror() in daemon.c.

I do consider `git daemon` less important these days because we have two
transports that are secure (which `git://` is not): HTTPS and SSH. That
suggests that the `daemon.c` change might have a very low impact to begin
with.

The other changes affect `cvsserver` which I consider even less important.
In fact, I would be in favor of deprecating it soon and of deleting it in
due time.

All the changes in this patch look good to me: ACK!

Thank you!
Dscho

>
> After this change, the only remaining uses of the previous words are
> in release notes for older versions of Git.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  daemon.c           |  8 ++++----
>  git-cvsserver.perl |  2 +-
>  sha1dc/sha1.c      | 12 ++++++------
>  transport.c        |  8 ++++----
>  4 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/daemon.c b/daemon.c
> index 58f1077885c..ed7c53b1110 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 allowlist /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 allowlist", 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 does allowlist checking.
>  	 * 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 allowlist");
>
>  	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..7d13b0a5ac1 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 allowlist\n";
>  }
>
>  # Environment handling for running under git-shell
> diff --git a/sha1dc/sha1.c b/sha1dc/sha1.c
> index dede2cbddf9..b4a5f23c1ec 100644
> --- a/sha1dc/sha1.c
> +++ b/sha1dc/sha1.c
> @@ -86,30 +86,30 @@
>         defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \
>         defined(__sparc))
>  /*
> - * Should define Big Endian for a whitelist of known processors. See
> + * Should define Big Endian for a allowlist of known processors. See
>   * https://sourceforge.net/p/predef/wiki/Endianness/ and
>   * http://www.oracle.com/technetwork/server-storage/solaris/portingtosolaris-138514.html
>   */
>  #define SHA1DC_BIGENDIAN
>
> -/* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> */
> +/* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> */
>  #elif (defined(_AIX) || defined(__hpux))
>
>  /*
> - * Defines Big Endian on a whitelist of OSs that are known to be Big
> + * Defines Big Endian on a allowlist of OSs that are known to be Big
>   * Endian-only. See
>   * https://lore.kernel.org/git/93056823-2740-d072-1ebd-46b440b33d7e@felt.demon.nl/
>   */
>  #define SHA1DC_BIGENDIAN
>
> -/* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> or <os whitelist> */
> +/* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> or <os allowlist> */
>  #elif defined(SHA1DC_ON_INTEL_LIKE_PROCESSOR)
>  /*
>   * As a last resort before we do anything else we're not 100% sure
> - * about below, we blacklist specific processors here. We could add
> + * about below, we denylist specific processors here. We could add
>   * more, see e.g. https://wiki.debian.org/ArchitectureSpecificsMemo
>   */
> -#else /* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> or <os whitelist> or <processor blacklist> */
> +#else /* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> or <os allowlist> or <processor denylist> */
>
>  /* We do nothing more here for now */
>  /*#error "Uncomment this to see if you fall through all the detection"*/
> diff --git a/transport.c b/transport.c
> index 52db7a3cb09..321bbe382cc 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_allowlist(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 *allowlist = protocol_allowlist();
> +	if (allowlist)
> +		return string_list_has_string(allowlist, type);
>
>  	switch (get_protocol_config(type)) {
>  	case PROTOCOL_ALLOW_ALWAYS:
> --
> gitgitgadget
>

Re: [PATCH 3/3] *: use allowlist and denylist

[Jeff King]

On Wed, Jul 13, 2022 at 01:20:50PM +0000, Derrick Stolee via GitGitGadget wrote:

> @@ -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 allowlist");

Here's another s/a/an/ case. I think there are a few others in comments
in other parts of the patch.

-Peff

Re: [PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[Johannes Schindelin]

Hi Stolee,

On Wed, 13 Jul 2022, Derrick Stolee via GitGitGadget wrote:

> The terms "allowlist" and "denylist" are self-defining. One "allows" things
> while the other "denies" things.
>
> These are better terms over "whitelist" and "blacklist" which require prior
> knowledge of the terms or cultural expectations around what each color
> "means".

I agree that "allowlist" and "denylist" are much better terms. As you say,
they are more obvious even for non-native speakers, but we do want to take
the cultural implications into account and be mindful to avoid needlessly
controversial language.

Therefore, I am very much in favor of this patch series: ACK!

Thank you for putting in the work to make this happen,
Dscho

Re: [PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> The terms "allowlist" and "denylist" are self-defining. One "allows" things
> while the other "denies" things.
>
> These are better terms over "whitelist" and "blacklist" which require prior
> knowledge of the terms or cultural expectations around what each color
> "means".

Half Devil's advocate mode on, as I got up on the wrong side of the
bed this morning.

I am very much for consistent uses of allow/deny and I think it is a
good idea to review and apply this series.

But I'd prefer to see us more honest to ourselves.  Like it or not,
the code comment and documentation are targetted toward those who
can read English, and when you say something is whitelisted in
English, you know exactly what it means, due to shared knowledge of
historical use of the word.

We are doing this change in the name of inclusion.  I find it
intellectually dishonest to avoid saying that true reason, and
instead say the allow/deny pair is more "precise".  They are not
more precise.  In fact, the fact why you have to choose between deny
and block and defend deny over block shows that these words are less
precise.  People who use white/black do not have to choose between
black and other colors and say "white/red may be OK but we choose
black because..." to defend the choice of their words.

The reason we do this change is because the project thinks that it
is the right thing to encourage the adoption of these more inclusive
words, together with other projects that did the same.

In addition, they are words more widely accepted in today's world,
and new folks are more likely to be educated with these words.  As
time goes by, the historical white/black will be less understood, so
it makes it a future-proofing change, as well.

Re: [PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[Derrick Stolee]

On 7/13/2022 12:18 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> The terms "allowlist" and "denylist" are self-defining. One "allows" things
>> while the other "denies" things.
>>
>> These are better terms over "whitelist" and "blacklist" which require prior
>> knowledge of the terms or cultural expectations around what each color
>> "means".
> 
> Half Devil's advocate mode on, as I got up on the wrong side of the
> bed this morning.
> 
> I am very much for consistent uses of allow/deny and I think it is a
> good idea to review and apply this series.
> 
> But I'd prefer to see us more honest to ourselves.  Like it or not,
> the code comment and documentation are targetted toward those who
> can read English, and when you say something is whitelisted in
> English, you know exactly what it means, due to shared knowledge of
> historical use of the word.
> 
> We are doing this change in the name of inclusion.  I find it
> intellectually dishonest to avoid saying that true reason, and
> instead say the allow/deny pair is more "precise".  They are not
> more precise.  In fact, the fact why you have to choose between deny
> and block and defend deny over block shows that these words are less
> precise.  People who use white/black do not have to choose between
> black and other colors and say "white/red may be OK but we choose
> black because..." to defend the choice of their words.
> 
> The reason we do this change is because the project thinks that it
> is the right thing to encourage the adoption of these more inclusive
> words, together with other projects that did the same.
> 
> In addition, they are words more widely accepted in today's world,
> and new folks are more likely to be educated with these words.  As
> time goes by, the historical white/black will be less understood, so
> it makes it a future-proofing change, as well.
 
I like how you start by saying you are playing devil's advocate,
but then go on to add more reasons to support the work. It's good
feedback to make the case stronger.

Thanks,
-Stolee

Re: [PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[Junio C Hamano]

Derrick Stolee <derrickstolee@github.com> writes:

> I like how you start by saying you are playing devil's advocate,
> but then go on to add more reasons to support the work. It's good
> feedback to make the case stronger.

I may have offered alternatives, but I am not "adding more" reasons.

Re: [PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[Ævar Arnfjörð Bjarmason]

On Wed, Jul 13 2022, Derrick Stolee via GitGitGadget wrote:

> The terms "allowlist" and "denylist" are self-defining. One "allows" things
> while the other "denies" things.

I've got a preference for things that can be found in widely available
dictionaries, these words seem to be tech neologisms.

The resulting wording also seems a bit ackward to me, e.g. we now say
that some tests are "allowlisted [...] as passing with no memory
leaks". Are we denying or allowing them to pass? No, they're going to
either pass or not.

So to me "whitelist" or "blacklist" is more natural when used in the
descriptive sense, whereas "allow" and "deny" are verbs, so that seems
to impart a sense of actively allowing or denying something.

> These are better terms over "whitelist" and "blacklist" which require prior
> knowledge of the terms or cultural expectations around what each color
> "means".

Apparently whitelist is defined in terms of blacklist, which per
Wikipedia originates in some 17th century play:
https://en.wikipedia.org/wiki/Blacklisting#Origins_of_the_term

> [...]
> Some recommend using "blocklist", but I personally prefer "denylist". To me,
> "blocking" something seems permanent. "Denying" something seems ephemeral
> and related to a specific request being denied due to some (possibly
> mutable) state. I'm open to suggestions here. There are many fewer
> replacements needed in this case.

I suspect the actual motivation is closer to that summarized in :
https://en.wikipedia.org/wiki/Whitelist#Controversy

Personally I'd really prefer if we didn't take these sort of changes,
and took the view that if something was readily understood that it was
good enough.

The CodingGuidelines note that we use a mix of US & UK english, so
forbidding certain words & basically requiring some of us to keep
abreast of the latest political trends in America is a bit too much. I'd
just like to write code, please...

> I did not make any change to our CodingGuidelines. Hopefully having clear
> usage throughout the codebase will be enough to promote using consistent
> terminology.

...particularly since I think what's being implied here is that we can
expect interested parties to be setting up the relevant E-Mail filters,
and asking patch submitters to change wording in the same way as this
series does.

We also have 30-40 uses of both terms in-tree, so it seems implausible
that people are mainly copying existing wording.

A few of the hunks here are changing docs I added, and I just added
those "naturally", i.e. I happened to think of those words to describe
what I was trying to get across).

Re: [PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Wed, Jul 13 2022, Derrick Stolee via GitGitGadget wrote:
>
>> The terms "allowlist" and "denylist" are self-defining. One "allows" things
>> while the other "denies" things.
>
> I've got a preference for things that can be found in widely available
> dictionaries, these words seem to be tech neologisms.

FWIW, I share the same.  I suspect that "whitelist" may be found in
more dictionaries than "allowlist".

e.g.

    https://www.merriam-webster.com/dictionary/allowlist
    https://www.merriam-webster.com/dictionary/whitelist

A statement "We have audience who are not native English speakers,
and may not share cultural background" may not be incorrect at all,
but that does not justify s/whitelist/allowlist/.  We end up with
sentences written with non-words that these non-natives cannot even
look up in dictionary.

If we can rephrase without using these invented words, we should do
so, especially when the result becomes even easier to read than the
original that used "whitelist".  I've shown a few examples in my
other messages in this thread.

Thanks.

Re: [PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[Derrick Stolee]

On 7/13/2022 6:28 PM, Junio C Hamano wrote:
> If we can rephrase without using these invented words, we should do
> so, especially when the result becomes even easier to read than the
> original that used "whitelist".  I've shown a few examples in my
> other messages in this thread.

Based on those examples, I agree that the best thing to do is to
rephrase to avoid the term altogether. This avoids confusion when
the reader does not know the term, as well as sometimes being more
consistent with the phrasing in the same document.

Thanks,
-Stolee

Re: [PATCH 0/3] Use "allowlist" and "denylist" tree-wide

[Ævar Arnfjörð Bjarmason]

On Wed, Jul 13 2022, Derrick Stolee via GitGitGadget wrote:

>  sha1dc/sha1.c                   | 12 ++++++------

Aside from anything else I've commented on: Please drop this part of the
change. If you'd like to change this take it up with upstream:
https://github.com/cr-marcstevens/sha1collisiondetection/

As a "git log" on "sha1dc sha1collisiondetection" will show we try to
keep a 1=1 mapping to upstream with this code, this would both diverge
us from upstream, and diverge sha1dc from our own submodule copy in
sha1collisiondetection/".

[PATCH v2 0/3] Remove use of "whitelist"

[Derrick Stolee via GitGitGadget]

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.

Thanks, -Stolee

Derrick Stolee (3):
  Documentation: remove use of whitelist
  t/*: avoid "whitelist"
  *: avoid "whitelist"

 Documentation/git-cvsserver.txt |  2 +-
 Documentation/git-daemon.txt    | 15 +++++++--------
 Documentation/git.txt           |  3 +--
 daemon.c                        |  8 ++++----
 git-cvsserver.perl              |  2 +-
 t/README                        |  9 ++++-----
 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         |  2 +-
 t/test-lib.sh                   |  2 +-
 transport.c                     |  8 ++++----
 13 files changed, 31 insertions(+), 34 deletions(-)


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

Range-diff vs v1:

 1:  ec81aac05c4 ! 1:  19632a2d245 Documentation: use allowlist and denylist
     @@ Metadata
      Author: Derrick Stolee <derrickstolee@github.com>
      
       ## Commit message ##
     -    Documentation: use allowlist and denylist
     +    Documentation: remove use of whitelist
      
     -    Using "allowlist" and "denylist" is a more precise definition of the
     -    functionality they provide. The previous color-based words assume
     -    cultural interpretation to provide the meaning.
     +    The word "whitelist" has cultural implications that are not inclusive.
     +    Thankfully, it is not difficult to reword and avoid its use.
      
     -    Focus on replacements in the Documentation/ directory since these are
     -    not functional uses.
     +    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 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.
     +
     +    Helped-by: Jeff King <peff@peff.net>
     +    Helped-by: Junio C Hamano <gitster@pobox.com>
          Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
       ## Documentation/git-cvsserver.txt ##
     @@ Documentation/git-cvsserver.txt: circumstances, allowing easier restricted usage
       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-directory allowlist. The
     ++GIT_CVSSERVER_ROOT specifies a single allowed directory. The
       repository must still be configured to allow access through
       git-cvsserver, as described above.
       
      
       ## Documentation/git-daemon.txt ##
     -@@ Documentation/git-daemon.txt: It verifies that the directory has the magic file "git-daemon-export-ok", and
     +@@ Documentation/git-daemon.txt: that service if it is enabled.
     + It verifies that the directory has the magic file "git-daemon-export-ok", and
       it will refuse to export any Git directory that hasn't explicitly been marked
       for export this way (unless the `--export-all` parameter is specified). If you
     - pass some directory paths as 'git daemon' arguments, you can further restrict
     +-pass some directory paths as 'git daemon' arguments, you can further restrict
      -the offers to a whitelist comprising of those.
     -+the offers to a allowlist comprising of those.
     ++pass some directory paths as 'git daemon' arguments, the offers are limited to
     ++repositories within those directories.
       
       By default, only `upload-pack` service is enabled, which serves
       'git fetch-pack' and 'git ls-remote' clients, which are invoked
     @@ 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.
     -+	allowlist is specified.
     ++	specific directories are specified.
       
       --base-path=<path>::
       	Remap all the path requests as relative to the given path.
     @@ Documentation/git-daemon.txt: OPTIONS
       	and %D for the absolute path of the named repository.
       	After interpolation, the path is validated against the directory
      -	whitelist.
     -+	allowlist.
     ++	list.
       
       --export-all::
       	Allow pulling from all directories that look like Git repositories
     @@ 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 allowlist 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.
       
     @@ Documentation/git-daemon.txt: git		9418/tcp		# Git Version Control System
       'git daemon' as inetd server::
       	To set up 'git daemon' as an inetd service that handles any
      -	repository under the whitelisted set of directories, /pub/foo
     -+	repository under the allowlisted set of directories, /pub/foo
     - 	and /pub/bar, place an entry like the following into
     - 	/etc/inetd all on one line:
     +-	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:
       +
     + ------------------------------------------------
     + 	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
     +-	protocol not mentioned will be disallowed (i.e., this is a
      -	whitelist, not a blacklist). See the description of
     -+	allowlist, not a denylist). See the description of
     ++	protocol not mentioned will be disallowed. See the description of
       	`protocol.allow` in linkgit:git-config[1] for more details.
       
       `GIT_PROTOCOL_FROM_USER`::
 2:  0d82c2a09c5 ! 2:  3c3c8c20bcb t/*: use allowlist
     @@ Metadata
      Author: Derrick Stolee <derrickstolee@github.com>
      
       ## Commit message ##
     -    t/*: use allowlist
     +    t/*: avoid "whitelist"
      
     -    Using "allowlist" is a more precise definition of the functionality
     -    provided. The previous color-based word assume cultural interpretation
     -    to provide the meaning.
     +    The word "whitelist" has cultural implications that are not inclusive.
     +    Thankfully, it is not difficult to reword and avoid its use.
      
          Focus on changes in the test scripts, since most of the changes are in
     -    comments and test names. The one exception is the renamed test_allowlist
     -    helper.
     +    comments and test names. The renamed test_allow_var helper is only used
     +    once inside the widely-used test_proto helper.
      
          Signed-off-by: Derrick Stolee <derrickstolee@github.com>
      
     @@ t/README: GIT_TEST_SPLIT_INDEX=<boolean> forces split-index mode on the whole
       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
     -+SANITIZE=leak will run only those tests that have allowlisted
     -+themselves as passing with no memory leaks. Tests can be allowlisted
     - 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.
     +-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 ##
      @@
     @@ t/lib-proto-disable.sh
       
      -# Test clone/fetch/push with GIT_ALLOW_PROTOCOL whitelist
      -test_whitelist () {
     -+# Test clone/fetch/push with GIT_ALLOW_PROTOCOL allowlist
     -+test_allowlist () {
     ++# Test clone/fetch/push with GIT_ALLOW_PROTOCOL environment variable
     ++test_allow_var () {
       	desc=$1
       	proto=$2
       	url=$3
     @@ t/lib-proto-disable.sh: test_config () {
       #   $3 - the URL to try cloning
       test_proto () {
      -	test_whitelist "$@"
     -+	test_allowlist "$@"
     ++	test_allow_var "$@"
       
       	test_config "$@"
       }
     @@ 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 allowlist' '
     ++test_expect_success 'curl redirects respect allowed protocols' '
       	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 allowlisting with submodules'
     ++test_description='test protocol restrictions 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 allowlist' '
     ++test_expect_success 'user can override with environment variable' '
       	GIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module
       '
       
     @@ t/t9400-git-cvsserver-server.sh: test_expect_success 'req_Root (export-all)' \
          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 allowlist)' \
     ++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: test_set_prereq () {
       		!*)
       			;;
      -		# (Temporary?) whitelist of things we can't easily
     -+		# (Temporary?) allowlist of things we can't easily
     ++		# (Temporary?) list of things we can't easily
       		# pretend not to support
       		SYMLINKS)
       			;;
     @@ t/test-lib.sh: then
       fi
       
      -# skip non-whitelisted tests when compiled with SANITIZE=leak
     -+# skip non-allowlisted 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
 3:  8aaceedb7a8 ! 3:  0d862cbbebe *: use allowlist and denylist
     @@ Metadata
      Author: Derrick Stolee <derrickstolee@github.com>
      
       ## Commit message ##
     -    *: use allowlist and denylist
     +    *: avoid "whitelist"
      
     -    Using "allowlist" and "denylist" is a more precise definition of the
     -    functionality they provide. The previous color-based words assume
     -    cultural interpretation to provide the meaning.
     +    The word "whitelist" has cultural implications that are not inclusive.
     +    Thankfully, it is not difficult to reword and avoid its use.
      
     -    These changes to the Git codebase are mostly cosmetic. Several comments
     -    are updated. The renamed protocol_allowlist() method is local to
     -    transport.c so does not update any header file API definition. There are
     -    some untranslated error messages that are reworded, so this _might_
     -    affect error parsers. However, two of the three error messages are
     -    around option parsing, so they are "fast failures". The one perhaps
     -    meaningful change is the logerror() in daemon.c.
     +    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.
      
     -    After this change, the only remaining uses of the previous words are
     -    in release notes for older versions of Git.
     +    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: static const char *path_ok(const char *directory, struct hostinfo *hi)
       		 * 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 allowlist /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)
       	}
       
      -	logerror("'%s': not in whitelist", path);
     -+	logerror("'%s': not in allowlist", 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
       	 * is ok with us doing this.
       	 *
      -	 * path_ok() uses enter_repo() and does whitelist checking.
     -+	 * path_ok() uses enter_repo() and does allowlist 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)
       
       	if (strict_paths && (!ok_paths || !*ok_paths))
      -		die("option --strict-paths requires a whitelist");
     -+		die("option --strict-paths requires a allowlist");
     ++		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: $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 allowlist\n";
     ++    die "--export-all can only be used together with an explicit directory list\n";
       }
       
       # Environment handling for running under git-shell
      
     - ## sha1dc/sha1.c ##
     -@@
     -        defined(__MIPSEB__) || defined(__MIPSEB) || defined(_MIPSEB) || \
     -        defined(__sparc))
     - /*
     -- * Should define Big Endian for a whitelist of known processors. See
     -+ * Should define Big Endian for a allowlist of known processors. See
     -  * https://sourceforge.net/p/predef/wiki/Endianness/ and
     -  * http://www.oracle.com/technetwork/server-storage/solaris/portingtosolaris-138514.html
     -  */
     - #define SHA1DC_BIGENDIAN
     - 
     --/* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> */
     -+/* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> */
     - #elif (defined(_AIX) || defined(__hpux))
     - 
     - /*
     -- * Defines Big Endian on a whitelist of OSs that are known to be Big
     -+ * Defines Big Endian on a allowlist of OSs that are known to be Big
     -  * Endian-only. See
     -  * https://lore.kernel.org/git/93056823-2740-d072-1ebd-46b440b33d7e@felt.demon.nl/
     -  */
     - #define SHA1DC_BIGENDIAN
     - 
     --/* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> or <os whitelist> */
     -+/* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> or <os allowlist> */
     - #elif defined(SHA1DC_ON_INTEL_LIKE_PROCESSOR)
     - /*
     -  * As a last resort before we do anything else we're not 100% sure
     -- * about below, we blacklist specific processors here. We could add
     -+ * about below, we denylist specific processors here. We could add
     -  * more, see e.g. https://wiki.debian.org/ArchitectureSpecificsMemo
     -  */
     --#else /* Not under GCC-alike or glibc or *BSD or newlib or <processor whitelist> or <os whitelist> or <processor blacklist> */
     -+#else /* Not under GCC-alike or glibc or *BSD or newlib or <processor allowlist> or <os allowlist> or <processor denylist> */
     - 
     - /* We do nothing more here for now */
     - /*#error "Uncomment this to see if you fall through all the detection"*/
     -
       ## transport.c ##
      @@ transport.c: 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_allowlist(void)
     ++static const struct string_list *protocol_allow_list(void)
       {
       	static int enabled = -1;
       	static struct string_list allowed = STRING_LIST_INIT_DUP;
     @@ transport.c: static enum protocol_allow_config get_protocol_config(const char *t
      -	const struct string_list *whitelist = protocol_whitelist();
      -	if (whitelist)
      -		return string_list_has_string(whitelist, type);
     -+	const struct string_list *allowlist = protocol_allowlist();
     -+	if (allowlist)
     -+		return string_list_has_string(allowlist, 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:

-- 
gitgitgadget

[PATCH v2 1/3] Documentation: remove use of whitelist

[Derrick Stolee via GitGitGadget]

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.

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 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.

Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-cvsserver.txt |  2 +-
 Documentation/git-daemon.txt    | 15 +++++++--------
 Documentation/git.txt           |  3 +--
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 4dc57ed2547..e90b03402a5 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -313,7 +313,7 @@ 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.
 
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index fdc28c041c7..7a0539cb411 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -32,8 +32,8 @@ that service if it is enabled.
 It verifies that the directory has the magic file "git-daemon-export-ok", and
 it will refuse to export any Git directory that hasn't explicitly been marked
 for export this way (unless the `--export-all` parameter is specified). If you
-pass some directory paths as 'git daemon' arguments, you can further restrict
-the offers to a whitelist comprising of those.
+pass some directory paths as 'git daemon' arguments, the offers are limited to
+repositories within those directories.
 
 By default, only `upload-pack` service is enabled, which serves
 'git fetch-pack' and 'git ls-remote' clients, which are invoked
@@ -50,7 +50,7 @@ OPTIONS
 	Match paths exactly (i.e. don't allow "/foo/repo" when the real path is
 	"/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.
 
 --base-path=<path>::
 	Remap all the path requests as relative to the given path.
@@ -73,7 +73,7 @@ OPTIONS
 	%IP for the server's IP address, %P for the port number,
 	and %D for the absolute path of the named repository.
 	After interpolation, the path is validated against the directory
-	whitelist.
+	list.
 
 --export-all::
 	Allow pulling from all directories that look like Git repositories
@@ -218,7 +218,7 @@ standard output to be sent to the requestor as an error message when
 it declines the service.
 
 <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.
 
@@ -264,9 +264,8 @@ git		9418/tcp		# Git Version Control System
 
 'git daemon' as inetd server::
 	To set up 'git daemon' as an inetd service that handles any
-	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:
 +
 ------------------------------------------------
 	git stream tcp nowait nobody  /usr/bin/git
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 302607a4967..dd5061563eb 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -886,8 +886,7 @@ 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.
 
 `GIT_PROTOCOL_FROM_USER`::
-- 
gitgitgadget

Re: [PATCH v2 1/3] Documentation: remove use of whitelist

[Ævar Arnfjörð Bjarmason]

On Fri, Jul 15 2022, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>
> [...]
> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
> index fdc28c041c7..7a0539cb411 100644
> --- a/Documentation/git-daemon.txt
> +++ b/Documentation/git-daemon.txt
> @@ -32,8 +32,8 @@ that service if it is enabled.
>  It verifies that the directory has the magic file "git-daemon-export-ok", and
>  it will refuse to export any Git directory that hasn't explicitly been marked
>  for export this way (unless the `--export-all` parameter is specified). If you
> -pass some directory paths as 'git daemon' arguments, you can further restrict
> -the offers to a whitelist comprising of those.
> +pass some directory paths as 'git daemon' arguments, the offers are limited to
> +repositories within those directories.
>  
>  By default, only `upload-pack` service is enabled, which serves
>  'git fetch-pack' and 'git ls-remote' clients, which are invoked
> @@ -50,7 +50,7 @@ OPTIONS
>  	Match paths exactly (i.e. don't allow "/foo/repo" when the real path is
>  	"/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.

Structurally this series should be changed so that like changes are
coupled together, this would be much easier to review with the daemon.c
changes in 3/3.

But that also shows that this change is needed, but really lacking
compared to what we could do here, which is that both the the SYNOPSIS
and the heading here should be, respectively:


    [--strict-paths=<path>...]

And:

    --strict-paths=<path>...:

I.e. all we're trying to get across here is "this option has a mandatory
argument", so let's just say something like that explicitly? I think in
this case we don't need the prose at all, the synopsis + heading + error
would be enough.

More generally: As I noted on v1 I think the underlying motivation for
the series is mistaken, but I'm also happy for any excuse people can
find to improve our documentation.

But as I pointed out on your similar earlier series to expunge gendered
pronouns from the docs I think best practices in our docs just happen to
align with what you want.

I.e. in that case we prefer a style that isn't introducing "actors"
anyway, so even without that motivation the prose could be improved.

And ditto here, I think the use of the term "whitelist" is fine in and
of itself, but in this case we can clearly improve the docs anyway.

I just think that the origin of the change really shows in this case, I
haven't yet reviewed the rest but suspect I'll find something
similar. I.e. there's a clear improvement to be made, but since it was
first made with s/whitelist/allowlist/g, and now an attempt to
s/whitelist//g in some form we lose sight of the larger picture.

Which in thihs case is that perhaps the sentence isn't needed at all,
and that the synopsis & title is the real thing worth fixing.

So, if what motivates doc improvements in your case is working on your
local dictionary black^Hdenylist :): great, we can probably improve our
docs in any case. But please spend a bit of time eyeballing the change
without an eye to that motivation, sometimes we can improve it much more
without much effert...

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 302607a4967..dd5061563eb 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -886,8 +886,7 @@ 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.
>  
>  `GIT_PROTOCOL_FROM_USER`::

I agree with Junio's earlier feedback about "in other words" being a
telltale sign of prose that needs improving.

But the point of the previous prose (such as it was) was to elaborate on
th existing "allow" to say "oh, allow means the same as whitelist",
surely?

So I think we really could just delete this "in other words" entirely,
i.e. it's basically saying "you are allowed to eat ice cream (in other
words, you are not disallowed)", it's not adding anything anymore. The
"(...)" can just be removed.

Re: [PATCH v2 1/3] Documentation: remove use of whitelist

[Derrick Stolee]

On 7/15/2022 6:47 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Jul 15 2022, Derrick Stolee via GitGitGadget wrote:
> 
>> From: Derrick Stolee <derrickstolee@github.com>
>> [...]
>> diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
>> index fdc28c041c7..7a0539cb411 100644
>> --- a/Documentation/git-daemon.txt
>> +++ b/Documentation/git-daemon.txt
>> @@ -32,8 +32,8 @@ that service if it is enabled.
>>  It verifies that the directory has the magic file "git-daemon-export-ok", and
>>  it will refuse to export any Git directory that hasn't explicitly been marked
>>  for export this way (unless the `--export-all` parameter is specified). If you
>> -pass some directory paths as 'git daemon' arguments, you can further restrict
>> -the offers to a whitelist comprising of those.
>> +pass some directory paths as 'git daemon' arguments, the offers are limited to
>> +repositories within those directories.
>>  
>>  By default, only `upload-pack` service is enabled, which serves
>>  'git fetch-pack' and 'git ls-remote' clients, which are invoked
>> @@ -50,7 +50,7 @@ OPTIONS
>>  	Match paths exactly (i.e. don't allow "/foo/repo" when the real path is
>>  	"/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.
> 
> Structurally this series should be changed so that like changes are
> coupled together, this would be much easier to review with the daemon.c
> changes in 3/3.

Sure. That makes sense. The point here is that git-daemon's documentation
and error messages currently make the word "whitelist" a critical point to
understanding how the feature works. Instead, we can explain it more
clearly using other language. Since this is the biggest place where such
important is placed on the word, then making the changes isolated to this
command makes sense.
 
> But that also shows that this change is needed, but really lacking
> compared to what we could do here, which is that both the the SYNOPSIS
> and the heading here should be, respectively:
> 
> 
>     [--strict-paths=<path>...]
> 
> And:
> 
>     --strict-paths=<path>...:
> 
> I.e. all we're trying to get across here is "this option has a mandatory
> argument", so let's just say something like that explicitly? I think in
> this case we don't need the prose at all, the synopsis + heading + error
> would be enough.

This example is misunderstanding that --strict-paths is a boolean
option and changes how the list of "undecorated" arguments at the end
is interpreted.

The point is that there is an optional list of directories given as
arguments, and the --strict-paths mode changes those directories to
not include recursive subdirectories as repo roots.
>>  	`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.
>>  
>>  `GIT_PROTOCOL_FROM_USER`::
> 
> I agree with Junio's earlier feedback about "in other words" being a
> telltale sign of prose that needs improving.
> 
> But the point of the previous prose (such as it was) was to elaborate on
> th existing "allow" to say "oh, allow means the same as whitelist",
> surely?
> 
> So I think we really could just delete this "in other words" entirely,
> i.e. it's basically saying "you are allowed to eat ice cream (in other
> words, you are not disallowed)", it's not adding anything anymore. The
> "(...)" can just be removed.

I guess I stopped at the first level of "in other words", that being the
"i.e." parenthetical. I didn't realize that this was already nested inside
an aside that was unnecessary.

Thanks,
-Stolee

[PATCH v2 2/3] t/*: avoid "whitelist"

[Derrick Stolee via GitGitGadget]

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.

Focus on changes in the test scripts, since most of the changes are in
comments and test names. The renamed test_allow_var helper is only used
once inside the widely-used test_proto helper.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/README                        | 9 ++++-----
 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         | 2 +-
 t/test-lib.sh                   | 2 +-
 7 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/t/README b/t/README
index 309a31133c6..56d5ebb5798 100644
--- a/t/README
+++ b/t/README
@@ -367,11 +367,10 @@ 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.
diff --git a/t/lib-proto-disable.sh b/t/lib-proto-disable.sh
index 83babe57d95..890622be816 100644
--- a/t/lib-proto-disable.sh
+++ b/t/lib-proto-disable.sh
@@ -1,7 +1,7 @@
 # Test routines for checking protocol disabling.
 
-# Test clone/fetch/push with GIT_ALLOW_PROTOCOL whitelist
-test_whitelist () {
+# Test clone/fetch/push with GIT_ALLOW_PROTOCOL environment variable
+test_allow_var () {
 	desc=$1
 	proto=$2
 	url=$3
@@ -183,7 +183,7 @@ test_config () {
 #   $2 - machine-readable name of the protocol
 #   $3 - the URL to try cloning
 test_proto () {
-	test_whitelist "$@"
+	test_allow_var "$@"
 
 	test_config "$@"
 }
diff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh
index af8772fadaa..bbeebee02f1 100755
--- a/t/t5812-proto-disable-http.sh
+++ b/t/t5812-proto-disable-http.sh
@@ -16,7 +16,7 @@ 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_must_fail env GIT_ALLOW_PROTOCOL=http:https \
 			   GIT_SMART_HTTP=0 \
 		git clone "$HTTPD_URL/ftp-redir/repo.git" 2>stderr &&
diff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh
index 06f55a1b8a0..990f034149d 100755
--- a/t/t5815-submodule-protos.sh
+++ b/t/t5815-submodule-protos.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='test protocol whitelisting with submodules'
+test_description='test protocol restrictions with submodules'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-proto-disable.sh
 
@@ -36,7 +36,7 @@ test_expect_success 'update of ext not allowed' '
 	test_must_fail git -C dst submodule update ext-module
 '
 
-test_expect_success 'user can override whitelist' '
+test_expect_success 'user can override with environment variable' '
 	GIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module
 '
 
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 210ddf09e30..51b798cb493 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -221,7 +221,7 @@ 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)' \
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 6da7273f1d5..6fe62329d8b 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -651,7 +651,7 @@ 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
 		SYMLINKS)
 			;;
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 55857af601b..fff85f4b425 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1408,7 +1408,7 @@ 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
-- 
gitgitgadget

Re: [PATCH v2 2/3] t/*: avoid "whitelist"

[Ævar Arnfjörð Bjarmason]

On Fri, Jul 15 2022, Derrick Stolee via GitGitGadget wrote:

> 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.
>
> Focus on changes in the test scripts, since most of the changes are in
> comments and test names. The renamed test_allow_var helper is only used
> once inside the widely-used test_proto helper.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  t/README                        | 9 ++++-----
>  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         | 2 +-
>  t/test-lib.sh                   | 2 +-
>  7 files changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/t/README b/t/README
> index 309a31133c6..56d5ebb5798 100644
> --- a/t/README
> +++ b/t/README
> @@ -367,11 +367,10 @@ 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.

It's hard to improve your own verbage, but I think in this case my
original version can be improved still:

	GIT_TEST_PASSING_SANITIZE_LEAK=<bool> when compiled with
	SANITIZE=leak will, when true, only run those tests that declare
	themselves leak-free by setting "TEST_PASSES_SANITIZE_LEAK=true"
	before sourcing "test-lib.sh". This test mode is used by the
	"linux-leaks" CI target.

> -test_expect_success 'curl redirects respect whitelist' '
> +test_expect_success 'curl redirects respect allowed protocols' '

Isn't the real problem here that this is inaccurate with regards to
"curl", i.e. AFAIK from browsing transport.c the whitelist of protocols
has nothing to do with curl, we parse that out and apply it before we
ever get to the specific transport layer.

So this should just be "http(s) transport respects GIT_ALLOW_PROTOCOL",
no?

> -test_description='test protocol whitelisting with submodules'
> +test_description='test protocol restrictions with submodules'

Minor: I think this shows the awkwardness of using a word derived from
"allow". Before we could use "whitelist" and "whitelisting"
consistentlry, but now you have "allowed", "allowlist", "restrictions"
etc.

I guess you could say "test protocol allowances..." or something? Meh.

> -test_expect_success 'user can override whitelist' '
> +test_expect_success 'user can override with environment variable' '

Override what? This is a non-improvement. Saying "how" with "environment
variable" is good, but we dropped any mention of what's being tested.

> -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)'

Is it really that we don't have any directiory whatsoever, in that case
I think this is a canditate for splitting up. I.e. the test didn't test
what it was intending to test?

But if it's really "without <whatever you call the list of stuff you
don't want>" we should still say that, no?

> -		# (Temporary?) whitelist of things we can't easily
> +		# (Temporary?) list of things we can't easily
>  		# pretend not to support

We've had this since my dfe1a17df9b (tests: add a special setup where
prerequisites fail, 2019-05-13), so we can probably just drop
"(Temporary?)" there while we're at it...

> -# skip non-whitelisted tests when compiled with SANITIZE=leak
> +# skip unmarked tests when compiled with SANITIZE=leak

This subtly changes the meaning in a way I didn't intend when writing
this. I.e. the existing wording could be improved, but some of what
we're skipping isn't "unmarked", it's explicitly marked as "false"
(which we assume by default).

I.e. I read this new wording as saying "skip <unset>", whereas the
previous can encompas "<unset,false>", just not "<true>"...

Re: [PATCH v2 2/3] t/*: avoid "whitelist"

[Derrick Stolee]

On 7/15/2022 7:02 AM, Ævar Arnfjörð Bjarmason wrote:
>>  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.
> 
> It's hard to improve your own verbage, but I think in this case my
> original version can be improved still:
> 
> 	GIT_TEST_PASSING_SANITIZE_LEAK=<bool> when compiled with
> 	SANITIZE=leak will, when true, only run those tests that declare
> 	themselves leak-free by setting "TEST_PASSES_SANITIZE_LEAK=true"
> 	before sourcing "test-lib.sh". This test mode is used by the
> 	"linux-leaks" CI target.

Another iteration:

  GIT_TEST_PASSING_SANITIZE_LEAK=<bool> focuses the test suite on finding
  memory leaks. When the variable is true and Git is compiled with
  SANITIZE=leak, only run those tests that declare themselves leak-free by
  setting "TEST_PASSES_SANITIZE_LEAK=true" before sourcing "test-lib.sh".
  This test mode is used by the "linux-leaks" CI target.
 
>> -test_expect_success 'curl redirects respect whitelist' '
>> +test_expect_success 'curl redirects respect allowed protocols' '
> 
> Isn't the real problem here that this is inaccurate with regards to
> "curl", i.e. AFAIK from browsing transport.c the whitelist of protocols
> has nothing to do with curl, we parse that out and apply it before we
> ever get to the specific transport layer.
> 
> So this should just be "http(s) transport respects GIT_ALLOW_PROTOCOL",
> no?

Sounds good.

>> -test_description='test protocol whitelisting with submodules'
>> +test_description='test protocol restrictions with submodules'
> 
> Minor: I think this shows the awkwardness of using a word derived from
> "allow". Before we could use "whitelist" and "whitelisting"
> consistentlry, but now you have "allowed", "allowlist", "restrictions"
> etc.
> 
> I guess you could say "test protocol allowances..." or something? Meh.

Perhaps "filtering" is the best way to describe the higher-level
feature that these lists help to implement.

Thanks,
-Stolee

Re: [PATCH v2 2/3] t/*: avoid "whitelist"

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 19 2022, Derrick Stolee wrote:

> On 7/15/2022 7:02 AM, Ævar Arnfjörð Bjarmason wrote:
>>>  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.
>> 
>> It's hard to improve your own verbage, but I think in this case my
>> original version can be improved still:
>> 
>> 	GIT_TEST_PASSING_SANITIZE_LEAK=<bool> when compiled with
>> 	SANITIZE=leak will, when true, only run those tests that declare
>> 	themselves leak-free by setting "TEST_PASSES_SANITIZE_LEAK=true"
>> 	before sourcing "test-lib.sh". This test mode is used by the
>> 	"linux-leaks" CI target.
>
> Another iteration:
>
>   GIT_TEST_PASSING_SANITIZE_LEAK=<bool> focuses the test suite on finding
>   memory leaks.

In some ways it does the opposite of "focusing on finding memory leaks",
we're explicitly finding fewer memory leaks, what we're doing is
asserting that the tests we've whitelisted still pass.

>   SANITIZE=leak, only run those tests that declare themselves leak-free by
>   setting "TEST_PASSES_SANITIZE_LEAK=true" before sourcing "test-lib.sh".
>   This test mode is used by the "linux-leaks" CI target.

Perhaps:

	GIT_TEST_PASSING_SANITIZE_LEAK=true skips those tests that
	haven't declared themselves as leak-free by setting
	"TEST_PASSES_SANITIZE_LEAK=true" before sourcing
	"test-lib.sh". This test mode is used by the "linux-leaks" CI
	target.

Anyway, do you mind if this part is dropped from this series?

I have a set of patches I was meaning to submit to add a tri-state
[false,true,check] support for this, which this conflicts with.

"check" being a mode where we check that the tests in "false" aren't yet
passing. So for that I'll need to re-word this whole thing anyway. I'll
rephrase this while I'm at it to something I think you'll be OK with.

>>> -test_description='test protocol whitelisting with submodules'
>>> +test_description='test protocol restrictions with submodules'
>> 
>> Minor: I think this shows the awkwardness of using a word derived from
>> "allow". Before we could use "whitelist" and "whitelisting"
>> consistentlry, but now you have "allowed", "allowlist", "restrictions"
>> etc.
>> 
>> I guess you could say "test protocol allowances..." or something? Meh.
>
> Perhaps "filtering" is the best way to describe the higher-level
> feature that these lists help to implement.

*Shrug*. I think "filtering" more naturally refers to the process,
i.e. in Makefile syntax:

	$(filter-out $(A),$(LIST))
	$(filter $(B),$(LIST))

I'd call $(B) a whitelist (or whatever), $(A) a blacklist (or whatever),
but "filtering" the process of producing them.

Re: [PATCH v2 2/3] t/*: avoid "whitelist"

[Derrick Stolee]

On 7/19/2022 11:26 AM, Ævar Arnfjörð Bjarmason wrote:
> Anyway, do you mind if this part is dropped from this series?
> 
> I have a set of patches I was meaning to submit to add a tri-state
> [false,true,check] support for this, which this conflicts with.
> 
> "check" being a mode where we check that the tests in "false" aren't yet
> passing. So for that I'll need to re-word this whole thing anyway. I'll
> rephrase this while I'm at it to something I think you'll be OK with.

I had just split the changes regarding SANITIZE=leak into its own
patch, so it will be easy for me to drop it from my series. Thanks
for working on it.

Thanks,
-Stolee

Re: [PATCH v2 2/3] t/*: avoid "whitelist"

[Junio C Hamano]

Derrick Stolee <derrickstolee@github.com> writes:

>> 	GIT_TEST_PASSING_SANITIZE_LEAK=<bool> when compiled with
>> 	SANITIZE=leak will, when true, only run those tests that declare
>> 	themselves leak-free by setting "TEST_PASSES_SANITIZE_LEAK=true"
>> 	before sourcing "test-lib.sh". This test mode is used by the
>> 	"linux-leaks" CI target.
>
> Another iteration:
>
>   GIT_TEST_PASSING_SANITIZE_LEAK=<bool> focuses the test suite on finding
>   memory leaks. When the variable is true and Git is compiled with
>   SANITIZE=leak, only run those tests that declare themselves leak-free by
>   setting "TEST_PASSES_SANITIZE_LEAK=true" before sourcing "test-lib.sh".
>   This test mode is used by the "linux-leaks" CI target.

Sounds good.  These scripts opt into the sanitize-leak tests by
declaring themselves to be leak-free and that is captured very well
without using the *list word ;-)

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

[Derrick Stolee via GitGitGadget]

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(-)

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:
-- 
gitgitgadget

Re: [PATCH v2 3/3] *: avoid "whitelist"

[Ævar Arnfjörð Bjarmason]

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/

Re: [PATCH v2 0/3] Remove use of "whitelist"

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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.
>
> Thanks, -Stolee

Maybe I am biased, but for all the changes in these patches, I find
the updated text far easier to understand than a mere replacing of
the words s/white/allow/, even if I pretend that allowlist is
considered by everybody a proper part of English vocabulary.  After
all, I think most of the places did not have to say "whitelist" in
the first place.

Will queue.

Thanks.

Re: [PATCH v2 0/3] Remove use of "whitelist"

[Phillip Wood]

On 15/07/2022 07:30, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> 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.
>>
>> Thanks, -Stolee
> 
> Maybe I am biased, but for all the changes in these patches, I find
> the updated text far easier to understand than a mere replacing of
> the words s/white/allow/,

I was struck by that as well, the new wording is definitely an 
improvement, thanks for working on it Stolee

Best Wishes

Phillips.

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

[Derrick Stolee via GitGitGadget]

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

[PATCH v3 1/5] daemon: clarify directory arguments

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

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.

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.

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.

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.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-daemon.txt | 21 +++++++++++----------
 daemon.c                     |  8 ++++----
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index fdc28c041c7..236df516c73 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -32,8 +32,8 @@ that service if it is enabled.
 It verifies that the directory has the magic file "git-daemon-export-ok", and
 it will refuse to export any Git directory that hasn't explicitly been marked
 for export this way (unless the `--export-all` parameter is specified). If you
-pass some directory paths as 'git daemon' arguments, you can further restrict
-the offers to a whitelist comprising of those.
+pass some directory paths as 'git daemon' arguments, the offers are limited to
+repositories within those directories.
 
 By default, only `upload-pack` service is enabled, which serves
 'git fetch-pack' and 'git ls-remote' clients, which are invoked
@@ -50,7 +50,7 @@ OPTIONS
 	Match paths exactly (i.e. don't allow "/foo/repo" when the real path is
 	"/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.
+	directory arguments are provided.
 
 --base-path=<path>::
 	Remap all the path requests as relative to the given path.
@@ -73,7 +73,7 @@ OPTIONS
 	%IP for the server's IP address, %P for the port number,
 	and %D for the absolute path of the named repository.
 	After interpolation, the path is validated against the directory
-	whitelist.
+	list.
 
 --export-all::
 	Allow pulling from all directories that look like Git repositories
@@ -218,9 +218,11 @@ standard output to be sent to the requestor as an error message when
 it declines the service.
 
 <directory>::
-	A directory to add to the whitelist of allowed directories. Unless
-	--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
 --------
@@ -264,9 +266,8 @@ git		9418/tcp		# Git Version Control System
 
 'git daemon' as inetd server::
 	To set up 'git daemon' as an inetd service that handles any
-	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 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
diff --git a/daemon.c b/daemon.c
index 58f1077885c..0ae7d12b5c1 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 '<directory>' arguments");
 
 	if (base_path && !is_directory(base_path))
 		die("base-path '%s' does not exist or is not a directory",
-- 
gitgitgadget

[PATCH v3 2/5] git-cvsserver: clarify directory list

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The documentation and error messages for git-cvsserver include some
references to a "whitelist" that is not otherwise included in the
documentation. When different parts of the documentation do not use
common language, this can lead to confusion as to how things are meant
to operate.

Further, the word "whitelist" has cultural implications that make its
use non-inclusive. Thankfully, we can remove it while increasing
clarity.

Update Documentation/git-cvsserver.txt in a similar way to the previous
change to Documentation/git-daemon.txt. The optional '<directory>...'
list can specify a list of allowed directories. We refer to that list
directly inside of the documentation for the GIT_CVSSERVER_ROOT
environment variable.

While modifying this documentation, update the environment variables to
use a list format. We use the modern way of tabbing the description of
each variable in this section. We do _not_ update the description of
'<directory>...' to use tabs this way since the rest of the items in the
OPTIONS list do not use this modern formatting.

A single error message in the actual git-cvsserver.perl code refers to
the whitelist during argument parsing. Instead, refer to the directory
list that has been clarified in the documentation.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git-cvsserver.txt | 19 ++++++++++---------
 git-cvsserver.perl              |  2 +-
 t/t9400-git-cvsserver-server.sh |  2 +-
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 4dc57ed2547..53f111bc0ac 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -63,11 +63,10 @@ Print version information and exit
 Print usage information and exit
 
 <directory>::
-You can specify a list of allowed directories. If no directories
-are given, all are allowed. This is an additional restriction, gitcvs
-access still needs to be enabled by the `gitcvs.enabled` config option
-unless `--export-all` was given, too.
-
+The remaining arguments provide a list of directories. If no directories
+are given, then all are allowed. Repositories within these directories
+still require the `gitcvs.enabled` config option, unless `--export-all`
+is specified.
 
 LIMITATIONS
 -----------
@@ -311,11 +310,13 @@ ENVIRONMENT
 These variables obviate the need for command-line options in some
 circumstances, allowing easier restricted usage through git-shell.
 
-GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.
+GIT_CVSSERVER_BASE_PATH::
+	This variable replaces the argument to --base-path.
 
-GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The
-repository must still be configured to allow access through
-git-cvsserver, as described above.
+GIT_CVSSERVER_ROOT::
+	This variable specifies a single directory, replacing the
+	`<directory>...` argument list. The repository still requires the
+	`gitcvs.enabled` config option, unless `--export-all` is specified.
 
 When these environment variables are set, the corresponding
 command-line arguments may not be used.
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 4c8118010a8..7b757360e28 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/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 210ddf09e30..379b19f2f85 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -221,7 +221,7 @@ 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 list)' \
   '! (cat request-anonymous | git-cvsserver --export-all pserver >log 2>&1 || false)'
 
 test_expect_success 'req_Root (everything together)' \
-- 
gitgitgadget

[PATCH v3 3/5] git.txt: remove redundant language

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <derrickstolee@github.com>

The documentation for GIT_ALLOW_PROTOCOL has a sentence that adds no
value, since it repeats the meaning from the previous sentence (twice!).

The word "whitelist" has cultural implications that are not inclusive,
which brought attention to this sentence.

Helped-by: Jeff King <peff@peff.net>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 Documentation/git.txt | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 302607a4967..47a6095ff40 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -885,9 +885,7 @@ for full details.
 	If set to a colon-separated list of protocols, behave as if
 	`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
+	(overriding any existing configuration). See the description of
 	`protocol.allow` in linkgit:git-config[1] for more details.
 
 `GIT_PROTOCOL_FROM_USER`::
-- 
gitgitgadget

Re: [PATCH v3 3/5] git.txt: remove redundant language

[Jeff King]

On Tue, Jul 19, 2022 at 06:32:15PM +0000, Derrick Stolee via GitGitGadget wrote:

> From: Derrick Stolee <derrickstolee@github.com>
> 
> The documentation for GIT_ALLOW_PROTOCOL has a sentence that adds no
> value, since it repeats the meaning from the previous sentence (twice!).

I think the point the original was trying to make was emphasizing that
when the variable is set at all, then we operate in this new mode.

Maybe that's sufficiently conveyed in the first sentence, but perhaps
another way of expanding would be:

  If the variable is unset, it has no effect, and the normal
  configuration is respected.

But perhaps that is obvious enough from the previous sentence. Since I
was involved in drafting the original, I'm not sure I'm a good judge of
what is obvious and what is not. :)

-Peff

[PATCH v3 4/5] t: avoid "whitelist"

[Derrick Stolee via GitGitGadget]

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.

Focus on changes in the test scripts, since most of the changes are in
comments and test names. The renamed test_allow_var helper is only used
once inside the widely-used test_proto helper.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 t/lib-proto-disable.sh        | 6 +++---
 t/t5812-proto-disable-http.sh | 2 +-
 t/t5815-submodule-protos.sh   | 4 ++--
 t/test-lib-functions.sh       | 3 +--
 4 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/t/lib-proto-disable.sh b/t/lib-proto-disable.sh
index 83babe57d95..890622be816 100644
--- a/t/lib-proto-disable.sh
+++ b/t/lib-proto-disable.sh
@@ -1,7 +1,7 @@
 # Test routines for checking protocol disabling.
 
-# Test clone/fetch/push with GIT_ALLOW_PROTOCOL whitelist
-test_whitelist () {
+# Test clone/fetch/push with GIT_ALLOW_PROTOCOL environment variable
+test_allow_var () {
 	desc=$1
 	proto=$2
 	url=$3
@@ -183,7 +183,7 @@ test_config () {
 #   $2 - machine-readable name of the protocol
 #   $3 - the URL to try cloning
 test_proto () {
-	test_whitelist "$@"
+	test_allow_var "$@"
 
 	test_config "$@"
 }
diff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh
index af8772fadaa..d8da5f58d16 100755
--- a/t/t5812-proto-disable-http.sh
+++ b/t/t5812-proto-disable-http.sh
@@ -16,7 +16,7 @@ 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 '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 &&
diff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh
index 06f55a1b8a0..4d5956cc18f 100755
--- a/t/t5815-submodule-protos.sh
+++ b/t/t5815-submodule-protos.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-test_description='test protocol whitelisting with submodules'
+test_description='test protocol filtering with submodules'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-proto-disable.sh
 
@@ -36,7 +36,7 @@ test_expect_success 'update of ext not allowed' '
 	test_must_fail git -C dst submodule update ext-module
 '
 
-test_expect_success 'user can override whitelist' '
+test_expect_success 'user can filter protocols with GIT_ALLOW_PROTOCOL' '
 	GIT_ALLOW_PROTOCOL=ext git -C dst submodule update ext-module
 '
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 6da7273f1d5..8c44856eaec 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -651,8 +651,7 @@ test_set_prereq () {
 		# test_unset_prereq()
 		!*)
 			;;
-		# (Temporary?) whitelist of things we can't easily
-		# pretend not to support
+		# List of things we can't easily pretend to not support
 		SYMLINKS)
 			;;
 		# Inspecting whether GIT_TEST_FAIL_PREREQS is on
-- 
gitgitgadget

[PATCH v3 5/5] transport.c: avoid "whitelist"

[Derrick Stolee via GitGitGadget]

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.

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>
---
 transport.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

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:
-- 
gitgitgadget