sandbox-paths: rewrite read-only paths to use json config format

SimSaladin · SimSaladin · commit 97c779ba841c · 2025-06-13T17:07:08.000+02:00
Signed-off-by: Samuli Thomasson &lt;samuli.thomasson@pm.me&gt;
diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc
@@ -86,12 +86,12 @@ Settings::Settings()
     }
 
 #if (defined(__linux__) || defined(__FreeBSD__)) && defined(SANDBOX_SHELL)
-    sandboxPaths = tokenizeString<StringSet>("/bin/sh=" SANDBOX_SHELL);
+    sandboxPaths = SandboxPaths { { "/bin/sh", SandboxPath(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");
+    sandboxPaths.setDefault("/System/Library/Frameworks /System/Library/PrivateFrameworks /bin/sh /bin/bash /private/tmp /private/var/tmp /usr/lib");
     allowedImpureHostPrefixes = tokenizeString<StringSet>("/System/Library /usr/lib /dev /bin/sh");
 #endif
 }
@@ -296,6 +296,94 @@ template<> void BaseSetting<SandboxMode>::convertToArg(Args & args, const std::s
     });
 }
 
+NLOHMANN_JSON_SERIALIZE_ENUM(SandboxPath::MountOpt, {
+    {SandboxPath::MountOpt::ro,          "ro"},
+#ifdef __linux__
+    {SandboxPath::MountOpt::nodev,       "nodev"},
+    {SandboxPath::MountOpt::noexec,      "noexec"},
+    {SandboxPath::MountOpt::nosuid,      "nosuid"},
+    {SandboxPath::MountOpt::noatime,     "noatime"},
+    {SandboxPath::MountOpt::nodiratime,  "nodiratime"},
+    {SandboxPath::MountOpt::relatime,    "relatime"},
+    {SandboxPath::MountOpt::strictatime, "strictatime"},
+    {SandboxPath::MountOpt::private_,    "private"},
+    {SandboxPath::MountOpt::slave,       "slave"},
+    {SandboxPath::MountOpt::unbindable,  "unbindable"},
+#endif
+});
+
+NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE_WITH_DEFAULT(SandboxPath, source, optional, readOnly, options);
+
+/**
+ * Parses either old (strings) or new (json object) format sandbox-paths.
+ */
+SandboxPaths SandboxPath::parse(const std::string_view & str, const std::string & ctx)
+{
+    SandboxPaths res;
+
+    auto add = [&](std::string target, SandboxPath v) {
+        if (target == "")
+            throw UsageError("setting '%s' is an object whose keys are paths and paths cannot be empty", ctx);
+        target = canonPath(std::move(target));
+        if (v.source == "") v.source = target;
+        if (!res.try_emplace(target, std::move(v)).second)
+            throw UsageError("Sandbox path declared twice in '%s': %s", ctx, target);
+    };
+
+    if (str.starts_with('{')) {
+        for (auto & [k, v] : nlohmann::json::parse(str, nullptr, false, true).template get<SandboxPaths>())
+            add(k, std::move(v));
+    } else {
+        /* Parses legacy format sandbox-path e.g. "path[=source][?]".
+         * This format supports only a subset of options available with JSON format. */
+        for (std::string_view s : tokenizeString<Strings>(str)) {
+            bool optional = s.ends_with('?');
+            if (optional) s.remove_suffix(1);
+            if (size_t eq = s.find('='); eq != s.npos) {
+                add(std::string(s, 0, eq), { std::string(s.data() + eq + 1, s.size() - eq - 1), optional });
+            } else
+                add(std::string(s), { "", optional });
+        }
+    }
+    return res;
+}
+
+template<> SandboxPaths BaseSetting<SandboxPaths>::parse(const std::string & str) const
+{
+    return SandboxPath().parse(str, this->name);
+}
+
+template<> struct BaseSetting<SandboxPaths>::trait
+{
+    static constexpr bool appendable = true;
+};
+
+/* Omits keys that are set to their default values. */
+template<> std::string BaseSetting<SandboxPaths>::to_string() const
+{
+    if (value.empty())
+        return "";
+    nlohmann::json res = nlohmann::json::object();
+    for (const auto & [k, v] : value) {
+        auto po = nlohmann::json::object();
+        if (v.source != "" && v.source != k) po.emplace("source", v.source);
+        if (v.optional) po.emplace("optional", v.optional);
+        if (v.readOnly) po.emplace("readOnly", v.readOnly);
+        if (!v.options.empty()) po.emplace("options", v.options);
+        res.emplace(k, std::move(po));
+    }
+    return res.dump();
+}
+
+template<> void BaseSetting<SandboxPaths>::appendOrSet(SandboxPaths newValue, bool append)
+{
+    if (!append) value.clear();
+    for (auto & [k, v] : newValue)
+        value.insert_or_assign(std::move(k), std::move(v));
+}
+
+template class BaseSetting<SandboxPaths>;
+
 unsigned int MaxBuildJobsSetting::parse(const std::string & str) const
 {
     if (str == "auto") return std::max(1U, std::thread::hardware_concurrency());
diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh
@@ -5,6 +5,9 @@
 #include <limits>
 
 #include <sys/types.h>
+#ifdef __linux__
+#include <sys/mount.h>
+#endif
 
 #include "nix/util/types.hh"
 #include "nix/util/configuration.hh"
@@ -18,6 +21,58 @@ namespace nix {
 
 typedef enum { smEnabled, smRelaxed, smDisabled } SandboxMode;
 
+struct SandboxPath;
+using SandboxPaths = std::map<Path, SandboxPath, std::less<>>;
+struct SandboxPath
+{
+    typedef enum {
+#ifdef __linux__
+        ro = MS_RDONLY,
+        nodev = MS_NODEV,
+        noexec = MS_NOEXEC,
+        nosuid = MS_NOSUID,
+        noatime = MS_NOATIME,
+        nodiratime = MS_NODIRATIME,
+        relatime = MS_RELATIME,
+        strictatime = MS_STRICTATIME, /* overrides any atime/relatime */
+        private_ = MS_PRIVATE,
+        slave = MS_SLAVE,
+        unbindable = MS_UNBINDABLE
+#else
+        ro // FIXME: do any options make sense on other that linux?
+#endif
+    } MountOpt;
+
+#ifdef __linux__
+    /* Options to set when readOnly=true */
+    static constexpr std::array<MountOpt, 5> readOnlyDefaults = {
+        ro, nodev, noexec, nosuid, noatime
+    };
+#endif
+
+    Path source;
+
+    /**
+     * Ignore path if source is missing.
+     */
+    bool optional;
+
+    /**
+     * Enables MS_RDONLY, NODEV, NOSUID, NOEXEC and NOATIME. You can get finer
+     * control with 'options' instead.
+     * */
+    bool readOnly;
+
+    std::vector<MountOpt> options;
+
+    SandboxPath(const Path & source = "",
+        bool optional = false, bool readOnly = false, const std::vector<MountOpt> & options = { })
+        : source(source), optional(optional), readOnly(readOnly), options(options) { };
+    SandboxPath(const char * source) : SandboxPath(Path(source)) { };
+
+    static SandboxPaths parse(const std::string_view & str, const std::string& = "(unknown)");
+};
+
 struct MaxBuildJobsSetting : public BaseSetting<unsigned int>
 {
     MaxBuildJobsSetting(Config * options,
@@ -629,24 +684,76 @@ public:
         )",
         {"build-use-chroot", "build-use-sandbox"}};
 
-    Setting<PathSet> sandboxPaths{
+    Setting<SandboxPaths> sandboxPaths{
         this, {}, "sandbox-paths",
         R"(
-          A list of paths bind-mounted into Nix sandbox environments. Use the
-          syntax `target[=source][:ro][?]` to control the mount:
+          Paths to bind-mount into Nix sandbox environments.
+          Two syntaxes can be used:
+
+          1. Original (old) syntax: Strings separated by whitespace. Entries
+             are parsed as `TARGET[=SOURCE][?]`. Only the `TARGET` path is
+             required.
+
+             `SOURCE` can be set following an equals sign (`=`) to specify a
+             different source path (the value of `TARGET` is used by default
+             for the source path as well). For instance, `/bin=/nix-bin` would
+             mount path `/nix-bin` in `/bin` inside the sandbox.
+
+             A `?` suffix can be used to make it not an error if the `SOURCE`
+             path does not exist. Without it an error is raised for an
+             unavailable path. For instance, `/dev/nvidiactl?` specifies that
+             `/dev/nvidiactl` will only be mounted in the sandbox if it exists
+             in the host filesystem.
+
+          2. JSON syntax (new): Using this form more configurable settings
+             become available. All paths are specified in a single JSON object
+             so that every key is a target path inside the sandbox and the
+             corresponding values can contain additional (platform-specific)
+             settings.
+
+          For instance:
+
+          ```nix
+          sandbox-paths = {"/bin/sh":{}}                         # /bin/sh
+          sandbox-paths = {"/bin/sh":{"source":"/usr/bin/bash"}} # /bin/sh=/usr/bin/bash
+          sandbox-paths = {"/etc/nix/netrc":{"optional":true}}   # /etc/nix/netrc?
+          ```
+
+          Additional per-path options are available on Linux:
+
+          - `readOnly` (boolean)
 
-          - `=source` will mount a different path at target location; for
-            instance, `/bin=/nix-bin` will mount the path `/nix-bin` as `/bin`
-            inside the sandbox.
+            When this is `true`, the bind-mount is made read-only and
+            additional mount-point flags are enabled. In particular these
+            options are enabled by this flag: `ro`, `nosuid`, `nodev`,
+            `noexec` and `noatime`.
 
-          - `:ro` makes the mount read-only (Linux only).
+          - `options` (string array)
 
-          - `?` makes it not an error if *source* does not exist; for example,
-            `/dev/nvidiactl?` specifies that `/dev/nvidiactl` will only be
-            mounted in the sandbox if it exists in the host filesystem.
+            This setting can be used to add/modify (some) mount(-point) flags
+            directly. In addition to flags used by `readOnly` the following
+            flags can also be used: `nodiratime`, `relatime`, `strictatime`,
+            `unbindable`, `private`, `slave`.
 
-          If the source is in the Nix store, then its closure will be added to
-          the sandbox as well.
+          Full example:
+
+          ```nix
+          sandbox-paths = {
+            "/path/to" : {
+              "source"   : "/path/from",                  # ()
+              "optional" : true,                          # (false)
+              "readOnly" : true,                          # (false)
+              "options"  : [ "optionA", "optionB", ... ], # ()
+            },
+
+            # ...
+          }
+          ```
+
+          > **Note:**
+          >
+          > If the source is in the Nix store, then its closure will
+          > be added to the sandbox as well.
 
           Depending on how Nix was built, the default value for this option
           may be empty or provide `/bin/sh` as a bind-mount of `bash`.
diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc
@@ -105,15 +105,8 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder
     /**
      * Stuff we need to pass to initChild().
      */
-    struct ChrootPath {
-        Path source;
-        bool optional;
-        bool rdonly;
-        ChrootPath(Path source = "", bool optional = false, bool rdonly = false)
-            : source(source), optional(optional), rdonly(rdonly)
-        { }
-    };
-    typedef std::map<Path, ChrootPath> PathsInChroot; // maps target path to source path
+
+    typedef SandboxPaths PathsInChroot; // maps target path to source path
 
     typedef StringMap Environment;
     Environment env;
@@ -848,35 +841,16 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox()
 {
     PathsInChroot pathsInChroot;
 
-    auto addPathWithOptions = [&](std::string s) {
-        if (s.empty()) return;
-        bool optional = false;
-        bool rdonly = false;
-        if (s[s.size() - 1] == '?') {
-            optional = true;
-            s.pop_back();
-        }
-        if (s.size() > 3 && s.substr(s.size() - 3) == ":ro") {
-            rdonly = true;
-            s.resize(s.size() - 3);
-        }
-        size_t p = s.find('=');
-        if (p == std::string::npos)
-            pathsInChroot[s] = {s, optional, rdonly};
-        else
-            pathsInChroot[s.substr(0, p)] = {s.substr(p + 1), optional, rdonly};
-    };
-
     /* Allow a user-configurable set of directories from the
        host file system. */
-    for (auto i : settings.sandboxPaths.get()) {
-        addPathWithOptions(i);
-    }
+    for (const auto & [k, v] : settings.sandboxPaths.get())
+        pathsInChroot.insert_or_assign(k, v);
+
     if (hasPrefix(store.storeDir, tmpDirInSandbox()))
     {
         throw Error("`sandbox-build-dir` must not contain the storeDir");
     }
-    pathsInChroot[tmpDirInSandbox()] = tmpDir;
+    pathsInChroot.insert_or_assign(tmpDirInSandbox(), tmpDir);
 
     /* Add the closure of store paths to the chroot. */
     StorePathSet closure;
@@ -946,7 +920,8 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox()
                 if (line == "") {
                     state = stBegin;
                 } else {
-                    addPathWithOptions(line);
+                    for (const auto & [k, v] : SandboxPath().parse(line))
+                        pathsInChroot.try_emplace(k, v);
                 }
             }
         }
diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc
@@ -121,18 +121,38 @@ static void setupSeccomp()
 #  endif
 }
 
-static void doBind(const Path & source, const Path & target, bool optional = false, bool rdonly = false)
+static auto combineMountOpts(auto init, auto iter)
 {
+    return std::transform_reduce(iter.cbegin(), iter.cend(), init,
+        [](unsigned long a, unsigned long b) {
+            if (b & (MS_NOATIME | MS_RELATIME | MS_NODIRATIME | MS_STRICTATIME)) {
+                return (a & ~(MS_NOATIME | MS_NODIRATIME | MS_RELATIME | MS_STRICTATIME)) | b;
+            }
+            return a | b;
+        },
+        [](const SandboxPath::MountOpt & o) { return static_cast<unsigned long>(o); });
+};
+
+
+static void doBind(const SandboxPath & pval, const Path & target)
+{
+    auto source = pval.source;
+    auto optional = pval.optional;
     debug("bind mounting '%1%' to '%2%'", source, target);
 
     auto bindMount = [&]() {
         if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1)
             throw SysError("bind mount from '%1%' to '%2%' failed", source, target);
 
-        if (rdonly)
+        // set extra options when wanted
+        auto flags = pval.readOnly ? combineMountOpts(0, pval.readOnlyDefaults) : 0;
+        flags = combineMountOpts(flags, pval.options);
+        if (flags != 0) {
             // initial mount wouldn't respect MS_RDONLY, must remount
-            if (mount("", target.c_str(), "", MS_REMOUNT | MS_BIND | MS_RDONLY, 0) == -1)
-                throw (SysError("making bind mount '%s' read-only failed", target));
+            debug("remounting '%s' with flags: %d", target, flags);
+            if (mount("", target.c_str(), "", MS_REMOUNT | MS_BIND | flags, 0) == -1)
+                throw SysError("mount: updating bind-mount flags of '%s' failed", target);
+        }
     };
 
     auto maybeSt = maybeLstat(source);
@@ -677,7 +697,7 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder
             // For backwards-compatibility, resolve all the symlinks in the
             // chroot paths.
             auto canonicalPath = canonPath(i, true);
-            pathsInChroot.emplace(i, canonicalPath);
+            pathsInChroot.try_emplace(i, canonicalPath);
         }
 
         /* Bind-mount all the directories from the "host"
@@ -699,7 +719,7 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder
             } else
 #  endif
             {
-                doBind(i.second.source, chrootRootDir + i.first, i.second.optional);
+                doBind(i.second, chrootRootDir + i.first);
             }
         }
 
