From a712445a7a48a00148d0f2b41917c03d7259dabc Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 20 Aug 2025 12:21:42 -0400
Subject: [PATCH] Make `Settings::sandboxPaths` well-typed

Parsing logic is moved from `DerivationBuilder`, where is doesn't
belong, to `Settings` itself, where it does.
---
 src/libstore/globals.cc                       | 59 ++++++++++++++++++-
 src/libstore/include/nix/store/globals.hh     | 16 ++++-
 src/libstore/unix/build/derivation-builder.cc | 18 +-----
 3 files changed, 73 insertions(+), 20 deletions(-)

diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc
index 966d37090..612e79ab0 100644
--- a/src/libstore/globals.cc
+++ b/src/libstore/globals.cc
@@ -86,13 +86,22 @@ Settings::Settings()
     }
 
 #if (defined(__linux__) || defined(__FreeBSD__)) && defined(SANDBOX_SHELL)
-    sandboxPaths = tokenizeString<StringSet>("/bin/sh=" SANDBOX_SHELL);
+    sandboxPaths = {{"/bin/sh", {.source = SANDBOX_SHELL}}};
 #endif
 
     /* chroot-like behavior from Apple's sandbox */
 #ifdef __APPLE__
-    sandboxPaths = tokenizeString<StringSet>(
-        "/System/Library/Frameworks /System/Library/PrivateFrameworks /bin/sh /bin/bash /private/tmp /private/var/tmp /usr/lib");
+    for (PathView p : {
+             "/System/Library/Frameworks",
+             "/System/Library/PrivateFrameworks",
+             "/bin/sh",
+             "/bin/bash",
+             "/private/tmp",
+             "/private/var/tmp",
+             "/usr/lib",
+         }) {
+        sandboxPaths.get().insert_or_assign(std::string{p}, ChrootPath{.source = std::string{p}});
+    }
     allowedImpureHostPrefixes = tokenizeString<StringSet>("/System/Library /usr/lib /dev /bin/sh");
 #endif
 }
@@ -317,6 +326,42 @@ void BaseSetting<SandboxMode>::convertToArg(Args & args, const std::string & cat
     });
 }
 
+NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(ChrootPath, source, optional)
+
+template<>
+PathsInChroot BaseSetting<PathsInChroot>::parse(const std::string & str) const
+{
+    PathsInChroot pathsInChroot;
+    for (auto i : tokenizeString<StringSet>(str)) {
+        if (i.empty())
+            continue;
+        bool optional = false;
+        if (i[i.size() - 1] == '?') {
+            optional = true;
+            i.pop_back();
+        }
+        size_t p = i.find('=');
+        if (p == std::string::npos)
+            pathsInChroot[i] = {.source = i, .optional = optional};
+        else
+            pathsInChroot[i.substr(0, p)] = {.source = i.substr(p + 1), .optional = optional};
+    }
+    return pathsInChroot;
+}
+
+template<>
+std::string BaseSetting<PathsInChroot>::to_string() const
+{
+    std::vector<std::string> accum;
+    for (auto & [name, cp] : value) {
+        std::string s = name == cp.source ? name : name + "=" + cp.source;
+        if (cp.optional)
+            s += "?";
+        accum.push_back(std::move(s));
+    }
+    return concatStringsSep(" ", accum);
+}
+
 unsigned int MaxBuildJobsSetting::parse(const std::string & str) const
 {
     if (str == "auto")
@@ -329,6 +374,14 @@ unsigned int MaxBuildJobsSetting::parse(const std::string & str) const
     }
 }
 
+template<>
+void BaseSetting<PathsInChroot>::appendOrSet(PathsInChroot newValue, bool append)
+{
+    if (!append)
+        value.clear();
+    value.insert(std::make_move_iterator(newValue.begin()), std::make_move_iterator(newValue.end()));
+}
+
 static void preloadNSS()
 {
     /* builtin:fetchurl can trigger a DNS lookup, which with glibc can trigger a dynamic library load of
diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh
index 2ac4678e7..2cd92467c 100644
--- a/src/libstore/include/nix/store/globals.hh
+++ b/src/libstore/include/nix/store/globals.hh
@@ -24,6 +24,20 @@ SandboxMode BaseSetting<SandboxMode>::parse(const std::string & str) const;
 template<>
 std::string BaseSetting<SandboxMode>::to_string() const;
 
+template<>
+PathsInChroot BaseSetting<PathsInChroot>::parse(const std::string & str) const;
+template<>
+std::string BaseSetting<PathsInChroot>::to_string() const;
+
+template<>
+struct BaseSetting<PathsInChroot>::trait
+{
+    static constexpr bool appendable = true;
+};
+
+template<>
+void BaseSetting<PathsInChroot>::appendOrSet(PathsInChroot newValue, bool append);
+
 struct MaxBuildJobsSetting : public BaseSetting<unsigned int>
 {
     MaxBuildJobsSetting(
@@ -698,7 +712,7 @@ public:
         )",
         {"build-use-chroot", "build-use-sandbox"}};
 
-    Setting<PathSet> sandboxPaths{
+    Setting<PathsInChroot> sandboxPaths{
         this,
         {},
         "sandbox-paths",
diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc
index cef2340dd..f6546ec62 100644
--- a/src/libstore/unix/build/derivation-builder.cc
+++ b/src/libstore/unix/build/derivation-builder.cc
@@ -857,24 +857,10 @@ void DerivationBuilderImpl::startBuilder()
 
 PathsInChroot DerivationBuilderImpl::getPathsInSandbox()
 {
-    PathsInChroot pathsInChroot;
-
     /* Allow a user-configurable set of directories from the
        host file system. */
-    for (auto i : settings.sandboxPaths.get()) {
-        if (i.empty())
-            continue;
-        bool optional = false;
-        if (i[i.size() - 1] == '?') {
-            optional = true;
-            i.pop_back();
-        }
-        size_t p = i.find('=');
-        if (p == std::string::npos)
-            pathsInChroot[i] = {.source = i, .optional = optional};
-        else
-            pathsInChroot[i.substr(0, p)] = {.source = i.substr(p + 1), .optional = optional};
-    }
+    PathsInChroot pathsInChroot = settings.sandboxPaths.get();
+
     if (hasPrefix(store.storeDir, tmpDirInSandbox())) {
         throw Error("`sandbox-build-dir` must not contain the storeDir");
     }