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 965ffa525..a82f7f928 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 301283cdc..144ca27b1 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 62af9cd85..15c99e3c0 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 */