Skip to content

feat(nix): keep preset store and state dirs - #7581

Open
ojsef39 wants to merge 1 commit into
containerbase:mainfrom
JHOFER-Cloud:fix/nix-keep-preset-dirs
Open

ojsef39 wants to merge 1 commit into
containerbase:mainfrom
JHOFER-Cloud:fix/nix-keep-preset-dirs

Conversation

@ojsef39

@ojsef39 ojsef39 commented Oct 1, 2026 •

Copy link
Copy Markdown

Changes

The nix wrapper now only defaults NIX_STORE_DIR, NIX_DATA_DIR, NIX_LOG_DIR, NIX_STATE_DIR and NIX_CONF_DIR to 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 the nix-update manager 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 sed tools/v2/nix.sh to work around this. The wrapper is now generated by the CLI at install time, and the exports come after the tool env.sh, so there is no way left to override them from the image.

Please select one of the following:

  • This closes an existing Issue, Closes: Move Nix tool store dir #7339
  • This doesn't close an Issue, but I accept the risk that this PR may be closed if maintainers disagree with its opening or implementation

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.

  • No — I did not use AI for this contribution.
  • Yes — minimal assistance (e.g., IDE autocomplete, small code completions, grammar fixes). -> Opus 5.5
  • Yes — substantive assistance (AI-generated non‑trivial portions of code, tests, or documentation).
  • Yes — other (please describe):

Use of AI in replying to PR comments

Who answers review comments:

  • @username will read and reply directly. Name the account.
  • An agent will draft replies and @username will read them before they are posted. Name the account.
  • An agent will draft replies and reply autonomously. This is heavily discouraged, and we prefer that there are humans in the loop
  • Nobody has explicitly committed to replying.

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified tests

Summary by CodeRabbit

  • Improvements
    • Nix setup now respects existing values for store, data, log, state, and configuration directories. When any of these settings are unset or empty, it uses the corresponding containerbase cache directory instead.
    • Nix documentation now clarifies that a writable /nix directory can host the store and that binary caches serve /nix/store.

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.
@github-actions
github-actions Bot requested a review from viceice October 1, 2026 15:03
@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: Repository: containerbase/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 562d298d-52d2-4793-97de-2bbeda80e001

📥 Commits

Reviewing files that changed from the base of the PR and between f9558e2 and 985ad34.

📒 Files selected for processing (3)
  • src/cli/tools/nix.spec.ts
  • src/cli/tools/nix.ts
  • test/nix/Dockerfile

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The 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.

Changes

Nix store directory handling

Layer / File(s) Summary
Preserve Nix directory environment values
src/cli/tools/nix.ts, src/cli/tools/nix.spec.ts
The link command uses existing values for all five Nix directory variables and falls back to cache paths when values are unset or empty. Documentation and test expectations describe this behavior.
Verify Nix store paths in Docker
test/nix/Dockerfile
Docker tests check that the store path is not /nix/store without preset directories and is /nix/store when store, state, and log directories are set. The final image copies the test stage marker.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature · Severity of issue fixed: Medium

Suggested reviewers: viceice

Merge Risk: ⚪ Minimal · up to 985ad

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 Review

Security architecture risk: 🔵 Low · up to 985ad

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
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The immediate exposure is Nix processes using regenerated wrappers and the store, state, data, log, and configuration paths selected by their environment. Impact on other jobs or assets depends on runtime identity and shared filesystem permissions; tenant and production deployment scope are not established.

Security Findings and Attack Paths

  • inferred — Environment values gain reachability to Nix directory configuration, but the inspected path does not establish command injection or privilege escalation. Nix supplies fixed shell assignment templates, whose expanded environment contents are not recursively parsed as commands. Exploitation of a privileged caller accepting lower-trust settings remains unproven because production invocation evidence is unavailable.

Trust Boundaries and Controls

  • observed — The generated wrapper invokes initialization and Nix directly, without a privilege-switching command. The preset-path fixture uses a non-root identity and explicitly provisions ownership, providing counterevidence to authority acquisition in that tested arrangement.

Hardening Proposals

  • proposed — For deployments adopting external Nix directories, define directory ownership, sharing, and recovery policy explicitly, and avoid allowing lower-trust jobs to select configuration or state paths used by a more privileged identity.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #7339 requires a Nix store that can use the canonical /nix/store path or an equivalent local-store solution. NixInstallService.link now preserves preset NIX_STORE_DIR, NIX_DATA_DIR, `NIX…
Out of Scope Changes check ✅ Passed The changes are limited to Nix directory export behavior, related unit coverage, and Nix integration coverage. The documentation update explains the binary-cache use case. These changes directly suppo…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: preserving preset Nix store and state directories while retaining cache-based defaults when variables are unset.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

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.

Move Nix tool store dir

1 participant