Merge pull request moby#3594 from jedevc/sbom-supplement-fixes

Fix bugs in SBOM layer comment generation

Author: tonistiigi

client/client_test.go

@@ -62,6 +62,7 @@ import (
 	digest "github.com/opencontainers/go-digest"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
+	spdx "github.com/spdx/tools-golang/spdx/v2_3"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/crypto/ssh/agent"
 	"golang.org/x/sync/errgroup"
@@ -192,6 +193,7 @@ func TestIntegration(t *testing.T) {
 		testAttestationBundle,
 		testSBOMScan,
 		testSBOMScanSingleRef,
+		testSBOMSupplements,
 		testMultipleCacheExports,
 		testMountStubsDirectory,
 		testMountStubsTimestamp,
@@ -8312,6 +8314,154 @@ EOF
 	require.Subset(t, attest.Predicate, map[string]interface{}{"name": "fallback"})
 }
 
+func testSBOMSupplements(t *testing.T, sb integration.Sandbox) {
+	integration.CheckFeatureCompat(t, sb, integration.FeatureDirectPush, integration.FeatureSBOM)
+	requiresLinux(t)
+	c, err := New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+
+	registry, err := sb.NewRegistry()
+	if errors.Is(err, integration.ErrRequirements) {
+		t.Skip(err.Error())
+	}
+
+	p := platforms.MustParse("linux/amd64")
+	pk := platforms.Format(p)
+
+	frontend := func(ctx context.Context, c gateway.Client) (*gateway.Result, error) {
+		res := gateway.NewResult()
+
+		// build image
+		st := llb.Scratch().File(
+			llb.Mkfile("/foo", 0600, []byte{}),
+		)
+		def, err := st.Marshal(ctx)
+		if err != nil {
+			return nil, err
+		}
+		r, err := c.Solve(ctx, gateway.SolveRequest{
+			Definition: def.ToPB(),
+		})
+		if err != nil {
+			return nil, err
+		}
+		ref, err := r.SingleRef()
+		if err != nil {
+			return nil, err
+		}
+		_, err = ref.ToState()
+		if err != nil {
+			return nil, err
+		}
+		res.AddRef(pk, ref)
+
+		expPlatforms := &exptypes.Platforms{
+			Platforms: []exptypes.Platform{{ID: pk, Platform: p}},
+		}
+		dt, err := json.Marshal(expPlatforms)
+		if err != nil {
+			return nil, err
+		}
+		res.AddMeta(exptypes.ExporterPlatformsKey, dt)
+
+		// build attestations
+		doc := spdx.Document{
+			SPDXIdentifier: "DOCUMENT",
+			Files: []*spdx.File{
+				{
+					// foo exists...
+					FileSPDXIdentifier: "SPDXRef-File-foo",
+					FileName:           "/foo",
+				},
+				{
+					// ...but bar doesn't
+					FileSPDXIdentifier: "SPDXRef-File-bar",
+					FileName:           "/bar",
+				},
+			},
+		}
+		docBytes, err := json.Marshal(doc)
+		if err != nil {
+			return nil, err
+		}
+		st = llb.Scratch().
+			File(llb.Mkfile("/result.spdx", 0600, docBytes))
+		def, err = st.Marshal(ctx)
+		if err != nil {
+			return nil, err
+		}
+		r, err = c.Solve(ctx, gateway.SolveRequest{
+			Definition: def.ToPB(),
+		})
+		if err != nil {
+			return nil, err
+		}
+		refAttest, err := r.SingleRef()
+		if err != nil {
+			return nil, err
+		}
+		_, err = ref.ToState()
+		if err != nil {
+			return nil, err
+		}
+
+		res.AddAttestation(pk, gateway.Attestation{
+			Kind: gatewaypb.AttestationKindInToto,
+			Ref:  refAttest,
+			Path: "/result.spdx",
+			InToto: result.InTotoAttestation{
+				PredicateType: intoto.PredicateSPDX,
+			},
+			Metadata: map[string][]byte{
+				result.AttestationSBOMCore: []byte("result"),
+			},
+		})
+
+		return res, nil
+	}
+
+	// test the default fallback scanner
+	target := registry + "/buildkit/testsbom:latest"
+	_, err = c.Build(sb.Context(), SolveOpt{
+		FrontendAttrs: map[string]string{
+			"attest:sbom": "",
+		},
+		Exports: []ExportEntry{
+			{
+				Type: ExporterImage,
+				Attrs: map[string]string{
+					"name": target,
+					"push": "true",
+				},
+			},
+		},
+	}, "", frontend, nil)
+	require.NoError(t, err)
+
+	desc, provider, err := contentutil.ProviderFromRef(target)
+	require.NoError(t, err)
+
+	imgs, err := testutil.ReadImages(sb.Context(), provider, desc)
+	require.NoError(t, err)
+	require.Equal(t, 2, len(imgs.Images))
+
+	att := imgs.Find("unknown/unknown")
+	attest := struct {
+		intoto.StatementHeader
+		Predicate spdx.Document
+	}{}
+	require.NoError(t, json.Unmarshal(att.LayersRaw[0], &attest))
+	require.Equal(t, "https://in-toto.io/Statement/v0.1", attest.Type)
+	require.Equal(t, intoto.PredicateSPDX, attest.PredicateType)
+
+	require.Equal(t, "DOCUMENT", string(attest.Predicate.SPDXIdentifier))
+	require.Len(t, attest.Predicate.Files, 2)
+	require.Equal(t, attest.Predicate.Files[0].FileName, "/foo")
+	require.Regexp(t, "^layerID: sha256:", attest.Predicate.Files[0].FileComment)
+	require.Equal(t, attest.Predicate.Files[1].FileName, "/bar")
+	require.Empty(t, attest.Predicate.Files[1].FileComment)
+}
+
 func testMultipleCacheExports(t *testing.T, sb integration.Sandbox) {
 	integration.CheckFeatureCompat(t, sb, integration.FeatureMultiCacheExport)
 	c, err := New(sb.Context(), sb.Address())

exporter/containerimage/attestations.go

@@ -5,6 +5,7 @@ import (
 	"context"
 	"fmt"
 	"io/fs"
+	"path/filepath"
 	"strings"
 
 	intoto "github.com/in-toto/in-toto-golang/in_toto"
@@ -30,6 +31,9 @@ var intotoPlatform ocispecs.Platform = ocispecs.Platform{
 
 // supplementSBOM modifies SPDX attestations to include the file layers
 func supplementSBOM(ctx context.Context, s session.Group, target cache.ImmutableRef, targetRemote *solver.Remote, att exporter.Attestation) (exporter.Attestation, error) {
+	if target == nil {
+		return att, nil
+	}
 	if att.Kind != gatewaypb.AttestationKindInToto {
 		return att, nil
 	}
@@ -40,7 +44,7 @@ func supplementSBOM(ctx context.Context, s session.Group, target cache.Immutable
 	if !ok {
 		return att, nil
 	}
-	if n, _, _ := strings.Cut(att.Path, "."); n != string(name) {
+	if n, _, _ := strings.Cut(filepath.Base(att.Path), "."); n != string(name) {
 		return att, nil
 	}
 
@@ -172,6 +176,8 @@ func newFileLayerFinder(target cache.ImmutableRef, remote *solver.Remote) (fileL
 //
 // find is not concurrency-safe.
 func (c *fileLayerFinder) find(ctx context.Context, s session.Group, filename string) (cache.ImmutableRef, *ocispecs.Descriptor, error) {
+	filename = filepath.Join("/", filename)
+
 	// return immediately if we've already found the layer containing filename
 	if cache, ok := c.cache[filename]; ok {
 		return cache.ref, &cache.desc, nil
@@ -188,7 +194,9 @@ func (c *fileLayerFinder) find(ctx context.Context, s session.Group, filename st
 
 		found := false
 		for _, f := range files {
-			if strings.HasPrefix(f, ".wh.") {
+			f = filepath.Join("/", f)
+
+			if strings.HasPrefix(filepath.Base(f), ".wh.") {
 				// skip whiteout files, we only care about file creations
 				continue
 			}