From 2204634275cbb3337cfc7cc994b57f9aa4017d0d Mon Sep 17 00:00:00 2001
From: Ori Bernstein <ori@eigenstate.org>
Date: Sat, 17 Jul 2021 00:10:44 +0000
Subject: [PATCH] git/fetch: ensure we clean packfiles on failure

When pulling into a git repository that is group
writable as a non-owner, the pack file is left
in place because we do not have permission to
remove it.

We also leave it behind if we bail out early due
to an error, or due to only listing the changes.

This pushes down the creation of the file, and
cleans it up on error.

thanks to Anthony Martin for spotting the bug.
git/fetch: ensure we clean packfiles on failure

When pulling into a git repository that is group
writable as a non-owner, the pack file is left
in place because we do not have permission to
remove it.

We also leave it behind if we bail out early due
to an error, or due to only listing the changes.

This pushes down the creation of the file, and
cleans it up on error.

Also, while we're here, clean up index caching,
and ensure we close the fd in all cases.

thanks to Anthony Martin for spotting the bug.
---
 sys/src/cmd/git/fetch.c | 57 ++++++++++++++++++++++++++---------------
 sys/src/cmd/git/pack.c  | 17 ++++++------
 2 files changed, 45 insertions(+), 29 deletions(-)

diff --git a/sys/src/cmd/git/fetch.c b/sys/src/cmd/git/fetch.c
index 727cc3c46..558837e97 100644
--- a/sys/src/cmd/git/fetch.c
+++ b/sys/src/cmd/git/fetch.c
@@ -5,7 +5,6 @@
 
 char *fetchbranch;
 char *upstream = "origin";
-char *packtmp = ".git/objects/pack/fetch.tmp";
 int listonly;
 
 int
@@ -111,7 +110,7 @@ mkoutpath(char *path)
 	for(p=strchr(s+1, '/'); p; p=strchr(p+1, '/')){
 		*p = 0;
 		if(access(s, AEXIST) != 0){
-			fd = create(s, OREAD, DMDIR | 0755);
+			fd = create(s, OREAD, DMDIR | 0775);
 			if(fd == -1)
 				return -1;
 			close(fd);
@@ -162,13 +161,30 @@ handlecaps(char *caps)
 	}
 }
 
-int
-fetchpack(Conn *c, int pfd, char *packtmp)
+void
+fail(char *pack, char *idx, char *msg, ...)
 {
-	char buf[Pktmax], idxtmp[256], *sp[3];
+	char buf[ERRMAX];
+	va_list ap;
+
+	va_start(ap, msg);
+	snprint(buf, sizeof(buf), msg, ap);
+	va_end(ap);
+
+	remove(pack);
+	remove(idx);
+	fprint(2, "%s", buf);
+	exits(buf);
+}
+
+int
+fetchpack(Conn *c)
+{
+	char buf[Pktmax], *sp[3];
+	char *packtmp, *idxtmp;
 	Hash h, *have, *want;
 	int nref, refsz, first;
-	int i, n, req;
+	int i, n, req, pfd;
 	vlong packsz;
 	Object *o;
 
@@ -249,6 +265,15 @@ fetchpack(Conn *c, int pfd, char *packtmp)
 		sysfatal("read: %r");
 	buf[n] = 0;
 
+	if((packtmp = smprint(".git/objects/pack/fetch.%d.pack", getpid())) == nil)
+		sysfatal("smprint: %r");
+	if((idxtmp = smprint(".git/objects/idx/fetch.%d.idx", getpid())) == nil)
+		sysfatal("smprint: %r");
+	if(mkoutpath(packtmp) == -1)
+		sysfatal("could not create %s: %r", packtmp);
+	if((pfd = create(packtmp, ORDWR, 0664)) == -1)
+		sysfatal("could not create %s: %r", packtmp);
+
 	fprint(2, "fetching...\n");
 	packsz = 0;
 	while(1){
@@ -259,19 +284,17 @@ fetchpack(Conn *c, int pfd, char *packtmp)
 			sysfatal("fetch packfile: %r");
 		packsz += n;
 	}
+
 	closeconn(c);
 	if(seek(pfd, 0, 0) == -1)
-		sysfatal("packfile seek: %r");
+		fail(packtmp, idxtmp, "packfile seek: %r");
 	if(checkhash(pfd, packsz, &h) == -1)
-		sysfatal("corrupt packfile: %r");
+		fail(packtmp, idxtmp, "corrupt packfile: %r");
 	close(pfd);
-	n = strlen(packtmp) - strlen(".tmp");
-	memcpy(idxtmp, packtmp, n);
-	memcpy(idxtmp + n, ".idx", strlen(".idx") + 1);
 	if(indexpack(packtmp, idxtmp, h) == -1)
-		sysfatal("could not index fetched pack: %r");
+		fail(packtmp, idxtmp, "could not index fetched pack: %r");
 	if(rename(packtmp, idxtmp, h) == -1)
-		sysfatal("could not rename indexed pack: %r");
+		fail(packtmp, idxtmp, "could not rename indexed pack: %r");
 	return 0;
 }
 
@@ -287,7 +310,6 @@ usage(void)
 void
 main(int argc, char **argv)
 {
-	int pfd;
 	Conn c;
 
 	ARGBEGIN{
@@ -302,14 +324,9 @@ main(int argc, char **argv)
 	if(argc != 1)
 		usage();
 
-	if(mkoutpath(packtmp) == -1)
-		sysfatal("could not create %s: %r", packtmp);
-	if((pfd = create(packtmp, ORDWR, 0644)) == -1)
-		sysfatal("could not create %s: %r", packtmp);
-
 	if(gitconnect(&c, argv[0], "upload") == -1)
 		sysfatal("could not dial %s: %r", argv[0]);
-	if(fetchpack(&c, pfd, packtmp) == -1)
+	if(fetchpack(&c) == -1)
 		sysfatal("fetch failed: %r");
 	closeconn(&c);
 	exits(nil);
diff --git a/sys/src/cmd/git/pack.c b/sys/src/cmd/git/pack.c
index 2ffcdec57..c5519c7ab 100644
--- a/sys/src/cmd/git/pack.c
+++ b/sys/src/cmd/git/pack.c
@@ -197,23 +197,22 @@ loadpack(Packf *pf, char *name)
 		}
 	}
 	if((ifd = open(buf, OREAD)) == -1)
-		goto error;
-	if((d = dirfstat(ifd)) == nil)
-		goto error;
+		return -1;
+	if((d = dirfstat(ifd)) == nil){
+		close(ifd);
+		return -1;
+	}
 	pf->nidx = d->length;
 	pf->idx = emalloc(pf->nidx);
 	if(readn(ifd, pf->idx, pf->nidx) != pf->nidx){
+		close(ifd);
 		free(pf->idx);
 		free(d);
-		goto error;
+		return -1;
 	}
+	close(ifd);
 	free(d);
 	return 0;
-
-error:
-	if(ifd != -1)
-		close(ifd);
-	return -1;	
 }
 
 static void