From e1924dc32da35bfb0bfdbb9d0fc7bca25e552330 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Sun, 17 Dec 2023 22:49:42 -0800 Subject: [PATCH] sourcepolicy: add validations for nil values Signed-off-by: Tonis Tiigi (cherry picked from commit 4e2569e796aae398648082689d70ca1d4f4f74a8) (cherry picked from commit caea271063973c6903be08c1ebbc7c103f67805f) --- client/client_test.go | 1 + client/validation_test.go | 102 +++++++++++++++++++++++++++++++++++++ solver/llbsolver/bridge.go | 8 +++ solver/llbsolver/solver.go | 23 +++++++++ sourcepolicy/matcher.go | 3 ++ 5 files changed, 137 insertions(+) diff --git a/client/client_test.go b/client/client_test.go index 13b554816645..4765d1dd67dd 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -210,6 +210,7 @@ func TestIntegration(t *testing.T) { testValidateInvalidConfig, testValidatePlatformsEmpty, testValidatePlatformsInvalid, + testValidateSourcePolicy, ) } diff --git a/client/validation_test.go b/client/validation_test.go index 5dd5df6cf2ce..d375b2b37a0c 100644 --- a/client/validation_test.go +++ b/client/validation_test.go @@ -9,6 +9,7 @@ import ( "github.com/moby/buildkit/client/llb" "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/frontend/gateway/client" + sppb "github.com/moby/buildkit/sourcepolicy/pb" "github.com/moby/buildkit/util/testutil/integration" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/stretchr/testify/require" @@ -204,3 +205,104 @@ func testValidatePlatformsInvalid(t *testing.T, sb integration.Sandbox) { }) } } + +func testValidateSourcePolicy(t *testing.T, sb integration.Sandbox) { + requiresLinux(t) + + ctx := sb.Context() + + c, err := New(ctx, sb.Address()) + require.NoError(t, err) + defer c.Close() + + tcases := []struct { + name string + value *sppb.Policy + exp string + }{ + // this condition fails on marshaling atm + // { + // name: "nilrule", + // value: &sppb.Policy{ + // Rules: []*sppb.Rule{nil}, + // }, + // exp: "", + // }, + { + name: "nilselector", + value: &sppb.Policy{ + Rules: []*sppb.Rule{ + { + Action: sppb.PolicyAction_CONVERT, + }, + }, + }, + exp: "invalid nil selector in policy", + }, + { + name: "emptyaction", + value: &sppb.Policy{ + Rules: []*sppb.Rule{ + { + Action: sppb.PolicyAction(9000), + Selector: &sppb.Selector{ + Identifier: "docker-image://docker.io/library/alpine:latest", + }, + }, + }, + }, + exp: "unknown type", + }, + { + name: "nilupdates", + value: &sppb.Policy{ + Rules: []*sppb.Rule{ + { + Action: sppb.PolicyAction_CONVERT, + Selector: &sppb.Selector{ + Identifier: "docker-image://docker.io/library/alpine:latest", + }, + }, + }, + }, + exp: "missing destination for convert rule", + }, + } + + 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) { + def, err := llb.Image("alpine").Marshal(ctx) + if err != nil { + return nil, err + } + + req := client.SolveRequest{ + Evaluate: true, + Definition: def.ToPB(), + } + if viaFrontend { + req.SourcePolicies = []*sppb.Policy{ + tc.value, + } + } + return c.Solve(ctx, req) + } + + _, err = c.Build(ctx, SolveOpt{ + SourcePolicy: tc.value, + }, "", b, nil) + require.Error(t, err) + require.Contains(t, err.Error(), tc.exp) + + viaFrontend = true + _, err = c.Build(ctx, SolveOpt{}, "", b, nil) + require.Error(t, err) + require.Contains(t, err.Error(), tc.exp) + + }) + } +} diff --git a/solver/llbsolver/bridge.go b/solver/llbsolver/bridge.go index 27dc133620f3..db851a8f1f1a 100644 --- a/solver/llbsolver/bridge.go +++ b/solver/llbsolver/bridge.go @@ -79,6 +79,14 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp } var polEngine SourcePolicyEvaluator if srcPol != nil || len(pol) > 0 { + for _, p := range pol { + if p == nil { + return nil, errors.Errorf("invalid nil policy") + } + if err := validateSourcePolicy(*p); err != nil { + return nil, err + } + } if srcPol != nil { pol = append([]*spb.Policy{srcPol}, pol...) } diff --git a/solver/llbsolver/solver.go b/solver/llbsolver/solver.go index 9295e08c6372..e342dad8294c 100644 --- a/solver/llbsolver/solver.go +++ b/solver/llbsolver/solver.go @@ -447,6 +447,9 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro j.SetValue(keyEntitlements, set) if srcPol != nil { + if err := validateSourcePolicy(*srcPol); err != nil { + return nil, err + } j.SetValue(keySourcePolicy, *srcPol) } @@ -595,6 +598,23 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro }, nil } +func validateSourcePolicy(pol spb.Policy) error { + for _, r := range pol.Rules { + if r == nil { + return errors.New("invalid nil rule in policy") + } + if r.Selector == nil { + return errors.New("invalid nil selector in policy") + } + for _, c := range r.Selector.Constraints { + if c == nil { + return errors.New("invalid nil constraint in policy") + } + } + } + return nil +} + func runCacheExporters(ctx context.Context, exporters []RemoteCacheExporter, j *solver.Job, cached *result.Result[solver.CachedResult], inp *result.Result[cache.ImmutableRef]) (map[string]string, error) { eg, ctx := errgroup.WithContext(ctx) g := session.NewGroup(j.SessionID) @@ -991,6 +1011,9 @@ func loadSourcePolicy(b solver.Builder) (*spb.Policy, error) { return errors.Errorf("invalid source policy %T", v) } for _, f := range x.Rules { + if f == nil { + return errors.Errorf("invalid nil policy rule") + } r := *f srcPol.Rules = append(srcPol.Rules, &r) } diff --git a/sourcepolicy/matcher.go b/sourcepolicy/matcher.go index 79ab4032a5ae..2abe1039071f 100644 --- a/sourcepolicy/matcher.go +++ b/sourcepolicy/matcher.go @@ -10,6 +10,9 @@ import ( func match(ctx context.Context, src *selectorCache, ref string, attrs map[string]string) (bool, error) { for _, c := range src.Constraints { + if c == nil { + return false, errors.Errorf("invalid nil constraint for %v", src) + } switch c.Condition { case spb.AttrMatch_EQUAL: if attrs[c.Key] != c.Value {