Security: PoC: Failing tests lead to ssh access via action 'mxschmitt/action-tmate'

[Hagfjall]

Problems:

1. Anyone that will trigger workflow ci for pullrequest can get SSH access to the runner. Reason is that if tests are failing:

   cassandra-reaper/.github/workflows/ci.yaml

   Lines 69 to 72 in c1491e8

   	- name: Setup tmate session
   	if: "failure()"
   	uses: mxschmitt/action-tmate@v3
   	timeout-minutes: 30

   , it will automatically open a ssh server and wait for inbound connection. Whatever secrets are expose to the runner will also be exposed via the ssh-session.
2. Changing the workflow definition will lead to the PR's definition of the workflow to be run. Can be problematic since the PR author can change and inject arbitrary code/definitions. Being able to extract environments and secrets set on the repository (depending on how they are set)

[github-actions]

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

[adejanovski]

Whatever secrets are expose to the runner will also be exposed via the ssh-session.

@Hagfjall, this isn't accurate. Anyone external to the organization will not get access to the secrets in these workflows (tmate or not). Also the tmate session is restricted to the ssh key of the user that created the PR in the first place, so tmate sessions from PRs initiated a committer will only be accessible by this specific committer. No external user or even another committer can access the runner through the created session.

[Hagfjall]

@adejanovski glad to hear that your secrets are not accessible for this workflow if it is triggered by someone outside of the org, like me :) That's perfect!

Regarding the tmate, you are partly correct. The ssh session will be configured with a keypair IF, and only if, the actor has ssh key configured in his/hers github account. See https://github.com/mxschmitt/action-tmate/blob/a283f9441d2d96eb62436dc46d7014f5d357ac22/src/index.js#L132-L142
Can be improved by requiring a ssh key like this:

      uses: mxschmitt/action-tmate@v3
      with:
        limit-access-to-actor: true

I just wanted to communicate the security issue, and possible improvement as a proof of concept.
OK for me to close this PR, let me know if you want me to close it from my end.

[adejanovski]

Thanks for looking into this @Hagfjall, it's always good to be challenged around security.
I think your suggestion of using limit-access-to-actor: true should be implemented for improved security.
If you feel like pushing a PR for this, it would be much appreciated.
Thanks again!