From f85a6f9ffadfc001507d7c7e5aa738ea7d58fc00 Mon Sep 17 00:00:00 2001 From: csuzhangxc Date: Thu, 5 Nov 2026 17:19:18 +0800 Subject: [PATCH] fix: reject invalid monitoring CLI arguments --- .github/workflows/build-security.yaml | 2 +- cmd/cli_test.go | 49 +++++++++++++++++++++++++++ cmd/monitoring.go | 10 +++--- reload/main.go | 10 +++--- reload/main_test.go | 48 ++++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 11 deletions(-) create mode 100644 cmd/cli_test.go create mode 100644 reload/main_test.go diff --git a/.github/workflows/build-security.yaml b/.github/workflows/build-security.yaml index c6d671fe..270213c7 100644 --- a/.github/workflows/build-security.yaml +++ b/.github/workflows/build-security.yaml @@ -30,7 +30,7 @@ jobs: with: go-version-file: go.mod - run: go test ./pkg/common - - run: go test ./cmd/monitoring.go ./cmd/dm_rules_test.go ./cmd/persisted_rules_test.go + - run: go test ./cmd/monitoring.go ./cmd/cli_test.go ./cmd/dm_rules_test.go ./cmd/persisted_rules_test.go - run: python3 -m unittest discover -s scripts -p 'test_build_security.py' -v reloader: diff --git a/cmd/cli_test.go b/cmd/cli_test.go new file mode 100644 index 00000000..d4da4a7a --- /dev/null +++ b/cmd/cli_test.go @@ -0,0 +1,49 @@ +package main + +import ( + "context" + "flag" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestCLI(t *testing.T) { + if os.Getenv("MONITORING_CLI_TEST") == "1" { + os.Args = append([]string{"load"}, flag.Args()...) + main() + return + } + missing := filepath.Join(t.TempDir(), "missing.yaml") + for _, tc := range []struct { + name string + args []string + want string + ok bool + }{ + {"unknown flag", []string{"--config=" + missing, "--auto-puhs"}, "unknown flag: --auto-puhs", false}, + {"unknown flag before required flag", []string{"--unknown", "--config=" + missing}, "unknown flag: --unknown", false}, + {"missing required flag", nil, `required flag(s) "config" not set`, false}, + {"missing value", []string{"--config"}, "flag needs an argument", false}, + {"invalid boolean", []string{"--config=" + missing, "--auto-push=maybe"}, "invalid argument", false}, + {"positional argument", []string{"--config=" + missing, "unexpected"}, "unknown command", false}, + {"positional after separator", []string{"--config=" + missing, "--", "unexpected"}, "unknown command", false}, + {"help", []string{"--help"}, "Usage:", true}, + {"build script flags", []string{"--config=monitoring.yaml", "--tag=v8.5.7", "--token=", "--help"}, "Usage:", true}, + {"all supported flags", []string{"--config", "monitoring.yaml", "--tag", "v8.5.7", "--token=local-test", "--output-dir", missing, "--auto-push=false", "--help"}, "Usage:", true}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, os.Args[0], append([]string{"-test.run=^TestCLI$", "--"}, tc.args...)...) + cmd.Env = append(os.Environ(), "MONITORING_CLI_TEST=1") + out, err := cmd.CombinedOutput() + if ctx.Err() != nil || (err == nil) != tc.ok || !strings.Contains(string(out), tc.want) { + t.Fatalf("exit error: %v; timeout: %v; want success=%t and %q; output:\n%s", err, ctx.Err(), tc.ok, tc.want, out) + } + }) + } +} diff --git a/cmd/monitoring.go b/cmd/monitoring.go index 9e2f3e4e..dc7a69ec 100644 --- a/cmd/monitoring.go +++ b/cmd/monitoring.go @@ -73,7 +73,8 @@ var ( func main() { var rootCmd = &cobra.Command{ - Use: "load", + Use: "load", + Args: cobra.NoArgs, Run: func(co *cobra.Command, args []string) { defer func() { if err := recover(); err != nil { @@ -86,9 +87,6 @@ func main() { common.CheckErr(stepUp(), "init env failed") common.CheckErr(Start(), "generate monitoring configuration failed") }, - FParseErrWhitelist: cobra.FParseErrWhitelist{ - UnknownFlags: true, - }, } rootCmd.Flags().StringVar(&configFile, "config", "", "the monitoring configuration file.") @@ -98,7 +96,9 @@ func main() { rootCmd.Flags().BoolVar(&autoPush, "auto-push", false, "auto generate new branch from master and push auto-generate files to the branch") rootCmd.MarkFlagRequired("config") - rootCmd.Execute() + if err := rootCmd.Execute(); err != nil { + os.Exit(1) + } } func stepUp() error { diff --git a/reload/main.go b/reload/main.go index ed6f881e..2ffb97f0 100644 --- a/reload/main.go +++ b/reload/main.go @@ -36,7 +36,8 @@ var ( func main() { var rootCmd = &cobra.Command{ - Use: "reload", + Use: "reload", + Args: cobra.NoArgs, Run: func(co *cobra.Command, args []string) { if subStorePath != "" && (extract(subStorePath) == "" || extract(subStorePath) == "." || extract(subStorePath) == "..") { log.Fatal("invalid persistent rules version") @@ -46,9 +47,6 @@ func main() { } startServer() }, - FParseErrWhitelist: cobra.FParseErrWhitelist{ - UnknownFlags: true, - }, } rootCmd.Flags().StringVar(&listenAddress, "web.listen-address", "127.0.0.1:9089", "HTTP listen address; remote listeners require authentication and TLS.") @@ -62,7 +60,9 @@ func main() { rootCmd.Flags().StringVar(&promUrl, "prometheus-url", "http://localhost:9090", "the url to send a request to when the files are updated.") rootCmd.MarkFlagRequired("watch-path") - rootCmd.Execute() + if err := rootCmd.Execute(); err != nil { + os.Exit(1) + } } func startServer() { diff --git a/reload/main_test.go b/reload/main_test.go new file mode 100644 index 00000000..721627ac --- /dev/null +++ b/reload/main_test.go @@ -0,0 +1,48 @@ +package main + +import ( + "context" + "flag" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +func TestCLI(t *testing.T) { + if os.Getenv("RELOADER_CLI_TEST") == "1" { + os.Args = append([]string{"reload"}, flag.Args()...) + main() + return + } + missing := filepath.Join(t.TempDir(), "missing") + for _, tc := range []struct { + name string + args []string + want string + ok bool + }{ + {"unknown security flag", []string{"--watch-path=" + missing, "--web.auth-fiel=auth.json"}, "unknown flag: --web.auth-fiel", false}, + {"unknown flag before required flag", []string{"--unknown", "--watch-path=" + missing}, "unknown flag: --unknown", false}, + {"missing required flag", nil, `required flag(s) "watch-path" not set`, false}, + {"missing value", []string{"--watch-path"}, "flag needs an argument", false}, + {"invalid boolean", []string{"--watch-path=" + missing, "--web.allow-insecure=maybe"}, "invalid argument", false}, + {"positional argument", []string{"--watch-path=" + missing, "unexpected"}, "unknown command", false}, + {"positional after separator", []string{"--watch-path=" + missing, "--", "unexpected"}, "unknown command", false}, + {"help", []string{"--help"}, "Usage:", true}, + {"operator and security flags", []string{"--watch-path=/prometheus-rules/rules", "--root-store-path=/data", "--sub-store-path=tidb:v8.5.7", "--prometheus-url=http://127.0.0.1:9090", "--web.listen-address=0.0.0.0:9089", "--web.auth-file=/web/auth.json", "--web.tls-cert-file=/web/tls.crt", "--web.tls-key-file=/web/tls.key", "--web.allow-insecure=false", "--help"}, "Usage:", true}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, os.Args[0], append([]string{"-test.run=^TestCLI$", "--"}, tc.args...)...) + cmd.Env = append(os.Environ(), "RELOADER_CLI_TEST=1") + out, err := cmd.CombinedOutput() + if ctx.Err() != nil || (err == nil) != tc.ok || !strings.Contains(string(out), tc.want) { + t.Fatalf("exit error: %v; timeout: %v; want success=%t and %q; output:\n%s", err, ctx.Err(), tc.ok, tc.want, out) + } + }) + } +}