Security issue

[eine]

Coming from docker/build-push-action#53

Refs:

• fix: print non-empty stderr #25
• Handle credentials securely docker-archive/github-actions#21
• Example about how to securely login through Docker CLI actions/starter-workflows#96

Behaviour

docker/build-push-action#53 (comment)
It seems that the warning message is hidden from the users, which is misleading as it provides a false feeling of security. As seen in docker/login-action@adb7347/src/docker.ts#L36, on success stderr is not shown. The warning is precisely shown when the login is successful but insecure.

Steps to reproduce this issue

docker/build-push-action#53 (comment)
See eine/login-action@master (commits) and eine/login-action/runs/1354438643?check_suite_focus=true#step:3:8.

Expected behaviour

Login is secure or security warnings are not hidden.

Actual behaviour

Login is reported not to be secure, but warnings are hidden.

[crazy-max]

@eine

It seems that the warning message is hidden from the users, which is misleading as it provides a false feeling of security. As seen in adb7347/src/docker.ts#L36, on success stderr is not shown. The warning is precisely shown when the login is successful but insecure.

This issue concerns the credential store used on the GitHub Runner and not this action itself. Also as you can see on your own fork, credentials are removed when the job is finished.

[eine]

@crazy-max, see actions/starter-workflows#96 (and the ref to docker/cli#2089). Ideally developers/maintainers of Docker and GitHub Actions would communicate with each other for achieving a satisfactory solution.

[crazy-max]

@eine

Ideally developers/maintainers of Docker and GitHub Actions would communicate with each other for achieving a satisfactory solution.

Maybe GitHub could simply install the pass credential helper on the GitHub Runner. WDYT @clarkbw?

[clarkbw]

I've asked for this before. I'll push for it again.

[valentijnscholten]

Please not this not only affects the "build and push" action. Currently every workflow must/should start with a docker login as to decrease the chance of being hit by the new rate limiting.

[clarkbw]

In the short term can we only filter out the login message?

[eine]

@clarkbw this issue is about requesting relevant warnings not to be hidden from users. ATM, the warnings are filtered out: #25.

[crazy-max]

@eine @clarkbw actions/runner-images#2304 has been merged. Will be available ~January (actions/runner-images#2302 (comment)).