From ccf658ed5c1093e0bb3a3249688206cca7f1b2e1 Mon Sep 17 00:00:00 2001 From: Leandro Reina Date: Wed, 13 Aug 2025 16:40:55 +0200 Subject: [PATCH] Fix Git LFS SSH issues * Adds support for NIX_SSHOPTS * Properly uses the parsed port from URL (fixes #13337) * Don't guess the HTTP endpoint, use the response of git-lfs-authenticate * Add an SSH Git LFS test * Removed some unused test code --- doc/manual/rl-next/git-lfs-ssh.md | 11 +++ src/libfetchers/git-lfs-fetch.cc | 93 ++++++++++++------- src/libstore/include/nix/store/ssh.hh | 2 + src/libstore/ssh.cc | 24 +++-- .../fetch-git/test-cases/lfs/default.nix | 20 ++++ .../fetch-git/testsupport/gitea-repo.nix | 12 +-- tests/nixos/fetch-git/testsupport/gitea.nix | 51 ++++------ 7 files changed, 128 insertions(+), 85 deletions(-) create mode 100644 doc/manual/rl-next/git-lfs-ssh.md diff --git a/doc/manual/rl-next/git-lfs-ssh.md b/doc/manual/rl-next/git-lfs-ssh.md new file mode 100644 index 000000000..c49addf13 --- /dev/null +++ b/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. diff --git a/src/libfetchers/git-lfs-fetch.cc b/src/libfetchers/git-lfs-fetch.cc index 1337c5b83..35230ae88 100644 --- a/src/libfetchers/git-lfs-fetch.cc +++ b/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 #include @@ -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 & 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 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(); + 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(), authIt->get()}; + } + + return {url.to_string() + "/info/lfs", std::nullopt}; } typedef std::unique_ptr> GitConfig; @@ -181,13 +204,14 @@ static nlohmann::json pointerToPayload(const std::vector & items) std::vector Fetch::fetchUrls(const std::vector & 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 { + 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); diff --git a/src/libstore/include/nix/store/ssh.hh b/src/libstore/include/nix/store/ssh.hh index 6eb38acef..c7228464b 100644 --- a/src/libstore/include/nix/store/ssh.hh +++ b/src/libstore/include/nix/store/ssh.hh @@ -8,6 +8,8 @@ namespace nix { +Strings getNixSshOpts(); + class SSHMaster { private: diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index 8ed72c643..8a4614a0d 100644 --- a/src/libstore/ssh.cc +++ b/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 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}); diff --git a/tests/nixos/fetch-git/test-cases/lfs/default.nix b/tests/nixos/fetch-git/test-cases/lfs/default.nix index 686796fcc..289c37709 100644 --- a/tests/nixos/fetch-git/test-cases/lfs/default.nix +++ b/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})" ''; } diff --git a/tests/nixos/fetch-git/testsupport/gitea-repo.nix b/tests/nixos/fetch-git/testsupport/gitea-repo.nix index c8244207f..826ae52f9 100644 --- a/tests/nixos/fetch-git/testsupport/gitea-repo.nix +++ b/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} """) ''; }; diff --git a/tests/nixos/fetch-git/testsupport/gitea.nix b/tests/nixos/fetch-git/testsupport/gitea.nix index e63182639..ad88d4ff0 100644 --- a/tests/nixos/fetch-git/testsupport/gitea.nix +++ b/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 - """) - ''; }