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

.github: Don't persist credentials in repository #16052

Merged
merged 1 commit into from May 10, 2021

Conversation

pchaigno
Copy link
Member

@pchaigno pchaigno commented May 7, 2021

When using actions/checkout, the default behavior [1] is to persist git credentials in the checked out code. This is ill-advised, so let's disable with persist-credentials.

1 - https://github.com/actions/checkout#usage
2 - https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

@pchaigno pchaigno added area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/ci This PR makes changes to the CI. sig/contributing Impacts contribution workflow, guidelines, and tools. labels May 7, 2021
@pchaigno pchaigno marked this pull request as ready for review May 7, 2021 20:36
@pchaigno pchaigno requested review from a team as code owners May 7, 2021 20:36
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

SGTM. Do we need to test these before we merge them to make sure we're not relying on them?

@pchaigno
Copy link
Member Author

pchaigno commented May 7, 2021

Do we need to test these before we merge them to make sure we're not relying on them?

I'd prefer if we can, yes. But it seems GitHub workflows are on strike in this PR...

.github/workflows/bpf-checks.yaml Outdated Show resolved Hide resolved
@nbusseneau
Copy link
Member

I'd prefer if we can, yes. But it seems GitHub workflows are on strike in this PR...

This is because of the syntax error outlined above: https://github.com/cilium/cilium/actions/runs/821429393

.github/workflows/codeql-analysis.yml Outdated Show resolved Hide resolved
Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM.

When using actions/checkout, the default behavior [1] is to persist git
credentials in the checked out code. This is ill-advised, so let's
disable with persist-credentials.

1 - https://github.com/actions/checkout#usage
2 - https://securitylab.github.com/research/github-actions-preventing-pwn-requests/
Signed-off-by: Paul Chaignon <paul@cilium.io>
@pchaigno pchaigno force-pushed the workflows-dont-persist-credentials branch from 3933658 to 98fe2fb Compare May 10, 2021 10:42
@pchaigno
Copy link
Member Author

Team reviews are in and all GitHub workflows are passing. Marking as ready to merge.

@pchaigno pchaigno added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 10, 2021
@ti-mo ti-mo merged commit b553845 into cilium:master May 10, 2021
@pchaigno pchaigno deleted the workflows-dont-persist-credentials branch May 10, 2021 14:07
@qmonnet qmonnet mentioned this pull request Jun 1, 2021
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI. sig/contributing Impacts contribution workflow, guidelines, and tools.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants