Re: [PATCH v3 1/5] Documentation: define protected configuration

From: Glen Choo <chooglen@google.com>
Date: Jun 3, 2022, 15:57 UTC
Message-ID: <kl6l7d5xeo3s.fsf@chooglen-macbookpro.roam.corp.google.com>
In-Reply-To: <xmqqh75a1rmd.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 13 quoted lines

> "Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>>  safe.directory::
>> -	These config entries specify Git-tracked directories that are
>> -	considered safe even if they are owned by someone other than the
>> -	current user. By default, Git will refuse to even parse a Git
>> -	config of a repository owned by someone else, let alone run its
>> -	hooks, and this config setting allows users to specify exceptions,
>> -	e.g. for intentionally shared repositories (see the `--shared`
>> -	option in linkgit:git-init[1]).
>> +	'(Protected config only) ' These config entries specify
>
> What's the SP in "only) '" doing?

Silly typo. Thanks for the catch :)

Show 39 quoted lines

>> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
>> index aa2f41f5e70..a669983abd6 100644
>> --- a/Documentation/glossary-content.txt
>> +++ b/Documentation/glossary-content.txt
>> @@ -483,6 +483,24 @@ exclude;;
>>  	head ref. If the remote <<def_head,head>> is not an
>>  	ancestor to the local head, the push fails.
>>  
>> +[[def_protected_config]]protected configuration::
>> +	Protected configuration is configuration that Git considers more
>> +	trustworthy because it is unlikely to be tampered with by an
>> +	attacker. For security reasons, some configuration variables are
>> +	only respected when they are defined in protected configuration.
>> ++
>> +Protected configuration includes:
>> ++
>> +- system-level config, e.g. `/etc/git/config`
>> +- global config, e.g. `$XDG_CONFIG_HOME/git/config` and
>> +  `$HOME/.gitconfig`
>> +Protected configuration excludes:
>> ++
>> +- repository config, e.g. `$GIT_DIR/config` and
>> +  `$GIT_DIR/config.worktree`
>> +- the command line option `-c` and its equivalent environment variables
>
> The description is a bit unclear what "protected configuration"
> refers.
>
> If it is the scopes (as in "git config --show-scope") Git can trust
> more, in other words, a statement like this
>
>     safe.directory is honored only when it comes from a protected
>     configuration.
>
> is what you want to make easier to write by introducing a new
> phrase, perhaps use the word "scope" for more consistency?  E.g.
>
>     Only safe.directory that is defined in a trusted scope is
>     honored.

Good point. I think using scope would be a lot clearer, and maybe I will consider s/protected configuration/protected scope. I'm hesitant to call the scope "trusted", because I don't want to insinuate that repository config is "untrusted" since we _do_ trust it in most cases.

I don't think Documentation/git-config.txt has adequately defined what a 'scope' is though, even though scopes have been with us since 9acc591111 (config: add a notion of "scope", 2016-05-18). The best I could find is "--show-scope", introduced in 145d59f482 (config: add '--show-scope' to print the scope of a config value, 2020-02-10), which mentions scopes but doesn't link the idea back to the specific files or CLI options ("--system", "--global", etc).

So I'll see if I can improve the docs around scopes since that will help the language in this patch.