chore: add CLA check action

[acolombier]

Work in progress action to auto detect if CLA has been completed by the contributor

TODO:

• Cleanup and document IaC
• Document automation
• Hook to real sheet

[daschuer]

Wow, that was fast. Thank you very much.

@ywwg is in contact with @rryan to get access to the original sheet and form.

We have confirmed in a Mixxx e.V. resolution that we want to collect the GitHub user name, and want to keep the text of the agreement as it is to not to have to deal with two version of the agreement. We have also identified that want to keep the manual conformance of new contributors in an PR comment that they have signed the CLA. This serves as a second factor for identification. We may later improve that later, but lets keep the scope here minimal.

[daschuer]

Just noticed this warning in the CI Log:

⚠️ The "create_credentials_file" option is true, but the current GitHub workspace is empty. Did you forget to use "actions/checkout" before this step? If you do not intend to share authentication with future steps in this job, set "create_credentials_file" to false.
``

[daschuer]

How is this terraform part used?

I wonder how the credentials to look up the CLA form can be shared into a users PR. How is this finally done?

[acolombier]

We have also identified that want to keep the manual conformance of new contributors in an PR comment that they have signed the CLA

Yes, that makes sense. Note that this action perform as following so far:

• Contributors who have already signed the CLA -> PR is labelled with cla-checked
• Contributors who have not signed the CLA
  -> PR is labelled with cla-pending
  -> comment on the PR to request signing the CLA
• Contributors who had not signed the CLA, and now have
  -> PR is relabelled with cla-checked
  -> comment on the PR is updated to thank them

I guess it is good to add a manual check, but this will require core members to have to access to this form. Or do we want to restrict

How is this terraform part used?

I used this terraform to configure the GCP project to drive the automation, which includes:

• A GCP project (missing in IaC, to be added)
• The service account + identity pool (used OIDC authentication, so we have a password-less auth)
• Permissions for the service account (effectively, right to create an ID token, to authenticate to Sheet API)

I wonder how the credentials to look up the CLA form can be shared into a users PR. How is this finally done?

There is no password, we are using OIDC for that matter. This is the most secure way to authenticate to a cloud provider from a workflow, you may find more details here

[daschuer]

Can't an attacker just alter the workflow in his PR and read out the whole spreadsheet?

[daschuer]

Contributors who have already signed the CLA -> PR is labelled with cla-checked

This means that we will have this lable on every single RR. Maybe we can reduce the visual clutter by ommit this and have only to-do type labels?

[daschuer]

With the manual confirm, we mean that the contributor confirms that he has signed in a PR comment. This way we know the singer was him and not a random troll.

[acolombier]

No. The reason is that your OIDC claim will include the origin repo. For example, in your case, it would include daschuer/mixxx but we restrict auth to mixxxdj/mixxx (this is why I submitted this PR from the upstream directly!)

Now, in order to enhance security, I'm planning to restrict the source branch to only be from main or the release branch (will do when we merge obviously), since this is where the pull_request_target will trigger from.
To properly tight this up, we should enforce branch protection (current it is in "audit mode") but this would restrict push entirely.

I will push the last changes soon, with the correct event setup, when I get back to my keyboard, likely later in the weekend

[acolombier]

Maybe we can reduce the visual clutter by ommit this and have only to-do type labels?

Sounds good. I will also add the action as "required" so it would properly flag it before merge, in case the action wasn't to run for any reasons.

[acolombier]

With the manual confirm, we mean that the contributor confirms that he has signed in a PR comment. This way we know the singer was him and not a random troll.

We could ask them to comment back in the auto-comment? Something like "I confirm I have signed the CLA", tho I believe they can always delete their message later so not long term proof

[ywwg]

Yeah that's what we talked about, checking that there's a comment authored by the correct username that confirms signing

[daschuer]

I have not understand how the security works. In PR builds we have no access to mixxxdj credentials. If the we forward them, the users can change the workflow in the PR and retrieve the full spreadsheet. How do we handle that his is not happen?

[acolombier]

In PR builds we have no access to mixxxdj credentials. If the we forward them, the users can change the workflow in the PR and retrieve the full spreadsheet. How do we handle that his is not happen?

This is correct. This action is to expected to run as a pull_request action (which could indeed be tempered by a nefarious contributor), but as pull_request_target. The fundamental difference is that, in pull_request_target the "base" workflow (in other term, the YML file in main, 2.6 or 2.5) gets triggered, and cannot be modified (except of course if a PR is merged, or if one of the core contributor pushes something)

A second layer of security (regarding OIDC) is that, before using this Github OIDC token, you need to issue a token, which required id-token: write permission, which is only available for "head" based on mixxxdj/mixxx. This means that forks won't be able to issue a token originating from mixxxdj/mixxx.

Yeah that's what we talked about, checking that there's a comment authored by the correct username that confirms signing

Note that this PR already ensured that the author is the one that have signed the CLA. If we wanted to check that follow up comment (e.g "hey I have signed now") was made by the initial author of this same PR, this would be very easy, tho I am not sure this was help much. If somebody was to signed a PR they have authored (e.g as a troll or somebody that would want something pending CLA merged), it would still not pass the CLA checks.

Happy to jump on a call to explain further.

[daschuer]

Ah OK, that makes sense. A pull_request_target should work.

Note that this PR already ensured that the author is the one that have signed the CLA.

How is this done? For my understanding anyone can sign the form and enter the users GitHub name.

[JoergAtGithub]

If we wanted to check that follow up comment (e.g "hey I have signed now") was made by the initial author of this same PR, this would be very easy, tho I am not sure this was help much.

The idea was to prove, that the CLA was signed by the GitHub user and not by a third-party who just entered this username in the google docs.

[acolombier]

Ah that makes sense. In this case I can add a check that makes sure the follow up comment or 👍 is indeed issue by the author. Should be straightforward!

[github-actions]

CLA check

Thank you for signing the CLA!

[acolombier]

This is now ready for review. I have carried few more tests to confirm it should work. I would suggest we take this for a spin, as soon as we have access to the Google Sheet.

[daschuer]

@ywwg and @rryan, did you make progress sorting out the access issue?

[daschuer]

How is this step triggered?

[acolombier]

Either:

• A maintainer issue /check-cla
• A push/sync is made to the PR

[daschuer]

I think we should not delete the old comment. This keeps it better traceable what has happened.
We has the case that user expresses concerns. When we delete the original message this concerns comment looses context.

[acolombier]

Technically, the comment only gets edited so we keep the full history. I guess two comments isn't too noisy tho?

[daschuer]

I have still not understand how the tf files come into play. What happens if we delete all of them?
Is this required for: "google-github-actions/auth"?
I have not found an example that using it.

[acolombier]

I have still not understand how the tf files come into play.

They are only a reflection on how I have setup GCP resources for the automation. They are a like a recipe to get cloud resources from zero to a ready state, without requiring any manual operation (knows as "clickops"). I would suggest to read about IaC.

What happens if we delete all of them?

Nothing. They are only here in case:

1. We need to make a change
2. Something happens and we need to recreate the config

To make a simple parallel, think of it as our CMakefile. If you delete, any previously build version of Mixxx will continue function as before. Our binaries will remain available and usable. However, the day you need to build a new version OR will loose all binary and need to recreate one, you will go through the tedious process of setting everything by hand, whether it is to remember all build commands, deps or setup you need to do to get the project in your IDE or terminal, leading to likely time waste and errors.

The TLDR is, no need to understand them now. But if I leave the project, you will have a clear vision of what is set and how.