Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

git: switch to golang implementation #1048

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tonistiigi
Copy link
Member

This switches git source implementation to use gopkg.in/src-d/go-git instead of executing git binary.

Executing a binary is a possible security concern. We should at least run it inside container and drop privileges but that adds many more complications.

This implementation (re)enables SSH git repos that now use the same ssh forwarding as the RUN --mount=type=ssh does (meaning --ssh is required form client side to build). Adding this support while executing a binary would have been another security problem.

In the future, I'd also want to add support for local repos using this that would do the fetch through the session endpoint.

Currently I have found two issues:

  • go-git does not support http-dumb protocol. I'm not sure how popular these are anymore but it does cause one of the integration tests to fail atm. that uses it.
  • Support for shallow fetches seems to be limited. They are supported but there is no way to do a unshallow fetch after a shallow one. Eg. requesting a SHA after requesting a tag. I have disabled shallow fetches because of it for now. Will look into what it takes to add this support to library.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
doFetch = false
}
}

if doFetch {
// make sure no old lock files have leaked
os.RemoveAll(filepath.Join(gitDir, "shallow.lock"))
// os.RemoveAll(filepath.Join(gitDir, "shallow.lock"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add TODO

@tonistiigi
Copy link
Member Author

There is also a question of what to do with the host check on SSH. Currently, there is no validation. Github does have a page with their fingerprints https://help.github.com/en/articles/githubs-ssh-key-fingerprints so could at least validate that. But I'm concerned about what would happen if they add ecdsa or their keys leak. I don't want to break everyone in that case.

@AkihiroSuda
Copy link
Member

Can we have daemon flag to switch implementation?

@@ -66,7 +65,8 @@ require (
golang.org/x/time v0.0.0-20161028155119-f51c12702a4d
google.golang.org/genproto v0.0.0-20180817151627-c66870c02cf8
google.golang.org/grpc v1.20.1
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
gopkg.in/src-d/go-billy.v4 v4.2.1
gopkg.in/src-d/go-git.v4 v4.11.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this repository has been archived, and moved to https://github.com/go-git/go-git

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants