Skip to content

[MAJC-130] Restrict permissions for github actions workflows#26

Merged
mashalifshin merged 1 commit intomainfrom
majc-130-security-audit-github-actions-permissions
May 6, 2025
Merged

[MAJC-130] Restrict permissions for github actions workflows#26
mashalifshin merged 1 commit intomainfrom
majc-130-security-audit-github-actions-permissions

Conversation

@mashalifshin
Copy link
Copy Markdown
Contributor

@mashalifshin mashalifshin commented May 5, 2025

References

MAJC-130

Problem Statement

Address CodeQL scan results results that noticed we have unrestricted workflow permissions.

Proposed Changes

"Practice the Principle of Least Privilege" and give the github action's token the least possible permissions in the repo when running actions. It's also recommended generally in the guide to hardening Github actions, and we got 0/10 on this metric from the OSSF scorecard.

Verification Steps

  • Take a look at the diff and actions runs and make sure everything looks good there

@mashalifshin mashalifshin requested a review from a team as a code owner May 5, 2025 22:10
@cyptm cyptm self-assigned this May 5, 2025
@cyptm cyptm self-requested a review May 5, 2025 23:39
@cyptm
Copy link
Copy Markdown

cyptm commented May 5, 2025

@mashalifshin I'm not clear how the read permissions we've added are "restricting" permissions.

  contents: read
  pull-requests: write

Were the permissions by default "write permissions" or what am I missing?

@mashalifshin
Copy link
Copy Markdown
Contributor Author

@mashalifshin I'm not clear how the read permissions we've added are "restricting" permissions.

  contents: read
  pull-requests: write

Were the permissions by default "write permissions" or what am I missing?

Thank you for the speedy reviews @cyptm! 🙌

Great question! So this was flagged by an automated code scanning tool, CodeQL, and if you go to this link and click on "Show More" it will give a really good explanation, but TLDR is that if you don't specify this on jobs/workflows, then permissions are inherited from the repo, and if repo doesn't have them, then permissions come from the org. And until about 2 years ago, orgs were created with default read-write permissions ... so here we are. :-)

More great details about why this matters and how it can be a security risk from the security audit tool OSSF

@cyptm
Copy link
Copy Markdown

cyptm commented May 5, 2025

@mashalifshin I'm not clear how the read permissions we've added are "restricting" permissions.

  contents: read
  pull-requests: write

Were the permissions by default "write permissions" or what am I missing?

Thank you for the speedy reviews @cyptm! 🙌

Great question! So this was flagged by an automated code scanning tool, CodeQL, and if you go to this link and click on "Show More" it will give a really good explanation, but TLDR is that if you don't specify this on jobs/workflows, then permissions are inherited from the repo, and if repo doesn't have them, then permissions come from the org. And until about 2 years ago, orgs were created with default read-write permissions ... so here we are. :-)

More great details about why this matters and how it can be a security risk from the security audit tool OSSF

Makes total sense, thanks for the clarification @mashalifshin

@mashalifshin mashalifshin force-pushed the majc-130-security-audit-github-actions-permissions branch from 930f53c to fc48f02 Compare May 5, 2025 23:59
@mashalifshin mashalifshin merged commit 0dc5ca0 into main May 6, 2025
7 checks passed
@mashalifshin mashalifshin deleted the majc-130-security-audit-github-actions-permissions branch May 6, 2025 00:09
Almaju pushed a commit to Almaju/majc that referenced this pull request Sep 22, 2025
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.

2 participants