release: 2.91.2 "Dragon's Breath"

Release produced with releng/create_release.xsh

Change-Id: Iaae75be51d51a553c29720e00f98565d314783bb

doc/manual/src/release-notes/rl-2.91.md

@@ -1,59 +1,4 @@
 # Lix 2.91 "Dragon's Breath" (2024-08-12)
-# Lix 2.91.3 (2025-06-30)
-
-## Fixes
-- Revert CVE-2025-52992 failed mitigation [fj#883](https://git.lix.systems/lix-project/lix/issues/883) [fj#887](https://git.lix.systems/lix-project/lix/issues/887) [cl/3420](https://gerrit.lix.systems/c/lix/+/3420) [cl/3524](https://gerrit.lix.systems/c/lix/+/3524) [cl/3523](https://gerrit.lix.systems/c/lix/+/3523) [cl/3522](https://gerrit.lix.systems/c/lix/+/3522)
-
-  Following the initial mitigation of **CVE-2025-52992** in `cl/3420`, we
-  received reports of **unexpected deletion of in-use store paths**.
-
-  Upon investigation, we found that the patch did **not correctly cancel all
-  automatic deleters**, resulting in potentially critical path loss during normal
-  operation.
-
-  Given the severity and time-sensitive nature of the situation ([see incident
-  report](https://lix.systems/blog/2025-06-27-lix-critical-bug/)), we evaluated
-  possible options to repair the behavior safely. However, we concluded that a
-  rushed fix would either:
-
-  * **Overdelete**, i.e. breaking running systems, or,
-  * **Underdelete**, effectively **reopening CVE-2025-52992** while leaving
-    orphaned paths behind.
-
-  As **CVE-2025-52992 has no known exploit vector**, and correctness is critical
-  in the Lix project, we have **fully reverted the previous mitigations**.
-
-  Moving forward, the Lix team will rework this code path in a **long-term,
-  correctness-first fix** on the main branch. We will explore backporting it to
-  stable channels once its safety is assured.
-
-  We are deeply sorry for the stability incident and the Lix team remain
-  available for assisting you in recovering your systems.
-
-  Many thanks to [Raito Bezarius](https://git.lix.systems/raito) and [eldritch horrors](https://git.lix.systems/pennae) for this.
-- Fallback to safe temp dir when build-dir is unwritable [fj#876](https://git.lix.systems/lix-project/lix/issues/876) [cl/3502](https://gerrit.lix.systems/c/lix/+/3502)
-
-  Non-daemon builds started failing with a permission error after introducing the `build-dir` option:
-
-  ```
-  $ nix build --store ~/scratch nixpkgs#hello --rebuild
-  error: creating directory '/nix/var/nix/builds/nix-build-hello-2.12.2.drv-0': Permission denied
-  ```
-
-  This happens because:
-
-  1. These builds are not run via the daemon, which owns `/nix/var/nix/builds`.
-  2. The user lacks permissions for that path.
-
-  We considered making `build-dir` a store-level option and defaulting it to `<chroot-root>/nix/var/nix/builds` for chroot stores, but opted instead for a fallback: if the default fails, Nix now creates a safe build directory under `/tmp`.
-
-  To avoid CVE-2025-52991, the fallback uses an extra path component between `/tmp` and the build dir.
-
-  **Note**: this fallback clutters `/tmp` with build directories that are not cleaned up. To prevent this, explicitly set `build-dir` to a path managed by Lix, even for local workloads.
-
-  Many thanks to [Raito Bezarius](https://git.lix.systems/raito) and [eldritch horrors](https://git.lix.systems/pennae) for this.
-
-
 # Lix 2.91.2 (2025-06-23)
 
 ## Breaking Changes

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

@@ -363,9 +363,13 @@ void LocalDerivationGoal::cleanupPostOutputsRegisteredModeCheck()
 
 void LocalDerivationGoal::cleanupPostOutputsRegisteredModeNonCheck()
 {
-    /* Delete unused redirected outputs (when doing hash rewriting). */
-    for (auto & i : redirectedOutputs)
-        deletePath(worker.store.Store::toRealPath(i.second));
+    /* In the past, redirected outputs were manually tracked for deletion.
+     * Now that we have the scratch outputs cleaner which are a superset of
+     * redirected outputs, we just fire all uncancelled automatic deleters now.
+     *
+     * This should clean up any paths that IS NOT registered in the database.
+     */
+    scratchOutputsCleaner.clear();
 
     /* Delete the chroot (if we were using one). */
     autoDelChroot.reset(); /* this runs the destructor */
@@ -475,47 +479,17 @@ void LocalDerivationGoal::startBuilder()
             settings.thisSystem,
             concatStringsSep<StringSet>(", ", worker.store.systemFeatures));
 
-    try {
-        auto buildDir = worker.buildDirOverride.value_or(settings.buildDir.get());
+    createDirs(settings.buildDir.get());
 
-        createDirs(buildDir);
-
-        /* Create a temporary directory where the build will take
-           place. */
-        tmpDir =
-            createTempDir(buildDir, "nix-build-" + std::string(drvPath.name()), false, false, 0700);
-    } catch (SysError & e) {
-        /*
-         * Fallback to the global tmpdir and create a safe space there
-         * only if it's a permission error.
-         */
-        if (e.errNo != EACCES) {
-            throw;
-        }
-
-        auto globalTmp = defaultTempDir();
-        createDirs(globalTmp);
-#if __APPLE__
-        /* macOS filesystem namespacing does not exist, to avoid breaking builds, we need to weaken
-         * the mode bits on the top-level directory. This avoids issues like
-         * https://github.com/NixOS/nix/pull/11031. */
-        constexpr int toplevelDirMode = 0755;
-#else
-        constexpr int toplevelDirMode = 0700;
-#endif
-        auto nixBuildsTmp =
-            createTempDir(globalTmp, fmt("nix-builds-%s", geteuid()), false, false, toplevelDirMode);
-        warn(
-            "Failed to use the system-wide build directory '%s', falling back to a temporary "
-            "directory inside '%s'",
-            settings.buildDir.get(),
-            nixBuildsTmp
-        );
-        worker.buildDirOverride = nixBuildsTmp;
-        tmpDir = createTempDir(
-            nixBuildsTmp, "nix-build-" + std::string(drvPath.name()), false, false, 0700
-        );
-    }
+    /* Create a temporary directory where the build will take
+       place. */
+    tmpDir = createTempDir(
+        settings.buildDir.get(),
+        "nix-build-" + std::string(drvPath.name()),
+        false,
+        false,
+        0700
+    );
     /* The TOCTOU between the previous mkdir call and this open call is unavoidable due to
      * POSIX semantics.*/
     tmpDirFd = AutoCloseFD{open(tmpDir.c_str(), O_RDONLY | O_NOFOLLOW | O_DIRECTORY)};
@@ -553,6 +527,10 @@ void LocalDerivationGoal::startBuilder()
                    to use a temporary path */
                 makeFallbackPath(status.known->path);
         scratchOutputs.insert_or_assign(outputName, scratchPath);
+        /* Schedule this scratch output path for automatic deletion
+         * if we do not cancel it, e.g. when registering the outputs.
+         */
+        scratchOutputsCleaner.insert_or_assign(outputName, worker.store.printStorePath(scratchPath));
 
         /* Substitute output placeholders with the scratch output paths.
            We'll use during the build. */
@@ -575,8 +553,6 @@ void LocalDerivationGoal::startBuilder()
             std::string h2 { scratchPath.hashPart() };
             inputRewrites[h1] = h2;
         }
-
-        redirectedOutputs.insert_or_assign(std::move(fixedFinalPath), std::move(scratchPath));
     }
 
     /* Construct the environment passed to the builder. */
@@ -1870,8 +1846,6 @@ void LocalDerivationGoal::runChild()
 
         if (getEnv("_NIX_TEST_NO_SANDBOX") != "1") {
             Strings sandboxArgs;
-            sandboxArgs.push_back("_NIX_BUILD_TOP");
-            sandboxArgs.push_back(tmpDir);
             sandboxArgs.push_back("_GLOBAL_TMP_DIR");
             sandboxArgs.push_back(globalTmpDir);
             if (allowLocalNetworking) {
@@ -2408,6 +2382,10 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
                 localStore.registerValidPaths({{oldInfo.path, oldInfo}});
             }
 
+            /* Don't register anything, since we already have the
+               previous versions which we're comparing.
+               NOTE: this means that the `.check` path will be automatically deleted.
+            */
             continue;
         }
 
@@ -2431,8 +2409,13 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
         /* If it's a CA path, register it right away. This is necessary if it
            isn't statically known so that we can safely unlock the path before
            the next iteration */
-        if (newInfo.ca)
+        if (newInfo.ca) {
             localStore.registerValidPaths({{newInfo.path, newInfo}});
+            /* Cancel automatic deletion of that output if it was a scratch output. */
+            if (auto cleaner = scratchOutputsCleaner.extract(outputName)) {
+                cleaner.mapped().cancel();
+            }
+        }
 
         infos.emplace(outputName, std::move(newInfo));
     }
@@ -2460,6 +2443,13 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
             infos2.insert_or_assign(newInfo.path, newInfo);
         }
         localStore.registerValidPaths(infos2);
+
+        /* Cancel automatic deletion of that output if it was a scratch output that we just registered. */
+        for (auto & [outputName, _ ] : infos) {
+            if (auto cleaner = scratchOutputsCleaner.extract(outputName)) {
+                cleaner.mapped().cancel();
+            }
+        }
     }
 
     /* In case of a fixed-output derivation hash mismatch, throw an
@@ -2493,6 +2483,13 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
         builtOutputs.emplace(outputName, thisRealisation);
     }
 
+    /* NOTE: At this point, all outputs MAY NOT have been registered.
+     * Therefore, there may remains auto-deleters pending in the cleaner list (`scratchOutputsCleaner`).
+     *
+     * They will be finally deleted but we have no way to assert they all have been, e.g.
+     * `assert(scratchOutputsCleaner.size() == 0)` cannot be written.
+     */
+
     return builtOutputs;
 }
 

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

@@ -107,8 +107,6 @@ struct LocalDerivationGoal : public DerivationGoal
      * Hash rewriting.
      */
     StringMap inputRewrites, outputRewrites;
-    typedef map<StorePath, StorePath> RedirectedOutputs;
-    RedirectedOutputs redirectedOutputs;
 
     /**
      * The outputs paths used during the build.
@@ -125,6 +123,19 @@ struct LocalDerivationGoal : public DerivationGoal
      *   self-references.
      */
     OutputPathMap scratchOutputs;
+    /**
+     * Output paths used during the build are scheduled for
+     * automatic cleanup unless they have been successfully built.
+     *
+     * `registerOutputs` take care of cancelling the cleanups
+     * and clearing this vector.
+     *
+     * `startBuilder` take care of filling this vector
+     * as `scratchOutputs` gets filled.
+     *
+     * This is a map from output names to automatic delete handles.
+     */
+    std::map<std::string, AutoDelete> scratchOutputsCleaner;
 
     /**
      * Path registration info from the previous round, if we're

src/libstore/build/sandbox-defaults.sb

@@ -26,14 +26,12 @@ R""(
 ; Allow getpwuid.
 (allow mach-lookup (global-name "com.apple.system.opendirectoryd.libinfo"))
 
-; Access to /tmp and the build directory.
+; Access to /tmp.
 ; The network-outbound/network-inbound ones are for unix domain sockets, which
 ; we allow access to in TMPDIR (but if we allow them more broadly, you could in
 ; theory escape the sandbox)
 (allow file* process-exec network-outbound network-inbound
-       (literal "/tmp")
-       (subpath TMPDIR)
-       (subpath (param "_NIX_BUILD_TOP")))
+       (literal "/tmp") (subpath TMPDIR))
 
 ; Some packages like to read the system version.
 (allow file-read*

src/libstore/build/worker.hh

@@ -161,7 +161,6 @@ public:
 
     Store & store;
     Store & evalStore;
-    std::optional<Path> buildDirOverride;
 
     struct HookState {
         std::unique_ptr<HookInstance> instance;

tests/functional/build.sh

@@ -170,8 +170,3 @@ test "$(<<<"$out" grep -E '^error:' | wc -l)" = 3
 <<<"$out" grepQuiet -E "error: 2 dependencies of derivation '.*-x4\\.drv' failed to build"
 <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'"
 <<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
-
-# Ensure when if the system build dir is inaccessible, we can still build things
-BUILD_DIR=$(mktemp -d)
-chmod 0000 "$BUILD_DIR"
-nix --build-dir "$BUILD_DIR" build -E 'with import ./config.nix; mkDerivation { name = "test"; buildCommand = "echo rawr > $out"; }' --impure --no-link

tests/nixos/default.nix

@@ -146,6 +146,9 @@ in
 
   symlinkResolvconf = runNixOSTestFor "x86_64-linux" ./symlink-resolvconf.nix;
 
+  # Use this test to test things that cannot easily be tested under chroot Nix stores in functional test suite.
+  non-chroot-misc = runNixOSTestFor "x86_64-linux" ./non-chroot-misc;
+
   noNewPrivilegesInSandbox = runNixOSTestFor "x86_64-linux" ./no-new-privileges/sandbox.nix;
 
   noNewPrivilegesOutsideSandbox = runNixOSTestFor "x86_64-linux" ./no-new-privileges/no-sandbox.nix;

tests/nixos/non-chroot-misc/default.nix

@@ -0,0 +1,34 @@
+{ ... }:
+# Misc things we want to test inside of a non redirected, non chroot Nix store.
+let
+  nonAutoCleaningFailingDerivationCode = ''
+    derivation {
+      name = "scratch-failing";
+      system = builtins.currentSystem;
+      builder = "/bin/sh";
+      args = [ (builtins.toFile "builder.sh" "echo bonjour > $out; echo out: $out; false") ];
+    }
+  '';
+in
+{
+  name = "non-chroot-sandbox-misc";
+
+  nodes.machine = {
+  };
+
+  testScript = { nodes }: ''
+    import re
+    start_all()
+
+    # You might ask yourself why write such a convoluted thing?
+    # The condition for fooling Nix into NOT cleaning up the output path are non trivial and unclear.
+    # This is one of those: create a derivation, mkdir or touch the $out path, communicate it back.
+    # Even with a sandboxed Lix, you will observe leftovers before 2.93.0. After this version, this test passes.
+    result = machine.fail("""nix-build --substituters "" -E '${nonAutoCleaningFailingDerivationCode}' 2>&1""")
+    match = re.search(r'out: (\S+)', result)
+    assert match is not None, "Did not find Nix store path in the result of the failing build"
+    outpath = match.group(1).strip()
+    print(f"Found Nix store path: {outpath}")
+    machine.fail(f'stat {outpath}')
+  '';
+}

version.json

@@ -1,5 +1,5 @@
 {
-    "version": "2.91.3",
+    "version": "2.91.2",
     "official_release": true,
     "release_name": "Dragon's Breath"
 }