From 9bebd9879c9a9cee0e49f06d7fc63ba6f8e5cbc0 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Tue, 17 Oct 2023 19:33:05 +0200
Subject: [PATCH] cli: Add coverage for internal and sensitive commands

---
 bin/varnishd/cache/cache_main.c  | 33 ++++++++++++++++++++++++++++++++
 bin/varnishd/mgt/mgt_cli.c       | 24 +++++++++++++++++++++++
 bin/varnishtest/tests/b00083.vtc | 25 ++++++++++++++++++++++++
 include/tbl/cli_cmds.h           | 28 +++++++++++++++++++++++++++
 4 files changed, 110 insertions(+)
 create mode 100644 bin/varnishtest/tests/b00083.vtc

diff --git a/bin/varnishd/cache/cache_main.c b/bin/varnishd/cache/cache_main.c
index a4f2a4ece31..637eda69341 100644
--- a/bin/varnishd/cache/cache_main.c
+++ b/bin/varnishd/cache/cache_main.c
@@ -261,10 +261,43 @@ cli_debug_srandom(struct cli *cli, const char * const *av, void *priv)
 	VRND_SeedTestable(seed);
 }
 
+static void v_matchproto_(cli_func_t)
+cli_debug_sensitive(struct cli *cli, const char * const *av, void *priv)
+{
+
+	(void)priv;
+	(void)av;
+	VCLI_Out(cli, "This should be logged nowhere");
+}
+
+static void v_matchproto_(cmd_log_func_t)
+cli_debug_sensitive_log(const struct cli* cli, const char * const *av, struct vsb *vsb)
+{
+
+	(void)cli;
+
+	AN(av);
+	AN(vsb);
+
+	VSB_printf(vsb, "%s %s XXXXX", av[1]!=NULL ? av[1] : "(null)",
+	    av[2] != NULL ? av[2] : "(null)");
+}
+
+static void v_matchproto_(cli_func_t)
+cli_debug_internal(struct cli *cli, const char * const *av, void *priv)
+{
+
+	(void)priv;
+	(void)av;
+	VCLI_Out(cli, "This is an internal command");
+}
+
 static struct cli_proto debug_cmds[] = {
 	{ CLICMD_DEBUG_XID,		cli_debug_xid },
 	{ CLICMD_DEBUG_SHUTDOWN_DELAY,	cli_debug_shutdown_delay },
 	{ CLICMD_DEBUG_SRANDOM,		cli_debug_srandom },
+	{ CLICMD_DEBUG_SENSITIVE,	cli_debug_sensitive, NULL,  cli_debug_sensitive_log},
+	{ CLICMD_DEBUG_CLD_INTERNAL,	cli_debug_internal },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 687984707d5..e6393f7f386 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -115,8 +115,32 @@ mcf_panic(struct cli *cli, const char * const *av, void *priv)
 	abort();
 }
 
+static void v_matchproto_(cli_func_t)
+mcf_debug_internal(struct cli *cli, const char * const *av, void *priv)
+{
+
+	(void)av;
+	(void)priv;
+	unsigned i, s;
+	char *r;
+	if (!MCH_Running()) {
+		VCLI_Out(cli, "Child is not running");
+		return;
+	}
+
+	i = mgt_cli_askchild(&s, &r, "debug.cld_internal\n");
+	VCLI_SetResult(cli, s);
+	if (i) {
+		VCLI_Out(cli, "Child returned Error: (%d)", s);
+		return;
+	}
+	VCLI_Out(cli, "Child answered: (%d) %s", s, r);
+	free(r);
+}
+
 static struct cli_proto cli_debug[] = {
 	{ CLICMD_DEBUG_PANIC_MASTER,		mcf_panic },
+	{ CLICMD_DEBUG_INTERNAL,		mcf_debug_internal },
 	{ NULL }
 };
 
diff --git a/bin/varnishtest/tests/b00083.vtc b/bin/varnishtest/tests/b00083.vtc
new file mode 100644
index 00000000000..e4b58457720
--- /dev/null
+++ b/bin/varnishtest/tests/b00083.vtc
@@ -0,0 +1,25 @@
+varnishtest "test cli flags"
+
+server s0 {} -start
+
+varnish v1 -vcl+backend {} -start
+
+#internal commands should not be listed in the command list
+shell -err {varnishadm -n ${v1_name} help -a | grep debug.cld_internal}
+shell -err {varnishadm -n ${v1_name} help -j | grep debug.cld_internal}
+
+varnish v1 -clierr 101 "help debug.cld_internal"
+
+#internal commands should not be executed
+varnish v1 -cliexpect "Unknown request." "debug.cld_internal"
+
+#internal commands must be executable by MGT
+varnish v1 -cliexpect {\(200\) This is an internal command} "debug.internal"
+
+#sensitive commands should be logged according to their own implementation
+varnish v1 -cliok "debug.sensitive user secret"
+shell {varnishlog -n ${v1_name} -g raw -d -i CLI | grep "Rd debug.sensitive user XXXXX"}
+shell {varnishlog -n ${v1_name} -g raw -d -i CLI | grep "Wr 200 8 (hidden)"}
+varnish v1 -cliok "param.set debug +cli_show_sensitive"
+varnish v1 -cliok "debug.sensitive user secret"
+shell {varnishlog -n ${v1_name} -g raw -d -i CLI | grep "debug.sensitive user secret"}
diff --git a/include/tbl/cli_cmds.h b/include/tbl/cli_cmds.h
index c32f25212da..cbe3ee8e32a 100644
--- a/include/tbl/cli_cmds.h
+++ b/include/tbl/cli_cmds.h
@@ -472,6 +472,34 @@ CLI_CMD(DEBUG_PERSISTENT,
 	0, 2
 )
 
+CLI_CMD(DEBUG_SENSITIVE,
+	"debug.sensitive",
+	"debug.sensitive <id> <secret>",
+	"Output should not be logged.\n",
+	"",
+	CLI_F_DEBUG|
+	CLI_F_SENSITIVE,
+	2, 2
+)
+
+CLI_CMD(DEBUG_CLD_INTERNAL,
+	"debug.cld_internal",
+	"debug.cld_internal",
+	"May only be issued by MGT process.\n",
+	"",
+	CLI_F_INTERNAL,
+	0, 0
+)
+
+CLI_CMD(DEBUG_INTERNAL,
+	"debug.internal",
+	"debug.internal",
+	"Used to call cld_internal.\n",
+	"",
+	CLI_F_DEBUG,
+	0, 0
+)
+
 CLI_CMD(STORAGE_LIST,
 	"storage.list",
 	"storage.list [-j]",