[PATCH v2 07/10] builtin/diagnose.c: gate certain data behind '--all'

["Victoria Dye via GitGitGadget" <gitgitgadget@gmail.com>]

From: Victoria Dye <vdye@github.com>

Update 'git diagnose' to *not* include '.git/' directory contents by
default, instead requiring specification of a '--all' option to include it.
While helpful for debugging, the archived '.git/' directory contents may be
sensitive, as they can be used to reconstruct an entire repository.

To guard against users inadvertently including this information in
diagnostics and sharing it (e.g., with the mailing list), '.git/' directory
contents will only be included if '--all' is specified.

Signed-off-by: Victoria Dye <vdye@github.com>
---
 Documentation/git-diagnose.txt | 12 ++++++++++--
 builtin/diagnose.c             |  8 ++++++--
 contrib/scalar/scalar.c        |  2 +-
 diagnose.c                     | 14 ++++++++------
 diagnose.h                     |  2 +-
 t/t0092-diagnose.sh            | 17 ++++++++++++++++-
 6 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-diagnose.txt b/Documentation/git-diagnose.txt
index b12ef98f013..374b7402511 100644
--- a/Documentation/git-diagnose.txt
+++ b/Documentation/git-diagnose.txt
@@ -9,6 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git diagnose' [(-o | --output-directory) <path>] [(-s | --suffix) <format>]
+	       [-a | --all]
 
 DESCRIPTION
 -----------
@@ -26,8 +27,6 @@ The following information is captured in the archive:
     stores
   * The total count of loose objects, as well as counts broken down by
     `.git/objects` subdirectory
-  * The contents of the `.git`, `.git/hooks`, `.git/info`, `.git/logs`, and
-    `.git/objects/info` directories
 
 This tool differs from linkgit:git-bugreport[1] in that it collects much more
 detailed information with a greater focus on reporting the size and data shape
@@ -47,6 +46,15 @@ OPTIONS
 	form of a strftime(3) format string; the current local time will be
 	used.
 
+-a::
+--all::
+	Include more complete repository diagnostic information in the archive.
+	Specifically, this will add copies of `.git`, `.git/hooks`, `.git/info`,
+	`.git/logs`, and `.git/objects/info` directories to the output archive.
+	This additional data may be sensitive; a user can reconstruct the full
+	contents of the diagnosed repository with this information. Users should
+	exercise caution when sharing an archive generated with this option.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/diagnose.c b/builtin/diagnose.c
index c545c6bae1d..0a2a63fdfbc 100644
--- a/builtin/diagnose.c
+++ b/builtin/diagnose.c
@@ -4,7 +4,7 @@
 
 
 static const char * const diagnose_usage[] = {
-	N_("git diagnose [-o|--output-directory <file>] [-s|--suffix <format>]"),
+	N_("git diagnose [-o|--output-directory <file>] [-s|--suffix <format>] [-a|--all]"),
 	NULL
 };
 
@@ -13,6 +13,7 @@ int cmd_diagnose(int argc, const char **argv, const char *prefix)
 	struct strbuf zip_path = STRBUF_INIT;
 	time_t now = time(NULL);
 	struct tm tm;
+	int include_everything = 0;
 	char *option_output = NULL;
 	char *option_suffix = "%Y-%m-%d-%H%M";
 	char *prefixed_filename;
@@ -22,6 +23,9 @@ int cmd_diagnose(int argc, const char **argv, const char *prefix)
 			   N_("specify a destination for the diagnostics archive")),
 		OPT_STRING('s', "suffix", &option_suffix, N_("format"),
 			   N_("specify a strftime format suffix for the filename")),
+		OPT_BOOL_F('a', "all", &include_everything,
+			   N_("collect complete diagnostic information"),
+			   PARSE_OPT_NONEG),
 		OPT_END()
 	};
 
@@ -48,7 +52,7 @@ int cmd_diagnose(int argc, const char **argv, const char *prefix)
 	}
 
 	/* Prepare diagnostics */
-	if (create_diagnostics_archive(&zip_path))
+	if (create_diagnostics_archive(&zip_path, include_everything))
 		die_errno(_("unable to create diagnostics archive %s"),
 			  zip_path.buf);
 
diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
index 3983def760a..b10955531ce 100644
--- a/contrib/scalar/scalar.c
+++ b/contrib/scalar/scalar.c
@@ -534,7 +534,7 @@ static int cmd_diagnose(int argc, const char **argv)
 		goto diagnose_cleanup;
 	}
 
-	res = create_diagnostics_archive(&zip_path);
+	res = create_diagnostics_archive(&zip_path, 1);
 
 diagnose_cleanup:
 	strbuf_release(&zip_path);
diff --git a/diagnose.c b/diagnose.c
index 6c3774afb19..6be53d7a1f8 100644
--- a/diagnose.c
+++ b/diagnose.c
@@ -131,7 +131,7 @@ static int add_directory_to_archiver(struct strvec *archiver_args,
 	return res;
 }
 
-int create_diagnostics_archive(struct strbuf *zip_path)
+int create_diagnostics_archive(struct strbuf *zip_path, int include_everything)
 {
 	struct strvec archiver_args = STRVEC_INIT;
 	char **argv_copy = NULL;
@@ -176,11 +176,13 @@ int create_diagnostics_archive(struct strbuf *zip_path)
 	loose_objs_stats(&buf, ".git/objects");
 	strvec_push(&archiver_args, buf.buf);
 
-	if ((res = add_directory_to_archiver(&archiver_args, ".git", 0)) ||
-	    (res = add_directory_to_archiver(&archiver_args, ".git/hooks", 0)) ||
-	    (res = add_directory_to_archiver(&archiver_args, ".git/info", 0)) ||
-	    (res = add_directory_to_archiver(&archiver_args, ".git/logs", 1)) ||
-	    (res = add_directory_to_archiver(&archiver_args, ".git/objects/info", 0)))
+	/* Only include this if explicitly requested */
+	if (include_everything &&
+	    ((res = add_directory_to_archiver(&archiver_args, ".git", 0)) ||
+	     (res = add_directory_to_archiver(&archiver_args, ".git/hooks", 0)) ||
+	     (res = add_directory_to_archiver(&archiver_args, ".git/info", 0)) ||
+	     (res = add_directory_to_archiver(&archiver_args, ".git/logs", 1)) ||
+	     (res = add_directory_to_archiver(&archiver_args, ".git/objects/info", 0))))
 		goto diagnose_cleanup;
 
 	strvec_pushl(&archiver_args, "--prefix=",
diff --git a/diagnose.h b/diagnose.h
index e86e8a3c962..c0c5daf65e7 100644
--- a/diagnose.h
+++ b/diagnose.h
@@ -4,6 +4,6 @@
 #include "cache.h"
 #include "strbuf.h"
 
-int create_diagnostics_archive(struct strbuf *zip_path);
+int create_diagnostics_archive(struct strbuf *zip_path, int include_everything);
 
 #endif /* DIAGNOSE_H */
diff --git a/t/t0092-diagnose.sh b/t/t0092-diagnose.sh
index fa05bf6046f..3c808698d3f 100755
--- a/t/t0092-diagnose.sh
+++ b/t/t0092-diagnose.sh
@@ -22,7 +22,22 @@ test_expect_success UNZIP 'creates diagnostics zip archive' '
 	grep ".git/objects" out &&
 
 	"$GIT_UNZIP" -p "$zip_path" objects-local.txt >out &&
-	grep "^Total: [0-9][0-9]*" out
+	grep "^Total: [0-9][0-9]*" out &&
+
+	# Should not include .git directory contents
+	! "$GIT_UNZIP" -l "$zip_path" | grep ".git/"
+'
+
+test_expect_success UNZIP '--all includes .git data in archive' '
+	test_when_finished rm -rf report &&
+
+	git diagnose -o report -s test --all >out &&
+
+	# Should include .git directory contents
+	"$GIT_UNZIP" -l "$zip_path" | grep ".git/" &&
+
+	"$GIT_UNZIP" -p "$zip_path" .git/HEAD >out &&
+	test_file_not_empty out
 '
 
 test_done
-- 
gitgitgadget