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

[Brandon Williams <bmwill@google.com>]

On 11/02, Stefan Beller wrote:

> > This is useful to restrict recursive
> > +       submodule initialization from an untrusted repository.
> 
> ok. Though as a user submodules may not spring to mind immediately here.
> I think this is generally useful, too. e.g. an admin could put this in
> the system wide
> config to prevent certain protocols from being used.

Oh I pretty much copied the description from what exists for
`GIT_ALLOW_PROTOCOL` which included this bit about submodules.

> > Any protocol not
> > +       mentioned will be disallowed
> 
> For the regular fetch/clone/pull case. For the submodule case we still
> fall back to
> the hardcoded list of known good things?

Yep! This is done by explicitly setting GIT_ALLOW_PROTOCOL to the
hardcoded list if the user hasn't supplied a whitelist.

> 
> > (i.e., this is a whitelist, not a
> > +       blacklist).
> 
> That is very explicit, I'd drop it. However this inspires bike
> shedding on the name:
> What about core.protocolWhitelist instead?

Simply to keep the name similar to the env variable that already exists
for this functionality.

> So the env var is of higher priority than this config.


> > -: ${GIT_ALLOW_PROTOCOL=file:git:http:https:ssh}
> > +config_whitelist=$(git config core.allowProtocol)
> 
> So first we lookup the configured protocols.
> 
> > +: ${GIT_ALLOW_PROTOCOL=${config_whitelist:-file:git:http:https:ssh}}
> 
> Then if they are not configured use the current hard coded white list.

The lookup of the configured whitelist is done first but wont be used
unless GIT_ALLOW_PROTOCOL is unset.  If neither is set it will fallback
to the hardcoded list.

> Do we have any tests for this that could be extended? (Otherwise we'd
> maybe want to add a test for both the regular case as well as a forbidden
> submodule?)
> 

I can write a couple tests for a v2 of the patch.

-- 
Brandon Williams