Redact git and http refs

Signed-off-by: CrazyMax <[email protected]>

solver/llbsolver/solver.go

@@ -24,6 +24,7 @@ import (
 	"github.com/moby/buildkit/util/compression"
 	"github.com/moby/buildkit/util/entitlements"
 	"github.com/moby/buildkit/util/progress"
+	"github.com/moby/buildkit/util/urlutil"
 	"github.com/moby/buildkit/worker"
 	digest "github.com/opencontainers/go-digest"
 	"github.com/pkg/errors"
@@ -341,15 +342,15 @@ func mergeBuildInfo(ctx context.Context, res solver.ResultProxy, dtic []byte) ([
 			if _, ok := mbis[sref]; !ok {
 				mbis[sref] = exptypes.BuildInfo{
 					Type: exptypes.BuildInfoTypeGit,
-					Ref:  sref,
+					Ref:  urlutil.RedactCredentials(sref),
 					Pin:  di,
 				}
 			}
 		case *source.HTTPIdentifier:
 			if _, ok := mbis[sid.URL]; !ok {
 				mbis[sid.URL] = exptypes.BuildInfo{
 					Type: exptypes.BuildInfoTypeHTTP,
-					Ref:  sid.URL,
+					Ref:  urlutil.RedactCredentials(sid.URL),
 					Pin:  di,
 				}
 			}

source/git/gitsource.go

@@ -17,8 +17,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/moby/buildkit/util/bklog"
-
 	"github.com/moby/buildkit/cache"
 	"github.com/moby/buildkit/cache/metadata"
 	"github.com/moby/buildkit/client"
@@ -29,7 +27,9 @@ import (
 	"github.com/moby/buildkit/snapshot"
 	"github.com/moby/buildkit/solver"
 	"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"
 	bolt "go.etcd.io/bbolt"
@@ -78,7 +78,7 @@ func (gs *gitSource) mountRemote(ctx context.Context, remote string, auth []stri
 
 	sis, err := gs.md.Search(remoteKey)
 	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
@@ -87,19 +87,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
 	}
@@ -358,7 +358,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")
@@ -463,13 +463,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() {
@@ -527,7 +527,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 {
@@ -540,7 +540,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))
@@ -573,7 +573,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 {
@@ -700,12 +700,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:[email protected]/path.git" will be changed to
+// "https://xxxxx:[email protected]/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:[email protected]/this:that",
+			want: "https://xxxxx:[email protected]/this:that",
+		},
+		{
+			name: "blank Password",
+			url:  "https://[email protected]/this:that",
+			want: "https://[email protected]/this:that",
+		},
+		{
+			name: "blank Username",
+			url:  "https://:[email protected]/this:that",
+			want: "https://:[email protected]/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 {
+		t := t
+		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)
+			}
+		})
+	}
+}