Skip to content

login: print a big warning when using --password#270

Merged
thaJeztah merged 1 commit into
docker:masterfrom
tych0:warn-only-about-password-on-cli
Jul 8, 2017
Merged

login: print a big warning when using --password#270
thaJeztah merged 1 commit into
docker:masterfrom
tych0:warn-only-about-password-on-cli

Conversation

@tych0

@tych0 tych0 commented Jun 29, 2017

Copy link
Copy Markdown
Contributor

Task command lines are world readable via /proc/pid/cmdline, so this isn't
safe.

@codecov-io

codecov-io commented Jun 29, 2017

Copy link
Copy Markdown

Codecov Report

Merging #270 into master will decrease coverage by 1.58%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #270      +/-   ##
==========================================
- Coverage   48.44%   46.85%   -1.59%     
==========================================
  Files         173      172       -1     
  Lines       11748    11692      -56     
==========================================
- Hits         5691     5478     -213     
- Misses       5696     5902     +206     
+ Partials      361      312      -49
Comment thread cli/command/registry/login.go Outdated
clnt := dockerCli.Client()

if opts.password != "" {
fmt.Fprintf(dockerCli.Err(), "Using --password via the CLI is insecure.\n")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you change this to fmt.Fprintln()?

Perhaps prefix with WARNING! ;

fmt.Fprintln(dockerCli.Err(), "WARNING! Using --password via the CLI is insecure.")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Task command lines are world readable via /proc/pid/cmdline, so this isn't
safe.

Signed-off-by: Tycho Andersen <tycho@tycho.ws>
@tych0 tych0 force-pushed the warn-only-about-password-on-cli branch from 952d7da to c269ad2 Compare July 3, 2017 14:47

@vdemeester vdemeester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM 🐸
/cc @n4ss

@n4ss

n4ss commented Jul 3, 2017

Copy link
Copy Markdown
Contributor

LGTM!

@boaz0 boaz0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 🤠

@vieux

vieux commented Jul 4, 2017

Copy link
Copy Markdown
Contributor

LGTM ping @thaJeztah

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh, darn missed the ping

LGTM, thanks!

@thaJeztah thaJeztah merged commit c99530b into docker:master Jul 8, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.07.0 milestone Jul 8, 2017
nobiit pushed a commit to nobidev/docker-cli that referenced this pull request Nov 19, 2025
[17.10] re-vndr swarmkit to 1d2bc2e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment