Merge "libstore/build: just copy the magic /etc files into the sandbox" into main

src/libstore/build/local-derivation-goal.cc

@@ -410,7 +410,7 @@ static void doBind(const Path & source, const Path & target, bool optional = fal
     } else if (S_ISLNK(st.st_mode)) {
         // Symlinks can (apparently) not be bind-mounted, so just copy it
         createDirs(dirOf(target));
-        copyFile(source, target, /* andDelete */ false);
+        copyFile(source, target, {});
     } else {
         createDirs(dirOf(target));
         writeFile(target, "");
@@ -1811,8 +1811,25 @@ void LocalDerivationGoal::runChild()
                    happens when testing Nix building fixed-output derivations
                    within a pure derivation. */
                 for (auto & path : { "/etc/resolv.conf", "/etc/services", "/etc/hosts" })
-                    if (pathExists(path))
+                    if (pathExists(path)) {
-                        ss.push_back(path);
+                        // Copy the actual file, not the symlink, because we don't know where
+                        // the symlink is pointing, and we don't want to chase down the entire
+                        // chain.
+                        //
+                        // This means if your network config changes during a FOD build,
+                        // the DNS in the sandbox will be wrong. However, this is pretty unlikely
+                        // to actually be a problem, because FODs are generally pretty fast,
+                        // and machines with often-changing network configurations probably
+                        // want to run resolved or some other local resolver anyway.
+                        //
+                        // There's also just no simple way to do this correctly, you have to manually
+                        // inotify watch the files for changes on the outside and update the sandbox
+                        // while the build is running (or at least that's what Flatpak does).
+                        //
+                        // I also just generally feel icky about modifying sandbox state under a build,
+                        // even though it really shouldn't be a big deal. -K900
+                        copyFile(path, chrootRootDir + path, { .followSymlinks = true });
+                    }
 
                 if (settings.caFile != "")
                     pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true);
@@ -2542,7 +2559,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
                 // that there's no stale file descriptor pointing to it
                 Path tmpOutput = actualPath + ".tmp";
                 movePath(actualPath, tmpOutput);
-                copyFile(tmpOutput, actualPath, true);
+                copyFile(tmpOutput, actualPath, { .deleteAfter = true });
 
                 auto newInfo0 = newInfoFromCA(DerivationOutput::CAFloating {
                     .method = dof.ca.method,

src/libutil/filesystem.cc

@@ -103,31 +103,37 @@ void setWriteTime(const fs::path & p, const struct stat & st)
         throw SysError("changing modification time of '%s'", p);
 }
 
-void copy(const fs::directory_entry & from, const fs::path & to, bool andDelete)
+void copy(const fs::directory_entry & from, const fs::path & to, CopyFileFlags flags)
 {
     // TODO: Rewrite the `is_*` to use `symlink_status()`
     auto statOfFrom = lstat(from.path().c_str());
     auto fromStatus = from.symlink_status();
 
     // Mark the directory as writable so that we can delete its children
-    if (andDelete && fs::is_directory(fromStatus)) {
+    if (flags.deleteAfter && fs::is_directory(fromStatus)) {
         fs::permissions(from.path(), fs::perms::owner_write, fs::perm_options::add | fs::perm_options::nofollow);
     }
 
 
     if (fs::is_symlink(fromStatus) || fs::is_regular_file(fromStatus)) {
-        fs::copy(from.path(), to, fs::copy_options::copy_symlinks | fs::copy_options::overwrite_existing);
+        auto opts = fs::copy_options::overwrite_existing;
+
+        if (!flags.followSymlinks) {
+            opts |= fs::copy_options::copy_symlinks;
+        }
+
+        fs::copy(from.path(), to, opts);
     } else if (fs::is_directory(fromStatus)) {
         fs::create_directory(to);
         for (auto & entry : fs::directory_iterator(from.path())) {
-            copy(entry, to / entry.path().filename(), andDelete);
+            copy(entry, to / entry.path().filename(), flags);
         }
     } else {
         throw Error("file '%s' has an unsupported type", from.path());
     }
 
     setWriteTime(to, statOfFrom);
-    if (andDelete) {
+    if (flags.deleteAfter) {
         if (!fs::is_symlink(fromStatus))
             fs::permissions(from.path(), fs::perms::owner_write, fs::perm_options::add | fs::perm_options::nofollow);
         fs::remove(from.path());
@@ -135,9 +141,9 @@ void copy(const fs::directory_entry & from, const fs::path & to, bool andDelete)
 }
 
 
-void copyFile(const Path & oldPath, const Path & newPath, bool andDelete)
+void copyFile(const Path & oldPath, const Path & newPath, CopyFileFlags flags)
 {
-    return copy(fs::directory_entry(fs::path(oldPath)), fs::path(newPath), andDelete);
+    return copy(fs::directory_entry(fs::path(oldPath)), fs::path(newPath), flags);
 }
 
 void renameFile(const Path & oldName, const Path & newName)
@@ -160,7 +166,7 @@ void moveFile(const Path & oldName, const Path & newName)
         if (e.code().value() == EXDEV) {
             fs::remove(newPath);
             warn("Can’t rename %s as %s, copying instead", oldName, newName);
-            copy(fs::directory_entry(oldPath), tempCopyTarget, true);
+            copy(fs::directory_entry(oldPath), tempCopyTarget, { .deleteAfter = true });
             renameFile(tempCopyTarget, newPath);
         }
     }

src/libutil/util.hh

@@ -279,13 +279,26 @@ void renameFile(const Path & src, const Path & dst);
  */
 void moveFile(const Path & src, const Path & dst);
 
+struct CopyFileFlags
+{
+    /**
+     * Delete the file after copying.
+     */
+    bool deleteAfter = false;
+
+    /**
+     * Follow symlinks and copy the eventual target.
+     */
+    bool followSymlinks = false;
+};
+
 /**
  * Recursively copy the content of `oldPath` to `newPath`. If `andDelete` is
  * `true`, then also remove `oldPath` (making this equivalent to `moveFile`, but
  * with the guaranty that the destination will be “fresh”, with no stale inode
  * or file descriptor pointing to it).
  */
-void copyFile(const Path & oldPath, const Path & newPath, bool andDelete);
+void copyFile(const Path & oldPath, const Path & newPath, CopyFileFlags flags);
 
 /**
  * Wrappers arount read()/write() that read/write exactly the

tests/nixos/default.nix

@@ -158,4 +158,6 @@ in
   ca-fd-leak = runNixOSTestFor "x86_64-linux" ./ca-fd-leak;
 
   fetch-git = runNixOSTestFor "x86_64-linux" ./fetch-git;
+
+  symlinkResolvconf = runNixOSTestFor "x86_64-linux" ./symlink-resolvconf.nix;
 }

tests/nixos/symlink-resolvconf.nix

@@ -0,0 +1,28 @@
+{ pkgs, ... }:
+let
+  checkResolvconfInSandbox = pkgs.runCommand "resolvconf-works-in-sandbox" {
+    # must be an FOD to have a resolv.conf in the first place
+    outputHash = "sha256-47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=";
+    outputHashAlgo = "sha256";
+    outputHashType = "flat";
+  } ''
+    cat /etc/resolv.conf
+    touch $out
+  '';
+in {
+  name = "symlink-resolvconf";
+
+  nodes.machine = {
+    # Enabling resolved makes /etc/resolv.conf a symlink to /etc/static/resolv.conf, which is itself a symlink to /run.
+    # If this works, most other things probably will too.
+    services.resolved.enable = true;
+
+    virtualisation.additionalPaths = [checkResolvconfInSandbox.drvPath];
+  };
+
+  testScript = { nodes }: ''
+    start_all()
+
+    machine.succeed('nix-build --check ${checkResolvconfInSandbox.drvPath}')
+  '';
+}