On December 30, 2022 4:08 PM, Taylor Blau wrote: >On Wed, Dec 28, 2022 at 02:10:42PM -0800, Jonathan Nieder wrote: >> Hi Randall, >> >> rsbecker@xxxxxxxxxxxxx wrote: >> > Junio C Hamano wrote: >> >> >> This suspiciously sounds like what a1d4f67c (transport: make >> >> `protocol.file.allow` be "user" by default, 2022-07-29) is doing deliberately. >> > >> > I have tried using 'git config --local protocol.file.allow always' >> > and/or 'git config --local protocol.allow always' to get past this, >> > without success. >> >> Does `git config --global protocol.file.allow always` do the trick? >> >> >> Taylor, >> >> does this look like a corner case the 2.30.6 updates forgot to consider? >> >> I think it's the intended effect (preventing file:// submodules), but >> I wonder if this hints that we'd want that protection to be more >> targeted. A file:// submodule (as opposed to a bare path without URL >> scheme) wouldn't trigger the "git clone --local" behavior that that >> commit mentions wanting to protect against, so at first glance it >> would appear to be no more or less dangerous than cloning from a >> remote repository. > >Changing the default value of 'protocol.file.allow' isn't solely about whether or not >we use the `file://` scheme and transport or not. Instead, it's about preventing the >user from accidentally cloning local repositories containing sensitive data into the >working copy of a malicious repository. > >One example might be that I convince you to clone my malicious repository, which >has a Dockerfile that uploads everything in the container filesystem to some data >harvesting server. Since 'docker run' >automatically puts everything in '.' into the volume mount, anything in the working >copy of my malicious repository will get exfiltrated. > >The worry that I wrote about in a1d4f67c was that if I knew that you stored, say, >your SSH private key material in a repository that is at `$HOME/.git` (as is >sometimes common practice), then I could add a submodule at >/home/jrnieder/.git, and extract any sensitive data therein. > >So I think our new default is sensible here if we are concerned with preventing >such a case. I think the new default is reasonable but this did catch me by surprise as it broke our workflows. I guess I need to look at the release notes in more depth - that's my bad. With the caveat that I do not think this is working as intended, which I am finding, because changing the configuration does not make any behavioural difference on any platform I can test on.
