From e7f95783dbc0035827397d4a634ec4654ed15673 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Wed, 26 Nov 2025 19:38:42 +0100
Subject: [PATCH] Move GitHub input attribute validation into inputFromAttrs()

Previously inputFromAttrs() didn't do any validation. inputFromURL()
now calls inputFromAttrs(), so we only need to validate in one place.

Fixes #14655.
---
 src/libfetchers/github.cc | 63 ++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 30 deletions(-)

diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc
index cd7ce1b4e..b86fa926a 100644
--- a/src/libfetchers/github.cc
+++ b/src/libfetchers/github.cc
@@ -41,18 +41,16 @@ struct GitArchiveInputScheme : InputScheme
         /* This ignores empty path segments for back-compat. Older versions used a tokenizeString here. */
         auto path = url.pathSegments(/*skipEmpty=*/true) | std::ranges::to<std::vector<std::string>>();
 
-        std::optional<Hash> rev;
+        std::optional<std::string> rev;
         std::optional<std::string> ref;
         std::optional<std::string> host_url;
 
         auto size = path.size();
         if (size == 3) {
             if (std::regex_match(path[2], revRegex))
-                rev = Hash::parseAny(path[2], HashAlgorithm::SHA1);
-            else if (isLegalRefName(path[2]))
-                ref = path[2];
+                rev = path[2];
             else
-                throw BadURL("in URL '%s', '%s' is not a commit hash or branch/tag name", url, path[2]);
+                ref = path[2];
         } else if (size > 3) {
             std::string rs;
             for (auto i = std::next(path.begin(), 2); i != path.end(); i++) {
@@ -61,12 +59,7 @@ struct GitArchiveInputScheme : InputScheme
                     rs += "/";
                 }
             }
-
-            if (isLegalRefName(rs)) {
-                ref = rs;
-            } else {
-                throw BadURL("in URL '%s', '%s' is not a branch/tag name", url, rs);
-            }
+            ref = rs;
         } else if (size < 2)
             throw BadURL("URL '%s' is invalid", url);
 
@@ -74,40 +67,32 @@ struct GitArchiveInputScheme : InputScheme
             if (name == "rev") {
                 if (rev)
                     throw BadURL("URL '%s' contains multiple commit hashes", url);
-                rev = Hash::parseAny(value, HashAlgorithm::SHA1);
+                rev = value;
             } else if (name == "ref") {
-                if (!isLegalRefName(value))
-                    throw BadURL("URL '%s' contains an invalid branch/tag name", url);
                 if (ref)
                     throw BadURL("URL '%s' contains multiple branch/tag names", url);
                 ref = value;
-            } else if (name == "host") {
-                if (!std::regex_match(value, hostRegex))
-                    throw BadURL("URL '%s' contains an invalid instance host", url);
+            } else if (name == "host")
                 host_url = value;
-            }
             // FIXME: barf on unsupported attributes
         }
 
-        if (ref && rev)
-            throw BadURL("URL '%s' contains both a commit hash and a branch/tag name %s %s", url, *ref, rev->gitRev());
-
-        Input input{};
-        input.attrs.insert_or_assign("type", std::string{schemeName()});
-        input.attrs.insert_or_assign("owner", path[0]);
-        input.attrs.insert_or_assign("repo", path[1]);
+        Attrs attrs;
+        attrs.insert_or_assign("type", std::string{schemeName()});
+        attrs.insert_or_assign("owner", path[0]);
+        attrs.insert_or_assign("repo", path[1]);
         if (rev)
-            input.attrs.insert_or_assign("rev", rev->gitRev());
+            attrs.insert_or_assign("rev", *rev);
         if (ref)
-            input.attrs.insert_or_assign("ref", *ref);
+            attrs.insert_or_assign("ref", *ref);
         if (host_url)
-            input.attrs.insert_or_assign("host", *host_url);
+            attrs.insert_or_assign("host", *host_url);
 
         auto narHash = url.query.find("narHash");
         if (narHash != url.query.end())
-            input.attrs.insert_or_assign("narHash", narHash->second);
+            attrs.insert_or_assign("narHash", narHash->second);
 
-        return input;
+        return inputFromAttrs(settings, attrs);
     }
 
     const std::map<std::string, AttributeInfo> & allowedAttrs() const override
@@ -154,6 +139,24 @@ struct GitArchiveInputScheme : InputScheme
         getStrAttr(attrs, "owner");
         getStrAttr(attrs, "repo");
 
+        auto ref = maybeGetStrAttr(attrs, "ref");
+        auto rev = maybeGetStrAttr(attrs, "rev");
+        if (ref && rev)
+            throw BadURL(
+                "input %s contains both a commit hash ('%s') and a branch/tag name ('%s')",
+                attrsToJSON(attrs),
+                *rev,
+                *ref);
+
+        if (rev)
+            Hash::parseAny(*rev, HashAlgorithm::SHA1);
+
+        if (ref && !isLegalRefName(*ref))
+            throw BadURL("input %s contains an invalid branch/tag name", attrsToJSON(attrs));
+
+        if (auto host = maybeGetStrAttr(attrs, "host"); host && !std::regex_match(*host, hostRegex))
+            throw BadURL("input %s contains an invalid instance host", attrsToJSON(attrs));
+
         Input input{};
         input.attrs = attrs;
         return input;