Re: [PATCH 2/2] config: treat user and xdg config permission problems as errors
On Sat, Oct 13, 2012 at 05:04:02PM -0700, Jonathan Nieder wrote: Better to error out and ask the user to correct the problem. This only affects the user and xdg config files, since the user presumably has enough access to fix their permissions. If the system config file is unreadable, the best we can do is to warn about it so the user knows to notify someone and get on with work in the meantime. I'm on the fence about treating the systme config specially. On the one hand, I see the convenience if somebody has a bogus /etc/gitconfig and gets EPERM but can't fix it. On the other hand, if we get EIO, isn't that a good indication that we would want to die? For example, servers may depend on /etc/gitconfig to enforce security policy (e.g., setting transfer.fsckObjects or receive.deny*). Perhaps our default should be safe, and people can use GIT_CONFIG_NOSYSTEM to work around a broken machine. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] config: treat user and xdg config permission problems as errors
Jeff King wrote: For example, servers may depend on /etc/gitconfig to enforce security policy (e.g., setting transfer.fsckObjects or receive.deny*). Perhaps our default should be safe, and people can use GIT_CONFIG_NOSYSTEM to work around a broken machine. Very good point. How about these patches on top? Jonathan Nieder (2): config doc: advertise GIT_CONFIG_NOSYSTEM config: exit on error accessing any config file Documentation/git-config.txt | 8 config.c | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] config: treat user and xdg config permission problems as errors
On Sun, Oct 14, 2012 at 01:42:44AM -0700, Jonathan Nieder wrote: Jeff King wrote: For example, servers may depend on /etc/gitconfig to enforce security policy (e.g., setting transfer.fsckObjects or receive.deny*). Perhaps our default should be safe, and people can use GIT_CONFIG_NOSYSTEM to work around a broken machine. Very good point. How about these patches on top? Jonathan Nieder (2): config doc: advertise GIT_CONFIG_NOSYSTEM config: exit on error accessing any config file Documentation/git-config.txt | 8 config.c | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) This is my absolute favorite type of reply: the kind that you can apply with git am. The direction and the patches themselves look good to me. I agree with your reasoning in v2 of 3/2; it makes much more sense than v1. Thanks. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] config: treat user and xdg config permission problems as errors
Git reads multiple configuration files: settings come first from the
system config file (typically /etc/gitconfig), then the xdg config
file (typically ~/.config/git/config), then the user's dotfile
(~/.gitconfig), then the repository configuration (.git/config).
Git has always used access(2) to decide whether to use each file; as
an unfortunate side effect, that means that if one of these files is
unreadable (e.g., EPERM or EIO), git skips it. So if I use
~/.gitconfig to override some settings but make a mistake and give it
the wrong permissions then I am subject to the settings the sysadmin
chose for /etc/gitconfig.
Better to error out and ask the user to correct the problem.
This only affects the user and xdg config files, since the user
presumably has enough access to fix their permissions. If the system
config file is unreadable, the best we can do is to warn about it so
the user knows to notify someone and get on with work in the meantime.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
config.c | 4 ++--
git-compat-util.h | 1 +
wrapper.c | 8
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index 08e47e2e..e8875b8a 100644
--- a/config.c
+++ b/config.c
@@ -945,12 +945,12 @@ int git_config_early(config_fn_t fn, void *data, const
char *repo_config)
found += 1;
}
- if (xdg_config !access_or_warn(xdg_config, R_OK)) {
+ if (xdg_config !access_or_die(xdg_config, R_OK)) {
ret += git_config_from_file(fn, xdg_config, data);
found += 1;
}
- if (user_config !access_or_warn(user_config, R_OK)) {
+ if (user_config !access_or_die(user_config, R_OK)) {
ret += git_config_from_file(fn, user_config, data);
found += 1;
}
diff --git a/git-compat-util.h b/git-compat-util.h
index f567767f..dfe2e318 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -640,6 +640,7 @@ int remove_or_warn(unsigned int mode, const char *path);
* (ENOENT or ENOTDIR).
*/
int access_or_warn(const char *path, int mode);
+int access_or_die(const char *path, int mode);
/* 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 c1b919f3..7cbe96a0 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -416,6 +416,14 @@ int access_or_warn(const char *path, int mode)
return ret;
}
+int access_or_die(const char *path, int mode)
+{
+ int ret = access(path, mode);
+ if (ret errno != ENOENT errno != ENOTDIR)
+ die_errno(_(unable to access '%s'), path);
+ return ret;
+}
+
struct passwd *xgetpwuid_self(void)
{
struct passwd *pw;
--
1.8.0.rc2
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
