From d6299c3bb58072fe1809c79ae5604263ba03b0be Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Tue, 6 Jun 2023 18:29:15 +0200
Subject: [PATCH 01/15] cli: Remove unused flags

Some cli commands have flags (h,i,*) that are never parsed in the code,
they can safely be removed.
---
 bin/varnishd/cache/cache_cli.c | 4 ++--
 bin/varnishd/mgt/mgt_cli.c     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index 2dd203597e0..da8f9b21e33 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -115,8 +115,8 @@ CLI_Run(void)
 /*--------------------------------------------------------------------*/
 
 static struct cli_proto cli_cmds[] = {
-	{ CLICMD_PING,	"i", VCLS_func_ping, VCLS_func_ping_json },
-	{ CLICMD_HELP,	"i", VCLS_func_help, VCLS_func_help_json },
+	{ CLICMD_PING,	"", VCLS_func_ping, VCLS_func_ping_json },
+	{ CLICMD_HELP,	"", VCLS_func_help, VCLS_func_help_json },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index c3cada3d997..2f7668ab2d0 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -165,7 +165,7 @@ static const struct cli_cmd_desc CLICMD_WILDCARD[1] =
     {{ "*", "<wild-card-entry>", "<fall through to cacher>", "", 0, 999 }};
 
 static struct cli_proto cli_askchild[] = {
-	{ CLICMD_WILDCARD, "h*", mcf_askchild, mcf_askchild },
+	{ CLICMD_WILDCARD, "", mcf_askchild, mcf_askchild },
 	{ NULL }
 };
 

From 860d12fdae34598110ca05c29049fd1ab0f2e1da Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Wed, 7 Jun 2023 11:37:45 +0200
Subject: [PATCH 02/15] cli: Turn cmd flags into bitmask and move to struct
 cli_cmd_desc

Flags are much more convenient to handle when represented as bits rather
than a list of characters. In addition, they better fit in the struct
cli_cmd_desc than the struct cli_proto
---
 bin/varnishd/cache/cache_acceptor.c       |  4 +--
 bin/varnishd/cache/cache_ban.c            |  5 ++-
 bin/varnishd/cache/cache_cli.c            |  4 +--
 bin/varnishd/cache/cache_director.c       |  5 ++-
 bin/varnishd/cache/cache_fetch_proc.c     |  2 +-
 bin/varnishd/cache/cache_main.c           |  6 ++--
 bin/varnishd/cache/cache_panic.c          |  2 +-
 bin/varnishd/cache/cache_vcl.c            | 14 ++++----
 bin/varnishd/cache/cache_vrt_vmod.c       |  2 +-
 bin/varnishd/cache/cache_wrk.c            |  2 +-
 bin/varnishd/mgt/mgt_child.c              | 12 +++----
 bin/varnishd/mgt/mgt_cli.c                | 22 ++++++------
 bin/varnishd/mgt/mgt_param.c              |  6 ++--
 bin/varnishd/mgt/mgt_vcl.c                | 18 +++++-----
 bin/varnishd/storage/mgt_stevedore.c      |  2 +-
 bin/varnishd/storage/storage_persistent.c |  2 +-
 include/Makefile.am                       |  1 +
 include/tbl/cli_cmds.h                    | 42 +++++++++++++++++++++--
 include/tbl/cli_flags.h                   | 34 ++++++++++++++++++
 include/vcli_serve.h                      |  9 +++--
 lib/libvarnish/vcli_serve.c               | 15 +++++---
 21 files changed, 146 insertions(+), 63 deletions(-)
 create mode 100644 include/tbl/cli_flags.h

diff --git a/bin/varnishd/cache/cache_acceptor.c b/bin/varnishd/cache/cache_acceptor.c
index e229c7db4ca..72536747cb4 100644
--- a/bin/varnishd/cache/cache_acceptor.c
+++ b/bin/varnishd/cache/cache_acceptor.c
@@ -752,8 +752,8 @@ ccf_listen_address(struct cli *cli, const char * const *av, void *priv)
 /*--------------------------------------------------------------------*/
 
 static struct cli_proto vca_cmds[] = {
-	{ CLICMD_SERVER_START,		"", ccf_start },
-	{ CLICMD_DEBUG_LISTEN_ADDRESS,	"d", ccf_listen_address },
+	{ CLICMD_SERVER_START,		ccf_start },
+	{ CLICMD_DEBUG_LISTEN_ADDRESS,	ccf_listen_address },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/cache/cache_ban.c b/bin/varnishd/cache/cache_ban.c
index 0c6a6a2de4c..5e52e1175f5 100644
--- a/bin/varnishd/cache/cache_ban.c
+++ b/bin/varnishd/cache/cache_ban.c
@@ -944,9 +944,8 @@ ccf_ban_list(struct cli *cli, const char * const *av, void *priv)
 }
 
 static struct cli_proto ban_cmds[] = {
-	{ CLICMD_BAN,				"", ccf_ban },
-	{ CLICMD_BAN_LIST,			"", ccf_ban_list,
-	  ccf_ban_list },
+	{ CLICMD_BAN,		ccf_ban },
+	{ CLICMD_BAN_LIST,	ccf_ban_list,  ccf_ban_list },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index da8f9b21e33..21c4974f3aa 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -115,8 +115,8 @@ CLI_Run(void)
 /*--------------------------------------------------------------------*/
 
 static struct cli_proto cli_cmds[] = {
-	{ CLICMD_PING,	"", VCLS_func_ping, VCLS_func_ping_json },
-	{ CLICMD_HELP,	"", VCLS_func_help, VCLS_func_help_json },
+	{ CLICMD_PING,	VCLS_func_ping, VCLS_func_ping_json },
+	{ CLICMD_HELP,	VCLS_func_help, VCLS_func_help_json },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/cache/cache_director.c b/bin/varnishd/cache/cache_director.c
index 952dd717f84..2464ec83f8f 100644
--- a/bin/varnishd/cache/cache_director.c
+++ b/bin/varnishd/cache/cache_director.c
@@ -518,9 +518,8 @@ cli_backend_set_health(struct cli *cli, const char * const *av, void *priv)
 /*---------------------------------------------------------------------*/
 
 static struct cli_proto backend_cmds[] = {
-	{ CLICMD_BACKEND_LIST,		"",
-	     cli_backend_list, cli_backend_list },
-	{ CLICMD_BACKEND_SET_HEALTH,	"", cli_backend_set_health },
+	{ CLICMD_BACKEND_LIST,		cli_backend_list, cli_backend_list },
+	{ CLICMD_BACKEND_SET_HEALTH,	cli_backend_set_health },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/cache/cache_fetch_proc.c b/bin/varnishd/cache/cache_fetch_proc.c
index 150b473f992..7358857cfb3 100644
--- a/bin/varnishd/cache/cache_fetch_proc.c
+++ b/bin/varnishd/cache/cache_fetch_proc.c
@@ -243,7 +243,7 @@ debug_fragfetch(struct cli *cli, const char * const *av, void *priv)
 }
 
 static struct cli_proto debug_cmds[] = {
-	{ CLICMD_DEBUG_FRAGFETCH,		"d", debug_fragfetch },
+	{ CLICMD_DEBUG_FRAGFETCH,	debug_fragfetch },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/cache/cache_main.c b/bin/varnishd/cache/cache_main.c
index 3e093c50b2b..a4f2a4ece31 100644
--- a/bin/varnishd/cache/cache_main.c
+++ b/bin/varnishd/cache/cache_main.c
@@ -262,9 +262,9 @@ cli_debug_srandom(struct cli *cli, const char * const *av, void *priv)
 }
 
 static struct cli_proto debug_cmds[] = {
-	{ CLICMD_DEBUG_XID,		"d", cli_debug_xid },
-	{ CLICMD_DEBUG_SHUTDOWN_DELAY,	"d", cli_debug_shutdown_delay },
-	{ CLICMD_DEBUG_SRANDOM,		"d", cli_debug_srandom },
+	{ CLICMD_DEBUG_XID,		cli_debug_xid },
+	{ CLICMD_DEBUG_SHUTDOWN_DELAY,	cli_debug_shutdown_delay },
+	{ CLICMD_DEBUG_SRANDOM,		cli_debug_srandom },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/cache/cache_panic.c b/bin/varnishd/cache/cache_panic.c
index 221d9835cca..43dfe62087b 100644
--- a/bin/varnishd/cache/cache_panic.c
+++ b/bin/varnishd/cache/cache_panic.c
@@ -865,7 +865,7 @@ ccf_panic(struct cli *cli, const char * const *av, void *priv)
 /*--------------------------------------------------------------------*/
 
 static struct cli_proto debug_cmds[] = {
-	{ CLICMD_DEBUG_PANIC_WORKER,		"d",	ccf_panic },
+	{ CLICMD_DEBUG_PANIC_WORKER,		ccf_panic },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index 29d4b459fee..fac9dea17c1 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -1005,13 +1005,13 @@ vcl_cli_show(struct cli *cli, const char * const *av, void *priv)
 /*--------------------------------------------------------------------*/
 
 static struct cli_proto vcl_cmds[] = {
-	{ CLICMD_VCL_LOAD,		"", vcl_cli_load },
-	{ CLICMD_VCL_LIST,		"", vcl_cli_list, vcl_cli_list_json },
-	{ CLICMD_VCL_STATE,		"", vcl_cli_state },
-	{ CLICMD_VCL_DISCARD,		"", vcl_cli_discard },
-	{ CLICMD_VCL_USE,		"", vcl_cli_use },
-	{ CLICMD_VCL_SHOW,		"", vcl_cli_show },
-	{ CLICMD_VCL_LABEL,		"", vcl_cli_label },
+	{ CLICMD_VCL_LOAD,		vcl_cli_load },
+	{ CLICMD_VCL_LIST,		vcl_cli_list, vcl_cli_list_json },
+	{ CLICMD_VCL_STATE,		vcl_cli_state },
+	{ CLICMD_VCL_DISCARD,		vcl_cli_discard },
+	{ CLICMD_VCL_USE,		vcl_cli_use },
+	{ CLICMD_VCL_SHOW,		vcl_cli_show },
+	{ CLICMD_VCL_LABEL,		vcl_cli_label },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/cache/cache_vrt_vmod.c b/bin/varnishd/cache/cache_vrt_vmod.c
index 8a088a19d40..c64c9e00202 100644
--- a/bin/varnishd/cache/cache_vrt_vmod.c
+++ b/bin/varnishd/cache/cache_vrt_vmod.c
@@ -223,7 +223,7 @@ ccf_debug_vmod(struct cli *cli, const char * const *av, void *priv)
 }
 
 static struct cli_proto vcl_cmds[] = {
-	{ CLICMD_DEBUG_VMOD,			"d", ccf_debug_vmod },
+	{ CLICMD_DEBUG_VMOD,			ccf_debug_vmod },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/cache/cache_wrk.c b/bin/varnishd/cache/cache_wrk.c
index 8803b5048cc..d152895da35 100644
--- a/bin/varnishd/cache/cache_wrk.c
+++ b/bin/varnishd/cache/cache_wrk.c
@@ -736,7 +736,7 @@ debug_reqpoolfail(struct cli *cli, const char * const *av, void *priv)
 }
 
 static struct cli_proto debug_cmds[] = {
-	{ CLICMD_DEBUG_REQPOOLFAIL,		"d", debug_reqpoolfail },
+	{ CLICMD_DEBUG_REQPOOLFAIL,		debug_reqpoolfail },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/mgt/mgt_child.c b/bin/varnishd/mgt/mgt_child.c
index b4fd329bbd9..c1564e7a5e4 100644
--- a/bin/varnishd/mgt/mgt_child.c
+++ b/bin/varnishd/mgt/mgt_child.c
@@ -800,14 +800,14 @@ mch_cli_server_status_json(struct cli *cli, const char * const *av, void *priv)
 }
 
 static struct cli_proto cli_mch[] = {
-	{ CLICMD_SERVER_STATUS,		"", mch_cli_server_status,
+	{ CLICMD_SERVER_STATUS,		mch_cli_server_status,
 	  mch_cli_server_status_json },
-	{ CLICMD_SERVER_START,		"", mch_cli_server_start },
-	{ CLICMD_SERVER_STOP,		"", mch_cli_server_stop },
-	{ CLICMD_PANIC_SHOW,		"", mch_cli_panic_show,
+	{ CLICMD_SERVER_START,		mch_cli_server_start },
+	{ CLICMD_SERVER_STOP,		mch_cli_server_stop },
+	{ CLICMD_PANIC_SHOW,		mch_cli_panic_show,
 	  mch_cli_panic_show_json },
-	{ CLICMD_PANIC_CLEAR,		"", mch_cli_panic_clear },
-	{ CLICMD_PID,			"", mch_pid, mch_pid_json },
+	{ CLICMD_PANIC_CLEAR,		mch_cli_panic_clear },
+	{ CLICMD_PID,			mch_pid, mch_pid_json },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 2f7668ab2d0..5f7c396fa36 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -54,12 +54,12 @@
 #include "vss.h"
 #include "vtcp.h"
 
-#define CLI_CMD(U,l,s,h,d,m,M) \
-const struct cli_cmd_desc CLICMD_##U[1] = {{ l, s, h, d, m, M }};
+#define CLI_CMD(U,l,s,h,d,f,m,M) \
+const struct cli_cmd_desc CLICMD_##U[1] = {{ l, s, h, d, f, m, M }};
 #include "tbl/cli_cmds.h"
 
 static const struct cli_cmd_desc *cmds[] = {
-#define CLI_CMD(U,l,s,h,d,m,M) CLICMD_##U,
+#define CLI_CMD(U,l,s,h,d,f,m,M) CLICMD_##U,
 #include "tbl/cli_cmds.h"
 };
 
@@ -95,7 +95,7 @@ mcf_banner(struct cli *cli, const char *const *av, void *priv)
 /*--------------------------------------------------------------------*/
 
 static struct cli_proto cli_proto[] = {
-	{ CLICMD_BANNER,		"", mcf_banner },
+	{ CLICMD_BANNER,		mcf_banner },
 	{ NULL }
 };
 
@@ -115,7 +115,7 @@ mcf_panic(struct cli *cli, const char * const *av, void *priv)
 }
 
 static struct cli_proto cli_debug[] = {
-	{ CLICMD_DEBUG_PANIC_MASTER,		"d", mcf_panic },
+	{ CLICMD_DEBUG_PANIC_MASTER,		mcf_panic },
 	{ NULL }
 };
 
@@ -162,10 +162,10 @@ mcf_askchild(struct cli *cli, const char * const *av, void *priv)
 }
 
 static const struct cli_cmd_desc CLICMD_WILDCARD[1] =
-    {{ "*", "<wild-card-entry>", "<fall through to cacher>", "", 0, 999 }};
+    {{ "*", "<wild-card-entry>", "<fall through to cacher>", "", CLI_F_NONE, 0, 999 }};
 
 static struct cli_proto cli_askchild[] = {
-	{ CLICMD_WILDCARD, "", mcf_askchild, mcf_askchild },
+	{ CLICMD_WILDCARD, mcf_askchild, mcf_askchild },
 	{ NULL }
 };
 
@@ -317,10 +317,10 @@ mcf_help_json(struct cli *cli, const char * const *av, void *priv)
 }
 
 static struct cli_proto cli_auth[] = {
-	{ CLICMD_HELP,		"", mcf_help, mcf_help_json },
-	{ CLICMD_PING,		"", VCLS_func_ping, VCLS_func_ping_json },
-	{ CLICMD_AUTH,		"", mcf_auth },
-	{ CLICMD_QUIT,		"", VCLS_func_close },
+	{ CLICMD_HELP,		mcf_help, mcf_help_json },
+	{ CLICMD_PING,		VCLS_func_ping, VCLS_func_ping_json },
+	{ CLICMD_AUTH,		mcf_auth },
+	{ CLICMD_QUIT,		VCLS_func_close },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index d305a100698..2a7ce30ddf8 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -711,9 +711,9 @@ mcf_wash_param(struct cli *cli, struct parspec *pp, enum mcf_which_e which,
 /*--------------------------------------------------------------------*/
 
 static struct cli_proto cli_params[] = {
-	{ CLICMD_PARAM_SHOW,	"", mcf_param_show, mcf_param_show_json },
-	{ CLICMD_PARAM_SET,	"", mcf_param_set, mcf_param_set_json },
-	{ CLICMD_PARAM_RESET,	"", mcf_param_reset, mcf_param_set_json },
+	{ CLICMD_PARAM_SHOW,	mcf_param_show, mcf_param_show_json },
+	{ CLICMD_PARAM_SET,	mcf_param_set, mcf_param_set_json },
+	{ CLICMD_PARAM_RESET,	mcf_param_reset, mcf_param_set_json },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/mgt/mgt_vcl.c b/bin/varnishd/mgt/mgt_vcl.c
index 882c828aada..6c987cd771d 100644
--- a/bin/varnishd/mgt/mgt_vcl.c
+++ b/bin/varnishd/mgt/mgt_vcl.c
@@ -1097,15 +1097,15 @@ mgt_vcl_poker(const struct vev *e, int what)
 /*--------------------------------------------------------------------*/
 
 static struct cli_proto cli_vcl[] = {
-	{ CLICMD_VCL_LOAD,		"", mcf_vcl_load },
-	{ CLICMD_VCL_INLINE,		"", mcf_vcl_inline },
-	{ CLICMD_VCL_USE,		"", mcf_vcl_use },
-	{ CLICMD_VCL_STATE,		"", mcf_vcl_state },
-	{ CLICMD_VCL_DISCARD,		"", mcf_vcl_discard },
-	{ CLICMD_VCL_LIST,		"", mcf_vcl_list, mcf_vcl_list_json },
-	{ CLICMD_VCL_DEPS,		"", mcf_vcl_deps, mcf_vcl_deps_json },
-	{ CLICMD_VCL_LABEL,		"", mcf_vcl_label },
-	{ CLICMD_DEBUG_VCL_SYMTAB,	"d", mcf_vcl_symtab },
+	{ CLICMD_VCL_LOAD,		mcf_vcl_load },
+	{ CLICMD_VCL_INLINE,		mcf_vcl_inline },
+	{ CLICMD_VCL_USE,		mcf_vcl_use },
+	{ CLICMD_VCL_STATE,		mcf_vcl_state },
+	{ CLICMD_VCL_DISCARD,		mcf_vcl_discard },
+	{ CLICMD_VCL_LIST,		mcf_vcl_list, mcf_vcl_list_json },
+	{ CLICMD_VCL_DEPS,		mcf_vcl_deps, mcf_vcl_deps_json },
+	{ CLICMD_VCL_LABEL,		mcf_vcl_label },
+	{ CLICMD_DEBUG_VCL_SYMTAB,	mcf_vcl_symtab },
 	{ NULL }
 };
 
diff --git a/bin/varnishd/storage/mgt_stevedore.c b/bin/varnishd/storage/mgt_stevedore.c
index b62248365f7..b9250330aa0 100644
--- a/bin/varnishd/storage/mgt_stevedore.c
+++ b/bin/varnishd/storage/mgt_stevedore.c
@@ -125,7 +125,7 @@ stv_cli_list_json(struct cli *cli, const char * const *av, void *priv)
 /*--------------------------------------------------------------------*/
 
 static struct cli_proto cli_stv[] = {
-	{ CLICMD_STORAGE_LIST,		"", stv_cli_list, stv_cli_list_json },
+	{ CLICMD_STORAGE_LIST,		stv_cli_list, stv_cli_list_json },
 	{ NULL}
 };
 
diff --git a/bin/varnishd/storage/storage_persistent.c b/bin/varnishd/storage/storage_persistent.c
index 0f858771efd..27626d1db6d 100644
--- a/bin/varnishd/storage/storage_persistent.c
+++ b/bin/varnishd/storage/storage_persistent.c
@@ -671,7 +671,7 @@ debug_persistent(struct cli *cli, const char * const * av, void *priv)
 }
 
 static struct cli_proto debug_cmds[] = {
-	{ CLICMD_DEBUG_PERSISTENT,		"d", debug_persistent },
+	{ CLICMD_DEBUG_PERSISTENT,		debug_persistent },
 	{ NULL }
 };
 
diff --git a/include/Makefile.am b/include/Makefile.am
index 1a68ab4a38a..4bac90f3335 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -90,6 +90,7 @@ nobase_noinst_HEADERS = \
 	compat/daemon.h \
 	vfl.h \
 	libvcc.h \
+	tbl/cli_flags.h \
 	vcc_interface.h \
 	vcli_serve.h \
 	vcs_version.h \
diff --git a/include/tbl/cli_cmds.h b/include/tbl/cli_cmds.h
index 3295b5844ee..5e7aacfc4d3 100644
--- a/include/tbl/cli_cmds.h
+++ b/include/tbl/cli_cmds.h
@@ -35,6 +35,7 @@
  *	const char *	request_syntax (for short help)
  *	const char *	request_help (for long help)
  *	const char *	documentation (for sphinx)
+ *	unsigned	command flags
  *	int		minimum_arguments
  *	int		maximum_arguments
  */
@@ -46,6 +47,7 @@ CLI_CMD(BAN,
 	"ban <field> <operator> <arg> [&& <field> <oper> <arg> ...]",
 	"Mark obsolete all objects where all the conditions match.",
 	"  See :ref:`vcl(7)_ban` for details",
+	CLI_F_NONE,
 	3, -1
 )
 
@@ -66,7 +68,7 @@ CLI_CMD(BAN_LIST,
 	"  * Ban specification\n\n"
 	"  Durations of ban specifications get normalized, for example \"7d\""
 	" gets changed into \"1w\".",
-
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -75,6 +77,7 @@ CLI_CMD(VCL_LOAD,
 	"vcl.load <configname> <filename> [auto|cold|warm]",
 	"Compile and load the VCL file under the name provided.",
 	"",
+	CLI_F_NONE,
 	2, 3
 )
 
@@ -85,7 +88,7 @@ CLI_CMD(VCL_INLINE,
 
 	"  Multi-line VCL can be input using the here document"
 	" :ref:`ref_syntax`.",
-
+	CLI_F_NONE,
 	2, 3
 )
 
@@ -94,6 +97,7 @@ CLI_CMD(VCL_STATE,
 	"vcl.state <configname> auto|cold|warm",
 	"  Force the state of the named configuration.",
 	"",
+	CLI_F_NONE,
 	2, 2
 )
 
@@ -109,6 +113,7 @@ CLI_CMD(VCL_DISCARD,
 	" nothing is discarded."
 	" Each individual name pattern must match at least one named"
 	" configuration or label.",
+	CLI_F_NONE,
 	1, -1
 )
 
@@ -127,6 +132,7 @@ CLI_CMD(VCL_LIST,
 	"  * [ ``<-`` | ``->`` ] and label info last two fields)\n\n"
 	"    * ``->`` <vcl> : label \"points to\" the named <vcl>\n\n"
 	"    * ``<-`` (<n> label[s]): the vcl has <n> label(s)\n\n",
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -141,6 +147,7 @@ CLI_CMD(VCL_DEPS,
 	"  * Dependency: another VCL program it depends on\n\n"
 	"  Only direct dependencies are listed, and VCLs with"
 	" multiple dependencies are listed multiple times.",
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -149,6 +156,7 @@ CLI_CMD(VCL_SHOW,
 	"vcl.show [-v] [<configname>]",
 	"Display the source code for the specified configuration.",
 	"",
+	CLI_F_NONE,
 	0, 2
 )
 
@@ -157,6 +165,7 @@ CLI_CMD(VCL_USE,
 	"vcl.use <configname|label>",
 	"Switch to the named configuration immediately.",
 	"",
+	CLI_F_NONE,
 	1, 1
 )
 
@@ -167,6 +176,7 @@ CLI_CMD(VCL_LABEL,
 	"  A VCL label is like a UNIX symbolic link, "
 	" a name without substance, which points to another VCL.\n\n"
 	"  Labels are mandatory whenever one VCL references another.",
+	CLI_F_NONE,
 	2, 2
 )
 
@@ -177,6 +187,7 @@ CLI_CMD(PARAM_RESET,
 	"  The JSON output is the same as ``param.show -j <param>`` and"
 	" contains the updated value as it would be represented by a"
 	" subsequent execution of ``param.show``.\n\n",
+	CLI_F_NONE,
 	1,1
 )
 
@@ -192,6 +203,7 @@ CLI_CMD(PARAM_SHOW,
 	" ``-j`` is permitted. If a parameter is specified with ``<param>``,"
 	" show only that parameter. If ``changed`` is specified, show only"
 	" those parameters whose values differ from their defaults.",
+	CLI_F_NONE,
 	0, 2
 )
 
@@ -205,6 +217,7 @@ CLI_CMD(PARAM_SET,
 	"  This can be useful to later verify that a parameter value didn't"
 	" change and to use the value from the JSON output to reset the"
 	" parameter to the desired value.",
+	CLI_F_NONE,
 	2,2
 )
 
@@ -213,6 +226,7 @@ CLI_CMD(SERVER_STOP,
 	"stop",
 	"Stop the Varnish cache process.",
 	"",
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -221,6 +235,7 @@ CLI_CMD(SERVER_START,
 	"start",
 	"Start the Varnish cache process.",
 	"",
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -229,6 +244,7 @@ CLI_CMD(PING,
 	"ping [-j] [<timestamp>]",
 	"Keep connection alive.",
 	"  The response is formatted as JSON if ``-j`` is specified.",
+	CLI_F_NONE,
 	0, 1
 )
 
@@ -237,6 +253,7 @@ CLI_CMD(HELP,
 	"help [-j|<command>]",
 	"Show command/protocol help.",
 	"  ``-j`` specifies JSON output.",
+	CLI_F_NONE,
 	0, 1
 )
 
@@ -245,6 +262,7 @@ CLI_CMD(QUIT,
 	"quit",
 	"Close connection.",
 	"",
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -253,6 +271,7 @@ CLI_CMD(SERVER_STATUS,
 	"status [-j]",
 	"Check status of Varnish cache process.",
 	"  ``-j`` specifies JSON output.",
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -261,6 +280,7 @@ CLI_CMD(BANNER,
 	"banner",
 	"Print welcome banner.",
 	"",
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -269,6 +289,7 @@ CLI_CMD(AUTH,
 	"auth <response>",
 	"Authenticate.",
 	"",
+	CLI_F_NONE,
 	1, 1
 )
 
@@ -278,6 +299,7 @@ CLI_CMD(PANIC_SHOW,
 	"Return the last panic, if any.",
 	"  ``-j`` specifies JSON output -- the panic message is returned as an"
 	" unstructured JSON string.",
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -287,6 +309,7 @@ CLI_CMD(PANIC_CLEAR,
 	"Clear the last panic, if any,"
 	" -z will clear related varnishstat counter(s)",
 	"",
+	CLI_F_NONE,
 	0, 1
 )
 
@@ -295,6 +318,7 @@ CLI_CMD(DEBUG_LISTEN_ADDRESS,
 	"debug.listen_address",
 	"Report the actual listen address.",
 	"",
+	CLI_F_DEBUG,
 	0, 0
 )
 
@@ -334,6 +358,7 @@ CLI_CMD(BACKEND_LIST,
 	"  matching the fields described above. ``probe_message`` has the\n"
 	"  format ``[X, Y, \"state\"]`` as described above for Probe. JSON\n"
 	"  Probe details (``-j -p`` arguments) are director specific.",
+	CLI_F_NONE,
 	0, 2
 )
 
@@ -345,6 +370,7 @@ CLI_CMD(BACKEND_SET_HEALTH,
 	"    or some other dynamic mechanism, if any\n"
 	"  * ``healthy`` sets the backend as usable\n"
 	"  * ``sick`` sets the backend as unsable\n",
+	CLI_F_NONE,
 	2, 2
 )
 
@@ -353,6 +379,7 @@ CLI_CMD(DEBUG_FRAGFETCH,
 	"debug.fragfetch",
 	"Enable fetch fragmentation.",
 	"",
+	CLI_F_DEBUG,
 	1, 1
 )
 
@@ -364,6 +391,7 @@ CLI_CMD(DEBUG_REQPOOLFAIL,
 	"\tparam.set debug.reqpoolfail F__F_F\n\n"
 	"In the example above the first, fourth and sixth attempted\n"
 	"allocations will fail.",
+	CLI_F_DEBUG,
 	1, 1
 )
 
@@ -372,6 +400,7 @@ CLI_CMD(DEBUG_SHUTDOWN_DELAY,
 	"debug.shutdown.delay",
 	"Add a delay to the child process shutdown.",
 	"",
+	CLI_F_DEBUG,
 	1, 1
 )
 
@@ -380,6 +409,7 @@ CLI_CMD(DEBUG_XID,
 	"debug.xid [<xid> [<cachesize>]]",
 	"Examine or set XID. <cachesize> defaults to 1.",
 	"",
+	CLI_F_DEBUG,
 	0, 2
 )
 
@@ -388,6 +418,7 @@ CLI_CMD(DEBUG_SRANDOM,
 	"debug.srandom",
 	"Seed the random(3) function.",
 	"",
+	CLI_F_DEBUG,
 	0, 1
 )
 
@@ -396,6 +427,7 @@ CLI_CMD(DEBUG_PANIC_WORKER,
 	"debug.panic.worker",
 	"Panic the worker process.",
 	"",
+	CLI_F_DEBUG,
 	0, 0
 )
 
@@ -404,6 +436,7 @@ CLI_CMD(DEBUG_PANIC_MASTER,
 	"debug.panic.master",
 	"Panic the master process.",
 	"",
+	CLI_F_DEBUG,
 	0, 0
 )
 
@@ -412,6 +445,7 @@ CLI_CMD(DEBUG_VCL_SYMTAB,
 	"vcl.symtab",
 	"Dump the VCL symbol-tables.",
 	"",
+	CLI_F_DEBUG,
 	0, 0
 )
 
@@ -420,6 +454,7 @@ CLI_CMD(DEBUG_VMOD,
 	"debug.vmod",
 	"Show loaded vmods.",
 	"",
+	CLI_F_DEBUG,
 	0, 0
 )
 
@@ -432,6 +467,7 @@ CLI_CMD(DEBUG_PERSISTENT,
 	"\tsync\tClose current segment, open a new one\n"
 	"\tdump\tinclude objcores in silo summary",
 	"",
+	CLI_F_DEBUG,
 	0, 2
 )
 
@@ -440,6 +476,7 @@ CLI_CMD(STORAGE_LIST,
 	"storage.list [-j]",
 	"List storage devices.",
 	"  ``-j`` specifies JSON output.",
+	CLI_F_NONE,
 	0, 0
 )
 
@@ -448,6 +485,7 @@ CLI_CMD(PID,
 	"pid [-j]",
 	"Show the pid of the master process, and the worker if it's running.",
 	"  ``-j`` specifies JSON output.",
+	CLI_F_NONE,
 	0, 0
 )
 
diff --git a/include/tbl/cli_flags.h b/include/tbl/cli_flags.h
new file mode 100644
index 00000000000..11dfe411f1a
--- /dev/null
+++ b/include/tbl/cli_flags.h
@@ -0,0 +1,34 @@
+/*-
+ * Copyright (c) 2023 Varnish Software AS
+ * All rights reserved.
+ *
+ * Author: Walid Boudebouda <walid.boudebouda@varnish-software.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ */
+
+
+CLI_FLAG(NONE, 0)
+CLI_FLAG(DEBUG, (1 << 0))
+
+#undef CLI_FLAG
diff --git a/include/vcli_serve.h b/include/vcli_serve.h
index e1b721ea4ec..5d13e80fe3e 100644
--- a/include/vcli_serve.h
+++ b/include/vcli_serve.h
@@ -40,23 +40,28 @@ struct VCLS;
 
 typedef void cli_func_t(struct cli*, const char * const *av, void *priv);
 
+enum cli_flags {
+#define CLI_FLAG(U, v)	CLI_F_##U = v,
+#include "tbl/cli_flags.h"
+};
+
 struct cli_cmd_desc {
 	/* Must match CLI_CMD macro in include/tbl/cli_cmds.h */
 	const char			*request;
 	const char			*syntax;
 	const char			*help;
 	const char			*doc;
+	unsigned			flags;
 	int				minarg;
 	int				maxarg;
 };
 
-#define CLI_CMD(U,l,s,h,d,m,M) extern const struct cli_cmd_desc CLICMD_##U[1];
+#define CLI_CMD(U,l,s,h,d,f,m,M) extern const struct cli_cmd_desc CLICMD_##U[1];
 #include "tbl/cli_cmds.h"
 
 /* A CLI command */
 struct cli_proto {
 	const struct cli_cmd_desc	*desc;
-	const char			*flags;
 
 	/* Dispatch information */
 	cli_func_t			*func;
diff --git a/lib/libvarnish/vcli_serve.c b/lib/libvarnish/vcli_serve.c
index dbb384a0ffa..a4f3fcb6c70 100644
--- a/lib/libvarnish/vcli_serve.c
+++ b/lib/libvarnish/vcli_serve.c
@@ -155,7 +155,7 @@ VCLS_func_help(struct cli *cli, const char * const *av, void *priv)
 			help_helper(cli, clp, av);
 			return;
 		} else if (av[0] == NULL) {
-			d = strchr(clp->flags, 'd') != NULL ? 2 : 1;
+			d = clp->desc->flags & CLI_F_DEBUG ? 2 : 1;
 			if (filter & d)
 				help_helper(cli, clp, av);
 		}
@@ -171,6 +171,7 @@ VCLS_func_help_json(struct cli *cli, const char * const *av, void *priv)
 {
 	struct cli_proto *clp;
 	struct VCLS *cs;
+	const char* sep;
 
 	(void)priv;
 	cs = cli->cls;
@@ -178,6 +179,7 @@ VCLS_func_help_json(struct cli *cli, const char * const *av, void *priv)
 
 	VCLI_JSON_begin(cli, 2, av);
 	VTAILQ_FOREACH(clp, &cs->funcs, list) {
+		sep = "";
 		if (clp->auth > cli->auth)
 			continue;
 		VCLI_Out(cli, ",\n  {\n");
@@ -195,9 +197,14 @@ VCLS_func_help_json(struct cli *cli, const char * const *av, void *priv)
 		VCLI_Out(cli, ",\n");
 		VCLI_Out(cli, "\"maxarg\": %d", clp->desc->maxarg);
 		VCLI_Out(cli, ",\n");
-		VCLI_Out(cli, "\"flags\": ");
-		VCLI_JSON_str(cli, clp->flags);
-		VCLI_Out(cli, ",\n");
+		VCLI_Out(cli, "\"flags\": [");
+		#define CLI_FLAG(U, v) \
+			if (clp->desc->flags & v) { \
+				VCLI_Out(cli, "%s\""#U "\"", sep); \
+				sep = ", "; \
+			}
+		#include "tbl/cli_flags.h"
+		VCLI_Out(cli, "],\n");
 		VCLI_Out(cli, "\"json\": %s",
 		    clp->jsonfunc == NULL ? "false" : "true");
 		VCLI_Out(cli, "\n");

From 512a43eddbc5fbb20b3574da5b11698a59c1cdf5 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Wed, 7 Jun 2023 18:26:24 +0200
Subject: [PATCH 03/15] cli: Hide internal commands in help and man

Internal commands are commands that are only intended to be used
for internal communication between MGT process and the child process.
Thus, they should not be exposed to the user in cli help/man page.
---
 bin/varnishd/mgt/mgt_cli.c  | 4 +++-
 include/tbl/cli_flags.h     | 1 +
 lib/libvarnish/vcli_serve.c | 4 ++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 5f7c396fa36..433675bf188 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -162,7 +162,7 @@ mcf_askchild(struct cli *cli, const char * const *av, void *priv)
 }
 
 static const struct cli_cmd_desc CLICMD_WILDCARD[1] =
-    {{ "*", "<wild-card-entry>", "<fall through to cacher>", "", CLI_F_NONE, 0, 999 }};
+    {{ "*", "<wild-card-entry>", "<fall through to cacher>", "", CLI_F_INTERNAL, 0, 999 }};
 
 static struct cli_proto cli_askchild[] = {
 	{ CLICMD_WILDCARD, mcf_askchild, mcf_askchild },
@@ -693,6 +693,8 @@ mgt_DumpRstCli(void)
 		cp = cmds[z];
 		if (!strncmp(cp->request, "debug.", 6))
 			continue;
+		if (cp->flags & CLI_F_INTERNAL)
+			continue;
 		printf(".. _ref_cli_");
 		for (p = cp->request; *p; p++)
 			fputc(*p == '.' ? '_' : *p, stdout);
diff --git a/include/tbl/cli_flags.h b/include/tbl/cli_flags.h
index 11dfe411f1a..0dcd1f3dd84 100644
--- a/include/tbl/cli_flags.h
+++ b/include/tbl/cli_flags.h
@@ -30,5 +30,6 @@
 
 CLI_FLAG(NONE, 0)
 CLI_FLAG(DEBUG, (1 << 0))
+CLI_FLAG(INTERNAL, (1 << 1))
 
 #undef CLI_FLAG
diff --git a/lib/libvarnish/vcli_serve.c b/lib/libvarnish/vcli_serve.c
index a4f3fcb6c70..9f987d08534 100644
--- a/lib/libvarnish/vcli_serve.c
+++ b/lib/libvarnish/vcli_serve.c
@@ -151,6 +151,8 @@ VCLS_func_help(struct cli *cli, const char * const *av, void *priv)
 	VTAILQ_FOREACH(clp, &cs->funcs, list) {
 		if (clp->auth > cli->auth)
 			continue;
+		if (clp->desc->flags & CLI_F_INTERNAL)
+			continue;
 		if (av[0] != NULL && !strcmp(clp->desc->request, av[0])) {
 			help_helper(cli, clp, av);
 			return;
@@ -182,6 +184,8 @@ VCLS_func_help_json(struct cli *cli, const char * const *av, void *priv)
 		sep = "";
 		if (clp->auth > cli->auth)
 			continue;
+		if (clp->desc->flags & CLI_F_INTERNAL)
+			continue;
 		VCLI_Out(cli, ",\n  {\n");
 		VSB_indent(cli->sb, 2);
 		VCLI_Out(cli, "\"request\": ");

From 322efa3be5ad15c19a0c86833ed0a1ac8654f41a Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Tue, 13 Jun 2023 00:31:00 +0200
Subject: [PATCH 04/15] cli: Prohibit internal commands when called externally

Hiding internal commands in help is not sufficient, we must ensure
they are not executed by the user through cli and handle them as
unknown commands in that case.
---
 bin/varnishd/mgt/mgt_cli.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 433675bf188..95785e1d099 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -141,7 +141,20 @@ mcf_askchild(struct cli *cli, const char * const *av, void *priv)
 		    "Type 'help' for more info.");
 		return;
 	}
+
+	for (i = 0; i < ncmds; i++) {
+		if (strcmp(av[1], cmds[i]->request))
+			continue;
+		if (cmds[i]->flags & CLI_F_INTERNAL) {
+			VCLI_Out(cli, "Unknown request.\nType 'help' for more info.\n");
+			VCLI_SetResult(cli, CLIS_UNKNOWN);
+			return;
+		}
+		break;
+	}
+
 	VSB_clear(cli_buf);
+
 	for (i = 1; av[i] != NULL; i++) {
 		VSB_quote(cli_buf, av[i], strlen(av[i]), 0);
 		VSB_putc(cli_buf, ' ');

From 3950112d076ca47258a9af026827dfdd13560575 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Fri, 9 Jun 2023 19:02:43 +0200
Subject: [PATCH 05/15] cli: Remove auth from cli_proto and use flag instead

Since we only have two authentication levels and that we now have
proper flags, we can use them to distinguish commands that need
an authenticated cli to be used, and get rid of the auth attribute
in struct cli_proto. MCF_AUTH and MCF_NOAUTH aren't useful anymore
and can be safely removed.
---
 bin/varnishd/cache/cache_cli.c       |  2 +-
 bin/varnishd/mgt/mgt.h               |  2 --
 bin/varnishd/mgt/mgt_child.c         |  2 +-
 bin/varnishd/mgt/mgt_cli.c           | 26 ++++++++++++-------
 bin/varnishd/mgt/mgt_param.c         |  2 +-
 bin/varnishd/mgt/mgt_vcl.c           |  2 +-
 bin/varnishd/storage/mgt_stevedore.c |  2 +-
 include/tbl/cli_cmds.h               | 39 ++++++++++++++--------------
 include/tbl/cli_flags.h              |  5 ++--
 include/vcli_serve.h                 |  3 +--
 lib/libvarnish/vcli_serve.c          | 11 ++++----
 11 files changed, 51 insertions(+), 45 deletions(-)

diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index 21c4974f3aa..7bcbd23de56 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -66,7 +66,7 @@ CLI_AddFuncs(struct cli_proto *p)
 
 	AZ(add_check);
 	Lck_Lock(&cli_mtx);
-	VCLS_AddFunc(cache_cls, 0, p);
+	VCLS_AddFunc(cache_cls, p);
 	Lck_Unlock(&cli_mtx);
 }
 
diff --git a/bin/varnishd/mgt/mgt.h b/bin/varnishd/mgt/mgt.h
index 3d133db345b..89e576bcecf 100644
--- a/bin/varnishd/mgt/mgt.h
+++ b/bin/varnishd/mgt/mgt.h
@@ -97,8 +97,6 @@ void mgt_cli_secret(const char *S_arg);
 void mgt_cli_close_all(void);
 void mgt_DumpRstCli(void);
 void mgt_cli_init_cls(void);
-#define MCF_NOAUTH	0	/* NB: zero disables here-documents */
-#define MCF_AUTH	16
 
 /* mgt_jail.c */
 
diff --git a/bin/varnishd/mgt/mgt_child.c b/bin/varnishd/mgt/mgt_child.c
index c1564e7a5e4..4c6d13230d8 100644
--- a/bin/varnishd/mgt/mgt_child.c
+++ b/bin/varnishd/mgt/mgt_child.c
@@ -821,5 +821,5 @@ void
 MCH_Init(void)
 {
 
-	VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_mch);
+	VCLS_AddFunc(mgt_cls, cli_mch);
 }
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 95785e1d099..2922348113b 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -174,8 +174,16 @@ mcf_askchild(struct cli *cli, const char * const *av, void *priv)
 	free(q);
 }
 
-static const struct cli_cmd_desc CLICMD_WILDCARD[1] =
-    {{ "*", "<wild-card-entry>", "<fall through to cacher>", "", CLI_F_INTERNAL, 0, 999 }};
+static const struct cli_cmd_desc CLICMD_WILDCARD[1] = {{
+	"*",
+       "<wild-card-entry>",
+       "<fall through to cacher>",
+       "",
+       CLI_F_INTERNAL|
+       CLI_F_AUTH,
+       0,
+       999
+}};
 
 static struct cli_proto cli_askchild[] = {
 	{ CLICMD_WILDCARD, mcf_askchild, mcf_askchild },
@@ -303,7 +311,7 @@ mcf_auth(struct cli *cli, const char *const *av, void *priv)
 		VCLI_SetResult(cli, CLIS_CLOSE);
 		return;
 	}
-	cli->auth = MCF_AUTH;
+	cli->auth = 1;
 	memset(cli->challenge, 0, sizeof cli->challenge);
 	VCLI_SetResult(cli, CLIS_OK);
 	mcf_banner(cli, av, priv);
@@ -373,10 +381,10 @@ mgt_cli_init_cls(void)
 	mgt_cls = VCLS_New(NULL);
 	AN(mgt_cls);
 	VCLS_SetHooks(mgt_cls, mgt_cli_cb_before, mgt_cli_cb_after);
-	VCLS_AddFunc(mgt_cls, MCF_NOAUTH, cli_auth);
-	VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_proto);
-	VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_debug);
-	VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_askchild);
+	VCLS_AddFunc(mgt_cls, cli_auth);
+	VCLS_AddFunc(mgt_cls, cli_proto);
+	VCLS_AddFunc(mgt_cls, cli_debug);
+	VCLS_AddFunc(mgt_cls, cli_askchild);
 	cli_buf = VSB_new_auto();
 	AN(cli_buf);
 }
@@ -420,10 +428,10 @@ mgt_cli_setup(int fdi, int fdo, int auth, const char *ident,
 	REPLACE(cli->ident, ident);
 
 	if (!auth && secret_file != NULL) {
-		cli->auth = MCF_NOAUTH;
+		cli->auth = 0;
 		mgt_cli_challenge(cli);
 	} else {
-		cli->auth = MCF_AUTH;
+		cli->auth = 1;
 		mcf_banner(cli, NULL, NULL);
 	}
 	AZ(VSB_finish(cli->sb));
diff --git a/bin/varnishd/mgt/mgt_param.c b/bin/varnishd/mgt/mgt_param.c
index 2a7ce30ddf8..46017871658 100644
--- a/bin/varnishd/mgt/mgt_param.c
+++ b/bin/varnishd/mgt/mgt_param.c
@@ -772,7 +772,7 @@ MCF_InitParams(struct cli *cli)
 	MCF_ParamConf(MCF_DEFAULT, "accept_filter", "off");
 #endif
 
-	VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_params);
+	VCLS_AddFunc(mgt_cls, cli_params);
 
 	vsb = VSB_new_auto();
 	AN(vsb);
diff --git a/bin/varnishd/mgt/mgt_vcl.c b/bin/varnishd/mgt/mgt_vcl.c
index 6c987cd771d..8afb89e1fd3 100644
--- a/bin/varnishd/mgt/mgt_vcl.c
+++ b/bin/varnishd/mgt/mgt_vcl.c
@@ -1139,5 +1139,5 @@ mgt_vcl_init(void)
 
 	AZ(atexit(mgt_vcl_atexit));
 
-	VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_vcl);
+	VCLS_AddFunc(mgt_cls, cli_vcl);
 }
diff --git a/bin/varnishd/storage/mgt_stevedore.c b/bin/varnishd/storage/mgt_stevedore.c
index b9250330aa0..7ddb96e3d10 100644
--- a/bin/varnishd/storage/mgt_stevedore.c
+++ b/bin/varnishd/storage/mgt_stevedore.c
@@ -243,7 +243,7 @@ STV_Config_Final(void)
 	struct stevedore *stv;
 	ASSERT_MGT();
 
-	VCLS_AddFunc(mgt_cls, MCF_AUTH, cli_stv);
+	VCLS_AddFunc(mgt_cls, cli_stv);
 	STV_Foreach(stv)
 		if (!strcmp(stv->ident, TRANSIENT_STORAGE))
 			return;
diff --git a/include/tbl/cli_cmds.h b/include/tbl/cli_cmds.h
index 5e7aacfc4d3..c32f25212da 100644
--- a/include/tbl/cli_cmds.h
+++ b/include/tbl/cli_cmds.h
@@ -77,7 +77,7 @@ CLI_CMD(VCL_LOAD,
 	"vcl.load <configname> <filename> [auto|cold|warm]",
 	"Compile and load the VCL file under the name provided.",
 	"",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	2, 3
 )
 
@@ -88,7 +88,7 @@ CLI_CMD(VCL_INLINE,
 
 	"  Multi-line VCL can be input using the here document"
 	" :ref:`ref_syntax`.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	2, 3
 )
 
@@ -97,7 +97,7 @@ CLI_CMD(VCL_STATE,
 	"vcl.state <configname> auto|cold|warm",
 	"  Force the state of the named configuration.",
 	"",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	2, 2
 )
 
@@ -113,7 +113,7 @@ CLI_CMD(VCL_DISCARD,
 	" nothing is discarded."
 	" Each individual name pattern must match at least one named"
 	" configuration or label.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	1, -1
 )
 
@@ -132,7 +132,7 @@ CLI_CMD(VCL_LIST,
 	"  * [ ``<-`` | ``->`` ] and label info last two fields)\n\n"
 	"    * ``->`` <vcl> : label \"points to\" the named <vcl>\n\n"
 	"    * ``<-`` (<n> label[s]): the vcl has <n> label(s)\n\n",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 0
 )
 
@@ -147,7 +147,7 @@ CLI_CMD(VCL_DEPS,
 	"  * Dependency: another VCL program it depends on\n\n"
 	"  Only direct dependencies are listed, and VCLs with"
 	" multiple dependencies are listed multiple times.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 0
 )
 
@@ -165,7 +165,7 @@ CLI_CMD(VCL_USE,
 	"vcl.use <configname|label>",
 	"Switch to the named configuration immediately.",
 	"",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	1, 1
 )
 
@@ -176,7 +176,7 @@ CLI_CMD(VCL_LABEL,
 	"  A VCL label is like a UNIX symbolic link, "
 	" a name without substance, which points to another VCL.\n\n"
 	"  Labels are mandatory whenever one VCL references another.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	2, 2
 )
 
@@ -187,7 +187,7 @@ CLI_CMD(PARAM_RESET,
 	"  The JSON output is the same as ``param.show -j <param>`` and"
 	" contains the updated value as it would be represented by a"
 	" subsequent execution of ``param.show``.\n\n",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	1,1
 )
 
@@ -203,7 +203,7 @@ CLI_CMD(PARAM_SHOW,
 	" ``-j`` is permitted. If a parameter is specified with ``<param>``,"
 	" show only that parameter. If ``changed`` is specified, show only"
 	" those parameters whose values differ from their defaults.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 2
 )
 
@@ -217,7 +217,7 @@ CLI_CMD(PARAM_SET,
 	"  This can be useful to later verify that a parameter value didn't"
 	" change and to use the value from the JSON output to reset the"
 	" parameter to the desired value.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	2,2
 )
 
@@ -226,7 +226,7 @@ CLI_CMD(SERVER_STOP,
 	"stop",
 	"Stop the Varnish cache process.",
 	"",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 0
 )
 
@@ -235,7 +235,7 @@ CLI_CMD(SERVER_START,
 	"start",
 	"Start the Varnish cache process.",
 	"",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 0
 )
 
@@ -271,7 +271,7 @@ CLI_CMD(SERVER_STATUS,
 	"status [-j]",
 	"Check status of Varnish cache process.",
 	"  ``-j`` specifies JSON output.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 0
 )
 
@@ -280,7 +280,7 @@ CLI_CMD(BANNER,
 	"banner",
 	"Print welcome banner.",
 	"",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 0
 )
 
@@ -299,7 +299,7 @@ CLI_CMD(PANIC_SHOW,
 	"Return the last panic, if any.",
 	"  ``-j`` specifies JSON output -- the panic message is returned as an"
 	" unstructured JSON string.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 0
 )
 
@@ -309,7 +309,7 @@ CLI_CMD(PANIC_CLEAR,
 	"Clear the last panic, if any,"
 	" -z will clear related varnishstat counter(s)",
 	"",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 1
 )
 
@@ -436,6 +436,7 @@ CLI_CMD(DEBUG_PANIC_MASTER,
 	"debug.panic.master",
 	"Panic the master process.",
 	"",
+	CLI_F_AUTH|
 	CLI_F_DEBUG,
 	0, 0
 )
@@ -476,7 +477,7 @@ CLI_CMD(STORAGE_LIST,
 	"storage.list [-j]",
 	"List storage devices.",
 	"  ``-j`` specifies JSON output.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 0
 )
 
@@ -485,7 +486,7 @@ CLI_CMD(PID,
 	"pid [-j]",
 	"Show the pid of the master process, and the worker if it's running.",
 	"  ``-j`` specifies JSON output.",
-	CLI_F_NONE,
+	CLI_F_AUTH,
 	0, 0
 )
 
diff --git a/include/tbl/cli_flags.h b/include/tbl/cli_flags.h
index 0dcd1f3dd84..977b4987e26 100644
--- a/include/tbl/cli_flags.h
+++ b/include/tbl/cli_flags.h
@@ -29,7 +29,8 @@
 
 
 CLI_FLAG(NONE, 0)
-CLI_FLAG(DEBUG, (1 << 0))
-CLI_FLAG(INTERNAL, (1 << 1))
+CLI_FLAG(AUTH, (1 << 0))
+CLI_FLAG(DEBUG, (1 << 1))
+CLI_FLAG(INTERNAL, (1 << 2))
 
 #undef CLI_FLAG
diff --git a/include/vcli_serve.h b/include/vcli_serve.h
index 5d13e80fe3e..4cc1b4057ef 100644
--- a/include/vcli_serve.h
+++ b/include/vcli_serve.h
@@ -68,7 +68,6 @@ struct cli_proto {
 	cli_func_t			*jsonfunc;
 	void				*priv;
 
-	unsigned			auth;
 	VTAILQ_ENTRY(cli_proto)		list;
 };
 
@@ -103,7 +102,7 @@ void VCLS_SetHooks(struct VCLS *, cls_cbc_f *, cls_cbc_f *);
 void VCLS_SetLimit(struct VCLS *, volatile unsigned *);
 struct cli *VCLS_AddFd(struct VCLS *cs, int fdi, int fdo, cls_cb_f *closefunc,
     void *priv);
-void VCLS_AddFunc(struct VCLS *cs, unsigned auth, struct cli_proto *clp);
+void VCLS_AddFunc(struct VCLS *cs, struct cli_proto *clp);
 int VCLS_Poll(struct VCLS *cs, const struct cli*, int timeout);
 void VCLS_Destroy(struct VCLS **);
 
diff --git a/lib/libvarnish/vcli_serve.c b/lib/libvarnish/vcli_serve.c
index 9f987d08534..23912047a3a 100644
--- a/lib/libvarnish/vcli_serve.c
+++ b/lib/libvarnish/vcli_serve.c
@@ -149,7 +149,7 @@ VCLS_func_help(struct cli *cli, const char * const *av, void *priv)
 		}
 	}
 	VTAILQ_FOREACH(clp, &cs->funcs, list) {
-		if (clp->auth > cli->auth)
+		if ((clp->desc->flags & CLI_F_AUTH) && !cli->auth)
 			continue;
 		if (clp->desc->flags & CLI_F_INTERNAL)
 			continue;
@@ -182,7 +182,7 @@ VCLS_func_help_json(struct cli *cli, const char * const *av, void *priv)
 	VCLI_JSON_begin(cli, 2, av);
 	VTAILQ_FOREACH(clp, &cs->funcs, list) {
 		sep = "";
-		if (clp->auth > cli->auth)
+		if ((clp->desc->flags & CLI_F_AUTH) && !cli->auth)
 			continue;
 		if (clp->desc->flags & CLI_F_INTERNAL)
 			continue;
@@ -325,7 +325,7 @@ cls_exec(struct VCLS_fd *cfd, char * const *av)
 			continue;
 
 		VTAILQ_FOREACH(clp, &cs->funcs, list) {
-			if (clp->auth > cli->auth)
+			if ((clp->desc->flags & CLI_F_AUTH) && !cli->auth)
 				continue;
 			if (!strcmp(clp->desc->request, av[1])) {
 				cls_dispatch(cli, clp, av, na);
@@ -333,7 +333,7 @@ cls_exec(struct VCLS_fd *cfd, char * const *av)
 			}
 		}
 		if (clp == NULL &&
-		    cs->wildcard && cs->wildcard->auth <= cli->auth)
+		    cs->wildcard && (!(cs->wildcard->desc->flags & CLI_F_AUTH) || cli->auth))
 			cls_dispatch(cli, cs->wildcard, av, na);
 
 	} while (0);
@@ -560,7 +560,7 @@ cls_close_fd(struct VCLS *cs, struct VCLS_fd *cfd)
 }
 
 void
-VCLS_AddFunc(struct VCLS *cs, unsigned auth, struct cli_proto *clp)
+VCLS_AddFunc(struct VCLS *cs, struct cli_proto *clp)
 {
 	struct cli_proto *clp2;
 	int i;
@@ -569,7 +569,6 @@ VCLS_AddFunc(struct VCLS *cs, unsigned auth, struct cli_proto *clp)
 	AN(clp);
 
 	for (;clp->desc != NULL; clp++) {
-		clp->auth = auth;
 		if (!strcmp(clp->desc->request, "*")) {
 			cs->wildcard = clp;
 		} else {

From 82558a28cb76ec8e0cb0fe9108dae1028c8a3c95 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Mon, 31 Jul 2023 19:02:34 +0200
Subject: [PATCH 06/15] cli: Refactor cls_exec

Make the cli function lookup an independent function, and call it
prior to the "before" callback method. This will allow to
conditionnally execute instructions in the callback method depending
on the cli function we are currently handling.
---
 lib/libvarnish/vcli_serve.c | 112 ++++++++++++++++++++++++------------
 1 file changed, 74 insertions(+), 38 deletions(-)

diff --git a/lib/libvarnish/vcli_serve.c b/lib/libvarnish/vcli_serve.c
index 23912047a3a..f60ee89029d 100644
--- a/lib/libvarnish/vcli_serve.c
+++ b/lib/libvarnish/vcli_serve.c
@@ -79,6 +79,15 @@ struct VCLS {
 	struct cli_proto		*wildcard;
 };
 
+enum cmd_error_e {
+    CMD_ERR_NONE,
+    CMD_ERR_SYNTAX,
+    CMD_ERR_EMPTY,
+    CMD_ERR_ISUPPER,
+    CMD_ERR_NOTLOWER,
+    CMD_ERR_UNKNOWN
+};
+
 /*--------------------------------------------------------------------*/
 
 void v_matchproto_(cli_func_t)
@@ -266,6 +275,42 @@ cls_dispatch(struct cli *cli, const struct cli_proto *cp,
 		cp->func(cli, (const char * const *)av, cp->priv);
 }
 
+static struct cli_proto *
+cls_lookup(char * const *av, struct cli *cli, struct VCLS *cs, enum cmd_error_e *err) {
+
+	struct cli_proto *clp = NULL;
+	*err = CMD_ERR_NONE;
+
+	if (av[0] != NULL) {
+		*err = CMD_ERR_SYNTAX;
+		return (NULL);
+	}
+
+	if (av[1] == NULL) {
+		*err = CMD_ERR_EMPTY;
+		return (NULL);
+	}
+
+	if (isupper(av[1][0])) {
+		*err = CMD_ERR_NOTLOWER;
+		return (NULL);
+	}
+
+	if (!islower(av[1][0])) {
+		*err = CMD_ERR_UNKNOWN;
+		return (NULL);
+	}
+
+
+	VTAILQ_FOREACH(clp, &cs->funcs, list) {
+		if ((clp->desc->flags & CLI_F_AUTH) && !cli->auth)
+			continue;
+		if (!strcmp(clp->desc->request, av[1]))
+			break;
+	}
+
+	return (clp);
+}
 /*--------------------------------------------------------------------
  * We have collected a full cli line, parse it and execute, if possible.
  */
@@ -274,13 +319,14 @@ static int
 cls_exec(struct VCLS_fd *cfd, char * const *av)
 {
 	struct VCLS *cs;
-	struct cli_proto *clp;
+	struct cli_proto *clp = NULL;
 	struct cli *cli;
 	int na;
 	ssize_t len;
 	char *s;
 	unsigned lim;
 	int retval = 0;
+	enum cmd_error_e cmd_err = CMD_ERR_NONE;
 
 	CHECK_OBJ_NOTNULL(cfd, VCLS_FD_MAGIC);
 	cs = cfd->cls;
@@ -291,52 +337,42 @@ cls_exec(struct VCLS_fd *cfd, char * const *av)
 	AN(cli->cmd);
 
 	cli->cls = cs;
-
 	cli->result = CLIS_UNKNOWN;
+
 	VSB_clear(cli->sb);
 	VCLI_Out(cli, "Unknown request.\nType 'help' for more info.\n");
 
-	if (cs->before != NULL)
-		cs->before(cli);
-
-	do {
-		if (av[0] != NULL) {
-			VCLI_Out(cli, "Syntax Error: %s\n", av[0]);
-			VCLI_SetResult(cli, CLIS_SYNTAX);
-			break;
-		}
-
-		if (av[1] == NULL) {
-			VCLI_Out(cli, "Empty CLI command.\n");
-			VCLI_SetResult(cli, CLIS_SYNTAX);
-			break;
-		}
+	for (na = 0; av[na + 1] != NULL; (na)++)
+		continue;
 
-		if (isupper(av[1][0])) {
-			VCLI_Out(cli, "all commands are in lower-case.\n");
-			VCLI_SetResult(cli, CLIS_UNKNOWN);
-			break;
-		}
+	clp = cls_lookup(av, cli, cs, &cmd_err);
 
-		if (!islower(av[1][0]))
-			break;
-
-		for (na = 0; av[na + 1] != NULL; na++)
-			continue;
+	if (cs->before != NULL)
+		cs->before(cli);
 
-		VTAILQ_FOREACH(clp, &cs->funcs, list) {
-			if ((clp->desc->flags & CLI_F_AUTH) && !cli->auth)
-				continue;
-			if (!strcmp(clp->desc->request, av[1])) {
-				cls_dispatch(cli, clp, av, na);
+	if (clp == NULL) {
+		switch (cmd_err) {
+			case (CMD_ERR_SYNTAX):
+				VCLI_Out(cli, "Syntax Error: %s\n", av[0]);
+				VCLI_SetResult(cli, CLIS_SYNTAX);
+				break;
+			case (CMD_ERR_EMPTY):
+				VCLI_Out(cli, "Empty CLI command.\n");
+				VCLI_SetResult(cli, CLIS_SYNTAX);
+				break;
+			case (CMD_ERR_NOTLOWER):
+				VCLI_Out(cli, "all commands are in lower-case.\n");
+				VCLI_SetResult(cli, CLIS_UNKNOWN);
+				break;
+			case (CMD_ERR_UNKNOWN):
+				break;
+			default:
+				if (cs->wildcard && (!(cs->wildcard->desc->flags & CLI_F_AUTH) || cli->auth))
+					cls_dispatch(cli, cs->wildcard, av, na);
 				break;
-			}
 		}
-		if (clp == NULL &&
-		    cs->wildcard && (!(cs->wildcard->desc->flags & CLI_F_AUTH) || cli->auth))
-			cls_dispatch(cli, cs->wildcard, av, na);
-
-	} while (0);
+	} else
+		cls_dispatch(cli, clp, av, na);
 
 	AZ(VSB_finish(cli->sb));
 

From a3d5364caff6865ae3d6f046c6dcad51983d55a4 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Mon, 31 Jul 2023 19:04:20 +0200
Subject: [PATCH 07/15] cli: Allow sensitive commands to provide their own
 logging

Sensitive commands are commands that accept/return sensitive data as
argument/output (ex: private keys). This commit prevents commands
with the CLI_F_SENSITIVE flag and their responses from being logged
to syslog/vsl. Sensitive commands can provide a custom logging function
to define how the command and argument should be logged. If a sensitive
command does not provide an implementation, then only the command name
followed by "(hidden)" will be logged. For the command response, the
status will be logged followed by the string "(hidden)".
---
 bin/varnishd/cache/cache_cli.c | 27 +++++++++++---
 bin/varnishd/mgt/mgt_cli.c     | 66 ++++++++++++++++++++++++++--------
 include/tbl/cli_flags.h        |  1 +
 include/vcli_serve.h           |  7 +++-
 lib/libvarnish/vcli_serve.c    |  8 ++---
 5 files changed, 85 insertions(+), 24 deletions(-)

diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index 7bcbd23de56..f5aee200469 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -71,23 +71,40 @@ CLI_AddFuncs(struct cli_proto *p)
 }
 
 static void
-cli_cb_before(const struct cli *cli)
+cli_cb_before(const struct cli *cli, struct cli_proto *clp, const char * const *av)
 {
-
 	ASSERT_CLI();
+	(void)av;
+
+	if (clp != NULL &&  (clp->desc->flags & CLI_F_SENSITIVE)) {
+		VSB_clear(cli->cmd);
+		if (clp->logfunc != NULL)
+			clp->logfunc(cli, av, cli->cmd);
+		else
+			VSB_printf(cli->cmd, "%s (hidden)", av[1]);
+		AZ(VSB_finish(cli->cmd));
+	}
 	VSL(SLT_CLI, NO_VXID, "Rd %s", VSB_data(cli->cmd));
 	Lck_Lock(&cli_mtx);
 	VCL_Poll();
 }
 
 static void
-cli_cb_after(const struct cli *cli)
+cli_cb_after(const struct cli *cli, struct cli_proto *clp, const char * const *av)
 {
 
 	ASSERT_CLI();
+	(void)av;
+	const char *h = "(hidden)";
+
 	Lck_Unlock(&cli_mtx);
-	VSL(SLT_CLI, NO_VXID, "Wr %03u %zd %s",
-	    cli->result, VSB_len(cli->sb), VSB_data(cli->sb));
+	if (clp == NULL || !(clp->desc->flags & CLI_F_SENSITIVE)) {
+		VSL(SLT_CLI, NO_VXID, "Wr %03u %zd %s",
+		    cli->result, VSB_len(cli->sb), VSB_data(cli->sb));
+	} else {
+		VSL(SLT_CLI, NO_VXID, "Wr %03u %zd %s",
+		    cli->result, strlen(h), h);
+	}
 }
 
 void
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 2922348113b..ed962e17976 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -54,6 +54,7 @@
 #include "vss.h"
 #include "vtcp.h"
 
+
 #define CLI_CMD(U,l,s,h,d,f,m,M) \
 const struct cli_cmd_desc CLICMD_##U[1] = {{ l, s, h, d, f, m, M }};
 #include "tbl/cli_cmds.h"
@@ -119,6 +120,21 @@ static struct cli_proto cli_debug[] = {
 	{ NULL }
 };
 
+static const struct cli_cmd_desc *
+mgt_cmd_lookup(const char *cmdn)
+{
+       int i;
+
+       if (cmdn == NULL)
+		return (NULL);
+
+       for (i = 0; i < ncmds; i++) {
+               if (!strcmp(cmdn, cmds[i]->request))
+                       return (cmds[i]);
+       }
+       return (NULL);
+}
+
 /*--------------------------------------------------------------------*/
 
 static void v_matchproto_(cli_func_t)
@@ -127,6 +143,7 @@ mcf_askchild(struct cli *cli, const char * const *av, void *priv)
 	int i;
 	char *q;
 	unsigned u;
+	const struct cli_cmd_desc *cmd;
 
 	(void)priv;
 	/*
@@ -142,15 +159,11 @@ mcf_askchild(struct cli *cli, const char * const *av, void *priv)
 		return;
 	}
 
-	for (i = 0; i < ncmds; i++) {
-		if (strcmp(av[1], cmds[i]->request))
-			continue;
-		if (cmds[i]->flags & CLI_F_INTERNAL) {
-			VCLI_Out(cli, "Unknown request.\nType 'help' for more info.\n");
-			VCLI_SetResult(cli, CLIS_UNKNOWN);
-			return;
-		}
-		break;
+	cmd = mgt_cmd_lookup(av[1]);
+	if (cmd != NULL && cmd->flags & CLI_F_INTERNAL) {
+		VCLI_Out(cli, "Unknown request.\nType 'help' for more info.\n");
+		VCLI_SetResult(cli, CLIS_UNKNOWN);
+		return;
 	}
 
 	VSB_clear(cli_buf);
@@ -348,20 +361,45 @@ static struct cli_proto cli_auth[] = {
 /*--------------------------------------------------------------------*/
 
 static void
-mgt_cli_cb_before(const struct cli *cli)
+mgt_cli_cb_before(const struct cli *cli, struct cli_proto *clp, const char * const *av)
 {
+	const struct cli_cmd_desc *cmd;
+	int d;
+
+	cmd = (clp == NULL ? mgt_cmd_lookup(av[1]) : clp->desc);
 
 	if (cli->priv == stderr)
 		fprintf(stderr, "> %s\n", VSB_data(cli->cmd));
+
+	if (cmd != NULL && (cmd->flags & CLI_F_SENSITIVE)) {
+		d = (*VSB_data(cli->cmd) == '-');
+		VSB_clear(cli->cmd);
+		VSB_printf(cli->cmd, "%s", d ? "-" : "");
+		if (clp != NULL && clp->logfunc != NULL)
+			clp->logfunc(cli, av, cli->cmd);
+		else
+			VSB_printf(cli->cmd, "%s (hidden)", av[1]);
+		AZ(VSB_finish(cli->cmd));
+	}
+
 	MGT_Complain(C_CLI, "CLI %s Rd %s", cli->ident, VSB_data(cli->cmd));
 }
 
 static void
-mgt_cli_cb_after(const struct cli *cli)
+mgt_cli_cb_after(const struct cli *cli, struct cli_proto *clp, const char * const *av)
 {
-
-	MGT_Complain(C_CLI, "CLI %s Wr %03u %s",
-	    cli->ident, cli->result, VSB_data(cli->sb));
+	const struct cli_cmd_desc *cmd;
+
+	if (av) {
+		cmd = (clp == NULL ? mgt_cmd_lookup(av[1]) : clp->desc);
+		if (cmd == NULL || !(cmd->flags & CLI_F_SENSITIVE)) {
+			MGT_Complain(C_CLI, "CLI %s Wr %03u %s",
+			    cli->ident, cli->result, VSB_data(cli->sb));
+		} else {
+			MGT_Complain(C_CLI, "CLI %s Wr %03u %s",
+			    cli->ident, cli->result, "(hidden)");
+		}
+	}
 	if (cli->priv != stderr)
 		return;
 	if (cli->result == CLIS_TRUNCATED)
diff --git a/include/tbl/cli_flags.h b/include/tbl/cli_flags.h
index 977b4987e26..3def775b1f7 100644
--- a/include/tbl/cli_flags.h
+++ b/include/tbl/cli_flags.h
@@ -32,5 +32,6 @@ CLI_FLAG(NONE, 0)
 CLI_FLAG(AUTH, (1 << 0))
 CLI_FLAG(DEBUG, (1 << 1))
 CLI_FLAG(INTERNAL, (1 << 2))
+CLI_FLAG(SENSITIVE, (1 << 3))
 
 #undef CLI_FLAG
diff --git a/include/vcli_serve.h b/include/vcli_serve.h
index 4cc1b4057ef..a90c61d5533 100644
--- a/include/vcli_serve.h
+++ b/include/vcli_serve.h
@@ -37,8 +37,10 @@
 
 struct cli;	/* NB: struct cli is opaque at this level.  */
 struct VCLS;
+struct vsb;
 
 typedef void cli_func_t(struct cli*, const char * const *av, void *priv);
+typedef void cmd_log_func_t(const struct cli*, const char * const *av, struct vsb *vsb);
 
 enum cli_flags {
 #define CLI_FLAG(U, v)	CLI_F_##U = v,
@@ -66,6 +68,9 @@ struct cli_proto {
 	/* Dispatch information */
 	cli_func_t			*func;
 	cli_func_t			*jsonfunc;
+	/* Used by sensitive commands for custom logging */
+	cmd_log_func_t			*logfunc;
+
 	void				*priv;
 
 	VTAILQ_ENTRY(cli_proto)		list;
@@ -96,7 +101,7 @@ void VCLI_JSON_end(struct cli *cli);
 void VCLI_SetResult(struct cli *cli, unsigned r);
 
 typedef int cls_cb_f(void *priv);
-typedef void cls_cbc_f(const struct cli*);
+typedef void cls_cbc_f(const struct cli*, struct cli_proto *, const char * const *av);
 struct VCLS *VCLS_New(struct VCLS *);
 void VCLS_SetHooks(struct VCLS *, cls_cbc_f *, cls_cbc_f *);
 void VCLS_SetLimit(struct VCLS *, volatile unsigned *);
diff --git a/lib/libvarnish/vcli_serve.c b/lib/libvarnish/vcli_serve.c
index f60ee89029d..b311c512e93 100644
--- a/lib/libvarnish/vcli_serve.c
+++ b/lib/libvarnish/vcli_serve.c
@@ -348,7 +348,7 @@ cls_exec(struct VCLS_fd *cfd, char * const *av)
 	clp = cls_lookup(av, cli, cs, &cmd_err);
 
 	if (cs->before != NULL)
-		cs->before(cli);
+		cs->before(cli, clp, (const char *const *)av);
 
 	if (clp == NULL) {
 		switch (cmd_err) {
@@ -377,7 +377,7 @@ cls_exec(struct VCLS_fd *cfd, char * const *av)
 	AZ(VSB_finish(cli->sb));
 
 	if (cs->after != NULL)
-		cs->after(cli);
+		cs->after(cli, clp, (const char *const *)av);
 
 	cli->cls = NULL;
 
@@ -573,13 +573,13 @@ cls_close_fd(struct VCLS *cs, struct VCLS_fd *cfd)
 	if (cfd->match != NULL) {
 		cfd->cli->result = CLIS_TRUNCATED;
 		if (cs->after != NULL)
-			cs->after(cfd->cli);
+			cs->after(cfd->cli, NULL, NULL);
 		VSB_destroy(&cfd->last_arg);
 	} else if (cfd->cli->cmd != NULL) {
 		(void)VSB_finish(cfd->cli->cmd);
 		cfd->cli->result = CLIS_TRUNCATED;
 		if (cs->after != NULL)
-			cs->after(cfd->cli);
+			cs->after(cfd->cli, NULL, NULL);
 		VSB_destroy(&cfd->cli->cmd);
 	}
 	cs->nfd--;

From 4108c4b121349fbbf7f339330c3493cc77269cd6 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Mon, 19 Jun 2023 18:00:36 +0200
Subject: [PATCH 08/15] cli: Add cli_show_sensitive to debug parameters

When setting +cli_show_sensitive to the debug parameter, all commands
will be logged to syslog (if syslog_cli_traffic is enabled) and vsl
regardless if they are sensitive or not.
---
 bin/varnishd/cache/cache_cli.c | 8 ++++++--
 bin/varnishd/mgt/mgt_cli.c     | 8 ++++++--
 include/tbl/debug_bits.h       | 1 +
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index f5aee200469..3943cf36b39 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -76,7 +76,9 @@ cli_cb_before(const struct cli *cli, struct cli_proto *clp, const char * const *
 	ASSERT_CLI();
 	(void)av;
 
-	if (clp != NULL &&  (clp->desc->flags & CLI_F_SENSITIVE)) {
+	if (clp != NULL &&
+	    !DO_DEBUG(DBG_CLI_SHOW_SENSITIVE) &&
+	    (clp->desc->flags & CLI_F_SENSITIVE)) {
 		VSB_clear(cli->cmd);
 		if (clp->logfunc != NULL)
 			clp->logfunc(cli, av, cli->cmd);
@@ -98,7 +100,9 @@ cli_cb_after(const struct cli *cli, struct cli_proto *clp, const char * const *a
 	const char *h = "(hidden)";
 
 	Lck_Unlock(&cli_mtx);
-	if (clp == NULL || !(clp->desc->flags & CLI_F_SENSITIVE)) {
+	if (clp == NULL ||
+	    DO_DEBUG(DBG_CLI_SHOW_SENSITIVE) ||
+	    !(clp->desc->flags & CLI_F_SENSITIVE)) {
 		VSL(SLT_CLI, NO_VXID, "Wr %03u %zd %s",
 		    cli->result, VSB_len(cli->sb), VSB_data(cli->sb));
 	} else {
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index ed962e17976..8c1781080af 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -371,7 +371,9 @@ mgt_cli_cb_before(const struct cli *cli, struct cli_proto *clp, const char * con
 	if (cli->priv == stderr)
 		fprintf(stderr, "> %s\n", VSB_data(cli->cmd));
 
-	if (cmd != NULL && (cmd->flags & CLI_F_SENSITIVE)) {
+	if (cmd != NULL &&
+	    !MGT_DO_DEBUG(DBG_CLI_SHOW_SENSITIVE) &&
+	    (cmd->flags & CLI_F_SENSITIVE)) {
 		d = (*VSB_data(cli->cmd) == '-');
 		VSB_clear(cli->cmd);
 		VSB_printf(cli->cmd, "%s", d ? "-" : "");
@@ -392,7 +394,9 @@ mgt_cli_cb_after(const struct cli *cli, struct cli_proto *clp, const char * cons
 
 	if (av) {
 		cmd = (clp == NULL ? mgt_cmd_lookup(av[1]) : clp->desc);
-		if (cmd == NULL || !(cmd->flags & CLI_F_SENSITIVE)) {
+		if (cmd == NULL ||
+		    MGT_DO_DEBUG(DBG_CLI_SHOW_SENSITIVE)  ||
+		    !(cmd->flags & CLI_F_SENSITIVE)) {
 			MGT_Complain(C_CLI, "CLI %s Wr %03u %s",
 			    cli->ident, cli->result, VSB_data(cli->sb));
 		} else {
diff --git a/include/tbl/debug_bits.h b/include/tbl/debug_bits.h
index 0e60d254702..09c79c0436f 100644
--- a/include/tbl/debug_bits.h
+++ b/include/tbl/debug_bits.h
@@ -52,6 +52,7 @@ DEBUG_BIT(PROCESSORS,		processors,	"Fetch/Deliver processors")
 DEBUG_BIT(PROTOCOL,		protocol,	"Protocol debugging")
 DEBUG_BIT(VCL_KEEP,		vcl_keep,	"Keep VCL C and so files")
 DEBUG_BIT(LCK,			lck,		"Additional lock statistics")
+DEBUG_BIT(CLI_SHOW_SENSITIVE,	cli_show_sensitive, "Log sensitive commands to syslog and VSL")
 #undef DEBUG_BIT
 
 /*lint -restore */

From dc747929fff4d3ea590909886c3a5a47588451ea Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Tue, 25 Jul 2023 19:09:29 +0200
Subject: [PATCH 09/15] cli: Don't show child help if not authenticated

Cli help command does not show commands that we don't have
the required auth level to execute. However, when the child
process is running and the mgt receives a help command, it asks
the child to execute it and return the response to the client.
The child sees the command as comming from mgt, which has the
highest auth level, and thus displays all the commands.
This commit prevents this from happening by not forwarding
the help command to child when the cli is not authentified.
---
 bin/varnishd/mgt/mgt_cli.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 8c1781080af..7d15a7a101a 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -335,7 +335,7 @@ mcf_auth(struct cli *cli, const char *const *av, void *priv)
 static void v_matchproto_(cli_func_t)
 mcf_help(struct cli *cli, const char * const *av, void *priv)
 {
-	if (cli_o <= 0)
+	if (cli_o <= 0 || !cli->auth)
 		VCLS_func_help(cli, av, priv);
 	else
 		mcf_askchild(cli, av, priv);
@@ -344,7 +344,7 @@ mcf_help(struct cli *cli, const char * const *av, void *priv)
 static void v_matchproto_(cli_func_t)
 mcf_help_json(struct cli *cli, const char * const *av, void *priv)
 {
-	if (cli_o <= 0)
+	if (cli_o <= 0 || !cli->auth)
 		VCLS_func_help_json(cli, av, priv);
 	else
 		mcf_askchild(cli, av, priv);

From d28524bc4e553863e644823e8e121ae7d34ee2a3 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Tue, 19 Sep 2023 14:55:26 +0200
Subject: [PATCH 10/15] cache_cli: Get rid of superfluous cli_mtx

This lock was used to guard the child VCLS, however we only interract
with it through the cli_thread, which makes the locking pointless.
---
 bin/varnishd/cache/cache_cli.c | 6 ------
 include/tbl/locks.h            | 1 -
 2 files changed, 7 deletions(-)

diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index 3943cf36b39..890d1c0e4e2 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -44,7 +44,6 @@
 #include "vcli_serve.h"
 
 pthread_t		cli_thread;
-static struct lock	cli_mtx;
 static int		add_check;
 static struct VCLS	*cache_cls;
 
@@ -65,9 +64,7 @@ CLI_AddFuncs(struct cli_proto *p)
 {
 
 	AZ(add_check);
-	Lck_Lock(&cli_mtx);
 	VCLS_AddFunc(cache_cls, p);
-	Lck_Unlock(&cli_mtx);
 }
 
 static void
@@ -87,7 +84,6 @@ cli_cb_before(const struct cli *cli, struct cli_proto *clp, const char * const *
 		AZ(VSB_finish(cli->cmd));
 	}
 	VSL(SLT_CLI, NO_VXID, "Rd %s", VSB_data(cli->cmd));
-	Lck_Lock(&cli_mtx);
 	VCL_Poll();
 }
 
@@ -99,7 +95,6 @@ cli_cb_after(const struct cli *cli, struct cli_proto *clp, const char * const *a
 	(void)av;
 	const char *h = "(hidden)";
 
-	Lck_Unlock(&cli_mtx);
 	if (clp == NULL ||
 	    DO_DEBUG(DBG_CLI_SHOW_SENSITIVE) ||
 	    !(clp->desc->flags & CLI_F_SENSITIVE)) {
@@ -149,7 +144,6 @@ void
 CLI_Init(void)
 {
 
-	Lck_New(&cli_mtx, lck_cli);
 	cli_thread = pthread_self();
 
 	cache_cls = VCLS_New(heritage.cls);
diff --git a/include/tbl/locks.h b/include/tbl/locks.h
index 3de8e33e6a5..ff6a40ff3b4 100644
--- a/include/tbl/locks.h
+++ b/include/tbl/locks.h
@@ -33,7 +33,6 @@
 
 LOCK(ban)
 LOCK(busyobj)
-LOCK(cli)
 LOCK(director)
 LOCK(exp)
 LOCK(hcb)

From b62ea87efa5d03812f2320e38a13fcd77a0897d2 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Mon, 28 Aug 2023 20:13:47 +0200
Subject: [PATCH 11/15] cli: Assert VCLS calls are from owning thread

Make sure that we only interract with a VCLS through the thread that
created it.
---
 lib/libvarnish/vcli_serve.c | 38 ++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/lib/libvarnish/vcli_serve.c b/lib/libvarnish/vcli_serve.c
index b311c512e93..356988cc31f 100644
--- a/lib/libvarnish/vcli_serve.c
+++ b/lib/libvarnish/vcli_serve.c
@@ -42,6 +42,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <pthread.h>
 
 #include "vdef.h"
 #include "vas.h"
@@ -53,6 +54,13 @@
 #include "vsb.h"
 #include "vtim.h"
 
+#define ASSERT_CLI_THR(cs) AN(pthread_self() == cs->thr)
+#define CHECK_VCLS(cs) do			\
+{						\
+	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);	\
+	ASSERT_CLI_THR(cs);			\
+} while (0)
+
 struct VCLS_fd {
 	unsigned			magic;
 #define VCLS_FD_MAGIC			0x010dbd1e
@@ -77,6 +85,7 @@ struct VCLS {
 	cls_cbc_f			*before, *after;
 	volatile unsigned		*limit;
 	struct cli_proto		*wildcard;
+	pthread_t			thr;
 };
 
 enum cmd_error_e {
@@ -93,9 +102,13 @@ enum cmd_error_e {
 void v_matchproto_(cli_func_t)
 VCLS_func_close(struct cli *cli, const char *const *av, void *priv)
 {
+	struct VCLS *cs;
 
 	(void)av;
 	(void)priv;
+	cs = cli->cls;
+	CHECK_VCLS(cs);
+
 	VCLI_Out(cli, "Closing CLI connection");
 	VCLI_SetResult(cli, CLIS_CLOSE);
 }
@@ -106,9 +119,13 @@ void v_matchproto_(cli_func_t)
 VCLS_func_ping(struct cli *cli, const char * const *av, void *priv)
 {
 	time_t t;
+	struct VCLS *cs;
 
 	(void)av;
 	(void)priv;
+	cs = cli->cls;
+	CHECK_VCLS(cs);
+
 	t = time(NULL);
 	VCLI_Out(cli, "PONG %jd 1.0", (intmax_t)t);
 }
@@ -116,8 +133,13 @@ VCLS_func_ping(struct cli *cli, const char * const *av, void *priv)
 void v_matchproto_(cli_func_t)
 VCLS_func_ping_json(struct cli *cli, const char * const *av, void *priv)
 {
+	struct VCLS *cs;
+
 	(void)av;
 	(void)priv;
+	cs = cli->cls;
+	CHECK_VCLS(cs);
+
 	VCLI_JSON_begin(cli, 2, av);
 	VCLI_Out(cli, ", \"PONG\"\n");
 	VCLI_JSON_end(cli);
@@ -144,7 +166,7 @@ VCLS_func_help(struct cli *cli, const char * const *av, void *priv)
 
 	(void)priv;
 	cs = cli->cls;
-	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);
+	CHECK_VCLS(cs);
 
 	for (av += 2; av[0] != NULL && av[0][0] == '-'; av++) {
 		if (!strcmp(av[0], "-a")) {
@@ -186,7 +208,7 @@ VCLS_func_help_json(struct cli *cli, const char * const *av, void *priv)
 
 	(void)priv;
 	cs = cli->cls;
-	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);
+	CHECK_VCLS(cs);
 
 	VCLI_JSON_begin(cli, 2, av);
 	VTAILQ_FOREACH(clp, &cs->funcs, list) {
@@ -514,6 +536,7 @@ VCLS_New(struct VCLS *model)
 	AN(cs);
 	VTAILQ_INIT(&cs->fds);
 	VTAILQ_INIT(&cs->funcs);
+	cs->thr = pthread_self();
 	if (model != NULL)
 		VTAILQ_CONCAT(&cs->funcs, &model->funcs, list);
 	return (cs);
@@ -522,7 +545,7 @@ VCLS_New(struct VCLS *model)
 void
 VCLS_SetLimit(struct VCLS *cs, volatile unsigned *limit)
 {
-	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);
+	CHECK_VCLS(cs);
 	cs->limit = limit;
 }
 
@@ -530,7 +553,7 @@ void
 VCLS_SetHooks(struct VCLS *cs, cls_cbc_f *before, cls_cbc_f *after)
 {
 
-	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);
+	CHECK_VCLS(cs);
 	cs->before = before;
 	cs->after = after;
 }
@@ -540,7 +563,7 @@ VCLS_AddFd(struct VCLS *cs, int fdi, int fdo, cls_cb_f *closefunc, void *priv)
 {
 	struct VCLS_fd *cfd;
 
-	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);
+	CHECK_VCLS(cs);
 	assert(fdi >= 0);
 	assert(fdo >= 0);
 	ALLOC_OBJ(cfd, VCLS_FD_MAGIC);
@@ -601,7 +624,7 @@ VCLS_AddFunc(struct VCLS *cs, struct cli_proto *clp)
 	struct cli_proto *clp2;
 	int i;
 
-	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);
+	CHECK_VCLS(cs);
 	AN(clp);
 
 	for (;clp->desc != NULL; clp++) {
@@ -634,7 +657,7 @@ VCLS_Poll(struct VCLS *cs, const struct cli *cli, int timeout)
 	int i, j, k;
 	char buf[BUFSIZ];
 
-	CHECK_OBJ_NOTNULL(cs, VCLS_MAGIC);
+	CHECK_VCLS(cs);
 	if (cs->nfd == 0) {
 		errno = 0;
 		return (-1);
@@ -682,6 +705,7 @@ VCLS_Destroy(struct VCLS **csp)
 	struct cli_proto *clp;
 
 	TAKE_OBJ_NOTNULL(cs, csp, VCLS_MAGIC);
+	ASSERT_CLI_THR(cs);
 	VTAILQ_FOREACH_SAFE(cfd, &cs->fds, list, cfd2)
 		(void)cls_close_fd(cs, cfd);
 

From 1825eb212fc60b973f0d5d4ce0d688834df2bec9 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Wed, 20 Sep 2023 11:43:09 +0200
Subject: [PATCH 12/15] Makefile.am: Make cli_cmds.h private

---
 include/Makefile.am | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/Makefile.am b/include/Makefile.am
index 4bac90f3335..ef6d873f5bc 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -12,7 +12,6 @@ nobase_pkginclude_HEADERS = \
 	tbl/beresp_flags.h \
 	tbl/boc_state.h \
 	tbl/body_status.h \
-	tbl/cli_cmds.h \
 	tbl/debug_bits.h \
 	tbl/experimental_bits.h \
 	tbl/feature_bits.h \
@@ -90,6 +89,7 @@ nobase_noinst_HEADERS = \
 	compat/daemon.h \
 	vfl.h \
 	libvcc.h \
+	tbl/cli_cmds.h \
 	tbl/cli_flags.h \
 	vcc_interface.h \
 	vcli_serve.h \

From 1bd61cf90b55dd6f2f404f2333236925cb677ea5 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Tue, 17 Oct 2023 12:36:44 +0200
Subject: [PATCH 13/15] cli: Use macro for flags check

---
 bin/varnishd/cache/cache_cli.c | 12 +++++-------
 bin/varnishd/mgt/mgt_cli.c     | 16 ++++++----------
 include/vcli_serve.h           |  4 ++++
 lib/libvarnish/vcli_serve.c    | 23 ++++++++++++++++-------
 4 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/bin/varnishd/cache/cache_cli.c b/bin/varnishd/cache/cache_cli.c
index 890d1c0e4e2..aea3e707ef1 100644
--- a/bin/varnishd/cache/cache_cli.c
+++ b/bin/varnishd/cache/cache_cli.c
@@ -74,8 +74,7 @@ cli_cb_before(const struct cli *cli, struct cli_proto *clp, const char * const *
 	(void)av;
 
 	if (clp != NULL &&
-	    !DO_DEBUG(DBG_CLI_SHOW_SENSITIVE) &&
-	    (clp->desc->flags & CLI_F_SENSITIVE)) {
+	    VCLS_IsSensitive(clp->desc, DO_DEBUG(DBG_CLI_SHOW_SENSITIVE))) {
 		VSB_clear(cli->cmd);
 		if (clp->logfunc != NULL)
 			clp->logfunc(cli, av, cli->cmd);
@@ -95,14 +94,13 @@ cli_cb_after(const struct cli *cli, struct cli_proto *clp, const char * const *a
 	(void)av;
 	const char *h = "(hidden)";
 
-	if (clp == NULL ||
-	    DO_DEBUG(DBG_CLI_SHOW_SENSITIVE) ||
-	    !(clp->desc->flags & CLI_F_SENSITIVE)) {
+	if (clp != NULL &&
+	    VCLS_IsSensitive(clp->desc, DO_DEBUG(DBG_CLI_SHOW_SENSITIVE))) {
 		VSL(SLT_CLI, NO_VXID, "Wr %03u %zd %s",
-		    cli->result, VSB_len(cli->sb), VSB_data(cli->sb));
+		    cli->result, strlen(h), h);
 	} else {
 		VSL(SLT_CLI, NO_VXID, "Wr %03u %zd %s",
-		    cli->result, strlen(h), h);
+		    cli->result, VSB_len(cli->sb), VSB_data(cli->sb));
 	}
 }
 
diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index 7d15a7a101a..687984707d5 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -160,7 +160,7 @@ mcf_askchild(struct cli *cli, const char * const *av, void *priv)
 	}
 
 	cmd = mgt_cmd_lookup(av[1]);
-	if (cmd != NULL && cmd->flags & CLI_F_INTERNAL) {
+	if (cmd != NULL && VCLS_CMD_IS(cmd, INTERNAL)) {
 		VCLI_Out(cli, "Unknown request.\nType 'help' for more info.\n");
 		VCLI_SetResult(cli, CLIS_UNKNOWN);
 		return;
@@ -371,9 +371,7 @@ mgt_cli_cb_before(const struct cli *cli, struct cli_proto *clp, const char * con
 	if (cli->priv == stderr)
 		fprintf(stderr, "> %s\n", VSB_data(cli->cmd));
 
-	if (cmd != NULL &&
-	    !MGT_DO_DEBUG(DBG_CLI_SHOW_SENSITIVE) &&
-	    (cmd->flags & CLI_F_SENSITIVE)) {
+	if (VCLS_IsSensitive(cmd, MGT_DO_DEBUG(DBG_CLI_SHOW_SENSITIVE))) {
 		d = (*VSB_data(cli->cmd) == '-');
 		VSB_clear(cli->cmd);
 		VSB_printf(cli->cmd, "%s", d ? "-" : "");
@@ -394,14 +392,12 @@ mgt_cli_cb_after(const struct cli *cli, struct cli_proto *clp, const char * cons
 
 	if (av) {
 		cmd = (clp == NULL ? mgt_cmd_lookup(av[1]) : clp->desc);
-		if (cmd == NULL ||
-		    MGT_DO_DEBUG(DBG_CLI_SHOW_SENSITIVE)  ||
-		    !(cmd->flags & CLI_F_SENSITIVE)) {
+		if (VCLS_IsSensitive(cmd, MGT_DO_DEBUG(DBG_CLI_SHOW_SENSITIVE))) {
 			MGT_Complain(C_CLI, "CLI %s Wr %03u %s",
-			    cli->ident, cli->result, VSB_data(cli->sb));
+			    cli->ident, cli->result, "(hidden)");
 		} else {
 			MGT_Complain(C_CLI, "CLI %s Wr %03u %s",
-			    cli->ident, cli->result, "(hidden)");
+			    cli->ident, cli->result, VSB_data(cli->sb));
 		}
 	}
 	if (cli->priv != stderr)
@@ -756,7 +752,7 @@ mgt_DumpRstCli(void)
 		cp = cmds[z];
 		if (!strncmp(cp->request, "debug.", 6))
 			continue;
-		if (cp->flags & CLI_F_INTERNAL)
+		if (VCLS_CMD_IS(cp, INTERNAL))
 			continue;
 		printf(".. _ref_cli_");
 		for (p = cp->request; *p; p++)
diff --git a/include/vcli_serve.h b/include/vcli_serve.h
index a90c61d5533..4ef49390fa1 100644
--- a/include/vcli_serve.h
+++ b/include/vcli_serve.h
@@ -47,6 +47,9 @@ enum cli_flags {
 #include "tbl/cli_flags.h"
 };
 
+#define VCLS_CMD_IS(cmd, FLAG) ((cmd)->flags & CLI_F_##FLAG)
+#define VCLS_PROTO_IS(clp, FLAG) VCLS_CMD_IS((clp)->desc, FLAG)
+
 struct cli_cmd_desc {
 	/* Must match CLI_CMD macro in include/tbl/cli_cmds.h */
 	const char			*request;
@@ -105,6 +108,7 @@ typedef void cls_cbc_f(const struct cli*, struct cli_proto *, const char * const
 struct VCLS *VCLS_New(struct VCLS *);
 void VCLS_SetHooks(struct VCLS *, cls_cbc_f *, cls_cbc_f *);
 void VCLS_SetLimit(struct VCLS *, volatile unsigned *);
+unsigned VCLS_IsSensitive(const struct cli_cmd_desc *, unsigned);
 struct cli *VCLS_AddFd(struct VCLS *cs, int fdi, int fdo, cls_cb_f *closefunc,
     void *priv);
 void VCLS_AddFunc(struct VCLS *cs, struct cli_proto *clp);
diff --git a/lib/libvarnish/vcli_serve.c b/lib/libvarnish/vcli_serve.c
index 356988cc31f..ab3b287bbfa 100644
--- a/lib/libvarnish/vcli_serve.c
+++ b/lib/libvarnish/vcli_serve.c
@@ -180,15 +180,15 @@ VCLS_func_help(struct cli *cli, const char * const *av, void *priv)
 		}
 	}
 	VTAILQ_FOREACH(clp, &cs->funcs, list) {
-		if ((clp->desc->flags & CLI_F_AUTH) && !cli->auth)
+		if (VCLS_PROTO_IS(clp, AUTH) && !cli->auth)
 			continue;
-		if (clp->desc->flags & CLI_F_INTERNAL)
+		if (VCLS_PROTO_IS(clp, INTERNAL))
 			continue;
 		if (av[0] != NULL && !strcmp(clp->desc->request, av[0])) {
 			help_helper(cli, clp, av);
 			return;
 		} else if (av[0] == NULL) {
-			d = clp->desc->flags & CLI_F_DEBUG ? 2 : 1;
+			d = VCLS_PROTO_IS(clp, DEBUG) ? 2 : 1;
 			if (filter & d)
 				help_helper(cli, clp, av);
 		}
@@ -213,9 +213,9 @@ VCLS_func_help_json(struct cli *cli, const char * const *av, void *priv)
 	VCLI_JSON_begin(cli, 2, av);
 	VTAILQ_FOREACH(clp, &cs->funcs, list) {
 		sep = "";
-		if ((clp->desc->flags & CLI_F_AUTH) && !cli->auth)
+		if (VCLS_PROTO_IS(clp, AUTH) && !cli->auth)
 			continue;
-		if (clp->desc->flags & CLI_F_INTERNAL)
+		if (VCLS_PROTO_IS(clp, INTERNAL))
 			continue;
 		VCLI_Out(cli, ",\n  {\n");
 		VSB_indent(cli->sb, 2);
@@ -325,7 +325,7 @@ cls_lookup(char * const *av, struct cli *cli, struct VCLS *cs, enum cmd_error_e
 
 
 	VTAILQ_FOREACH(clp, &cs->funcs, list) {
-		if ((clp->desc->flags & CLI_F_AUTH) && !cli->auth)
+		if (VCLS_PROTO_IS(clp, AUTH) && !cli->auth)
 			continue;
 		if (!strcmp(clp->desc->request, av[1]))
 			break;
@@ -389,7 +389,7 @@ cls_exec(struct VCLS_fd *cfd, char * const *av)
 			case (CMD_ERR_UNKNOWN):
 				break;
 			default:
-				if (cs->wildcard && (!(cs->wildcard->desc->flags & CLI_F_AUTH) || cli->auth))
+				if (cs->wildcard && (!VCLS_PROTO_IS(cs->wildcard, AUTH) || cli->auth))
 					cls_dispatch(cli, cs->wildcard, av, na);
 				break;
 		}
@@ -716,6 +716,15 @@ VCLS_Destroy(struct VCLS **csp)
 	FREE_OBJ(cs);
 }
 
+unsigned
+VCLS_IsSensitive(const struct cli_cmd_desc *cmd, unsigned show) {
+
+	if (cmd == NULL || show)
+		return 0;
+
+	return VCLS_CMD_IS(cmd, SENSITIVE);
+}
+
 /**********************************************************************
  * Utility functions for implementing CLI commands
  */

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 14/15] 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]",

From a8866de32cbfec57cde417b924798469ed72cae4 Mon Sep 17 00:00:00 2001
From: Walid Boudebouda <walid.boudebouda@gmail.com>
Date: Mon, 6 Nov 2023 11:14:10 +0100
Subject: [PATCH 15/15] mgt_cli: Don't pass unknown commands to the child

Since mgt is aware of all known cli commands, unknown commands should
be blocked by mgt and not forwarded to the child process to prevent
any malicious command smuggling (using quotes for example).
---
 bin/varnishd/mgt/mgt_cli.c       | 2 +-
 bin/varnishtest/tests/b00008.vtc | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/bin/varnishd/mgt/mgt_cli.c b/bin/varnishd/mgt/mgt_cli.c
index e6393f7f386..cc09ebdf73f 100644
--- a/bin/varnishd/mgt/mgt_cli.c
+++ b/bin/varnishd/mgt/mgt_cli.c
@@ -184,7 +184,7 @@ mcf_askchild(struct cli *cli, const char * const *av, void *priv)
 	}
 
 	cmd = mgt_cmd_lookup(av[1]);
-	if (cmd != NULL && VCLS_CMD_IS(cmd, INTERNAL)) {
+	if (cmd == NULL || VCLS_CMD_IS(cmd, INTERNAL)) {
 		VCLI_Out(cli, "Unknown request.\nType 'help' for more info.\n");
 		VCLI_SetResult(cli, CLIS_UNKNOWN);
 		return;
diff --git a/bin/varnishtest/tests/b00008.vtc b/bin/varnishtest/tests/b00008.vtc
index 49d54b37511..cd50d6ba2db 100644
--- a/bin/varnishtest/tests/b00008.vtc
+++ b/bin/varnishtest/tests/b00008.vtc
@@ -51,3 +51,5 @@ varnish v1 -cliok "param.set cli_limit 128"
 varnish v1 -clierr 201 "param.show"
 
 varnish v1 -cliok "\"help\" \"help\""
+
+varnish v1 -clierr 101 "\"vcl.use foo\""