From 219510b6abcdd53647e4874d8528ac85c7528249 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Fri, 3 Feb 2023 15:31:36 +0100
Subject: [PATCH] GitArchiveInputScheme: Verify the locked tree hash

---
 src/libfetchers/fetchers.cc | 22 +++++++++++-----------
 src/libfetchers/fetchers.hh |  6 ++----
 src/libfetchers/github.cc   | 27 +++++++++++++++++++++++----
 3 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc
index 98dcf38e4..730727dc2 100644
--- a/src/libfetchers/fetchers.cc
+++ b/src/libfetchers/fetchers.cc
@@ -132,24 +132,24 @@ std::pair<StorePath, Input> Input::fetchToStore(ref<Store> store) const
     return {std::move(storePath), input};
 }
 
-void Input::checkLocks(Input & input) const
+void InputScheme::checkLocks(const Input & specified, const Input & final) const
 {
-    if (auto prevNarHash = getNarHash()) {
-        if (input.getNarHash() != prevNarHash)
+    if (auto prevNarHash = specified.getNarHash()) {
+        if (final.getNarHash() != prevNarHash)
             throw Error((unsigned int) 102, "NAR hash mismatch in input '%s', expected '%s'",
-                to_string(), prevNarHash->to_string(SRI, true));
+                specified.to_string(), prevNarHash->to_string(SRI, true));
     }
 
-    if (auto prevLastModified = getLastModified()) {
-        if (input.getLastModified() != prevLastModified)
+    if (auto prevLastModified = specified.getLastModified()) {
+        if (final.getLastModified() != prevLastModified)
             throw Error("'lastModified' attribute mismatch in input '%s', expected %d",
-                input.to_string(), *prevLastModified);
+                final.to_string(), *prevLastModified);
     }
 
-    if (auto prevRevCount = getRevCount()) {
-        if (input.getRevCount() != prevRevCount)
+    if (auto prevRevCount = specified.getRevCount()) {
+        if (final.getRevCount() != prevRevCount)
             throw Error("'revCount' attribute mismatch in input '%s', expected %d",
-                input.to_string(), *prevRevCount);
+                final.to_string(), *prevRevCount);
     }
 }
 
@@ -163,7 +163,7 @@ std::pair<ref<InputAccessor>, Input> Input::getAccessor(ref<Store> store) const
     try {
         auto [accessor, final] = scheme->getAccessor(store, *this);
         accessor->fingerprint = scheme->getFingerprint(store, final);
-        checkLocks(final);
+        scheme->checkLocks(*this, final);
         return {accessor, std::move(final)};
     } catch (Error & e) {
         e.addTrace({}, "while fetching the input '%s'", to_string());
diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh
index c2753da30..1b06e34b1 100644
--- a/src/libfetchers/fetchers.hh
+++ b/src/libfetchers/fetchers.hh
@@ -92,10 +92,6 @@ public:
     // For locked inputs, returns a string that uniquely specifies the
     // content of the input (typically a commit hash or content hash).
     std::optional<std::string> getFingerprint(ref<Store> store) const;
-
-private:
-
-    void checkLocks(Input & input) const;
 };
 
 /* The InputScheme represents a type of fetcher.  Each fetcher
@@ -142,6 +138,8 @@ struct InputScheme
     { return std::nullopt; }
 
     virtual std::optional<std::string> getFingerprint(ref<Store> store, const Input & input) const;
+
+    virtual void checkLocks(const Input & specified, const Input & final) const;
 };
 
 void registerInputScheme(std::shared_ptr<InputScheme> && fetcher);
diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc
index b6abb7a4a..756dcef8b 100644
--- a/src/libfetchers/github.cc
+++ b/src/libfetchers/github.cc
@@ -107,8 +107,11 @@ struct GitArchiveInputScheme : InputScheme
     {
         if (maybeGetStrAttr(attrs, "type") != type()) return {};
 
+        static std::unordered_set<std::string> known =
+            {"type", "owner", "repo", "ref", "rev", "narHash", "lastModified", "host", "treeHash"};
+
         for (auto & [name, value] : attrs)
-            if (name != "type" && name != "owner" && name != "repo" && name != "ref" && name != "rev" && name != "narHash" && name != "lastModified" && name != "host")
+            if (!known.contains(name))
                 throw Error("unsupported input attribute '%s'", name);
 
         getStrAttr(attrs, "owner");
@@ -155,6 +158,23 @@ struct GitArchiveInputScheme : InputScheme
         return input;
     }
 
+    std::optional<Hash> getTreeHash(const Input & input) const
+    {
+        if (auto treeHash = maybeGetStrAttr(input.attrs, "treeHash"))
+            return Hash::parseAny(*treeHash, htSHA1);
+        else
+            return std::nullopt;
+    }
+
+    void checkLocks(const Input & specified, const Input & final) const override
+    {
+        if (auto prevTreeHash = getTreeHash(specified)) {
+            if (getTreeHash(final) != prevTreeHash)
+                throw Error("Git tree hash mismatch in input '%s', expected '%s'",
+                    specified.to_string(), prevTreeHash->gitRev());
+        }
+    }
+
     std::optional<std::string> getAccessToken(const std::string & host) const
     {
         auto tokens = fetchSettings.accessTokens.get();
@@ -214,9 +234,6 @@ struct GitArchiveInputScheme : InputScheme
 
         auto treeHash = importTarball(*source);
 
-        // FIXME: verify against locked tree hash.
-        input.attrs.insert_or_assign("treeHash", treeHash.gitRev());
-
         cache->upsertFact(treeHashKey, treeHash.gitRev());
 
         return {std::move(input), treeHash};
@@ -226,6 +243,8 @@ struct GitArchiveInputScheme : InputScheme
     {
         auto [input, treeHash] = downloadArchive(store, _input);
 
+        input.attrs.insert_or_assign("treeHash", treeHash.gitRev());
+
         auto accessor = makeTarballCacheAccessor(treeHash);
 
         #if 0