{"thread":{"id":"61481","subject":"Git 2.45.1 - What is the right way to clone with global hooks disabled?","startedAt":"2024-05-15T15:53:38Z","lastAt":"2024-05-17T13:08:49Z","messageCount":7,"participants":["Brooke Kuhlmann","brian m. carlson","Johannes Schindelin","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"494822","messageId":"FAFA34CB-9732-4A0A-87FB-BDB272E6AEE8@alchemists.io","threadId":"61481","inReplyTo":null,"subject":"Git 2.45.1 - What is the right way to clone with global hooks disabled?","fromName":"Brooke Kuhlmann","fromEmail":"brooke@alchemists.io","sentAt":"2024-05-15T15:53:26Z","receivedAt":"2024-05-15T15:53:38Z","isPatch":false,"sender":{"key":"brooke@alchemists.io","avatar":"https://gravatar.com/avatar/4bd27d52fe87760d2bde1d384f0139eb81013709a0cf27ce654d48a7852727fa?d=mp&s=160"},"body":"Hello. 👋\n\nWith the release of Git 2.45.1, I can no longer do this when cloning trusted repositories (my own) for build and deployment purposes:\n\ngit clone --config core.hooksPath=/dev/null https://github.com/bkuhlmann/infusible\n\nI have automation in place where I want my global Git Hooks to be ignored when cloning like this. The solution is to do this:\n\nGIT_CLONE_PROTECTION_ACTIVE=false git clone --config core.hooksPath=/dev/null https://github.com/bkuhlmann/infusible\n\n...but is there a better, more secure, way to clone a repository while ignoring any global Git Hooks from firing without a lot of effort?\n\nThanks."},{"id":"494846","messageId":"ZkU2Vao8yBHYdUmJ@tapette.crustytoothpaste.net","threadId":"61481","inReplyTo":"FAFA34CB-9732-4A0A-87FB-BDB272E6AEE8@alchemists.io","subject":"Re: Git 2.45.1 - What is the right way to clone with global hooks disabled?","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-05-15T22:25:25Z","receivedAt":"2024-05-15T22:25:32Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2024-05-15 at 15:53:26, Brooke Kuhlmann wrote:\n> Hello. 👋\n> \n> With the release of Git 2.45.1, I can no longer do this when cloning trusted repositories (my own) for build and deployment purposes:\n> \n> git clone --config core.hooksPath=/dev/null https://github.com/bkuhlmann/infusible\n> \n> I have automation in place where I want my global Git Hooks to be ignored when cloning like this. The solution is to do this:\n> \n> GIT_CLONE_PROTECTION_ACTIVE=false git clone --config core.hooksPath=/dev/null https://github.com/bkuhlmann/infusible\n> \n> ...but is there a better, more secure, way to clone a repository while ignoring any global Git Hooks from firing without a lot of effort?\n\nI think the solution you have is the best one for 2.45.1 right now. The\npatches which introduced this change also introduced a regression with\nGit LFS, and I've proposed two revert patches, which would also restore\nthe old behaviour for you.\n\nMy reverts are somewhat controversial and there's been two different\napproaches proposed, but I don't believe they will solve your problem,\nso if the reverts are not merged, then I think the solution you have is\nthe best one for now. It's possible someone could send a different\npatch to address your use case, but I don't know of any plans to do so\nat the moment.\n\nI didn't see my patches in seen earlier, but it's possible that Junio\nhas just been busy with other things and may pick them up (or not) in\nthe future.\n\nIf they're not adopted, while I'm not personally planning to send\npatches for your use case, I do think it's a valuable and useful use\ncase for us to have and consider, so I hope someone does send a patch\nseparately.\n\n[0] https://lore.kernel.org/git/ZkO-b6Nswrn9H7Ed@tapette.crustytoothpaste.net/T/\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"494884","messageId":"5b3b0882-eb98-558b-3fd1-40cc4cec3ba0@gmx.de","threadId":"61481","inReplyTo":"FAFA34CB-9732-4A0A-87FB-BDB272E6AEE8@alchemists.io","subject":"Re: Git 2.45.1 - What is the right way to clone with global hooks disabled?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-05-16T12:13:57Z","receivedAt":"2024-05-16T12:14:03Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Brooke,\n\nOn Wed, 15 May 2024, Brooke Kuhlmann wrote:\n\n> With the release of Git 2.45.1, I can no longer do this when cloning\n> trusted repositories (my own) for build and deployment purposes:\n>\n> git clone --config core.hooksPath=/dev/null https://github.com/bkuhlmann/infusible\n\nAh, that's a clever trick. The clone protections try to prevent overriding\nthe `core.hooksPath` with a valid (but not intended by the user) path.\nWhat you are doing is to specifically turn hooks off *1*.\n\nI plan on sending out a patch series later either today or tomorrow to\naddress a couple of regressions introduced by v2.45.1, and this patch\nwould address your specific scenario:\n\n-- snip --\ndiff --git a/config.c b/config.c\nindex 85b37f2ee09..380f7777a6e 100644\n--- a/config.c\n+++ b/config.c\n@@ -1527,6 +1527,7 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n\n \tif (!strcmp(var, \"core.hookspath\")) {\n \t\tif (current_config_scope() == CONFIG_SCOPE_LOCAL &&\n+\t\t (!value || (*value && strcmp(value, \"/dev/null\"))) &&\n \t\t git_env_bool(\"GIT_CLONE_PROTECTION_ACTIVE\", 0))\n \t\t\tdie(_(\"active `core.hooksPath` found in the local \"\n \t\t\t \"repository config:\\n\\t%s\\nFor security \"\ndiff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh\nindex f6dc83e2aab..1eae346a6e3 100755\n--- a/t/t1350-config-hooks-path.sh\n+++ b/t/t1350-config-hooks-path.sh\n@@ -41,4 +41,8 @@ test_expect_success 'git rev-parse --git-path hooks' '\n \ttest .git/custom-hooks/abc = \"$(cat actual)\"\n '\n\n+test_expect_success 'core.hooksPath=/dev/null' '\n+\tgit clone -c core.hooksPath=/dev/null . no-templates\n+'\n+\n test_done\n\n> I have automation in place where I want my global Git Hooks to be\n> ignored when cloning like this. The solution is to do this:\n>\n> GIT_CLONE_PROTECTION_ACTIVE=false git clone --config core.hooksPath=/dev/null https://github.com/bkuhlmann/infusible\n>\n> ...but is there a better, more secure, way to clone a repository while\n> ignoring any global Git Hooks from firing without a lot of effort?\n\nThat is certainly a good work-around for now. Thank you for the report! I\nhope that we can address your use case in v2.45.2, hopefully soon.\n\nThanks,\nJohannes\n"},{"id":"494886","messageId":"30AB54F7-DB53-40AC-BB43-1544A8128AB2@alchemists.io","threadId":"61481","inReplyTo":"5b3b0882-eb98-558b-3fd1-40cc4cec3ba0@gmx.de","subject":"Re: Git 2.45.1 - What is the right way to clone with global hooks disabled?","fromName":"Brooke Kuhlmann","fromEmail":"brooke@alchemists.io","sentAt":"2024-05-16T13:05:12Z","receivedAt":"2024-05-16T13:05:25Z","isPatch":false,"sender":{"key":"brooke@alchemists.io","avatar":"https://gravatar.com/avatar/4bd27d52fe87760d2bde1d384f0139eb81013709a0cf27ce654d48a7852727fa?d=mp&s=160"},"body":"Hey Brian and Johannes, thanks for your feedback. 🙇🏻‍♂️\n\nBrian, true, the Git LFS issues don't necessarily help me.\n\nJohannes, yes, your patch would definitely be of great help to me. Thanks for doing that!\n\n\n> On May 16, 2024, at 6:13 AM, Johannes Schindelin wrote:\n> \n> Hi Brooke,\n> \n> On Wed, 15 May 2024, Brooke Kuhlmann wrote:\n> \n>> With the release of Git 2.45.1, I can no longer do this when cloning\n>> trusted repositories (my own) for build and deployment purposes:\n>> \n>> git clone --config core.hooksPath=/dev/null https://github.com/bkuhlmann/infusible\n> \n> Ah, that's a clever trick. The clone protections try to prevent overriding\n> the `core.hooksPath` with a valid (but not intended by the user) path.\n> What you are doing is to specifically turn hooks off *1*.\n> \n> I plan on sending out a patch series later either today or tomorrow to\n> address a couple of regressions introduced by v2.45.1, and this patch\n> would address your specific scenario:\n> \n> -- snip --\n> diff --git a/config.c b/config.c\n> index 85b37f2ee09..380f7777a6e 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1527,6 +1527,7 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n> \n> \tif (!strcmp(var, \"core.hookspath\")) {\n> \t\tif (current_config_scope() == CONFIG_SCOPE_LOCAL &&\n> +\t\t (!value || (*value && strcmp(value, \"/dev/null\"))) &&\n> \t\t git_env_bool(\"GIT_CLONE_PROTECTION_ACTIVE\", 0))\n> \t\t\tdie(_(\"active `core.hooksPath` found in the local \"\n> \t\t\t \"repository config:\\n\\t%s\\nFor security \"\n> diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh\n> index f6dc83e2aab..1eae346a6e3 100755\n> --- a/t/t1350-config-hooks-path.sh\n> +++ b/t/t1350-config-hooks-path.sh\n> @@ -41,4 +41,8 @@ test_expect_success 'git rev-parse --git-path hooks' '\n> \ttest .git/custom-hooks/abc = \"$(cat actual)\"\n> '\n> \n> +test_expect_success 'core.hooksPath=/dev/null' '\n> +\tgit clone -c core.hooksPath=/dev/null . no-templates\n> +'\n> +\n> test_done\n> \n>> I have automation in place where I want my global Git Hooks to be\n>> ignored when cloning like this. The solution is to do this:\n>> \n>> GIT_CLONE_PROTECTION_ACTIVE=false git clone --config core.hooksPath=/dev/null https://github.com/bkuhlmann/infusible\n>> \n>> ...but is there a better, more secure, way to clone a repository while\n>> ignoring any global Git Hooks from firing without a lot of effort?\n> \n> That is certainly a good work-around for now. Thank you for the report! I\n> hope that we can address your use case in v2.45.2, hopefully soon.\n> \n> Thanks,\n> Johannes\n\n"},{"id":"494889","messageId":"xmqq4jaxvm8z.fsf@gitster.g","threadId":"61481","inReplyTo":"5b3b0882-eb98-558b-3fd1-40cc4cec3ba0@gmx.de","subject":"Re: Git 2.45.1 - What is the right way to clone with global hooks disabled?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-16T14:56:28Z","receivedAt":"2024-05-16T14:56:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> I plan on sending out a patch series later either today or tomorrow to\n> address a couple of regressions introduced by v2.45.1, and this patch\n> would address your specific scenario:\n>\n> -- snip --\n> diff --git a/config.c b/config.c\n> index 85b37f2ee09..380f7777a6e 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -1527,6 +1527,7 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n>\n> \tif (!strcmp(var, \"core.hookspath\")) {\n> \t\tif (current_config_scope() == CONFIG_SCOPE_LOCAL &&\n> +\t\t (!value || (*value && strcmp(value, \"/dev/null\"))) &&\n> \t\t git_env_bool(\"GIT_CLONE_PROTECTION_ACTIVE\", 0))\n> \t\t\tdie(_(\"active `core.hooksPath` found in the local \"\n> \t\t\t \"repository config:\\n\\t%s\\nFor security \"\n\nThis does not make much sense to me. Why is /dev/null so special,\ncompared to say /etc/passwd?\n\nI do think the defence-in-depth aspect of the other half of what\nwent into 2.45.1 and friends, around the \"hooks\" theme has merit,\ni.e. \"any activated hooks in the resulting $GIT_DIR/hooks/ directory\nthat is different from what came from the templates directory is\nsuspicious\". It has a plausible attack scenario to realize such a\nsuspicious configuration by using directory name munging and other\ntricks to confuse \"git clone\" into thinking what the repository sent\nas a part of its payload belongs to $GIT_DIR/. It did have fallout\nas the way \"git lfs\" mucked with user repository's metadata by\nabusing the overly wide trust the user gave to its smudge filter [*]\ncrashed directly with the reasoning behind the \"hooks must match\ntemplate\" protection, which is \"Until the clone finishes and gives\ncontrol back to the end user, external influence like hooks must not\nmuck with the contents checked out without user's knowledge and\nconsent before the user has a chance to inspect the resulting\nrepository\". And it is a reasonable expectation to have when\ncloning a repository that has not proven to be trustworthy. So\ninstead of throwing the protection with bathwater, we should add a\nreasonable (i.e. easy to use for \"git lfs\" developers to follow)\nescape hatch that is a bit more nuanced than \"rip out the whole\nprotection\" revert or \"disable all of the GIT_CLONE_PROTECTION\nmechanisms\" escape hatch 2.45.1 and friends had.\n\nBut I cannot quite tell what the threat model this \"core.hookspath\"\none is trying to protect against. If some attacker manages to muck\nwith the configuration file, it is already game over, and they have\nbetter ways than pointing your hookspath to other places to take\nadvantage of their ability to write to your configuration file to\nattack you.\n\nSo, my recommendation for this one is to just rip the whole new\nlogic added in 2.45.1 and friends out of the \"core.hookspath\"\nhandling.\n\n\n[Footnote]\n\n * The user most likely consented only to allow the smudge filter to\n transform small token recorded in the object store into a large\n blob taken from elsewhere, which means it can read from the\n object store and write to the working tree, but the user may not\n necessarily agreed to give it full read/write/delete/create\n control over anything in $GIT_dir/ or in the configuration files.\n"},{"id":"494890","messageId":"xmqqy189u7jv.fsf@gitster.g","threadId":"61481","inReplyTo":"ZkU2Vao8yBHYdUmJ@tapette.crustytoothpaste.net","subject":"Re: Git 2.45.1 - What is the right way to clone with global hooks disabled?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-16T14:59:16Z","receivedAt":"2024-05-16T14:59:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> I didn't see my patches in seen earlier, but it's possible that Junio\n> has just been busy with other things and may pick them up (or not) in\n> the future.\n\nI knew about this message, and addressed your half of the \"hooks\"\ntopic in my response to Dscho's proposed patch to the other half of\nthe \"hooks\" topic, but I didn't realize Dscho's message did not Cc\nyou. It should appear at\n\n https://lore.kernel.org/git/xmqq4jaxvm8z.fsf@gitster.g/\n\nby the time I send this message.\n\nThanks.\n"},{"id":"494967","messageId":"6c5160b3-e5b3-996c-bdfa-90c0a38ba19a@gmx.de","threadId":"61481","inReplyTo":"xmqq4jaxvm8z.fsf@gitster.g","subject":"Re: Git 2.45.1 - What is the right way to clone with global hooks disabled?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-05-17T13:08:40Z","receivedAt":"2024-05-17T13:08:49Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 16 May 2024, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> > I plan on sending out a patch series later either today or tomorrow to\n> > address a couple of regressions introduced by v2.45.1, and this patch\n> > would address your specific scenario:\n> >\n> > -- snip --\n> > diff --git a/config.c b/config.c\n> > index 85b37f2ee09..380f7777a6e 100644\n> > --- a/config.c\n> > +++ b/config.c\n> > @@ -1527,6 +1527,7 @@ static int git_default_core_config(const char *var, const char *value, void *cb)\n> >\n> > \tif (!strcmp(var, \"core.hookspath\")) {\n> > \t\tif (current_config_scope() == CONFIG_SCOPE_LOCAL &&\n> > +\t\t (!value || (*value && strcmp(value, \"/dev/null\"))) &&\n> > \t\t git_env_bool(\"GIT_CLONE_PROTECTION_ACTIVE\", 0))\n> > \t\t\tdie(_(\"active `core.hooksPath` found in the local \"\n> > \t\t\t \"repository config:\\n\\t%s\\nFor security \"\n>\n> This does not make much sense to me. Why is /dev/null so special,\n> compared to say /etc/passwd?\n>\n> I do think the defence-in-depth aspect of the other half of what\n> went into 2.45.1 and friends, around the \"hooks\" theme has merit,\n> i.e. \"any activated hooks in the resulting $GIT_DIR/hooks/ directory\n> that is different from what came from the templates directory is\n> suspicious\". It has a plausible attack scenario to realize such a\n> suspicious configuration by using directory name munging and other\n> tricks to confuse \"git clone\" into thinking what the repository sent\n> as a part of its payload belongs to $GIT_DIR/. It did have fallout\n> as the way \"git lfs\" mucked with user repository's metadata by\n> abusing the overly wide trust the user gave to its smudge filter [*]\n> crashed directly with the reasoning behind the \"hooks must match\n> template\" protection, which is \"Until the clone finishes and gives\n> control back to the end user, external influence like hooks must not\n> muck with the contents checked out without user's knowledge and\n> consent before the user has a chance to inspect the resulting\n> repository\". And it is a reasonable expectation to have when\n> cloning a repository that has not proven to be trustworthy. So\n> instead of throwing the protection with bathwater, we should add a\n> reasonable (i.e. easy to use for \"git lfs\" developers to follow)\n> escape hatch that is a bit more nuanced than \"rip out the whole\n> protection\" revert or \"disable all of the GIT_CLONE_PROTECTION\n> mechanisms\" escape hatch 2.45.1 and friends had.\n>\n> But I cannot quite tell what the threat model this \"core.hookspath\"\n> one is trying to protect against.\n\nMy thinking was this: if, for whatever reason, it is possible for a `git\nclone` to write to the Git directory during a clone (and I had those\nsubmodules and recursive clones in mind, in particular), then an attacker\ncan not only manipulate Git into writing into the `hooks/` directory, but\nthey can also reroute `core.hooksPath`. And if we ignore the latter, the\nformer protections can be side-stepped rather easily.\n\n> If some attacker manages to muck with the configuration file, it is\n> already game over, and they have better ways than pointing your\n> hookspath to other places to take advantage of their ability to write to\n> your configuration file to attack you.\n\nHmm. You have a good point there. The aforementioned `smudge` filter would\nbe a prime target.\n\n> So, my recommendation for this one is to just rip the whole new\n> logic added in 2.45.1 and friends out of the \"core.hookspath\"\n> handling.\n\nI guess that's the easier path for now.\n\nWe should seriously think about better ways to protect against config\nmanipulations during clone operations, though. In particular with the\ncurrent state of the submodule support code, recursive clones continue to\nseem like highly likely target for attackers.\n\nCiao,\nJohannes\n\n> [Footnote]\n>\n> * The user most likely consented only to allow the smudge filter to\n> transform small token recorded in the object store into a large\n> blob taken from elsewhere, which means it can read from the\n> object store and write to the working tree, but the user may not\n> necessarily agreed to give it full read/write/delete/create\n> control over anything in $GIT_dir/ or in the configuration files.\n>\n"}]}