diff --git a/doc/manual/rl-next/correct-cleanup-redirected-stores.md b/doc/manual/rl-next/correct-cleanup-redirected-stores.md new file mode 100644 index 000000000..a5d4a55a8 --- /dev/null +++ b/doc/manual/rl-next/correct-cleanup-redirected-stores.md @@ -0,0 +1,18 @@ +--- +synopsis: "Correct cleanup in redirected stores" +issues: [] +cls: [3493] +category: "Fixes" +credits: ["horrors"] +--- + +Following CVE-2025-52992, the Lix team implemented automatic cleanup of +*scratch outputs*, store paths written but not yet registered (e.g. +`/nix/store/...`). + +In setups using redirected stores, cleanup was mistakenly applied to the +logical store path (always under `/nix/store`) rather than the actual physical +location on disk. + +This could result in accidental deletion from the system +store instead of the intended redirected store. diff --git a/doc/manual/rl-next/infallible-build-dirs.md b/doc/manual/rl-next/infallible-build-dirs.md new file mode 100644 index 000000000..563d4fcde --- /dev/null +++ b/doc/manual/rl-next/infallible-build-dirs.md @@ -0,0 +1,25 @@ +--- +synopsis: "Fallback to safe temp dir when build-dir is unwritable" +issues: [fj#876] +cls: [3501] +category: "Fixes" +credits: ["raito", "horrors"] +--- + +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 `/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. diff --git a/doc/manual/rl-next/valid-outputs-deletion.md b/doc/manual/rl-next/valid-outputs-deletion.md new file mode 100644 index 000000000..f56112f41 --- /dev/null +++ b/doc/manual/rl-next/valid-outputs-deletion.md @@ -0,0 +1,22 @@ +--- +synopsis: "Do not delete valid outputs after build" +issues: [fj#883] +cls: [3494] +category: "Fixes" +credits: ["horrors"] +--- + +In response to CVE-2025-52992, the Lix team introduced automatic deletion of +*scratch outputs*, store paths written but not yet registered (e.g. in +`/nix/store`). + +However, the control flow distinguishing scratch outputs from valid ones is +complex. A logic error caused valid outputs, especially those obtained via +closure copies (e.g. remote builds), to be deleted post-build. + +This led to breakage in Lix and could potentially render entire systems +unusable by removing critical libraries. + +We are sorry for the severity of this bug and are taking steps to prevent its +recurrence. If your system is affected, please reach out on our support +channels for recovery assistance. diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index c866a3b66..247943e5c 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -487,17 +487,47 @@ try { }); } - createDirs(settings.buildDir.get()); - - /* Create a temporary directory where the build will take - place. */ - tmpDir = createTempDir( - settings.buildDir.get(), - "nix-build-" + std::string(drvPath.name()), - false, - false, - 0700 - ); + try { + auto buildDir = worker.buildDirOverride.value_or(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 + ); + } /* 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)}; @@ -538,7 +568,9 @@ try { /* 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)); + scratchOutputsCleaner.emplace( + outputName, worker.store.toRealPath(worker.store.printStorePath(scratchPath)) + ); /* Substitute output placeholders with the scratch output paths. We'll use during the build. */ @@ -1739,6 +1771,11 @@ try { before this for loop. */ if (*scratchPath != finalStorePath) outputRewrites[std::string { scratchPath->hashPart() }] = std::string { finalStorePath.hashPart() }; + /* Cancel automatic deletion of that output if it was a scratch output that we just + * registered. */ + if (auto cleaner = scratchOutputsCleaner.extract(outputName)) { + cleaner.mapped().cancel(); + } }; auto orifu = get(outputReferencesIfUnregistered, outputName); @@ -2063,10 +2100,6 @@ try { the next iteration */ if (newInfo.ca) { TRY_AWAIT(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)); @@ -2107,13 +2140,6 @@ try { infos2.insert_or_assign(newInfo.path, newInfo); } TRY_AWAIT(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 diff --git a/lix/libstore/build/worker.hh b/lix/libstore/build/worker.hh index 7fc3d1fe9..d9dc36e34 100644 --- a/lix/libstore/build/worker.hh +++ b/lix/libstore/build/worker.hh @@ -195,6 +195,7 @@ public: Store & store; Store & evalStore; AsyncSemaphore substitutions, localBuilds; + std::optional buildDirOverride; private: kj::TaskSet children; diff --git a/tests/functional/build.sh b/tests/functional/build.sh index 58fba83aa..fc83f61f3 100644 --- a/tests/functional/build.sh +++ b/tests/functional/build.sh @@ -174,3 +174,8 @@ 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 diff --git a/tests/functional/linux-sandbox.sh b/tests/functional/linux-sandbox.sh index 82f363a09..526605e5f 100644 --- a/tests/functional/linux-sandbox.sh +++ b/tests/functional/linux-sandbox.sh @@ -81,3 +81,10 @@ testCert present fixed-output "$certsymlink" # Symlinks should be added in the sandbox directly and not followed nix-sandbox-build symlink-derivation.nix + +# Regression fj#883: derivations outputs disappearing after rebuild +# build the derivation for both its outputs and delete one of them. +# simulates substitution or copying only one output from a builder. +nix-store --delete $(nix-sandbox-build --no-out-link ./regression-fj883.nix -A base.lib) +# build a derivation depending on previous one. this should succeed +nix-sandbox-build --no-out-link ./regression-fj883.nix -A downstream diff --git a/tests/functional/regression-fj883.nix b/tests/functional/regression-fj883.nix new file mode 100644 index 000000000..2317145b7 --- /dev/null +++ b/tests/functional/regression-fj883.nix @@ -0,0 +1,15 @@ +with import ./config.nix; + +rec { + base = mkDerivation { + name = "base"; + outputs = [ "out" "lib" ]; + buildCommand = "echo > $out; echo > $lib"; + }; + + downstream = mkDerivation { + name = "downstream"; + deps = [ base.out base.lib ]; + buildCommand = "echo $deps > $out"; + }; +} diff --git a/version.json b/version.json index 22b83defe..a39a6e7e2 100644 --- a/version.json +++ b/version.json @@ -1,5 +1,5 @@ { - "version": "2.93.1", - "official_release": true, + "version": "2.93.2", + "official_release": false, "release_name": "Bici Bici" }