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: <https://github.com/NixOS/nixpkgs/pull/369694>.

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 <raito@lix.systems>
Co-authored-by: Alois Wohlschlager <alois1@gmx-topmail.de>
Co-authored-by: eldritch horrors <pennae@lix.systems>
Signed-off-by: Raito Bezarius <raito@lix.systems>
Change-Id: Iaf6ca8c506eeca145393ce100c64db12178daa62
This commit is contained in:
Lily Foster
2025-02-27 00:26:48 +01:00
committed by Raito Bezarius
co-authored by Raito Bezarius Alois Wohlschlager eldritch horrors
parent ffe2dd40f4
commit 62ee2aea29
8 changed files with 96 additions and 15 deletions
@@ -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 <https://github.com/NixOS/nixpkgs/pull/369694>.
+16 -9
View File
@@ -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,
+4 -1
View File
@@ -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;
}
+12 -2
View File
@@ -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
+2 -1
View File
@@ -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.
+5 -2
View File
@@ -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));
};
@@ -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}";
}
@@ -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\""