Re: [PATCH] transport: add core.allowProtocol config option

[Jonathan Nieder <jrnieder@xxxxxxxxx>]

(+peff and bburky, who introduced GIT_ALLOW_PROTOCOL)
Brandon Williams wrote:

> Add configuration option 'core.allowProtocol' to allow users to create a
> whitelist of allowed protocols for fetch/push/clone in their gitconfig.

Ooh.

This would be especially useful at $DAYJOB, where there is a custom
sso:// protocol that is often used by submodules.  Using an envvar to
whitelist it globally is painful because

 - it disables other protocols even when explicitly requested on a
   plain "git clone" command line by the user.  By comparison, the
   built-in git-submodule.sh whitelist only applies to submodules.

 - platform-specific instructions to set an environment variable can
   be more difficult than "just set this git configuration"

Another difficulty with setting GIT_ALLOW_PROTOCOL globally is that it
requires copy/pasting the default value from upstream and then adding
the values I want.  There's no straightforward way to get the current
value and add to it, in case I want to benefit from future upstream
fixes to the default list.

That is, would it be possible to use something like

	[protocol "sso"]
		allow = always

instead of

	[core]
		allowProtocol = file:git:http:https:....:sso

?

[...]
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -27,7 +27,8 @@ cd_to_toplevel
>  #
>  # If the user has already specified a set of allowed protocols,
>  # we assume they know what they're doing and use that instead.
> -: ${GIT_ALLOW_PROTOCOL=file:git:http:https:ssh}
> +config_whitelist=$(git config core.allowProtocol)
> +: ${GIT_ALLOW_PROTOCOL=${config_whitelist:-file:git:http:https:ssh}}

optional: To avoid config parsing when GIT_ALLOW_PROTOCOL is already
set, could do something like

 if ! test "${GIT_ALLOW_PROTOCOL+set}"
 then
	GIT_ALLOW_PROTOCOL=$(
		git config --name-only --get-regexp 'protocol\..*\.allow' always |
		sed -e 's/^protocol.//' -e 's/.allow$//' |
		tr '\n' ':'
	)
	GIT_ALLOW_PROTOCOL=${GIT_ALLOW_PROTOCOL%:}
	: ${GIT_ALLOW_PROTOCOL:=file:git:http:https:ssh}
 fi

[...]
> --- a/transport.c
> +++ b/transport.c
> @@ -652,7 +652,7 @@ static const struct string_list *protocol_whitelist(void)
>  
>  	if (enabled < 0) {
>  		const char *v = getenv("GIT_ALLOW_PROTOCOL");
> -		if (v) {
> +		if (v || !git_config_get_value("core.allowProtocol", &v)) {
>  			string_list_split(&allowed, v, ':', -1);

This has the effect of always disabling other protocols when
core.allowProtocol is set.  Is that intended?

Like the default list used by submodule, I'd be happiest if this only
applied to repositories cloned implicitly instead of those passed
directly to 'git clone'.

That reminds me: external tools also set GIT_ALLOW_PROTOCOL when the
user hasn't set it explicitly, like git-submodule.sh does.  E.g.
repo <https://gerrit.googlesource.com/git-repo/+/466b8c4e/git_command.py#171>,
mercurial <https://www.mercurial-scm.org/repo/hg/file/b032a7b676c6/mercurial/subrepo.py#l1404>.
Other external tools consume GIT_ALLOW_PROTOCOL, like 'go get'
<https://go.googlesource.com/go/+/55620a0e/src/cmd/go/vcs.go#64>.
Can we make it more convenient for them to support this configuration
too?

An example approach would be a GIT_ALLOW_PROTOCOL var returned by
"git var".

That way git-submodule.sh could do

	: ${GIT_ALLOW_PROTOCOL=$(git var GIT_ALLOW_PROTOCOL)}

and it would just work.  Other tools could do the same, with a
fallback to the current default until new enough git is in widespread
use.

Thanks and hope that helps,
Jonathan