Skip to content

Commit 8e5ca78

Browse files
xokdviumxokdvium
authored
Merge pull request #13743 from kip93/fix/lfs-ssh
Fix Git LFS SSH issues
2 parents 68839b9 + ccf658e commit 8e5ca78

7 files changed

Lines changed: 128 additions & 85 deletions

File tree

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
synopsis: "Fix Git LFS SSH issues"
3+
prs: [13743]
4+
issues: [13337]
5+
---
6+
7+
Fixed some outstanding issues with Git LFS and SSH.
8+
9+
* Added support for `NIX_SSHOPTS`.
10+
* Properly use the parsed port from URL.
11+
* 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‎

Lines changed: 61 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "nix/util/url.hh"
66
#include "nix/util/users.hh"
77
#include "nix/util/hash.hh"
8+
#include "nix/store/ssh.hh"
89

910
#include <git2/attr.h>
1011
#include <git2/config.h>
@@ -15,10 +16,9 @@
1516

1617
namespace nix::lfs {
1718

18-
// if authHeader is "", downloadToSink assumes no auth is expected
1919
static void downloadToSink(
2020
const std::string & url,
21-
const std::string & authHeader,
21+
const std::optional<std::string> & authHeader,
2222
// FIXME: passing a StringSink is superfluous, we may as well
2323
// return a string. Or use an abstract Sink for streaming.
2424
StringSink & sink,
@@ -27,8 +27,8 @@ static void downloadToSink(
2727
{
2828
FileTransferRequest request(url);
2929
Headers headers;
30-
if (!authHeader.empty())
31-
headers.push_back({"Authorization", authHeader});
30+
if (authHeader.has_value())
31+
headers.push_back({"Authorization", *authHeader});
3232
request.headers = headers;
3333
getFileTransfer()->download(std::move(request), sink);
3434

@@ -42,30 +42,53 @@ static void downloadToSink(
4242
"hash mismatch while fetching %s: expected sha256:%s but got sha256:%s", url, sha256Expected, sha256Actual);
4343
}
4444

45-
static std::string getLfsApiToken(const ParsedURL & url)
45+
namespace {
46+
47+
struct LfsApiInfo
48+
{
49+
std::string endpoint;
50+
std::optional<std::string> authHeader;
51+
};
52+
53+
} // namespace
54+
55+
static LfsApiInfo getLfsApi(const ParsedURL & url)
4656
{
4757
assert(url.authority.has_value());
58+
if (url.scheme == "ssh") {
59+
auto args = getNixSshOpts();
4860

49-
// FIXME: Not entirely correct.
50-
auto [status, output] = runProgram(
51-
RunOptions{
52-
.program = "ssh",
53-
.args = {url.authority->to_string(), "git-lfs-authenticate", url.path, "download"},
54-
});
61+
if (url.authority->port)
62+
args.push_back(fmt("-p%d", *url.authority->port));
5563

56-
if (output.empty())
57-
throw Error(
58-
"git-lfs-authenticate: no output (cmd: ssh %s git-lfs-authenticate %s download)",
59-
url.authority.value_or(ParsedURL::Authority{}).to_string(),
60-
url.path);
64+
std::ostringstream hostnameAndUser;
65+
if (url.authority->user)
66+
hostnameAndUser << *url.authority->user << "@";
67+
hostnameAndUser << url.authority->host;
68+
args.push_back(std::move(hostnameAndUser).str());
69+
70+
args.push_back("--");
71+
args.push_back("git-lfs-authenticate");
72+
args.push_back(url.path);
73+
args.push_back("download");
6174

62-
auto queryResp = nlohmann::json::parse(output);
63-
if (!queryResp.contains("header"))
64-
throw Error("no header in git-lfs-authenticate response");
65-
if (!queryResp["header"].contains("Authorization"))
66-
throw Error("no Authorization in git-lfs-authenticate response");
75+
auto [status, output] = runProgram({.program = "ssh", .args = args});
6776

68-
return queryResp["header"]["Authorization"].get<std::string>();
77+
if (output.empty())
78+
throw Error("git-lfs-authenticate: no output (cmd: 'ssh %s')", concatStringsSep(" ", args));
79+
80+
auto queryResp = nlohmann::json::parse(output);
81+
auto headerIt = queryResp.find("header");
82+
if (headerIt == queryResp.end())
83+
throw Error("no header in git-lfs-authenticate response");
84+
auto authIt = headerIt->find("Authorization");
85+
if (authIt == headerIt->end())
86+
throw Error("no Authorization in git-lfs-authenticate response");
87+
88+
return {queryResp.at("href").get<std::string>(), authIt->get<std::string>()};
89+
}
90+
91+
return {url.to_string() + "/info/lfs", std::nullopt};
6992
}
7093

7194
typedef std::unique_ptr<git_config, Deleter<git_config_free>> GitConfig;
@@ -181,13 +204,14 @@ static nlohmann::json pointerToPayload(const std::vector<Pointer> & items)
181204

182205
std::vector<nlohmann::json> Fetch::fetchUrls(const std::vector<Pointer> & pointers) const
183206
{
184-
ParsedURL httpUrl(url);
185-
httpUrl.scheme = url.scheme == "ssh" ? "https" : url.scheme;
186-
FileTransferRequest request(httpUrl.to_string() + "/info/lfs/objects/batch");
207+
auto api = lfs::getLfsApi(this->url);
208+
auto url = api.endpoint + "/objects/batch";
209+
const auto & authHeader = api.authHeader;
210+
FileTransferRequest request(url);
187211
request.post = true;
188212
Headers headers;
189-
if (this->url.scheme == "ssh")
190-
headers.push_back({"Authorization", lfs::getLfsApiToken(this->url)});
213+
if (authHeader.has_value())
214+
headers.push_back({"Authorization", *authHeader});
191215
headers.push_back({"Content-Type", "application/vnd.git-lfs+json"});
192216
headers.push_back({"Accept", "application/vnd.git-lfs+json"});
193217
request.headers = headers;
@@ -260,11 +284,16 @@ void Fetch::fetch(
260284
try {
261285
std::string sha256 = obj.at("oid"); // oid is also the sha256
262286
std::string ourl = obj.at("actions").at("download").at("href");
263-
std::string authHeader = "";
264-
if (obj.at("actions").at("download").contains("header")
265-
&& obj.at("actions").at("download").at("header").contains("Authorization")) {
266-
authHeader = obj["actions"]["download"]["header"]["Authorization"];
267-
}
287+
auto authHeader = [&]() -> std::optional<std::string> {
288+
const auto & download = obj.at("actions").at("download");
289+
auto headerIt = download.find("header");
290+
if (headerIt == download.end())
291+
return std::nullopt;
292+
auto authIt = headerIt->find("Authorization");
293+
if (authIt == headerIt->end())
294+
return std::nullopt;
295+
return *authIt;
296+
}();
268297
const uint64_t size = obj.at("size");
269298
sizeCallback(size);
270299
downloadToSink(ourl, authHeader, sink, sha256, size);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
namespace nix {
1010

11+
Strings getNixSshOpts();
12+
1113
class SSHMaster
1214
{
1315
private:

‎src/libstore/ssh.cc‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,18 @@ static void checkValidAuthority(const ParsedURL::Authority & authority)
5151
}
5252
}
5353

54+
Strings getNixSshOpts()
55+
{
56+
std::string sshOpts = getEnv("NIX_SSHOPTS").value_or("");
57+
58+
try {
59+
return shellSplitString(sshOpts);
60+
} catch (Error & e) {
61+
e.addTrace({}, "while splitting NIX_SSHOPTS '%s'", sshOpts);
62+
throw;
63+
}
64+
}
65+
5466
SSHMaster::SSHMaster(
5567
const ParsedURL::Authority & authority,
5668
std::string_view keyFile,
@@ -82,16 +94,8 @@ void SSHMaster::addCommonSSHOpts(Strings & args)
8294
{
8395
auto state(state_.lock());
8496

85-
std::string sshOpts = getEnv("NIX_SSHOPTS").value_or("");
86-
87-
try {
88-
std::list<std::string> opts = shellSplitString(sshOpts);
89-
for (auto & i : opts)
90-
args.push_back(i);
91-
} catch (Error & e) {
92-
e.addTrace({}, "while splitting NIX_SSHOPTS '%s'", sshOpts);
93-
throw;
94-
}
97+
auto sshArgs = getNixSshOpts();
98+
args.insert(args.end(), sshArgs.begin(), sshArgs.end());
9599

96100
if (!keyFile.empty())
97101
args.insert(args.end(), {"-i", keyFile});

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,5 +224,25 @@
224224
""")
225225
226226
client.succeed(f"cmp {repo.path}/beeg {fetched_self_lfs}/beeg >&2")
227+
228+
229+
with subtest("Ensure fetching with SSH generates the same output"):
230+
client.succeed(f"{repo.git} push origin-ssh main >&2")
231+
client.succeed("rm -rf ~/.cache/nix") # Avoid using the cached output of the http fetch
232+
233+
fetchGit_ssh_expr = f"""
234+
builtins.fetchGit {{
235+
url = "{repo.remote_ssh}";
236+
rev = "{lfs_file_rev}";
237+
ref = "main";
238+
lfs = true;
239+
}}
240+
"""
241+
fetched_ssh = client.succeed(f"""
242+
nix eval --debug --impure --raw --expr '({fetchGit_ssh_expr}).outPath'
243+
""")
244+
245+
assert fetched_ssh == fetched_lfs, \
246+
f"fetching with ssh (store path {fetched_ssh}) yielded a different result than using http (store path {fetched_lfs})"
227247
'';
228248
}

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,15 @@ in
4949
self.name = name
5050
self.path = "/tmp/repos/" + name
5151
self.remote = "http://gitea:3000/test/" + name
52-
self.remote_ssh = "ssh://gitea/root/" + name
52+
self.remote_ssh = "ssh://gitea:3001/test/" + name
5353
self.git = f"git -C {self.path}"
5454
self.private = private
5555
self.create()
5656
5757
def create(self):
58-
# create ssh remote repo
58+
# create remote repo
5959
gitea.succeed(f"""
60-
git init --bare -b main /root/{self.name}
61-
""")
62-
# create http remote repo
63-
gitea.succeed(f"""
64-
curl --fail -X POST http://{gitea_admin}:{gitea_admin_password}@gitea:3000/api/v1/user/repos \
60+
curl --fail -X POST http://{gitea_user}:{gitea_password}@gitea:3000/api/v1/user/repos \
6561
-H 'Accept: application/json' -H 'Content-Type: application/json' \
6662
-d {shlex.quote( f'{{"name":"{self.name}", "default_branch": "main", "private": {boolToJSON(self.private)}}}' )}
6763
""")
@@ -70,7 +66,7 @@ in
7066
mkdir -p {self.path} \
7167
&& git init -b main {self.path} \
7268
&& {self.git} remote add origin {self.remote} \
73-
&& {self.git} remote add origin-ssh root@gitea:{self.name}
69+
&& {self.git} remote add origin-ssh {self.remote_ssh}
7470
""")
7571
'';
7672
};

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

Lines changed: 16 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -35,28 +35,20 @@ in
3535
server = {
3636
DOMAIN = "gitea";
3737
HTTP_PORT = 3000;
38+
SSH_PORT = 3001;
39+
START_SSH_SERVER = true;
3840
};
3941
log.LEVEL = "Info";
4042
database.LOG_SQL = false;
4143
};
42-
services.openssh.enable = true;
43-
networking.firewall.allowedTCPPorts = [ 3000 ];
44+
networking.firewall.allowedTCPPorts = [
45+
3000
46+
3001
47+
];
4448
environment.systemPackages = [
4549
pkgs.git
4650
pkgs.gitea
4751
];
48-
49-
users.users.root.openssh.authorizedKeys.keys = [ clientPublicKey ];
50-
51-
# TODO: remove this after updating to nixos-23.11
52-
nixpkgs.pkgs = lib.mkForce (
53-
import nixpkgs {
54-
inherit system;
55-
config.permittedInsecurePackages = [
56-
"gitea-1.19.4"
57-
];
58-
}
59-
);
6052
};
6153
client =
6254
{ pkgs, ... }:
@@ -67,38 +59,33 @@ in
6759
];
6860
};
6961
};
70-
defaults =
71-
{ pkgs, ... }:
72-
{
73-
environment.systemPackages = [ pkgs.jq ];
74-
};
7562

7663
setupScript = ''
7764
import shlex
7865
7966
gitea.wait_for_unit("gitea.service")
8067
81-
gitea_admin = "test"
82-
gitea_admin_password = "test123test"
68+
gitea_user = "test"
69+
gitea_password = "test123test"
8370
8471
gitea.succeed(f"""
8572
gitea --version >&2
8673
su -l gitea -c 'GITEA_WORK_DIR=/var/lib/gitea gitea admin user create \
87-
--username {gitea_admin} --password {gitea_admin_password} --email test@client'
74+
--username {gitea_user} --password {gitea_password} --email test@client'
8875
""")
8976
9077
client.wait_for_unit("multi-user.target")
9178
gitea.wait_for_open_port(3000)
79+
gitea.wait_for_open_port(3001)
9280
93-
gitea_admin_token = gitea.succeed(f"""
94-
curl --fail -X POST http://{gitea_admin}:{gitea_admin_password}@gitea:3000/api/v1/users/test/tokens \
81+
gitea.succeed(f"""
82+
curl --fail -X POST http://{gitea_user}:{gitea_password}@gitea:3000/api/v1/user/keys \
9583
-H 'Accept: application/json' -H 'Content-Type: application/json' \
96-
-d {shlex.quote( '{"name":"token", "scopes":["all"]}' )} \
97-
| jq -r '.sha1'
98-
""").strip()
84+
-d {shlex.quote( '{"title":"key", "key":"${clientPublicKey}", "read_only": false}' )} >&2
85+
""")
9986
10087
client.succeed(f"""
101-
echo "http://{gitea_admin}:{gitea_admin_password}@gitea:3000" >~/.git-credentials-admin
88+
echo "http://{gitea_user}:{gitea_password}@gitea:3000" >~/.git-credentials-admin
10289
git config --global credential.helper 'store --file ~/.git-credentials-admin'
10390
git config --global user.email "test@client"
10491
git config --global user.name "Test User"
@@ -118,13 +105,7 @@ in
118105
echo "Host gitea" >>~/.ssh/config
119106
echo " StrictHostKeyChecking no" >>~/.ssh/config
120107
echo " UserKnownHostsFile /dev/null" >>~/.ssh/config
121-
echo " User root" >>~/.ssh/config
108+
echo " User gitea" >>~/.ssh/config
122109
""")
123-
124-
# ensure ssh from client to gitea works
125-
client.succeed("""
126-
ssh root@gitea true
127-
""")
128-
129110
'';
130111
}

0 commit comments

Comments
 (0)