manifest: introduce internal/manifest with private types and freeze public manifest.List

copy/copy.go

@@ -17,6 +17,7 @@ import (
 	"github.com/containers/image/v5/internal/image"
 	"github.com/containers/image/v5/internal/imagedestination"
 	"github.com/containers/image/v5/internal/imagesource"
+	internalManifest "github.com/containers/image/v5/internal/manifest"
 	"github.com/containers/image/v5/internal/pkg/platform"
 	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/internal/set"
@@ -317,7 +318,7 @@ func Image(ctx context.Context, policyContext *signature.PolicyContext, destRef,
 		if err != nil {
 			return nil, fmt.Errorf("reading manifest for %s: %w", transports.ImageName(srcRef), err)
 		}
-		manifestList, err := manifest.ListFromBlob(mfest, manifestType)
+		manifestList, err := internalManifest.ListFromBlob(mfest, manifestType)
 		if err != nil {
 			return nil, fmt.Errorf("parsing primary manifest as list for %s: %w", transports.ImageName(srcRef), err)
 		}
@@ -417,11 +418,11 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
 	if err != nil {
 		return nil, fmt.Errorf("reading manifest list: %w", err)
 	}
-	originalList, err := manifest.ListFromBlob(manifestList, manifestType)
+	originalList, err := internalManifest.ListFromBlob(manifestList, manifestType)
 	if err != nil {
 		return nil, fmt.Errorf("parsing manifest list %q: %w", string(manifestList), err)
 	}
-	updatedList := originalList.Clone()
+	updatedList := originalList.CloneInternal()
 
 	sigs, err := c.sourceSignatures(ctx, unparsedToplevel, options,
 		"Getting image list signatures",
@@ -525,7 +526,7 @@ func (c *copier) copyMultipleImages(ctx context.Context, policyContext *signatur
 	c.Printf("Writing manifest list to image destination\n")
 	var errs []string
 	for _, thisListType := range append([]string{selectedListType}, otherManifestMIMETypeCandidates...) {
-		attemptedList := updatedList
+		var attemptedList internalManifest.ListPublic = updatedList
 
 		logrus.Debugf("Trying to use manifest list type %s…", thisListType)
 

directory/directory_src.go

@@ -8,9 +8,9 @@ import (
 
 	"github.com/containers/image/v5/internal/imagesource/impl"
 	"github.com/containers/image/v5/internal/imagesource/stubs"
+	"github.com/containers/image/v5/internal/manifest"
 	"github.com/containers/image/v5/internal/private"
 	"github.com/containers/image/v5/internal/signature"
-	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/types"
 	"github.com/opencontainers/go-digest"
 )

internal/image/docker_list.go

@@ -4,7 +4,7 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/containers/image/v5/manifest"
+	"github.com/containers/image/v5/internal/manifest"
 	"github.com/containers/image/v5/types"
 )
 

internal/image/oci_index.go

@@ -4,7 +4,7 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/containers/image/v5/manifest"
+	"github.com/containers/image/v5/internal/manifest"
 	"github.com/containers/image/v5/types"
 )
 

internal/manifest/common.go

@@ -0,0 +1,72 @@
+package manifest
+
+import (
+	"encoding/json"
+	"fmt"
+)
+
+// AllowedManifestFields is a bit mask of “essential” manifest fields that ValidateUnambiguousManifestFormat
+// can expect to be present.
+type AllowedManifestFields int
+
+const (
+	AllowedFieldConfig AllowedManifestFields = 1 << iota
+	AllowedFieldFSLayers
+	AllowedFieldHistory
+	AllowedFieldLayers
+	AllowedFieldManifests
+	AllowedFieldFirstUnusedBit // Keep this at the end!
+)
+
+// ValidateUnambiguousManifestFormat rejects manifests (incl. multi-arch) that look like more than
+// one kind we currently recognize, i.e. if they contain any of the known “essential” format fields
+// other than the ones the caller specifically allows.
+// expectedMIMEType is used only for diagnostics.
+// NOTE: The caller should do the non-heuristic validations (e.g. check for any specified format
+// identification/version, or other “magic numbers”) before calling this, to cleanly reject unambiguous
+// data that just isn’t what was expected, as opposed to actually ambiguous data.
+func ValidateUnambiguousManifestFormat(manifest []byte, expectedMIMEType string,
+	allowed AllowedManifestFields) error {
+	if allowed >= AllowedFieldFirstUnusedBit {
+		return fmt.Errorf("internal error: invalid allowedManifestFields value %#v", allowed)
+	}
+	// Use a private type to decode, not just a map[string]any, because we want
+	// to also reject case-insensitive matches (which would be used by Go when really decoding
+	// the manifest).
+	// (It is expected that as manifest formats are added or extended over time, more fields will be added
+	// here.)
+	detectedFields := struct {
+		Config    any `json:"config"`
+		FSLayers  any `json:"fsLayers"`
+		History   any `json:"history"`
+		Layers    any `json:"layers"`
+		Manifests any `json:"manifests"`
+	}{}
+	if err := json.Unmarshal(manifest, &detectedFields); err != nil {
+		// The caller was supposed to already validate version numbers, so this should not happen;
+		// let’s not bother with making this error “nice”.
+		return err
+	}
+	unexpected := []string{}
+	// Sadly this isn’t easy to automate in Go, without reflection. So, copy&paste.
+	if detectedFields.Config != nil && (allowed&AllowedFieldConfig) == 0 {
+		unexpected = append(unexpected, "config")
+	}
+	if detectedFields.FSLayers != nil && (allowed&AllowedFieldFSLayers) == 0 {
+		unexpected = append(unexpected, "fsLayers")
+	}
+	if detectedFields.History != nil && (allowed&AllowedFieldHistory) == 0 {
+		unexpected = append(unexpected, "history")
+	}
+	if detectedFields.Layers != nil && (allowed&AllowedFieldLayers) == 0 {
+		unexpected = append(unexpected, "layers")
+	}
+	if detectedFields.Manifests != nil && (allowed&AllowedFieldManifests) == 0 {
+		unexpected = append(unexpected, "manifests")
+	}
+	if len(unexpected) != 0 {
+		return fmt.Errorf(`rejecting ambiguous manifest, unexpected fields %#v in supposedly %s`,
+			unexpected, expectedMIMEType)
+	}
+	return nil
+}

internal/manifest/common_test.go

@@ -0,0 +1,91 @@
+package manifest
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateUnambiguousManifestFormat(t *testing.T) {
+	const allAllowedFields = AllowedFieldFirstUnusedBit - 1
+	const mt = "text/plain" // Just some MIME type that shows up in error messages
+
+	type test struct {
+		manifest string
+		allowed  AllowedManifestFields
+	}
+
+	// Smoke tests: Success
+	for _, c := range []test{
+		{"{}", allAllowedFields},
+		{"{}", 0},
+	} {
+		err := ValidateUnambiguousManifestFormat([]byte(c.manifest), mt, c.allowed)
+		assert.NoError(t, err, c)
+	}
+	// Smoke tests: Failure
+	for _, c := range []test{
+		{"{}", AllowedFieldFirstUnusedBit}, // Invalid "allowed"
+		{"@", allAllowedFields},            // Invalid JSON
+	} {
+		err := ValidateUnambiguousManifestFormat([]byte(c.manifest), mt, c.allowed)
+		assert.Error(t, err, c)
+	}
+
+	fields := map[AllowedManifestFields]string{
+		AllowedFieldConfig:    "config",
+		AllowedFieldFSLayers:  "fsLayers",
+		AllowedFieldHistory:   "history",
+		AllowedFieldLayers:    "layers",
+		AllowedFieldManifests: "manifests",
+	}
+	// Ensure this test covers all defined AllowedManifestFields values
+	allFields := AllowedManifestFields(0)
+	for k := range fields {
+		allFields |= k
+	}
+	assert.Equal(t, allAllowedFields, allFields)
+
+	// Every single field is allowed by its bit, and rejected by any other bit
+	for bit, fieldName := range fields {
+		json := []byte(fmt.Sprintf(`{"%s":[]}`, fieldName))
+		err := ValidateUnambiguousManifestFormat(json, mt, bit)
+		assert.NoError(t, err, fieldName)
+		err = ValidateUnambiguousManifestFormat(json, mt, allAllowedFields^bit)
+		assert.Error(t, err, fieldName)
+	}
+}
+
+// Test that parser() rejects all of the provided manifest fixtures.
+// Intended to help test manifest parsers' detection of schema mismatches.
+func testManifestFixturesAreRejected(t *testing.T, parser func([]byte) error, fixtures []string) {
+	for _, fixture := range fixtures {
+		manifest, err := os.ReadFile(filepath.Join("testdata", fixture))
+		require.NoError(t, err, fixture)
+		err = parser(manifest)
+		assert.Error(t, err, fixture)
+	}
+}
+
+// Test that parser() rejects validManifest with an added top-level field with any of the provided field names.
+// Intended to help test callers of validateUnambiguousManifestFormat.
+func testValidManifestWithExtraFieldsIsRejected(t *testing.T, parser func([]byte) error,
+	validManifest []byte, fields []string) {
+	for _, field := range fields {
+		// end (the final '}') is not always at len(validManifest)-1 because the manifest can end with
+		// white space.
+		end := bytes.LastIndexByte(validManifest, '}')
+		require.NotEqual(t, end, -1)
+		updatedManifest := []byte(string(validManifest[:end]) +
+			fmt.Sprintf(`,"%s":[]}`, field))
+		err := parser(updatedManifest)
+		// Make sure it is the error from validateUnambiguousManifestFormat, not something that
+		// went wrong with creating updatedManifest.
+		assert.ErrorContains(t, err, "rejecting ambiguous manifest", field)
+	}
+}

internal/manifest/docker_schema2.go

@@ -0,0 +1,15 @@
+package manifest
+
+import (
+	"github.com/opencontainers/go-digest"
+)
+
+// Schema2Descriptor is a “descriptor” in docker/distribution schema 2.
+//
+// This is publicly visible as c/image/manifest.Schema2Descriptor.
+type Schema2Descriptor struct {
+	MediaType string        `json:"mediaType"`
+	Size      int64         `json:"size"`
+	Digest    digest.Digest `json:"digest"`
+	URLs      []string      `json:"urls,omitempty"`
+}