pb: add extra validation to protobuf types

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com> (cherry picked from commit 00060c60c26b07015133edacfa32f569ceefea2e) (cherry picked from commit c890068b0da9d746cfa0f2627e0ee5cc60f869d3)
moby · Jan 31, 2024 · 7718bd5 · 7718bd5
1 parent e1924dc
commit 7718bd5
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 12 deletions.
diff --git a/client/validation_test.go b/client/validation_test.go
@@ -17,6 +17,7 @@ import (
 
 func testValidateNullConfig(t *testing.T, sb integration.Sandbox) {
 	requiresLinux(t)
+	integration.CheckFeatureCompat(t, sb, integration.FeatureOCIExporter)
 
 	ctx := sb.Context()
 
@@ -55,6 +56,7 @@ func testValidateNullConfig(t *testing.T, sb integration.Sandbox) {
 
 func testValidateInvalidConfig(t *testing.T, sb integration.Sandbox) {
 	requiresLinux(t)
+	integration.CheckFeatureCompat(t, sb, integration.FeatureOCIExporter)
 
 	ctx := sb.Context()
 
@@ -96,11 +98,12 @@ func testValidateInvalidConfig(t *testing.T, sb integration.Sandbox) {
 		},
 	}, "", b, nil)
 	require.Error(t, err)
-	require.Contains(t, err.Error(), "invalid image config for export: missing os")
+	require.Contains(t, err.Error(), "invalid image config: os and architecture must be specified together")
 }
 
 func testValidatePlatformsEmpty(t *testing.T, sb integration.Sandbox) {
 	requiresLinux(t)
+	integration.CheckFeatureCompat(t, sb, integration.FeatureOCIExporter)
 
 	ctx := sb.Context()
 
@@ -139,6 +142,7 @@ func testValidatePlatformsEmpty(t *testing.T, sb integration.Sandbox) {
 
 func testValidatePlatformsInvalid(t *testing.T, sb integration.Sandbox) {
 	requiresLinux(t)
+	integration.CheckFeatureCompat(t, sb, integration.FeatureOCIExporter)
 
 	ctx := sb.Context()
 
@@ -271,7 +275,6 @@ func testValidateSourcePolicy(t *testing.T, sb integration.Sandbox) {
 
 	for _, tc := range tcases {
 		t.Run(tc.name, func(t *testing.T) {
-
 			var viaFrontend bool
 
 			b := func(ctx context.Context, c client.Client) (*client.Result, error) {
@@ -302,7 +305,6 @@ func testValidateSourcePolicy(t *testing.T, sb integration.Sandbox) {
 			_, err = c.Build(ctx, SolveOpt{}, "", b, nil)
 			require.Error(t, err)
 			require.Contains(t, err.Error(), tc.exp)
-
 		})
 	}
 }
diff --git a/control/control.go b/control/control.go
@@ -405,6 +405,9 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*
 
 	var cacheImports []frontend.CacheOptionsEntry
 	for _, im := range req.Cache.Imports {
+		if im == nil {
+			continue
+		}
 		cacheImports = append(cacheImports, frontend.CacheOptionsEntry{
 			Type:  im.Type,
 			Attrs: im.Attrs,

diff --git a/frontend/gateway/client/attestation.go b/frontend/gateway/client/attestation.go
@@ -30,8 +30,14 @@ func AttestationToPB[T any](a *result.Attestation[T]) (*pb.Attestation, error) {
 }
 
 func AttestationFromPB[T any](a *pb.Attestation) (*result.Attestation[T], error) {
+	if a == nil {
+		return nil, errors.Errorf("invalid nil attestation")
+	}
 	subjects := make([]result.InTotoSubject, len(a.InTotoSubjects))
 	for i, subject := range a.InTotoSubjects {
+		if subject == nil {
+			return nil, errors.Errorf("invalid nil attestation subject")
+		}
 		subjects[i] = result.InTotoSubject{
 			Kind:   subject.Kind,
 			Name:   subject.Name,

diff --git a/frontend/gateway/gateway.go b/frontend/gateway/gateway.go
@@ -646,12 +646,21 @@ func (lbf *llbBridgeForwarder) registerResultIDs(results ...solver.Result) (ids
 func (lbf *llbBridgeForwarder) Solve(ctx context.Context, req *pb.SolveRequest) (*pb.SolveResponse, error) {
 	var cacheImports []frontend.CacheOptionsEntry
 	for _, e := range req.CacheImports {
+		if e == nil {
+			return nil, errors.Errorf("invalid nil cache import")
+		}
 		cacheImports = append(cacheImports, frontend.CacheOptionsEntry{
 			Type:  e.Type,
 			Attrs: e.Attrs,
 		})
 	}
 
+	for _, p := range req.SourcePolicies {
+		if p == nil {
+			return nil, errors.Errorf("invalid nil source policy")
+		}
+	}
+
 	ctx = tracing.ContextWithSpanFromContext(ctx, lbf.callCtx)
 	res, err := lbf.llbBridge.Solve(ctx, frontend.SolveRequest{
 		Evaluate:       req.Evaluate,
@@ -1077,6 +1086,12 @@ func (lbf *llbBridgeForwarder) ReleaseContainer(ctx context.Context, in *pb.Rele
 }
 
 func (lbf *llbBridgeForwarder) Warn(ctx context.Context, in *pb.WarnRequest) (*pb.WarnResponse, error) {
+	// validate ranges are valid
+	for _, r := range in.Ranges {
+		if r == nil {
+			return nil, status.Errorf(codes.InvalidArgument, "invalid source range")
+		}
+	}
 	err := lbf.llbBridge.Warn(ctx, in.Digest, string(in.Short), frontend.WarnOpts{
 		Level:      int(in.Level),
 		SourceInfo: in.Info,

diff --git a/util/tracing/transform/attribute.go b/util/tracing/transform/attribute.go
@@ -13,6 +13,9 @@ func Attributes(attrs []*commonpb.KeyValue) []attribute.KeyValue {
 
 	out := make([]attribute.KeyValue, 0, len(attrs))
 	for _, a := range attrs {
+		if a == nil {
+			continue
+		}
 		kv := attribute.KeyValue{
 			Key:   attribute.Key(a.Key),
 			Value: toValue(a.Value),
@@ -42,37 +45,45 @@ func toValue(v *commonpb.AnyValue) attribute.Value {
 func boolArray(kv []*commonpb.AnyValue) attribute.Value {
 	arr := make([]bool, len(kv))
 	for i, v := range kv {
-		arr[i] = v.GetBoolValue()
+		if v != nil {
+			arr[i] = v.GetBoolValue()
+		}
 	}
 	return attribute.BoolSliceValue(arr)
 }
 
 func intArray(kv []*commonpb.AnyValue) attribute.Value {
 	arr := make([]int64, len(kv))
 	for i, v := range kv {
-		arr[i] = v.GetIntValue()
+		if v != nil {
+			arr[i] = v.GetIntValue()
+		}
 	}
 	return attribute.Int64SliceValue(arr)
 }
 
 func doubleArray(kv []*commonpb.AnyValue) attribute.Value {
 	arr := make([]float64, len(kv))
 	for i, v := range kv {
-		arr[i] = v.GetDoubleValue()
+		if v != nil {
+			arr[i] = v.GetDoubleValue()
+		}
 	}
 	return attribute.Float64SliceValue(arr)
 }
 
 func stringArray(kv []*commonpb.AnyValue) attribute.Value {
 	arr := make([]string, len(kv))
 	for i, v := range kv {
-		arr[i] = v.GetStringValue()
+		if v != nil {
+			arr[i] = v.GetStringValue()
+		}
 	}
 	return attribute.StringSliceValue(arr)
 }
 
 func arrayValues(kv []*commonpb.AnyValue) attribute.Value {
-	if len(kv) == 0 {
+	if len(kv) == 0 || kv[0] == nil {
 		return attribute.StringSliceValue([]string{})
 	}
 

diff --git a/util/tracing/transform/span.go b/util/tracing/transform/span.go
@@ -32,14 +32,20 @@ func Spans(sdl []*tracepb.ResourceSpans) []tracesdk.ReadOnlySpan {
 		}
 
 		for _, sdi := range sd.ScopeSpans {
-			sda := make([]tracesdk.ReadOnlySpan, len(sdi.Spans))
-			for i, s := range sdi.Spans {
-				sda[i] = &readOnlySpan{
+			if sdi == nil {
+				continue
+			}
+			sda := make([]tracesdk.ReadOnlySpan, 0, len(sdi.Spans))
+			for _, s := range sdi.Spans {
+				if s == nil {
+					continue
+				}
+				sda = append(sda, &readOnlySpan{
 					pb:        s,
 					is:        sdi.Scope,
 					resource:  sd.Resource,
 					schemaURL: sd.SchemaUrl,
-				}
+				})
 			}
 			out = append(out, sda...)
 		}
@@ -170,6 +176,9 @@ var _ tracesdk.ReadOnlySpan = &readOnlySpan{}
 
 // status transform a OTLP span status into span code.
 func statusCode(st *tracepb.Status) codes.Code {
+	if st == nil {
+		return codes.Unset
+	}
 	switch st.Code {
 	case tracepb.Status_STATUS_CODE_ERROR:
 		return codes.Error
@@ -186,6 +195,9 @@ func links(links []*tracepb.Span_Link) []tracesdk.Link {
 
 	sl := make([]tracesdk.Link, 0, len(links))
 	for _, otLink := range links {
+		if otLink == nil {
+			continue
+		}
 		// This redefinition is necessary to prevent otLink.*ID[:] copies
 		// being reused -- in short we need a new otLink per iteration.
 		otLink := otLink
@@ -226,6 +238,9 @@ func spanEvents(es []*tracepb.Span_Event) []tracesdk.Event {
 		if messageEvents >= maxMessageEventsPerSpan {
 			break
 		}
+		if e == nil {
+			continue
+		}
 		messageEvents++
 		events = append(events,
 			tracesdk.Event{