Skip to content

workflows/main: re-enable read-only cachix for untrusted builds#398

Open
MattSturgeon wants to merge 1 commit into
NixOS:masterfrom
MattSturgeon:enable-cachix
Open

workflows/main: re-enable read-only cachix for untrusted builds#398
MattSturgeon wants to merge 1 commit into
NixOS:masterfrom
MattSturgeon:enable-cachix

Conversation

@MattSturgeon
Copy link
Copy Markdown
Contributor

@MattSturgeon MattSturgeon commented May 21, 2026

This partially reverts 903898a from #393.

Cachix is configured on all runs, but the authToken is only configured on push events to avoid exposing it to untrusted PR code.

cc @mdaniels5757

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 21, 2026

Nixpkgs diff

name: nixos-nixfmt
authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}'
authToken: ${{ github.event_name == 'push' && secrets.CACHIX_AUTH_TOKEN || '' }}
skipPush: ${{ github.event_name != 'push' }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The fact that this boolean expression is not exactly the opposite of the expression for authToken makes my brain itch.

Do we need this here because an empty authToken means something different than no auth token at all? My reading of the relevant code is that setting authToken to empty string will cause us to skip pushes (https://github.com/cachix/cachix-action/blob/5f2d7c5294214f71b873db4b969586b980625e71/src/main.ts#L113).

Or is this just here to be explicit? If so, that seems fine to me, but I'd consider making the expression the boolean opposite of theauthToken one just to reduce this "brain itch" it's giving me.

Copy link
Copy Markdown
Contributor Author

@MattSturgeon MattSturgeon May 21, 2026

Choose a reason for hiding this comment

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

Or is this just here to be explicit?

Yes. An empty/missing authToken does mean we can't write to cachix. But I wasn't sure if not setting skipPush explicitly would fail to push instead of gracefully skipping the push.

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

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants