gemnasium safe directory git command overwrites existing entries
Summary
Now that Allow list build dir and gemnasium-dir in Git c... (gitlab-org/security-products/analyzers/gemnasium!1084 - merged) • Oscar Tovar • 18.5 has been merged, the analyzer executes /usr/bin/git -C /gemnasium-db config --global safe.directory /gemnasium-db before initiating a scan, however, this command overwrites any existing entries for the git safe.directory configuration.
For example:
-
Add a
safe.directoryentry$ docker run -it --rm registry.gitlab.com/security-products/gemnasium:6.1.11 ash / # git config --global --add safe.directory /var -
List
safe.directoryentries:/ # git config --global --list | grep -i safe safe.directory=/var -
Run the
safe.directorycommand from repo.go:/ # /usr/bin/git -C /gemnasium-db config --global safe.directory /gemnasium-db -
List
safe.directoryentries:/ # git config --global --list | grep -i safe safe.directory=/gemnasium-dbNotice that the existing
safe.directory=/varvalue has been overwritten.
Impact
The impact of this bug is minimal and only seems to prevent us from implementing new testing behaviour such as Can't run gemnasium integration tests on Mac OS X (#571810) • Adam Cohen • 18.7.
Steps to reproduce
We can also demonstrate this issue by causing git to throw an error when trying to overwrite multiple safe.directory values:
-
Create multiple
safe.directoryentries:$ docker run -it --rm registry.gitlab.com/security-products/gemnasium:6.1.11 ash / # git config --global --add safe.directory /var / # git config --global --add safe.directory /var -
Run the
safe.directorycommand from repo.go:/ # /usr/bin/git -C /gemnasium-db config --global safe.directory /gemnasium-db warning: safe.directory has multiple values error: cannot overwrite multiple values with a single value Use a regexp, --add or --replace-all to change safe.directory.Notice an error is thrown by
git
What is the current bug behavior?
gemnasium analyzer overwrites existing safe.directory values
What is the expected correct behavior?
gemnasium analyzer does not overwrite existing safe.directory values
Implementation plan
-
Use --addin the thesafe.directorycommand:diff --git a/advisory/repo.go b/advisory/repo.go index 97a62d1e..65d50479 100644 --- a/advisory/repo.go +++ b/advisory/repo.go @@ -109,7 +109,7 @@ func (r Repo) Update() error { // the owner of the gemnasium repository copy doesn't match the user // that's operating on it. // See issue https://gitlab.com/gitlab-org/gitlab/-/issues/551333 - argsList = append(argsList, []string{"config", "--global", "safe.directory", r.Path}) + argsList = append(argsList, []string{"config", "--global", "--add", "safe.directory", r.Path}) // add "git remote set-url" command if needed if remoteURL := r.RemoteURL; remoteURL != "" {` -
Add an integration test to prove the above change works