Read-only sandbox paths mounts (Linux)

src/libstore/include/nix/store/globals.hh

@@ -632,13 +632,18 @@ public:
     Setting<PathSet> sandboxPaths{
         this, {}, "sandbox-paths",
         R"(
-          A list of paths bind-mounted into Nix sandbox environments. You can
-          use the syntax `target=source` to mount a path in a different
-          location in the sandbox; for instance, `/bin=/nix-bin` will mount
-          the path `/nix-bin` as `/bin` inside the sandbox. If *source* is
-          followed by `?`, then it is 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.
+          A list of paths bind-mounted into Nix sandbox environments. Use the
+          syntax `target[=source][:ro][?]` to control the mount:
+
+          - `=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.
+
+          - `:ro` makes the mount read-only (Linux only).
+
+          - `?` 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.
 
           If the source is in the Nix store, then its closure will be added to
           the sandbox as well.

src/libstore/path.cc

@@ -77,6 +77,9 @@ StorePath StorePath::random(std::string_view name)
 
 StorePath MixStoreDirMethods::parseStorePath(std::string_view path) const
 {
+    if (path.empty())
+        throw BadStorePath("empty path is not a valid store path");
+
     // On Windows, `/nix/store` is not a canonical path. More broadly it
     // is unclear whether this function should be using the native
     // notion of a canonical path at all. For example, it makes to

src/libstore/unix/build/derivation-builder.cc

@@ -108,8 +108,9 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder
     struct ChrootPath {
         Path source;
         bool optional;
-        ChrootPath(Path source = "", bool optional = false)
-            : source(source), optional(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
@@ -847,20 +848,29 @@ DerivationBuilderImpl::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;
+    auto addPathWithOptions = [&](std::string s) {
+        if (s.empty()) return;
         bool optional = false;
-        if (i[i.size() - 1] == '?') {
+        bool rdonly = false;
+        if (s[s.size() - 1] == '?') {
             optional = true;
-            i.pop_back();
+            s.pop_back();
         }
-        size_t p = i.find('=');
+        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[i] = {i, optional};
+            pathsInChroot[s] = {s, optional, rdonly};
         else
-            pathsInChroot[i.substr(0, p)] = {i.substr(p + 1), optional};
+            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);
     }
     if (hasPrefix(store.storeDir, tmpDirInSandbox()))
     {
@@ -936,11 +946,7 @@ DerivationBuilderImpl::PathsInChroot DerivationBuilderImpl::getPathsInSandbox()
                 if (line == "") {
                     state = stBegin;
                 } else {
-                    auto p = line.find('=');
-                    if (p == std::string::npos)
-                        pathsInChroot[line] = line;
-                    else
-                        pathsInChroot[line.substr(0, p)] = line.substr(p + 1);
+                    addPathWithOptions(line);
                 }
             }
         }

src/libstore/unix/build/linux-derivation-builder.cc

@@ -121,13 +121,18 @@ static void setupSeccomp()
 #  endif
 }
 
-static void doBind(const Path & source, const Path & target, bool optional = false)
+static void doBind(const Path & source, const Path & target, bool optional = false, bool rdonly = false)
 {
     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)
+            // 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));
     };
 
     auto maybeSt = maybeLstat(source);