threads / rfc / 13845

RFC patchUse SUDO_UID to guess committer identity

Subject: [RFC PATCH] Use SUDO_UID to guess committer identity

## tl;dr

4 messages between Jun 7, 2008 and Jun 8, 2008. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Shawn O. Pearce· Jun 7, 2008, 07:11 UTC · lore

When invoking Git commands though sudo against a bare repository with reflogs enabled we should attempt to record the actual user's information in the reflog, not the identity of the user sudo entered.

For example when executing:
	sudo -u gitadm git --git-dir=/srv/git.git branch -f pu master
We want record information about the caller of sudo, not gitadm.

Relying on $SUDO_UID in this case isn't as bad as it might seem. Under sudo $HOME is left as the real user's home directory and $HOME/.gitconfig is used to supply user.name and user.email. However if the real user does not have ~/.gitconfig or did not set user.name/email we need to guess it from their GECOS information.

NO SBO - FOR DISCUSSION ONLY
---
 ident.c |   21 ++++++++++++++++++---
 1 files changed, 18 insertions(+), 3 deletions(-)
Show changes to ident.c +18 −3
diff --git a/ident.c b/ident.c
index b35504a..c821d5f 100644
--- a/ident.c
+++ b/ident.c
@@ -7,6 +7,21 @@
  */
 #include "cache.h"
 
+static uid_t caller_uid(void)
+{
+	const char *sudo_uid = getenv("SUDO_UID");
+	char *end;
+	unsigned long who;
+
+	if (!sudo_uid || !*sudo_uid)
+		return getuid();
+
+	who = strtoul(sudo_uid, &end, 10)
+	if (*end)
+		return getuid();
+	return (uid_t)who;
+}
+
 static char git_default_date[50];
 
 static void copy_gecos(const struct passwd *w, char *name, size_t sz)
@@ -76,7 +91,7 @@ static void setup_ident(void)
 
 	/* Get the name ("gecos") */
 	if (!git_default_name[0]) {
-		pw = getpwuid(getuid());
+		pw = getpwuid(caller_uid());
 		if (!pw)
 			die("You don't exist. Go away!");
 		copy_gecos(pw, git_default_name, sizeof(git_default_name));
@@ -90,7 +105,7 @@ static void setup_ident(void)
 				sizeof(git_default_email));
 		else {
 			if (!pw)
-				pw = getpwuid(getuid());
+				pw = getpwuid(caller_uid());
 			if (!pw)
 				die("You don't exist. Go away!");
 			copy_email(pw);
@@ -208,7 +223,7 @@ const char *fmt_ident(const char *name, const char *email,
 		}
 		if (error_on_no_name)
 			die("empty ident %s <%s> not allowed", name, email);
-		pw = getpwuid(getuid());
+		pw = getpwuid(caller_uid());
 		if (!pw)
 			die("You don't exist. Go away!");
 		strlcpy(git_default_name, pw->pw_name,

-- 
Shawn.
Junio C Hamano· Jun 7, 2008, 21:05 UTC · re: Shawn O. Pearce · lore

Re: [RFC PATCH] Use SUDO_UID to guess committer identity

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 9 quoted lines
> When invoking Git commands though sudo against a bare repository
> with reflogs enabled we should attempt to record the actual user's
> information in the reflog, not the identity of the user sudo entered.
>
> For example when executing:
>
> 	sudo -u gitadm git --git-dir=/srv/git.git branch -f pu master
>
> We want record information about the caller of sudo, not gitadm.

If you are using something esoteric like pseudo, isn't it too much to ask to also use existing GIT_COMMITTER_NAME, or are there reasons why it is not sufficient?

Shawn O. Pearce· Jun 8, 2008, 00:23 UTC · re: Junio C Hamano · lore

Re: [RFC PATCH] Use SUDO_UID to guess committer identity

Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > When invoking Git commands though sudo against a bare repository
> > with reflogs enabled we should attempt to record the actual user's
> > information in the reflog, not the identity of the user sudo entered.
> >
> > For example when executing:
> >
> > 	sudo -u gitadm git --git-dir=/srv/git.git branch -f pu master
> >
> > We want record information about the caller of sudo, not gitadm.
> 
> If you are using something esoteric like pseudo, isn't it too much to ask
> to also use existing GIT_COMMITTER_NAME, or are there reasons why it is
> not sufficient?

The issue is when users run commands though sudo, but forget to set a value for GIT_COMMITTER_NAME/EMAIL, or to configure ~/.gitconfig in their personal account. Now git has to guess the values for these based on the gecos of getuid(), and getuid() is returning the uid of the service account sudo entered (gitadm), not the real user's account. So the reflogs show generic "GIT Admin" and not who it was.

Eh, I'm myself not entirely happy with the patch. It honors the real user's $HOME/.gitconfig user.name/email settings and not the SUDO_UID data. I'd almost prefer favoring SUDO_UID over whatever we inherit in from the enviroment or from $HOME/.gitconfig when it comes to committer identity.

-- 
Shawn.
Junio C Hamano· Jun 8, 2008, 00:57 UTC · re: Shawn O. Pearce · lore

Re: [RFC PATCH] Use SUDO_UID to guess committer identity

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 16 quoted lines
> Junio C Hamano <gitster@pobox.com> wrote:
>> "Shawn O. Pearce" <spearce@spearce.org> writes:
>> 
>> > When invoking Git commands though sudo against a bare repository
>> > with reflogs enabled we should attempt to record the actual user's
>> > information in the reflog, not the identity of the user sudo entered.
>> >
>> > For example when executing:
>> >
>> > 	sudo -u gitadm git --git-dir=/srv/git.git branch -f pu master
>> >
>> > We want record information about the caller of sudo, not gitadm.
> ...
> The issue is when users run commands though sudo, but forget to set a
> value for GIT_COMMITTER_NAME/EMAIL, or to configure ~/.gitconfig in
> their personal account.

In your scenario, is the above "sudo -u gitadm" the exact command line the end users type, or is it wrapped in the script you give to them?

Show 5 quoted lines
> Eh, I'm myself not entirely happy with the patch.  It honors the
> real user's $HOME/.gitconfig user.name/email settings and not the
> SUDO_UID data.  I'd almost prefer favoring SUDO_UID over whatever
> we inherit in from the enviroment or from $HOME/.gitconfig when it
> comes to committer identity.

The thing is, I personally hate pseudo and wish that your solution did not rely on SUDO_UID which is too specific to that hack.

Sometimes people need to lie to their SCM when doing things in behalf of somebody else, and I agree we would want to give them a way to do so. And we do, just like RCS and CVS honor LOGNAME.

If you have /etc/hosts under RCS control but you do not want all the log entries to say 'root', and you would do:

	$ cd /etc
	$ su
	root# edit /etc/hosts
	root# LOGNAME=me ci -u -m 'Add host bar' hosts

When omebody asks you to help him fixing his bug, you go to his keyboard, show him how it should be done, and you concude the session with:

	his shell$ LOGNAME=me ci -u -m 'Fix foo' foo.c

The point is that the same mechanism works (because it is designed to) in both cases.

← back to recent threads