#11631 GitHub Workflows security hardening

[sashashura]

Fixes #11631

This PR adds explicit permissions section to workflows. This is a security best practice because by default workflows run with extended set of permissions (except from on: pull_request from external forks). By specifying any permission explicitly all others are set to none. By using the principle of least privilege the damage a compromised workflow can do (because of an injection or compromised third party tool or action) is restricted.
It is recommended to have most strict permissions on the top level and grant write permissions on job level case by case.

[altendky]

Thanks for making me see this! I was totally unaware of this feature. On the off chance, do you know when this was added? Or did I always just miss it...

[sashashura]

Read/write permissions for non pull_request triggers were from very beginning. Explicit control of permissions was introduced about a year ago https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/

[adiroiban]

Thanks for the update.
I guess that this is ok.
A bit better than what we had before, mostly future-proof.

I think we were always fine, as we make simple usage of GHA.

We don't use pull_request_target.

We have recently introduced workflow_dispatch but I think this was always safe.
I can't find much info about workflow_dispatch security.
I know Jean-Paul has tried using it with forks and it failed.

---

Also, I hope we can keep GHA usage simple, to reduce our dependency on GHA and at the same time, improve security.

[adiroiban]

ok. so we need this in write mode, only to update the "stable" branch.

The "stable" branch was added as a hack, since Read the docs, doesn't recognizes our twisted-12.23.45 tag format, and can't determine the latest stable version.

Maybe for better security, the "stable" branch update can be moved into a separate job that is performed after release-publish.

So, this would go further with the principle of least privilege

[sashashura]

Yes, you got the idea! Restrict as much as possible. I just usually try to minimize the changes to avoid breaking anything. I have split it into a separate job. Please, take a look.

[adiroiban]

Many thanks for the changes.

I push a change that adds some extra comment/docs for why we need the extra job.

I have triggered the auto-merge for this. I hope all will be ok.

Not sure how our 'all-success' works with the skip state.
I guess it might fail.

The idea is that we want all-success as a single way to monitor the success of the jobs for a commit... but we might need to remove the stable branch update job.

[adiroiban]

Thanks, Sasha for the change.

Much appreciated to get help with the supply chain security.
It's a big topic these days and I think it was something overlooked for a long time.

---

Before merging this I would like to get a second opinion from @altendky regarding https://github.com/twisted/twisted/pull/11632/files#diff-245392b692a50c38ecab4381b118862db514035c10983f3bd4f4b7f1f4be4692R440

I think that not failing on skip is ok.

---

The previous issue was that on fail, the job was considered skipped, so we didn't had a final failure.