refactor(client): check name validation for keys add|import|rename (#18950)

Author: levisyin

CHANGELOG.md

@@ -55,6 +55,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
+* (client/keys) [#18950](https://github.com/cosmos/cosmos-sdk/pull/18950) Improve `<appd> keys add`, `<appd> keys import` and `<appd> keys rename` by checking name validation.
 * (baseapp) [#18915](https://github.com/cosmos/cosmos-sdk/pull/18915) Add a new `ExecModeVerifyVoteExtension` exec mode and ensure it's populated in the `Context` during `VerifyVoteExtension` execution.
 * (types) [#18888](https://github.com/cosmos/cosmos-sdk/pull/18888) Speedup DecCoin.Sort() if len(coins) <= 1
 * (types) [#18875](https://github.com/cosmos/cosmos-sdk/pull/18875) Speedup coins.Sort() if len(coins) <= 1

client/keys/add.go

@@ -8,6 +8,7 @@ import (
 	"errors"
 	"fmt"
 	"sort"
+	"strings"
 
 	"github.com/cosmos/go-bip39"
 	"github.com/spf13/cobra"
@@ -123,6 +124,9 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf
 	var err error
 
 	name := args[0]
+	if strings.TrimSpace(name) == "" {
+		return errors.New("the provided name is invalid or empty after trimming whitespace")
+	}
 	interactive, _ := cmd.Flags().GetBool(flagInteractive)
 	kb := ctx.Keyring
 	outputFormat := ctx.OutputFormat

client/keys/add_test.go

@@ -47,6 +47,17 @@ func Test_runAddCmdBasic(t *testing.T) {
 		_ = kb.Delete("keyname2")
 	})
 
+	// test empty name
+	cmd.SetArgs([]string{
+		"",
+		fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome),
+		fmt.Sprintf("--%s=%s", flags.FlagOutput, flags.OutputFormatText),
+		fmt.Sprintf("--%s=%s", flags.FlagKeyType, hd.Secp256k1Type),
+		fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
+	})
+	mockIn.Reset("y\n")
+	require.ErrorContains(t, cmd.ExecuteContext(ctx), "the provided name is invalid or empty after trimming whitespace")
+
 	cmd.SetArgs([]string{
 		"keyname1",
 		fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome),

client/keys/import.go

@@ -2,8 +2,10 @@ package keys
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
 	"os"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -26,6 +28,10 @@ func ImportKeyCommand() *cobra.Command {
 			if err != nil {
 				return err
 			}
+			name := args[0]
+			if strings.TrimSpace(name) == "" {
+				return errors.New("the provided name is invalid or empty after trimming whitespace")
+			}
 			buf := bufio.NewReader(clientCtx.Input)
 
 			bz, err := os.ReadFile(args[1])
@@ -54,6 +60,10 @@ func ImportKeyHexCommand() *cobra.Command {
 			if err != nil {
 				return err
 			}
+			name := args[0]
+			if strings.TrimSpace(name) == "" {
+				return errors.New("the provided name is invalid or empty after trimming whitespace")
+			}
 			keyType, _ := cmd.Flags().GetString(flags.FlagKeyType)
 			return clientCtx.Keyring.ImportPrivKeyHex(args[0], args[1], keyType)
 		},

client/keys/import_test.go

@@ -177,3 +177,37 @@ func Test_runImportHexCmd(t *testing.T) {
 		})
 	}
 }
+
+func Test_runImportCmdWithEmptyName(t *testing.T) {
+	cmd := ImportKeyCommand()
+	cmd.Flags().AddFlagSet(Commands().PersistentFlags())
+	mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
+	// Now add a temporary keybase
+	kbHome := t.TempDir()
+	cdc := moduletestutil.MakeTestEncodingConfig().Codec
+	kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn, cdc)
+	require.NoError(t, err)
+
+	clientCtx := client.Context{}.
+		WithKeyringDir(kbHome).
+		WithKeyring(kb).
+		WithInput(mockIn).
+		WithCodec(cdc)
+	ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
+	cmd.SetArgs([]string{
+		"", "fake-file",
+		fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
+	})
+
+	require.ErrorContains(t, cmd.ExecuteContext(ctx), "the provided name is invalid or empty after trimming whitespace")
+
+	cmd = ImportKeyHexCommand()
+	cmd.Flags().AddFlagSet(Commands().PersistentFlags())
+	testutil.ApplyMockIODiscardOutErr(cmd)
+	cmd.SetArgs([]string{
+		"", "fake-hex",
+		fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
+	})
+
+	require.ErrorContains(t, cmd.ExecuteContext(ctx), "the provided name is invalid or empty after trimming whitespace")
+}

client/keys/rename.go

@@ -2,7 +2,9 @@ package keys
 
 import (
 	"bufio"
+	"errors"
 	"fmt"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -31,6 +33,9 @@ private keys stored in a ledger device cannot be renamed with the CLI.
 			}
 
 			oldName, newName := args[0], args[1]
+			if strings.TrimSpace(newName) == "" {
+				return errors.New("the new name cannot be empty or consist solely of whitespace")
+			}
 
 			k, err := clientCtx.Keyring.Key(oldName)
 			if err != nil {

client/keys/rename_test.go

@@ -27,6 +27,7 @@ func Test_runRenameCmd(t *testing.T) {
 	yesF, _ := cmd.Flags().GetBool(flagYes)
 	require.False(t, yesF)
 
+	invalidName := ""
 	fakeKeyName1 := "runRenameCmd_Key1"
 	fakeKeyName2 := "runRenameCmd_Key2"
 
@@ -46,6 +47,9 @@ func Test_runRenameCmd(t *testing.T) {
 
 	ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
 
+	cmd.SetArgs([]string{fakeKeyName1, invalidName, fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome)})
+	require.ErrorContains(t, cmd.ExecuteContext(ctx), "the new name cannot be empty or consist solely of whitespace")
+
 	// rename a key 'blah' which doesnt exist
 	cmd.SetArgs([]string{"blah", "blaah", fmt.Sprintf("--%s=%s", flags.FlagKeyringDir, kbHome)})
 	err = cmd.ExecuteContext(ctx)