From 4c76db8e7c98e7a9afd21b0ed4123d8ee28509cf Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 20 Aug 2025 17:52:07 -0400
Subject: [PATCH] Make sure `settings.sandboxedPaths` is closed outside
 `DerivationBuilder`

This is a nicer separation of concerns --- `DerivationBuilder` just
mounts the extra paths you tell it too, and the outside world is
responsible for making sure those extra paths make sense.

Since the closure only depends on global settings, and not
per-derivation information, we also have the option of moving this up
further and caching it across all local builds. (I only just realized
this after having done this refactor. I am not doing that change at this
time, however.)
---
 src/libstore/build/derivation-building-goal.cc | 18 ++++++++++++++++++
 .../nix/store/build/derivation-builder.hh      |  8 ++++++++
 src/libstore/unix/build/derivation-builder.cc  | 18 +-----------------
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc
index 965ffa5257b..a82f7f9281d 100644
--- a/src/libstore/build/derivation-building-goal.cc
+++ b/src/libstore/build/derivation-building-goal.cc
@@ -677,9 +677,26 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
             auto * localStoreP = dynamic_cast<LocalStore *>(&worker.store);
             assert(localStoreP);
 
+            decltype(DerivationBuilderParams::defaultPathsInChroot) defaultPathsInChroot = settings.sandboxPaths.get();
             decltype(DerivationBuilderParams::finalEnv) finalEnv;
             decltype(DerivationBuilderParams::extraFiles) extraFiles;
 
+            /* Add the closure of store paths to the chroot. */
+            StorePathSet closure;
+            for (auto & i : defaultPathsInChroot)
+                try {
+                    if (worker.store.isInStore(i.second.source))
+                        worker.store.computeFSClosure(worker.store.toStorePath(i.second.source).first, closure);
+                } catch (InvalidPath & e) {
+                } catch (Error & e) {
+                    e.addTrace({}, "while processing sandbox path '%s'", i.second.source);
+                    throw;
+                }
+            for (auto & i : closure) {
+                auto p = worker.store.printStorePath(i);
+                defaultPathsInChroot.insert_or_assign(p, ChrootPath{.source = p});
+            }
+
             try {
                 if (drv->structuredAttrs) {
                     auto json = drv->structuredAttrs->prepareStructuredAttrs(
@@ -748,6 +765,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                     *drvOptions,
                     inputPaths,
                     initialOutputs,
+                    std::move(defaultPathsInChroot),
                     std::move(finalEnv),
                     std::move(extraFiles),
                 });
diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh
index 301283cdc23..144ca27b12b 100644
--- a/src/libstore/include/nix/store/build/derivation-builder.hh
+++ b/src/libstore/include/nix/store/build/derivation-builder.hh
@@ -59,6 +59,12 @@ struct DerivationBuilderParams
 
     const BuildMode & buildMode;
 
+    /**
+     * Extra paths we want to be in the chroot, regardless of the
+     * derivation we are building.
+     */
+    PathsInChroot defaultPathsInChroot;
+
     struct EnvEntry
     {
         /**
@@ -96,6 +102,7 @@ struct DerivationBuilderParams
         const DerivationOptions & drvOptions,
         const StorePathSet & inputPaths,
         std::map<std::string, InitialOutput> & initialOutputs,
+        PathsInChroot defaultPathsInChroot,
         std::map<std::string, EnvEntry, std::less<>> finalEnv,
         StringMap extraFiles)
         : drvPath{drvPath}
@@ -105,6 +112,7 @@ struct DerivationBuilderParams
         , inputPaths{inputPaths}
         , initialOutputs{initialOutputs}
         , buildMode{buildMode}
+        , defaultPathsInChroot{std::move(defaultPathsInChroot)}
         , finalEnv{std::move(finalEnv)}
         , extraFiles{std::move(extraFiles)}
     {
diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc
index 62af9cd8593..15c99e3c002 100644
--- a/src/libstore/unix/build/derivation-builder.cc
+++ b/src/libstore/unix/build/derivation-builder.cc
@@ -836,29 +836,13 @@ PathsInChroot DerivationBuilderImpl::getPathsInSandbox()
 {
     /* Allow a user-configurable set of directories from the
        host file system. */
-    PathsInChroot pathsInChroot = settings.sandboxPaths.get();
+    PathsInChroot pathsInChroot = defaultPathsInChroot;
 
     if (hasPrefix(store.storeDir, tmpDirInSandbox())) {
         throw Error("`sandbox-build-dir` must not contain the storeDir");
     }
     pathsInChroot[tmpDirInSandbox()] = {.source = tmpDir};
 
-    /* Add the closure of store paths to the chroot. */
-    StorePathSet closure;
-    for (auto & i : pathsInChroot)
-        try {
-            if (store.isInStore(i.second.source))
-                store.computeFSClosure(store.toStorePath(i.second.source).first, closure);
-        } catch (InvalidPath & e) {
-        } catch (Error & e) {
-            e.addTrace({}, "while processing sandbox path '%s'", i.second.source);
-            throw;
-        }
-    for (auto & i : closure) {
-        auto p = store.printStorePath(i);
-        pathsInChroot.insert_or_assign(p, ChrootPath{.source = p});
-    }
-
     PathSet allowedPaths = settings.allowedImpureHostPrefixes;
 
     /* This works like the above, except on a per-derivation level */