Refactor url redacting util

source/git/gitsource.go

@@ -28,6 +28,7 @@ import (
 	"github.com/moby/buildkit/source"
 	"github.com/moby/buildkit/util/bklog"
 	"github.com/moby/buildkit/util/progress/logs"
+	"github.com/moby/buildkit/util/urlutil"
 	"github.com/moby/locker"
 	"github.com/pkg/errors"
 	"google.golang.org/grpc/codes"
@@ -70,7 +71,7 @@ func (gs *gitSource) ID() string {
 func (gs *gitSource) mountRemote(ctx context.Context, remote string, auth []string, g session.Group) (target string, release func(), retErr error) {
 	sis, err := searchGitRemote(ctx, gs.cache, remote)
 	if err != nil {
-		return "", nil, errors.Wrapf(err, "failed to search metadata for %s", redactCredentials(remote))
+		return "", nil, errors.Wrapf(err, "failed to search metadata for %s", urlutil.RedactCredentials(remote))
 	}
 
 	var remoteRef cache.MutableRef
@@ -79,19 +80,19 @@ func (gs *gitSource) mountRemote(ctx context.Context, remote string, auth []stri
 		if err != nil {
 			if errors.Is(err, cache.ErrLocked) {
 				// should never really happen as no other function should access this metadata, but lets be graceful
-				bklog.G(ctx).Warnf("mutable ref for %s  %s was locked: %v", redactCredentials(remote), si.ID(), err)
+				bklog.G(ctx).Warnf("mutable ref for %s  %s was locked: %v", urlutil.RedactCredentials(remote), si.ID(), err)
 				continue
 			}
-			return "", nil, errors.Wrapf(err, "failed to get mutable ref for %s", redactCredentials(remote))
+			return "", nil, errors.Wrapf(err, "failed to get mutable ref for %s", urlutil.RedactCredentials(remote))
 		}
 		break
 	}
 
 	initializeRepo := false
 	if remoteRef == nil {
-		remoteRef, err = gs.cache.New(ctx, nil, g, cache.CachePolicyRetain, cache.WithDescription(fmt.Sprintf("shared git repo for %s", redactCredentials(remote))))
+		remoteRef, err = gs.cache.New(ctx, nil, g, cache.CachePolicyRetain, cache.WithDescription(fmt.Sprintf("shared git repo for %s", urlutil.RedactCredentials(remote))))
 		if err != nil {
-			return "", nil, errors.Wrapf(err, "failed to create new mutable for %s", redactCredentials(remote))
+			return "", nil, errors.Wrapf(err, "failed to create new mutable for %s", urlutil.RedactCredentials(remote))
 		}
 		initializeRepo = true
 	}
@@ -342,7 +343,7 @@ func (gs *gitSourceHandler) CacheKey(ctx context.Context, g session.Group, index
 
 	buf, err := gitWithinDir(ctx, gitDir, "", sock, knownHosts, gs.auth, "ls-remote", "origin", ref)
 	if err != nil {
-		return "", nil, false, errors.Wrapf(err, "failed to fetch remote %s", redactCredentials(remote))
+		return "", nil, false, errors.Wrapf(err, "failed to fetch remote %s", urlutil.RedactCredentials(remote))
 	}
 	out := buf.String()
 	idx := strings.Index(out, "\t")
@@ -447,13 +448,13 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
 			// TODO: is there a better way to do this?
 		}
 		if _, err := gitWithinDir(ctx, gitDir, "", sock, knownHosts, gs.auth, args...); err != nil {
-			return nil, errors.Wrapf(err, "failed to fetch remote %s", redactCredentials(gs.src.Remote))
+			return nil, errors.Wrapf(err, "failed to fetch remote %s", urlutil.RedactCredentials(gs.src.Remote))
 		}
 	}
 
 	checkoutRef, err := gs.cache.New(ctx, nil, g, cache.WithRecordType(client.UsageRecordTypeGitCheckout), cache.WithDescription(fmt.Sprintf("git snapshot for %s#%s", gs.src.Remote, ref)))
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to create new mutable for %s", redactCredentials(gs.src.Remote))
+		return nil, errors.Wrapf(err, "failed to create new mutable for %s", urlutil.RedactCredentials(gs.src.Remote))
 	}
 
 	defer func() {
@@ -511,7 +512,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
 		}
 		_, err = gitWithinDir(ctx, checkoutDirGit, checkoutDir, sock, knownHosts, nil, "checkout", "FETCH_HEAD")
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to checkout remote %s", redactCredentials(gs.src.Remote))
+			return nil, errors.Wrapf(err, "failed to checkout remote %s", urlutil.RedactCredentials(gs.src.Remote))
 		}
 		gitDir = checkoutDirGit
 	} else {
@@ -524,7 +525,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
 		}
 		_, err = gitWithinDir(ctx, gitDir, cd, sock, knownHosts, nil, "checkout", ref, "--", ".")
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to checkout remote %s", redactCredentials(gs.src.Remote))
+			return nil, errors.Wrapf(err, "failed to checkout remote %s", urlutil.RedactCredentials(gs.src.Remote))
 		}
 		if subdir != "." {
 			d, err := os.Open(filepath.Join(cd, subdir))
@@ -557,7 +558,7 @@ func (gs *gitSourceHandler) Snapshot(ctx context.Context, g session.Group) (out
 
 	_, err = gitWithinDir(ctx, gitDir, checkoutDir, sock, knownHosts, gs.auth, "submodule", "update", "--init", "--recursive", "--depth=1")
 	if err != nil {
-		return nil, errors.Wrapf(err, "failed to update submodules for %s", redactCredentials(gs.src.Remote))
+		return nil, errors.Wrapf(err, "failed to update submodules for %s", urlutil.RedactCredentials(gs.src.Remote))
 	}
 
 	if idmap := mount.IdentityMapping(); idmap != nil {
@@ -675,12 +676,12 @@ func tokenScope(remote string) string {
 func getDefaultBranch(ctx context.Context, gitDir, workDir, sshAuthSock, knownHosts string, auth []string, remoteURL string) (string, error) {
 	buf, err := gitWithinDir(ctx, gitDir, workDir, sshAuthSock, knownHosts, auth, "ls-remote", "--symref", remoteURL, "HEAD")
 	if err != nil {
-		return "", errors.Wrapf(err, "error fetching default branch for repository %s", redactCredentials(remoteURL))
+		return "", errors.Wrapf(err, "error fetching default branch for repository %s", urlutil.RedactCredentials(remoteURL))
 	}
 
 	ss := defaultBranch.FindAllStringSubmatch(buf.String(), -1)
 	if len(ss) == 0 || len(ss[0]) != 2 {
-		return "", errors.Errorf("could not find default branch for repository: %s", redactCredentials(remoteURL))
+		return "", errors.Errorf("could not find default branch for repository: %s", urlutil.RedactCredentials(remoteURL))
 	}
 	return ss[0][1], nil
 }

util/urlutil/redact.go

@@ -0,0 +1,33 @@
+package urlutil
+
+import (
+	"net/url"
+)
+
+const mask = "xxxxx"
+
+// RedactCredentials takes a URL and redacts username and password from it.
+// e.g. "https://user:password@host.tld/path.git" will be changed to
+// "https://xxxxx:xxxxx@host.tld/path.git".
+func RedactCredentials(s string) string {
+	ru, err := url.Parse(s)
+	if err != nil {
+		return s // string is not a URL, just return it
+	}
+	var (
+		hasUsername bool
+		hasPassword bool
+	)
+	if ru.User != nil {
+		hasUsername = len(ru.User.Username()) > 0
+		_, hasPassword = ru.User.Password()
+	}
+	if hasUsername && hasPassword {
+		ru.User = url.UserPassword(mask, mask)
+	} else if hasUsername {
+		ru.User = url.User(mask)
+	} else if hasPassword {
+		ru.User = url.UserPassword(ru.User.Username(), mask)
+	}
+	return ru.String()
+}

util/urlutil/redact_test.go

@@ -0,0 +1,45 @@
+package urlutil
+
+import "testing"
+
+func TestRedactCredentials(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://xxxxx:xxxxx@host.tld/this:that",
+		},
+		{
+			name: "blank Password",
+			url:  "https://user@host.tld/this:that",
+			want: "https://xxxxx@host.tld/this:that",
+		},
+		{
+			name: "blank Username",
+			url:  "https://:password@host.tld/this:that",
+			want: "https://:xxxxx@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 := RedactCredentials(tt.url), tt.want; g != w {
+				t.Fatalf("got: %q\nwant: %q", g, w)
+			}
+		})
+	}
+}