-
Notifications
You must be signed in to change notification settings - Fork 259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stop adding GitHub SSH keys #171
Conversation
@mpdude This will break alot of workflows. Why not fetch the keys from |
😭
Am I missing anything? |
@mpdude No you are right, this should work. Containerized workflows should just pick up the host keys and accept them. And admins should remove everything from known_hosts on self-hosted. I will update my actions to v0.8.0. |
Sorry if it's a stupid question, but for GitHub-hosted action, do we now need to add GitHub's keys in the |
@faahim It may be a stupid answer because I'm not familiar with GitHub Actions, but I'd think you'd rather need to remove old and outdated entries from |
See #174 (comment) for a good way to fetch the GitHub SSH host keys from their API. Fetching over SSH is more secure than trusting |
Since webfactory#171 was merged, this action no longer touches `known_hosts`; this line should have been removed from the README at that time.
Since #171 was merged, this action no longer touches `known_hosts`; this line should have been removed from the README at that time.
We need to fix the SSH keys shipped with this action: https://github.blog/2023-03-23-we-updated-our-rsa-ssh-host-key/
But, we have another issue (#106) with regards to host keys: On self-hosted runners which are not ephemeral the
known_host
file fills up with repeated entries, because every action run adds a new line with the same host keys.Also, on those machines, the old key will still be in the
known_hosts
file.IMHO this action should not be responsible for shipping SSH host keys, that's too much responsibility.
This section in the code is a leftover from early days when GitHub provided runners did not include SSH keys at all. For a long time already, GH takes care of placing their SSH keys in their runner images.
For self-hosted runners, those people setting up the runner should fetch and verify SSH keys themselves and put it into the
known_hosts
file.I know this is a breaking change and is going to annoy users. But on the other hand, there is no better opportunity to drop this feature than with an emergency-style key revocation as today.
Closes #106, closes #129, closes #169, closes #170, closes #172.