On Thu, Jun 30, 2022 at 06:13:54PM +0000, Glen Choo via GitGitGadget wrote: > This series does not change the default behavior, but in the long-run, a > "no-embedded" option might be a safe and usable default [2]. "never" is too > restrictive and unlikely to be the default. Thanks for this summary, and sorry for not taking a look at this series earlier. I spent quite a bit of time with v1 and the RFC patches but haven't had as much time as I would have liked to devote to reviewing the later rounds. I'm happy with this direction. I think "never" is an understandable direction to go in for compliance reasons, or in deployments where bare repositories are not expected. And I am glad that we are treating it as unlikely to be the default in the future, I agree that it is likely too restrictive for that to make sense. So leaving the default behavior unchanged, and providing a big hammer to prevent bare repository discovery entirely seems like a good first step for this feature. I'm interested to see a potential future no-embedded implementation, since I think that has a reasonable chance of being a sensible default, depending on how it's implemented. > For security reasons, discovery.bare cannot be read from repository-level > config (because we would end up trusting the embedded bare repository that > we aren't supposed to trust to begin with). Since this would introduce a 3rd > variable that is only read from 'protected/trusted configuration' (the > others are safe.directory and uploadpack.packObjectsHook) this series also > defines and creates a shared implementation for 'protected configuration' This concept is new from the earlier rounds, so I'll be curious to see how it plays out as I take a closer look at the patches. Thanks again for your work on this complicated and tricky-to-get-right feature ;-). Thanks, Taylor
