Re: [Question] etc/gitconfig and .gitconfig missing errors

From: Jeff King <peff@peff.net>
Date: Jan 13, 2025, 06:01 UTC
Message-ID: <20250113060138.GC767856@coredump.intra.peff.net>
In-Reply-To: <021501db643a$9c5357b0$d4fa0710$@nexbridge.com>

On Sat, Jan 11, 2025 at 10:07:57AM -0500, rsbecker@nexbridge.com wrote:

Show 5 quoted lines

> Question from my community. The standard git install does not
> automatically create the /usr/local/etc directory if it does not
> exist, so the git config --system fails.  If etc/gitconfig is missing,
> the command also fails. Similarly, if ~/.gitconfig is missing, git
> config -global fails.

It's not the missing file that is the problem, but the containing directory, right? I.e.:

  $ make ETC_GITCONFIG=/tmp/foo/bar

  [reading returns an error, but that is natural because we did not
   find the config key in question]
  $ ./git config --system foo.bar; echo $?
  1

  [but writing will fail to create the lock file]
  $ ./git config --system foo.bar baz
  error: could not lock config file /tmp/foo/bar: No such file or directory

> Is this intended? I have been telling my people that they should
> pre-create those files with appropriate security. I have mused that
> having git auto-create this file may introduce potential CVEs,
> explaining the situation. Is this assumption correct?

This is what I'd expect. We do often automatically create leading directories as necessary within the repository, but it gets weird outside of that (and you probably wouldn't have permissions anyway).

It's less weird perhaps as part of "make install", but it still feels like it is outside of Git's scope, and is more of a packaging issue. If you're feeding /usr/local/etc to Git's build knobs, then you probably should be the one making sure it exists.

All of this applies doubly so for ~/.gitconfig. We cannot do it at "make install" time, since there may be many users. We could auto-create it at run-time, but if the user's home directory does not exist that is likely either a sign that something is terrible misconfigured, or the user in question is not meant to have a home directory (daemon user ids, etc). So creating it would probably be surprising.

I do think the error message could be a little more friendly; the problem is not that /tmp/foo/bar does not exist, but that some leading directory does not. But that is the nature of ENOENT.

We don't even get to see the actual lock filename fed to open(), since it is done behind the scenes in hold_lock_file_for_update(). Possibly we should be using unable_to_lock_message(). Or hmm, looks like we can feed a flag to do it for us, like:

diff --git a/config.c b/config.c
index 50f2d17b39..5f8e0c667a 100644
--- a/config.c
+++ b/config.c
@@ -3206,9 +3206,9 @@ int repo_config_set_multivar_in_file_gently(struct repository *r,
 	 * The lock serves a purpose in addition to locking: the new
 	 * contents of .git/config will be written into it.
 	 */
-	fd = hold_lock_file_for_update(&lock, config_filename, 0);
+	fd = hold_lock_file_for_update(&lock, config_filename,
+				       LOCK_REPORT_ON_ERROR);
 	if (fd < 0) {
-		error_errno(_("could not lock config file %s"), config_filename);
 		ret = CONFIG_NO_LOCK;
 		goto out_free;
 	}

That is only marginally better, though:

  $ ./git config --system foo.bar baz
  error: Unable to create '/tmp/foo/bar.lock': No such file or directory

It does diagnose other problems like EEXIST, and IMHO it would not be
unreasonable for it to recognize ENOENT and see if we can even stat()
the surrounding directory.

-Peff