{"thread":{"id":"61557","subject":"[BUG] Cannot set safe.directory with command-scoped configuration when cloning","startedAt":"2024-05-28T14:16:53Z","lastAt":"2024-05-29T10:23:08Z","messageCount":2,"participants":["Chris Burr","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"495753","messageId":"GVAP278MB01511C708AE804393BA3B7A48BF12@GVAP278MB0151.CHEP278.PROD.OUTLOOK.COM","threadId":"61557","inReplyTo":null,"subject":"[BUG] Cannot set safe.directory with command-scoped configuration when cloning","fromName":"Chris Burr","fromEmail":"christopher.burr@cern.ch","sentAt":"2024-05-28T14:16:48Z","receivedAt":"2024-05-28T14:16:53Z","isPatch":false,"sender":{"key":"christopher.burr@cern.ch","avatar":null},"body":"With git version 2.45.1 calling \"git clone dir1 dir2\" fails if dir1 is owned by another user due to the protections for CVE-2024-32004.\n\nAccording to the documentation I expected to be able to set the safe.directory option with either \"-c\" or \"GIT_CONFIG_\" environment variables. From the \"safe.directory\" documentation:\n\n> This config setting is only respected in protected configuration\n> Protected configuration refers to the system, global, and command scopes. \n> Command scope includes both the GIT_CONFIG_ variables and the \"-c\" flag.\n\nThis works for operations like \"git status\" but not \"git clone\".\n\nTo reproduce, first make a repository with your current user:\n\n$ mkdir repo-1 && cd repo-1 && git init && cd ..\nInitialized empty Git repository in /private/tmp/repo-1/.git/\n\nNow clone from \"repo-1\" using sudo (or any other user account, if using sudo SUDO_UID needs to be unset):\n\n$ sudo env SUDO_UID= git clone -c 'safe.directory=*' repo-1/ cloned-repo/\n\nSimilarly setting the corrosponding environment variables also doesn't work:\n\n$ sudo env SUDO_UID= GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=safe.directory 'GIT_CONFIG_VALUE_0=*' git clone repo-1 cloned-repo\n\nIn both cases, the command fails with:\n\nCloning into 'cloned-repo'...\nfatal: detected dubious ownership in repository at '/private/tmp/repo-1//.git'\nTo add an exception for this directory, call:\n\n\tgit config --global --add safe.directory /private/tmp/repo-1//.git\nfatal: Could not read from remote repository.\n\nPlease make sure you have the correct access rights\nand the repository exists.\n"},{"id":"495809","messageId":"20240529102307.GF1098944@coredump.intra.peff.net","threadId":"61557","inReplyTo":"GVAP278MB01511C708AE804393BA3B7A48BF12@GVAP278MB0151.CHEP278.PROD.OUTLOOK.COM","subject":"Re: [BUG] Cannot set safe.directory with command-scoped configuration when cloning","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-05-29T10:23:07Z","receivedAt":"2024-05-29T10:23:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 28, 2024 at 02:16:48PM +0000, Chris Burr wrote:\n\n> According to the documentation I expected to be able to set the\n> safe.directory option with either \"-c\" or \"GIT_CONFIG_\" environment\n> variables. From the \"safe.directory\" documentation:\n> \n> > This config setting is only respected in protected configuration\n> > Protected configuration refers to the system, global, and command scopes. \n> > Command scope includes both the GIT_CONFIG_ variables and the \"-c\" flag.\n> \n> This works for operations like \"git status\" but not \"git clone\".\n\nYeah, the problem is that Git clears many repo-related environment\nvariables when it \"switches\" from one repo to another. And in the case\nof clone, there are two \"sides\": the clone process in the local repo,\nand the upload-pack process which runs in the remote repo. And so when\nwe spawn upload-pack, we clear the environment, including the config\nvariables.\n\nAnd the recent changes in v2.45.1, etc, are for upload-pack to be more\ncareful about the repository in which it runs (unless safe.directory\nsays it's OK). So that's where the config is checked, but your values\ndon't make it there. And that's true of other variables, too. Running\n\"git -c receive.denyDeletes=false push\" won't do what you want.\n\nNow at first glance I think you could argue that the config variables\nshould be retained in this case. After all, they are attached to the\ncommand you ran, not a specific repository. But the main reason we clear\nthem is that for most transports, they wouldn't be retained anyway! If\nyour remote is over http, git://, or even most configurations of ssh\n(which will not pass arbitrary config variables), the other side\nlikewise would not see them. So this is keeping the local-repo case\nconsistent with other transports.\n\nThe workaround is generally to attach the config directly to the\nupload-pack process, like:\n\n  git clone -u 'git -c safe.directory=\"*\" upload-pack' ...\n\nBut:\n\n  1. Obviously that is horrible and not something users should have to\n     know about.\n\n  2. It doesn't always work! If you specify --no-local, then it does,\n     because we kick off a single upload-pack process and read\n     everything from it. But without that, the local clone process tries\n     to optimize things by hard-linking the original files. And that\n     code does its own ownership check. So you need:\n\n       git -c safe.directory=\"*\" \\\n         clone -u 'git -c safe.directory=\"*\" upload-pack' \\\n\t ...\n\n     to cover both processes.\n\nI'm not sure how to make it more friendly, though. Passing config across\nthe local-remote barrier is a can of worms, since many servers would not\nwant to trust client-provided config.\n\nThe simplest solution I can think of is that the ownership check should\nnot go into upload-pack at all. It should be totally in git-clone, which\nshould check the directory itself (obviously only when it knows the\ntransport is the local filesystem). That would largely protect the\noriginal case we cared about (accidentally interacting with an untrusted\nrepo on the local filesystem) and leave other more complex ones\nuntouched (like git-daemon serving user-owned processes as the \"nobody\"\nuser).\n\nBut I did just think of this approach, so there might be corner cases\nthat aren't well covered. And certainly the git-daemon thing _could_ be\na security risk, but I think for the most part if you configured\ngit-daemon to serve other people's repos, you know what you're doing.\n\n-Peff\n"}]}