Skip to content

feat(ci): substitute from the shared binary cache - #59

Open
nikzen wants to merge 1 commit into
mainfrom
feat/nix-cache
Open

feat(ci): substitute from the shared binary cache#59
nikzen wants to merge 1 commit into
mainfrom
feat/nix-cache

Conversation

@nikzen

@nikzen nikzen commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Every workflow builds the toolchain from source, while the Cachix the rest of the company uses sits unused next to it.

Reading. A step in steps.setup, so every generated workflow gets it. It may fail without failing the workflow: a cache out of reach should cost a run its time and not its result.

Writing. Only main and tags. A pull request and a merge queue entry both build what a branch decided, and a run there that could write would let any branch put a store path in front of every other repository's builds. The ref decides this rather than the event name, which gets two cases wrong: a run started by hand is not a push, and a push is whatever branch a repository chose to run on.

Filling. No extra workflow. The cachix action pushes what a trusted ref already built, so image and web jobs that run on main fill the cache without a job of their own.

Which cache. Named, not hardcoded. Product repositories keep the default famedly. This repository is public, so it takes famedly-oss: GitHub will not hand a private cache's token to a public workflow, and a private cache cannot be read without one. Secrets follow the name: CACHIX_AUTH_TOKEN_<NAME> and CACHIX_SIGNING_KEY_<NAME>, hyphens turned into underscores.

Addresses the review on the previous shape of this PR: the populate workflow is gone, and this public repository no longer talks to famedly.

Test plan

  • prek --all-files --stage pre-push
  • every generated workflow carries the read step
  • skipPush is false only for refs/heads/main and refs/tags/*
  • a run on main in a product repository pushes what it built, and a later pull request substitutes it

Comment thread nix/general/workflows/ci-steps.nix Outdated
Comment thread nix/general/workflows/ci-steps.nix Outdated
Comment thread nix/general/workflows/populate-binary-cache.nix Outdated

@tlater-famedly tlater-famedly left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we need a separate workflow for uploads; the cachix action will implicitly fill the cache with any used artifacts, and the other workflows will be exercising those. Let's reduce the amount of workflow clutter.

#50 already covers enabling cachix, on the other hand, it's just blocked by the fact that our CI secrets don't currently work this way.

Comment thread .github/workflows/populate-binary-cache.yml Outdated
Every workflow built the toolchain from source, while the Cachix the rest
of the company uses sat next to it unused.

The step lives in `steps.setup`, so every generated workflow gets it.
Only `main` and the tags write: a pull request and a merge queue entry
both build what a branch decided, and a run there that could write would
let any branch put a store path in front of every other repository's
builds. The ref decides this rather than the event name, which gets two
cases wrong: a run started by hand is not a `push`, and a `push` is
whatever branch a repository chose to run on.

Reading may fail without failing the workflow. What a trusted ref builds
is pushed by the same step, so a workflow that already runs on `main`
fills the cache without a job of its own.

The cache is named, not hardcoded. Product repositories keep `famedly`.
This one is public, so it takes `famedly-oss`: GitHub will not hand a
private cache's token to a public workflow, and a private cache cannot
be read without one.

Signed-off-by: Niklas Zender <n.zender@famedly.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit ad98ae0. Configure here.

signingKey = "\${{ secrets.CACHIX_SIGNING_KEY_${suffix} }}";

skipPush = "\${{ github.ref != 'refs/heads/main' && !startsWith(github.ref, 'refs/tags/') }}";
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cache credentials leak on PRs

High Severity

skipPush stops the action from uploading, but authToken and signingKey are still passed on every ref. cachix-action authenticates the runner (cachix authtoken persists the token), and later steps run pull-request code that can read those credentials and write to the shared cache — the poisoning path this change is meant to close.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ad98ae0. Configure here.

Comment thread flake.nix
# Public, so the public cache. GitHub will not hand a private cache's
# token to a public workflow, and a private cache cannot be read
# without one.
famedly.standards.ci.binaryCache.name = "famedly-oss";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cache never fills from this repo

Medium Severity

This repository only runs CI on pull_request and merge_group, so skipPush is always true and nothing trusted ever pushes. There is no populate-binary-cache (or other main/tag) job, and the shared step always sets continueOnError, so famedly-oss stays empty and later runs still build from source.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit ad98ae0. Configure here.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants