> Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes: > > > Because this configuration is not supported by all protocol versions, > > and because this configuration seems to be of limited usefulness (only > > useful for people who use manual credential entry and on servers that > > are OK with exposing refs but not objects, and even in this case, helps > > only in a no-op fetch), delete the test that verifies that this > > configuration works. > > A possible and different conclusion that naturally follow your first > "because" could be "fix protocol versions whose support of this > configuration is broken", and your second "because" is with "seems > to be", that makes it quite weak. > > Quite honestly, I hate to see a proposed log message that downplays > its potential negative effects without sufficient justification. That is true, and that's fair. > Isn't this feature primarily for those who want to poll from an > automated job (and naturally you want to assign as little privilege > as possible to such an automated job) with ls-remote and only run an > authenticated fetch, perhaps manually, with or without cred helper, > when the automated polling job finds there is something worthwhile > to fetch? What this test is checking seems to be a quite effective > way to achieve that useful workflow, at least to me, and offhand I > do not think of other ways to easily achieve the same. > > The "ls-remote" communication may not even touch any outside network > but may be happening all within a single organization, in which case > "are OK with exposing refs" is making a security mountain out of a > molehill. If it were a truly problematic hole that makes it a > security issue, wouldn't we deleting this test and at the same time > plugging the hole for earlier protocol versions? Thanks - that's a reasonable use case. > Having said all that, I do not care too deeply. Would a much better > longer-term solution for those who want to poll and fetch only when > there is something new be to allow clients to subscribe to a feed > that hangs while there is nothing new, and lets the upstream side > continuously feed incremental updates to the receiving client, as > its refs are updated, or something? As long as such a thing is in > our vision (it is OK if nobody is currently working on it) to become > replacement, I do not think it is a huge loss to lose the ability for > unauthenticated ls-remote with authenticated fetch. I just remembered that one way we can support the existing use case is to inline the ls-refs call that we make as an Extra Parameter [1]. This would restore the ability to obtain refs through only the info/refs path. Perhaps this is the component in our vision that we need - I'll write another patch that merely forces GIT_TEST_PROTOCOL_VERSION=0 and has a NEEDSWORK comment that explains this. [1] https://github.com/git/git/blob/master/Documentation/technical/pack-protocol.txt
