login-action icon indicating copy to clipboard operation
login-action copied to clipboard

docker auth: improve missing user/pwd

Open Frankkkkk opened this issue 9 months ago • 3 comments

Specify the "missing username and password" error message. This makes debugging the action easier when for example mistyping the username or the password.

Thanks and cheers!

Frankkkkk avatar Apr 30 '24 19:04 Frankkkkk

Thanks for this PR but I think the error message to users should remain generic. You can still check what value is empty in the login step (expand Run docker/login-action@v3):

image

crazy-max avatar May 07 '24 08:05 crazy-max

Hi @crazy-max , thanks for your message. Any reason why you want the error message to be generic? I genuinely can't see why?

These actions are also used outside Github (for example on Gitea or Forgejo. Basically on any act runner). On those, the details are not shown: image Thus why it adds more info Thanks

Frankkkkk avatar May 07 '24 09:05 Frankkkkk

Done, thanks. Curious question: the dist file are so large; how do you make sure that a malicious user didn't embed some nefarious content in it ? Subsidiary one: how do you deal with conflicts on MRs ? Do all contributors always have to rebase on every change on main? Thanks!

Frankkkkk avatar May 16 '24 07:05 Frankkkkk

Curious question: the dist file are so large; how do you make sure that a malicious user didn't embed some nefarious content in it ?

We have a scanner and CodeQL running as well to avoid that. Also considering our dependencies tree we are quite confident we consume vetted packages: https://github.com/docker/login-action/blob/70fccc794acd729b2b22dd6a326895f286447728/package.json#L27-L34

Subsidiary one: how do you deal with conflicts on MRs ? Do all contributors always have to rebase on every change on main?

Yes indeed this is currently painful but no other choice atm. GitHub plans to change that in the future to use GitHub Packages to ship actions instead (was told about this two years ago though).

crazy-max avatar May 27 '24 11:05 crazy-max