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

[v0.10 backport] build: strip credentials from remote url on collecting Git provenance info

.github/workflows/build.yml

@@ -21,8 +21,8 @@ on:
       - 'docs/**'
 
 env:
-  BUILDX_VERSION: "v0.10.0"
-  BUILDKIT_IMAGE: "moby/buildkit:v0.11.1"
+  BUILDX_VERSION: "latest"
+  BUILDKIT_IMAGE: "moby/buildkit:latest"
   REPO_SLUG: "docker/buildx-bin"
   DESTDIR: "./bin"
 

bake/compose.go

@@ -78,6 +78,7 @@ func ParseCompose(cfgs []compose.ConfigFile, envs map[string]string) (*Config, e
 			// compose does not support nil values for labels
 			labels := map[string]*string{}
 			for k, v := range s.Build.Labels {
+				v := v
 				labels[k] = &v
 			}
 

bake/hcl_test.go

@@ -670,6 +670,24 @@ func TestJSONFunctions(t *testing.T) {
 	require.Equal(t, ptrstr("pre-<FOO-abc>"), c.Targets[0].Args["v1"])
 }
 
+func TestJSONInvalidFunctions(t *testing.T) {
+	dt := []byte(`{
+	"target": {
+		"app": {
+			"args": {
+				"v1": "myfunc(\"foo\")"
+			}
+		}
+	}}`)
+
+	c, err := ParseFile(dt, "docker-bake.json")
+	require.NoError(t, err)
+
+	require.Equal(t, 1, len(c.Targets))
+	require.Equal(t, c.Targets[0].Name, "app")
+	require.Equal(t, ptrstr(`myfunc("foo")`), c.Targets[0].Args["v1"])
+}
+
 func TestHCLFunctionInAttr(t *testing.T) {
 	dt := []byte(`
 	function "brace" {

bake/hclparser/expr.go

@@ -14,15 +14,7 @@ func funcCalls(exp hcl.Expression) ([]string, hcl.Diagnostics) {
 	if !ok {
 		fns, err := jsonFuncCallsRecursive(exp)
 		if err != nil {
-			return nil, hcl.Diagnostics{
-				&hcl.Diagnostic{
-					Severity: hcl.DiagError,
-					Summary:  "Invalid expression",
-					Detail:   err.Error(),
-					Subject:  exp.Range().Ptr(),
-					Context:  exp.Range().Ptr(),
-				},
-			}
+			return nil, wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr())
 		}
 		return fns, nil
 	}

bake/hclparser/hclparser.go

@@ -62,7 +62,9 @@ type parser struct {
 	doneB     map[*hcl.Block]map[string]struct{}
 }
 
-func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.Diagnostics {
+var errUndefined = errors.New("undefined")
+
+func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}, allowMissing bool) hcl.Diagnostics {
 	fns, hcldiags := funcCalls(exp)
 	if hcldiags.HasErrors() {
 		return hcldiags
@@ -70,15 +72,10 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.D
 
 	for _, fn := range fns {
 		if err := p.resolveFunction(fn); err != nil {
-			return hcl.Diagnostics{
-				&hcl.Diagnostic{
-					Severity: hcl.DiagError,
-					Summary:  "Invalid expression",
-					Detail:   err.Error(),
-					Subject:  exp.Range().Ptr(),
-					Context:  exp.Range().Ptr(),
-				},
+			if allowMissing && errors.Is(err, errUndefined) {
+				continue
 			}
+			return wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr())
 		}
 	}
 
@@ -128,27 +125,17 @@ func (p *parser) loadDeps(exp hcl.Expression, exclude map[string]struct{}) hcl.D
 				}
 			}
 			if err := p.resolveBlock(blocks[0], target); err != nil {
-				return hcl.Diagnostics{
-					&hcl.Diagnostic{
-						Severity: hcl.DiagError,
-						Summary:  "Invalid expression",
-						Detail:   err.Error(),
-						Subject:  v.SourceRange().Ptr(),
-						Context:  v.SourceRange().Ptr(),
-					},
+				if allowMissing && errors.Is(err, errUndefined) {
+					continue
 				}
+				return wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr())
 			}
 		} else {
 			if err := p.resolveValue(v.RootName()); err != nil {
-				return hcl.Diagnostics{
-					&hcl.Diagnostic{
-						Severity: hcl.DiagError,
-						Summary:  "Invalid expression",
-						Detail:   err.Error(),
-						Subject:  v.SourceRange().Ptr(),
-						Context:  v.SourceRange().Ptr(),
-					},
+				if allowMissing && errors.Is(err, errUndefined) {
+					continue
 				}
+				return wrapErrorDiagnostic("Invalid expression", err, exp.Range().Ptr(), exp.Range().Ptr())
 			}
 		}
 	}
@@ -167,7 +154,7 @@ func (p *parser) resolveFunction(name string) error {
 		if _, ok := p.ectx.Functions[name]; ok {
 			return nil
 		}
-		return errors.Errorf("undefined function %s", name)
+		return errors.Wrapf(errUndefined, "function %q does not exit", name)
 	}
 	if _, ok := p.progressF[name]; ok {
 		return errors.Errorf("function cycle not allowed for %s", name)
@@ -217,7 +204,7 @@ func (p *parser) resolveFunction(name string) error {
 		return diags
 	}
 
-	if diags := p.loadDeps(f.Result.Expr, params); diags.HasErrors() {
+	if diags := p.loadDeps(f.Result.Expr, params, false); diags.HasErrors() {
 		return diags
 	}
 
@@ -255,7 +242,7 @@ func (p *parser) resolveValue(name string) (err error) {
 	if _, builtin := p.opt.Vars[name]; !ok && !builtin {
 		vr, ok := p.vars[name]
 		if !ok {
-			return errors.Errorf("undefined variable %q", name)
+			return errors.Wrapf(errUndefined, "variable %q does not exit", name)
 		}
 		def = vr.Default
 	}
@@ -270,7 +257,7 @@ func (p *parser) resolveValue(name string) (err error) {
 		return
 	}
 
-	if diags := p.loadDeps(def.Expr, nil); diags.HasErrors() {
+	if diags := p.loadDeps(def.Expr, nil, true); diags.HasErrors() {
 		return diags
 	}
 	vv, diags := def.Expr.Value(p.ectx)
@@ -314,14 +301,7 @@ func (p *parser) resolveValue(name string) (err error) {
 func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err error) {
 	name := block.Labels[0]
 	if err := p.opt.ValidateLabel(name); err != nil {
-		return hcl.Diagnostics{
-			&hcl.Diagnostic{
-				Severity: hcl.DiagError,
-				Summary:  "Invalid name",
-				Detail:   err.Error(),
-				Subject:  &block.LabelRanges[0],
-			},
-		}
+		return wrapErrorDiagnostic("Invalid name", err, &block.LabelRanges[0], &block.LabelRanges[0])
 	}
 
 	if _, ok := p.doneB[block]; !ok {
@@ -395,7 +375,7 @@ func (p *parser) resolveBlock(block *hcl.Block, target *hcl.BodySchema) (err err
 		return diag
 	}
 	for _, a := range content.Attributes {
-		diag := p.loadDeps(a.Expr, nil)
+		diag := p.loadDeps(a.Expr, nil, true)
 		if diag.HasErrors() {
 			return diag
 		}
@@ -573,15 +553,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics {
 				return diags
 			}
 			r := p.vars[k].Body.MissingItemRange()
-			return hcl.Diagnostics{
-				&hcl.Diagnostic{
-					Severity: hcl.DiagError,
-					Summary:  "Invalid value",
-					Detail:   err.Error(),
-					Subject:  &r,
-					Context:  &r,
-				},
-			}
+			return wrapErrorDiagnostic("Invalid value", err, &r, &r)
 		}
 	}
 
@@ -604,15 +576,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics {
 					}
 				}
 			}
-			return hcl.Diagnostics{
-				&hcl.Diagnostic{
-					Severity: hcl.DiagError,
-					Summary:  "Invalid function",
-					Detail:   err.Error(),
-					Subject:  subject,
-					Context:  context,
-				},
-			}
+			return wrapErrorDiagnostic("Invalid function", err, subject, context)
 		}
 	}
 
@@ -673,15 +637,7 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics {
 					continue
 				}
 			} else {
-				return hcl.Diagnostics{
-					&hcl.Diagnostic{
-						Severity: hcl.DiagError,
-						Summary:  "Invalid attribute",
-						Detail:   err.Error(),
-						Subject:  &b.LabelRanges[0],
-						Context:  &b.DefRange,
-					},
-				}
+				return wrapErrorDiagnostic("Invalid block", err, &b.LabelRanges[0], &b.DefRange)
 			}
 		}
 
@@ -726,21 +682,34 @@ func Parse(b hcl.Body, opt Opt, val interface{}) hcl.Diagnostics {
 			if diags, ok := err.(hcl.Diagnostics); ok {
 				return diags
 			}
-			return hcl.Diagnostics{
-				&hcl.Diagnostic{
-					Severity: hcl.DiagError,
-					Summary:  "Invalid attribute",
-					Detail:   err.Error(),
-					Subject:  &p.attrs[k].Range,
-					Context:  &p.attrs[k].Range,
-				},
-			}
+			return wrapErrorDiagnostic("Invalid attribute", err, &p.attrs[k].Range, &p.attrs[k].Range)
 		}
 	}
 
 	return nil
 }
 
+// wrapErrorDiagnostic wraps an error into a hcl.Diagnostics object.
+// If the error is already an hcl.Diagnostics object, it is returned as is.
+func wrapErrorDiagnostic(message string, err error, subject *hcl.Range, context *hcl.Range) hcl.Diagnostics {
+	switch err := err.(type) {
+	case *hcl.Diagnostic:
+		return hcl.Diagnostics{err}
+	case hcl.Diagnostics:
+		return err
+	default:
+		return hcl.Diagnostics{
+			&hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  message,
+				Detail:   err.Error(),
+				Subject:  subject,
+				Context:  context,
+			},
+		}
+	}
+}
+
 func setLabel(v reflect.Value, lbl string) int {
 	// cache field index?
 	numFields := v.Elem().Type().NumField()

bake/remote.go

@@ -34,9 +34,9 @@ func ReadRemoteFiles(ctx context.Context, nodes []builder.Node, url string, name
 	var files []File
 
 	var node *builder.Node
-	for _, n := range nodes {
+	for i, n := range nodes {
 		if n.Err == nil {
-			node = &n
+			node = &nodes[i]
 			continue
 		}
 	}

build/build.go

@@ -465,8 +465,19 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt Op
 			so.FrontendAttrs[k] = v
 		}
 	}
+
 	if _, ok := opt.Attests["attest:provenance"]; !ok && supportsAttestations {
-		so.FrontendAttrs["attest:provenance"] = "mode=min,inline-only=true"
+		const noAttestEnv = "BUILDX_NO_DEFAULT_ATTESTATIONS"
+		var noProv bool
+		if v, ok := os.LookupEnv(noAttestEnv); ok {
+			noProv, err = strconv.ParseBool(v)
+			if err != nil {
+				return nil, nil, errors.Wrap(err, "invalid "+noAttestEnv)
+			}
+		}
+		if !noProv {
+			so.FrontendAttrs["attest:provenance"] = "mode=min,inline-only=true"
+		}
 	}
 
 	switch len(opt.Exports) {

build/git.go

@@ -64,10 +64,16 @@ func getGitAttributes(ctx context.Context, contextPath string, dockerfilePath st
 		return res, nil
 	}
 
-	if sha, err := gitc.FullCommit(); err != nil {
+	if sha, err := gitc.FullCommit(); err != nil && !gitutil.IsUnknownRevision(err) {
 		return res, errors.Wrapf(err, "buildx: failed to get git commit")
 	} else if sha != "" {
-		if gitc.IsDirty() {
+		checkDirty := false
+		if v, ok := os.LookupEnv("BUILDX_GIT_CHECK_DIRTY"); ok {
+			if v, err := strconv.ParseBool(v); err == nil {
+				checkDirty = v
+			}
+		}
+		if checkDirty && gitc.IsDirty() {
 			sha += "-dirty"
 		}
 		if setGitLabels {

build/git_test.go

@@ -131,6 +131,7 @@ func TestGetGitAttributes(t *testing.T) {
 
 func TestGetGitAttributesDirty(t *testing.T) {
 	setupTest(t)
+	t.Setenv("BUILDX_GIT_CHECK_DIRTY", "true")
 
 	// make a change to test dirty flag
 	df := []byte("FROM alpine:edge\n")

docs/reference/buildx_build.md

@@ -88,6 +88,9 @@ BuildKit currently supports:
   Use `--attest=type=provenance` to generate provenance for an image at
   build-time. Alternatively, you can use the [`--provenance` shorthand](#provenance).
 
+  By default, a minimal provenance attestation will be created for the build
+  result, which will only be attached for images pushed to registries.
+
   For more information, see [here](https://docs.docker.com/build/attestations/slsa-provenance/).
 
 ### <a name="allow"></a> Allow extra privileged entitlement (--allow)
@@ -411,8 +414,13 @@ The `registry` exporter is a shortcut for `type=image,push=true`.
 
 Set the target platform for the build. All `FROM` commands inside the Dockerfile
 without their own `--platform` flag will pull base images for this platform and
-this value will also be the platform of the resulting image. The default value
-will be the current platform of the buildkit daemon.
+this value will also be the platform of the resulting image.
+
+The default value is the platform of the BuildKit daemon where the build runs.
+The value takes the form of `os/arch` or `os/arch/variant`. For example,
+`linux/amd64` or `linux/arm/v7`. Additionally, the `--platform` flag also supports
+a special `local` value, which tells BuildKit to use the platform of the BuildKit
+client that invokes the build.
 
 When using `docker-container` driver with `buildx`, this flag can accept multiple
 values as an input separated by a comma. With multiple values the result will be
@@ -477,8 +485,20 @@ $ docker buildx build --load --progress=plain .
 
 ### <a name="provenance"></a> Create provenance attestations (--provenance)
 
-Shorthand for [`--attest=type=provenance`](#attest). Enables provenance
-attestations for the build result.
+Shorthand for [`--attest=type=provenance`](#attest), used to configure
+provenance attestations for the build result. For example,
+`--provenance=mode=max` can be used as an abbreviation for
+`--attest=type=provenance,mode=max`.
+
+Additionally, `--provenance` can be used with boolean values to broadly enable
+or disable provenance attestations. For example, `--provenance=false` can be
+used to disable all provenance attestations, while `--provenance=true` can be
+used to enable all provenance attestations.
+
+By default, a minimal provenance attestation will be created for the build
+result, which will only be attached for images pushed to registries.
+
+For more information, see [here](https://docs.docker.com/build/attestations/slsa-provenance/).
 
 ### <a name="push"></a> Push the build result to a registry (--push)
 
@@ -487,8 +507,16 @@ build result to registry.
 
 ### <a name="sbom"></a> Create SBOM attestations (--sbom)
 
-Shorthand for [`--attest=type=sbom`](#attest). Enables SBOM attestations for
-the build result.
+Shorthand for [`--attest=type=sbom`](#attest), used to configure SBOM
+attestations for the build result. For example,
+`--sbom=generator=<user>/<generator-image>` can be used as an abbreviation for
+`--attest=type=sbom,generator=<user>/<generator-image>`.
+
+Additionally, `--sbom` can be used with boolean values to broadly enable or
+disable SBOM attestations. For example, `--sbom=false` can be used to disable
+all SBOM attestations.
+
+For more information, see [here](https://docs.docker.com/build/attestations/sbom/).
 
 ### <a name="secret"></a> Secret to expose to the build (--secret)
 

store/storeutil/storeutil.go

@@ -93,6 +93,7 @@ func GetNodeGroup(txn *store.Txn, dockerCli command.Cli, name string) (*store.No
 						Endpoint: name,
 					},
 				},
+				DockerContext: true,
 			}
 			if ng.LastActivity, err = txn.GetLastActivity(ng); err != nil {
 				return nil, err

util/gitutil/gitutil.go

@@ -3,6 +3,8 @@ package gitutil
 import (
 	"bytes"
 	"context"
+	"net/url"
+	"os"
 	"os/exec"
 	"strings"
 
@@ -69,21 +71,21 @@ 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")
 }
 
 func (c *Git) FullCommit() (string, error) {
-	return c.clean(c.run("show", "--format=%H", "HEAD", "--quiet"))
+	return c.clean(c.run("show", "--format=%H", "HEAD", "--quiet", "--"))
 }
 
 func (c *Git) ShortCommit() (string, error) {
-	return c.clean(c.run("show", "--format=%h", "HEAD", "--quiet"))
+	return c.clean(c.run("show", "--format=%h", "HEAD", "--quiet", "--"))
 }
 
 func (c *Git) Tag() (string, error) {
@@ -116,6 +118,9 @@ func (c *Git) run(args ...string) (string, error) {
 		cmd.Dir = c.wd
 	}
 
+	// Override the locale to ensure consistent output
+	cmd.Env = append(os.Environ(), "LC_ALL=C")
+
 	stdout := bytes.Buffer{}
 	stderr := bytes.Buffer{}
 	cmd.Stdout = &stdout
@@ -134,3 +139,25 @@ func (c *Git) clean(out string, err error) (string, error) {
 	}
 	return out, err
 }
+
+func IsUnknownRevision(err error) bool {
+	if err == nil {
+		return false
+	}
+	// https://github.com/git/git/blob/a6a323b31e2bcbac2518bddec71ea7ad558870eb/setup.c#L204
+	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()
+}

util/gitutil/gitutil_test.go

@@ -46,6 +46,32 @@ func TestGitShortCommit(t *testing.T) {
 	require.Equal(t, 7, len(out))
 }
 
+func TestGitFullCommitErr(t *testing.T) {
+	Mktmp(t)
+	c, err := New()
+	require.NoError(t, err)
+
+	GitInit(c, t)
+
+	_, err = c.FullCommit()
+	require.Error(t, err)
+	require.True(t, IsUnknownRevision(err))
+	require.False(t, IsAmbiguousArgument(err))
+}
+
+func TestGitShortCommitErr(t *testing.T) {
+	Mktmp(t)
+	c, err := New()
+	require.NoError(t, err)
+
+	GitInit(c, t)
+
+	_, err = c.ShortCommit()
+	require.Error(t, err)
+	require.True(t, IsUnknownRevision(err))
+	require.False(t, IsAmbiguousArgument(err))
+}
+
 func TestGitTagsPointsAt(t *testing.T) {
 	Mktmp(t)
 	c, err := New()
@@ -163,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)
+			}
+		})
+	}
+}

util/gitutil/testutil.go

@@ -2,6 +2,7 @@ package gitutil
 
 import (
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -73,3 +74,11 @@ func fakeGit(c *Git, args ...string) (string, error) {
 	allArgs = append(allArgs, args...)
 	return c.clean(c.run(allArgs...))
 }
+
+func IsAmbiguousArgument(err error) bool {
+	if err == nil {
+		return false
+	}
+	errMsg := strings.ToLower(err.Error())
+	return strings.Contains(errMsg, "use '--' to separate paths from revisions")
+}

util/imagetools/loader.go

@@ -21,8 +21,11 @@ import (
 	"golang.org/x/sync/errgroup"
 )
 
-const (
-	annotationReference = "vnd.docker.reference.digest"
+var (
+	annotationReferences = []string{
+		"com.docker.reference.digest",
+		"vnd.docker.reference.digest", // TODO: deprecate/remove after migration to new annotation
+	}
 )
 
 type contentCache interface {
@@ -167,8 +170,13 @@ func (l *loader) fetch(ctx context.Context, fetcher remotes.Fetcher, desc ocispe
 		}
 		r.mu.Unlock()
 
-		ref, ok := desc.Annotations[annotationReference]
-		if ok {
+		found := false
+		for _, annotationReference := range annotationReferences {
+			ref, ok := desc.Annotations[annotationReference]
+			if !ok {
+				continue
+			}
+
 			refdgst, err := digest.Parse(ref)
 			if err != nil {
 				return err
@@ -176,7 +184,10 @@ func (l *loader) fetch(ctx context.Context, fetcher remotes.Fetcher, desc ocispe
 			r.mu.Lock()
 			r.refs[refdgst] = append(r.refs[refdgst], desc.Digest)
 			r.mu.Unlock()
-		} else {
+			found = true
+			break
+		}
+		if !found {
 			p := desc.Platform
 			if p == nil {
 				p, err = l.readPlatformFromConfig(ctx, fetcher, mfst.Config)