8000
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
88DF
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.

10 participants

0