From 62ee2aea297528568098bae103e9a3af7658b4b5 Mon Sep 17 00:00:00 2001 From: Lily Foster Date: Sun, 16 Feb 2025 00:08:10 +0100 Subject: [PATCH] libexpr: rectify filtering logic for filter builtins under chroot stores Under chroot or diverted store setups, the filtering logic of `builtins.filterSource` and `builtins.path` (which shares the same filtering logic as `filterSource`) would incorrectly pass physical paths to the filter function instead of logical store paths. This caused actual breakage in nixpkgs when the `lib.fileset` library was introduced. Due to this unresolved bug in Nix, the library was forbidden from use: . To the best of our knowledge, this bug has existed since CppNix 2.3. The existing tests were strengthened to cover these cases, but additional testing may be required, particularly regarding symlink handling. References: https://github.com/NixOS/nix/pull/12512 (CppNix fix to the problem using "union" abstractions). Co-authored-by: Raito Bezarius Co-authored-by: Alois Wohlschlager Co-authored-by: eldritch horrors Signed-off-by: Raito Bezarius Change-Id: Iaf6ca8c506eeca145393ce100c64db12178daa62 --- .../filterSource-filter-chroot-stores.md | 19 ++++++++++++++ lix/libexpr/primops.cc | 25 ++++++++++++------- tests/functional/filter-source.nix | 5 +++- tests/functional/filter-source.sh | 14 +++++++++-- tests/functional/meson.build | 3 ++- tests/functional/path.nix | 7 ++++-- tests/functional/redirected-filter-source.nix | 17 +++++++++++++ tests/functional/redirected-filter-source.sh | 21 ++++++++++++++++ 8 files changed, 96 insertions(+), 15 deletions(-) create mode 100644 doc/manual/rl-next/filterSource-filter-chroot-stores.md create mode 100644 tests/functional/redirected-filter-source.nix create mode 100644 tests/functional/redirected-filter-source.sh diff --git a/doc/manual/rl-next/filterSource-filter-chroot-stores.md b/doc/manual/rl-next/filterSource-filter-chroot-stores.md new file mode 100644 index 000000000..206e5d9c3 --- /dev/null +++ b/doc/manual/rl-next/filterSource-filter-chroot-stores.md @@ -0,0 +1,19 @@ +--- +synopsis: fix usage of `builtins.filterSource` and `builtins.path` with the filter argument when using chroot stores +issues: [11503] +credits: [lily, alois31, horrors] +category: Fixes +--- + +The semantics of `builtins.filterSource` (and the `filter` argument for +`builtins.path`) have been adjusted regarding how paths inside the Nix store +are handled. + +Previously, when evaluating whether a path should be included, the filtering +function received the **physical path** if the source was inside the chroot store. + +Now, it receives the **logical path** instead. + +This ensures consistency in path handling and avoids potential +misinterpretations of paths within the evaluator, which led to various fallouts +mentioned in . diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index 7e6bd459e..13f2a1b6b 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -1508,31 +1508,38 @@ static void addPath( // FIXME: handle CA derivation outputs (where path needs to // be rewritten to the actual output). auto rewrites = state.aio.blockOn(state.ctx.paths.realiseContext(context)); - path = state.ctx.paths.toRealPath(rewriteStrings(path, rewrites), context); + path = rewriteStrings(path, rewrites); + + Path realPath = path; StorePathSet refs; + // If the path is in the store, it can mean either a physical path or a logical path in a + // chroot store. Query the chroot store for its presence to find out which is the case. if (state.ctx.store->isInStore(path)) { try { auto [storePath, subPath] = state.ctx.store->toStorePath(path); // FIXME: we should scanForReferences on the path before adding it refs = state.ctx.store->queryPathInfo(storePath)->references; - path = state.ctx.store->toRealPath(storePath) + subPath; + realPath = state.ctx.store->toRealPath(path); } catch (Error &) { // FIXME: should be InvalidPathError } } - path = evalSettings.pureEval && expectedHash - ? path - : state.ctx.paths.checkSourcePath(CanonPath(path)).canonical().abs(); + realPath = evalSettings.pureEval && expectedHash + ? realPath + : state.ctx.paths.checkSourcePath(CanonPath(realPath)).canonical().abs(); - PathFilter filter = filterFun ? ([&](const Path & path) { - auto st = lstat(path); + PathFilter filter = filterFun ? ([&](const Path & p) { + auto st = lstat(p); /* Call the filter function. The first argument is the path, the second is a string indicating the type of the file. */ Value arg1; - arg1.mkString(path); + if (isInDir(p, realPath)) + arg1.mkString(path + "/" + std::string(p, realPath.size() + 1)); + else + arg1.mkString(p); Value arg2; arg2.mkString( @@ -1559,7 +1566,7 @@ static void addPath( if (!expectedHash || !state.ctx.store->isValidPath(*expectedStorePath)) { auto dstPath = state.aio.blockOn(fetchToStore( *state.ctx.store, - state.ctx.paths.checkSourcePath(CanonPath(path)), + state.ctx.paths.checkSourcePath(CanonPath(realPath)), name, method, &filter, diff --git a/tests/functional/filter-source.nix b/tests/functional/filter-source.nix index 907163639..7d9f712b4 100644 --- a/tests/functional/filter-source.nix +++ b/tests/functional/filter-source.nix @@ -1,3 +1,5 @@ +{ filterin }: + with import ./config.nix; mkDerivation { @@ -6,7 +8,8 @@ mkDerivation { input = let filter = path: type: type != "symlink" + && (builtins.substring 0 (builtins.stringLength filterin) (builtins.toString path) == filterin) && baseNameOf path != "foo" && !((import ./lang/lib.nix).hasSuffix ".bak" (baseNameOf path)); - in builtins.filterSource filter ((builtins.getEnv "TEST_ROOT") + "/filterin"); + in builtins.filterSource filter filterin; } diff --git a/tests/functional/filter-source.sh b/tests/functional/filter-source.sh index ba34d2eac..864a65395 100644 --- a/tests/functional/filter-source.sh +++ b/tests/functional/filter-source.sh @@ -18,8 +18,18 @@ checkFilter() { test ! -L $1/link } -nix-build ./filter-source.nix -o $TEST_ROOT/filterout1 +nix-build ./filter-source.nix --argstr filterin $TEST_ROOT/filterin -o $TEST_ROOT/filterout1 checkFilter $TEST_ROOT/filterout1 -nix-build ./path.nix -o $TEST_ROOT/filterout2 +nix-build ./path.nix --argstr filterin $TEST_ROOT/filterin -o $TEST_ROOT/filterout2 checkFilter $TEST_ROOT/filterout2 + +if canUseSandbox; then + filterinStorePath=$(nix-store --add-fixed --recursive sha256 $TEST_ROOT/filterin --store $TEST_HOME/.local/share/nix/root) + + nix-build ./filter-source.nix --argstr filterin $filterinStorePath -o $TEST_ROOT/filterout3 --store $TEST_HOME/.local/share/nix/root --builders 'auto - - 1 1' + checkFilter $TEST_ROOT/filterout3 + + nix-build ./path.nix --argstr filterin $filterinStorePath -o $TEST_ROOT/filterout4 --store $TEST_HOME/.local/share/nix/root --builders 'auto - - 1 1' + checkFilter $TEST_ROOT/filterout4 +fi diff --git a/tests/functional/meson.build b/tests/functional/meson.build index ff6116329..1c15e079c 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -200,7 +200,8 @@ functional_tests_scripts = [ 'substitute-truncated-nar.sh', 'regression-484.sh', 'regression-reference-checks.sh', - 'external-commands.sh' + 'external-commands.sh', + 'redirected-filter-source.sh', ] # Plugin tests require shared libraries support. diff --git a/tests/functional/path.nix b/tests/functional/path.nix index 883c3c41b..2d87c4745 100644 --- a/tests/functional/path.nix +++ b/tests/functional/path.nix @@ -1,3 +1,5 @@ +{ filterin }: + with import ./config.nix; mkDerivation { @@ -5,9 +7,10 @@ mkDerivation { builder = builtins.toFile "builder" "ln -s $input $out"; input = builtins.path { - path = ((builtins.getEnv "TEST_ROOT") + "/filterin"); + path = filterin; filter = path: type: - type != "symlink" + type != "symlink" + && (builtins.substring 0 (builtins.stringLength filterin) (builtins.toString path) == filterin) && baseNameOf path != "foo" && !((import ./lang/lib.nix).hasSuffix ".bak" (baseNameOf path)); }; diff --git a/tests/functional/redirected-filter-source.nix b/tests/functional/redirected-filter-source.nix new file mode 100644 index 000000000..88d206ac4 --- /dev/null +++ b/tests/functional/redirected-filter-source.nix @@ -0,0 +1,17 @@ +{ path }: +let + pathStr = builtins.toString path; +in +builtins.path { + path = builtins.toString path; + name = "src"; + filter = + fullPath: type: + let + obtained = builtins.substring 0 (builtins.stringLength pathStr) fullPath; + in + if obtained == pathStr then + true + else + builtins.throw "bug detected, expected: ${pathStr}, got: ${obtained}"; +} diff --git a/tests/functional/redirected-filter-source.sh b/tests/functional/redirected-filter-source.sh new file mode 100644 index 000000000..cd9824b25 --- /dev/null +++ b/tests/functional/redirected-filter-source.sh @@ -0,0 +1,21 @@ +# This exercises some of the behaviors we expect under redirected stores w.r.t. to `filterSource`. +# They serve as anti regression testing towards the `lib.fileset` breakage in nixpkgs. +# See: https://github.com/NixOS/nixpkgs/pull/369694. + +source common.sh + +TEST_STORE="$TEST_ROOT/teststore" +TEST_DIR="$TEST_ROOT/testsrc" + +mkdir -p "$TEST_STORE" +mkdir -p "$TEST_DIR" +chmod -R u+w $TEST_STORE +rm -fr $TEST_STORE + +cp redirected-filter-source.nix "$TEST_DIR/default.nix" + +nix-instantiate --eval --store "$TEST_STORE" "$TEST_DIR" --arg path "\"$TEST_DIR\"" + +path_in_store="$(nix-store --add "$TEST_DIR")" +nix-store --store "$TEST_STORE" --add "$TEST_DIR" > /dev/null +nix-instantiate --eval --store "$TEST_STORE" "$TEST_DIR" --arg path "builtins.storePath \"$path_in_store\""