Skip to content

cli/command/registry: preserve all whitespace in secrets#6784

Open
thaJeztah wants to merge 2 commits intodocker:masterfrom
thaJeztah:login_cleanups
Open

cli/command/registry: preserve all whitespace in secrets#6784
thaJeztah wants to merge 2 commits intodocker:masterfrom
thaJeztah:login_cleanups

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Feb 7, 2026

cli/command/registry: remove uses of "gotest.tools/v3/fs"

cli/command/registry: preserve all whitespace in secrets

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 a21a5f4,
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)

- Human readable description for the release notes

Preserve leading and trailing whitespace when storing passwords.

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 50.00000% with 10 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cli/command/registry/login.go 55.55% 5 Missing and 3 partials ⚠️
cli/command/registry.go 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates docker login credential handling to preserve leading/trailing whitespace in secrets (treating the password as an opaque value), addressing #6782 and partially reverting behavior introduced in a21a5f4 / #5550.

Changes:

  • Add readSecretFromStdin() to trim only terminal line-endings (LF/CRLF/CR) when reading --password-stdin, without trimming other whitespace.
  • Stop mutating argPassword via strings.TrimSpace in PromptUserForCredentials, while still treating whitespace-only passwords as empty for prompting.
  • Update registry login tests: remove gotest.tools/v3/fs usage, use t.TempDir(), and add regression tests for passwords containing leading/trailing spaces and newline handling via stdin.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
cli/command/registry/login.go Introduces readSecretFromStdin() and uses it for --password-stdin to preserve whitespace (except final line-ending).
cli/command/registry/login_test.go Refactors temp file handling away from fs, injects stdin for password-stdin cases, and adds whitespace-related regression tests.
cli/command/registry.go Avoids trimming argPassword while still detecting whitespace-only passwords as empty for prompting.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


argPassword = strings.TrimSpace(argPassword)
if argPassword == "" {
isEmpty := strings.TrimSpace(argPassword) == ""
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change avoids trimming argPassword, but passwords entered interactively are still trimmed because prompt.ReadInput() applies strings.TrimSpace(scanner.Text()) (internal/prompt/prompt.go:58). As a result, leading/trailing whitespace in passwords will still be lost when the password is prompted, which contradicts the PR’s goal of preserving whitespace. Consider adding a non-trimming prompt function (or an option/flag to ReadInput) for secrets so the returned password preserves whitespace (while still stripping the final line-ending).

Suggested change
isEmpty := strings.TrimSpace(argPassword) == ""
isEmpty := argPassword == ""

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional; a whitespace-only password should not be considered valid, and so ignored here (to handle accidental user-input)

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 a21a5f4,
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 <github@gone.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

password with trailing space not accepted

2 participants