Promote security considerations to the top

[m-kuhn]

No description provided.

[dscho]

I'd like to suggest a few things that I believe will improve the README even further.

[dscho]

Could we add a comment here saying that this requires a public SSH key to be added to the GitHub profile, and a direct link to the profile where that could be done for convenience?

[m-kuhn]

Can you make a suggestion?

[mxschmitt]

To this link I guess? https://docs.github.com/en/authentication/connecting-to-github-with-ssh/adding-a-new-ssh-key-to-your-github-account

[dscho]

I was thinking more about https://github.com/settings/ssh/new, but both links would work for me.

[dscho]

In addition to linking to https://github.com/settings/ssh/new, I would like to see in a comment that the key is needed to protect any secrets that are available in the workflow step, and that the limit-access-to-actor setting can be set to false for convenience if there is no risk of spilling secrets.

In my mind, this improvement is needed before this PR can be merged.

[m-kuhn]

Thanks for reviewing, if you can add the missing suggestion wordings, I'll be happy to apply them.

[dscho]

In addition to linking to https://github.com/settings/ssh/new, I would like to see in a comment that the key is needed to protect any secrets that are available in the workflow step, and that the limit-access-to-actor setting can be set to false for convenience if there is no risk of spilling secrets.

In my mind, this improvement is needed before this PR can be merged.

[dscho]

Here, we need to inform users that secrets are not made available by default, but that the risk comes from configuring token in the action-tmate Action or from adding ${{ secret.X }} values to the env block, or from writing out secret values to disk in preceding workflow steps.

[m-kuhn]

for potential follow-uppers:
... or from configurations/certificates/private keys/... that are generated/downloaded/... by previous steps having access to secrets. Or from subsequent steps deploying signed code that has been produced including injected bits or from a whole variety of other reasons we did not think about right now. Things that other maintainers of a project have introduced and you may not be aware of at the very instance you try to deploy this project because your customer wants it delivered by 6 o clock in the morning and you don't have time to read a PHD on security so you just copy paste the first code sample that promises to get the job done. Things that can be leaked exactly once and then are lost. Just like a credit card or social security number that you really don't want to see end up somewhere on the internet. So probably the 90% of users who don't understand SSH keys will neither understand this so we better stay on the safe side and try to be careful if in doubt.
As project maintainer we can now blame our collaborators for not being careful enough and ourselves for giving them access to things they shouldn't play with carelessly. But we've all been in situations that are a bit stressful and cause even experienced people to do hit a button a bit too fast.

[m-kuhn]

Hey @dscho thank you for the review.
The time I have available for continuing here is running out, so I'll leave it up to someone to pick up this work and finish it -- or to close this PR if no one is available. I think it would be a valuable addition.
Thanks again for making this great action available.

[mxschmitt]

Seems stale and not a general interest in changing this behavior since it significantly reduces the user experience.

If we change it, then we need to add a new mode, which is backwards compatible and falls back to no ssh key access, if no ssh keys are added to the account.