Skip to content

fix(ci): status check workflows permissions#81

Merged
asdolo merged 1 commit intomasterfrom
fix/ci_status_check_workflows_permissions
Oct 25, 2024
Merged

fix(ci): status check workflows permissions#81
asdolo merged 1 commit intomasterfrom
fix/ci_status_check_workflows_permissions

Conversation

@asdolo
Copy link
Copy Markdown
Collaborator

@asdolo asdolo commented Oct 22, 2024

What does this do?

This PR changes the Lint, Test and Type-Check workflows to specifically set the minimum permissions they need.

Why did you do this?

The workflows (and thus, the status checks of all Pull Requests and pushes) weren't working if the default permissions granted to the GITHUB_TOKEN were set to "Read" instead of "Read and Write" in the project repository.

Nonetheless, GItHub recommends to "only allow the minimum required access" to the workflow files.

What is this?

  • The GITHUB_TOKEN secret is used by all of these workflows to perform operations such as reading our PRs and creating comments on them.
  • If no permissions are specified in a workflow file, the GITHUB_TOKEN will have the default permissions set in the repository configuration (Repository settings > Actions > General > Workflow permissions).
  • There are two possible values for the default permissions: "Read and write" and "Read."
image
  • If the default permissions are set to "Read and write", the workflows run without issues.
  • If the default permissions are set to "Read", the workflows fail because they require write permissions.
  • It may seem that the solution is to simply set "Read and write" as the default permissions in the repository, but this presents two main issues:
    1. Organization owners can prevent you from granting write access to the GITHUB_TOKEN at the repository level. In other words, sometimes we can't even change the default permissions to "Read and write." More information here.
    2. It’s just not recommended. Write permissions will apply to all workflows, which is excessive and could pose a security risk.
  • For these two reasons, assuming the default permissions of the GITHUB_TOKEN are set to "Read", we recommend specifying and overriding the permissions for each workflow file on a more fine-grained basis.

Who/what does this impact?

Repositories with restricted access.

How did you test this?

I tested the workflow in a temporal test repository.

@asdolo asdolo self-assigned this Oct 22, 2024
@asdolo asdolo requested a review from a team as a code owner October 22, 2024 22:59
@github-actions
Copy link
Copy Markdown

github-actions bot commented Oct 22, 2024

💯 Test Coverage

Lines Statements Branches Functions
Coverage: 51%
50.69% (256/505) 46.63% (104/223) 36.12% (69/191)

😎 Tests Results

Tests Skipped Failures Errors Time
68 0 💤 0 ❌ 0 🔥 27.69s ⏱️
👀 Tests Details • (51%)
File% Stmts% Branch% Funcs% LinesUncovered Line #s
All files50.6946.6336.1251.66 
report-only-changed-files is enabled. No files were changed in this commit :)

@asdolo asdolo force-pushed the fix/ci_status_check_workflows_permissions branch from a4cf907 to 84601b0 Compare October 22, 2024 23:20
@asdolo asdolo force-pushed the fix/ci_status_check_workflows_permissions branch from 84601b0 to 4440645 Compare October 22, 2024 23:22
Copy link
Copy Markdown
Collaborator

@fernandatoledo fernandatoledo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description of this PR is super complete! 👏

@asdolo asdolo merged commit 58c8dc1 into master Oct 25, 2024
@asdolo asdolo deleted the fix/ci_status_check_workflows_permissions branch October 25, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants