Merge pull request #11664 from DeterminateSystems/show-sandbox-setup-error

Propagate errors from early sandbox initialization to the parent

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

@@ -433,6 +433,41 @@ static void doBind(const Path & source, const Path & target, bool optional = fal
 };
 #endif
 
+/**
+ * Rethrow the current exception as a subclass of `Error`.
+ */
+static void rethrowExceptionAsError()
+{
+    try {
+        throw;
+    } catch (Error &) {
+        throw;
+    } catch (std::exception & e) {
+        throw Error(e.what());
+    } catch (...) {
+        throw Error("unknown exception");
+    }
+}
+
+/**
+ * Send the current exception to the parent in the format expected by
+ * `LocalDerivationGoal::processSandboxSetupMessages()`.
+ */
+static void handleChildException(bool sendException)
+{
+    try {
+        rethrowExceptionAsError();
+    } catch (Error & e) {
+        if (sendException) {
+            writeFull(STDERR_FILENO, "\1\n");
+            FdSink sink(STDERR_FILENO);
+            sink << e;
+            sink.flush();
+        } else
+            std::cerr << e.msg();
+    }
+}
+
 void LocalDerivationGoal::startBuilder()
 {
     if ((buildUser && buildUser->getUIDCount() != 1)
@@ -949,6 +984,7 @@ void LocalDerivationGoal::startBuilder()
                root. */
             openSlave();
 
+            try {
                 /* Drop additional groups here because we can't do it
                    after we've created the new user namespace. */
                 if (setgroups(0, 0) == -1) {
@@ -969,12 +1005,19 @@ void LocalDerivationGoal::startBuilder()
 
                 writeFull(sendPid.writeSide.get(), fmt("%d\n", child));
                 _exit(0);
+            } catch (...) {
+                handleChildException(true);
+                _exit(1);
+            }
         });
 
         sendPid.writeSide.close();
 
-        if (helper.wait() != 0)
+        if (helper.wait() != 0) {
+            processSandboxSetupMessages();
+            // Only reached if the child process didn't send an exception.
             throw Error("unable to start build process");
+        }
 
         userNamespaceSync.readSide = -1;
 
@@ -1050,7 +1093,12 @@ void LocalDerivationGoal::startBuilder()
     pid.setSeparatePG(true);
     worker.childStarted(shared_from_this(), {builderOut.get()}, true, true);
 
-    /* Check if setting up the build environment failed. */
+    processSandboxSetupMessages();
+}
+
+
+void LocalDerivationGoal::processSandboxSetupMessages()
+{
     std::vector<std::string> msgs;
     while (true) {
         std::string msg = [&]() {
@@ -1078,7 +1126,8 @@ void LocalDerivationGoal::startBuilder()
 }
 
 
-void LocalDerivationGoal::initTmpDir() {
+void LocalDerivationGoal::initTmpDir()
+{
     /* In a sandbox, for determinism, always use the same temporary
        directory. */
 #if __linux__
@@ -2237,14 +2286,8 @@ void LocalDerivationGoal::runChild()
 
         throw SysError("executing '%1%'", drv->builder);
 
-    } catch (Error & e) {
+    } catch (...) {
-        if (sendException) {
+        handleChildException(sendException);
-            writeFull(STDERR_FILENO, "\1\n");
-            FdSink sink(STDERR_FILENO);
-            sink << e;
-            sink.flush();
-        } else
-            std::cerr << e.msg();
         _exit(1);
     }
 }

src/libstore/unix/build/local-derivation-goal.hh

@@ -210,6 +210,11 @@ struct LocalDerivationGoal : public DerivationGoal
      */
     void initEnv();
 
+    /**
+     * Process messages send by the sandbox initialization.
+     */
+    void processSandboxSetupMessages();
+
     /**
      * Setup tmp dir location.
      */

tests/functional/supplementary-groups.sh

@@ -9,7 +9,7 @@ needLocalStore "The test uses --store always so we would just be bypassing the d
 
 TODO_NixOS
 
-unshare --mount --map-root-user bash <<EOF
+unshare --mount --map-root-user -- bash -e -x <<EOF
   source common.sh
 
   # Avoid store dir being inside sandbox build-dir
@@ -24,15 +24,13 @@ unshare --mount --map-root-user bash <<EOF
   cmd=(nix-build ./hermetic.nix --arg busybox "$busybox" --arg seed 1 --no-out-link)
 
   # Fails with default setting
-  # TODO better error
   setLocalStore store1
-  expectStderr 1 "\${cmd[@]}" | grepQuiet "unable to start build process"
+  expectStderr 1 "\${cmd[@]}" | grepQuiet "setgroups failed"
 
   # Fails with `require-drop-supplementary-groups`
-  # TODO better error
   setLocalStore store2
   NIX_CONFIG='require-drop-supplementary-groups = true' \
-    expectStderr 1 "\${cmd[@]}" | grepQuiet "unable to start build process"
+    expectStderr 1 "\${cmd[@]}" | grepQuiet "setgroups failed"
 
   # Works without `require-drop-supplementary-groups`
   setLocalStore store3