discussion: git fetcher now needs either file:// or -s


Sam Liddicott
 

These recent security changes to git prevent git.py cloning of local
repos that contain symlinks in the .git directories (e.g. a repo-tool
manifest checkout)

* https://github.com/git/git/commit/36596fd2dfa473cf1069d23776e62cc156e7b5c6
clone: better handle symlinked files at .git/objects/
* https://github.com/git/git/commit/6f054f9fb3a501c35b55c65e547a244f14c38d56
builtin/clone.c: disallow --local clones with symlinks
* https://github.com/git/git/commit/bffc762f87ae8d18c6001bf0044a76004245754c
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS

To prevent some local clones from failing, the bitbake git fetcher
could use the -s like this:

- clone_cmd = "LANG=C %s clone --bare --mirror \"%s\" %s
--progress" % (ud.basecmd, repourl, ud.clonedir)
+ clone_cmd = "LANG=C %s clone -s --bare --mirror \"%s\" %s
--progress" % (ud.basecmd, repourl, ud.clonedir)

Instead of "-s", others may prefer to restore the url prefix file://
which also works,

- if repourl.startswith("file://"):
- repourl = repourl[7:]

I don't know which bests represents the original intention behind
removing file://, but "-s" gives a quicker clone of what for me is a
symlinked source repo anyway, and for the lifetime of a build I'm not
expecting the local original source repo to vanish in a way that will
affect the build.

Sam


Theodore A. Roth
 

On Tue, Feb 28, 2023 at 6:36 AM Sam Liddicott <sam@...> wrote:

These recent security changes to git prevent git.py cloning of local
repos that contain symlinks in the .git directories (e.g. a repo-tool
manifest checkout)

* https://github.com/git/git/commit/36596fd2dfa473cf1069d23776e62cc156e7b5c6
clone: better handle symlinked files at .git/objects/
* https://github.com/git/git/commit/6f054f9fb3a501c35b55c65e547a244f14c38d56
builtin/clone.c: disallow --local clones with symlinks
* https://github.com/git/git/commit/bffc762f87ae8d18c6001bf0044a76004245754c
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS

To prevent some local clones from failing, the bitbake git fetcher
could use the -s like this:

- clone_cmd = "LANG=C %s clone --bare --mirror \"%s\" %s
--progress" % (ud.basecmd, repourl, ud.clonedir)
+ clone_cmd = "LANG=C %s clone -s --bare --mirror \"%s\" %s
--progress" % (ud.basecmd, repourl, ud.clonedir)

Instead of "-s", others may prefer to restore the url prefix file://
which also works,

- if repourl.startswith("file://"):
- repourl = repourl[7:]

I don't know which bests represents the original intention behind
removing file://, but "-s" gives a quicker clone of what for me is a
symlinked source repo anyway, and for the lifetime of a build I'm not
expecting the local original source repo to vanish in a way that will
affect the build.

Sam
My organisation is also experiencing this issue with recipes that
contain SRC_URI values
pointing to `file://` source that was checked out using the
google-repo tool (which happens
to create symlinks to `.git` living outside of the SRC_URI referenced
git repository).

Our work around for this, is to have each developer impacted by this
append `--no-local`
to the `clone_cmd` as such:

diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 5bb8393133..17c4b8ca18 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -373,6 +373,8 @@ class Git(FetchMethod):
clone_cmd = "LANG=C %s clone --bare --mirror %s %s
--progress" % (ud.basecmd, shlex.quote(repourl), ud.clonedir)
if ud.proto.lower() != 'file':
bb.fetch2.check_network_access(d, clone_cmd, ud.url)
+ else:
+ clone_cmd += " --no-local"
progresshandler = GitProgressHandler(d)
runfetchcmd(clone_cmd, d, log=progresshandler)

Digging into the git history for this functionality, it appears to
have been introduced way back in 2012 to speed up cloning from local
source repos (e.g. things like the kernel source). The work arounds
provided above, would probably have a negative impact on that
particular user case though.

Would really love to see a fix for this included in bitbake upstream
so we don't have to manually patch.

Ted Roth


Robert Yang
 

Hello,

I've sent a patch for it and added you to the CC list:

fetch/git: Fix local clone url to make it work with repo

// Robert

On 3/13/23 9:35 PM, Theodore A. Roth via lists.openembedded.org wrote:
On Tue, Feb 28, 2023 at 6:36 AM Sam Liddicott <sam@...> wrote:

These recent security changes to git prevent git.py cloning of local
repos that contain symlinks in the .git directories (e.g. a repo-tool
manifest checkout)

* https://github.com/git/git/commit/36596fd2dfa473cf1069d23776e62cc156e7b5c6
clone: better handle symlinked files at .git/objects/
* https://github.com/git/git/commit/6f054f9fb3a501c35b55c65e547a244f14c38d56
builtin/clone.c: disallow --local clones with symlinks
* https://github.com/git/git/commit/bffc762f87ae8d18c6001bf0044a76004245754c
dir-iterator: prevent top-level symlinks without FOLLOW_SYMLINKS

To prevent some local clones from failing, the bitbake git fetcher
could use the -s like this:

- clone_cmd = "LANG=C %s clone --bare --mirror \"%s\" %s
--progress" % (ud.basecmd, repourl, ud.clonedir)
+ clone_cmd = "LANG=C %s clone -s --bare --mirror \"%s\" %s
--progress" % (ud.basecmd, repourl, ud.clonedir)

Instead of "-s", others may prefer to restore the url prefix file://
which also works,

- if repourl.startswith("file://"):
- repourl = repourl[7:]

I don't know which bests represents the original intention behind
removing file://, but "-s" gives a quicker clone of what for me is a
symlinked source repo anyway, and for the lifetime of a build I'm not
expecting the local original source repo to vanish in a way that will
affect the build.

Sam
My organisation is also experiencing this issue with recipes that
contain SRC_URI values
pointing to `file://` source that was checked out using the
google-repo tool (which happens
to create symlinks to `.git` living outside of the SRC_URI referenced
git repository).
Our work around for this, is to have each developer impacted by this
append `--no-local`
to the `clone_cmd` as such:
diff --git a/bitbake/lib/bb/fetch2/git.py b/bitbake/lib/bb/fetch2/git.py
index 5bb8393133..17c4b8ca18 100644
--- a/bitbake/lib/bb/fetch2/git.py
+++ b/bitbake/lib/bb/fetch2/git.py
@@ -373,6 +373,8 @@ class Git(FetchMethod):
clone_cmd = "LANG=C %s clone --bare --mirror %s %s
--progress" % (ud.basecmd, shlex.quote(repourl), ud.clonedir)
if ud.proto.lower() != 'file':
bb.fetch2.check_network_access(d, clone_cmd, ud.url)
+ else:
+ clone_cmd += " --no-local"
progresshandler = GitProgressHandler(d)
runfetchcmd(clone_cmd, d, log=progresshandler)
Digging into the git history for this functionality, it appears to
have been introduced way back in 2012 to speed up cloning from local
source repos (e.g. things like the kernel source). The work arounds
provided above, would probably have a negative impact on that
particular user case though.
Would really love to see a fix for this included in bitbake upstream
so we don't have to manually patch.
Ted Roth