Skip to content

Commit

Permalink
refactor(client): check name validation for keys add|import|rename (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
levisyin authored Jan 8, 2024
1 parent 41c84d6 commit c307001
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"sort"
"strings"

"github.com/cosmos/go-bip39"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions client/keys/add_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
10 changes: 10 additions & 0 deletions client/keys/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package keys

import (
"bufio"
"errors"
"fmt"
"os"
"strings"

"github.com/spf13/cobra"

Expand All @@ -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])
Expand Down Expand Up @@ -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)
},
Expand Down
34 changes: 34 additions & 0 deletions client/keys/import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
5 changes: 5 additions & 0 deletions client/keys/rename.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package keys

import (
"bufio"
"errors"
"fmt"
"strings"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions client/keys/rename_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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)
Expand Down

0 comments on commit c307001

Please sign in to comment.