From 193a7390c4443b0100dfd7bda3be475bb3c99e9a Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 27 Mar 2025 12:40:47 -0400 Subject: [PATCH] refactor(client): check name validation for `keys add|import|rename` (backport #18950) (#24152) Co-authored-by: levisyin <150114626+levisyin@users.noreply.github.com> Co-authored-by: aljo242 --- CHANGELOG.md | 1 + client/keys/add.go | 12 ++++++++++++ client/keys/add_test.go | 11 +++++++++++ client/keys/import.go | 10 ++++------ client/keys/import_test.go | 34 ++++++++++++++++++++++++++++++++++ client/keys/rename.go | 5 +++++ client/keys/rename_test.go | 4 ++++ 7 files changed, 71 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01bc95b764..a7c257942c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements +* (client/keys) [#18950](https://github.com/cosmos/cosmos-sdk/pull/18950) Improve ` keys add`, ` keys import` and ` keys rename` by checking name validation. * (client/keys) [#18703](https://github.com/cosmos/cosmos-sdk/pull/18703) Improve ` keys add` and ` keys show` by checking whether there are duplicate keys in the multisig case. * (client/keys) [#18745](https://github.com/cosmos/cosmos-sdk/pull/18745) Improve ` keys export` and ` keys mnemonic` by adding --yes option to skip interactive confirmation. * (x/bank) [#24106](https://github.com/cosmos/cosmos-sdk/pull/24106) `SendCoins` now checks for `SendRestrictions` before instead of after deducting coins using `subUnlockedCoins`. diff --git a/client/keys/add.go b/client/keys/add.go index 81e2071716..01d169cb06 100644 --- a/client/keys/add.go +++ b/client/keys/add.go @@ -10,6 +10,7 @@ import ( "io" "os" "sort" + "strings" "github.com/cosmos/go-bip39" "github.com/spf13/cobra" @@ -115,6 +116,14 @@ func runAddCmdPrepare(cmd *cobra.Command, args []string) error { return runAddCmd(clientCtx, cmd, args, buf) } +func checkName(name string) error { + if strings.TrimSpace(name) == "" { + return errors.New("the provided name is invalid or empty after trimming whitespace") + } + + return nil +} + /* input - bip39 mnemonic @@ -129,6 +138,9 @@ func runAddCmd(ctx client.Context, cmd *cobra.Command, args []string, inBuf *buf var err error name := args[0] + if err = checkName(name); err != nil { + return err + } interactive, _ := cmd.Flags().GetBool(flagInteractive) noBackup, _ := cmd.Flags().GetBool(flagNoBackup) showMnemonic := !noBackup diff --git a/client/keys/add_test.go b/client/keys/add_test.go index 78148f45bf..f9da7b30d6 100644 --- a/client/keys/add_test.go +++ b/client/keys/add_test.go @@ -39,6 +39,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), diff --git a/client/keys/import.go b/client/keys/import.go index f178b1bf5a..b30b3f80d5 100644 --- a/client/keys/import.go +++ b/client/keys/import.go @@ -2,10 +2,8 @@ package keys import ( "bufio" - "errors" "fmt" "os" - "strings" "github.com/spf13/cobra" @@ -29,8 +27,8 @@ func ImportKeyCommand() *cobra.Command { return err } name := args[0] - if strings.TrimSpace(name) == "" { - return errors.New("the provided name is invalid or empty after trimming whitespace") + if err := checkName(name); err != nil { + return err } buf := bufio.NewReader(clientCtx.Input) @@ -61,8 +59,8 @@ func ImportKeyHexCommand() *cobra.Command { return err } name := args[0] - if strings.TrimSpace(name) == "" { - return errors.New("the provided name is invalid or empty after trimming whitespace") + if err := checkName(name); err != nil { + return err } keyType, _ := cmd.Flags().GetString(flags.FlagKeyType) diff --git a/client/keys/import_test.go b/client/keys/import_test.go index 78daf6e41f..ced9e478e6 100644 --- a/client/keys/import_test.go +++ b/client/keys/import_test.go @@ -189,3 +189,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") +} diff --git a/client/keys/rename.go b/client/keys/rename.go index f703c60f20..f37d2307e1 100644 --- a/client/keys/rename.go +++ b/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 { diff --git a/client/keys/rename_test.go b/client/keys/rename_test.go index 8df5d641ba..132210c1f1 100644 --- a/client/keys/rename_test.go +++ b/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)