Conversation
Only default NIX_STORE_DIR, NIX_DATA_DIR, NIX_LOG_DIR, NIX_STATE_DIR and NIX_CONF_DIR to the containerbase cache when they are not already set, so an image that provides a writable /nix can keep nix on /nix/store and use binary caches.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: containerbase/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe Nix link command preserves existing Nix directory environment values and uses cache paths when they are unset or empty. Docker tests check Nix store behavior with default and preset directories. ChangesNix store directory handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change allows images to preset Nix directories while retaining cache-based defaults. The Docker checks exercise the normal install path, and no material merge-blocking issue was identified. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change preserves configured Nix directories without visibly granting additional privileges. Risk depends on who supplies those settings and owns the selected directories; production caller identities and shared-directory isolation remain unverified. Retained concerns Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Changes
The nix wrapper now only defaults
NIX_STORE_DIR,NIX_DATA_DIR,NIX_LOG_DIR,NIX_STATE_DIRandNIX_CONF_DIRto the containerbase cache when they aren't already set (${NIX_STORE_DIR:-<cache>/store}etc.), as suggested in #7339.Context
Binary caches only serve paths under
/nix/store, because the store prefix is part of every store path's hash. A nix whose store lives below the cache can't substitute anything, so anything that has to realise a store path builds it from source, starting with the whole stdenv bootstrap:warning: binary cache 'https://cache.nixos.org' is for Nix stores with prefix '/nix/store', not '/tmp/containerbase/cache/nix/store'nix flake update, which is what Renovate's nix manager runs today, doesn't hit this. Anything that builds or fetches through nix does. My use case is a Renovate fork that computes fixed-output hashes after a version bump, which is thenix-updatemanager linked in #7339. I've thought about upstreaming that manager, but I don't think it's stable enough yet to propose. This change lets the fork run on the stock base image in the meantime, and makes it possible for anyone else who needs nix to actually build or fetch through it.Until #7446 an image could
sedtools/v2/nix.shto work around this. The wrapper is now generated by the CLI at install time, and the exports come after the toolenv.sh, so there is no way left to override them from the image.Please select one of the following:
AI assistance disclosure
Did you use AI tools to create any part of this pull request?
Please select one option and, if yes, briefly describe how AI was used (e.g., code, tests, docs) and which tool(s) you used.
Use of AI in replying to PR comments
Who answers review comments:
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via:
Summary by CodeRabbit
/nixdirectory can host the store and that binary caches serve/nix/store.