Re: [PATCH] doc: remove mentions of .gitmodules !command syntax

From: Petar Vutov <pvutov@imap.cc>
Date: Jul 12, 2023, 17:30 UTC
Message-ID: <d775437e-7fa3-189b-a1c3-4fd358dd9768@imap.cc>
In-Reply-To: <xmqqleflt75z.fsf@gitster.g>

On 7/12/23 18:40, Junio C Hamano wrote:

Show 32 quoted lines

> pvutov@imap.cc writes:
> 
>> From: Petar Vutov <pvutov@imap.cc>
>>
>> To mitigate CVE-2019-19604, the capability to configure
>> `git submodule update` to execute custom commands was
>> removed in v2.20.2.
>>
>> The git-submodule documentation still mentions the now-unsupported
>> syntax, which is misleading.
>>
>> Remove the leftover documentation.
> 
> The change during v2.20.2 timeperiod you have in mind may be
> e904deb8 (submodule: reject submodule.update = !command in
> .gitmodules, 2019-12-05).  The key phrase is "in .gitmodules"
> as it did not forbid writing update command in the configuration.
> 
> The pre-context lines of your patch (see below) say that the 'custom
> command' option and 'none' option are only available via the
> `submodule.<name>.update` configuration variable.  IOW, this part of
> the documentation does not talk about the .gitmodules file---it
> talks about what you can say in the configuration file (which is
> under your local control).
> 
> I think the existing text that came from fc01a5d2 (submodule update
> documentation: don't repeat ourselves, 2016-12-27) may be
> misleading, and may has room for improvement, but I do not think it
> is wrong per-se.  If we remove it, there is nowhere else that teaches
> users !cmd can be set in their configuration files, or is there?
> 
> Thanks.

Thanks for the review. I was not aware of the .gitconfig use case. I hit that paragraph while trying to enforce sparse-checkout via .gitmodules. Yet the gitmodules doc is clear enough:

"See description of update command in git-submodule[1] for their meaning. For security reasons, the !command form is not accepted here."

Clearly I followed the link in the first sentence without reading the second :)

Perhaps the term "configuration variable" in

"The following update procedures are only available via the submodule.<name>.update configuration variable:"

is more specific and technical than immediately obvious - I would have expected the contents of .gitmodules to be a form of (repository) configuration. But that is just bikeshedding.

Show 23 quoted lines

> 
>> Signed-off-by: Petar Vutov <pvutov@imap.cc>
>> ---
>>   Documentation/git-submodule.txt | 6 ------
>>   1 file changed, 6 deletions(-)
>>
>> diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
>> index 4d3ab6b9f9..b40ac72f75 100644
>> --- a/Documentation/git-submodule.txt
>> +++ b/Documentation/git-submodule.txt
>> @@ -163,12 +163,6 @@ checked out in the submodule.
>>   The following 'update' procedures are only available via the
>>   `submodule.<name>.update` configuration variable:
>>   
>> -	custom command;; arbitrary shell command that takes a single
>> -	    argument (the sha1 of the commit recorded in the
>> -	    superproject) is executed. When `submodule.<name>.update`
>> -	    is set to '!command', the remainder after the exclamation mark
>> -	    is the custom command.
>> -
>>   	none;; the submodule is not updated.
>>   
>>   If the submodule is not yet initialized, and you just want to use the