libstore/build: automatic clean up of unsuccessfully built scratch outputs

When a build fails, its scratch output paths are not cleaned up.

Until recently, this was deemed not a problem but as part of the effort
to harden the Nix builds and protect these paths against being part of a
staged attack (race conditions, etc.), we automatically cleanup after
failed builds.

Fixes CVE-2025-52992.

Change-Id: I58481b1cc83826298b9d80d37fecf81f117ccb09
Signed-off-by: Raito Bezarius <raito@lix.systems>

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 */
@@ -523,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. */
@@ -545,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. */
@@ -2376,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;
         }
 
@@ -2399,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));
     }
@@ -2428,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
@@ -2461,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