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
