From 61f03db68252536714f2cc0f7de5271b73d8071e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 6 Feb 2026 17:56:16 +0100 Subject: [PATCH] cli/command/registry: refactor reading from stdin Extract the code as a utility function, and add some GoDoc to describe the behavior. Signed-off-by: Sebastiaan van Stijn --- cli/command/registry/login.go | 43 ++++++++++++++++++++++++++---- cli/command/registry/login_test.go | 39 ++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index 913c000c4cc2..e7f590976580 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -1,6 +1,7 @@ package registry import ( + "bytes" "context" "errors" "fmt" @@ -88,6 +89,38 @@ func verifyLoginFlags(flags *pflag.FlagSet, opts loginOptions) error { return nil } +// readSecretFromStdin reads the secret from r and returns it as a string. +// It trims terminal line-endings (LF, CRLF, or CR), which may be added when +// inputting interactively or piping input. The value is otherwise treated as +// opaque, preserving any other whitespace, including newlines, per [NIST SP 800-63B §5.1.1.2]. +// Note that trimming whitespace may still happen elsewhere (see [NIST SP 800-63B (revision 4) §3.1.1.2]); +// +// > Verifiers **MAY** make limited allowances for mistyping (e.g., removing +// > leading and trailing whitespace characters before verification, allowing +// > the verification of passwords with differing cases for the leading character) +// +// [NIST SP 800-63B §5.1.1.2]: https://pages.nist.gov/800-63-3/sp800-63b.html#memsecretver +// [NIST SP 800-63B (revision 4) §3.1.1.2]: https://pages.nist.gov/800-63-4/sp800-63b.html#passwordver +func readSecretFromStdin(r io.Reader) (string, error) { + b, err := io.ReadAll(r) + if err != nil { + return "", err + } + if len(b) == 0 { + return "", nil + } + + for _, eol := range [][]byte{[]byte("\r\n"), []byte("\n"), []byte("\r")} { + var ok bool + b, ok = bytes.CutSuffix(b, eol) + if ok { + break + } + } + + return string(b), nil +} + func verifyLoginOptions(dockerCLI command.Streams, opts *loginOptions) error { if opts.password != "" { _, _ = fmt.Fprintln(dockerCLI.Err(), "WARNING! Using --password via the CLI is insecure. Use --password-stdin.") @@ -97,14 +130,14 @@ func verifyLoginOptions(dockerCLI command.Streams, opts *loginOptions) error { if opts.user == "" { return errors.New("username is empty") } - - contents, err := io.ReadAll(dockerCLI.In()) + p, err := readSecretFromStdin(dockerCLI.In()) if err != nil { return err } - - opts.password = strings.TrimSuffix(string(contents), "\n") - opts.password = strings.TrimSuffix(opts.password, "\r") + if strings.TrimSpace(p) == "" { + return errors.New("password is empty") + } + opts.password = p } return nil } diff --git a/cli/command/registry/login_test.go b/cli/command/registry/login_test.go index 072a72feabf6..9537f008b486 100644 --- a/cli/command/registry/login_test.go +++ b/cli/command/registry/login_test.go @@ -8,6 +8,7 @@ import ( "io" "path/filepath" "testing" + "testing/iotest" "time" "github.com/creack/pty" @@ -289,6 +290,38 @@ func TestRunLogin(t *testing.T) { }, }, }, + { + doc: "password stdin empty", + priorCredentials: map[string]configtypes.AuthConfig{}, + input: loginOptions{ + serverAddress: "reg1", + user: "my-username", + passwordStdin: true, + }, + expectedErr: `password is empty`, + expectedCredentials: map[string]configtypes.AuthConfig{ + "reg1": { + Username: "my-username", + ServerAddress: "reg1", + }, + }, + }, + { + doc: "password stdin read error", + priorCredentials: map[string]configtypes.AuthConfig{}, + input: loginOptions{ + serverAddress: "reg1", + user: "my-username", + passwordStdin: true, + }, + expectedErr: `TEST_READ_ERR`, + expectedCredentials: map[string]configtypes.AuthConfig{ + "reg1": { + Username: "my-username", + ServerAddress: "reg1", + }, + }, + }, { doc: "password stdin with line-endings", priorCredentials: map[string]configtypes.AuthConfig{}, @@ -315,7 +348,11 @@ func TestRunLogin(t *testing.T) { cli := test.NewFakeCli(&fakeClient{}) cli.SetConfigFile(cfg) if tc.input.passwordStdin { - cli.SetIn(streams.NewIn(io.NopCloser(bytes.NewBufferString(tc.stdIn)))) + if tc.expectedErr == "TEST_READ_ERR" { + cli.SetIn(streams.NewIn(io.NopCloser(iotest.ErrReader(errors.New(tc.expectedErr))))) + } else { + cli.SetIn(streams.NewIn(io.NopCloser(bytes.NewBufferString(tc.stdIn)))) + } } for _, priorCred := range tc.priorCredentials {