cli/command/registry: preserve all whitespace in secrets#6784
cli/command/registry: preserve all whitespace in secrets#6784thaJeztah wants to merge 2 commits intodocker:masterfrom
Conversation
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
7742eae to
ed7ad1a
Compare
There was a problem hiding this comment.
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
argPasswordviastrings.TrimSpaceinPromptUserForCredentials, while still treating whitespace-only passwords as empty for prompting. - Update registry login tests: remove
gotest.tools/v3/fsusage, uset.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) == "" |
There was a problem hiding this comment.
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).
| isEmpty := strings.TrimSpace(argPassword) == "" | |
| isEmpty := argPassword == "" |
There was a problem hiding this comment.
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>
ed7ad1a to
378bbc2
Compare
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);
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)