Merge pull request #1664 from crazy-max/v0.10_backport_stripcreds

tonistiigi · web-flow · commit c513d34049e4 · 2023-03-06T16:25:59.000Z
[v0.10 backport] build: strip credentials from remote url on collecting Git provenance info
diff --git a/util/gitutil/gitutil.go b/util/gitutil/gitutil.go
@@ -3,6 +3,7 @@ package gitutil
 import (
 	"bytes"
 	"context"
+	"net/url"
 	"os"
 	"os/exec"
 	"strings"
@@ -70,11 +71,11 @@ func (c *Git) RootDir() (string, error) {
 func (c *Git) RemoteURL() (string, error) {
 	// Try to get the remote URL from the origin remote first
 	if ru, err := c.clean(c.run("remote", "get-url", "origin")); err == nil && ru != "" {
-		return ru, nil
+		return stripCredentials(ru), nil
 	}
 	// If that fails, try to get the remote URL from the upstream remote
 	if ru, err := c.clean(c.run("remote", "get-url", "upstream")); err == nil && ru != "" {
-		return ru, nil
+		return stripCredentials(ru), nil
 	}
 	return "", errors.New("no remote URL found for either origin or upstream")
 }
@@ -147,3 +148,16 @@ func IsUnknownRevision(err error) bool {
 	errMsg := strings.ToLower(err.Error())
 	return strings.Contains(errMsg, "unknown revision or path not in the working tree") || strings.Contains(errMsg, "bad revision")
 }
+
+// stripCredentials takes a URL and strips username and password from it.
+// e.g. "https://user:password@host.tld/path.git" will be changed to
+// "https://host.tld/path.git".
+// TODO: remove this function once fix from BuildKit is vendored here
+func stripCredentials(s string) string {
+	ru, err := url.Parse(s)
+	if err != nil {
+		return s // string is not a URL, just return it
+	}
+	ru.User = nil
+	return ru.String()
+}
diff --git a/util/gitutil/gitutil_test.go b/util/gitutil/gitutil_test.go
@@ -189,3 +189,45 @@ func TestGitRemoteURL(t *testing.T) {
 		})
 	}
 }
+
+func TestStripCredentials(t *testing.T) {
+	cases := []struct {
+		name string
+		url  string
+		want string
+	}{
+		{
+			name: "non-blank Password",
+			url:  "https://user:password@host.tld/this:that",
+			want: "https://host.tld/this:that",
+		},
+		{
+			name: "blank Password",
+			url:  "https://user@host.tld/this:that",
+			want: "https://host.tld/this:that",
+		},
+		{
+			name: "blank Username",
+			url:  "https://:password@host.tld/this:that",
+			want: "https://host.tld/this:that",
+		},
+		{
+			name: "blank Username, blank Password",
+			url:  "https://host.tld/this:that",
+			want: "https://host.tld/this:that",
+		},
+		{
+			name: "invalid URL",
+			url:  "1https://foo.com",
+			want: "1https://foo.com",
+		},
+	}
+	for _, tt := range cases {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			if g, w := stripCredentials(tt.url), tt.want; g != w {
+				t.Fatalf("got: %q\nwant: %q", g, w)
+			}
+		})
+	}
+}
