[PATCH] config: allow inaccessible configuration under $HOME

[Jonathan Nieder <jrnieder@gmail.com>]

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