Add top level permissions to all Github Actions workflows

[faern]

The default permission on the repository is already set to read only. So in practice this makes no difference. But this makes that more explicit, and less relying on the repository being correctly configured.

This also makes security scanning tools such as OpenSSF scorecard happier about the overall security of our repository.

I think we should start always defining permissions on our actions. However, we currently don't have anything that enforces it, and given how little actual security it gives us right now it's definitely not worth automating. I just wanted to do this little manual sweep to "improve" the current state.

---

This change is 

[linear]

TEC-5 Define Github Actions workflow privileges according to least privilege

[albin-mullvad]

Reviewed 26 of 28 files at r1, all commit messages.
Reviewable status: 26 of 28 files reviewed, 1 unresolved discussion (waiting on @faern)

---

.github/workflows/ios-end-to-end-tests-merge-to-main.yml line 14 at r1 (raw file):

      - ios/**

permissions:

I'm not sure it's a good idea to move these permissions outside the job, since that would make any (potential future) job in this workflow gain those permissions. I believe it's better to put permissions: {} on the top level and then let each job specific exactly what they need.

[faern]

Reviewable status: 26 of 28 files reviewed, 1 unresolved discussion (waiting on @albin-mullvad)

---

.github/workflows/ios-end-to-end-tests-merge-to-main.yml line 14 at r1 (raw file):

Previously, albin-mullvad wrote…

I'm not sure it's a good idea to move these permissions outside the job, since that would make any (potential future) job in this workflow gain those permissions. I believe it's better to put permissions: {} on the top level and then let each job specific exactly what they need.

In general I agree. I was just being pragmatic since this workflow has a single job. The same pattern exists in two other files in this PR. For example cargo-audit is very unlikely to ever get more jobs. And if it does, creating permissions is pretty core to a CVE checking job. So do you think the same logic holds there?

If so, is there ever a reason to specify non-empty top level permissions? I don't see the point in always specifying empty top level permissions and then re-define them with more permissions for workflows that very clearly do one thing and have one job.

[albin-mullvad]

Reviewed 2 of 28 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @faern)

---

.github/workflows/ios-end-to-end-tests-merge-to-main.yml line 14 at r1 (raw file):

Previously, faern (Linus Färnstrand) wrote…

In general I agree. I was just being pragmatic since this workflow has a single job. The same pattern exists in two other files in this PR. For example cargo-audit is very unlikely to ever get more jobs. And if it does, creating permissions is pretty core to a CVE checking job. So do you think the same logic holds there?

If so, is there ever a reason to specify non-empty top level permissions? I don't see the point in always specifying empty top level permissions and then re-define them with more permissions for workflows that very clearly do one thing and have one job.

Alright, good point, as long as we are aware of the impact of this PR. I'm fine with keeping this change as-is, but I think we need to be careful doing the same in bigger workflows.

[buggmagnet]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @albin-mullvad and @faern)

---

.github/workflows/ios-end-to-end-tests-merge-to-main.yml line 14 at r1 (raw file):

Previously, albin-mullvad wrote…

Alright, good point, as long as we are aware of the impact of this PR. I'm fine with keeping this change as-is, but I think we need to be careful doing the same in bigger workflows.

I had a small chat with @albin-mullvad on this, I also agree with his conclusion.
AFAICT, as long as the default permissions for the GITHUB_TOKEN are set to restrictive, most jobs should have no permissions. (Please take this with a grain of salt, this is my understanding of the documentation, but I'm not 100% sure whether I got it right)

As long as we don't define any new jobs in this yml file, the change should have no impact.

With that being said, I don't have any strong opinions on the changes in this PR, so I'm fine with them.

[faern]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @albin-mullvad and @buggmagnet)

---

.github/workflows/ios-end-to-end-tests-merge-to-main.yml line 14 at r1 (raw file):

Previously, buggmagnet wrote…

I had a small chat with @albin-mullvad on this, I also agree with his conclusion.
AFAICT, as long as the default permissions for the GITHUB_TOKEN are set to restrictive, most jobs should have no permissions. (Please take this with a grain of salt, this is my understanding of the documentation, but I'm not 100% sure whether I got it right)

As long as we don't define any new jobs in this yml file, the change should have no impact.

With that being said, I don't have any strong opinions on the changes in this PR, so I'm fine with them.

100% agree that we should not just apply a bunch of global permissions to larger/more complex workflows. For workflows with more than one job, where only one job need permissions, we should only give that single job the needed permissions if practical.

[albin-mullvad]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[buggmagnet]

Reviewed 27 of 28 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

[github-actions]

🚨 End to end tests failed. Please check the failed workflow run.