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

Workflow security fixes #351446

Merged
merged 3 commits into from
Oct 26, 2024
Merged

Workflow security fixes #351446

merged 3 commits into from
Oct 26, 2024

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Oct 26, 2024

At NixCon we've had 13x1 and @basti564 report security problems with some Nixpkgs workflows, thank you! See the commit messages and contents for more details.

I explained these changes to @Mic92, so that he could immediately merge it.

We disabled the old compromised workflows, so that they wouldn't run anymore for all branches.

Ping @NixOS/security


Add a 👍 reaction to pull requests you find important.

infinisil and others added 3 commits October 26, 2024 15:01
Co-Authored-By: 13x1 <tori@disroot.org>
Co-Authored-By: basti564 <e3e@disroot.org>
read-all permissions gives access to e.g. security-events, which these
don't need, and can easily lead to leaks

Co-Authored-By: 13x1 <tori@disroot.org>
Co-Authored-By: basti564 <e3e@disroot.org>
In the previous two commits, security issues with these workflows were
fixed. In order for these to not be exploitable for PRs to branches that
don't have the fixes yet (including read-only branches like
nixos-unstable), these workflows are renamed, so that the old ones can
be turned off manually via GitHub interface.

Co-Authored-By: 13x1 <tori@disroot.org>
Co-Authored-By: basti564 <e3e@disroot.org>
@infinisil infinisil requested a review from Mic92 October 26, 2024 14:20
@Mic92 Mic92 merged commit 88bcaef into NixOS:master Oct 26, 2024
3 of 6 checks passed
@infinisil infinisil deleted the workflow-security-fixes branch October 26, 2024 14:21
infinisil added a commit that referenced this pull request Oct 26, 2024
yorickvP pushed a commit that referenced this pull request Oct 26, 2024
After #351446

(cherry picked from commit cd691f8)
@LeSuisse
Copy link
Contributor

LeSuisse commented Oct 26, 2024

Thanks for the ping and backports. Does the app credential have been rotated?

(And arghhhhhh GHA are minefield for anything non trivial :/ )

@infinisil
Copy link
Member Author

@LeSuisse No need to, the private key couldn't be leaked, while the tokens generated from it are revoked after the workflow completes, and if something went wrong in 1 hour at the latest.

@infinisil
Copy link
Member Author

And yeah GitHub Actions are a tricky beast, I'm very thankful to have been made aware of these problems!

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.

3 participants