Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(OSSF): update token permissions to improve ossf scorecard #763

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

laukik-target
Copy link
Contributor

@laukik-target laukik-target commented Oct 26, 2024

Description:

This PR aims to improve the repository's security posture by adhering to the principle of least privilege and enhancing the OSSF Scorecard score. The following changes have been made to reduce excessive permissions in GitHub Actions workflows and ensure a more secure CI/CD pipeline.

A small fix related to bringing up the git proxy locally is also fixed.

Issue: Bump OSSF score above 9.0 ⬆️

Should improve this score:
image

Changes:

1. Updated token permissions: Scoped permissions at both the top-level and job level for workflows such as:

  • ci.yml: Restricted permissions to only contents: read, statuses: write as necessary.
  • codeql.yml: Added top-level permissions (contents: read, security-events: write) to ensure CodeQL can upload security events.
  • npm.yml: Scoped permissions to packages: write for publishing, with only contents: read for other operations.
  • pr-lint.yml: Limited PR-related workflows to pull-requests: write and statuses: write to reduce unnecessary permissions.
  • scorecard.yml: Scoped permissions to security-events: write and id-token: write for result publication and scorecard analysis.

2. Enhanced security:

  • Applied the principle of least privilege to limit the scope of token access.
  • Updated all workflows to ensure they use only the permissions necessary for their tasks, preventing unnecessary access to other GitHub components.

3. Improved OSSF Scorecard rating:

  • These changes directly address issues raised by the OSSF Scorecard tool, enhancing the repository’s security rating by ensuring token permissions follow best security practices.

Benefits:

  • Reduced risk of unauthorized access due to overly permissive tokens.
  • Enhanced overall security posture by minimizing the attack surface in workflows.
  • Improved transparency and maintainability by explicitly defining token permissions in each workflow.
  • Positive impact on OSSF Scorecard score.

Checklist:
✅Updated workflows with restrictive token permissions.
✅Addressed OSSF Scorecard recommendations for token permissions.

Copy link

linux-foundation-easycla bot commented Oct 26, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link

netlify bot commented Oct 26, 2024

Deploy Preview for endearing-brigadeiros-63f9d0 canceled.

Name Link
🔨 Latest commit 2a87487
🔍 Latest deploy log https://app.netlify.com/sites/endearing-brigadeiros-63f9d0/deploys/675af4adb1a7ed0008676b9b

@laukik-target laukik-target marked this pull request as ready for review October 26, 2024 19:41
@laukik-target
Copy link
Contributor Author

@JamieSlome I am a citi Employee, I Sent an authorization request as a Corporate Contributor for EasyCLA. Can you please approve it?

@JamieSlome
Copy link
Member

@laukik-target - great PR ❤️ Can you re-open using commits created with your Citi e-mail address?

@JamieSlome JamieSlome self-requested a review October 29, 2024 08:05
@laukik-target
Copy link
Contributor Author

@laukik-target - great PR ❤️ Can you re-open using commits created with your Citi e-mail address?

I guess, It is good to merge now.

package.json Outdated Show resolved Hide resolved
@laukik-target
Copy link
Contributor Author

@JamieSlome Changes were done. Requesting for re-review & approval.

@laukik-target
Copy link
Contributor Author

laukik-target commented Nov 6, 2024

@JamieSlome Can you please re-review and approve the PR

Copy link

codecov bot commented Nov 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.11%. Comparing base (3f355df) to head (2a87487).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #763   +/-   ##
=======================================
  Coverage   63.11%   63.11%           
=======================================
  Files          47       47           
  Lines        1681     1681           
=======================================
  Hits         1061     1061           
  Misses        620      620           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JamieSlome
Copy link
Member

JamieSlome commented Nov 8, 2024

@laukik-target - are we able to re-base all of your 10 commits to use your Citi e-mail address instead? All 10 commits should use your public Citi e-mail address instead of the current Gmail address 👍 Let me know if you need help on how to do this.

@laukik-target laukik-target force-pushed the main branch 3 times, most recently from 3be9c3c to 974459d Compare November 8, 2024 13:23
@laukik-target
Copy link
Contributor Author

@laukik-target - are we able to re-base all of your 10 commits to use your Citi e-mail address instead? All 10 commits should use your public Citi e-mail address instead of the current Gmail address 👍 Let me know if you need help on how to do this.

@JamieSlome I squashed my commit to single one for clear understanding.
And updated my email to citi email for my single commit but now EasyCLA check has failed.

Please help

@laukik-target
Copy link
Contributor Author

@JamieSlome
PR is ready to merge now.
please check.

fix: update npm start script to run locally

revert changes for npm start and remove packages write permission
@laukik-target
Copy link
Contributor Author

@JamieSlome Can you please merge this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants