search

docker / login-action

GitHub Action to login against a Docker registry
https://github.com/marketplace/actions/docker-login
Apache License 2.0
1.04k stars 190 forks source link

Security issue #30

Open eine opened 3 years ago

eine commented 3 years ago

Coming from docker/build-push-action#53

Refs:

Behaviour

https://github.com/docker/build-push-action/issues/53#issuecomment-721898162 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

https://github.com/docker/build-push-action/issues/53#issuecomment-721898162 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 commented 3 years ago

@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 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.

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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

valentijnscholten commented 3 years ago

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 commented 3 years ago

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

eine commented 3 years ago

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

crazy-max commented 3 years ago

@eine @clarkbw actions/virtual-environments#2304 has been merged. Will be available ~January (https://github.com/actions/virtual-environments/issues/2302#issuecomment-749140395).