git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v7 1/2] fetch-pack: redact packfile urls in traces

From
Ivan Frade via GitGitGadget <gitgitgadget@gmail.com>
Date
Nov 10, 2021, 23:51 UTC
Message-ID
<bbfdc346ededf1479b5622739be658ee0be2f506.1636588289.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1052.v7.git.1636588289.gitgitgadget@gmail.com>
From: Ivan Frade <ifrade@google.com>

In some setups, packfile uris act as bearer token. It is not recommended to expose them plainly in logs, although in special circunstances (e.g. debug) it makes sense to write them.

Redact the packfile URL paths by default, unless the GIT_TRACE_REDACT variable is set to false. This mimics the redacting of the Authorization header in HTTP.

Signed-off-by: Ivan Frade <ifrade@google.com>
---
 Documentation/git.txt  |  5 +++--
 fetch-pack.c           |  5 +++++
 pkt-line.c             | 40 ++++++++++++++++++++++++++++++++-
 pkt-line.h             |  1 +
 t/t5702-protocol-v2.sh | 51 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 99 insertions(+), 3 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 281c5f8caef..13f83a2a3a1 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -832,8 +832,9 @@ for full details.
 
 `GIT_TRACE_REDACT`::
 	By default, when tracing is activated, Git redacts the values of
-	cookies, the "Authorization:" header, and the "Proxy-Authorization:"
-	header. Set this variable to `0` to prevent this redaction.
+	cookies, the "Authorization:" header, the "Proxy-Authorization:"
+	header and packfile URIs. Set this variable to `0` to prevent this
+	redaction.
 
 `GIT_LITERAL_PATHSPECS`::
 	Setting this variable to `1` will cause Git to treat all
diff --git a/fetch-pack.c b/fetch-pack.c
index a9604f35a3e..8b8c75f33aa 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1653,8 +1653,13 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 				receive_wanted_refs(&reader, sought, nr_sought);
 
 			/* get the pack(s) */
+			if (git_env_bool("GIT_TRACE_REDACT", 1))
+				reader.options |= PACKET_READ_REDACT_URI_PATH;
 			if (process_section_header(&reader, "packfile-uris", 1))
 				receive_packfile_uris(&reader, &packfile_uris);
+			/* We don't expect more URIs. Reset to avoid expensive URI check. */
+			reader.options &= ~PACKET_READ_REDACT_URI_PATH;
+
 			process_section_header(&reader, "packfile", 0);
 
 			/*
diff --git a/pkt-line.c b/pkt-line.c
index 2dc8ac274bd..8e43c2def4c 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -370,6 +370,32 @@ int packet_length(const char lenbuf_hex[4])
 	return (val < 0) ? val : (val << 8) | hex2chr(lenbuf_hex + 2);
 }
 
+static char *find_packfile_uri_path(const char *buffer)
+{
+	const char *URI_MARK = "://";
+	char *path;
+	int len;
+
+	/* First char is sideband mark */
+	buffer += 1;
+
+	len = strspn(buffer, "0123456789abcdefABCDEF");
+	/* size of SHA1 and SHA256 hash */
+	if (!(len == 40 || len == 64) || buffer[len] != ' ')
+		return NULL; /* required "<hash>SP" not seen */
+
+	path = strstr(buffer + len + 1, URI_MARK);
+	if (!path)
+		return NULL;
+
+	path = strchr(path + strlen(URI_MARK), '/');
+	if (!path || !*(path + 1))
+		return NULL;
+
+	/* position after '/' */
+	return ++path;
+}
+
 enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
 						size_t *src_len, char *buffer,
 						unsigned size, int *pktlen,
@@ -377,6 +403,7 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
 {
 	int len;
 	char linelen[4];
+	char *uri_path_start;
 
 	if (get_packet_data(fd, src_buffer, src_len, linelen, 4, options) < 0) {
 		*pktlen = -1;
@@ -427,7 +454,18 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
 		len--;
 
 	buffer[len] = 0;
-	packet_trace(buffer, len, 0);
+	if (options & PACKET_READ_REDACT_URI_PATH &&
+	    (uri_path_start = find_packfile_uri_path(buffer))) {
+		const char *redacted = "<redacted>";
+		struct strbuf tracebuf = STRBUF_INIT;
+		strbuf_insert(&tracebuf, 0, buffer, len);
+		strbuf_splice(&tracebuf, uri_path_start - buffer,
+			      strlen(uri_path_start), redacted, strlen(redacted));
+		packet_trace(tracebuf.buf, tracebuf.len, 0);
+		strbuf_release(&tracebuf);
+	} else {
+		packet_trace(buffer, len, 0);
+	}
 
 	if ((options & PACKET_READ_DIE_ON_ERR_PACKET) &&
 	    starts_with(buffer, "ERR "))
diff --git a/pkt-line.h b/pkt-line.h
index 467ae013573..6d2a63db238 100644
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -87,6 +87,7 @@ void packet_fflush(FILE *f);
 #define PACKET_READ_CHOMP_NEWLINE        (1u<<1)
 #define PACKET_READ_DIE_ON_ERR_PACKET    (1u<<2)
 #define PACKET_READ_GENTLE_ON_READ_ERROR (1u<<3)
+#define PACKET_READ_REDACT_URI_PATH      (1u<<4)
 int packet_read(int fd, char *buffer, unsigned size, int options);
 
 /*
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index d527cf6c49f..78f85b0714a 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -1107,6 +1107,57 @@ test_expect_success 'packfile-uri with transfer.fsckobjects fails when .gitmodul
 	test_i18ngrep "disallowed submodule name" err
 '
 
+test_expect_success 'packfile-uri path redacted in trace' '
+	P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+	rm -rf "$P" http_child log &&
+
+	git init "$P" &&
+	git -C "$P" config "uploadpack.allowsidebandall" "true" &&
+
+	echo my-blob >"$P/my-blob" &&
+	git -C "$P" add my-blob &&
+	git -C "$P" commit -m x &&
+
+	git -C "$P" hash-object my-blob >objh &&
+	git -C "$P" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
+	git -C "$P" config --add \
+		"uploadpack.blobpackfileuri" \
+		"$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
+
+	GIT_TRACE_PACKET="$(pwd)/log" \
+	git -c protocol.version=2 \
+		-c fetch.uriprotocols=http,https \
+		clone "$HTTPD_URL/smart/http_parent" http_child &&
+
+	grep -F "clone< \\1$(cat packh) $HTTPD_URL/<redacted>" log
+'
+
+test_expect_success 'packfile-uri path not redacted in trace when GIT_TRACE_REDACT=0' '
+	P="$HTTPD_DOCUMENT_ROOT_PATH/http_parent" &&
+	rm -rf "$P" http_child log &&
+
+	git init "$P" &&
+	git -C "$P" config "uploadpack.allowsidebandall" "true" &&
+
+	echo my-blob >"$P/my-blob" &&
+	git -C "$P" add my-blob &&
+	git -C "$P" commit -m x &&
+
+	git -C "$P" hash-object my-blob >objh &&
+	git -C "$P" pack-objects "$HTTPD_DOCUMENT_ROOT_PATH/mypack" <objh >packh &&
+	git -C "$P" config --add \
+		"uploadpack.blobpackfileuri" \
+		"$(cat objh) $(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" &&
+
+	GIT_TRACE_PACKET="$(pwd)/log" \
+	GIT_TRACE_REDACT=0 \
+	git -c protocol.version=2 \
+		-c fetch.uriprotocols=http,https \
+		clone "$HTTPD_URL/smart/http_parent" http_child &&
+
+	grep -F "clone< \\1$(cat packh) $HTTPD_URL/dumb/mypack-$(cat packh).pack" log
+'
+
 test_expect_success 'http:// --negotiate-only' '
 	SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" &&
 	URI="$HTTPD_URL/smart/server" &&

-- 
gitgitgadget
Previous: Ivan Frade via GitGitGadgetNext: Ivan Frade via GitGitGadget
Message 41 of 43 in “fetch-pack: redact packfile urls in traces”
  1. 0/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 8, 2021
  2. 1/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 8, 2021
  3. Ævar Arnfjörð BjarmasonOct 8, 2021
  4. Ivan FradeOct 8, 2021
  5. 2/2 Documentation: packfile-uri hash can be longer than 40 hex charsIvan Frade via GitGitGadget, Oct 8, 2021
  6. Ævar Arnfjörð BjarmasonOct 8, 2021
  7. fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 9, 2021
  8. Junio C HamanoOct 11, 2021
  9. Ivan FradeOct 26, 2021
  10. fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 19, 2021
  11. Ævar Arnfjörð BjarmasonOct 20, 2021
  12. 0/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 26, 2021
  13. 1/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 26, 2021
  14. Junio C HamanoOct 28, 2021
  15. Ivan FradeOct 28, 2021
  16. Junio C HamanoOct 28, 2021
  17. 2/2 http-fetch: redact url on die() messageIvan Frade via GitGitGadget, Oct 26, 2021
  18. Ævar Arnfjörð BjarmasonOct 28, 2021
  19. Eric SunshineOct 28, 2021
  20. Ivan FradeOct 28, 2021
  21. Ivan FradeOct 28, 2021
  22. Junio C HamanoOct 29, 2021
  23. Ævar Arnfjörð BjarmasonNov 9, 2021
  24. 0/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 28, 2021
  25. 1/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 28, 2021
  26. Junio C HamanoOct 28, 2021
  27. Ivan FradeOct 29, 2021
  28. Junio C HamanoOct 29, 2021
  29. Jonathan TanNov 8, 2021
  30. 2/2 http-fetch: redact url on die() messageIvan Frade via GitGitGadget, Oct 28, 2021
  31. 0/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 29, 2021
  32. 1/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Oct 29, 2021
  33. Jonathan TanNov 8, 2021
  34. Ævar Arnfjörð BjarmasonNov 9, 2021
  35. Ivan FradeNov 10, 2021
  36. Ævar Arnfjörð BjarmasonNov 11, 2021
  37. Ivan FradeNov 10, 2021
  38. 2/2 http-fetch: redact url on die() messageIvan Frade via GitGitGadget, Oct 29, 2021
  39. Jonathan TanNov 8, 2021
  40. 0/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Nov 10, 2021
  41. 1/2 fetch-pack: redact packfile urls in tracesIvan Frade via GitGitGadget, Nov 10, 2021
  42. 2/2 http-fetch: redact url on die() messageIvan Frade via GitGitGadget, Nov 10, 2021
  43. Junio C HamanoNov 12, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.