[PATCH] git-tag -s must fail if gpg cannot sign the tag.

[Carlos Rica <jasampler@gmail.com>]

If the user has misconfigured `user.signingkey` in their .git/config
or just doesn't have any secret keys on their keyring and they ask
for a signed tag with `git tag -s` we better make sure the resulting
tag was actually signed by gpg.

Prior versions of builtin git-tag allowed this failure to slip
by without error as they were not checking the return value of
the finish_command() so they did not notice when gpg exited with
an error exit status.  They also did not fail if gpg produced an
empty output or if read_in_full received an error from the read
system call while trying to read the pipe back from gpg.

Finally, we did not actually honor any return value from the do_sign
function as it returns ssize_t but was being stored into an unsigned
long.  This caused the compiler to optimize out the die condition,
allowing git-tag to continue along and create the tag object.

However, when gpg gets a wrong username, it exits before any read was done
and then the writing process receives SIGNPIPE and program is terminated.
By ignoring this signal, anyway, the function write_or_die gets EPIPE from
write_in_full and exits returning 0 to the system without a message.
Here we better call to write_in_full directly so the next read can fail
printing a message and return safely to the caller.

With these issues fixed `git-tag -s` will now fail to create the
tag and will report a non-zero exit status to its caller, thereby
allowing automated helper scripts to detect (and recover from)
failure if gpg is not working properly.

Signed-off-by: Carlos Rica <jasampler@gmail.com>
---

   This patch was initially sent by Shawn in a previous email:
   http://marc.info/?l=git&m=118905254711808&w=2
   I made many tests trying to know the reason for the problem
   and these changes are the result.  I'm not sure how to send this
   right, because the patch was almost entirely from Shawn and I
   only changed a few things.  Please, see if the new test now pass
   for everybody.  Comments about the change are also welcomed.

   I always CC to my mentor, Johannes Sch., because that was
   usual when the GSoC project was active, so I continue
   doing it that way to make him able to track my contributions.
   Tell me if that's no longer necessary or it'is anoying.

 builtin-tag.c  |   14 ++++++++++----
 t/t7004-tag.sh |    7 +++++++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index 348919c..c9be69a 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -200,6 +200,10 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)
 			bracket[1] = '\0';
 	}

+	/* When the username signingkey is bad, program could be terminated
+	 * because gpg exits without reading and then write gets SIGNPIPE. */
+	signal(SIGPIPE, SIG_IGN);
+
 	memset(&gpg, 0, sizeof(gpg));
 	gpg.argv = args;
 	gpg.in = -1;
@@ -212,12 +216,13 @@ static ssize_t do_sign(char *buffer, size_t size, size_t max)
 	if (start_command(&gpg))
 		return error("could not run gpg.");

-	write_or_die(gpg.in, buffer, size);
+	write_in_full(gpg.in, buffer, size);
 	close(gpg.in);
 	gpg.close_in = 0;
 	len = read_in_full(gpg.out, buffer + size, max - size);

-	finish_command(&gpg);
+	if (finish_command(&gpg) || !len || len < 0)
+		return error("gpg failed to sign the tag");

 	if (len == max - size)
 		return error("could not read the entire signature from gpg.");
@@ -310,9 +315,10 @@ static void create_tag(const unsigned char *object, const char *tag,
 	size += header_len;

 	if (sign) {
-		size = do_sign(buffer, size, max_size);
-		if (size < 0)
+		ssize_t r = do_sign(buffer, size, max_size);
+		if (r < 0)
 			die("unable to sign the tag");
+		size = r;
 	}

 	if (write_sha1_file(buffer, size, tag_type, result) < 0)
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 606d4f2..0d07bc3 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -990,6 +990,13 @@ test_expect_success \
 	git diff expect actual
 '

+# try to sign with bad user.signingkey
+git config user.signingkey BobTheMouse
+test_expect_failure \
+	'git-tag -s fails if gpg is misconfigured' \
+	'git tag -s -m tail tag-gpg-failure'
+git config --unset user.signingkey
+
 # try to verify without gpg:

 rm -rf gpghome
-- 
1.5.0