| Message ID | ec81aac05c40318755f5311a20e8f9cc55d289fc.1657718450.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use "allowlist" and "denylist" tree-wide | expand |
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
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
"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.
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`::