Store::getFSAccessor: Do not include the store dir

Rather than "mounting" the store inside an empty virtual filesystem,
just return the store as a virtual filesystem. This is more modular.

(FWIW, it also supports two long term hopes of mind:

1. More capability-based Nix language mode. I dream of a "super pure
   eval" where you can only use relative path literals (See #8738), and
   any `fetchTree`-fetched stuff + the store are all disjoint (none is
   mounted in another) file systems.

2. Windows, where the store dir may include drive letters, etc., and is
   thus unsuitable to be the prefix of any `CanonPath`s.

)

Co-authored-by: Eelco Dolstra <edolstra@gmail.com>

src/libexpr/eval.cc

@@ -264,7 +264,25 @@ EvalState::EvalState(
                 auto storeFS = makeMountedSourceAccessor(
                     {
                         {CanonPath::root, makeEmptySourceAccessor()},
-                        {CanonPath(store->storeDir), makeFSSourceAccessor(realStoreDir)}
+                        /* 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

src/libfetchers/store-path-accessor.cc

@@ -5,11 +5,7 @@ namespace nix {
 
 ref<SourceAccessor> makeStorePathAccessor(ref<Store> store, const StorePath & storePath)
 {
-    // FIXME: should use `store->getFSAccessor()`
-    auto root = std::filesystem::path{store->toRealPath(storePath)};
-    auto accessor = makeFSSourceAccessor(root);
-    accessor->setPathDisplay(root.string());
-    return accessor;
+    return projectSubdirSourceAccessor(store->getFSAccessor(), storePath.to_string());
 }
 
 }

src/libstore/build/worker.cc

@@ -524,7 +524,7 @@ bool Worker::pathContentsGood(const StorePath & path)
         res = false;
     else {
         auto current = hashPath(
-            {store.getFSAccessor(), CanonPath(store.printStorePath(path))},
+            {store.getFSAccessor(), CanonPath(path.to_string())},
             FileIngestionMethod::NixArchive, info->narHash.algo).first;
         Hash nullHash(HashAlgorithm::SHA256);
         res = info->narHash == nullHash || info->narHash == current;

src/libstore/dummy-store.cc

@@ -83,7 +83,9 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store
     { callback(nullptr); }
 
     virtual ref<SourceAccessor> getFSAccessor(bool requireValidPath) override
-    { unsupported("getFSAccessor"); }
+    {
+        return makeEmptySourceAccessor();
+    }
 };
 
 static RegisterStoreImplementation<DummyStore, DummyStoreConfig> regDummyStore;

src/libstore/local-fs-store.cc

@@ -33,30 +33,35 @@ struct LocalStoreAccessor : PosixSourceAccessor
     bool requireValidPath;
 
     LocalStoreAccessor(ref<LocalFSStore> store, bool requireValidPath)
-        : store(store)
+        : PosixSourceAccessor(std::filesystem::path{store->realStoreDir.get()})
+        , store(store)
         , requireValidPath(requireValidPath)
-    { }
-
-    CanonPath toRealPath(const CanonPath & path)
     {
-        auto [storePath, rest] = store->toStorePath(path.abs());
+    }
+
+
+    void requireStoreObject(const CanonPath & path)
+    {
+        auto [storePath, rest] = store->toStorePath(store->storeDir + path.abs());
         if (requireValidPath && !store->isValidPath(storePath))
             throw InvalidPath("path '%1%' is not a valid store path", store->printStorePath(storePath));
-        return CanonPath(store->getRealStoreDir()) / storePath.to_string() / CanonPath(rest);
     }
 
     std::optional<Stat> maybeLstat(const CanonPath & path) override
     {
-        /* Handle the case where `path` is (a parent of) the store. */
-        if (isDirOrInDir(store->storeDir, path.abs()))
+        /* Also allow `path` to point to the entire store, which is
+           needed for resolving symlinks. */
+        if (path.isRoot())
             return Stat{ .type = tDirectory };
 
-        return PosixSourceAccessor::maybeLstat(toRealPath(path));
+        requireStoreObject(path);
+        return PosixSourceAccessor::maybeLstat(path);
     }
 
     DirEntries readDirectory(const CanonPath & path) override
     {
-        return PosixSourceAccessor::readDirectory(toRealPath(path));
+        requireStoreObject(path);
+        return PosixSourceAccessor::readDirectory(path);
     }
 
     void readFile(
@@ -64,12 +69,14 @@ struct LocalStoreAccessor : PosixSourceAccessor
         Sink & sink,
         std::function<void(uint64_t)> sizeCallback) override
     {
-        return PosixSourceAccessor::readFile(toRealPath(path), sink, sizeCallback);
+        requireStoreObject(path);
+        return PosixSourceAccessor::readFile(path, sink, sizeCallback);
     }
 
     std::string readLink(const CanonPath & path) override
     {
-        return PosixSourceAccessor::readLink(toRealPath(path));
+        requireStoreObject(path);
+        return PosixSourceAccessor::readLink(path);
     }
 };
 

src/libstore/local-store.cc

@@ -1102,7 +1102,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
                     auto & specified = *info.ca;
                     auto actualHash = ({
                         auto accessor = getFSAccessor(false);
-                        CanonPath path { printStorePath(info.path) };
+                        CanonPath path { info.path.to_string() };
                         Hash h { HashAlgorithm::SHA256 }; // throwaway def to appease C++
                         auto fim = specified.method.getFileIngestionMethod();
                         switch (fim) {

src/libstore/remote-fs-accessor.cc

@@ -51,7 +51,7 @@ ref<SourceAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std:
 
 std::pair<ref<SourceAccessor>, CanonPath> RemoteFSAccessor::fetch(const CanonPath & path)
 {
-    auto [storePath, restPath_] = store->toStorePath(path.abs());
+    auto [storePath, restPath_] = store->toStorePath(store->storeDir + path.abs());
     auto restPath = CanonPath(restPath_);
 
     if (requireValidPath && !store->isValidPath(storePath))

src/libstore/store-api.cc

@@ -1233,7 +1233,7 @@ static Derivation readDerivationCommon(Store & store, const StorePath & drvPath,
     auto accessor = store.getFSAccessor(requireValidPath);
     try {
         return parseDerivation(store,
-            accessor->readFile(CanonPath(store.printStorePath(drvPath))),
+            accessor->readFile(CanonPath(drvPath.to_string())),
             Derivation::nameFromPath(drvPath));
     } catch (FormatError & e) {
         throw Error("error parsing derivation '%s': %s", store.printStorePath(drvPath), e.msg());

src/libutil/include/nix/util/source-accessor.hh

@@ -222,4 +222,10 @@ ref<SourceAccessor> makeMountedSourceAccessor(std::map<CanonPath, ref<SourceAcce
  */
 ref<SourceAccessor> makeUnionSourceAccessor(std::vector<ref<SourceAccessor>> && accessors);
 
+/**
+ * Creates a new source accessor which is confined to the subdirectory
+ * of the given source accessor.
+ */
+ref<SourceAccessor> projectSubdirSourceAccessor(ref<SourceAccessor>, CanonPath subdirectory);
+
 }

src/libutil/meson.build

@@ -142,6 +142,7 @@ sources = [config_priv_h] + files(
   'signature/signer.cc',
   'source-accessor.cc',
   'source-path.cc',
+  'subdir-source-accessor.cc',
   'strings.cc',
   'suggestions.cc',
   'tarfile.cc',

src/libutil/source-accessor.cc

@@ -114,9 +114,11 @@ CanonPath SourceAccessor::resolveSymlinks(
                     if (!linksAllowed--)
                         throw Error("infinite symlink recursion in path '%s'", showPath(path));
                     auto target = readLink(res);
-                    res.pop();
-                    if (isAbsolute(target))
+                    if (isAbsolute(target)) {
                         res = CanonPath::root;
+                    } else {
+                        res.pop();
+                    }
                     todo.splice(todo.begin(), tokenizeString<std::list<std::string>>(target, "/"));
                 }
             }

src/libutil/subdir-source-accessor.cc

@@ -0,0 +1,59 @@
+#include "nix/util/source-accessor.hh"
+
+namespace nix {
+
+struct SubdirSourceAccessor : SourceAccessor
+{
+    ref<SourceAccessor> parent;
+
+    CanonPath subdirectory;
+
+    SubdirSourceAccessor(ref<SourceAccessor> && parent, CanonPath && subdirectory)
+        : parent(std::move(parent))
+        , subdirectory(std::move(subdirectory))
+    {
+        displayPrefix.clear();
+    }
+
+    std::string readFile(const CanonPath & path) override
+    {
+        return parent->readFile(subdirectory / path);
+    }
+
+    void readFile(const CanonPath & path, Sink & sink, std::function<void(uint64_t)> sizeCallback) override
+    {
+        return parent->readFile(subdirectory / path, sink, sizeCallback);
+    }
+
+    bool pathExists(const CanonPath & path) override
+    {
+        return parent->pathExists(subdirectory / path);
+    }
+
+    std::optional<Stat> maybeLstat(const CanonPath & path) override
+    {
+        return parent->maybeLstat(subdirectory / path);
+    }
+
+    DirEntries readDirectory(const CanonPath & path) override
+    {
+        return parent->readDirectory(subdirectory / path);
+    }
+
+    std::string readLink(const CanonPath & path) override
+    {
+        return parent->readLink(subdirectory / path);
+    }
+
+    std::string showPath(const CanonPath & path) override
+    {
+        return displayPrefix + parent->showPath(subdirectory / path) + displaySuffix;
+    }
+};
+
+ref<SourceAccessor> projectSubdirSourceAccessor(ref<SourceAccessor> parent, CanonPath subdirectory)
+{
+    return make_ref<SubdirSourceAccessor>(std::move(parent), std::move(subdirectory));
+}
+
+}

src/nix-store/nix-store.cc

@@ -563,7 +563,7 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise)
 #endif
             if (!hashGiven) {
                 HashResult hash = hashPath(
-                    {store->getFSAccessor(false), CanonPath { store->printStorePath(info->path) }},
+                    {store->getFSAccessor(false), CanonPath { info->path.to_string() }},
                     FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256);
                 info->narHash = hash.first;
                 info->narSize = hash.second;

src/nix/cat.cc

@@ -6,21 +6,21 @@ using namespace nix;
 
 struct MixCat : virtual Args
 {
-    std::string path;
-
-    void cat(ref<SourceAccessor> accessor)
+    void cat(ref<SourceAccessor> accessor, CanonPath path)
     {
-        auto st = accessor->lstat(CanonPath(path));
+        auto st = accessor->lstat(path);
         if (st.type != SourceAccessor::Type::tRegular)
-            throw Error("path '%1%' is not a regular file", path);
+            throw Error("path '%1%' is not a regular file", path.abs());
         logger->stop();
 
-        writeFull(getStandardOutput(), accessor->readFile(CanonPath(path)));
+        writeFull(getStandardOutput(), accessor->readFile(path));
     }
 };
 
 struct CmdCatStore : StoreCommand, MixCat
 {
+    std::string path;
+
     CmdCatStore()
     {
         expectArgs({
@@ -44,7 +44,8 @@ struct CmdCatStore : StoreCommand, MixCat
 
     void run(ref<Store> store) override
     {
-        cat(store->getFSAccessor());
+        auto [storePath, rest] = store->toStorePath(path);
+        cat(store->getFSAccessor(), CanonPath{storePath.to_string()} / CanonPath{rest});
     }
 };
 
@@ -52,6 +53,8 @@ struct CmdCatNar : StoreCommand, MixCat
 {
     Path narPath;
 
+    std::string path;
+
     CmdCatNar()
     {
         expectArgs({
@@ -76,7 +79,7 @@ struct CmdCatNar : StoreCommand, MixCat
 
     void run(ref<Store> store) override
     {
-        cat(makeNarAccessor(readFile(narPath)));
+        cat(makeNarAccessor(readFile(narPath)), CanonPath{path});
     }
 };
 

src/nix/env.cc

@@ -88,8 +88,8 @@ struct CmdShell : InstallablesCommand, MixEnvironment
             if (true)
                 pathAdditions.push_back(store->printStorePath(path) + "/bin");
 
-            auto propPath = accessor->resolveSymlinks(
-                CanonPath(store->printStorePath(path)) / "nix-support" / "propagated-user-env-packages");
+            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<Paths>(accessor->readFile(propPath)))
                     todo.push(store->parseStorePath(p));

src/nix/ls.cc

@@ -8,8 +8,6 @@ using namespace nix;
 
 struct MixLs : virtual Args, MixJSON
 {
-    std::string path;
-
     bool recursive = false;
     bool verbose = false;
     bool showDirectory = false;
@@ -38,7 +36,7 @@ struct MixLs : virtual Args, MixJSON
         });
     }
 
-    void listText(ref<SourceAccessor> accessor)
+    void listText(ref<SourceAccessor> accessor, CanonPath path)
     {
         std::function<void(const SourceAccessor::Stat &, const CanonPath &, std::string_view, bool)> doPath;
 
@@ -77,26 +75,27 @@ struct MixLs : virtual Args, MixJSON
                 showFile(curPath, relPath);
         };
 
-        auto path2 = CanonPath(path);
-        auto st = accessor->lstat(path2);
-        doPath(st, path2,
-            st.type == SourceAccessor::Type::tDirectory ? "." : path2.baseName().value_or(""),
+        auto st = accessor->lstat(path);
+        doPath(st, path,
+            st.type == SourceAccessor::Type::tDirectory ? "." : path.baseName().value_or(""),
             showDirectory);
     }
 
-    void list(ref<SourceAccessor> accessor)
+    void list(ref<SourceAccessor> accessor, CanonPath path)
     {
         if (json) {
             if (showDirectory)
                 throw UsageError("'--directory' is useless with '--json'");
-            logger->cout("%s", listNar(accessor, CanonPath(path), recursive));
+            logger->cout("%s", listNar(accessor, path, recursive));
         } else
-            listText(accessor);
+            listText(accessor, std::move(path));
     }
 };
 
 struct CmdLsStore : StoreCommand, MixLs
 {
+    std::string path;
+
     CmdLsStore()
     {
         expectArgs({
@@ -120,7 +119,8 @@ struct CmdLsStore : StoreCommand, MixLs
 
     void run(ref<Store> store) override
     {
-        list(store->getFSAccessor());
+        auto [storePath, rest] = store->toStorePath(path);
+        list(store->getFSAccessor(), CanonPath{storePath.to_string()} / CanonPath{rest});
     }
 };
 
@@ -128,6 +128,8 @@ struct CmdLsNar : Command, MixLs
 {
     Path narPath;
 
+    std::string path;
+
     CmdLsNar()
     {
         expectArgs({
@@ -152,7 +154,7 @@ struct CmdLsNar : Command, MixLs
 
     void run() override
     {
-        list(makeNarAccessor(readFile(narPath)));
+        list(makeNarAccessor(readFile(narPath)), CanonPath{path});
     }
 };
 

src/nix/why-depends.cc

@@ -172,7 +172,7 @@ struct CmdWhyDepends : SourceExprCommand, MixOperateOnOptions
         struct BailOut { };
 
         printNode = [&](Node & node, const std::string & firstPad, const std::string & tailPad) {
-            CanonPath pathS(store->printStorePath(node.path));
+            CanonPath pathS(node.path.to_string());
 
             assert(node.dist != inf);
             if (precise) {

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 "is not in the Nix store"
+expect 1 nix shell -f shell-hello.nix forbidden-symlink -c hello 2>&1 | grepQuiet "points outside source tree"
 
 # Test that we're not setting any more environment variables than necessary.
 # For instance, we might set an environment variable temporarily to affect some