Merge pull request #12531 from obsidiansystems/store-accessor-root

`Store::getFSAccessor`: Do not include the store dir

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,11 +285,6 @@ EvalState::EvalState(
 
             auto realStoreDir = dirOf(store->toRealPath(StorePath::dummy));
             if (settings.pureEval || store->storeDir != realStoreDir) {
-                auto storeFS = makeMountedSourceAccessor(
-                    {
-                        {CanonPath::root, makeEmptySourceAccessor()},
-                        {CanonPath(store->storeDir), makeFSSourceAccessor(realStoreDir)}
-                    });
                 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/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()`
+    return projectSubdirSourceAccessor(store->getFSAccessor(), storePath.to_string());
-    auto root = std::filesystem::path{store->toRealPath(storePath)};
-    auto accessor = makeFSSourceAccessor(root);
-    accessor->setPathDisplay(root.string());
-    return accessor;
 }
 
 }

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. */
+        /* Also allow `path` to point to the entire store, which is
-        if (isDirOrInDir(store->storeDir, path.abs()))
+           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, CanonPath path)
-
-    void cat(ref<SourceAccessor> accessor)
     {
-        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

@@ -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 = accessor->resolveSymlinks(
+            pathAdditions.push_back(binDir.abs());
+
+            auto propPath = state->storeFS->resolveSymlinks(
                 CanonPath(store->printStorePath(path)) / "nix-support" / "propagated-user-env-packages");
-            if (auto st = accessor->maybeLstat(propPath); st && st->type == SourceAccessor::tRegular) {
+            if (auto st = state->storeFS->maybeLstat(propPath); st && st->type == SourceAccessor::tRegular) {
-                for (auto & p : tokenizeString<Paths>(accessor->readFile(propPath)))
+                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);
     }

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(path);
-        auto st = accessor->lstat(path2);
+        doPath(st, path,
-        doPath(st, path2,
+            st.type == SourceAccessor::Type::tDirectory ? "." : path.baseName().value_or(""),
-            st.type == SourceAccessor::Type::tDirectory ? "." : path2.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) {