[v8,10/15] bugreport: add config values from safelist

Commit Message

Emily Shaffer Feb. 20, 2020, 1:58 a.m. UTC

Teach bugreport to gather the values of config options which are present
in 'bugreport-config-safelist.h', and show their origin scope.

Many config options are sensitive, and many Git add-ons use config
options which git-core does not know about; it is better only to gather
config options which we know to be safe, rather than excluding options
which we know to be unsafe.

Rather than including the path to someone's config, which can reveal
filesystem layout and project names, just name the scope (e.g. system,
global, local) of the config source.

Signed-off-by: Emily Shaffer <[e-mail]>
---
 Documentation/git-bugreport.txt |  1 +
 Makefile                        |  2 ++
 bugreport.c                     | 34 +++++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)

Comments

Junio C Hamano Feb. 20, 2020, 8:47 p.m. UTC | #1

Emily Shaffer <[e-mail]> writes:

> @@ -94,6 +120,7 @@ int cmd_main(int argc, const char **argv)
>  	char *option_output = NULL;
>  	char *option_suffix = "%F-%H%M";
>  	struct stat statbuf;
> +	int nongit_ok = 0;
>  
>  	const struct option bugreport_options[] = {
>  		OPT_STRING('o', "output-directory", &option_output, N_("path"),
> @@ -102,6 +129,10 @@ int cmd_main(int argc, const char **argv)
>  			   N_("specify a strftime format suffix for the filename")),
>  		OPT_END()
>  	};
> +
> +	/* Prerequisite for hooks and config checks */
> +	setup_git_directory_gently(&nongit_ok);

Now this starts to break "-o" option when the command is run from a
subdirectory of a git working tree, I suspect.

The setup() will discover that you are in a Git controlled working
tree, goes up to the top-level of the working tree, so a user may
expect

	cd t && git bugreport -o frotz

to create a new directory t/frotz, but it would instead create the
directory frotz next to t/, no?

Using OPT_FILENAME and feeding correct prefix to parse_options()
will correct this.

>  	argc = parse_options(argc, argv, "", bugreport_options,
>  			     bugreport_usage, 0);

Patch

diff --git a/Documentation/git-bugreport.txt b/Documentation/git-bugreport.txt
index f7a13c0374..efe9719ea4 100644
--- a/Documentation/git-bugreport.txt
+++ b/Documentation/git-bugreport.txt
@@ -30,6 +30,7 @@  The following information is captured automatically:
  - Compiler-specific info string
  - 'git remote-https --build-info'
  - $SHELL
+ - Selected config values
 
 This tool is invoked via the typical Git setup process, which means that in some
 cases, it might not be able to launch - for example, if a relevant config file
diff --git a/Makefile b/Makefile
index 6bdd3b9337..bf4f3ede2a 100644
--- a/Makefile
+++ b/Makefile
@@ -2135,6 +2135,8 @@  git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
 
 help.sp help.s help.o: command-list.h
 
+bugreport.sp bugreport.s bugreport.o: bugreport-config-safelist.h
+
 builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
 builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
 	'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
diff --git a/bugreport.c b/bugreport.c
index f143b9a8f8..70fb3b8fa6 100644
--- a/bugreport.c
+++ b/bugreport.c
@@ -6,6 +6,9 @@ 
 #include "help.h"
 #include "compat/compiler.h"
 #include "run-command.h"
+#include "config.h"
+#include "bugreport-config-safelist.h"
+#include "khash.h"
 
 static void get_git_remote_https_version_info(struct strbuf *version_info)
 {
@@ -52,6 +55,29 @@  static void get_system_info(struct strbuf *sys_info)
 	strbuf_complete_line(sys_info);
 }
 
+static void get_safelisted_config(struct strbuf *config_info)
+{
+	size_t idx;
+	struct string_list_item *it = NULL;
+	struct key_value_info *kv_info = NULL;
+
+	for (idx = 0; idx < ARRAY_SIZE(bugreport_config_safelist); idx++) {
+		const struct string_list *list =
+			git_config_get_value_multi(bugreport_config_safelist[idx]);
+
+		if (!list)
+			continue;
+
+		strbuf_addf(config_info, "%s:\n", bugreport_config_safelist[idx]);
+		for_each_string_list_item(it, list) {
+			kv_info = it->util;
+			strbuf_addf(config_info, "  %s (%s)\n", it->string,
+				    kv_info ? config_scope_name(kv_info->scope)
+					    : "source unknown");
+		}
+	}
+}
+
 static const char * const bugreport_usage[] = {
 	N_("git bugreport [-o|--output-directory <file>] [-s|--suffix <format>]"),
 	NULL
@@ -94,6 +120,7 @@  int cmd_main(int argc, const char **argv)
 	char *option_output = NULL;
 	char *option_suffix = "%F-%H%M";
 	struct stat statbuf;
+	int nongit_ok = 0;
 
 	const struct option bugreport_options[] = {
 		OPT_STRING('o', "output-directory", &option_output, N_("path"),
@@ -102,6 +129,10 @@  int cmd_main(int argc, const char **argv)
 			   N_("specify a strftime format suffix for the filename")),
 		OPT_END()
 	};
+
+	/* Prerequisite for hooks and config checks */
+	setup_git_directory_gently(&nongit_ok);
+
 	argc = parse_options(argc, argv, "", bugreport_options,
 			     bugreport_usage, 0);
 
@@ -132,6 +163,9 @@  int cmd_main(int argc, const char **argv)
 	get_header(&buffer, "System Info");
 	get_system_info(&buffer);
 
+	get_header(&buffer, "Safelisted Config Info");
+	get_safelisted_config(&buffer);
+
 	report = fopen_for_writing(report_path.buf);
 
 	if (report == NULL) {