git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] config: allow inaccessible configuration under $HOME

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Apr 12, 2013, 19:14 UTC
Message-ID
<20130412191433.GR27070@google.com>
In-Reply-To
<20130412170505.GA2383@sigill.intra.peff.net>

One unexpected downside to the changes v1.7.12.1~2^2~4 (config: warn on inaccessible files, 2012-08-21) and v1.8.1.1~22^2~2 (config: treat user and xdg config permission problems as errors, 2012-10-13) is that they often trip when git is being run as a server. The appropriate daemon (sshd, inetd, git-daemon, etc) starts as "root", creates a listening socket, and then drops privileges, meaning that when git commands are invoked they cannot access $HOME and die with

 fatal: unable to access '/root/.config/git/config': Permission denied

The intent was always to prevent important configuration (think "[transfer] fsckobjects") from being ignored when the configuration is unintentionally unreadable (for example with ENOMEM due to a DoS attack). But that is not worth breaking these cases of drop-privileges-without-resetting-HOME that were working fine before.

Treat user and xdg configuration that is inaccessible due to permissions (EACCES) as though no user configuration was provided at all.

An alternative approach would be to check if $HOME is readable, but that would not solve the problem in cases where the user who dropped privileges had a globally readable HOME with only .config or .gitconfig being private.

This does not change the behavior when "git config" is being used to write to a user's config file, when /etc/gitconfig or .git/config is unreadable (since those are more serious configuration errors), or when ~/.gitconfig or ~/.config/git is unreadable due to problems other than permissions.

Thanks to Mike Galbraith, W. Trevor King, and Jeff King for their patient explanations.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jeff King wrote:
> Given how tight the exception is, I almost wonder if we should drop the
> environment variable completely, and just never complain about this case
> (in other words, just make it a loosening of 96b9e0e3).
Yeah, I think that would be better.
How about this?  (Still needs tests.)
 builtin/config.c  |  4 ++--
 config.c          | 10 +++++-----
 dir.c             |  4 ++--
 git-compat-util.h |  5 +++--
 wrapper.c         | 10 ++++++----
 5 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 33c9bf9..19ffcaf 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -379,8 +379,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 			 */
 			die("$HOME not set");
 
-		if (access_or_warn(user_config, R_OK) &&
-		    xdg_config && !access_or_warn(xdg_config, R_OK))
+		if (access_or_warn(user_config, R_OK, 0) &&
+		    xdg_config && !access_or_warn(xdg_config, R_OK, 0))
 			given_config_file = xdg_config;
 		else
 			given_config_file = user_config;
diff --git a/config.c b/config.c
index aefd80b..830ee14 100644
--- a/config.c
+++ b/config.c
@@ -58,7 +58,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc
 		path = buf.buf;
 	}
 
-	if (!access_or_die(path, R_OK)) {
+	if (!access_or_die(path, R_OK, 0)) {
 		if (++inc->depth > MAX_INCLUDE_DEPTH)
 			die(include_depth_advice, MAX_INCLUDE_DEPTH, path,
 			    cf && cf->name ? cf->name : "the command line");
@@ -954,23 +954,23 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
 
 	home_config_paths(&user_config, &xdg_config, "config");
 
-	if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK)) {
+	if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0)) {
 		ret += git_config_from_file(fn, git_etc_gitconfig(),
 					    data);
 		found += 1;
 	}
 
-	if (xdg_config && !access_or_die(xdg_config, R_OK)) {
+	if (xdg_config && !access_or_die(xdg_config, R_OK, ACCESS_EACCES_OK)) {
 		ret += git_config_from_file(fn, xdg_config, data);
 		found += 1;
 	}
 
-	if (user_config && !access_or_die(user_config, R_OK)) {
+	if (user_config && !access_or_die(user_config, R_OK, ACCESS_EACCES_OK)) {
 		ret += git_config_from_file(fn, user_config, data);
 		found += 1;
 	}
 
-	if (repo_config && !access_or_die(repo_config, R_OK)) {
+	if (repo_config && !access_or_die(repo_config, R_OK, 0)) {
 		ret += git_config_from_file(fn, repo_config, data);
 		found += 1;
 	}
diff --git a/dir.c b/dir.c
index 91cfd99..9cb2f3d 100644
--- a/dir.c
+++ b/dir.c
@@ -1637,9 +1637,9 @@ void setup_standard_excludes(struct dir_struct *dir)
 		home_config_paths(NULL, &xdg_path, "ignore");
 		excludes_file = xdg_path;
 	}
-	if (!access_or_warn(path, R_OK))
+	if (!access_or_warn(path, R_OK, 0))
 		add_excludes_from_file(dir, path);
-	if (excludes_file && !access_or_warn(excludes_file, R_OK))
+	if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
 		add_excludes_from_file(dir, excludes_file);
 }
 
diff --git a/git-compat-util.h b/git-compat-util.h
index cde442f..51a29b8 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -691,8 +691,9 @@ int remove_or_warn(unsigned int mode, const char *path);
  * Call access(2), but warn for any error except "missing file"
  * (ENOENT or ENOTDIR).
  */
-int access_or_warn(const char *path, int mode);
-int access_or_die(const char *path, int mode);
+#define ACCESS_EACCES_OK (1U << 0)
+int access_or_warn(const char *path, int mode, unsigned flag);
+int access_or_die(const char *path, int mode, unsigned flag);
 
 /* Warn on an inaccessible file that ought to be accessible */
 void warn_on_inaccessible(const char *path);
diff --git a/wrapper.c b/wrapper.c
index bac59d2..e860ad1 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -408,18 +408,20 @@ void warn_on_inaccessible(const char *path)
 	warning(_("unable to access '%s': %s"), path, strerror(errno));
 }
 
-int access_or_warn(const char *path, int mode)
+int access_or_warn(const char *path, int mode, unsigned flag)
 {
 	int ret = access(path, mode);
-	if (ret && errno != ENOENT && errno != ENOTDIR)
+	if (ret && errno != ENOENT && errno != ENOTDIR &&
+	    (!(flag & ACCESS_EACCES_OK) || errno != EACCES))
 		warn_on_inaccessible(path);
 	return ret;
 }
 
-int access_or_die(const char *path, int mode)
+int access_or_die(const char *path, int mode, unsigned flag)
 {
 	int ret = access(path, mode);
-	if (ret && errno != ENOENT && errno != ENOTDIR)
+	if (ret && errno != ENOENT && errno != ENOTDIR &&
+	    (!(flag & ACCESS_EACCES_OK) || errno != EACCES))
 		die_errno(_("unable to access '%s'"), path);
 	return ret;
 }

-- 
1.8.2.1
Previous: Junio C HamanoNext: Jeff King
Message 31 of 39 in “regression: "96b9e0e3 config: treat user and xdg config permission problems as errors" busted git-daemon”
  1. Mike GalbraithApr 10, 2013
  2. W. Trevor KingApr 10, 2013
  3. Mike GalbraithApr 11, 2013
  4. Jeff KingApr 11, 2013
  5. Mike GalbraithApr 11, 2013
  6. Junio C HamanoApr 11, 2013
  7. Jeff KingApr 11, 2013
  8. Jonathan NiederApr 11, 2013
  9. Jeff KingApr 11, 2013
  10. Jonathan NiederApr 11, 2013
  11. Junio C HamanoApr 11, 2013
  12. W. Trevor KingApr 11, 2013
  13. Junio C HamanoApr 11, 2013
  14. Jeff KingApr 11, 2013
  15. W. Trevor KingApr 12, 2013
  16. Junio C HamanoApr 12, 2013
  17. Jeff KingApr 12, 2013
  18. Junio C HamanoApr 12, 2013
  19. Jeff KingApr 12, 2013
  20. Mike GalbraithApr 12, 2013
  21. W. Trevor KingApr 12, 2013
  22. Jeff KingApr 12, 2013
  23. Junio C HamanoApr 12, 2013
  24. Jeff KingApr 12, 2013
  25. Jeff KingApr 12, 2013
  26. Junio C HamanoApr 12, 2013
  27. Jeff KingApr 12, 2013
  28. Junio C HamanoApr 12, 2013
  29. Jeff KingApr 12, 2013
  30. Junio C HamanoApr 12, 2013
  31. config: allow inaccessible configuration under $HOMEJonathan Nieder, Apr 12, 2013
  32. Jeff KingApr 12, 2013
  33. fixup! config: allow inaccessible configuration under $HOMEJonathan Nieder, Apr 12, 2013
  34. config: allow inaccessible configuration under $HOMEJonathan Nieder, Apr 12, 2013
  35. Mike GalbraithApr 13, 2013
  36. Jason A. DonenfeldMay 25, 2013
  37. Junio C HamanoApr 12, 2013
  38. Mike GalbraithApr 12, 2013
  39. Jeff KingApr 11, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.