From adea821d8766976f6e0006575aba39404b649e40 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?=
 <theophane.hufschmitt@tweag.io>
Date: Mon, 6 May 2024 15:10:18 +0200
Subject: [PATCH] libstore: Fix sandbox=relaxed

The fix for the Darwin vulnerability in ecdbc3b207eaec1a2cafd2a0d494bcbabdd60a11
also broke setting `__sandboxProfile` when `sandbox=relaxed` or
`sandbox=false`. This cppnix change fixes `sandbox=relaxed` and
adds a suitable test.

Co-Authored-By: Artemis Tosini <lix@artem.ist>
Co-Authored-By: Eelco Dolstra <edolstra@gmail.com>
Change-Id: I40190f44f3e1d61846df1c7b89677c20a1488522
---
 src/libstore/build/local-derivation-goal.cc |  5 ++++-
 tests/functional/extra-sandbox-profile.nix  | 19 +++++++++++++++++
 tests/functional/extra-sandbox-profile.sh   | 23 +++++++++++++++++++++
 tests/functional/meson.build                |  1 +
 4 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 tests/functional/extra-sandbox-profile.nix
 create mode 100644 tests/functional/extra-sandbox-profile.sh

diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index da1db5771..0fa2eca8d 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -175,6 +175,10 @@ void LocalDerivationGoal::killSandbox(bool getStats)
 
 void LocalDerivationGoal::tryLocalBuild()
 {
+#if __APPLE__
+    additionalSandboxProfile = parsedDrv->getStringAttr("__sandboxProfile").value_or("");
+#endif
+
     unsigned int curBuilds = worker.getNrLocalBuilds();
     if (curBuilds >= settings.maxBuildJobs) {
         state = &DerivationGoal::tryToBuild;
@@ -193,7 +197,6 @@ void LocalDerivationGoal::tryLocalBuild()
                 throw Error("derivation '%s' has '__noChroot' set, "
                     "but that's not allowed when 'sandbox' is 'true'", worker.store.printStorePath(drvPath));
 #if __APPLE__
-            additionalSandboxProfile = parsedDrv->getStringAttr("__sandboxProfile").value_or("");
             if (additionalSandboxProfile != "")
                 throw Error("derivation '%s' specifies a sandbox profile, "
                     "but this is only allowed when 'sandbox' is 'relaxed'", worker.store.printStorePath(drvPath));
diff --git a/tests/functional/extra-sandbox-profile.nix b/tests/functional/extra-sandbox-profile.nix
new file mode 100644
index 000000000..aa680b918
--- /dev/null
+++ b/tests/functional/extra-sandbox-profile.nix
@@ -0,0 +1,19 @@
+{ destFile, seed }:
+
+with import ./config.nix;
+
+mkDerivation {
+  name = "simple";
+  __sandboxProfile = ''
+    # Allow writing any file in the filesystem
+    (allow file*)
+  '';
+  inherit seed;
+  buildCommand = ''
+    (
+      set -x
+      touch ${destFile}
+      touch $out
+    )
+  '';
+}
diff --git a/tests/functional/extra-sandbox-profile.sh b/tests/functional/extra-sandbox-profile.sh
new file mode 100644
index 000000000..ac3ca036f
--- /dev/null
+++ b/tests/functional/extra-sandbox-profile.sh
@@ -0,0 +1,23 @@
+source common.sh
+
+if [[ $(uname) != Darwin ]]; then skipTest "Need Darwin"; fi
+
+DEST_FILE="${TEST_ROOT}/foo"
+
+testSandboxProfile () (
+    set -e
+
+    sandboxMode="$1"
+
+    rm -f "${DEST_FILE}"
+    nix-build --no-out-link ./extra-sandbox-profile.nix \
+        --option sandbox "$sandboxMode" \
+        --argstr seed "$RANDOM" \
+        --argstr destFile "${DEST_FILE}"
+
+    ls -l "${DEST_FILE}"
+)
+
+testSandboxProfile "false"
+expectStderr 2 testSandboxProfile "true"
+testSandboxProfile "relaxed"
diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index 3f4a970a7..d0966a60c 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -182,6 +182,7 @@ functional_tests_scripts = [
   'debugger.sh',
   'plugins.sh',
   'test-libstoreconsumer.sh',
+  'extra-sandbox-profile.sh',
 ]
 
 # TODO(Qyriad): this will hopefully be able to be removed when we remove the autoconf+Make