Merge pull request #12512 from DeterminateSystems/store-fs

Use a union source accessor to put chroot stores in the logical location

src/libcmd/common-eval-args.cc

@@ -37,7 +37,7 @@ EvalSettings evalSettings {
                 auto [accessor, lockedRef] = flakeRef.resolve(state.store).lazyFetch(state.store);
                 auto storePath = nix::fetchToStore(*state.store, SourcePath(accessor), FetchMode::Copy, lockedRef.input.getName());
                 state.allowPath(storePath);
-                return state.rootPath(state.store->toRealPath(storePath));
+                return state.rootPath(state.store->printStorePath(storePath));
             },
         },
     },
@@ -179,7 +179,7 @@ SourcePath lookupFileArg(EvalState & state, std::string_view s, const Path * bas
             state.fetchSettings,
             EvalSettings::resolvePseudoUrl(s));
         auto storePath = fetchToStore(*state.store, SourcePath(accessor), FetchMode::Copy);
-        return state.rootPath(CanonPath(state.store->toRealPath(storePath)));
+        return state.rootPath(CanonPath(state.store->printStorePath(storePath)));
     }
 
     else if (hasPrefix(s, "flake:")) {
@@ -188,7 +188,7 @@ SourcePath lookupFileArg(EvalState & state, std::string_view s, const Path * bas
         auto [accessor, lockedRef] = flakeRef.resolve(state.store).lazyFetch(state.store);
         auto storePath = nix::fetchToStore(*state.store, SourcePath(accessor), FetchMode::Copy, lockedRef.input.getName());
         state.allowPath(storePath);
-        return state.rootPath(CanonPath(state.store->toRealPath(storePath)));
+        return state.rootPath(CanonPath(state.store->printStorePath(storePath)));
     }
 
     else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') {

src/libexpr/eval.cc

@@ -246,15 +246,42 @@ EvalState::EvalState(
     , repair(NoRepair)
     , emptyBindings(0)
     , rootFS(
-        settings.restrictEval || settings.pureEval
+        ({
-        ? ref<SourceAccessor>(AllowListSourceAccessor::create(getFSSourceAccessor(), {},
+            /* In pure eval mode, we provide a filesystem that only
-            [&settings](const CanonPath & path) -> RestrictedPathError {
+               contains the Nix store.
-                auto modeInformation = settings.pureEval
+
-                    ? "in pure evaluation mode (use '--impure' to override)"
+               If we have a chroot store and pure eval is not enabled,
-                    : "in restricted mode";
+               use a union accessor to make the chroot store available
-                throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation);
+               at its logical location while still having the
-            }))
+               underlying directory available. This is necessary for
-        : getFSSourceAccessor())
+               instance if we're evaluating a file from the physical
+               /nix/store while using a chroot store. */
+            auto accessor = getFSSourceAccessor();
+
+            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});
+            }
+
+            /* Apply access control if needed. */
+            if (settings.restrictEval || settings.pureEval)
+                accessor = AllowListSourceAccessor::create(accessor, {},
+                    [&settings](const CanonPath & path) -> RestrictedPathError {
+                        auto modeInformation = settings.pureEval
+                            ? "in pure evaluation mode (use '--impure' to override)"
+                            : "in restricted mode";
+                        throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation);
+                    });
+
+            accessor;
+        }))
     , corepkgsFS(make_ref<MemorySourceAccessor>())
     , internalFS(make_ref<MemorySourceAccessor>())
     , derivationInternal{corepkgsFS->addFile(
@@ -344,7 +371,7 @@ void EvalState::allowPath(const Path & path)
 void EvalState::allowPath(const StorePath & storePath)
 {
     if (auto rootFS2 = rootFS.dynamic_pointer_cast<AllowListSourceAccessor>())
-        rootFS2->allowPrefix(CanonPath(store->toRealPath(storePath)));
+        rootFS2->allowPrefix(CanonPath(store->printStorePath(storePath)));
 }
 
 void EvalState::allowClosure(const StorePath & storePath)
@@ -422,16 +449,6 @@ void EvalState::checkURI(const std::string & uri)
 }
 
 
-Path EvalState::toRealPath(const Path & path, const NixStringContext & context)
-{
-    // FIXME: check whether 'path' is in 'context'.
-    return
-        !context.empty() && store->isInStore(path)
-        ? store->toRealPath(path)
-        : path;
-}
-
-
 Value * EvalState::addConstant(const std::string & name, Value & v, Constant info)
 {
     Value * v2 = allocValue();
@@ -2051,7 +2068,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
     else if (firstType == nPath) {
         if (!context.empty())
             state.error<EvalError>("a string that refers to a store path cannot be appended to a path").atPos(pos).withFrame(env, *this).debugThrow();
-        v.mkPath(state.rootPath(CanonPath(canonPath(str()))));
+        v.mkPath(state.rootPath(CanonPath(str())));
     } else
         v.mkStringMove(c_str(), context);
 }
@@ -2432,7 +2449,7 @@ SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext
     auto path = coerceToString(pos, v, context, errorCtx, false, false, true).toOwned();
     if (path == "" || path[0] != '/')
         error<EvalError>("string '%1%' doesn't represent an absolute path", path).withTrace(pos, errorCtx).debugThrow();
-    return rootPath(CanonPath(path));
+    return rootPath(path);
 }
 
 
@@ -3086,7 +3103,7 @@ std::optional<SourcePath> EvalState::resolveLookupPathPath(const LookupPath::Pat
                 fetchSettings,
                 EvalSettings::resolvePseudoUrl(value));
             auto storePath = fetchToStore(*store, SourcePath(accessor), FetchMode::Copy);
-            return finish(rootPath(store->toRealPath(storePath)));
+            return finish(rootPath(store->printStorePath(storePath)));
         } catch (Error & e) {
             logWarning({
                 .msg = HintFmt("Nix search path entry '%1%' cannot be downloaded, ignoring", value)

src/libexpr/eval.hh

@@ -412,17 +412,6 @@ public:
 
     void checkURI(const std::string & uri);
 
-    /**
-     * When using a diverted store and 'path' is in the Nix store, map
-     * 'path' to the diverted location (e.g. /nix/store/foo is mapped
-     * to /home/alice/my-nix/nix/store/foo). However, this is only
-     * done if the context is not empty, since otherwise we're
-     * probably trying to read from the actual /nix/store. This is
-     * intended to distinguish between import-from-derivation and
-     * sources stored in the actual /nix/store.
-     */
-    Path toRealPath(const Path & path, const NixStringContext & context);
-
     /**
      * Parse a Nix expression from the specified file.
      */

src/libexpr/primops.cc

@@ -145,8 +145,7 @@ static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, st
     try {
         if (!context.empty() && path.accessor == state.rootFS) {
             auto rewrites = state.realiseContext(context);
-            auto realPath = state.toRealPath(rewriteStrings(path.path.abs(), rewrites), context);
+            path = {path.accessor, CanonPath(rewriteStrings(path.path.abs(), rewrites))};
-            path = {path.accessor, CanonPath(realPath)};
         }
         return resolveSymlinks ? path.resolveSymlinks(*resolveSymlinks) : path;
     } catch (Error & e) {
@@ -2479,21 +2478,11 @@ static void addPath(
     const NixStringContext & context)
 {
     try {
-        StorePathSet refs;
-
         if (path.accessor == state.rootFS && state.store->isInStore(path.path.abs())) {
             // FIXME: handle CA derivation outputs (where path needs to
             // be rewritten to the actual output).
             auto rewrites = state.realiseContext(context);
-            path = {state.rootFS, CanonPath(state.toRealPath(rewriteStrings(path.path.abs(), rewrites), context))};
+            path = {path.accessor, CanonPath(rewriteStrings(path.path.abs(), rewrites))};
-
-            try {
-                auto [storePath, subPath] = state.store->toStorePath(path.path.abs());
-                // FIXME: we should scanForReferences on the path before adding it
-                refs = state.store->queryPathInfo(storePath)->references;
-                path = {state.rootFS, CanonPath(state.store->toRealPath(storePath) + subPath)};
-            } catch (Error &) { // FIXME: should be InvalidPathError
-            }
         }
 
         std::unique_ptr<PathFilter> filter;

src/libflake/flake/flake.cc

@@ -337,7 +337,7 @@ static Flake readFlake(
                 auto storePath = fetchToStore(*state.store, setting.value->path(), FetchMode::Copy);
                 flake.config.settings.emplace(
                     state.symbols[setting.name],
-                    state.store->toRealPath(storePath));
+                    state.store->printStorePath(storePath));
             }
             else if (setting.value->type() == nInt)
                 flake.config.settings.emplace(
@@ -423,7 +423,7 @@ static Flake getFlake(
     auto storePath = copyInputToStore(state, lockedRef.input, originalRef.input, accessor);
 
     // Re-parse flake.nix from the store.
-    return readFlake(state, originalRef, resolvedRef, lockedRef, state.rootPath(state.store->toRealPath(storePath)), lockRootAttrPath);
+    return readFlake(state, originalRef, resolvedRef, lockedRef, state.rootPath(state.store->printStorePath(storePath)), lockRootAttrPath);
 }
 
 Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool useRegistries)
@@ -784,7 +784,7 @@ LockedFlake lockFlake(
                                     // FIXME: allow input to be lazy.
                                     auto storePath = copyInputToStore(state, lockedRef.input, input.ref->input, accessor);
 
-                                    return {state.rootPath(state.store->toRealPath(storePath)), lockedRef};
+                                    return {state.rootPath(state.store->printStorePath(storePath)), lockedRef};
                                 }
                             }();
 
@@ -921,21 +921,6 @@ LockedFlake lockFlake(
     }
 }
 
-std::pair<StorePath, Path> sourcePathToStorePath(
-    ref<Store> store,
-    const SourcePath & _path)
-{
-    auto path = _path.path.abs();
-
-    if (auto store2 = store.dynamic_pointer_cast<LocalFSStore>()) {
-        auto realStoreDir = store2->getRealStoreDir();
-        if (isInDir(path, realStoreDir))
-            path = store2->storeDir + path.substr(realStoreDir.size());
-    }
-
-    return store->toStorePath(path);
-}
-
 void callFlake(EvalState & state,
     const LockedFlake & lockedFlake,
     Value & vRes)
@@ -953,7 +938,7 @@ void callFlake(EvalState & state,
 
         auto lockedNode = node.dynamic_pointer_cast<const LockedNode>();
 
-        auto [storePath, subdir] = sourcePathToStorePath(state.store, sourcePath);
+        auto [storePath, subdir] = state.store->toStorePath(sourcePath.path.abs());
 
         emitTreeAttrs(
             state,

src/libflake/flake/flake.hh

@@ -234,16 +234,6 @@ void callFlake(
     const LockedFlake & lockedFlake,
     Value & v);
 
-/**
- * Map a `SourcePath` to the corresponding store path. This is a
- * temporary hack to support chroot stores while we don't have full
- * lazy trees. FIXME: Remove this once we can pass a sourcePath rather
- * than a storePath to call-flake.nix.
- */
-std::pair<StorePath, Path> sourcePathToStorePath(
-    ref<Store> store,
-    const SourcePath & path);
-
 }
 
 void emitTreeAttrs(

src/libutil/meson.build

@@ -167,6 +167,7 @@ sources = files(
   'tarfile.cc',
   'terminal.cc',
   'thread-pool.cc',
+  'union-source-accessor.cc',
   'unix-domain-socket.cc',
   'url.cc',
   'users.cc',

src/libutil/mounted-source-accessor.cc

@@ -23,12 +23,6 @@ struct MountedSourceAccessor : SourceAccessor
         return accessor->readFile(subpath);
     }
 
-    bool pathExists(const CanonPath & path) override
-    {
-        auto [accessor, subpath] = resolve(path);
-        return accessor->pathExists(subpath);
-    }
-
     std::optional<Stat> maybeLstat(const CanonPath & path) override
     {
         auto [accessor, subpath] = resolve(path);
@@ -69,6 +63,12 @@ struct MountedSourceAccessor : SourceAccessor
             path.pop();
         }
     }
+
+    std::optional<std::filesystem::path> getPhysicalPath(const CanonPath & path) override
+    {
+        auto [accessor, subpath] = resolve(path);
+        return accessor->getPhysicalPath(subpath);
+    }
 };
 
 ref<SourceAccessor> makeMountedSourceAccessor(std::map<CanonPath, ref<SourceAccessor>> mounts)

src/libutil/source-accessor.hh

@@ -216,4 +216,10 @@ ref<SourceAccessor> makeFSSourceAccessor(std::filesystem::path root);
 
 ref<SourceAccessor> makeMountedSourceAccessor(std::map<CanonPath, ref<SourceAccessor>> mounts);
 
+/**
+ * Construct an accessor that presents a "union" view of a vector of
+ * underlying accessors. Earlier accessors take precedence over later.
+ */
+ref<SourceAccessor> makeUnionSourceAccessor(std::vector<ref<SourceAccessor>> && accessors);
+
 }

src/libutil/union-source-accessor.cc

@@ -0,0 +1,82 @@
+#include "source-accessor.hh"
+
+namespace nix {
+
+struct UnionSourceAccessor : SourceAccessor
+{
+    std::vector<ref<SourceAccessor>> accessors;
+
+    UnionSourceAccessor(std::vector<ref<SourceAccessor>> _accessors)
+        : accessors(std::move(_accessors))
+    {
+        displayPrefix.clear();
+    }
+
+    std::string readFile(const CanonPath & path) override
+    {
+        for (auto & accessor : accessors) {
+            auto st = accessor->maybeLstat(path);
+            if (st)
+                return accessor->readFile(path);
+        }
+        throw FileNotFound("path '%s' does not exist", showPath(path));
+    }
+
+    std::optional<Stat> maybeLstat(const CanonPath & path) override
+    {
+        for (auto & accessor : accessors) {
+            auto st = accessor->maybeLstat(path);
+            if (st)
+                return st;
+        }
+        return std::nullopt;
+    }
+
+    DirEntries readDirectory(const CanonPath & path) override
+    {
+        DirEntries result;
+        for (auto & accessor : accessors) {
+            auto st = accessor->maybeLstat(path);
+            if (!st)
+                continue;
+            for (auto & entry : accessor->readDirectory(path))
+                // Don't override entries from previous accessors.
+                result.insert(entry);
+        }
+        return result;
+    }
+
+    std::string readLink(const CanonPath & path) override
+    {
+        for (auto & accessor : accessors) {
+            auto st = accessor->maybeLstat(path);
+            if (st)
+                return accessor->readLink(path);
+        }
+        throw FileNotFound("path '%s' does not exist", showPath(path));
+    }
+
+    std::string showPath(const CanonPath & path) override
+    {
+        for (auto & accessor : accessors)
+            return accessor->showPath(path);
+        return SourceAccessor::showPath(path);
+    }
+
+    std::optional<std::filesystem::path> getPhysicalPath(const CanonPath & path) override
+    {
+        for (auto & accessor : accessors) {
+            auto p = accessor->getPhysicalPath(path);
+            if (p)
+                return p;
+        }
+        return std::nullopt;
+    }
+};
+
+ref<SourceAccessor> makeUnionSourceAccessor(std::vector<ref<SourceAccessor>> && accessors)
+{
+    return make_ref<UnionSourceAccessor>(std::move(accessors));
+}
+
+}

src/nix/flake.cc

@@ -216,7 +216,7 @@ struct CmdFlakeMetadata : FlakeCommand, MixJSON
         auto & flake = lockedFlake.flake;
 
         // Currently, all flakes are in the Nix store via the rootFS accessor.
-        auto storePath = store->printStorePath(sourcePathToStorePath(store, flake.path).first);
+        auto storePath = store->printStorePath(store->toStorePath(flake.path.path.abs()).first);
 
         if (json) {
             nlohmann::json j;
@@ -1079,7 +1079,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun
 
         StorePathSet sources;
 
-        auto storePath = sourcePathToStorePath(store, flake.flake.path).first;
+        auto storePath = store->toStorePath(flake.flake.path.path.abs()).first;
 
         sources.insert(storePath);
 

tests/functional/chroot-store.sh

@@ -2,6 +2,28 @@
 
 source common.sh
 
+# Regression test for #11503.
+mkdir -p "$TEST_ROOT/directory"
+cat > "$TEST_ROOT/directory/default.nix" <<EOF
+  let
+    root = ./.;
+    filter = path: type:
+      let
+        rootStr = builtins.toString ./.;
+      in
+        if builtins.substring 0 (builtins.stringLength rootStr) (builtins.toString path) == rootStr then true
+        else builtins.throw "root path\n\${rootStr}\nnot prefix of path\n\${builtins.toString path}";
+  in
+    builtins.filterSource filter root
+EOF
+
+result="$(nix-store --add-fixed --recursive sha256 "$TEST_ROOT/directory")"
+nix-instantiate --eval "$result"
+nix-instantiate --eval "$result" --store "$TEST_ROOT/2nd-store"
+nix-store --add-fixed --recursive sha256 "$TEST_ROOT/directory" --store "$TEST_ROOT/2nd-store"
+nix-instantiate --eval "$result" --store "$TEST_ROOT/2nd-store"
+
+# Misc tests.
 echo example > "$TEST_ROOT"/example.txt
 mkdir -p "$TEST_ROOT/x"