From c307001349d9171d45855aeedd8c4ff877cf7cba Mon Sep 17 00:00:00 2001 From: levisyin <150114626+levisyin@users.noreply.github.com> Date: Mon, 8 Jan 2024 20:42:24 +0800 Subject: [PATCH] refactor(client): check name validation for `keys add|import|rename` (#18950) --- CHANGELOG.md | 1 + client/keys/add.go | 4 ++++ 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, 69 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d41323763c..f014579d92 100644 --- a/CHANGELOG.md +++ b/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 ` keys add`, ` keys import` and ` 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 diff --git a/client/keys/add.go b/client/keys/add.go index 13dd35ec47..4e6e4f27f7 100644 --- a/client/keys/add.go +++ b/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 diff --git a/client/keys/add_test.go b/client/keys/add_test.go index 6dff014b22..95cf334781 100644 --- a/client/keys/add_test.go +++ b/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), diff --git a/client/keys/import.go b/client/keys/import.go index 98ccb6547f..9a86da2879 100644 --- a/client/keys/import.go +++ b/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) }, diff --git a/client/keys/import_test.go b/client/keys/import_test.go index 0cff5065d8..006f8ce672 100644 --- a/client/keys/import_test.go +++ b/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") +} 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 dad91168f0..62253d382b 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)