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.

Sign up for GitHub

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

Jump to bottom

fix(ci): fix secret on CI checkout #1678

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(ci): fix secret on CI checkout #1678

wants to merge 1 commit into from

Conversation

mayeul-zama
Copy link
Contributor

No description provided.

@cla-bot cla-bot bot added the cla-signed label Oct 14, 2024
@mayeul-zama mayeul-zama mentioned this pull request Oct 14, 2024
Merged
IceTDrinker
IceTDrinker reviewed Oct 21, 2024
View reviewed changes
Copy link
Member

@IceTDrinker IceTDrinker left a comment

Choose a reason for hiding this comment

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

check the least access rule here when possible

@soonum
Copy link
Contributor

soonum commented Oct 22, 2024

check the least access rule here when possible

I have an idea regarding that.

  1. replacing all usage of FHE_ACTIONS_TOKEN for downloading Slab repo to SLAB_ACTIONS_TOKEN
  2. reducing permissions to read-only whenever possible for FHE_ACTIONS_TOKEN
  3. creating a new token for the releases since it needs a write permission in the repo

The second step will need careful analysis to avoid side-effect that could pop-up later.

@IceTDrinker
Copy link
Member

Ok, should this be merged and then what you propose done later ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IceTDrinker IceTDrinker IceTDrinker left review comments

@soonum soonum Awaiting requested review from soonum

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
cla-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mayeul-zama @soonum @IceTDrinker