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

Disable credentials persistence in Github checkout action #3325

Merged
merged 2 commits into from
Nov 1, 2024

Conversation

happz
Copy link
Collaborator

@happz happz commented Oct 29, 2024

According to a couple of articles, the default should be false, but it's not, which makes the token exposed to actions that do not need it. According to a linter I tried just for fun, we should enforce it to close this hole.

[1] actions/checkout#485
[2] https://github.com/woodruffw/zizmor

Pull Request Checklist

  • implement the feature

@happz happz added code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. test coverage Improvements or additions to test coverage of tmt itself labels Oct 29, 2024
@happz happz added this to the 1.39 milestone Oct 29, 2024
.github/workflows/shellcheck.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@psss psss left a comment

Choose a reason for hiding this comment

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

Looks ok, but is this needed if we're not using any token to clone git repositories?

.github/workflows/shellcheck.yml Outdated Show resolved Hide resolved
.github/workflows/pre-commit.yml Show resolved Hide resolved
@happz happz added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Oct 31, 2024
@happz happz force-pushed the github-actions-persist-credentials branch from 7381b97 to 0da6deb Compare October 31, 2024 13:13
@psss psss force-pushed the github-actions-persist-credentials branch from 0da6deb to 09a263e Compare November 1, 2024 10:00
According to a couple of articles, the default should be `false`, but
it's not, which makes the token exposed to actions that do not need it.
According to a linter I tried just for fun, we should enforce it to
close this hole.

[1] actions/checkout#485
[2] https://github.com/woodruffw/zizmor
@psss psss force-pushed the github-actions-persist-credentials branch from 09a263e to 4186dfd Compare November 1, 2024 10:03
@psss
Copy link
Collaborator

psss commented Nov 1, 2024

Checks, which are affected by this change are green, merging.

@psss psss merged commit 79ac2fa into main Nov 1, 2024
7 of 22 checks passed
@psss psss deleted the github-actions-persist-credentials branch November 1, 2024 10:07
@psss psss self-assigned this Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code | no functional change "No Functional Change" intended. Patch should not change tmt's behavior in any way. status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. test coverage Improvements or additions to test coverage of tmt itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants