nix shell: Resolve symlinks in storeFS

`storeFS` is the `MountedSourceAccessor` that wraps `store->getFSAccessor()`.

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});

src/libexpr/include/nix/expr/eval.hh

@@ -265,6 +265,11 @@ public:
     /** `"unknown"` */
     Value vStringUnknown;
 
+    /**
+     * The accessor corresponding to `store`.
+     */
+    const ref<SourceAccessor> storeFS;
+
     /**
      * The accessor for the root filesystem.
      */

src/nix/env.cc

@@ -65,11 +65,11 @@ struct CmdShell : InstallablesCommand, MixEnvironment
 
     void run(ref<Store> 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<StorePath> done;
         std::queue<StorePath> todo;
         for (auto & path : outPaths)
@@ -85,13 +85,16 @@ struct CmdShell : InstallablesCommand, MixEnvironment
             if (!done.insert(path).second)
                 continue;
 
-            if (true)
+            auto binDir = state->storeFS->resolveSymlinks(CanonPath(store->printStorePath(path)) / "bin");
-                pathAdditions.push_back(store->printStorePath(path) + "/bin");
+            if (!store->isInStore(binDir.abs()))
+                throw Error("path '%s' is not in the Nix store", binDir);
 
-            auto propPath =
+            pathAdditions.push_back(binDir.abs());
-                accessor->resolveSymlinks(CanonPath(path.to_string()) / "nix-support" / "propagated-user-env-packages");
+
-            if (auto st = accessor->maybeLstat(propPath); st && st->type == SourceAccessor::tRegular) {
+            auto propPath = state->storeFS->resolveSymlinks(
-                for (auto & p : tokenizeString<Paths>(accessor->readFile(propPath)))
+                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<Paths>(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);
     }

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