Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Jan 8, 2024
Merged
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("name is empty after trimming the white space")
levisyin marked this conversation as resolved.
Show resolved Hide resolved
}
levisyin marked this conversation as resolved.
Show resolved Hide resolved
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), "name is empty after trimming the white space")

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("name is empty after trimming the white space")
levisyin marked this conversation as resolved.
Show resolved Hide resolved
}
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("name is empty after trimming the white space")
}
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_runImportCmdWithEmptyName(t *testing.T) {
cmd := ImportKeyCommand()
cmd.Flags().AddFlagSet(Commands().PersistentFlags())
mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
// Now add a temporary keybase
kbHome := filepath.Join(t.TempDir())

Check failure on line 186 in client/keys/import_test.go

View workflow job for this annotation

GitHub Actions / golangci-lint

badCall: suspicious Join on 1 argument (gocritic)

Check failure on line 186 in client/keys/import_test.go

View workflow job for this annotation

GitHub Actions / Analyze

badCall: suspicious Join on 1 argument (gocritic)
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), "name is empty after trimming the white space")

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), "name is empty after trimming the white space")
}
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("new name is empty after trimming the white space")
levisyin marked this conversation as resolved.
Show resolved Hide resolved
}

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), "new name is empty after trimming the white space")

// 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
Loading