fix(install): use ssh keys for private git repos

[Eckhardt-D]

What does this PR do?

Fixes #11993
Fixes #10641
Fixes #6927
Fixes #2464
Fixes #4068

Do not convert SCPLike and ssh:// urls to https URLs so that the system can use ssh keys when the resolution prefix is ssh:// - check if the host is in known hosts after git@. NB, not 100% sure if the skipping of verifyResolutions for .git tag is the way to go, might need some help there.

• Documentation or TypeScript types (it's okay to leave the rest blank in this case)
• Code changes

How did you verify your code works?

I ran bun install <private-repo-url> on a few private repo packages I have access to on different platforms.

and also had a package.json test as follows:

{
  "name": "test-git-install-bun",
  "module": "index.ts",
  "type": "module",
  "dependencies": {
    "gw-client": "ssh://git@gitlab.com/<org>/gateway-client.git",
    "gw-client-2": "git@gitlab.com:<org>/gateway-client.git",
    "gw-client-3": "git+ssh://git@gitlab.com/<org>/gateway-client.git"
  }
}

• I included a test for the new code, or an existing test covers it

☝️ No, because it's dependent on ssh keys.

Caveat

This is the first time I ever wrote Zig so it may be a good idea to double check my work 🤣 especially what I did in src/install/lockfile.zig, I could not for the life of me figure out why verifyResolutions is failing so I silenced it for git repos basically, don't think that's the way to go.

[github-actions]

❌ @Eckhardt-D, your commit has failing tests :(

🪟💻 1 failing tests Windows x64 baseline

• test/cli/install/migration/complex-workspace.test.ts 21 failing

🪟💻 3 failing tests Windows x64

• test/cli/install/migration/complex-workspace.test.ts 21 failing
• test/cli/install/migration/out-of-sync.test.ts 1 failing
• test/cli/install/registry/bun-install-registry.test.ts 1 failing

View logs

[Eckhardt-D]

@dylan-conway This commit be70b4d introduces quite a bit of changes for caveats I found in the initial implementation:

1. Users expect e.g. gitlab:owner/repo to work whether it's private or public. The previous implementation didn't address that.

Again, the code is probably not the best (first time Zigging) - but it does what I am trying to convey:

1. Tries cloning with HTTP URL
2. Tries SSH if HTTP fails

The biggest thing is that previously the catch block of the tryHTTPS clone would not exit non zero, but just prompt. By setting the process env vars to make Git non-interactive it forces it to throw then SSH is tried.

One edge case this caused and that is addressed is that if an https URL is not a valid git repo, it would also not report the error - currently my fix is to read from stderr of the Git process when exit != 0. That seems flaky, but the stderr message has not changed in many years and can't think of another way to exit early on HTTP if the repo does not exist.

[dylan-conway]

Again, the code is probably not the best (first time Zigging) - but it does what I am trying to convey:

1. Tries cloning with HTTP URL
2. Tries SSH if HTTP fails

This seems like the correct approach, npm does the same thing https://github.com/npm/cli/blob/22731831e22011e32fa0ca12178e242c2ee2b33d/node_modules/pacote/lib/git.js#L82-L84

One edge case this caused and that is addressed is that if an https URL is not a valid git repo, it would also not report the error - currently my fix is to read from stderr of the Git process when exit != 0. That seems flaky, but the stderr message has not changed in many years and can't think of another way to exit early on HTTP if the repo does not exist.

There might be something we can find in npm's codebase for how they choose to fallback to ssh

[Eckhardt-D]

There might be something we can find in npm's codebase for how they choose to fallback to ssh

NPM Also Looks for a specific message in the stderr of the response and converts it to a different error, however - they use ls-remote first to check the repo. source1 source2

This adds yet another network call to the process 🐌 given that ultimately there will be an error in the ssh call after the https try, if the stderr output ever changes, the only side effect would be that Bun unnecessarily tries ssh after https, which I think is better than ALWAYS running ls-remote?

[Eckhardt-D]

🪟💻 4 failing tests Windows x64 baseline

Looking at the crash report, it looks like the exception is coming from SYSTEM32/ntdll.dll which hopefully is because of NtCreateFile not working because of file path lengths 🙏 could be fixed by latest update.

[dylan-conway]

After #11917 (comment) and #11917 (comment) are addressed this pr should be almost ready to go. There is one new CI failure in complex-workspace.test.ts on windows baseline, but I believe #11917 (comment) will fix it

[dylan-conway]

There is one new CI failure in complex-workspace.test.ts on windows baseline, but I believe #11917 (comment) will fix it

Actually it's not new, seeing it here as well #12022 (comment)

[dylan-conway]

Looks good, thank you!

[UmeshMohan-Dozee]

bun add git+ssh://github.com/lodash/lodash.git#4.17.21 works if lodash is a public repository. If lodash is a private repo, it fails with the following messages:

bun add v1.1.24 (85a32991)
error: "git fetch" for "git+ssh://github.com/lodash/lodash.git#4.17.21" failed

error: "git fetch" for "git+ssh://github.com/lodash/lodash.git#4.17.21" failed

error: InstallFailed cloning repository for git+ssh://github.com/lodash/lodash.git#4.17.21
error: git+ssh://github.com/lodash/lodash.git#4.17.21 failed to resolve