From c435940a9cc3094dd773ddc4e3539e6e2d3c86f4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 6 Feb 2026 17:50:45 +0100 Subject: [PATCH 1/2] cli/command/registry: remove uses of "gotest.tools/v3/fs" Signed-off-by: Sebastiaan van Stijn --- cli/command/registry/login_test.go | 42 ++++++++++++++---------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/cli/command/registry/login_test.go b/cli/command/registry/login_test.go index 9dfb13dfe751..dc469a07a68a 100644 --- a/cli/command/registry/login_test.go +++ b/cli/command/registry/login_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/creack/pty" + "github.com/docker/cli/cli/config/configfile" configtypes "github.com/docker/cli/cli/config/types" "github.com/docker/cli/cli/streams" "github.com/docker/cli/internal/prompt" @@ -19,7 +20,6 @@ import ( "github.com/moby/moby/client" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" - "gotest.tools/v3/fs" ) const ( @@ -71,8 +71,9 @@ func TestLoginWithCredStoreCreds(t *testing.T) { }, } ctx := context.Background() + tmpDir := t.TempDir() cli := test.NewFakeCli(&fakeClient{}) - cli.ConfigFile().Filename = filepath.Join(t.TempDir(), "config.json") + cli.SetConfigFile(configfile.New(filepath.Join(tmpDir, "config.json"))) for _, tc := range testCases { _, err := loginWithStoredCredentials(ctx, cli, tc.inputAuthConfig) if tc.expectedErrMsg != "" { @@ -290,16 +291,15 @@ func TestRunLogin(t *testing.T) { for _, tc := range testCases { t.Run(tc.doc, func(t *testing.T) { - tmpFile := fs.NewFile(t, "test-run-login") - defer tmpFile.Remove() + tmpDir := t.TempDir() + cfg := configfile.New(filepath.Join(tmpDir, "config.json")) cli := test.NewFakeCli(&fakeClient{}) - configfile := cli.ConfigFile() - configfile.Filename = tmpFile.Path() + cli.SetConfigFile(cfg) for _, priorCred := range tc.priorCredentials { - assert.NilError(t, configfile.GetCredentialsStore(priorCred.ServerAddress).Store(priorCred)) + assert.NilError(t, cfg.GetCredentialsStore(priorCred.ServerAddress).Store(priorCred)) } - storedCreds, err := configfile.GetAllCredentials() + storedCreds, err := cfg.GetAllCredentials() assert.NilError(t, err) assert.DeepEqual(t, storedCreds, tc.priorCredentials) @@ -310,7 +310,7 @@ func TestRunLogin(t *testing.T) { } assert.NilError(t, loginErr) - outputCreds, err := configfile.GetAllCredentials() + outputCreds, err := cfg.GetAllCredentials() assert.Check(t, err) assert.DeepEqual(t, outputCreds, tc.expectedCredentials) }) @@ -356,11 +356,10 @@ func TestLoginNonInteractive(t *testing.T) { for _, registryAddr := range registries { for _, tc := range testCases { t.Run(tc.doc, func(t *testing.T) { - tmpFile := fs.NewFile(t, "test-run-login") - defer tmpFile.Remove() + tmpDir := t.TempDir() + cfg := configfile.New(filepath.Join(tmpDir, "config.json")) cli := test.NewFakeCli(&fakeClient{}) - cfg := cli.ConfigFile() - cfg.Filename = tmpFile.Path() + cli.SetConfigFile(cfg) options := loginOptions{ serverAddress: registryAddr, } @@ -419,11 +418,10 @@ func TestLoginNonInteractive(t *testing.T) { for _, registryAddr := range registries { for _, tc := range testCases { t.Run(tc.doc, func(t *testing.T) { - tmpFile := fs.NewFile(t, "test-run-login") - defer tmpFile.Remove() + tmpDir := t.TempDir() + cfg := configfile.New(filepath.Join(tmpDir, "config.json")) cli := test.NewFakeCli(&fakeClient{}) - cfg := cli.ConfigFile() - cfg.Filename = tmpFile.Path() + cli.SetConfigFile(cfg) serverAddress := registryAddr if serverAddress == "" { serverAddress = "https://index.docker.io/v1/" @@ -465,17 +463,15 @@ func TestLoginTermination(t *testing.T) { _ = p.Close() }) + tmpDir := t.TempDir() + cfg := configfile.New(filepath.Join(tmpDir, "config.json")) cli := test.NewFakeCli(&fakeClient{}, func(fc *test.FakeCli) { fc.SetOut(streams.NewOut(tty)) fc.SetIn(streams.NewIn(tty)) }) - tmpFile := fs.NewFile(t, "test-login-termination") - defer tmpFile.Remove() + cli.SetConfigFile(cfg) - configFile := cli.ConfigFile() - configFile.Filename = tmpFile.Path() - - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(t.Context()) t.Cleanup(cancel) runErr := make(chan error) From ac3814068abc582cac036e1fa15485f99de45a54 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 6 Feb 2026 17:56:16 +0100 Subject: [PATCH 2/2] cli/command/registry: preserve all whitespace in secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preserve all whitespace and treat the secret as an opaque value, leaving it to the registry to (in)validate. We still check for empty values in some places. This partially reverts a21a5f42433267052441cdb7ebe7604dfcc7f159, but checks for empty (whitespace-only) passwords without mutating the value. This better aligns with [NIST SP 800-63B §5.1.1.2], which describes that the value should be treated as opaque, preserving any other whitespace, including newlines. 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 Signed-off-by: Sebastiaan van Stijn --- cli/command/registry.go | 4 +-- cli/command/registry/login.go | 41 ++++++++++++++++++---- cli/command/registry/login_test.go | 55 ++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 8 deletions(-) diff --git a/cli/command/registry.go b/cli/command/registry.go index 452e2d7354cc..75b979e18561 100644 --- a/cli/command/registry.go +++ b/cli/command/registry.go @@ -144,8 +144,8 @@ func PromptUserForCredentials(ctx context.Context, cli Cli, argUser, argPassword } } - argPassword = strings.TrimSpace(argPassword) - if argPassword == "" { + isEmpty := strings.TrimSpace(argPassword) == "" + if isEmpty { restoreInput, err := prompt.DisableInputEcho(cli.In()) if err != nil { return registrytypes.AuthConfig{}, err diff --git a/cli/command/registry/login.go b/cli/command/registry/login.go index 913c000c4cc2..364107a238ba 100644 --- a/cli/command/registry/login.go +++ b/cli/command/registry/login.go @@ -1,11 +1,11 @@ package registry import ( + "bytes" "context" "errors" "fmt" "io" - "strings" "github.com/containerd/errdefs" "github.com/docker/cli/cli" @@ -88,6 +88,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 +129,11 @@ 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") + opts.password = p } return nil } diff --git a/cli/command/registry/login_test.go b/cli/command/registry/login_test.go index dc469a07a68a..23294c354b75 100644 --- a/cli/command/registry/login_test.go +++ b/cli/command/registry/login_test.go @@ -1,6 +1,7 @@ package registry import ( + "bytes" "context" "errors" "fmt" @@ -92,6 +93,7 @@ func TestRunLogin(t *testing.T) { testCases := []struct { doc string priorCredentials map[string]configtypes.AuthConfig + stdIn string input loginOptions expectedCredentials map[string]configtypes.AuthConfig expectedErr string @@ -287,6 +289,56 @@ func TestRunLogin(t *testing.T) { }, }, }, + { + doc: "password with leading and trailing spaces", + priorCredentials: map[string]configtypes.AuthConfig{}, + input: loginOptions{ + serverAddress: "reg1", + user: "my-username", + password: " my password with spaces ", + }, + expectedCredentials: map[string]configtypes.AuthConfig{ + "reg1": { + Username: "my-username", + Password: " my password with spaces ", + ServerAddress: "reg1", + }, + }, + }, + { + doc: "password stdin with line-endings", + priorCredentials: map[string]configtypes.AuthConfig{}, + stdIn: " my password with spaces \r\n", + input: loginOptions{ + serverAddress: "reg1", + user: "my-username", + passwordStdin: true, + }, + expectedCredentials: map[string]configtypes.AuthConfig{ + "reg1": { + Username: "my-username", + Password: " my password with spaces ", + ServerAddress: "reg1", + }, + }, + }, + { + doc: "password stdin with multiple line-endings", + priorCredentials: map[string]configtypes.AuthConfig{}, + stdIn: " my password\nwith spaces \r\n\r\n", + input: loginOptions{ + serverAddress: "reg1", + user: "my-username", + passwordStdin: true, + }, + expectedCredentials: map[string]configtypes.AuthConfig{ + "reg1": { + Username: "my-username", + Password: " my password\nwith spaces \r\n", + ServerAddress: "reg1", + }, + }, + }, } for _, tc := range testCases { @@ -295,6 +347,9 @@ func TestRunLogin(t *testing.T) { cfg := configfile.New(filepath.Join(tmpDir, "config.json")) cli := test.NewFakeCli(&fakeClient{}) cli.SetConfigFile(cfg) + if tc.input.passwordStdin { + cli.SetIn(streams.NewIn(io.NopCloser(bytes.NewBufferString(tc.stdIn)))) + } for _, priorCred := range tc.priorCredentials { assert.NilError(t, cfg.GetCredentialsStore(priorCred.ServerAddress).Store(priorCred))