Skip to content

Commit f479b51

Browse files
daeMOn63daeMOn63
authored
fix: file keyring fails to add/import/export keys when input is not stdin (fix #9566) (#9821)
## Description Add a test case to reproduce the issue described in #9566. The test currently fails, and I've pointed some possible solutions over #9566 (comment). But I feel this needs more works in order to provide a more robust solution. I'll keep poking at better options, but taking any pointers if anyone has ideas.
1 parent 1cc93d2 commit f479b51

File tree

7 files changed

+232
-80
lines changed

7 files changed

+232
-80
lines changed

‎client/context.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package client
22

33
import (
4+
"bufio"
45
"encoding/json"
56
"io"
67
"os"
@@ -68,7 +69,10 @@ func (ctx Context) WithKeyringOptions(opts ...keyring.Option) Context {
6869

6970
// WithInput returns a copy of the context with an updated input.
7071
func (ctx Context) WithInput(r io.Reader) Context {
71-
ctx.Input = r
72+
// convert to a bufio.Reader to have a shared buffer between the keyring and the
73+
// the Commands, ensuring a read from one advance the read pointer for the other.
74+
// see https://github.com/cosmos/cosmos-sdk/issues/9566.
75+
ctx.Input = bufio.NewReader(r)
7276
return ctx
7377
}
7478

‎client/keys/add.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ Example:
8282
}
8383

8484
func runAddCmdPrepare(cmd *cobra.Command, args []string) error {
85-
buf := bufio.NewReader(cmd.InOrStdin())
8685
clientCtx, err := client.GetClientQueryContext(cmd)
8786
if err != nil {
8887
return err
8988
}
9089

90+
buf := bufio.NewReader(clientCtx.Input)
9191
return runAddCmd(clientCtx, cmd, args, buf)
9292
}
9393

‎client/keys/add_test.go

+46-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/cosmos/cosmos-sdk/simapp"
1919
"github.com/cosmos/cosmos-sdk/testutil"
2020
sdk "github.com/cosmos/cosmos-sdk/types"
21+
bip39 "github.com/cosmos/go-bip39"
2122
)
2223

2324
func Test_runAddCmdBasic(t *testing.T) {
@@ -30,7 +31,7 @@ func Test_runAddCmdBasic(t *testing.T) {
3031
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
3132
require.NoError(t, err)
3233

33-
clientCtx := client.Context{}.WithKeyringDir(kbHome)
34+
clientCtx := client.Context{}.WithKeyringDir(kbHome).WithInput(mockIn)
3435
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
3536

3637
t.Cleanup(func() {
@@ -227,3 +228,47 @@ func Test_runAddCmdDryRun(t *testing.T) {
227228
})
228229
}
229230
}
231+
232+
func TestAddRecoverFileBackend(t *testing.T) {
233+
cmd := AddKeyCommand()
234+
cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
235+
236+
mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
237+
kbHome := t.TempDir()
238+
239+
clientCtx := client.Context{}.WithKeyringDir(kbHome).WithInput(mockIn)
240+
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
241+
242+
cmd.SetArgs([]string{
243+
"keyname1",
244+
fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
245+
fmt.Sprintf("--%s=%s", cli.OutputFlag, OutputFormatText),
246+
fmt.Sprintf("--%s=%s", flags.FlagKeyAlgorithm, string(hd.Secp256k1Type)),
247+
fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendFile),
248+
fmt.Sprintf("--%s", flagRecover),
249+
})
250+
251+
keyringPassword := "12345678"
252+
253+
entropySeed, err := bip39.NewEntropy(mnemonicEntropySize)
254+
require.NoError(t, err)
255+
256+
mnemonic, err := bip39.NewMnemonic(entropySeed)
257+
require.NoError(t, err)
258+
259+
mockIn.Reset(fmt.Sprintf("%s\n%s\n%s\n", mnemonic, keyringPassword, keyringPassword))
260+
require.NoError(t, cmd.ExecuteContext(ctx))
261+
262+
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendFile, kbHome, mockIn)
263+
require.NoError(t, err)
264+
265+
t.Cleanup(func() {
266+
mockIn.Reset(fmt.Sprintf("%s\n%s\n", keyringPassword, keyringPassword))
267+
_ = kb.Delete("keyname1")
268+
})
269+
270+
mockIn.Reset(fmt.Sprintf("%s\n%s\n", keyringPassword, keyringPassword))
271+
info, err := kb.Key("keyname1")
272+
require.NoError(t, err)
273+
require.Equal(t, "keyname1", info.GetName())
274+
}

‎client/keys/export.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@ FULLY AWARE OF THE RISKS. If you are unsure, you may want to do some research
3131
and export your keys in ASCII-armored encrypted format.`,
3232
Args: cobra.ExactArgs(1),
3333
RunE: func(cmd *cobra.Command, args []string) error {
34-
buf := bufio.NewReader(cmd.InOrStdin())
3534
clientCtx, err := client.GetClientQueryContext(cmd)
3635
if err != nil {
3736
return err
3837
}
38+
buf := bufio.NewReader(clientCtx.Input)
3939
unarmored, _ := cmd.Flags().GetBool(flagUnarmoredHex)
4040
unsafe, _ := cmd.Flags().GetBool(flagUnsafe)
4141

‎client/keys/export_test.go

+90-49
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package keys
22

33
import (
4+
"bufio"
45
"context"
56
"fmt"
67
"testing"
@@ -17,55 +18,95 @@ import (
1718
)
1819

1920
func Test_runExportCmd(t *testing.T) {
20-
cmd := ExportKeyCommand()
21-
cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
22-
mockIn := testutil.ApplyMockIODiscardOutErr(cmd)
23-
24-
// Now add a temporary keybase
25-
kbHome := t.TempDir()
26-
27-
// create a key
28-
kb, err := keyring.New(sdk.KeyringServiceName(), keyring.BackendTest, kbHome, mockIn)
29-
require.NoError(t, err)
30-
t.Cleanup(func() {
31-
kb.Delete("keyname1") // nolint:errcheck
32-
})
33-
34-
path := sdk.GetConfig().GetFullBIP44Path()
35-
_, err = kb.NewAccount("keyname1", testutil.TestMnemonic, "", path, hd.Secp256k1)
36-
require.NoError(t, err)
37-
38-
// Now enter password
39-
args := []string{
40-
"keyname1",
41-
fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
42-
fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, keyring.BackendTest),
21+
testCases := []struct {
22+
name string
23+
keyringBackend string
24+
extraArgs []string
25+
userInput string
26+
mustFail bool
27+
expectedOutput string
28+
}{
29+
{
30+
name: "--unsafe only must fail",
31+
keyringBackend: keyring.BackendTest,
32+
extraArgs: []string{"--unsafe"},
33+
mustFail: true,
34+
},
35+
{
36+
name: "--unarmored-hex must fail",
37+
keyringBackend: keyring.BackendTest,
38+
extraArgs: []string{"--unarmored-hex"},
39+
mustFail: true,
40+
},
41+
{
42+
name: "--unsafe --unarmored-hex fail with no user confirmation",
43+
keyringBackend: keyring.BackendTest,
44+
extraArgs: []string{"--unsafe", "--unarmored-hex"},
45+
userInput: "",
46+
mustFail: true,
47+
expectedOutput: "",
48+
},
49+
{
50+
name: "--unsafe --unarmored-hex succeed",
51+
keyringBackend: keyring.BackendTest,
52+
extraArgs: []string{"--unsafe", "--unarmored-hex"},
53+
userInput: "y\n",
54+
mustFail: false,
55+
expectedOutput: "2485e33678db4175dc0ecef2d6e1fc493d4a0d7f7ce83324b6ed70afe77f3485\n",
56+
},
57+
{
58+
name: "file keyring backend properly read password and user confirmation",
59+
keyringBackend: keyring.BackendFile,
60+
extraArgs: []string{"--unsafe", "--unarmored-hex"},
61+
// first 2 pass for creating the key, then unsafe export confirmation, then unlock keyring pass
62+
userInput: "12345678\n12345678\ny\n12345678\n",
63+
mustFail: false,
64+
expectedOutput: "2485e33678db4175dc0ecef2d6e1fc493d4a0d7f7ce83324b6ed70afe77f3485\n",
65+
},
4366
}
4467

45-
mockIn.Reset("123456789\n123456789\n")
46-
cmd.SetArgs(args)
47-
48-
clientCtx := client.Context{}.
49-
WithKeyringDir(kbHome).
50-
WithKeyring(kb)
51-
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
52-
53-
require.NoError(t, cmd.ExecuteContext(ctx))
54-
55-
argsUnsafeOnly := append(args, "--unsafe")
56-
cmd.SetArgs(argsUnsafeOnly)
57-
require.Error(t, cmd.ExecuteContext(ctx))
58-
59-
argsUnarmoredHexOnly := append(args, "--unarmored-hex")
60-
cmd.SetArgs(argsUnarmoredHexOnly)
61-
require.Error(t, cmd.ExecuteContext(ctx))
62-
63-
argsUnsafeUnarmoredHex := append(args, "--unsafe", "--unarmored-hex")
64-
cmd.SetArgs(argsUnsafeUnarmoredHex)
65-
require.Error(t, cmd.ExecuteContext(ctx))
66-
67-
mockIn, mockOut := testutil.ApplyMockIO(cmd)
68-
mockIn.Reset("y\n")
69-
require.NoError(t, cmd.ExecuteContext(ctx))
70-
require.Equal(t, "2485e33678db4175dc0ecef2d6e1fc493d4a0d7f7ce83324b6ed70afe77f3485\n", mockOut.String())
68+
for _, tc := range testCases {
69+
t.Run(tc.name, func(t *testing.T) {
70+
kbHome := t.TempDir()
71+
defaultArgs := []string{
72+
"keyname1",
73+
fmt.Sprintf("--%s=%s", flags.FlagHome, kbHome),
74+
fmt.Sprintf("--%s=%s", flags.FlagKeyringBackend, tc.keyringBackend),
75+
}
76+
77+
cmd := ExportKeyCommand()
78+
cmd.Flags().AddFlagSet(Commands("home").PersistentFlags())
79+
80+
cmd.SetArgs(append(defaultArgs, tc.extraArgs...))
81+
mockIn, mockOut := testutil.ApplyMockIO(cmd)
82+
83+
mockIn.Reset(tc.userInput)
84+
mockInBuf := bufio.NewReader(mockIn)
85+
86+
// create a key
87+
kb, err := keyring.New(sdk.KeyringServiceName(), tc.keyringBackend, kbHome, bufio.NewReader(mockInBuf))
88+
require.NoError(t, err)
89+
t.Cleanup(func() {
90+
kb.Delete("keyname1") // nolint:errcheck
91+
})
92+
93+
path := sdk.GetConfig().GetFullBIP44Path()
94+
_, err = kb.NewAccount("keyname1", testutil.TestMnemonic, "", path, hd.Secp256k1)
95+
require.NoError(t, err)
96+
97+
clientCtx := client.Context{}.
98+
WithKeyringDir(kbHome).
99+
WithKeyring(kb).
100+
WithInput(mockInBuf)
101+
ctx := context.WithValue(context.Background(), client.ClientContextKey, &clientCtx)
102+
103+
err = cmd.ExecuteContext(ctx)
104+
if tc.mustFail {
105+
require.Error(t, err)
106+
} else {
107+
require.NoError(t, err)
108+
require.Equal(t, tc.expectedOutput, mockOut.String())
109+
}
110+
})
111+
}
71112
}

‎client/keys/import.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ func ImportKeyCommand() *cobra.Command {
1818
Long: "Import a ASCII armored private key into the local keybase.",
1919
Args: cobra.ExactArgs(2),
2020
RunE: func(cmd *cobra.Command, args []string) error {
21-
buf := bufio.NewReader(cmd.InOrStdin())
2221
clientCtx, err := client.GetClientQueryContext(cmd)
2322
if err != nil {
2423
return err
2524
}
25+
buf := bufio.NewReader(clientCtx.Input)
2626

2727
bz, err := ioutil.ReadFile(args[1])
2828
if err != nil {

0 commit comments

Comments
 (0)