Fix incorrect cli default values and default command (#34765)

Author: wxiaoguang

cmd/admin_auth_smtp.go

@@ -39,12 +39,10 @@ func smtpCLIFlags() []cli.Flag {
 		&cli.BoolFlag{
 			Name:  "force-smtps",
 			Usage: "SMTPS is always used on port 465. Set this to force SMTPS on other ports.",
-			Value: true,
 		},
 		&cli.BoolFlag{
 			Name:  "skip-verify",
 			Usage: "Skip TLS verify.",
-			Value: true,
 		},
 		&cli.StringFlag{
 			Name:  "helo-hostname",
@@ -54,7 +52,6 @@ func smtpCLIFlags() []cli.Flag {
 		&cli.BoolFlag{
 			Name:  "disable-helo",
 			Usage: "Disable SMTP helo.",
-			Value: true,
 		},
 		&cli.StringFlag{
 			Name:  "allowed-domains",
@@ -64,7 +61,6 @@ func smtpCLIFlags() []cli.Flag {
 		&cli.BoolFlag{
 			Name:  "skip-local-2fa",
 			Usage: "Skip 2FA to log on.",
-			Value: true,
 		},
 		&cli.BoolFlag{
 			Name:  "active",

cmd/admin_auth_smtp_test.go

@@ -60,10 +60,8 @@ func TestAddSMTP(t *testing.T) {
 					Auth: "PLAIN",
 					Host: "localhost",
 					Port: 25,
-					// ForceSMTPS: true,
-					// SkipVerify: true,
 				},
-				TwoFactorPolicy: "skip",
+				TwoFactorPolicy: "",
 			},
 		},
 		{
@@ -73,12 +71,12 @@ func TestAddSMTP(t *testing.T) {
 				"--host", "localhost",
 				"--port", "25",
 				"--auth-type", "LOGIN",
-				"--force-smtps=false",
-				"--skip-verify=false",
+				"--force-smtps",
+				"--skip-verify",
 				"--helo-hostname", "example.com",
-				"--disable-helo=false",
+				"--disable-helo=true",
 				"--allowed-domains", "example.com,example.org",
-				"--skip-local-2fa=false",
+				"--skip-local-2fa",
 				"--active=false",
 			},
 			source: &auth_model.Source{
@@ -89,13 +87,13 @@ func TestAddSMTP(t *testing.T) {
 					Auth:           "LOGIN",
 					Host:           "localhost",
 					Port:           25,
-					ForceSMTPS:     false,
-					SkipVerify:     false,
+					ForceSMTPS:     true,
+					SkipVerify:     true,
 					HeloHostname:   "example.com",
-					DisableHelo:    false,
+					DisableHelo:    true,
 					AllowedDomains: "example.com,example.org",
 				},
-				TwoFactorPolicy: "",
+				TwoFactorPolicy: "skip",
 			},
 		},
 	}
@@ -157,13 +155,10 @@ func TestUpdateSMTP(t *testing.T) {
 				Name:     "old name",
 				IsActive: true,
 				Cfg: &smtp.Source{
-					Auth:       "PLAIN",
-					Host:       "old host",
-					Port:       26,
-					ForceSMTPS: true,
-					SkipVerify: true,
+					Auth: "PLAIN",
+					Host: "old host",
+					Port: 26,
 				},
-				TwoFactorPolicy: "",
 			},
 			args: []string{
 				"--id", "1",
@@ -177,13 +172,10 @@ func TestUpdateSMTP(t *testing.T) {
 				Name:     "test",
 				IsActive: true,
 				Cfg: &smtp.Source{
-					Auth:       "PLAIN",
-					Host:       "localhost",
-					Port:       25,
-					ForceSMTPS: true,
-					SkipVerify: true,
+					Auth: "PLAIN",
+					Host: "localhost",
+					Port: 25,
 				},
-				TwoFactorPolicy: "skip",
 			},
 		},
 		{
@@ -197,10 +189,7 @@ func TestUpdateSMTP(t *testing.T) {
 					Auth:           "PLAIN",
 					Host:           "old host",
 					Port:           26,
-					ForceSMTPS:     true,
-					SkipVerify:     true,
 					HeloHostname:   "old.example.com",
-					DisableHelo:    false,
 					AllowedDomains: "old.example.com",
 				},
 				TwoFactorPolicy: "",
@@ -211,12 +200,12 @@ func TestUpdateSMTP(t *testing.T) {
 				"--host", "localhost",
 				"--port", "25",
 				"--auth-type", "LOGIN",
-				"--force-smtps=false",
-				"--skip-verify=false",
+				"--force-smtps",
+				"--skip-verify",
 				"--helo-hostname", "example.com",
-				"--disable-helo=true",
+				"--disable-helo",
 				"--allowed-domains", "example.com,example.org",
-				"--skip-local-2fa=true",
+				"--skip-local-2fa",
 				"--active=false",
 			},
 			authSource: &auth_model.Source{
@@ -228,8 +217,8 @@ func TestUpdateSMTP(t *testing.T) {
 					Auth:           "LOGIN",
 					Host:           "localhost",
 					Port:           25,
-					ForceSMTPS:     false,
-					SkipVerify:     false,
+					ForceSMTPS:     true,
+					SkipVerify:     true,
 					HeloHostname:   "example.com",
 					DisableHelo:    true,
 					AllowedDomains: "example.com,example.org",
@@ -252,11 +241,8 @@ func TestUpdateSMTP(t *testing.T) {
 						Name:     "test",
 						IsActive: true,
 						Cfg: &smtp.Source{
-							Auth:       "PLAIN",
-							SkipVerify: true,
-							ForceSMTPS: true,
+							Auth: "PLAIN",
 						},
-						TwoFactorPolicy: "skip",
 					}, nil
 				},
 

cmd/cmd.go

@@ -132,3 +132,13 @@ func PrepareConsoleLoggerLevel(defaultLevel log.Level) func(context.Context, *cl
 		return ctx, nil
 	}
 }
+
+func isValidDefaultSubCommand(cmd *cli.Command) (string, bool) {
+	// Dirty patch for urfave/cli's strange design.
+	// "./gitea bad-cmd" should not start the web server.
+	rootArgs := cmd.Root().Args().Slice()
+	if len(rootArgs) != 0 && rootArgs[0] != cmd.Name {
+		return rootArgs[0], false
+	}
+	return "", true
+}

cmd/cmd_test.go

@@ -0,0 +1,38 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package cmd
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/urfave/cli/v3"
+)
+
+func TestDefaultCommand(t *testing.T) {
+	test := func(t *testing.T, args []string, expectedRetName string, expectedRetValid bool) {
+		called := false
+		cmd := &cli.Command{
+			DefaultCommand: "test",
+			Commands: []*cli.Command{
+				{
+					Name: "test",
+					Action: func(ctx context.Context, command *cli.Command) error {
+						retName, retValid := isValidDefaultSubCommand(command)
+						assert.Equal(t, expectedRetName, retName)
+						assert.Equal(t, expectedRetValid, retValid)
+						called = true
+						return nil
+					},
+				},
+			},
+		}
+		assert.NoError(t, cmd.Run(t.Context(), args))
+		assert.True(t, called)
+	}
+	test(t, []string{"./gitea"}, "", true)
+	test(t, []string{"./gitea", "test"}, "", true)
+	test(t, []string{"./gitea", "other"}, "other", false)
+}

cmd/main.go

@@ -152,6 +152,8 @@ func NewMainApp(appVer AppVersion) *cli.Command {
 		CmdDocs,
 	}
 
+	// TODO: we should eventually drop the default command,
+	// but not sure whether it would break Windows users who used to double-click the EXE to run.
 	app.DefaultCommand = CmdWeb.Name
 
 	app.Flags = append(app.Flags, cli.VersionFlag)

cmd/manager_logging.go

@@ -119,7 +119,6 @@ var (
 								Name:    "rotate",
 								Aliases: []string{"r"},
 								Usage:   "Rotate logs",
-								Value:   true,
 							},
 							&cli.Int64Flag{
 								Name:    "max-size",
@@ -130,7 +129,6 @@ var (
 								Name:    "daily",
 								Aliases: []string{"d"},
 								Usage:   "Rotate logs daily",
-								Value:   true,
 							},
 							&cli.IntFlag{
 								Name:    "max-days",
@@ -141,7 +139,6 @@ var (
 								Name:    "compress",
 								Aliases: []string{"z"},
 								Usage:   "Compress rotated logs",
-								Value:   true,
 							},
 							&cli.IntFlag{
 								Name:    "compression-level",

cmd/web.go

@@ -251,6 +251,10 @@ func runWeb(_ context.Context, cmd *cli.Command) error {
 		}
 	}()
 
+	if subCmdName, valid := isValidDefaultSubCommand(cmd); !valid {
+		return fmt.Errorf("unknown command: %s", subCmdName)
+	}
+
 	managerCtx, cancel := context.WithCancel(context.Background())
 	graceful.InitManager(managerCtx)
 	defer cancel()