From 9d3595646d2e24a005d606679f6f0fea19ef25d9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 26 Mar 2025 21:07:17 +0100 Subject: [PATCH] nix shell: Resolve symlinks in storeFS `storeFS` is the `MountedSourceAccessor` that wraps `store->getFSAccessor()`. --- src/libexpr/eval.cc | 47 ++++++++++++++-------------- src/libexpr/include/nix/expr/eval.hh | 5 +++ src/nix/env.cc | 21 +++++++------ tests/functional/shell.sh | 2 +- 4 files changed, 42 insertions(+), 33 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index da973fec9..9d60676f5 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -246,6 +246,30 @@ EvalState::EvalState( } , repair(NoRepair) , emptyBindings(0) + , storeFS( + makeMountedSourceAccessor( + { + {CanonPath::root, makeEmptySourceAccessor()}, + /* In the pure eval case, we can simply require + valid paths. However, in the *impure* eval + case this gets in the way of the union + mechanism, because an invalid access in the + upper layer will *not* be caught by the union + source accessor, but instead abort the entire + lookup. + + This happens when the store dir in the + ambient file system has a path (e.g. because + another Nix store there), but the relocated + store does not. + + TODO make the various source accessors doing + access control all throw the same type of + exception, and make union source accessor + catch it, so we don't need to do this hack. + */ + {CanonPath(store->storeDir), store->getFSAccessor(settings.pureEval)}, + })) , rootFS( ({ /* In pure eval mode, we provide a filesystem that only @@ -261,29 +285,6 @@ EvalState::EvalState( auto realStoreDir = dirOf(store->toRealPath(StorePath::dummy)); if (settings.pureEval || store->storeDir != realStoreDir) { - auto storeFS = makeMountedSourceAccessor( - { - {CanonPath::root, makeEmptySourceAccessor()}, - /* In the pure eval case, we can simply require - valid paths. However, in the *impure* eval - case this gets in the way of the union - mechanism, because an invalid access in the - upper layer will *not* be caught by the union - source accessor, but instead abort the entire - lookup. - - This happens when the store dir in the - ambient file system has a path (e.g. because - another Nix store there), but the relocated - store does not. - - TODO make the various source accessors doing - access control all throw the same type of - exception, and make union source accessor - catch it, so we don't need to do this hack. - */ - {CanonPath(store->storeDir), store->getFSAccessor(settings.pureEval)}, - }); accessor = settings.pureEval ? storeFS : makeUnionSourceAccessor({accessor, storeFS}); diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index 0933c6e89..61da225fc 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -265,6 +265,11 @@ public: /** `"unknown"` */ Value vStringUnknown; + /** + * The accessor corresponding to `store`. + */ + const ref storeFS; + /** * The accessor for the root filesystem. */ diff --git a/src/nix/env.cc b/src/nix/env.cc index ee314bf26..277bd0fdd 100644 --- a/src/nix/env.cc +++ b/src/nix/env.cc @@ -65,11 +65,11 @@ struct CmdShell : InstallablesCommand, MixEnvironment void run(ref store, Installables && installables) override { + auto state = getEvalState(); + auto outPaths = Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, OperateOn::Output, installables); - auto accessor = store->getFSAccessor(); - std::unordered_set done; std::queue todo; for (auto & path : outPaths) @@ -85,13 +85,16 @@ struct CmdShell : InstallablesCommand, MixEnvironment if (!done.insert(path).second) continue; - if (true) - pathAdditions.push_back(store->printStorePath(path) + "/bin"); + auto binDir = state->storeFS->resolveSymlinks(CanonPath(store->printStorePath(path)) / "bin"); + if (!store->isInStore(binDir.abs())) + throw Error("path '%s' is not in the Nix store", binDir); - auto propPath = - accessor->resolveSymlinks(CanonPath(path.to_string()) / "nix-support" / "propagated-user-env-packages"); - if (auto st = accessor->maybeLstat(propPath); st && st->type == SourceAccessor::tRegular) { - for (auto & p : tokenizeString(accessor->readFile(propPath))) + pathAdditions.push_back(binDir.abs()); + + auto propPath = state->storeFS->resolveSymlinks( + CanonPath(store->printStorePath(path)) / "nix-support" / "propagated-user-env-packages"); + if (auto st = state->storeFS->maybeLstat(propPath); st && st->type == SourceAccessor::tRegular) { + for (auto & p : tokenizeString(state->storeFS->readFile(propPath))) todo.push(store->parseStorePath(p)); } } @@ -108,7 +111,7 @@ struct CmdShell : InstallablesCommand, MixEnvironment // Release our references to eval caches to ensure they are persisted to disk, because // we are about to exec out of this process without running C++ destructors. - getEvalState()->evalCaches.clear(); + state->evalCaches.clear(); execProgramInStore(store, UseLookupPath::Use, *command.begin(), args); } diff --git a/tests/functional/shell.sh b/tests/functional/shell.sh index a03b641a1..51032ff1b 100755 --- a/tests/functional/shell.sh +++ b/tests/functional/shell.sh @@ -21,7 +21,7 @@ nix shell -f shell-hello.nix 'hello^*' -c hello2 | grep 'Hello2' nix shell -f shell-hello.nix hello-symlink -c hello | grep 'Hello World' # Test that symlinks outside of the store don't work. -expect 1 nix shell -f shell-hello.nix forbidden-symlink -c hello 2>&1 | grepQuiet "points outside source tree" +expect 1 nix shell -f shell-hello.nix forbidden-symlink -c hello 2>&1 | grepQuiet "is not in the Nix store" # Test that we're not setting any more environment variables than necessary. # For instance, we might set an environment variable temporarily to affect some