Merge pull request #13743 from kip93/fix/lfs-ssh

Fix Git LFS SSH issues

doc/manual/rl-next/git-lfs-ssh.md

@@ -0,0 +1,11 @@
+---
+synopsis: "Fix Git LFS SSH issues"
+prs: [13743]
+issues: [13337]
+---
+
+Fixed some outstanding issues with Git LFS and SSH.
+
+* Added support for `NIX_SSHOPTS`.
+* Properly use the parsed port from URL.
+* Better use of the response of `git-lfs-authenticate` to determine API endpoint when the API is not exposed on port 443.

src/libfetchers/git-lfs-fetch.cc

@@ -5,6 +5,7 @@
 #include "nix/util/url.hh"
 #include "nix/util/users.hh"
 #include "nix/util/hash.hh"
+#include "nix/store/ssh.hh"
 
 #include <git2/attr.h>
 #include <git2/config.h>
@@ -15,10 +16,9 @@
 
 namespace nix::lfs {
 
-// if authHeader is "", downloadToSink assumes no auth is expected
 static void downloadToSink(
     const std::string & url,
-    const std::string & authHeader,
+    const std::optional<std::string> & authHeader,
     // FIXME: passing a StringSink is superfluous, we may as well
     // return a string. Or use an abstract Sink for streaming.
     StringSink & sink,
@@ -27,8 +27,8 @@ static void downloadToSink(
 {
     FileTransferRequest request(url);
     Headers headers;
-    if (!authHeader.empty())
-        headers.push_back({"Authorization", authHeader});
+    if (authHeader.has_value())
+        headers.push_back({"Authorization", *authHeader});
     request.headers = headers;
     getFileTransfer()->download(std::move(request), sink);
 
@@ -42,30 +42,53 @@ static void downloadToSink(
             "hash mismatch while fetching %s: expected sha256:%s but got sha256:%s", url, sha256Expected, sha256Actual);
 }
 
-static std::string getLfsApiToken(const ParsedURL & url)
+namespace {
+
+struct LfsApiInfo
+{
+    std::string endpoint;
+    std::optional<std::string> authHeader;
+};
+
+} // namespace
+
+static LfsApiInfo getLfsApi(const ParsedURL & url)
 {
     assert(url.authority.has_value());
+    if (url.scheme == "ssh") {
+        auto args = getNixSshOpts();
 
-    // FIXME: Not entirely correct.
-    auto [status, output] = runProgram(
-        RunOptions{
-            .program = "ssh",
-            .args = {url.authority->to_string(), "git-lfs-authenticate", url.path, "download"},
-        });
+        if (url.authority->port)
+            args.push_back(fmt("-p%d", *url.authority->port));
 
-    if (output.empty())
-        throw Error(
-            "git-lfs-authenticate: no output (cmd: ssh %s git-lfs-authenticate %s download)",
-            url.authority.value_or(ParsedURL::Authority{}).to_string(),
-            url.path);
+        std::ostringstream hostnameAndUser;
+        if (url.authority->user)
+            hostnameAndUser << *url.authority->user << "@";
+        hostnameAndUser << url.authority->host;
+        args.push_back(std::move(hostnameAndUser).str());
 
-    auto queryResp = nlohmann::json::parse(output);
-    if (!queryResp.contains("header"))
-        throw Error("no header in git-lfs-authenticate response");
-    if (!queryResp["header"].contains("Authorization"))
-        throw Error("no Authorization in git-lfs-authenticate response");
+        args.push_back("--");
+        args.push_back("git-lfs-authenticate");
+        args.push_back(url.path);
+        args.push_back("download");
 
-    return queryResp["header"]["Authorization"].get<std::string>();
+        auto [status, output] = runProgram({.program = "ssh", .args = args});
+
+        if (output.empty())
+            throw Error("git-lfs-authenticate: no output (cmd: 'ssh %s')", concatStringsSep(" ", args));
+
+        auto queryResp = nlohmann::json::parse(output);
+        auto headerIt = queryResp.find("header");
+        if (headerIt == queryResp.end())
+            throw Error("no header in git-lfs-authenticate response");
+        auto authIt = headerIt->find("Authorization");
+        if (authIt == headerIt->end())
+            throw Error("no Authorization in git-lfs-authenticate response");
+
+        return {queryResp.at("href").get<std::string>(), authIt->get<std::string>()};
+    }
+
+    return {url.to_string() + "/info/lfs", std::nullopt};
 }
 
 typedef std::unique_ptr<git_config, Deleter<git_config_free>> GitConfig;
@@ -181,13 +204,14 @@ static nlohmann::json pointerToPayload(const std::vector<Pointer> & items)
 
 std::vector<nlohmann::json> Fetch::fetchUrls(const std::vector<Pointer> & pointers) const
 {
-    ParsedURL httpUrl(url);
-    httpUrl.scheme = url.scheme == "ssh" ? "https" : url.scheme;
-    FileTransferRequest request(httpUrl.to_string() + "/info/lfs/objects/batch");
+    auto api = lfs::getLfsApi(this->url);
+    auto url = api.endpoint + "/objects/batch";
+    const auto & authHeader = api.authHeader;
+    FileTransferRequest request(url);
     request.post = true;
     Headers headers;
-    if (this->url.scheme == "ssh")
-        headers.push_back({"Authorization", lfs::getLfsApiToken(this->url)});
+    if (authHeader.has_value())
+        headers.push_back({"Authorization", *authHeader});
     headers.push_back({"Content-Type", "application/vnd.git-lfs+json"});
     headers.push_back({"Accept", "application/vnd.git-lfs+json"});
     request.headers = headers;
@@ -260,11 +284,16 @@ void Fetch::fetch(
     try {
         std::string sha256 = obj.at("oid"); // oid is also the sha256
         std::string ourl = obj.at("actions").at("download").at("href");
-        std::string authHeader = "";
-        if (obj.at("actions").at("download").contains("header")
-            && obj.at("actions").at("download").at("header").contains("Authorization")) {
-            authHeader = obj["actions"]["download"]["header"]["Authorization"];
-        }
+        auto authHeader = [&]() -> std::optional<std::string> {
+            const auto & download = obj.at("actions").at("download");
+            auto headerIt = download.find("header");
+            if (headerIt == download.end())
+                return std::nullopt;
+            auto authIt = headerIt->find("Authorization");
+            if (authIt == headerIt->end())
+                return std::nullopt;
+            return *authIt;
+        }();
         const uint64_t size = obj.at("size");
         sizeCallback(size);
         downloadToSink(ourl, authHeader, sink, sha256, size);

src/libstore/include/nix/store/ssh.hh

@@ -8,6 +8,8 @@
 
 namespace nix {
 
+Strings getNixSshOpts();
+
 class SSHMaster
 {
 private:

src/libstore/ssh.cc

@@ -51,6 +51,18 @@ static void checkValidAuthority(const ParsedURL::Authority & authority)
     }
 }
 
+Strings getNixSshOpts()
+{
+    std::string sshOpts = getEnv("NIX_SSHOPTS").value_or("");
+
+    try {
+        return shellSplitString(sshOpts);
+    } catch (Error & e) {
+        e.addTrace({}, "while splitting NIX_SSHOPTS '%s'", sshOpts);
+        throw;
+    }
+}
+
 SSHMaster::SSHMaster(
     const ParsedURL::Authority & authority,
     std::string_view keyFile,
@@ -82,16 +94,8 @@ void SSHMaster::addCommonSSHOpts(Strings & args)
 {
     auto state(state_.lock());
 
-    std::string sshOpts = getEnv("NIX_SSHOPTS").value_or("");
-
-    try {
-        std::list<std::string> opts = shellSplitString(sshOpts);
-        for (auto & i : opts)
-            args.push_back(i);
-    } catch (Error & e) {
-        e.addTrace({}, "while splitting NIX_SSHOPTS '%s'", sshOpts);
-        throw;
-    }
+    auto sshArgs = getNixSshOpts();
+    args.insert(args.end(), sshArgs.begin(), sshArgs.end());
 
     if (!keyFile.empty())
         args.insert(args.end(), {"-i", keyFile});

tests/nixos/fetch-git/test-cases/lfs/default.nix

@@ -224,5 +224,25 @@
         """)
 
     client.succeed(f"cmp {repo.path}/beeg {fetched_self_lfs}/beeg >&2")
+
+
+    with subtest("Ensure fetching with SSH generates the same output"):
+      client.succeed(f"{repo.git} push origin-ssh main >&2")
+      client.succeed("rm -rf ~/.cache/nix") # Avoid using the cached output of the http fetch
+
+      fetchGit_ssh_expr = f"""
+        builtins.fetchGit {{
+          url = "{repo.remote_ssh}";
+          rev = "{lfs_file_rev}";
+          ref = "main";
+          lfs = true;
+        }}
+      """
+      fetched_ssh = client.succeed(f"""
+        nix eval --debug --impure --raw --expr '({fetchGit_ssh_expr}).outPath'
+      """)
+
+      assert fetched_ssh == fetched_lfs, \
+        f"fetching with ssh (store path {fetched_ssh}) yielded a different result than using http (store path {fetched_lfs})"
   '';
 }

tests/nixos/fetch-git/testsupport/gitea-repo.nix

@@ -49,19 +49,15 @@ in
           self.name = name
           self.path = "/tmp/repos/" + name
           self.remote = "http://gitea:3000/test/" + name
-          self.remote_ssh = "ssh://gitea/root/" + name
+          self.remote_ssh = "ssh://gitea:3001/test/" + name
           self.git = f"git -C {self.path}"
           self.private = private
           self.create()
 
         def create(self):
-          # create ssh remote repo
+          # create remote repo
           gitea.succeed(f"""
-            git init --bare -b main /root/{self.name}
-          """)
-          # create http remote repo
-          gitea.succeed(f"""
-            curl --fail -X POST http://{gitea_admin}:{gitea_admin_password}@gitea:3000/api/v1/user/repos \
+            curl --fail -X POST http://{gitea_user}:{gitea_password}@gitea:3000/api/v1/user/repos \
               -H 'Accept: application/json' -H 'Content-Type: application/json' \
               -d {shlex.quote( f'{{"name":"{self.name}", "default_branch": "main", "private": {boolToJSON(self.private)}}}' )}
           """)
@@ -70,7 +66,7 @@ in
             mkdir -p {self.path} \
             && git init -b main {self.path} \
             && {self.git} remote add origin {self.remote} \
-            && {self.git} remote add origin-ssh root@gitea:{self.name}
+            && {self.git} remote add origin-ssh {self.remote_ssh}
           """)
     '';
   };

tests/nixos/fetch-git/testsupport/gitea.nix

@@ -35,28 +35,20 @@ in
           server = {
             DOMAIN = "gitea";
             HTTP_PORT = 3000;
+            SSH_PORT = 3001;
+            START_SSH_SERVER = true;
           };
           log.LEVEL = "Info";
           database.LOG_SQL = false;
         };
-        services.openssh.enable = true;
-        networking.firewall.allowedTCPPorts = [ 3000 ];
+        networking.firewall.allowedTCPPorts = [
+          3000
+          3001
+        ];
         environment.systemPackages = [
           pkgs.git
           pkgs.gitea
         ];
-
-        users.users.root.openssh.authorizedKeys.keys = [ clientPublicKey ];
-
-        # TODO: remove this after updating to nixos-23.11
-        nixpkgs.pkgs = lib.mkForce (
-          import nixpkgs {
-            inherit system;
-            config.permittedInsecurePackages = [
-              "gitea-1.19.4"
-            ];
-          }
-        );
       };
     client =
       { pkgs, ... }:
@@ -67,38 +59,33 @@ in
         ];
       };
   };
-  defaults =
-    { pkgs, ... }:
-    {
-      environment.systemPackages = [ pkgs.jq ];
-    };
 
   setupScript = ''
     import shlex
 
     gitea.wait_for_unit("gitea.service")
 
-    gitea_admin = "test"
-    gitea_admin_password = "test123test"
+    gitea_user = "test"
+    gitea_password = "test123test"
 
     gitea.succeed(f"""
       gitea --version >&2
       su -l gitea -c 'GITEA_WORK_DIR=/var/lib/gitea gitea admin user create \
-        --username {gitea_admin} --password {gitea_admin_password} --email test@client'
+        --username {gitea_user} --password {gitea_password} --email test@client'
     """)
 
     client.wait_for_unit("multi-user.target")
     gitea.wait_for_open_port(3000)
+    gitea.wait_for_open_port(3001)
 
-    gitea_admin_token = gitea.succeed(f"""
-      curl --fail -X POST http://{gitea_admin}:{gitea_admin_password}@gitea:3000/api/v1/users/test/tokens \
+    gitea.succeed(f"""
+      curl --fail -X POST http://{gitea_user}:{gitea_password}@gitea:3000/api/v1/user/keys \
         -H 'Accept: application/json' -H 'Content-Type: application/json' \
-        -d {shlex.quote( '{"name":"token", "scopes":["all"]}' )} \
-        | jq -r '.sha1'
-    """).strip()
+        -d {shlex.quote( '{"title":"key", "key":"${clientPublicKey}", "read_only": false}' )} >&2
+    """)
 
     client.succeed(f"""
-      echo "http://{gitea_admin}:{gitea_admin_password}@gitea:3000" >~/.git-credentials-admin
+      echo "http://{gitea_user}:{gitea_password}@gitea:3000" >~/.git-credentials-admin
       git config --global credential.helper 'store --file ~/.git-credentials-admin'
       git config --global user.email "test@client"
       git config --global user.name "Test User"
@@ -118,13 +105,7 @@ in
       echo "Host gitea" >>~/.ssh/config
       echo "  StrictHostKeyChecking no" >>~/.ssh/config
       echo "  UserKnownHostsFile /dev/null" >>~/.ssh/config
-      echo "  User root" >>~/.ssh/config
+      echo "  User gitea" >>~/.ssh/config
     """)
-
-    # ensure ssh from client to gitea works
-    client.succeed("""
-      ssh root@gitea true
-    """)
-
   '';
 }