Write interactive prompts to stderr and handle echo-disable failures#32393
Open
Mukuwul wants to merge 1 commit into
Open
Write interactive prompts to stderr and handle echo-disable failures#32393Mukuwul wants to merge 1 commit into
Mukuwul wants to merge 1 commit into
Conversation
Interactive prompts from 'helm registry login' (username/password/token) and the signing-key passphrase prompts in 'helm package' and 'helm plugin package' are printed to stdout. Anyone capturing stdout (scripts, pipelines) gets prompt text mixed into the output; the docker/kubectl/oras convention is to keep prompts on stderr so stdout stays machine-readable. Also in registry login's readLine: - the term.DisableEcho error was silently discarded, so a failure to disable echo would print the password to the terminal with no warning; it now returns an error instead - echo handling is only attempted when stdin is actually a terminal, so piped input (e.g. 'echo pw | helm registry login ...' without --password-stdin) keeps working as before Signed-off-by: Mukul <nmukul32@gmail.com>
pull-request-size
Bot
added
the
size/S
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Helm’s interactive CLI prompting behavior so prompts don’t pollute stdout when users capture command output for scripting, aligning helm registry login and signing passphrase prompts with common tooling conventions (docker/kubectl/oras).
Changes:
- Write
helm registry loginusername/password/token prompts to stderr instead of stdout. - Write signing-key passphrase prompts for
helm packageandhelm plugin packageto stderr instead of stdout. - In
readLine(registry login), only attempt echo suppression when stdin is a terminal, and return an error if disabling echo fails (avoiding silent password echo).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/cmd/registry_login.go | Routes prompts/newlines to stderr and tightens terminal echo-disable behavior in readLine. |
| pkg/cmd/plugin_package.go | Sends signing passphrase prompt/newline to stderr to keep stdout machine-readable. |
| pkg/action/package.go | Sends signing passphrase prompt/newline to stderr for helm package --sign flows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Closes #32392
Moves the interactive prompts from
helm registry login(username/password/token) and the signing-key passphrase prompts inhelm package/helm plugin packagefrom stdout to stderr, following the docker/kubectl/oras convention so stdout stays machine-readable when captured.In
registry_login.go'sreadLineit also:term.DisableEchofails instead of silently continuing — previously a failure there meant the password was echoed to the terminal with no warningecho pw | helm registry login …without--password-stdin) behaves exactly as before — this also means the new DisableEcho error path can't fire for pipesSpecial notes for your reviewer:
Behavior-wise stdout consumers only ever saw prompt text (never data) from these paths, so nothing that parses stdout can break; the prompts simply become visible again when stdout is redirected. Verified with
go build/go veton the three packages; theTestPackagefailures on my Windows machine are pre-existing portability issues also present on clean main (see #32389).If applicable:
docs neededlabel should be applied if so)