Split out VisibleStore and ReferrersStore from Store

More progress on issue NixOS#5729

`queryAllValidPaths` and `queryReferrers` were two methods that failing
`unsupported` default implementation. This is not type safe. Now, those
methods are in their own classes, and there is no more `unsupported`
default instances of them.

(I thought we could get away with one class for both, but sadly that is
not quite the case: `LocalBinaryCacheStore` and `S3BinaryCacheStore`
implement the former but the latter.)

src/libcmd/command.cc

@@ -1,6 +1,8 @@
 #include "command.hh"
 #include "store-api.hh"
+#include "store-cast.hh"
 #include "local-fs-store.hh"
+#include "visible-store.hh"
 #include "derivations.hh"
 #include "nixexpr.hh"
 #include "profiles.hh"
@@ -169,10 +171,11 @@ void BuiltPathsCommand::run(ref<Store> store, Installables && installables)
 {
     BuiltPaths paths;
     if (all) {
+        auto & visibleStore = require<VisibleStore>(*store);
         if (installables.size())
             throw UsageError("'--all' does not expect arguments");
         // XXX: Only uses opaque paths, ignores all the realisations
-        for (auto & p : store->queryAllValidPaths())
+        for (auto & p : visibleStore.queryAllValidPaths())
             paths.push_back(BuiltPath::Opaque{p});
     } else {
         paths = Installable::toBuiltPaths(getEvalStore(), store, realiseMode, operateOn, installables);

src/libstore/build/local-derivation-goal.cc

@@ -1194,10 +1194,16 @@ struct RestrictedStoreConfig : virtual LocalFSStoreConfig
     const std::string name() { return "Restricted Store"; }
 };
 
-/* A wrapper around LocalStore that only allows building/querying of
-   paths that are in the input closures of the build or were added via
-   recursive Nix calls. */
-struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual LocalFSStore, public virtual GcStore
+/**
+ * A wrapper around `LocalStore` that only allows building/querying of
+ * paths that are in the input closures of the build or were added via
+ * recursive Nix calls.
+ */
+struct RestrictedStore
+    : public virtual RestrictedStoreConfig
+    , public virtual LocalFSStore
+    , public virtual GcStore
+    , public virtual ReferrersStore
 {
     ref<LocalStore> next;
 

src/libstore/daemon.cc

@@ -6,6 +6,7 @@
 #include "store-cast.hh"
 #include "gc-store.hh"
 #include "log-store.hh"
+#include "referrers-store.hh"
 #include "path-with-outputs.hh"
 #include "finally.hh"
 #include "archive.hh"
@@ -343,9 +344,10 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         if (op == wopQueryReferences)
             for (auto & i : store->queryPathInfo(path)->references)
                 paths.insert(i);
-        else if (op == wopQueryReferrers)
-            store->queryReferrers(path, paths);
-        else if (op == wopQueryValidDerivers)
+        else if (op == wopQueryReferrers) {
+            auto & referrersStore = require<ReferrersStore>(*store);
+            referrersStore.queryReferrers(path, paths);
+        } else if (op == wopQueryValidDerivers)
             paths = store->queryValidDerivers(path);
         else paths = store->queryDerivationOutputs(path);
         logger->stopWork();
@@ -803,7 +805,8 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
 
     case wopQueryAllValidPaths: {
         logger->startWork();
-        auto paths = store->queryAllValidPaths();
+        auto & visibleStore = require<VisibleStore>(*store);
+        auto paths = visibleStore.queryAllValidPaths();
         logger->stopWork();
         worker_proto::write(*store, to, paths);
         break;

src/libstore/local-binary-cache-store.cc

@@ -1,4 +1,5 @@
 #include "binary-cache-store.hh"
+#include "visible-store.hh"
 #include "globals.hh"
 #include "nar-info-disk-cache.hh"
 
@@ -20,7 +21,7 @@ struct LocalBinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
     }
 };
 
-class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public virtual BinaryCacheStore
+class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public virtual BinaryCacheStore, public virtual VisibleStore
 {
 private:
 

src/libstore/local-store.hh

@@ -7,6 +7,7 @@
 #include "store-api.hh"
 #include "local-fs-store.hh"
 #include "gc-store.hh"
+#include "referrers-store.hh"
 #include "sync.hh"
 #include "util.hh"
 
@@ -51,7 +52,7 @@ struct LocalStoreConfig : virtual LocalFSStoreConfig
     std::string doc() override;
 };
 
-class LocalStore : public virtual LocalStoreConfig, public virtual LocalFSStore, public virtual GcStore
+class LocalStore : public virtual LocalStoreConfig, public virtual LocalFSStore, public virtual GcStore, public virtual ReferrersStore
 {
 private:
 

src/libstore/misc.cc

@@ -3,6 +3,8 @@
 #include "globals.hh"
 #include "local-store.hh"
 #include "store-api.hh"
+#include "referrers-store.hh"
+#include "store-cast.hh"
 #include "thread-pool.hh"
 #include "topo-sort.hh"
 #include "callback.hh"
@@ -15,18 +17,19 @@ void Store::computeFSClosure(const StorePathSet & startPaths,
     StorePathSet & paths_, bool flipDirection, bool includeOutputs, bool includeDerivers)
 {
     std::function<std::set<StorePath>(const StorePath & path, std::future<ref<const ValidPathInfo>> &)> queryDeps;
-    if (flipDirection)
+    if (flipDirection) {
+        auto & referrersStore = require<ReferrersStore>(*this);
         queryDeps = [&](const StorePath& path,
                         std::future<ref<const ValidPathInfo>> & fut) {
             StorePathSet res;
             StorePathSet referrers;
-            queryReferrers(path, referrers);
+            referrersStore.queryReferrers(path, referrers);
             for (auto& ref : referrers)
                 if (ref != path)
                     res.insert(ref);
 
             if (includeOutputs)
-                for (auto& i : queryValidDerivers(path))
+                for (auto& i : referrersStore.queryValidDerivers(path))
                     res.insert(i);
 
             if (includeDerivers && path.isDerivation())
@@ -35,7 +38,7 @@ void Store::computeFSClosure(const StorePathSet & startPaths,
                         res.insert(*maybeOutPath);
             return res;
         };
-    else
+    } else
         queryDeps = [&](const StorePath& path,
                         std::future<ref<const ValidPathInfo>> & fut) {
             StorePathSet res;

src/libstore/referrers-store.hh

@@ -0,0 +1,38 @@
+#pragma once
+///@file
+
+#include "visible-store.hh"
+
+namespace nix {
+
+/**
+ * A store that allows querying referrers.
+ *
+ * The referrers relation is the dual of the references relation, the
+ * latter being the "regular" one we are usually interested in.
+ *
+ * This is no inherent reason why this should be a subclass of
+ * `VisibleStore`; it just so happens that every extent store object we
+ * have to day that implements `queryReferrers()` also implements
+ * `queryAllValidPaths()`. If that ceases to be the case, we can revisit
+ * this; until this having this interface inheritance means fewer
+ * interface combinations to think about.
+ */
+struct ReferrersStore : virtual VisibleStore
+{
+    inline static std::string operationName = "Query referrers";
+
+    /**
+     * Queries the set of incoming FS references for a store path.
+     * The result is not cleared.
+     *
+     * @param path The path of the store object we care about incoming
+     * references to.
+     *
+     * @param [out] referrers The set in which to collect the referrers
+     * of `path`.
+     */
+    virtual void queryReferrers(const StorePath & path, StorePathSet & referrers) = 0;
+};
+
+}

src/libstore/remote-store.hh

@@ -7,6 +7,7 @@
 #include "store-api.hh"
 #include "gc-store.hh"
 #include "log-store.hh"
+#include "referrers-store.hh"
 
 
 namespace nix {
@@ -39,7 +40,9 @@ struct RemoteStoreConfig : virtual StoreConfig
 class RemoteStore : public virtual RemoteStoreConfig,
     public virtual Store,
     public virtual GcStore,
-    public virtual LogStore
+    public virtual LogStore,
+    public virtual ReferrersStore
+
 {
 public:
 

src/libstore/s3-binary-cache-store.cc

@@ -2,6 +2,7 @@
 
 #include "s3.hh"
 #include "s3-binary-cache-store.hh"
+#include "visible-store.hh"
 #include "nar-info.hh"
 #include "nar-info-disk-cache.hh"
 #include "globals.hh"
@@ -260,7 +261,7 @@ struct S3BinaryCacheStoreConfig : virtual BinaryCacheStoreConfig
     }
 };
 
-struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual S3BinaryCacheStore
+struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual S3BinaryCacheStore, public VisibleStore
 {
     std::string bucketName;
 

src/libstore/store-api.hh

@@ -336,17 +336,6 @@ public:
     virtual StorePathSet queryValidPaths(const StorePathSet & paths,
         SubstituteFlag maybeSubstitute = NoSubstitute);
 
-    /**
-     * Query the set of all valid paths. Note that for some store
-     * backends, the name part of store paths may be replaced by 'x'
-     * (i.e. you'll get /nix/store/<hash>-x rather than
-     * /nix/store/<hash>-<name>). Use queryPathInfo() to obtain the
-     * full store path. FIXME: should return a set of
-     * std::variant<StorePath, HashPart> to get rid of this hack.
-     */
-    virtual StorePathSet queryAllValidPaths()
-    { unsupported("queryAllValidPaths"); }
-
     constexpr static const char * MissingName = "x";
 
     /**
@@ -403,13 +392,6 @@ protected:
 
 public:
 
-    /**
-     * Queries the set of incoming FS references for a store path.
-     * The result is not cleared.
-     */
-    virtual void queryReferrers(const StorePath & path, StorePathSet & referrers)
-    { unsupported("queryReferrers"); }
-
     /**
      * @return all currently valid derivations that have `path` as an
      * output.

src/libstore/visible-store.hh

@@ -0,0 +1,42 @@
+#pragma once
+///@file
+
+#include "store-api.hh"
+
+namespace nix {
+
+/**
+ * A Store that exposes all store objects.
+ *
+ * ### Privacy and Security
+ *
+ * For the base `Store` class, we aim for `StorePath`s to act as
+ * capabilities: only store objects which are reachable from the store
+ * objects the user has (i.e. those directly-referenced objects and
+ * their reference closure).
+ *
+ * A `VisibleStore` breaks this by exposing these methods that allow
+ * discovering other store objects, outside the "reachable set" as
+ * defined above. This is necessary to implement certain operations, but
+ * care must taken exposing this functionality to the user as it makes
+ * e.g. secret management and other security properties trickier to get
+ * right.
+ */
+struct VisibleStore : virtual Store
+{
+    inline static std::string operationName = "Query all valid paths";
+
+    /**
+     * Query the set of all valid paths. Note that for some store
+     * backends, the name part of store paths may be replaced by 'x'
+     * (i.e. you'll get `/nix/store/<hash>-x` rather than
+     * `/nix/store/<hash>-<name>`). Use `queryPathInfo()` to obtain the
+     * full store path.
+     *
+     * \todo should return a set of `std::variant<StorePath, HashPart>`
+     * to get rid of this hack.
+     */
+    virtual StorePathSet queryAllValidPaths() = 0;
+};
+
+}

src/nix-store/nix-store.cc

@@ -6,6 +6,7 @@
 #include "store-cast.hh"
 #include "gc-store.hh"
 #include "log-store.hh"
+#include "referrers-store.hh"
 #include "local-store.hh"
 #include "monitor-fd.hh"
 #include "serve-protocol.hh"
@@ -335,6 +336,7 @@ static void opQuery(Strings opFlags, Strings opArgs)
         case qReferences:
         case qReferrers:
         case qReferrersClosure: {
+            auto & referrersStore = require<ReferrersStore>(*store);
             StorePathSet paths;
             for (auto & i : opArgs) {
                 auto ps = maybeUseOutputs(store->followLinksToStorePath(i), useOutput, forceRealise);
@@ -346,7 +348,7 @@ static void opQuery(Strings opFlags, Strings opArgs)
                     }
                     else if (query == qReferrers) {
                         StorePathSet tmp;
-                        store->queryReferrers(j, tmp);
+                        referrersStore.queryReferrers(j, tmp);
                         for (auto & i : tmp)
                             paths.insert(i);
                     }
@@ -495,12 +497,13 @@ static void opReadLog(Strings opFlags, Strings opArgs)
 
 static void opDumpDB(Strings opFlags, Strings opArgs)
 {
+    auto & visibleStore = require<VisibleStore>(*store);
     if (!opFlags.empty()) throw UsageError("unknown flag");
     if (!opArgs.empty()) {
         for (auto & i : opArgs)
             cout << store->makeValidityRegistration({store->followLinksToStorePath(i)}, true, true);
     } else {
-        for (auto & i : store->queryAllValidPaths())
+        for (auto & i : visibleStore.queryAllValidPaths())
             cout << store->makeValidityRegistration({i}, true, true);
     }
 }